Repository navigation
sdk(knowledge): carry a bounded diff excerpt per evidence event - #137
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: 1b2458a5f6
ℹ️ 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 spent + len(data) + reserve > _MAX_BATCH_DIFF_BYTES: | ||
| data = exhausted |
There was a problem hiding this comment.
When more than 20 nonempty excerpts are allowed, replacing an excerpt with the exhaustion marker does not latch the exhausted state, so a smaller later excerpt can still be admitted. For example, after nineteen 8 KiB excerpts, an eighth-kibibyte twentieth excerpt is marked omitted while a one-byte twenty-first excerpt is included. This contradicts the documented first-come behavior and makes the marker falsely claim the budget is exhausted; once an event cannot be admitted, subsequent nonempty events should remain omitted (or the current event should consume the remaining prefix budget).
Useful? React with 👍 / 👎.
96b1b8f to
2d9f8d0
Compare
The distillation prompt showed the model only a merged PR's title, body and changed paths, never the change itself; one rule per fifty merged PRs was the result. KnowledgeEvidenceEvent gains an optional diff_excerpt, the host's selection of the change's most rule-bearing hunks. The SDK bounds it to 8 KiB per event (a marked prefix cut, never a silent drop) and 160 KiB per batch, spent in event order so the host sets priority by ordering, fences it with the rest of the evidence, and tells the model to read it for the contract the change fixed or introduced. Defaults are empty and lossless, so existing hosts are unaffected. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Js5CsiwSYjcAYvqti1Lp1Y
1b2458a to
02d2c45
Compare
Stacked on #136 (base branch
fix/knowledge-catalog-capacity); merge that first.The distillation prompt showed the model only a merged PR's title, up to 3000 characters of body, and changed paths, never the change itself. Across the 68 merged PRs processed so far that yielded one rule.
Change
KnowledgeEvidenceEventgains an optionaldiff_excerpt: the host's selection of the change's most rule-bearing hunks (the reviewbot will pass non-test, non-doc hunks first).<untrusted_data>escaping applies) and the contract text tells the model to read it for the contract the change fixed or introduced.to_dict()carries the field; non-string values are rejected. No knowledge API version change beyond the 1.1.0 in sdk(knowledge): widen the catalog to owner rule pages with capacity #136.Validation
test_phase1_cluster.py::test_drift_guard_unimportable_checked_class_fails_guard(vllm import, fails identically on cleanorigin/main).check_doc_links.py,check_doc_citations.py,check_spec_freshness.py --strictpass.🤖 Generated with Claude Code
https://claude.ai/code/session_01Js5CsiwSYjcAYvqti1Lp1Y