fix: ingestion crash bugs and resilience found during a live batch ingest - #42
Merged
Merged
Conversation
_split_long_paragraph computed the next start as `end - overlap_chars`, assuming end always lands at start + max_chars. Boundary snapping (added in a prior change) can make end land much earlier than that; subtracting the full overlap from a short end can then leave start unchanged or push it backward, hanging forever. This was the actual root cause behind every ingestion crash chased during a live batch run — pypdfium2 memory usage was a red herring. Caps the overlap to (end - start - 1) so start always advances by at least one character. Adds a regression test with the exact adversarial shape that triggered it: an early sentence boundary followed by a long run with no further boundaries or spaces.
pypdfium2 pages/bitmaps wrap native memory that isn't reclaimed by Python's GC without an explicit close() — a scanned PDF accumulated one rendered page's worth of native memory per page for the life of the document. Both are now closed per page, with a gc.collect() afterward since ctypes-backed objects can defer their finalizer to a GC pass. Also caps the render scale so the bitmap's longest side never exceeds ~2200px, regardless of the page's own physical size: urban-planning / architectural PDFs routinely embed oversized sheets (A0/A1 or larger) as a single page, and at the previous flat scale=2.0 those pages could be tens of megapixels each. Neither of these turned out to be the actual crash cause found during this session (see the chunker infinite-loop fix), but they're real memory-hygiene issues worth keeping regardless. Adds tesseract-ocr-spa to the Docker image for Spanish-language OCR fallback (previously English-only).
Batch ingestion (ingest_directory, rebuild_collection) previously re-raised on the first file that failed to parse/embed, aborting the whole batch and losing visibility into which of the remaining files would have succeeded. ingest_paths now processes every file, retries failures once at the end of the batch, and reports any that still fail via a new failed_sources list (path + error), logged at WARNING on the first failure and ERROR if the retry also fails. Single-file endpoints (POST /ingest, /ingest/upload) get the same one-time retry but still raise (502) on a hard failure, keeping their existing contract. Also fixes a real performance bug found while diagnosing a large batch ingest: bm25_index.refresh() was called after every successful file instead of once per batch. refresh() rebuilds the entire BM25 corpus from scratch, so an N-file batch was doing O(n^2) work instead of O(n). Reordered delete_by_source to run after embedding succeeds rather than before, so a failed embed call no longer deletes the source's existing (still valid) vectors before the retry has a chance to replace them.
Adds a file-picker upload path alongside the existing server-path-based /ingest, for callers that only have a local file (browser "Choose File"), not a path on the machine running the API. Streams the upload to disk in 1MiB chunks under UPLOAD_DIR while enforcing UPLOAD_MAX_BYTES by counting real bytes transferred rather than trusting Content-Length (413 + partial-file cleanup on overflow), validates the extension against the same parser allow-list used by path-based ingest (415 on mismatch), and documents its limitations (no AV scan, extension-only validation, single file per request, persistent storage so later /collections/rebuild can re-embed it) in the endpoint's OpenAPI description, rendered in Swagger. Requires python-multipart (FastAPI's UploadFile/Form dependency).
… default Adds a one-shot localrag-setup compose service that pulls OLLAMA_EMBED_MODEL / OLLAMA_LLM_MODEL via the existing `uv run localrag setup` CLI command before localrag-api starts, so `docker compose up` no longer needs a manual `docker exec ... ollama pull` step. Reuses the CLI's own pull logic rather than duplicating it as shell commands. Switches the default chat model from llama3.2 to gemma3:4b (better quality, 128K context, native multilingual support). Adds restart: unless-stopped to all 5 services. During this session's ingestion debugging, localrag-api was killed (exit 137) with no restart policy configured, so it just stayed dead until manually restarted — this was masking real crashes as "the API is unreachable" rather than "it crashed and needs attention", and meant one bad request could take the whole deployment down until someone noticed. docker-compose.override.yml previously diverged from the base compose file for hot-reload dev ergonomics; now it's identical to base plus one added bind mount (Windows-only, WSL2) so documents can be dropped into a Windows folder instead of copied into the WSL2 filesystem by hand. Fixes a pre-existing YAML indentation bug in infra/prometheus/prometheus.yml that had been silently breaking Prometheus and, transitively, Grafana (Prometheus failed to start on `docker compose up`, so anything depends_on'ing it never started either) since the file was introduced. Adds .gitignore entry for data/uploads/ (POST /ingest/upload destination — user-uploaded documents, not repo content).
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
1 participant
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.
Context
Deployed the stack via Docker and ran a real batch ingest of 37 PDFs (Spanish urban-planning documents). The container crashed repeatedly (exit 137). This PR is the result of chasing that down — the actual root cause turned out to be different from the initial diagnosis, and several other real issues surfaced along the way.
Issues found and their fixes
Root cause of every crash — infinite loop in chunk overlap (
localrag/ingestion/structural_chunker.py)_split_long_paragraphcomputed the nextstartasend - overlap_chars, assumingendalways lands atstart + max_chars. Boundary snapping (sentence/word-aware splitting) can makeendland much earlier; subtracting the full overlap from a shortendthen leavesstartunchanged or pushes it backward — hangs forever. Triggered by real Spanish legal-document text with a sparse early sentence boundary followed by a long unbroken run. Fixed by capping overlap toend - start - 1, guaranteeing forward progress. Regression test included.OCR memory hygiene (
localrag/ingestion/parsers/pdf.py) — a plausible-looking but ultimately incorrect initial diagnosis, kept because it's still a real issue:.close()d per page, plusgc.collect()since ctypes-backed finalizers can be deferred to a GC pass.scale=2.0could produce tens-of-megapixels bitmaps for those.tesseract-ocr-spato the Docker image (OCR fallback was English-only).Batch ingestion had no resilience (
localrag/ingestion/service.py,localrag/api/service.py,localrag/api/schemas.py) — a single bad file aborted the entire batch with no visibility into what would have succeeded.ingest_directory/rebuild_collectionnow process every file, retry failures once at the end, and report permanent failures via a newfailed_sourcesfield (path + error) in the JSON response, logged at WARNING then ERROR. Single-file endpoints keep their existing raise-on-failure contract but get the same one-time retry.O(n²) BM25 rebuild (
localrag/ingestion/service.py) —bm25_index.refresh()was called after every successfully ingested file instead of once per batch.refresh()rebuilds the entire BM25 corpus from scratch, so an N-file batch did O(n²) work. Now called once per batch.delete_by_sourceordering bug (localrag/ingestion/service.py) — ran before the embed call instead of after, so a failed embed left a source with zero vectors instead of its previous (still valid) ones. Reordered.POST /ingest/upload(localrag/api/routers/ingest.py) — new multipart file-picker upload endpoint, for callers that only have a local file rather than a server path. Streams to disk in 1MiB chunks enforcing a real byte-counted size cap (notContent-Length), validates extension against the parser allow-list, documents all limitations in the OpenAPI description (rendered in Swagger).No restart policy on any container —
localrag-apigot SIGKILL'd during debugging and just stayed dead with no auto-recovery. Addedrestart: unless-stoppedto all 5 services.Prometheus/Grafana silently never started (
infra/prometheus/prometheus.yml) — pre-existing YAML indentation bug broke Prometheus's own config parsing; anythingdepends_on-ing it (Grafana) never started either. Fixed.Manual model pull step — added a one-shot
localrag-setupcompose service that pullsOLLAMA_EMBED_MODEL/OLLAMA_LLM_MODELvia the existinguv run localrag setupCLI command beforelocalrag-apistarts. Also switched the default chat model togemma3:4b(better quality, 128K context, native multilingual).Dev/prod compose divergence + Windows ergonomics —
docker-compose.override.ymlpreviously diverged from base (hot-reload command). Now identical to base plus one Windows-only (WSL2) bind mount for drag-and-drop document ingestion.data/uploads/wasn't gitignored — added, since it's the/ingest/uploaddestination (user documents, not repo content).Test plan
uv run pytest -q -m "not integration"— 104 passeduv run ruff check ./ruff format --check .— cleanuv run mypy localrag— clean (pre-existing untyped-lib warnings only)