The unparseable-body arm takes its bytes off spool_bytes (#905 finding 3)

[philcunliffe]

Issue #905 records five deferred findings from the review of PR #851 (branch fix/issue-843, head 1378a82c). PR #851 is still open, so four of the five describe code that exists only on that unmerged branch and cannot be reproduced, tested, or fixed from master. This PR fixes the one finding that is present on master today, and states plainly why each of the others stays deferred.

Fixed: finding 3, the unparseable-body arm leaves spool_bytes drifted high

What was wrong. loadSpooledBodies deletes a body file it cannot parse immediately (LLP 0252 #project-then-delete: an undeleted body is a raw prompt sitting on disk) and counts it in unparseable. The bytes it removed were reported nowhere, so nothing ever subtracted them from state.spoolBytes.

Root cause. The read reports bytes for one arm only: consumedBytes, the files it hands back for the caller to delete after the batch's writes land. The unparseable arm deletes inside the read and returns no byte figure, so the call site in source.js had nothing to subtract. The gauge then over-reported until the next spool sweep restated it, and the sweep runs on a 60s timer (SPOOL_SWEEP_INTERVAL_MS), so hyp status can read a spool_bytes that counts content already off the disk for most of a minute. LLP 0257 S16 says the published number is the spool's current byte size, so this is a violation of the spec clause, not just noise.

Fix. loadSpooledBodies now returns unparseableBytes, counted only when the unlink actually succeeded (an unremovable file is still occupying the cap, and subtracting it would under-report in the other direction, which is the same reasoning PR #851 applied to deleteSpooledBodies). The listener subtracts it at the call site, immediately beside where it already accumulates bodiesUnparseable, and the byte total also rides the existing claude.telemetry.body_unparseable warning and the batch span.

Regression test (test/plugins/claude-telemetry-spool-accounting.test.js), two cases, the first driven through the real transport (a listener on an ephemeral port, OTLP/JSON over the wire, a fake gateway behind it):

1. an unparseable body takes its bytes off spool_bytes when it is deleted - seeds the spool with one truncated and one valid body before start so the start-time sweep primes the gauge, POSTs a batch referencing both, then asserts the spool directory is empty and the published spool_bytes equals the bytes actually on disk.
2. loadSpooledBodies reports the bytes its unparseable arm removed - pins the reporting itself, including that the parseable file's bytes stay in consumedBytes and its file stays on disk until the writes land.

Before/after (ground-truth gate). With the two source files stashed and the test file kept:

not ok 1 - an unparseable body takes its bytes off spool_bytes when it is deleted
  expected: 0
  actual: 61
not ok 2 - loadSpooledBodies reports the bytes its unparseable arm removed
  expected: 61
# tests 2  # pass 0  # fail 2

With the fix applied: # tests 2 # pass 2 # fail 0.

Full suite: npm test -> # tests 4486 # pass 4485 # fail 0 # skipped 1. npm run typecheck -> clean. Both @ref anchors used here (LLP 0257#status-and-health) resolve to a real section, and S16 is what the fix restores rather than a broader claim the spec does not make.

Still deferred, with reasons

Finding 1 (the catch-block comment that claims a retry "writes the same assistant rows with no attributes.usage"). That comment was introduced by PR #851 and does not exist on master; there is nothing here to correct. It has to be fixed on fix/issue-843 or after #851 merges, and the issue itself ties it to #879 landing.

Finding 2 (snapshot/restore widens a cross-request race on the shared usage index). restoreUnclaimedUsage and the usageBeforeProjection snapshot exist only on fix/issue-843. On master there is no restore, so the described failure (a failed request's restore resurrecting usage another concurrent request already spent) cannot be reproduced from here and no test could fail-then-pass. Fixing it also means choosing between serializing the OTLP handler and scoping the index per batch, which is a design call on top of an unmerged change.

Finding 3's sibling arms. The drop arm's identical drift (suppressSession deleting bodies without subtracting) is already fixed on fix/issue-843; touching it here would only manufacture a conflict, so this PR changes the unparseable arm only.

Finding 4 (restored usage entries land at the tail, inverting eviction age). The issue records this as a deliberate tradeoff inside a correct fix, bounded by USAGE_INDEX_LIMIT, to "revisit only if an age-aware structure is introduced". Nothing to fix; it also lives only on the unmerged branch.

Finding 5 (LLP 0257 has no clause for retry-input survival). LLP 0257 is Accepted. Per CLAUDE.md, changing what an Accepted doc settled requires minting a new request that extends it, and the issue conditions that on a maintainer agreeing the requirement belongs in the spec. That is a human decision, not an automatable one, and the two @refs it would restore are on the unmerged branch. Left for a maintainer.

Because four of the five findings cannot be reproduced on master, #905 should stay open after this merges, to be re-triaged against fix/issue-843 once PR #851 lands.

Residual risk

source.js is touched three lines above a hunk PR #851 also edits, so the second of the two to merge may need a trivial conflict resolution. The behaviour change is confined to a gauge: no row, no dataset, and no deletion decision moves.

Fixes #905

[philcunliffe]

Duplicate fix: this PR and #912 close the same defect

What neutral was doing. The issue-fix reconciler dispatched one worker per open neutral:fix issue this tick. Issue #905 (deferred findings from PR #849) and issue #904 (deferred findings from PR #851) each carried the same deferred finding, so two workers, blind to each other by design, both fixed it. This PR is the result for #905; PR #912 is the result for #904.

Why it cannot proceed. Both PRs edit the same two files with the same intent, so merging both would double-apply the change and the second will conflict:

• hypaware-core/plugins-workspace/claude/src/telemetry/bodies.js - both add unparseableBytes to loadSpooledBodies's return and both size the unparseable-body deletion.
• hypaware-core/plugins-workspace/claude/src/telemetry/source.js - both subtract that figure from state.spoolBytes at the call site.
• Each brings its own regression test for the same behaviour: test/plugins/claude-telemetry-spool-accounting.test.js here, test/plugins/claude-telemetry-unparseable-body.test.js on An unparseable body leaves spool_bytes reporting bytes it already deleted (#904) #912.

Neither is wrong. They differ in emphasis:

	#912 (fix/issue-904)	#913 (this PR, fix/issue-905)
Test reach	listener + seeded body, plus a reader-only case	full transport (ephemeral port, OTLP/JSON over the wire), plus a reader-only case
Unlink failure	only counts bytes once fs.rm resolves	same
@ref target	LLP 0252#project-then-delete docblock extended	LLP 0257#status-and-health, chosen deliberately over LLP 0253#byte-cap

What neutral needs from you. Which one should land?

1. Keep An unparseable body leaves spool_bytes reporting bytes it already deleted (#904) #912, close this PR, and leave Follow-up: deferred review findings from PR #849 #905 open so its four remaining deferred findings can be re-triaged against fix/issue-843 once Deferred #843 findings: listener counters, the refusal log, and the usage a failed batch consumed #851 merges.
2. Keep The unparseable-body arm takes its bytes off spool_bytes (#905 finding 3) #913 (this PR) for the wider transport-level test and the LLP 0257 reference, and close An unparseable body leaves spool_bytes reporting bytes it already deleted (#904) #912.
3. Keep one and graft the other's test onto it - say which is the base.

Neutral is not choosing between two maintainer-equivalent implementations on its own. It has left #912 riding the normal ladder purely because it was opened first, not because it judged it better.

Also worth knowing: both branches touch source.js within a few lines of a hunk that the still-open PR #851 (fix/issue-843) also edits, so whichever lands after #851 will need a small conflict resolution.

How to unstick. Reply with a comment on this PR (or push to the branch); neutral monitors this thread and will re-engage with your guidance on its next tick.