Repository navigation
kb init: respect a repository routed by its adapter manifest - #266
Conversation
The first real run (afd-plugin skeleton, PR #265) failed four unit tests for one reason: afd-plugin is routed by its adapter manifest's review_routes, and direct_routing prefers a knowledge-side _routes.yaml whenever one exists. The skeleton wrote one, so routing status went from ready to scope_fallback, the golden routes drifted, and owners pointed at _index.md pages with no Direct quick map. It also appended its 16 rules to the top-level rules.md, one of the adapter's briefing docs, and the 8000-char briefing cap then truncated the index. Every stage now reads its owner table through init_coverage.owner_table with Direct's precedence: the routes file, else review_routes, else none. On a manifest-routed repository no stage writes _routes.yaml. What the skeleton and modules stages would have written as owners or prefixes becomes the exact review_routes entry the manifest lacks, on the checklist (adapters are human-gated). Doc-invariant rules go to the page of the owner that most specifically reaches the code they are about (evidence lines plus the paths the rule names); rules no owner reaches keep the rules page. Deepen writes onto the owner pages the manifest routes to. Pages in knowledge.briefing_docs are never appended to: rules meant for one go to rules-init.md beside it, with a checklist line. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6204aeebb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| if path in self.base: | ||
| self.record.checklist.append(f"{path} exists: the skeleton stage leaves routes to the modules stage") | ||
| return | ||
| if self.route_source == "manifest": |
There was a problem hiding this comment.
When _routes.yaml exists but has owners: [] and the manifest supplies review_routes, owner_table() correctly selects route_source == "manifest", but this presence check returns before _suggest_routes() runs. The same scenario also makes _checklist() read the empty file rather than self.owners, so the skeleton omits required adapter-route suggestions and incorrectly reports manifest-owned pages as unreachable. Branch on the resolved route source or on a nonempty route table rather than mere file existence.
Useful? React with 👍 / 👎.
| directory itself named without its slash (``facts.claims_in`` strips it | ||
| from a rule's backticked ``pkg/``), so a rule about a directory reaches | ||
| the owner of that directory.""" | ||
| return path.startswith(prefix) or (prefix.endswith("/") and path == prefix[:-1]) |
There was a problem hiding this comment.
For a manifest route whose prefix casing differs from the repository path, this case-sensitive comparison disagrees with production Direct routing: _adapter_changed_file_routes() case-folds both the changed path and prefix before matching. In that case init treats files that Direct successfully routes as uncovered, emits duplicate route suggestions, and can omit or misplace deepen rules. Normalize with the same case-folding semantics used by Direct.
Useful? React with 👍 / 👎.
| for key in ("briefing_docs", "briefing_docs_extra", "performance_briefing_docs"): | ||
| for item in (kn or {}).get(key) or []: |
There was a problem hiding this comment.
When a manifest temporarily contains both briefing_docs_extra and deprecated performance_briefing_docs, RepoAdapter.briefing() uses the legacy list only when the new list is empty (extra_docs = extra_docs or legacy), whereas this helper unions both lists. Init consequently treats legacy-only pages that are not actually rendered into the briefing as unwritable and redirects their rules to rules-init.md, changing placement and potentially creating unnecessary pages. Apply the same fallback semantics as the briefing renderer.
Useful? React with 👍 / 👎.
Summary
The first real
kb initrun (afd-plugin skeleton, PR #265, closed) failed four unit tests for one reason: afd-plugin is routed by its adapter manifest'sreview_routes, anddirect_routingprefers a knowledge-side_routes.yamlwhenever one exists. The skeleton wrote one, so routing status wentready→scope_fallback, the golden routes drifted, and owners pointed at_index.mdpages with no Direct quick map. It also appended its 16 rules to the top-levelrules.md, one of the adapter'sknowledge.briefing_docs, and the 8000-char briefing cap then truncated the index.Fix
init_coverage.owner_table(routes_text, manifest)gives every stage its owner table with Direct's precedence: a routes file that names owners →routes_file; else the manifest'sreview_routes→manifest(a valid but empty routes file defers to the manifest, as Direct does, and stays untouched); elsenone. A malformed routes file blocks at the shared input boundary._routes.yaml. Owners or prefixes the skeleton/modules stages would have written become the exactreview_routesentry the manifest lacks, on the checklist (adapters are human-gated).pkg/reaches the owner ofpkg/althoughclaims_instrips the slash); rules no owner reaches keep the rules page. Deepen writes onto the manifest's owner pages.knowledge.briefing_docsare never appended to: rules meant for one go torules-init.mdbeside it (checklist line), and coverage counts that page as the owner's, so deepen does not regenerate forever.Tests
test/test_kb_init_manifest_routed.py(11 tests: owner table incl. name collisions and the empty-file fallback, malformed file blocks, skeleton/modules/deepen on a manifest-routed toy, directory-naming rule placement, rules-init counting, knowledge-routed toy unchanged); the four tests #265 failed pass; all kb init suites pass; gate approved; local CI mirror green.Stacked next: the Direct quick-map PR for knowledge-routed repositories.
🤖 Generated with Claude Code