Skip to content

Commit 73eaf75

Browse files
authored
fix(proof): enforce EvidenceRule.test_id/must_pass in proof runner (#729) (#802)
* fix(proof): enforce EvidenceRule.test_id/must_pass in proof runner (#729) A requirement is no longer SATISFIED by any green pytest run — each must_pass evidence rule is enforced by a pytest run scoped to its test_id (-k), and a named test that was never written is a FAILED obligation. - EvidenceRule gains an owning gate field, stamped at capture, persisted in the ledger; legacy rows derive it from the test_id prefix convention - gates.run/_run_pytest accept a test_selector; with a selector, "no tests matched" (exit 5) is a failure, not an empty suite - _run_gate enforces pytest-style rules on every runnable gate: SEC runs ruff AND its test_sec_* rules (codex review finding) - UNIT stub now defines test_unit_<slug> to match its rule, and stub slugs share obligations.slugify with rules (was [:50] vs [:60]) - drop unused pytest import in tests/test_new_feature.py (pre-existing ruff failure) * review: guard empty gate checks, warn on unenforceable rules, pin gate=None no-op Addresses claude-review findings on #802: explicit FAILED when gates.run returns no checks, per-requirement warning for rules with no resolvable gate, docstring note on per-rule subprocesses, and a test pinning that gate=None rules fall back to whole-suite behavior. * review: harden enforcement per CodeRabbit findings - invalid persisted gate values fall back to test_id prefix derivation instead of crashing requirement load - SKIPPED/ERROR pytest checks fail enforcement (pytest unavailable is not proof) - must_pass rules without a pytest-style test_id fail loudly instead of vanishing silently
1 parent 8a98e63 commit 73eaf75

9 files changed

Lines changed: 533 additions & 42 deletions

File tree

codeframe/core/gates.py

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,7 @@ def run(
287287
gates: Optional[list[str]] = None,
288288
verbose: bool = False,
289289
auto_install_deps: bool = True,
290+
test_selector: Optional[str] = None,
290291
) -> GateResult:
291292
"""Run verification gates.
292293
@@ -295,6 +296,8 @@ def run(
295296
gates: Specific gates to run (None = all available)
296297
verbose: Whether to capture full output
297298
auto_install_deps: Whether to auto-install missing dependencies before test gates (default: True)
299+
test_selector: Optional pytest ``-k`` keyword expression; only applies to
300+
the pytest gate. With a selector, "no tests matched" is a failure.
298301
299302
Returns:
300303
GateResult with all check results
@@ -358,7 +361,7 @@ def run(
358361
# Run each gate
359362
for gate_name in gates:
360363
if gate_name == "pytest":
361-
check = _run_pytest(repo_path, verbose)
364+
check = _run_pytest(repo_path, verbose, test_selector=test_selector)
362365
elif gate_name == "ruff":
363366
check = _run_ruff(repo_path, verbose)
364367
elif gate_name == "mypy":
@@ -476,8 +479,10 @@ def _detect_available_gates(repo_path: Path) -> list[str]:
476479
return gates
477480

478481

479-
def _run_pytest(repo_path: Path, verbose: bool = False) -> GateCheck:
480-
"""Run pytest."""
482+
def _run_pytest(
483+
repo_path: Path, verbose: bool = False, test_selector: Optional[str] = None
484+
) -> GateCheck:
485+
"""Run pytest, optionally scoped to a ``-k`` keyword expression."""
481486
import time
482487

483488
start = time.time()
@@ -496,6 +501,8 @@ def _run_pytest(repo_path: Path, verbose: bool = False) -> GateCheck:
496501
cmd = ["uv", "run", "pytest", "-v", "--tb=short"]
497502
else:
498503
cmd = ["pytest", "-v", "--tb=short"]
504+
if test_selector:
505+
cmd += ["-k", test_selector]
499506

500507
result = subprocess.run(
501508
cmd,
@@ -535,9 +542,12 @@ def _run_pytest(repo_path: Path, verbose: bool = False) -> GateCheck:
535542
status = GateStatus.FAILED
536543
elif result.returncode == 5:
537544
# Exit code 5: no tests collected
538-
# Check if this is a clean "no tests" or an error during collection
545+
# With an explicit selector, "nothing matched" means the named
546+
# test doesn't exist — that is a failure, not an empty suite.
539547
output_lower = output.lower()
540-
if "error" in output_lower or "importerror" in output_lower or "modulenotfounderror" in output_lower:
548+
if test_selector:
549+
status = GateStatus.FAILED
550+
elif "error" in output_lower or "importerror" in output_lower or "modulenotfounderror" in output_lower:
541551
# Collection error disguised as "no tests collected"
542552
status = GateStatus.FAILED
543553
elif "no tests ran" in output_lower or "collected 0 items" in output_lower:

codeframe/core/proof/ledger.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,12 +190,32 @@ def _obligations_from_json(raw: str) -> list[Obligation]:
190190

191191

192192
def _evidence_rules_to_json(rules: list[EvidenceRule]) -> str:
193-
return json.dumps([{"test_id": r.test_id, "must_pass": r.must_pass} for r in rules])
193+
return json.dumps([
194+
{"test_id": r.test_id, "must_pass": r.must_pass, "gate": r.gate.value if r.gate else None}
195+
for r in rules
196+
])
194197

195198

196199
def _evidence_rules_from_json(raw: str) -> list[EvidenceRule]:
200+
from codeframe.core.proof.obligations import gate_from_test_id
201+
202+
def _resolve_gate(d: dict) -> Optional[Gate]:
203+
# Legacy rows lack "gate"; invalid persisted values (schema drift,
204+
# manual edits) also fall back to test_id prefix derivation.
205+
try:
206+
return Gate(d["gate"])
207+
except (KeyError, ValueError):
208+
return gate_from_test_id(d["test_id"])
209+
197210
data = json.loads(raw)
198-
return [EvidenceRule(test_id=d["test_id"], must_pass=d.get("must_pass", True)) for d in data]
211+
return [
212+
EvidenceRule(
213+
test_id=d["test_id"],
214+
must_pass=d.get("must_pass", True),
215+
gate=_resolve_gate(d),
216+
)
217+
for d in data
218+
]
199219

200220

201221
def _waiver_to_json(waiver: Optional[Waiver]) -> Optional[str]:

codeframe/core/proof/models.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ class EvidenceRule:
110110

111111
test_id: str
112112
must_pass: bool = True
113+
gate: Optional[Gate] = None # owning gate; None on legacy rows without a derivable prefix
113114

114115

115116
@dataclass

codeframe/core/proof/obligations.py

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -83,21 +83,38 @@ def get_obligations(glitch_type: GlitchType) -> list[Obligation]:
8383
return [Obligation(gate=g) for g in gates]
8484

8585

86+
TEST_ID_PREFIXES: dict[Gate, str] = {
87+
Gate.UNIT: "test_unit_",
88+
Gate.CONTRACT: "test_contract_",
89+
Gate.E2E: "test_e2e_",
90+
Gate.VISUAL: "test_visual_",
91+
Gate.A11Y: "test_a11y_",
92+
Gate.PERF: "test_perf_",
93+
Gate.SEC: "test_sec_",
94+
Gate.DEMO: "test_demo_",
95+
Gate.MANUAL: "manual_check_",
96+
}
97+
98+
99+
def gate_from_test_id(test_id: str) -> Gate | None:
100+
"""Derive the owning gate from a test_id prefix (for legacy persisted rules)."""
101+
for gate, prefix in TEST_ID_PREFIXES.items():
102+
if test_id.startswith(prefix):
103+
return gate
104+
return None
105+
106+
107+
def slugify(text: str) -> str:
108+
"""Create a safe identifier from text.
109+
110+
Shared by evidence rules and stub generation so a generated stub's
111+
function name matches the rule's test_id exactly.
112+
"""
113+
slug = re.sub(r"[^a-z0-9]+", "_", text.lower().strip())[:60].strip("_")
114+
return slug or "unnamed"
115+
116+
86117
def suggest_evidence_rules(gate: Gate, description: str) -> list[EvidenceRule]:
87118
"""Generate starter evidence rules for an obligation gate."""
88-
# Create a sensible test ID from the description
89-
slug = re.sub(r"[^a-z0-9]+", "_", description.lower().strip())[:60].strip("_")
90-
91-
prefix_map = {
92-
Gate.UNIT: "test_unit_",
93-
Gate.CONTRACT: "test_contract_",
94-
Gate.E2E: "test_e2e_",
95-
Gate.VISUAL: "test_visual_",
96-
Gate.A11Y: "test_a11y_",
97-
Gate.PERF: "test_perf_",
98-
Gate.SEC: "test_sec_",
99-
Gate.DEMO: "test_demo_",
100-
Gate.MANUAL: "manual_check_",
101-
}
102-
prefix = prefix_map.get(gate, "test_")
103-
return [EvidenceRule(test_id=f"{prefix}{slug}", must_pass=True)]
119+
prefix = TEST_ID_PREFIXES.get(gate, "test_")
120+
return [EvidenceRule(test_id=f"{prefix}{slugify(description)}", must_pass=True, gate=gate)]

codeframe/core/proof/runner.py

Lines changed: 82 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@
99
import logging
1010
import uuid
1111
from datetime import datetime, timezone
12-
from typing import Optional
12+
from typing import Optional, Sequence
1313

1414
from codeframe.core.proof import ledger
1515
from codeframe.core.proof.evidence import attach_evidence
1616
from codeframe.core.proof.models import (
1717
PROOF_CONFIG_FILENAME,
18+
EvidenceRule,
1819
Gate,
1920
GateOutcome,
2021
ProofRun,
@@ -80,12 +81,26 @@ def _load_proof_config(workspace: Workspace) -> tuple[Optional[set[Gate]], str]:
8081
}
8182

8283

83-
def _run_gate(workspace: Workspace, gate: Gate) -> tuple[GateOutcome, str]:
84+
def _run_gate(
85+
workspace: Workspace,
86+
gate: Gate,
87+
rules: Sequence[EvidenceRule] = (),
88+
) -> tuple[GateOutcome, str]:
8489
"""Execute a single gate and return (outcome, output).
8590
8691
Uses core/gates.py for gates that have direct tool support. Gates without
8792
an automated runner return UNVERIFIABLE — the obligation could not be
8893
checked, which is distinct from running and failing.
94+
95+
For pytest-backed gates, each ``must_pass`` evidence rule is enforced
96+
individually: pytest runs scoped to the rule's ``test_id`` (via ``-k``),
97+
and a named test that doesn't exist is a FAILED obligation — a green
98+
whole-suite run proves nothing about a test that was never written.
99+
Rules with ``must_pass=False`` are informational only.
100+
101+
Each enforced rule deliberately gets its own pytest subprocess — do not
102+
collapse them into one ``-k "a or b"`` run; per-rule exit codes are what
103+
distinguish "named test missing" from "collected but failing".
89104
"""
90105
core_gate_name = _GATE_TO_CORE.get(gate)
91106
if not core_gate_name:
@@ -95,13 +110,61 @@ def _run_gate(workspace: Workspace, gate: Gate) -> tuple[GateOutcome, str]:
95110
)
96111

97112
try:
98-
from codeframe.core.gates import run as run_gates
99-
result = run_gates(workspace, gates=[core_gate_name], verbose=False)
100-
output_parts = [
101-
f"{check.name}: {check.status.value}" for check in result.checks
102-
]
103-
outcome = GateOutcome.PASSED if result.passed else GateOutcome.FAILED
104-
return outcome, "\n".join(output_parts)
113+
from codeframe.core import gates as core_gates
114+
115+
# Only pytest-style test_ids can be enforced by a scoped pytest run;
116+
# e.g. SEC's test_sec_* rules are pytest tests even though the SEC
117+
# gate's own runner is ruff.
118+
enforced = [r for r in rules if r.must_pass and r.test_id.startswith("test_")]
119+
unenforceable = [r for r in rules if r.must_pass and not r.test_id.startswith("test_")]
120+
121+
lines: list[str] = []
122+
all_passed = True
123+
124+
for rule in enforced:
125+
result = core_gates.run(
126+
workspace,
127+
gates=["pytest"],
128+
verbose=False,
129+
test_selector=rule.test_id,
130+
)
131+
check = result.checks[0] if result.checks else None
132+
if check is None:
133+
lines.append(f"{rule.test_id}: FAILED — no gate check returned")
134+
all_passed = False
135+
elif check.exit_code == 5:
136+
lines.append(f"{rule.test_id}: FAILED — named test missing (not collected)")
137+
all_passed = False
138+
elif check.status == core_gates.GateStatus.PASSED:
139+
lines.append(f"{rule.test_id}: passed")
140+
else:
141+
# SKIPPED (pytest unavailable) and ERROR (timeout) are not
142+
# proof — enforcement needs a positive pass, unlike the
143+
# whole-suite path where SKIPPED counts as passing.
144+
lines.append(f"{rule.test_id}: FAILED ({check.status.value})")
145+
all_passed = False
146+
147+
# A must_pass rule we cannot enforce must not silently count as
148+
# satisfied — that is the exact bug this module exists to prevent.
149+
for rule in unenforceable:
150+
lines.append(f"{rule.test_id}: FAILED — must_pass rule has no pytest-style test_id")
151+
all_passed = False
152+
153+
# Run the gate's own runner unless it is pytest and the enforced rules
154+
# already covered it (scoped runs replace the whole-suite run).
155+
if core_gate_name != "pytest" or not enforced:
156+
result = core_gates.run(workspace, gates=[core_gate_name], verbose=False)
157+
lines.extend(
158+
f"{check.name}: {check.status.value}" for check in result.checks
159+
)
160+
all_passed = all_passed and result.passed
161+
162+
for rule in rules:
163+
if not rule.must_pass:
164+
lines.append(f"{rule.test_id}: informational (must_pass=False, not enforced)")
165+
166+
outcome = GateOutcome.PASSED if all_passed else GateOutcome.FAILED
167+
return outcome, "\n".join(lines)
105168
except Exception as exc:
106169
logger.warning("Gate %s failed to run: %s", gate.value, exc)
107170
return GateOutcome.FAILED, str(exc)
@@ -183,6 +246,13 @@ def run_proof(
183246

184247
req_results: list[tuple[Gate, GateOutcome]] = []
185248

249+
unresolved = [r.test_id for r in req.evidence_rules if r.gate is None]
250+
if unresolved:
251+
logger.warning(
252+
"REQ %s: %d evidence rule(s) with no resolvable gate are not enforced: %s",
253+
req.id, len(unresolved), unresolved,
254+
)
255+
186256
for obl in req.obligations:
187257
# Apply gate filter
188258
if gate_filter and obl.gate != gate_filter:
@@ -192,8 +262,9 @@ def run_proof(
192262
if enabled_gates is not None and obl.gate not in enabled_gates:
193263
continue
194264

195-
# Run the gate
196-
outcome, output = _run_gate(workspace, obl.gate)
265+
# Run the gate, enforcing this requirement's evidence rules for it
266+
gate_rules = [r for r in req.evidence_rules if r.gate == obl.gate]
267+
outcome, output = _run_gate(workspace, obl.gate, gate_rules)
197268

198269
# Write artifact
199270
artifact_path = artifact_dir / f"{req.id}_{obl.gate.value}_{run_id}.txt"

codeframe/core/proof/stubs.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
import pytest
1313
1414
15-
def test_{slug}():
15+
def test_unit_{slug}():
1616
"""Proves: {description}"""
1717
# Arrange
1818
# TODO: Set up test data
@@ -128,10 +128,13 @@ def test_sec_{slug}():
128128

129129

130130
def _slugify(text: str) -> str:
131-
"""Create a safe identifier from text."""
132-
import re
133-
slug = re.sub(r"[^a-z0-9]+", "_", text.lower().strip())[:50].strip("_")
134-
return slug or "unnamed"
131+
"""Create a safe identifier from text.
132+
133+
Delegates to obligations.slugify so stub function names always match the
134+
evidence-rule test_ids that enforce them (issue #729).
135+
"""
136+
from codeframe.core.proof.obligations import slugify
137+
return slugify(text)
135138

136139

137140
def generate_stubs(req: Requirement) -> dict[Gate, str]:

tests/cli/test_proof_commands.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ def test_run_mixed_fail_and_unverifiable_exits_one(self, mock_run_gate, ws):
212212
),
213213
)
214214

215-
def _outcome(_ws, gate):
215+
def _outcome(_ws, gate, _rules=()):
216216
if gate == Gate.UNIT:
217217
return (GateOutcome.FAILED, "assertion failed")
218218
return (GateOutcome.UNVERIFIABLE, "cannot verify")

0 commit comments

Comments
 (0)