fix(usage): split Pi and OMP reasoning and recover swarm agents - #137
Conversation
Walkthrough本次变更统一了 Pi/OMP 的 reasoning token 解析、校验、聚合和展示口径。CLI 的 JSON 与表格输出将 reasoning 合并到 output，并采用溢出安全的总量计算；OMP/Pi parser version、swarm 身份解析及 TUI 缓存版本同步更新。 Changes推理 token 解析与聚合
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant OMP JSONL
participant Pi/OMP Parser
participant Monthly Aggregator
participant CLI Report
OMP JSONL->>Pi/OMP Parser: 读取 usage 与 swarm 路径
Pi/OMP Parser->>Monthly Aggregator: 输出归一化 TokenBreakdown
Monthly Aggregator->>CLI Report: 提供 MonthlyUsage 与 reasoning
CLI Report->>CLI Report: 合并 output/reasoning 并计算总量
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
makoMakoGo
left a comment
makoMakoGo
left a comment
There was a problem hiding this comment.
Blocking findings: the Pi/OMP usage split matches the upstream bucket definitions, but this change currently drops reasoning from the monthly report and silently coerces an impossible source breakdown. Both affect data correctness and should be fixed before merge.
|
Review correction: P2 is withdrawn. The |
Treat client reasoning fields as subsets of inclusive output, validate source totals, and include reasoning in CLI totals. Recover canonical OMP swarm agent identity from official artifact paths and invalidate stale parser/TUI caches.
Treat inclusive output as authoritative when a provider reports reasoning above output, preserving exact source totals instead of aborting the report. This handles the observed Xiaomi MiMo length-stop response with output 32000 and reasoningTokens 32123.
ad699e1 to
1feba94
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
crates/tokscale-core/src/adapters/omp.rs (1)
20-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win建议补充注释,说明该版本号同时覆盖 swarm 识别变更
常量名为
OMP_USAGE_AND_SWARM_REVISION,但注释只解释了 reasoning clamp 的动机,未提及 swarm agent 识别(.swarm_<name>/context/...规范路径识别)同样是触发该版本跳变的原因。parser_version是缓存失效的关键字段,补全注释有助于后续再次调整版本号时理解历史动机。📝 建议补充注释
-// Earlier OMP revisions were emitted before malformed inclusive-reasoning -// breakdowns were clamped to their authoritative output bucket. +// Earlier OMP revisions were emitted before malformed inclusive-reasoning +// breakdowns were clamped to their authoritative output bucket, and before +// canonical `.swarm_<name>/context/...` artifacts were recognized as stable +// swarm agent identities. const OMP_USAGE_AND_SWARM_REVISION: u32 = crate::adapters::MODEL_ID_CANONICALIZATION_REVISION + 4;🤖 Prompt for 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. In `@crates/tokscale-core/src/adapters/omp.rs` around lines 20 - 22, 补充 OMP_USAGE_AND_SWARM_REVISION 上方的注释，明确说明该版本号同时覆盖 swarm agent 的识别变更，包括规范路径 `.swarm_<name>/context/...` 的识别；保留现有关于 malformed inclusive-reasoning breakdowns 被归并到权威输出桶的说明。crates/tokscale-cli/tests/cli_tests.rs (1)
2378-2512: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win建议为
monthly命令补充等价的 clamp 回归测试
test_models_report_clamps_reasoning_above_output只覆盖了models命令的 reasoning 超过 output 时的 clamp 场景,而本次修复的 P1 阻塞项恰恰是关于monthly报表丢弃 reasoning 桶的问题。建议为monthly --json(以及可选的表格输出)补充一条对称的回归测试,直接覆盖该场景,而不仅依赖两个命令共享同一套底层解析逻辑这一间接保证。As per path instructions, "Add integration tests under the relevant crate
tests/directory for CLI/session behavior."#[test] fn test_monthly_report_clamps_reasoning_above_output() { let tmp = TempDir::new().expect("failed to create temp dir"); let base = tmp.path(); prime_pricing_cache(base); let sessions = base.join(".omp/agent/sessions"); fs::create_dir_all(&sessions).unwrap(); fs::write( sessions.join("monthly-invalid-reasoning-breakdown.jsonl"), concat!( r#"{"type":"session","id":"monthly-reasoning-overflow-session","timestamp":"2026-01-01T00:00:00.000Z","cwd":"/tmp"}"#, "\n", r#"{"type":"message","id":"monthly-reasoning-overflow-message","parentId":null,"timestamp":"2026-01-01T00:00:01.000Z","message":{"role":"assistant","model":"gpt-5.5","provider":"openai","usage":{"input":100,"output":50,"cacheRead":10,"cacheWrite":5,"reasoningTokens":51,"totalTokens":165}}}"#, "\n" ), ) .unwrap(); let output = cmd_with_home(base) .args(["monthly", "--json", "--client", "omp", "--no-spinner"]) .output() .unwrap(); assert!(output.status.success(), "command failed: {output:?}"); let json: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); assert_eq!(json["entries"][0]["output"], 50); assert!(json["entries"][0].get("reasoning").is_none()); }🤖 Prompt for 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. In `@crates/tokscale-cli/tests/cli_tests.rs` around lines 2378 - 2512, Extend the integration coverage in the monthly reporting tests by adding a test equivalent to test_models_report_clamps_reasoning_above_output for the monthly command. Create a session fixture with reasoningTokens greater than output, invoke monthly --json, and assert successful execution, output remains 50, and no reasoning field is emitted; optionally include matching table-output assertions if supported by the existing test style.Source: Path instructions
crates/tokscale-cli/src/commands/models.rs (1)
21-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
checked_token_total/displayed_output与monthly.rs中的等价逻辑重复
monthly.rs中已经存在功能相同的checked_token_sum(用于displayed_output/monthly_token_total),这里新增的checked_token_total(及displayed_output/displayed_token_total)是几乎逐字重复的第二份实现,仅 panic 文案不同。提交历史中有一条 "Share token-total calculations across monthly reporting",但实际结果是两个命令文件各自维护了一份几乎相同的溢出安全求和逻辑,后续两处实现出现分歧的风险随之增加。建议将该求和辅助函数(以及可能的displayed_output抽象)提取到 CLI 命令间共享的模块中。🤖 Prompt for 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. In `@crates/tokscale-cli/src/commands/models.rs` around lines 21 - 38, 将 models.rs 中的 checked_token_total、displayed_output 和 displayed_token_total 与 monthly.rs 的等价逻辑合并到 CLI 命令间共享模块，删除两处重复实现并让两类命令复用同一套溢出安全求和辅助函数；保留现有 displayed_output 与总 token 计算语义及溢出处理行为。
🤖 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.
Nitpick comments:
In `@crates/tokscale-cli/src/commands/models.rs`:
- Around line 21-38: 将 models.rs 中的 checked_token_total、displayed_output 和
displayed_token_total 与 monthly.rs 的等价逻辑合并到 CLI
命令间共享模块，删除两处重复实现并让两类命令复用同一套溢出安全求和辅助函数；保留现有 displayed_output 与总 token
计算语义及溢出处理行为。
In `@crates/tokscale-cli/tests/cli_tests.rs`:
- Around line 2378-2512: Extend the integration coverage in the monthly
reporting tests by adding a test equivalent to
test_models_report_clamps_reasoning_above_output for the monthly command. Create
a session fixture with reasoningTokens greater than output, invoke monthly
--json, and assert successful execution, output remains 50, and no reasoning
field is emitted; optionally include matching table-output assertions if
supported by the existing test style.
In `@crates/tokscale-core/src/adapters/omp.rs`:
- Around line 20-22: 补充 OMP_USAGE_AND_SWARM_REVISION 上方的注释，明确说明该版本号同时覆盖 swarm
agent 的识别变更，包括规范路径 `.swarm_<name>/context/...` 的识别；保留现有关于 malformed
inclusive-reasoning breakdowns 被归并到权威输出桶的说明。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 89d58a33-cd19-463a-a81d-ebbc2305f2ac
📒 Files selected for processing (11)
crates/tokscale-cli/src/commands/models.rscrates/tokscale-cli/src/commands/monthly.rscrates/tokscale-cli/src/tui/cache.rscrates/tokscale-cli/tests/cli_tests.rscrates/tokscale-core/src/adapters/omp.rscrates/tokscale-core/src/adapters/pi.rscrates/tokscale-core/src/aggregate/accumulators.rscrates/tokscale-core/src/aggregate/parity_tests.rscrates/tokscale-core/src/lib.rscrates/tokscale-core/src/sessions/pi.rsdocs/adr/0006-agent-identity-for-agents-tab.md
There was a problem hiding this comment.
All reported issues were addressed across 10 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
1 participant
Summary
reasoningand OMPreasoningTokensout of their inclusive source output buckets and require exact sourcetotalTokensoutput + reasoningback into inclusiveOutputacrossmodels,monthly, andhourlytext/JSON reports and every human-facing TUI output field.swarm_<name>/context/swarm-<name>-<agent>-<iteration>.jsonlartifacts as stable OMP swarm agents while preserving the full artifact stem as the instanceRoot cause
OMP defines
outputas the inclusive output-token count andreasoningTokensas a breakdown within it. A realxiaomi-token-plan / mimo-v2.5response stopped at the length limit withoutput=32000andreasoningTokens=32123, while bothtotalTokensand cost accounted for only 32000 output tokens. The Xiaomi usage detail therefore violated its own subset invariant.Tokscale now clamps only that malformed breakdown: the example becomes internal reasoning 32000 and non-reasoning output 0, while its exact source total remains unchanged. This prevents one provider metadata anomaly from aborting the entire OMP report. Negative token counts, missing required buckets, and mismatched
totalTokensremain hard errors.Report projection
Internal aggregation, pricing, caches, and explicit raw token-breakdown exports keep reasoning separate from non-reasoning output. Human-facing CLI reports and TUI views use inclusive output instead:
This projection applies to the
models,monthly, andhourlytext/JSON reports and to TUI Models, Overview, Stats, Hourly, Daily, Monthly, and Weekly output fields. Wrapped rankings use the complete five-bucket internal total. No surface adds a Reasoning column merely to expose the internal split.The local source-coverage survey and rationale for this fork policy are recorded in #138.
Validation
cargo fmt --all -- --checkcargo test --workspace— 2137 passed, 5 ignoredcargo clippy --workspace --all-targets --all-features -- -D warningsreasoningTokens=51, sourceoutput=50becomes internal reasoning 50/non-reasoning output 0, while reports remain Output 50, Total 165, with no warning fieldThe existing 596 all-zero assistant usage records remain skipped.