Repository navigation
Include Claude Code subagent transcripts - #209
Conversation
Discover nested Claude subagent JSONL files and preserve their top-level project identity. Reconcile cross-file deduplication across CLI, TUI, watcher, history, and upload paths, and backfill newly visible Claude usage for existing cloud users after a successful upload.
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughClaude Code now discovers nested subagent transcripts, deduplicates cross-source messages, and requests full reloads when source changes affect shared contributions. Watcher, analyzer upload preparation, and persisted backfill state are updated to support this behavior. ChangesClaude reload and backfill
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant FileWatcher
participant RealtimeStatsManager
participant AnalyzerRegistry
participant ClaudeCodeAnalyzer
participant UploadState
participant BackgroundUpload
FileWatcher->>RealtimeStatsManager: FileChanged or FileDeleted
RealtimeStatsManager->>AnalyzerRegistry: Check full-reload capability
AnalyzerRegistry->>ClaudeCodeAnalyzer: Reload complete stats
ClaudeCodeAnalyzer->>AnalyzerRegistry: Canonical messages
BackgroundUpload->>UploadState: Load backfill state
BackgroundUpload->>BackgroundUpload: Select Claude messages
BackgroundUpload->>UploadState: Mark backfill completed
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/analyzers/claude_code.rs (1)
209-213: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
is_available()does a full discovery scan instead of short-circuiting.The trait doc explicitly invites overriding
is_available()to stop after finding one file, but this implementation still runs the entirediscover_sources_intraversal (allocating and sorting the fullVec<DataSource>) just to check non-emptiness — and as a side effect it overwritesdiscovery_was_completefrom this unrelated scan (compounding the concern raised above).⚡ Early-exit availability check
fn is_available(&self) -> bool { Self::data_dir() .filter(|directory| directory.is_dir()) - .is_some_and(|projects_dir| !self.discover_sources_in(&projects_dir).is_empty()) + .is_some_and(|projects_dir| { + WalkDir::new(&projects_dir) + .min_depth(2) + .into_iter() + .filter_entry(|entry| is_claude_transcript_tree_path(&projects_dir, entry.path())) + .filter_map(Result::ok) + .any(|entry| { + entry.file_type().is_file() + && is_claude_transcript_path(&projects_dir, entry.path()) + }) + }) }🤖 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 `@src/analyzers/claude_code.rs` around lines 209 - 213, Update ClaudeCodeAnalyzer::is_available to perform a lightweight early-exit search for the first discoverable source instead of calling discover_sources_in and building the full sorted Vec<DataSource>. Ensure this availability-only check does not mutate discovery_was_complete, while preserving the existing data-directory validation and boolean result.src/upload.rs (1)
516-542: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMagic string duplicates
ClaudeCodeAnalyzer::display_name().
analyzer_stats.analyzer_name == "Claude Code"re-derives a literal that's already defined asClaudeCodeAnalyzer::display_name()insrc/analyzers/claude_code.rs. If that display name ever changes, this comparison silently stops matching (no compile error), quietly breaking the backfill gate.♻️ Reference a shared constant instead of a literal
- if backfill_claude && analyzer_stats.analyzer_name == "Claude Code" { + if backfill_claude + && analyzer_stats.analyzer_name == crate::analyzers::claude_code::ClaudeCodeAnalyzer::DISPLAY_NAME + {(requires exposing a
pub const DISPLAY_NAME: &str = "Claude Code";used by bothdisplay_name()and this comparison)🤖 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 `@src/upload.rs` around lines 516 - 542, Replace the duplicated "Claude Code" literal in the upload-state backfill comparison with a shared public constant exposed by ClaudeCodeAnalyzer, such as DISPLAY_NAME. Update ClaudeCodeAnalyzer::display_name() to use that constant, then reference it in the comparison so both paths stay synchronized.
🤖 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 `@src/analyzer.rs`:
- Around line 783-800: The full-reload analyzer messages are computed twice
during one watcher event. Update src/watcher.rs lines 167-199 so
reload_analyzer_stats passes the messages from its get_stats call through
trigger_auto_upload_if_enabled, and update src/analyzer.rs lines 783-800 so
load_messages_for_upload reuses those messages instead of calling get_stats
again; preserve the existing incremental parsing behavior for non-full-reload
analyzers.
---
Nitpick comments:
In `@src/analyzers/claude_code.rs`:
- Around line 209-213: Update ClaudeCodeAnalyzer::is_available to perform a
lightweight early-exit search for the first discoverable source instead of
calling discover_sources_in and building the full sorted Vec<DataSource>. Ensure
this availability-only check does not mutate discovery_was_complete, while
preserving the existing data-directory validation and boolean result.
In `@src/upload.rs`:
- Around line 516-542: Replace the duplicated "Claude Code" literal in the
upload-state backfill comparison with a shared public constant exposed by
ClaudeCodeAnalyzer, such as DISPLAY_NAME. Update
ClaudeCodeAnalyzer::display_name() to use that constant, then reference it in
the comparison so both paths stay synchronized.
🪄 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: 180ba064-5e73-4653-9e56-a20bfb15b4e0
📒 Files selected for processing (7)
src/analyzer.rssrc/analyzers/claude_code.rssrc/analyzers/claude_code_history.rssrc/analyzers/tests/claude_code.rssrc/config.rssrc/upload.rssrc/watcher.rs
Pass canonical messages from analyzer-wide watcher reloads into the upload path instead of reparsing the full Claude corpus. Short-circuit availability checks and share the Claude display name used by the backfill gate.
…ng snapshots A streaming assistant message is written as several JSONL records under one (requestId, message.id) key. Only output_tokens grows between snapshots; the input and cache fields are identical on every record. merge_message_into summed every token field whenever a record carried a fingerprint not seen before, so those identical fields were counted once per snapshot, and cost followed. Main transcripts repeat the final usage on every record, so the fingerprint never changes there and the rule was harmless. Subagent transcripts do not, and they became reachable when Piebald-AI#209 lifted the depth cap, which is what armed this. Selection now keeps the whole record with the higher output_tokens, falling back to the four-field total when output ties. A per-field maximum would assemble a record that was never written, and cost derives from those fields, so the resulting cost would correspond to no observation. The comment states the protocol assumption the rule rests on, so a future writer path emitting additive chunks has somewhere obvious to start. The non-token counters are merged with max on both paths now rather than summed on one of them: they are per-message totals restated on every record, not per-record increments. Three existing assertions changed with the rule. Their fixtures give the two records under one key different input and cache values, a shape that does not occur in real transcripts. A fixture with the real shape is added. Refs Piebald-AI#220
Why
Claude Code writes Task, Explore, general-purpose, and custom-agent transcripts beneath
~/.claude/projects/<project>/<session>/subagents/**. Splitrail only discovered main transcripts at~/.claude/projects/<project>/<session>.jsonl, so subagent-heavy workflows could omit most Claude messages and token usage from local statistics, live TUI updates, uploads, and leaderboard totals.Simply lifting the directory-depth cap is insufficient. Main and subagent transcripts can repeat the same Claude response, while Splitrail's TUI cache and incremental upload paths calculate per-source contributions. Those paths need the same cross-file
local_hashand token-fingerprint semantics as complete CLI statistics or the newly discovered files can inflate totals.What changed
<project>/<session>/subagents/**, while rejecting metadata and unrelated nested JSONL formats.local_hashrecords, accumulate distinct token fingerprints once, and keep records without alocal_hashindependent.last_date_uploaded; preserve monotonic upload checkpoints and retry the backfill after failures.User impact
Claude Code users who rely on subagents will see previously omitted usage appear after upgrading. Local totals should increase to include subagent work without double-counting responses repeated in main transcripts. Existing configured Cloud users will perform one canonical Claude history backfill; stable message hashes allow already-uploaded records to be deduplicated server-side.
Validation
cargo build --quietcargo test --quiet— 393 passedcargo clippy --quiet -- -D warningscargo doc --quietcargo fmt --all --quiet --checkgit diff --checkSummary by CodeRabbit
New Features
Bug Fixes