Repository navigation
sdk(knowledge): widen the catalog to owner rule pages with capacity - #136
Conversation
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: 96b1b8f851
ℹ️ 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".
| documents: list[str] = [] | ||
| for path in sorted(self._knowledge.rglob("rules.md")): | ||
| entries: list[KnowledgeCatalogEntry] = [] | ||
| for path in sorted(self._knowledge.rglob("rules*.md")): |
There was a problem hiding this comment.
When vllm-omni proposals target a newly exposed topic page that stores rules as level-three headings—for example, knowledge/repos/vllm-omni/components/diffusion/rules-platform-kernels.md:15 contains ### DIFF-1t—this expansion is incompatible with the existing _HEADING and _rule_exists logic, which recognize only ##. A proposal matching the page's style is rejected, while a ## DIFF-1t proposal is accepted as new and can be appended beside the existing rule because neither fixed validator checks duplicate rule IDs. Recognize both heading levels or exclude incompatible pages before cataloging them; otherwise automated intake can reject valid proposals or persist duplicate conclusions.
AGENTS.md reference: AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
Automatic knowledge batches could only append to each owner's rules.md entry page, and several of those pages sit within a few hundred bytes of the tree validator's 32 KiB split gate. Every rule routed there failed validation after apply, rolled the whole batch back, and needed a human to split the page by hand (#133, #134). The catalog now lists every owner rule page: the rules.md entry page and the owner's type: rule rules-<topic>.md topic pages the tree already uses. catalog_entries() carries each page's remaining bytes and non-empty lines, mirrored from check_knowledge_tree.py, the prompt shows that room, and validate_proposals rejects a proposal that would not fit ("page full") before anything is written, consuming room across one batch so two rules aimed at one page cannot both pass. Knowledge API 1.0.0 -> 1.1.0, backward compatible. The knowledge contract sentence that named rules.md as the only PR learning target now names the owner's rule pages; SPEC/sdk.md and SPEC/contract.md record the new surface and version. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Js5CsiwSYjcAYvqti1Lp1Y
96b1b8f to
2d9f8d0
Compare
Automatic knowledge batches (omni-reviewbot daily intake) could only append to each owner's
rules.mdentry page, and several of those pages sit within a few hundred bytes ofcheck_knowledge_tree.py's 32 KiB split gate (scheduler 32,496 B, minimax-h3 31,752 B, model-executor 31,389 B, serving 31,197 B). Every rule routed there failed validation after apply, rolled the whole batch back, and needed a human to split the page by hand (#133, #134). The 09-06 and 09-07 production batches failed exactly this way.Change
KnowledgeCurator.catalog()now lists every owner rule page: therules.mdentry page plus the owner'stype: rulerules-<topic>.mdtopic pages the tree already uses. Look-alike names andtype: guidepages are never targets.catalog_entries()/KnowledgeCatalogEntrycarry each page'sfree_bytesandfree_lines, mirrored from the validator's split gate (>= 32 KiBor>= 500non-empty lines fails). The prompt shows that room and tells the model to prefer a topic page that fits or shorten.validate_proposals()rejects a proposal that would not fit with apage full: ...reason before anything is written, charging the newlineapplyinserts on an unterminated page and consuming room across one batch so two rules aimed at the same page cannot both pass.1.0.0→1.1.0, backward compatible (no signature changes;catalog()still returns IDs).SPEC/sdk.mdandSPEC/contract.mdnow name the owner's rule pages rather than onlyrules.md.Nothing in
knowledge/content changes. ReviewBot adopts this by pinning the merged SHA; its doctor checks the version's presence, not its value.Validation
pytest: full suite passes excepttest_phase1_cluster.py::test_drift_guard_unimportable_checked_class_fails_guard, which fails identically on a cleanorigin/maincheckout in this environment (vllm import).page fullbefore validation, capacity consumed across a batch, unterminated-page boundary (accepted proposal renders exactly 32,767 bytes).check_knowledge_tree.py,check_wiki_lint.py,check_doc_links.py,check_doc_citations.py,check_spec_freshness.py --strictall pass.🤖 Generated with Claude Code
https://claude.ai/code/session_01Js5CsiwSYjcAYvqti1Lp1Y