fix(cgp): §B3-honest frame token_cost (+ unbreak clippy CI) - #355
Merged
Conversation
The CGP pin rebase (#354) claimed to adopt "canonical token accounting" but kept the old estimate: the store declared `estimate_tokens(content) + estimate_tokens(title)` and the graph declared `estimate_tokens(content).max(1)`. Under the pinned protocol, §B3 requires `token_cost == budget_tokens(content)` — the canonical inline count (ceil(bytes/4)), exact, no tolerance. Neither builder was honest: the store added the title's tokens (always non-zero) and used chars not bytes; the graph's `.max(1)` diverges on empty content. The existing conformance gate didn't catch it: its probe query returns zero frames from the mismatched seed, so §B3 never fired on a real frame. Fix: both production builders now declare `budget_tokens(content)`. `pack_to_budget` already packs against each frame's own token_cost, so the summed-budget invariant is preserved. New `recalled_frames_declare_honest_token_cost` drives the real `recall` builder with a query that provably surfaces a frame and asserts `declares_honest_token_cost()` — it fails on the pre-fix code (token_cost 18 vs canonical 15) and passes after. A matching assertion guards the graph builder. Dead `estimate_tokens` helpers removed.
`main` (b0117f3) fails `cargo clippy --workspace --all-targets -- -D warnings` — the exact CI gate — on two warnings that landed unprotected: - `run_shared_candidates` (pipeline.rs, from #352) lacks the `#[allow(clippy::too_many_arguments)]` its five sibling functions in the same file already carry. - `use contextgraph_types::Representation` (contextgraph.rs, from #354) is used only in a `#[cfg(test)]` helper, so it is unused in the bin build. Import removed; the one test use is fully-qualified. Both unrelated to the token_cost fix but required for this PR's CI to be green.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
macanderson
marked this pull request as ready for review
July 23, 2026 14:41
This was referenced Jul 23, 2026
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
1 participant
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.
Context
The user asked to rebase the CGP pin forward. While I was doing that, #354 landed the pin bump on
main(58a933a2→9fb559aa) — so the rebase itself is already done. This PR is the residual gap #354 left, plus the CI breakage it landed with.The bug:
mainclaims §B3-honest, isn't#354's description says it adopted the sweep's "canonical token accounting." It didn't. Both production frame builders still declare the old estimate:
stella-context):estimate_tokens(content) + estimate_tokens(title)— adds the title's tokens (always non-zero) and counts chars, not bytes.stella-graph):estimate_tokens(content).max(1)— the.max(1)diverges on empty content.Under the pinned protocol, §B3 requires
token_cost == budget_tokens(content)(canonicalceil(bytes/4), exact, no tolerance). Neither builder is honest.The conformance gate (#347) didn't catch it — its probe query returns zero frames from a mismatched seed, so §B3 never fires on a real frame (a vacuous pass).
The fix (red → green, proven)
contextgraph_types::budget_tokens(content).pack_to_budgetalready packs against each frame's owntoken_cost, so the summed-budget invariant is preserved.recalled_frames_declare_honest_token_costdrives the realrecallbuilder with a query that provably surfaces a frame (mirrorsrecall_returns_cited_budget_respecting_frames) and assertsdeclares_honest_token_cost. It fails on pre-fix code (token_cost 18vs canonical15— the 3-token gap is the title) and passes after. A matching assertion guards the graph builder. Deadestimate_tokenshelpers removed.Second commit: unbreak CI (pre-existing, unrelated)
maincurrently failscargo clippy --workspace --all-targets -- -D warnings— the exact CI gate — on two warnings that merged unprotected: a missing#[allow(clippy::too_many_arguments)]onrun_shared_candidates(from #352, its five siblings have it) and an unusedRepresentationimport (from #354, used only in a test helper). Cleared so this PR's CI is green. Heads-up:mainis red on CI right now — worth branch-protecting (tracked P0).Verification