fix(ingestion): bound memory across upload→process pipeline for all file types#110
Merged
Conversation
…ile types Audit-driven fixes for the same class of unbounded-memory bug behind the recent chunker/embeddings/memory-extraction OOMs, found across the rest of the pipeline: - json handler: stop JSON.stringify(JSON.parse(text), null, 2) — re-indenting inflated dense exports 2–5x and destroyed byte offsets. Pass bytes through. - chunker: enforce MAX_CHUNKS_HARD_CAP (new) via ChunkLimitError so one doc can't produce unbounded chunks/embeddings/rows; fail fast as chunks are produced. ZIP entries fail per-entry; single files fail the upload. - chunker (semantic): fall back to fixed chunking past MAX_SEMANTIC_SENTENCES so a ~9.9MB doc can't embed ~10^5 sentences and hold them all at once. - embeddings: add aggregate per-batch token caps for Ollama and Google (mirroring OpenAI's 200K cap) via a shared buildTokenBoundedBatches helper. - embed-store: stream embed→insert in windows so a document's full embedding array is never resident at once. - xlsx handler: bound the parse with sheetRows and the CSV with a cell budget; a huge workbook no longer balloons heap on XLSX.read (702MB→208MB in repro). - html handler: cap html-to-text depth/child-node limits and fall back to a non-recursive tag strip so deeply-nested markup can't overflow the call stack and crash the worker. - wire WARN_FILE_SIZE_MB (previously unused) as an advisory large-doc log. Regression tests per fix (chunker cap + semantic fallback, JSON no-inflation, batch token bounding). pnpm verify passes. https://claude.ai/code/session_01L1NUnaJbMha7LUQsRoxFkE
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
WalkthroughThis PR hardens the document processing pipeline against resource exhaustion by introducing a configurable MAX_CHUNKS_HARD_CAP and ChunkLimitError, enforcing chunk caps in fixed and semantic chunkers with fallbacks, adding embedAndStoreChunks to window embedding generation and persistence, switching upload paths to use the new embed/store flow and emitting advisory size warnings, adding token-bounded batching for non-OpenAI providers, and tightening HTML/JSON/XLSX extraction limits and related tests. Sequence Diagram(s)sequenceDiagram
participant Client
participant UploadAPI
participant UploadProcessor
participant warnIfLargeContent
participant smartChunk
participant embedAndStoreChunks
participant EmbeddingProvider
participant DB
Client->>UploadAPI: POST /upload
UploadAPI->>UploadProcessor: process upload
UploadProcessor->>warnIfLargeContent: check extracted text size
UploadProcessor->>smartChunk: generate chunks (maxChunks)
UploadProcessor->>embedAndStoreChunks: stream chunks + generateEmbeddings
embedAndStoreChunks->>EmbeddingProvider: generateEmbeddings(window)
EmbeddingProvider-->>embedAndStoreChunks: embeddings(window)
embedAndStoreChunks->>DB: createChunks(window with embeddings)
DB-->>embedAndStoreChunks: persisted
embedAndStoreChunks-->>UploadProcessor: done
UploadProcessor-->>UploadAPI: respond with document
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@src/services/__tests__/chunker.test.ts`:
- Around line 127-137: The test input isn't producing many sentences because
splitIntoSentencesWithSpans only splits when the next sentence starts with a
capital letter; change the repeated sample from 'x. '.repeat(25_001) to a string
that yields capitalized sentence starts (e.g., 'A. '.repeat(25_001) or 'X.
'.repeat(25_001)) so chunkTextSemantic will actually hit the
MAX_SEMANTIC_SENTENCES guard and exercise the fallback path; keep the rest of
the test (generateEmbeddings mock, expectations) unchanged.
In `@src/services/chunker.ts`:
- Around line 15-19: The semantic chunking path currently drops the per-call
hard-cap; thread maxChunks through the semantic types and call sites by adding
maxChunks to SemanticChunkOptions, updating chunkTextSemantic() to accept and
forward maxChunks, and passing through maxChunks in both semantic fallback
locations and in smartChunk() so smartChunk(..., { maxChunks }) is honored for
CHUNKING_MODE='semantic' as well as fixed mode; update any places that call
chunkTextSemantic or the semantic fallback to use the propagated maxChunks value
instead of falling back to config.MAX_CHUNKS_HARD_CAP.
In `@src/services/embed-store.ts`:
- Around line 22-33: Ensure each generated-embedding batch matches the chunk
window before persisting: after calling generateEmbeddings(window.map((c) =>
c.content)) in the loop that uses EMBED_WINDOW, validate that embeddings.length
=== window.length and throw or reject (with a clear error) if they differ
instead of calling createChunks; reference the loop variables chunks, window,
embeddings and the functions generateEmbeddings and createChunks so the check
sits immediately after embeddings is returned and prevents inserting
incomplete/NULL embedding vectors.
In `@src/services/processor/handlers/xlsx.ts`:
- Around line 57-67: The cell-budget cap logic currently forces at least one row
via Math.max(1, ...) which can exceed MAX_XLSX_CELLS when budget < cols; change
the calculation in the block around allowedRows / cappedSheet so allowedRows =
Math.floor(budget / Math.max(cols, 1)) (no min-1), and handle the zero case by
not calling XLSX.utils.sheet_to_csv (e.g., set csv = '' or return an empty
result) when allowedRows === 0; update references to range, allowedRows,
cappedSheet, and the XLSX.utils.sheet_to_csv call so no row is rendered if the
remaining budget cannot cover a full row.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 871c268a-ca10-40ee-b9f2-bca48d55b645
📒 Files selected for processing (15)
.env.exampledocs/ENV.mdsrc/api/upload.tssrc/services/__tests__/chunker.test.tssrc/services/__tests__/embeddings.test.tssrc/services/chunker.tssrc/services/embed-store.tssrc/services/embeddings.tssrc/services/processor/__tests__/json.test.tssrc/services/processor/handlers/html.tssrc/services/processor/handlers/json.tssrc/services/processor/handlers/xlsx.tssrc/services/upload-processor.tssrc/utils/config.tssrc/utils/errors.ts
…lsx budget, test CodeRabbit review on #110 (all 4 verified valid): - chunker: thread maxChunks through SemanticChunkOptions, chunkTextSemantic, both fixed-chunk fallbacks, and smartChunk so a per-call cap is honored in CHUNKING_MODE=semantic, not just fixed mode. - embed-store: validate embeddings.length === window.length before createChunks so a short provider batch fails loudly instead of persisting NULL vectors. - xlsx: drop the Math.max(1, ...) floor on allowedRows — when the remaining cell budget can't cover one full row, stop instead of overshooting the cap. - chunker.test: the semantic-fallback test used 'x. ' (lowercase), which the capital-letter sentence splitter treats as ONE sentence, so it exercised the single-sentence path, not the MAX_SEMANTIC_SENTENCES guard. Use 'X. ' x30000. pnpm verify passes (264 tests). https://claude.ai/code/session_01L1NUnaJbMha7LUQsRoxFkE
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The `budget = 0` in the allowedRows<=0 branch is immediately followed by `break`, so it is never read (CodeQL: useless assignment to local variable). https://claude.ai/code/session_01L1NUnaJbMha7LUQsRoxFkE
jeffgreendesign
pushed a commit
that referenced
this pull request
Jun 12, 2026
…lsx budget, test CodeRabbit review on #110 (all 4 verified valid): - chunker: thread maxChunks through SemanticChunkOptions, chunkTextSemantic, both fixed-chunk fallbacks, and smartChunk so a per-call cap is honored in CHUNKING_MODE=semantic, not just fixed mode. - embed-store: validate embeddings.length === window.length before createChunks so a short provider batch fails loudly instead of persisting NULL vectors. - xlsx: drop the Math.max(1, ...) floor on allowedRows — when the remaining cell budget can't cover one full row, stop instead of overshooting the cap. - chunker.test: the semantic-fallback test used 'x. ' (lowercase), which the capital-letter sentence splitter treats as ONE sentence, so it exercised the single-sentence path, not the MAX_SEMANTIC_SENTENCES guard. Use 'X. ' x30000. pnpm verify passes (264 tests). https://claude.ai/code/session_01L1NUnaJbMha7LUQsRoxFkE
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A real Spotify-export ZIP upload surfaced three production bugs in the async ingestion pipeline (already fixed:
9ad0cc7,ed2fdca,27cc2ee) — all the same class of "memory proportional to (or a multiple of) adversarial input, with no ceiling." This audits the rest of the pipeline for siblings across every supported file type (.txt .md .pdf .docx .csv .xlsx .json .html/.htmand ZIP-of-those) and fixes them.A throwaway repro harness (peak
heapUsedper stage on large/adversarial fixtures) confirmed each fix; two issues it caught were worse than static analysis suggested (see F6/F7).What changed
JSON.stringify(JSON.parse(text), null, 2)inflated dense exports 2–5× and destroyed byte offsetsMAX_CHUNKS_HARD_CAP(50k) →ChunkLimitError; per-entry fail in ZIP, fail upload for single filesbuildTokenBoundedBatchesadds aggregate token caps to all three providersembed-store.tsstreams embed→insert in 128-chunk windowsXLSX.read()parses the entire workbook before any budget appliessheetRowsparse cap + cell budget (repro: 702MB → 208MB)html-to-text's recursive walk and crashed the workerAlso wires
WARN_FILE_SIZE_MB(previously unused) as an advisory large-document log, and documents the new env var in.env.example/docs/ENV.md.Notes / scope
MAX_CHUNKS_PER_FILE(500) is kept as the advisory soft-warn threshold it always was; the newMAX_CHUNKS_HARD_CAP(50k) is the actual reject ceiling, set high so legitimate large uploads still process.Tests
One regression test per fix (chunker cap + semantic fallback, JSON no-inflation, batch token bounding), in the timeout-guarded large-input style of
chunker.test.ts.pnpm verifypasses (264 tests, lint, typecheck, security, tool-sync, build, docs build).Follow-up (not in this PR)
A separate throughput/volume audit produced findings T1–T8 (the async architecture is solid; main gaps are: activate the dormant Cloud Tasks path + raise the Cloud Run timeout, add embedding/Anthropic retry-with-backoff, set queue
maxConcurrentDispatches, use Neon's pooled connection string). The one clear code follow-up is provider retry/backoff; the rest are config/deploy.https://claude.ai/code/session_01L1NUnaJbMha7LUQsRoxFkE
Generated by Claude Code