Build the census before opening the file it overwrites - #106
Merged
Merged
Conversation
json.dump(census(), open(DATA + "/prefix_census.json", "w"), indent=1) Two problems in one line. The handle is never closed, so every run emits ResourceWarning — and the obvious fix for that would destroy the census. Arguments evaluate left to right, so `census()` runs to completion before `open()` truncates. Write it the idiomatic way instead, with the `with open` first, and the order reverses: a scan that raises leaves the committed file at zero bytes. census() raises for ordinary reasons — roots.mech_root when a checkout is absent, record_paths when a glob matches nothing — both deliberate fail-closed behaviour. So the current line is safe by accident, and the accident is one refactor away from losing data exactly when something else already went wrong (#102). Building the document first keeps the ordering guarantee explicit rather than incidental, and closes the handle. Verified both directions. A failing scan (`MECHS_ROOT=/nonexistent`) exits with the checkout message and leaves the census byte-identical. A successful scan against a ten-file synthetic corpus writes all nine Mechs, non-empty, valid JSON — run in a temp tree so the committed census was never a participant. Writing to a temp file and renaming would be stronger still, since an interrupted write could not leave a partial census either. Not done here: this removes the trap, and atomic replacement is a larger change to make across the four scripts that write derived data. Co-Authored-By: Claude Opus 5 (1M context) <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.
Closes #102.
Two problems in one line. The handle is never closed, so every run emits
ResourceWarning: unclosed file— and the obvious fix for that would destroy the census.Arguments evaluate left to right, so
census()runs to completion beforeopen()truncates. Write it the idiomatic way —— and the order reverses: a scan that raises leaves the committed file at zero bytes. And
census()raises for ordinary reasons:roots.mech_rootwhen a checkout is absent,record_pathswhen a glob matches nothing, both deliberately fail-closed.So the line is safe by accident, and the accident is one refactor away from losing data precisely when something else has already gone wrong. This was not hypothetical — an adversarial review measured it, truncating the tracked census to 0 bytes with that refactor in place.
Verified both directions
MECHS_ROOT=/nonexistent)The successful run was done in a temp tree, so the committed census was never a participant in the test.
Not done here
Writing to a temp file and renaming would be stronger — an interrupted write could not leave a partial census either. That is a larger change across the four scripts that write derived data, and this PR removes the trap without it.
27 tests pass; the only changed file is
scripts/fleet/prefix_census.py.🤖 Generated with Claude Code