fix(desktop): read Usage from Runtime Host - #2641
Conversation
e76722b to
bc753ae
Compare
There was a problem hiding this comment.
Codex automated review — corrected after checking #2128 acceptance
I reviewed exact head ee488a7a44808af162200cdbf1862936435cd0fd across the Runtime Host usage authority, canonical/legacy ledger merge, Desktop multi-query projection, recovery markers, fixtures, and upgrade behavior.
Correction: I removed the original P1 about retaining Session-metadata-only usage after checking the linked #2128 acceptance criteria. That issue explicitly requires removing the legacy Session scanner and avoiding a second durable Usage authority, so treating those rows as a mandatory fallback would contradict the stated design.
One confirmed P2 remains: the five Desktop queries are not bound to a stable Host revision, so an upsert that changes tokens/cost/status without changing row count or coarse provenance can produce an internally inconsistent UsageStats; see the remaining inline finding.
A second P2 contract mismatch is visible in the same page: legacy telemetry stores totalTokens as input + output + reasoning, while canonical projection and each request row use input + output. The Host merge and Desktop projection add/show those totals directly, so reasoning-heavy legacy rows make summary/provider/model totals disagree with the request table. This should be normalized at the Usage authority (or the contract/labels must explicitly change), rather than patched independently in the renderer.
This is a cohesive usage-authority migration and I do not recommend splitting it. Current checks are green.
Disclosure: This is an automated review performed by Codex using delegated adversarial review passes and a final evidence check. It has not been independently verified by Astro-Han or another human reviewer, does not constitute human approval, and does not represent the final judgment of a human reviewer.
ee488a7 to
539ca8e
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for closing the two original consistency issues. At 539ca8e2, Desktop now requires one Host revision across all five Usage views, and the current recorder no longer adds reasoning tokens twice.
After tracing the remaining failure paths, I think the authority cutover is still incomplete at one shared boundary: Usage reads have moved to Runtime Host, but protocol compatibility, repair ownership, test producers, and legacy token provenance have not all moved with them.
-
The new required
revisionfield changes theusage.querywire schema without advancing the compatibility epoch. A new Desktop can therefore accept an older remote Host during the handshake and fail only when it decodes the first old-format Usage response. After rebasing onto currentmain, the epoch should advance beyond 20 so incompatibility is rejected before domain requests are admitted. -
A Usage query currently mutates the authority inside its own revision fence.
readCanonicalUsage()repairs pending projections, while eachrecordandclearadvances the same Usage revision that the query compares before and after reading. More than 32 pending runs exhaust the three retries; a single pending run containing both a valid attempt and an unreadable event can fail indefinitely because its marker remains and the valid attempt is recorded again on every retry. In both cases the query returnsinternal_failureinstead of the qualified result thatpendingRepairsandunreadableRecordswere designed to represent.The smallest complete fix is to run at most one bounded repair pass before entering the fenced, read-only projection. The cleaner final state would give repair its own lifecycle owner and make idempotent writes advance the revision only when stored data actually changes.
-
The
settings-usagefixture still writes only transcripttoken_usageand tool messages. Once the old scanner is deleted, those records are no longer inputs to the production Usage path, so the fixture seeds empty Host Usage stores and no longer tests the page it claims to cover. It should seed the Host-owned telemetry/model-call seam, including missing usage and an aborted tool, rather than restoring or emulating the deleted scanner. -
The legacy total-token normalization still infers provenance from whether
rawUsage.total_tokensexists. That cannot distinguish an old derived fallback from an explicitly supplied camel-casetotalTokens. For example, an explicitinput=20,output=8,reasoning=2,totalTokens=30without raw snake-case usage is silently rewritten to 28. Please define the invariant at the persistence boundary—either total is alwaysinput + output, or reported versus derived total carries explicit provenance—rather than guessing it during reads.
These are different symptoms of the same incomplete cutover. I would keep the old scanner deleted and finish the migration at the natural owners instead of adding renderer retries, fallback reads, or compatibility paths. The PR remains one cohesive Usage-authority change and does not need to be split, but I would hold the merge until these boundaries are closed and the rebased CI is green.
简体中文
感谢修复最初的两处一致性问题。在 539ca8e2 上，Desktop 已要求五个 Usage 视图使用同一个 Host revision，当前 recorder 也不再把 reasoning token 重复计入 total。
继续追踪剩余失败路径后，我认为这次 authority cutover 仍缺少最后一层闭环：Usage 读取已经迁移到 Runtime Host，但协议兼容、repair owner、测试数据生产者和历史 token provenance 还没有全部随之迁移。
-
新增必需的
revision字段改变了usage.querywire schema，却没有提升 compatibility epoch。新版 Desktop 因此可能在握手时接受旧版远程 Host，直到解析第一条旧格式 Usage 响应时才失败。rebase 到当前main后，应将 epoch 从当前的 20 继续提升，使不兼容在接收领域请求之前就被拒绝。 -
Usage 查询目前会在自己的 revision fence 内修改同一 authority。
readCanonicalUsage()会修复 pending projection，而每次record和clear都会推进查询前后正在比较的 Usage revision。超过 32 个 pending run 时会耗尽三次重试；如果一个 pending run 同时包含合法 attempt 和不可解码事件，其 marker 会保留，合法 attempt 又会在每次重试中重复写入，从而可能永久失败。两种情况最终都返回internal_failure，而不是pendingRepairs和unreadableRecords原本用于表达的不完整但可用结果。最小完整修复是在进入只读 revision fence 之前最多执行一次 bounded repair。更干净的终态是让 repair 拥有独立生命周期，并让幂等写入只在存储内容真正变化时推进 revision。
-
settings-usagefixture 仍然只写 transcript 中的token_usage和 tool messages。旧 scanner 删除后，这些记录已不再是生产 Usage 路径的输入，因此 fixture 实际留下的是空的 Host Usage stores，已经无法覆盖它声称测试的页面。应通过 Host-owned telemetry/model-call seam 写入数据，并包含 missing usage 和 aborted tool，而不是恢复或模拟已经删除的 scanner。 -
legacy total-token normalization 仍通过
rawUsage.total_tokens是否存在来推断来源。它无法区分旧的 derived fallback 和显式传入的 camel-casetotalTokens。例如显式的input=20、output=8、reasoning=2、totalTokens=30，在没有 raw snake-case usage 时会被静默改写为 28。这里应在持久化边界明确合同：要么 total 始终等于input + output，要么为 reported/derived total 保存明确 provenance，而不是在读取时猜测。
这些是同一次 cutover 尚未闭环的不同表现。我建议继续删除旧 scanner，并在自然 owner 上完成迁移，不要增加 renderer 重试、fallback read 或兼容旧路径。这个 PR 仍是一个完整、内聚的 Usage authority 变更，不需要拆分；但上述边界闭合并且 rebase 后 CI 重新通过之前，我建议先不要合并。
Disclosure: I used Codex to assist with source tracing and adversarial review. I checked the cited production paths and failure conditions and take responsibility for this review.
539ca8e to
9fe90f2
Compare
📝 WalkthroughProblem solvedDesktop Settings → Usage now reads from the reconnectable Runtime Host client instead of the legacy session scanner. The change preserves missing token data, unknown costs, aborted tool-call status, custom pricing, consistent time ranges, and snapshot consistency across summaries, aggregates, and request rows. It also adds coverage notices and removes the legacy usage authority. Source of truthThe PR extends the existing Host-owned canonical usage, telemetry, and pricing stores. It does not create a parallel Usage authority. It removes the legacy scanner, Solution scope and complexityThe solution is coherent for the stated requirements. The added complexity supports:
Deletion and simplification opportunitiesThe removed legacy usage paths are appropriate because Host-backed reads replace them. No further deletion is evident without weakening coverage for snapshot retries, repairs, provenance, pricing, token normalization, aborted tools, and UI states. Risks and validationUser-visible Usage values, statuses, pricing, notices, and table columns change. The Runtime Host compatibility epoch changes from Reads can fail after repeated snapshot changes or incomplete repairs. Consumers of removed Storage APIs require migration. The change summary reports linting, formatting, affected tests, Desktop build checks, and 129 Storybook scenarios. Direct check results are not available. The current shell context also reports no local diff, so the final required-check status remains unverified. Review-relevant risksThe reported changes affect user-visible Usage behavior. Material changes in this area require independent human review under repository policy. The reported changes affect public TypeScript contracts and Runtime Host protocol responses. Material changes in these areas require independent human review under repository policy. The reported changes affect Runtime Host compatibility and release behavior. Material changes in these areas require independent human review under repository policy. The person performing the merge reviews the final diff. A maintainer makes the final determination. WalkthroughThe change moves Desktop usage retrieval to the canonical runtime-host authority. It adds revision-fenced snapshots, bounded repairs, provenance-aware values, aborted tool states, canonical token normalization, direct E2E fixtures, and coverage-aware usage UI. ChangesCanonical usage flow
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🔵 Low · up to The Usage migration is otherwise mergeable, but a changed storage test can leave a background writer running when an assertion fails, potentially causing hung or misleading test runs; make the cleanup path unconditional before relying on that regression test. Sequence Diagram(s)sequenceDiagram
participant UsagePage
participant DesktopIPC
participant RuntimeHost
participant UsageStores
UsagePage->>DesktopIPC: request usage range
DesktopIPC->>RuntimeHost: query summaries, buckets, logs, and pricing
RuntimeHost->>UsageStores: repair and read canonical snapshot
UsageStores-->>RuntimeHost: usage data and revision
RuntimeHost-->>DesktopIPC: validated pages
DesktopIPC-->>UsagePage: projected UsageStats
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
9fe90f2 to
39b071f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/runtime-host/src/server/canonical-usage-reader.ts (1)
55-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider making
repairrequired.Both callers pass
repairexplicitly, and the coordinator already defines aNO_REPAIRsentinel for the no-reader case. The default silently reportspendingRepairs: 0, which is the one claimCanonicalUsageRepairStatsexists to prevent a caller from making by accident. A required parameter forces each new caller to state its repair posture.♻️ Require the repair posture
- repair: CanonicalUsageRepairStats = { remaining: 0, unreadableEvents: 0 }, + repair: CanonicalUsageRepairStats,
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: dc7840c8-3d9d-4c30-bf0b-5851846306d0
📒 Files selected for processing (30)
apps/desktop/src/main/__tests__/runtime-host-usage-projection.test.tsapps/desktop/src/main/e2e-fixture.tsapps/desktop/src/main/e2e-fixture/scenarios-usage.tsapps/desktop/src/main/runtime-host-boot.tsapps/desktop/src/main/runtime-host-usage-ipc-main.tsapps/desktop/src/renderer/locales/settings-usage-copy.tsapps/desktop/src/renderer/settings/usage-settings-page.tsxapps/desktop/src/shared/desktop-session-projection.tsapps/desktop/stories/settings/settings-pages.stories.tsxpackages/core/src/__tests__/model-call-usage-projection.test.tspackages/core/src/model-call-usage-projection.tspackages/core/src/settings.tspackages/core/src/usage-stats/types.tspackages/runtime-host/src/__tests__/hosted-execution-runner.test.tspackages/runtime-host/src/__tests__/usage-pricing-client-correlation.test.tspackages/runtime-host/src/__tests__/usage-pricing-protocol.test.tspackages/runtime-host/src/protocol/index.tspackages/runtime-host/src/protocol/usage-pricing.tspackages/runtime-host/src/server/canonical-usage-reader.tspackages/runtime-host/src/server/usage-pricing-coordinator.tspackages/runtime/src/__tests__/cost.test.tspackages/runtime/src/telemetry/record-llm-call.tspackages/runtime/src/telemetry/types.tspackages/storage/src/__tests__/settings-store-usage.test.tspackages/storage/src/__tests__/usage-stores.test.tspackages/storage/src/settings-store.tspackages/storage/src/sqlite-usage-store.tspackages/storage/src/telemetry-file-schema.tspackages/storage/src/usage-stats-store.tspackages/storage/src/usage-stores.ts
💤 Files with no reviewable changes (4)
- packages/storage/src/tests/settings-store-usage.test.ts
- apps/desktop/src/shared/desktop-session-projection.ts
- packages/storage/src/usage-stats-store.ts
- apps/desktop/src/main/runtime-host-boot.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.
39b071f to
b093aff
Compare
|
Note The previously reviewed commits are no longer reachable (likely due to a force-push or rebase), so CodeRabbit is performing a full review instead of an incremental one. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: cc8cf103-9b14-4652-a49b-ae57a41acfcd
📒 Files selected for processing (4)
apps/desktop/src/main/__tests__/runtime-host-usage-projection.test.tsapps/desktop/src/main/runtime-host-usage-ipc-main.tspackages/storage/src/__tests__/usage-stores.test.tspackages/storage/src/usage-stores.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/storage/src/usage-stores.ts
- apps/desktop/src/main/runtime-host-usage-ipc-main.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 0 remain after this review.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for consolidating Usage under the Runtime Host and removing the parallel desktop storage path. I reviewed this head with two independent @reviewer passes plus a read-only ollama-cloud/deepseek-v4-flash:high pass. The revision/provenance fence and explicit partial/unpriced coverage are good directions.
The integration currently breaks at the Desktop Host boundary in two places. First, settings:usageStats moved to scoped Runtime Host IPC, but the preload still invokes the old process-scoped signature. Second, the new projection returns Host-local session IDs to a renderer whose session identity is the composite desktop key. Together these make the Usage page fail to load, or make “open session” unroutable even after loading is fixed.
The first-principles design is one selected-Host request contract and one Host→Desktop identity projection. Pass the target scope before range, make the renderer choose the Host explicitly, and convert every optional Usage sessionId with that Host's desktop session-key function at the main-process projection boundary. Do not make the renderer infer Host identity from a local ID.
Current CI is green, but the added tests call the projection helper directly and do not cross the scoped IPC/preload/navigation seam. No local test suite was run during this review; conclusions are based on source, test, and current CI inspection. Codex coordinated the passes and performed the final adjudication; external-model output was treated as unverified until checked against the code.
中文摘要
感谢把 Usage 收敛到 Runtime Host 并删除 Desktop 的并行 storage 路径。revision/provenance fence，以及 partial/unpriced 的明确呈现，方向正确。
当前在 Desktop Host 边界有两个集成故障：settings:usageStats 已迁到 scoped Runtime Host IPC，但 preload 仍使用旧的 process-scoped 参数；新 projection 又把 Host-local session ID 直接交给需要 composite desktop key 的 renderer。结果是 Usage 页面无法加载，或修复加载后“打开会话”仍无法路由。
最小设计是一套 selected-Host request contract 和一处 Host→Desktop identity projection：scope 必须在 range 之前传入，renderer 明确选择 Host，并在 main projection 边界把每个 Usage sessionId 转成该 Host 的 desktop session key。不要让 renderer 从裸 local ID 推断 Host。
当前 CI 全绿，但新增测试绕过了 scoped IPC/preload/navigation seam。本次未运行本地测试套件；外部模型输出在核对代码前均视为未验证输入。
ddc93c9 to
046520c
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The PR correctly keeps the AgentRun stream as the durable model-call authority and treats Usage as a projection. One compatibility defect remains: the strict wire shape changed without version isolation. The smallest fix is to make the compatibility epoch accurately describe the new response contract.
AI-assisted review disclosure: Codex verified the finding against the current head, strict protocol codecs, client collection, and live PR scope. Two independent reviewer-agent passes and an OpenCode Go DeepSeek V4 Flash (high) adversarial pass were used as inputs; findings outside this PR's effective diff were discarded. No local tests were run.
中文复核
让 AgentRun stream 作为模型调用的持久权威、Usage 只做投影，方向正确。当前保留一个兼容性问题：严格 wire shape 已变化，但 compatibility epoch 没有隔离新旧协议。最小修复是让 epoch 如实表达新的响应契约。
本次为 AI 辅助审查：Codex 在最新 head 上核验严格协议 codec、客户端收集逻辑与实时 PR 范围；另使用两次独立 reviewer 及一次 OpenCode Go DeepSeek V4 Flash（high）对抗审查，不属于本 PR 有效 diff 的 finding 已剔除。未运行本地测试。
|
/agentic_review |
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
me2seeks
left a comment
me2seeks
left a comment
There was a problem hiding this comment.
Publishing the verified inline fix responses.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The architectural direction is right: Runtime Host is now the Usage authority, the old Session scanner is gone, Desktop IDs are scoped correctly, and the protocol/revision boundaries are much clearer.
I’m leaving COMMENT because the canonical projection still has three related consistency gaps:
- [P1]
packages/runtime-host/src/server/execution-model-composition.ts:220-233— unrecoverable authority/marker window. The AgentRun append commits first; the pending marker is written afterward and marker failure is swallowed. A crash between them—or marker failure followed by projection failure—leaves neither a Usage row nor a marker. Repair only scans markers even though the comment claims a lost marker is found by full reprojection, so spend can disappear permanently withpendingRepairs: 0. Recovery must be discoverable from a durable outbox/commit seam or a checkpointed authority sweep; add failure injection after the authority append. - [P2]
packages/storage/src/model-call-ledger.ts:282-290— empty authority reads erase the repair signal. Zero events decode as zero attempts/zero unreadable events, so the marker is cleared and the run is counted repaired. Treat an empty result as incomplete unless terminal absence is proven, retain the marker, and add an empty-authority regression. - [P2] Query-triggered repair is not shared across Desktop’s concurrent views. I left this one inline at the changed owner.
The first-principles end state is to keep Usage queries pure reads and give reconciliation one lifecycle owner that can discover authority records without relying on a best-effort marker. The existing historical-total provenance thread also remains valid but is not duplicated here.
AI-assisted review disclosure: two Codex reviewers independently reviewed exact head 1f3e1fb; Codex then traced the findings through the authority append, marker, repair, and Desktop multi-view read paths.
中文说明
整体方向正确，但当前仍有三个一致性缺口：authority append 与 marker 之间可能永久漏记；空 authority 读取会错误清除 marker；多个并发 Usage query 会各自触发 repair 并读取到不同的 pending 状态。最干净的终态是让 query 保持纯读，由单一 lifecycle/background owner 通过 durable outbox 或 authority checkpoint 完成 reconciliation。
1f3e1fb to
8fe5c40
Compare
|
The lone red check is unrelated to this Usage delta: Desktop E2E passed 41 tests and failed only |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for consolidating Desktop usage onto the Runtime Host authority. The current head preserves the intended authority and revision-fence boundaries, and the supplied Usage screenshots cover the visible change. I discarded two initially suspected repair-marker issues after confirming those paths are byte-identical to current main.
One new-call provenance P2 remains inline below. The earlier top-level finding about ambiguous legacy stored totals also remains valid and is not duplicated: a historical camelCase provider-reported total can lack raw snake-case evidence, so the new reader can silently rewrite it while the UI says legacy rows keep stored values. Please choose one explicit legacy invariant and add that regression.
Provenance is nearly complete, but substantive commit cba443c2 has no Generated-by trailer while the PR declares Maka assistance; please add the trailer or explicitly confirm that commit was human-only. The unrelated slash-command E2E timeout appears out of scope, but the required checks still need a green rerun before merge.
Reviewed with Codex as an AI-assisted code review. I verified the current diff against main, relevant telemetry paths, screenshots, CI, and commit provenance; no external model output was used.
中文说明
本 PR 的 Host authority、revision fence 和 Usage 截图方向正确；两个最初怀疑的 repair-marker 问题经核对与 main 完全一致，已排除。当前仍有一个新调用的来源错标 P2，以及此前已指出、这里不重复发 inline 的历史 total provenance 歧义。另请补 cba443c2 的 Generated-by trailer，或明确该 commit 为纯人工；合并前还需把 required checks 重跑至绿色。
8fe5c40 to
f202838
Compare
|
Review remediation is now on |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Reviewed exact head f20283805372930595a767e1d25822dca67d412a.
The usage provenance path now explicitly carries reported versus derived totals for new calls. For source-less legacy rows, the implementation adopts one clear invariant—preserve the stored total instead of guessing provenance and silently rewriting it—and the regression coverage follows that invariant through summaries and buckets. All review threads are resolved, all checks are green, the three requested Usage screenshots are available, and all 18 commits plus the PR body contain complete Maka/Codex provenance disclosure. I found no remaining P0–P3 issues in this PR.
AI-assisted review disclosure: Codex reviewed the exact-head diff, prior findings and remediation, focused tests, current CI and threads, screenshots, and provenance metadata. No external model was used. Astro-Han authorized this review campaign.
中文说明
新调用已完整区分 reported/derived total；无 provenance 的历史数据采用明确且一致的规则，保留已存储 total，不再猜测来源后静默改写。相关 summary/bucket 回归、线程、CI、三张 Usage 截图及 18 个提交的 AI trailer 均已核实，没有剩余 P0–P3。
Withdrawing this approval after the final current-main integration scan found a compatibility-epoch collision introduced by #3103. A replacement exact-head COMMENT with the concrete fix follows.
9cea014 to
808a588
Compare
|
Rebased onto latest main ( Conflict resolutions:
Re-ran affected suites on the rebased head — all pass, no failures attributable to main's changes:
|
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
COMMENT. The core architectural intent is right and I want to say that first: runtime-host-boot.ts drops the unscoped ipcMain.handle("settings:usageStats") and projectDesktopUsageStats, packages/storage/src/usage-stats-store.ts and SettingsStore.usageStats are gone, and the only surviving path is the scoped Host client. There is no second Usage source of truth left on the Desktop side. Everything below is about the machinery layered on top of that path, not about the direction.
The epoch is correct and I am closing my earlier thread on it: protocol/index.ts:75 is 28 against a base of 27, the comment states why (the required revision field and the new optional usageBasis on LlmUsageLogProjection change the response shape), and handshake-compatibility.test.ts now pins RUNTIME_HOST_COMPATIBILITY_EPOCH - 1 so it will not go stale on the next rebase. One thing to watch, not a finding: #3236, #3199 and #3133 are all also taking 28 from a base at 27, and #3282 shows 28 while stacked on #3236. The = 28 line itself merges cleanly because every branch writes identical text and each PR's own assert epoch > 27 still passes, so whichever lands second silently ends up sharing an epoch with the first. Whoever merges these needs to re-check the value at merge time; I filed #3313 about removing the class.
Two P1s, three P2s inline. The two P1s compound each other — the longer a full-window read takes, the likelier the fence under it moves — so I would treat them as one problem with two halves.
Three smaller things not worth their own threads:
- Changing the range issues two complete snapshot loads.
usage-settings-page.tsx:93-97callsprops.onReload(range)afterupdateUsage({ range }), and becauseusageis client-owned the same update also lands insetClientSettings, changingsettings.usage.range, which the effect atsettings-surface.tsx:583-589lists as a dependency. The reload ticket makes that correct but not cheap, and it doubles the exposure to both P1s. Dropping either the explicitonReloador the dependency fixes it. - Duplication a smaller change would subsume.
loadAllBuckets/loadAllLlmLogs/loadAllToolLogsare three near-identical paging loops differing only in their result guard;projectCustomPricingRowsre-implements thesource === "custom"filter already in theusage:pricing:listhandler; andsameUsageSnapshotstill cross-checkstotalRequestsagainst three independently summed row counts after already requiring revision equality across all five views. - Stale copy.
settings-usage-copy.ts'stoolEmptyBodystill promises "calls, successes, errors, and average duration" in both locales now that an Aborted column exists.
One thing I looked at and am explicitly not filing: usageRevision is a per-facade in-memory counter starting at 0 (usage-stores.ts:335), so a facade re-created mid-read would let two queries report revision: 0 over different data and sameUsageSnapshot would accept a torn snapshot. I could not establish that a drain-and-reopen is reachable between two queries of one snapshot, so this is inference only. Seeding the counter from the Host epoch would remove the class cheaply if you happen to be in there.
Test gaps, none P-rated:
- Pagination has no coverage at all — every fake response in
runtime-host-usage-projection.test.ts:137-215returnsnextOffset: null, so roughly 150 lines of new paging, the mid-pagination revision guard, thenextOffset <= offsetguard and therows.length !== totalguard are all unexercised. SNAPSHOT_ATTEMPTSexhaustion is untested, and so is the Desktop's handling of a Hostinternal_failure— which is the first P1.usage-stores.test.ts's "preserves ambiguous legacy totals and trusts explicit current provenance" passes identically withtotalTokensSourcedeleted, becausecanonicalTotalTokensignores it. It asserts pass-through, not provenance-sensitive behaviour, so the body's "normalized the legacy total-token contract" is not proven by any test.- The new coordinator test asserts an intermediate (
pendingReprojections().length === 44) rather than a consistent Desktop-visible snapshot, which is why it does not reach the batch-boundary P2.
biome format is clean on all changed packages/** files. Note for anyone re-checking: apps/desktop/** and packages/ui/** are excluded from formatter.includes in biome.jsonc, and biome format --stdin-file-path does not apply that exclusion for a path that does not exist locally, so new desktop files produce false diffs that way.
AI disclosure: this review was assisted by Claude (Opus), which performed the initial code search and cross-checking. Every finding published here I re-derived myself against the source at 808a588a3, and I sharpened the batch-boundary mechanism and dropped the one finding I could not establish as reachable.
808a588 to
e60fcf6
Compare
Generated-by: Maka
Generated-by: Maka
Generated-by: Maka
Generated-by: Maka
Generated-by: Maka
Generated-by: Maka
Generated-by: Maka
Generated-by: Maka
Generated-by: Maka
Generated-by: Maka
Generated-by: Maka
The Host-backed Usage projection hardcoded an empty pricing list, so the pricing tab showed no rows and a zero count even when the Host held custom overrides. Fold the Host pricing snapshot into the same read and project only custom entries, splitting each model key into provider and model. Generated-by: Maka
usageSnapshotRevision looped until the write barrier stopped moving, so a Usage query behind a continuous write stream (an active turn recording model calls) could hang instead of failing fast. Bound the settle to 32 attempts and throw, letting the coordinator surface a read failure; a regression test drives a continuous write stream and asserts the rejection. Generated-by: Maka
If assert.rejects fails, the setImmediate writer loop previously kept running after withInteractiveRoot removed the temporary root, keeping the process alive and masking the original assertion failure. Move the stop, writer wait, and closes into a finally block. Generated-by: Maka
Generated-by: Maka
Generated-by: Codex
Generated-by: Maka
Generated-by: Codex
Generated-by: Codex
Host: - usage.query gains an optional snapshot ticket; one bounded repair pass is pinned to it, so a paginating Desktop snapshot can no longer trigger a fresh pass (and its revision bump) between pages. - Fence exhaustion and unsettled revision reads now answer the typed, retryable usage_revision_changed outcome instead of internal_failure. - groupBy:'tool' buckets carry the terminal-status breakdown (success/error/aborted), so clients do not page every tool row to build the tool table. Storage: - canonicalTotalTokens consumes totalTokensSource: explicit derived totals normalize to input + output at the frozen read boundary (reasoning is an output detail), while reported and source-less legacy totals pass through. - usageSnapshotRevision's settle exhaustion is the typed UsageRevisionUnsettledError, classified as revision_unsettled. Desktop: - One paging loop for every Usage view; log views are capped at five pages per source, bounding snapshot round trips and payload. Aggregates still cover the full window, and the requests tab labels the capped view with the recorded total. - The Host's fence-exhaustion outcome folds into the whole-snapshot retry; changing the range no longer issues two complete snapshot loads; the request rows display the stored canonical total so they reconcile with the summary and bucket totals. Generated-by: Maka
e60fcf6 to
edace14
Compare
|
All findings from the 2026-08-20 review are addressed at Epoch note: main landed the relay Fast service-tier change as epoch 28 while this was in flight, so this branch now advances the usage wire contract from 28 to 29, keeping both comments. Minor items:
Test gaps, all closed:
CI: the 中文说明本轮 review 的 2 个 P1、3 个 P2 均已修复并附各自要求的回归测试（见各 thread 回复）；三个小项（范围切换双次加载、分页循环重复、pricing 过滤重复、过时文案）也已落地。由于 main 在此期间把 relay Fast service-tier 占为 epoch 28，本分支 rebase 后把 usage 协议推进到 29 并保留两边注释。package 通道的 CDP 30s 失败是全仓 harness flake（纯文档分支同错），与本 diff 无关；本地四个套件全绿。 |
2 participants
Summary
The Desktop adapter uses one explicit time window and retries when paged Host projections do not describe the same snapshot. Coverage stays page-level because legacy, unreadable, and pending records cannot be attributed to individual provider or model buckets without guessing.
Fixes #2128
Verification
npm run lintnpm run format:checknpx knip --workspace apps/desktopnpm --workspace @maka/core test(821 passed)npm --workspace @maka/storage test(795 passed, 16 skipped)npm --workspace @maka/runtime-host test(838 passed)npm --workspace @maka/desktop test(1158 passed)npm --workspace @maka/desktop run build:rendererScreenshots
Request log
Provider totals
Tool totals
Checklist
Does this PR entail a change in behavior?
简体中文
摘要
Desktop adapter 使用同一个明确时间窗口；如果分页读取到的 Host 投影不属于同一快照，则会重新读取，而不是拼接后返回。覆盖情况保持在页面级，因为历史、无法读取和待修复的记录不能在不猜测的情况下归到某个供应商或模型。
验证
AI use
Tool(s) and scope: Maka authored the existing Usage implementation, tests, verification, documentation, and visual evidence. OpenAI Codex authored the Runtime Host compatibility-epoch follow-up and the latest total-token provenance review remediation with its regression tests.
Final squash trailers:
Generated-by: MakaGenerated-by: Codex