diff --git a/pdd/commands/generate.py b/pdd/commands/generate.py index d806ffc8b1..c7ae27f313 100644 --- a/pdd/commands/generate.py +++ b/pdd/commands/generate.py @@ -826,6 +826,21 @@ def test( "[bold green]Story regression test " f"{action}:[/bold green] {generated.test_file}" ) + # #2392: the traceability fallback used to be silent, so every + # signal downstream read as "protected" for a test that never + # executes the code under test. Say so at the point of creation. + if not generated.is_behavioral: + from ..story_test_generation import ( + TRACEABILITY_FALLBACK_WARNING, + ) + + console.print( + f"[bold yellow]Warning:[/bold yellow] {TRACEABILITY_FALLBACK_WARNING}." + ) + console.print( + " Add a machine-readable [bold]## Entry Point[/bold] to the " + "story contract and re-run to get a behavioural test." + ) result_dict = { "success": True, "message": "Story regression test generated.", diff --git a/pdd/commands/story.py b/pdd/commands/story.py index 979814056b..fc7884cc33 100644 --- a/pdd/commands/story.py +++ b/pdd/commands/story.py @@ -11,6 +11,7 @@ from ..story_regression_gate import ( STATUS_MISSING, STATUS_PASSING, + STATUS_STORY_REGRESSION_TRACEABILITY_ONLY, STATUS_STALE, evaluate_story_regression, ) @@ -304,6 +305,10 @@ def add_story( # pylint: disable=too-many-branches STATUS_MISSING: "missing", STATUS_STALE: "stale", STATUS_PASSING: "has-test", + # Present and fresh, but the test never executes the code under test + # (pdd#2392). Labelled distinctly so a documentary story is not read as + # protected. + STATUS_STORY_REGRESSION_TRACEABILITY_ONLY: "has-traceability-test", } diff --git a/pdd/story_regression_gate.py b/pdd/story_regression_gate.py index 503a9174b8..cadbb2fe55 100644 --- a/pdd/story_regression_gate.py +++ b/pdd/story_regression_gate.py @@ -35,7 +35,7 @@ def test_sync_round_trips(): import logging import shutil import subprocess -from dataclasses import dataclass +from dataclasses import dataclass, replace from pathlib import Path from typing import Dict, List, Optional, Sequence @@ -44,7 +44,11 @@ def test_sync_round_trips(): from .construct_paths import _find_pddrc_file, _load_pddrc_config from .path_resolution import find_project_root_from_path from .story_regression import StoryTestMap, build_story_map -from .story_test_generation import story_bundle_hash +from .story_test_generation import ( + STORY_TEST_MODE_CONSTANT, + STORY_TEST_MODE_TRACEABILITY, + story_bundle_hash, +) from .user_story_tests import ( STORY_PREFIX, STORY_SUFFIX, @@ -72,6 +76,13 @@ def test_sync_round_trips(): STATUS_PASSING = "story-regression-present" STATUS_MISSING = STATUS_STORY_REGRESSION_MISSING STATUS_STALE = STATUS_STORY_REGRESSION_STALE +# A story whose only fresh test is a generated traceability (text-pinning) test. +# The test exists and is current, but it never executes the code the story is +# about, so it cannot catch a behavioural regression. Reported distinctly from +# ``story-regression-ok`` so a documentary story is not mistaken for a protected +# one (pdd#2392). This is a *narrower* claim than OK, not a failure: the gate +# treats it as satisfying presence/freshness exactly as before. +STATUS_STORY_REGRESSION_TRACEABILITY_ONLY = "story-regression-traceability-only" _GATE_MODES = frozenset({"off", "warn", "strict"}) _DEFAULT_MODE = "warn" @@ -93,6 +104,11 @@ class StoryMarker: test_file: str test_name: str lineno: int + # Generated tests declare PDD_STORY_TEST_MODE; hand-written ones do not, so + # ``None`` means "unknown", which is treated as behavioural. Only a test + # that explicitly declares itself traceability-only is reported as such -- + # the gate must never downgrade a test it cannot classify (pdd#2392). + mode: Optional[str] = None @dataclass(frozen=True) @@ -329,7 +345,13 @@ def _markers_in_source(source: str, test_file: str) -> List[StoryMarker]: constants=constants, ) ) - return found + # A generated test declares its mode as a module-level string constant, so + # it rides along in the constants map the marker scan already builds. Every + # marker in a module shares that module's mode. + mode = constants.get(STORY_TEST_MODE_CONSTANT) + if mode is None: + return found + return [replace(marker, mode=mode) for marker in found] def discover_story_markers(tests_dir) -> Dict[str, StoryMarker]: @@ -449,23 +471,49 @@ def _classify_story_from_markers( detail="No @pytest.mark.story regression test resolves to this story.", ) - # Fresh if ANY claiming test records an acceptable hash. - for marker in story_markers: - if marker.story_hash and marker.story_hash in acceptable_hashes: - return StoryRegressionResult( - story_id=story_id, - story_path=str(path), - status=STATUS_STORY_REGRESSION_OK, - current_hash=current_hash, - recorded_hash=marker.story_hash, - test_file=marker.test_file, - test_name=marker.test_name, - detail=( + # Fresh if ANY claiming test records an acceptable hash. Prefer a + # behavioural claim over a traceability-only one: a story protected by a + # real behavioural test must not be reported as documentary just because a + # text-pinning sibling also claims it. + fresh = [ + marker + for marker in story_markers + if marker.story_hash and marker.story_hash in acceptable_hashes + ] + if fresh: + behavioural = [ + marker for marker in fresh if marker.mode != STORY_TEST_MODE_TRACEABILITY + ] + marker = behavioural[0] if behavioural else fresh[0] + traceability_only = not behavioural + return StoryRegressionResult( + story_id=story_id, + story_path=str(path), + status=( + STATUS_STORY_REGRESSION_TRACEABILITY_ONLY + if traceability_only + else STATUS_STORY_REGRESSION_OK + ), + current_hash=current_hash, + recorded_hash=marker.story_hash, + test_file=marker.test_file, + test_name=marker.test_name, + detail=( + ( + "Story has a fresh, linked traceability-only test: it pins the " + "story/contract text but does NOT execute the code the story " + "is about, so it cannot catch a behavioural regression. Add a " + "machine-readable ## Entry Point to the contract and re-run " + "`pdd test --from-story` to get a behavioural test." + ) + if traceability_only + else ( "Story has a fresh, linked regression test " "(pass/fail is verified separately by the story lane, " "`pytest -m story`)." - ), - ) + ) + ), + ) # None fresh: prefer a marker that at least records a hash for a precise message. hashed = [m for m in story_markers if m.story_hash] @@ -816,7 +864,13 @@ def has_regression_test(self) -> bool: @property def passed(self) -> bool: - return self.status == STATUS_PASSING + # Traceability-only is a narrower *description* of the same + # present-and-fresh outcome, not a failure, so it must not flip this + # flag and silently tighten any caller's gate (pdd#2392). + return self.status in ( + STATUS_PASSING, + STATUS_STORY_REGRESSION_TRACEABILITY_ONLY, + ) def as_dict(self) -> dict[str, object]: return { @@ -860,6 +914,25 @@ def _recorded_story_hashes(test_path: Path, sid: str) -> List[str]: return hashes +def _declares_traceability_only(test_path: Path) -> bool: + """Whether *test_path* declares itself a generated traceability-only test. + + Absence of the marker constant means "unknown", which is treated as + behavioural: a hand-written test must never be downgraded because it does + not carry generator metadata (pdd#2392). + """ + try: + source = test_path.read_text(encoding="utf-8") + except OSError: + return False + try: + tree = ast.parse(source) + except SyntaxError: + return False + constants = _module_string_constants(tree) + return constants.get(STORY_TEST_MODE_CONSTANT) == STORY_TEST_MODE_TRACEABILITY + + def evaluate_story_regression( story_path: Path, *, @@ -886,10 +959,14 @@ def evaluate_story_regression( recorded: dict[str, str] = {} all_recorded: set = set() + # A story is documentary only when EVERY linked test declares itself + # traceability-only; one behavioural sibling is enough to call it protected. + linked_paths: List[Path] = [] for nodeid in tests: test_path = _test_file_for_nodeid(nodeid, tests_dir) if test_path is None: continue + linked_paths.append(test_path) found = _recorded_story_hashes(test_path, sid) if found: recorded[nodeid] = found[0] @@ -915,9 +992,16 @@ def evaluate_story_regression( # recorded hash matching either form is fresh; otherwise the story changed. acceptable = _acceptable_story_hashes(story_path) if all_recorded & acceptable: + traceability_only = bool(linked_paths) and all( + _declares_traceability_only(path) for path in linked_paths + ) return StoryRegressionEvaluation( story_id=sid, - status=STATUS_PASSING, + status=( + STATUS_STORY_REGRESSION_TRACEABILITY_ONLY + if traceability_only + else STATUS_PASSING + ), current_hash=current_hash, tests=tests, recorded_hashes=recorded, diff --git a/pdd/story_test_generation.py b/pdd/story_test_generation.py index de6fc4799b..5af4df052e 100644 --- a/pdd/story_test_generation.py +++ b/pdd/story_test_generation.py @@ -14,6 +14,24 @@ _HEADING_RE = re.compile(r"^(?P#{2,6})\s+(?P.+?)\s*$", re.MULTILINE) _RULE_RE = re.compile(r"\bR-?\d+\b", re.IGNORECASE) +# Name of the module-level constant every generated test declares. The story +# regression gate reads it with the same AST scan it already uses for story +# hashes, so a documentary test can be told apart from a behavioural one +# without executing anything. +STORY_TEST_MODE_CONSTANT = "PDD_STORY_TEST_MODE" +# The test imports the contract's entry point, invokes it, and asserts the +# Oracle / Negative Cases over the return value. +STORY_TEST_MODE_BEHAVIORAL = "behavioral" +# The test pins the story+contract bundle hash only. It does NOT execute the +# code the story is about, so it detects edits to the story text rather than +# regressions in behaviour. +STORY_TEST_MODE_TRACEABILITY = "traceability" + +TRACEABILITY_FALLBACK_WARNING = ( + "contract declares no ## Entry Point; generating a traceability-only test " + "that will not exercise the code" +) + @dataclass(frozen=True) class GeneratedStoryTest: @@ -25,6 +43,14 @@ class GeneratedStoryTest: story_hash: str changed: bool test_count: int + # Defaulted so existing positional construction keeps working; every + # generator path sets it explicitly. + mode: str = STORY_TEST_MODE_TRACEABILITY + + @property + def is_behavioral(self) -> bool: + """Whether the generated test actually exercises the code under test.""" + return self.mode == STORY_TEST_MODE_BEHAVIORAL def as_dict(self) -> dict[str, object]: return { @@ -34,6 +60,7 @@ def as_dict(self) -> dict[str, object]: "story_hash": self.story_hash, "changed": self.changed, "test_count": self.test_count, + "mode": self.mode, } @@ -154,7 +181,6 @@ def _render_test( story_rel = _relative_literal(story_path, output_path) contract_rel = _relative_literal(contract_path, output_path) if contract_path else None oracle_name = _test_name(slug, "oracle_contract") - negative_name = _test_name(slug, "negative_cases") rules_suffix = "_".join(rule_ids[:3]).replace("-", "").lower() if rules_suffix: oracle_name = _test_name(slug, f"{rules_suffix}_oracle_contract") @@ -165,6 +191,12 @@ def _render_test( '"""Generated story-backed regression tests.', "", "This file is deterministic and safe to run without LLM/cloud credentials.", + "", + "TRACEABILITY ONLY: this test pins the story+contract bundle hash. It does", + "NOT import or execute the code the story is about, so it cannot catch a", + "behavioural regression -- deleting the module under test leaves it green.", + "Add a machine-readable ## Entry Point to the contract and regenerate to", + "get a behavioural test instead.", '"""', "from pathlib import Path", "", @@ -172,6 +204,7 @@ def _render_test( "", f'PDD_STORY_ID = "{slug}"', f'PDD_STORY_HASH = "{bundle_hash}"', + f'{STORY_TEST_MODE_CONSTANT} = "{STORY_TEST_MODE_TRACEABILITY}"', f'STORY_PATH = Path(__file__).resolve().parent / "{story_rel}"', ] if contract_rel: @@ -180,13 +213,8 @@ def _render_test( lines.append("CONTRACT_PATH = None") lines.extend( [ - "", - "", - "def _story_bundle() -> str:", - " story = STORY_PATH.read_text(encoding=\"utf-8\")", - " if CONTRACT_PATH is not None and CONTRACT_PATH.exists():", - " return story + \"\\n\\n\" + CONTRACT_PATH.read_text(encoding=\"utf-8\")", - " return story", + f"PDD_STORY_ORACLE_CLAUSES = {oracle_list}", + f"PDD_STORY_NEGATIVE_CLAUSES = {negative_list}", "", "", "def _bundle_hash() -> str:", @@ -200,29 +228,21 @@ def _render_test( "", "@pytest.mark.story(story_id=PDD_STORY_ID)", f"def {oracle_name}():", - " assert _bundle_hash() == PDD_STORY_HASH", - f" expected = {oracle_list}", - " bundle = _story_bundle()", - " assert expected, \"story has no Oracle or Acceptance Criteria clauses\"", - " for clause in expected:", - " assert clause in bundle", + " # The only assertion a traceability test can honestly make: the", + " # story+contract bundle is unchanged since this test was generated.", + " #", + " # It deliberately does NOT assert that each clause appears in the", + " # bundle. `_bundle_hash()` covers the same bytes those clauses were", + " # extracted from, so such a check can never fail while the hash", + " # matches, and never runs when it does not. The clauses are kept", + " # above as documentation of what this story claims.", + " assert _bundle_hash() == PDD_STORY_HASH, (", + " \"Story or contract changed since this test was generated; \"", + " \"re-run: pdd test --from-story \" + str(STORY_PATH)", + " )", "", ] ) - if negative_items: - lines.extend( - [ - "", - "@pytest.mark.story(story_id=PDD_STORY_ID)", - f"def {negative_name}():", - " assert _bundle_hash() == PDD_STORY_HASH", - f" expected = {negative_list}", - " bundle = _story_bundle()", - " for clause in expected:", - " assert clause in bundle", - "", - ] - ) return "\n".join(lines) + "\n" @@ -251,6 +271,7 @@ def _generate_behavioral_test( story_hash=result.story_hash, changed=before != after, test_count=result.test_count, + mode=STORY_TEST_MODE_BEHAVIORAL, ) @@ -331,5 +352,6 @@ def generate_story_regression_test( test_file=output_path, story_hash=bundle_hash, changed=changed, - test_count=2 if negative_items else 1, + test_count=1, + mode=STORY_TEST_MODE_TRACEABILITY, ) diff --git a/tests/test_story_test_generation.py b/tests/test_story_test_generation.py index 390f0a008e..07665db737 100644 --- a/tests/test_story_test_generation.py +++ b/tests/test_story_test_generation.py @@ -7,9 +7,13 @@ STATUS_MISSING, STATUS_PASSING, STATUS_STALE, + STATUS_STORY_REGRESSION_TRACEABILITY_ONLY, evaluate_story_regression, ) -from pdd.story_test_generation import generate_story_regression_test +from pdd.story_test_generation import ( + STORY_TEST_MODE_TRACEABILITY, + generate_story_regression_test, +) def _story(tmp_path: Path) -> Path: @@ -153,7 +157,15 @@ def test_story_regression_gate_detects_missing_passing_and_stale(tmp_path: Path) generated = generate_story_regression_test(story) story_map = build_story_map(tmp_path / "tests") passing = evaluate_story_regression(story, tests_dir=tmp_path / "tests", story_map=story_map) - assert passing.status == STATUS_PASSING + # This contract declares no ## Entry Point, so the generated test is + # traceability-only. It is present and fresh -- but reported distinctly from + # STATUS_PASSING so it cannot be mistaken for behavioural cover (#2392). + assert passing.status == STATUS_STORY_REGRESSION_TRACEABILITY_ONLY + assert passing.status != STATUS_PASSING + # ...and it still counts as present, so no caller's gate silently tightens. + assert passing.passed is True + assert passing.has_regression_test is True + assert generated.mode == STORY_TEST_MODE_TRACEABILITY assert generated.story_hash == passing.current_hash story.write_text( diff --git a/tests/test_story_test_generator.py b/tests/test_story_test_generator.py index cac910a1a5..a3ae4f3c0b 100644 --- a/tests/test_story_test_generator.py +++ b/tests/test_story_test_generator.py @@ -188,14 +188,18 @@ def test_pdd_test_from_story_cli_without_entry_point_writes_text_pin(tmp_path: P assert "checkout_app" not in text # no entry-point import in text-pin mode assert "import importlib" not in text assert "_bundle_hash() == PDD_STORY_HASH" in text # pins the bundle hash - assert "Eligible checkout refunds are accepted." in text # pins clauses + # Clauses are recorded as documentation constants, not asserted: a + # clause-in-bundle check can never fail while the hash matches (#2392). + assert "Eligible checkout refunds are accepted." in text assert "Ineligible checkout refunds are rejected." in text assert "@pytest.mark.story(story_id=PDD_STORY_ID)" in text + # The mode is declared so the gate can tell this apart from a behavioural test. + assert 'PDD_STORY_TEST_MODE = "traceability"' in text - # The text-pin tests run green against the current story bundle... + # The text-pin test runs green against the current story bundle... passing = _run_generated_test(tmp_path, output) assert passing.returncode == 0, passing.stdout + passing.stderr - assert "2 passed" in passing.stdout + assert "1 passed" in passing.stdout # ...and go stale-red when the story text changes under the pinned hash. story.write_text(