Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
47 changes: 47 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,53 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),

## [Unreleased]

### Fixed
- **The SESTRAV-VERIFY GNN evaluation harness was completely allele-blind whenever
invoked from anywhere but the repo root.** `StructuralPeptideMHCDataset.__init__`
(`src/verify/structural_gnn.py`) loaded its 34-residue MHC pocket-sequence table via
`Path("src/verify/mhc_pseudo_sequences.json")` - CWD-relative. A CWD other than the repo
root (an installed `sestrav` package invoked from anywhere else, verified against a real
built-and-installed wheel run from an unrelated directory) made that path silently fail
to resolve, `pseudo_seqs` default to `{}`, and every HLA allele fall through to an
identical `"A"*34` all-alanine placeholder. `generate_canonical_groove_coords` never uses
its own `allele_name` argument either, so this table was the ONLY source of allele signal
anywhere in the graph - two peptide-allele pairs sharing a peptide produced byte-identical
MHC pocket node features and edge attributes regardless of true allele identity, with no
error, only a per-allele `logger.warning`.
**Scope: `src/verify/structural_gnn.py` is the GNN evaluation/verification harness
(`sestrav_evaluator.py`'s "SESTRAV-VERIFY" report, gating `promote_gnn.py`'s scorecard).
It is the ONLY production importer of `StructuralPeptideMHCDataset` in the repository.
The v2.3 production GNN path (`src/gnn/`, `GraphPredictorV2`, `GINEConv`) is a separate
module tree with zero allele references and is NOT affected** - GNN promotion remains
deferred (GPU-gated) and no certified result in the results ledger passes through this
code path.
Fix: the table is now resolved via a module-level `Path(__file__).resolve().parent /
"mhc_pseudo_sequences.json"`, independent of CWD, and an absent or content-empty table
(only the tracked file's own `_source` documentation key, or truly empty) now raises
`FileNotFoundError`/`ValueError` at construction instead of silently defaulting - the
existence check alone would not have caught a present-but-gutted file falling through the
same placeholder path.
A second, independent defect closed in the same change: the wheel built from this
repository shipped **zero non-`.py` data files** (verified: 75 entries, none outside
`.dist-info/` metadata) - `mhc_pseudo_sequences.json` was never packaged, so even a
CWD-correct invocation of an installed package would still have hit the missing-file
path. Added `[tool.setuptools.package-data]` (`"src.verify" = ["*.json"]`); a wheel
rebuilt after this change carries both tracked JSON files, verified by unzipping it.
A third, adjacent defect: `sestrav_evaluator.py`'s `evaluate_single_virus` already caught
a `StructuralPeptideMHCDataset` construction failure and fell back to mock predictions,
but did so silently - a per-virus exception left `run_evaluation_pipeline`'s top-level
`metadata["use_mock_fallback"]` at `False` (computed once before the per-virus loop),
so a report could read `use_mock_fallback: false` while some virus's scores were
actually mock. Added a `used_mock_fallback` flag threaded through both `except` arms
(main-cohort and breakout-mutant) into each virus's own result, and the pipeline now
recomputes its top-level flag as an OR across every virus after the loop completes.
Ten new or rewritten tests - seven added and three rewritten, across
`test_structural_gnn.py` and `test_sestrav_evaluator_gnn.py`, with one obsolete test
removed (`test_dataset_no_pseudo_seqs_file`, which pinned the silent-empty-table
behaviour this change deliberately converts into a raise);
every guard and every fallback-flag site was individually mutation-tested by reverting
it in isolation and confirming its dedicated test fails, then restoring it and confirming
the suite is green again. `ruff check`/`ruff format`/`mypy` clean on all touched files.
### Security
- **A1: the release workflow now attaches its SLSA build-provenance attestation as a
release asset, closing the reason OpenSSF Scorecard's Signed-Releases check scores 0.**
Expand Down
20 changes: 20 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,26 @@ sestrav = "src.cli:main"
where = ["."]
include = ["sestrav*", "src*", "functions*"]

[tool.setuptools.package-data]
# The two tracked non-.py files under the packaged source trees. Until this
# declaration, a built wheel carried zero data files - `src.verify`'s own
# mhc_pseudo_sequences.json was silently absent from every installed
# package (verified: a wheel built from `git archive HEAD` has 75 entries and
# no non-.py members outside its own `.dist-info/` metadata directory).
#
# The mechanism is NOT that setuptools defaults to excluding package data -
# it does not. Resolved against this very file (setuptools 83.0.0,
# `apply_configuration(Distribution(), "pyproject.toml")`), `include_package_data`
# comes back **True**; only a bare `Distribution()` with no pyproject applied
# reports `None`. Under pyproject.toml that flag is on by default.
#
# What it is on by default for is files the **sdist manifest** covers, and this
# project has neither a `MANIFEST.in` nor `setuptools-scm` (`[build-system]
# .requires` is setuptools + wheel only), so the manifest covers nothing and
# `include_package_data=True` has nothing to act on. Declaring the files here
# is what actually ships them.
"src.verify" = ["*.json"]

# ---------------------------------------------------------------------------
# Coding standards (OpenSSF Silver: coding_standards / coding_standards_enforced)
# ---------------------------------------------------------------------------
Expand Down
30 changes: 30 additions & 0 deletions src/verify/sestrav_evaluator.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,24 @@ def evaluate_single_virus(
logger.warning(f"Empty dataset for {virus_name}. Skipping.")
return {}

# Tracks an UNPLANNED fallback specifically - i.e. a real-GNN attempt that
# raised - as distinct from use_mock/model-is-None, which the caller
# already knows about and reflects in report["metadata"]. Without this,
# a per-virus exception fell back to mock scores while the report's
# top-level use_mock_fallback stayed False, mislabelling the mock scores
# as real ones.
used_mock_fallback = False

# 1. Main cohort prediction
if use_mock or not HAS_PYG or model is None:
logger.info(f"[{virus_name}] Using mock/fallback GNN predictor.")
scores = run_mock_predictions(df)
if not HAS_PYG and model is not None:
# A caller passed a real model while PyG is unavailable - the
# top-level "model is None" check a pipeline caller reflects in
# its own metadata would miss this; mark it here so it is never
# silently invisible to a direct (non-pipeline) caller either.
used_mock_fallback = True
else:
try:
dataset = StructuralPeptideMHCDataset(df)
Expand All @@ -201,6 +215,7 @@ def evaluate_single_virus(
f"Failed GNN evaluation for {virus_name}: {e}. Falling back to mock predictions."
)
scores = run_mock_predictions(df)
used_mock_fallback = True

y_true = df["label"].values
roc_auc = calculate_roc_auc(y_true, scores)
Expand Down Expand Up @@ -230,6 +245,8 @@ def evaluate_single_virus(
wt_scores = run_mock_predictions(df_pos)
anchor_scores = run_mock_predictions(df_anchor, is_mutated=True, mutation_type="anchor")
tcr_scores = run_mock_predictions(df_tcr, is_mutated=True, mutation_type="tcr")
if not HAS_PYG and model is not None:
used_mock_fallback = True
else:
try:
ds_wt = StructuralPeptideMHCDataset(df_pos)
Expand All @@ -246,6 +263,7 @@ def evaluate_single_virus(
df_anchor, is_mutated=True, mutation_type="anchor"
)
tcr_scores = run_mock_predictions(df_tcr, is_mutated=True, mutation_type="tcr")
used_mock_fallback = True

# Degradation rates
mean_wt = float(np.mean(wt_scores))
Expand Down Expand Up @@ -282,6 +300,7 @@ def evaluate_single_virus(
"sample_count": len(df),
"positive_ratio": float(np.mean(y_true)),
"escape_mutant_cross_validation": mutant_results,
"used_mock_fallback": used_mock_fallback,
}


Expand Down Expand Up @@ -373,6 +392,17 @@ def run_evaluation_pipeline(
virus_results = evaluate_single_virus(virus_name, df, model, device, use_mock=use_mock)
report["viral_families"][virus_name] = virus_results

# A per-virus dataset construction can raise and silently fall back to
# mock scores (evaluate_single_virus's except arms) even when this run
# was never asked for mock and a real model loaded - "use_mock_fallback"
# was computed once above, before any virus ran, and never reflected
# that. Recompute it now so a report claiming real GNN scores never
# coexists with a virus that actually used mock ones.
any_virus_fallback = any(
v.get("used_mock_fallback", False) for v in report["viral_families"].values()
)
report["metadata"]["use_mock_fallback"] = use_mock or (model is None) or any_virus_fallback

# Compile global summary statistics
all_aucs = [v["roc_auc"] for v in report["viral_families"].values() if "roc_auc" in v]
all_prcs = [v["prc_auc"] for v in report["viral_families"].values() if "prc_auc" in v]
Expand Down
50 changes: 43 additions & 7 deletions src/verify/structural_gnn.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@
# 34 pocket residue index mapping according to NetMHCpan
MHC_POCKET_COUNT = 34

# Resolved relative to THIS module's own file, not the process CWD. A module
# attribute (not a call-time default) so tests can monkeypatch it.
_PSEUDO_SEQ_PATH = Path(__file__).resolve().parent / "mhc_pseudo_sequences.json"


def generate_canonical_groove_coords(
peptide: str, allele_name: str
Expand Down Expand Up @@ -116,13 +120,45 @@ def __init__(self, df: pd.DataFrame, transform=None, pre_transform=None):
self.unique_alleles = sorted(list(set(self.alleles)))
self.allele_to_idx = {a: i for i, a in enumerate(self.unique_alleles)}

# Load exact 34-residue structural strings
pseudo_seq_path = Path("src/verify/mhc_pseudo_sequences.json")
if pseudo_seq_path.exists():
with open(pseudo_seq_path, "r") as f:
self.pseudo_seqs = json.load(f)
else:
self.pseudo_seqs = {}
# Load exact 34-residue structural strings. `pos` and edge connectivity
# never depend on allele identity either way - generate_canonical_
# groove_coords ignores its allele_name argument - so this table is the
# ONLY source of allele signal anywhere in the graph: without it, every
# allele's MHC pocket node features (and the edge_attr entries derived
# from their charges) collapse to an identical all-alanine placeholder,
# regardless of true identity. This raises rather than silently
# defaulting to an empty table - consistent with the wrong-length guard
# below (D30) - at the point the table is CONSTRUCTED. Whether that
# propagates to the caller as a visible crash or gets caught and
# reported as a fallback is the caller's choice; see
# src/verify/sestrav_evaluator.py's used_mock_fallback for the one
# production caller, which chooses the latter and records it.
if not _PSEUDO_SEQ_PATH.exists():
raise FileNotFoundError(
f"MHC pseudo-sequence table not found at {_PSEUDO_SEQ_PATH}. "
"Every allele's 34 pocket-residue features are derived from "
"this table; without it every allele silently collapses to "
"an identical all-alanine placeholder, which makes the model "
"allele-blind rather than merely missing one allele's "
"structural signal (docs/claims_register.md D30). Reinstall "
"the package or restore the tracked file at "
"src/verify/mhc_pseudo_sequences.json."
)
with open(_PSEUDO_SEQ_PATH, "r") as f:
self.pseudo_seqs = json.load(f)
# A present-but-empty (or metadata-only) table is the same catastrophe
# via a different door: every allele would fall through to the
# per-allele warn-and-placeholder branch below, achieving the exact
# blindness the FileNotFoundError above exists to prevent, silently.
# "_source" is a documentation key in the tracked file, not an allele.
if not any(k != "_source" for k in self.pseudo_seqs):
raise ValueError(
f"MHC pseudo-sequence table at {_PSEUDO_SEQ_PATH} contains no "
"allele entries (only metadata keys, if any). Every allele "
"would silently collapse to an identical all-alanine "
"placeholder - the same allele-blindness an absent file "
"produces. Restore the tracked file's real content."
)

self.mhc_node_tensors = torch.zeros(
(len(self.unique_alleles), MHC_POCKET_COUNT, 5), dtype=torch.float32
Expand Down
96 changes: 96 additions & 0 deletions tests/test_sestrav_evaluator_gnn.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,69 @@ def test_evaluate_single_virus_real_gnn():
# Breakout metrics are populated from the live model, not the mock fallback.
assert "mean_wildtype_score" in esc
assert 0.0 <= esc["anchor_sensitivity_success_rate"] <= 1.0
# A clean real-GNN run must report that it never fell back.
assert res["used_mock_fallback"] is False


def test_evaluate_single_virus_reports_unplanned_fallback(monkeypatch):
"""Regression pin: an exception mid-real-GNN-attempt must be visible in
the result, not merely swallowed into mock scores that look identical to
a deliberately-requested mock run.

Before this fix, evaluate_single_virus's except arms fell back to mock
predictions silently - the caller had no way to distinguish "asked for
mock" from "real GNN attempt failed and used mock instead", and
run_evaluation_pipeline's top-level use_mock_fallback flag stayed False
in the second case even though every score in the report was mock.
"""
import src.verify.sestrav_evaluator as ev
from src.verify.structural_gnn import StructuralGNN

def _boom(df):
raise RuntimeError("simulated dataset construction failure")

monkeypatch.setattr(ev, "StructuralPeptideMHCDataset", _boom)

df = _cohort()
model = StructuralGNN()
res = evaluate_single_virus("EBV", df, model=model, device=_CPU, use_mock=False)

assert res["used_mock_fallback"] is True
# The metric machinery still ran, over mock scores - the point is that
# the caller now KNOWS that, not that scoring stopped.
assert "roc_auc" in res


def test_evaluate_single_virus_reports_breakout_only_fallback(monkeypatch):
"""Regression pin for the SECOND except arm specifically (the breakout-
mutant path), independent of the main-cohort arm pinned above.

The main cohort succeeds normally here - only the three breakout dataset
constructions (ds_wt/ds_anchor/ds_tcr) fail. Deleting only the breakout
arm's `used_mock_fallback = True` must fail this test even though the
main-cohort arm's assignment (and its own test) are both untouched.
"""
import src.verify.sestrav_evaluator as ev
from src.verify.structural_gnn import StructuralGNN, StructuralPeptideMHCDataset

call_count = {"n": 0}

def _fail_after_first(df):
call_count["n"] += 1
if call_count["n"] == 1:
return StructuralPeptideMHCDataset(df)
raise RuntimeError("simulated breakout dataset construction failure")

monkeypatch.setattr(ev, "StructuralPeptideMHCDataset", _fail_after_first)

df = _cohort()
model = StructuralGNN()
res = evaluate_single_virus("EBV", df, model=model, device=_CPU, use_mock=False)

assert call_count["n"] > 1, "breakout branch was never reached - test setup is broken"
assert res["used_mock_fallback"] is True
esc = res["escape_mutant_cross_validation"]
assert "mean_wildtype_score" in esc


# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -104,6 +167,39 @@ def test_run_evaluation_pipeline_loads_wrapped_checkpoint(tmp_path):
assert (tmp_path / "results" / "validation_report.json").exists()


def test_run_evaluation_pipeline_reports_per_virus_fallback_in_metadata(tmp_path, monkeypatch):
"""Regression pin for the PIPELINE-level recompute, distinct from the
evaluate_single_virus-level pin above.

use_mock=False and a real checkpoint loads successfully, so
report["metadata"]["use_mock_fallback"] is computed as False before the
per-virus loop runs - but the one virus in this run fails GNN dataset
construction and falls back to mock scores. Deleting the pipeline's
post-loop recompute (the any_virus_fallback OR) leaves the top-level flag
at its pre-loop value, mislabelling every score in the report as real.
"""
import src.verify.sestrav_evaluator as ev
from src.verify.structural_gnn import StructuralGNN

def _boom(df):
raise RuntimeError("simulated dataset construction failure")

monkeypatch.setattr(ev, "StructuralPeptideMHCDataset", _boom)

targets = _write_targets(tmp_path)
ckpt = tmp_path / "gnn.pth"
torch.save({"model_state_dict": StructuralGNN().state_dict()}, ckpt) # nosec B614

report = run_evaluation_pipeline(
targets,
model_checkpoint_path=ckpt,
results_dir=tmp_path / "results",
use_mock=False,
)
assert report["viral_families"]["EBV"]["used_mock_fallback"] is True
assert report["metadata"]["use_mock_fallback"] is True


def test_run_evaluation_pipeline_loads_bare_state_dict(tmp_path):
from src.verify.structural_gnn import StructuralGNN

Expand Down
Loading
Loading