Skip to content

Share one transcript walk between Charts API-value and heatmap (SBS-909) - #328

Closed
tsouth89 wants to merge 1 commit into
mainfrom
fix/sbs-909-charts-reuse-scan
Closed

Share one transcript walk between Charts API-value and heatmap (SBS-909)#328
tsouth89 wants to merge 1 commit into
mainfrom
fix/sbs-909-charts-reuse-scan

Conversation

@tsouth89

Copy link
Copy Markdown
Owner

Summary

Opening Charts fired Estimated API value and When you work as two independent walks of the same local-log trees, in series. Both cards now share one pass per provider that fills reset windows and hourly buckets. A second reader inside five minutes reuses that walk. Hours older than the heatmap 30-day axis stay in the report for the API-value card and are dropped from the grid.

A user who opens Charts on a large corpus now pays for one machine-wide scan, not two.

Linear: https://linear.app/southboundsoftware/issue/SBS-909

Test plan

  • Open Charts on a large Codex/Claude/Grok corpus. Both cards should appear after one walk.
  • Switch away and back within five minutes: neither card should rescan.
  • Custom API-value range is a different window set and may walk again.
  • Confirm a 30-day heatmap cell is not darkened by older activity.

Quality gate

  • cargo fmt --all --check: pass
  • cargo test --manifest-path rust/Cargo.toml: 1004 passed, 6 failed. The 6 are unchanged Windows-path assertions on this Linux box (codex_sessions path separators, grok_costs project basenames). New SBS-909 tests passed. Windows CI is the required runner.
  • cargo clippy --all-targets -- -D warnings: fails here on two pre-existing unused Windows-only items (secure_file.rs:489, updater.rs:442). No findings in cost_scanner.rs or chart.rs.
  • Desktop crate: could not compile (glib-2.0 missing). Not a Linux harness as Windows CI.
  • Frontend CI not rerun; no frontend files changed.
  • Store-submission PowerShell script not run (Linux box).

Fail without the fix

Reverted only the cache hit in ChartsTabCorpusCache::reports. charts_tab_corpus tests failed with left: 6, right: 3 (one walk per provider per card instead of one walk per provider). Restored; tests pass.

Sweep leftovers

  • Spend budget and spend-anomaly still scan on their own (different windows). Spend-anomaly is still Codex+Claude only.
  • Provider Charts/Compare still do a 30-day scoped scan.
  • No provider parallelism, no per-file incremental cache, no first-card streaming.
  • Custom range evicts the single-slot default cache.

Gaps

  • Desktop test heatmap_hours_drop_days_outside_the_axis not executed here (no glib).
  • No large-corpus profile.
  • Frontend CI not rerun.

The Charts tab opened both cards with independent scans of the same
local-log trees. One cached pass now fills windows and hourly buckets
so a cold open does not pay twice.
@cloudflare-workers-and-pages

Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
ceiling 1a4f44a Commit Preview URL

Branch Preview URL
Aug 17 2026, 02:10 PM

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Caution

Review failed

An error occurred during the review process. Please try again later.


Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

Automated review

Found 1 issue:

  1. Two layered 5-minute caches can serve stale heatmap after corpus expires

    rust/src/cost_scanner.rs:1495 · disposition: advisory · confidence: medium · severity: low · quick win

    charts_tab_corpus_cache (5 min) and activity_heatmap_cache (5 min, in chart.rs) are independent. If the corpus cache expires but the heatmap cache is still valid, get_local_activity_heatmap() returns the old grid without rescanning, while a subsequent API-value call will rescan. The two TTLs can drift, making the heatmap appear fresher than the underlying data.

    Prompt for AI agents

    In rust/src/cost_scanner.rs around line 1495: Either derive heatmap TTL from corpus loaded_at or invalidate heatmap cache when corpus cache misses. Document the intended staleness window. Verify against the current code first; if no longer valid, skip with a brief reason. Keep the change minimal.

For coding agents: fix BLOCK and FIX IF QUICK findings now; everything else is tracked or informational; never exceed one CodeRev fix round per PR.

Advisory. Findings generated by grok-subscription and muse-spark-1.2-contributor, each filtered through a 3-vote refutation panel with the changed code in evidence.

@tsouth89

Copy link
Copy Markdown
Owner Author

Heads up: #329 covers the same ground from a different angle. Instead of sharing one walk between the two cards, it parses each transcript file once into a record index and resumes appended files from their last byte offset, so any number of callers can fold the same records however they need. It touches the same two files as this PR and will conflict. Worth deciding which route to keep before merging either.

tsouth89 added a commit that referenced this pull request Aug 18, 2026
Ported from #328, which found it: the weekday-by-hour view took whatever
hours the report carried, so an hour from a day outside the 30-day axis
would read as activity on a day the calendar strip never shows. The two
views are the same data asked two ways and must agree about which days
exist.
@tsouth89

Copy link
Copy Markdown
Owner Author

Ported the one thing this PR has that #329 does not: the heatmap grid now keeps only hours that fall on its own 30-day axis, with a test. Good catch, and it is a real fix either way.

The rest of the two PRs is the same job done differently, and they conflict in both files. My read is #329 supersedes this one: it parses each transcript once into a record index rather than sharing one walk between the two cards, so a second reader is 1.8s instead of 9s, a restart is not a cold start, and any number of callers can fold the same records. This one is smaller though, so it is your call which to keep.

@tsouth89

Copy link
Copy Markdown
Owner Author

Closing in favour of #329, which covers the same goal by parsing each transcript once into a record index rather than sharing one walk between the two cards. It keeps a second reader at ~1.8s instead of ~9s, survives a restart, and lets any caller fold the same records.

The one thing this PR had that #329 did not, hours outside the heatmap day axis leaking into the grid, is ported there with a test and credited to this PR. Reopen if you would rather take this route; nothing here is wasted.

@tsouth89 tsouth89 closed this Aug 18, 2026
tsouth89 added a commit that referenced this pull request Aug 18, 2026
## Summary

Opening Charts took about thirty seconds on a machine holding gigabytes
of local transcripts, and it took that long again on the next tab
switch. Three surfaces each walked the same Codex and Claude logs from
the top: Estimated API value over ninety days, the activity heatmap over
thirty, and provider charts on top of both. Nothing was kept between
opens, and clicking Yesterday or 30 days re-ran the whole scan for
numbers the card already held.

Each transcript file is now parsed once into a packed record index kept
beside the settings file. A Claude file that grew is resumed from the
byte offset the last read stopped at. Both cards keep their last result
on disk and paint it while a refresh runs behind, and the caches are
rebuilt in the background shortly after launch, so the first scan of the
day lands where nobody is waiting on it.

While measuring I found parsing had stopped being the bottleneck. A
record lands in the file summary, every caller window, its day and its
hour, and the per-record work that does not depend on the summary was
being redone each time: a pricing lookup that reads the clock and
normalizes the model name, plus three string allocations, roughly
fourteen times per record. That is now done once per record.

### Measured

On 1.7 GB of Codex logs and 1.8 GB of Claude logs, 1458 files:

| | before | after |
|---|---|---|
| Estimated API value | ~22s | **2.2s** |
| Activity heatmap | ~12s | **2.3s** |
| Claude 60-day scan | 17.1s (at 90d) | **1.8s** |
| Codex 30-day scan | 4.4s | **0.2s** |
| First scan ever, no index | | 11.3s + 2.9s |

The index is 17 MB for those 3.5 GB.

### What the index does not store

Aggregates. Reset windows land on arbitrary instants, so day or hour
buckets would round the numbers this app exists to report. It stores
records; every caller keeps its own fold, unchanged.

### Correctness

The stored dollars are computed at parse time, so the index is discarded
when the pricing catalog changes. Its bytes are hashed rather than its
mtime, which moves on a daily refetch that changed nothing. A file whose
first 4 KB changed is treated as a different file, a shrunken file is
re-read whole, a half-written trailing line is left for the next read, a
truncated index file is rejected rather than panicking, and an entry
built for a shallower window than the scan wants is a miss. Codex
rollouts carry cumulative counts and parser state across lines, so a
changed rollout is re-read whole rather than resumed.

## Test plan

Beyond the unit tests:

- Cold scan versus indexed scan on 3.5 GB of real logs: byte-identical
output, both providers.
- Built a 24 MB fixture from a real transcript with rewritten message
ids, so cross-file de-duplication could not mask a bad read. Indexed
half of it, appended the rest, then compared the resumed parse against a
full re-parse of the final file: identical, and the append really did
add $181 of usage.
- Open Charts, switch away and back, click through Today / Yesterday /
30 days: no rescan.
- A custom range is a different key and scans once, then caches.

Found and fixed one real bug this way: `9999-12-31 + 1 day` formats as
`+10000-01-01`, which sorts below every real date, so a wide parse range
excluded every Codex record. The suite caught it.

## Quality gate

- `cargo fmt --all --check`: pass
- `cargo clippy --manifest-path rust/Cargo.toml --all-targets -- -D
warnings`: pass
- `cargo clippy --manifest-path apps/desktop-tauri/src-tauri/Cargo.toml
--all-targets -- -D warnings`: pass
- `cargo test -p codexbar --lib`: 1037 passed, 1 failed. The failure is
`cli::tty_runner::tests::test_run_sends_script_through_pty`, which fails
the same way on a clean checkout of `main`.
- Desktop crate tests: 578 passed
- Frontend: 636 passed, 83 files

## Overlaps with #328

#328 shares one transcript walk between the two cards and reuses it for
five minutes. This takes a different route: parse each file once, keep
the records, and let every caller fold them however it needs. They touch
the same two files and will conflict. Worth picking one before merging
either. No Linear ticket for this one, it came out of a report that
Charts took half a minute to load.


<!-- Macroscope's pull request summary starts here -->
<!-- Macroscope will only edit the content between these invisible
markers, and the markers themselves will not be visible in the GitHub
rendered markdown. -->
<!-- If you delete either of the start / end markers from your PR's
description, Macroscope will append its summary at the bottom of the
description. -->
> [!NOTE]
> ### Parse each transcript once when Charts opens by adding a
persistent, resumable record index
> - Introduces a binary on-disk index
([`usage_index.rs`](https://github.com/tsouth89/ceiling/pull/329/files#diff-573151345c6502ce27bc19e2085b0e4b32ec729e713e155ce718bd0f69e5da7a))
for Claude and Codex transcript records. On subsequent opens, unchanged
files are served from cache; append-only files resume from the last
parsed byte offset instead of re-reading from the start.
> - Adds parallel provider scanning in
[`chart.rs`](https://github.com/tsouth89/ceiling/pull/329/files#diff-dd7afa22800a23cd5d615302c3cea0603e1c29fe1e777943eb5f4e6acd7e2afe),
disk-backed stale-while-revalidate caching via
[`scan_cache.rs`](https://github.com/tsouth89/ceiling/pull/329/files#diff-08b25fa86fea835c4eb448f0f904085e2640f0f6cdda16a65614a9ad5bbc6a85),
and a 5-minute TTL for API value and activity heatmap results.
> - Emits a `local-scan-refreshed` event when a background refresh
completes; `ActivityHeatmapCard` and `TotalApiValueCard` listen via the
new `useLocalScanRefresh` hook and re-fetch without remounting.
> - Adds launch-time prewarm: if the user previously loaded either card,
caches are refreshed 15 seconds after startup.
> - The index is invalidated when the pricing catalog changes (via a
pricing fingerprint) or entries exceed a 400-day TTL. Entries older than
the horizon are dropped on decode rather than panicking.
> - Behavioral Change: default API value scan horizon drops from 90 to
60 days; `get_local_api_value_totals` and `get_local_activity_heatmap`
Tauri commands now require an `AppHandle` parameter.
>
> <!-- Macroscope's review summary starts here -->
>
> <sup><a href="https://app.macroscope.com">Macroscope</a> summarized
2c56d6e.</sup>
> <!-- Macroscope's review summary ends here -->
>
<!-- Macroscope's pull request summary ends here -->

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Performance**
- Faster Charts loading through incremental transcript processing and
concurrent provider scans.
- Results persist between launches, with background warm-up for quicker
initial displays.
- Updated files and pricing changes are detected automatically to keep
results accurate.

- **Charts**
- Local API-value and activity heatmap results are cached and reused for
five minutes.
  - API-value views now default to a 60-day range.
  - Charts refresh automatically when background scanning completes.
- Switching between built-in periods reuses loaded data instead of
triggering another scan.

- **Bug Fixes**
- Improved handling of partially written usage records and replaced
files.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: tsouth89 <tsouth89@users.noreply.github.com>
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.

1 participant