Skip to content
This repository was archived by the owner on Jul 30, 2026. It is now read-only.

fix(tui): confine group-by scope and pin canonical model identity - #165

Merged
makoMakoGo merged 4 commits into
personal/local-clientsfrom
fix/group-by-view-scope
Jul 18, 2026
Merged

fix(tui): confine group-by scope and pin canonical model identity#165
makoMakoGo merged 4 commits into
personal/local-clientsfrom
fix/group-by-view-scope

Conversation

@makoMakoGo

@makoMakoGo makoMakoGo commented Jul 18, 2026

Copy link
Copy Markdown
Owner

概要

#161 影响矩阵中已确认的决策,实施 Group By 作用范围清理(最小修复 + 契约固化;不含廉价再投影的结构优化)。

核心变化:Group By 收敛为「Models 类表格的展示投影」——所有 "Top Model" 排名钉死 bare canonical model ID,canonical 身份由显式 model_id 字段承担,workspace 维度以独立列/字段呈现,不再拼进标签字符串。

变更

  • 排名钉死 canonical:Daily 主表 Top Model 列与 Period 汇总 Model* 列改用 model_id 跨 source 合并,胜者/标签不再随分组翻转(等价 Stats rank_canonical_models 语义)
  • 身份契约显式化:DailyModelInfo / HourlyModelInfo 新增 model_id(bare canonical)与 workspace_key / workspace_label;display_name 恒为裸模型名;color_key 回归纯着色;删除 model_color_key_group_by / _provider_id 占位参数与 map_key 身份回退死分支
  • Workspace 独立列:Workspace+Model 分组下,Models 表与 Daily/Period 详情均以独立 Workspace 列展示(列结构一致性:详情 = 特定时间范围的 Models tab),Model 列回归裸模型名
  • g 键守卫:仅 Models / Daily / Monthly / Weekly 可打开分组选择器,其余 Tab 不再触发无谓的阻塞全量重载
  • session 刷新解耦:分组切换不再 force 全量重扫 session snapshot(仅 digest 驱动,含性能收益);手动 r 与文件变更行为不变
  • Overview 着色去分组化:shade map 按 canonical 聚合,4 种分组模式下同一模型颜色一致
  • TUI 导出自描述:JSON 顶层 groupBy + 行级 workspaceKey / workspaceLabel(对齐 models CLI 习惯)
  • 缓存 schema 37 → 38:旧版本缓存安全失效(legacy miss),不产生反序列化错误
  • ADR 0026 记录 Group By 视图作用范围契约

测试

新增回归:4 种分组下 Top Model 排名一致、Workspace 列渲染(Models 表 + 详情)、g 守卫(4 可用 / 6 不可用 Tab)、Hourly 4 分组渲染逐字节不变、ClientProviderModel 投影等价、session_reload_force、着色分组不变、导出自描述、缓存 legacy-miss。

cargo test -p tokscale-core -p tokscale-cli 全绿(1454 + 691 + 140 + 10 + 3 + 2 + 1 passed);cargo fmt --checkcargo clippy --all-targets 干净。

未做(后续 PR)

Refs #161

Summary by CodeRabbit

  • 新功能
    • 新增工作区分组视图:工作区作为独立列呈现,并在导出 JSON 中按实际分组附带 groupBy 与工作区字段。
  • 改进
    • g 快捷键仅在适用页面打开 Group By;切换分组触发更精细的会话刷新策略。
    • 统一模型身份(modelId)贯穿排名与颜色,跨不同分组投影显示保持一致。
  • 缓存与导出
    • 缓存升级并校验模型身份完整性,避免缺失身份导致的错误加载。
  • 文档
    • 更新分组与模型身份契约说明。

- pin Daily/Period top-model rankings to bare canonical model IDs
- add explicit model_id/workspace fields to daily/hourly model info
- show workspace as its own column in Workspace+Model grouping
- gate the group-by picker to tabs that consume grouped projections
- decouple session refresh from group-by reloads
- make model shade resolution grouping-invariant
- make TUI export self-describing (groupBy, workspaceKey/workspaceLabel)
- record the view-scope contract in ADR 0026
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@makoMakoGo, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 53 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a8b73732-db71-4c46-8fb4-b3a9bf472922

📥 Commits

Reviewing files that changed from the base of the PR and between 71afcce and 25d7372.

📒 Files selected for processing (1)
  • crates/tokscale-cli/src/tui/cache.rs

Walkthrough

本次变更建立显式的 model_id 与 workspace 身份字段,更新聚合、缓存、颜色、分组重载、导出及多类 TUI 表格,使不同 GroupBy 投影使用一致的模型身份与展示数据。

Changes

分组身份与工作区投影

Layer / File(s) Summary
核心身份契约与聚合输出
crates/tokscale-core/src/usage_views.rs, crates/tokscale-core/src/aggregate/tui.rs, docs/adr/...
Daily/Hourly 模型信息新增 model_id,workspace 通过结构化字段传递,聚合和契约测试同步更新。
缓存身份字段与兼容处理
crates/tokscale-cli/src/tui/cache.rs
缓存 schema 从 37 升至 38,并序列化、还原 model_id 与 workspace 字段;缺失模型身份的缓存返回 Miss
分组重载、颜色与导出
crates/tokscale-cli/src/tui/app.rs, crates/tokscale-cli/src/tui/mod.rs, crates/tokscale-cli/src/tui/colors.rs
分组重载区分 Sessions 强制刷新,g 仅在适用 Tab 生效,颜色解析使用 canonical provider,导出使用已加载的分组。
工作区列与明细渲染
crates/tokscale-cli/src/tui/ui/model_usage_layout.rs, crates/tokscale-cli/src/tui/ui/models.rs, crates/tokscale-cli/src/tui/ui/daily.rs, crates/tokscale-cli/src/tui/ui/period.rs
WorkspaceModel 下独立显示 Workspace 列和裸模型名,并更新列宽、布局及渲染测试。
聚合、排名与投影一致性
crates/tokscale-cli/src/tui/ui/overview.rs, crates/tokscale-cli/src/tui/ui/overview_snapshot.rs, crates/tokscale-cli/src/tui/ui/stats.rs, crates/tokscale-cli/src/tui/ui/hourly.rs, crates/tokscale-cli/src/tui/ui/footer.rs
Overview、Snapshot、Stats、Hourly 和页脚改用 model_id,并验证不同分组投影下的输出一致。
导出与工作区显示支持
crates/tokscale-cli/src/tui/export.rs, crates/tokscale-cli/src/tui/ui/widgets.rs
导出增加 groupBy,WorkspaceModel 下附加 workspace 字段,并提供未知 workspace 的显示回退。

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers: junhoyeo, ivgolovach

Poem

小兔抱着 model_id,
跳过 workspace 的草地。
分组换了颜色仍相依,
Sessions 安静不惊起。
导出带上 groupBy,
代码萝卜甜又脆。

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 54.13% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题准确概括了本次变更的核心:限制 Group By 作用域并固定规范模型身份。
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/group-by-view-scope

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@makoMakoGo

Copy link
Copy Markdown
Owner Author

@codex review

@codspeed-hq

codspeed-hq Bot commented Jul 18, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 27 untouched benchmarks


Comparing fix/group-by-view-scope (25d7372) with personal/local-clients (bf6f7d4)

Open in CodSpeed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cbb267f6b0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/tokscale-cli/src/tui/mod.rs Outdated
Comment thread crates/tokscale-cli/src/tui/app.rs Outdated
Comment thread crates/tokscale-cli/src/tui/app.rs Outdated
Comment thread crates/tokscale-cli/src/tui/cache.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/tokscale-cli/src/tui/ui/models.rs`:
- Around line 22-24: Update workspace_label to fall back from
model.workspace_label to model.workspace_key when the label is missing, while
preserving the existing Unknown workspace fallback when both are absent. Reuse
the existing workspace_label_or_unknown helper and align the behavior with the
label-to-key fallback used by Daily/Period details.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ec3f0d7b-1a84-452c-9e1e-a19ca6d210c7

📥 Commits

Reviewing files that changed from the base of the PR and between bf6f7d4 and f15fd7d.

📒 Files selected for processing (20)
  • crates/tokscale-cli/src/tui/app.rs
  • crates/tokscale-cli/src/tui/cache.rs
  • crates/tokscale-cli/src/tui/colors.rs
  • crates/tokscale-cli/src/tui/data/mod.rs
  • crates/tokscale-cli/src/tui/export.rs
  • crates/tokscale-cli/src/tui/mod.rs
  • crates/tokscale-cli/src/tui/ui/daily.rs
  • crates/tokscale-cli/src/tui/ui/footer.rs
  • crates/tokscale-cli/src/tui/ui/hourly.rs
  • crates/tokscale-cli/src/tui/ui/model_usage_layout.rs
  • crates/tokscale-cli/src/tui/ui/models.rs
  • crates/tokscale-cli/src/tui/ui/overview.rs
  • crates/tokscale-cli/src/tui/ui/overview_snapshot.rs
  • crates/tokscale-cli/src/tui/ui/period.rs
  • crates/tokscale-cli/src/tui/ui/stats.rs
  • crates/tokscale-cli/src/tui/ui/widgets.rs
  • crates/tokscale-core/src/aggregate/parity_tests.rs
  • crates/tokscale-core/src/aggregate/tui.rs
  • crates/tokscale-core/src/usage_views.rs
  • docs/adr/0026-group-by-view-scope-contract.md

Comment thread crates/tokscale-cli/src/tui/ui/models.rs
- fall back to workspace_key in the Models workspace column
- retry sessions when a health-forced reload rides a group switch
- label exports with the grouping of the loaded projection
- reject current-schema cache entries missing modelId
@makoMakoGo

Copy link
Copy Markdown
Owner Author

Bot review 处置结果(commit 71afcce5)

逐条核实代码后:采纳 4 条,驳回 1 条(过时)。

采纳并修复:

  1. CodeRabbit — Models 表 workspace 列 label 缺失回退 key:属实,与 Daily/Period 详情的 label.or(key) 回退不一致。已修为 workspace_label.or(workspace_key) 再兜底 Unknown,新增测试 workspace_column_falls_back_to_key_when_label_missing
  2. Codex P2 — health 强制 reload 时 session 应跟着重试:属实,这是本 PR 引入 session 解耦时未覆盖的交互。session_reload_force 改为 force && (!group_only || health.requires_source_retry())——group-only 仅在健康时抑制 session 刷新;新增测试覆盖 degraded/健康两种情形。
  3. Codex P3 — 导出标记了未生效的分组:属实(选择器立即写 app.group_by,数据异步才换)。新增 data_group_by 跟踪当前投影的实际分组(随 BackgroundLoad::Loaded 传递),导出改用它;新增测试 export_group_by_tracks_loaded_data_not_pending_picker_selection
  4. Codex P3 — 当前 schema 缓存缺 modelId 应判 Miss:采纳为加固(ADR 0001:身份为空不应静默合并)。load_cache 在转换后校验 daily/hourly 所有 model_id 非空,否则 Miss;新增 2 个测试,并修正了一个直接把旧 fixture 改版本号、本身即缺 modelId 的既有测试。

驳回(过时):

  • Codex P3 — footer [g] 提示:review 针对的是 cbb267f6;此问题已在 f15fd7de 修复(提示与按键守卫共用 group_by_applies_to_current_tab()),无需再改。

验证:cargo test -p tokscale-cli 全绿(698+140+1+2),fmt/clippy 干净。

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/tokscale-cli/src/tui/cache.rs`:
- Around line 2076-2096: 更新 write_identity_cache_without_model_id 中的回归夹具,使用当前的
CACHE_SCHEMA_VERSION 动态生成 schemaVersion,而不是硬编码 38。保留其余 JSON 内容和
cached_models_missing_identity 测试行为不变。
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3ebfd982-82df-459b-8e08-7f5cf1e63113

📥 Commits

Reviewing files that changed from the base of the PR and between f15fd7d and 71afcce.

📒 Files selected for processing (4)
  • crates/tokscale-cli/src/tui/app.rs
  • crates/tokscale-cli/src/tui/cache.rs
  • crates/tokscale-cli/src/tui/mod.rs
  • crates/tokscale-cli/src/tui/ui/models.rs
🚧 Files skipped from review as they are similar to previous changes (2)
  • crates/tokscale-cli/src/tui/app.rs
  • crates/tokscale-cli/src/tui/ui/models.rs

Comment thread crates/tokscale-cli/src/tui/cache.rs
@makoMakoGo
makoMakoGo merged commit a9ba64e into personal/local-clients Jul 18, 2026
8 checks passed
@makoMakoGo
makoMakoGo deleted the fix/group-by-view-scope branch July 19, 2026 05:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant