Discharge the cleanup round's last five debts - #38
Open
leeovery wants to merge 7 commits into
Open
Conversation
ledger.py's per-word hint tables and migration 1's translation tables were two spellings of one fact: which words the old engine wrote and what they became. The boundary now owns them as public RENAMED_KINDS / RETIRED_STATUSES, migration 1 translates from those same objects, and its filename-prefix pairs are derived from the kind renames (outputs are named <kind>-<hash6>.md, so a kind rename IS a prefix rename). The pinned error messages — the migration-hint text the tests and skills rely on — are byte-identical. Bug class made structurally impossible: the copies-drift-apart class from the review record. Concretely, a future vocabulary retirement landing in the migration's table but not the boundary's would make the loud error stop naming the migration that fixes the line (falling through to the generic unknown-value raise with no sync hint), and one landing in the boundary's table alone would hint at a sync that fixes nothing. With one table there is no second copy to forget. Pinning tests: TestOneVocabularySpelling asserts identity between the migration's tables and the boundary's, and that the filename prefixes are exactly the derivation. Verified by mutation: reintroducing local copies in migration_1 fails both identity pins; hard-coding a stale prefix tuple fails the derivation pin. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
pipeline/registry.py built DRIVERS as a module-level list the moment anything imported it — seven driver instances plus a Capabilities registry, constructed by import order and shared process-wide, against the no-import-time-state rule the rest of the engine keeps. default_drivers() now builds that same list on demand and every consumer calls it at its entry: exclude and lint once per verb, normalize once per run (threaded through kind_of, which now takes the registry it detects with), migration 2 once per apply, and run._unit_owners builds one only for the standing-report callers that hold no RunContext. The typed literal in build_drivers stays the Protocol-conformance point, and the order — web catch-all last — is still defined in exactly one place. Failure mode made structurally impossible: import-order-dependent shared state. A module-level registry is one object every caller in the process aliases — a test or future caller mutating it (reordering, appending a driver) would silently change detection for every other caller, and construction at import runs before any entry point can choose its wiring. With construction behind a call, each caller owns a fresh list and nothing executes at import. Pinning tests: TestNoImportTimeState asserts the module holds no DRIVERS attribute and no module-level built sequence, and that two default_drivers() calls return distinct lists of distinct instances. Verified by mutation: reintroducing the module-level literal fails the first pin; memoizing default_drivers through a global fails the second. The existing order pins (web last, the seven-kind design order) now run against default_drivers() and still guard the catch-all's position. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
urllib_transport read every body whole before any size check ran, so the §7 media ceiling was enforced only after an oversize download had already been buffered — a 2GB enclosure behind a lying or absent Content-Length cost 2GB of memory to refuse. The transport now takes the caller's byte ceiling: with limit set it reads in chunks and stops one byte past the ceiling, so the caller's len(body) > limit check still fires while the rest of the body is never requested. fetch_classified carries the ceiling through, and the media stage passes MEDIA_MAX_BYTES on its GET. Under-ceiling bodies are byte-identical; over-ceiling bodies land as the same skipped outcome with the same reason, now at a bounded cost. The ceiling rides only the fetches that have one. The enclosure GET stays whole-body by design (a 150MB episode is the work, cached to disk), as do inbox's asset download (size known from the API metadata and verified after; the capture must materialize whole) and the JSON API reads — every .read( in the HTTP paths was walked and none of the others has a ceiling to enforce, so imposing one would change outcomes. Failure mode made structurally impossible: the unbounded buffer between GET and size check. The check and the read are now one contract — the ceiling is enforced while the body arrives, so no server response can make the media stage hold more than ceiling+1 bytes, whatever the headers claim. Pinning tests, each verified against its mutation: the chunk-yielding fake proves an over-ceiling body is read to exactly limit+1 bytes and no further (fails when the bounded read reverts to response.read(), and under the remaining=limit off-by-one that would silently truncate an exactly-oversize body into a "successful" download); a body at the ceiling arrives byte-identical; and the run-level pin asserts the media GET carries MEDIA_MAX_BYTES (fails when the wiring is dropped). FakeTransport honors and records limits the way the real seam does. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fetched_count walked every ledger entry per admission to answer the 12-URL cap, so admitting N promotions against an M-entry ledger cost N*M owner resolutions — quadratic in the item's ledger, paid inside the one door every unit enters through. The drain now keeps a per-item table: built lazily from the entries in one pass, updated in record — the only place entries change — and dropped whenever resolve_owners runs, because the counts attribute by exactly the answer owner_of gives and a rename mid-run changes that answer wholesale. The counting rule itself is now one named predicate (_spends_url_budget) instead of an inline condition spelled where it is used. Failure mode made structurally impossible: a maintained count that can drift from the recount it replaced. The two ways such a cache goes stale are writes that bypass it and attributions that outlive it; record is the single mutation door (the table update rides it, before the superseded line is lost), and the owner map's only writer invalidates the table in the same breath — so there is no code path that changes an entry or an attribution without the table following. Pinning test: TestFetchedCountStaysARecount drives one drain through seeding, a cap-refused promotion, a --force admission past the bound, a skip that returns budget, a media line that spends none, and a rename's re-attribution, asserting after each step that fetched_count equals an independently spelled full recount. Verified by mutation: dropping the resolve_owners invalidation, the superseded-line decrement, or the new-line increment each fails the test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ledger.append opened and closed the file per line, so a drain paid an open, a buffer, a close and a parent-mkdir for every one of its writes — hundreds per real run. ledger.Appender now holds one append-mode handle, opened lazily on the first line; _Drain.record writes through it and the verbs close it as they finish. ledger.append stays as the one-shot form for the single-line callers (exclude's landing correction, fixtures), implemented over the same Appender so there is one write path. Durability, stated: open-per-line guaranteed that when append returned the line had reached the OS (close flushes), making it visible to every reader — the mark verb re-reads the ledger immediately after appending, and the run report reads its own writes back — and safe against the process crashing; it never fsynced, so an OS crash or power loss could lose the tail. The held handle flushes per line and therefore provides exactly that same guarantee, no more claimed and no less delivered. Append mode keeps another writer's interleaved line intact, and the union-merge file format is byte-identical. Failure mode made structurally impossible: the per-line reopen storm — with the handle owned by the drain and opened once by construction, no ledger write inside a run can reintroduce an open per line. Pinning tests, each verified against its mutation: byte-identity of a pooled run of appends against per-line appends (fails under an "a"→"w" open); every line readable by a fresh load before the handle closes (fails when the per-line flush is dropped — the buffered lines are invisible to readers, the durability regression); appends landing at the file's current end around an interleaved one-shot writer (fails under both mutations); and the run-level pin that six ledger lines cost exactly one open-for-append (fails when record reverts to per-line ledger.append). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
§2 still described the Protocol-conformance point as the module-level `DRIVERS: list[SourceDriver] = […]` assignment, which the previous commit removed — the typed literal lives in build_drivers's return now, constructed on demand. Doc truth only; no code. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The held append handle (3a55d42) lost every write that followed a concurrent ledger rewrite. compact, exclude's drop_items and the migrations rewrite the file via atomic.write_text — a same-dir temp file, then one replace, which swaps the inode under the path. The old per-line reopen re-resolved the path every write, so post-rewrite appends landed in the new file; the held handle kept the replaced inode, sending every later line of the run into an orphaned file no reader sees. At product level: a compact in another terminal during a long enrich run left the report claiming done while the ledger still resolved queued, with the read-back misdiagnosing the loss as a backwards clock. The per-line reopen was load-bearing against atomic rewrites, and removing it removed that protection. The fix keeps the handle and restores the property: each append revalidates that the handle still names the file at the path — os.fstat on the fd against Path.stat on the path, compared on (st_dev, st_ino) — and reopens on mismatch, or when the path is briefly gone. One stat pair per line, still far cheaper than the open/write/close it replaced; the happy path stays a single open per run (the one-open pin still passes). The Appender docstring now names the discovered property so the stat is never optimised away. The corrected durability contract, honestly stated — 3a55d42's body claimed equivalence this writer did not have: open-per-line guaranteed each returned line was with the OS AND written to the file the path named at that moment; the held handle now guarantees both again (flush per line; identity revalidated per line). The window that remains — a replace landing between the check and the write — is the same window open-per-line always had between its open and its write. Neither shape fsyncs, as before. Regression tests, verified by revert (all three fail with the revalidation stripped back to the naive held handle, and failed on the defective code before the fix): the seam repro (append, compact, append twice — every post-compact line durable in the current file and resolved by load); the path briefly missing (the next append recreates the file); and the product-level repro (a compact between two record calls — all three units end done and the report carries no did-not-take-effect note). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
The §16 hygiene items, closing the cleanup round. Observable outcomes proven neutral by a differential review driving both trees — with one real regression the review caught and this branch fixes.
default_drivers()replaces the import-time registry: nothing constructs at import, one build per verb, order pinned (web catch-all last)🤖 Generated with Claude Code