Skip to content

perf(codex): prune historical files for date ranges - #1599

Closed
zhangxaochen wants to merge 1 commit into
ccusage:mainfrom
zhangxaochen:codex/perf-codex-date-filter
Closed

perf(codex): prune historical files for date ranges#1599
zhangxaochen wants to merge 1 commit into
ccusage:mainfrom
zhangxaochen:codex/perf-codex-date-filter

Conversation

@zhangxaochen

@zhangxaochen zhangxaochen commented Aug 13, 2026

Copy link
Copy Markdown

Summary:

  • prefilter Codex session files by a conservative --since mtime boundary before replay planning and JSONL parsing
  • keep exact event-level date filtering authoritative
  • preserve Codex detection when the selected window contains no Codex rows

Testing:

  • cargo test -p ccusage-adapter-codex (77 passed)
  • cargo test -p ccusage-adapter-all (37 passed)
  • cargo fmt --all --check
  • release build: cargo build --release -p ccusage
  • local codex daily --last 1 --offline --no-cost --json: 166-191 ms warm

Closes #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).

  • Retains only files with mtime >= (since − 1 day), respecting timezone; skips prefilter if since is missing or after now.
  • Unifies ccusage-adapter-all to always load grouped Codex data; detection is now !groups.is_empty() || codex::has_data().
  • Adds 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.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Improved Codex usage loading by excluding files outside the requested date range.
    • Codex remains detected even when no usage records match the selected dates.
    • Added safeguards for invalid, future, or unavailable date information.
  • Tests

    • Added coverage for filtering outdated usage files.
    • Added regression coverage for Codex detection when filtered results are empty.

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
@zhangxaochen

Copy link
Copy Markdown
Author

@coderabbitai review this PR, please. @cubic-dev-ai review this PR if available.

@github-actions

Copy link
Copy Markdown
Contributor

This PR was auto-closed. Only contributors approved with lgtm can open PRs. Open an issue first.

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 lgtmi, your future issues will stay open. If a maintainer replies lgtm, your future issues and PRs will stay open.

See CONTRIBUTING.md.

@github-actions github-actions Bot closed this Aug 13, 2026
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 36f956d9-69fb-4d7a-939e-6fbf3f5a905d

📥 Commits

Reviewing files that changed from the base of the PR and between ed1929b and 8a4ce8e.

📒 Files selected for processing (4)
  • rust/adapters/codex/src/aggregate.rs
  • rust/adapters/codex/src/lib.rs
  • rust/crates/ccusage-adapter-all/src/loader.rs
  • rust/crates/ccusage-adapter-all/src/tests.rs

📝 Walkthrough

Walkthrough

Codex aggregation now filters historical usage files before replay planning. Unified loading uses codex::load_groups and preserves Codex detection when date filtering produces no rows. Tests cover file retention and empty-range detection.

Changes

Codex date-bounded loading

Layer / File(s) Summary
Codex file prefilter
rust/adapters/codex/src/aggregate.rs
Report files are filtered by modification time before replay planning for multi-source and single-directory loading. Invalid or future start dates, and unavailable current time, skip filtering. Tests verify recent and outdated file handling.
Unified loading and source detection
rust/adapters/codex/src/lib.rs, rust/crates/ccusage-adapter-all/src/loader.rs, rust/crates/ccusage-adapter-all/src/tests.rs
Unified loading calls codex::load_groups. has_data() reports whether Codex sources contain files, so Codex remains detected when no rows match the date range. The loader test verifies this behavior.

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
Loading

Possibly related PRs

Suggested reviewers: ryoppippi

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@cubic-dev-ai

cubic-dev-ai Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai review this PR, please. @cubic-dev-ai review this PR if available.

@zhangxaochen I have started the AI code review. It will take a few minutes to complete.

@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

@zhangxaochen I will review pull request #1599.

⚠️ Action not completed

Pull request is closed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Pullfrog  | ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @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| {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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"),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Codex date-bounded reports scan all historical session files

1 participant