Filter hidden terms from getDocPage() and doc deduplication#720
Filter hidden terms from getDocPage() and doc deduplication#720
getDocPage() and doc deduplication#720Conversation
Tests that getDocPage() should filter hidden terms from custom DocFragments before producing the final DocPage. All term types that support the hidden field (option, argument, command, passthrough) are covered, as well as section fragments with mixed entries. #494 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
getDocPage() preserved hidden terms from custom DocFragments instead of filtering them. Built-in parsers already filter hidden entries in their own getDocFragments() implementations, but custom Parser implementations had no such guarantee. buildDocPage() now checks isDocHidden() on each entry's term before adding it to the section, ensuring the DocPage data structure itself is clean regardless of the fragment source. #494 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
#494 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tests that deduplicateDocEntries() and deduplicateDocFragments() prefer visible entries over doc-hidden ones when deduplicating by surface syntax. Covers direct entries, untitled sections, and titled sections. #494 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
deduplicateDocEntries() and deduplicateDocFragments() now prefer visible entries over doc-hidden ones when deduplicating by surface syntax key. Previously, the first occurrence always won regardless of hidden visibility, so or(hidden, visible) with the same option name would discard the visible copy. Also exports isDocEntryHidden() from doc.ts as a shared helper for checking whether a DocEntry's term is hidden from documentation. #494 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
buildDocPage() now skips section fragments with no visible entries after filtering, preventing empty sections from leaking into the DocPage. Titled sections are positioned at the first fragment that contains visible entries, not the first fragment overall. Also consolidates the isDocEntryHidden helper: the duplicate in parser.ts is removed in favor of the exported version from doc.ts. #494 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
#494 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
deduplicateDocFragments() eagerly reserved a titled section's position at its first fragment, even if that fragment contained only hidden entries. When dedup later revived a visible entry from a later fragment, the section appeared before sections that were actually visible first. Now a titled section's placeholder is only inserted when the first fragment with visible entries is encountered. Sections whose entries are all hidden are omitted entirely. #494 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The in-place replacement strategy let hidden entries influence the ordering of visible entries: hidden(--x), --y, --x produced --x, --y instead of --y, --x. deduplicateDocEntries() and deduplicateDocFragments() now skip hidden entries entirely before deduplicating, so hidden terms can never influence position or ordering. This removes the complex UntitledLocation/replacement logic in favor of simple skip-then-dedup-with-Set. #494 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the accuracy and clarity of generated documentation by refining how hidden terms are handled. The core change ensures that doc-hidden entries are filtered out early in the documentation building process and are ignored during deduplication. This prevents the rendering of blank rows, maintains correct ordering of visible entries, and ensures that titled sections are positioned logically based on their visible content, leading to a more robust and user-friendly documentation output. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #720 +/- ##
==========================================
- Coverage 93.75% 93.74% -0.02%
==========================================
Files 39 39
Lines 19499 19526 +27
Branches 5345 5356 +11
==========================================
+ Hits 18282 18304 +22
- Misses 1189 1195 +6
+ Partials 28 27 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the issue of hidden terms being preserved in documentation pages. The changes in buildDocPage and the deduplication functions are logical and well-implemented. The new tests are thorough and provide good coverage for the new behavior.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR filters doc-hidden entries and hidden-only sections during DocPage construction and deduplication. It adds isDocEntryHidden(), updates deduplicateDocEntries() and deduplicateDocFragments() to skip hidden entries/fragments and to place titled sections at the first fragment containing visible entries, and changes parser/buildDocPage logic to omit hidden entries and drop empty sections. Tests were added for deduplication and getDocPageSync behavior; CHANGES.md updated. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGES.md`:
- Around line 138-144: Update the changelog entry to document the titled-section
placement/omission behavior: note that titled sections are now anchored to the
first DocFragment that contains visible entries (after buildDocPage filtering)
and that titled sections whose all fragments are doc-hidden are omitted
entirely; reference the affected functions/types (getDocPage, buildDocPage,
deduplicateDocEntries, deduplicateDocFragments, DocFragments, DocPage) so
readers know this ties to the filtering/deduplication changes and clarify the
user-visible change in section anchoring and omission.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 01a58e78-c2e4-4911-a679-f570918e4929
📒 Files selected for processing (5)
CHANGES.mdpackages/core/src/doc.test.tspackages/core/src/doc.tspackages/core/src/parser.test.tspackages/core/src/parser.ts
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Add that titled sections are now positioned at their first fragment with visible entries, and all-hidden titled sections are omitted entirely. #494 #720 (comment) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the issue of hidden terms appearing in documentation pages. The changes are implemented at the right levels—in buildDocPage to filter entries before section assembly, and in deduplicateDocEntries/deduplicateDocFragments to prevent hidden entries from affecting the order of visible ones. The addition of comprehensive tests for these changes in doc.test.ts and parser.test.ts ensures the new logic is robust and covers various edge cases. The code is clean, follows the project's style, and the fix is well-described in the changelog. Overall, this is a high-quality contribution.
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
getDocPage()accepted customDocFragmentsfromParserimplementations but did not filter out entries whose terms are doc-hidden before producing the finalDocPage. This causedformatDocPage()to render blank rows for hidden entries. This PR fixes the issue at two levels:buildDocPage()now filters hidden entries before assembling sections, anddeduplicateDocEntries()/deduplicateDocFragments()now skip hidden entries before deduplication so they cannot influence the ordering or positioning of visible entries.The core design principle is that hidden entries are filtered out before any structural decisions (dedup key tracking, section creation, position assignment) rather than being replaced or patched up after the fact. This keeps the logic simple: hidden entries are invisible to the dedup and section-building machinery, and visible entries follow standard first-occurrence-wins ordering.
Concretely, a custom parser returning fragments like:
previously produced a
DocPagecontaining both entries, with--secretrendered as a blank row. After this fix, only--visibleappears in theDocPage.For
or()/longestMatch()combinations where branches share the same option with different hidden visibility, hidden entries no longer suppress visible duplicates or reorder the help output. For example,or(option("--x", { hidden: "doc" }), option("--y"), option("--x"))correctly produces--y, --xrather than--x, --y.Titled sections in
deduplicateDocFragments()are now positioned at their first fragment that contains visible entries, so hidden-only fragments from earlier branches do not cause sections to appear before they should.The existing render-level filter in
formatDocPage()is left in place as defense-in-depth for structurally degenerate entries.Test plan
getDocPageSync()filters hidden entries (hidden: true,"doc","help") and preserveshidden: "usage"entries, covering direct entry fragments, section fragments, and all term types (option, argument, command)buildDocPage()does not create empty sections from hidden-only fragments and positions titled sections at their first visible entrydeduplicateDocEntries()anddeduplicateDocFragments()skip hidden entries, preserve visible entry ordering, correctly handle titled/untitled sections, and omit all-hidden titled sectionsmise test)Fixes #494