Skip to content

fix(llm): reconcile cache-hit accounting between the two usage meters - #23

Merged
oratis merged 1 commit into
mainfrom
claude/distracted-dirac-24cf7a
Aug 9, 2026
Merged

fix(llm): reconcile cache-hit accounting between the two usage meters#23
oratis merged 1 commit into
mainfrom
claude/distracted-dirac-24cf7a

Conversation

@oratis

@oratis oratis commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Summary

Follow-up to #20. That PR stopped cache hits from billing real money, but closed only half the gap — and the half it left open is the one that matters for /api/usage being a billing basis.

The two meters still disagreed — on tokens

#20 zeroed usdCents while deliberately keeping the token counts "for visibility". Those tokens still reach agent_runs. But getStats() skips cache hits entirely, so:

  • money agreed at zero ✅
  • tokensSUM(agent_runs.input_tokens) ran ahead of the in-memory counters by exactly the cached volume, with nothing anywhere recording why ❌

A ledger meant to back tiered billing was unexplainable from the other meter: it just looked inexplicably larger.

Cache hits now land in their own stats.cached bucket, whose usdCents is spend avoided rather than incurred. Two identities hold per process:

SUM(agent_runs.cost_usd_cents) === totalUsdCents
SUM(agent_runs.input_tokens)   === Σ byProvider[*].inputTokens + cached.inputTokens

Money agrees outright; tokens agree once cached volume is added back. I kept #20's cost-based semantics (bill for money that left the building) rather than reopening the pricing question — switching to usage-based is a product decision, and it belongs in complete(), which is the only place that knows a response was cached.

A zero cost was being re-inflated into a charge

content-video.js read res.usage?.usdCents || 25. Zero is a real answer — a cache hit reports it by design, and so does any live call on a model absent from PRICING, or one small enough to round down. || read all three as "cost unknown" and charged the 25¢ estimate anyway, billing for spend that never happened. Now ??, so only genuinely absent usage falls back.

Docs

usage-ledger.js now documents the semantics it depends on, so nobody "repairs" a well-cached month (many tokens, little spend) by re-deriving cost from token counts.

Scope worth recording

The original over-billing was latent, not active. Only calls with temperature 0-or-absent are cacheable, and the sole such call site — community.js:210 (inbox classify) — discards res.usage entirely. Every other one of the 15 call sites passes a non-zero temperature, so the cache is never consulted. No cached cost has ever reached agent_runs or /api/usage.

This is contract hygiene for the 13 of 15 call sites that do forward usage — the trap springs the moment anyone adds a cacheable call.

Test plan

  • npm test 665/665 (9 new)
  • Each new test verified to fail without its fix: reverting ??|| fails 1; removing recordCacheHit() fails 4
  • One test guards against zeroing the stored cache entry instead of a per-hit copy, which would silently decay cachedUsdCents to zero after the first read
  • No client changes — the new stats.cached field rides into /api/agents/cost's llmStats additively, and no client code reads it

Not done deliberately

Option (c) from the original write-up — a persisted cached_usd_cents column on agent_runs for gross-vs-net reporting. Under the shipped cost-based semantics it isn't needed for correctness, and cachedUsdCents doesn't reach the DB anyway (agent-runtime/index.js picks only tokens + usdCents). It would need a migration on the dual SQLite/Postgres path — worth its own decision.

Unrelated, spotted in passing

server/usage-ledger.js contains two raw NUL bytes (not the \0 escape) used as a map-key separator, so git classifies the file as binary — git diff shows Bin 9679 -> 10779 bytes instead of a real diff, and git blame is useless on it. It predates this branch (identical in HEAD, from #17) and is invisible in review precisely because the file can't be diffed. Being fixed separately; flagging it here because it's why this PR's diff for that file renders as binary.

🤖 Generated with Claude Code

#20 stopped cache hits from billing real money, but closed only half the
gap. It zeroed `usdCents` while deliberately keeping the token counts "for
visibility" — and those tokens still reach `agent_runs`. Since `getStats()`
skips cache hits entirely, money agreed at zero while
SUM(agent_runs.input_tokens) ran ahead of the in-memory counters by exactly
the cached volume, with nothing anywhere recording why. The ledger that is
meant to back tiered billing was unexplainable from the other meter.

Cache hits are now counted in their own `stats.cached` bucket, whose
`usdCents` is spend avoided rather than incurred. Two identities hold per
process: SUM(cost_usd_cents) === totalUsdCents, and SUM(input_tokens) ===
Σ byProvider[*].inputTokens + cached.inputTokens. Money agrees outright;
tokens agree once cached volume is added back.

- llm: `stats.cached` + `recordCacheHit()`, cleared by `resetStats()`;
  the stored cache entry is never mutated, so `cachedUsdCents` no longer
  risks decaying to zero on a repeat hit
- content-video: `res.usage?.usdCents ?? 25`, not `||`. Zero is a real
  answer — a cache hit, or any model absent from PRICING — and `||`
  charged the 25¢ estimate anyway, billing for spend that never happened
- usage-ledger: document the cost-based semantics, so nobody "repairs" a
  well-cached month by re-deriving cost from token counts
- llm: export `cacheKey` so hit accounting is testable without a network

Scope worth recording: the original over-billing was latent, not active.
Only `temperature` 0-or-absent calls are cacheable, and the one such call
site (community.js classify) discards `res.usage`. No cached cost ever
reached the ledger. This is contract hygiene for the 13 of 15 call sites
that do forward usage — the trap springs on the next cacheable call added.

Tests: 9 new. Each was verified to fail without its fix (reverting `??`
fails 1; removing `recordCacheHit` fails 4). npm test 665/665.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@oratis
oratis merged commit d8eac58 into main Aug 9, 2026
5 checks passed
@oratis
oratis deleted the claude/distracted-dirac-24cf7a branch August 9, 2026 15:07
oratis added a commit that referenced this pull request Aug 9, 2026
`summarize()` 用 NUL 作 `month`/`agent_id` 复合键的分隔符,这个选择本身是对的
(NUL 不可能和 `YYYY-MM` 或任何 agent_id 冲突)。问题出在编码:两个 NUL 是以
**裸字节 0x00** 直接写进源文件的,而不是转义序列。

由于 NUL 落在文件前 8000 字节内,git 把整个文件判定为二进制,后果有两层:
- `git diff` 只显示 `Bin 9679 -> 10779 bytes`,`git blame` 完全失效,
  `file` 报 `data` —— 这个 bug 把自己从 review 里藏了起来(它是 #17 带进来的)
- 更糟的是**文件无法三方合并**:#23 在 main 上也改了这个文件,于是本分支
  第一版直接 CONFLICTING,git 对二进制文件没有合并策略可用

改动:
- 两处裸 0x00 换成 `\u0000` 转义 —— 注释里的复合键说明,以及 `cellKey` 模板字符串
- 运行时字符串完全不变:模板字面量里的 `\u0000` 求值仍是 U+0000,
  `getWorkspaceUsage` / `getPlatformUsage` 的分组逐字节一致

验证:
- 文件 NUL 计数 0;`file` 现在报 `Unicode text, UTF-8 text`
- 全仓受跟踪文件扫描,没有第二个文件有同样问题
- `npm test` 678/678 通过;`usage-ledger.test.js` 单独跑 29/29
- 对抗性分组检查:agent_id `07-x` 在 `-` 分隔符下会与 `2026-08-07`/`x` 撞键,
  用 NUL 分隔后仍正确分桶(同月同 agent 合并、不同 agent 分离、总计正确)

已 rebase 到当前 main(含 #23 对本文件的改动),冲突按「取 main 版本 + 重新施加
同一处转义」解决。

注意:本次 diff 对 HEAD 仍显示 `Bin`,因为 git 只要有一侧是二进制就走二进制路径,
而旧 blob 确实含 NUL。合并后该文件的 `git diff` / `git blame` 恢复正常。
#29 的 `.gitattributes` 会让这类 diff 即使跨 NUL blob 也强制按文本渲染。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oratis added a commit that referenced this pull request Aug 9, 2026
…lake

Review follow-up on this PR. The numbers went stale during review: #23
(+9 tests, +1 file) and #24 (+7, +1) merged while this branch sat open,
so "656 / 69 files" was already wrong by the time it could land — the
exact failure mode the PR exists to fix.

Measured on main at 7df4183: 678 tests across 71 files. Corrected in all
five places (CLAUDE.md ×2, memory.md §5.2 / §7.3 / Last-reviewed footer,
which also still said "post #20").

Also documents why a clean checkout can show 2-5 red files: `npm test` is
`node --test`, which runs files concurrently against the one shared
influencex.db at the repo root, so writes collide and report
`{ code: 'SQLITE_BUSY' }` on a rotating cast of files. It reproduces on
main with no changes applied, and the triage step is a serialized re-run
(--test-concurrency=1 → stable 678/678). Without this written down the
next session reads the flake as its own regression — which is what the
rest of this PR is trying to prevent.

Verified: 678/678 serialized on main, and on main + #25 + this branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oratis added a commit that referenced this pull request Aug 9, 2026
…binary (#30)

The compound cache key in summarize() used a raw 0x00 byte written
directly into the source — a literal control character, not the escape
sequence. Git classifies a file with a NUL in its first 8000 bytes as
binary, so `git diff` reported "Bin 9679 -> 10779 bytes" instead of the
change, `git blame` stopped working, and `file` reported `data` rather
than UTF-8 text.

Worth noting how it got in: I merged it in #17, and the reason nobody
caught it is the bug itself — the file was undiffable, so review had
nothing to look at.

The separator choice is fine: NUL cannot collide with a YYYY-MM month or
any agent_id. Only the encoding was wrong. Both occurrences are now the
escape sequence, which evaluates to the same U+0000 character, so
getWorkspaceUsage / getPlatformUsage group byte-identically.

Verified: 0 NUL bytes remain, `file` reports UTF-8 text, the template
still evaluates to a string containing U+0000, and an end-to-end
aggregation still separates same-month agents correctly. npm test
678/678.

Found by the background session on PR #27, which was branched before #23
and would have reverted that PR's documentation on this file; applied
directly to main instead.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
oratis added a commit that referenced this pull request Aug 9, 2026
* docs: 修正入门文档里已经失效的运维事实

排查一份 brand-voice embedding 的 bug 报告时发现问题本身已经在 #20
(`0c5323b`)修掉了,但顺手核对上下文的过程中撞上好几处文档与现状不符 ——
每一处都会让下一个会话走一段冤枉路,所以单独收一个 docs PR。

改了什么:

- **prod DB 口令来源**:CLAUDE.md 和 memory.md 有 5 处说密码"在 .env 里",
  但这台机器上根本没有 .env(只有 .env.example)。全部改为从 Secret Manager
  取:`gcloud secrets versions access latest --secret=DATABASE_URL`。
- **测试数字**:CLAUDE.md 同时写着 377 和 234,memory.md 写 234 —— 实际是
  656 个服务端测试 / 69 个文件、~1 秒。前端测试也不再是"4 个组件测试"或
  "还没有",是 13 个 vitest 文件 + 5 条 Playwright(3 个 spec)。
- **Sentry / OTEL**:memory.md §5.4 还写着"没有(Sprint 1 待加)",与同文件
  §6 已关闭表和 CLAUDE.md 自相矛盾。两者早已接入,只是要配 DSN / OTLP 才上报。
- **brand_voices 生产现状**:借这次机会连 prod 只读查了一次 —— 表 0 行。
  也就是说 embedding 写入路径坏了这么久没有造成数据损失,不需要 backfill。
  这条结论写进 memory.md §6,省得以后有人再问一遍。
- 新增一条"仍然成立":embedding 只在 `POST /api/brand-voices` 创建时写,
  目前没有 update 路由所以无害,但以后加编辑接口必须重新 embed。

验证:纯文档改动,未动任何代码路径;`npm test` 656/656 通过。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* docs: correct this PR's own test counts, and record the SQLITE_BUSY flake

Review follow-up on this PR. The numbers went stale during review: #23
(+9 tests, +1 file) and #24 (+7, +1) merged while this branch sat open,
so "656 / 69 files" was already wrong by the time it could land — the
exact failure mode the PR exists to fix.

Measured on main at 7df4183: 678 tests across 71 files. Corrected in all
five places (CLAUDE.md ×2, memory.md §5.2 / §7.3 / Last-reviewed footer,
which also still said "post #20").

Also documents why a clean checkout can show 2-5 red files: `npm test` is
`node --test`, which runs files concurrently against the one shared
influencex.db at the repo root, so writes collide and report
`{ code: 'SQLITE_BUSY' }` on a rotating cast of files. It reproduces on
main with no changes applied, and the triage step is a serialized re-run
(--test-concurrency=1 → stable 678/678). Without this written down the
next session reads the flake as its own regression — which is what the
rest of this PR is trying to prevent.

Verified: 678/678 serialized on main, and on main + #25 + this branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
oratis added a commit that referenced this pull request Aug 9, 2026
…nded (#31)

Two defects found while reviewing the #23#30 batch, both now on main.

**Wrong PR credited.** #29's .gitattributes header and memory.md §6 both
say #27 fixed the usage-ledger NUL bytes. #27 was closed as a duplicate —
#30 landed the identical fix (verified byte-for-byte identical trees).
Anyone following the reference lands on a closed PR with an empty diff.

**The count treadmill.** #26 corrected five hardcoded test counts from
234/377/656 to the then-accurate 678. #28 merged minutes later and made it
679, so a PR whose entire purpose was de-staling docs shipped a number that
was stale on arrival. Five copies of a figure that changes on every
test-bearing merge cannot stay right.

Counts are now stated as a magnitude pinned to a commit ("~680 as of
c7c7d5b") with an instruction to measure instead of cite, and the two
places that only needed "all green" no longer carry a number at all. The
prose that never had one ("Vitest files under client/src/{...}") aged fine
through this whole batch, which is the argument.

memory.md §5.2 records the pattern so the next doc pass doesn't reinstate
it. The remaining 234/656 mentions are deliberate — they are the history
being explained, not live facts.

Verified: 679/679 serialized on this branch; grep confirms no stale
hardcoded count or bare #27 reference survives.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant