P3-15′-d: the ADR-0006 D3 criterion-(b) relaxed-architecture backend - #115
Merged
Merged
Conversation
…tive consensus Signed 2026-08-07 (bioedca, CLAUDE.md §7 item 2). D3 named "R2DT 2.0 / cmalign"; both fail, so A4 pins criterion (b)'s element localization to the A3 per-candidate mlocarna `#=GC SS_cons` comparative consensus instead. Why, measured rather than argued: - R2DT is unreachable here. api.anaconda.org returns "could not be found" for bioconda/r2dt, conda-forge/r2dt AND bioconda/traveler, while bioconda/infernal resolves on the same call (the probe is powered). ADR-0002 D1 records no container runtime on the cluster. - R2DT would add no independent structure anyway: its Rfam path IS cmalign against Rfam CMs (PMID:39921562, PMID:34108470); Traveler adds layout. With no template hit it "will not attempt to draw a structure". - RF00230 is class-I scoped (PMID:32882008); the cmalign route also needs D8's alignability cutoff, which D17 freezes at P6 with no value. Pins the instrument and its anti-circularity constraints only — never RF00230, never an Rfam/class-I template, never the model's own output. No algorithm is fabricated (§10.3); D3's freeze-on-held-out-canonical + unit-test obligation is unchanged. D6's short-Stem-I threshold ships keyword-required with no default, supplied per round; D17's P6 freeze is NOT consumed. The relaxation only ever spares more, so an imperfect value errs fail-closed. Disclosed cost: (a) and (b) now share the homolog-MSA supply, so ADR-0005 D14's "three independent model-independent disjuncts" no longer holds for two of them. Recorded, not absorbed. Not amended: D3's predicate logic + both relaxations, D6/D7/D8 values, D17, D9 row 5, D5/D8's specifier path, A1/A2/A3. No ADR-0002 amendment is required (envs/locarna.yml + envs/rscape.yml already pinned and locked). R2DT as a §17 figure deliverable is untouched. Validation: doc-only; no code, no value pinned, no DVC object, no SLURM job. No test parses ADR-0006 (only ADR-0005 is read, by test_aux_ablation_check). Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
P3-15'-d. Implements criterion (b) off the A3/A4 de-novo comparative consensus,
completing the last missing disjunct of the ADR-0005 D14 mining spare rule.
Inputs
data/interim/homolog_msa/certified_positive.sto (real job-766 mlocarna consensus)
data/processed/master_clean_v0.parquet (curated Structure/discriminator)
data/processed/splits/split_assignments.parquet (ADR-0004 held-out carve)
Outputs
src/tbox_finder/mining/architecture.py criterion_b verbatim per D3, plus the
de-novo localizer: Stockholm parse (DELEGATED to msa_shuffle.read_pfam_alignment,
not forked), WUSS/dot-bracket pair table, maximal-helix + flanked-bulge
decomposition, and the NCCA-pairing test.
src/tbox_finder/mining/architecture_producer.py per-candidate run-shard/merge status
table, D3's held-out-canonical freeze, derive_relaxed_arch_supply_available.
reports/p3/architecture_freeze.json the D3 measurement (pins nothing).
slurm/p3/architecture_producer.sbatch the CPU-only (b) leg.
slurm/p2/mine_round_producer.sbatch PROMOTES each msa.sto out of
$BUILD/work before the EXIT trap destroys it (A4 cross-reference impact).
mine_round.RELAXED_ARCH_SUPPLY_AVAILABLE + --relaxed-arch-status + paired refusals;
remine's two-way --relaxed-arch-available/--no- pair + its exit-4 preflight.
Two judgements, disclosed not absorbed
1. D3's relaxations are NOT symmetric. Read as a plain requirement-drop they fire
together on every translational locus, so criterion_b would be unconditionally True
across the class-II corpus -- and by D9 row 5 that makes Tier-2N unreachable.
criterion_b keeps the ADR's literal text (so the P6 freeze tests D3, not this fix);
the localizer resolves "required only where detectable" three-valued, and
architecture_status refuses `passed` on no evidence, routing to unavailable/spared.
2. The NCCA motif is DERIVED, not transcribed (10.3): computed from the tRNA 3' end
N73-C74-C75-A76 by antiparallel Watson-Crick pairing, giving UGGN. Four agreeing
peer-reviewed sources back the fact (PMID:12547201; DOI:10.1016/j.bbagrm.2008.09.001;
PMID:15879350; DOI:10.1016/j.bpj.2009.12.2831); the repo's own curated corpus agrees
at 99.79% over 23,208 records.
Validation
freeze over 8,715 held-out canonical records (joined by ingest.record_hash, not
positionally): the run containing the curated discriminator is 7 nt in 7,983 of 8,605
(92.8%) and carries the derived UGGN motif in 99.87% at 4 nt of pairing.
unit+golden 5,032 passed; failure set identical to a matched main baseline apart from
three parquet-only tests this local env cannot run (CI installs pyarrow==24.0.0; their
logic was executed directly in tbox-finder-data and all three hold).
19 sabotages / 19 RED against their NAMED test. ruff + black clean; snakemake --lint OK.
AS BUILT: sizing bulges in ALIGNMENT COLUMNS returned zero bulges on the real consensus --
a 16-column run is 8 residues in the candidate's own row. Sizing is now in degapped
residues of the candidate's own sequence. A gapless hand-built fixture would not have
caught it.
n_mined is NOT moved off 0 by this: per A4, (b) reads the same consensus (a) does, so its
supply delegates to derive_msa_supply_available and every candidate reports `unavailable`
=> spared until the full-corpus (a)/(b) producer array has run. relaxed_architecture also
stays out of TIER2N_PROTECTIVE_DISJUNCTS (D9 row 5), pinned by a test.
Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a relaxed-architecture backend for Stockholm consensus evaluation. The change includes distributed status production, held-out freeze measurements, MSA promotion, Slurm execution, and mining and re-mining supply integration. ChangesRelaxed architecture backend
Estimated code review effort: 5 (Critical) | ~90+ minutes Sequence Diagram(s)sequenceDiagram
participant mine_round_producer
participant architecture_producer
participant architecture_slurm
participant mine_round
mine_round_producer->>mine_round_producer: clear stale and promote current msa.sto files
architecture_slurm->>architecture_producer: run shard with explicit thresholds
architecture_producer-->>architecture_slurm: write shard status table and marker
architecture_slurm->>architecture_producer: merge complete shard tables
architecture_producer-->>architecture_slurm: write merged architecture status table
mine_round->>architecture_producer: load candidate status table
architecture_producer-->>mine_round: return per-candidate architecture status
mine_round->>mine_round: apply status to candidate evidence and spare-rule checks
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
… merge Two majors from the CodeRabbit CLI on eb0cd44, both real. 1. slurm/p2/mine_round_producer.sbatch (major, Data Integrity): the promotion loop copied only on success, so a candidate that produced an msa.sto on an EARLIER run but is not producible on this one kept the stale alignment in $ROUND_DIR/msa. The (b) producer would then score that stale consensus instead of resolving `unavailable` => spared. Each candidate's promoted msa.sto is now cleared FIRST, whether or not this run produced one. The loop iterates this shard's own workroot, so the cleared set is exactly the candidates this task owns -- no cross-shard race. 2. slurm/p3/architecture_producer.sbatch (major, Stability): two tasks could pass the marker-completeness check concurrently and both merge, and `_write` uses Path.write_text, which is not an atomic replace -- a second writer can interleave and leave a truncated table that still parses. Now uses the same BLOCKING flock + MERGE_OK idiom as slurm/p3/stage2_producer.sbatch, and reads the merged artifact back through the CONSUMER (load_status_map) rather than the writer. tests/unit/test_sbatch_rm_targets.py 41 passed (up from 40 -- the new `rm -f` target is parsed and accepted, not an unresolvable command-substitution widening to `*`). ruff + black clean. Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
tests/unit/test_architecture.py (1)
540-554: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe AST scan misses parameter names and keyword-argument names.
The walk collects
ast.Name,ast.Attribute,ast.Constant, and import names. It does not collectast.argorast.keyword. A parameter namedcmalign_bitscore, or a call such asf(rf00230_path=...), would pass the guard. Add both node types so the scan covers the whole executable surface.♻️ Proposed change
elif isinstance(node, ast.Attribute): tokens.append(node.attr) + elif isinstance(node, ast.arg): + tokens.append(node.arg) + elif isinstance(node, ast.keyword) and node.arg: + tokens.append(node.arg) elif isinstance(node, (ast.Import, ast.ImportFrom)):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_architecture.py` around lines 540 - 554, Extend the AST token collection in the architecture scan to include parameter names from ast.arg and keyword-argument names from ast.keyword, alongside the existing ast.Name and ast.Attribute handling. Ensure both names are added to haystack so forbidden identifiers such as cmalign_bitscore and rf00230_path are detected by the existing assertions.tests/unit/test_architecture_producer.py (1)
163-207: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd merge tests for the two silent-loss cases.
TestMergecovers duplicates, config mismatch, and the empty path list. It does not cover a shard whoserowslist is empty, or a shard with noconfigblock. Both merge cleanly today. See the comment onsrc/tbox_finder/mining/architecture_producer.pylines 292-317. Add the two cases together with the refusals, so the guard cannot regress.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_architecture_producer.py` around lines 163 - 207, Extend TestMerge with refusal tests for a shard containing an empty rows list and for a shard missing its config block. Construct each malformed shard directly, call architecture_producer.merge_status_tables, and assert ProducerError with the appropriate existing validation message, ensuring both silent-loss cases are guarded.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@slurm/p2/mine_round_producer.sbatch`:
- Around line 94-103: Update the MSA publication flow around the loop that
copies `msa.sto` into `MSA_OUT` so each producer attempt starts from a clean,
run-scoped authoritative set. Remove or replace any existing `$MSA_OUT` contents
before iterating, ensuring candidates that fail to produce an MSA cannot retain
artifacts from earlier attempts while preserving the current promotion logic.
In `@slurm/p3/architecture_producer.sbatch`:
- Around line 55-59: The documented sbatch submission command must provide all
six rule parameters required by the script’s variables at lines 97-102. Update
the submit command’s --export value to include each required parameter alongside
ROUND_DIR, or replace it with a checked wrapper that validates and passes them
before invoking sbatch.
- Around line 169-190: Guard the merge path after confirming all shard markers
with an atomic mkdir lock at $MARKER_DIR/.merge.lock. If acquiring the lock
fails, exit successfully as a non-merger; only the task that creates the lock
may validate status files and run the architecture_producer merge writing
MERGED_OUT.
In `@src/tbox_finder/mining/architecture_producer.py`:
- Around line 482-490: Update the ncca_recovery measurement loop around
ncca_bulge_status so its bulge_size_range targets only the curated containing
bulge rather than all flanked bulges in the record; retain the existing
ncca_hits counting behavior, then regenerate reports/p3/architecture_freeze.json
with the corrected per-bulge results.
- Around line 292-317: Update the shard-table merge logic around the payload
loop to reject tables with a missing or empty rows collection and reject tables
without a config mapping containing the required rule keys, rather than
defaulting either value to empty data. Validate these conditions before
appending to rows or configs, preserve the existing duplicate candidate and
cross-config checks, and align the validation with the sibling synteny merge
behavior.
In `@src/tbox_finder/mining/architecture.py`:
- Around line 190-192: Update the wobble check in the NCCA bulge matching logic
to compare the bulge base with the recovered acceptor base, using
WATSON_CRICK[motif_base] before consulting WOBBLE_PAIRS; apply the same
correction to the additional occurrence. Update
test_wobble_is_off_unless_asked_for to use a real G·U case, such as bulge G with
motif base A, while preserving the default allow_wobble=False behavior.
In `@src/tbox_finder/mining/remine.py`:
- Around line 802-807: Update the remine argument setup around _supply_flag_pair
to add the matching relaxed-arch-status input for the relaxed-arch-available
declaration. In remine’s apply-spare-rule flow, validate the
declaration/status-table pair consistently with mine_round.apply_spare_rule,
load the table via architecture_producer.load_status_map, and pass the resulting
relaxed_arch_status map to candidate_evidence and read_fp_manifest.
---
Nitpick comments:
In `@tests/unit/test_architecture_producer.py`:
- Around line 163-207: Extend TestMerge with refusal tests for a shard
containing an empty rows list and for a shard missing its config block.
Construct each malformed shard directly, call
architecture_producer.merge_status_tables, and assert ProducerError with the
appropriate existing validation message, ensuring both silent-loss cases are
guarded.
In `@tests/unit/test_architecture.py`:
- Around line 540-554: Extend the AST token collection in the architecture scan
to include parameter names from ast.arg and keyword-argument names from
ast.keyword, alongside the existing ast.Name and ast.Attribute handling. Ensure
both names are added to haystack so forbidden identifiers such as
cmalign_bitscore and rf00230_path are detected by the existing assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3cbe0527-51ad-4678-929a-26d9d75a4ebd
⛔ Files ignored due to path filters (2)
analyses/phase3_log.qmdis excluded by!**/*.qmddocs/decisions/ADR-0006-validation-decision-rule-and-tiering.mdis excluded by!**/*.md,!docs/decisions/**
📒 Files selected for processing (11)
reports/p3/architecture_freeze.jsonslurm/p2/mine_round_producer.sbatchslurm/p3/architecture_producer.sbatchsrc/tbox_finder/mining/architecture.pysrc/tbox_finder/mining/architecture_producer.pysrc/tbox_finder/mining/mine_round.pysrc/tbox_finder/mining/remine.pysrc/tbox_finder/mining/synteny_producer.pytests/unit/test_architecture.pytests/unit/test_architecture_producer.pytests/unit/test_synteny_producer.py
…nrunnable Third major from the GitHub app on eb0cd44, and the only one of its three not already closed by 18307fe. The documented submit command exported only ROUND_DIR, but the script `${VAR:?}`- requires six rule parameters (STEM_I_NT_THRESHOLD, MIN_NAMED_HELICES, MIN_HELIX_PAIRS, BULGE_MIN_NT, BULGE_MAX_NT, NCCA_PAIRING_NT). A submission copied from the header would have exited before producing a single shard. All six are now in the documented --export, flagged as placeholders rather than pins, and pointed at reports/p3/architecture_freeze.json for the choice. Also records that `sbatch --test-only` validates the SLURM HEADERS ONLY and does not run the body, so it cannot catch a missing rule parameter -- the §9.3 preflight now names `bash -n` beside it. The app's other two majors were already fixed in 18307fe (it marks the stale-MSA one "Addressed in commit 18307fe" itself). ⚠ Its suggested fix for the merge race -- a non-blocking `mkdir` lock -- is NOT taken: that is the exact interleaving slurm/p3/stage2_producer.sbatch documents as leaving NOBODY merging (task A holds the lock and counts 1 marker; B writes its marker, fails the lock, exits; A finds its count short and exits too). The blocking flock already in 18307fe is the repo's own precedent and does not have that hole. Finding right, suggested fix wrong. Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
…status table Major, and the most consequential finding of the step. `mine_round`'s apply-spare-rule gained `--relaxed-arch-status` in eb0cd44; `remine`'s — the leg the P3 round actually invokes — did not. A P3 round could therefore declare relaxed_arch_available=True, pass its exit-4 preflight, and still read `unavailable` for every candidate: a clean-looking zero yield indistinguishable from an honest one, which is exactly the state the paired refusals exist to make unrepresentable. [[fixed-one-of-two-identical-things]] once more: two apply paths, one wired. Threaded through the whole chain rather than at the CLI alone: remine_candidate_evidence(..., relaxed_arch_status=) delegates to the P2 builder read_remine_manifest(..., relaxed_arch_status=) delegates to read_fp_manifest _load_relaxed_arch_status() beside _load_synteny_status, same broad except-list (a truncated table raises JSONDecodeError, a non-object root AttributeError, a malformed row KeyError/TypeError -- all input faults that would otherwise bypass the documented refusal path) run_remine_round(..., relaxed_arch_status_table=) + BOTH paired refusals apply-spare-rule --relaxed-arch-status Adds TestTheP3RoundCarriesBToo (8 tests) -- the end-to-end coverage the review asked for, each naming a specific link in the chain rather than only the end state. Sabotage-verified: reverting the remine wiring turns exactly the new end-to-end tests RED (3 of 8), and the restore is clean. unit+golden 5,040 passed; failure set unchanged vs the matched main baseline apart from the three parquet-only tests CI covers. ruff + black clean. Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
…ble test my own test had locked in ⚠ I acted on only 1 of r2's 14 findings at first, having read the CLI output while it was still streaming and seen only its tail. Re-read in full; the rest are addressed here or recorded for the next round. CORRECTNESS (all major unless noted): 1. architecture._pairs_with — THE WOBBLE ARM WAS INVERTED, and my own test locked it in ([[tests-can-specify-the-bug]]). `motif_base` is ALREADY the Watson-Crick complement of the acceptor base, so comparing the candidate base against it is not a pairing question. A G·U wobble is between the bulge base and the ACCEPTOR base. Measured consequence on the default NCCA/UGGN: allow_wobble=True admitted only NON-pairing bases and could never admit a real wobble, since UGGN carries no A or C motif position for (G,U)/(U,G) to match. Now compares against WATSON_CRICK[motif_base]. test_wobble_is_off_unless_asked_for asserted the inverted rule; rewritten to the real interface, plus a direction-only unit pin independent of bulge geometry. 2. architecture.degapped_span — deleted IUPAC ambiguity codes as if they were gaps. That understated the residue count the bulge-size test reads AND made residues either side of an ambiguity code adjacent in the scanned window, so a contiguous motif match could be reported where none exists. A FALSE POSITIVE — the one direction this module must not get wrong, since it does not err toward sparing. Only GAP_SYMBOLS are dropped now; ambiguity codes survive as opaque residues and _pairs_with already rejects them. 3. architecture.ncca_bulge_status — an all-wildcard register matched every base, so ncca_pairing_nt=1 (reachable: no default, the round supplies it) reported BULGE_DETECTED on every admissibly-sized bulge regardless of sequence. Unconstrained registers are dropped; a motif with no constrained position at all now refuses. 4. mine_round._cmd_apply_spare_rule — validated only the MSA declaration, so it accepted --relaxed-arch-available on a checkout that cannot evidence the (b) supply. The fail-OPEN direction, and the one `remine` already refuses: two apply paths must agree about what "declared" costs or the weaker one is the way around the gate. Now exits 4, with a positive control that proves the gate was passed rather than merely not hit. 5. NOT taken, with the reason: the duplicate-row-name guard. The finding assumed `names` and `sequences` are built independently; they are not — `names` derives FROM the delegate's output, which already accumulates by name, so n_sequences == len(sequences) by construction and cannot overstate the A2 depth floor. A guard there would be unreachable code asserting an invariant the construction provides. Replaced with the invariant stated in a comment and pinned by test_n_sequences_cannot_exceed_the_rows_actually_present. unit+golden 5,050 passed; failure set unchanged vs the matched main baseline apart from the three parquet-only tests CI covers. ruff + black clean. STILL OPEN from r2 (recorded in TODO.md, not yet addressed): atomic msa.sto promotion; per-candidate rather than per-shard read-error handling; bulge-range validation at ArchitectureRunConfig construction; shard-table row-shape validation before merge; a shared MSA-filename constant; ncca_recovery scoping in the freeze; --allow-wobble never supplied by the sbatch; an unrecognized bulge_state degrading to failed; report value (not only key) mismatches. Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
Closes r2. Final round tally was 15 findings (10 major, 5 minor), not the 14 I reported: six were fixed in cf1cfc7, one declined with reasons there, and these nine are the rest. MAJOR 1. architecture_producer.freeze — ncca_recovery scanned EVERY flanked bulge while the denominator counted only records whose discriminator-bearing bulge was located, so an unrelated bulge could satisfy the motif and inflate the numerator. `ncca_bulge_status` gains `target=`; the freeze now scores the located bulge alone. RE-DERIVED: only k=3 moved (99.930% -> 99.884%, 4 records). The headline k=4 99.87% and the modal 7 nt are UNCHANGED, so the number I flagged as unverified in the last summary holds. 2. Atomic promotion in mine_round_producer.sbatch — `cp` wrote straight to the path (b) reads, so a task killed mid-copy left a partial file. A truncation that drops trailing rows but KEEPS #=GC SS_cons parses as a valid alignment of reduced depth and would be scored as real evidence against the A2 floor. Now copies to a temp name in the destination dir and `mv`s (atomic within a fs). 3. ArchitectureRunConfig.__post_init__ — an inverted bulge range is empty, so no bulge satisfies it, every candidate resolves to `failed` => MINED, and the round reports a sweep it never evaluated. Reachable from the CLI precisely because nothing is defaulted. Refuses now, as do non-positive counts. 4. merge_status_tables — three separate shape faults in tables written by other processes: a missing config coerced to {} makes all shards compare EQUAL and publishes an empty config into provenance; a non-Mapping row raises AttributeError, which `main` does not catch, so the merge leg exits with a traceback instead of FATAL/exit 1; a row missing candidate_id passes the duplicate check as "" and a SECOND such row reports "appears in more than one shard table", naming the wrong fault. All three named explicitly. 5. MSA_FILENAME now DEFINED in covariation_producer (which writes the file) and IMPORTED by architecture_producer. My comment had claimed this already; it did not. A forked literal would let a rename make every (b) candidate silently `unavailable` while the shard tables still merged cleanly. MINOR 6. Localization.__post_init__ validates bulge_state against BULGE_STATES — architecture_status tests two states by equality, so a typo or a state round-tripped through a serialized payload fell through to a DECIDED NEGATIVE (failed => minable) rather than a refusal. 7. slurm/p3: ALLOW_WOBBLE is now a required export. Without it the round's G-U policy was decided by a code default, the outcome the producer docstring says must not happen. --min-sequences stays omitted: it is the A2 Pin 2 pin. 8. validate_status_payload reports CONFLICTING VALUES, not only key-set diffs. On same-key/different-value drift both lists were empty and the operator read "(declared-only [], rows-only [])" -- naming no fault at all. 9. evaluate_candidate's except widened as asked -- but SABOTAGE SHOWED IT REDUNDANT: `parse_stockholm` already converts OSError/UnicodeError/ValueError to ArchitectureError, so narrowing it back stayed GREEN. The test therefore moved to the line that actually carries the guarantee (TestReadErrorsBecomeArchitectureErrors); sabotaging THAT turns 3 of 4 red. The wider clause is kept as defence for a future caller that reaches `localize` without going through `parse_stockholm`, and says so. Validation: unit+golden 5,074 passed, failure set unchanged vs the matched main baseline apart from the three parquet-only tests CI covers. 10/11 new sabotages RED against their NAMED test; the 11th is finding 9's leak, resolved above by moving the test rather than by claiming the count. ruff + black clean; snakemake --lint OK. Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
3 findings (2 major, 1 minor), down from r2's 15. All real. 1. MAJOR — `producer_entry_points_present` WAS VACUOUS. It imported the module that was already executing, so neither the import nor the hasattr could ever fail: the clause handed `all(clauses.values())` a hardcoded True. That is the exact failure mode this function's own docstring warns about, and the parametrized clause test never caught it because it iterates SUPPLY_CLAUSES and this clause was not in that tuple — a clause outside the tuple is a clause nothing breaks. Compare mine_round.derive_msa_supply_available, whose `producer_present` probes a DIFFERENT module, which is what makes it breakable. Replaced, not merely deleted, by the clause that derivation uses for the same job and which IS breakable: `producer_status_wired` measures that the round actually STAMPS a produced (b) status onto the candidate, by calling the wiring rather than reading a signature. A producer that ships but whose output the round drops is the silent no-op the gate exists to refuse. PRODUCER_ENTRY_POINTS removed; a test pins that the vacuous clause is not re-added. 2. MAJOR — validate_status_payload did not guard non-Mapping rows, though merge_status_tables does. `.get` on a string row raises AttributeError, which `main` does not catch, so the leg exits with a traceback instead of FATAL/exit 1. load_status_map reads tables written by other processes: row shape is an input, not an invariant. Same finding as r2's merge guard, one call site over -- "you fixed one of two identical things", fourth time this step. 3. MINOR — `n_heldout_canonical_measured` re-tested `isinstance(n_measured, int)`, which accepts True, while the clause excluded bool. A report carrying `true` made the clause read False while the field reported `true`: two fields disagreeing inside one payload. Reuses the clause's verdict now. Validation: 3/3 new sabotages RED against their NAMED test; the full campaign re-run on this head is 19/19 + 10/11 (the documented leak whose test was relocated) + 3/3. unit+golden 5,080 passed, failure set unchanged vs the matched main baseline apart from the three parquet-only tests CI covers. ruff + black clean. Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/unit/test_architecture_producer.py (1)
869-886: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
MSA_FILENAMEinstead of the hardcoded"msa.sto"literal.
test_the_msa_filename_is_imported_from_the_producer_that_writes_iton Lines 893-898 exists to prevent a second filename literal. These two tests then hardcode"msa.sto". If the producer renames the file, these tests keep constructing the old path and both candidates readunavailablefor the wrong reason.♻️ Proposed change
- target = tmp_path / candidate_slug("c1") / "msa.sto" + target = tmp_path / candidate_slug("c1") / architecture_producer.MSA_FILENAME- bad = tmp_path / candidate_slug("bad") / "msa.sto" + bad = tmp_path / candidate_slug("bad") / architecture_producer.MSA_FILENAME🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_architecture_producer.py` around lines 869 - 886, Replace the hardcoded "msa.sto" paths in the affected tests with the imported MSA_FILENAME constant from the producer that writes the file. Update both target and bad path construction while preserving the existing unreadable-file scenarios.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/test_architecture.py`:
- Around line 104-112: Update the docstring of
test_duplicate_row_names_refuse_rather_than_collapsing to state that
read_pfam_alignment accumulates duplicate names into one concatenated row and
that names therefore contains one entry; remove the incorrect claim that names
retains both rows or that n_sequences is overstated. Leave the test logic and
inline comment unchanged.
---
Nitpick comments:
In `@tests/unit/test_architecture_producer.py`:
- Around line 869-886: Replace the hardcoded "msa.sto" paths in the affected
tests with the imported MSA_FILENAME constant from the producer that writes the
file. Update both target and bad path construction while preserving the existing
unreadable-file scenarios.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4cb5a61e-66b9-4df6-876a-76fc8a0b9545
📒 Files selected for processing (9)
reports/p3/architecture_freeze.jsonslurm/p2/mine_round_producer.sbatchslurm/p3/architecture_producer.sbatchsrc/tbox_finder/mining/architecture.pysrc/tbox_finder/mining/architecture_producer.pysrc/tbox_finder/mining/covariation_producer.pysrc/tbox_finder/mining/mine_round.pytests/unit/test_architecture.pytests/unit/test_architecture_producer.py
🚧 Files skipped from review as they are similar to previous changes (5)
- reports/p3/architecture_freeze.json
- slurm/p2/mine_round_producer.sbatch
- slurm/p3/architecture_producer.sbatch
- src/tbox_finder/mining/architecture_producer.py
- src/tbox_finder/mining/architecture.py
… were vacuous 4 findings, ALL MINOR — no majors for the first time this step (15 -> 3 -> 4, but the r4 set is entirely non-critical). All real. 1. The freeze's PROVENANCE was unvalidated. It is evidence, so how it was produced is part of the claim, and `build_provenance` permits an empty `inputs` and a null env lock — the committed report had BOTH. Two fixes: `_provenance` now passes `env_lock=ENV_LOCK` (envs/data.conda-lock.yml; stage2_producer sets one, synteny_producer does not — followed the better precedent), and a new `freeze_provenance_names_its_inputs` clause. ⚠ The clause counts EITHER repo-relative `inputs` OR sha256'd `extra.external_inputs`: the corpus is DVC-tracked and absent from a worktree, so the freeze reads it by absolute path and the hashed form is what gets recorded — and that form is strictly MORE traceable. What must not pass is a report naming neither. Freeze re-derived; the measured numbers are unchanged. 2. `derive-supply` could exit with a traceback. The `candidate_evidence` probe is now wrapped fail-closed. ⚠ The test for that guard then failed from a DIFFERENT place: the delegation to `derive_msa_supply_available`, whose own probe is unwrapped, so a broken round took this function down THROUGH the delegation even with the local probe guarded. One of two call sites again — fifth time this step. Both are wrapped now. 3. MY OWN TEST WAS VACUOUS. `test_the_recovery_shares_reconcile_against_their_own _counts` iterates `ncca_recovery`; on an empty mapping the body never runs and the test passes having checked nothing — the exact species this file names elsewhere, in this file. Now asserts the arm set and the denominator are non-empty first. Sabotage-verified by emptying the DATA while KEEPING the guard (my first attempt removed the guard too, which is why it read as a leak — a malformed sabotage proves nothing). 4. A test name promised an exit code its body never exercised (`..._is_refused_with_exit_4` only checked the predicate). Renamed to name the predicate, with a pointer to the test that does assert 4; a failure would otherwise send an operator to the wrong gate. ⚠ AND ONE MORE OF MINE, CAUGHT BY SABOTAGE RATHER THAN BY REVIEW: `test_the_committed_freeze_names_its_env_lock` read the COMMITTED report, so dropping `env_lock=ENV_LOCK` from the writer left it GREEN — the bytes on disk were written by the fixed code. An assertion over an artifact cannot see the producer that wrote it. Split into a producer-side test that CALLS `_provenance` (red under sabotage) plus the cheap artifact-side check. Validation: 5/5 r4 sabotages RED against their NAMED test after both corrections. unit+golden 5,085 passed; failure set unchanged vs the matched main baseline apart from the three parquet-only tests CI covers. ruff + black clean. Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
The app reviewed eb0cd44 with SEVEN line comments; I had read only three (truncated output, the same mistake as with the r2 CLI buffer). Reading all seven: four converge with CLI findings already fixed (wobble direction, ncca_recovery scoping, remine --relaxed-arch-status, config-less merge). Two gaps were NOT covered by the CLI, which is exactly why both paths run: 1. MAJOR — a shard table with ZERO ROWS merged cleanly. This is the invisible loss: a dropped or truncated shard contributes nothing, every candidate it should have carried is simply absent from the merged table, and `mine_round` reads each absent candidate as `unavailable` => SPARED. Every count still reconciles against every other count, so nothing in the report looks wrong -- the round quietly decided less than it claims. Refused now. 2. MAJOR (extension of the r2 config fix) -- a config that is non-empty but INCOMPLETE still passed. REQUIRED_CONFIG_KEYS now names the six rule parameters explicitly (listed, not derived from whatever keys a payload carries: a required-key set read from its own evidence is vacuously satisfied exactly when the evidence is missing). Each key is broken ALONE by a parametrized test. 3. MINOR (on 0f39ced) -- a stale docstring of mine claimed `names` keeps both rows while the dict merges. It does not: `read_pfam_alignment` accumulates by name, so duplicates CONCATENATE. The inline comment two lines below already said so, and the docstring contradicted it. Corrected to describe what the test actually pins. Validation: 2/2 new sabotages RED against their NAMED test. unit+golden green; failure set unchanged vs the matched main baseline apart from the three parquet-only tests CI covers. ruff + black clean. Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
…put" 2 findings, both minor. Same underlying principle as r2's merge guards: an on-disk artifact written by another process has a shape that is an INPUT, not an invariant. 1. `len()` on a non-sized `provenance["inputs"]` raised TypeError out of a function documented as fail-closed on every clause. `main` does not catch TypeError, so `derive-supply` would exit with a traceback instead of the FATAL / exit 1 contract every other leg follows. The existing malformed-report test covered only the "not JSON" case. Type-guarded now. 2. `stem1_length` was type-checked with `isinstance(s1, (int, float))`. On an integer-dtype column `itertuples` yields `np.int64`, which is NOT a Python `int`, so every value would be silently DROPPED and the extent range would read as if the column were empty. Uses `numbers.Real` with bool excluded (bool IS a Real, which is exactly why it must be excluded explicitly). ⚠ LATENT, not active: the current corpus carries NaN so the column is float64, which is why the committed report has 8,578 values. Freeze re-derived and verified BYTE-IDENTICAL apart from provenance -- k=4 = 99.87%, modal bulge 7 nt, stem_i n = 8,578 all unchanged. Validation: 1/1 new sabotage RED against its NAMED test. unit+golden green; failure set unchanged vs the matched main baseline apart from the three parquet-only tests CI covers. ruff + black clean. Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
Owner
Author
|
| Filename | Overview |
|---|---|
| src/tbox_finder/mining/architecture.py | Adds validated Stockholm parsing, consensus decomposition, motif localization, and three-valued architecture-status evaluation. |
| src/tbox_finder/mining/architecture_producer.py | Adds candidate evaluation, shard production, strict table merging and validation, freeze reporting, and supply derivation. |
| src/tbox_finder/mining/mine_round.py | Integrates relaxed-architecture evidence and fail-closed availability checks into the mining round. |
| src/tbox_finder/mining/remine.py | Adds paired availability controls and relaxed-architecture status handling to re-mining. |
| slurm/p2/mine_round_producer.sbatch | Atomically promotes per-candidate consensus alignments while clearing stale outputs. |
| slurm/p3/architecture_producer.sbatch | Adds parameter-required architecture shard execution and a serialized, validated merge workflow. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Candidate and homolog search] --> B[mlocarna Stockholm alignment]
B --> C[Promoted msa.sto]
C --> D[Architecture shard evaluation]
D --> E[Validated merged status table]
E --> F[Mining and re-mining evidence]
F --> G{Relaxed architecture status}
G -->|passed| H[Spare candidate]
G -->|unavailable| H
G -->|failed| I[Continue remaining mining-rule evaluation]
Reviews (1): Last reviewed commit: "fix(mining): CodeRabbit CLI r5 — two min..." | Re-trigger Greptile
Records the review history, the two truncated-buffer misreads that made me report a round as handled when it was not, the CLI/app disagreement that justifies running both paths, the four vacuous-or-wrong tests of my own (two caught by sabotage rather than review), and the confirmation that the published freeze numbers survived re-derivation. Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
4 findings (1 major, 3 minor), all acted on. Plus a correction to a claim I
propagated from r5 without checking it.
⚠ CORRECTION — r5's "Accept NumPy integer stem1_length values" WAS WRONG, and I
repeated its reasoning in a code comment and a commit message. The claim was that
`itertuples` yields np.int64 on an integer-dtype column, so `isinstance(s1, (int,
float))` silently dropped every value. MEASURED on this pandas: `itertuples` yields
a plain Python `int` for BOTH mixed- and homogeneous-dtype frames. The narrower
check was never dropping anything and the committed report was never wrong. The
`numbers.Real` change is KEPT — it is strictly more permissive, costs nothing, and
the numpy-scalar behaviour is genuinely version-dependent — but the comment now says
what is true rather than what the review asserted.
r6 findings:
1. MAJOR — the stem-I test exercised np.int64/Real semantics in ISOLATION, not the
`freeze_report` loop. Now builds an integer-dtype parquet fixture and calls
`freeze_report`, asserting n/min/max and that the record was genuinely measured.
2. MINOR — the discriminator needle was normalised `.upper()` but not T->U, while
`degapped_span` maps T->U on the haystack. A DNA-spelled discriminator would fail
the substring test, the record would be skipped, and the freeze's DENOMINATOR
would shrink with nothing raising. (The review said the asymmetry was in CASE;
it was in ALPHABET -- the helper already upper-cases. Right finding, wrong axis.)
3. MINOR — `supply_declaration_unevidenced(derivation=None)` had no test, so
replacing `(derivation or {})` with `derivation.get(...)` would raise
AttributeError with nothing turning red.
4. MINOR — `derive-supply` REPORTS, it does not gate; an honest `available: false`
must still exit 0. Now pinned.
⚠ TWO OF MY r6 TESTS WERE THEMSELVES ISOLATION-ONLY and stayed GREEN under sabotage
of the code they claimed to cover -- the same anti-pattern this whole step is about,
caught by sabotage rather than by review. Both now go through `freeze_report`;
sabotaging the needle drops the carve from 1 to 0, verified by execution.
Validation: 3/3 CLI-runnable sabotages RED; the two parquet-path sabotages verified
by direct execution in tbox-finder-data (stem-I n 1->1 exposed the false premise
above; needle 1->0 confirms the real one). unit+golden 5,098 passed; failure set
unchanged vs the matched main baseline apart from the three parquet-only tests CI
covers. ruff + black clean.
Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
2 findings. One real and important, one whose premise does not hold here. 1. MAJOR — the (b) merge checked each shard's internal consistency and refused an empty or duplicated shard, but NOTHING compared the merged result against the corpus it was meant to cover. A partial or substituted candidate set merged cleanly, every absent candidate classified `unavailable` => SPARED, and the round decided less than it reported while every count still reconciled. This is the corpus-level counterpart of r6's empty-shard refusal, and it is what stage2_producer's --min-coverage does for the (a) leg. `assert_covers_manifest` requires EXACT equality, not coverage >= a fraction: an EXTRA id is as wrong as a missing one, since it means the merge consumed a table from a different manifest. Wired as `merge --manifest`, made REQUIRED in slurm/p3/architecture_producer.sbatch's merge leg (before MERGE_OK), and recorded in the merged table's `coverage` block and provenance. A test pins that the sbatch actually passes it -- a guard the production leg does not invoke is worthless. 2. NOT TAKEN, premise checked and false — "CI excludes data/ through sparse checkout, so both (a)-evidence tests fail". CI uses a plain actions/checkout@v5 with no sparse checkout; all three evidence files are git-tracked and present; and CI pytest is GREEN on this branch, which includes the test asserting the (a) supply IS evidenced. Recorded rather than silently skipped. Validation: 4/4 new sabotages RED against their NAMED test, including one that strips --manifest from the sbatch. unit+golden green; failure set unchanged vs the matched main baseline apart from the three parquet-only tests CI covers. ruff + black clean. Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
…isagree on an id
5 findings (1 major, 4 minor), all real.
1. MAJOR — `evaluate_candidate` accepted `candidate_id` OR `id`; `assert_covers_manifest`
read `candidate_id` only. A manifest carrying just `id` produced status rows fine and
then raised KeyError('candidate_id') at the coverage check, which `main` catches and
reports as `FATAL: 'candidate_id'` -- naming no fault at all. One reader
(`candidate_id_of`) now serves both, so they cannot drift.
2. MINOR — `stem_i_nt_threshold` was not validated positive. A non-positive value
silently DISABLES the D6 short-Stem-I relaxation for every non-translational
candidate (no extent is < 0), so candidates that depend on it resolve `failed` =>
MINED. Fail-open; now refused with the other counts.
3. MINOR — the supply probe called `candidate_evidence` positionally. A parameter
reorder would mis-bind, the broad `except` would swallow the TypeError, and the
clause would fail closed for a reason unrelated to the wiring it measures. Bound by
keyword now.
4. MINOR — the freeze conflated TWO different skips in one shrinking denominator. They
mean different things: no usable curated discriminator is a CURATION gap; a
discriminator in no flanked bulge is a LOCALIZER miss. Counted separately now, and
the decomposition is informative: of 8,715 held-out records, 92 curation gaps + 18
localizer misses + 8,605 measured (they reconcile exactly). So on records that HAVE a
usable discriminator the localizer finds its bulge in 8,605/8,623 = 99.79%.
5. MINOR — my r7 sbatch guard did `split(marker)[1][:200]`, which raises IndexError when
the leg is absent (reporting the wrong fault) and fails for the wrong reason if the
invocation runs past 200 chars. Asserts the marker first and slices to the next
command boundary.
Freeze re-derived: k=4 = 99.87%, modal bulge 7 nt, ncca_recovery unchanged; only the new
carve counters are added.
Validation: 4/4 CLI-runnable sabotages RED against their NAMED test (incl. renaming the
sbatch merge marker); the parquet-path skip-counter test verified by direct execution in
tbox-finder-data (1 counted / 1 curation gap / 1 localizer miss). unit+golden 5,108
passed; failure set unchanged vs the matched main baseline apart from the three
parquet-only tests CI covers. ruff + black clean.
Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
…MINABLE
4 findings (1 major, 3 minor), all real.
1. MAJOR — `ncca_bulge_status` refused a bulge_size_range whose MAXIMUM could not hold
the motif, but not one whose MINIMUM could not. A bulge of low..want-1 residues
PASSES the size filter, is therefore "considered", cannot possibly hold the motif,
and so reads BULGE_ABSENT -- a decided negative => MINABLE -- instead of
BULGE_UNDETECTABLE => spared. Fail-open, and reachable because nothing is defaulted.
Guarding `low` subsumes the old check since `high >= low` is already enforced.
2. MINOR — `validate_status_payload`'s `rederived` is a dict comprehension, so two rows
with the SAME candidate_id collapse to the last; if they carry the same status,
`rederived` still equals `declared` and the table passes. `merge_status_tables`
refuses duplicates but `load_status_map` reads tables written by other processes and
reaches this function without that guard. ("A duplicate key merges instead of
colliding" -- the same species as the contig-id case already in memory.)
3. MINOR — the module docstring pointed at `named_elements_present`, which is a
parameter and a dataclass field but not a function. The function is
`named_elements_status`.
4. MINOR — my own r8 sbatch guard sliced to the first BLANK LINE, which is not a shell
command boundary: a `--manifest` belonging to a FOLLOWING command would satisfy both
assertions, so a merge leg that dropped the manifest could still read green. Now
consumes the marker line plus its backslash continuations only, with a test that
builds exactly that adversarial case.
Validation: 3/3 CLI-runnable sabotages RED against their NAMED test. unit+golden 5,114
passed; failure set unchanged vs the matched main baseline apart from the three
parquet-only tests CI covers. ruff + black clean.
Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
… number 4 findings (1 major, 3 minor), all real. 1. MAJOR — `min_sequences` was the ONE outcome-deciding number in the predicate module that was not range-checked (`min_pairs`, `min_named_helices`, `ncca_pairing_nt` and `bulge_size_range` all were). A floor below 1 makes `n_sequences < min_sequences` always false, silently DISABLING the ADR-0006 A2 Pin 2 depth check, so a candidate reaches criterion_b on an alignment of any depth -- fail-open. Refused now, and the applied floor is recorded on every arm of the returned detail so provenance shows WHICH floor was used rather than merely that one was. 2. MINOR — `freeze` did not expose `--max-ncca-pairing-nt`, so the report always covered arms 1..4 while `run-shard` accepts any `--ncca-pairing-nt`. A round with a wider motif would not have its actual arm measured. Exposed, defaulting to the acceptor motif's width rather than a literal 4. 3. MINOR — three tests imported pandas directly while their siblings used `importorskip`, so they ERRORED rather than skipped without a parquet engine. ⚠ This also clears the three long-standing local "regressions": the branch's failure set is now IDENTICAL to the matched main baseline, with the parquet tests skipping cleanly. 4. MINOR — two tests encoded conflicting contracts: one asserted the real checkout always evidences the supply, the other only that the derivation and the constant agree. A deliberate "unavailable on this machine" declaration would have failed the first for a reason that is not a defect. The first is now gated on the constant. ⚠ AND ONE MORE OF MY TESTS WAS TOO WEAK, caught by sabotage: the arm-width test checked that the parser EXPOSES the option, so hardcoding the `main` call to 4 stayed green. It now drives the CLI end to end and reads the written report — verified by execution: sabotaged it emits arms 1..4 where 1..2 were requested. Validation: 3/3 CLI-runnable sabotages RED plus the CLI-path one verified by execution. unit+golden 5,119 passed with ZERO regressions vs the matched main baseline. ruff + black clean. Claude-Session: https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
This was referenced Aug 8, 2026
Merged
Merged
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
2 participants
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 the last missing disjunct of the ADR-0005 D14 mining spare rule. Both §7 route gates were answered 2026-08-07 and ADR-0006 Amendment A4 is signed and carried in
695e874— no new ADR here.What lands
mining/architecture.pycriterion_bverbatim per D3, plus the de-novo localizer: Stockholm parse (delegated to the existingmsa_shuffle.read_pfam_alignment, not forked), WUSS/dot-bracket pair table, maximal-helix + flanked-bulge decomposition, NCCA-pairing testmining/architecture_producer.pyrun-shard/mergestatus table, D3's held-out-canonicalfreeze,derive_relaxed_arch_supply_availablereports/p3/architecture_freeze.jsonslurm/p3/architecture_producer.sbatchslurm/p2/mine_round_producer.sbatchmsa.stoout of$BUILD/workbefore the EXIT trap destroys it (A4 cross-reference impact)mine_round/remineRELAXED_ARCH_SUPPLY_AVAILABLE,--relaxed-arch-status+ paired refusals, the two-way--relaxed-arch-available/--no-…pair, exit-4 unevidenced preflightThe D3 freeze — measured, not fabricated
On the 8,715 held-out canonical records (
source=='corpus' ∧ nested_role=='heldout', joined byingest.record_hash— not positionally; the join is asserted to bind):discriminatoris 7 nt in 7,983 of 8,605 (92.8 %) — the literature's 7-nt antiterminator bulge, recovered independently from structure;UGGNacceptor motif is present in 99.87 % at 4 nt of pairing (99.93 % at 3 nt).Every rule parameter ships keyword-required-with-no-default; the round supplies it and it rides in provenance. D17's P6 freeze is not consumed.
Two judgements, disclosed rather than absorbed
translational, and a translational locus is class II), socriterion_bwould be unconditionallyTrueacross the class-II corpus — and by D9 row 5 ((c)✓ ∧ (a)✓ ∧ (b)✗) that makes Tier-2N unreachable: the flagship class defined out of existence by its own predicate.criterion_bkeeps the ADR's literal text so the P6 freeze tests D3 rather than my fix; the localizer resolves "required only where detectable" three-valued (absent= detectable-and-missing, a real failure;undetectable= nothing to judge, the only state the relaxation excuses), andarchitecture_statusrefusespassedwhen both relaxations are active with neither element observed →unavailable⇒ spared.acceptor_pairing_motifcomputes it from the tRNA 3′ terminusN73-C74-C75-A76by antiparallel Watson–Crick pairing →UGGN. §10.1 gate cleared with four agreeing peer-reviewed sources [PMID:12547201; DOI:10.1016/j.bbagrm.2008.09.001; PMID:15879350; DOI:10.1016/j.bpj.2009.12.2831]; the repo's own curated corpus agrees at 99.79 % over 23,208 records.AS BUILT — the defect a hand-built fixture would have missed
The first bulge finder sized runs in alignment columns, and returned zero bulges on the real certified consensus: a 16-column run is only 8 residues in the candidate's own row, a 37-column run 11. Sizing now happens in the degapped residues of the candidate's own sequence. A gapless fixture passes either way — smoke-testing on the real job-766 artifact is what exposed it.
Validation
mainbaseline apart from three parquet-only tests this local env cannot run (CI installspyarrow==24.0.0; their logic was executed directly intbox-finder-dataand all three hold).snakemake --lintOK._refuse_unevidencedindexedderivations[key], so adding the (b) entry KeyError'd a sibling test. It now.gets and fails closed with the gap named.⚠ This does not move
n_minedoff 0, and says soPer A4, (b) reads the same consensus (a) does, so its supply derivation delegates to
derive_msa_supply_availablerather than probing twice. Until the full-corpus (a)/(b) producer array has run, everymsa.stois absent and the producer reportsunavailable⇒ spared — fail-closed and honest.relaxed_architecturealso stays out ofTIER2N_PROTECTIVE_DISJUNCTS, so a (b)-only round is still refused at readiness; a test pins that building the backend did not change that.https://claude.ai/code/session_017VRtJL7TgZioPxpCUuENmM
Summary by CodeRabbit
New Features
Bug Fixes
Tests