fix(history): stream request-history index ingestion (#1189) - #1287
Conversation
Replaces the full-tail allocation in the request-history indexer with a 64 KiB streaming reader. The previous `readCompleteTail` allocated `size - indexedOffset` bytes in one shot before parsing, so a large append created a proportional transient allocation even though the SQLite index is a disposable projection. Records are now assembled across chunk boundaries, and a complete record above 1 MiB is omitted from the projection only. `usage.jsonl` stays canonical and is never truncated or rewritten; `indexedRows` still counts successfully projected records, and the indexed offset only advances past a newline so a torn final record is re-read rather than skipped. `insert.finalize()` remains unconditional in `finally` — an unterminated prepared statement keeps the DB file busy on Windows after close. Republished from #1189 by luvs01, whose branch was 300 commits behind dev. Rebased onto f5147cb with no conflicts; authorship preserved below. Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe request-history indexer now reads ChangesRequest-history streaming ingestion
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02ec799fe1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // The SQLite index is a disposable projection. Complete JSONL records above | ||
| // this bound are omitted from the projection; the canonical usage.jsonl is | ||
| // never truncated or rewritten by the indexer. | ||
| export const REQUEST_HISTORY_MAX_RECORD_BYTES = 1024 * 1024; |
There was a problem hiding this comment.
When usage.jsonl contains a complete valid record larger than 1 MiB, the indexer advances past it but permanently omits it from request history and routing analytics. This conflicts with docs-site/src/content/docs/reference/configuration/routing.md, which promises “full history,” and the returned metadata provides no truncation signal, so users can unknowingly make decisions from incomplete results. Document the limit and affected endpoints, or expose an explicit omission indicator.
AGENTS.md reference: src/AGENTS.md:L28-L28
Useful? React with 👍 / 👎.
1 participant
Summary
Republishes @luvs01's #1189 on current
dev. Their branch was 300 commits behind, so this is a maintainer rebase rather than a new change; the commit carries aCo-authored-bytrailer and the fix is theirs.The request-history indexer allocated
size - indexedOffsetbytes in one shot before parsing the appended tail, so a large append created a proportional transient allocation even though the SQLite index is a disposable projection.readCompleteTailis replaced by a reusable 64 KiB streaming reader that assembles complete JSONL records across chunk boundaries.A complete record above 1 MiB is omitted from the projection only.
usage.jsonlstays canonical and is never truncated or rewritten,indexedRowsstill counts successfully projected records, and the indexed offset only advances past a newline so a torn final record is re-read rather than skipped.insert.finalize()remains unconditional infinally— an unterminated prepared statement keeps the DB file busy on Windows after close.Closes nothing on its own; supersedes #1189, which can be closed once this lands.
Verification
Rebased onto
f5147cbc8and re-verified after that rebase, not before it:bun run test— 9991 pass / 7 skip / 0 fail across 625 filesbun test tests/request-history-index.test.ts— 20 pass / 0 failbun run typecheck— cleanREQUEST_HISTORY_MAX_RECORD_BYTEStoNumber.MAX_SAFE_INTEGERfails exactly one test (19 pass / 1 fail); restoring the 1 MiB bound returns 20 pass. The bound is load-bearing, not decorative.rg 'ingestText|readCompleteTail'finds no caller outsidesrc/routing/history/indexer.tsondev, so removing them breaks nothing.Checklist
Summary by CodeRabbit