fix(codex): stream large session files - #1123

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.

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 @coderabbitai help to get the list of available commands and usage tips.}

@coderabbitai review
@cubic-dev-ai review

@coderabbitai review
@cubic-dev-ai review

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

#!/bin/bash
# Get PR details
gh pr view 1123 --repo ryoppippi/ccusage --json title,body,files,additions,deletions,changedFiles

# Get the diff
gh pr diff 1123 --repo ryoppippi/ccusage

#!/bin/bash
# Get the full file from the PR branch
gh api repos/ryoppippi/ccusage/contents/rust/crates/ccusage/src/codex_loader.rs \
  --jq '.content' | base64 -d

echo "---BRANCH INFO---"
gh pr view 1123 --repo ryoppippi/ccusage --json headRefName,headRepositoryOwner

@pretendhigh Great PR — the streaming refactor is clean and the approach is solid. Here's the review:

Review of rust/crates/ccusage/src/codex_loader.rs

✅ What's done well

• Clean separation: visit_codex_session_file stays as the public API, while the new private visit_codex_session_reader(impl BufRead, ...) holds the testable logic. This pattern makes unit testing straightforward with std::io::Cursor.
• Buffer reuse: Declaring let mut line = Vec::new() once outside the loop and calling line.clear() per iteration avoids repeated allocations — good for hot paths over large files.
• read_until + serde_json compatibility: read_until(b'\n', &mut line) includes the trailing \n in the buffer, but serde_json::from_slice tolerates trailing whitespace and memmem::find is unaffected, so this is correct.
• Parallel path automatically benefits: read_codex_session_file delegates to visit_codex_session_file unchanged, so the multi-threaded path in read_codex_session_files_parallel streams too — no extra work needed.
• Imports are consistent: byte_lines is correctly removed since read_codex_session_file itself no longer uses it directly.
• mut placement: Correctly moved from the visit parameter in visit_codex_session_file (which only passes it through) to visit_codex_session_reader (which actually calls it).

⚠️ Minor observation: unbounded line buffer on malformed input

read_until(b'\n', ...) will read until a \n or EOF. If a session file is malformed and contains no newlines at all (or has a single multi-GB JSON object on one line), the buffer will grow to hold the entire file contents — the same worst case as the original fs::read. For the known Codex JSONL format this is unlikely, but it's worth noting. A defensive cap like:

loop {
    line.clear();
    let bytes_read = reader.read_until(b'\n', &mut line)?;
    if bytes_read == 0 {
        break;
    }
    // Optional: skip pathologically large lines
    if line.len() > MAX_LINE_SIZE {
        continue;
    }
    // ...
}

…could protect against degenerate inputs, though it may be overkill given the real-world file format.

✅ Error handling

The ? on read_until propagates IO errors through visit_codex_session_reader → visit_codex_session_file → read_codex_session_file (which discards via let _ = ...). This is actually a slight improvement over the original behaviour, where any mid-read error was silently swallowed along with the rest of the file.

✅ Test

parses_codex_usage_from_streaming_reader is comprehensive — it covers model extraction via turn_context, token parsing via event_msg/token_count, session ID derivation, and all individual token fields. Using std::io::Cursor to exercise the reader abstraction is exactly the right approach.

Overall: The logic is correct, the abstraction is clean, and the test covers the key path well. The line-buffer growth observation is minor and the existing format makes it a non-issue in practice. LGTM. 🐇

Historical audit: this pull request was auto-closed by the legacy contributor gate. That closure did not assess technical importance.

Audit result: needs review. The current state does not prove resolution, but a fresh technical or product check is required before deciding whether the underlying request is still relevant. This closed PR will not be revived as-is; create a new PR only after reviewing the related issue and current main.