Skip to content

fix(test --from-story): make the traceability fallback visible, not silent #2393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
agarwal-ishaan wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix(test --from-story): make the traceability fallback visible, not silent #2393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 15 additions & 0 deletions pdd/commands/generate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down
5 changes: 5 additions & 0 deletions pdd/commands/story.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from ..story_regression_gate import (
STATUS_MISSING,
STATUS_PASSING,
STATUS_STORY_REGRESSION_TRACEABILITY_ONLY,
STATUS_STALE,
evaluate_story_regression,
)
Expand Down Expand Up @@ -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",
}


Expand Down
122 changes: 103 additions & 19 deletions pdd/story_regression_gate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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,
Expand Down Expand Up @@ -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"
Expand All @@ -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)
Expand Down Expand Up @@ -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]:
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
*,
Expand All @@ -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]
Expand All @@ -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,
Expand Down
80 changes: 51 additions & 29 deletions pdd/story_test_generation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,24 @@
_HEADING_RE = re.compile(r"^(?P<marks>#{2,6})\s+(?P<title>.+?)\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:
Expand All @@ -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 {
Expand All @@ -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,
}


Expand Down Expand Up @@ -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")
Expand All @@ -165,13 +191,20 @@ 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",
"",
"import pytest",
"",
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:
Expand All @@ -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:",
Expand All @@ -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"


Expand Down Expand Up @@ -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,
)


Expand Down Expand Up @@ -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,
)
Loading