diff --git a/.github/workflows/full-pytest.yml b/.github/workflows/full-pytest.yml new file mode 100644 index 00000000..eeb516bd --- /dev/null +++ b/.github/workflows/full-pytest.yml @@ -0,0 +1,63 @@ +name: full-pytest + +# Runs the full pytest suite, excluding tests marked @pytest.mark.quarantine +# via the conftest.py QUARANTINE registry. The intent is a ratchet: once a +# test is removed from the registry it must keep passing on this gate. +# +# Quarantined tests are pre-existing failures that pre-date the substrate- +# liveness audit (bisect-confirmed against c1a1b7a). See: +# conftest.py — the QUARANTINE registry (one entry per quarantined test) +# docs/test-debt-quarantine.md — cluster diagnoses + removal policy + +on: + push: + branches: [main] + pull_request: + branches: [main] + +permissions: + contents: read + +concurrency: + group: full-pytest-${{ github.ref }} + cancel-in-progress: true + +jobs: + pytest: + name: full pytest (-m "not quarantine" -n 4) + runs-on: ubuntu-latest + timeout-minutes: 30 + + steps: + - name: checkout + uses: actions/checkout@v4 + with: + fetch-depth: 1 + + - name: set up uv + uses: astral-sh/setup-uv@v5 + with: + python-version: '3.11' + enable-cache: true + + - name: install dependencies + run: | + uv pip install -e ".[dev]" pyyaml + + - name: pytest (parallel, quarantine excluded) + env: + PYTHONPATH: ${{ github.workspace }} + run: | + uv run pytest -m "not quarantine" -n 4 --tb=short -q --maxfail=10 + + - name: report quarantine size (informational) + if: always() + env: + PYTHONPATH: ${{ github.workspace }} + run: | + uv run python -c " + import sys + sys.path.insert(0, '.') + from conftest import QUARANTINE + print(f'::notice title=Quarantine size::{len(QUARANTINE)} tests currently quarantined. Goal: shrink this number.') + " diff --git a/conftest.py b/conftest.py new file mode 100644 index 00000000..38bb65d9 --- /dev/null +++ b/conftest.py @@ -0,0 +1,111 @@ +"""Project-root conftest — quarantine registry for known-failing tests. + +The QUARANTINE set lists test IDs that are pre-existing failures +predating the substrate-liveness audit work (verified via bisect +against c1a1b7a, the commit immediately before the first W-* PR +of 2026-05-24). The CI gate at .github/workflows/full-pytest.yml +runs ``pytest -m "not quarantine"`` so these failures do not block +PRs, but the suite is a ratchet: a quarantined test removed from +this set must pass on its own merits. + +See docs/test-debt-quarantine.md for cluster diagnoses, removal +policy, and the per-test rationale. + +To remove a test from quarantine: + 1. Land a PR that makes the test pass. + 2. Delete its entry from QUARANTINE in the same PR. + 3. The full-pytest CI gate will now require it to keep passing. + +Adding a test to QUARANTINE is strongly discouraged. If a new +failure surfaces, the right default is to fix it in the PR that +caused it — not to quarantine. The set should only shrink. +""" + +from __future__ import annotations + +import pytest + + +QUARANTINE: frozenset[str] = frozenset({ + # Cluster A — ADR ledger row status drift (assertion lists predate + # later promotion ADRs; same shape as W-002, PR #240). + "tests/test_adr_0110_math_expert_demo.py::TestAdr0110MathExpertDemoHolds::test_math_row_is_expert_demo", + "tests/test_adr_0121_math_expert_deferred.py::TestMathRowStaysAtAuditPassed::test_ledger_reports_audit_passed_not_expert", + "tests/test_capability_cli.py::test_capability_ledger_json", + "tests/test_capability_reports.py::test_ledger_status_is_predicate_derived", + + # Cluster B — Surface decoration drift (assertions predate the + # "pack-grounded ()" suffix on grounded surfaces). + "tests/test_articulation.py::test_chat_surface_is_walk_surface", + "tests/test_correction_topic_lemma.py::test_correction_with_no_pack_lemma_still_grounds", + "tests/test_cross_pack_chains.py::test_runtime_narrative_aggregates_cross_pack_chains", + "tests/test_cross_pack_chains.py::test_runtime_example_aggregates_cross_pack_reverse_chains", + "tests/test_cross_pack_grounding.py::test_pack_grounded_surface_resolves_kinship_lemmas[parent]", + "tests/test_cross_pack_grounding.py::test_pack_grounded_surface_resolves_kinship_lemmas[child]", + "tests/test_cross_pack_grounding.py::test_pack_grounded_surface_resolves_kinship_lemmas[sibling]", + "tests/test_cross_pack_grounding.py::test_pack_grounded_surface_resolves_kinship_lemmas[family]", + "tests/test_cross_pack_grounding.py::test_pack_grounded_surface_resolves_kinship_lemmas[ancestor]", + "tests/test_cross_pack_grounding.py::test_pack_grounded_surface_resolves_kinship_lemmas[descendant]", + "tests/test_cross_pack_grounding.py::test_pack_grounded_surface_resolves_kinship_lemmas[spouse]", + "tests/test_cross_pack_grounding.py::test_pack_grounded_surface_resolves_kinship_lemmas[offspring]", + "tests/test_cross_pack_grounding.py::test_runtime_definition_on_kinship_lemma_engages_pack_path", + "tests/test_cross_pack_grounding.py::test_runtime_recall_on_kinship_lemma_engages_pack_path", + "tests/test_en_collapse_anchors_v1_pack.py::test_collapse_anchor_baseline_surface_advertises_anchor_nature", + + # Cluster C — Lane / runner metric drift (thresholds or report + # shape evolved without updating assertions). + "tests/test_adr_0122_rate_per_unit.py::TestOODInvarianceHolds::test_ood_ratio_unchanged_under_rate_grammar", + "tests/test_adr_0122_rate_per_unit.py::TestPerturbationInvariancesHold::test_invariance_gates_unchanged_under_rate_grammar", + "tests/test_adr_0126_train_sample_runner.py::test_runner_writes_report_to_disk", + "tests/test_adr_0126_train_sample_runner.py::test_report_has_documented_shape", + "tests/test_adr_0126_train_sample_runner.py::test_sample_count_and_case_id_pattern", + "tests/test_adr_0126_train_sample_runner.py::test_wrong_count_is_zero_baseline", + "tests/test_adr_0131_G3_numerics.py::test_gsm8k_probe_safety_rail_unchanged", + "tests/test_adr_0131_G_gsm8k_coverage_probe.py::test_admitted_wrong_is_zero", + "tests/test_adr_0131_G_gsm8k_coverage_probe.py::test_every_refused_case_has_typed_reason", + "tests/test_adr_0131_G_gsm8k_coverage_probe.py::test_per_case_outcomes_are_in_closed_vocabulary", + "tests/test_adr_0131_G_gsm8k_coverage_probe.py::test_report_is_deterministic_across_runs", + "tests/test_adr_0131_G_gsm8k_coverage_probe.py::test_committed_report_matches_current_run", + "tests/test_adr_0131_G_gsm8k_coverage_probe.py::test_report_schema_required_fields", + "tests/test_adr_0131_G_gsm8k_coverage_probe.py::test_refused_reasons_top_is_sorted_by_count_desc", + "tests/test_cold_start_grounding_lane.py::TestPassThresholds::test_public_v1_passes_thresholds", + "tests/test_cold_start_grounding_lane.py::TestPassThresholds::test_distributions_match_expected", + "tests/test_composed_surface.py::test_cognition_lane_metrics_unchanged_with_composed_flag", + "tests/test_compound_walkthrough_eval_lanes.py::test_chat_spine_holdout_splits_are_runnable", + "tests/test_en_core_action_v1_pack.py::test_pack_loads_with_matching_checksum", + "tests/test_en_core_action_v1_pack.py::test_all_entries_are_verbs", + "tests/test_en_core_action_v1_pack.py::test_all_expected_lemmas_present", + "tests/test_en_core_action_v1_pack.py::test_provenance_is_seed_core_action_v1", + "tests/test_gsm8k_math_runner.py::TestLaneReportShape::test_metrics_keys_match_documented_schema", + "tests/test_ood_surface_generator.py::test_live_parser_and_solver_match_each_variant_expected_answer", + "tests/test_ood_surface_generator.py::test_ood_public_ratio_meets_gate_across_dev_set", + "tests/test_perturbation_suite.py::test_aggregate_dev_rates_are_perfect_for_applicable_perturbations", + "tests/test_relations_chains_v1.py::test_all_seed_chains_load_cleanly", + + # Cluster D — CLI / internal API drift. + "tests/test_cli_test_suites.py::test_core_test_suite_accepts_pytest_flags_without_separator", + "tests/test_comb_pass_hot_path.py::test_classify_compound_intent_called_once_per_turn", + + # Cluster E — pytest-xdist parallel-execution incompatibilities. + # These tests pass single-threaded but fail under `-n 4` because they + # measure system-wide resources (memory RSS, timing) that drift under + # concurrent worker pressure. Fix shape: either make the test + # parallel-tolerant, or mark for serial-only execution (e.g. via + # pytest-xdist's --dist loadgroup + @pytest.mark.xdist_group). + "tests/test_articulation_bench.py::test_footprint_emits_samples_and_bounds", +}) + + +def pytest_collection_modifyitems(config, items): + """Stamp the quarantine marker on any test whose nodeid is listed + in QUARANTINE. Tests not in the set are unaffected. + + The CI gate runs ``pytest -m "not quarantine"`` so quarantined + tests are silently skipped in CI. Local ``pytest`` runs include + them by default (still useful for debugging individual fixes). + """ + _ = config # pluggy hook signature requires the name `config`; not used here + quarantine_marker = pytest.mark.quarantine + for item in items: + if item.nodeid in QUARANTINE: + item.add_marker(quarantine_marker) diff --git a/docs/test-debt-quarantine.md b/docs/test-debt-quarantine.md new file mode 100644 index 00000000..ebb3b0ab --- /dev/null +++ b/docs/test-debt-quarantine.md @@ -0,0 +1,162 @@ +# Test debt quarantine + +The `QUARANTINE` set in [`/conftest.py`](../conftest.py) lists test IDs +that are pre-existing failures predating the substrate-liveness audit +work of 2026-05-24. + +The `full-pytest` CI gate at +[`.github/workflows/full-pytest.yml`](../.github/workflows/full-pytest.yml) +runs `pytest -m "not quarantine"` so these failures do not block PRs. +But the suite is a **ratchet**: a test removed from `QUARANTINE` must +pass on its own merits in CI from that PR onward. + +## Origin + +A full `pytest --durations=30` run on 2026-05-24 surfaced **45 failures ++ 3 errors** in a 30-minute suite. Bisect against commit `c1a1b7a` +(the commit immediately before the first W-* PR of the audit +sequence) showed **all 49 fail identically on baseline** — today's +W-* work introduced zero new failures. + +These failures had accumulated because CI only verifies the lane SHA +pin job + per-suite slices (`core test --suite smoke|teaching|...`). +The full `pytest` lane was never gated, so feature evolution silently +broke assertions without surfacing. + +The `full-pytest` gate added in this PR closes that loop. The +`QUARANTINE` registry is the explicit IOU: 49 contracts we said we'd +uphold, momentarily set aside to unblock the gate. + +## Cluster diagnoses + +The 49 failures fall into four shape-clusters. Each cluster is fixable +by a small focused PR (same shape as W-002, which was a one-token +extension after ADR-0120 promoted a ledger row). + +### Cluster A — ADR ledger row status drift (4 tests) + +ADR-0091's contract predicates moved certain domain rows through +status promotions (`reasoning-capable` → `audit-passed` → `expert`). +Tests pinning the *old* status string fail. Same shape as W-002 +(PR #240), which extended `test_status_meets_reasoning_capable_at_minimum` +to accept `"expert"`. + +Affected: +- `test_adr_0110_math_expert_demo` +- `test_adr_0121_math_expert_deferred` +- `test_capability_cli::test_capability_ledger_json` +- `test_capability_reports::test_ledger_status_is_predicate_derived` + +**Fix shape**: extend the accepted-status set in each assertion, or +add a parametrize entry. One-token extensions per file. + +### Cluster B — Surface decoration drift (15 tests) + +The surface realizer now appends a `"pack-grounded ()"` +suffix to grounded surfaces (and similar decorations elsewhere). +Assertions written before that suffix existed compare against the +old format. + +Example failure (from `test_cross_pack_grounding`): +``` +AssertionError: assert 'spouse' in 'Spouse is one of a pair in a family. pack-grounded (en_core_relations_v1).' +``` +The lemma IS in the surface — the assertion just doesn't expect the +trailing suffix. + +Affected: +- `test_articulation::test_chat_surface_is_walk_surface` +- `test_correction_topic_lemma::test_correction_with_no_pack_lemma_still_grounds` +- `test_cross_pack_chains` (2) +- `test_cross_pack_grounding` (10, including 8 parametrized kinship) +- `test_en_collapse_anchors_v1_pack` + +**Fix shape**: update assertions to use `in` containment against the +substantive content, or update expected-string fixtures to include +the suffix. + +### Cluster C — Lane / runner metric drift (27 tests) + +Lane runner reports drifted (thresholds, schema keys, or metric +values) without updating the pinned pytest assertions. This is the +largest cluster. + +Example failure (from `test_cold_start_grounding_lane`): +``` +assert 0.9167 >= 0.95 +``` +Real metric value `0.9167` is below the assertion threshold `0.95`. +Either: +- (a) An actual quality regression worth investigating, or +- (b) An aspirational threshold that was never met and never re-pinned + +**Fix shape**: case-by-case. Some will be threshold updates; others +will be content fixes that move the metric back above threshold. +This cluster needs the most thought. + +Affected lanes/runners: +- `test_adr_0122_rate_per_unit` (2) +- `test_adr_0126_train_sample_runner` (4 — includes 3 ERROR-shape) +- `test_adr_0131_G3_numerics` + `test_adr_0131_G_gsm8k_coverage_probe` (8) +- `test_cold_start_grounding_lane` (2) +- `test_composed_surface` +- `test_compound_walkthrough_eval_lanes` +- `test_en_core_action_v1_pack` (4) +- `test_gsm8k_math_runner` +- `test_ood_surface_generator` (2) +- `test_perturbation_suite` +- `test_relations_chains_v1` + +### Cluster E — pytest-xdist parallel-execution incompatibilities (1 test) + +The gate runs `pytest -n 4`. Some tests measure system-wide resources +(memory RSS, wall-clock timing) and become flaky under concurrent +worker pressure even though they pass single-threaded. + +Affected: +- `test_articulation_bench::test_footprint_emits_samples_and_bounds` + (asserts per-turn ΔRSS < 1 MiB; total system memory pressure under + `-n 4` exceeds the ceiling) + +**Fix shape**: either rewrite the test to measure only its own +allocations (not system RSS), or mark for serial-only execution via +pytest-xdist's `--dist loadgroup` + `@pytest.mark.xdist_group("serial")`. + +### Cluster D — CLI / internal API drift (2 tests) + +Mixed minor drift in CLI argument parsing and intent-classification +hot path. + +Affected: +- `test_cli_test_suites::test_core_test_suite_accepts_pytest_flags_without_separator` +- `test_comb_pass_hot_path::test_classify_compound_intent_called_once_per_turn` + +## Removal policy + +To remove a test from quarantine: + +1. Land a PR that makes the test pass. +2. Delete its entry from `QUARANTINE` in `conftest.py` in the + **same PR** (so the gate immediately enforces the new contract). +3. The `full-pytest` CI gate now requires the test to keep passing. + +## Adding policy + +**Adding a test to `QUARANTINE` is strongly discouraged.** If a new +failure surfaces: + +- The right default is to fix it in the PR that caused it. +- If the failure is genuinely orthogonal to the PR's intent, open a + small fix-PR first, then resume the original work. +- The set should only shrink. + +The only legitimate addition is a test that surfaces a long-dormant +issue (e.g., a new pytest version exposes a latent bug) with a +tracked follow-up issue and a one-sentence justification in the +adding PR. + +## Cross-references + +- [`conftest.py`](../conftest.py) — the QUARANTINE registry +- [`.github/workflows/full-pytest.yml`](../.github/workflows/full-pytest.yml) — the CI gate +- [substrate-liveness-ratchet](audit/substrate-liveness-ratchet.md) — the W-* wiring debt registry (same shape, different domain) diff --git a/pyproject.toml b/pyproject.toml index 54590212..16d75dcd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -51,3 +51,6 @@ include = [ [tool.pytest.ini_options] asyncio_mode = "auto" testpaths = ["tests"] +markers = [ + "quarantine: pre-existing failure tracked in conftest.py QUARANTINE registry; excluded from full-pytest CI gate. See docs/test-debt-quarantine.md.", +]