From 326fe60251c00382afc163322f5db74a0e888431 Mon Sep 17 00:00:00 2001 From: j Date: Sun, 28 Jun 2026 18:47:42 +0700 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20pre-wet-lab=20polish=20=E2=80=94=20?= =?UTF-8?q?error=20handling,=20validation,=20CHANGELOG,=20WAVE2=20context?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CRITICAL fix: - cli.py pilot-panel: bare JSONDecodeError on malformed JSONL → structured error with line number and 120-char line preview; returns exit code 1 (was unhandled crash) MEDIUM fixes: - cli.py validate-scoring stdout now includes n_positives, n_negatives, benchmark_type, auprc alongside auroc (previously needed --out file to see these fields) - cli.py synthesis-order: explicit CSV header validation before row iteration; missing candidate_id/sequence columns → structured error with found_columns list - WAVE2_PLAN.md: added SEED-008 Trp mechanism divergence note (disagreement ~0.37 is a mechanistic marker — Boman W=-3.398 vs Wimley-White W=-1.85 kcal/mol — not uncertainty; safe to expand Trp-rich scaffold in Wave 2 if SEED-008 hits) HIGH fix: - CHANGELOG.md Unreleased section updated with PRs #61-66 impact; stale "phase3.yaml: 0.30" (wrong) corrected to 0.40 (unified in PR #61) LOW fix: - presynth_check.py check_sequence(): validates all 20 canonical amino acids before QC; raises ValueError with bad-residue list on non-canonical input Tests added (1191 → 1200): - test_presynth_qc.py::TestCheckSequenceInputValidation (6 tests) - test_cli.py::test_validate_scoring_stdout_includes_n_and_auprc - test_cli.py::test_pilot_panel_malformed_jsonl_returns_error - test_cli.py::test_synthesis_order_missing_columns_returns_error --- CHANGELOG.md | 36 +++++++++++-- docs/WAVE2_PLAN.md | 9 ++++ src/openamp_foundry/cli.py | 33 ++++++++++-- src/openamp_foundry/qc/presynth_check.py | 8 +++ tests/test_cli.py | 66 ++++++++++++++++++++++++ tests/test_presynth_qc.py | 31 +++++++++++ 6 files changed, 173 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10f9b4ff..87c72a76 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,33 @@ All notable changes to OpenAMP Foundry are documented here. **Status:** Pipeline frozen for synthesis batch ordering. All changes below were quality improvements made before committing the ~$10k wet-lab synthesis budget. -### Added +### Latest fixes (PRs #61–67) + +- **PR #67** — JSONL error handling in `pilot-panel` CLI (CRITICAL: bare crash on malformed + line → structured error with line number and preview); synthesis-order CSV header validation; + `validate-scoring` stdout now includes `n_positives`, `n_negatives`, `benchmark_type`, `auprc`; + `check_sequence()` validates canonical AAs and minimum length before QC; WAVE2_PLAN.md SEED-008 + Trp mechanism divergence note; CHANGELOG updated for PRs #61–66 +- **PR #66** — Removed duplicate validation entry REF-GIG-001 (magainin-2 = REF-MAG-001 counted + twice); corrected AUROC 0.8086→0.8047 (pipeline), 0.7890→0.7846 (phase3); config-identity + sentinel tightened from `< 0.83` to `< 0.795`; orphaned DECOY-GIG-001 removed from + scrambled_decoys.csv; default `recall_ks` updated to [10, 20, 43]; 1191 tests passing +- **PR #65** — Trp-weighted aromatic bonus (1.5× Trp vs Phe/Tyr, PR #65); safety `abs()` bug fix + (negative charge_density does not cause hemolysis); Eisenberg scale comment corrected; synthesis + pool regenerated (SEED-006: 11→10, SEED-009: 18→19); AUROC 0.8164→0.8086 +- **PR #64** — Stale docs corrected: max_disagreement 0.30→0.40 for phase3.yaml in WET_LAB_HANDOFF; + reference set "45-sequence" → "72-sequence"; SEED-005 exemplar corrected (KRFFKKIGSALKFA → + seed KRLFKKIGSALKFL with note about VAR_009) +- **PR #63** — Phase3.yaml AUROC regression test added: gate (>0.70), sentinel (≥0.75), config- + identity check (<0.83 then <0.795 in PR #66); `make validate-scoring-phase3` Makefile target +- **PR #62** — Discovery probability updated to 10–18% (6 confirmed scaffold families; honest + corrected estimate replacing prior over-confident numbers) +- **PR #61** — CRITICAL FIX: SEED-008 (puroindoline-a, Trp-rich) reinstated in synthesis pool + after Boman-scale artifact diagnosis; all 10 seeds generated; max_disagreement raised to 0.40 + for both configs to accommodate mechanism-divergent Trp-rich scaffolds; SEED-004 correctly + excluded by safety gate; 6 mechanism-diverse scaffold families confirmed + +### Added (earlier sprint) - `docs/WET_LAB_HANDOFF.md` — score interpretation table, synthesis decision thresholds, per-flag QC remediation guide, MIC/hemolysis assay protocols, troubleshooting section @@ -27,7 +53,7 @@ improvements made before committing the ~$10k wet-lab synthesis budget. - `check_panel()` missing-column error tests for `candidate_id` and `sequence` - `make ci` target: runs `lint` then `test` in one command -### Fixed +### Fixed (earlier sprint) - `ensemble_score()` no longer crashes with KeyError for missing weight keys; emits `UserWarning` listing missing names and defaults to 0.0 (PR #24) @@ -35,13 +61,13 @@ improvements made before committing the ~$10k wet-lab synthesis budget. - `load_candidates_csv()` validates CSV header before iterating rows; raises descriptive `ValueError` instead of bare `KeyError` (PR #25) - Disagreement gate (`max_disagreement`) added to eligibility filter in `pipeline.py` - (PR #25); `pipeline.yaml: 0.40`, `phase3.yaml: 0.30` + (PR #25); `pipeline.yaml: 0.40`, `phase3.yaml: 0.40` (unified in PR #61) - `WET_LAB_HANDOFF.md` μH boundary table corrected to `> 0.55` (matching code) vs - closed interval; phase3 disagreement threshold (0.30) explicitly named in troubleshooting + closed interval; phase3 disagreement threshold correctly set to 0.40 in PR #61 ### Tests -- **814 tests passing** (up from initial ~400 at start of sprint) +- **1191 tests passing** (up from initial ~400 at start of sprint; ~814 after earlier sprint) - All ruff lint checks passing --- diff --git a/docs/WAVE2_PLAN.md b/docs/WAVE2_PLAN.md index 64eec705..77575117 100644 --- a/docs/WAVE2_PLAN.md +++ b/docs/WAVE2_PLAN.md @@ -114,6 +114,15 @@ Expected activity: AMPs from the current scaffold classes are typically more act MRSA than Gram-negative MDR strains. SEED-009 (Bac2A, proline-rich) may have better Gram-negative activity than the membrane-disrupting seeds. +> **Scoring note for SEED-008 (puroindoline-a, Trp-rich):** SEED-008 candidates show model +> disagreement ~0.37 due to mechanistic divergence between the two scorers, NOT prediction +> uncertainty. The Boman scale (W = −3.398 kcal/mol) penalises Trp as highly hydrophobic and +> protein-binding, systematically under-ranking Trp-rich AMPs. The Wimley-White interfacial +> scale (W = −1.85 kcal/mol) correctly models Trp's interfacial membrane-insertion mechanism. +> PR #65 added a 1.5× Trp aromatic bonus to partially compensate. If SEED-008 variants are +> active in Wave 1, Wave 2 should expand the Trp-rich scaffold with confidence — disagreement +> is a mechanistic marker, not a model failure. + #### C4 — Mechanism confirmation (priority 4, for top 1–2 hits only) If budget permits, run one mechanistic assay: diff --git a/src/openamp_foundry/cli.py b/src/openamp_foundry/cli.py index b351160c..2c9eb3b2 100644 --- a/src/openamp_foundry/cli.py +++ b/src/openamp_foundry/cli.py @@ -483,12 +483,21 @@ def _run_pilot_panel(args: argparse.Namespace) -> int: candidates = [] with open(ranked_path, encoding="utf-8") as f: - for line in f: + for line_num, line in enumerate(f, start=1): line = line.strip() - if line: + if not line: + continue + try: row = _json.loads(line) - if row.get("selected"): - candidates.append(row) + except _json.JSONDecodeError as exc: + print(_json.dumps({ + "status": "error", + "message": f"Malformed JSON on line {line_num} of {args.ranked}: {exc}", + "line_preview": line[:120], + })) + return 1 + if row.get("selected"): + candidates.append(row) panel = select_pilot_panel( candidates, @@ -538,8 +547,12 @@ def _run_validate_scoring(args: argparse.Namespace) -> int: write_json(args.out, result) summary = { "status": "ok", + "benchmark_type": result.get("benchmark_type"), + "n_positives": result.get("n_positives"), + "n_negatives": result.get("n_negatives"), "auroc": result["auroc"], "auroc_above_random": result["auroc_above_random"], + "auprc": result.get("auprc"), "recall_at_10": result.get("recall_at_10"), "recall_at_20": result.get("recall_at_20"), "recall_at_43": result.get("recall_at_43"), @@ -697,7 +710,17 @@ def _run_synthesis_order(args: argparse.Namespace) -> int: panel = [] with open(panel_path, newline="", encoding="utf-8") as f: - for row in _csv.DictReader(f): + reader = _csv.DictReader(f) + required_cols = {"candidate_id", "sequence"} + if not required_cols.issubset(set(reader.fieldnames or [])): + missing = required_cols - set(reader.fieldnames or []) + print(json.dumps({ + "status": "error", + "message": f"Panel CSV missing required columns: {sorted(missing)}", + "found_columns": list(reader.fieldnames or []), + })) + return 1 + for row in reader: panel.append(row) mu_h_map = {} diff --git a/src/openamp_foundry/qc/presynth_check.py b/src/openamp_foundry/qc/presynth_check.py index 3a919cb1..7d2046fe 100644 --- a/src/openamp_foundry/qc/presynth_check.py +++ b/src/openamp_foundry/qc/presynth_check.py @@ -187,9 +187,17 @@ def to_dict(self) -> dict: } +_CANONICAL_AAS: frozenset[str] = frozenset("ACDEFGHIKLMNPQRSTVWY") + + def check_sequence(candidate_id: str, seq: str, mu_h: float = 0.0) -> SynthQC: """Run all pre-synthesis QC checks on a single peptide sequence.""" seq = seq.upper().strip() + bad = sorted({aa for aa in seq if aa not in _CANONICAL_AAS}) + if bad: + raise ValueError( + f"{candidate_id}: non-canonical amino acids in sequence: {bad!r}" + ) qc = SynthQC(candidate_id=candidate_id, sequence=seq, length=len(seq)) qc.molecular_weight_da = _molecular_weight(seq) diff --git a/tests/test_cli.py b/tests/test_cli.py index b7073702..d3c8f7e7 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -162,3 +162,69 @@ def test_presynth_qc_command_contains_summary_table(tmp_path): text = out_path.read_text() assert "Summary Table" in text assert "Candidates checked: 2" in text + + +def test_validate_scoring_stdout_includes_n_and_auprc(tmp_path, capsys): + """validate-scoring stdout must include n_positives, n_negatives, benchmark_type, auprc.""" + import pathlib + amp_csv = pathlib.Path("examples/validation/known_amps.csv") + bg_csv = pathlib.Path("examples/validation/random_background.csv") + if not amp_csv.exists() or not bg_csv.exists(): + import pytest + pytest.skip("Validation data not found — run from project root") + out = str(tmp_path / "report.json") + main([ + "validate-scoring", + "--amp-csv", str(amp_csv), + "--decoy-csv", str(bg_csv), + "--config", "configs/pipeline.yaml", + "--benchmark-type", "standard", + "--out", out, + ]) + captured = capsys.readouterr() + data = json.loads(captured.out) + assert "n_positives" in data, "stdout missing n_positives" + assert "n_negatives" in data, "stdout missing n_negatives" + assert "benchmark_type" in data, "stdout missing benchmark_type" + assert "auprc" in data, "stdout missing auprc" + assert data["n_positives"] == 43 + assert data["n_negatives"] == 44 + assert data["benchmark_type"] == "standard" + assert 0.0 < data["auprc"] < 1.0 + + +def test_pilot_panel_malformed_jsonl_returns_error(tmp_path, capsys): + """pilot-panel must return structured error on malformed JSONL, not crash.""" + bad_jsonl = tmp_path / "bad.jsonl" + bad_jsonl.write_text('{"candidate_id": "X", "selected": true}\n{BROKEN JSON LINE\n') + out_csv = str(tmp_path / "panel.csv") + out_md = str(tmp_path / "panel.md") + rc = main([ + "pilot-panel", + "--ranked", str(bad_jsonl), + "--out-csv", out_csv, + "--out-md", out_md, + ]) + assert rc == 1, "Expected non-zero exit code on malformed JSONL" + captured = capsys.readouterr() + data = json.loads(captured.out) + assert data["status"] == "error" + assert "line 2" in data["message"] + assert "line_preview" in data + + +def test_synthesis_order_missing_columns_returns_error(tmp_path, capsys): + """synthesis-order must return structured error when panel CSV lacks required columns.""" + bad_panel = tmp_path / "bad_panel.csv" + bad_panel.write_text("pilot_rank,candidate_id\n1,SEED-003_VAR_001\n") # missing 'sequence' + out_csv = str(tmp_path / "order.csv") + rc = main([ + "synthesis-order", + "--panel-csv", str(bad_panel), + "--out-csv", out_csv, + ]) + assert rc == 1, "Expected non-zero exit code on missing columns" + captured = capsys.readouterr() + data = json.loads(captured.out) + assert data["status"] == "error" + assert "sequence" in data["message"] diff --git a/tests/test_presynth_qc.py b/tests/test_presynth_qc.py index 864fa898..c76e7400 100644 --- a/tests/test_presynth_qc.py +++ b/tests/test_presynth_qc.py @@ -685,3 +685,34 @@ def test_to_dict_contains_trp_fields(self): assert "trp_photolability_risk" in d assert d["tryptophan_count"] == 5 assert d["trp_photolability_risk"] is True + + +class TestCheckSequenceInputValidation: + """check_sequence() validates canonical amino acids before running QC.""" + + def test_canonical_sequence_accepted(self): + qc = check_sequence("ok", "KWKLFKKIGSALKFL") + assert qc.sequence == "KWKLFKKIGSALKFL" + + def test_lowercase_accepted_after_normalisation(self): + # check_sequence uppercases before validation + qc = check_sequence("lower", "kwklfkk") + assert qc.sequence == "KWKLFKK" + + def test_non_canonical_single_raises(self): + with pytest.raises(ValueError, match="non-canonical"): + check_sequence("bad", "KWKLFKKX") + + def test_non_canonical_multiple_raises(self): + with pytest.raises(ValueError, match="non-canonical"): + check_sequence("bad2", "KWKXBZLFKK") + + def test_error_message_names_bad_residues(self): + with pytest.raises(ValueError, match="X"): + check_sequence("bad3", "KWKLFKX") + + def test_all_20_canonical_aas_accepted(self): + # One representative from each canonical AA — all should pass without error + for aa in "ACDEFGHIKLMNPQRSTVWY": + qc = check_sequence(f"test_{aa}", aa * 5) + assert qc.length == 5 From 61b229f84bb71a2d3b60ca9b778a78f1dd2fc6ed Mon Sep 17 00:00:00 2001 From: j Date: Sun, 28 Jun 2026 18:51:45 +0700 Subject: [PATCH 2/2] fix: remove overpromised min-length claim from CHANGELOG; annotate dataset-size assertions --- CHANGELOG.md | 4 ++-- tests/test_cli.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87c72a76..1141560f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,8 +14,8 @@ improvements made before committing the ~$10k wet-lab synthesis budget. - **PR #67** — JSONL error handling in `pilot-panel` CLI (CRITICAL: bare crash on malformed line → structured error with line number and preview); synthesis-order CSV header validation; `validate-scoring` stdout now includes `n_positives`, `n_negatives`, `benchmark_type`, `auprc`; - `check_sequence()` validates canonical AAs and minimum length before QC; WAVE2_PLAN.md SEED-008 - Trp mechanism divergence note; CHANGELOG updated for PRs #61–66 + `check_sequence()` validates canonical amino acids (all 20 standard AAs) before QC; + WAVE2_PLAN.md SEED-008 Trp mechanism divergence note; CHANGELOG updated for PRs #61–66 - **PR #66** — Removed duplicate validation entry REF-GIG-001 (magainin-2 = REF-MAG-001 counted twice); corrected AUROC 0.8086→0.8047 (pipeline), 0.7890→0.7846 (phase3); config-identity sentinel tightened from `< 0.83` to `< 0.795`; orphaned DECOY-GIG-001 removed from diff --git a/tests/test_cli.py b/tests/test_cli.py index d3c8f7e7..b2d66a43 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -187,7 +187,7 @@ def test_validate_scoring_stdout_includes_n_and_auprc(tmp_path, capsys): assert "n_negatives" in data, "stdout missing n_negatives" assert "benchmark_type" in data, "stdout missing benchmark_type" assert "auprc" in data, "stdout missing auprc" - assert data["n_positives"] == 43 + assert data["n_positives"] == 43 # 44 AMPs - 1 duplicate (REF-GIG-001 removed in PR #66) assert data["n_negatives"] == 44 assert data["benchmark_type"] == "standard" assert 0.0 < data["auprc"] < 1.0