Code review · T1, fixture workflows and golden fixtures¶
review-code on autopilot/T1, range 3eab49c..HEAD, 2026-10-27, tick 8. Reviewers: code-reviewer and security-reviewer, independent, in parallel. Format: README.md.
| Round | Head | code-reviewer | security-reviewer |
|---|---|---|---|
| 1 | e2064ff |
changes (1 blocker, 2 majors, 6 minors) | approve (0 blockers, 0 majors, 7 minors) |
| 2 | 7202d9f |
approve (0 blockers, 0 majors, 4 minors) | approve on the fix delta (0 new findings) |
Round-1 fixes are in 7202d9f. code-simplifier ran on 7202d9f and changed nothing: every candidate was used by the tests or saved no lines. Test command:
python3 -m unittest tests/pipelines/synthetic/test_golden.py && python3 tests/pipelines/synthetic/run_scenarios.py --engine snakemake --workdir /tmp/t1 --golden check
CODE-REVIEW verdict=approve blockers=0 majors=0 minors=4 simplify=0
SECURITY-REVIEW verdict=approve blockers=0 majors=0 minors=5
Findings¶
One line each. The reviewers' full evidence is in the session of 2026-10-27, tick 8.
| ID | Severity | Finding | Applied in |
|---|---|---|---|
| CR-01 | blocker → minor | rnaseq and sarek gate stop-and-resume not run and not tracked | 7202d9f: deferral made explicit, inbox #47 (assumed — pending Thomas), pending sentence in dev-env §3 |
| CR-02 | major | --golden check passed silently when no fixtures matched the engine version |
7202d9f (golden_dir_for(): version must be X.Y.Z, missing directory exits 2, missing PR-scenario fixture is an error; unit tests) |
| CR-03 | major | The --until evidence contradicts the owning spec, checking-scenarios §12.3, and dev-env restated the rule |
7202d9f: inbox #48; dev-env §10.1 keeps only the harness fact |
| CR-04 | minor | dev-env §11 still named the 26.10.3 matrix; inbox #42 not updated | 7202d9f |
| CR-05 | minor | Unclosed file when reading scenarios.yaml |
7202d9f |
| CR-06 | minor | Sample filter matched by substring (S1 in S10) |
7202d9f (whole-token match, unit test) |
| CR-07 | minor | smk_rows can attach the last exit code seen to the wrong job when two jobs fail at once |
fixed 2026-10-28, 367c2d4 |
| CR-08 | minor | Two parsers read the same engine outputs with different rules | open: merging them changes the Snakemake exit codes, not a pure simplification |
| CR-09 | simplify | Golden work ran without --golden |
7202d9f |
| CR-10 | minor | dev-env §11 still said "nf-prov 1.8.x with 26.10.3" | review-record commit ("1.8.x with 26.10.x") |
| SEC-T1-04 | minor | Engine version used unchecked as a path segment | 7202d9f (with CR-02) |
| SEC-T1-05 | minor | Golden check could pass without comparing anything | 7202d9f (with CR-02) |
| SEC-T1-01 | minor | ${{ matrix.* }} expansions inside run: in pipelines-synthetic.yml (static values, older than the step) |
fixed 2026-10-28, f1d7dbc |
| SEC-T1-02 | minor | docker.runOptions mount path unquoted; containers run as root |
fixed 2026-10-28, d696568 |
| SEC-T1-03 | minor | python:3.12 digest written twice with no label and no update path |
fixed 2026-10-28, 1dd2995 |
| SEC-T1-06 | minor | Predictable shared workdir /tmp/spectio-scenarios is deleted and reused as the launch directory (older than the step) |
fixed 2026-10-28, review record |
| SEC-T1-07 | minor | uv run --with pyyaml unpinned in the new unit-test step |
fixed 2026-10-28, 0919b77 |