Repository navigation
test(architecture): ratchet feature-code reaches past the resolution catalogue - #333
Merged
Merged
Conversation
…catalogue Feature code keeps growing private resolvers because the catalogue (`CstQuery`, `NodeExt`, `StrExt`) is not named anywhere an agent reads by default, and nothing stops a reach into `resolver::infer` or the raw index maps. - tests/architecture_ratchet.rs counts those reaches in src/features, src/semantic_tokens and src/inlay_hints.rs against today's baselines and fails on any drift, so a count can only go down. - AGENTS.md names `CstQuery`, `NodeExt` and `StrExt` by the question each answers, and states the rule the test enforces. - docs/agent-reference.md §12 gains the relocation rule and "extend, don't fork". No behaviour change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Aggregate counts allow replacement bypasses, and missing scan roots can silently reduce coverage.
2 open findings
What changed in this PR
Adds an architecture ratchet preventing feature code from bypassing resolution APIs.
Changes:
- Scans feature sources for raw resolver/index access.
- Documents preferred resolution primitives and extension practices.
| File | Description |
|---|---|
tests/architecture_ratchet.rs |
Adds bypass-count enforcement. |
AGENTS.md |
Adds architectural guidance. |
docs/agent-reference.md |
Documents reuse and extension patterns. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ears Review feedback on #333: - A total per pattern let one removed bypass pay for a new one elsewhere. Baselines are now per file, so a reach in a new file fails even when another file dropped one. A swap inside a single file still passes; the header says so. - A renamed or deleted feature root was skipped silently. Each root in FEATURE_ROOTS must now exist. - Paths are reported relative to the crate root with '/' separators, so the baselines hold on Windows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Why
Review of #329 showed an agent reusing every primitive named in
AGENTS.mdand reinventing most of the ones that are not: about 340 lines of line-text parsing next to an existing CST walker, directindexer.definitionsreads, and copies ofnamed_arg_labelandparam_names_from_sig. The catalogue exists, but nothing an agent reads by default names it, and nothing fails when feature code reaches around it.What
tests/architecture_ratchet.rsscanssrc/features,src/semantic_tokensandsrc/inlay_hints.rs(production files, non-comment lines) for literal reaches intoresolver::infer*and the raw index maps, and compares each count with a baseline taken frommaintoday. Any drift fails: more means a new bypass, fewer means the baseline must be lowered in the same change. The failure message lists every site.AGENTS.md: Key Files entries forCstQuery,NodeExtandStrExt, phrased as the question each answers, plus one Code Quality rule stating what the test enforces.docs/agent-reference.md§12: a private helper needed by a second feature moves next to its input type; an almost-right primitive gets its return type widened rather than a sibling.Baselines
resolver::infer::resolver::infer_lines::infer_variable_typeinfer_type_in_lines.definitions./.jar_definitions..files./.jar_files..live_lines.extension_by_receiverLimits
The scan is literal text. A reach spelled through a re-export (
use crate::resolver::{infer_variable_type_raw}is caught by name, other re-exports are not) or split across lines escapes it. The durable fix is to makeresolver::inferprivate once its count reaches zero; the file header says so.Verification
cargo test: 2103 passed, 0 failed.cargo clippy --all-targets -- -D warnings: clean.indexer.definitions.get(..)call to a feature file reports.definitions.: baseline 0, found 1with the file and line.No behaviour change. #329 as currently written will fail this test once it is rebased.
🤖 Generated with Claude Code