chore: local informational check for dormant BDD scenarios

[pmezzich]

Summary

Implements the #1603 outcome: a local, informational check that tells you which BDD scenarios touched by your change never actually run, per the maintainer guidance in that thread (run it alongside run_all_tests.sh or ruff, keep it out of CI so it can't become noise).

make check-dormant
# or
uv run python scripts/check_dormant_scenarios.py [--base REF | --paths ... | --all] [--strict]

Mechanism. The script maps the branch's BDD-relevant diff (step modules, feature files, test modules) to the test modules that bind them, then runs just those modules with DATABASE_URL removed from the environment. That split does all the work with no Docker and no Postgres, in seconds:

• wired scenarios skip at the integration_db gate ("requires PostgreSQL DATABASE_URL") — they would run in CI;
• dormant scenarios xfail at the harness/step layer ("No harness wired...", "not yet wired...", "Step definition not found") — they run nowhere.

Output groups dormant scenarios by reason with transport params collapsed, and lists documented spec-gap xfails separately (those are intentional, shown for awareness only). Touched conftest/tests/harness/generic-steps files affect every scenario, so the default run prints a note pointing at --all instead of silently sweeping the world.

Informational by design: exit code is always 0 unless --strict is passed. Judgment stays with the developer; it just stops operating blind.

Execution proof

$ uv run python scripts/check_dormant_scenarios.py --paths tests/bdd/steps/domain/uc026_package_media_buy.py
  49 DORMANT scenario(s) in the touched area — these never execute anywhere:
  [49] No harness wired for None
      - test_create_package_via_mcp__all_required_fields_provided
      ...

$ uv run python scripts/check_dormant_scenarios.py --paths tests/bdd/steps/domain/uc003_update_media_buy.py
  54 DORMANT scenario(s) in the touched area — these never execute anywhere:
  [54] UC-003 harness not yet wired for non-extension scenarios
      ...

$ uv run python scripts/check_dormant_scenarios.py     # clean branch
check-dormant: no BDD-relevant changes vs upstream/main — nothing to check

The UC-026 number is real: all of that feature is currently dormant (no T-UC-026 branch in _detect_uc, MediaBuyDualEnv has no consumers) while conftest comments still describe it as graduated — exactly the failure mode this check surfaces.

Test plan

• 9 unit tests pin the pure logic (xfail classification incl. nested-bracket outline params, path→module mapping, conftest-touch note): tests/unit/test_check_dormant_scenarios.py
• Live runs above (dormant module, wired-but-partially-dormant module, clean branch)
• ruff check / ruff format clean
• Informational: exit 0 by default, --strict flips to 1 on findings

[KonstantinMirin]

Review — PR #1622

Overview — This is a genuinely useful tool and the core design is right: dropping DATABASE_URL to split "would run in CI" (skips at the db gate) from "never runs anywhere" (xfails at the harness/step layer) is a clean trick, and the mapping tests run against the real repo tree with zero mocks. What needs fixing before merge clusters around one theme: a tool whose whole job is catching quiet false-greens currently has two quiet false-greens of its own — a crashed pytest run prints the all-clear, and the classifier's reason vocabulary is a hand-copied snapshot of the conftest that is already drifted on arrival.

Should fix

1. A crashed run reads as the all-clear. run_without_db() returns only out.stdout and discards the return code and stderr; classify() only reads XFAIL lines. So when a touched module fails to import — exactly the state a developer mid-edit is in when they'd reach for this tool — pytest exits 2 with zero XFAIL lines and the script prints no dormant scenarios in the touched area — everything you changed either runs or is a documented gap and exits 0. --strict is defeated the same way. Reproducible: add import nonexistent_module_xyz to any tests/bdd/test_uc*.py and run --paths on it.

Fix: return the CompletedProcess (or (returncode, stdout, stderr)), and in main() treat pytest return codes outside (0, 1) as "run did not complete — results unreliable": print stderr + tail of stdout and exit non-zero regardless of --strict. Pin it with a unit test.

2. The classifier vocabulary is a hand-maintained copy of the conftest's xfail reasons — and it's already wrong in both directions. The dormant/documented split lives or dies on string-matching the reasons tests/bdd/conftest.py emits, but _DORMANT_MARKERS is a disconnected copy:

• Missing: the conftest's auto-xfail hook converts two exception classes into wiring-gap xfails — StepDefinitionNotFoundError → "Step definition not found: ..." (covered) and NotImplementedError → "Not implemented: ..." (conftest.py:100-101, not covered). Harness stubs raise NotImplementedError today (tests/harness/dispatchers.py:338,347), so the day one fires, those scenarios land in the "DOCUMENTED spec-production gaps (fine)" bucket — the exact false reassurance #1603 exists to eliminate.
• Phantom: "step definition is not found" matches nothing anywhere in the tree.
• Third copy: scripts/enumerate_bdd_issues.py:86-98 already encodes the same dormant-vs-documented taxonomy with yet another phrasing.

Nothing pins the coupling either: SAMPLE_OUTPUT in the unit tests is hand-written to match the script's own markers, so rewording a pytest.xfail(...) reason in conftest blinds the tool while all 9 tests stay green.

Fix: extract the reason strings into one shared module (e.g. tests/bdd/xfail_taxonomy.py); conftest builds its pytest.xfail(...) reasons from those constants and classify() imports the same constants for matching — an import, not a comment. Add "not implemented" to the markers with a Not implemented: ... line in SAMPLE_OUTPUT asserted into the dormant bucket, and drop the phantom entry.

3. Same helper re-implemented instead of extracted (DRY within this diff). Two shapes:

• Path normalization path.strip().replace("\\", "/") is copy-pasted at three sites in the new script (changed_paths, is_bdd_relevant, map_paths_to_modules). The next normalization fix (git's quoting of non-ASCII paths, rename arrows) lands at one site and misses two. Extract _norm(path) and normalize once at ingestion.
• nodeid → scenario-name collapse nodeid.split("::")[-1].split("[", 1)[0] duplicates scripts/enumerate_bdd_issues.py:53 character-for-character modulo the maxsplit. Put scenario_name(nodeid) in the same shared module as the taxonomy constants and import it from both scripts.

4. The tool's strongest diagnostics are untested. map_paths_to_modules() has four note-producing branches; the tests cover only the conftest one. Untested: a .feature file no module binds ("every scenario in it is dormant" — the loudest signal the tool emits), a ucNNN step module with no matching test module, and a domain step module without a ucNNN name. Inverting the if binders: condition survives the whole suite. These are pure functions of path + repo tree — add TestMapPaths cases with e.g. tests/bdd/features/BR-UC-999-nonexistent.feature and a uc999_*.py path asserting modules == set() and the specific note text.

Nice to have

• git status --porcelain rename entries: line[3:] on R old.py -> new.py yields a pseudo-path that silently drops an uncommitted git mv of a feature/step file — a moment scenarios go dormant. line[3:].split(" -> ")[-1].
• changed_paths diffs "..HEAD" when merge-base returns empty (shallow clone) — bail with a message instead of an empty diff.
• _XFAIL_LINE's (\S+?) drops nodeids whose param ids contain spaces; anchor on the - reason separator instead.
• --paths given only unmapped/non-BDD paths exits with no output at all — print the "nothing mapped" parity message the git-diff branch already has.
• test_transport_params_collapse_to_one_scenario: counting duplicates inside a set can never exceed 1 — assert the bucket's exact contents instead (that actually fails if collapsing breaks).
• Docstring names a person; convention everywhere else is issue number only — #1603 carries the provenance.
• Sibling scripts carry #!/usr/bin/env python3; this one doesn't.

Notes / prior-review follow-ups

First review round — no prior threads to reconcile. Worth crediting: no allowlist or ratchet moves anywhere in the diff; the informational-by-default/--strict-opt-in shape matches the #1603 direction; and unit tests are the right vehicle here (dev tooling, not protocol behavior — no BDD scenario demanded), with the mock-free real-tree mapping tests being the right kind of coupling.

[KonstantinMirin]

This tuple is a hand-copied snapshot of the reasons tests/bdd/conftest.py emits, and it's already drifted both ways: the conftest's second auto-xfail wiring-gap reason "Not implemented: ..." (conftest.py:100-101, raised today by tests/harness/dispatchers.py:338,347 stubs) is missing — those scenarios would be bucketed as "DOCUMENTED spec-production gaps (fine)", the exact false reassurance #1603 targets — while "step definition is not found" matches nothing in the tree. Extract the reason strings into a shared module (e.g. tests/bdd/xfail_taxonomy.py) that conftest builds its pytest.xfail(...) reasons from and this classifier imports, add "not implemented", drop the phantom, and add a Not implemented: line to SAMPLE_OUTPUT asserted into the dormant bucket.

[KonstantinMirin]

Returning only stdout swallows collection/import errors: pytest exits 2 with zero XFAIL lines, classify() sees nothing, and main() prints the all-clear and exits 0 — even under --strict. Reproduced: append import nonexistent_module_xyz to a touched module and run --paths on it → "no dormant scenarios". Return the CompletedProcess, treat return codes outside (0, 1) as "run did not complete — results unreliable" (print stderr, exit non-zero regardless of --strict), and pin it with a unit test.

[KonstantinMirin]

strip().replace("\\", "/") normalization is repeated at three sites in this file (here, is_bdd_relevant, map_paths_to_modules). Extract _norm(path) and normalize once at ingestion (here and for --paths in main()), so the next normalization fix can't land at one site and miss the others.

[KonstantinMirin]

Second copy of the path normalization — route through the shared _norm() (or assume pre-normalized input once changed_paths/--paths normalize at ingestion).

[KonstantinMirin]

Third copy of the path normalization — same _norm() extraction.

[KonstantinMirin]

This nodeid→scenario collapse duplicates scripts/enumerate_bdd_issues.py:53 (nid.split("::")[-1].split("[")[0]). Extract scenario_name(nodeid) into the same shared module as the xfail-reason constants and import it from both scripts.

[KonstantinMirin]

line[3:] on a porcelain rename entry (R old.py -> new.py) yields the pseudo-path "old.py -> new.py", so an uncommitted git mv of a feature/step file — exactly a moment scenarios go dormant — is silently dropped. line[3:].split(" -> ")[-1].

[KonstantinMirin]

Convention everywhere else in scripts//src//tests/ is issue-number-only provenance — drop the personal name, #1603 carries it.

[KonstantinMirin]

map_paths_to_modules() has four note-producing branches but only the conftest one is tested here. The untested ones include the tool's loudest signal — a .feature file no module binds ("every scenario in it is dormant") — plus the ucNNN-with-no-test-module and non-ucNNN domain-module notes; inverting the if binders: condition survives this suite. Add cases with e.g. tests/bdd/features/BR-UC-999-nonexistent.feature and a uc999_*.py path asserting modules == set() and the specific note text.

[KonstantinMirin]

classify() returns sets, so counting how many members equal a name can never exceed 1 — this asserts Python's set, not the collapsing logic. Assert the bucket's exact contents instead, e.g. assert dormant["No harness wired for None"] == {"test_create_package_via_mcp", "test_outline"} — that fails if a bracketed variant leaks in.