perf(codex): prune historical files for date ranges - #1599
Conversation
Use a conservative file-mtime prefilter before replay planning and parsing while keeping event-level date filtering authoritative. Preserve Codex detection when the selected range has no rows. Closes ccusage#1598
|
@coderabbitai review this PR, please. @cubic-dev-ai review this PR if available. |
|
This PR was auto-closed. Only contributors approved with Maintainers review auto-closed issues and reopen worthwhile ones. Issues that do not meet the quality bar in CONTRIBUTING.md may not be reopened or receive a reply. If a maintainer replies See CONTRIBUTING.md. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughCodex aggregation now filters historical usage files before replay planning. Unified loading uses ChangesCodex date-bounded loading
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant UnifiedLoader
participant codex_load_groups
participant retain_report_files
participant CodexSources
UnifiedLoader->>codex_load_groups: request date-bounded groups
codex_load_groups->>retain_report_files: filter report files
retain_report_files->>CodexSources: inspect file modification times
CodexSources-->>retain_report_files: return eligible files
retain_report_files-->>codex_load_groups: return filtered files
codex_load_groups-->>UnifiedLoader: return matching groups
UnifiedLoader->>CodexSources: check has_data
CodexSources-->>UnifiedLoader: return source detection status
Possibly related PRs
Suggested reviewers: ✨ 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 |
@zhangxaochen I have started the AI code review. It will take a few minutes to complete. |
|
|
There was a problem hiding this comment.
Important
This PR can undercount in-range Codex usage when a session file's mtime is older than the one-day buffer.
Reviewed changes
- Codex file pruning — Applies a
--since-based mtime prefilter before replay planning and JSONL parsing, with a one-day widening buffer. - Grouped loading and detection — Routes Codex loading through grouped aggregation for all date-range cases and preserves agent detection when filtering yields no rows.
- Regression coverage — Adds tests for pruning old files and detecting Codex when all event dates are outside the requested range.
@v0 or keep the SHA fresh with Dependabot | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using GPT Luna (free via Pullfrog for OSS) | 𝕏
| } | ||
| // ponytail: mtime is a one-day-widened proxy; inspect file tails if preserved mtimes matter. | ||
| let cutoff = start.saturating_sub(MILLIS_PER_DAY); | ||
| files.retain(|file| { |
There was a problem hiding this comment.
This makes the file's mtime a hard exclusion, but the authoritative date decision is based on each JSONL event timestamp. A restored or copied session, or a long-lived or fork parent whose mtime predates the one-day buffer, can therefore contain usage on or after --since that never reaches codex_period_for, silently undercounting the report.
Technical details
# Mtime can exclude reportable events
## Affected sites
- `rust/adapters/codex/src/aggregate.rs:148-155` — drops the complete file using only `modified >= start - MILLIS_PER_DAY`.
- `rust/adapters/codex/src/aggregate.rs:228-232` — only retained files are parsed and passed through the exact event-date check.
## Required outcome
- The prefilter must not discard a file that may contain an event inside the requested date range, including a parent file needed for fork replay.
## Suggested approach
- Add a regression fixture with an in-range event whose mtime is older than the cutoff, and either inspect enough file content to establish a safe bound or widen/disable the optimization for files whose mtime cannot safely represent their event range.There was a problem hiding this comment.
4 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="rust/adapters/codex/src/aggregate.rs">
<violation number="1" location="rust/adapters/codex/src/aggregate.rs:147">
P1: When a Codex file has a preserved mtime more than one day before `--since`, this prefilter drops the entire file and loses in-window usage. Do not treat the one-day mtime window as conservative for preserved mtimes; fall back to inspecting the file (or otherwise retain it) when its mtime cannot prove that no requested events exist.</violation>
</file>
<file name="rust/crates/ccusage-adapter-all/src/loader.rs">
<violation number="1" location="rust/crates/ccusage-adapter-all/src/loader.rs:631">
P2: When the selected window produces no groups, this recursively scans every Codex source a second time because `load_groups` already collected those files. Return detection from the load pass or reuse its source-presence result to avoid doubling filesystem traversal for out-of-range queries.</violation>
</file>
<file name="rust/crates/ccusage-adapter-all/src/tests.rs">
<violation number="1" location="rust/crates/ccusage-adapter-all/src/tests.rs:545">
P3: This test for the PR's headline behavior never exercises the new mtime pruning path. `retain_report_files` (rust/adapters/codex/src/aggregate.rs) returns early when `start > now`, and since the test passes a future `since` of `20990102`, pruning is skipped entirely. The test can only prove detection survives event-level date filtering; if the new mtime pruning wrongly dropped Codex detection, this test would still pass. Choose a `since` in the past (with the fixture file mtime fresh) so the pruning branch runs, or add a companion case that asserts detection after files are actually pruned.</violation>
</file>
<file name="rust/adapters/codex/src/lib.rs">
<violation number="1" location="rust/adapters/codex/src/lib.rs:33">
P3: In this perf-focused PR, `has_data()` calls `paths::collect_codex_usage_files(&source.dir)`, which walks the entire session tree, collects every `.jsonl` path, and sorts them — all just to test emptiness. It only short-circuits across sources (`.any`), not once a file is found within a source. That contradicts the AGENTS.md rule that "Detection short-circuits as soon as one usable source file is found." For a large `~/.codex` the detection path re-walks the whole directory. Add an early-exit check (e.g. read_dir and return true on the first file/dir) instead of collecting the full sorted list.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| return; | ||
| } | ||
| // ponytail: mtime is a one-day-widened proxy; inspect file tails if preserved mtimes matter. | ||
| let cutoff = start.saturating_sub(MILLIS_PER_DAY); |
There was a problem hiding this comment.
P1: When a Codex file has a preserved mtime more than one day before --since, this prefilter drops the entire file and loses in-window usage. Do not treat the one-day mtime window as conservative for preserved mtimes; fall back to inspecting the file (or otherwise retain it) when its mtime cannot prove that no requested events exist.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At rust/adapters/codex/src/aggregate.rs, line 147:
<comment>When a Codex file has a preserved mtime more than one day before `--since`, this prefilter drops the entire file and loses in-window usage. Do not treat the one-day mtime window as conservative for preserved mtimes; fall back to inspecting the file (or otherwise retain it) when its mtime cannot prove that no requested events exist.</comment>
<file context>
@@ -125,6 +130,31 @@ pub(super) fn load_groups_from_directory(
+ return;
+ }
+ // ponytail: mtime is a one-day-widened proxy; inspect file tails if preserved mtimes matter.
+ let cutoff = start.saturating_sub(MILLIS_PER_DAY);
+ files.retain(|file| {
+ file.metadata()
</file context>
| codex::filter_events_by_date(&mut events, shared)?; | ||
| let groups = codex::aggregate_events(&events, kind, shared.timezone.as_deref())?; | ||
| let groups = codex::load_groups(shared, kind)?; | ||
| let detected = !groups.is_empty() || codex::has_data(); |
There was a problem hiding this comment.
P2: When the selected window produces no groups, this recursively scans every Codex source a second time because load_groups already collected those files. Return detection from the load pass or reuse its source-presence result to avoid doubling filesystem traversal for out-of-range queries.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At rust/crates/ccusage-adapter-all/src/loader.rs, line 631:
<comment>When the selected window produces no groups, this recursively scans every Codex source a second time because `load_groups` already collected those files. Return detection from the load pass or reuse its source-presence result to avoid doubling filesystem traversal for out-of-range queries.</comment>
<file context>
@@ -627,23 +627,8 @@ fn load_codex_rows(
- codex::filter_events_by_date(&mut events, shared)?;
- let groups = codex::aggregate_events(&events, kind, shared.timezone.as_deref())?;
+ let groups = codex::load_groups(shared, kind)?;
+ let detected = !groups.is_empty() || codex::has_data();
let speed = codex::resolve_codex_speed(CodexSpeed::Auto);
Ok(AgentRows {
</file context>
|
|
||
| let result = loader::load_rows( | ||
| AgentReportKind::Daily, | ||
| &fixture_shared("20990102", "20990102"), |
There was a problem hiding this comment.
P3: This test for the PR's headline behavior never exercises the new mtime pruning path. retain_report_files (rust/adapters/codex/src/aggregate.rs) returns early when start > now, and since the test passes a future since of 20990102, pruning is skipped entirely. The test can only prove detection survives event-level date filtering; if the new mtime pruning wrongly dropped Codex detection, this test would still pass. Choose a since in the past (with the fixture file mtime fresh) so the pruning branch runs, or add a companion case that asserts detection after files are actually pruned.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At rust/crates/ccusage-adapter-all/src/tests.rs, line 545:
<comment>This test for the PR's headline behavior never exercises the new mtime pruning path. `retain_report_files` (rust/adapters/codex/src/aggregate.rs) returns early when `start > now`, and since the test passes a future `since` of `20990102`, pruning is skipped entirely. The test can only prove detection survives event-level date filtering; if the new mtime pruning wrongly dropped Codex detection, this test would still pass. Choose a `since` in the past (with the fixture file mtime fresh) so the pruning branch runs, or add a companion case that asserts detection after files are actually pruned.</comment>
<file context>
@@ -525,6 +525,31 @@ fn multi_section_codex_fixture_matches_standalone_sections_for_daily_and_session
+
+ let result = loader::load_rows(
+ AgentReportKind::Daily,
+ &fixture_shared("20990102", "20990102"),
+ )
+ .unwrap();
</file context>
| paths::codex_usage_sources().is_ok_and(|sources| { | ||
| sources | ||
| .iter() | ||
| .any(|source| !paths::collect_codex_usage_files(&source.dir).is_empty()) |
There was a problem hiding this comment.
P3: In this perf-focused PR, has_data() calls paths::collect_codex_usage_files(&source.dir), which walks the entire session tree, collects every .jsonl path, and sorts them — all just to test emptiness. It only short-circuits across sources (.any), not once a file is found within a source. That contradicts the AGENTS.md rule that "Detection short-circuits as soon as one usable source file is found." For a large ~/.codex the detection path re-walks the whole directory. Add an early-exit check (e.g. read_dir and return true on the first file/dir) instead of collecting the full sorted list.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At rust/adapters/codex/src/lib.rs, line 33:
<comment>In this perf-focused PR, `has_data()` calls `paths::collect_codex_usage_files(&source.dir)`, which walks the entire session tree, collects every `.jsonl` path, and sorts them — all just to test emptiness. It only short-circuits across sources (`.any`), not once a file is found within a source. That contradicts the AGENTS.md rule that "Detection short-circuits as soon as one usable source file is found." For a large `~/.codex` the detection path re-walks the whole directory. Add an early-exit check (e.g. read_dir and return true on the first file/dir) instead of collecting the full sorted list.</comment>
<file context>
@@ -26,6 +26,14 @@ pub use types::{
+ paths::codex_usage_sources().is_ok_and(|sources| {
+ sources
+ .iter()
+ .any(|source| !paths::collect_codex_usage_files(&source.dir).is_empty())
+ })
+}
</file context>

Summary:
--sincemtime boundary before replay planning and JSONL parsingTesting:
cargo test -p ccusage-adapter-codex(77 passed)cargo test -p ccusage-adapter-all(37 passed)cargo fmt --all --checkcargo build --release -p ccusagecodex daily --last 1 --offline --no-cost --json: 166-191 ms warmCloses #1598
Summary by cubic
Prefilters Codex session files by a conservative mtime cutoff when a since date is provided, reducing replay planning and JSONL parsing. Keeps event-level date filtering authoritative and ensures Codex is still detected when the selected window has no rows (addresses Linear #1598).
ccusage-adapter-allto always load grouped Codex data; detection is now!groups.is_empty() || codex::has_data().codex::has_data()to detect presence of any Codex files and tests covering file pruning and detection when the date filter excludes all rows.Written for commit 8a4ce8e. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests