Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
e0a7947
review A: data integrity (docs/29 findings 1, 2, 4, 9, 11, 17, 18, 21)
RobbinBouwmeester Sep 7, 2026
7587265
docs(io): plain-text reference to the private reject_null in require_…
RobbinBouwmeester Sep 7, 2026
2af9a69
review B: workers (docs/29 findings 3, 6, 7, 8, 12, 20)
RobbinBouwmeester Sep 7, 2026
fdcbf3b
docs: regenerate the config reference after the entrapment worker edit
RobbinBouwmeester Sep 7, 2026
3cd4363
fix(desktop): own the results folder, ask the engine about converters…
RobbinBouwmeester Sep 7, 2026
5eedbb8
fix(calibrate,audit,report,provenance): continuous LOESS boundaries, …
RobbinBouwmeester Sep 7, 2026
91452e9
ci: expect the transfer-basis columns in the experiment-wide TSV headers
RobbinBouwmeester Sep 7, 2026
33dea8c
docs: record the HYE B01 LOESS before/after and the A-D implementatio…
RobbinBouwmeester Sep 7, 2026
d7874f2
docs: record the second LOESS pair (DeepLC re-predicted library) in t…
RobbinBouwmeester Sep 7, 2026
5a8d2d4
fix: follow-up review E (docs/30 R1-R9): fine-tune batch sentinel, po…
RobbinBouwmeester Sep 8, 2026
21b8ac7
docs: record the 2026-09-08 release check (full default HYE experimen…
RobbinBouwmeester Sep 8, 2026
9e4ff51
fix: whole-repository review F (docs/31 F1-F10): prescan sentinel, tr…
RobbinBouwmeester Sep 8, 2026
2853ff2
docs: record the HYE B01 merge check for the LOESS extrapolation chan…
RobbinBouwmeester Sep 8, 2026
63ba928
Merge pull request #67 from CompOmics/review/f-hardening
RobbinBouwmeester Sep 8, 2026
b2d0e50
Merge branch 'main' into review/e-followup
RobbinBouwmeester Sep 8, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,22 @@ updates:
- minor
- patch

# The desktop application is a separate Cargo workspace with its own lockfile
# (desktop/Cargo.lock), which the entry above does not read (docs/29, work
# package C). Tauri and its toolkit crates are updated within their major version.
- package-ecosystem: cargo
directory: /desktop
schedule:
interval: monthly
open-pull-requests-limit: 3
groups:
desktop-minor-and-patch:
patterns:
- "*"
update-types:
- minor
- patch

# Base images, so the digest pins in the Dockerfile stay current instead of
# freezing the image on a base that stops receiving security updates. Pinning
# without an update path is how a reproducible image becomes an unpatched one.
Expand Down
11 changes: 11 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,17 @@ jobs:
- name: Audit
run: cargo audit --deny unsound --deny yanked

# The desktop application has its own lockfile, which the step above does not
# read (docs/29, work package C). One advisory is ignored deliberately:
# RUSTSEC-2024-0429, an unsound iterator in glib 0.18's `VariantStrIter`. Tauri
# 2's Linux toolkit is gtk 0.18, which pins glib 0.18; the fix is glib 0.20; and
# the application never touches a GVariant. Drop the ignore when Tauri moves to
# gtk4 / glib 0.20.
- name: Audit the desktop lockfile
run: >-
cargo audit --file ../../desktop/Cargo.lock --deny unsound --deny yanked
--ignore RUSTSEC-2024-0429

build-test:
name: build + test (${{ matrix.os }})
runs-on: ${{ matrix.os }}
Expand Down
228 changes: 228 additions & 0 deletions CHANGELOG.md

Large diffs are not rendered by default.

35 changes: 21 additions & 14 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,12 @@ Key semantics:
eliminate a target against its decoy. Peptide-level q estimation subsequently
performs picked target-decoy competition through the shared
`base_peptide_id`; keep that pairing intact.
- `retain_top_peaks > 1` currently writes diagnostic peak alternatives only.
Those alternatives do not yet become feature/rescore rows.
- `extract.retain_top_peaks > 1` (default 1) writes the alternative peaks as
additional `psms_extracted` rows with `peak_rank >= 1` (plus a diagnostic
`.peaks.parquet`), `features` carries `peak_rank`, `compete` keys on it, and
`rescore` keeps one row per candidate and records `selected_peak_rank`. The
plumbing exists; what the default still lacks is entrapment validation on two
acquisitions.

## Validated sensitivity workflow

Expand Down Expand Up @@ -203,9 +207,9 @@ The mechanism is peak-group formation rather than scoring: with most peaks gone,
group.

That reading comes from the cap dose-response, not from the audit ladder's own
label. `NO_PEAK_GROUP` cannot be used as evidence for it: `audit.rs` reads a
label. `DID_NOT_SURVIVE_EXTRACTION` cannot be used as evidence for it: `audit.rs` reads a
per-candidate audit table that `extract` does not write (`emit_candidate_audit`
is unwired), so the reason map is always empty and the `_ => NoPeakGroup`
is unwired), so the reason map is always empty and the `_ => DidNotSurviveExtraction`
catch-all absorbs presence failures, matched-fraction failures AND every
extraction-gate rejection alike. Treat the label as "did not survive extract",
and do not decompose it further until the audit table is actually produced.
Expand Down Expand Up @@ -361,14 +365,15 @@ fine-tuning also is not guaranteed deterministic.
single pooled run do not produce identical `q_value` columns. Batch to fit RAM,
and compare per-run counts on `run_psm_q`.
- Pooled rescore scales linearly, measured 0.834 ms/PSM on the streaming
backend. Two feature matrices, two widths: the Python worker's is
`n_psms x n_features x 4` bytes (f32), while the Rust `feats` that `rescore`
builds is `Vec<Vec<f64>>`, so `n_psms x n_features x 8` plus a heap allocation
and 24 bytes of spine per PSM. `native_tda` additionally runs all folds in
backend. Two feature matrices, one width: the Python worker's is
`n_psms x n_features x 4` bytes (f32), and the Rust `FeatureMatrix` that
`rescore` builds (`rescoring.rs`) is flat f32 as well, so the same
`n_psms x n_features x 4`. `native_tda` additionally runs all folds in
parallel, each holding an owned standardised copy of its training slice, so its
peak is roughly `(1 + folds)x` the matrix. The stage logs the figure before it
allocates, and `rescore.max_feature_matrix_gib` turns exceeding a ceiling into
an error at startup rather than an OS kill hours in.
peak is roughly `(1 + folds)x` the matrix. `rescore.max_feature_matrix_gib` is
checked against that layout, from the parquet footers and the selected feature
count, before the allocation (docs/29 #11), so exceeding the ceiling is an error
at startup rather than an OS kill hours in.

### Rescore cost: handoff, feature selection, training-set reduction

Expand Down Expand Up @@ -554,7 +559,8 @@ sensitivity result for it.

Do not enable these by default from a single AIF count:

- model-visible top-K peaks (currently diagnostic sidecar only);
- model-visible top-K peaks (`extract.retain_top_peaks > 1`; implemented through
features, compete and rescore, default 1);
- adaptive RT windows;
- held-out RT window sizing (`rt_im_train.window_holdout_frac`). Implemented and
measured on the AIF benchmark: +1.1% peptides with DeepLC 4.1.0 at unchanged
Expand All @@ -581,8 +587,9 @@ and FDR population: the entrapment pool, HYE and AIF of docs/28 all ran under it
The selected apex was historically correct/strongest only about 48-52% of the
time while the correct peak appeared in the top five about 86-88%. Promoting
top-K alternatives through features/rescore is therefore the best plausible
sensitivity project, but it needs a `candidate_id + peak_rank` contract and
entrapment validation before default activation.
sensitivity project. The `candidate_id + peak_rank` contract exists (`peak_rank`
on every extracted row, `selected_peak_rank` on the scored row, and the MBR worker
joins it); what default activation still needs is the entrapment validation.

## Coding conventions

Expand Down
38 changes: 37 additions & 1 deletion bench/feature_selection/fs_lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import hashlib
import os
import re
import subprocess
import time

os.environ.setdefault("KMP_DUPLICATE_LIB_OK", "TRUE")
Expand All @@ -27,6 +28,38 @@
# train_fdr 0.01) are passed to the worker as MUMDIA_NN_FOLDS / ITERS / TRAIN_FDR
# (rescore.rs), overriding the worker's own docstring default of 5 iterations. The measured
# HYE run trained 10 iterations per fold and never hit the churn stop.
def recipe_metadata(cfg, n_features_used, seed):
"""What a benchmark row was produced with, recorded beside it (docs/29 #20).

A study evaluates a re-implementation of the worker, so the reader needs the code
revision, the fold rule, the feature count, the seed and the training settings to
compare it with what production later ran. Remaining differences from the worker
(its explicit base-peptide fold keys, its standardisation backend) are stated, not
hidden behind "faithful".
"""
here = os.path.dirname(os.path.abspath(__file__))
try:
sha = subprocess.run(
["git", "rev-parse", "--short", "HEAD"], cwd=here, capture_output=True, text=True, check=True
).stdout.strip()
except (OSError, subprocess.CalledProcessError):
sha = "unknown"
c = {**WORKER_DEFAULTS, **(cfg or {})}
training = {k: (list(c[k]) if isinstance(c[k], tuple) else c[k]) for k in (
"iters", "epochs", "hidden", "dropout", "lr", "wd", "batch", "train_fdr",
"neg_ratio", "neg_select", "margin_frac", "train_sub")}
return {
"code_sha": sha,
"folds": c["folds"],
"fold_key": "md5(base sequence, DECOY_ prefix stripped) % folds",
"fold_key_differs_from_worker": "the worker folds on explicit base_peptide_id pairs when the PIN carries them",
"n_features_used": int(n_features_used),
"seed": int(seed),
"preprocessing": "in-memory backend: median/IQR standardisation; worker's streaming backend uses mean/std",
"training": training,
}


WORKER_DEFAULTS = dict(
folds=3,
iters=10,
Expand Down Expand Up @@ -92,8 +125,11 @@ def load_pin(
y = (tb.column("Label").to_numpy() == 1).astype(np.float32)
cids = np.array([int(x.rsplit("_", 1)[-1]) for x in tb.column("SpecId").to_pylist()], np.int64)
peps = [strip_pep(p) for p in tb.column("Peptide").to_pylist()]
pep_hash = np.array([int(hashlib.md5(p.encode()).hexdigest(), 16) for p in peps], dtype=object)
base_seq = np.array([p[6:] if p.startswith("DECOY_") else p for p in peps], dtype=object)
# The fold key hashes the base sequence, so a target and its paired decoy share a
# fold as they do in the worker's explicit pairing. Until 2026-09-07 the hash was
# taken before the `DECOY_` prefix came off, and the pair could split (docs/29 #20).
pep_hash = np.array([int(hashlib.md5(p.encode()).hexdigest(), 16) for p in base_seq], dtype=object)
tb_prot = tb.column("Proteins").to_pylist() if entrapment else None
del tb
n, nf = len(y), len(feat_cols)
Expand Down
3 changes: 2 additions & 1 deletion bench/feature_selection/fs_objective.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ def main():
for sd in seeds:
t0 = time.time()
row, r = fs_lib.bench_subset(name, d, cols, cfg={**cfg, "seed_base": sd})
row.update(dataset=args.tag, seed=sd, rows=int(len(d["y"])), when=time.strftime("%Y-%m-%d %H:%M:%S"))
row.update(dataset=args.tag, seed=sd, rows=int(len(d["y"])), when=time.strftime("%Y-%m-%d %H:%M:%S"),
recipe=json.dumps(fs_lib.recipe_metadata({**cfg, "seed_base": sd}, len(cols), sd), sort_keys=True))
pd.DataFrame([row]).to_csv(out_csv, mode="a", header=not os.path.exists(out_csv), index=False)
if args.save_oof and name == args.save_oof:
np.save(os.path.join(args.out_dir, f"{args.tag}_oof_{name}_seed{sd}.npy"), r["score"].astype(np.float32))
Expand Down
3 changes: 3 additions & 0 deletions bench/feature_selection/fs_train_sweep.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ def main():
rows=int(len(d["y"])),
n_features_used=len(cols) if cols else len(d["feat_cols"]),
cfg=json.dumps(over, sort_keys=True),
recipe=json.dumps(fs_lib.recipe_metadata({**over, "seed_base": sd},
len(cols) if cols else len(d["feat_cols"]), sd),
sort_keys=True),
when=time.strftime("%Y-%m-%d %H:%M:%S"),
)
pd.DataFrame([row]).to_csv(out_csv, mode="a", header=not os.path.exists(out_csv), index=False)
Expand Down
15 changes: 10 additions & 5 deletions ci/check_smoke.py
Original file line number Diff line number Diff line change
Expand Up @@ -425,10 +425,13 @@ def hashes(path):
"run-experiment writes the experiment-wide peptides.tsv and proteins.tsv")
if rep_pep.is_file():
hdr = rep_pep.read_text(encoding="utf-8").splitlines()[0].split("\t")
# The two trailing columns are the acceptance basis of a match-between-runs
# row (docs/29 #19); without MBR they read `false` and empty.
c.ok(hdr == ["precursor", "stripped_sequence", "charge", "protein", "q_value",
"score", "n_runs", "quantity_a", "quantity_b"],
"experiment peptides.tsv has the experiment-wide columns and one quantity "
"column per run", "\t".join(hdr))
"score", "n_runs", "quantity_a", "quantity_b", "is_transferred",
"transfer_q"],
"experiment peptides.tsv has the experiment-wide columns, one quantity "
"column per run and the transfer basis", "\t".join(hdr))
# Rows are asserted on the standalone rewrite at q 0.05 (smoke.sh): the pooled
# peptide-level q of this fixture cannot reach 1 percent, so the root pair is
# legitimately header-only at the default threshold.
Expand All @@ -444,8 +447,10 @@ def hashes(path):
"identical inputs: every precursor is quantified in both runs")
if rep_prot.is_file():
hdr = rep_prot.read_text(encoding="utf-8").splitlines()[0].split("\t")
c.ok(hdr == ["protein_group", "q_value", "n_runs", "lfq_a", "lfq_b"],
"experiment proteins.tsv has one LFQ column per run", "\t".join(hdr))
c.ok(hdr == ["protein_group", "q_value", "n_runs", "lfq_a", "lfq_b",
"is_transferred", "transfer_q"],
"experiment proteins.tsv has one LFQ column per run and the transfer "
"basis", "\t".join(hdr))
for r in ("a", "b"):
c.ok(not (exp / r / "peptides.tsv").exists(),
f"no per-run peptides.tsv under {r}: the grouped q is experiment-wide")
Expand Down
28 changes: 27 additions & 1 deletion ci/check_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class of error that reached the remote.

from __future__ import annotations

import subprocess
import sys
from pathlib import Path

Expand Down Expand Up @@ -61,8 +62,33 @@ def _no_duplicates(loader: StrictLoader, node, deep: bool = False) -> dict:
)


def workflow_files() -> list[Path]:
"""The tracked workflow files; the directory glob only where git cannot answer.

An untracked scratch copy beside the real workflows was checked as if it shipped
(docs/29 #21); the tracked set is what CI runs.
"""
try:
out = subprocess.run(
["git", "ls-files", "-z", "--", str(WORKFLOWS)],
cwd=WORKFLOWS.parents[1],
capture_output=True,
check=True,
).stdout
files = sorted(
WORKFLOWS.parents[1] / rel
for rel in out.decode("utf-8").split("\0")
if rel.endswith((".yml", ".yaml"))
)
if files:
return files
except (OSError, subprocess.CalledProcessError):
pass
return sorted(WORKFLOWS.glob("*.yml")) + sorted(WORKFLOWS.glob("*.yaml"))


def main() -> int:
files = sorted(WORKFLOWS.glob("*.yml")) + sorted(WORKFLOWS.glob("*.yaml"))
files = workflow_files()
if not files:
print(f"no workflows found under {WORKFLOWS}", file=sys.stderr)
return 1
Expand Down
49 changes: 46 additions & 3 deletions ci/gen_config_reference.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import difflib
import json
import re
import subprocess
import sys
from pathlib import Path

Expand Down Expand Up @@ -723,6 +724,35 @@ def reachable_enums(
# ---------------------------------------------------------------------------


def tracked_files(root: Path, suffix: str, fallback, recursive: bool = True) -> list[Path]:
"""Files under `root` with `suffix` that git tracks; `fallback()` when git cannot say.

`git ls-files` is asked for `root` and the answer is filtered, so the input set of
this generator is the tracked source and nothing a developer left beside it.
"""
try:
out = subprocess.run(
["git", "ls-files", "-z", "--", str(root)],
cwd=REPO_ROOT,
capture_output=True,
check=True,
).stdout
except (OSError, subprocess.CalledProcessError):
return fallback()
files = []
for rel in out.decode("utf-8").split("\0"):
if not rel or not rel.endswith(suffix):
continue
path = REPO_ROOT / rel
if not recursive and path.parent != root:
continue
if path.is_file():
files.append(path)
if not files:
return fallback()
return sorted(files)


def parse_profiles(text: str) -> dict[str, list[tuple[str, str]]]:
"""Extract `--profile NAME` overrides from `Config::apply_profile`."""
m = re.search(r"pub fn apply_profile\(.*?\n \}\n", text, re.S)
Expand Down Expand Up @@ -1120,10 +1150,23 @@ def build_document() -> tuple[str, dict[str, object]]:
if not (REPO_ROOT / target).is_file():
sys.exit(f"error: ITEM_STRUCT_DOC['{name}'] points at missing {target}")

rust_files = sorted(
p for p in CRATES_DIR.rglob("*.rs") if "target" not in p.parts
# Tracked sources only. A filesystem glob also picked up untracked scratch copies
# (`*-covr2.rs`, `*-covr2.py`) that sit beside the real files on a developer's
# machine, and their environment-variable reads then entered this document and
# failed `--check` in a workspace that was fine as far as git was concerned
# (docs/29 #21). Outside a git checkout, the release archive for instance, the
# glob is the only option and the archive holds tracked files only.
rust_files = tracked_files(
CRATES_DIR,
".rs",
lambda: sorted(p for p in CRATES_DIR.rglob("*.rs") if "target" not in p.parts),
)
py_files = tracked_files(
SCRIPTS_DIR,
".py",
lambda: sorted(SCRIPTS_DIR.glob("*.py")),
recursive=False,
)
py_files = sorted(SCRIPTS_DIR.glob("*.py"))
rust_reads, rust_sets, env_unresolved = scan_rust_env(rust_files)
py_reads, py_sets = scan_python_env(py_files)

Expand Down
23 changes: 22 additions & 1 deletion desktop/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,22 @@ next run would start in a dirty folder.

Closing the window cancels every running search, for the same reason.

The terminal state is published in one place. `cancel` records the intent and kills
the tree; the thread that reaps the engine reads that intent and publishes `cancelled`,
`done` (the engine finished before the kill landed, so its outputs are complete) or
`failed`. Until then the status stays `running` with `cancel_requested` set and the
interface shows "Stopping". Two writers used to race here, and a stopped run could be
shown as failed with the last log line as its error (docs/29 #14).

## Output ownership

Two engines writing one results folder interleave their artifacts with no error from
either. A run reserves its results folder before the engine is spawned, by canonical
path so that spellings and, on Windows, case name one folder, and releases it when its
end is published; a second Start into an active folder is refused with the owning run
named. The frontend also refuses to start while a Start is in progress or while the run
it follows is still running, because it can show and stop only one (docs/29 #5).

## How progress works

No log parsing. Every engine stage writes `<artifact>.report.json` beside its output,
Expand Down Expand Up @@ -321,7 +337,12 @@ Two things worth knowing:
they get it during the run rather than before it.
- **Preflight blocks a vendor format whose converter is missing**, naming which
converter, rather than letting the engine fail after the interface has switched to
the progress screen.
the progress screen. The converters are asked of the engine with the request's own
configuration (`doctor --json --config`), so one named in `convert.thermo_raw_parser`
or `convert.msconvert` counts, and the rule is the engine's: a Thermo `.raw` with the
parser at `auto` and only msconvert present runs, with a note; a parser the
configuration names and that is missing blocks, as it errors in the engine
(docs/29 #13).
- **Bruker gets an ion-mobility warning** on the Setup screen and under the picker.
MuMDIA's pipeline is 3D, so diaPASEF loses the separation that makes it selective.
Saying so is the difference between a user reading a low count as a MuMDIA result
Expand Down
Loading
Loading