Fix: Accept host token counts and keep base64 media out of the ratio by boazdavid · Pull Request #30 · rossoctl/accountant

boazdavid · 2026-11-04T13:21:13Z

Summary

One bytes-per-token ratio misprices images and dense JSON. This PR does proposals 1 and 2 of #11.

Closes #11

What changed

`Size.Tokens`: the host may pass the tokens of a range. The library uses them as given. That size is not `Estimated`. 0 means not measured, so bytes are converted.
`Size.MediaBytes`: the host may mark the base64 part of a range. It is priced at 1 token per 250 bytes.
`PosHash.Media`: `NewBody` counts base64 image and document payload bytes. The ratio now uses text bytes over the billed tokens less the media tokens.
A `Size` with negative figures or `MediaBytes > Bytes` is refused as `negative_size`.
Docs updated: `docs/spec.md` R8, `docs/design.md` §4.2 rule 7, §5.3, Q3, and `internal/engine/doc.go`.
Proposal 3 (error range) is not done.

How it was tested

New tests in `internal/engine/size_tokens_test.go`:

Body with a 400,000-byte image and 3 text messages of 4000 bytes: a 4000-byte cut now books about 1000 tokens (was 54).
Host `Tokens: 5000` for 10,000 JSON bytes: books 5000, not Estimated (ratio gave 3500).
`MediaBytes` size: books the image at the fixed price.
`gofmt -l .`, `go vet ./...`, `go test ./...`, `go mod tidy -diff`: all clean.

Open questions

The media price (1 token per 250 base64 bytes) is a fixed constant. It matches the issue's figure (400,000 bytes = 1600 tokens). A per-provider value may be needed later.
No generated JSON case was added. The Go tests pin the numbers.

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

…e ratio Size gains Tokens (used as given, not Estimated) and MediaBytes. PosHash gains Media, set by NewBody for base64 image and document blocks. The bytes-per-token ratio now covers text only; media is priced at 1 token per 250 bytes. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: DAVID BOAZ <DAVIDBO@il.ibm.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	`bda774052b03`	blocking

Verdict: blocking

Findings

internal/engine/compute.go:945 — check 6 (conservation) fails on every request where the host sends Size.Tokens that differ from the ratio
- What fails: a correct host report produces a conservation CheckFailure and an accountant check failed warning. Size.Tokens exists for dense content where the ratio is wrong (accountant#11). On exactly those requests, the two sides of check 6 now use different conventions: the ops side uses the host's exact tokens, the bill-difference side still uses the ratio.
- Proof: trace. Request 1: client [json 10,000 B, prose 40,000 B], forwarded [prose 40,000 B], op rewrite SizeBefore{Bytes: 10000, Tokens: 5000}, MessageSplice{At:0, Removed:1}, Usage{Input: 12000}. Phase unknown, so conservation is skipped; the op is stored active with its Tokens. Request 2 (same thread, 1 minute later, exact5m): client [json, prose, prose2 4,000 B], forwarded [prose, prose2], Usage{CacheRead: 12000, Input: 12000*4000/40000}. Ratio = 13,200 text tokens / 44,000 text bytes = 0.3. price (compute.go:894) puts the host's 5,000 tokens into p.Saved. conservation (compute.go:945) computes want = c.ratio.upTo(c.client, last) - billed = 16,200 − 13,200 = 3,000 and net = 5,000. d = -2,000, tolerance = ops + 1 = 2. The check fails: bill difference=3000 ops=5000 tokens. The same drift happens when a Size covers media the host did not mark (MediaBytes 0 is a documented legal state, types.go:132-134): the ops side prices those bytes at the text ratio, while the want side prices the client body's media at 250 B/token.
- Fix: in conservation, correct want by the difference between the host tokens and the ratio estimate of the same bytes (or skip check 6 for spans priced exactly). Add a two-request test with Tokens. All new tests are first requests, where the phase is unknown and conservation is skipped, so the gap has no test.
internal/engine/fingerprint.go:67 — mediaBytes misses the OpenAI image form and counts matching text as media
- What fails: an OpenAI-style body ({"type":"image_url","image_url":{"url":"data:image/png;base64,..."}}) contains no "base64" string in quotes, so its payload stays in the text ratio. The skew this PR fixes remains for those requests. In the other direction, a text position that contains "base64" and later a "data" key (a tool result that echoes an API schema) is counted as media and priced at 250 B/token instead of the ratio.
- Proof: mediaBytes (fingerprint.go:67-96) searches only for the byte sequence "base64" followed by "data". Design rule 7 (design.md §4.2) says "NewBody counts the base64 payload of image and document blocks", and the library serves OpenAI bodies too (§5.1 names OpenAI models and their 128-token rounding). The doc comment at fingerprint.go:64-66 names only the Anthropic source shape, so the code and the rule disagree on coverage.
- Fix: also match the data-URL form (;base64, inside a string value). Or state in design.md rule 7 that only the Anthropic source form is recognized, and that a host with another form must build the Body itself and set PosHash.Media.
docs/design.md:181 — the §4.1 type listing still says Size is bytes only
- What fails: design.md:181-184 reads "Size is bytes only. A context operator does not count tokens". The code now has Size.Tokens and Size.MediaBytes (types.go:123-135), and the new R8 says a host may send tokens. The §5.1 PosHash listing (design.md:674-678) also lacks the new Media field that rule 7 names.
- Proof: the diff adds the fields and updates R8, rule 7, §5.3, Q3 and doc.go, but not the two type listings.
- Fix: update both listings.

Non-blocking

The new tests pass the same Body as client and forwarded while the operation removes bytes, so the sum check fails inside every oneOp test. The tests pass because they do not assert res.Checks. A consistent pair of bodies would also cover the checks, and would have caught finding 1.
newRatio (body.go:126-130) clamps tokens to 0 when the billed tokens are below the media tokens. All text estimates then become 0. The direction is safe (understatement), but it is silent.

What I checked

The full diff of head bda7740 against base 52d732f, and the code around each change: body.go, compute.go, fingerprint.go, types.go, doc.go, size_tokens_test.go.
Every converted call site of the old ratio.of(cumBytes(...)): extent, readCoversExtent, cacheEffects, addBreak, rescue, headTTL, split, conservation, and the ping-ratio in pingRead. The media split is applied consistently on both sides of each comparison.
sizeTokens and the Estimated rule against §4.2 rule 7 and R8: host tokens are exact, byte conversions are estimates, a zero-byte size is exact zero.
invalid() now also refuses MediaBytes > Bytes and negative Tokens; isChange, negative, the sum check and the spans check still work on bytes only, so the new fields do not change them.
The test helpers (t0, exact5m, msg, findOp, hasUnpriced, tierSum) exist and the new tests compile against them.
The API surface: no new exported name; PosHash is already hidden in api_test.go.
Ran gofmt -l ., go vet ./... and go test ./... in this worktree. All clean.
Docs: spec R8, design rule 7, §5.3, Q3 and doc.go match the code, except the two listings in finding 3.

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	`bda774052b03`	blocking

Verdict: blocking

Findings

internal/engine/fingerprint.go:67 — mediaBytes counts plain text as media, so the request ratio misprices every saving.
- What fails: a message whose text holds the quoted string "base64" (a tool result such as {"encoding":"base64","data":"..."}, a tool schema with {"enum":["base64"]}, or text that quotes an image block) is not a media block. The provider bills it as text. NewBody still counts its "data" string as PosHash.Media.
- Proof: mediaBytes matches "base64" anywhere (fingerprint.go:71), then the next "data" key anywhere after it (fingerprint.go:76). It checks no JSON structure. Trace: message 1 is {"encoding":"base64","data":" plus 80,000 chars plus "} (80,040 bytes), message 2 is 19,960 bytes of prose. The provider bills 25,000 tokens (4 bytes a token). newRatio (body.go:126) gives tokens = 25,000 - 320 = 24,680, bytes = 20,000. The ratio is 1.234 tokens a byte. The true ratio is 0.25. A mask that trims 4,000 prose bytes books 4,936 saved tokens. The true saving is 1,000. All effects that use the ratio (extent, breaks, rescues) are wrong by the same factor.
- Fix: parse the block structure before counting. Use a streaming encoding/json decode and count data only inside an image or document source block with "type":"base64". The decode can stream, so the payload is still never kept (R2). Or check the context around the marker (for example, require "media_type" in the same object). Design rule 7 (design.md:274) says NewBody counts "the base64 payload of image and document blocks"; the code counts more than that.
internal/engine/fingerprint.go:68 — data-URL images are not detected, so spec R8 is false for them.
- What fails: a host on the OpenAI wire format sends {"type":"image_url","image_url":{"url":"data:image/png;base64,<payload>"}}. The marker "base64" needs a quote on each side. In ;base64, there is none, and "data: has no closing quote after data. So mediaBytes returns 0. The image bytes stay in the text ratio. That is the exact mispricing of issue #11: a 400,000-byte image with 1,600 tokens pulls the ratio down, and text savings are booked far too low (the PR's own test says 54 instead of 1,000).
- Proof: spec R8 (spec.md:282) says "Base64 image and document bytes are not in the conversion ratio", with no format limit. The code delivers that only for Anthropic-style source blocks. The two marker searches in mediaBytes (fingerprint.go:71,76) cannot match the data-URL form.
- Fix: also match the data-URL form (count the bytes after base64, up to the closing quote), or narrow R8 and design rule 7 to the source-block form and say that a host on the data-URL format must build its Body itself with PosHash.Media.
internal/engine/compute.go:945 — the conservation check fails on a correct report that uses Size.Tokens.
- What fails: check 6 compares net (the op tokens, now the host's own Tokens) with want (the client body, still converted with the request ratio). Host tokens are sent exactly when the ratio is wrong (dense JSON, issue #11). The tolerance is tol+1, about one token an op. So a correct report fails the check, and Compute logs accountant check failed for it.
- Proof: trace. Client body: messages of 10,000 bytes of dense JSON and 40,000 bytes of prose. Forwarded body: the prose only. Op: SizeBefore{Bytes: 10000, Tokens: 5000}, SizeAfter{}. Usage: Input: 10000. The sum check passes (10,000 = 50,000 - 40,000). Ratio: 10,000 tokens over 40,000 bytes = 0.25. price books 5,000 saved tokens. conservation: net = 5000, want = 0.25 x 50,000 - 10,000 = 2500, d = -2500, tolerance 2. The check fails. The PR's own TestHostTokensAreUsedAsTheyAre also produces a conservation (and a sum) CheckFailure; the test does not assert on Checks, so it passes anyway.
- Fix: in conservation, convert each size at the ratio even when Tokens is set, so both sides of the comparison use the same conversion. Add a test with Size.Tokens and noChecks (the helper exists in helpers_test.go:107).
docs/design.md:798 — the cross-host sentence in §5.3 is now wrong.
- What fails: "The same tokens in both hosts holds whenever the two hosts receive the same bodies and the same operations." One host may send Size.Tokens and the other may not. The same bodies and operations then give different tokens, because one host gets its own count and the other gets the ratio conversion.
- Proof: sizeTokens (compute.go:236) returns s.Tokens when it is set, and the ratio conversion when it is not. The old guarantee held because no host sent a count. The new sentence keeps the old conclusion but drops the condition it rested on.
- Fix: add the condition. For example: "The same tokens in both hosts holds whenever the two hosts receive the same bodies and the same operations, and send the same token counts."

Non-blocking

The three pricing tests in size_tokens_test.go pass the same body as client and forwarded, while the op reports a size change. Each one silently fails the sum and conservation checks. Assert noChecks in them; that would have surfaced finding 3.
mediaBytes allocates []byte(marker) on every loop pass (fingerprint.go:71,76). Lift the markers to package-level constants. The skip loop at fingerprint.go:82 also takes only spaces and :, so pretty-printed JSON with a tab or a newline between "data" and its value is missed.
Size.MediaBytes > Bytes is refused as negative_size (compute.go:194, types.go:138). The reason name misleads: the figure is not negative.
PosHash.Media from a host-built Body gets no validation, unlike Size. A host that sets Media above Bytes makes ratio.bytes negative, and ratio.of then prices all text as 0 tokens. Size has invalid(); Body has nothing.

What I checked

Read CLAUDE.md, docs/spec.md (R2, R7, R8, R11), docs/design.md (§4.2 rule 7, §5.3, §6.5), and internal/engine/doc.go.
Read the full diff of bda7740 against 52d732f, and the code around each change: body.go, compute.go, fingerprint.go, types.go, size_tokens_test.go.
Traced by hand: mediaBytes on text and on both media formats, newRatio, ratio.of/tokensOf/upTo/between, sizeTokens, price, conservation, extent, and the three new tests.
Ran gofmt -l ., go vet ./..., and go test ./... in my worktree. All clean.
Checked the public API: the change adds fields only, no new exported names, so accountant.go and api_test.go need no change.
Checked the generated cases: test/gen/gen_cases.py and test/cases are unchanged, and the cases pass.

…kens mediaBytes now parses JSON and counts only base64 image/document source data and base64 data URLs. Check 6 compares host-token sizes at the ratio. Media above Bytes is capped, and a bill below the media price fails the media_tokens check. Docs list Size.Tokens, MediaBytes and PosHash.Media. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: DAVID BOAZ <DAVIDBO@il.ibm.com>

boazdavid · 2026-11-04T13:35:01Z

Round 1 fixes are in commit 8ebda2a.

Both reviewers, media detection (too loose; data URLs): fixed.

mediaBytes now parses each position with streaming encoding/json. It counts source.data only in an image or document block whose source has type: base64. It keeps the count, not the payload.
A data URL under url or image_url counts the bytes after base64,.
Negative tests in TestMediaFormats give Media 0 for text that quotes "base64" and "data", an enum with "base64", a url source, and a base64 source inside a text block.

Both reviewers, conservation with Size.Tokens (check 6): fixed. hostGap is the host tokens minus the ratio figure for the same bytes. Check 6 subtracts it. TestConservationHoldsWithHostTokensOverTwoRequests asserts noChecks on request 2.

Docs: fixed. design.md section 5.3 says "and send the same token counts". The 4.1 Size and 5.1 PosHash listings have the new fields. Rule 7 describes the parsing.

Non-blocking:

The three pricing tests use separate client and forwarded bodies and assert noChecks.
Media above Bytes in a host-built Body is capped, so ratio.bytes cannot go negative.
A bill below the media price now fails a new check, media_tokens.
MediaBytes > Bytes still reports negative_size. The name covers any malformed Size. Not changed.

gofmt, go vet, go test ./... and go mod tidy -diff are 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	resumed	1 of 2	2 of 2	`8ebda2ab02ca`	blocking

Verdict: blocking

Findings

docs/design.md:1168 — §6.5 does not describe the new check 6 comparison, and the media_tokens check is missing from the list.
- What fails: §6.5 check 6 still says the check compares "the request total of the applied context operations" with the bill difference. Since hostGap (compute.go:904, compute.go:952), the check subtracts the host's Size.Tokens out of net first, so it compares the ratio-converted total, not the request total. The new media_tokens check (compute.go:966) is also absent from the §6.5 list, which ends at check 7. Rule 7 (design.md:276) names media_tokens, but §6.5 is the canonical list of checks. CLAUDE.md makes the design the source of truth: a change to Compute that the docs do not describe is a bug.
- Proof: the fix commit 8ebda2a changes conservation and adds media_tokens, and touches design.md only at the Size listing, rule 7, the PosHash listing, and §5.3. Section 6.5 is unchanged. Consequence of the new comparison: check 6 no longer catches a wrong Size.Tokens. Trace: a host bug reports Tokens: 500000 for a 10,000-byte range whose true count is 5,000. price books a 500,000-token saving. hostGap = 500000 - 3500, so net becomes the ratio figure, want equals it, and no check fires. The sum check sees bytes only. The round-1 head caught this case; the fix removed the false alarms and the true alarm together.
- Fix: update §6.5: check 6 compares the ratio-converted figures when Size.Tokens is set, and add media_tokens to the list. If the lost detection matters, bound hostGap: fail a check when the host's tokens imply a byte-per-token rate outside a sane range for the range's bytes.

Earlier findings

My round-1 findings:

mediaBytes counts plain text as media — fixed. The parser is now structural: walkObject (fingerprint.go:134) counts a data string only when the source object has type: base64 and the parent block is image or document. TestMediaFormats covers the negative cases I named: the {"encoding":"base64","data":"..."} tool result, the enum schema, and text that quotes both markers. I traced each case through walkValue/walkObject; all return 0.
Data-URL images are not detected — fixed. dataURLPayload (fingerprint.go:95) counts the bytes after ;base64, under a url or image_url key. Residual over-breadth noted under Non-blocking.
Check 6 fails on correct host tokens — fixed by hostGap and TestConservationHoldsWithHostTokensOverTwoRequests. I traced the two-request test: request 2 has d = 0 in conservation and noChecks passes. The doc gap left by this fix is finding 1 above.
§5.3 cross-host sentence — fixed. design.md:800 now says "and send the same token counts".

Reviewer 2's round-1 findings:

Check 6 and host tokens — fixed for Size.Tokens, same as my finding 3. The secondary clause (a Size that covers media the host did not mark) is not fixed, and I accept that: I traced client [image 400,040 B, prose 40,000 B], forwarded [prose], billed 10,000, SizeBefore{Bytes: 400040} with no MediaBytes. The op books 100,010 saved tokens; the bill difference is 1,610. The check failure is then a true alarm about an inconsistent report, not a false one, because the booked saving itself is wrong. With MediaBytes marked, my trace gives net = 1610, want = 1610, and the check passes.
Media detection (data URLs; loose match) — fixed, same as my findings 1 and 2.
The §4.1 Size and §5.1 PosHash listings — fixed. design.md:181 lists Tokens and MediaBytes; design.md:677 lists Media; the 16-byte note at design.md:695 is correct for 8 + 4 + 4.

Non-blocking

Two of the three pricing tests still discard res and do not assert noChecks: TestHostTokensAreUsedAsTheyAre (_, p := priceOne(...)) and TestMediaBytesOnASizeUseTheFixedMediaPrice (_ = res). The PR comment of 2026-11-04T13:35:01Z says all three assert noChecks. My traces show both tests do pass their checks today, so only the assert is missing.
The data-URL match (fingerprint.go:155, case "url", "image_url") takes any object and any MIME type. A non-image data URL, for example data:text/html;base64,... in a tool result under a url key, counts as media and is priced at 250 bytes a token. Require an image or document MIME prefix, or the enclosing block type.
cumMedia (body.go:116) caps Media above Bytes, but a negative Media from a host-built body passes through: min(-5000, 100) = -5000. That inflates both sides of the ratio. A host bug, and silent.

What I checked

Last review: round 1, bda7740, 2026-11-04T13:29:52Z.
New commits read: 1 — 8ebda2a "fix(engine): Parse media blocks, keep conservation exact with host tokens".
Read in full: 2 reviews, 1 comment, 0 inline comments (from .review-in-reviews.txt, .review-in-comments.txt, .review-in-inline.txt; the inline list is empty).
New items since my last review: the fix commit 8ebda2a (reviewed in full below), and the author's comment of 2026-11-04T13:35:01Z (checked each claim against the code; the noChecks claim is wrong for two tests, see Non-blocking). Reviewer 2's review of 13:28:54Z predates my last review; I read it and addressed its findings above.
Read the full diff of 8ebda2a against bda7740, and the changed files around each change: fingerprint.go, compute.go, body.go, size_tokens_test.go, docs/design.md.
Traced by hand: mediaBytes on all TestMediaFormats cases plus escaped-JSON text and nested blocks; dataURLPayload; hostGap on both new conservation tests; media_tokens; cumMedia capping; the marked and unmarked MediaBytes conservation traces under "Earlier findings".
Ran gofmt -l ., go vet ./..., and go test ./... in my worktree. All clean.
Checked the public API: the fix adds unexported names only (walkValue, walkObject, valueInfo, dataURLPayload), so accountant.go and api_test.go need no change.

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	`8ebda2ab02ca`	blocking

Verdict: blocking

The fix commit repairs the Size.Tokens side of check 6, parses JSON for media, and updates the doc listings. I verified each fix by hand. Two gaps remain. Both are narrow, and one of them is the half of my round-1 finding 1 that the fix does not cover.

Findings

internal/engine/types.go:132 — a Size over a media range with MediaBytes 0 is a legal input, and the library then overstates the saving about 60 times and fails check 6
- What fails: types.go:131-134 says MediaBytes 0 means "the range has no media, or the host did not mark it". When the host does not mark it, price (compute.go:894) converts the whole range at the text ratio. The bodies convert the same bytes at 250 B/token. The price is wrong in the unsafe direction (§8: never invent money), and check 6 fails, although the report is legal. hostGap (compute.go:904) reconciles only the Tokens convention; the media convention stays unreconciled. This is the second trigger of my round-1 finding 1, which the fix does not cover.
- Proof: trace. Client [imageBlock(400000) (400,067 B, Media 400,000), text 4,000 B], forwarded [nil, text 4,000 B] (the image message removed), op rewrite at messages[0], SizeBefore{Bytes: 400067} (MediaBytes 0), MessageSplice{At:0, Removed:1}, Usage{Input: 1000}. Ratio: 1,000 tokens over 4,000 text bytes = 0.25. Sum check passes (400,067 = 404,067 − 4,000). Price: of(400067) = 100,016 saved tokens. The true bill difference is of(4067) + 1600 − 1000 = 1,616. Check 6: want = 1616, net = 100016 (hostGap 0), fails by 98,400, and the detail names no media. So the sum check passes, the price is 62 times too high, and only an unnamed check failure signals it.
- Fix: in price, when a span's MediaBytes is 0, take the media under the span's positions from the client Body (cumMedia over the flat range; the library already has it). Or send the operation to Unpriced with a reason. Or change types.go and design rule 7 to say a Size that covers media must mark it, and fail a named check for it. Add a test with an unmarked cut of an image message.
internal/engine/fingerprint.go:155 — a base64 document under file_data (OpenAI input_file) is not counted, so R8 is still false for it
- What fails: the OpenAI Responses form {"type":"input_file","file_data":"data:application/pdf;base64,..."} is a document block with base64 bytes. walkObject counts data URLs only under the keys url and image_url (fingerprint.go:155). The file_data payload stays in the text ratio, so the skew of accountant#11 remains for requests with inline files.
- Proof: R8 (spec.md:282) says "Base64 image and document bytes are not in the conversion ratio", with no key list. dataURLPayload (fingerprint.go:95) handles the form, but no call site feeds it file_data. TestMediaFormats has no case for it. Residual of my round-1 finding 2 and reviewer 1's finding 2: the url and image_url forms are now covered, this one is not.
- Fix: add case "file_data": *n += dataURLPayload(child.str) beside the url case, and a TestMediaFormats case.

Earlier findings

My 1 (check 6 fails with Size.Tokens): fixed for the Tokens trigger. hostGap (compute.go:904) subtracts the host tokens' excess over the ratio figure, and TestConservationHoldsWithHostTokensOverTwoRequests asserts noChecks on a hot second request. I recomputed the test by hand: ratio 9,500/40,500, host gap 2,628, want 2,322, net 2,322, d 0. The media trigger is not fixed: finding 1 above.
My 2 (mediaBytes matches text, misses data URLs): fixed. The parser counts an image or document block's base64 source.data, and data URLs under url and image_url, in both the string and the object form. TestMediaFormats covers the negative cases (quoted text, enum, url source, base64 source inside a text block) and I traced the parser on each. Residual: file_data, finding 2 above.
My 3 (stale doc listings): fixed. The §4.1 Size listing and the §5.1 PosHash listing have the new fields, and the "4 of padding" line now says media.
Reviewer 1's 1 and 2 (media matching): fixed, same code as my 2. Checked myself.
Reviewer 1's 3 (conservation with Size.Tokens): fixed, same code as my 1.
Reviewer 1's 4 (§5.3 cross-host sentence): fixed. design.md:803 now adds "and send the same token counts".
Reviewer 1's non-blocking items: the marker allocation and whitespace skip are gone with the parser; Media above Bytes in a host-built Body is capped (body.go:116, TestHostBuiltMediaAboveBytesIsCapped); negative_size for MediaBytes > Bytes is rejected by the author with the reason that the name covers any malformed Size. I accept that reason: the name is cosmetic and the doc lists it as the malformed-size reason.

Non-blocking

The new check media_tokens is missing from the check-name lists: result.go:26, the design.md §6.3 CheckFailure listing (design.md:1074) and the §6.5 numbered list. It is described only inside rule 7 (design.md:280).
The author's note says "The three pricing tests ... assert noChecks". Two of the four do not: TestHostTokensAreUsedAsTheyAre and TestMediaBytesOnASizeUseTheFixedMediaPrice (the _ = res line). They pass their checks by my arithmetic, but the assertion costs nothing.

What I checked

Last review: round 1, bda7740, 2026-11-04T13:28:54Z
New commits read: 1 (8ebda2a "fix(engine): Parse media blocks, keep conservation exact with host tokens")
Read in full: 2 reviews, 1 comment, 0 inline comments (from the three files)
New items since my last review: reviewer 1's round-1 review (I checked each of its four findings against the code myself before reading the fix); the author's fix comment of 2026-11-04T13:35:01Z (I verified every claim it makes; two small inaccuracies are in Non-blocking); commit 8ebda2a (full diff, plus fingerprint.go, compute.go, body.go, types.go and size_tokens_test.go around each change).
Traced by hand: hostGap arithmetic and the two-request conservation test; media_tokens (fires at billed 100 against 1,600 media tokens); walkValue/walkObject/dataURLPayload on the Anthropic source block, the OpenAI object and string image_url forms, pretty-printed JSON, keys in other order, blocks in a list, and every negative case in TestMediaFormats; the cumMedia cap; the four rewritten pricing tests.
Ran gofmt -l ., go vet ./... and go test ./... in this worktree at head 8ebda2a. All clean.
Checked the docs: rule 7, §5.3, the §4.1 and §5.1 listings and doc.go match the code, except the check-name lists in Non-blocking.

… file_data Merge origin/main. An unmarked Size over a media range takes its media from the client body. Size.Tokens outside 0.05..2.0 tokens a byte is ignored and fails check size_tokens. file_data data URLs count; only image and PDF data URLs are media. PosHash.Media is in the persist wire form. Docs list checks 6, 8 and 9. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: DAVID BOAZ <DAVIDBO@il.ibm.com>

boazdavid · 2026-11-04T13:53:39Z

Round 2 fixes are in commit 926f950, merged with main (PRs 26, 27 and 28 are in).

Reviewer 2 finding 1 (unmarked Size over media): fixed. rangeMedia takes the media under an unmarked span from the client Body. TestUnmarkedCutOfAnImageMessageUsesTheBodysMedia asserts about 1618 tokens saved (1600 plus the wrapper text) and noChecks.

Reviewer 2 finding 2 (file_data; non-image data URLs): fixed. file_data counts beside url and image_url. A data URL counts only if it starts with data:image/ or data:application/pdf. New TestMediaFormats cases: the input_file PDF counts, data:text/html;base64, gives 0.

Reviewer 1 finding 1 (design 6.5; lost detection of a wrong Size.Tokens): fixed.

Design 6.5 check 6 says sizes are compared at the ratio.
media_tokens and size_tokens are in the 6.5 list, the 6.3 listing and result.go.
New check size_tokens: a Size.Tokens outside 0.05 to 2.0 tokens per text byte is ignored, the bytes are used, and the check is reported. Test: 500000 tokens for 10,000 bytes gives 3125 saved tokens from the bytes, Estimated.

Non-blocking, done: two tests now assert noChecks. A negative Media in a host-built body clamps to 0. PosHash.Media is in the persist wire form from PR 26, with a round-trip test.

Each fix was reverted in turn and its test fails alone with -run. gofmt, go vet, go test ./... and go mod tidy -diff are clean.

Note: the 0.05 to 2.0 limits are separate constants. Merging PR 28 added the same limits for the usage ratio. A maintainer may want one shared constant.

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 3	`926f95031d53`	approve

Verdict: approve

Findings

None. Every finding from rounds 1 and 2 is fixed, and I found no new defect in commit 926f950 or in the main commits the merge brought in.

Earlier findings

My round-2 finding:

§6.5 did not describe the new check 6 comparison, and media_tokens was missing from the list — fixed. design.md:1180 now says each size is compared at the request ratio and the host-token gap is taken out of the total. Checks 8 (media_tokens, design.md:1186) and 9 (size_tokens, design.md:1189) are in the §6.5 list, the CheckFailure comment (design.md:1086, result.go:26) names both. The lost detection is restored: plausible (compute.go:242) bounds Size.Tokens at 0.05 to 2.0 tokens a byte; a count outside is ignored, the bytes are used, and check 9 fires. I traced TestImplausibleHostTokensFailACheckAndUseBytes: 500,000 tokens for 10,000 bytes gives size_tokens, 3,125 saved from the bytes, Estimated, and conservation still holds.

Reviewer 2's round-2 findings:

A legal unmarked Size over a media range overstates the saving about 60 times — fixed. rangeMedia (compute.go:253) takes the media of an unmarked before-size span from the client body: min(max(s.Bytes - text, 0), media) over the span's flat range. I traced TestUnmarkedCutOfAnImageMessageUsesTheBodysMedia: an unmarked cut of a 400,000-byte image message now books wrapper/4 + 1600 tokens at the media price, conservation gives d = 0, and noChecks passes.
file_data (OpenAI input_file) is not counted — fixed. walkObject counts a data URL under url, image_url or file_data (fingerprint.go:164), and dataURLPayload (fingerprint.go:104) accepts only data:image/ and data:application/pdf. TestMediaFormats has the input_file PDF case (counts 6) and the data:text/html case (0). I traced both through the parser.

My round-2 non-blocking items: the two missing noChecks asserts are in (both tests now assert); a negative Media clamps to 0 (body.go:119, TestNegativeHostMediaIsClamped); the data-URL MIME restriction is in. The negative_size reason name was rejected by the author with the reason that it covers any malformed Size; I accept that reason.

Non-blocking

extent is quadratic again. cumBytes has a prefix sum (body.go:47), but cumMedia (body.go:116) still loops, and upTo (body.go:150) calls it for every position extent tries. With no candidate breakpoints, extent is Θ(n²) even when every Media is 0. This undoes the linear extent that PR #27 just delivered; BenchmarkExtentNoBreakpoints now measures the quadratic path. Fix: add a media prefix sum beside cum and make cumMedia index it.
rangeMedia derives media only for before sizes (isBefore, compute.go:253). An after size that adds media and leaves MediaBytes 0 still converts at the text ratio. The client body does not hold the added content, so the derivation cannot reach it; the host's duty stands. Design rule 7 (design.md:280) says "a size"; say "a SizeBefore" so the doc matches the code.
Corner of the two points above: a host that adds an image, sends the true Tokens, and leaves MediaBytes 0 has its count refused. plausible sees text = all bytes, and 0.05 x 400,068 exceeds the true 1,617 media tokens, so check 9 fires on a correct count and the bytes convert at the text ratio. Requires the host to skip its documented duty; note only.
Design rule 7 still says "the payload of a data:...;base64, URL" with no key list or MIME limit. The code counts only url, image_url and file_data, and only data:image/ or data:application/pdf. The fingerprint.go comment is exact; one line in rule 7 would make the design exact too.
The author's own note stands: the 0.05 to 2.0 limits (compute.go:236) duplicate PR 28's usage-ratio limits. One shared constant would keep them from drifting.

What I checked

Last review: round 2, 8ebda2a, 2026-11-04T13:40:53Z.
New commits read: 6 — 926f950 (merge: round-2 fixes plus main), 34dd528 (merge PR #26), a331fc5 (merge PR #27), c47012e (harden snapshot after review), b1af2f3 (Body prefix sums, splicer index), 537e44f (Snapshot, Restore, Trimmed).
Read in full: 4 reviews, 2 comments, 0 inline comments (from .review-in-reviews.txt, .review-in-comments.txt, .review-in-inline.txt; the inline list is empty).
New items since my last review: reviewer 2's round-2 review of 13:41:04Z (both findings fixed, verified by my own traces above); the author's comment of 13:53:39Z claiming the fixes (each claim checked: all hold, including the noChecks asserts this time); commit 926f950 (full diff read, plus compute.go, fingerprint.go, body.go, size_tokens_test.go, persist.go around each change); the four main commits that the merge brought in (read in full; they came reviewed from PRs #26 and #27, so I checked them for interaction with this PR: the cum prefix sum beside Media, cumBytes' fallback for restored bodies, the splicers loop against toFwd, and the wirePos.Media addition with its round-trip test).
Read this PR's own diff against the new base 34dd528 (10 files) and confirmed it adds no exported name and imports only the standard library (encoding/json, strings).
Traced by hand: rangeMedia on the unmarked image cut (also with a partial range and a range outside the body); plausible on dense JSON, a media-heavy marked range, and the 500,000-token bug; the size_tokens and media_tokens checks; hostGap with the derived media; dataURLPayload on image, PDF, HTML and plain URLs; the persist round trip of Media; Snapshot determinism (sorted keys) and Restore validation.
Ran gofmt -l ., go vet ./..., and go test ./... in my worktree at head 926f950. All clean.

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 3	`926f95031d53`	blocking

Verdict: blocking

The fix commit repairs the before side of the unmarked-media gap, bounds Size.Tokens, and counts file_data. I verified each fix by hand. Two findings remain. Finding 1 is the after side of the same gap. Finding 2 is the merge state: the branch does not contain PR 28, although the fix comment says it does.

Findings

internal/engine/compute.go:941 — rangeMedia derives media only for before-sizes, so an unmarked SizeAfter over media is priced at the text ratio
- What fails: types.go:131-134 says MediaBytes 0 means "the range has no media, or the host did not mark it". Unmarked is legal input. price calls rangeMedia(sp.after, sp, false). That call returns s.MediaBytes unchanged, because compute.go:254 exits when !isBefore. So the whole after range converts at the text ratio. A host that does not mark media on a before-size does not mark it on an after-size either. The fix covers one side of such reports only.
- Proof, an operator that adds an image (second request of a thread, so the phase is hot and check 6 runs): client [text 4,000 B], forwarded [imageBlock(400000) (400,067 B, Media 400,000), text 4,000 B], op rewrite with MessageSplice{At:0, Removed:0, Inserted:1}, SizeBefore{Bytes: 0}, SizeAfter{Bytes: 400067} with MediaBytes 0, Usage{Input: 2616} (1,600 media tokens plus 1,016 text tokens). Ratio: 1,016 text tokens over 4,067 text bytes. Price: at = tokensOf(400067, 0) = of(400067) = 99,942, so the op books 99,942 Caused tokens. The true cost is 1,618 (the media at 250 B a token plus the wrapper text). The price is about 62 times too high, in the Caused direction. hostGap is 0, so check 6 fails by about 98,300 tokens. The forwarded body holds the media (NewBody marks PosHash.Media), but rangeMedia never reads the forwarded body.
- The same gap hits a rewrite that keeps the image and trims text around it: before 404,067 unmarked gets its media derived (the fixed side), after 403,067 unmarked prices at of(403067), and a saving of about 250 tokens is booked as about 98,000 Caused tokens.
- A second effect: with a correct host count on the after size (SizeAfter{Bytes: 403067, Tokens: 2366}), plausible (compute.go:242) sees text = 403,067 and rejects 2366 < 0.05 x 403,067. A correct count is reported as size_tokens and then ignored.
- No test covers an after-size over media. TestMediaBytesOnASizeUseTheFixedPrice and TestUnmarkedCutOfAnImageMessageUsesTheBodysMedia both cut the media away: the after size is zero.
- The doc disagrees with the code. Design rule 7 says "If a size does not mark it, the library takes the media of the range from the client body". The sentence covers any size. The code covers before-sizes only.
- Fix: derive the after side from the forwarded body, the same way. price already maps a span into forwarded space (toFwd(k, sp.from), compute.go:961). Take the media under the mapped range from c.fwd, with the same clamp. Add a test: an unmarked after-size that still holds the image.
The branch does not contain PR 28, so it collides with main's tip, and the fix comment says it does
- What fails: the fix comment of 2026-11-04T13:53:39Z says "merged with main (PRs 26, 27 and 28 are in)". PR 28 (main merge 616fb7a) is not in head 926f950. The head's second parent is 34dd528. Main moved to 616fb7a after that. The head tree has no usage_validation_test.go and no bad_usage, ratio_range or breakpoints check anywhere.
- Proof: git ls-tree 926f950 -- internal/engine/usage_validation_test.go prints nothing; a grep for ratio_range over the head tree finds nothing. On main, design 6.5 has checks 8 (bad_usage), 9 (ratio_range) and 10 (breakpoints), and compute.go:886 defines maxTokensPerByte = 2.0. This PR adds its own checks 8 (media_tokens) and 9 (size_tokens) at the same place in the 6.5 list, and compute.go:237 defines maxTokensPerByte = 2.0 again. A merge that keeps both declarations does not compile, because both are in package engine. The 6.5 list also conflicts textually: both sides add different items after item 7.
- One conflict is semantic. Main's badRatio refuses a body with 0 text bytes, as an infinite ratio. After the merge, a forwarded body that is all media has 0 text bytes, and this PR prices it correctly (media at 250 B a token, no text). The merged badRatio must not refuse that body.
- Fix: merge 616fb7a into the branch. Renumber the 6.5 list so media_tokens and size_tokens follow the usage checks. Keep one constant pair for the 0.05 to 2.0 bounds; the author's note already proposes this. Correct the merge claim in the PR conversation.

Earlier findings

My round-2 finding 1 (unmarked Size over media): half fixed. The before side is fixed. I traced TestUnmarkedCutOfAnImageMessageUsesTheBodysMedia by hand: derived media 400,000, bt = 1616, hostGap = 0, conservation passes, noChecks holds. The after side is not fixed. It is finding 1 above.
My round-2 finding 2 (file_data): fixed. walkObject counts file_data beside url and image_url (fingerprint.go:164). dataURLPayload takes only data:image/ and data:application/pdf (fingerprint.go:105). I traced the new TestMediaFormats cases: the input_file PDF counts 6, and data:text/html gives 0.
My round-1 finding 1 (check 6 with Size.Tokens): fixed by hostGap. I re-verified it on the two-request test.
My round-1 findings 2 and 3: fixed in round 2. Unchanged since.
Reviewer 1's round-2 finding 1 (design 6.5; the lost detection of a wrong Size.Tokens): fixed. Check 6 in 6.5 says each size is compared at the ratio. media_tokens and size_tokens are in the 6.5 list, the 6.3 listing and result.go. plausible bounds the count: 500,000 tokens for 10,000 bytes is ignored and the check fires. I traced TestImplausibleHostTokensFailACheckAndUseBytes: 3,125 saved from the bytes, Estimated. The list numbering collides with main's PR 28. That is finding 2.
Reviewer 1's round-1 findings 1 to 4: fixed in round 2. Unchanged since.
Reviewer 1's round-2 non-blocking items: done. All four pricing tests assert noChecks. A negative Media clamps to 0 (body.go:119, TestNegativeHostMediaIsClamped). The persist wire form carries PosHash.Media (TestPersistKeepsMedia).
My round-2 non-blocking items: done. media_tokens and size_tokens are in result.go, the 6.3 listing and the 6.5 list.

Non-blocking

cumMedia (body.go:116) sums in a loop on every call. upTo calls it, and extent with no candidate breakpoints calls upTo for every position. That extent is O(n^2) again, although b1af2f3 says "NewBody now builds a prefix-sum array, so cumBytes is O(1) and extent with no breakpoints is linear". cumBytes has the b.cum fast path. cumMedia has none. Build a media prefix sum in NewBody beside b.cum. BenchmarkExtentNoBreakpoints measures this and asserts nothing.
Design rule 7 names the data-URL form as "data:...;base64,". The code takes only data:image/ and data:application/pdf. Name the two prefixes in the doc.

What I checked

Last review: round 2, 8ebda2ab02ca7f6c4175b137c280766491849c22, 2026-11-04T13:41:04Z.
New commits read: 926f950 (the branch merged with main at 34dd528) and the commits it brought in — 537e44f, b1af2f3, c47012e — each in full. For finding 2 I also read main's tip 616fb7a and its commit 63237c6 (PR 28), which sit outside the range.
Read in full: 4 reviews, 2 comments, 0 inline comments (from .review-in-reviews.txt, .review-in-comments.txt, .review-in-inline.txt; the inline list is empty).
New items since my last review: the fix commit 926f950 and the author's comment of 2026-11-04T13:53:39Z. I verified each claim against the code. Two are wrong: "PRs 26, 27 and 28 are in" (28 is not, finding 2), and "about 1618 tokens saved" (the test asserts 1,616 = 1,600 + 67/4 inside its own ±2 band; a slip in the comment, not in the code).
Traced by hand: rangeMedia on the before side of the unmarked-cut test, and on the after side of an addition and of an in-place rewrite (finding 1); plausible on correct and wrong counts; the sizeTokens fallback and hostGap in all four rewritten tests; dataURLPayload on data:image/, data:application/pdf and data:text/html; media_tokens; the persist round trip; the 6.5 list and result.go against the code.
Ran gofmt -l ., go vet ./... and go test ./... at head 926f950. All clean.
Checked the public API: the fix adds unexported names only (plausible, rangeMedia, sizeTokens, the two bound constants), so accountant.go and api_test.go need no change.

…body Merge origin/main (#26, #27, #28). Keep one maxTokensPerByte and reuse minTokensPerByte for size_tokens. Number media_tokens 11 and size_tokens 12 after bad_usage, ratio_range and breakpoints. rangeMedia now maps the after-size range into the forwarded body. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: DAVID BOAZ <DAVIDBO@il.ibm.com>

boazdavid · 2026-11-04T14:48:18Z

Round 3 fixes are in commit 1433ad2. git merge-base --is-ancestor 616fb7a HEAD succeeds: PRs 26, 27 and 28 are in.

Reviewer 2 finding 2 (the branch lacked PR 28): fixed.

One maxTokensPerByte and one minTokensPerByte, both from main. The size_tokens check uses them. My own copies are gone.
Design 6.5: media_tokens is check 11 and size_tokens is check 12, after main's 8 bad_usage, 9 ratio_range and 10 breakpoints. The check-name lists in design.md and result.go carry both names.
go generate ./test/cases left the JSON cases unchanged.
Correction: my round 2 note said PR 28 was in this branch. It was not. It is now.

Reviewer 2 finding 1 (unmarked after-size over media): fixed.

rangeMedia takes the media of an unmarked after-size from the forwarded body, over the range mapped by toFwd. It uses toFwdSkipping for an inserted item, as price does. It clamps at 0 and at the media in the range.
Design rule 7 says: the media comes from the client body for a before-size and from the forwarded body for an after-size.
New tests in size_tokens_test.go: TestUnmarkedAfterSizeThatAddsAnImageUsesTheForwardedMedia (about 1618 tokens caused), TestUnmarkedAfterSizeThatKeepsTheImageSavesTheTextOnly (about 250 saved, 0 caused), TestHostTokensOnAnAfterSizeOverMediaAreNotRejected (no size_tokens failure, 250 saved, not Estimated). All assert noChecks.
I made rangeMedia skip the forwarded body for after-sizes. Each new test fails alone with -run.

gofmt, go vet, go test ./... and go mod tidy -diff are clean.

boazdavid

Reviewer	Backend	Context	Session	Reviewer #	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 4	`1433ad2ea132`	blocking

Verdict: blocking

The round-3 fixes are real and correct. The merge with main brought PR 28 in. But the merge left two doc/code disagreements. One contradicts the fix comment. Both are one-line fixes.

What I checked

Last review: round 3, 926f95031d537667a44f5dfbd156573d2d345620, 2026-11-04T14:08:47Z.
New commits read: 4 — 1433ad2 (fix: derive media of unmarked after-sizes from the forwarded body; merge with main), 616fb7a (merge PR 28), 63237c6 (order checks 8 to 10 and refuse a zero or infinite ratio; read in full in round 3 and again at head), 8b272ba (validate usage, breakpoints and TTL strings; read in full this round). The merge kept both sides: the PR-vs-base diff still holds all of rounds 1 to 3, and validate/checkUsage/usage_validation_test.go are present.
Read in full: 6 reviews, 3 comments, 0 inline comments.
New items since my last review: commit 1433ad2 with the PR 28 commits it brought, and the author's comment of 2026-11-04T14:48:18Z. I checked each claim in that comment. All hold except one: "The check-name lists in design.md and result.go carry both names" — they carry this PR's two names only (finding 2).
Traced by hand: the three new after-size tests; rangeMedia's after-side mapping against price's d < 0 branch; badRatio's zero tests against newRatio's media clamp (finding 1); validate/skipsPricing wiring in calc and price; the §6.5 renumbering and the check-name lists.
Ran at head 1433ad2: gofmt -l . (empty), go vet ./..., go test ./.... All clean.
Public API: 1433ad2's own changes add unexported names only. The three new reason aliases in accountant.go come from main's 8b272ba, with api_test.go updated.

Findings

Finding 1: check 11 in design §6.5 says text prices at 0 tokens when the bill is below the media price. At head, that bill refuses all pricing.

What fails. docs/design.md check 11 (Media tokens): a bill below the forwarded body's media price leaves "the text ratio is 0 and every text size prices at 0 tokens". Since the merge, the same condition fails check 9 first. newRatio (internal/engine/body.go:130) clamps tokens to 0 when the bill is below media/250. badRatio (internal/engine/compute.go) returns true when t == 0. So validate returns ratio_range, skipsPricing() is true, price sends every operation to Unpriced, and nothing is priced. The sentence describes behavior that can no longer happen. Also, media_tokens never fires alone: a bill below the media price makes t == 0, so ratio_range always fires beside it.

Proof. Trace TestBilledBelowMediaPriceIsAFailedCheck's own input at head: body [imageBlock(400000), text 40], Usage{Input: 100}. newRatio gives tokens = max(100 − 1600, 0) = 0 and bytes = 40. badRatio fires on t == 0. validate returns ratio_range before check 11 runs. Checks holds both media_tokens and ratio_range, and the operation is Unpriced. The test still passes, because it only looks for media_tokens. No test pins the new behavior. A second case: a bill exactly equal to the media price ("not above" is allowed by check 11's wording) also gives t == 0 and fails check 9.

Fix. Pick one, then make doc and code agree:

Update the check 11 sentence to the merged behavior: check 9 fails too, nothing is priced, the operations are Unpriced. Add one test that pins it.
Keep the documented behavior: in badRatio, do not treat t == 0 as out of range when the body has media. Check 11 already names that bill, and its reason is more precise.

I recommend option 1. A bill below the media price is a usage that does not fit the body, and PR 28's rule to refuse it is defensible. The defect is the stale sentence, not the behavior.

Finding 2: the check-name lists in `result.go` and design §6.3 omit `bad_usage`, `ratio_range` and `breakpoints`.

What fails. checkUsage puts bad_usage, ratio_range or breakpoints into CheckFailure.Check. The comment at internal/engine/result.go:26 and the CheckFailure listing in design §6.3 (docs/design.md:1085-1086) name only sum through shared_warm, media_tokens and size_tokens. A host that switches on check names has no doc for three of the names the library can emit. This also contradicts the fix comment of 2026-11-04T14:48:18Z: "The check-name lists in design.md and result.go carry both names."

Proof. At head 1433ad2: result.go:26 reads "sum" | "order" | "unique_change" | "spans" | "attribution" | "conservation" | "shared_warm" | "media_tokens" | "size_tokens". design.md:1085-1086 lists the same names. validate in compute.go returns ReasonBadUsage, ReasonRatioRange or ReasonBreakpoints, and calc appends CheckFailure{Check: c.invalid} to Result.Checks through checkUsage.

Fix. Add "bad_usage" | "ratio_range" | "breakpoints" to both lists.

Earlier findings, status

My round 3.

Finding 1 (unmarked after-size over media): fixed. rangeMedia now takes an after-size's media from the forwarded body over the range mapped by toFwd/toFwdSkipping, the same mapping price uses for its d < 0 branch. The clamp is min(max(s.Bytes−text, 0), media). I traced all three new tests by hand:
- TestUnmarkedAfterSizeThatAddsAnImageUsesTheForwardedMedia: after media 400,000, after text 67 bytes, caused 1616 (within the asserted 1614 to 1620), hostGap 0, noChecks.
- TestUnmarkedAfterSizeThatKeepsTheImageSavesTheTextOnly: before and after media both 400,000, the saved tokens are the removed text only: 250 (within 248 to 252), caused 0, noChecks.
- TestHostTokensOnAnAfterSizeOverMediaAreNotRejected: host tokens above the media price, after media 400,000, saved 250 exact, no size_tokens, not Estimated.
Finding 2 (the branch lacked PR 28): fixed. 616fb7a is an ancestor of head, one shared constant pair, §6.5 renumbered to checks 11 and 12, and the author corrected the round-2 claim. The merge's leftovers are findings 1 and 2 above.

My rounds 1 and 2. All findings fixed in 8ebda2a and 926f950. The fixes are still in the diff against the new base. No regression.

Reviewer 1's rounds 1 and 2. I checked each fix myself in earlier rounds. All fixed. No regression.

Reviewer 1's round 3 (approve, non-blocking). I checked each item myself:

After-size media gap: fixed by 1433ad2 (same as my round-3 finding 1).
"a size" → "a SizeBefore" in design rule 7: done.
Shared constants: done.
Plausible corner on after-size media: fixed by the after-side derivation.
Extent quadratic and MIME prefixes in rule 7: still open. See Non-blocking.

Non-blocking

cumMedia (internal/engine/body.go:116) loops over positions on each call. extent with no candidate breakpoints calls upTo for every position, so it is O(n²). PR 27 added b.cum for cumBytes; the same prefix sum for media removes the loop. Carried from round 3 and from reviewer 1's round 3.
Design rule 7 still says "data:...;base64, URL" without the two accepted prefixes (data:image/, data:application/pdf). The code checks them; the rule does not name them. Carried from round 3.
A host-built all-media Body (Media == Bytes on all positions) gives n == 0 in badRatio and fails ratio_range. Main refuses it too, through the 0.05 lower bound, so this is not a regression. Note only.

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 4	`1433ad2ea132`	blocking

Verdict: blocking

The after side of the unmarked-media gap is fixed, and the merge state is fixed. I verified both by hand. One finding remains. It is the semantic half of reviewer 2's round-3 finding 2: the merged badRatio now judges the media-adjusted ratio, and the design still describes both check 9 and check 11 as they were before the merge.

Findings

internal/engine/compute.go:968 — the merged badRatio judges the media-adjusted ratio, so check 9 disagrees with the design text and check 11 can no longer do what the design says
- What fails: newCalc sets c.ratio = newRatio(billed, r.Forwarded) (compute.go:336) and then runs validate (compute.go:337). Since this PR, newRatio is media-adjusted: tokens = billed − media/250, bytes = text bytes (body.go:130). badRatio (compute.go:968) reads that ratio. The design text for check 9 (design.md:1189) states the raw formula: "(Input + CacheRead + CacheWrite) / bytes". The two disagree, in both directions.
- Proof, direction 1 (the code accepts what the doc refuses): forwarded [text 40,000 B, imageBlock(400000) (400,067 B, Media 400,000)], billed 11,617 (1,600 media tokens plus 10,017 text tokens at 0.25 a byte). Code: t = 10,017, n = 40,067, ratio 0.25, in range, the request is priced. Doc formula: 11,617 / 440,067 = 0.026, outside 0.05 to 2.0, so per the design the request must go unpriced with ratio_range. A media-heavy request is the normal case this PR exists for, so the doc's formula refuses the PR's own main case.
- Proof, direction 2 (check 11 is dead letter): check 11 media_tokens fires when the media price is above the billed prompt tokens (compute.go:1099, design.md:1197). In that same case t = max(billed − media/250, 0) = 0, so badRatio fires first (validate runs before price), c.invalid = ratio_range, and the request prices nothing (compute.go:1017). Design check 11 says the consequence is "the text ratio is 0 and every text size prices at 0 tokens" (design.md:1198), and rule 7 says "if the bill is below the media price, check media_tokens fails" (design.md:279). That behavior no longer exists. On head 926f950 (before the merge) the request priced text at 0 tokens and media at 250 B a token, and reported media_tokens. On head 1433ad2 every operation goes to Unpriced with reason ratio_range, and media_tokens appears in Checks only as a second failure. Reviewer 2's round-3 finding 2 named this conflict: "The merged badRatio must not refuse that body". The merge kept the refusal.
- Proof, a third case: badRatio returns true when n == 0 (compute.go:970). The design calls that "an infinite ratio (billed tokens and an empty body)" (design.md:1191). With the media adjustment, n is the text bytes. A host-built Body with Bytes == Media on a position (a pure media position; Size.invalid refuses only Media > Bytes, and Body has no check) has n = 0 and is refused, although the body is not empty and the media needs no ratio. Through NewBody the JSON wrapper always leaves text bytes, so this needs a host-built body.
- No test covers the change. TestBilledBelowMediaPriceIsAFailedCheck (size_tokens_test.go:160) still passes, because checks runs unconditionally and appends media_tokens (compute.go:139, 1099), but it does not assert Unpriced. The usage-validation tests use media-free bodies. TestRatioOutOfRangePricesNothing (usage_validation_test.go:11) covers the raw-ratio case only.
- Fix: pick one behavior and write it once in the design. Either keep "prices nothing" and update check 9 to state the media-adjusted formula, and rewrite or drop check 11 (its condition is now subsumed by check 9, and rule 7's parenthetical must change with it); or keep check 11's behavior by leaving a bill below the media price to check 11 (t == 0 with media present) instead of check 9. Add a test that asserts the outcome — the Unpriced reason, or the priced tiers — for a bill below the media price.

Earlier findings

Reviewer 2's round-3 findings:

Unmarked after-size over media — fixed. rangeMedia (compute.go:251) maps the after range into the forwarded body with toFwd (or toFwdSkipping for an inserted item, compute.go:259, the same choice price makes at compute.go:1044) and derives the media there. I traced all three new tests by hand. TestUnmarkedAfterSizeThatAddsAnImageUsesTheForwardedMedia: toFwdSkipping(0, messages[0]) stays at messages[0] (it skips the item's own splice), the range holds the image message, derived media 400,000, at = of(67) + 1600 = 1616 Caused, conservation d = 0, noChecks. TestUnmarkedAfterSizeThatKeepsTheImageSavesTheTextOnly: both sizes derive media 400,000, saved of(1200+w) − of(200+w) ≈ 250, Caused 0, noChecks. TestHostTokensOnAnAfterSizeOverMediaAreNotRejected: derived media 400,000 makes plausible see text 1200+w, so the host's 1,900/1,650 pass, saved 250 exact, not Estimated, no size_tokens. This also closes my round-3 corner note (a correct host count on an unmarked after-size over media).
The branch lacked PR 28 — fixed as to the merge state, not as to the semantics. git merge-base --is-ancestor 616fb7a HEAD holds. One minTokensPerByte/maxTokensPerByte pair (compute.go:942), used by check 9 and check 12. Design §6.5 numbers media_tokens 11 and size_tokens 12 after main's 8 to 10 (design.md:1187-1201). The semantic half — the merged badRatio against the media-adjusted ratio — is my finding 1 above.

My round-3 non-blocking items:

extent quadratic through cumMedia — not fixed. cumMedia still loops (body.go:116) and upTo calls it per position. Still non-blocking.
rangeMedia derived before-sizes only — fixed, by this round's commit.
The plausible corner on an unmarked after-size — fixed, TestHostTokensOnAnAfterSizeOverMediaAreNotRejected.
Rule 7 names no key list or MIME limit for the data-URL form — not fixed. design.md:278 still says "a data:...;base64, URL"; the code counts only url, image_url and file_data, and only data:image/ and data:application/pdf (fingerprint.go). Still non-blocking.
Duplicate 0.05/2.0 limit constants — fixed. One shared pair at compute.go:942.

Non-blocking

res.Checks lists media_tokens before ratio_range although check 9 is the reason the request prices nothing (compute.go:139 against compute.go:140). The order is cosmetic; the reason is correct.

What I checked

Last review: round 3, 926f95031d537667a44f5dfbd156573d2d345620, 2026-11-04T13:58:11Z.
New commits read: 4 — 1433ad2 (derive media of unmarked after-sizes from the forwarded body), 616fb7a (merge PR 28), 63237c6 (order checks 8 to 10 and refuse a zero or infinite ratio), 8b272ba (validate usage, breakpoints and TTL strings).
Read in full: 6 reviews, 3 comments, 0 inline comments (from .review-in-reviews.txt, .review-in-comments.txt, .review-in-inline.txt; the inline list is empty).
New items since my last review: reviewer 2's round-3 review of 14:08:47Z (both findings checked against the code myself; see Earlier findings); the author's comment of 14:48:18Z (each claim checked: the merge-base holds, the constants are shared, §6.5 is renumbered, and the three new tests do what the comment says; the round-2 correction is fair); commit 1433ad2 and the PR 28 commits 8b272ba and 63237c6 that the merges brought in, each in full, with the interaction of PR 28's checks 8 to 10 against this PR's media-adjusted ratio as the focus.
Traced by hand: rangeMedia on the after side of all three new tests, including the toFwdSkipping mapping for the inserted item; plausible and hostGap with the derived media; newRatio and badRatio on a media-heavy body, on a bill below the media price, and on a host-built all-media body; the validate order against the checks order in calc (compute.go:129); nextState on an invalid-usage request (active operations are kept, only the cache state goes unknown, design.md:1203).
Ran gofmt -l ., go vet ./... and go test ./... at head 1433ad2. All clean.
Checked the public API: the three new reasons (ReasonBadUsage, ReasonRatioRange, ReasonBreakpoints) are aliased in accountant.go:123-125, and TestEveryUnpricedReasonIsInTheDesign covers them. No other new exported name; the imports stay standard-library only.

…dia bodies Merge origin/main. Check 9 uses tokens per text byte after removing the media price. A bill below the media price fails media_tokens and ratio_range and prices nothing. A body of media only passes when the bill covers the media. readReaches prices media at the fixed price. Check-name lists now name bad_usage, ratio_range and breakpoints. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: DAVID BOAZ <DAVIDBO@il.ibm.com>

…edia-11

boazdavid · 2026-11-04T15:12:47Z

Round 4 fixes are in commit e70e6eb (merged with main again).

Both reviewers, check 9 against the media-adjusted ratio: fixed.

badRatio refuses an empty body, refuses a bill below the media price, accepts a body with no text bytes when the bill covers the media, and applies the 0.05 to 2.0 range to tokens per text byte.
Design 6.5 check 9 states the media-adjusted formula: media bytes are out of the denominator, the media price is out of the numerator.
Check 11 and rule 7 say a bill below the media price fails both media_tokens and ratio_range, prices nothing, and sends every operation to Unpriced with reason ratio_range. I kept ratio_range as the reported reason.
The check-name lists in result.go and design 6.3 now include bad_usage, ratio_range and breakpoints.
Main added ttlSkew and readReaches(bytes). I kept readReaches but made it take (Body, i) and price with ratio.upTo, so media is priced at the fixed media price there too. I changed both callers.

Tests (size_tokens_test.go): TestBillBelowTheMediaPricePricesNothing, TestBillEqualToTheMediaPriceDoesNotFailMediaTokens, TestMediaHeavyRequestIsPriced (2500 saved, not refused), TestAllMediaBodyIsNotRefused.

Mutation check, each alone with -run: the all-media, equality and raw-formula tests fail when their fix is reverted.

Known gap: TestBillBelowTheMediaPricePricesNothing does not fail when only the new below-media branch is removed. The old t == 0 branch also refuses that bill, and with t == 0 removed as well the test still passed. I did not investigate why. TestAllMediaBodyIsNotRefused tests badRatio directly, not through a full Compute.

gofmt, go vet, go test ./... and go mod tidy -diff are clean. A trial merge of main (with PR 31) is clean and green.

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	5 of 5	`ccbc09ba8fb3`	blocking

Verdict: blocking

The round-4 fixes are real and correct, and the docs now match the merged check 9 and check 11. But the merge brought main's PR 24 in, and its new tier-rule code was not converted to the media split. One finding remains.

Findings

internal/engine/compute.go:944 — writeBP counts media bytes at the text ratio, so a prefix below MinCacheable is treated as reaching it and the wrong tier is booked
- What fails: writeBP picks the breakpoint that writes content at position p: the first breakpoint at or after p whose prefix reaches EntryFacts.MinCacheable tokens. The prefix figure is c.ratio.of(body.cumBytes(x.at)) — all bytes of the prefix, media included, at the text ratio. Since this PR, the text ratio is 0.05 to 2.0 tokens a byte and media prices at 1 token for 250 bytes (0.004 a byte). So the figure counts media bytes 12 to 500 times too high. A prefix whose true token count is below MinCacheable passes the test, the content is booked as a cache write (1.25× or 2× the input rate), and the design says it must be input. The saving is overstated. That is the unsafe direction (§8: never invent money).
- Proof: trace. MinCacheable = 2048 (a real model value, design §5.1). Client body [sys 400 B, m0 400 B, imageBlock(300000) (Media 300,000)], breakpoint at messages[1], TTL 5m. Usage{Input: 1700}: 500 text tokens at ratio 1.0 plus 1,200 media tokens. badRatio passes (media price 1,200 ≤ 1,700; t = 500, n ≈ 567, ratio ≈ 0.88). An operation cuts m0 from 400 to 100 bytes and saves 300 tokens at messages[0], after B*. writeBP at the breakpoint: of(cumBytes) = of(~300,500) ≈ 300,500 ≥ 2048, so the breakpoint qualifies and the 300 tokens are booked CacheWrite5m. The true prefix is of(~500 text) + 300,000/250 = 500 + 1,200 = 1,700 < 2048. Design §6.2 says the row "no breakpoint at or after it ends a prefix of at least EntryFacts.MinCacheable tokens" gives input, not write. The host prices a saved 5m-write token at 1.25× an input token (§7.1), so the saving is overstated by 25% of the booked tokens.
- Every other conversion site at head is media-aware (upTo, between, tokensOf): I grepped all of compute.go; writeBP is the one site left. The same call also sets the tier of a break's moved tokens (addBreak, compute.go:797) and feeds interaction's read-tier guard (compute.go:1135; a wrong choice there only drops the interaction, the safe direction).
- No test covers it: mincacheable_test.go is media-free, and size_tokens_test.go sets no MinCacheable.
- Fix: in writeBP, use the media-aware prefix: c.ratio.upTo(body, x.at) >= c.r.Facts.MinCacheable. Add one test beside TestACutInAClientPrefixBelowMinCacheableIsInput with an image in the prefix.

Earlier findings, status

My round 4.

Finding 1 (check 11 said text prices at 0 tokens; at head the bill below the media price refused all pricing): fixed, the doc way. Design check 9 now states the media-adjusted formula; check 11 and rule 7 now say the bill below the media price fails both media_tokens and ratio_range, prices nothing, and sends every operation to Unpriced with reason ratio_range. TestBillBelowTheMediaPricePricesNothing pins the outcome. The media_tokens detail string says the same.
Finding 2 (check-name lists omitted bad_usage, ratio_range, breakpoints): fixed. result.go:30 and design §6.3 list all eleven names.

Reviewer 1's round 4.

Finding 1 (check 9 judged the media-adjusted ratio while the design stated the raw formula; check 11 was dead letter): fixed, by the same changes. I checked both directions myself. Direction 1 (media-heavy acceptance): TestMediaHeavyRequestIsPriced — billed 1,600 + (30,000+w)/4, t ≈ 7,507, n ≈ 30,092, ratio ≈ 0.25, saved 2,500, noChecks. Direction 2 (check 11's consequence): the rewritten check 11 and TestBillBelowTheMediaPricePricesNothing. Direction 3 (the all-media body): TestAllMediaBodyIsNotRefused — bill 10 over media 2,500 passes (n == 0, t == 0), bill 9 fails the below-media branch.

Non-blocking items from round 4.

cumMedia O(n) per call, quadratic extent: not done. Carried.
Rule 7 MIME prefixes (data:image/, data:application/pdf): not done. Carried.
The all-media host-built body: addressed. The n == 0 branch of badRatio accepts it when the bill equals the media price.

The author's known gap, explained

The comment of 2026-11-04T15:12:47Z says TestBillBelowTheMediaPricePricesNothing still passes with the below-media branch removed, and with t == 0 removed too. The reason: that test's body has text bytes (n > 0). With the bill below the media price, newRatio clamps t to 0, so the plain range test t < 0.05*n fires on the same input. The below-media branch and the t == 0 branch are redundant whenever text bytes exist. The below-media branch is load-bearing only for a body with no text bytes (n == 0), where the range test cannot fire; TestAllMediaBodyIsNotRefused pins that case directly at the badRatio level (bill 9 over media 10). The gap is benign, but only the unit test covers the wiring.

Non-blocking

cumMedia (body.go) loops over positions on each call; extent with no candidate breakpoints is O(n²). A media prefix sum beside b.cum removes the loop. Carried from rounds 3 and 4.
Design rule 7 still names the data-URL form as "data:...;base64," without the two accepted prefixes. Carried.
design.md:1231 says an all-media body "passes when the bill covers the media price". The code passes only a bill exactly equal to the price: a bill above it gives t != 0 and fails ratio_range. "Equals" would be exact. A host-built body corner; NewBody bodies always have wrapper text.
TestAllMediaBodyIsNotRefused calls badRatio directly. A full Compute test with a host-built all-media body would pin validate → skipsPricing → Unpriced on that path too.
res.Checks lists media_tokens before ratio_range although ratio_range is the reason nothing is priced. Cosmetic; the media_tokens detail now names ratio_range.

What I checked

Last review: round 4, 1433ad2ea13234a939b22846a0bbd8ea1f29b785, 2026-11-04T14:56:49Z.
New commits read: 16 — ccbc09b (merge main with PR 31), e70e6eb (fix: judge check 9 on the media-adjusted ratio, accept all-media bodies; merge with main), 191ba5d (merge PR 31), e26aa32, 261f85c, 3db5f9d, 981a3fc (PR 31, Result.Interaction), 4289446 (merge PR 29), 1b12c5e, 06f807a, bbe7e9f, 61c6be3 (PR 29, clock order, TTL edge, forks), 182baa8 (merge PR 24), e72b59e, ca6ffc5, 6f645fd (PR 24, minimum cacheable length). The main-side commits came reviewed from their own PRs; I read them at their points of contact with this PR's media split, which is where finding 1 lives.
Read in full: 8 reviews, 4 comments, 0 inline comments (from the three files).
New items since my last review: reviewer 1's round-4 review of 14:56:54Z (its finding overlaps my round-4 finding 1; I checked the fix myself, both directions); the author's comment of 15:12:47Z (each claim checked: all hold; the known gap is explained above); commit e70e6eb and the main commits the merges brought in (PRs 24, 29 and 31).
Read the full compute.go diff for the whole range (it carries both this PR's changes and main's new code), the design.md, spec.md, result.go and size_tokens_test.go diffs in full, and mincacheable_test.go around the tier-rule tests.
Traced by hand: badRatio on all four documented cases (media-heavy, below-media, equal-bill, all-media); the four new tests; writeBP with media in the prefix (finding 1); readReaches (media-aware, both callers); addTier's new signature and all three call sites; interaction's guard; noCacheTokens; the out-of-order and fork rules (no token conversions, no media contact).
Ran at head ccbc09ba: gofmt -l . (empty), go vet ./..., go test ./.... All clean.
Public API: e70e6eb adds no exported name. InteractionCaused and MinCacheable come from main's PRs 31 and 24, in the base. The PR's diff against 191ba5d touches the same 10 files as before; no test/cases change.

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	5 of 5	`ccbc09ba8fb3`	blocking

Verdict: blocking

My round-4 finding is fixed, and both of reviewer 2's round-4 findings are fixed. I verified each by hand. One new finding remains. It is an interaction between this PR's media ratio and PR 24's minimum-cacheable rule, brought in by the merge: writeBP prices media bytes at the text ratio when it decides whether a prefix reaches the minimum.

Findings

internal/engine/compute.go:944 — writeBP prices media bytes at the text ratio for the minimum-cacheable decision, so a prefix below the model minimum is judged cacheable and tokens are booked as a write
- What fails: PR 24's tier rule writes content only at a breakpoint whose prefix reaches EntryFacts.MinCacheable, in tokens. writeBP estimates that prefix with c.ratio.of(body.cumBytes(x.at)): all bytes, media included, at the text ratio. Everywhere else the library prices media at the fixed 250 bytes a token through ratio.upTo (body.go:150), which extent, readReaches, pingRead, rescue, headTTL, split and conservation all use. The author adapted readReaches for exactly this reason this round (author comment, 15:12:47Z) but left writeBP on the raw form. Check 9 floors the text ratio at 0.05 tokens a byte, so media counts at 12.5 times its modeled price or more. The coded prefix is never below the true one, so a breakpoint can qualify in the code and fail with the provider.
- Proof: trace. EntryFacts.MinCacheable = 1024. Client [text 2,000 B, imageBlock(100000) (100,067 B, Media 100,000), text 400 B], forwarded [text 2,000 B, imageBlock(100000)], a client breakpoint at messages[2], Usage{CacheRead: 500, Input: 400} (the provider reads the old entry's 500 tokens and inputs the image's 400; nothing is written, because the longest prefix is 500 + 400 + 100 = 1,000 tokens, below 1024). Check 9 passes: 500 text tokens over 2,067 text bytes. noCacheTokens is false (CacheRead > 0). An op cuts messages[2] from 400 to 0 bytes: d = of(400) = 100 saved. writeBP prices the breakpoint's prefix as of(cumBytes(2)) = of(102,467) = 25,617 ≥ 1024, so addTier (compute.go:971) books Saved.CacheWrite5m = 100. The truth is Saved.Input = 100: the provider wrote nothing, so the cut saves input tokens. The saving is overstated by 25 percent (write weight 1.25 against input 1.0), the unsafe direction (§8). The same estimate runs on the client body through interaction (compute.go:1135), which calls addTier with the client body. A second effect: with breakpoints of different TTLs, the first "qualifying" breakpoint can be earlier than the true one, so tokens land in CacheWrite5m instead of CacheWrite1h.
- No test covers it: mincacheable_test.go uses text bodies only, one token a byte. The media tests in size_tokens_test.go set no MinCacheable.
- Origin: the merge. On main, PR 24's of(cumBytes) was the plain byte ratio, the only estimate there was. This PR made the ratio text-only and moved media out of it. writeBP was not adapted.
- Fix: compare c.ratio.upTo(body, x.at) >= c.r.Facts.MinCacheable. Add a test: a media prefix below the minimum with MinCacheable set, asserting the input tier.

Earlier findings

My round-4 finding:

The merged badRatio judged the media-adjusted ratio while the design stated the raw formula, and check 11 described behavior that no longer existed — fixed. badRatio (compute.go:1016) now refuses an empty body, refuses a bill below the media price, applies the 0.05 to 2.0 range to tokens per text byte, and passes an all-media body when the bill equals the media price (n == 0 → t != 0). Design check 9 (design.md:1226) states the media-adjusted formula; check 11 (design.md:1237) and rule 7 (design.md:282) now say a bill below the media price fails both checks, prices nothing, and sends every operation to Unpriced with reason ratio_range. The media_tokens detail string (compute.go:1192) says the same. I traced the four new tests by hand: TestBillBelowTheMediaPricePricesNothing (Input 100 against 1,600 media tokens: ratio_range and media_tokens, all Unpriced), TestBillEqualToTheMediaPriceDoesNotFailMediaTokens (Input 1,600: no media_tokens, and Input 1,626 prices with noChecks), TestMediaHeavyRequestIsPriced (billed 9,116 over 30,067 text bytes: 2,500 saved, noChecks), TestAllMediaBodyIsNotRefused (unit test on badRatio: 10 tokens for 2,500 media bytes passes, 9 fails).

The author's known gap: TestBillBelowTheMediaPricePricesNothing does not fail when the below-media branch and the t == 0 branch are both removed. I checked the reason and accept it. With both branches removed, the range clause still refuses that input: t = 0 and n = 107, and 0 < 0.05 x 107. The test pins the outcome (nothing priced), only not through one branch. The all-media below-media case is pinned directly by TestAllMediaBodyIsNotRefused. The behavior is covered.

Reviewer 2's round-4 findings:

Check 11's stale sentence (text prices at 0 tokens) — fixed. The same rewrite as my finding 1 above: design.md:1237 and the detail string now say nothing is priced. TestBillBelowTheMediaPricePricesNothing pins it.
The check-name lists in result.go and design §6.3 omitted bad_usage, ratio_range and breakpoints — fixed. result.go:30 and the design §6.3 CheckFailure listing name all eleven check names.

My round-4 non-blocking item: res.Checks still lists media_tokens before ratio_range. Unchanged, still cosmetic. The reason is correct.

Non-blocking

Design check 9 says a body of media only "passes when the bill covers the media price" (design.md:1231). The code passes only when the bill equals the price exactly: n == 0 → return t != 0 refuses a bill above the price (compute.go:1025). Refusing an over-billed all-media body is right (the extra tokens fit no content); the word "covers" is the loose part.
Carried from round 3: cumMedia (body.go:116) loops per call, so extent stays Θ(n²) with no candidate breakpoints.
Carried from round 3: design rule 7 still names the data-URL form as "data:...;base64," with no key list or MIME limit. The code counts only url, image_url and file_data, and only data:image/ and data:application/pdf.

What I checked

Last review: round 4, 1433ad2ea13234a939b22846a0bbd8ea1f29b785, 2026-11-04T14:56:54Z.
New commits read: 16 — ccbc09b (merge main), e70e6eb (fix: judge check 9 on the media-adjusted ratio), 191ba5d (merge PR 31), e26aa32 (fix: call addTier from interaction), 261f85c (test: fix Interaction fixtures), 3db5f9d (test: cover Interaction limits), 981a3fc (feat: report Result.Interaction), 4289446 (merge PR 29), 1b12c5e (style: align calcState fields), 06f807a (docs: qualify R26), bbe7e9f (fix: seed forks only when safe), 61c6be3 (fix: usage beats the clock), 182baa8 (merge PR 24), e72b59e (fix: no-cache usage overrides the read tier), ca6ffc5 (fix: review fixes for the minimum cacheable length), 6f645fd (fix: apply the minimum cacheable length).
Read in full: 8 reviews, 4 comments, 0 inline comments (from .review-in-reviews.txt, .review-in-comments.txt, .review-in-inline.txt; the inline list is empty).
New items since my last review: reviewer 2's round-4 review of 14:56:49Z (both findings checked against the code myself; both fixed); the author's comment of 15:12:47Z (each claim checked: the new badRatio branches, the design rewrites, the check-name lists and the four tests all hold; the known-gap reason verified and accepted); commit e70e6eb and the PR 24, 29 and 31 commits the merges brought in, each in full, checked for interaction with this PR's media ratio. The interactions I traced: writeBP against media prefixes (finding 1), readReaches (the author's media adaptation, compute.go:646, prices the previous forwarded prefix with ratio.upTo — correct), noCacheTokens against media bodies, addTier's new signature at all four callers (price twice, interaction via e26aa32, addBreak), and the clock and fork rules (no ratio involvement; covered by the new clock_fork_test.go and the four generated cases, which I read through the gen_cases.py diff and ran).
Traced by hand: the new badRatio on a media-heavy body, a bill below, equal to and above the media price, an empty body and a host-built all-media body; all four new tests' numbers; writeBP on a media prefix below and above the minimum; the interaction guards (rescue present, no SharedWarm, span at or before the ping extent, read tier only, DryRun out) against the design §6.2 "What v0 reports" text; the out-of-order At rule in phaseAndBoundary and nextState.
Ran gofmt -l ., go vet ./... and go test ./... at head ccbc09b. All clean.
Checked the public API: the new names are fields of already-aliased types (Result.InteractionCaused, EntryFacts.MinCacheable), so accountant.go needs no change; the imports stay standard-library only.

boazdavid · 2026-11-04T15:22:18Z

Status after review round 5. Both reviewers blocked head ccbc09b on one finding. Nothing is merged from this PR. Main has moved: PR 25 merged (d3b2135).

The open finding (both reviewers): writeBP (compute.go ~944) decides whether a prefix reaches MinCacheable with c.ratio.of(body.cumBytes(x.at)). That counts media bytes at the text ratio. Everywhere else the library prices media at 1 token per 250 bytes. A prefix with an image can look far longer than it is, so a write is booked where the truth is input. Both reviewers traced a concrete case. The merge with PR 24 caused it: writeBP was not adapted when media left the ratio.

Fix, one line plus a test: c.ratio.upTo(body, x.at) >= c.r.Facts.MinCacheable, and a test with an image in the prefix, beside TestACutInAClientPrefixBelowMinCacheableIsInput.

Also open: TestBillBelowTheMediaPricePricesNothing does not fail alone when its branch is removed. interaction over a span that contains media has no test.

The review budget is spent. A person decides how to finish this PR.

writeBP estimated a breakpoint's prefix with ratio.of(cumBytes), which counts base64 media bytes at the text ratio. A media prefix below EntryFacts.MinCacheable was then judged cacheable, and a saving that was input got booked as a cache write. Use ratio.upTo, as every other conversion site does. Also say that an all-media body passes only when the bill equals the media price. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: DAVID BOAZ <DAVIDBO@il.ibm.com>

boazdavid · 2026-11-04T15:24:36Z

Reply to the round-5 reviews of both reviewers.

Finding 1 (both reviewers): writeBP priced media at the text ratio. Fixed in f9343b7.

writeBP now compares c.ratio.upTo(body, x.at) with MinCacheable.
No other non-test call of ratio.of stays in internal/engine.
New test: TestAMediaPrefixBelowMinCacheableIsInput. The prefix has a 250,000-byte image (1,000 tokens) and the true prefix is 2,667 tokens, below 4096. Before the fix the test fails with CacheWrite5m: 300. After the fix it gives Input: 300.

Non-blocking (both reviewers): "covers" in design check 9. Fixed in f9343b7. The text now says that an all-media body passes only when the bill equals the media price, and that a bill above that price fails. The comment on TestAllMediaBodyIsNotRefused says the same.

Not done in this PR: the cumMedia prefix sum and the MIME list in rule 7. Both stay non-blocking.

gofmt -l ., go vet ./... and go test ./... are clean at f9343b7.

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	6 of 6	`f9343b7232ba`	approve

Verdict: approve

The round-5 finding is fixed. I verified the fix and the new test by hand. I found no new defect in commit f9343b7.

Findings

None.

Earlier findings

My round-5 finding (and reviewer 2's round-5 finding, the same defect):

writeBP priced media bytes at the text ratio for the minimum-cacheable decision — fixed. writeBP (compute.go:944) now compares c.ratio.upTo(body, x.at) with MinCacheable. upTo prices media at the fixed 250 bytes a token, as every other conversion site does. I grepped internal/engine: no non-test call of ratio.of remains outside body.go, where tokensOf passes it text bytes only. I traced the new test, TestAMediaPrefixBelowMinCacheableIsInput, by hand: system 1,000 B, an image of 250,000 media bytes, m0 cut from 600 to 300 B, MinCacheable 4096, ratio 1.0 (billed 2,367 = 1,000 read + 1,367 input). The prefix at the messages[1] breakpoint is of(1,667) + 1,000 = 2,667 tokens, below 4096, so the 300 saved tokens are Input, and noChecks holds (sum 300 = 300; conservation want = 2,667 − 2,367 = 300, net = 300, d = 0). On the old code the same prefix priced as of(251,667) = 251,667, the breakpoint qualified, and the test fails with CacheWrite5m: 300. The fix is real and the test pins it.

My round-5 non-blocking item (design check 9 said "covers" where the code passes only an exact bill): fixed. design.md:1231 now says an all-media body "passes only when the bill equals the media price" and that a bill above it fails because the extra tokens fit no content. The comment on TestAllMediaBodyIsNotRefused says the same.

Reviewer 2's round-5 non-blocking items, checked myself: the "covers" wording is fixed as above; cumMedia and the rule 7 MIME list stay open, and the author says so in the comment of 15:24:36Z; TestAllMediaBodyIsNotRefused still calls badRatio directly (a full Compute test would pin the validate wiring too); res.Checks still lists media_tokens before ratio_range, cosmetic.

Non-blocking

Carried since round 3, agreed open by the author: cumMedia (body.go) loops per call, so extent with no candidate breakpoints stays Θ(n²). A media prefix sum beside b.cum removes the loop.
Carried since round 3, agreed open by the author: design rule 7 names the data-URL form as "data:...;base64," with no key list or MIME limit. The code counts only url, image_url and file_data, and only data:image/ and data:application/pdf.
The author's own status note (comment of 15:22:18Z): interaction over a span that contains media has no test. The path is media-aware now — the span's tokens come from price (covered by the size_tokens_test.go tests) and the tier from the fixed addTier/writeBP (covered by TestAMediaPrefixBelowMinCacheableIsInput) — but no test pins the composition. A one-line image inside exactInteraction's client body would.
TestAllMediaBodyIsNotRefused tests badRatio directly, not through Compute.

What I checked

Last review: round 5, ccbc09ba8fb3d596ebe4a29adb5c6f96161f97e4, 2026-11-04T15:20:11Z.
New commits read: 1 — f9343b7 (fix: price media at the media rate in the minimum-cacheable test).
Read in full: 10 reviews, 6 comments, 0 inline comments (from .review-in-reviews.txt, .review-in-comments.txt, .review-in-inline.txt; the inline list is empty).
New items since my last review: reviewer 2's round-5 review of 15:19:43Z (its finding is the same defect as my round-5 finding; I verified the fix myself above, and I add no new proof); the author's status comment of 15:22:18Z (the summary of the open items is accurate; the two "also open" items are the mutation-isolation gap I checked and accepted in round 5, and the untested interaction-with-media path, now under Non-blocking); the author's fix comment of 15:24:36Z (each claim checked: the writeBP change, the absence of other ratio.of calls, the new test's numbers, the design wording — all hold); commit f9343b7 (full diff, 4 files, and each changed file around the change).
Traced by hand: writeBP with the fixed upTo on a media prefix below and above the minimum; the full TestAMediaPrefixBelowMinCacheableIsInput trace (ratio, read extent, B*, sum, conservation, tier); the design check 9 sentence against the badRatio code (n == 0 → t != 0).
Ran gofmt -l ., go vet ./... and go test ./... at head f9343b7. All clean.
Checked the public API: the commit changes no exported name; the imports stay standard-library 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	6 of 6	`f9343b7232ba`	approve

Verdict: approve

No findings. The round-5 finding is fixed with the exact one-line change both reviewers proposed, plus a mutation-sensitive test. I found no new defect in f9343b7.

Earlier findings, status

My round 5, finding 1 (writeBP priced media at the text ratio). Fixed in f9343b7.

writeBP (compute.go:944) now compares c.ratio.upTo(body, x.at) with EntryFacts.MinCacheable. upTo is the media-aware prefix (text at the ratio, media at 1 token for 250 bytes), the same form extent, readReaches and conservation use.
I grepped internal/engine: no non-test call of ratio.of remains. The author's claim in the comment of 2026-11-04T15:24:36Z holds.
The new test TestAMediaPrefixBelowMinCacheableIsInput pins it. I traced both ways. Fixed code: the client prefix at the breakpoint is sys 1,000 + image 67 text + 1,000 media + m0 600 = 2,667 tokens, below MinCacheable 4,096, so the cut's 300 tokens are Input. Old code: of(cumBytes) = of(251,667) ≈ 251,667 ≥ 4,096, so the test fails with CacheWrite5m: 300. The test is mutation-sensitive. The ratio is exactly 1.0 (billed 2,367 less media 1,000 over 1,367 text bytes), so the asserted Input: 300 is exact, and noChecks holds.
The same fix covers the two side effects I and reviewer 1 named: a break's moved tokens (addBreak) and interaction's read-tier guard, which use the same writeBP.

Reviewer 1's round 5, finding 1. The same defect, found independently. Fixed by the same line. I checked it myself; nothing to add.

My round 5, non-blocking ("covers" in design check 9). Fixed. design.md:1231 now says an all-media body "passes only when the bill equals the media price. A bill above it fails this check, because the extra tokens fit no content." That matches the code (n == 0 → refuse when t != 0). The comment on TestAllMediaBodyIsNotRefused says the same.

Carried non-blocking items. The author states in the 15:24:36Z comment that the cumMedia prefix sum and the MIME list in rule 7 stay non-blocking and are not done in this PR. I agree they do not block.

What I checked

Last review: round 5, ccbc09ba8fb3d596ebe4a29adb5c6f96161f97e4, 2026-11-04T15:19:43Z.
New commits read: 1 — f9343b7 (fix: price media at the media rate in the minimum-cacheable test). I read its full diff: compute.go (the writeBP line), docs/design.md (the check 9 sentence), mincacheable_test.go (the new test), size_tokens_test.go (the comment fix).
Read in full: 9 reviews, 5 comments, 0 inline comments (from the three files).
New items since my last review: reviewer 1's round-5 review of 15:20:11Z (its finding is the same defect as my round-5 finding; I verified the fix myself rather than relying on either claim); the author's status comment of 15:22:18Z (budget note, no code claim); the author's fix comment of 15:24:36Z (each claim checked: the one-line fix, the ratio.of grep, the test's numbers and its failure before the fix, the doc wording — all hold); commit f9343b7 itself.
Traced by hand: the new test's ratio (1.0 exact), its prefix arithmetic (2,667 below 4,096), and its behavior under the old of(cumBytes) form (a write, so the test fails); the design check 9 sentence against badRatio's n == 0 branch; the PR's full diff against the base 191ba5d (the same 10 files as round 5, plus the new test in mincacheable_test.go; no test/cases change, no new exported name, standard-library imports only).
Ran at head f9343b7: gofmt -l . (empty), go vet ./..., go test ./.... All clean.

Non-blocking, for the record

cumMedia loops over positions on each call, so extent with no candidate breakpoints is O(n²). A media prefix sum beside b.cum removes it. Deferred by the author; carried since round 3.
Design rule 7 names the data-URL form as "data:...;base64," without the two accepted prefixes (data:image/, data:application/pdf). Deferred by the author; carried since round 3.
TestAllMediaBodyIsNotRefused calls badRatio directly, not through a full Compute. The wiring (validate → skipsPricing → Unpriced) is pinned only on the text-byte path.
res.Checks lists media_tokens before ratio_range. Cosmetic; the reason and the detail string are correct.
interaction over a span that contains media has no test (the author's own note in the 15:22:18Z comment). Its guards use the now-correct writeBP, so the risk is low.

boazdavid commented Nov 4, 2026

View reviewed changes

boazdavid added 2 commits November 4, 2026 17:11

Merge remote-tracking branch 'origin/main' into fix/size-tokens-and-m…

ccbc09b

…edia-11

boazdavid commented Nov 4, 2026

View reviewed changes

boazdavid merged commit d31126b into main Nov 4, 2026
2 checks passed

boazdavid deleted the fix/size-tokens-and-media-11 branch November 4, 2026 15:30

boazdavid mentioned this pull request Nov 4, 2026

One bytes-per-token ratio for the whole body misprices images, JSON and code #11

Closed

Conversation

boazdavid commented Nov 4, 2026

Summary

What changed

How it was tested

Open questions

Uh oh!

boazdavid left a comment

Choose a reason for hiding this comment

Findings

Non-blocking

What I checked

Uh oh!

boazdavid left a comment

Choose a reason for hiding this comment

Findings

Non-blocking

What I checked

Uh oh!

boazdavid commented Nov 4, 2026

Uh oh!

boazdavid left a comment

Choose a reason for hiding this comment

Findings

Earlier findings

Non-blocking

What I checked

Uh oh!

boazdavid left a comment

Choose a reason for hiding this comment

Findings

Earlier findings

Non-blocking

What I checked

Uh oh!

boazdavid commented Nov 4, 2026

Uh oh!

boazdavid left a comment

Choose a reason for hiding this comment

Findings

Earlier findings

Non-blocking

What I checked

Uh oh!

boazdavid left a comment

Choose a reason for hiding this comment

Findings

Earlier findings

Non-blocking

What I checked

Uh oh!

boazdavid commented Nov 4, 2026

Uh oh!

boazdavid left a comment

Choose a reason for hiding this comment

Verdict: blocking

What I checked

Findings

Finding 1: check 11 in design §6.5 says text prices at 0 tokens when the bill is below the media price. At head, that bill refuses all pricing.

Finding 2: the check-name lists in result.go and design §6.3 omit bad_usage, ratio_range and breakpoints.

Earlier findings, status

Non-blocking

Uh oh!

boazdavid left a comment

Choose a reason for hiding this comment

Findings

Earlier findings

Non-blocking

What I checked

Uh oh!

boazdavid commented Nov 4, 2026

Uh oh!

boazdavid left a comment

Choose a reason for hiding this comment

Findings

Earlier findings, status

The author's known gap, explained

Non-blocking

What I checked

Uh oh!

Finding 2: the check-name lists in `result.go` and design §6.3 omit `bad_usage`, `ratio_range` and `breakpoints`.