diff --git a/bin/findings.py b/bin/findings.py index be1073c..c344e8b 100644 --- a/bin/findings.py +++ b/bin/findings.py @@ -95,6 +95,8 @@ "excessive-post-ship-iteration", # v1.1 Fix 3 — behavioral claim with structural-only verification "verification-too-shallow-for-claim", + # v1.2 Fix C — Tier-3 thin negative-path coverage alongside-finding + "tier3-negative-paths-thin-coverage", } # Severity mapping for Tier 3 contradiction tuple kinds (v0.5.2). @@ -118,6 +120,8 @@ "adversarial-pathway": "block", # v0.8 — contract-filter audit sentinel "tier3-filter-applied": "info", + # v1.2 Fix C — thin negative-path coverage alongside demotion + "tier3-negative-paths-thin-coverage": "warn", } SEVERITIES = {"block", "warn", "info"} diff --git a/bin/llm_judge.py b/bin/llm_judge.py index f6a4887..45e8888 100644 --- a/bin/llm_judge.py +++ b/bin/llm_judge.py @@ -1005,11 +1005,29 @@ def _faithfulness_malformed_finding() -> Finding: ) +def _thin_coverage_finding(original_finding: Finding) -> Finding: + """Return a tier3-negative-paths-thin-coverage warn alongside a demoted negative-path-omission.""" + step_n = original_finding.location.step + msg = ( + f"Step {step_n} has fewer than 3 negative-paths entries (thin coverage); " + "consider adding more failure branches." + )[:findings.MAX_MESSAGE_LEN] + return Finding( + tier=3, + kind="tier3-negative-paths-thin-coverage", + severity="warn", + location=original_finding.location, + message=msg, + dismissable=True, + ) + + def _verify_block_tuples_with_citations( tuple_findings: list[Finding], step_table: dict, *, config: JudgeConfig, + step_objects: list | None = None, ) -> list[Finding]: """Run a second batched API call to verify block-severity contradiction tuples. @@ -1032,15 +1050,41 @@ def _verify_block_tuples_with_citations( Returns a new finding list with the same non-block findings plus either verified-or-demoted block findings. """ + # Build negative-paths count lookup from step_objects (Fix C). + # Done first so thin-coverage logic can run even when no block findings exist. + _neg_paths_count: dict[int, int] = {} + if step_objects: + for obj in step_objects: + if isinstance(obj, dict): + sn = obj.get("step") + np = obj.get("negative_paths") or obj.get("negative-paths") or [] + else: + sn = getattr(obj, "step", None) + np = getattr(obj, "negative_paths", None) or [] + if sn is not None: + _neg_paths_count[int(sn)] = len(np) if np else 0 + # Separate block (verifiable) from non-block (pass through). block_indices: list[int] = [] for idx, f in enumerate(tuple_findings): if f.kind in _BLOCK_CONTRADICTION_KINDS and f.severity == "block": block_indices.append(idx) - # Short-circuit: nothing to verify. + # Fix C: emit thin-coverage alongside-finding for any negative-path-omission + # finding whose step has fewer than 3 negative-paths entries. + # This runs regardless of whether block findings exist. + thin_coverage_additions: list[Finding] = [] + for f in tuple_findings: + if f.kind == "negative-path-omission": + step_n = f.location.step + np_count = _neg_paths_count.get(step_n, 0) if step_n is not None else 0 + if np_count < 3: + thin_coverage_additions.append(_thin_coverage_finding(f)) + + # Short-circuit: nothing to verify via faithfulness check. if not block_indices: - return list(tuple_findings) + result = list(tuple_findings) + thin_coverage_additions + return result # Build a minimal representation of block findings to send to DeepSeek. block_summaries = [] @@ -1149,7 +1193,7 @@ def _verify_block_tuples_with_citations( if should_demote: result[orig_idx] = _faithfulness_demote_finding(original_finding) - return result + return result + thin_coverage_additions # ── Deterministic contract-resolution post-filter ──────────────────────────── @@ -1404,4 +1448,6 @@ def evaluate( # Second pass: cite-and-verify for block-severity tuples (v0.6 faithfulness check). # Zero extra cost when no block tuples; one batched call otherwise. - return _verify_block_tuples_with_citations(primary_findings, step_table, config=config) + return _verify_block_tuples_with_citations( + primary_findings, step_table, config=config, step_objects=step_objects + ) diff --git a/bin/spec_ast.py b/bin/spec_ast.py index 17e5122..d4528c5 100644 --- a/bin/spec_ast.py +++ b/bin/spec_ast.py @@ -1641,7 +1641,7 @@ def _check_self_cycle_produces(steps: list[dict]) -> list[_findings.Finding]: continue # Self-cycle confirmed display = tok if len(tok) <= 60 else "..." + tok[-57:] - msg = f"Step {step_n} action consumes {display!r} which it also produces (self-cycle)." + msg = f"Step {step_n} action references {display!r} which it also declares in produces (possible self-cycle)." if len(msg) > 140: msg = msg[:137] + "..." results.append(_findings.Finding( @@ -1653,8 +1653,8 @@ def _check_self_cycle_produces(steps: list[dict]) -> list[_findings.Finding]: ), message=msg, suggested_fix=( - "Move the file to a prior step's produces:, or remove it from " - "this step's produces: if it is an input, not an output." + "If action only names the path, remove from produces:. " + "If action reads X, change verification to assert idempotency." )[:140], )) diff --git a/bin/walker.py b/bin/walker.py index 75caeba..9a487cb 100644 --- a/bin/walker.py +++ b/bin/walker.py @@ -315,6 +315,12 @@ def record_answer(state: WalkState, *, concern_id: str, answer: str) -> WalkStat state.answered[concern_id] = answer del state.pending[i] state.round_count += 1 + # Fix D: per-round visibility for operators. + try: + from bin import _status as _st + _st.emit("info", "walker.round", round=state.round_count, pending=len(state.pending)) + except Exception: # noqa: BLE001 + pass # Flip seed-family flags if concern_id == "seed-lifecycle": state.lifecycle_asked = True @@ -1392,6 +1398,140 @@ def generate_negative_path_concerns( return concerns +# ── Step-action precision concerns (Fix B) ─────────────────────────────────── + +# pip install without pinned version: no `==`, no `@`, no `-r` constraint file, no `--constraint`. +_PRECISION_PIP_UNVERSIONED_RE = re.compile( + r"\bpip\s+install\b(?!.*(?:==|@\s*\S|(?:-r|-c|--constraint|--requirement)\s+\S))", +) +# python -m with no subcommand/argument after the package name. +_PRECISION_PYTHON_M_BARE_RE = re.compile( + r"\bpython3?\s+-m\s+([\w.]+)\s*$" +) +# Bare URL with no version token nearby (http(s):// not followed by a version token on the same line). +_PRECISION_BARE_URL_RE = re.compile( + r"https?://\S+" +) +_PRECISION_VERSION_TOKEN_RE = re.compile( + r"(?:==|@\s*v?[\d.]+|/v[\d.]+/|/[\d.]+/)" +) +# LLM vendor SDK + bare model identifier patterns. +_PRECISION_LLM_VENDOR_RE = re.compile( + r"\b(anthropic|openai|deepseek|ollama|client\.messages\.create|client\.chat\.completions|client\.responses\.create)\b", + re.IGNORECASE, +) +_PRECISION_BARE_MODEL_ID_RE = re.compile( + r"\b(gpt-\d+[a-z0-9-]*|claude-[a-z0-9-]+|deepseek-[a-z0-9-]+|llama-[a-z0-9-]+|mistral-[a-z0-9-]+|gemma-[a-z0-9-]+|command-[a-z0-9-]+)\b", + re.IGNORECASE, +) + + +def _check_step_precision(step_n: int, action: str) -> list[tuple[str, str]]: + """Return list of (concern_id, summary) for vague shapes in *action*. + + Checks (structural patterns): + 1. pip install without version pin. + 2. python -m with no subcommand args. + 3. Bare URL with no version-pin verification nearby. + 4. Bare model ID alongside LLM vendor SDK call. + """ + concerns: list[tuple[str, str]] = [] + a = action.strip() + + # 1. pip install without version pin + if _PRECISION_PIP_UNVERSIONED_RE.search(a): + concerns.append(( + f"precision-pip-{step_n}", + ( + f"Step {step_n} action `{a[:60]}` has `pip install` without a pinned version — " + "add `==X.Y.Z` or a constraint file (e.g. `-c constraints.txt`) to make the " + "build reproducible." + )[:280], + )) + + # 2. python -m with no subcommand args + m = _PRECISION_PYTHON_M_BARE_RE.search(a) + if m: + pkg = m.group(1) + concerns.append(( + f"precision-python-m-{step_n}", + ( + f"Step {step_n} action `{a[:60]}` invokes `python -m {pkg}` with no subcommand " + "or arguments — specify the subcommand (e.g. `python -m myapp serve`) so the " + "intent is unambiguous." + )[:280], + )) + + # 3. Bare URL with no version-pin token in the same action + url_m = _PRECISION_BARE_URL_RE.search(a) + if url_m: + url = url_m.group(0)[:60] + if not _PRECISION_VERSION_TOKEN_RE.search(a): + concerns.append(( + f"precision-url-{step_n}", + ( + f"Step {step_n} action contains a bare URL (`{url}`) with no version pin — " + "pin to a specific version tag or commit so the download is reproducible." + )[:280], + )) + + # 4. Bare model ID alongside LLM vendor SDK + if _PRECISION_LLM_VENDOR_RE.search(a): + model_m = _PRECISION_BARE_MODEL_ID_RE.search(a) + if model_m: + model_id = model_m.group(0) + concerns.append(( + f"precision-model-{step_n}", + ( + f"Step {step_n} action uses bare model ID `{model_id}` alongside a vendor SDK — " + "document the model selection logic (env var, config key, or version-locked constant) " + "so the choice is explicit and auditable." + )[:280], + )) + + return concerns + + +def generate_step_precision_concerns( + state: WalkState, + steps: list[dict], +) -> list[Concern]: + """Emit edge-case concerns for vague action shapes in each step. + + Checks four structural patterns per step action (Fix B): + 1. pip install without version pin. + 2. python -m with no subcommand. + 3. Bare URL with no version-pin verification. + 4. Bare model ID alongside LLM vendor SDK. + + Idempotent: never emits a concern whose id already exists in state. + """ + existing_ids: set[str] = ( + {c.id for c in state.asked} + | {c.id for c in state.pending} + | set(state.answered) + ) + concerns: list[Concern] = [] + for step in steps: + step_n = step.get("step") + if step_n is None: + continue + action: str = step.get("action", "") or "" + if not action: + continue + for concern_id, summary in _check_step_precision(step_n, action): + if concern_id in existing_ids: + continue + concerns.append(Concern( + id=concern_id, + kind="edge-case", + receivers=["human"], + depends_on=[], + summary=summary, + )) + return concerns + + # ── Scaffold-precondition concern ───────────────────────────────────────────── # Stdlib top-level modules for `python -m ` heuristic — these do NOT need diff --git a/docs/glossary.md b/docs/glossary.md index 0504efd..86a70ae 100644 --- a/docs/glossary.md +++ b/docs/glossary.md @@ -1189,3 +1189,21 @@ Status codes: dotted identifiers like `walker.init`. Terms: `term:` prefix - user_action: Run `spectre catalog upgrade-taxonomy --spec --to ` when you want to consider the newer axes, or ignore. - related: term:taxonomy-version - since: v1.0 + +## tier3-negative-paths-thin-coverage +- kind: finding +- dev: Emitted alongside any `negative-path-omission` finding whose step has fewer than 3 `negative-paths:` entries. No demotion is involved — `negative-path-omission` is info-severity and never enters the faithfulness demotion path. The pairing signals "the LLM judge flagged a missing failure branch AND the step's structural coverage is thin." Tier-3 warn, dismissable. +- pm: A step's failure-branch coverage is thin (fewer than 3 entries) and the automated review flagged a missing failure scenario. Consider adding more failure scenarios to the step's negative-paths section. +- triggered_by: Co-occurrence of a `negative-path-omission` finding (LLM-judge output) and `< 3` negative-paths entries on the affected step. +- user_action: Add more negative-paths entries to the flagged step (at least 3 entries covering different failure modes), or dismiss if the step genuinely has only one or two realistic failure branches. +- related: negative-path-omission +- since: v1.2 + +## walker.round +- kind: status +- dev: Emitted after each concern answer in the walker interview loop. Fields: round=N (1-based count of answered concerns), pending=K (remaining non-stale concerns). Provides per-round visibility into walk progress without exposing convergence decisions. +- pm: The walker just finished interview round N. There are K questions still to answer. +- triggered_by: walker.record_answer increments round_count. +- user_action: No action required. Monitor round and pending counts to gauge walk progress. Operator interpretation only — walker.round does not imply any threshold or convergence signal. +- related: walker.yield, walker.coverage +- since: v1.2 diff --git a/tests/test_llm_judge_thin_coverage.py b/tests/test_llm_judge_thin_coverage.py new file mode 100644 index 0000000..f03ea35 --- /dev/null +++ b/tests/test_llm_judge_thin_coverage.py @@ -0,0 +1,174 @@ +"""Tests for Fix C: tier3-negative-paths-thin-coverage alongside-finding. + +Three cases exercised through llm_judge.evaluate() with mocked HTTP: + 1. Thin fires: negative-path-omission present + step has < 3 negative-paths. + 2. Thick doesn't fire: negative-path-omission present + step has >= 3 negative-paths. + 3. Kind is not negative-path-omission: thin-coverage must not fire. + +Mocks urllib.request.urlopen (the real HTTP boundary), calls real llm_judge.evaluate(). + +Pragma guard: assertion-style names only. One assertion per test. +Tests asserting absence/emptiness use _returns_empty/_is_none/_no_ naming. +""" +from __future__ import annotations + +import json +from unittest import mock + +from bin import llm_judge + + +# ── Spec fixture ────────────────────────────────────────────────────────────── + +_SPEC = """\ +# Thin Coverage Spec + +## 6. Steps + +```yaml +- step: 5 + why: run the pipeline + action: python3 run.py --input data.csv + verification: test -f output.json +``` + +## 8. Receiver Calibration + +### 8.1 Hard contract +- mutates: /tmp/out +- never-touches: /etc +- decision-budget: none +- reboot-survival: none +""" + +_CFG = llm_judge.JudgeConfig( + enabled=True, + api_key_env="TEST_DEEPSEEK_KEY", + model="deepseek-v4-flash", +) + + +def _api_resp(content: str) -> mock.MagicMock: + payload = json.dumps( + {"choices": [{"message": {"content": content}}]} + ).encode() + resp = mock.MagicMock() + resp.read.return_value = payload + resp.__enter__ = mock.Mock(return_value=resp) + resp.__exit__ = mock.Mock(return_value=None) + return resp + + +def _step_objects(neg_path_count: int) -> list[dict]: + """step_objects with a specific number of negative-paths entries for step 5.""" + return [ + { + "step": 5, + "negative_paths": [ + {"trigger": f"fail{i}", "handler": "abort"} + for i in range(neg_path_count) + ], + } + ] + + +# negative-path-omission has "info" severity — no faithfulness second call. +_NEG_PATH_OMISSION_TUPLE = { + "kind": "negative-path-omission", + "step": 5, + "rationale": "step 5 has no negative-paths documented", +} + +# missing-producer has "block" severity — triggers a faithfulness second call. +_MISSING_PRODUCER_TUPLE = { + "kind": "missing-producer", + "consumer_step": 5, + "missing": "some-artifact", + "rationale": "no producer step found for some-artifact", +} + + +# ── Case 1: thin fires (< 3 negative-paths) ────────────────────────────────── + + +@mock.patch("urllib.request.urlopen") +def test_thin_coverage_emitted_when_neg_path_omission_present_and_paths_is_zero(mock_urlopen, monkeypatch): + """Thin fires: negative-path-omission present + step has 0 negative-paths.""" + monkeypatch.setenv("TEST_DEEPSEEK_KEY", "fake-key") + # Only one API call — negative-path-omission is info severity, no faithfulness check. + mock_urlopen.return_value = _api_resp(json.dumps([_NEG_PATH_OMISSION_TUPLE])) + result = llm_judge.evaluate(_SPEC, config=_CFG, step_objects=_step_objects(0)) + assert any(f.kind == "tier3-negative-paths-thin-coverage" for f in result) + + +@mock.patch("urllib.request.urlopen") +def test_thin_coverage_emitted_when_neg_path_omission_present_and_paths_is_two(mock_urlopen, monkeypatch): + """Thin fires: negative-path-omission present + step has 2 negative-paths (< 3).""" + monkeypatch.setenv("TEST_DEEPSEEK_KEY", "fake-key") + mock_urlopen.return_value = _api_resp(json.dumps([_NEG_PATH_OMISSION_TUPLE])) + result = llm_judge.evaluate(_SPEC, config=_CFG, step_objects=_step_objects(2)) + assert any(f.kind == "tier3-negative-paths-thin-coverage" for f in result) + + +@mock.patch("urllib.request.urlopen") +def test_thin_coverage_finding_severity_is_warn(mock_urlopen, monkeypatch): + monkeypatch.setenv("TEST_DEEPSEEK_KEY", "fake-key") + mock_urlopen.return_value = _api_resp(json.dumps([_NEG_PATH_OMISSION_TUPLE])) + result = llm_judge.evaluate(_SPEC, config=_CFG, step_objects=_step_objects(1)) + thin = next((f for f in result if f.kind == "tier3-negative-paths-thin-coverage"), None) + assert thin is not None and thin.severity == "warn" + + +@mock.patch("urllib.request.urlopen") +def test_thin_coverage_finding_is_dismissable(mock_urlopen, monkeypatch): + monkeypatch.setenv("TEST_DEEPSEEK_KEY", "fake-key") + mock_urlopen.return_value = _api_resp(json.dumps([_NEG_PATH_OMISSION_TUPLE])) + result = llm_judge.evaluate(_SPEC, config=_CFG, step_objects=_step_objects(0)) + thin = next((f for f in result if f.kind == "tier3-negative-paths-thin-coverage"), None) + assert thin is not None and thin.dismissable is True + + +@mock.patch("urllib.request.urlopen") +def test_thin_coverage_finding_location_step_matches_original(mock_urlopen, monkeypatch): + monkeypatch.setenv("TEST_DEEPSEEK_KEY", "fake-key") + mock_urlopen.return_value = _api_resp(json.dumps([_NEG_PATH_OMISSION_TUPLE])) + result = llm_judge.evaluate(_SPEC, config=_CFG, step_objects=_step_objects(1)) + thin = next((f for f in result if f.kind == "tier3-negative-paths-thin-coverage"), None) + assert thin is not None and thin.location.step == 5 + + +# ── Case 2: thick doesn't fire (>= 3 negative-paths) ──────────────────────── + + +@mock.patch("urllib.request.urlopen") +def test_thin_coverage_not_emitted_when_neg_paths_count_is_three(mock_urlopen, monkeypatch): + """Thick: same finding but step has 3 negative-paths — no thin finding.""" + monkeypatch.setenv("TEST_DEEPSEEK_KEY", "fake-key") + mock_urlopen.return_value = _api_resp(json.dumps([_NEG_PATH_OMISSION_TUPLE])) + result = llm_judge.evaluate(_SPEC, config=_CFG, step_objects=_step_objects(3)) + assert not any(f.kind == "tier3-negative-paths-thin-coverage" for f in result) + + +@mock.patch("urllib.request.urlopen") +def test_thin_coverage_not_emitted_when_neg_paths_count_is_five(mock_urlopen, monkeypatch): + monkeypatch.setenv("TEST_DEEPSEEK_KEY", "fake-key") + mock_urlopen.return_value = _api_resp(json.dumps([_NEG_PATH_OMISSION_TUPLE])) + result = llm_judge.evaluate(_SPEC, config=_CFG, step_objects=_step_objects(5)) + assert not any(f.kind == "tier3-negative-paths-thin-coverage" for f in result) + + +# ── Case 3: different kind — thin-coverage must not fire ───────────────────── + + +@mock.patch("urllib.request.urlopen") +def test_thin_coverage_not_emitted_when_finding_kind_is_not_negative_path_omission(mock_urlopen, monkeypatch): + """missing-producer finding with 0 negative-paths → thin-coverage must NOT fire.""" + monkeypatch.setenv("TEST_DEEPSEEK_KEY", "fake-key") + # missing-producer is block severity → triggers faithfulness second call. + cite_resp_content = json.dumps([{"index": 0, "step": 5, "citation": "python3 run.py"}]) + mock_urlopen.side_effect = [ + _api_resp(json.dumps([_MISSING_PRODUCER_TUPLE])), + _api_resp(cite_resp_content), + ] + result = llm_judge.evaluate(_SPEC, config=_CFG, step_objects=_step_objects(0)) + assert not any(f.kind == "tier3-negative-paths-thin-coverage" for f in result) diff --git a/tests/test_spec_ast_self_cycle.py b/tests/test_spec_ast_self_cycle.py index 0728151..7af36aa 100644 --- a/tests/test_spec_ast_self_cycle.py +++ b/tests/test_spec_ast_self_cycle.py @@ -53,6 +53,30 @@ def test_self_cycle_emits_finding(): p.unlink(missing_ok=True) +# ── Fix M: self-cycle wording ───────────────────────────────────────────────── + +def test_self_cycle_message_uses_references_wording(): + """Fix M: message must say 'references ... declares in produces' not 'consumes ... produces'.""" + p = _write_spec(_SELF_CYCLE_YAML) + try: + fs = spec_ast.classify(p) + f = next(x for x in fs if x.kind == "self-cycle-produces") + assert "references" in f.message and "declares in produces" in f.message + finally: + p.unlink(missing_ok=True) + + +def test_self_cycle_suggested_fix_mentions_idempotency(): + """Fix M: suggested_fix must offer idempotency as the second branch.""" + p = _write_spec(_SELF_CYCLE_YAML) + try: + fs = spec_ast.classify(p) + f = next(x for x in fs if x.kind == "self-cycle-produces") + assert "idempotency" in (f.suggested_fix or "") + finally: + p.unlink(missing_ok=True) + + def test_self_cycle_severity_is_block(): p = _write_spec(_SELF_CYCLE_YAML) try: diff --git a/tests/test_walker_round_emission.py b/tests/test_walker_round_emission.py new file mode 100644 index 0000000..35d83c5 --- /dev/null +++ b/tests/test_walker_round_emission.py @@ -0,0 +1,76 @@ +"""Tests for Fix D: walker.round status emission in record_answer. + +Two cases: + 1. walker.round is emitted after record_answer increments round_count. + 2. The emitted fields contain the correct round number and pending count. + +Calls real walker.record_answer and captures _status.emit via monkeypatching +the _status module's emit function (not mocking record_answer itself). + +Pragma guard: assertion-style names only. One assertion per test. +Tests asserting absence/emptiness use _returns_empty/_is_none/_no_ naming. +""" +from __future__ import annotations + +import pathlib +from unittest import mock + +from bin import walker +from bin import _status + + +# ── Helpers ─────────────────────────────────────────────────────────────────── + + +def _state_with_concerns(*concern_ids: str) -> walker.WalkState: + """Return a WalkState with the given concerns in pending.""" + state = walker.WalkState( + spec_intent="test intent", + spec_draft_path=pathlib.Path("/tmp/test.spec.md.draft"), + ) + for cid in concern_ids: + state.pending.append(walker.Concern( + id=cid, + kind="edge-case", + receivers=["human"], + depends_on=[], + summary=f"Test concern {cid}", + )) + return state + + +# ── Case 1: walker.round is emitted ────────────────────────────────────────── + + +def test_record_answer_emits_walker_round_status(): + """record_answer must emit an info status with code 'walker.round'.""" + state = _state_with_concerns("c-1", "c-2") + emitted_codes: list[str] = [] + + def _capture_emit(level: str, code: str, **kwargs): + emitted_codes.append(code) + + with mock.patch("bin._status.emit", side_effect=_capture_emit): + walker.record_answer(state, concern_id="c-1", answer="yes") + + assert "walker.round" in emitted_codes + + +# ── Case 2: emitted fields are correct ─────────────────────────────────────── + + +def test_record_answer_emits_walker_round_with_correct_round_and_pending(): + """Emitted walker.round event must have round=1 and pending=1 after first answer.""" + state = _state_with_concerns("c-1", "c-2") + captured_kwargs: list[dict] = [] + + def _capture_emit(level: str, code: str, **kwargs): + if code == "walker.round": + captured_kwargs.append({"level": level, **kwargs}) + + with mock.patch("bin._status.emit", side_effect=_capture_emit): + walker.record_answer(state, concern_id="c-1", answer="done") + + assert len(captured_kwargs) == 1 + ev = captured_kwargs[0] + assert ev["round"] == 1 and ev["pending"] == 1 diff --git a/tests/test_walker_step_precision.py b/tests/test_walker_step_precision.py new file mode 100644 index 0000000..9974055 --- /dev/null +++ b/tests/test_walker_step_precision.py @@ -0,0 +1,167 @@ +"""Tests for Fix B: generate_step_precision_concerns in bin/walker.py. + +Covers four vague-action shapes: + 1. pip install without version pin. + 2. python -m with no subcommand args. + 3. Bare URL with no version-pin verification token. + 4. Bare model ID alongside LLM vendor SDK call. +Plus one non-vague control per shape. + +Pragma guard: assertion-style names only. One assertion per test. +Tests asserting absence/emptiness use _returns_empty/_is_none/_no_ naming. +""" +import pathlib + +from bin import walker + + +# ── Helpers ─────────────────────────────────────────────────────────────────── + + +def _state() -> walker.WalkState: + return walker.WalkState( + spec_intent="test", + spec_draft_path=pathlib.Path("/tmp/test.spec.md.draft"), + ) + + +def _step(n: int, action: str) -> dict: + return { + "step": n, + "why": "test", + "action": action, + "produces": ["file:/tmp/out.json"], + "requires": [], + "negative_paths": [], + } + + +def _precision_concerns(steps: list[dict]) -> list[walker.Concern]: + return walker.generate_step_precision_concerns(_state(), steps) + + +# ── Shape 1: pip install without version pin ────────────────────────────────── + + +def test_pip_install_unversioned_emits_concern(): + steps = [_step(1, "pip install requests flask")] + cs = _precision_concerns(steps) + assert any("precision-pip-1" == c.id for c in cs) + + +def test_pip_install_with_pinned_version_returns_no_concern(): + steps = [_step(1, "pip install requests==2.31.0 flask==3.0.0")] + cs = _precision_concerns(steps) + assert not any(c.id.startswith("precision-pip-") for c in cs) + + +def test_pip_install_with_constraint_file_returns_no_concern(): + steps = [_step(1, "pip install -r requirements.txt -c constraints.txt")] + cs = _precision_concerns(steps) + assert not any(c.id.startswith("precision-pip-") for c in cs) + + +def test_pip_install_concern_kind_is_edge_case(): + steps = [_step(1, "pip install mypackage")] + cs = _precision_concerns(steps) + pip_c = next((c for c in cs if c.id.startswith("precision-pip-")), None) + assert pip_c is not None and pip_c.kind == "edge-case" + + +def test_pip_install_concern_receiver_is_human(): + steps = [_step(1, "pip install mypackage")] + cs = _precision_concerns(steps) + pip_c = next((c for c in cs if c.id.startswith("precision-pip-")), None) + assert pip_c is not None and "human" in pip_c.receivers + + +# ── Shape 2: python -m with no subcommand args ───────────────────────── + + +def test_python_m_bare_no_args_emits_concern(): + steps = [_step(2, "python3 -m myapp")] + cs = _precision_concerns(steps) + assert any("precision-python-m-2" == c.id for c in cs) + + +def test_python_m_with_subcommand_returns_no_concern(): + steps = [_step(2, "python3 -m myapp serve --port 8080")] + cs = _precision_concerns(steps) + assert not any(c.id.startswith("precision-python-m-") for c in cs) + + +def test_python_m_bare_concern_mentions_subcommand(): + steps = [_step(2, "python3 -m myapp")] + cs = _precision_concerns(steps) + c = next((c for c in cs if c.id.startswith("precision-python-m-")), None) + assert c is not None and "subcommand" in c.summary.lower() + + +# ── Shape 3: Bare URL with no version-pin token ─────────────────────────────── + + +def test_bare_url_no_version_emits_concern(): + steps = [_step(3, "curl -fsSL https://get.docker.com | sh")] + cs = _precision_concerns(steps) + assert any("precision-url-3" == c.id for c in cs) + + +def test_url_with_version_tag_returns_no_concern(): + steps = [_step(3, "curl -fsSL https://example.com/v2.3.1/install.sh | sh")] + cs = _precision_concerns(steps) + assert not any(c.id.startswith("precision-url-") for c in cs) + + +def test_bare_url_concern_mentions_version_pin(): + steps = [_step(3, "curl -fsSL https://get.example.com | sh")] + cs = _precision_concerns(steps) + c = next((c for c in cs if c.id.startswith("precision-url-")), None) + assert c is not None and "version" in c.summary.lower() + + +# ── Shape 4: Bare model ID alongside LLM vendor SDK ────────────────────────── + + +def test_bare_model_id_with_vendor_sdk_emits_concern(): + steps = [_step(4, "python3 run.py # calls client.messages.create model=claude-opus-4")] + cs = _precision_concerns(steps) + assert any("precision-model-4" == c.id for c in cs) + + +def test_model_id_without_vendor_sdk_returns_no_concern(): + # A bare model string but no LLM vendor SDK keyword — no concern. + steps = [_step(4, "python3 run.py --model gpt-4o --output result.json")] + cs = _precision_concerns(steps) + assert not any(c.id.startswith("precision-model-") for c in cs) + + +def test_bare_model_concern_kind_is_edge_case(): + steps = [_step(4, "client.messages.create(model='claude-opus-4')")] + cs = _precision_concerns(steps) + c = next((c for c in cs if c.id.startswith("precision-model-")), None) + assert c is not None and c.kind == "edge-case" + + +# ── Idempotency ─────────────────────────────────────────────────────────────── + + +def test_precision_concern_is_idempotent_when_already_answered(): + state = _state() + steps = [_step(1, "pip install requests")] + # Pre-populate the answered dict with the concern id. + state.answered["precision-pip-1"] = "pinned to 2.31.0" + cs = walker.generate_step_precision_concerns(state, steps) + assert not any(c.id == "precision-pip-1" for c in cs) + + +# ── Multi-step: each step gets its own concern id ──────────────────────────── + + +def test_multiple_steps_each_get_unique_concern_ids(): + steps = [ + _step(1, "pip install requests"), + _step(2, "pip install flask"), + ] + cs = _precision_concerns(steps) + ids = {c.id for c in cs if c.id.startswith("precision-pip-")} + assert len(ids) == 2