feat(reporting): isolate damaged local data and expose source health - #142
Conversation
A third-party source failure no longer aborts the whole local report. Discovery, snapshot, planning, and parse failures are contained to their source unit as structured SourceHealth (Complete/Partial/Unavailable plus per-reason rejection counts), collected into DataHealth and carried by AggregatedViews and report payloads. parse_checked returns per-unit outcomes instead of a batch Result, so one damaged artifact cannot erase other sources' data. CLI reports keep exit code 0, emit a health object in JSON, and print a stderr health summary. ADR 0021 records the contract; ADRs 0001/0020 gain failure-domain clarifications.
Zed is the tracer bullet for record-level isolation: a thread with a null/missing model, missing provider, undecodable payload, or missing timestamp is rejected and counted under a stable reason key while the rest of the database keeps parsing. A row that cannot be decoded at all interrupts the scan as Partial, keeping confirmed records uncached. Intentional filtering (imported, non-hosted providers, zero usage) remains uncounted.
Checkpoint Fable’s in-progress implementation for issue #141.
![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 across 87 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Finish Fable's issue #141 implementation by keeping third-party failures isolated, aggregating issue samples, ignoring Codex usage-less token events, and restoring Kiro warm-cache performance.
Continues fable's bad-data isolation implementation.
There was a problem hiding this comment.
All reported issues were addressed across 22 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/tokscale-cli/tests/cli_tests.rs (1)
1110-1135: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win测试名与新行为不一致。
函数名
test_opencode_obsolete_sqlite_schema_is_an_explicit_cli_error暗示命令会以错误方式退出,但断言体已改为.success(),该场景现在只是通过failedSources/stderr 摘要暴露来源失败,而不是 CLI 层面的错误。建议将函数名更新为类似test_opencode_obsolete_sqlite_schema_is_reported_as_failed_source_without_failing,以准确反映新契约(命令成功退出、失败信息体现在健康度字段中)。✏️ 建议的重命名
-fn test_opencode_obsolete_sqlite_schema_is_an_explicit_cli_error() { +fn test_opencode_obsolete_sqlite_schema_is_reported_as_failed_source_without_failing() {🤖 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 1110 - 1135, 将测试函数 test_opencode_obsolete_sqlite_schema_is_an_explicit_cli_error 重命名为能准确描述当前契约的名称，明确命令成功退出且过时 schema 通过 failedSources/错误摘要报告，而不是作为 CLI 错误失败；保持现有断言不变。crates/tokscale-core/src/sessions/commandcode.rs (1)
102-182: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win被拒绝的 assistant 记录会永久清零
turn_input_chars，导致下一条有效消息的 input token 被低估第105行
turn_input_chars = 0;在任何拒绝检查（token 总量溢出、缺失 sessionId、缺失/非法 timestamp）成功之前就已执行。一旦该 assistant 记录随后被拒绝（continue），之前累积的用户输入字符就已丢失，不会归还给下一条被接受的 assistant 消息，导致其inputtoken（进而费用）被低估。测试rejected_assistant_clears_pending_turn_start_before_next_assistant已验证is_turn_start存在同样"提前消费"的问题，但未对 token 数值做断言，因此本问题未被覆盖。建议只有在确认记录会被提交（push）之后才清零
turn_input_chars并消费pending_turn_start：建议修复
Some("assistant") => { let input = estimate_tokens(turn_input_chars); let output = estimate_tokens(chars); - turn_input_chars = 0; if input == 0 && output == 0 { pending_turn_start = false; + turn_input_chars = 0; continue; } - let is_turn_start = std::mem::take(&mut pending_turn_start); let tokens = TokenBreakdown { input, output, cache_read: 0, cache_write: 0, reasoning: 0, }; if tokens.checked_total().is_none() { scanned.rejections.record(...); continue; } let Some(resolved_session) = ... else { scanned.rejections.record(...); continue; }; let timestamp = match entry.timestamp.as_deref() { ... None => { scanned.rejections.record(...); continue; } }; if timestamp <= 0 { scanned.rejections.record(...); continue; } + turn_input_chars = 0; + let is_turn_start = std::mem::take(&mut pending_turn_start); let dedup_key = ...;注意：采纳此修复会改变
rejected_assistant_clears_pending_turn_start_before_next_assistant测试的预期结果（is_turn_start会变为true），需要同步更新该测试。🤖 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/sessions/commandcode.rs` around lines 102 - 182, 在处理 assistant 记录的分支中，将 turn_input_chars 的清零和 pending_turn_start 的消费延迟到所有校验通过且 UnifiedMessage 已成功加入 scanned.messages 之后；被拒绝的记录必须保留两者供下一条有效 assistant 使用。同步更新 rejected_assistant_clears_pending_turn_start_before_next_assistant 测试，使下一条有效消息的 is_turn_start 预期为 true。crates/tokscale-core/src/sessions/claudecode.rs (1)
233-268: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winTier2 flat 布局父路径拼接存在路径穿越风险,未复用同 PR 中已有的路径校验
parent_session_id来自 sidechain JSONL 文件自身的sessionId字段(不可信输入),这里只校验了非空(在调用方entry.session_id...filter(|parent_id| !parent_id.trim().is_empty())),随后在 251-253 行直接project_dir.join(format!("{}.jsonl", parent_session_id)),没有校验其是否为单段合法文件名。若sessionId被构造为包含../的字符串,可使查找逃出预期的项目目录,读取到的.jsonl文件内容(经lookup_subagent_type_in_parent解析后)会作为agent标签写入UnifiedMessage并展示给用户,构成路径穿越/信息泄露。同一 PR 的
crates/tokscale-core/src/adapters/claude.rs::resolve_flat_parent_dependency已经对完全相同语义的parent_session_id做了单段路径校验,但该校验未同步到这里。🔒 建议修复:复用单段路径校验
// Flat layout: parent dir is 1 level up if let Some(project_dir) = sidechain_path.parent() { + let mut components = std::path::Path::new(parent_session_id).components(); + let is_single_normal_segment = + matches!(components.next(), Some(std::path::Component::Normal(_))) + && components.next().is_none(); + if !is_single_normal_segment { + return Ok(None); + } let candidate = project_dir.join(format!("{}.jsonl", parent_session_id)); match std::fs::metadata(&candidate) {🤖 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/sessions/claudecode.rs` around lines 233 - 268, Update find_parent_session_path to validate parent_session_id as a single safe path component before constructing the flat-layout candidate, reusing the existing single-segment validation used by resolve_flat_parent_dependency rather than duplicating logic. Reject invalid values without joining them into project_dir, while preserving valid parent-session lookup behavior.
🧹 Nitpick comments (6)
crates/tokscale-core/src/sessions/droid.rs (1)
167-177: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win建议为“零 token 记录不计入 rejection”分支补充单测。
token_total == 0时直接return Ok(scanned),既不 push 消息也不记录 rejection——这正是 PR 契约中“排除已确认的零 token 记录”的关键分支,但当前测试集中没有覆盖它(对比openclaw.rs已有zero_usage_without_identity_is_filtered_without_rejection对称测试)。建议补充一个类似的 droid 测试用例,断言scanned.messages.is_empty() && scanned.rejections.is_empty()。As per path instructions for
crates/tokscale-core/src/**/*.rs: "Add focused Rust unit tests near the implementation for pure logic."🤖 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/sessions/droid.rs` around lines 167 - 177, 在 droid 的相关单元测试区域补充一个针对零 token 记录的 focused Rust 测试，覆盖处理逻辑中 token_total == 0 的分支；构造有效但总 token 数为零的记录，并断言结果中的 scanned.messages 和 scanned.rejections 均为空，确保该记录被过滤且不计入 rejection。Source: Path instructions
crates/tokscale-core/src/sessions/kiro.rs (1)
548-559: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
kiro_rejection_reason依赖脆弱的错误文案子串匹配。该函数通过
error.operation()/error.to_string()的子串("model_id"、"timestamp")推断RecordRejectionReason,而不是让各个invalid_at_path调用点显式携带分类。若日后修改任一处错误文案(例如把 "concrete model_id" 改写成别的措辞),该分类会静默退化为MalformedRecord,不会有编译期或测试失败提示,从而污染健康度统计中按 reason 展示的指标。建议让产生"缺模型/缺时间戳"错误的调用点显式返回/携带RecordRejectionReason,而不是靠字符串反推。🤖 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/sessions/kiro.rs` around lines 548 - 559, Refactor kiro_rejection_reason and the Kiro session validation flow so missing-model and missing-timestamp classifications are explicitly carried from each relevant invalid_at_path call site, rather than inferred from error.operation() or error.to_string() substrings. Preserve MalformedRecord for other parse failures, and update the surrounding error/result types and callers as needed to propagate RecordRejectionReason directly.crates/tokscale-core/src/sessions/claudecode.rs (1)
1145-1164: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
record_claude_error_rejection依赖字符串子串匹配推断拒绝原因,较为脆弱该函数通过
error.operation().contains("model")/detail.contains("missing model")/error.operation().contains("timestamp")等文本匹配来决定RecordRejectionReason,而不是使用显式/结构化的分类信息。当前所有测试用例都恰好匹配成功,但这种匹配对错误文案(operation 字符串、io::Error 消息措辞)的后续调整很敏感——一旦文案变化,可能被静默错误分类为MalformedRecord,而这会直接影响 PR 核心交付物(Issues 面板中展示的拒绝原因统计)的准确性。建议让产生这些错误的函数(如resolve_subagent_name、extract_claude_tool_result_message、finalize_headless_state)显式携带RecordRejectionReason,而不是靠下游猜测。🤖 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/sessions/claudecode.rs` around lines 1145 - 1164, Replace the string-based classification in record_claude_error_rejection with structured rejection metadata propagated from error-producing functions such as resolve_subagent_name, extract_claude_tool_result_message, and finalize_headless_state. Ensure each relevant SessionParseError carries its explicit RecordRejectionReason, and have record_claude_error_rejection record that reason while preserving the existing detail sample.crates/tokscale-core/src/adapters/claude.rs (1)
139-213: 🚀 Performance & Scalability | 🔵 Trivialdiscover 阶段可能对同一 sidechain 文件产生重复 I/O
configure_claude_parent_dependency→resolve_flat_parent_dependency会在 discover 阶段完整读取并逐行解析agent-*.jsonl(无 meta.json 时)以判断 sidechain 归属,而sessions/claudecode.rs::parse_claude_file_with_home在 parse 阶段会再次完整读取并解析同一个文件。对于命中此 fallback 的 legacy flat 布局文件,这意味着每次扫描都会读两遍相同内容。PR 描述提到该分支 benchmark 显示 RSS 更高、冷启动更慢,这类重复 I/O 可能是贡献因素之一,建议后续评估是否可以把 discover 阶段判定的结果(sidechain/parent 关系)缓存下来供 parse 阶段复用,避免二次读取。🤖 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/claude.rs` around lines 139 - 213, 避免 discover 与 parse 阶段重复读取 legacy flat 布局的 sidechain 文件：调整 configure_claude_parent_dependency 和 resolve_flat_parent_dependency，使 discover 阶段解析出的 sidechain/parent 关系可缓存并供 parse_claude_file_with_home 复用；确保缓存覆盖 parent、非 sidechain 与无法解析的结果，并保持现有依赖及 fingerprint 行为不变。crates/tokscale-core/src/adapters/warp.rs (1)
3-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win三个适配器的“记录拒绝修订号”常量硬编码,与同批次其它适配器的派生写法不一致。
WARP_RECORD_REJECTION_REVISION、HERMES_RECORD_REJECTION_REVISION、KILO_RECORD_REJECTION_REVISION均直接写成字面量4,而同一 PR 中的OPENCLAW_RECORD_REJECTION_REVISION(MODEL_ID_CANONICALIZATION_REVISION + 1)、ANTIGRAVITY_CACHE_RECORD_REJECTION_REVISION、CODEBUDDY_*_RECORD_REJECTION_REVISION等均采用基于基线常量派生的写法。当前数值恰好一致(3+1=4),但一旦MODEL_ID_CANONICALIZATION_REVISION未来被调整,这三处硬编码不会自动跟随变化,容易导致该三个适配器的缓存修订号意外与其它适配器发生数值重叠或未按预期递增,从而影响缓存失效的正确性判断。建议统一改为派生写法以保持一致性并降低未来遗漏的风险。
crates/tokscale-core/src/adapters/warp.rs#L3-L15:将const WARP_RECORD_REJECTION_REVISION: u32 = 4;改为crate::adapters::MODEL_ID_CANONICALIZATION_REVISION + 1形式的派生常量。crates/tokscale-core/src/adapters/hermes.rs#L5-L17:同上,将HERMES_RECORD_REJECTION_REVISION改为派生写法。crates/tokscale-core/src/adapters/kilo.rs#L3-L14:同上,将KILO_RECORD_REJECTION_REVISION改为派生写法。♻️ 建议修复示例(以 warp.rs 为例)
-const WARP_RECORD_REJECTION_REVISION: u32 = 4; +const WARP_RECORD_REJECTION_REVISION: u32 = + crate::adapters::MODEL_ID_CANONICALIZATION_REVISION + 1;🤖 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/warp.rs` around lines 3 - 15, Replace the literal revision values with derived constants based on MODEL_ID_CANONICALIZATION_REVISION plus one. Update WARP_RECORD_REJECTION_REVISION in crates/tokscale-core/src/adapters/warp.rs (lines 3-15), HERMES_RECORD_REJECTION_REVISION in crates/tokscale-core/src/adapters/hermes.rs (lines 5-17), and KILO_RECORD_REJECTION_REVISION in crates/tokscale-core/src/adapters/kilo.rs (lines 3-14); make no other changes.crates/tokscale-core/src/lib_tests.rs (1)
5833-5873: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win测试名与断言语义不符,建议重命名。
test_missing_configured_opencode_database_is_an_explicit_error的名字仍暗示"显式报错",但实测断言(Line 5838-5864)已经改为期望load_local_messages_for_test(...)成功返回(.unwrap()),仅通过loaded.health.failed_sources()==1表达降级——这正是本 PR 想要建立的"隔离为来源健康问题而非硬错误"的行为。保留旧名字容易让后续读者误以为该场景仍会返回Err。✏️ 建议重命名
-fn test_missing_configured_opencode_database_is_an_explicit_error() { +fn test_missing_configured_opencode_database_is_isolated_as_source_health() {🤖 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/lib_tests.rs` around lines 5833 - 5873, Rename test_missing_configured_opencode_database_is_an_explicit_error to reflect that a missing configured OpenCode database is isolated as a source health failure while loading still succeeds. Keep the existing assertions and test behavior unchanged.
🤖 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/issues.rs`:
- Around line 234-258: 在 record_table_row 中修正 handling 单元格的
truncate_display_width 调用，移除 row.handling 前多余的引用，使其传入 &str 而非 &&str；保留
row.source 和 row.issue 的现有引用方式不变。
In `@crates/tokscale-core/src/adapters/omp.rs`:
- Around line 491-517: Update omp_parent_health_units to construct parent health
SourceUnit values with ParserId::OmpParentHealth and OMP_PARENT_HEALTH_REVISION,
matching the omp_parent_health_unit batch path; leave the health status and
rejection handling unchanged.
In `@crates/tokscale-core/src/sessions/commandcode.rs`:
- Around line 76-85: Update the JSONL parsing error branches in commandcode.rs
(76-85) and kimi.rs (106-116) to record RecordRejectionReason::MalformedRecord
for the invalid line and continue scanning subsequent lines instead of setting
scanned.interrupted and breaking. Keep scanned.interrupted reserved for read/I/O
failures.
In `@crates/tokscale-core/src/sessions/junie.rs`:
- Around line 195-214: Update the duration_ms parsing in the session message
construction around number_field(usage, "time") so an unparseable time records
duration_ms as None and continues processing the already validated message.
Remove the rejection-and-continue path for this optional field while preserving
the existing positive-duration filtering and message insertion flow.
In `@crates/tokscale-core/src/sessions/kiro.rs`:
- Around line 427-435: 修正 Kiro 会话校验中 header 与 sqlite 两条路径的错误文案：在
context_window_tokens 为负且 ctx_pct 大于 0 时，明确说明基于百分比的估算需要非负
context_window_tokens，而当前值为负导致无法估算。仅更新两处 invalid_at_path 的消息，保持拒绝条件和其余错误处理不变。
In `@crates/tokscale-core/src/sessions/roocode.rs`:
- Around line 84-96: Update the usage_events tuple and its push site to retain
each event’s original event_number alongside timestamp, token_breakdown, and
provider. In the MissingModel branch, iterate over the retained original numbers
when recording MissingModel rejections, then adjust the message-construction
loop to destructure the expanded tuple while preserving existing message fields.
In `@crates/tokscale-core/src/sessions/warp.rs`:
- Around line 265-291: Update load_query_metadata so prepare/query failures for
the auxiliary ai_queries metadata do not populate scanned.interrupted or
downgrade the Warp scan to Partial. Record the failure through the existing
rejection or non-fatal degradation mechanism, then return empty metadata while
preserving the already loaded agent_conversations results and cacheable source
status.
In `@crates/tokscale-core/src/source_health.rs`:
- Around line 268-302: Update the record-group initialization in
SourceHealth::to_report so SourceHealthReport.status uses the current
source.status instead of the fixed "complete" value. Preserve the existing
grouping and rejection aggregation behavior while ensuring partial or
unavailable sources retain their status in HealthReport.sources.
---
Outside diff comments:
In `@crates/tokscale-cli/tests/cli_tests.rs`:
- Around line 1110-1135: 将测试函数
test_opencode_obsolete_sqlite_schema_is_an_explicit_cli_error
重命名为能准确描述当前契约的名称，明确命令成功退出且过时 schema 通过 failedSources/错误摘要报告，而不是作为 CLI
错误失败；保持现有断言不变。
In `@crates/tokscale-core/src/sessions/claudecode.rs`:
- Around line 233-268: Update find_parent_session_path to validate
parent_session_id as a single safe path component before constructing the
flat-layout candidate, reusing the existing single-segment validation used by
resolve_flat_parent_dependency rather than duplicating logic. Reject invalid
values without joining them into project_dir, while preserving valid
parent-session lookup behavior.
In `@crates/tokscale-core/src/sessions/commandcode.rs`:
- Around line 102-182: 在处理 assistant 记录的分支中，将 turn_input_chars 的清零和
pending_turn_start 的消费延迟到所有校验通过且 UnifiedMessage 已成功加入 scanned.messages
之后；被拒绝的记录必须保留两者供下一条有效 assistant 使用。同步更新
rejected_assistant_clears_pending_turn_start_before_next_assistant 测试，使下一条有效消息的
is_turn_start 预期为 true。
---
Nitpick comments:
In `@crates/tokscale-core/src/adapters/claude.rs`:
- Around line 139-213: 避免 discover 与 parse 阶段重复读取 legacy flat 布局的 sidechain
文件：调整 configure_claude_parent_dependency 和 resolve_flat_parent_dependency，使
discover 阶段解析出的 sidechain/parent 关系可缓存并供 parse_claude_file_with_home 复用；确保缓存覆盖
parent、非 sidechain 与无法解析的结果，并保持现有依赖及 fingerprint 行为不变。
In `@crates/tokscale-core/src/adapters/warp.rs`:
- Around line 3-15: Replace the literal revision values with derived constants
based on MODEL_ID_CANONICALIZATION_REVISION plus one. Update
WARP_RECORD_REJECTION_REVISION in crates/tokscale-core/src/adapters/warp.rs
(lines 3-15), HERMES_RECORD_REJECTION_REVISION in
crates/tokscale-core/src/adapters/hermes.rs (lines 5-17), and
KILO_RECORD_REJECTION_REVISION in crates/tokscale-core/src/adapters/kilo.rs
(lines 3-14); make no other changes.
In `@crates/tokscale-core/src/lib_tests.rs`:
- Around line 5833-5873: Rename
test_missing_configured_opencode_database_is_an_explicit_error to reflect that a
missing configured OpenCode database is isolated as a source health failure
while loading still succeeds. Keep the existing assertions and test behavior
unchanged.
In `@crates/tokscale-core/src/sessions/claudecode.rs`:
- Around line 1145-1164: Replace the string-based classification in
record_claude_error_rejection with structured rejection metadata propagated from
error-producing functions such as resolve_subagent_name,
extract_claude_tool_result_message, and finalize_headless_state. Ensure each
relevant SessionParseError carries its explicit RecordRejectionReason, and have
record_claude_error_rejection record that reason while preserving the existing
detail sample.
In `@crates/tokscale-core/src/sessions/droid.rs`:
- Around line 167-177: 在 droid 的相关单元测试区域补充一个针对零 token 记录的 focused Rust
测试，覆盖处理逻辑中 token_total == 0 的分支；构造有效但总 token 数为零的记录，并断言结果中的 scanned.messages 和
scanned.rejections 均为空，确保该记录被过滤且不计入 rejection。
In `@crates/tokscale-core/src/sessions/kiro.rs`:
- Around line 548-559: Refactor kiro_rejection_reason and the Kiro session
validation flow so missing-model and missing-timestamp classifications are
explicitly carried from each relevant invalid_at_path call site, rather than
inferred from error.operation() or error.to_string() substrings. Preserve
MalformedRecord for other parse failures, and update the surrounding
error/result types and callers as needed to propagate RecordRejectionReason
directly.
🪄 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: e9b6ec9b-43b0-4179-9161-c044bc7e50b0
📒 Files selected for processing (88)
Cargo.tomlcrates/tokscale-cli/src/commands/clients.rscrates/tokscale-cli/src/commands/graph.rscrates/tokscale-cli/src/commands/hourly.rscrates/tokscale-cli/src/commands/models.rscrates/tokscale-cli/src/commands/monthly.rscrates/tokscale-cli/src/commands/shared.rscrates/tokscale-cli/src/commands/time_metrics.rscrates/tokscale-cli/src/commands/wrapped.rscrates/tokscale-cli/src/tui/app.rscrates/tokscale-cli/src/tui/cache.rscrates/tokscale-cli/src/tui/export.rscrates/tokscale-cli/src/tui/mod.rscrates/tokscale-cli/src/tui/ui/footer.rscrates/tokscale-cli/src/tui/ui/header.rscrates/tokscale-cli/src/tui/ui/issues.rscrates/tokscale-cli/src/tui/ui/mod.rscrates/tokscale-cli/tests/cli_tests.rscrates/tokscale-core/src/adapters/antigravity.rscrates/tokscale-core/src/adapters/cache.rscrates/tokscale-core/src/adapters/claude.rscrates/tokscale-core/src/adapters/codebuddy.rscrates/tokscale-core/src/adapters/codebuff.rscrates/tokscale-core/src/adapters/codex.rscrates/tokscale-core/src/adapters/discover.rscrates/tokscale-core/src/adapters/error.rscrates/tokscale-core/src/adapters/file.rscrates/tokscale-core/src/adapters/goose.rscrates/tokscale-core/src/adapters/hermes.rscrates/tokscale-core/src/adapters/junie.rscrates/tokscale-core/src/adapters/kilo.rscrates/tokscale-core/src/adapters/kiro.rscrates/tokscale-core/src/adapters/mod.rscrates/tokscale-core/src/adapters/omp.rscrates/tokscale-core/src/adapters/openclaw.rscrates/tokscale-core/src/adapters/opencode.rscrates/tokscale-core/src/adapters/pi.rscrates/tokscale-core/src/adapters/trae.rscrates/tokscale-core/src/adapters/vscode_tasks.rscrates/tokscale-core/src/adapters/warp.rscrates/tokscale-core/src/adapters/zed.rscrates/tokscale-core/src/aggregate/accumulators.rscrates/tokscale-core/src/aggregate/engine.rscrates/tokscale-core/src/aggregate/graph_result.rscrates/tokscale-core/src/aggregate/parity_tests.rscrates/tokscale-core/src/aggregate/tui.rscrates/tokscale-core/src/aggregate/views.rscrates/tokscale-core/src/lib.rscrates/tokscale-core/src/lib_tests.rscrates/tokscale-core/src/message_cache.rscrates/tokscale-core/src/sessions/amp.rscrates/tokscale-core/src/sessions/antigravity.rscrates/tokscale-core/src/sessions/antigravity_cli.rscrates/tokscale-core/src/sessions/claudecode.rscrates/tokscale-core/src/sessions/cline.rscrates/tokscale-core/src/sessions/codebuddy.rscrates/tokscale-core/src/sessions/codebuff.rscrates/tokscale-core/src/sessions/codex.rscrates/tokscale-core/src/sessions/commandcode.rscrates/tokscale-core/src/sessions/copilot.rscrates/tokscale-core/src/sessions/cursor.rscrates/tokscale-core/src/sessions/droid.rscrates/tokscale-core/src/sessions/gemini.rscrates/tokscale-core/src/sessions/goose.rscrates/tokscale-core/src/sessions/grok.rscrates/tokscale-core/src/sessions/hermes.rscrates/tokscale-core/src/sessions/junie.rscrates/tokscale-core/src/sessions/kilo.rscrates/tokscale-core/src/sessions/kilocode.rscrates/tokscale-core/src/sessions/kimi.rscrates/tokscale-core/src/sessions/kiro.rscrates/tokscale-core/src/sessions/mux.rscrates/tokscale-core/src/sessions/openclaw.rscrates/tokscale-core/src/sessions/opencode.rscrates/tokscale-core/src/sessions/pi.rscrates/tokscale-core/src/sessions/qwen.rscrates/tokscale-core/src/sessions/roocode.rscrates/tokscale-core/src/sessions/trae.rscrates/tokscale-core/src/sessions/warp.rscrates/tokscale-core/src/sessions/zcode.rscrates/tokscale-core/src/sessions/zed.rscrates/tokscale-core/src/source_health.rscrates/tokscale-core/src/usage_views.rscrates/tokscale-core/tests/codebuff.rscrates/tokscale-core/tests/hermes.rsdocs/adr/0001-no-silent-fallback.mddocs/adr/0020-strict-source-identity-and-error-contract.mddocs/adr/0021-isolated-source-failure-domains.md
👮 Files not reviewed due to content moderation or server errors (16)
- crates/tokscale-core/tests/codebuff.rs
- crates/tokscale-core/src/sessions/antigravity_cli.rs
- crates/tokscale-core/src/sessions/antigravity.rs
- crates/tokscale-core/src/sessions/opencode.rs
- crates/tokscale-core/src/sessions/codex.rs
- crates/tokscale-core/src/sessions/codebuff.rs
- crates/tokscale-core/src/sessions/codebuddy.rs
- crates/tokscale-core/src/sessions/gemini.rs
- crates/tokscale-core/src/sessions/amp.rs
- crates/tokscale-core/src/sessions/cursor.rs
- crates/tokscale-core/src/adapters/cache.rs
- crates/tokscale-core/src/sessions/zcode.rs
- crates/tokscale-core/src/sessions/qwen.rs
- crates/tokscale-core/src/adapters/file.rs
- crates/tokscale-core/src/sessions/copilot.rs
- crates/tokscale-core/src/sessions/grok.rs
Walkthrough本次变更引入来源健康模型，将解析器、适配器、缓存、聚合结果、CLI 输出和 TUI Issues 页面统一扩展为携带拒绝记录、来源失败和扫描中断信息，并将多数第三方数据错误从整体失败改为局部记录与继续处理。 Changes来源健康与失败隔离
Estimated code review effort: 5 (Critical) | ~120 minutes 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 |
Continues Fable's bad-data isolation implementation.
There was a problem hiding this comment.
All reported issues were addressed across 45 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/tokscale-core/src/sessions/copilot.rs (1)
83-137: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift两阶段扫描可能是性能回归的根因之一,建议核实是否可合并为单次遍历
根据行区间变更说明,
parse_copilot_file现在分两阶段调用for_each_json_record:第一阶段collect_trace_contexts遍历整个文件构建 trace 上下文,第二阶段再次调用for_each_json_record提取 usage 候选。这意味着同一个 JSONL 源文件会被完整读取与反序列化两次。PR 描述中提到"the benchmark reports increased RSS and slower cold execution",两次全量文件扫描很可能是该回归的主要贡献因素之一。建议确认是否可以将 trace-context 收集与候选提取合并进单次遍历(例如先收集 trace 上下文所需的最小信息,同时缓存反序列化后的记录供第二阶段复用,避免重复 I/O 与 JSON 解析)。
🤖 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/sessions/copilot.rs` around lines 83 - 137, 合并 parse_copilot_file 中由 collect_trace_contexts 和 usage 候选提取触发的两次 for_each_json_record 扫描，使每个 JSONL 文件仅进行一次读取与反序列化；在同一遍历中先收集所需的最小 trace 上下文并提取候选，保持现有 record_limit、错误处理和结果行为不变，避免重复 I/O 与 JSON 解析。crates/tokscale-core/src/sessions/cursor.rs (1)
172-178: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
input/output/cache_read的.max(0)属冗余死代码147-155 行已经对
input_with_cache_write、input_without_cache_write、cache_read、output_tokens四个值统一做了< 0校验并拒绝,因此input(=input_without_cache_write)、output_tokens、cache_read到此处已保证非负,172-175 行的.max(0)是多余的防御性代码。而紧邻的cache_write(169 行)恰恰因为已被证明非负而没有做裁剪,风格不一致,容易让后续维护者误判这些字段仍可能为负。♻️ 建议简化
let tokens = TokenBreakdown { - input: input.max(0), - output: output_tokens.max(0), - cache_read: cache_read.max(0), + input, + output: output_tokens, + cache_read, cache_write, reasoning: 0, };🤖 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/sessions/cursor.rs` around lines 172 - 178, Remove the redundant .max(0) calls from input, output_tokens, and cache_read when constructing TokenBreakdown in the cursor handling flow, passing the already-validated nonnegative values directly. Keep cache_write and the reasoning field unchanged.
🤖 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/issues.rs`:
- Around line 149-190: Update issue_rows and handling_label to gracefully handle
unknown level and handling strings instead of panicking. Preserve the existing
labels and colors for known values, while falling back to displaying the
original value for new or persisted cache values, following issue_label’s
behavior.
---
Nitpick comments:
In `@crates/tokscale-core/src/sessions/copilot.rs`:
- Around line 83-137: 合并 parse_copilot_file 中由 collect_trace_contexts 和 usage
候选提取触发的两次 for_each_json_record 扫描，使每个 JSONL 文件仅进行一次读取与反序列化；在同一遍历中先收集所需的最小 trace
上下文并提取候选，保持现有 record_limit、错误处理和结果行为不变，避免重复 I/O 与 JSON 解析。
In `@crates/tokscale-core/src/sessions/cursor.rs`:
- Around line 172-178: Remove the redundant .max(0) calls from input,
output_tokens, and cache_read when constructing TokenBreakdown in the cursor
handling flow, passing the already-validated nonnegative values directly. Keep
cache_write and the reasoning field unchanged.
🪄 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: 428f5015-4ddd-449f-a42a-eb3d9e3c0b87
📒 Files selected for processing (45)
crates/tokscale-cli/src/commands/clients.rscrates/tokscale-cli/src/commands/graph.rscrates/tokscale-cli/src/commands/wrapped.rscrates/tokscale-cli/src/tui/cache.rscrates/tokscale-cli/src/tui/export.rscrates/tokscale-cli/src/tui/ui/issues.rscrates/tokscale-cli/tests/cli_tests.rscrates/tokscale-core/src/adapters/codex.rscrates/tokscale-core/src/adapters/file.rscrates/tokscale-core/src/adapters/omp.rscrates/tokscale-core/src/adapters/opencode.rscrates/tokscale-core/src/adapters/zed.rscrates/tokscale-core/src/lib_tests.rscrates/tokscale-core/src/message_cache.rscrates/tokscale-core/src/sessions/amp.rscrates/tokscale-core/src/sessions/antigravity.rscrates/tokscale-core/src/sessions/antigravity_cli.rscrates/tokscale-core/src/sessions/claudecode.rscrates/tokscale-core/src/sessions/codebuddy.rscrates/tokscale-core/src/sessions/codebuff.rscrates/tokscale-core/src/sessions/codex.rscrates/tokscale-core/src/sessions/commandcode.rscrates/tokscale-core/src/sessions/copilot.rscrates/tokscale-core/src/sessions/cursor.rscrates/tokscale-core/src/sessions/droid.rscrates/tokscale-core/src/sessions/gemini.rscrates/tokscale-core/src/sessions/goose.rscrates/tokscale-core/src/sessions/grok.rscrates/tokscale-core/src/sessions/hermes.rscrates/tokscale-core/src/sessions/junie.rscrates/tokscale-core/src/sessions/kilo.rscrates/tokscale-core/src/sessions/kimi.rscrates/tokscale-core/src/sessions/kiro.rscrates/tokscale-core/src/sessions/mux.rscrates/tokscale-core/src/sessions/openclaw.rscrates/tokscale-core/src/sessions/opencode.rscrates/tokscale-core/src/sessions/pi.rscrates/tokscale-core/src/sessions/qwen.rscrates/tokscale-core/src/sessions/roocode.rscrates/tokscale-core/src/sessions/trae.rscrates/tokscale-core/src/sessions/warp.rscrates/tokscale-core/src/sessions/zcode.rscrates/tokscale-core/src/sessions/zed.rscrates/tokscale-core/src/source_health.rsdocs/adr/0021-isolated-source-failure-domains.md
💤 Files with no reviewable changes (1)
- crates/tokscale-core/src/adapters/zed.rs
🚧 Files skipped from review as they are similar to previous changes (37)
- crates/tokscale-cli/src/tui/export.rs
- crates/tokscale-cli/src/commands/clients.rs
- crates/tokscale-cli/src/commands/graph.rs
- crates/tokscale-cli/src/commands/wrapped.rs
- crates/tokscale-core/src/sessions/hermes.rs
- crates/tokscale-core/src/sessions/antigravity_cli.rs
- crates/tokscale-core/src/sessions/antigravity.rs
- crates/tokscale-core/src/sessions/commandcode.rs
- crates/tokscale-core/src/sessions/qwen.rs
- docs/adr/0021-isolated-source-failure-domains.md
- crates/tokscale-core/src/sessions/trae.rs
- crates/tokscale-core/src/sessions/kimi.rs
- crates/tokscale-core/src/sessions/goose.rs
- crates/tokscale-cli/src/tui/cache.rs
- crates/tokscale-core/src/sessions/mux.rs
- crates/tokscale-core/src/adapters/opencode.rs
- crates/tokscale-core/src/sessions/kilo.rs
- crates/tokscale-core/src/sessions/zcode.rs
- crates/tokscale-core/src/sessions/amp.rs
- crates/tokscale-core/src/sessions/zed.rs
- crates/tokscale-cli/tests/cli_tests.rs
- crates/tokscale-core/src/sessions/codebuff.rs
- crates/tokscale-core/src/sessions/junie.rs
- crates/tokscale-core/src/sessions/warp.rs
- crates/tokscale-core/src/adapters/codex.rs
- crates/tokscale-core/src/sessions/claudecode.rs
- crates/tokscale-core/src/sessions/droid.rs
- crates/tokscale-core/src/adapters/omp.rs
- crates/tokscale-core/src/sessions/codex.rs
- crates/tokscale-core/src/adapters/file.rs
- crates/tokscale-core/src/sessions/roocode.rs
- crates/tokscale-core/src/sessions/grok.rs
- crates/tokscale-core/src/sessions/openclaw.rs
- crates/tokscale-core/src/lib_tests.rs
- crates/tokscale-core/src/sessions/opencode.rs
- crates/tokscale-core/src/sessions/codebuddy.rs
- crates/tokscale-core/src/sessions/gemini.rs
Reuse each OMP parent digest across dependent children, scan independent parents in parallel, and avoid clean empty shards while retaining cached rejection summaries.
|
The Roo Code allocation finding from the latest cubic review body was addressed in 4fefead. The private payload helpers now return |
There was a problem hiding this comment.
All reported issues were addressed across 9 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Resolve nested child labels from dynamic OMP agent names. Mark scans partial when source snapshots diverge and discard stale parent-health rejections.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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-core/src/adapters/cache.rs`:
- Around line 252-253: 更新 cacheable_output 判定及其相关缓存流程，使完整扫描即使没有 messages 和
rejections 也能写入空 shard；保留非完整扫描的现有过滤行为，并同步调整覆盖该逻辑的测试，验证 clean-empty source
可持久化且后续可恢复为 Complete 状态。
- Around line 191-206: Update the precomputed_snapshot_mismatch path around
scan_source so stale OmpParentTaskAgentIndex data cannot derive agent labels.
When the snapshot mismatches, rebuild the parent index before scanning, or
disable parent-index-dependent label derivation until the index is refreshed;
ensure no confirmed token data is retained with labels produced from the old
index.
🪄 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: 72328907-cee9-4d53-82ad-b7b618459a31
📒 Files selected for processing (7)
crates/tokscale-core/src/adapters/cache.rscrates/tokscale-core/src/adapters/omp.rscrates/tokscale-core/src/message_cache.rscrates/tokscale-core/src/sessions/commandcode.rscrates/tokscale-core/src/sessions/kimi.rscrates/tokscale-core/src/sessions/pi.rsdocs/adr/0021-isolated-source-failure-domains.md
🚧 Files skipped from review as they are similar to previous changes (5)
- crates/tokscale-core/src/sessions/commandcode.rs
- docs/adr/0021-isolated-source-failure-domains.md
- crates/tokscale-core/src/sessions/kimi.rs
- crates/tokscale-core/src/adapters/omp.rs
- crates/tokscale-core/src/message_cache.rs
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
Regarding the
Applying OMO canonicalization to OMP would introduce the cross-client namespace coupling that the finding claims to prevent. No code change is warranted for this finding. |
1 participant
Summary
IssuesTUI tab with a health summary and aggregate issue detailsScannedSourceor the equivalent Codex incremental outcomeBehavior
Malformed records are rejected and counted while later independently interpretable records in the same source continue. Missing model, provider, or timestamp values and malformed payloads are data-health issues. Intentional filters such as imported records, unsupported providers, and confirmed zero-token records remain uncounted.
Unreadable sources become
Unavailable; interrupted scans becomePartialand retain records confirmed before the interruption. Neither state aborts unrelated sources. Tokscale contract violations, invalid requests, configuration errors, and cache write failures remain hard errors.Every examined source is classified into exactly one public state:
Clean: scan completed without rejected recordsDegraded: scan completed after rejecting one or more recordsPartial: scan stopped, but confirmed records remainFailed: source was unavailable and produced no recordsRejected-record counts remain a separate dimension because one degraded source may contain multiple damaged records.
Public health output is deliberately bounded. Issues are grouped by source, reason, status, and handling outcome. Raw paths, parser operations, parser messages, record payloads, representative samples, and per-session error lists remain internal and never enter the TUI, aggregate cache, or public JSON payload.
TUI and CLI
IssuestabLEVEL,SOURCE,ISSUE,SOURCES,RECORDS, andHANDLINGfieldshealthprojection0; nonzero exits remain reserved for invalid usage and internal errorsCache and shared inputs
Partial, stale parent-health rejections are discarded, and no shard is written.meta.jsonsidecar is absentParser coverage
All 31 registered production adapters now expose record health. The previous vector-result session parsers use
ScannedSource; Codex retains its stateful incremental path with equivalent rejection and interruption semantics. Codex rejects independently malformedtoken_countrecords without discarding later valid usage, while damaged state-bearing records stop the source asPartialbefore corrupting parser state.Breaking API
parse_local_unified_messagesandparse_local_unified_messages_with_pricingnow returnLocalReport<Vec<UnifiedMessage>>instead of a bare vector. Callers read.dataand may inspect.healthand.metadata. Public report structures also gain the bounded serializedhealthfield.Validation
cargo test --workspacecargo clippy --workspace --all-targets -- -D warningscargo fmt --all -- --checkgit diff --checkcargo build --release -p tokscale-cliRemaining boundary
Discovery failures that occur before individual
SourceUnitidentities exist are attributed to the adapter being discovered. Per-root and per-path discovery isolation is a separate boundary documented in ADR 0021.Closes #141.