fix(opencode): honor --since/--until in the SQLite loader and JSON scan - #1492
Conversation
Push date bounds into the OpenCode loader instead of reading every DB row and JSON file on each invocation, which hangs on large installs. - SQLite: indexed `WHERE time_created` push-down with -1d/+2d slack, falling back to an unfiltered scan when the schema lacks the column (the in-loop date check still applies, so no row is silently dropped). - JSON files: extract `time.created` from the raw payload and apply the same `format_date_tz` check as the authoritative `filter_loaded_entries_by_date`, failing open to a full parse so no in-range row is ever dropped. Combines the indexed SQL push-down from #1188 with the content-based JSON extraction from #1220, per maintainer guidance. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The first pass hand-parsed the YYYYMMDD bounds with byte slicing and then widened them by a day of slack. Two problems came out of that: - Byte slicing panicked on any multi-byte bound. `ccusage opencode --since "abあcde"` aborted with "byte index 4 is not a char boundary", and through the aggregate command it became `agent loader panicked`, killing the whole report. `--since`/`--until` are unvalidated free-form strings, so this was reachable from the CLI and from config files. - The slack needed a paragraph of comment to argue that the pre-filter stayed wider than the report's own filter. Resolve both bounds to half-open millisecond instants in the reporting timezone instead, via a shared `date_range_bounds_ms` helper: - `parse_compact_date` validates bounds byte-wise, so a bound that is not eight ASCII digits leaves that side of the window open and the report's string filter decides, exactly as it did before this branch. - The SQL push-down and the pre-parse row check use those instants directly, so there is no slack to justify, and the per-row check is an integer comparison rather than a formatted string plus an allocation. - `date_within_range` now backs the loader pre-filter, the loaded-entry filter and the summary filter, so the window rule has one definition instead of three copies. Disable the push-down when `message.time_created` is not millisecond scale. The column repeats the payload's `time.created` on every OpenCode build checked (max delta 0 over 600 local rows), but a second-scale column compared against millisecond bounds would silently exclude every row instead of merely scanning more than needed. Drop the "indexed" claim from the comments: the only index is (session_id, time_created, id), so a bare time_created range cannot seek it and SQLite reports SCAN message. The push-down still pays off by not materializing and parsing `data` for out-of-range rows. Tests: the SQLite helper now mirrors the real schema, with `time_created` repeating the payload timestamp, so the push-down is what most database tests exercise; a separate legacy-schema helper covers the unfiltered fallback. New coverage pins the timezone-local bounds at UTC+14 and UTC-12, a spring-forward day being 23 hours long, the second-scale fallback, multi-byte bounds, and `extract_message_timestamp` directly.
`WHERE time_created >= ?` cannot seek OpenCode's only index, (session_id, time_created, id), because its leading column is session_id. SQLite therefore scanned the table and pulled every `data` blob through the page cache just to evaluate the bound. Moving the bound into a subquery that selects `id` alone lets the index answer it as a covering scan, leaving only in-range rows to be fetched by primary key. Measured on a synthetic database with the real schema and index, 200k rows and 0.83 GB spanning 400 days, querying a 7-day window: - database only: 949.6 ms -> 32.1 ms (29.6x) - with a 20k-file legacy JSON dump alongside: 1.584 s -> 300 ms (5.3x), the remainder being file reads the loader still performs - no window at all: 1.422 s -> 1.236 s, so the unfiltered path is not penalized Token totals are identical between both builds for every variant, and the 67-comparison real-data parity sweep still reports no differences.
The aggregate report builds its "Detected:" list from whether each adapter returned any entries before date filtering. Now that the OpenCode loader narrows to the window as it reads, a query matching nothing made an installed OpenCode vanish from that list, unlike every other adapter. Ask the source directly instead, the way the qwen adapter already does: `has_source` looks for the SQLite database, then for the first message file, and stops there — so detection stays cheap next to a large legacy dump. Reported by a codex review of this branch.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
ccusage-guide | 29d792b | Commit Preview URL Branch Preview URL |
Jul 27 2026, 11:36 AM |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughShared date-window utilities now drive summary and loaded-entry filtering. OpenCode applies timezone-aware filtering during SQLite and JSON loading, adds timestamp extraction and SQL pushdown, and preserves source detection when all data falls outside the requested window. ChangesOpenCode date-window handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant load_entries_from_directory
participant DateWindow
participant SQLite_loader
participant JSON_message_loader
load_entries_from_directory->>DateWindow: derive bounds from SharedArgs and timezone
DateWindow->>SQLite_loader: pass date window
DateWindow->>JSON_message_loader: pass date window
SQLite_loader->>SQLite_loader: push down query and check payload timestamps
JSON_message_loader->>JSON_message_loader: extract timestamps before JSON parsing
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — pushes --since/--until into the OpenCode SQLite and JSON loader so the date window is applied during load rather than after, with a shared window rule, timezone-resolved bounds, index-aware SQL, and fail-open fallbacks.
- Push date window into SQLite via covering-index subquery —
WHERE id IN (SELECT id FROM message WHERE time_created >= ?1 AND time_created < ?2)answered from(session_id, time_created, id)without readingdatablobs; falls back to unfiltered scan when schema lackstime_createdor the column isn't millisecond-scale. - Skip out-of-range JSON files before full parse —
extract_message_timestampscans raw text fortime.createdmillis; fails open on non-UTF-8, missing key, or quoted/negative numbers so the full parse decides. - Unify the date window rule —
date_within_rangereplaces three separate copies insummary.rs,claude/mod.rs, and the old opencode inline logic. - Keep OpenCode in the aggregate report's detected agents —
opencode::has_data()checks source presence independently of the date window so out-of-range queries don't drop OpenCode fromDetected:. - Prevent panic on multi-byte bounds —
parse_compact_dateworks byte-wise onvalue.as_bytes()so--since "abあcde"no longer panics with an index-boundary error.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@rust/crates/ccusage/src/adapter/opencode/loader.rs`:
- Around line 136-149: Update has_json_file to inspect each directory entry with
entry.file_type() rather than path.is_dir(), treating only actual directories as
recursive candidates and skipping symlinks. Preserve the existing JSON-extension
check for non-directories and the current read-directory failure behavior.
- Around line 310-323: Update extract_message_timestamp to match only the
canonical `"time":{"created":...}` key shape at the top-level JSON structure,
rather than any occurrence inside payload text. Limit the created-value scan to
the enclosing time object and require its valid closing boundary, returning None
for malformed or unterminated objects so incorrect timestamps never cause
entries to be skipped.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: be2cf0bc-73fa-402b-b227-7a1e5906bf04
📒 Files selected for processing (6)
rust/crates/ccusage/src/adapter/all/loader.rsrust/crates/ccusage/src/adapter/claude/mod.rsrust/crates/ccusage/src/adapter/opencode/loader.rsrust/crates/ccusage/src/adapter/opencode/mod.rsrust/crates/ccusage/src/date_utils.rsrust/crates/ccusage/src/summary.rs
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 6 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/crates/ccusage/src/adapter/opencode/loader.rs">
<violation number="1" location="rust/crates/ccusage/src/adapter/opencode/loader.rs:410">
P1: Mixed-scale historical rows can still be dropped because this arbitrary eight-row sample treats the whole column as milliseconds. Make the pushdown fail open for any unverified scale values, or remove the exclusionary predicate when scale consistency cannot be established.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| /// the push-down and leaves the payload check to filter. | ||
| fn time_created_is_millis(connection: &sqlite::Connection) -> bool { | ||
| let Ok(mut statement) = connection | ||
| .prepare("SELECT max(time_created) FROM (SELECT time_created FROM message LIMIT 8)") |
There was a problem hiding this comment.
P1: Mixed-scale historical rows can still be dropped because this arbitrary eight-row sample treats the whole column as milliseconds. Make the pushdown fail open for any unverified scale values, or remove the exclusionary predicate when scale consistency cannot be established.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At rust/crates/ccusage/src/adapter/opencode/loader.rs, line 410:
<comment>Mixed-scale historical rows can still be dropped because this arbitrary eight-row sample treats the whole column as milliseconds. Make the pushdown fail open for any unverified scale values, or remove the exclusionary predicate when scale consistency cannot be established.</comment>
<file context>
@@ -221,6 +307,118 @@ fn entry_id(entry: &LoadedEntry) -> Option<&str> {
+/// the push-down and leaves the payload check to filter.
+fn time_created_is_millis(connection: &sqlite::Connection) -> bool {
+ let Ok(mut statement) = connection
+ .prepare("SELECT max(time_created) FROM (SELECT time_created FROM message LIMIT 8)")
+ else {
+ return false;
</file context>
There was a problem hiding this comment.
Fully detecting mixed-scale columns would need a full-column min/scan, which is exactly the cost the LIMIT-8 sample avoids on tens-of-GB databases; the code already fails open for uniform non-millisecond columns and the authoritative payload recheck bounds any residual risk, so this is a conscious sampling trade-off rather than a minimal-fix bug.
There was a problem hiding this comment.
Accurate, and left in place deliberately — recording the reasoning here so it is next to the code rather than only in the PR thread.
Detecting mixed scales reliably needs a full-column scan, which is the work this push-down exists to avoid: on the 33 GB install from the original report, that scan is the bug. The LIMIT 8 sample is answered from the covering index and never touches data, which is what makes it affordable.
What bounds the risk:
- a uniformly non-millisecond column disables the push-down entirely and falls back to an unfiltered scan
- 7ee95ee widens the pushed-down bounds by a day, so sub-day column/payload drift can no longer cause a SQL-level exclusion
- the payload's
time.createdremains authoritative for every row that is returned
What stays exposed: genuinely mixed scales within one column, or drift beyond a day. Neither appears on any OpenCode build I can inspect — the column and payload are written in the same statement, and a local install shows a maximum delta of 0 across 600 rows.
2e1af2d renamed the helper to time_created_looks_like_millis and rewrote its doc to say the sample proves nothing about rows it did not read, so the limitation you flagged is now stated where a reader will find it.
There was a problem hiding this comment.
The parent comment was too broad for this PR. The sampling heuristic is intentional here: it avoids the full-column scan that would be the real bug on large installs, and the code now states its limits next to the helper. The payload check still remains authoritative, so the windowing change only affects how much data gets scanned, not which entries survive.
ccusage
@ccusage/ccusage-darwin-arm64
@ccusage/ccusage-darwin-x64
@ccusage/ccusage-linux-arm64
@ccusage/ccusage-linux-x64
@ccusage/ccusage-win32-x64
commit: |
ccusage performance comparisonPR SHA: This compares the Rust PR release binary against the configured base package on the same CI runner. Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the PR package against the configured base package on the same CI runner. Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
…object Two findings from a CodeRabbit review of this branch. `extract_message_timestamp` searched for the first `"time"` anywhere in the payload and then for `"created":` without a bound, so a `time` object belonging to something else could contribute the number. A failed scan is harmless — the caller falls back to a full parse — but a wrong one silently drops an in-range entry. Require the canonical `"time":` key, require the object to open right after it, and keep the `created` search inside that object. Checked against 1037 real records (600 database rows, 437 message files): still agrees with a full parse on every one, with no fallbacks. `has_json_file` judged entries with `path.is_dir()`, which follows symlinks, while the collection pass it feeds uses `file_type()` and skips them. Detection could therefore claim files that would never be read, and a symlinked cycle would recurse until the stack gave out. Match `collect_files_with_extension` instead.
|
@coderabbitai thanks — both findings were valid and are fixed in d72e237. Raw-text scan latching onto an unrelated Agreed, and the asymmetry you point out is the important part: a scan that gives up is free (the caller falls back to a full parse), but a scan that guesses wrong silently drops an in-range entry. Applied your tightening: require the I re-validated against real data rather than just the unit tests — 1037 records from a local install (600 Also added the two cases that matter for the false-positive class:
And you were right about the test expectation: the unterminated
Fixed. Worth noting it was not only a robustness issue: the collection pass this detection feeds ( Verification for this push
|
|
Tip For best results, initiate chat on the files or code changes.
🐇✅ If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — tightens has_json_file to skip symlinks via file_type() and scopes extract_message_timestamp to the message's own time object so nested time objects elsewhere in the payload cannot contribute a false match.
- Skip symlinks in
has_json_file— inspectsentry.file_type()instead ofpath.is_dir(), matchingcollect_files_with_extensionbehavior and preventing infinite recursion on symlinked cycles. - Scope
extract_message_timestampto the message's owntimeobject — parses the{ ... }body immediately after"time":so atimeobject nested insideparts[].timecannot leak into the scan. Fails open to a full parse when the object boundaries don't line up.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
ccusage performance comparisonPR SHA: This compares the Rust PR release binary against the configured base package on the same CI runner. Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the PR package against the configured base package on the same CI runner. Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
A cubic review pointed out that the push-down filters on `message.time_created` while the report filters on the payload's `time.created`, and the scale check only proves those share a unit, not a value. A column drifting from its payload by a few hours would therefore exclude a row near a window edge — silently, since SQL never hands it to the payload check. Widen only the pushed-down bounds by a day on each side. The exact window still runs per row against the payload, so the margin costs a few extra rows to scan and cannot change which entries survive. The new test fails without it. The remaining exposure is a column drifting by more than a day, or a column with mixed scales that the sampled rows do not reveal. Both would need a full-column scan to detect, which is the cost this push-down exists to avoid; the local install shows a maximum delta of 0 across 600 rows.
|
Addressed the review findings on the push-down in 7ee95ee. Summary of where each one landed: Column/payload drift (cubic P1,
|
There was a problem hiding this comment.
All reported issues were addressed across 1 file (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.
✅ No new issues found.
Reviewed changes — widens the SQL push-down window by a day on each side so a message.time_created column that drifts from the payload's time.created costs a few extra rows to scan rather than silently excluding a row the report wanted. The per-row payload check remains the authoritative gate.
- Widen the SQL push-down window with a day of margin —
DateWindow::widened_for_pushdown()addsMILLIS_PER_DAYon each bound before passing the window toprepare_message_query. The exact window is still applied per-row against the payload text in the loop. - Test that payload wins over a drifting column —
pushdown_margin_keeps_rows_whose_column_drifts_from_the_payloadplaces the column 26 hours after the payload date and verifies the row still surfaces with the payload's date.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
ccusage performance comparisonPR SHA: This compares the PR package against the configured base package on the same CI runner. Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the Rust PR release binary against the configured base package on the same CI runner. Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
|
@coderabbitai review The push-down changed since your last pass: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target="$(fd -a -t f '^loader\.rs$' rust | rg '/adapter/opencode/loader\.rs$' | head -n1)"
date_utils="$(fd -a -t f '^date_utils\.rs$' rust | head -n1)"
printf '%s\n' '=== Commit and changed files ==='
git show --stat --oneline 7ee95eee -- 2>/dev/null || true
printf '%s\n' '=== OpenCode loader outline ==='
ast-grep outline "$target" --items all --view expanded 2>/dev/null || true
printf '%s\n' '=== Relevant loader symbols and contexts ==='
rg -n -C 10 \
'DateWindow|widened_for_pushdown|time_created|scale|pushdown|date_range_bounds_ms|load_entries_from_database|payload|created' \
"$target" || true
printf '%s\n' '=== Relevant tests ==='
rg -n -C 8 \
'pushdown_margin_keeps_rows_whose_column_drifts_from_the_payload|scale|widened_for_pushdown|pushdown' \
"$target" || true
printf '%s\n' '=== Date bounds implementation ==='
ast-grep outline "$date_utils" --items all --view expanded 2>/dev/null || true
rg -n -C 8 'date_range_bounds_ms|start_of_day_ms|checked_|saturat' "$date_utils" || trueLength of output: 60036 🏁 Script executed: #!/bin/bash
set -euo pipefail
target="$(fd -a -t f '^loader\.rs$' rust | rg '/adapter/opencode/loader\.rs$' | head -n1)"
parser="$(fd -a -t f '^parser\.rs$' rust | rg '/adapter/opencode/parser\.rs$' | head -n1)"
printf '%s\n' '=== Database loop (original versus pushed-down window use) ==='
sed -n '187,278p' "$target"
printf '%s\n' '=== Commit diff ==='
git show --format=fuller --find-renames 7ee95eee -- "$target"
printf '%s\n' '=== Payload parser timestamp/date construction ==='
ast-grep outline "$parser" --items all --view expanded 2>/dev/null || true
rg -n -C 8 'message_value_to_entry|time\.created|created|format_date|TimestampMs' "$parser" || true
printf '%s\n' '=== All scale-sampler references and explanatory wording ==='
rg -n -C 5 'time_created_is_millis|MIN_MILLIS_SCALE|same millisecond scale|Sampling a few|LIMIT 8' rustLength of output: 22758
The widened window is correctly confined to SQL selection. The remaining limitations are real but intentionally accepted:
The commit message states both limitations candidly. I would only soften the source comments to match that reality—for example, Lines 204–205 should say the sample appears millisecond-scaled rather than “is known to hold the same millisecond scale as the payload,” and Lines 375–377 should qualify the drift protection as applying within the one-day margin. No blocking correctness finding on the margin behavior. |
Follow-up to a CodeRabbit review of 7ee95ee. The comments claimed more than the code establishes: sampling eight values cannot show that the whole column is millisecond-scaled, and matching scales say nothing about matching values. Rename `time_created_is_millis` to `time_created_looks_like_millis` and state the limits in place of the guarantee, including that the margin absorbs drift only up to a day. No behavior change.
|
@coderabbitai agreed on both wordings — fixed in 2e1af2d, comments only.
Thanks for pushing on this — the code was already honest about the limits in the commit message, but the comments a future reader lands on were not. |
|
Tip For best results, initiate chat on the files or code changes.
🐇✅ ✏️ Learnings added
|
ccusage performance comparisonPR SHA: This compares the Rust PR release binary against the configured base package on the same CI runner. Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the PR package against the configured base package on the same CI runner. Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
…til-adapter Resolves the workspace split from #1428, which moved the files this branch touches into separate crates: - `date_utils` now lives in `ccusage-core`. The window helpers added here (`date_within_range`, `date_range_bounds_ms`) are `pub` rather than `pub(crate)`, because the opencode adapter that calls them is its own crate now; `parse_compact_date` and `start_of_day_ms` stay private. `parse_digits` keeps main's private, overflow-checked version. - `filter_loaded_entries_by_date` moved from the claude adapter to `ccusage-adapter-common`. Main's copy still inlined the date predicate, so it now calls the shared `date_within_range` instead — the point of collapsing the three copies in the first place. - `opencode::has_data` follows the adapter crate's new `pub` surface.
ccusage performance comparisonPR SHA: This compares the Rust PR release binary against the configured base package on the same CI runner. Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
ccusage performance comparisonPR SHA: This compares the PR package against the configured base package on the same CI runner. Package runtime diagnosticsCompares the PR package wrapper, the installed native optional dependency binary, and the workspace release binary on the same large fixture. This identifies whether slow package results come from JavaScript wrapper overhead, the published native binary build, or the Rust core itself. Fixtures: Claude
Committed fixture performanceCommitted small fixtures for stable PR-to-PR feedback and explicit Claude/Codex command coverage. Fixtures: Claude
Large real-world-shaped fixture performanceGenerated fixtures shaped from aggregate local log statistics: thousands of JSONL files, many small sessions, and a long tail of larger sessions. No real prompts, paths, or outputs are stored in the fixtures. Fixtures: Claude
Artifact size
Lower medians and smaller artifacts are better. CI runner noise still applies; use same-run ratios as directional PR feedback, not release guarantees. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rust/crates/ccusage-adapter-all/src/loader.rs (1)
134-147: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a regression test for the OpenCode
detectedfallback.This closure's
rows.detected = rows.detected || opencode::has_data()fix addresses a real gap (OpenCode's loader pre-filters entries by the date window, so "non-empty entries" no longer proves source existence). None of the tests in this file's suite exercise this specific aggregate-wiring path (e.g., DB/JSON data exists entirely outside--since/--untilbutopencodeshould still land indetected_agents). Givenopencode::load_entriesis the only loader with this window-narrowing behavior, a small end-to-end test here would guard against regressions ifload_summary_agent_rows's detection heuristic or the fallback ordering changes.🤖 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 `@rust/crates/ccusage-adapter-all/src/loader.rs` around lines 134 - 147, Add a regression test in the loader test suite covering the aggregate path around the OpenCode loader closure. Seed OpenCode data entirely outside the requested since/until window, run the report-loading flow, and assert OpenCode remains in detected_agents despite having no in-range entries. Exercise the rows.detected fallback through opencode::has_data(), without changing the existing production logic.
🤖 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.
Nitpick comments:
In `@rust/crates/ccusage-adapter-all/src/loader.rs`:
- Around line 134-147: Add a regression test in the loader test suite covering
the aggregate path around the OpenCode loader closure. Seed OpenCode data
entirely outside the requested since/until window, run the report-loading flow,
and assert OpenCode remains in detected_agents despite having no in-range
entries. Exercise the rows.detected fallback through opencode::has_data(),
without changing the existing production logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d9dd8179-5dc1-4fb1-a7db-dc7abdb61e94
📒 Files selected for processing (6)
rust/adapters/common/src/lib.rsrust/adapters/opencode/src/lib.rsrust/adapters/opencode/src/loader.rsrust/crates/ccusage-adapter-all/src/loader.rsrust/crates/ccusage-core/src/date_utils.rsrust/crates/ccusage-core/src/summary.rs
Summary
ccusage opencode(and the aggregateccusage) read every row of the OpenCode SQLitemessagetable and re-parsed everystorage/message/**/*.jsonfile on each invocation, no matter what--since/--untilasked for.SharedArgs.since/SharedArgs.untilwere already plumbed into the adapter; the loader simply never looked at them, so the date window was only applied after everything had been loaded. On long-lived installs, especially those carrying the pre-SQLite JSON dump, the loader can run for minutes on a query that covers a few days.This PR narrows the work in the loader, without changing what a given window means.
Behavior
Bound semantics are unchanged: both ends stay inclusive, and the authoritative decision is still the compact
YYYYMMDDstring comparison every report already performed. The loader now skips work that comparison would have discarded anyway, so output is byte-identical tomain(see Verification).Implementation
rust/crates/ccusage/src/date_utils.rsdate_range_bounds_msresolves--since/--untilto half-open millisecond bounds[since 00:00, day-after-until 00:00)in the reporting timezone, so a window means the same instants the report means. DST days resolve throughjiff, so a spring-forward day is 23 hours long and a midnight that does not exist moves to the first instant that does.parse_compact_datevalidates a bound byte-wise. Anything that is not eight ASCII digits (partial bounds like202601, multi-byte input,20260231) yields no instant, which leaves that side of the window open and defers entirely to the string filter.date_within_rangeis now the single definition of the window rule, shared by the loader pre-filter,filter_loaded_entries_by_date, andfilter_and_sort_summaries. It previously existed as three separate copies.rust/crates/ccusage/src/adapter/opencode/loader.rsWHERE id IN (SELECT id FROM message WHERE time_created >= ?1 AND time_created < ?2), or an open-ended variant. Selectingidalone lets the existing(session_id, time_created, id)index answer the bound as a covering scan, so out-of-range rows never have theirdatablob read. A bareWHERE time_created >= ?1cannot seek that index — its leading column issession_id— and SQLite falls back to a table scan that pulls every payload through the page cache, which is what made the first revision of this branch far slower than it needed to be.time.createdscan of the raw text before the fullserde_jsonparse; out-of-range ones are dropped without being deserialized.time_createdmirrors the payload'stime.createdon every build checked here, but the column is only a proxy for the payload the report filters on, so a loose SQL window means a drifting column costs extra rows scanned rather than excluding a row the report wanted. The exact window still runs per row.Correctness safeguards
Every pre-filter fails open, so none of them can drop an entry the report would have kept:
time_createdcolumntime_createdis not millisecond-scale (e.g. seconds, zeroes)time.createdtimebefore its owntimeobject, finds nocreated, and the full parse decidestime_createddrifts from its payload by up to a dayOn the schema note:
message.time_createdrepeats the payload'stime.createdon the builds checked here (max delta 0 across 600 local rows), which is what makes the push-down sound. The scale check is there because a column holding seconds compared against millisecond bounds would silently exclude every row — an empty report is a much worse failure than a slow one.Performance
Synthetic fixture (
OPENCODE_DATA_DIR), release builds,hyperfine:opencode.db: 200,000 rows, 0.83 GB, current schema and indexstorage/message/: 20,000 JSON files (legacy dump)Measured against an
origin/main-equivalent build, with the day of SQL margin in place:The fixture fits in the page cache, so a multi-gigabyte install should widen the database-only gap rather than narrow it; that has not been measured here. The remaining ~360 ms in the second row is the legacy dump's file reads, which still happen — only the parse is skipped.
Token totals are identical between the two builds for the measured window.
Verification
cargo test --workspace: 495 tests pass (after merging currentmain).cargo clippy --workspace --all-targets -- -D warningsandcargo fmt --all -- --checkare clean.origin/main-equivalent build on a local install (600 DB rows, 437 JSON files):opencodedaily and session JSON compared across 12 window shapes — including a partial bound and a multi-byte one — plus every present date individually inPacific/Kiritimati(UTC+14),Etc/GMT+12,UTC,Asia/TokyoandAmerica/New_York: 67 comparisons, all byte-identical. The Claude-sidedaily,sessionandblocksJSON were compared as well, to cover the shareddate_within_rangerefactor: 6 comparisons, identical (sessionoutput is 1.6 MB).extract_message_timestampwas checked against 1037 real records (600 DB rows + 437 files): it agrees with a full JSON parse on every one, and never mis-extracts.ccusage opencode --since "abあcde"used to abort withbyte index 4 is not a char boundary(andagent loader panickedthrough the aggregate command). It now behaves likemainand reports no matching data.ccusage --since 20200101 --until 20200102(a window with no data) lists the sameDetected:agents as before the change, OpenCode included.Notes for reviewers
time.createdis used instead. Files are still read, but out-of-range ones are no longer parsed.LIMIT 8; it does not touch thedatacolumn. It proves the column shares a unit with the payload, not a value — sub-day drift is handled by the SQL margin instead. A column with genuinely mixed scales, or drift beyond a day, would need a full-column scan to detect, which is the cost this push-down exists to avoid; the local install shows a maximum delta of 0 across 600 rows.file_type(), the same ascollect_files_with_extension, so the two passes agree about symlinks: detection never claims a source whose files the collection pass would refuse to read.Relationship to #1188
This supersedes #1188 and keeps its first commit with the original authorship (@justi). That PR identified the gap and the push-down approach; the commits on top of it address review findings:
--since/--untilvalues, which also took down the aggregateccusagecommandYYYYMMDDarithmetic it needed, replaced by timezone-resolved instantsDetected:list whenever the window excluded every entry, since that list is derived from pre-filter entriesNo CLI surface, docs, or configuration schema changes.
History
The same user-visible gap was reported in #801, requested in #867, and previously fixed against the now-removed TypeScript
@ccusage/opencodepackage in #960 (closed with: "If a gap remains, it needs a fresh narrow PR against main").Summary by CodeRabbit
New Features
Bug Fixes