docs: index all 9 feature guides on every surface + drift guard (LAB-1013) - #260
docs: index all 9 feature guides on every surface + drift guard (LAB-1013)#26027Bslash6 wants to merge 6 commits into
Conversation
…1013) Five of nine docs/features/ guides (interop-mode, l1-invalidation, reference-caching, rust-serialization, ssrf-protection) were unreachable from README.md, docs/README.md, and llms.txt — born orphaned because nothing checked index reachability. - Link all nine guides from all three index surfaces (deliberate call: the top-level README indexes the full set, not a curated subset). - Add tests/unit/test_feature_guide_index.py: globs docs/features/*.md and fails with the offending filenames if any guide is missing from an index. Lives in tests/unit/ because CI's PR lane only collects tests/unit/ and tests/critical/ — a guard in tests/docs/ would never run in CI.
This comment has been minimized.
This comment has been minimized.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughThe change adds five feature-guide links to three documentation indexes. A parametrised unit test checks that every feature guide appears in each index. ChangesFeature guide indexing
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![kodus-27b[bot]](https://avatars.githubusercontent.com/u/223823690?s=60&v=4){kind=small}
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/test_feature_guide_index.py`:
- Around line 23-39: Update _is_linked to ignore non-rendered Markdown content,
including fenced code blocks and HTML comments, before evaluating inline and
reference-style links. Preserve detection of rendered links, and add regression
cases covering both fenced code and HTML comments.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fbe5edd9-4422-410a-98af-f9060082ce44
📒 Files selected for processing (4)
README.mddocs/README.mdllms.txttests/unit/test_feature_guide_index.py
…omments (LAB-1013) CodeRabbit: link-shaped text inside a fenced code block or HTML comment satisfied _is_linked without rendering, so the guard could pass while a guide stayed unreachable. Strip non-rendered content before matching, fail loud on unpaired fences (silent pairing skew would reopen the same false-pass), and pin both cases with a regression test.
This comment has been minimized.
This comment has been minimized.
|
@coderabbitai review |
|
@kody start-review |
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/test_feature_guide_index.py (1)
48-53: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMatch only local guide paths.
re.escapeprotects the guide name, but both expressions still matchfeatures/{guide_name}anywhere in a URL. For example,[X](https://example.test/features/x.md)is accepted. A used reference to the same external URL is also accepted. The guard can therefore pass while the repository guide is absent. Match onlyfeatures/{guide_name}ordocs/features/{guide_name}at the link target boundary. Add negative cases totest_is_linked_counts_rendered_links_only.Proposed matching change
target = re.escape(f"features/{guide_name}") - if re.search(rf"\]\([^)]*{target}\)", index_text): + local_target = rf"(?:docs/)?{target}" + if re.search(rf"\]\({local_target}(?:[?#][^)]*)?\)", index_text): return True - for m in re.finditer(rf"^\[([^\]]+)\]:\s*\S*{target}\s*$", index_text, re.MULTILINE): + for m in re.finditer( + rf"^\[([^\]]+)\]:\s*{local_target}(?:[?#]\S*)?\s*$", + index_text, + re.MULTILINE, + ):Also applies to: 57-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_feature_guide_index.py` around lines 48 - 53, Restrict the link and reference regexes in the relevant guide-link helper to local targets only: accept targets beginning with features/{guide_name} or docs/features/{guide_name} at the link boundary, and reject external URLs containing those paths. Extend test_is_linked_counts_rendered_links_only with negative cases covering external inline and reference links.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/test_feature_guide_index.py`:
- Around line 30-34: Update _strip_non_rendered to remove HTML comments before
validating or stripping fenced blocks, so fence markers inside comments are
ignored and cannot pair with rendered content. Preserve the existing
unpaired-fence assertion after comment removal, and add a regression test
covering a fence contained within an HTML comment.
---
Outside diff comments:
In `@tests/unit/test_feature_guide_index.py`:
- Around line 48-53: Restrict the link and reference regexes in the relevant
guide-link helper to local targets only: accept targets beginning with
features/{guide_name} or docs/features/{guide_name} at the link boundary, and
reject external URLs containing those paths. Extend
test_is_linked_counts_rendered_links_only with negative cases covering external
inline and reference links.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 765d47af-24cc-423f-9340-a0209994a772
📒 Files selected for processing (1)
tests/unit/test_feature_guide_index.py
…(LAB-1013) Two CodeRabbit findings on the previous fix, both reproduced against e797ba5 before changing anything: - Sequential strip passes are wrong in *both* orderings. Fences-first made a ``` inside a closed <!-- --> look like an unpaired fence and raised AssertionError; the reordering CodeRabbit proposed only moves the bug, since a <!-- inside a code fence is literal text and would then eat the rendered content after it. One alternation fixes both: re.sub scans left to right, so whichever construct opens first consumes the other — which is how markdown resolves the overlap. - _is_linked matched features/<name> anywhere in a link target, so https://elsewhere.example/features/x.md counted as a local guide (confirmed True on both the inline and reference-style forms). Anchor the path to the target's start, allowing only ./ and docs/ prefixes. Every real link across the three surfaces is repo-relative, so this narrows nothing that exists; an unrecognised form now fails loud rather than passing falsely. Regression tests pin both, and both fail against the old helpers.
This comment has been minimized.
This comment has been minimized.
Review remediation — 031c7ca1. "Match only local guide paths" (outside-diff, Major) — fixed. path = re.escape(f"features/{guide_name}")
target = rf"(?:\./)?(?:docs/)?{path}"Every real link across the three index surfaces is repo-relative ( 2. Fence/comment strip ordering (Major) — fixed, see the inline reply. Single alternation rather than the proposed reordering, because sequential passes are wrong in both directions. Verification:
|
pip-audit fails the PR lane on GHSA-6hr6-w5qg-qmwg: h2 <=4.4.0 forwards duplicate Host headers, a request-smuggling primitive when HTTP/2 is downgraded to HTTP/1.1. h2 is transitive via httpx[http2]; lock-only bump (hpack 4.1.0 -> 4.2.0 comes along). pip-audit clean locally on the same 'uv sync --group dev' resolution CI uses; full unit suite green.
|
@coderabbitai review |
|
@kody start-review |
|
…B-1013) A shared optional (?:docs/)? prefix counted a link that 404s on its own surface (docs/features/x.md inside docs/README.md, or features/x.md inside the top-level README) as indexed — a false pass against the guard's charter of reader reachability. INDEX_FILES now maps each surface to its exact prefix and _is_linked anchors on it; the two copy-paste-between-surfaces mistakes are covered as negative cases. Also trims speculative tolerances the panel cut: ./-prefixed links and #anchor/?query suffixes (no surface uses either; if one appears the guard fails loud, the safe direction), the offsite-URL self-test (emergent from the anchored prefix, not a separate code path), and the interleaved fence/comment self-test (construct absent from all three files).
This comment has been minimized.
This comment has been minimized.
Expert-panel re-review (round 2 requested on LAB-1634) — 20c70dcFull panel re-run (bug-hunter, security, craftsman, pragmatism) on the current diff. One surviving MAJ, now fixed: Per-surface link prefixes in the index guard. The shared optional Panel cut list applied in the same commit (net −5 lines): the Verified: 1915 unit tests pass, ruff lint/format clean, and mutation probes confirm the guard fails loud on an orphaned guide, an unused reference definition, a fenced/commented link, and a wrong-prefix link on every surface. Kody note: the standing CHANGES_REQUESTED (2026-08-07T06:12Z) cites |
|
@coderabbitai review |
|
@kody start-review |
|
Kody flagged that 20c70dc's exact-prefix rewrite dropped the suffix tolerance, so a correctly-indexed link like docs/features/x.md#anchor would be reported as an orphan — a drift guard whose failure message lies about a true fact. Restore the tolerance as one atom on the shared target so both link forms (inline and reference-definition) get it; [^)\s]* keeps it bounded by the inline ) and the definition's line end. The character after .md must still be #, ?, ) or end-of-definition, so x.mdx cannot false-pass — pinned by the new negative assert. Panel-reviewed (4 agents): SHIP, no findings, no cuts; the restore is guard correctness, not the speculative tolerance the panel cut.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
@kody start-review |
|
@coderabbitai review |
|
|
|
||
| def test_is_linked_accepts_fragment_and_query_suffixes(): | ||
| """#fragment / ?query target the same file — indexed; other suffixes are not it.""" | ||
| assert _is_linked("[X](docs/features/x.md#anchor)", "x.md", "docs/features/") |
There was a problem hiding this comment.
Violates team rule 'Don’t Use `assert` for Data Validation': Ensure that assert is not used for validating user input or critical checks. Assertions can be disabled in optimized mode (python -O). Recommend using explicit validation with if conditions and raising proper exceptions.
Also found in:
tests/unit/test_feature_guide_index.py:82-82tests/unit/test_feature_guide_index.py:83-83tests/unit/test_feature_guide_index.py:85-85
Prompt for LLM
File tests/unit/test_feature_guide_index.py:
Line 81:
Violates team rule 'Don’t Use `assert` for Data Validation': Ensure that `assert` is not used for validating user input or critical checks. Assertions can be disabled in optimized mode (`python -O`). Recommend using explicit validation with `if` conditions and raising proper exceptions.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Summary
Five of the nine
docs/features/guides (interop-mode,l1-invalidation,reference-caching,rust-serialization,ssrf-protection) were unreachable from every index surface —README.md,docs/README.md, andllms.txtall listed the same original four. Guides were "born orphaned" because nothing checked index reachability (LAB-1013).Changes
README.md, table rows indocs/README.md,- [Name](path): descriptionlines inllms.txt). Deliberate call: the top-level README indexes the full set rather than a curated subset.tests/unit/test_feature_guide_index.pyglobsdocs/features/*.mdand fails with the offending filenames if any guide is not linked from an index. It verifies rendered reachability: for reference-style links it requires the definition's label to actually be used, so an orphaned[label]: docs/features/x.mddefinition with no table row does not pass.tests/unit/(nottests/docs/) because CI's PR lane collects onlytests/unit/andtests/critical/— a guard intests/docs/would never run in CI.Proof the guard works
interop-modeline fromllms.txt→AssertionError: Feature guides missing from llms.txt: ['interop-mode.md']AssertionError: Feature guides missing from README.md: ['interop-mode.md']Verification
uv run pytest tests/unit/ -m "not slow" -n auto— 1913 passeduv run pytest tests/critical/ -m "not slow"— 234 passeduv run pytest --markdown-docs docs/— 121 passeduv run ruff check/ruff format --check— cleanSummary by CodeRabbit
Documentation
Tests