Skip to content

indexer: single-pass in-memory tarball ingestion (#106) - #115

Merged
IceRhymers merged 1 commit into
integration/indexer-performancefrom
feat/106-streaming-ingest
Jul 25, 2026
Merged

indexer: single-pass in-memory tarball ingestion (#106)#115
IceRhymers merged 1 commit into
integration/indexer-performancefrom
feat/106-streaming-ingest

Conversation

@IceRhymers

Copy link
Copy Markdown
Owner

Refs #106

What

Replaces extract-to-disk tarball ingestion (indexer/fetch.py:extract_tarball +
tarfile.extractall(filter="data")) with a single-pass streaming iterator,
indexer/ingest.py:iter_tar_source_files, that reads members directly off the
tar stream (tf.next() / tf.extractfile()). No getmembers(), no
extractall(), nothing written to disk beyond the compressed tarball itself.

This is PR A of a two-PR sequence for #106 (per the Critic-approved plan in
.omc/plans/issue-106/approved-plan.md). PR B — adding indexer/ingest.py to
tests/unit/test_semantics_version_tripwire.py's SEMANTICS_PATHS — is
deliberately not in this diff; it must land as a separate, immediate
follow-up once this merges (see "Why two PRs" below).

Why it matters

  • AC1 — one gzip decompression instead of two.
  • AC2 — per-worker peak local disk drops from ~2.5 GB (tarball + its
    extracted tree) to ~0.5 GB (the tarball alone). config.yaml,
    indexer/repo_config.py, and the runbook's disk-guard section are updated to
    the new arithmetic; the concurrency default (4) is intentionally left as-is,
    with a note that infra: re-derive worker, disk, and memory limits after streaming + delta #109 owns re-deriving it.
  • AC3 — corpus is byte-identical. indexer/parse.py is untouched (verify:
    git diff --stat origin/integration/indexer-performance...HEAD -- indexer/parse.py
    is empty) and stays as the executable oracle a new parity test
    (tests/unit/test_ingest_parity.py) pins the streaming path against via live
    set-equality, not a golden fixture. INDEX_SEMANTICS_VERSION is not bumped.
  • AC4 — runbook §2/§3, config.yaml, indexer/repo_config.py, and
    indexer/AGENTS.md updated to the new disk/timing numbers.

Security-relevant behavior change

filter="data"'s built-in protections don't exist for a raw stream, so this PR
hand-implements the equivalent checks and, in the process, closes two latent
path-confusion bugs the old extract-to-disk path had (probe-verified against
Python 3.12's tarfile, not inferred from its docs):

Member shape Old behavior New behavior
Absolute name /etc/passwd silently dropped raises ValueError
Absolute name /{TOP}/evil.py silently indexed under a name the archive never had raises ValueError
../evil.txt branch failed raises ValueError
{TOP}/../evil.txt silently dropped raises ValueError
{TOP}/../{TOP}/evil.py silently indexed raises ValueError
Symlink/hardlink → absolute or escaping target branch failed raises ValueError (hardlinks now explicitly in scope for the same rule as symlinks)
Benign internal hardlink / special file (device, FIFO, socket) hardlink materialized+indexed; special file failed the branch both skip + log a warning (relaxable now that nothing is written to disk)
Empty archive / no top-level dir branch failed still raises — this is the one failure mode that would otherwise be invisible in production: a zero-file result on an already-indexed branch silently stamps it "current at HEAD" without ever having re-read it
Truncated download (extraction would fail loudly on the incomplete tree) now explicitly drains the stream's gzip trailer after the read loop so a truncated/corrupt archive raises rather than silently yielding a partial or empty file set

The member-name .. check is deliberately evaluated on the raw, unnormalized
path (rejecting any literal ".." component) before any normalisation —
normalising first would silently defeat two of the rows above. The link-target
escape check is separately implemented in pure posixpath string arithmetic
(not os.path.realpath, which would resolve against the real filesystem/cwd —
silently wrong in a stream context with no destination directory).

The decompression-bomb cap (MAX_EXTRACTED_BYTES, moved from fetch.py to
ingest.py and re-scoped from a disk cap to a work cap) is enforced two ways:
accumulated regular-file size (matching the old accounting exactly) and the tar
stream's cumulative member.offset, so archives made mostly of non-regular
members can't bypass the cap by carrying no counted data. tarfile.open is
pinned to mode="r:gz" (not "r:*") since GitHub only ever serves gzip, and
accepting bzip2/LZMA would otherwise raise the disk-to-decompressed
amplification ceiling substantially.

This diff went through an independent code-reviewer pass (verdict: APPROVE)
and an independent security-reviewer pass (verdict: LOW risk, all
probe-verified hardening rows above independently reproduced) in separate
contexts from implementation; findings from both were fixed before this PR was
opened.

Operator-visible changes

  • Disk-guard log line now reads 0.5 GB peak instead of 2.5 GB peak.
  • The indexer: per-phase timing instrumentation for indexing runs #103 phase-timing line (total resolve download extract parse embed db sweep other)
    drops extract= — decompression cost now lands inside parse= instead, since
    there's no longer a separate extraction phase. Nine fields → eight, at every
    site: code, tests (_TIMING_RE, and the timing tests, including a rewrite —
    not just a re-tune — of the test that bound the now-deleted
    job.extract_tarball), the runbook, and indexer/AGENTS.md.
  • other= shrinks materially — no more rm -rf of a multi-GB extracted tree at
    teardown.
  • Tied semantic (RRF) search results can re-shuffle, but only for files a run
    actually rewrote
    — never for the corpus as a whole, and not at all on a
    branch that re-indexes as fully unchanged (the common case, since indexer: file-level delta indexing keyed on (path, content_sha) #104's
    delta gate classifies an already-indexed, unchanged file with zero writes and
    a preserved files.id). A dedicated integration test
    (tests/integration/test_job_ingest_delta.py) proves this end-to-end: index
    a branch via the old code path, then re-index the identical tarball through
    the new streaming path with the delta gate open, and confirm zero writes.

Gates (all run fresh in this worktree, not relayed from CI)

$ uv run ruff check . && uv run ruff format --check . && uv run mypy app indexer webui
All checks passed!
140 files already formatted
Success: no issues found in 37 source files

$ uv run pytest -m "unit or observability"
1221 passed, 261 deselected, 1 warning in 103.88s

(Baseline at this branch's base commit, 17aeb4f, was 1187 passed, 260 deselected.)

$ GITHUB_BASE_REF=integration/indexer-performance uv run pytest tests/unit/test_semantics_version_tripwire.py -v
9 passed

(Confirmed green and not skipped — both CI workflows checkout at depth 1, so
this test likely does skip in CI; the git diff --stat check above is gate 5's
primary AC3 evidence for that reason.)

$ git diff --stat origin/integration/indexer-performance...HEAD -- indexer/parse.py indexer/store.py app/db/models.py tests/unit/test_semantics_version_tripwire.py
(empty)

make test-integrationlocal-environment limitation, documented for the
reviewer
: this repo's CI has no provisioned Postgres gate
(ci-lakebase.yml is inert repo-wide — gh variable list is empty), so this
can't be deferred to CI. Run locally against a local pgvector/pgvector:pg16
container (codesearch-pg) with PGUSER=codesearch PGPASSWORD=codesearch PGDATABASE=codesearch PGHOST=localhost exported (the Makefile target itself
doesn't set these). Result: 205 passed, 8 failed, 43 errors, 3 xfailed, 2 xpassed. The failures/errors are a pre-existing local-environment gap, not a
regression
: this container is vanilla pgvector/pg16, not a real Lakebase
branch, so it lacks the lakebase_tokenizer/lakebase_ann/lakebase_bm25
extensions that test_migrations.py (rev 0004+), test_reconcile.py,
test_semantic_rrf.py, test_store_chunk_writer.py, and test_webui_semantic.py
need — reproduced identically with this branch's changes stashed. Three of the
eight FAILED (test_commit_search.py x2, test_mcp_server.py) also
reproduce byte-identically on the unmodified base branch. Every suite that
can run locally passes, including the new
tests/integration/test_job_ingest_delta.py, test_job_reconcile.py, and
test_store_delta.py.

$ make webui-verify-dist
✓ built in 1.02s
(clean git status on webui/frontend/dist afterward — no diff, no frontend change)

Definition of done (PR A)

  • indexer: per-phase timing instrumentation for indexing runs #103 and indexer: file-level delta indexing keyed on (path, content_sha) #104 merged on integration/indexer-performance — branched from 17aeb4f
  • git diff --stat empty on indexer/parse.py, indexer/store.py, app/db/models.py, tests/unit/test_semantics_version_tripwire.py
  • INDEX_SEMANTICS_VERSION unchanged
  • indexer/ingest.py added; extract_tarball deleted; no getmembers/extractall anywhere in indexer/
  • MAX_EXTRACTED_BYTES moved to ingest.py, re-scoped as a work cap, and bounded against non-regular-member bypass
  • AC1 test measures decompressed bytes pulled, not getmembers() calls
  • AC2 test asserts nothing but the tarball in the worker temp dir
  • Oracle parity test green; fixture contains no member filter="data" would itself reject
  • Empty-archive and truncated-archive raises both tested
  • Every probe-verified hardening row has a named test
  • Link-target check is pure posixpath arithmetic, not an os.path.realpath transliteration
  • tf.members.clear() is the first statement of the loop body
  • REQUIRED_FREE_BYTES == MAX_TARBALL_BYTES; disk-guard message reworded
  • Runbook, config.yaml, repo_config.py, indexer/AGENTS.md, fetch.py, job.py docstrings updated
  • make lint / make test green with pasted output above; count above the 1187/260 baseline
  • extract= retirement complete across code, tests, runbook, AGENTS.mdgrep -rn 'extract=' indexer/ tests/ docs/ | grep -v 'extract_file\|extract symbols' returns only the two prose sentences explaining the retirement
  • The rewritten timing test's recomputed totals verified correct, not just asserted
  • Parity test asserts lang/size field-by-field (including an uppercase-suffix case, since (path, content_sha)-keyed delta indexing can't self-heal a lang/size divergence)
  • Integration re-index test shows an already-indexed branch reclassifying as all-unchanged through the new path
  • Independent code-reviewer (APPROVE) + security-reviewer (LOW risk) passes complete, findings fixed
  • PR targets integration/indexer-performance, body says Refs #106 (never Closes)

Not in this PR, by design: arming SEMANTICS_PATHS on indexer/ingest.py
(PR B, immediately after this merges — adding a new file to that list is itself
a tripwire offender in any diff whose base predates it, including
master...integration/indexer-performance's eventual arc merge, so it's
sequenced deliberately rather than bundled here).

Replace extract-to-disk tarball ingestion with a streaming iterator
(indexer/ingest.py:iter_tar_source_files) that reads directly off the
tar stream via tarfile.next()/extractfile(). This drops the second
full gzip decompression that extractall() used to pay and removes the
extracted tree from disk entirely, so a worker's peak local disk is
just the compressed tarball (0.5 GB) instead of the tarball plus its
expansion (2.5 GB).

Since filter="data"'s built-in protections are gone with extractall(),
the new path reimplements them by hand over the raw stream: absolute
names, ".." traversal (checked on the raw path component, before any
normalisation, since normalising first hides two latent path-confusion
bugs the old extract-to-disk path had), and absolute/escaping symlink
and hardlink targets all now raise instead of silently dropping,
silently indexing under the wrong path, or (for links) failing the
branch as before. An incremental cap bounds decompressed bytes per
branch, keyed off both accumulated regular-file size and the tar
stream's cumulative offset so non-regular members can't bypass it, and
a truncated download is caught by draining the stream's trailer rather
than silently indexing a partial or empty corpus.

indexer/parse.py is untouched and stays as the executable oracle a new
parity test pins the streaming path against; INDEX_SEMANTICS_VERSION
is not bumped. The #103 phase-timing line drops its now-nonexistent
extract= field (nine fields to eight) since decompression is folded
into parse= instead. Docs, config.yaml, and indexer/AGENTS.md are
updated to the new disk/timing arithmetic.

Refs #106
@IceRhymers
IceRhymers merged commit ad04748 into integration/indexer-performance Jul 25, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant