feat(#5630): add scoped search filters to docs site - #5704
Conversation
PR Summary by QodoAdd scoped filters to VitePress local search modal
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
|
🤖 Finished Review · ✅ Success · Started 9:16 AM UTC · Completed 9:37 AM UTC |
Site previewPreview: https://01d070e6-site.fullsend-ai.workers.dev Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Code Review by Qodo
1. Hashless id truncation
|
ReviewFindingsMedium
Low
Previous runReviewFindingsMedium
Low
Previous run (2)ReviewFindingsMedium
Low
Labels: PR adds search scoping feature to the VitePress documentation site |
7598df0 to
062e993
Compare
|
🤖 Finished Review · ✅ Success · Started 9:42 AM UTC · Completed 10:00 AM UTC |
waynesun09
left a comment
There was a problem hiding this comment.
Reviewed the scoped search filter changes. Left inline comments covering keyboard/screen-reader accessibility of the new scope toggles, undeclared dependencies pulled in only via hoisting, a TypeScript excess-property gap on the new config field, missing test coverage for the filter predicate, an overly broad stylelint rule disable, and a docs folder left out of every search scope.
33bb3e6 to
2a980ee
Compare
|
🤖 Review · |
|
🤖 Finished Review · ✅ Success · Started 2:45 PM UTC · Completed 2:59 PM UTC |
There was a problem hiding this comment.
See the review comment for full details.
Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:
website/.vitepress/config.ts(file-level): Line 611 · [medium] logic-error
Top-level docs pages (vision.md, architecture.md, runtimes.md, glossary.md, roadmap.md, landscape.md, doc-site.md, admin-oauth-worker.md, web-admin-deployment.md, index.md) live at paths like /docs/vision, /docs/architecture, etc. None of the defined scope prefixes cover these paths. When any scope pill is active, these pages become unreachable in search. The PR's own docs/doc-site.md update states 'Every docs/ subfolder that produces rendered pages must appear in at least one scope' — root-level files violate this stated invariant.
Suggested fix: Add a catch-all scope (e.g., 'Concepts' covering root-level paths) or include these paths in an existing scope.
website/.vitepress/config.ts(file-level): Line 611 · [low] intent-alignment
Issue #5630 requests per-folder filtering with 10 specific top-level doc folders listed individually. The implementation groups them into 4 aggregate scopes (Guides, Design Docs, Experiments, Contributing), so users cannot filter to just ADRs or just problems independently. The scope configuration is data-driven, making per-folder scopes trivial to add later.
Suggested fix: Confirm with the issue author that 4-scope grouping is the intended UX, or provide per-folder scopes as originally requested.
website/.vitepress/config.ts(file-level): Line 625 · [low] dead-config
The Contributing scope includes the prefix /docs/testing/, but srcExclude contains /testing/, which prevents any file under docs/testing/ from being built. The prefix will never match a rendered page.
website/.vitepress/theme/components/VPLocalSearchBox.vue:1: [low] script-attribute-ordering
The script tag uses <script lang="ts" setup> but existing Vue components in this repo use <script setup lang="ts">. However, this is a vendored upstream file and matching upstream's attribute order minimizes diff noise during future VitePress upgrades.
website/.vitepress/search.d.ts:6: [low] type-duplication
The scope shape { label: string; prefixes: string[] } is defined inline here and also as the named SearchScope interface in searchScopes.ts. Since this is a module-augmentation .d.ts file, adding a runtime import would change the file's semantics — the duplication is pragmatic.
2a980ee to
68572ad
Compare
|
🤖 Review · |
waynesun09
left a comment
There was a problem hiding this comment.
Automated multi-agent review pass (3 independent reviewers: 2x Claude, Grok). Left 3 new inline findings below (1 high, 2 medium) plus a reply flagging that an earlier "resolved" thread's underlying bug is still present in the current code.
Two other findings from this pass — root-level docs pages becoming unreachable under a scope filter, and a dead /docs/testing/ prefix — were already fixed by the latest push before I could post them. No action needed there.
Override VitePress's VPLocalSearchBox with scope pills that filter search results by path prefix. Extract the filtering predicate into a tested utility. Add a vitepress type augmentation for the scopes config field. Make scope checkboxes keyboard-accessible and visible to assistive tech. Scope stylelint exemptions to inline comments instead of global rule disables. Declare hoisted vitepress transitive deps explicitly in website/package.json. Closes #5630 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Hector Martinez <hemartin@redhat.com>
68572ad to
f958fcb
Compare
|
🤖 Finished Review · ✅ Success · Started 3:39 PM UTC · Completed 3:55 PM UTC |
waynesun09
left a comment
There was a problem hiding this comment.
Both HIGH findings from the review pass are resolved and verified in code at f958fcb9:
- Enter-key-on-checkbox navigation bug — fixed (checkbox inputs now excluded from the Enter handler's fallthrough)
- Root-level pages unreachable under a scope filter — fixed earlier via the "Others" catch-all scope, with test coverage
The two MEDIUM findings from this pass are also addressed (vitepress pinned to exact 1.6.4, PR description corrected). One MEDIUM item remains open (hashless search-result id truncation) but it's inherited from upstream VitePress unchanged, has narrow reachability, and degrades gracefully — not blocking.
All CI checks pass. Approving.
|
🤖 Finished Retro · ❌ Failure · Started 3:50 PM UTC · Completed 3:51 PM UTC |
|
Review skipped — this PR is already merged. The Posted by fullsend post-review check |
Summary
VPLocalSearchBoxwith a custom component that reads search scope definitions fromthemeConfig.search.options.scopeslabelandprefixesarray; users toggle pill-style buttons in the search modal to restrict results to matching path prefixesdist/client/...) that are not public APIstylelint-config-html/vueso stylelint can parse.vueSFCs;:deep/:globalpseudo-classes are handled by inlinestylelint-disablecomments in the componentCloses #5630
🤖 Generated with Claude Code