Skip to content

fix(history): stream request index ingestion - #1189

Draft
luvs01 wants to merge 2 commits into
lidge-jun:devfrom
luvs01:agent/fix-history-index-stream-tail
Draft

fix(history): stream request index ingestion#1189
luvs01 wants to merge 2 commits into
lidge-jun:devfrom
luvs01:agent/fix-history-index-stream-tail

Conversation

@luvs01

@luvs01 luvs01 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace full-tail allocation with a reusable 64 KiB streaming reader;
  • assemble complete JSONL records across chunk boundaries with a 1 MiB projection limit;
  • retain partial final records until a later refresh completes them;
  • advance the indexed offset past complete malformed or oversized records while leaving the canonical usage log untouched;
  • preserve batched SQLite inserts and explicit statement finalization.

Why

The request-history index previously allocated size - indexedOffset bytes before parsing the appended tail. A large append could therefore create a proportional one-shot allocation even though the SQLite index is disposable and each record can be processed incrementally.

The new reader bounds transient input memory. A complete record above 1 MiB is intentionally omitted only from the derived SQLite projection; usage.jsonl remains canonical and is never truncated or rewritten. indexedRows continues to count successfully projected records.

Verification

  • Bun 1.3.14: tests/request-history-index.test.ts 20/20 passed.
  • Bun 1.4.0-canary.1 (b22e0e6d0): the same suite 20/20 passed.
  • Regressions cover a valid record crossing 64 KiB, an oversized valid record followed by an indexable record, exact offset accounting, canonical-log size preservation, and partial-line completion.
  • bun x tsc --noEmit: passed.
  • bun scripts/privacy-scan.ts: passed.
  • git diff --check: passed.
  • Independent read-only review found no blocking P0/P1 issue in offset, chunk, cleanup, or memory-bound behavior.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. (The projection contract is documented in source and tests; no public API changed.)
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Summary by CodeRabbit

  • Bug Fixes

    • Improved request-history indexing for large files by processing data in smaller chunks.
    • Correctly handles records that span read boundaries.
    • Skips oversized records without altering the canonical usage log.
    • Ensures incomplete trailing records are safely preserved for subsequent processing.
    • Maintains accurate indexing as additional portions of the history become available.
  • Tests

    • Added coverage for chunk-spanning records, partial records, and oversized-record handling.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The request-history indexer now reads the usage log in 64 KiB chunks. It preserves partial JSONL records, skips records above 1 MiB, tracks complete-record offsets, detects source changes, and closes resources. Tests cover partial, chunk-spanning, and oversized records.

Changes

Request-history ingestion

Layer / File(s) Summary
Bounded JSONL ingestion and cleanup
src/routing/history/indexer.ts
Lines 79–83 add read-chunk and maximum-record-size constants. Lines 217–280 replace whole-tail loading with chunked reads, fragment handling, oversized-record skipping, offset tracking, source-change detection, and batch commits. Lines 290–292 retain prepared-statement finalization during cleanup.
Chunk and oversized-record validation
tests/request-history-index.test.ts
Lines 2–29 add filesystem helpers and streaming constants. Lines 209–222 verify partial-line offsets. Lines 224–236 cover records spanning read chunks. Lines 238–252 cover oversized-record skipping, subsequent indexing, offset metadata, and canonical-log preservation.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant RequestHistoryIndexer
  participant SourceFile
  participant PreparedStatement
  participant RequestHistoryIndex
  RequestHistoryIndexer->>SourceFile: Read a 64 KiB chunk
  SourceFile-->>RequestHistoryIndexer: Return chunk bytes
  RequestHistoryIndexer->>PreparedStatement: Insert each complete valid record
  PreparedStatement->>RequestHistoryIndex: Store indexed rows
  RequestHistoryIndexer->>RequestHistoryIndex: Commit rows and next complete-record offset
  RequestHistoryIndexer->>PreparedStatement: Finalize during cleanup
Loading

Possibly related PRs

  • lidge-jun/opencodex#1004: Both changes modify request-history JSONL indexing in src/routing/history/indexer.ts and its tests.

Suggested reviewers: wibias

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: streaming request-history index ingestion.
✨ 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.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (1/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 1/4).

Review readiness checklist

  • ✅ All CI tests are green on my local testing.
  • ⬜ I pushed my PR to the latest dev commit.
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

1/4 boxes ticked.

This PR stays in draft until every box above is ticked.

@github-actions github-actions Bot added the bug Something isn't working label Aug 7, 2026
@luvs01

luvs01 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

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.

@coderabbitai coderabbitai 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.

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 `@tests/request-history-index.test.ts`:
- Around line 228-243: Update the test “streaming refresh skips an oversized
record without changing the canonical log” to capture the canonical log contents
before queryRequestHistory() and compare them with the contents afterward, while
retaining the existing size assertion. Verify byte-for-byte content preservation
so equal-length truncate-and-rewrite defects are detected.
- Around line 214-226: Extend the streaming refresh test around
queryRequestHistory to cover an incomplete final JSON record: append the
serialized record without its newline, verify indexedOffset remains at the prior
complete-record offset, then append the newline and refresh again. Assert the
record is indexed exactly once after completion and remains available without
duplication.
🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 34a6df38-76b7-4f25-a981-c0e3a9031755

📥 Commits

Reviewing files that changed from the base of the PR and between 44dce33 and 6e7269d.

📒 Files selected for processing (2)
  • src/routing/history/indexer.ts
  • tests/request-history-index.test.ts

Comment thread tests/request-history-index.test.ts
Comment thread tests/request-history-index.test.ts
@lidge-jun
lidge-jun marked this pull request as ready for review August 8, 2026 09:16
@github-actions
github-actions Bot marked this pull request as draft August 8, 2026 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant