Skip to content

perf(usage): bound timeline aggregation scratch memory and avoid argument overflow - #765

Closed
luvs01 wants to merge 7 commits into
devfrom
perf/timeline-aggregation-bounds
Closed

luvs01 wants to merge 7 commits into
devfrom
perf/timeline-aggregation-bounds

Conversation

@luvs01

@luvs01 luvs01 commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner

Summary

Recover the unfinished performance/resource audit on fork dev at 306a88e08b387a7d3d2ee51bf04e72530285e5b4.

  • Iterate request-map values for timeline max and average instead of constructing per-request arrays and passing every value as a separate Math.max argument.
  • Remove the extra flatMap array when folding lower-ranked series into other.
  • Preserve same-model attempt grouping, request-weighted averages, missing measurements, filtering, ranking and the 23-plus-other contract. No provider, credential, routing, disk format or public option changes.
  • Add five cases to the existing registered timeline suite: large folded populations for max/average, a large single-series max, and attempt-aware weighted folding for max/average. Update the companion contract.

This reduces temporary allocations and prevents argument-count/stack overflow for large buckets. It does not make the entire accumulator constant-memory: its existing request maps still scale with traffic. No whole-process RAM percentage or throughput improvement is claimed.

Verification

Current recovery environment: Node 22.16.0, no Bun installation and no container network/DNS access. Source files were read through the GitHub connection. Reconstructed original runtime, test and document bytes were checked against their Git blob SHAs before editing.

Executed: an algorithm-only Node harness transpiled the actual timeline module with injected attribution, label and total collaborators. Both baseline high-volume max cases threw RangeError; the patched folded max, folded average, single-series max, weighted max and weighted average checks passed (two baseline reproductions, five patched checks). This is not a native Bun test pass and does not validate the production collaborators.

Not executed in this environment: native Bun tests, repository typecheck, structure/privacy gates, full suite, Windows/macOS tests, live-provider requests. Earlier interrupted-session test reports are not credited as exact-head validation for these reconstructed test additions.

Required exact-head hosted validation includes:

bun run test --parallel=2 ./tests/usage/usage-timeline.test.ts ./tests/usage/anthropic2-usage-attribution.test.ts ./tests/cli/cli-companion-timeline.test.ts
bun run typecheck
bun run structure:check
bun run privacy:scan

Draft until native/exact-head validation is confirmed. No merge, deployment, live configuration or policy change.

Co-authored-by: luvs01 27862058+luvs01@users.noreply.github.com

Summary by CodeRabbit

  • Bug Fixes
    • Usage timelines now calculate averages and maximums more reliably across large request histories, including when multiple series are grouped into “other.”
    • Grouped averages now reflect individual request measurements rather than averaging series averages.
  • Documentation
    • Updated timeline documentation to describe aggregation behavior and coverage for large request populations.

…line aggregation

Iterate request maps for max/average without materializing request-sized arrays.
Preserve per-request attempt grouping and weighted other-series aggregation.
Add high-volume and weighting regressions to the existing registered suite.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

No actionable comments were generated in the recent review.

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 306a88e and 6537577.

📒 Files selected for processing: 3
  • src/usage/timeline.ts
  • structure/companion.md
  • tests/usage/usage-timeline.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The timeline accumulator now calculates maximum and average values by streaming request values. Tests cover large request populations and folded-series weighting. The timeline documentation describes the aggregation behavior and scratch-storage scaling.

Changes: Timeline aggregation

Layer / File(s) Summary
Stream timeline aggregation
src/usage/timeline.ts, tests/usage/usage-timeline.test.ts, structure/companion.md
Per-series and folded “other” aggregation now use running sums, maxima, and counts instead of materializing arrays. Tests cover large request populations and combining same-model attempts before request weighting. The documentation describes the aggregation behavior and scratch-storage scaling.

Priority: Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: lidge-jun

Merge Risk: Minimal · up to 65375

No actionable merge-blocking issue is established; the change is mergeable after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names the primary change: bounding timeline aggregation scratch memory and avoiding argument overflow in the usage timeline code. The changeset matches this. src/usage/timeline.ts replaces p…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Title check

Explanation

The title names the primary change: bounding timeline aggregation scratch memory and avoiding argument overflow in the usage timeline code. The changeset matches this. src/usage/timeline.ts replaces per-request arrays and spread/reduce calls with streaming sum, max, and count accumulation, and the tests cover high-volume max and average cases. The title is specific and does not describe unrelated work.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@github-actions

Copy link
Copy Markdown

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the enhancement New feature or request label Oct 23, 2026

luvs01 commented Oct 23, 2026

Copy link
Copy Markdown
Owner Author

Recovery verification update: GitHub Actions Cross-platform CI run 40031176342 completed successfully for this PR head 65375777e43032b16baec407bc5405bcfe6738f9. React Doctor (40031176339) and Catalog async contracts (40031176363) also succeeded.

This supersedes the description's pending-hosted-validation note for this exact head. The local evidence remains the explicitly limited Node compatibility harness; no native local Bun or all-platform execution is claimed. Scope-skipped jobs are not counted as executed. The PR remains draft and unmerged; this update is not independent review approval, a release, or a deployment.

…jun#7642)

GHSA-mrhx-c7xc-m3w5 and GHSA-27r3-jxmh-p828 blocked the 2.92.0 publish.
Leave the package version at 2.93.0 on dev.

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
@luvs01

luvs01 commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner Author

No description provided.

lidge-jun and others added 5 commits October 24, 2026 09:01
…e-jun#7644)

Co-authored-by: lidge-jun <243035832+lidge-jun@users.noreply.github.com>
…idge-jun#7528)

Settle async Windows ACL work and native-main owner locks before fixture removal, so hosted-Windows teardowns no longer fail with EPERM after passing assertions. No retry, sleep or timeout changes.

Refs lidge-jun#7515
…deterministic (lidge-jun#7552)

Make the codex-runtime, client-connect and loopback-listener tests deterministic: bounded child deadlines with process milestones, and a port-0 public listener in place of the free-port-then-bind race. No sleeps or timeout increases.

Refs lidge-jun#7403 lidge-jun#7404

Co-authored-by: Shreyas652 <66720396+Shreyas652@users.noreply.github.com>
lidge-jun#7598)

Keep the cold first-exec of a fresh Bun copy and the PowerShell start out of two timed cases: the PATH-fallback cases share one warmed Bun copy created in a budgeted beforeAll, and the CurrentUser Root certificate inspection is replayed once before its timed case. The 10s product deadline is unchanged; a new seam test drives a deadline breach to unknown on every platform.

Refs lidge-jun#7578 lidge-jun#7569

luvs01 commented Oct 24, 2026

Copy link
Copy Markdown
Owner Author

Closing this tracking PR after carrying the validated change into the upstream review queue. Commit 2d427e2a305c99bfc9a0f538e9d54d681c359ab7 preserves all 3 change-owned file blobs from 65375777e43032b16baec407bc5405bcfe6738f9 and integrates current dev security dependencies. Bun 1.4.4 Windows validation passed 15 cases/53 assertions; scoped static checks and independent source review passed. The branch is preserved; this is not a merge or release claim.

@luvs01 luvs01 closed this Oct 24, 2026
@github-actions

Copy link
Copy Markdown

⏳ DRAFT

  • UI screenshot required.

What to do

  • Add a screenshot of the UI change to the PR description.

This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants