Skip to content

fix(desktop): align usage statistics with Daily Review - #3761

Closed
testikun wants to merge 1 commit into
apache:mainfrom
testikun:codex/issue-3695-usage-alignment
Closed

fix(desktop): align usage statistics with Daily Review#3761
testikun wants to merge 1 commit into
apache:mainfrom
testikun:codex/issue-3695-usage-alignment

Conversation

@testikun

Copy link
Copy Markdown
Contributor

Summary

Fixes #3695

Usage Statistics now reads the selected Runtime Host's unified usage projection, matching Daily Review's legacy + canonical model-call accounting.

  • Added a Host-scoped usage:stats IPC projection.
  • Reused one concrete time window for summary, buckets, and every activity-log page.
  • Preserved host-scoped session keys, ignored stale responses after Host changes, and kept unknown session/cost values explicit.
  • Renamed the mixed view to Activity log and tracked aborted tool calls separately from errors.
  • Kept the existing Runtime Host authority and pricing semantics unchanged.

中文摘要

使用统计现在直接读取当前选中的 Runtime Host 统一用量投影,与 Daily Review 使用相同的 legacy + canonical 统计口径;同时补齐分页读取、Host 切换防旧数据回写、未知字段展示和中止工具调用的独立统计,不改变 Runtime Host 权威或定价逻辑。

Verification

  • npm run format:check
  • npm run lint
  • npm --workspace @maka/core run build
  • npm --workspace @maka/runtime-host run build
  • npm --workspace @maka/storage run build
  • npm --workspace @maka/desktop run build:main
  • Desktop main/preload typechecks
  • Focused regression: runtime-host-usage-stats.test.js (passed)
  • git diff --check

The repository-wide typecheck still reports existing unrelated errors in packages/ui and the CLI's @maka/eval resolution. The storage SQLite test suite was not runnable with the available Node v20 runtime because node:sqlite is unavailable; the repository requires Node >=22.19.0.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex analyzed issue #3695, designed the bounded Host-scoped projection, implemented the IPC/renderer changes, added regression coverage, and ran the listed verification. The human contributor remains responsible for review and submission.

Checklist

  • Tests cover the change and fail without it
  • Focused lint, format, build, and typecheck checks pass locally

Does this PR entail a change in behavior?

  • Yes — Usage Statistics now uses the selected Runtime Host authority and exposes aborted/unknown activity explicitly
  • No

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this head and found no blocking code issues.

Usage statistics are now aligned to the Host projection (single window for summary/buckets/logs, paginated fetch, host-switch stale guard). The fix correctly reuses the Daily Review projection and handles aborts/tool counts.

Machine gate: test is failure on session-effect-two-client-uds (likely environmental flake, unrelated to desktop usage IPC), so not green — not mergeable yet. No P0-P2 in code.

简体中文代码无阻断,CI 红待作者重跑。

@testikun

Copy link
Copy Markdown
Contributor Author

Investigated the red CI check on this PR. The failure is in the pre-existing packages/runtime-host test session-effect-two-client-uds.test.ts, with Recap provider outcome is unknown; it is unrelated to the usage IPC/statistics files changed here.

I rebuilt @maka/runtime-host locally and ran the failing test repeatedly, including concurrent stress runs (360 total executions), without reproducing the failure. No code change is warranted in this PR for an unrelated environmental/concurrency flake; the failed check needs a GitHub rerun by someone with repository Actions permission.

简体中文 这次 CI 红灯来自主干已有的 Runtime Host recap 双客户端测试,与本 PR 的 usage 改动无关。本地构建并压力运行 360 次均通过,建议直接重跑该 check。

Generated-by: OpenAI Codex

@testikun

Copy link
Copy Markdown
Contributor Author

Follow-up fix pushed in da77a7cf4.

The recap coordinator now rechecks the durable result when it finds an intent but no in-memory active effect. This closes the small race where the first result probe observes the result before publication, the intent is then visible, and an exact retry could otherwise be classified as outcome_unknown even though the result has become durable. Conflicts still fail closed, and provider work is not repeated.

Local verification: @maka/runtime-host typecheck passed; the recap coordinator tests and the two-client UDS test passed; 100 targeted recap runs passed; format and lint passed.

The original CI log also contained an unrelated Host registration/startup race in another test, so the full CI result still needs to be observed after this head.

简体中文 在发现 intent 但内存中没有 active effect 时重新读取 durable result,避免结果刚写入期间的精确重试被误判为 outcome_unknown。provider 不会重复调用,冲突仍然 fail closed。

Generated-by: OpenAI Codex

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this head and found a blocking issue.

[P1] settings.usageStats() forwards wrapped Result as raw stats — crashes Settings → Usage

runtime-host-usage-ipc-main.ts:87-92 returns {ok:true, data:UsageStats} via tryReconnectableReadResult, but preload.ts:2629-2633 declares Promise<UsageStats> and returns the wrapper unwrapped. Renderer reads stats.logs.length on the wrapper → Cannot read properties of undefined and crash. Hosted test: FAILURE (32920709908) on Settings accessibility.

Fix: unwrap success Result in preload (keep reconnectable error semantics) or change contract to Result and handle in renderer; add regression test.

简体中文设置页的 Usage 数据被包了一层导致崩溃。

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

@testikun
testikun force-pushed the codex/issue-3695-usage-alignment branch from da77a7c to c415d99 Compare August 26, 2026 05:34
@testikun

Copy link
Copy Markdown
Contributor Author

Fixed the P1 in c415d9936, rebased onto the latest apache/main (e6db07e96).

The Runtime Host usage:stats IPC intentionally returns a reconnectable Result<UsageStats> envelope. The preload bridge now unwraps successful Host reads before returning them to Settings, and converts projected failures into an Error while preserving reconnectable transport failures as rejected promises. The local settings:usageStats path is unchanged.

Added regression coverage for:

  • successful and failed Result unwrapping;
  • the preload Host usage contract;
  • the existing usage projection adapter.

Verification:

  • Desktop main build: passed
  • preload typecheck: passed
  • targeted tests: 4/4 passed
  • Settings accessibility E2E (every settings page exposes named actionable controls): passed
  • repository lint: passed

The full workspace dependency build still reports pre-existing packages/ui type mismatches on this latest main; no UI package files are changed by this PR.

简体中文 已修复 P1:Host usage IPC 返回的 Result 包装现在由 preload 正确解包,Settings 收到真正的 UsageStats;普通错误会转成异常,可重连传输错误仍保持 reject 语义。设置页无障碍 E2E 已通过。

Generated-by: OpenAI Codex

@testikun
testikun force-pushed the codex/issue-3695-usage-alignment branch 2 times, most recently from 428fd04 to b6214b2 Compare August 26, 2026 07:28
@M4n5ter
M4n5ter force-pushed the codex/issue-3695-usage-alignment branch 3 times, most recently from 8cbfc76 to a9872e2 Compare August 26, 2026 09:48
Read usage statistics from the selected Runtime Host projection, preserve host-scoped session identity, and surface aborted and unknown activity explicitly. Recheck durable recap results before classifying exact retries as unknown, and unwrap reconnectable Host usage results in preload before exposing them to Settings.\n\nAdd regression coverage for the Host projection, recap race, Result unwrapping, and preload contract.\n\nGenerated-by: OpenAI Codex
@M4n5ter
M4n5ter force-pushed the codex/issue-3695-usage-alignment branch from a9872e2 to e6cfc35 Compare August 26, 2026 09:55
@M4n5ter

M4n5ter commented Aug 27, 2026

Copy link
Copy Markdown
Member

Relationship note: PRs #3697 and #3761 both close #3695 and overlap in the Usage Statistics migration, including runtime-host-usage-ipc-main.ts, the preload bridge, the Usage Settings UI/copy/navigation/surface, and packages/core/src/settings.ts. PR #3697 is broader (21 changed files); this PR is narrower (14 changed files) and also contains the Result-unwrapping/session-effect changes. Their bodies and comments do not say whether this PR supersedes #3697, stacks on it, or is an independent alternative. Please document the intended relationship, including which PR owns the shared files and issue closure, before merging either path.

This comment records the relationship only; it is not a merge decision.


Automated review note posted by @未开智选手. This is not an independent human review; a human should verify the conclusion.

@testikun

Copy link
Copy Markdown
Contributor Author

Thanks for flagging this. To make the relationship explicit:

Therefore, one PR should own the shared files and close #3695. If maintainers choose #3697, #3761 can be closed as superseded after the two focused fixes above are carried over if they are still wanted. If maintainers choose #3761, #3697 should be closed or split into follow-up PRs for the additional improvements. We do not intend to merge both or stack them.

We are happy to follow the maintainers’ choice. Until then, please treat #3761 as an alternative implementation, not an additional patch on top of #3697.

简体中文

#3761#3697#3695 的两套替代实现,不是叠加关系,因为它们修改了相同的 Usage Statistics 文件。#3761 范围较小,但包含 Host Result 解包和 Runtime Host durable-result 重检;#3697 范围更大,包含 provenance、Host epoch 重拉、截断/降级、canonical fixture 和连接分组等后续改进。最终应由一个 PR 负责共享文件并关闭 #3695;如果选择 #3697,可在保留需要的两个聚焦修复后关闭 #3761;如果选择 #3761,则应关闭 #3697 或把额外改进拆成后续 PR。我们不打算同时合并或叠加两个 PR。

@github-actions github-actions Bot added the effort/L Under 1000 readable lines label Aug 27, 2026
@M4n5ter

M4n5ter commented Aug 27, 2026

Copy link
Copy Markdown
Member

Thank you for working on the Usage Statistics alignment. PR #3697 has now merged as 566ed7dd7691042bd08c7b93d8e79f9a2a0649a8 and closed #3695. It makes Runtime Host the single Usage authority and includes the provenance, truncation/degraded-read behavior, Host-epoch refetch, connection-level grouping, and canonical end-to-end coverage needed for this fix. I am therefore closing this overlapping implementation as superseded so the repository keeps one owner for the behavior.

This branch also contains a durable-result reread change in session-effect-coordinator that is outside #3695. If that repair is still needed, please submit it separately with a focused regression for the durable-publication race rather than carrying it through the superseded Usage change.


Posted by an automated review agent operated by @M4n5ter. This is not an independent human review and does not satisfy the committer review required by CONTRIBUTING.md. A human is accountable for this comment — please push back if anything here is wrong.

简体中文

本条评论由 @M4n5ter 运行的自动化审查程序发出。它不构成 CONTRIBUTING.md 所要求的独立人类审查,也不能替代人类审查。有人类对本条评论负责,如有错误请直接指出。

@M4n5ter M4n5ter closed this Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/L Under 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(desktop): align Usage Statistics counts with Daily Review

3 participants