fix(history): stream request index ingestion - #1189
Conversation
📝 WalkthroughWalkthroughThe 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. ChangesRequest-history ingestion
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
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Review readiness checklist
1/4 boxes ticked. This PR stays in draft until every box above is ticked. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
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 `@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
📒 Files selected for processing (2)
src/routing/history/indexer.tstests/request-history-index.test.ts
Summary
Why
The request-history index previously allocated
size - indexedOffsetbytes 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.jsonlremains canonical and is never truncated or rewritten.indexedRowscontinues to count successfully projected records.Verification
tests/request-history-index.test.ts20/20 passed.b22e0e6d0): the same suite 20/20 passed.bun x tsc --noEmit: passed.bun scripts/privacy-scan.ts: passed.git diff --check: passed.Checklist
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
Tests