Follow-up: deferred review findings from PR #851

[philcunliffe]

Triage of PR #851 (head 1378a82c) hit the review-round cap with findings still open. Every residual finding was re-verified against the head and judged non-blocking, so the PR is cleared to merge and the deferred items are recorded here. Refs #851, #843.

Deferred findings

1. A retried batch after a failed dataset write recovers nothing (pre-existing on master)

The write-failure retry loss itself is already tracked as #879: aiGatewayRowsFromProjectedExchange commits state.seenMessages during row expansion (hypaware-core/plugins-workspace/ai-gateway/src/message_projector.js:774 at 1378a82c) with no rollback when storage.appendRows throws (hypaware-core/plugins-workspace/ai-gateway/src/exchange_writer.js:59-66), so the exporter's retried batch projects zero rows. PR #851's restoreUnclaimedUsage restore is the correct usage-index half and becomes load-bearing once the writer is made atomic; it does not fix the loss on its own.

Deferred remainder for this issue: the catch-block comment at hypaware-core/plugins-workspace/claude/src/telemetry/source.js:688-698 still claims a retry "writes the same assistant rows with no attributes.usage". It does not (the retry writes nothing, per #879). Correct the comment when #879 lands, or sooner.

2. Snapshot/restore widens a cross-request race on the shared usage index (round 1, finding 5)

hypaware-core/plugins-workspace/claude/src/telemetry/source.js:661 snapshots usageByRequestId; src/core/otlp/server.js awaits handlers per request with no serialization, so overlapping /v1/logs POSTs can interleave and a failed request's restore can resurrect usage another request already spent. It leaks a capped slot until eviction rather than corrupting rows (request_id never recurs). Fix means serializing the handler or scoping the index per batch.

3. The unparseable-body arm leaves spool_bytes drifted high (round 1, finding 6)

hypaware-core/plugins-workspace/claude/src/telemetry/bodies.js:104-108 deletes an unparseable body with fs.rm(file, { force: true }) and counts it in unparseable, but never subtracts its bytes from state.spoolBytes, so the gauge over-reports until the next sweep restates it. Same drift class findings 9b/3 fixed for the drop and projected arms. Fix means returning an unparseableBytes field from loadSpooledBodies and deducting it at the call site (hypaware-core/plugins-workspace/claude/src/telemetry/source.js:~677).

4. Restored usage entries land at the tail, inverting eviction age (round 1, finding 4 residual)

Round 2's restoreUnclaimedUsage (hypaware-core/plugins-workspace/claude/src/telemetry/projection.js:301-306) fixed the cap bypass: the restore now re-applies trimUsageIndex. The residual nuance is ordering: restored (older) entries re-insert at the tail of the Map, so they now sort as newest and the next trim sheds genuinely fresh unclaimed entries first. Deliberate in the round 2 fix (a claimed entry the retry needs must survive the re-trim) and bounded by the cap; revisit only if an age-aware structure is introduced.

5. LLP 0257 has no clause for retry-input survival (round 2 follow-up question)

The requirement the removed @refs described (a retried batch re-projects from the same inputs, so those inputs must survive a failed write) is real but stated nowhere in LLP 0257, which is Accepted. Per CLAUDE.md, if a maintainer agrees it belongs in the spec, mint a new request extending 0257 (append Extended-by: on the old doc), then restore the two removed annotations at hypaware-core/plugins-workspace/claude/src/telemetry/source.js and test/plugins/claude-telemetry-listener-accounting.test.js pointing at the new clause.

Why none of these block #851

Finding 1 is pre-existing on master in the shared writer path and unchanged by the PR (tracked in #879); 2 is a self-healing leak whose exposure predates the PR; 3 is a bounded, sweep-corrected observability drift; 4 is a deliberate tradeoff inside a correct fix; 5 is a spec-process question. None causes a new production defect if #851 merges.

[philcunliffe]

PR #912 (draft) fixes finding 3 (the unparseable-body arm's spool_bytes drift) with a failing-then-passing regression test at test/plugins/claude-telemetry-unparseable-body.test.js.

Findings 1, 2 and 4 remain deferred because they are not reachable from master: each describes code that PR #851 introduces, and #851 is still open (fix/issue-843, head 1378a82c). Verified against origin/master at b1afa7c2 - there is no catch-block usage comment to correct, no usageBeforeProjection snapshot, and no restoreUnclaimedUsage. They should be taken up as review comments on #851 or as a follow-up once it merges.

Finding 5 remains deferred as a maintainer decision: LLP 0257 is Accepted, so adding a retry-input-survival clause means minting a new request that extends it plus an Extended-by: forward-ref. Whether that clause belongs in the spec, and its wording, is the judgement call the issue routes to a human.