fix(corpus): an unreadable path in the corpus was reported as an absent one

[sturlese]

Found by an autonomous bughunt sweep.

Bug

Absence from files.jsonl is not neutral. The id drops out of inventory.json, and clean's classify_pending then emits reason: "deleted" (state.py:123 — membership only, disk presence is never consulted), which drives remove_page + delete_facts (main.py:150). So any path enumerate-files silently omits is a document it silently destroys. Three ways that happened:

1. An unreadable file let _md5 raise straight out of the loop, killing the stage on the first one — after the whole corpus was already walked and hashed. Repairing several meant one discovery per full re-walk. The corpus is mounted read-only by contract (paths.py), so chmod may not even be available.
2. An unreadable directory was silent: os.walk defaults to onerror=None, which swallows a failed listdir and yields nothing, so an org-unit folder dropped every document beneath it with no error anywhere.
3. Worst: os.path.isfile ran before any guard, and it swallows OSError internally and returns False. Every stat-level failure was therefore skipped silently at exit 0 — EACCES via a non-traversable parent, EIO, ESTALE, and the real listdir→stat race. A directory with mode 0o400 is the sharp case: the r bit lets os.walk list the names so onerror never fires, and every file under it vanished.

Verified end to end against examples/demo-corpus — chmod 000 on one file dropped local-8cb904b68e2b from the inventory and produced a deleted verdict for it while the source sat untouched on disk. For a directory, three documents at once.

Fix

The stat moves inside the guard (with an explicit S_ISREG check inheriting isfile's other job — opening a fifo blocks the stage forever), onerror feeds the same list, and the stage raises once naming every offending path — capped at 50, since an unreadable mount root otherwise produced a 2.66 MB stderr message. Type is PathError, which cli.py already renders as ERROR: … with exit 2; no CLI change.

Failing rather than skipping is the load-bearing decision. An earlier version of this fix skipped-and-logged; that is precisely the silent-deletion path above, so a transient permission blip or EIO would have destroyed a page and every figure extracted from it.

ENOENT is deliberately not fatal — the one case where absence is simply true: the file was listed and is now gone. taxonomy.json's system-noise verdict covers .crdownload/.download/.tmp, i.e. the project states in-flight sync artifacts live in corpora, and vanishing is what they do; failing there would let a half-finished download block the pipeline. Every other errno means exists but unreadable, where reporting absence would be a lie.

Between blocking the pipeline and deleting data, blocking is the recoverable one. A regression test pins that direction so a future "it shouldn't stop for one file" change has to confront the deletion it would cause.

Test

+8 in pipeline/corpus/tests/test_enumerate_inventory.py (permission-based ones skipped under root): all unreadable files reported in one pass; two unreadable directories in one pass; a stat-level failure guarded (flat 0o400, no monkeypatching); a file that vanished mid-walk treated as absent, via a real sibling race; non-regular files still skipped; OSError breadth across EIO/ESTALE and narrowness vs a hashing bug; the cap and the report's ordering; and the test that documents why failing is the safe direction.

Full CI-equivalent suite green locally (7 packages, ruff, golden evals 25/25 with both curation: metrics unchanged); corpus 57 tests, +8.

Mutation-tested with hash-verified restores:

mutant	result
revert to main (raise on first file)	2 failed
skip instead of fail (the wrong fix)	2 failed
restore os.path.isfile before the try (stat unguarded)	2 failed
onerror re-raises (abort on first bad dir)	1 failed
onerror swallows instead of records	1 failed
drop onerror entirely	1 failed
catch only PermissionError	2 failed
except Exception instead of OSError	1 failed
make a vanished file fatal	7 failed
drop the S_ISREG guard	hangs on a fifo (killed at 45s)
truncate the listing but keep the count	4 failed
drop sorted(names)	2 failed
record a dir without its trailing /	1 failed

One known gap, stated rather than papered over: dropping the pre-existing dirs.sort() survives, because pinning cross-directory order needs control of readdir order that a portable test cannot assume. The within-directory half is pinned.

🤖 Generated with Claude Code

[sturlese]

Gate round 1 — findings confirmed and fixed, fix reworked twice

Adversarial review (opus, told to refute from correctness / regression / consumer-impact angles) plus my own end-to-end tracing. Round 1 did not rubber-stamp — it changed the fix twice, once in direction and once in scope.

Before opening, I inverted the fix. The original was skip-and-log. Tracing the blast radius against examples/demo-corpus showed skipping is the deletion path: chmod 000 on one file dropped local-8cb904b68e2b from the inventory and produced a deleted verdict for it, source untouched on disk. Skipping would let a transient blip destroy a page and its facts.

CONFIRMED (correctness), from the review — the guard was bypassed for every stat-level failure. os.path.isfile ran first and swallows OSError internally, so only _md5's open() reached the except. Demonstrated with no monkeypatching: a real listdir→stat ENOENT race, and a flat 0o400 directory losing every file at exit 0. This falsified the error message's own claim and my OSError-breadth rationale — my breadth test passed only because it faked the error at _md5. Fixed by moving the stat inside the guard, plus S_ISREG to keep isfile's other job (opening a fifo blocks the stage forever — mutation-confirmed: dropping it hangs the suite).

CONFIRMED (correctness) — the nested case under-reported and named an unfixable path. Unit A/sub/ was reported while Unit A/a.pdf was silently lost, and the operator could not chmod the reported path. The stat fix resolves both: each file is now named individually.

CONFIRMED (test-gap) — 3 surviving mutants, all now killed: stat outside the guard; an onerror that re-raises (the dir test used a single bad directory, so it could not tell collecting from aborting); and sorted(names).

CONFIRMED (overstatement) — the message said "or exclude them", but no exclusion knob exists at or before this stage; taxonomy.json filtering runs in classify-files, afterwards. Reworded. That same finding surfaced the .crdownload insight that reshaped the fix: the project expects in-flight sync artifacts in a corpus, so ENOENT is now non-fatal while every other errno stays fatal — absence is true for a vanished file, a lie for an unreadable one.

CONFIRMED (robustness) — the listing was unbounded: 20k unreadable files measured 2.66 MB of stderr. Capped at 50 plus a count.

Refuted attacks (fix survives): PathError is correct — one catcher repo-wide (cli.py → exit 2), and nothing keys on 2 vs 1. Report ordering is deterministic (20 runs, 1 distinct ordering). And the blast-radius claim is not overstated: confirmed at state.py:123-125 + main.py:150-158, including 3 deleted verdicts for one unreadable directory with all sources present on disk.

Not demonstrated, and worth a human's eye: whether hard-blocking can break a corpus where unreadable paths are normal. The reviewer swept a real Google Drive FUSE mirror (4001 dirs, onerror count 0) and two mounted HFS volumes (0 failures) without reproducing it. Note the ENOENT carve-out is what keeps the common case (a vanishing .crdownload) non-fatal; if a corpus turns up with genuinely unreadable-but-present paths, the right answer is an ignore list at this stage, not going back to skipping.

One gap stated rather than papered over: dropping the pre-existing dirs.sort() survives, since pinning cross-directory order would need readdir guarantees no portable test can assume.

Verdict: CLEAN after fixes. Full CI-equivalent suite green locally, 13 mutants killed, all CI checks green. Proceeding to gate round 2 given how much the fix changed.

[sturlese]

Gate round 2 — the ENOENT carve-out was refuted; fixed. Not merging: 2 rounds used, this needs your call.

Round 2 attacked the newest decision (the ENOENT carve-out) and broke it with an executed reproduction. Findings are fixed and pushed, but per the loop's own rule — two gate rounds is the limit, and this round found confirmed correctness defects — I am leaving this open for you rather than merging it.

CONFIRMED (correctness, highest) — the carve-out silently deleted live documents. My justification, "ENOENT is the one case where absence is simply true," conflated this path is gone with this document is gone. Rename an ancestor directory mid-walk and every not-yet-stat'd file under it raises ENOENT while the documents sit alive under the new name — which the walk never visits, because that name wasn't in the listing os.walk already took:

[enumerate-files] vanished during the walk: 'Unit A/c.md'
[enumerate-files] vanished during the walk: 'Unit A/d.md'
run_stage returned: 2 (exit code would be 0)
files.jsonl: ['./Unit A/a.md', './Unit A/b.md']
actually on disk: ['a.md', 'b.md', 'c.md', 'd.md']

With a single-unit corpus the stage returns [] entirely. And there is no proportional mass-deletion guard downstream — only CLEAN_MAX_DOCS and CLEAN_DRY_RUN. Not a regression (pre-fix isfile skipped identically), but it is exactly the hazard class this PR claims to close, so "three ways" was incomplete.

Fixed with the discriminator the review identified: ENOENT is absence only while os.path.isdir(root) still holds. The parent surviving proves the file itself went away; the parent gone means an ancestor moved, which is the unreadable case. Verified — the same reproduction now reports all three live documents and writes no artifact.

CONFIRMED (overstatement) — the cap could hide the one entry that mattered. The prefix was walk order, not blast radius, so 50 unreadable files in an early unit truncated away a later unreadable directory hiding 5000 documents — the one-per-re-walk pathology reappearing through my own cap. Directories now lead the report.

Header wording fixed: it asserted paths "exist but could not be read", which is false for a vanished directory. Now "could not be enumerated", with "or re-run once the corpus has stopped changing".

Partially accepted, and I'll say why I didn't follow it: round 2 argued a vanished directory should get the same non-fatal treatment for symmetry with .crdownload. I kept it fatal. For a file, the parent-survived check proves the file itself went away; for a directory there is no such check — "removed" and "renamed" are indistinguishable from inside onerror, and guessing "removed" when it was renamed reports every document beneath it as deleted. A needless block is recoverable; that is not. Documented as a deliberate asymmetry rather than left implicit.

Both test gaps closed (an absolute-path dir entry satisfied a substring assertion; the cap's > vs >= boundary at exactly 50). Doc drift fixed: the module docstring and docs/pipeline/corpus.md both described a stage that cannot fail, and the failure is now part of its contract.

Round-1 fixes all survived independent re-attack, with two useful confirmations: S_ISREG has a real non-fifo job (a directory can reach the loop when os.walk can't classify it, and _md5 would raise a spurious fatal IsADirectoryError), and the errno taxonomy is exact in CPython 3.14 — only errno 2 is FileNotFoundError; ENOTDIR/ELOOP/ENAMETOOLONG/ESTALE all stay fatal. Determinism holds across 25 runs; consumers, demo path and CI mode (57 → 59 tests, 97.25% coverage) unaffected.

Mutation set: 13 mutants, zero survivors, including every mutant round 2 found surviving.

Why this wants a human

The fix was reworked three times and each round found the previous version wrong — first in direction (skip → fail), then in reach (the isfile bypass), then in classification (per-errno instead of per-cause). It is green and thoroughly pinned now, but the pattern says the surrounding contract is subtler than a single automated iteration should settle. Two things worth your judgment:

1. clean deletes on inventory absence without checking disk presence (state.py:125). Every defect above is only dangerous because of that. A presence check there would make this stage's correctness far less load-bearing — that is a design decision, not a bugfix, so I did not touch it. Logged as a backlog lead.
2. The container runs as root on purpose (pipeline/corpus/Dockerfile), so the EACCES half of this fix cannot trigger in the documented workflow; only EIO/ESTALE can. The permission cases bite the direct-CLI path only. Not a defect, but it narrows how much this fix buys in production.

All 10 CI checks were green on the previous commit; the current one is pushed and re-running.