Skip to content

fix: Judge cache liveness with the TTL the entry was written with - #47

Merged
boazdavid merged 3 commits into
mainfrom
fix/ttl-phase
Nov 4, 2026
Merged

boazdavid merged 3 commits into
mainfrom
fix/ttl-phase

Conversation

@boazdavid

Copy link
Copy Markdown
Contributor

Summary

  • The phase now uses the TTL the previous request wrote, per breakpoint. It no longer uses the TTL of the current request.
  • A billed CacheRead that covers a breakpoint prefix beats the clock at any gap.
  • Fixes two wrong results: a 20x tier error (mixed 1h/5m) and a 12.5x tier error (re-ask at a different TTL).

Fixes #40
Fixes #41

What changed

  • CacheState.EntryTTL keeps EntryFacts.TTL of the writing request. nextState sets it. CacheState.TTL is now "1h" when any breakpoint under the cached extent is 1h.
  • phaseAndBoundary splits the previous extent into parts, one for each breakpoint of the previous request. Each part lives for its own TTL. The phase is hot when any part is alive. prevExt is the end of the longest live part.
  • A ping rescue compares the entry gap with the longest TTL of the entry.
  • A billed read that covers a part (same tolerance as before) makes the phase hot at any gap. The 10 s ttlSkew is removed.
  • seededFromParent uses the parent's entry TTL.
  • Docs: design.md (5.2, CacheState, EntryFacts), spec.md (R45), cached-prefix.md, testing.md, issue_cases.md (new section 9b), doc.go.

How it was tested

Open questions

  • Changed existing tests and one case. The 10 s margin rule is gone, so these encoded the old behavior and changed:
    • clock/ttl-edge-past-margin/cold now bills no read at 11 s. The old one billed a full read and expected cold. That is the rule the issue asks to remove. The fix is right, the case was wrong.
    • Unit tests TestAReadPastTheSkewMarginStaysCold, TestAPingTooLateIsNoRescue, TestAPingThatCachedNothingIsNoRescue now bill no read where they expect cold. With a billed read the provider proves the entry was alive.
    • The check ttl-edge-read-covers-extent/hot baseline is the same usage, as before.
  • Is a read that covers only the 1h head enough to call a mixed entry hot? We say yes: the phase is hot with the head as the extent. The 5m tail is then priced as written again.
  • The gap is still measured from the start of the previous request. The issue also suggests a host fact PrevEnd. Not done here.
  • Snapshots from before this change have EntryTTL 0. The code then falls back to the current Facts.TTL, as before. No snapshot version bump.

Technical notes

  • A breakpoint with an empty TTL takes the entry TTL.
  • When no previous breakpoint ends the extent, the extent itself is a part at the entry TTL.
  • headTTL (ttlRescue) still keys on CacheState.TTL == "1h" and a gap over 5m. That is now set when any 1h breakpoint is under the extent, so a host that reports Facts.TTL 5m with a 1h head still gets the rescue effect.

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

The clock used EntryFacts.TTL of the current request for the whole
entry. This gave a wrong phase and a wrong tier.

- Keep the TTL of the writing request in CacheState.EntryTTL.
- Judge each breakpoint of the previous request with its own TTL.
  A 1h head is alive after 10 minutes. A 5m tail is not.
- Let a billed CacheRead that covers a breakpoint prefix beat the
  clock at any gap. The 10 s ttlSkew margin is removed.
- Seed a fork with the TTL of the parent entry.
- Add unit tests and four generated cases. Update the docs.
- Case clock/ttl-edge-past-margin/cold now bills no read. The old
  version billed a read and expected cold. That rule is gone.

Fixes #40
Fixes #41

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: DAVID BOAZ <DAVIDBO@il.ibm.com>

@boazdavid boazdavid left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 2 02b5a4a88dd4 blocking

Verdict: blocking

Findings

  1. internal/engine/state.go:249 — The fork seed reads only CacheState.EntryTTL, so a mixed entry cannot seed a fork while its 1h head is alive.
    • What fails: the parent writes a mixed entry (a 1h mark on messages[1], a 5m mark on messages[2], Facts.TTL 5m, extent 5,000 tokens). A fork of that thread arrives 10 minutes later. It keeps the whole extent and uses the same model, and it bills no read. seededFromParent compares the gap with EntryTTL (5m), rejects the seed, and the fork stays with no PrevPrint. The phase is then unknown and penalty_unknown is added. By the new mixed rule (design §5.2, "The phase is hot when any part is alive"), the 1h head is alive at 10 minutes, so the fork should be hot with the head as the extent.
    • Proof: state.go:249-254 gates on ps.Cache.EntryTTL only. compute.go:688-701 builds the entry parts from the parent's own breakpoints, and phaseAndBoundary returns hot with extent 3,000 for that same state in a thread (ttl_test.go TestAMixedHeadIsHotOnTheClockAlone). The same state gives unknown through a fork. The shape is the one docs/cached-prefix.md names for Claude Code (1h head, 5m tail), and ttl_test.go has no test for a fork from a mixed entry. The error only loses a saving and adds a penalty; it does not invent one.
    • Fix: gate the seed on the longest part TTL of the parent's entry, not on EntryTTL alone. This is the same rule as phaseAndBoundary, and it keeps the seed's invariant ("a seed can only give a hot phase"). Add a test: a mixed parent, a fork at 10 minutes, want hot.

Non-blocking

  • state.go:257: the seed also requires the fork to keep the parent's whole extent. With a mixed entry whose 5m tail has expired, a fork that keeps only the 1h head is not seeded. The rule is pre-existing; the mixed case makes it matter more.
  • compute.go:675: segTTL maps every string other than "1h" and "5m" to the entry TTL, with no warning. A typo in Breakpoint.TTL silently changes the lifetime of a part.
  • design.md §5.2 "Mixed TTLs" does not say which TTL the part beyond the last breakpoint has. The code gives it EntryTTL (the entrySegments comment says so). Add one sentence to the doc.

What I checked

  • Read CLAUDE.md, docs/spec.md and docs/design.md, then the full diff d31126b..02b5a4a, all 19 files, and the code around each change.
  • Read internal/engine/ttl_test.go, clock_fork_test.go and effects_test.go in full, and the new docs sections (spec R45, design §5.2, issue_cases §9b, testing).
  • Ran gofmt -l . (it printed nothing), go vet ./... and go test ./.... All clean.
  • Ran test/gen/gen_cases.py into a temp folder and diffed the output with test/cases. The committed JSON matches the generator; only cases.go and cases_test.go differ, as expected.
  • Traced the four new cases and the two changed clock cases against the table in docs/issue_cases.md §9b. The phases, boundaries and token tiers agree.
  • Checked the phase rules against the design rows: per-part liveness, the entry TTL for the "cached nothing" rows, the ping rescue gate on the longest part TTL, and the read-beats-the-clock rule (exact lifetime only, per-segment readReaches, tolerance of decision 7).
  • Did not read the other review or any PR comment before posting this one.

@boazdavid boazdavid left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 2 02b5a4a88dd4 blocking

Verdict: blocking

Findings

  1. internal/engine/compute.go:584 — a billed read does not beat the clock when only part of the entry died

    • What fails: a mixed entry (a 1h head and a 5m tail), a gap of 6 minutes, and a billed CacheRead that covers the whole previous entry. The 1h head is alive on the clock, so the first switch case runs and sets prevExt to the head. The call passes nil as the read predicate, so the read evidence for the dead 5m tail is dropped. A change in the tail is then priced after B* as a 5m write, although the provider billed those tokens as a read. The saving is overstated about 12.5 times (5m write $3.75/M against read $0.30/M). That is the unsafe direction: R45 says a doubtful figure may only lower a saving, and design §5.2 says a billed read beats the clock at any gap, for a part, not only when every part is dead.
    • Proof: I ran this scenario as a test in a scratch copy of the head (not committed). r0: messages a=2000, b=3000, c=2000; marks messages[1] "1h" and messages[2] "5m"; Facts.TTL 5m exact; a change shrinks messages[2] from 2000 to 500; usage write 5500, of which 5000 at 1h. r1 at +6m: adds d=500, usage read 5500, write 500. Result: phase=hot, Boundary messages[1], the change's Saved is {CacheWrite5m: 1500}. The read of 5500 covers the whole previous entry, so the 1500 tokens were read: the correct figure is {CacheRead: 1500} and the boundary messages[2]. The read-beats-the-clock rule runs only in the default (all parts dead) branch, at compute.go:601.
    • Fix: inside case alive:, extend the extent with the read evidence before using it, without touching alive itself: if c.usage.CacheRead > 0 { if l2, ok := liveExtent(segs, gap, func(at int) bool { return c.readReaches(prev.Forwarded, at) }); ok { live = l2 } }. I ran this shape in the scratch copy: my scenario gives {CacheRead: 1500}, and go test ./... passes in full (the ping tests fail only if alive itself is computed with the read predicate, because a read-proven hot phase then preempts the rescue branch; keep the two apart). Add a test like the scenario above, and a generated case for it.
  2. internal/engine/compute.go:1358 — the new TTL upgrade in nextState has no test

    • What fails: next.Cache.TTL is now "1h" when a 1h forwarded breakpoint lies at or before the cached extent, whatever Facts.TTL says. That string gates the ttlRescue leg in headTTL (compute.go:939). So a request with Facts.TTL 5m and a 1h mark now produces a ttlRescue on the next request where the code before this PR produced none. That is a new behavior of Compute with no test: the headttl cases (E3 to E5 in test/gen/gen_cases.py, interaction_test.go:166) all set Facts.TTL 1h, and the mixed-TTL tests in ttl_test.go (mixedHead) carry no headttl: operation, so the state's TTL string is never read there.
    • Proof: grep over internal/engine/*_test.go and test/gen/gen_cases.py: every headttl test passes facts ttl 1h; every mixed-TTL test has no operation with a headttl: ChangeID.
    • Fix: add a unit test or a generated case: r0 with Facts.TTL 5m, marks messages[1] "1h" and messages[2] "5m", and a headttl operation; r1 after more than 5 minutes reads the head; assert the ttlRescue effect appears.

Non-blocking

  • The clock branch still gates on the current request's Facts.TTL > 0 (compute.go:572), and seededFromParent does the same (state.go:235). A request whose own TTL is 0 gets phase unknown although the state holds the entry's EntryTTL. The docs do not say this. The direction is safe (unknown, penalty_unknown, a read can still prove hot).
  • seededFromParent judges the parent's entry by the single EntryTTL, not per breakpoint. A fork inside a live 1h head but past EntryTTL 5m is not seeded: unknown. Safe direction.
  • When one part is clock-alive and a ping kept a clock-dead part alive, the alive case wins and the ping gets no rescue. This under-credits the window. It matches the doc's "the entry gap is compared with the longest TTL" reading, so I do not count it a defect, but the docs could say it.
  • CacheState.EntryTTL round-trips through Snapshot/Restore for free (the whole CacheState is marshaled, persist.go:92). No explicit round-trip test exists. Low risk.

What I checked

  • Read CLAUDE.md, docs/spec.md, docs/design.md in full, and the changed parts of docs/cached-prefix.md, docs/testing.md, docs/issue_cases.md.
  • Read the full diff (19 files) and the code around each change: phaseAndBoundary, entrySegments, liveExtent, segTTL, nextState, seededFromParent, headTTL, cacheEffects, persist.go.
  • Traced the four new generated cases by hand (r0 and r1, per part): the expected phases, tiers, boundaries and baseline usages in gen_cases.py match the spec rules.
  • Ran, in this worktree: gofmt -l . (clean), go vet ./... (clean), go test ./... (all pass).
  • Ran the generator in a scratch copy of the head and compared test/cases: the committed JSON cases are in sync.
  • Proved finding 1 with a scratch test in a copy of the head (no file of this worktree changed), and checked the fix shape against the full suite.
  • Checked the docs against the code: spec R45, design §5.2 (Which TTL, Mixed TTLs, A billed read beats the clock), doc.go and issue_cases.md §9b all describe the new behavior, except the gap in finding 1.

…st part

Review round 1 of PR 47 found three faults.

- A billed read now extends the extent over a clock-dead 5m tail
  when the 1h head is alive. A tail change is a read, not a write.
- A fork is seeded by the longest part TTL of the parent entry.
- Add tests for the ttlRescue with a 1h mark under Facts.TTL 5m,
  the stored TTL string rule and the Snapshot round trip.
- Add two generated cases. Update design.md.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: DAVID BOAZ <DAVIDBO@il.ibm.com>
@boazdavid

Copy link
Copy Markdown
Contributor Author

Round 1 replies (commit 01d41bb).

Reviewer 2, finding 1 (read vs dead tail): fixed. In case alive: a billed read now extends the extent over a clock-dead part. alive stays clock-only, so the ping tests still pass. Your scenario fails on the old head (Saved CacheWrite5m 1500) and now gives CacheRead 1500, boundary messages[2]. Tests: TestABilledReadProvesADeadTailWhileTheHeadIsAlive, TestADeadTailWithNoReadIsAWrite. Case: ttl/mixed-dead-tail-read/hot-after-6-minutes.

Reviewer 1, finding 1 (fork of a mixed entry): fixed. The seed gate uses the longest part TTL (same rule as the phase). Tests: TestAForkOfAMixedEntryIsSeededWhileTheHeadIsAlive (failed before), TestAForkOfAMixedEntryPastEveryPartIsNotSeeded. Case: ttl/mixed-fork/seeded-while-head-alive.

Reviewer 2, finding 2 (nextState TTL upgrade): kept, with a test. Arithmetic: head 1255 tokens written at 1h; a 5m tail would be dead at +10m; the head read saves 1255 x (3.75 - 0.30) per M, so the ttlRescue is correct. Test: TestAOneHourMarkUnderAFiveMinuteFactGivesTheTTLRescue.

Non-blocking, all done:

  • segTTL unknown string: documented in code and design.md, tested (TestAnUnknownStoredBreakpointTTLTakesTheEntryTTL). No warning: it would break the slog-per-failed-check pattern.
  • design.md: part beyond the last breakpoint has EntryTTL; ping on a dead part gets no rescue when another part is clock-alive; the clock branch and fork seed need current Facts.TTL > 0; fork that keeps only the 1h head is not seeded (safe direction, not fixed: the seed needs the whole extent).
  • Snapshot/Restore: TestEntryTTLSurvivesSnapshotRestore.

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

@boazdavid boazdavid left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 2 01d41bb321bd blocking

Verdict: blocking

Round 1 dispositions

  • Reviewer 1, finding 1 (a fork of a mixed entry is not seeded while the 1h head is alive): fixed. state.go:253-257 now raises the seed gate to the longest part TTL, with TestAForkOfAMixedEntryIsSeededWhileTheHeadIsAlive, TestAForkOfAMixedEntryPastEveryPartIsNotSeeded and the case ttl/mixed-fork/seeded-while-head-alive. I traced all three; they pin the fix.
  • Reviewer 2, finding 1 (a billed read does not beat the clock for a dead tail under a live head): fixed. compute.go:587-592 extends the extent with the read evidence inside case alive:, and alive stays clock-only, so the ping rescue is not preempted. TestABilledReadProvesADeadTailWhileTheHeadIsAlive and TestADeadTailWithNoReadIsAWrite pin both sides. I checked the fix shape against the ping tests; they still pass.
  • Reviewer 2, finding 2 (the nextState TTL upgrade has no test): kept by the author, with a test. I checked the arithmetic and accept. The head was written at 1h (CacheWrite1h 1,255 under the extent), the gap of 10 minutes is past 5 minutes, and the billed read of the head proves it was alive. The ttlRescue of 1,255 x ($3.75 - $0.30)/M is the value of the 1h mark. TestAOneHourMarkUnderAFiveMinuteFactGivesTheTTLRescue pins it.
  • Non-blocking items from both reviews: all addressed. I verified each: segTTL documented and tested, and its comment's claim is true (badBreakpoints, compute.go:1132-1144, rejects an unknown TTL on the current request's breakpoints); design.md now states the tail-beyond-the-last-breakpoint TTL, the ping-versus-clock-alive rule, the Facts.TTL > 0 gates, and the unseeded fork that keeps only the head; TestEntryTTLSurvivesSnapshotRestore covers the round trip.

Findings

  1. docs/issue_cases.md:511 — Section 9b is stale: it says four cases pin the fix, but six cases now cite it as their source.
    • What fails: commit 01d41bb adds ttl/mixed-dead-tail-read/hot-after-6-minutes and ttl/mixed-fork/seeded-while-head-alive. Both JSON files carry "source": "issue_cases.md#40". Section 9b does not describe either case. The intro also says every case has messages a = 2,000, b = 3,000, c = 2,000 tokens, a change that shrinks b by 2,000 tokens, and a saving of 2,000 tokens. The dead-tail case shrinks c by 1,500 and saves 1,500. The fork case has no change and no op saving. A reader who follows the source link cannot find the case.
    • Proof: the two JSON files name issue_cases.md#40 in their source field. sed -n '/## 9b/,/## 10/p' docs/issue_cases.md on HEAD shows "Four cases pin the fix" and a four-row table. The diff stat of 01d41bb lists seven files; docs/issue_cases.md is not among them.
    • Fix: add the two cases to section 9b (two table rows, or a short paragraph "Review round 1 added two cases"), and soften the "Every case has ..." sentence so it covers the new shapes. Or give the two cases a different source if they must stay out of the issue catalog.

Non-blocking

  • docs/testing.md, cache-state row, does not name the new combination: a billed read extends a live 1h head over a clock-dead 5m tail at 6 minutes. The row names the mixed case and the read-beats case apart. One clause would do.
  • TestEntryTTLSurvivesSnapshotRestore: the phase leg ("hot" at +30 minutes) also passes without EntryTTL, because the parent's 1h breakpoint sets the part TTL by itself. The field check st.Cache.EntryTTL != time.Hour is the part that pins the round trip. The test is fine; the phase leg proves less than it looks like.

What I checked

  • Last review: round 1, 02b5a4a, 2026-11-04T19:59:33Z.
  • New commits read: 1 (01d41bb "fix: Let a billed read extend a live head and seed forks by the longest part").
  • Read in full: 2 reviews, 1 comment, 0 inline comments (from the three files; the inline file is empty).
  • New items since my last review: reviewer 2's round-1 review (read in full; both findings checked against the code), the author's round-1 reply (each claim checked against the diff), commit 01d41bb (full diff plus the code around each change).
  • Read the new code paths line by line: the read extension in case alive (compute.go:587-592, alive stays clock-only, max keeps the clock extent when the read proves less), the seed loop over entrySegs (state.go:253-257, the slice is built before the loop, so the raise is correct), and the entrySegs refactor (both callers guarantee a fingerprint and an extent).
  • Read all six new tests in ttl_test.go and traced their scenarios by hand: the dead-tail pair, the fork pair, the ttlRescue test (checked headTTL and badBreakpoints too), the segTTL test, and the Snapshot round trip.
  • Read the two new generated cases and their expected phases, boundaries, tiers and baselines; traced the conservation of both.
  • Ran gofmt -l . (it printed nothing), go vet ./... and go test ./.... All clean.
  • Ran test/gen/gen_cases.py into a temp folder and diffed the output with test/cases. The committed JSON matches the generator; only cases.go and cases_test.go differ, as expected.
  • Checked each new design.md paragraph against the code: the tail-beyond-the-last-breakpoint TTL, the unknown breakpoint TTL, the ping-versus-clock-alive rule, the read-extends-a-live-head rule, the fork seed rule, and the Facts.TTL > 0 gates. All agree, except the issue_cases.md gap in finding 1.

@boazdavid boazdavid left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 2 01d41bb321bd approve

Verdict: approve

Earlier findings

  1. compute.go — a billed read does not beat the clock when only part of the entry died (round 1, my finding 1): fixed.

    • The fix at compute.go:584-593 uses the shape I proposed: alive stays clock-only, and inside case alive: the extent is extended with readReaches when the usage bills a read. The max(live, l2) is redundant (the second pass includes the clock-alive parts) but harmless.
    • I checked the critical invariant myself in round 1: if alive itself were computed with the read predicate, the ping tests fail. The author kept the two apart, and go test ./... is clean in this worktree.
    • Tests pin both directions: TestABilledReadProvesADeadTailWhileTheHeadIsAlive (read covers the dead tail, want CacheRead) and TestADeadTailWithNoReadIsAWrite (head-only read, want CacheWrite5m). The generated case ttl/mixed-dead-tail-read/hot-after-6-minutes covers it end to end.
  2. compute.go — the TTL string upgrade in nextState had no test (round 1, my finding 2): fixed.

    • TestAOneHourMarkUnderAFiveMinuteFactGivesTheTTLRescue added. I replicated it in a scratch copy of the head: Facts.TTL 5m, a 1h forwarded mark, and the next request at +10m produces the ttlRescue effect, with no failed checks. The arithmetic checks out: the head is 1,255 tokens, and the rescue saves the 5m write price against the read price.
  3. state.go — the fork seed read only EntryTTL (round 1, reviewer 1's finding): fixed.

    • seededFromParent now takes the longest part TTL from entrySegs (state.go:253-257), the same rule as phaseAndBoundary. The preconditions hold: PrevPrint, Cache.Extent and the model are checked before the loop.
    • Tests: TestAForkOfAMixedEntryIsSeededWhileTheHeadIsAlive (fails before the fix, per the author) and TestAForkOfAMixedEntryPastEveryPartIsNotSeeded. Generated case: ttl/mixed-fork/seeded-while-head-alive.
  4. Fork that keeps only the 1h head is not seeded (round 1, reviewer 1, non-blocking): rejected by the author. I accept.

    • The seed keeps its pre-existing rule: the fork must keep the parent's whole extent. Not seeding is the safe direction: it can only lose a saving, not invent one. The rule is now stated in design.md.

New commit

  • entrySegments was split into entrySegments and entrySegs. The extraction is behavior-identical; seededFromParent and phaseAndBoundary share one rule now.
  • The non-blocking notes from round 1 are all addressed: segTTL unknown strings are documented and tested, the part beyond the last breakpoint has EntryTTL in code and doc, the Facts.TTL > 0 gates are documented, and TestEntryTTLSurvivesSnapshotRestore covers the snapshot round-trip.
  • The docs match the code. The new "a read extends a live head" paragraph and the fork-seed rule state the fixed behavior.
  • The commit message has the DCO sign-off, the Assisted-By line, and a conventional title.

Non-blocking

  • TestAOneHourMarkUnderAFiveMinuteFactGivesTheTTLRescue gives the client body the same 1h mark at sys(0) that the forwarded body has (copied from the shape of TestNoInteractionOnATTLRescueWithNoPing). In that exact scenario the client would also read its head at 10 minutes, so the rescue would not be earned against the client counterfactual. The test still pins the gate my round-1 finding 2 asked for, and the realistic shape (client 5m mark, forwarded 1h mark) is covered by the E3 generated case, where BP() defaults ttl="5m". If you touch this test later, set the client mark to "5m".
  • The conservation check cannot judge this either way: it sums tokens across tiers, so a write-to-read move nets zero. Not a defect of this PR; noting it because I checked it.

What I checked

  • Last review: round 1, 02b5a4a, 2026-11-04T20:02:22Z.
  • New commits read: 1 (01d41bb, "fix: Let a billed read extend a live head and seed forks by the longest part", 7 files). Read the full diff and the code around each change.
  • Read in full: 2 reviews, 1 comment, 0 inline comments.
  • New items since my last review: commit 01d41bb (diff plus the surrounding code in compute.go, state.go and ttl_test.go), the author's round-1 reply at 2026-11-04T20:05:09Z (checked each reply against the code), and the two new generated cases plus the changed BP() helper in test/gen/gen_cases.py.
  • Ran, in this worktree: gofmt -l . (clean), go vet ./... (clean), go test ./... (all pass).
  • Ran the generator in a scratch copy of the head and compared test/cases: the committed JSON cases are in sync. Removed the scratch copy after.
  • Replicated the new ttlRescue unit test in the scratch copy with logging: the effect fires, and no check fails.
  • Hand-traced both new generated cases (ttl/mixed-dead-tail-read, ttl/mixed-fork): the expected phases, boundaries, baseline usages and saved tiers match the spec rules, including the 6-minute gap, the read of 5,500 covering the dead 5m tail, and the fork seeded by its 1h head.
  • Checked the fix shape against the ping tests and the keep-alive generated cases: alive stays clock-only, so the ping rescue branch keeps its place.

Section 9b named four cases and one shape. Add the mixed dead-tail
read case and the mixed fork case with their arithmetic.
Name the combination in the testing.md cache-state row.

Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com>
Signed-off-by: DAVID BOAZ <DAVIDBO@il.ibm.com>
@boazdavid

Copy link
Copy Markdown
Contributor Author

Round 2 reply (commit ef373eb): fixed. docs/issue_cases.md section 9b now says six cases, limits the a/b/c shape to the first four, and describes the two review cases with their arithmetic (1,500 x $0.30/M = $0.00045 read vs 1,500 x $3.75/M = $0.005625 write; fork seeded, no saving). docs/testing.md cache-state row names the combination. The snapshot test comment now says the field check pins the round trip.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant