review F: whole-repository review docs/31 (F1-F10)

[RobbinBouwmeester]

Sixth stacked review PR, and the first from a whole-repository review rather than a diff: docs/31_code_review_2026-09-08_full.md, work package F. Base is review/e-followup (#66); merge #61, A, B, C, D, E, then this.

Twelve parallel finders plus a verification sweep produced twenty-one findings at 21b8ac7. Eleven were re-verified against the source before being recorded. Package F closes the six confirmed correctness and security defects and the four regressions this review series introduced; the remaining ten are listed in the document as a second tier.

Confirmed defects

	Change	Test
F1	prescan read the (-inf, +inf) sentinel that rt-im-train writes for "calibration unavailable, search the whole gradient" as "cannot be screened" and dropped the candidate. On the documented FASTA/MS2PIP path that discarded the entire library at exit 0, with a zero-row survivors table and a NaN target/decoy ratio; the single-label bail is gated on surv.len() > 1000 so it never fired. An unbounded window now screens over the whole gradient, a candidate with no window row is treated the same, both are counted and warned about, and screening everything away is an error naming the likely causes.	count and bail
F2	A present-but-wrong-typed is_transferred was swallowed as "no transfers", silently removing every match-between-runs identification from both TSV reports while the parquet still carried them. mbr_worker.py writes it through pandas, so a nullable dtype or int8 reaches the stage. Present means readable now; only absent falls back.	a_malformed_transfer_column_is_an_error_not_a_silent_loss_of_every_transfer, a_table_that_never_saw_mbr_reports_no_transfers_without_complaint
F3	sidecar::resolve_script tried the working directory first, the ordering python::resolve_script_dir was hardened against; that resolver only claims a directory holding mbr_worker.py or deeplc_worker.py, so a directory with any of the other ten workers arrived here relative and ran from the CWD. Absolute as given, then exe dir, then <exe>/scripts, CWD last.	the_shipped_directory_beside_the_binary_wins_over_the_working_directory +2
F4	Loess::predict indexed grid_x[-1] for a NaN query; one library row with a null predicted_irt reaches it, because the parquet reader maps a null f32 to NaN. Returns NaN, and rt-im-train treats a non-finite iRT as "no calibrated RT" and counts it.	a_nan_query_returns_nan_instead_of_indexing_out_of_bounds
F5	Three rescorer backends, two standardisations: in-memory TSV used median/IQR, parquet and streaming used mean/std, so a pool scored differently by rescore.handoff and across the 4 GB threshold. All three use mean/std, leaving the shipped default and every published benchmark unchanged. The fold-key companion is length-checked (a short one left the tail unscored at a fabricated mid-rank score that satisfies the completeness contract), and the backend-size estimate counts features by name.	a_short_fold_key_file_is_refused_rather_than_leaving_rows_unfolded
F6	refuse_output_over_input was wired into 2 of 18 stages, so compete --features f.parquet --out f.parquet replaced the run's widest artifact with the competed subset at exit 0. Now on every output of search-seed, rt-im-train, extract, features, compete, rescore, quant, audit.	writing_the_output_over_the_input_is_refused

Regressions from this review series

	Change	Test
F7 (from #65)	The LOESS boundary extrapolation slope was the pointwise local slope at the sparsest, most one-sided point of the fit: unbounded, free to be negative and invert the iRT-to-RT map, multiplying an unbounded distance. It is the secant of the fitted curve over its end decile, clamped non-negative and to at most 4x the global slope.	boundary_extrapolation_stays_monotone_and_bounded_under_noise, on noisy anchors rather than the noiseless quadratic the original test used
F8 (from #66)	A stop between the reap and the end of publish_exit could tree-kill a recycled pid. The waiter retires the pid the instant wait returns.	a_stop_after_the_reap_has_no_pid_to_kill
F9 (from #66)	The conversion lock spun without pause on an undeletable stale lock, mistook clock skew and a peer's partial file for evidence about its own holder, could be held twice, and leaked every interrupted conversion's partial mzML. Bounded paced take-overs, a deadline, a token read back, future mtime counts as fresh, per-destination partial probe, and a sweep under the lock.	five tests in raw::tests
F10 (from #62)	The pair-key drop removed positional isomers that predicted correctly, bounded only by the library being emptied. Direct misses and collateral are counted separately and exceeding 2% is an error.	a_large_unpredicted_fraction_is_a_failure_not_a_warning, a_positional_isomer_still_shares_its_pair_key_by_design

Two changes deliberately not made

• F10 keeps the pair key position-free. A positional key would stop matching a reverse decoy to its target, because a decoy carries its modifications at mirrored positions, so a target could be dropped while its decoy stayed. That trades a sensitivity defect for an FDR one. The collateral is counted and bounded instead, and a test pins the trade.
• F9 adds no recipe-keyed conversion cache. Unique names, an owned lock and the staleness rules cover the reproduced failure; two recipes aimed at one destination stay a documented limitation of writing beside the input.

Not in this PR

The second tier is recorded in docs/31: unbounded LogBins allocation, the scramble decoy PRNG carrying no peptide dependence, the vacuous NaN-RT smoke assertion, the SBOM's dangling refs, the inert DeepLC import-order test, the /3.0 top-3 features, and five smaller items.

Local checks

Engine cargo fmt --check, clippy -D warnings, cargo test --workspace (280 tests, up from 266); desktop clippy + cargo test --lib (82); pytest tests/python (68 passed, 12 skipped); python -m compileall scripts; ci/gen_config_reference.py regenerated; ci/check_doc_refs.py; ci/check_desktop_ui.py.

🤖 Generated with Claude Code