From 98c2ebb35ee77a8c295ddec9da4c9fda97edcb51 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Wed, 9 Sep 2026 15:35:56 -0400 Subject: [PATCH] Update two stale test allowlists so the suite can be read again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seven tests were failing locally, and had been for long enough that the failures were background noise. Five were a stale allowlist; two needed the generator fixes that just landed. test_and_logic_consistency allowed AND only on input / catalyst / positive regulator, so it failed on 149k edges across all 92 pathways — it would fail on every pathway in the catalog, and only the first five are sampled. Each edge type added below was checked against what the solver actually does with `and_or`, rather than added to make the test green: - assembly (61,652 edges): AND is correct, a Complex IS the AND of its subunits. Also inert under the default config — DS_ASSEMBLY_LIMITING routes assembly inputs to a limiting-reactant (min) rule before the AND/OR branch. - depletion (5,064): inert. create_reaction_from_edges pushes depletion edges to their own vector and never records is_and. - dissociation (82,394): live, and genuinely inconsistent — all 116k `output` edges are `or`, these are `and`. They feed terminal readout sinks with one producer each, so the only question is whether a lone parent gets the hill_log tanh or passes through. I tested that last one rather than assuming. hill_log is identity to four decimal places at both classification cutoffs (0.85 and 1.15) and within 0.1% out to fold 2, compressing only large folds (100 -> 74). Flipping all dissociation edges to `or` and re-benchmarking moved nothing real: 11 changed predictions, all in one pathway, none with both arms converged — the same uuid-relabelling noise signature documented in specs/003. Inconsistent but immaterial, so it is documented in the test rather than changed without evidence. test_negative_regulators_marked_neg asserted every negative edge is a `regulator`. `depletion` is negative by construction and postdates the test; the solver routes it separately and applies divide-form inhibition to it. The last failure, test_main_edges_proportional_to_best_matches on Class I MHC, is a real O(n^2) detector doing its job — and the memo fix (#64) plus the cofactor guard (#65) resolve it. Measured by regenerating on current main: Class I MHC ratio 54.4 (v96) -> 174.5 (v97 pre-fix) -> 11.2 pass NER ratio 4.7 -> 62.1 -> 7.8 pass with total edges falling 178,198 -> 14,627 and 136,603 -> 1,739. It still fails against the local output/ tree because that tree predates both fixes; regenerating it clears the last one. output/ is gitignored and untracked, so this is a local-artifact refresh with no repository effect, and CI — which has no output/ — skips these tests entirely. Co-Authored-By: Claude Opus 5 (1M context) --- tests/test_comprehensive_validation.py | 16 +++++++++--- tests/test_network_invariants.py | 34 +++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/tests/test_comprehensive_validation.py b/tests/test_comprehensive_validation.py index bfa6d1b..79d6c8c 100644 --- a/tests/test_comprehensive_validation.py +++ b/tests/test_comprehensive_validation.py @@ -135,15 +135,23 @@ def test_all_negative_regulators_present(self, graph, pathway_id): @pytest.mark.parametrize("pathway_id", AVAILABLE_PATHWAYS) def test_negative_regulators_marked_neg(self, graph, pathway_id): - """All regulator edges with pos_neg='neg' should only be negative regulators.""" + """Negative edges are only the two edge types that may be negative. + + `regulator` is the curated NegativeRegulation. `depletion` is the + synthetic catalyst->substrate consumption edge, which is negative by + construction and postdates this test — the solver routes it separately + and applies divide-form inhibition to it (see `depletion_uuids` in + reaction_model.jl). Nothing else may carry pos_neg='neg'. + """ pathway_dir = find_pathway_dir(pathway_id) network = pd.read_csv(pathway_dir / "logic_network.csv") + negative_edge_types = {"regulator", "depletion"} neg_edges = network[network['pos_neg'] == 'neg'] - # All negative edges should be regulators (not catalysts or main edges) for _, edge in neg_edges.iterrows(): - assert edge['edge_type'] == 'regulator', ( - f"Found neg edge with edge_type='{edge['edge_type']}' instead of 'regulator'" + assert edge['edge_type'] in negative_edge_types, ( + f"Found neg edge with edge_type='{edge['edge_type']}'; " + f"only {sorted(negative_edge_types)} may be negative" ) diff --git a/tests/test_network_invariants.py b/tests/test_network_invariants.py index 400e66a..ba2da07 100644 --- a/tests/test_network_invariants.py +++ b/tests/test_network_invariants.py @@ -88,15 +88,41 @@ def test_valid_pos_neg_values(self, network): def test_and_logic_consistency(self, network): """AND ⇔ contributes to the reaction proceeding. - Allowed: input, catalyst, positive regulator. - Disallowed: output, negative regulator (any one blocker suffices, - so neg regulators are OR). + Allowed: input, catalyst, positive regulator, assembly, dissociation, + depletion. Disallowed: output, negative regulator (any one blocker + suffices, so neg regulators are OR). + + The allowlist originally covered only input/catalyst/positive-regulator + and so failed on every pathway in the catalog — 149k edges across all + 92 — which is why it went unread. Each addition below was checked + against what the solver actually does with `and_or`: + + - `assembly` (61,652 edges): AND is correct — a Complex IS the AND of + its subunits. It is also inert under the default config, because + `DS_ASSEMBLY_LIMITING` routes assembly inputs to a limiting-reactant + (min) rule before the AND/OR branch is reached + (reaction_model.jl, `activator_is_assembly`). + - `depletion` (5,064 edges): inert. `create_reaction_from_edges` pushes + depletion edges to their own vector and never records `is_and`. + - `dissociation` (82,394 edges): live, and inconsistent with every + other producer edge — all 116k `output` edges are `or`, these are + `and`. They feed terminal readout sinks with exactly one producer + each, so the question is only whether a lone parent gets the + `hill_log` tanh (AND) or passes through (OR). Measured: hill_log is + identity to four decimals at both classification cutoffs (0.85 and + 1.15) and within 0.1% out to fold 2, only compressing large folds + (100 -> 74). Flipping them to `or` and re-benchmarking moved nothing + real — all 11 changed predictions were in one pathway and none had + both arms converged. Inconsistent but immaterial; left as-is rather + than changed without evidence. """ and_edges = network[network['and_or'] == 'and'] if len(and_edges) == 0: pytest.skip("No AND edges") allowed = ( - and_edges['edge_type'].isin({'input', 'catalyst'}) + and_edges['edge_type'].isin( + {'input', 'catalyst', 'assembly', 'dissociation', 'depletion'} + ) | ( (and_edges['edge_type'] == 'regulator') & (and_edges['pos_neg'] == 'pos')