Skip to content

Audit fixes before v0.3.0-alpha.1: told TIFF fallback, ledger, docs, test hygiene, CI hardening - #129

Merged
guijacquemet merged 3 commits into
mainfrom
claude/audit-fixes-pre-alpha
Oct 13, 2026
Merged

guijacquemet merged 3 commits into
mainfrom
claude/audit-fixes-pre-alpha

Conversation

@guijacquemet

@guijacquemet guijacquemet commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Fixes the items of the rules audit (/home/user/AUDIT_rules_tools_template.md) that had to land before the tag v0.3.0-alpha.1. Smallest changes, no refactoring. Two commits: code, docs and tests; then CI hardening. Not merged.

What changes, per item

1. Silent fallbacks

  • convert._memmap_tiff: every "read it the plain way" return goes through _not_mapped(path, reason). For a file of 8 MiB or more (MEMMAP_MIN_BYTES) it writes one warning, TIFF <name> was read into memory, not memory-mapped: <reason>, to the log file (log.warning) and to the worker's stderr (LabConstrictor worker: ..., like the worker's other notes). Reasons: compressed, tiled, multi-series (n series), big-endian byte order, truncated, not contiguous, pyramid levels, OME/other series kind, pixel type, LC_TIFF_MEMMAP=0, or a failed mapping (exception type and text). Files under 8 MiB, and mapped files, say nothing. The refusal conditions are the same as before, only split so each has its reason (no behaviour change for what is mapped).
  • docs/OPERATIONS.md "Fallbacks (intended)": rows for the TIFF fallback and for Worker._orphan_exit.
  • worker._orphan_exit: kept ignore_errors=True (a failing remove must not stop the worker leaving), but a folder that still exists is logged with log.warning to the log file only (stderr has no reader once the host is gone; _complain there could raise on a closed pipe and stop the exit).
  • Rechecked the 17 handlers of the audit: convert.py:181 (OverflowError, refused two lines later), the two ProcessLookupError, registry.py:417 (race, commented, entry reported elsewhere), retention OSError returns (end in the report skipped or the documented lock exit code), cli FileExistsError, prune guard, diagnostics, exporter, protocol, conformance: all are told elsewhere or are correct control flow and are left as they are. Only the two above were silent without a reason.

2. Ledger and HOST_FEATURES

3. Install from the tag. README.md (the main.zip block removed, the tag block kept), OPERATIONS.md Install (Tools by tag; the Napari plugin has no tag and stays refs/heads/main, labelled as development version), LABCONSTRICTOR_INTEGRATION.md line 15, HUMAN_TEST_PROTOCOL.md line 207 (the URL only; the napari[pyqt5] text there is untouched, as asked).

4. OPERATIONS contradictions

  • Line about temp folders: hosts never prune, they write the markers and call the Tools CLI.
  • features.py docstring: states the exception (a flag added later for already shipped behaviour is allowed when it is the only way to tell versions apart; shm_order_v1).
  • Tags: lightweight tags created by the GitHub release are the standard; procedure steps 3 and 4 merged ("create the release with the new tag name on the merge commit"); "Never move or recreate a tag" kept.
  • Windows: registry trust checks (owner, group/other-writable, interpreter inside the prefix, world-writable folders) are POSIX only; "Windows is unchecked" and why.
  • Playground rule written in TESTING.md ("The Playground rule", with where it is checked) and pointed to from OPERATIONS.md.

5. Test hygiene. tests/README.md no longer hard-codes a count (and names the permanent skips); the dead try/except ImportError and skipIf(retention is None) in test_conformance.py removed (retention is imported directly, the cli/runs fallback branch too); test_regressions_run1_gc now asserts gc.garbage == []; TESTING.md lists smoke_migration. Also in TESTING.md: the sabotage claim now says "in the table" and names what is covered, and the known-failures row says the list is empty.

6. CI hardening (second commit). The only third-party action is conda-incubator/setup-miniconda (smoke_install, smoke_migration): pinned to fc2d68f6413eb2d87b895e92f8584b5b94a10167 # v3.3.0 (SHA of the tag v3.3.0, equal to the old v3 tag, resolved with git ls-remote and fetched to confirm the commit exists). tj-actions/changed-files is not used in this repository (it is in the template). GitHub's own actions/* stay on major tags; TESTING.md says so. release_sandbox: new inputs fiji_ref and qupath_ref (default main, passed to git clone --branch through env; the default branch of both repositories is main, so behaviour is unchanged); napari_ref description says what it is. TESTING.md: smoke workflows are dispatch-only (no schedule, no PR trigger; smoke also on pushes to claude/smoke-bridge) and a table of which ones default to branch names.

Tests changed

  • Added tests/test_input_loading.py::MappedTiff: test_a_file_that_could_not_be_mapped_says_why_in_the_log_and_on_stderr (compressed, tiled, multi-series, big-endian, not contiguous, truncated; one line, file name, reason, same text on stderr), test_the_environment_switch_is_told_for_a_big_file, test_nothing_is_said_for_a_small_file_or_a_mapped_one.
  • Added tests/test_worker_orphan_exit.py (2 tests).
  • Added 3 rows to tests/test_sabotage.py (log line removed, small-file gate removed, orphan log removed) and updated the existing row "pages of a TIFF stack are not checked for gaps" because the line it breaks moved. Sabotage-checked: each new row's break makes its slice fail (run with LC_SABOTAGE_ONLY); while writing the compressed-file test it also caught a wrong reason ("no contiguous pixel data" for a compressed file), fixed before the commit.
  • Changed tests/test_conformance.py (guard removed, nothing else), tests/test_regressions_run1_gc.py (assertion added). No test was skipped, loosened or deleted.

Not verified

CI

ci on the PR head and a full-matrix workflow_dispatch run (6 test jobs, lint, compat) are green: https://github.com/CellMigrationLab/LabConstrictor-Tools/actions/runs/38425033947

🤖 Generated with Claude Code

https://claude.ai/code/session_01Y2SN8SuEakTdiWy3F2WUqN

…er, docs, test hygiene

- convert._memmap_tiff: a TIFF of 8 MiB or more that is read the plain way logs one warning (file, reason) to the log and the worker's stderr
- worker._orphan_exit: a temp folder that cannot be removed is logged
- ledger rows for #124 and #125, real PR numbers instead of "this PR", pending section replaced by a note
- docs: tag install URLs, OPERATIONS contradictions, HOST_FEATURES rows, Playground rule, features rule exception
- tests: new tests and sabotage rows, dead retention guard removed, gc test asserts, test count not hard-coded

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y2SN8SuEakTdiWy3F2WUqN
…s Fiji and QuPath refs as inputs

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y2SN8SuEakTdiWy3F2WUqN
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y2SN8SuEakTdiWy3F2WUqN
@guijacquemet
guijacquemet marked this pull request as ready for review October 13, 2026 16:06
@guijacquemet
guijacquemet merged commit 7c09317 into main Oct 13, 2026
14 checks passed
@guijacquemet
guijacquemet deleted the claude/audit-fixes-pre-alpha branch October 14, 2026 16:08
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.

2 participants