Repository navigation
audit #83: decouple the 9 connascence hotspots (_CTX deleted, both mmap paths pinned) - #133
Merged
Merged
Conversation
added 2 commits
October 16, 2026 11:28
…kup, error map, _CTX) Most of the 2026-10-09 audit #83 list landed with the CLI wave (PR #90); this finishes the deltas: - #1 BigTIFF: targeted 9-12-byte value decode test (spec: 8-byte value field, wider values stored at an offset; the pre-#83 pipeline copy read up to 12 bytes inline and zstdmt up to 8 - the unified parser keeps the spec rule). - #2 Magic 512: the oracle probe geometry (width 500 x TILE_PX, partial width so GDAL zero-pads the edge tile - audit #81 F-1) was duplicated in Pipeline._turbo_zstd_ok and the doctor's _doc_zstdmt; now a single _ORACLE_PROBE_W constant + _oracle_probe_tile() builder, with a pin that the width stays a non-multiple of TILE_PX. - #3 Transform names: library path (Pipeline.calibrate refit branch and identity fallback) resolved StopIteration on an unknown name; now a named ValueError via the new calibrate.transform_named() single source, shared with make_transform. - #8 Exit-code contract: _map_pipeline_errors gains the issue's option-A default mapping - an unmapped exception type is a named one-liner and exit 1 (the code was already 1 for uncaught exceptions; the rendering is now contract-stable). - #9 _CTX: the module-global worker state dict is gone; the _WorkerConfig and the turbo DN pair ride in each _process_block task tuple (cfg, dn, bx, by). The DN holder is released after the block pool so the final run-temp unlink still precedes no held mmap (Windows WinError 32, pinned by the os_portability sim).
…udit #83 review) - pipeline.py: the (val_dn, sup_dn) holder tuple rode the task args and was released only by the happy-path dn = None after the block pool. A worker raising (or Ctrl-C) inside the pool left the memmaps live when the finally's _remove_run_temps() unlinked them -> WinError 32 on real Windows, masking the real error. The finally now releases dn too. - tests: exception-path sim (forced _process_block failure) in test_os_portability.py pins the invariant on both paths; mutation- verified (fails with the one-liner reverted). - tests: _doc_zstdmt probe read-back pins the on-disk width to _ORACLE_PROBE_W, so a third probe writer reverting to a literal width is caught; mutation-verified (width=400 -> fail). - tests: two concurrent Pipeline.run() calls with different configs must each match their single-run reference (the _CTX-deletion hazard).
marzukia
approved these changes
Oct 16, 2026
marzukia
added a commit
that referenced
this pull request
Oct 16, 2026
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
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.
Scope — issue #83 (2026-10-09 audit): decouple the 9 connascence hotspots
500width literal + tile builder duplicated inPipeline._turbo_zstd_okand doctor_doc_zstdmt→ single_ORACLE_PROBE_Wconstant +_oracle_probe_tile(), pinned by test (width must stay a non-multiple of TILE_PX)StopIterationon unknowntname→ newcalibrate.transform_named()single source, named ValueError listing the known setwantedcolumn order /_RUN_TEMP_FILES/ saturation→CV / CV edge vs_UNRESOLVED_M— verified already done on main (reviewer spot-checked)_map_pipeline_errors: unmapped exception types render as a named one-liner, exit 1;SystemExit/KeyboardInterruptpass through_CTXmodule global deleted — worker tasks now carry(cfg, dn, bx, by), so two concurrentPipeline.run()calls cannot clobber mid-run state; new two-thread test pins itReview history
Adversarial review (owner lens: connascence, bugs, fragility, DRY) round 1: FAIL — 1 major + 2 minors:
_CTXdeletion re-introduced the WinError 32 it fixed —dnholder released on the happy path only; on a worker exception the outer finally unlinked_val_dn.npy/_sup_dn.npywhile the memmaps were still live (Windows-only, the success-path sim couldn't see it; reviewer's violation probe: base clean, branch['_val_dn.npy', '_sup_dn.npy'])_CTXdeletionFix round (all mutation-verified red/green):
dn = Noneadded to the outer finally (pipeline.py:4555) + new exception-path sim (forced worker raise, same recorded-weakref + unlink probe) — both success AND exception paths now pinned_doc_zstdmt()probe file parsed, width asserted equal to_ORACLE_PROBE_WGates (re-run on final tip f30260… f302460)
uv run pytest -q→ 557 passed, 2 skipped (554 base + 3 new; skips = macOS-only cinterp)bench/check_anchors.py→ PASS, byte-identical (melb10 + melb500, cinterp)ruff check .+ruff format --check .→ cleanNo public API change:
_process_blockstays module-private;transform_namedis additive in the internalppgrid.calibratemodule.Worker:
pi-bg/20261015-233551-2111124(71166d8 + fix f302460). Reviewer:pi-bg/20261016-002959-154547. Verdict:~/.pi-bg-art/review-83-20261015-233551-2111124.md.