[boazdavid]

The read-beats-the-hashes rule set B* to the end of the previous extent when the read came within 128 tokens plus 2% of it. A delivered cachesplit on a changed system block reads only the stable half. On a short conversation that half is within the tolerance, so the split effect was lost (context-guru#909). The rule now skips this case. New case split/short-conversation/tail-changed. docs/design.md updated. gofmt, vet and go test pass.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

[boazdavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 2 (glm, user of @boazdavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.241	offload-review skill, @boazdavid's session	fresh	2 of 2	1 of 5	23e2ed1c3cc1	blocking

Verdict: blocking

Findings

1. internal/engine/compute.go:832 — the new exception fires on a full read too, and books a phantom split effect
   • What fails: a delivered cachesplit plus a spurious hash change of the system block plus a read that covers the whole previous extent. The code books a split saving of the stable half. The correct result is no split effect. Nothing changed in content, so the client body would have read the entry in full too.
   • Proof: the cause is the one docs/issue_cases.md documents for accountant#20 and #21: "A host that hashes a message with its cache_control field sees a new hash when the client moves the mark. The text did not change." Take the s909 case and move the mark onto the system block instead of a message. Step 1: the client system text is the same, but its hash changed, so clientFD = 0. The forwarded volatile block's hash changed the same way, so fwdFD = 1. The provider read the whole entry, so usage.CacheRead = 3170 and readCoversExtent holds (want = 3170 <= 3170 + 128 + 63, compute.go:914). clientFD (0) <= prevExtCli (1) and prevExtCli (1) > bstar (-1) hold, so the B* rule at compute.go:832 would fire. splitChangedSystem (compute.go:1627) returns true: clientFD (0) < sectionEnd(1), the cachesplit item is reported again so it is not ended, and the forwarded system section has 2 positions against the client's 1. So B* stays -1. Then split (compute.go:1149) runs: fwdFD = 1 > 0, CacheRead > 0, clientFD < sectionEnd(1), and n = min(upTo(fwd, 0), 3170) = 3000 with bstarFwd = -1, so it books a split effect of 3,000 tokens, about +$0.01035. Without this commit the rule sets B* = prevExtCli, bstarFwd reaches past the whole extent, n <= 0, and no effect is booked. The existing guard case split/tail-unchanged-hash-differs/no-effect covers this cause only on a message; on the system block the new guard removes the protection. The doc states the exception only for the tolerance window ("On a short conversation that half is within the read tolerance ... of the whole previous extent, so the read looks like a full read", design.md:643), but the code applies it to every read that covers the extent, including a read that reaches past the forwarded first difference. A read that passes fwdFD is proof that the hash difference moved no content, and a real tail change always stops the read at the tail, short of the extent.
   • Fix: do not apply the exception when the billed read reaches past the forwarded first difference. For example, in the B* guard, when splitChangedSystem() is true, also require that the read stops at the split: c.usage.CacheRead <= c.ratio.upTo(c.fwd, c.fwdFD-1) + 128 + c.usage.CacheRead/50. The s909 case keeps its effect (read = 3000 = upTo(fwd, 0)), and the phantom case does not (read = 3170 > 3000). Add a guard case: the split/tail-unchanged-hash-differs case with the moved mark on the system block, and it must book no split effect. Update design.md §5.1 to state the condition you pick.

Non-blocking

• internal/engine/compute.go:1628 — clientFD < sectionEnd(1) also counts a change in the tools section as a change of the system block. The read then covers nothing, so the B* rule cannot fire and nothing breaks. split (compute.go:1164) uses the same test, so the two agree. A lower bound of c.client.Start[1] would say what the comment says.
• test/gen/gen_cases.py:1358 — the case's source is more_examples.md#S909, but docs/more_examples.md has no S909 section. The write-up lives only in the generator comment and the JSON description. I44 has the same gap, so this follows a precedent. write_issue with issue_cases.md#context-guru-909 would name a place a reader can find.
• internal/engine/compute.go:1627 — the scan for a cg:cachesplit: item is now the third copy of the same loop (split, headTTL, splitChangedSystem). The three can drift apart.
• docs/design.md:647 — "B* then stays before the changed block". In the s909 case B* is none (-1), not a position before the block. Minor wording.

What I checked

• Read CLAUDE.md, docs/spec.md, docs/design.md (§4.4, §5.1, §5.2), and docs/issue_cases.md (the #20 and #21 cases).
• Read the full diff and the code around each change: phaseAndBoundary, split, readCoversExtent, readable, addBreak, firstDiff, sectionEnd, items, forwardPositions.
• Traced the new case split/short-conversation/tail-changed through the code. The new guard is what books the effect: without it, B* = prevExtCli = 1, bstarFwd covers 3,220 tokens, and n <= 0. The case's figures check out: read 3,000 + write 220 = 3,220 billed; baseline writes 3,220; the split saves 3,000 x ($3.75 - $0.30) = +$0.01035.
• Ran the generator into /tmp and compared: every JSON file in test/cases is identical to the generator output, so the CI go generate gate passes.
• Ran go vet ./..., go test ./..., gofmt -l ., and go mod tidy -diff. All clean.

[boazdavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 1 (glm, user of @boazdavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.241	offload-review skill, @boazdavid's session	fresh	1 of 2	1 of 5	23e2ed1c3cc1	blocking

Verdict: blocking

Findings

1. test/gen/gen_cases.py:1355 — the new case names a source that does not exist
   • What fails: s909() writes the case with source: more_examples.md#S909. docs/more_examples.md
     has no scenario S909. Its scenario tables hold A1 to H6 only, and its section 6 says each of its
     cases has source: more_examples.md#<ID>. So the case cites a section that no reader can open,
     and the doc's own claim is now false for this case. The fix's issue (context-guru#909) is named
     only in the new design §5.1 paragraph. No doc describes the scenario the case pins.
   • Proof: grep -n "S909" docs/*.md returns nothing but the new design paragraph. The same-shape
     case from issue #45 (split/delivered/stable-half-read) is sourced the repo's way: a section in
     docs/issue_cases.md ("Case 14", #45) through write_issue.
   • Fix: do one of these. Add an S909 row to section 3 of docs/more_examples.md and an entry in its
     section 6. Or move the case to write_issue and add a section for context-guru#909 to
     docs/issue_cases.md, as #45 and the context-guru#768 review cases did.

Non-blocking

• docs/design.md:643 says the exception is "a delivered cachesplit on a system block the client
  changed". splitChangedSystem (internal/engine/compute.go:1628) fires on a client diff anywhere
  before the end of the system section, a tools change included. split() (compute.go:1164) reads
  the same comparison the same way, so code and code agree. In the extra cases the raise could not
  fire anyway: a diff at the front keeps the read far below the extent. Only the doc phrase is narrower
  than the gate.
• splitChangedSystem repeats the cachesplit item scan and the delivered expression of split().
  A shared helper would keep the two in step when one changes.
• The new case pins no expected.boundary. The split effect pins it in effect: n = 3000 holds only
  when B* stays before the system block. The case fails without the fix (see below), so this is a
  note, not a gap.

What I checked

• Read CLAUDE.md, docs/spec.md, docs/design.md in full, and the full diff
  (5d9e4f7..HEAD: internal/engine/compute.go, docs/design.md, test/gen/gen_cases.py, the new
  JSON case).
• Traced the new case by hand through phaseAndBoundary, cacheEffects and split. Numbers: previous
  extent 3,170 tokens; read 3,000; tolerance 3,000 + 128 + 60 = 3,188 ≥ 3,170, so readCoversExtent
  is true. clientFD = 0 (the client changed system[0]), prevExtCli = 1, so the raise would set
  B* to messages[0] and split would get n = 3,000 − 3,170 < 0: no split effect. With
  !c.splitChangedSystem() the raise is skipped, B* stays before the system block, and the split
  books 3,000 write→read, $0.01035. The case fails without the fix, so it is a real regression test.
• Checked the exception's scope: it sits inside case PhaseHot, needs a non-ended cachesplit item and
  a delivered split, and mirrors the gates of split(). When it fires and the read does not cover the
  extent, nothing changes, because the raise needs that read. The counterfactual is right: the client
  changed its own system block, so the client body would have written, and saved tokens after B* take
  the write tier.
• Recomputed the FNV-1a cumulative body hashes of all four bodies of the new case with the generator's
  own algorithm. All match the JSON, so the JSON is the generator's output for this gen_cases.py.
• Ran gofmt -l . (nothing), go vet ./... and go test ./.... All clean and green.
• Checked the rules: no new import (R3 holds), no API change (accountant.go untouched), Compute
  stays deterministic with no I/O (R26). docs/design.md §5.1 describes the new behavior in the same
  PR, as CLAUDE.md rule 5 requires.

[boazdavid]

Round 1 fixes are in 3d20a8a.

1. Phantom effect on a full read (blocking): fixed. The guard case split/tail-unchanged-hash-differs-system/no-effect failed on 23e2ed1 first (2 effects, a 3,000-token split). The exception now needs CacheRead <= upTo(fwd, fwdFD-1) with no tolerance, because the tolerance is the window where the two cases look alike. The s909 case keeps its effect (3,000 <= 3,000). Design 5.1 states the condition and the within-tolerance-but-past-fwdFD case.
2. Missing S909 source (blocking): fixed. The case now uses write_issue and issue_cases.md has a section for context-guru#909 with the hand arithmetic for both cases.
3. Non-blocking: the design text now says the gate also fires on a tools change, and the wording "B* stays before the changed block" is replaced by "none, or before the changed block". The cachesplit scan and the delivered test are now shared helpers (splitItem, splitDelivered); headTTL scans a different prefix, so it keeps its own loop. The s909 case was not given an expected.boundary: B* is none there, and the case format only pins a non-nil boundary. The guard case pins messages[0].
   Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

[boazdavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 2 (glm, user of @boazdavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.241	offload-review skill, @boazdavid's session	resumed	2 of 2	2 of 5	3d20a8a49724	approve

Verdict: approve

Findings

None.

Earlier findings

1. Mine, round 1: the exception fired on a full read and booked a phantom split effect — fixed.
   • splitChangedSystem (compute.go:1645) now ends with c.fwdFD > 0 && c.usage.CacheRead <= c.ratio.upTo(c.fwd, c.fwdFD-1). I checked the no-tolerance choice: my round-1 fix proposal kept the 128-plus-2% tolerance, and that version does not work. In the phantom case the read is 3,170 and the prefix is 3,000, so 3,170 <= 3,000 + 128 + 63 = 3,191 holds and the phantom would stay. The author's reason ("the tolerance is the very window in which the two cases look alike") is right, and the code comment states it.
   • The new guard case split/tail-unchanged-hash-differs-system/no-effect is my round-1 scenario built as a case: the same shape as s909, the system hash differs, the provider reads all 3,170. I traced it through the code: splitChangedSystem returns false (3,170 > 3,000), the B* raise fires, bstar = 1, bstarFwd = 2, so split gets n = 3,000 - 3,220 < 0 and books nothing. The break lands at row 2 with From = messages[1] (readEnd 3, inside extent's own tolerance) and To = messages[0] (B*), and moves no tokens. The boundary is pinned as messages[0]. All matches the JSON.
   • The s909 case keeps its effect: read 3,000 <= upTo(fwd, 0) = 3,000, so the exception fires.
2. Reviewer 1, round 1: the case named a source that does not exist — fixed, and I checked it myself. The case now uses write_issue with issue_cases.md#context-guru-909, and docs/issue_cases.md has a new section for context-guru#909 that states both cases and their arithmetic. I checked the numbers: 3,000 + 128 + 60 = 3,188 >= 3,170, and 3,170 > 3,000. The section also records the history: the first version of the fix booked the phantom.
3. Non-blocking notes, both reviews — all addressed. Design §5.1 now says "a client diff before the end of the system section (the tools section counts too)" and "none, or before the changed block". The scan and the delivered test are shared helpers (splitItem, splitDelivered); headTTL keeps its own loop with a comment that says why. The s909 case pins no boundary because its B* is none and the format pins only a non-nil one; the guard case pins messages[0]. I accept the reason.

Non-blocking

• internal/engine/compute.go:1648 — the comparison CacheRead <= upTo(fwd, fwdFD-1) has no tolerance, but upTo is the byte-ratio estimate while CacheRead is the provider's true count. A genuine split whose stable half tokenizes denser than the body average makes the estimate run low, and the effect is then lost. This is the safe direction (a doubtful figure lowers a saving), and any tolerance reopens the phantom, so I accept it. The case suite pins only exact 4-bytes-per-token bodies, so no case covers an inexact ratio here.

What I checked

• Last review: round 1, 23e2ed1, 2026-11-05T06:25:47Z.
• New commits read: 1 — 3d20a8a "fix: Stop the short-turn split exception from booking a phantom effect on a full read".
• Read in full: 2 reviews, 1 comment, 0 inline comments (from .review-in-reviews.txt, .review-in-comments.txt, .review-in-inline.txt).
• New items since my last review: the fix commit 3d20a8a (reviewed below), the author's round-1 reply comment (checked each claim against the code), reviewer 1's round-1 review (read in full; its finding is fixed).
• Read the full diff of 3d20a8a and the code around each change: splitChangedSystem, splitItem, splitDelivered, split, the B* guard at compute.go:832, extent, upTo, clientToFwd.
• Traced the new guard case by hand through phaseAndBoundary, cacheEffects, split, and addBreak. The trace matches the expected JSON in full: no split effect, one in-baseline break that moves no tokens, boundary messages[0].
• Ran the generator into /tmp and compared: every JSON file in test/cases is identical to the generator output, so the CI go generate gate passes.
• Ran go vet ./..., go test ./..., gofmt -l .. All clean.
• Checked the rules: no new import (R3 holds), accountant.go untouched, design.md §5.1 and issue_cases.md describe the new behavior in the same PR.

[boazdavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 1 (glm, user of @boazdavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.241	offload-review skill, @boazdavid's session	resumed	1 of 2	2 of 5	3d20a8a49724	approve

Verdict: approve

Findings

None. The new commit fixes both blocking findings and every non-blocking note. I found no new defect in the new code.

Earlier findings

1. Mine, round 1 — the case names a source that does not exist (more_examples.md#S909): fixed.
   test/gen/gen_cases.py now writes the case with write_issue("split/short-conversation/tail-changed", "context-guru-909", ...). The generated JSON says "source": "issue_cases.md#context-guru-909". docs/issue_cases.md has the new section "Cases from context-guru#909" with a two-row table: the tail-changed case (read 3,000 stops at the split, +$0.01035) and the new no-effect guard case (read 3,170 passes the forwarded first difference, no effect). The table arithmetic matches the cases. A reader can now open the source.
2. Reviewer 2, round 1 — the exception books a phantom split effect on a full read: fixed.
   splitChangedSystem (internal/engine/compute.go:1645) now also requires c.fwdFD > 0 && c.usage.CacheRead <= c.ratio.upTo(c.fwd, c.fwdFD-1), with no tolerance. I traced the new guard case split/tail-unchanged-hash-differs-system/no-effect by hand: read 3,170 against upTo(fwd, fwdFD-1) = upTo(fwd, 0) = 3,000, so the new condition is false, splitChangedSystem is false, the B* raise at compute.go:832 fires, B* = messages[0] as the case pins, and split gets n = min(3,000, 3,170) − 3,170 < 0: no split effect. The s909 case keeps its effect, because its read is 3,000 = upTo(fwd, 0). The author's fix-summary states the guard case failed on 23e2ed1 with 2 effects and a 3,000-token split, so it is a real regression test.
   • One check on the reviewer's suggested fix: their formula kept the tolerance (+ 128 + CacheRead/50). For the guard case it gives 3,170 <= 3,000 + 128 + 63 = 3,191, which is true, so the phantom effect would still be booked. The author's no-tolerance comparison is the necessary choice. The stated reason ("the tolerance is the very window in which the two cases look alike") is correct.
3. Non-blocking notes, round 1 (both reviewers): all addressed.
   • The design §5.1 text now says the client diff gate is "before the end of the system section — a change in the tools section counts too", so the doc matches the gate.
   • The wording is now "B* stays where the hashes put it (none, or before the changed block)".
   • The cachesplit scan and the delivered test are shared helpers (splitItem, splitDelivered). I checked split and splitChangedSystem against them: same scan (last live cg:cachesplit: item), same delivered test (forwarded system section longer than the client one). headTTL scans a different prefix (headttl:), so its own loop is right.
   • The s909 case has no expected.boundary, and the author's answer holds: B* is none there, and compare() (test/cases/cases_test.go:177) pins a boundary only when the expected one is non-nil. The guard case pins messages[0].

What I checked

• Last review: round 1, 23e2ed1, 2026-11-05T06:26:15Z.
• New commits read: 1 (3d20a8a "fix: Stop the short-turn split exception from booking a phantom effect on a full read").
• Read in full: 2 reviews, 1 comment, 0 inline comments (from the three files).
• New items since my last review:
  • Reviewer 2's round-1 review (phantom split effect on a full read). Verified the finding and the fix; see item 2 above.
  • The author's fix-summary comment (2026-11-05T06:31:24Z). Checked each claim against the code; all hold.
  • Commit 3d20a8a. Read the full diff (internal/engine/compute.go, docs/design.md, docs/issue_cases.md, test/gen/gen_cases.py, the re-sourced s909 JSON, the new guard JSON) and the code around each change.
• Traced the new guard case r1 through phaseAndBoundary, cacheEffects and split. Numbers: previous extent 3,170; read 3,170; clientFD = 0 (system hash changed), fwdFD = 1, prevExtCli = 1. The new condition fails (3,170 > 3,000), the raise sets B* = messages[0], bstarFwd covers the whole extent, and split books nothing. The row-2 break (messages[1] to messages[0], no tokens, inBaseline) matches the expected effects, and conservation holds: 0 = 0.
• Traced the s909 case again on the new head. Its read is 3,000, so the new condition holds (3,000 <= 3,000), splitChangedSystem is true, the raise is skipped, and the split still books 3,000 write-to-read, +$0.01035. The fix did not break the case it was written for.
• Recomputed the FNV-1a cumulative body hashes of the guard case's four bodies with the generator's own algorithm (seed 14695981039346656037, cumulative across content ids, 4 bytes/token). All match the JSON, so the JSON is the generator's output for this gen_cases.py. The literals in the JSON match the generator's r909_system_hash.
• Checked the rules: no new import (R3 holds), no API change (accountant.go untouched), Compute stays deterministic with no I/O (R26). docs/design.md §5.1 now states the new condition (read stops at the split, no tolerance) and the within-tolerance-but-past-fwdFD case, so the doc and the code agree in the same PR.
• Ran gofmt -l . (nothing), go vet ./... and go test ./.... All clean and green.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com