[boazdavid]

Summary

Refs #44. Items 1 to 5 and the int32 guard are done. Item 6 (model ids) is skipped.

What changed

• Size above the body. A rewrite whose SizeBefore.Bytes (or a span's) is above the client body size is not priced. It goes to Unpriced with the new reason size_range, and check 13 reports it. Other operations are priced as before. This is the one narrow exception to "a failed check does not stop pricing". A 9.2e18-token saving cannot reach the totals now. The sum check still runs.
• Usage cap. Any usage tier above 2^40 gives bad_usage. The sum of three tiers cannot wrap.
• Call figures. A spend Call with negative Output, or CostUSD negative, NaN or Inf, or a bad Usage, goes to Unpriced with bad_usage and a bad_usage check. This works in Compute and in ComputeStandalone. Valid calls are unchanged.
• Compute doc. The comment says a second Compute prices against stored state, that a retry double-advances it, and that Preview exists. design.md says the same.
• int32 bytes. NewBody saturates a position size at MaxInt32 (no wrap).
• Docs: design.md checks 8 and 13, the failure-table row, and the reason lists. New exported const ReasonSizeRange in accountant.go.

How it was tested

• New tests in internal/engine/validation_inputs_test.go. They failed to build before the fix, then passed.

• gofmt -l . prints nothing. go vet ./... and go test ./... pass.

• One existing test used a span sum of 999 on a 200-byte body. It now uses 150, so it still tests the spans check.

• New generated cases (from test/gen/gen_cases.py, listed in docs/issue_cases.md section 9b): check/size-above-client-body/unpriced (expected values computed by hand in the case note: 5,000 tokens x $3.75/M = $0.01875 for the honest op), check/bad-call-negative-output/unpriced, check/bad-call-negative-cost/unpriced.

• Not expressible as cases: usage above the 2^40 cap or sums that wrap (a case bills what its forwarded body holds), and NaN/Inf CostUSD (JSON has no NaN). The Go tests in validation_inputs_test.go keep them.

Open questions

• Model ids (item 6) skipped. A host hook needs a change in the model comparison in compute.go (phase logic) and in state.go. Other branches edit those files. It is not small. The context-guru host can canonicalise Model before it passes it.
• The cap 2^40 and the size rule are choices. Say if you want other values.

Technical notes

• The size rule compares only SizeBefore.Bytes, because SizeAfter may be larger than the client body when an operator adds content.
• Check 13 applies to reported rewrites only. Active replays are stored from earlier valid reports.

Refs #44

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 2	be1cbf1ef9dc	blocking

Verdict: blocking

Findings

1. internal/engine/compute.go:253 — a size_range report deletes the active change that shares its ChangeID.

   • What fails: a thread holds an active change c1. A later request reports c1 again (the observe-to-enforce switch, §4.6) with SizeBefore.Bytes above the client body. The report is a bug, so check 13 should only unprice it. Instead the thread loses the change for good.
   • Proof: isChange (compute.go:253) tests ChangeID, kind and negative only. It does not test tooBig. So withReported (state.go:191) puts c1 into reported. The loop at state.go:199 then drops the active c1 from out, because its ChangeID is reported. The replacement comes from activeOps (compute.go:1328), which skips items with it.bad != "" — and items (compute.go:223) marks the too-big report size_range. So out holds neither: the active op is dropped and the bad report is not stored. From the next request on, the change is gone from ThreadState.Active while still in the body. Every later request fails the sum check and loses the saving, and "a lost state does not heal by itself" (§4.6). A report with a negative size does not do this: isChange excludes it, so the active op stays. The comment on isChange ("A malformed report is not a change, so it must not replace an active one. items and EndRequest both use it, so they cannot disagree", compute.go:250-252) states the rule that this case now breaks.
   • Fix: add the tooBig condition to isChange (both call sites are methods on *Request, so the client body is at hand). A too-big report then replaces nothing, and the active op is priced as usual.

2. internal/engine/compute.go:91 — maxTierTokens = 1 << 40 breaks the build on 32-bit platforms.

   • What fails: GOOS=linux GOARCH=386 go build ./... does not compile.
   • Proof: the build prints maxTierTokens (untyped int constant 1099511627776) overflows int at compute.go:96 and compute.go:110. On a 32-bit int, the constant is not representable, so every comparison against it is a compile error. Both lines are added by this PR; the base has no such constant. Any host that builds accountant for 386 or 32-bit arm now fails. The design's own reason for the cap ("keeps the sum of the tiers from wrapping an int", §6.5 check 8) also fails there: a 32-bit int cannot hold 2^40 at all.
   • Fix: keep the cap as an int64 and compare int64(n) > maxTierTokens, or lower the cap to a value that fits an int32 and update design.md §6.5 check 8.

3. test/cases/check-size-above-client-body-unpriced.json:3 — the case description states two wrong numbers.

   • What fails: a reader who checks the description against the case finds different figures.
   • Proof: the description says "a client body of 56,000 bytes". The request that runs the check (r1) has a client body of 80,000 bytes: 8,000 + 4 x 12,000 + 24,000 (56,000 is the r0 body). It also says "no 9.2e18-token saving reaches the totals". The op reports before and after both at 2e12 tokens, so without the check it would save 0 tokens, not 9.2e18. The 9.2e18 figure (about math.MaxInt) belongs to the Go test TestOpLargerThanClientBodyIsNotPriced, not to this case. The same two figures are in test/gen/gen_cases.py (i44_size_range) and docs/issue_cases.md §9b. The expected figures of the case are correct; only the text is wrong.
   • Fix: correct the two numbers in the i44_size_range text in gen_cases.py, run go generate ./test/cases, and correct docs/issue_cases.md §9b.

Non-blocking

• Check 13 bounds SizeBefore only, as the design says. A bogus SizeAfter can still put unbounded Caused tokens into the totals. The sum and conservation checks catch it, but they do not stop pricing. Worth a line in the design if this gap is intended.
• When a request fails check 8, 9 or 10 and an operation is also size_range, checkUsage (compute.go:1126) appends the per-operation check before the request-level one, so Checks reads [size_range, bad_usage]. The design's "the first check that fails, in that order" names the request-level reasons only. Cosmetic.
• The change in effects_test.go (999 to 150) is right: 999 is above the 200-byte client body, so the new check would fire before the spans check and hide it.

What I checked

• Read CLAUDE.md, docs/spec.md and docs/design.md in full, the full diff of both commits, and the code around each change (compute.go, state.go, fingerprint.go, result.go, the tests, the generator).
• Ran go vet ./... and go test ./... in my folder. All three pass.
• Verified the three new JSON cases against gen_cases.py: I recomputed the FNV position hashes with the generator's algorithm (they match), and checked the usage, baseline, boundary, expected ops, unpriced entries and checks against the generator helpers (sim, rewrite, eop, ping_step).
• Checked the new code against design.md §6.5 checks 8 and 13, §8, and the errors table, and against the isChange rule in §4.6. Traced items, withReported and activeOps for the re-report case (finding 1).
• Ran GOOS=linux GOARCH=386 go build ./... to test the 32-bit build (finding 2).
• Stdlib only: the new imports are math (compute.go, fingerprint.go). accountant.go re-exports ReasonSizeRange. boundary_test.go still passes.
• Round 1: I did not read the PR conversation or the other review.

[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 2	be1cbf1ef9dc	blocking

Verdict: blocking

The validation itself is sound: the caps, the NaN and Inf tests, and the new cases are correct. go vet ./..., go test ./... and gofmt -l . are clean. But check 13 has three defects. One loses a stored change, one unprices a correct report, and one leaves the after-size open.

Findings

1. internal/engine/compute.go:253 — a size_range report replaces and drops an active change

   • What fails: request 0 stores change X as active. Request 1 reports the same ChangeID with SizeBefore.Bytes above the client body. isChange returns true, because it excludes only negative(op). So withReported removes the active X from the state (state.go:191-203), and activeOps drops the bad report too (compute.go:1328). The change leaves the ThreadState. Its replayed saving is lost on this request and on every later one, and the sum check fails on each of them.
   • Proof: isChange (compute.go:253-255) tests !negative(op) but not tooBig. Its own comment states the rule: "A malformed report is not a change, so it must not replace an active one (§4.6)". Check 13 (design.md §6.5) names a size above the client body "a bug", so the report is malformed. A negative-size report with the same ChangeID does not replace the active change; a too-big one does. The two malformed cases must behave the same way.
   • Fix: add the tooBig test to isChange. Pass the client bytes in, or share one malformed(op, clientBytes) helper between isChange, items and negative.

2. internal/engine/compute.go:119 — check 13 bounds a downstream operator by the client body, but the operator receives the intermediate body

   • What fails: an upstream operator inserts content (a MessageSplice with removed: 0, for example a retrieval plugin that adds 100 KB to a 30 KB client body). A rewrite that runs after it covers the inserted content, so its SizeBefore.Bytes is the size of the body it received. That body is larger than the client body. tooBig marks the report size_range, and a correct report is not priced.
   • Proof: reported sizes are in the operator's own pipeline space. items undoes the splices of the upstream items to map positions back to client space (compute.go:230-246), and the sawOriginal comment says an operator that ran after a reducer "received fewer bytes than the client sent" (compute.go:514-517). The received body differs from the client body in both directions. Case H2 shows a downstream operator that reports in the summary's output space. The design's premise for check 13, "The operator cannot have received more bytes than the client sent" (design.md §6.5, check 13), holds only for the first operator of the pipeline.
   • Fix: bound the before-size by the largest body the operator could have received, not by the client body alone. A sound bound is the client total plus the sum of max(0, SizeAfter.Bytes - SizeBefore.Bytes) over the request's rewrite operations. Say the bound in design.md.

3. internal/engine/compute.go:123 — SizeAfter is not validated, so an impossible after-size still reaches the totals

   • What fails: a rewrite reports SizeAfter.Bytes = 8e12 with Tokens = 1.6e13. That is 2 tokens a byte, so check 12 (size_tokens) passes. tooBig reads only SizeBefore fields, so nothing stops it. price accepts the tokens as exact (compute.go:343-351) and adds about 1.6e13 tokens to PricedOp.Caused. The totals hold a figure that cannot be real. The direction is the safe one (it lowers the net saving), but the figure is wrong and unbounded, and the same holds for each span's SizeAfter.
   • Proof: tooBig (compute.go:119-132) tests op.SizeBefore.Bytes and each s.SizeBefore.Bytes only. plausible (compute.go:297-303) bounds tokens by bytes, not bytes by the body. Design.md check 13 gives the reason for the check: "a size above the whole body is wrong in every case, and its tokens would otherwise reach the totals unbounded". That reason applies to the after-size too.
   • Fix: extend check 13 to SizeAfter.Bytes, op level and spans. Use the bound from finding 2, because a downstream operator's output can also be larger than the final forwarded body.

Non-blocking

• clamp32 (fingerprint.go:65, fingerprint.go:181-187) changes Compute for a position over 2 GiB: the byte count now saturates instead of wrapping negative. No doc describes this (CLAUDE.md rule 5). Add one line to design.md.
• Active operations are never re-checked with tooBig against a smaller client body (a compaction). The sum check flags the result later. Consider whether check 13 should say this.
• The size_range detail (compute.go:1131-1133) prints the op-level SizeBefore and SizeAfter even when a span triggered the failure. The numbers can then look in range. Print the span that failed.
• Facts.PingUsage is not validated by check 8. A negative ping usage is handled safely today (pingRead returns nil, compute.go:599), but the check does not name it.

What I checked

• CLAUDE.md, docs/spec.md, docs/design.md (§4.5, §4.6, §6.5, D14) and docs/issue_cases.md.
• The full diff d31126b..HEAD, both commits, and the code around each change: compute.go, state.go, fingerprint.go, body.go, result.go, accountant.go.
• The three new JSON cases against test/gen/gen_cases.py (i44): ids, bodies, expected figures. The hand figures in the descriptions match (5,000 x $3.75/M = $0.01875; 56,000-byte client body).
• The new Go tests in internal/engine/validation_inputs_test.go, and the changed effects_test.go and api_test.go.
• go vet ./... and go test ./...: both clean. gofmt -l . prints nothing.
• Live tests were not run. I read the code only.

[boazdavid]

Round 1 replies. Fixed in 7cd5f3c.

1. Fixed. isChange now uses a shared malformed(op, limits) (negative or out of bounds), so a size_range report replaces no active change. Test: TestSizeRangeReportKeepsActiveChange (it failed with the old isChange). Case: check/size-above-client-body/keeps-active-change.
2. Fixed. maxTierTokens is an int64 constant and the code compares int64(n). GOOS=linux GOARCH=386 go build ./... and go vet pass. Tests that need 64-bit values skip on 32-bit.
3. Fixed. Bound for SizeBefore: client total plus the sum of max(0, After-Before) over the rewrites. A report that fails a bound adds to no sum (found by repetition), so a bogus report cannot raise its own bound. Test TestSizeRangeBoundIncludesInsertedBytes, case check/size-after-inserting-operator/priced. design.md check 13 states it.
4. Fixed. SizeAfter is bounded by the forwarded total plus the sum of max(0, Before-After), at op level and for each span. Test TestSizeAfterAboveLimitIsNotPriced.
5. Fixed. The case text now says 80,000 client bytes and that the report saves 0 without the check; the 9.2e18 figure is only in the Go test. Regenerated; docs/issue_cases.md section 9b updated.
6. Done: design.md line for the clamp32 change; the detail prints the failing span's sizes and both bounds; design.md notes active ops are not re-checked (the sum check flags it). Facts.PingUsage: one line in check 8 says it is not checked (not validated, skipped by design; ping reads are handled safely).
   Non-blocking, check order in Checks ([size_range, bad_usage]): not changed, cosmetic.

[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 2	7cd5f3ce0199	blocking

Verdict: blocking

The commit 7cd5f3c fixes all six round-1 findings, and the fixes are correct where I could trace them. go vet ./..., go test ./... and gofmt -l . are clean. Two defects remain in the new bound code of check 13. Both are narrow, but one contradicts design.md and the other contradicts the fix for round-1 finding 1.

Earlier findings

• My finding 1 (a size_range report replaces an active change): fixed. isChange now uses malformed(op, l) at both call sites (compute.go:346, state.go:191). TestSizeRangeReportKeepsActiveChange proves the active change stays priced and stays in the state, and the case check/size-above-client-body/keeps-active-change pins it.
• My finding 2 (check 13 bounds a downstream operator by the client body): fixed. The before bound is now the client total plus the sum of max(0, After - Before) over the rewrites (compute.go:135-198). I checked the bound is sound: the body an operator receives is the client body plus the net change of the operators before it, and that net change is at most the sum of the positive expansions. TestSizeRangeBoundIncludesInsertedBytes and the case check/size-after-inserting-operator/priced cover it. The repetition that drops a failing report from the sums is deterministic and terminates.
• My finding 3 (SizeAfter not validated): fixed. The after bound is the forwarded total plus the sum of max(0, Before - After) (compute.go:124-127). I checked the soundness: an operator's output sits in the body after it, which is the forwarded body plus what later operators removed. Op level and spans are both checked (compute.go:214-221). TestSizeAfterAboveLimitIsNotPriced covers both.
• Reviewer 2, finding 1 (same as my finding 1): fixed, see above.
• Reviewer 2, finding 2 (the maxTierTokens constant breaks the 32-bit build): fixed. The constant is now int64 and every comparison converts (int64(n) > maxTierTokens, compute.go:95, compute.go:110). The tests that need 64-bit values skip on a 32-bit int. I verified this by reading; I did not run a 32-bit build.
• Reviewer 2, finding 3 (wrong numbers in the case description): fixed. The description now says 80,000 client bytes and 60,000 forwarded bytes, which match the r1 bodies (8,000 + 4 x 12,000 + 24,000 and 8,000 + 4 x 12,000 + 4,000), and it says the report would save 0 tokens without the check, which matches the equal before and after. docs/issue_cases.md section 9b matches.
• My round-1 non-blocking items: all four are done (clamp32 line in design.md, the active-ops note, the failing span in the detail, the PingUsage note). Reviewer 2's check-order note was declined as cosmetic; I agree.

Findings

1. internal/engine/compute.go:212 — the DryRun after bound contradicts design.md and unprices a honest observe-mode report

   • What fails: a DryRun rewrite reports what it would do, and nothing was sent (R12). An observe-mode retrieval on a 32,000-byte body reports SizeBefore 0 and SizeAfter 100,000: it would insert 100,000 bytes. over tests a DryRun's SizeAfter against l.before (32,000 plus real insertions). 100,000 is above it, so the report goes to Unpriced with size_range. The "would have saved" figure is lost, although the report is honest. A DryRun is priced by design (state_test.go:128: "a DryRun is priced as 'would have saved'").
   • Proof: compute.go:212 reads (op.DryRun && a.Bytes > l.before). Design.md §6.5 check 13 says SizeAfter.Bytes is at most "the forwarded body total plus the sum of max(0, SizeBefore.Bytes - SizeAfter.Bytes)", with no DryRun exception. So the code and the doc disagree on the DryRun after bound, in both directions: a size that passes the doc's rule can fail the code's, and the reverse. A DryRun adds to neither sum (compute.go:171), so nothing bounds its hypothetical after size in the sums; only reductions stay under l.before for free, which means the check bites exactly on expansions, the honest case.
   • Fix: for a DryRun, check only SizeBefore (it received a real body). Leave its hypothetical SizeAfter to check 12 (tokens per byte), because the figure never enters the totals (R12). Write the rule in design.md check 13.

2. internal/engine/compute.go:141 — a malformed re-report removes the active change from the bound sums

   • What fails: an active change R inserted 100,000 bytes and is still applied. A request re-reports R with a too-big size and also reports a honest downstream rewrite D, whose SizeBefore covers the insertion (say 130,000 bytes). The re-report puts R into reported (compute.go:141-146 tests only ChangeID and kind). So R leaves es (compute.go:153-157) and adds to neither bound. The bad re-report itself is dropped from the sums by the repetition. l.before falls back to the client total, 32,000. D's 130,000 is above it, so D is unpriced with size_range, although its report is honest and the insertion is in the body.
   • Proof: items and withReported use isChange(op, l), which excludes malformed reports, so the active R stays applied and priced (the round-1 fix). But limits builds its reported map with a weaker test, so the same malformed report that replaces nothing still removes R from the sums. The two rules disagree. The trace above needs numbers only from code that exists: client 32,000 bytes, R with SizeBefore 0 and SizeAfter 100,000, a re-report of R with SizeBefore 8e12, D with SizeBefore 130,000.
   • Fix: apply the same rule as items: an active operation leaves the sums only when a report that is not malformed replaces it. Build the map after the bounds settle, or keep the active entry in es and drop it only when a well-formed replacement survives the repetition.

Non-blocking

• Design.md check 8 says "The cap keeps the sum of the tiers from wrapping an int (on a 32-bit int no count can reach it)". On a 32-bit int the cap never fires, so the sum of three tiers can still wrap. The outcome is safe (the wrapped ratio fails check 9 and nothing is priced), but the sentence states more than the code does.
• TestSizeAfterAboveLimitIsNotPriced has the same 32-bit skip twice (validation_inputs_test.go:143 and validation_inputs_test.go:157). The second is dead code.

What I checked

• Last review: round 1, be1cbf1, 2026-11-04T19:53:14Z.
• New commits read: 1 (7cd5f3c "fix: Bound size_range by the pipeline, keep active changes").
• Read in full: 2 reviews, 1 comment, 0 inline comments (from the three files .review-in-reviews.txt, .review-in-comments.txt, .review-in-inline.txt).
• New items since my last review: the commit 7cd5f3c (full diff and the code around each change: compute.go, state.go, validation_inputs_test.go, gen_cases.py, design.md, issue_cases.md); the author's round-1 reply comment (checked each of its six claims against the code and tests); the two new JSON cases (keeps-active-change, size-after-inserting-operator-priced), which I checked against the generator and recomputed by hand (132,000 = 32,000 + 100,000; 5,000 x $0.30/M = $0.00150; net -5,000 x $3.75/M = -$0.01875); the corrected description of check/size-above-client-body/unpriced.
• The bound soundness arguments for findings 2 and 3 of round 1, by trace (see "Earlier findings").
• go vet ./... and go test ./...: both clean. gofmt -l . prints nothing.
• Live tests and the 32-bit build were not run. I read the code only.

[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 2	7cd5f3ce0199	blocking

Verdict: blocking

The round-1 findings are all fixed and the fixes are correct; I verified each one. But the new limits design adds two false-positive paths in check 13. One unprices an honest observe-mode report. One lets a bogus re-report unprice an honest downstream report.

Findings

1. internal/engine/compute.go:212 — the DryRun after bound unprices an honest observe-mode report.

   • What fails: an observe-mode operator computes an insertion larger than the client body. Its DryRun report goes to Unpriced with size_range, although the report is correct.
   • Proof: the over closure tests (op.DryRun && a.Bytes > l.before) (compute.go:212). A DryRun adds to neither sum (compute.go:171), so l.before holds only the client body and the applied insertions. Trace: client body 2,000 bytes; observe-mode retrieval computes an insertion of 50,000 bytes; report: DryRun rewrite, SizeBefore 0 bytes, SizeAfter 50,000. Then l = {2000, 2000} and over(0, 50000) fires on the DryRun arm: 50,000 > 2,000. items marks the report size_range (compute.go:318), so it is priced as nothing, and isChange rejects it (compute.go:349), so it is not stored. Design §8 promises the opposite: on_error: observe gives "DryRun = true. Priced and shown as 'would have saved', never in totals" (design.md:1342). A computed insertion is the cost side of the same rule (R33: an injected tool declaration, a restore). The rationale of check 13 does not apply: a DryRun's tokens never reach the totals (R12, design.md:566), so nothing is unbounded. The rule is also not in the docs: design.md:1263-1270 names two bounds and says only that a DryRun adds to no sum. No test covers it: no DryRun appears in validation_inputs_test.go or in test/gen/gen_cases.py.
   • Fix: for a DryRun, check SizeBefore.Bytes only. The input it read is real; the output is hypothetical. Or bound the after by l.after instead. Say the chosen rule in design.md check 13. Add a test with a DryRun insertion.

2. internal/engine/compute.go:141 — a malformed re-report lowers the bound for the other operations.

   • What fails: an operator re-reports an active change with a bogus size. The active change stays priced (the round-1 fix works), but an honest downstream report on the same request becomes size_range, is unpriced, and is not stored.
   • Proof: limits builds its reported map with a raw test, ChangeID plus kind only (compute.go:141-146). items and withReported use isChange, which also rejects malformed reports (compute.go:299-304, state.go:189-195). The two maps disagree for a malformed re-report. Trace: request 0 stores an active insertion ci (SizeBefore 0, SizeAfter 100,000; client body 32,000). Request 1 reports ci again with SizeBefore.Bytes 8e12, plus a downstream rewrite o2 with before 100,000 and after 20,000. In limits, the raw map marks ci reported, so the active ci is left out of es (compute.go:153-157) and its +100,000 never raises l.before. The drop loop then drops both the bogus re-report and o2 (100,000 > 32,000), and the fixed point returns l = {32000, fwd}. In items, o2 fails tooBig, so it goes to Unpriced (compute.go:318) and isChange(o2) is false, so the honest change is not stored either (state.go:191). The sum check also fails: the active ci is in the sum, o2 is not. The comment on limits states the goal: "one bogus report must not raise the bound for itself or for the others" (compute.go:129-130). This defect is the inverse: one bogus report lowers the bound for the others. A negative re-report triggers the same path. TestSizeRangeReportKeepsActiveChange does not catch it: its active change is a reduction, and no other reported op depends on the bound.
   • Fix: build the reported map in limits with the same rule as items. A negative or too-big re-report replaces nothing. Simplest: exclude negative re-reports from the map outright, and keep an active entry in es when its replacing report is dropped by the fixed point. Add a test: an active insertion, a bogus re-report, an honest downstream op; the downstream op must stay priced.

Earlier findings

My round-1 findings:

1. Active change deleted by a size_range report — Fixed. isChange now uses malformed (compute.go:227, 348-350), and withReported passes r.limits(s) (state.go:190-194). Verified by hand and by TestSizeRangeReportKeepsActiveChange plus the case check/size-above-client-body/keeps-active-change. See finding 2: one path still diverges from this rule.
2. maxTierTokens = 1 << 40 breaks the 32-bit build — Fixed. The constant is int64 and the comparisons use int64(n) (compute.go:91, 96, 110). Verified: GOOS=linux GOARCH=386 go build ./... and GOOS=linux GOARCH=arm go vet ./... are clean. The 64-bit tests skip on 32-bit.
3. Wrong numbers in the case description — Fixed. The text now says 80,000 client bytes and that the report saves 0 tokens without the check. The 9.2e18 figure stays in the Go test. gen_cases.py, the JSON and docs/issue_cases.md §9b agree.

Reviewer 1's round-1 findings (fixes in 7cd5f3c; I verified each, and add no new proof of my own):

1. Same as my finding 1 — Fixed, same code.
2. Downstream operator bounded by the client body — Fixed. l.before is the client total plus the sum of max(0, After-Before) over the rewrites (compute.go:120-123, 174-186). The bound is sound: an operator's input is at most the client body plus every insertion. TestSizeRangeBoundIncludesInsertedBytes and the case check/size-after-inserting-operator/priced prove both sides (90,000 priced; 108,001 flagged).
3. SizeAfter unbounded — Fixed. l.after is the forwarded total plus the sum of max(0, Before-After), at op level and per span (compute.go:124-127, 211-221). Verified TestSizeAfterAboveLimitIsNotPriced: 8e12 bytes at 1.6e13 tokens (2 tokens a byte, so check 12 passes) is now size_range.

Author rejections: none. The check-order item (Checks reads [size_range, bad_usage]) stays unchanged by choice, cosmetic. Accepted.

Non-blocking

• TestSizeAfterAboveLimitIsNotPriced runs the same strconv.IntSize < 64 skip twice (validation_inputs_test.go:161-163 and 170-172). The second is dead code.
• The fixed point terminates: each pass drops at least one entry, and e.out sticks. The sums saturate (compute.go:161-166). I checked the overflow paths; none.
• Check 13 has no DryRun coverage at all, in the Go tests or in the generated cases (see finding 1).

What I checked

• Last review: round 1, be1cbf1, 2026-11-04T19:52:48Z
• New commits read: 1 (7cd5f3c "fix: Bound size_range by the pipeline, keep active changes")
• Read in full: 2 reviews, 1 comment, 0 inline comments (from the three files)
• New items since my last review:
  • Commit 7cd5f3c: read the full diff (be1cbf1..7cd5f3c) and the code around each change: compute.go, state.go, validation_inputs_test.go, gen_cases.py, both new JSON cases, the updated case, design.md check 8 and check 13, issue_cases.md §9b.
  • Comment by boazdavid at 2026-11-04T20:01:44Z (round-1 replies): read in full; each claim is checked above.
  • Reviewer 1's round-1 review: read in full this round; I did not read it before my round-1 post.
• Ran go vet ./... and go test ./...: clean.
• Ran GOOS=linux GOARCH=386 go build ./... and GOOS=linux GOARCH=arm go vet ./...: clean.
• Verified both new JSON cases against gen_cases.py: I recomputed the FNV position hashes with the generator's algorithm; they match.
• Traced limits, bigSize, malformed, isChange, items, withReported and activeOps by hand for: the keeps-active case, the insertion-bound case, the DryRun insertion (finding 1), and the malformed re-report (finding 2).
• Live tests were not run. I read the code only.

[boazdavid]

Round 2 replies. Fixed in e1fb111.

1. DryRun: fixed. A DryRun operation is bounded by SizeBefore only. SizeAfter is left to check 12. design.md check 13 states it. Test TestDryRunInsertionStaysPriced (fails on 7cd5f3c). No generated case: an expected op in the case format has no dryRun field, so the harness conservation check would count the hypothetical cost. Noted in docs/issue_cases.md.
2. Re-report: fixed. In limits, an active operation leaves the sums only while a well-formed report (same rule as isChange: ChangeID, kind, not negative, within bounds after repetition) replaces it. Test TestMalformedReReportKeepsInsertionInBound (fails on 7cd5f3c with o2 size_range). Case check/malformed-rereport-keeps-insertion/priced, hand figure -5,000 x $3.75/M = -$0.01875.
3. Dead 32-bit skip removed. design.md check 8 now says: on 64-bit the cap keeps the sum from wrapping; on 32-bit the cap cannot be reached and a wrapped sum gives a ratio that fails check 9, so nothing is priced.

[boazdavid]

Merged origin/main (#46, #47) into the branch with a merge commit (no force-push). Round-2 head e1fb111 stays reachable.

Conflicts:

• design.md: one reason list; it now has both media_tokens and size_range.
• api_test.go: the reason list has both ReasonMediaTokens and ReasonSizeRange.
• gen_cases.py: both sets of generators kept (c42..c43 and i44). No case id collided.
• issue_cases.md: main's section 9b (Mixed 1h/5m breakpoints: one EntryFacts.TTL gives the wrong phase and tier (billed CacheRead is ignored) #40/Phase uses the TTL of the current request, not of the entry that was written (12.5x tier error on a re-ask at 1h) #41) kept; mine is now 9c.
• JSON cases regenerated with go generate ./test/cases.

Interactions I traced:

• Fix: Search extent in log time and price media shapes and large images #46 media_tokens: check 13 sets it.bad before price runs, so a size_range op never reaches mediaUnpriced/leaveOutOfConservation. A report that is both over the image and over the bound is size_range. The honest op next to it stays priced. New test TestSizeRangeWinsOverMediaTokens.
• fix: Judge cache liveness with the TTL the entry was written with #47 per-breakpoint TTL phase: limits and check 13 read only sizes and Forwarded byte totals, never TTL or phase. Bad ops are already skipped by the phase code as for negative sizes. I found no interaction and added no TTL test.

[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	3 of 5	b2fda5cb0cb9	approve

Verdict: approve

The commit e1fb111 fixes both round-2 findings, and the fixes are correct. I traced each fix by hand. The merge b2fda5c brings main (#46 media shapes, #47 TTL phase) into the branch. The conflict resolutions are correct, and check 13 keeps its priority over the new media_tokens rule. I found no new defect. go vet ./..., go test ./... and gofmt -l . are clean on b2fda5c.

Earlier findings

• My round-2 finding 1 (the DryRun after bound contradicts design.md and unprices an honest observe-mode report): fixed. over now tests b.Bytes > l.before || (!op.DryRun && a.Bytes > l.after) (compute.go:218). A DryRun is bounded by SizeBefore only. Design.md check 13 states the rule with its reason: the SizeAfter is a guess that never reaches the totals (R12), so only check 12 looks at it. TestDryRunInsertionStaysPriced (validation_inputs_test.go:180) covers both sides: a 50,000-byte DryRun insertion on a 2,000-byte client body stays priced, and a 50,000-byte DryRun SizeBefore is size_range. The author gave a reason for no JSON case: the case format has no dryRun field on an expected op, so the harness conservation check would count the hypothetical cost. The reason is correct, and docs/issue_cases.md section 9c records it.
• My round-2 finding 2 (a malformed re-report removes the active change from the bound sums): fixed. limits now puts every active operation into es, and an active operation leaves the sums only while replaced(e) says a well-formed report replaces it (compute.go:157-165, compute.go:176). replaced applies the same rule as isChange: the same non-empty ChangeID, kind rewrite or setting, not negative, and inside the bounds after the repetition. I traced the fixed point on the new case: round 1 skips the active insertion because the bad re-report is not dropped yet, and drops the bad re-report (8e12 bytes above 6,000); round 2 counts the active insertion again, so the bound is 106,000, and no report drops. The result matches isChange at the fixed point, so limits and items cannot disagree. TestMalformedReReportKeepsInsertionInBound (validation_inputs_test.go:205) and the case check/malformed-rereport-keeps-insertion-priced pin it.
• Reviewer 2, round-2 findings 1 and 2 (the same two defects): fixed, see above.
• My round-2 non-blocking items: both done. The dead 32-bit skip in TestSizeAfterAboveLimitIsNotPriced is gone. Design.md check 8 now says that on a 32-bit int the cap cannot be reached and a wrapped sum fails check 9, so nothing is priced (design.md:1281).
• Round-1 findings (mine: the active change replaced by a size_range report, the client-body bound on a downstream operator, the unvalidated SizeAfter; reviewer 2: the 32-bit build break, the wrong numbers in a case description): all stay fixed at b2fda5c. I re-read isChange, items, tooBig and badCall at the merged head.
• The author rejected no finding. Every finding from rounds 1 and 2 was fixed.

Findings

None. I checked e1fb111 and b2fda5c as a first review and found no new defect.

The merge b2fda5c

• Conflict resolutions: design.md has one reason list with both media_tokens and size_range (design.md:1172); api_test.go lists both ReasonMediaTokens and ReasonSizeRange (api_test.go:82); gen_cases.py keeps both generator sets and no case id collided; issue_cases.md keeps main's section 9b and moves this PR's cases to 9c. All correct.
• Interaction with #46 (media_tokens): price tests it.bad != "" before mediaUnpriced (compute.go:1335, compute.go:1343), and items sets it.bad = ReasonSizeRange before price runs (compute.go:324). So a report that is both over the bound and over the media price is size_range, and it never reaches mediaUnpriced or leaveOutOfConservation. TestSizeRangeWinsOverMediaTokens (validation_inputs_test.go:243) pins it: a 50,000,000-byte report over a 2 MB image is size_range, and the honest operation next to it stays priced. The author's claim holds.
• Interaction with #47 (TTL phase): limits, tooBig, items and checkUsage read only sizes, ChangeID, kind and body byte totals. They read no TTL, phase or extent. The new EntryTTL field changes stored state only. I found no interaction. The author's claim holds.
• The PR's net diff against merged main (5fa1986..b2fda5c) contains only the reviewed validation code, its tests and cases, and small doc comments. The merge changed none of the PR's logic.

Non-blocking

• replaced scans es for each active operation, and the repetition runs at most once per reported operation. The worst case is cubic in the number of operations of one request. A request holds a few operations, so this is not a problem. No change needed.

What I checked

• Last review: round 2, 7cd5f3c, 2026-11-04T20:09:25Z.
• New commits read: 2 (e1fb111 "fix: Bound DryRun by SizeBefore only, keep insertions in bounds", b2fda5c "merge: Bring main into fix/validate-inputs"). The merge brings in main's #46 and #47 (5829b8e, 5fa1986 and their commits); I read their net effect as a diff against e1fb111, not as a re-review of main.
• Read in full: 4 reviews, 3 comments, 0 inline comments (from the three files .review-in-reviews.txt, .review-in-comments.txt, .review-in-inline.txt).
• New items since my last review: the full diff of e1fb111 (compute.go, design.md, validation_inputs_test.go, gen_cases.py, issue_cases.md); the two new tests TestDryRunInsertionStaysPriced and TestMalformedReReportKeepsInsertionInBound; the new case check-malformed-rereport-keeps-insertion-priced, which I recomputed by hand (client 6,000 bytes; bound 6,000 + 100,000 = 106,000; o2's 8e12 fails, o3's 100,000 passes; r1 usage 1,500 + 5,000 = 6,500; net -5,000 x $3.75/M = -$0.01875); the merge commit b2fda5c (conflict resolution, the new TestSizeRangeWinsOverMediaTokens, and the merged compute.go regions limits, items, price, checkUsage at their new line numbers); the author's round-2 reply and merge comments, with each claim checked against the code.
• go vet ./... and go test ./...: both clean on b2fda5c. gofmt -l . prints nothing.
• Live tests, a 32-bit build and a regeneration of the JSON cases were not run. I read the code and the cases only.

[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	3 of 5	b2fda5cb0cb9	blocking

Verdict: blocking

Both round-2 findings are fixed, and I verified the fixes. The merge of main (#46, #47) is clean: no case id collided, the reason lists hold both media_tokens and size_range, and the claimed interaction (size_range wins over media_tokens) is real and tested. One defect remains in the limits repetition: its first pass still judges honest reports against a bound that omits the active change, and those drops stick.

Findings

1. internal/engine/compute.go:176 — the first pass of the limits repetition judges honest reports against a bound that omits the active change, and the drops stick.
   • What fails: an operator re-reports an active insertion with an impossible size. Two other reports on the same request are honest. Both go to Unpriced with size_range, neither is stored, and the sum check fails.
   • Proof: within a pass, the sum loop runs before the drop loop. So in pass 1 the malformed re-report still has out = false, replaced returns true for the active change (compute.go:157-166), and the active change adds to no sum (compute.go:176). Pass 1's l.before is therefore below the true bound. A report above that too-small bound is dropped in pass 1, and e.out sticks (compute.go:195). Trace:
     • Client body 32,000 bytes. Active ci (stored on request 0): SizeBefore 0, SizeAfter 100,000.
     • Request 1 reports: bad (ChangeID "ci", before 8e12, after 100,000), v (before 0, after 90,000, an insertion), d (before 222,000, after 72,000, a cut). Forwarded body: 32,000 + 100,000 + 90,000 - 150,000 = 72,000.
     • Every size is honest: d received 222,000 = client + ci + v; v's after 90,000 is at most fwd 72,000 + removals 150,000 = 222,000.
     • Pass 1: l = {32000, 72000}; ci is suppressed because bad is not out yet; v adds 90,000, so l.before = 122,000. Drop loop: bad out; d out, because 222,000 > 122,000.
     • Pass 2: bad is out, so ci counts again and l.before = 222,000. But d is out, so its removal of 150,000 never enters l.after. Drop loop: v out, because 90,000 > 72,000.
     • Pass 3 returns l = {132000, 72000}.
     • items: v fails the after bound (90,000 > 72,000) and d fails the before bound (222,000 > 132,000). Both are size_range (compute.go:323-324), isChange rejects both (compute.go:353), so neither is stored, and the sum check fails: the active ci is in the sum, v and d are not.
     • The intended result: only bad is dropped, l = {222000, 222000}, and v and d are priced.
   • The comment on replaced says "A report that fails a bound is dropped below, and then the active one counts again" (compute.go:155-156). The active one does count again. But the reports that the too-small first bound dropped never come back, and their lost contributions then flag further honest reports. This breaks the stated goal of limits: one bogus report must not change the bound for the others (compute.go:130-131). Both new tests miss it: in TestMalformedReReportKeepsInsertionInBound and the case check/malformed-rereport-keeps-insertion/priced, the honest downstream op is re-checked in items against the final bound and passes. The victim here depends on another casualty's contribution to the sums.
   • Fix: seed the repetition from the loose side. In the first pass, count every active change and every non-negative report, and suppress nothing. Drop and suppress only from the second pass on, with the previous pass's bounds. The first bound is then at least the true bound, so no honest report fails it, and the later passes only tighten toward the true bound. Add a test with the three-report chain above: v and d must stay priced.

Earlier findings

My round-2 findings:

1. The DryRun after bound unprices an honest observe-mode report — Fixed. The over closure now tests b.Bytes > l.before || (!op.DryRun && a.Bytes > l.after) (compute.go:218); a DryRun's SizeAfter is no longer bounded. Design.md check 13 states the rule (design.md:1311-1312). TestDryRunInsertionStaysPriced covers both directions: an insertion of 50,000 bytes on a 2,000-byte body stays priced, and a SizeBefore of 50,000 is still size_range. The reason for no generated case (the case format has no dryRun field on an expected op, so the harness conservation check would count the hypothetical cost) is sound and is written in docs/issue_cases.md.
2. A malformed re-report lowers the bound for the other operations — Fixed for the shape I traced. An active change now leaves the sums only while a well-formed report replaces it (compute.go:155-166); once the drop loop removes the malformed re-report, the active change counts again in the next pass. TestMalformedReReportKeepsInsertionInBound and the case check/malformed-rereport-keeps-insertion/priced pin it, and I verified the case against the generator (see below). See finding 1: the repair leaves a window in the first pass.

Reviewer 1's round-2 findings (posted before mine; I checked both myself):

1. DryRun after bound — Fixed, same code as my finding 1.
2. Malformed re-report removes the active change from the bound sums — Fixed, same code as my finding 2, with the same remaining window.

Round-2 non-blocking items: done. The dead 32-bit skip in TestSizeAfterAboveLimitIsNotPriced is removed. Design.md check 8 now states the 32-bit rule exactly: the cap cannot be reached there, and a sum that wraps gives a ratio that fails check 9, so nothing is priced.

The author's merge comment (2026-11-04T20:36:25Z): each claim checked. The reason lists in design.md and api_test.go hold both media_tokens and size_range (design.md:1172, api_test.go:82). No case id collided, and the regeneration did not touch this PR's i44 cases (the merge diff of test/cases/ holds only main's new cases). The interaction claim is real: items sets it.bad before price runs, and the price switch tests it.bad before mediaUnpriced (compute.go:1336-1344), so a report over both the image and the bound is size_range. TestSizeRangeWinsOverMediaTokens pins it. I also checked the second claim by reading: limits, bigSize and check 13 read only sizes and Forwarded byte totals, never TTL or phase, so #47's per-breakpoint TTL has no path into them.

Non-blocking

• Main's #46 and #47 code arrived through the merge and is now the PR base. I read all of it (compute.go, body.go, fingerprint.go, state.go, types.go, doc.go, persist.go and the test diffs). I found no serious defect in it and no further interaction with check 13. The extent binary search is sound: upTo grows with the position index, so the fitting positions are a prefix, and TestExtentMatchesLinearScan checks it against the old linear scan on random bodies, also for a body decoded from JSON.
• The generated JSON bodies of main's new cases (clock-, media-, ttl-, extent- cases, about 5,700 lines) are the only part of the round's diff I did not read line by line. They are generated by gen_cases.py and the case harness runs them (go test ./test/cases passes). This PR's own new case I verified by recomputation (below).

What I checked

• Last review: round 2, 7cd5f3c, 2026-11-04T20:12:01Z
• New commits read: 11 in 7cd5f3c..HEAD. This PR's: e1fb111 "fix: Bound DryRun by SizeBefore only, keep insertions in bounds", b2fda5c "merge: Bring main into fix/validate-inputs". From main through the merge (PRs #46 and #47, now the PR base): d1335cb, 5dab1b8, 02b5a4a, 4fdeb52, 01d41bb, 0f9942d, ef373eb, 5829b8e, 5fa1986.
• Read in full: 4 reviews, 3 comments, 0 inline comments (from the three files)
• New items since my last review:
  • Commit e1fb111: read the full diff and the code around each change (compute.go, validation_inputs_test.go, gen_cases.py, design.md, issue_cases.md, the new case).
  • Commit b2fda5c (the merge): read the full code diff against both sides — all engine code and test files, cases.go, cases_test.go, api_test.go, design.md, issue_cases.md — and checked that the regeneration left this PR's cases unchanged.
  • The nine main-side commits: read as they arrive through the merge diff (see Non-blocking for the generated JSON bodies).
  • Author comment at 2026-11-04T20:34:31Z (round-2 replies): read; each claim checked above.
  • Author comment at 2026-11-04T20:36:25Z (merge notes): read; each claim checked above.
  • Reviewer 1's round-2 review (2026-11-04T20:09:25Z, posted before my round-2 post): read in full this round.
• Ran gofmt -l . (nothing), go vet ./... and go test ./...: clean.
• Ran GOOS=linux GOARCH=386 go build ./... and GOOS=linux GOARCH=arm go vet ./...: clean.
• Verified the new case check/malformed-rereport-keeps-insertion-priced.json against gen_cases.py: recomputed the FNV position hashes and byte counts of all four bodies with the generator's algorithm; they match. The expected figures agree with the Go test (20,000 saved, 25,000 caused, net -5,000 x $3.75/M = -$0.01875).
• Traced limits by hand through all three passes of the finding-1 scenario, and traced the fix's behavior for the valid re-report case (an observe-to-enforce switch): the loose first pass converges to the intended bound in the second pass.
• Live tests were not run. I read the code only.

[boazdavid]

Round 3 reply. Fixed in the head commit of this push.

The finding is right. In pass 1 the malformed re-report still had out=false, so the active change was suppressed and the bound was too low. I reproduced your chain. TestHonestChainBesideMalformedReReport failed on b2fda5c (v and d were size_range) and passes now. The new generated case check/honest-chain-beside-malformed-rereport/priced also fails on the old code.

Rule change: I did not seed pass 1 only. I made every active operation always count, in every pass. Reason: suppressing the active change in later passes has the same flaw, because whether a re-report is honest is what the passes decide. With the active change always counted, every pass counts a set that holds all honest operations, so the bound is never below the true one, a drop is always a wrong report, and the loop ends (a drop is permanent, at most one per operation). A bogus report still cannot raise its own bound: it is dropped and then adds nothing. The cost is a bound that is loose by the size of a replaced operation, which only lets a wrong report through.
One detail: EndRequest stores a copy of each report as active. storedCopy skips an active op that has the same ChangeID and the same sizes as a report of this request, so it counts once. A re-report with other sizes still leaves the old active op in the sums.

design.md check 13 and docs/issue_cases.md section 9c are updated. Hand figure of the case: -22,500 + 37,500 - 25,000 = net -10,000 write tokens x $3.75/M = -$0.0375.

[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	4 of 5	180a47b420d5	approve

Verdict: approve

The commit 180a47b fixes reviewer 2's round-3 finding, and the fix is sound. I traced the new repetition rule and the storedCopy dedup by hand, and I checked the invariant the author claims: every pass counts a set that holds all honest operations, so the bound is never below the true one. I found no new defect. go vet ./..., go test ./... and gofmt -l . are clean on 180a47b.

Earlier findings

• Reviewer 2, round-3 finding 1 (the first pass of the limits repetition judges honest reports against a bound that omits the active change, and the drops stick): fixed, and the finding was real. I missed it in my round-3 review, and I say so plainly: my round-3 trace had only one honest report beside the malformed re-report, and that report was dropped from the sums but still passed items against the final bound, so nothing failed. Reviewer 2's chain has a second honest report that depends on the first casualty's contribution to the sums, and both are unpriced. I re-traced that chain on b2fda5c and confirm it: pass 1 suppresses the active insertion (the re-report is not dropped yet), drops the honest cut (222,000 above 122,000), pass 2 loses that cut's 150,000 from the after bound and drops the honest insertion (90,000 above 72,000), and items then flags both against the final bound {132,000, 72,000}.
• The fix in 180a47b: an active operation is never taken out of the sums (compute.go:192). replaced is gone. I verified the invariant by induction: in every pass the counted set holds all honest reports (they are non-negative and not yet dropped) and every real change at least once (an active copy that storedCopy excludes has a same-size twin report in the set, and an honest twin is never the first honest drop). So every pass's bound is at least the true bound, an honest report never fails it, each drop removes a wrong report only, and the loop ends because a drop is permanent. The cost is a bound that is loose by the size of a replaced operation. That only lets a wrong report through; the sum check still backstops it. Design.md check 13 states the rule and the price (design.md:1314-1319).
• The author's reason for not using reviewer 2's exact fix (seed pass 1 loose, suppress from pass 2): suppressing the active change in later passes has the same flaw, because whether a re-report is honest is what the passes decide. The reason is right. Always counting is simpler and provably never below the true bound.
• storedCopy (compute.go:145-163): an active copy is excluded only when a report of this request has the same ChangeID, the same byte sizes at op level and on every span, and is not negative. I checked the edge cases. A negative twin does not exclude the copy. A spend that shares the ChangeID of a change (§6.5 check 3) has zero sizes, so it excludes only a copy with zero sizes, which adds nothing to the sums. A DryRun twin with the same sizes excludes the copy, and the twin itself is skipped from the sums because it is a DryRun: correct, because the change is then not applied and no downstream operator can see it. A twin whose sizes fail the bound excludes the copy too, but the copy has the same sizes, so its contribution is not real in the current body, and no honest report can depend on it. The common path is covered: a change re-carried every request with the same sizes counts once, not twice.
• My round-3 review had no findings, and my round-3 non-blocking item (the replaced scan made the worst case cubic) is gone with replaced. The new code is one storedCopy pass over actives times reports, then at most one pass per dropped report.
• All round-1 and round-2 findings (mine and reviewer 2's) stay fixed at 180a47b. I re-read isChange, items, tooBig, bigSize and the DryRun rule at the new head. The author rejected no finding.

Findings

None. I checked 180a47b as a first review and found no new defect.

Non-blocking

• The loose bound lets a wrong re-report through check 13 when it stays within the size of the active operation it replaces. The sum check flags the phantom tokens, so the backstop holds. The design doc states the trade-off. No change needed.

What I checked

• Last review: round 3, b2fda5c, 2026-11-05T04:44:59Z.
• New commits read: 1 (180a47b "fix: Find the size bounds from the loose side").
• Read in full: 6 reviews, 4 comments, 0 inline comments (from the three files .review-in-reviews.txt, .review-in-comments.txt, .review-in-inline.txt).
• New items since my last review: reviewer 2's round-3 review (2026-11-05T04:49:11Z) — read in full, finding re-traced on b2fda5c and confirmed; the author's round-3 reply comment (2026-11-05T04:52:49Z) — each claim checked against the code; commit 180a47b — the full diff (compute.go, design.md, issue_cases.md, validation_inputs_test.go, gen_cases.py) and the code around each change.
• I traced limits on the new test TestHonestChainBesideMalformedReReport (validation_inputs_test.go:268): client 32,000; active ci 0 to 100,000; bad re-report 8e12; v 0 to 90,000; d 222,000 to 72,000; forwarded 72,000. Pass 1 counts everything non-negative, bound {222,000, large}, drops only bad; pass 2 returns {222,000, 222,000}; v and d pass in items and the sum check holds. The test fails on b2fda5c by the trace above (v and d both size_range).
• I recomputed the new case check/honest-chain-beside-malformed-rereport-priced by hand: client 34,000 bytes (8,500 tokens); r0 forwarded 134,000; r1 forwarded 74,000 = 2,000 system + 72,000; r1 usage 18,500 = 8,500 baseline + net 10,000 caused; d's 222,000 is the whole messages section (16,000 + 100,000 + 90,000 + 16,000); expected net -22,500 + 37,500 - 25,000 = -10,000 write tokens x $3.75/M = -$0.0375; only bad is unpriced with size_range. All figures match the generator and the engine's bounds.
• go vet ./... and go test ./...: both clean on 180a47b. gofmt -l . prints nothing.
• Live tests, a 32-bit build and a regeneration of the JSON cases were not run. I read the code and the cases only.

[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	4 of 5	180a47b420d5	approve

Verdict: approve

The round-3 finding is fixed. Commit 180a47b removes the replaced rule and counts every active operation in every pass of the limits repetition. A new storedCopy helper stops the double count when a report and its stored copy have the same sizes. I traced the fix on the failing chain from my round-3 review, on the new Go test, and on the new generated case. I found no new defect.

Earlier findings

My round-3 finding 1 (the first pass of the repetition judges honest reports against a bound that omits the active change, and the drops stick) — fixed.

• The code: active operations enter es and never leave it (compute.go:177-181). The sum loop skips an entry only for out, kind, DryRun or negative sizes (compute.go:195). It no longer suppresses an active operation behind a re-report. The drop loop marks only reported entries, and a drop is permanent (compute.go:213-217).
• I traced the chain from my finding under the new code. Client 32,000 bytes. Active ci inserts 100,000. Request 1 reports bad (ci, before 8e12), v (inserts 90,000), d (cuts 222,000 to 72,000). Forwarded 72,000.
  • Pass 1: l.before = 32,000 + 100,000 (ci) + 90,000 (v) = 222,000. Drop loop: only bad drops, because 8e12 > 222,000. d stays, because 222,000 ≤ 222,000.
  • Pass 2: l.after = 72,000 + 150,000 (d) = 222,000. No drop. The loop returns l = {222000, 222000}.
  • items: v and d pass both bounds, so both are priced. bad is size_range and replaces nothing. This is the intended result from my round-3 review.
• The invariant holds. Every pass counts all active operations plus all reports that have not dropped. A report can only leave by failing a bound, and each pass's bound counts a set that holds every honest operation. So no honest report fails a bound, and the loop ends, because each drop is permanent and there is at most one drop per operation. A bogus report cannot raise its own bound either: once it drops, it adds to no sum.
• storedCopy (compute.go:145-163) is sound for its job. It compares Bytes only, and the sums are bytes, so Tokens and MediaBytes do not matter. activeOps stores the copy with the same sizes in client space, so a true copy matches. I probed the edges:
  • A re-report with other sizes leaves the old active operation counted. The bound is loose by that operation's size. Design.md check 13 states this cost (design.md, check 13). A loose bound only lets a wrong report through, so it is safe.
  • While the matching report stands, the skip is sum-neutral: the report contributes the same amounts.
  • I tried to build a case where the skip plus a later drop unprices an honest report, and found none. An active insertion's same-size re-report cannot drop while the insertion is replayed, because its SizeAfter is covered by the forwarded body. An active reduction runs before every reported operation in the pipeline model, so no reported SizeAfter bound depends on its removal.
  • A DryRun copy adds to no sum, so skipping it changes nothing. The !negative(op) guard is redundant, because a negative report cannot have the same sizes as a stored change. Harmless.
• The test TestHonestChainBesideMalformedReReport (validation_inputs_test.go:268) is my round-3 trace as code. It asserts that bad is size_range, that v and d stay priced, and that the sum check passes. The author states it failed on b2fda5c. The code shape confirms this: on b2fda5c, pass 1 suppressed ci behind bad, dropped d, and then dropped v in pass 2.
• The author rejected no finding this round. The rule change goes beyond my fix proposal (seed pass 1 only) and closes the same flaw for later passes too. The stronger rule is right: whether a re-report is honest is what the passes decide, so the suppression must not depend on it.

Reviewer 1's round-3 review (approve, no findings): read in full. I agree with it. Both of us traced the e1fb111 fixes and the merge; my round-3 finding was the only open item, and it is now closed.

Findings

None. I checked 180a47b as a first review and found no new defect.

Non-blocking

• Two colluding bogus reports can still pass the sums-based bound. Example: B1 reports an insertion of 8e12 bytes and B2 reports a removal of the same bytes. Each raises a bound for the other. This is a property of the design, it predates this PR, and the new comment states the cost of the loose side (compute.go:136-137). A single report cannot raise its own bound. No change needed.

What I checked

• Last review: round 3, b2fda5c, 2026-11-05T04:49:11Z.
• New commits read: 1 (180a47b "fix: Find the size bounds from the loose side").
• Read in full: 6 reviews, 4 comments, 0 inline comments (from the three files .review-in-reviews.txt, .review-in-comments.txt, .review-in-inline.txt).
• New items since my last review:
  • Commit 180a47b: read the full diff (compute.go, design.md, validation_inputs_test.go, gen_cases.py, issue_cases.md, the new case) and the code around each change. limits, storedCopy, tooBig, malformed, isChange and the items size_range case read again at their new line numbers.
  • The new test TestHonestChainBesideMalformedReReport: read; traced it against limits by hand.
  • The new case check/honest-chain-beside-malformed-rereport-priced.json: read in full; recomputed the FNV position hashes of all four bodies with the generator's algorithm (client 2,000 + 16,000 + 16,000 bytes; forwarded r0 2,000 + 16,000 + 100,000 + 16,000; forwarded r1 2,000 + 72,000). All hashes and byte counts match. The expected figures agree with the hand figure: v causes 22,500, d saves 37,500, the active insertion causes 25,000, net -10,000 write tokens x $3.75/M = -$0.0375.
  • Author comment at 2026-11-05T04:52:49Z (round-3 reply): read; each claim checked above.
  • Reviewer 1's round-3 review (2026-11-05T04:44:59Z): read in full this round.
• Ran gofmt -l . (nothing), go vet ./... and go test ./...: clean.
• Ran GOOS=linux GOARCH=386 go build ./... and GOOS=linux GOARCH=arm go vet ./...: clean.
• Traced the fix by hand through both passes of the finding-1 chain, and probed storedCopy against the edge cases listed above.
• Live tests were not run. I read the code only.