From 13432f487ce057b7006b35ca20b530dc5b854b5c Mon Sep 17 00:00:00 2001 From: omit-test Date: Sun, 24 May 2026 00:24:29 +0200 Subject: [PATCH 1/6] fix pr289 code review findings --- README.md | 2 + .../TDD_EVIDENCE.md | 6 +++ .../specfact-code-review/module-package.yaml | 5 +-- .../src/specfact_code_review/run/commands.py | 45 ++++++++++++++++--- .../src/specfact_code_review/run/findings.py | 4 +- .../tools/ai_bloat_runner.py | 4 ++ .../specfact_code_review/run/test_commands.py | 45 ++++++++++++++----- .../tools/test_ai_bloat_runner.py | 17 +++++++ 8 files changed, 105 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index d40a1f4..cb03ec7 100644 --- a/README.md +++ b/README.md @@ -79,6 +79,8 @@ The pre-commit hook auto-runs this step and re-stages updated manifests on non-` Use semantic versioning per bundle payload: patch for bug fixes and backward-compatible metadata or schema additions, minor for additive commands or public API capabilities, and major for breaking command, API, or schema changes. +Manifest `integrity.checksum` values cover the canonical module source payload; registry +`checksum_sha256` and `.tar.gz.sha256` files cover the published tarball artifact, so these hashes can differ. ### When signatures are required diff --git a/openspec/changes/code-review-12-guided-simplification-enforcement/TDD_EVIDENCE.md b/openspec/changes/code-review-12-guided-simplification-enforcement/TDD_EVIDENCE.md index b79c2c0..4823bc9 100644 --- a/openspec/changes/code-review-12-guided-simplification-enforcement/TDD_EVIDENCE.md +++ b/openspec/changes/code-review-12-guided-simplification-enforcement/TDD_EVIDENCE.md @@ -22,6 +22,10 @@ - Result: failed as expected before final detector tightening. - Evidence: 2 failed. - Missing contract areas: safe-mechanical dead-branch detection required the current duplicate guard to be terminal and have no `else`. +- `hatch run pytest tests/unit/specfact_code_review/tools/test_ai_bloat_runner.py::test_dead_branch_ignores_duplicate_guard_after_assignment tests/unit/specfact_code_review/run/test_commands.py::test_apply_simplification_fixes_keeps_dead_branch_after_assignment tests/unit/specfact_code_review/run/test_commands.py::test_run_review_once_applies_simplification_fixes_before_rerun -q` + - Result: failed as expected before PR 289 dev-branch review fixes. + - Evidence: 3 failed. + - Missing contract areas: duplicate guard state invalidation, dead-branch autofix state invalidation, and applied simplification evidence in the post-fix report. ## Passing After @@ -43,6 +47,8 @@ - Result after complexity cleanup: 3 passed. - `hatch run pytest tests/unit/specfact_code_review/tools/test_ai_bloat_runner.py::test_dead_branch_flags_duplicate_prior_return_guard tests/unit/specfact_code_review/tools/test_ai_bloat_runner.py::test_dead_branch_ignores_duplicate_guard_after_else_path tests/unit/specfact_code_review/tools/test_ai_bloat_runner.py::test_dead_branch_ignores_nonterminal_duplicate_guard tests/unit/specfact_code_review/tools/test_ai_bloat_runner.py::test_dead_branch_ignores_duplicate_guard_with_else tests/unit/specfact_code_review/tools/test_ai_bloat_runner.py::test_dead_branch_ignores_impure_duplicate_guard -q` - Result after final detector tightening: 5 passed. +- `hatch run pytest tests/unit/specfact_code_review/tools/test_ai_bloat_runner.py::test_dead_branch_ignores_duplicate_guard_after_assignment tests/unit/specfact_code_review/run/test_commands.py::test_apply_simplification_fixes_keeps_dead_branch_after_assignment tests/unit/specfact_code_review/run/test_commands.py::test_run_review_once_applies_simplification_fixes_before_rerun -q` + - Result after PR 289 dev-branch review fixes: 3 passed. - `hatch run contract-test` - Result after PR review fixes: 758 passed, 2 warnings. - `hatch run smart-test` diff --git a/packages/specfact-code-review/module-package.yaml b/packages/specfact-code-review/module-package.yaml index 536fb50..441392b 100644 --- a/packages/specfact-code-review/module-package.yaml +++ b/packages/specfact-code-review/module-package.yaml @@ -1,5 +1,5 @@ name: nold-ai/specfact-code-review -version: 0.47.24 +version: 0.47.25 commands: - code tier: official @@ -23,5 +23,4 @@ description: Official SpecFact code review bundle package. category: codebase bundle_group_command: code integrity: - checksum: sha256:32bdb881f64482da2f10785b952e9ee39270aa4df34d27b24d068aae9a09d7a7 - signature: 8Uf+zsIzLokI9U4yVB10W1NtVFjI66HzKz3uzZPnqLwHi3eKrcDW9g6L97ncRU13gdOoQMvs/S2OQkNIRwaABA== + checksum: sha256:c4a3e6e322093e97eb7bbd3220c8e289a7fec0736ea9113fb9722779322dc64f diff --git a/packages/specfact-code-review/src/specfact_code_review/run/commands.py b/packages/specfact-code-review/src/specfact_code_review/run/commands.py index 0fbeabf..a2b9805 100644 --- a/packages/specfact-code-review/src/specfact_code_review/run/commands.py +++ b/packages/specfact-code-review/src/specfact_code_review/run/commands.py @@ -16,7 +16,7 @@ from rich.console import Console from rich.table import Table -from specfact_code_review.run.findings import ReviewFinding, ReviewReport +from specfact_code_review.run.findings import EvidenceRef, ReviewFinding, ReviewReport from specfact_code_review.run.runner import ReviewFocus, run_review @@ -280,23 +280,47 @@ def _apply_fixes(files: list[Path]) -> None: raise RuntimeError(f"Auto-fix command failed: {' '.join(command)}: {error_output}") -def _apply_simplification_fixes(report: ReviewReport) -> int: - """Apply deterministic safe-mechanical simplification rewrites from a report.""" +def _apply_simplification_fixes(report: ReviewReport) -> list[ReviewFinding]: + """Apply deterministic safe-mechanical simplification rewrites and return applied evidence.""" fixers: dict[str, Callable[[ReviewFinding], bool]] = { "ai-bloat.dead-branch": _apply_dead_branch_fix, "ai-bloat.pass-through-try-except": _apply_pass_through_try_except_fix, "ai-bloat.redundant-intermediate": _apply_redundant_intermediate_fix, "ai-bloat.verbose-bool-return": _apply_verbose_bool_return_fix, } - applied = 0 + applied: list[ReviewFinding] = [] for finding in _fixable_simplifications_by_stable_line_order(report.findings): fixer = fixers.get(finding.rule) if fixer is None: continue - applied += int(fixer(finding)) + if fixer(finding): + applied.append(_applied_simplification_finding(finding)) return applied +def _applied_simplification_finding(finding: ReviewFinding) -> ReviewFinding: + deletion_lines = max(1, finding.estimated_deletion_lines or 1) + before_ref = EvidenceRef(path=finding.file, start_line=finding.line, end_line=finding.line + deletion_lines - 1) + after_ref = EvidenceRef(path=finding.file, start_line=finding.line, end_line=finding.line) + return finding.model_copy( + update={ + "action_status": "applied", + "before_ref": before_ref, + "after_ref": after_ref, + "improvement": f"Applied safe-mechanical rewrite for {finding.rule}.", + } + ) + + +def _with_applied_simplification_findings(report: ReviewReport, applied_findings: list[ReviewFinding]) -> ReviewReport: + if not applied_findings: + return report + data = report.model_dump() + data["findings"] = [*report.findings, *applied_findings] + data["simplification_summary"] = None + return ReviewReport(**data) + + def _fixable_simplifications_by_stable_line_order(findings: list[ReviewFinding]) -> list[ReviewFinding]: indexed_findings = [ (index, finding) @@ -326,6 +350,7 @@ def _apply_duplicate_terminal_guard_fix( prior_terminal_tests: set[str] = set() for stmt in function_node.body: if not isinstance(stmt, ast.If) or not _is_pure_test(stmt.test): + prior_terminal_tests.clear() continue test_key = ast.dump(stmt.test, include_attributes=False) if _matches_duplicate_terminal_guard(stmt, finding.line, test_key, prior_terminal_tests): @@ -338,6 +363,8 @@ def _apply_duplicate_terminal_guard_fix( ) if _terminal_return(stmt.body) and not stmt.orelse: prior_terminal_tests.add(test_key) + else: + prior_terminal_tests.clear() return False @@ -634,14 +661,16 @@ def _run_review_with_status( review_focus=flags.review_focus, ) report = _run_review_once(files, base) + applied_simplification_findings: list[ReviewFinding] = [] if flags.fix: if flags.review_focus == "simplify": status.update("Applying safe mechanical simplification fixes...") - _apply_simplification_fixes(report) + applied_simplification_findings = _apply_simplification_fixes(report) status.update("Applying Ruff autofixes...") _apply_fixes(files) status.update("Re-running review after autofixes...") report = _run_review_once(files, base) + report = _with_applied_simplification_findings(report, applied_simplification_findings) return report @@ -656,13 +685,14 @@ def _run_review_once(files: list[Path], flags: _ReviewLoopFlags) -> ReviewReport review_level=flags.review_level, focus=flags.review_focus, ) + applied_simplification_findings: list[ReviewFinding] = [] if flags.fix: if flags.review_focus == "simplify": if flags.progress_callback is not None: flags.progress_callback("Applying safe mechanical simplification fixes...") else: progress_console.print("[dim]Applying safe mechanical simplification fixes...[/dim]") - _apply_simplification_fixes(report) + applied_simplification_findings = _apply_simplification_fixes(report) if flags.progress_callback is not None: flags.progress_callback("Applying Ruff autofixes...") else: @@ -682,6 +712,7 @@ def _run_review_once(files: list[Path], flags: _ReviewLoopFlags) -> ReviewReport review_level=flags.review_level, focus=flags.review_focus, ) + report = _with_applied_simplification_findings(report, applied_simplification_findings) return report diff --git a/packages/specfact-code-review/src/specfact_code_review/run/findings.py b/packages/specfact-code-review/src/specfact_code_review/run/findings.py index 20426bd..50a00d1 100644 --- a/packages/specfact-code-review/src/specfact_code_review/run/findings.py +++ b/packages/specfact-code-review/src/specfact_code_review/run/findings.py @@ -322,9 +322,7 @@ def _normalize_timestamp(cls, value: datetime) -> datetime: def _derive_governance_fields(self) -> ReviewReport: if self.simplification_summary is None: self.simplification_summary = _build_simplification_summary(self.findings) - if self.simplification_summary is not None or any( - finding.has_guided_simplification_metadata() for finding in self.findings - ): + if self.simplification_summary is not None: self.schema_version = "1.2" elif any(finding.has_simplification_metadata() for finding in self.findings): self.schema_version = "1.1" diff --git a/packages/specfact-code-review/src/specfact_code_review/tools/ai_bloat_runner.py b/packages/specfact-code-review/src/specfact_code_review/tools/ai_bloat_runner.py index 10174c1..db453c1 100644 --- a/packages/specfact-code-review/src/specfact_code_review/tools/ai_bloat_runner.py +++ b/packages/specfact-code-review/src/specfact_code_review/tools/ai_bloat_runner.py @@ -217,8 +217,10 @@ def _dead_branch_findings( prior_terminal_tests: set[str] = set() for stmt in function_node.body: if not isinstance(stmt, ast.If): + prior_terminal_tests.clear() continue if not _is_pure_test(stmt.test): + prior_terminal_tests.clear() continue test_key = ast.dump(stmt.test, include_attributes=False) if test_key in prior_terminal_tests and _terminal_return(stmt.body) and not stmt.orelse: @@ -249,6 +251,8 @@ def _dead_branch_findings( ) if _terminal_return(stmt.body) and not stmt.orelse: prior_terminal_tests.add(test_key) + else: + prior_terminal_tests.clear() return findings diff --git a/tests/unit/specfact_code_review/run/test_commands.py b/tests/unit/specfact_code_review/run/test_commands.py index 9fc8a36..e8bbee2 100644 --- a/tests/unit/specfact_code_review/run/test_commands.py +++ b/tests/unit/specfact_code_review/run/test_commands.py @@ -323,7 +323,7 @@ def test_apply_simplification_fixes_inlines_redundant_intermediate(tmp_path: Pat _safe_mechanical_report(target, line=2, rule="ai-bloat.redundant-intermediate") ) - assert applied == 1 + assert len(applied) == 1 assert target.read_text(encoding="utf-8") == "def total(values: list[int]) -> int:\n return sum(values)\n" @@ -336,7 +336,7 @@ def test_apply_simplification_fixes_skips_non_safe_guidance(tmp_path: Path) -> N applied = run_commands._apply_simplification_fixes(report) - assert applied == 0 + assert len(applied) == 0 assert target.read_text(encoding="utf-8") == source @@ -351,7 +351,7 @@ def test_apply_simplification_fixes_collapses_verbose_bool_return(tmp_path: Path _safe_mechanical_report(target, line=2, rule="ai-bloat.verbose-bool-return") ) - assert applied == 1 + assert len(applied) == 1 assert target.read_text(encoding="utf-8") == "def allowed(role: str) -> bool:\n return role == 'admin'\n" @@ -371,7 +371,7 @@ def test_apply_simplification_fixes_removes_dead_branch(tmp_path: Path) -> None: _safe_mechanical_report(target, line=4, rule="ai-bloat.dead-branch") ) - assert applied == 1 + assert len(applied) == 1 assert target.read_text(encoding="utf-8") == ( "def classify(value: int) -> str:\n if value > 10:\n return 'large'\n return 'small'\n" ) @@ -395,7 +395,7 @@ def test_apply_simplification_fixes_keeps_dead_branch_with_else(tmp_path: Path) _safe_mechanical_report(target, line=4, rule="ai-bloat.dead-branch") ) - assert applied == 0 + assert len(applied) == 0 assert target.read_text(encoding="utf-8") == source @@ -415,7 +415,28 @@ def test_apply_simplification_fixes_keeps_impure_duplicate_guard(tmp_path: Path) _safe_mechanical_report(target, line=4, rule="ai-bloat.dead-branch") ) - assert applied == 0 + assert len(applied) == 0 + assert target.read_text(encoding="utf-8") == source + + +def test_apply_simplification_fixes_keeps_dead_branch_after_assignment(tmp_path: Path) -> None: + target = tmp_path / "sample.py" + source = ( + "def classify(value: int) -> str:\n" + " if value > 10:\n" + " return 'large'\n" + " value = 12\n" + " if value > 10:\n" + " return 'now large'\n" + " return 'small'\n" + ) + target.write_text(source, encoding="utf-8") + + applied = run_commands._apply_simplification_fixes( + _safe_mechanical_report(target, line=5, rule="ai-bloat.dead-branch") + ) + + assert len(applied) == 0 assert target.read_text(encoding="utf-8") == source @@ -434,7 +455,7 @@ def test_apply_simplification_fixes_removes_pass_through_try_except(tmp_path: Pa _safe_mechanical_report(target, line=2, rule="ai-bloat.pass-through-try-except") ) - assert applied == 1 + assert len(applied) == 1 assert target.read_text(encoding="utf-8") == "def parse(raw: str) -> object:\n return parse_json(raw)\n" @@ -466,7 +487,7 @@ def test_apply_simplification_fixes_uses_bottom_up_line_order(tmp_path: Path) -> applied = run_commands._apply_simplification_fixes(report) - assert applied == 2 + assert len(applied) == 2 assert target.read_text(encoding="utf-8") == ( "def total(values: list[int]) -> int:\n" " return sum(values)\n" @@ -487,7 +508,7 @@ def test_apply_simplification_fixes_skips_when_source_no_longer_matches(tmp_path _safe_mechanical_report(target, line=2, rule="ai-bloat.redundant-intermediate") ) - assert applied == 0 + assert len(applied) == 0 assert target.read_text(encoding="utf-8") == source @@ -518,7 +539,11 @@ def test_run_review_once_applies_simplification_fixes_before_rerun(monkeypatch: ), ) - assert report.findings == [] + assert len(report.findings) == 1 + assert report.findings[0].action_status == "applied" + assert report.findings[0].before_ref is not None + assert report.findings[0].after_ref is not None + assert report.findings[0].improvement is not None assert target.read_text(encoding="utf-8") == "def total(values: list[int]) -> int:\n return sum(values)\n" diff --git a/tests/unit/specfact_code_review/tools/test_ai_bloat_runner.py b/tests/unit/specfact_code_review/tools/test_ai_bloat_runner.py index c388ad1..13493cd 100644 --- a/tests/unit/specfact_code_review/tools/test_ai_bloat_runner.py +++ b/tests/unit/specfact_code_review/tools/test_ai_bloat_runner.py @@ -120,6 +120,23 @@ def classify(value: int) -> str: assert run_ai_bloat([target]) == [] +def test_dead_branch_ignores_duplicate_guard_after_assignment(tmp_path: Path) -> None: + target = _write( + tmp_path, + """ +def classify(value: int) -> str: + if value > 10: + return "large" + value = 12 + if value > 10: + return "now large" + return "small" +""", + ) + + assert run_ai_bloat([target]) == [] + + def test_dead_branch_ignores_impure_duplicate_guard(tmp_path: Path) -> None: target = _write( tmp_path, From 3584a6f59099e08fb0017239aa8ce0e3f0be3771 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 23 May 2026 22:25:01 +0000 Subject: [PATCH 2/6] chore(modules): ci sign changed modules --- packages/specfact-code-review/module-package.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/specfact-code-review/module-package.yaml b/packages/specfact-code-review/module-package.yaml index 441392b..997bf3b 100644 --- a/packages/specfact-code-review/module-package.yaml +++ b/packages/specfact-code-review/module-package.yaml @@ -24,3 +24,4 @@ category: codebase bundle_group_command: code integrity: checksum: sha256:c4a3e6e322093e97eb7bbd3220c8e289a7fec0736ea9113fb9722779322dc64f + signature: 5u/SY65k4sfeOlyUXszvUUuo9VNo+szD6z7GOUT51gZiEHsQOqb2N/EzLxgHb0hTnGmEftzLyChFk6fMMBuKCg== From b44db19fbc78214e227d018f821f995a2a87401d Mon Sep 17 00:00:00 2001 From: omit-test Date: Sun, 24 May 2026 00:50:32 +0200 Subject: [PATCH 3/6] improve simplify prompt guidance --- .vibe/skills/specfact-code-review/SKILL.md | 17 ++++- docs/bundles/code-review/run.md | 11 +++ docs/modules/code-review.md | 25 +++++-- .../TDD_EVIDENCE.md | 20 ++++-- .../specfact-code-review/module-package.yaml | 3 +- .../skills/specfact-code-review/SKILL.md | 18 ++--- .../specfact_code_review/review/commands.py | 33 +++++++++ packages/specfact-project/module-package.yaml | 5 +- .../resources/prompts/specfact.08-simplify.md | 28 ++++++-- skills/specfact-code-review/SKILL.md | 17 ++++- .../review/test_commands.py | 18 +++++ tests/unit/test_guided_simplify_resources.py | 69 ++++++++++++++----- 12 files changed, 209 insertions(+), 55 deletions(-) diff --git a/.vibe/skills/specfact-code-review/SKILL.md b/.vibe/skills/specfact-code-review/SKILL.md index 61bba0c..7f5bcc1 100644 --- a/.vibe/skills/specfact-code-review/SKILL.md +++ b/.vibe/skills/specfact-code-review/SKILL.md @@ -1,20 +1,27 @@ --- name: specfact-code-review -description: House rules for AI coding sessions derived from review findings +description: Use for SpecFact code review workflows, especially when the user asks to remove AI bloat, simplify code, apply clean-code patterns, reduce boilerplate, fix review findings, or interpret SpecFact guidance. allowed-tools: [] --- -# House Rules - AI Coding Context (v4) +# SpecFact Code Review Skill Updated: 2026-05-22 | Module: nold-ai/specfact-code-review +Use this skill as an interactive cleanup coach, not a raw lint executor. When a user says "remove AI bloat", "simplify", "apply clean code", "fix SpecFact review", or similar, run the SpecFact review workflow, explain decisions in the user's language, show exact patch previews, and validate after small changes. + ## DO +- Treat `specfact code review run --help` as authoritative; use `--instructions` as the fallback AI workflow when prompts/skills are unavailable - For simplification queues, run `specfact code review run --scope changed --focus simplify --json --out .specfact/code-review-simplify.json` - Ask for walkthrough level when interactive: vibe coder, junior developer, senior/pro, or headless agent; auto-adjust if obvious -- Interpret `guidance_kind`: `safe_mechanical` may apply after local safety checks, `needs_tests` requires tests first, `design_judgment` needs human choice, `preserve` means keep and log `preserve_reason` +- For vibe coders, present each finding as a decision card: plain-language issue, why it might need to stay, exact patch preview, validation plan, and recommended choice +- Interpret `guidance_kind`: `safe_mechanical` may apply after local safety checks, `needs_tests` requires tests first, `design_judgment` needs human choice with intent evidence, `preserve` means keep and log `preserve_reason` +- For `design_judgment`, inspect API, callback, framework hook, adapter, public symbol, CLI boundary, compatibility shim, and readability intent; if intent is unclear, default to keep or skip - Log each simplification action as recommended, applied, kept, skipped, failed, with evidence of improvement or preserved contract - In headless mode, process one file at a time and emit an action table: file, line, rule, guidance_kind, recommended_action, action_status, evidence +- Run targeted tests or rerun simplify review after each accepted file or very small batch; if validation cannot prove safety, downgrade to `needs_tests` or `skipped` +- For merge-quality review, run `specfact code review run --scope changed --bug-hunt --json --out .specfact/code-review.json` - Ask whether tests should be included before repo-wide review; default to excluding tests unless test changes are the target - Use intention-revealing names; avoid placeholder public names like data/process/handle - Keep functions under 120 LOC, shallow nesting, and <= 5 parameters (KISS) @@ -28,6 +35,10 @@ Updated: 2026-05-22 | Module: nold-ai/specfact-code-review ## DON'T +- Don't copy prompt templates into AI IDEs when this installed skill can carry the reusable workflow guidance +- Don't treat simplification findings as AI-authorship proof or apply batch rewrites without explicit user approval +- Don't ask non-expert users to infer code intent from a raw warning; provide the evidence and safest recommendation +- Don't apply `design_judgment` findings just because the patch looks shorter - Don't enable known noisy findings unless you explicitly want strict/full review output - Don't use bare except: or except Exception: pass - Don't add # noqa / # type: ignore without inline justification diff --git a/docs/bundles/code-review/run.md b/docs/bundles/code-review/run.md index 67bb885..928494f 100644 --- a/docs/bundles/code-review/run.md +++ b/docs/bundles/code-review/run.md @@ -40,6 +40,7 @@ The pipeline reviews **`.py`** and **`.pyi`** only. The **`--focus docs`** facet | `--no-tests` | Skip the TDD gate | | `--fix` | Apply Ruff autofixes, then rerun the review | | `--interactive` | Prompt for scope decisions before execution | +| `--instructions` | Print AI-facing simplify / clean-code workflow instructions and exit without running review | ## Invalid combinations @@ -98,6 +99,16 @@ specfact code review run --scope full --focus docs specfact code review run --scope changed --focus simplify --json --out .specfact/code-review-simplify.json ``` +### AI instructions fallback + +When an IDE does not support bundled prompts or skills, print the same guided simplify workflow for an AI assistant: + +```bash +specfact code review run --instructions +``` + +The output explains how to remove AI bloat and apply clean-code simplifications using SpecFact evidence, including `safe_mechanical`, `needs_tests`, `design_judgment`, and `preserve` handling, patch previews, conservative keep/skip defaults, and per-file validation. + ### Positional files (explicit Python paths) Do not pass **`--scope`** or **`--path`** when **`FILES...`** are present. diff --git a/docs/modules/code-review.md b/docs/modules/code-review.md index 4eb1bd2..cd52af2 100644 --- a/docs/modules/code-review.md +++ b/docs/modules/code-review.md @@ -61,6 +61,8 @@ Options (aligned with `specfact code review run --help`): - `--fix`: apply Ruff autofixes and re-run the review before printing results - `--interactive`: ask whether changed test files should be included before auto-detected review runs +- `--instructions`: print AI-facing simplify / clean-code workflow instructions + and exit without requiring installed slash prompts or skills ### Invalid combinations @@ -114,6 +116,18 @@ The review pipeline also emits `ai_bloat` findings for code shapes commonly ampl These findings are `severity=info`, advisory-only, and score-neutral. They are written to `.specfact/code-review.json` when the report includes all severities; for simplification queues, write `.specfact/code-review-simplify.json` with `--focus simplify` so `/specfact.08-simplify` can filter them by `category=ai_bloat` for per-change confirmed rewrites. They do not claim AI authorship; they identify simplification candidates. +For the lowest-friction AI onboarding path, start with the built-in instruction +printer instead of requiring a user to install IDE prompts or skills first: + +```bash +specfact code review run --instructions +``` + +Paste that output into any AI coding assistant and ask it to simplify or remove +AI bloat with SpecFact. The instructions explain the expected report file, +`guidance_kind` handling, patch-preview decision cards, conservative defaults +for `design_judgment`, and per-file validation. + Positional `FILES...` cannot be mixed with **`--scope`** or **`--path`** (see **Invalid combinations** above). @@ -399,11 +413,12 @@ Then rerun the ledger command from the same repository checkout. ## Code review skill The `specfact-code-review` bundle ships a compact `SKILL.md` for Codex CLI, -Claude, Vibe, and Cursor-compatible IDEs. Use it as the reusable alternative to -copying prompt templates into every AI IDE: it carries the CLI-grounded review -workflow, simplification queue guidance, self-healing `--help` behavior, and -house rules derived from the reward ledger. The default charter encodes the -clean-code principles directly: +Claude, Vibe, and Cursor-compatible IDEs. New users do not need to install it +first: `specfact code review run --instructions` prints the same guided +simplify workflow for any AI assistant. Install the skill later when the IDE +supports automatic skill loading and you want the reusable workflow attached to +phrases such as "remove AI bloat", "simplify", or "apply clean-code patterns". +The default charter encodes the clean-code principles directly: - Naming: use intention-revealing names instead of placeholders. - KISS: keep functions small, shallow, and narrow in parameters. diff --git a/openspec/changes/code-review-12-guided-simplification-enforcement/TDD_EVIDENCE.md b/openspec/changes/code-review-12-guided-simplification-enforcement/TDD_EVIDENCE.md index 4823bc9..8e8c930 100644 --- a/openspec/changes/code-review-12-guided-simplification-enforcement/TDD_EVIDENCE.md +++ b/openspec/changes/code-review-12-guided-simplification-enforcement/TDD_EVIDENCE.md @@ -49,6 +49,14 @@ - Result after final detector tightening: 5 passed. - `hatch run pytest tests/unit/specfact_code_review/tools/test_ai_bloat_runner.py::test_dead_branch_ignores_duplicate_guard_after_assignment tests/unit/specfact_code_review/run/test_commands.py::test_apply_simplification_fixes_keeps_dead_branch_after_assignment tests/unit/specfact_code_review/run/test_commands.py::test_run_review_once_applies_simplification_fixes_before_rerun -q` - Result after PR 289 dev-branch review fixes: 3 passed. +- `hatch run pytest tests/unit/test_guided_simplify_resources.py -q` + - Result after prompt/skill user-experience tightening: 2 passed. +- `hatch run validate-prompt-commands` + - Result after prompt/skill user-experience tightening: prompt command validation passed with no findings. +- `hatch run pytest tests/unit/specfact_code_review/review/test_commands.py::test_review_run_help_lists_simplify_focus tests/unit/specfact_code_review/review/test_commands.py::test_review_run_instructions_prints_ai_workflow_without_running_review tests/unit/docs/test_code_review_docs_parity.py::test_code_review_run_doc_mentions_public_ty_options tests/unit/test_guided_simplify_resources.py tests/unit/specfact_code_review/rules/test_updater.py::test_load_bundled_skill_content_returns_valid_structure_when_available -q` + - Result after adding the AI instructions fallback and docs: 6 passed. +- `hatch run specfact code review run --instructions` + - Result after adding the AI instructions fallback: printed the guided simplify / clean-code workflow and exited successfully without running review analysis. - `hatch run contract-test` - Result after PR review fixes: 758 passed, 2 warnings. - `hatch run smart-test` @@ -56,19 +64,21 @@ - `hatch run type-check` - Result: 0 errors, 0 warnings, 0 notes. - `hatch run lint` - - Result: 10.00/10. + - Result after AI instructions fallback: 10.00/10. - `hatch run yaml-lint` - - Result: validated 6 manifests and `registry/index.json`. + - Result after AI instructions fallback: validated 6 manifests and `registry/index.json`. - `hatch run check-bundle-imports` - Result: import boundary check passed. - `hatch run validate-prompt-commands` - Result: prompt command validation passed with no findings. - `hatch run verify-modules-signature --payload-from-filesystem --enforce-version-bump --version-check-base origin/dev` - - Result after PR review fixes: verified 6 module manifests. + - Result after AI instructions fallback: verified 6 module manifests. - `hatch run specfact code review run --bug-hunt --json --out .specfact/code-review.json --scope changed` - Result after final PR review fixes: PASS, CI exit 0, score 120, 0 findings. +- `hatch run specfact code review run --bug-hunt --include-tests --json --out .specfact/code-review.json --scope changed` + - Result after AI instructions fallback: PASS, CI exit 0, 0 findings. - `openspec validate code-review-12-guided-simplification-enforcement --strict` - - Result: valid. + - Result after AI instructions fallback: valid. ## Local Dev-Link Validation @@ -94,4 +104,4 @@ ## Signing Note -`hatch run verify-modules-signature --payload-from-filesystem --require-signature --enforce-version-bump --version-check-base origin/main` passed before the final source edits, verifying the existing `0.47.23` signature was a real cryptographic signature. The final local payload was then bumped to `0.47.24` and refreshed with `hatch run sign-modules --changed-only --base-ref origin/dev --bump-version patch --allow-unsigned --payload-from-filesystem`, because no private signing key is available in the local worktree. Cryptographic signature restoration remains an approval-time or post-merge signing step. +`hatch run verify-modules-signature --payload-from-filesystem --require-signature --enforce-version-bump --version-check-base origin/main` passed before the final source edits, verifying the existing `0.47.23` signature was a real cryptographic signature. The final local payload is refreshed at `0.47.25` for `specfact-code-review` and `0.41.16` for `specfact-project` with `hatch run sign-modules --changed-only --base-ref origin/dev --bump-version patch --allow-unsigned --payload-from-filesystem`, because no private signing key is available in the local worktree. Cryptographic signature restoration remains an approval-time or post-merge signing step. diff --git a/packages/specfact-code-review/module-package.yaml b/packages/specfact-code-review/module-package.yaml index 997bf3b..0f84204 100644 --- a/packages/specfact-code-review/module-package.yaml +++ b/packages/specfact-code-review/module-package.yaml @@ -23,5 +23,4 @@ description: Official SpecFact code review bundle package. category: codebase bundle_group_command: code integrity: - checksum: sha256:c4a3e6e322093e97eb7bbd3220c8e289a7fec0736ea9113fb9722779322dc64f - signature: 5u/SY65k4sfeOlyUXszvUUuo9VNo+szD6z7GOUT51gZiEHsQOqb2N/EzLxgHb0hTnGmEftzLyChFk6fMMBuKCg== + checksum: sha256:668408981debae406bd6758c3b7191e5cbb98976f659069dded90afe04c1a8b3 diff --git a/packages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.md b/packages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.md index 7e3464b..a642508 100644 --- a/packages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.md +++ b/packages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.md @@ -1,22 +1,22 @@ --- name: specfact-code-review -description: CLI-grounded SpecFact code review workflow and house rules for AI coding sessions +description: Use for SpecFact code review workflows, especially when the user asks to remove AI bloat, simplify code, apply clean-code patterns, reduce boilerplate, fix review findings, or interpret SpecFact guidance. allowed-tools: [] --- - -# House Rules - AI Coding Context / SpecFact Code Review Skill (v2) - +# SpecFact Code Review Skill Updated: 2026-05-22 | Module: nold-ai/specfact-code-review +Use this skill as an interactive cleanup coach, not a raw lint executor. When a user says "remove AI bloat", "simplify", "apply clean code", "fix SpecFact review", or similar, run the SpecFact review workflow, explain decisions in the user's language, show exact patch previews, and validate after small changes. ## DO -- Use this skill when asked to run, interpret, or act on SpecFact code review in Codex CLI or another AI IDE -- Treat `specfact code review run --help` as authoritative; self-heal stale options by checking help before changing workflow +- Treat `specfact code review run --help` as authoritative; use `--instructions` as the fallback AI workflow when prompts/skills are unavailable - For simplification queues, run `specfact code review run --scope changed --focus simplify --json --out .specfact/code-review-simplify.json` - Ask for walkthrough level when interactive: vibe coder, junior developer, senior/pro, or headless agent; auto-adjust if obvious -- Interpret `guidance_kind`: `safe_mechanical` may apply after local safety checks, `needs_tests` requires tests first, `design_judgment` needs human choice, `preserve` means keep and log `preserve_reason` +- For vibe coders, present each finding as a decision card: plain-language issue, why it might need to stay, exact patch preview, validation plan, and recommended choice +- Interpret `guidance_kind`: `safe_mechanical` may apply after local safety checks, `needs_tests` requires tests first, `design_judgment` needs human choice with intent evidence, `preserve` means keep and log `preserve_reason` +- For `design_judgment`, inspect API, callback, framework hook, adapter, public symbol, CLI boundary, compatibility shim, and readability intent; if intent is unclear, default to keep or skip - Log each simplification action as recommended, applied, kept, skipped, failed, with evidence of improvement or preserved contract - In headless mode, process one file at a time and emit an action table: file, line, rule, guidance_kind, recommended_action, action_status, evidence +- Run targeted tests or rerun simplify review after each accepted file or very small batch; if validation cannot prove safety, downgrade to `needs_tests` or `skipped` - For merge-quality review, run `specfact code review run --scope changed --bug-hunt --json --out .specfact/code-review.json` -- Ask whether tests should be included before repo-wide review; default to excluding tests unless test changes are the target - Use intention-revealing names; avoid placeholder public names like data/process/handle - Keep functions under 120 LOC, shallow nesting, and <= 5 parameters (KISS) - Delete unused private helpers and speculative abstractions quickly (YAGNI) @@ -28,6 +28,8 @@ Updated: 2026-05-22 | Module: nold-ai/specfact-code-review ## DON'T - Don't copy prompt templates into AI IDEs when this installed skill can carry the reusable workflow guidance - Don't treat simplification findings as AI-authorship proof or apply batch rewrites without explicit user approval +- Don't ask non-expert users to infer code intent from a raw warning; provide the evidence and safest recommendation +- Don't apply `design_judgment` findings just because the patch looks shorter - Don't enable known noisy findings unless you explicitly want strict/full review output - Don't use bare except: or except Exception: pass - Don't add # noqa / # type: ignore without inline justification diff --git a/packages/specfact-code-review/src/specfact_code_review/review/commands.py b/packages/specfact-code-review/src/specfact_code_review/review/commands.py index 3dc39e0..2b80b35 100644 --- a/packages/specfact-code-review/src/specfact_code_review/review/commands.py +++ b/packages/specfact-code-review/src/specfact_code_review/review/commands.py @@ -25,6 +25,31 @@ app = typer.Typer(help="Code command extensions for structured review workflows.", no_args_is_help=True) review_app = typer.Typer(help="Governed code review workflows.", no_args_is_help=True) +_RUN_INSTRUCTIONS = """\ +SpecFact code review instructions for AI assistants + +Use this when the user asks to remove AI bloat, simplify code, apply clean-code patterns, reduce boilerplate, or act on SpecFact review findings. + +1. Generate evidence first: + specfact code review run --scope changed --focus simplify --json --out .specfact/code-review-simplify.json + +2. Treat guidance_kind as the action contract: + - safe_mechanical: apply only after local safety checks pass. + - needs_tests: add or identify targeted tests before changing behavior. + - design_judgment: inspect intent evidence and ask before editing. + - preserve: keep by default and record preserve_reason. + +3. For vibe-coder or junior users, present each finding as a decision card: + Finding, plain-language issue, why it might need to stay, exact patch preview, validation plan, recommended choice. + +4. For design_judgment findings, check API, callback, framework hook, adapter, public symbol, CLI boundary, compatibility shim, and readability intent. If intent is unclear, default to keep or skip. + +5. Apply one file at a time. After each accepted file or very small batch, run targeted tests or rerun: + specfact code review run --scope changed --focus simplify --json --out .specfact/code-review-simplify.json + +6. Log every action as recommended, applied, kept, skipped, or failed with evidence. Never batch-apply design_judgment findings just because the patch is shorter. +""" + @dataclass(frozen=True) class _ReviewRunCliInputs: @@ -115,9 +140,17 @@ def run( no_tests: bool = typer.Option(False, "--no-tests"), fix: bool = typer.Option(False, "--fix"), interactive: bool = typer.Option(False, "--interactive"), + instructions: bool = typer.Option( + False, + "--instructions", + help="Print AI-facing instructions for guided simplify / clean-code review and exit.", + ), ) -> None: """Run the full code review workflow.""" _ = ctx.resilient_parsing + if instructions: + typer.echo(_RUN_INSTRUCTIONS) + raise typer.Exit(code=0) focus_list, resolved_include_tests, resolved_include_noise = _resolve_review_run_flags( _ReviewRunCliInputs( files=files, diff --git a/packages/specfact-project/module-package.yaml b/packages/specfact-project/module-package.yaml index fb55ab0..615eb05 100644 --- a/packages/specfact-project/module-package.yaml +++ b/packages/specfact-project/module-package.yaml @@ -1,5 +1,5 @@ name: nold-ai/specfact-project -version: 0.41.15 +version: 0.41.16 commands: - project - plan @@ -27,5 +27,4 @@ core_compatibility: '>=0.40.0,<1.0.0' description: Official SpecFact project bundle package. bundle_group_command: project integrity: - checksum: sha256:1dea73c218b64924a003e14e155c25d238ff62d00b6b70cd8a92cbfc4e9834fa - signature: ybXH/L4RAztljN/nnD/R6EFVt4T5HS0FSFb3dKfmixzQLzbQZ7xjj95sY4Iu9qZywuLfd7V6f3s48mu5QPLMAQ== + checksum: sha256:4985bf438fb47aeced3d15a47866909a914492c52701a68e0c1ef5fef07ef8e2 diff --git a/packages/specfact-project/resources/prompts/specfact.08-simplify.md b/packages/specfact-project/resources/prompts/specfact.08-simplify.md index f50e8f1..32bec99 100644 --- a/packages/specfact-project/resources/prompts/specfact.08-simplify.md +++ b/packages/specfact-project/resources/prompts/specfact.08-simplify.md @@ -24,27 +24,30 @@ Simplify `ai_bloat` and metadata-backed simplification findings from `.specfact/ ## Guidance Character -Act as a conservative code-review simplification assistant. Use the Code Review bundle's deterministic findings as evidence, explain one cleanup at a time, and keep the user in control. Do not infer AI authorship and do not chase broad refactors. +Act as a conservative code-review simplification assistant for users who ask to remove AI bloat, simplify code, apply clean-code patterns, reduce boilerplate, or work through SpecFact simplification findings. Use the Code Review bundle's deterministic findings as evidence, explain one cleanup at a time, and keep the user in control. Do not infer AI authorship and do not chase broad refactors. Before walking findings, ask for the walkthrough level unless the user already specified it: -- `vibe coder`: explain why the finding matters, what to check, and what will change in plain language. +- `vibe coder`: make this an interactive cleanup session. Explain why the finding matters, what could break, what exact patch you propose, and which test or review check will prove it stayed safe. - `junior developer`: explain the clean-code principle, the safety checks, and the exact edit. - `senior/pro`: keep guidance concise and focus on contract risk, blast radius, and verification. - `headless agent`: do not ask interactive questions; choose the safest flow from metadata and write a concise action log. Auto-adjust if the conversation makes the level obvious. +For `design_judgment`, unknown intent defaults to keep or skip. Do not ask a vibe coder to infer architecture intent from a raw warning. Instead, inspect and explain whether the code appears to preserve an API, callback signature, framework hook, adapter seam, public symbol, CLI boundary, readability name, or compatibility shim. If that evidence is absent, propose a small patch preview and ask for approval. + ## CLI Grounding Before reading or editing source, verify the current command surface when needed: ```bash specfact code review run --help +specfact code review run --instructions specfact code review run --scope changed --focus simplify --json --out .specfact/code-review-simplify.json ``` -If `--focus simplify` is unavailable in the installed CLI, self-heal by inspecting `specfact code review run --help`, then run the closest non-destructive JSON review command that preserves advisory findings, usually without `--level error`. +If this slash prompt or the installed skill is unavailable in another AI IDE, tell the user they can run `specfact code review run --instructions` and paste that output to the AI assistant. If `--focus simplify` is unavailable in the installed CLI, self-heal by inspecting `specfact code review run --help`, then run the closest non-destructive JSON review command that preserves advisory findings, usually without `--level error`. ## Workflow @@ -66,23 +69,34 @@ Group findings by `intent_key` first when present, then by file or domain and ru Use `guidance_kind` as the action contract: -- `safe_mechanical`: local, high-confidence cleanup; can be applied after checking the listed `safety_checks`. +- `safe_mechanical`: local, high-confidence cleanup; can be applied only after checking the listed `safety_checks` against current code. - `needs_tests`: only apply after targeted tests exist or are added for the behavior. -- `design_judgment`: explain tradeoffs and ask before editing. +- `design_judgment`: inspect intent evidence first, explain tradeoffs in plain language, default to keep/skip when intent is unclear, and ask before editing. - `preserve`: keep by default; record the `preserve_reason` as a false-positive or intentional-pattern note. +For vibe-coder and junior walkthroughs, present findings as a decision card instead of a raw lint warning: + +```text +Finding: at : +Plain-language issue: +Why it might need to stay: +Proposed patch preview: after summary or diff> +Validation plan: +Recommended choice: apply | keep | skip for now +``` + ### Step 3: Confirm Each Rewrite For each candidate: 1. Show file, line, rule, `guidance_kind`, `recommended_action`, clean-code principle, current snippet, and related locations. 2. Explain the rationale and the required `safety_checks` at the selected walkthrough level. -3. Draft the replacement or preserve decision. +3. Draft the exact replacement or preserve decision as a patch preview before editing. 4. Ask the user to choose: accept, reject, skip, or explain; use `keep` as the reject reason for `preserve` findings. In `headless agent` mode, apply only `safe_mechanical` items whose safety checks are locally provable. 5. Record `action_status` as one of: recommended, applied, kept, skipped, failed. Never batch multiple files into one confirmation in interactive mode. -Apply only accepted edits. +Apply only accepted edits. After each accepted file or very small batch, run the most targeted relevant test or review command before continuing. If tests are missing or too broad to prove safety, downgrade the action to `needs_tests` or `skipped` instead of applying a `design_judgment` rewrite. In `headless agent` mode, process candidates one file at a time and write this action log: diff --git a/skills/specfact-code-review/SKILL.md b/skills/specfact-code-review/SKILL.md index 61bba0c..7f5bcc1 100644 --- a/skills/specfact-code-review/SKILL.md +++ b/skills/specfact-code-review/SKILL.md @@ -1,20 +1,27 @@ --- name: specfact-code-review -description: House rules for AI coding sessions derived from review findings +description: Use for SpecFact code review workflows, especially when the user asks to remove AI bloat, simplify code, apply clean-code patterns, reduce boilerplate, fix review findings, or interpret SpecFact guidance. allowed-tools: [] --- -# House Rules - AI Coding Context (v4) +# SpecFact Code Review Skill Updated: 2026-05-22 | Module: nold-ai/specfact-code-review +Use this skill as an interactive cleanup coach, not a raw lint executor. When a user says "remove AI bloat", "simplify", "apply clean code", "fix SpecFact review", or similar, run the SpecFact review workflow, explain decisions in the user's language, show exact patch previews, and validate after small changes. + ## DO +- Treat `specfact code review run --help` as authoritative; use `--instructions` as the fallback AI workflow when prompts/skills are unavailable - For simplification queues, run `specfact code review run --scope changed --focus simplify --json --out .specfact/code-review-simplify.json` - Ask for walkthrough level when interactive: vibe coder, junior developer, senior/pro, or headless agent; auto-adjust if obvious -- Interpret `guidance_kind`: `safe_mechanical` may apply after local safety checks, `needs_tests` requires tests first, `design_judgment` needs human choice, `preserve` means keep and log `preserve_reason` +- For vibe coders, present each finding as a decision card: plain-language issue, why it might need to stay, exact patch preview, validation plan, and recommended choice +- Interpret `guidance_kind`: `safe_mechanical` may apply after local safety checks, `needs_tests` requires tests first, `design_judgment` needs human choice with intent evidence, `preserve` means keep and log `preserve_reason` +- For `design_judgment`, inspect API, callback, framework hook, adapter, public symbol, CLI boundary, compatibility shim, and readability intent; if intent is unclear, default to keep or skip - Log each simplification action as recommended, applied, kept, skipped, failed, with evidence of improvement or preserved contract - In headless mode, process one file at a time and emit an action table: file, line, rule, guidance_kind, recommended_action, action_status, evidence +- Run targeted tests or rerun simplify review after each accepted file or very small batch; if validation cannot prove safety, downgrade to `needs_tests` or `skipped` +- For merge-quality review, run `specfact code review run --scope changed --bug-hunt --json --out .specfact/code-review.json` - Ask whether tests should be included before repo-wide review; default to excluding tests unless test changes are the target - Use intention-revealing names; avoid placeholder public names like data/process/handle - Keep functions under 120 LOC, shallow nesting, and <= 5 parameters (KISS) @@ -28,6 +35,10 @@ Updated: 2026-05-22 | Module: nold-ai/specfact-code-review ## DON'T +- Don't copy prompt templates into AI IDEs when this installed skill can carry the reusable workflow guidance +- Don't treat simplification findings as AI-authorship proof or apply batch rewrites without explicit user approval +- Don't ask non-expert users to infer code intent from a raw warning; provide the evidence and safest recommendation +- Don't apply `design_judgment` findings just because the patch looks shorter - Don't enable known noisy findings unless you explicitly want strict/full review output - Don't use bare except: or except Exception: pass - Don't add # noqa / # type: ignore without inline justification diff --git a/tests/unit/specfact_code_review/review/test_commands.py b/tests/unit/specfact_code_review/review/test_commands.py index 2719a65..96aaf5b 100644 --- a/tests/unit/specfact_code_review/review/test_commands.py +++ b/tests/unit/specfact_code_review/review/test_commands.py @@ -22,6 +22,24 @@ def test_review_run_help_lists_simplify_focus() -> None: assert result.exit_code == 0 assert "simplify" in result.output + assert "--instructions" in result.output + + +def test_review_run_instructions_prints_ai_workflow_without_running_review(monkeypatch: Any) -> None: + def _fail_run_command(_files: list[Path], **_kwargs: object) -> tuple[int, str | None]: + raise AssertionError("run_command should not be called for --instructions") + + monkeypatch.setattr("specfact_code_review.review.commands.run_command", _fail_run_command) + + result = runner.invoke(app, ["review", "run", "--instructions"]) + + assert result.exit_code == 0 + assert "remove AI bloat" in result.output + assert "safe_mechanical" in result.output + assert "design_judgment" in result.output + assert "exact patch preview" in result.output + assert "default to keep or skip" in result.output + assert "specfact code review run --scope changed --focus simplify" in result.output def test_review_run_interactive_prompts_for_test_inclusion(monkeypatch: Any) -> None: diff --git a/tests/unit/test_guided_simplify_resources.py b/tests/unit/test_guided_simplify_resources.py index bcacec4..28e7997 100644 --- a/tests/unit/test_guided_simplify_resources.py +++ b/tests/unit/test_guided_simplify_resources.py @@ -15,31 +15,62 @@ ) +def _assert_contains_all(text: str, required: tuple[str, ...]) -> None: + missing = [item for item in required if item not in text] + + assert missing == [] + + def test_simplify_prompt_guides_interactive_walkthrough_levels() -> None: text = PROMPT.read_text(encoding="utf-8") - assert "vibe coder" in text - assert "junior developer" in text - assert "senior/pro" in text - assert "headless agent" in text - assert "safe_mechanical" in text - assert "needs_tests" in text - assert "design_judgment" in text - assert "preserve" in text - assert "recommended, applied, kept, skipped, failed" in text - assert "this report is the evidence file" in text - assert "| file | line | rule | guidance_kind | recommended_action | action_status | evidence |" in text + _assert_contains_all( + text, + ( + "vibe coder", + "junior developer", + "senior/pro", + "headless agent", + "safe_mechanical", + "needs_tests", + "design_judgment", + "preserve", + "recommended, applied, kept, skipped, failed", + "this report is the evidence file", + "decision card", + "Why it might need to stay", + "Proposed patch preview", + "Validation plan", + "unknown intent defaults to keep or skip", + "API, callback signature, framework hook, adapter seam, public symbol", + "| file | line | rule | guidance_kind | recommended_action | action_status | evidence |", + ), + ) def test_code_review_skill_teaches_llms_how_to_apply_simplification_guidance() -> None: for skill_path in SKILL_COPIES: text = skill_path.read_text(encoding="utf-8") - assert "Ask for walkthrough level" in text - assert "safe_mechanical" in text - assert "needs_tests" in text - assert "design_judgment" in text - assert "preserve" in text - assert "recommended, applied, kept, skipped, failed" in text - assert "In headless mode, process one file at a time" in text - assert "file, line, rule, guidance_kind, recommended_action, action_status, evidence" in text + _assert_contains_all( + text, + ( + "Ask for walkthrough level", + "safe_mechanical", + "needs_tests", + "design_judgment", + "preserve", + "recommended, applied, kept, skipped, failed", + "In headless mode, process one file at a time", + "file, line, rule, guidance_kind, recommended_action, action_status, evidence", + "remove AI bloat", + "apply clean code", + "interactive cleanup coach", + "decision card", + "exact patch preview", + "API, callback, framework hook, adapter, public symbol", + "default to keep or skip", + "Run targeted tests or rerun simplify review after each accepted file", + "Don't ask non-expert users to infer code intent from a raw warning", + ), + ) From 49be799f6652dac07ea957c918ba2d9e3f0d0ded Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 23 May 2026 22:55:45 +0000 Subject: [PATCH 4/6] chore(modules): ci sign changed modules --- packages/specfact-code-review/module-package.yaml | 1 + packages/specfact-project/module-package.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/specfact-code-review/module-package.yaml b/packages/specfact-code-review/module-package.yaml index 0f84204..26b54a6 100644 --- a/packages/specfact-code-review/module-package.yaml +++ b/packages/specfact-code-review/module-package.yaml @@ -24,3 +24,4 @@ category: codebase bundle_group_command: code integrity: checksum: sha256:668408981debae406bd6758c3b7191e5cbb98976f659069dded90afe04c1a8b3 + signature: 2UX+OBZKN0sxlE9z9U5oA47MocfYqMZM2khZoVMKM0ci3HJvh5/RYSWU1TuT+kbCgZmBeKsaB457KtWIxH+PBQ== diff --git a/packages/specfact-project/module-package.yaml b/packages/specfact-project/module-package.yaml index 615eb05..98b9675 100644 --- a/packages/specfact-project/module-package.yaml +++ b/packages/specfact-project/module-package.yaml @@ -28,3 +28,4 @@ description: Official SpecFact project bundle package. bundle_group_command: project integrity: checksum: sha256:4985bf438fb47aeced3d15a47866909a914492c52701a68e0c1ef5fef07ef8e2 + signature: od7YByTQZRdevYEK7FZjZqY+j6f13gmrnl3iFndP8oSZeNLj1ntdn083WA96hCmadYiKkFpws/kgDwZurGFIAA== From 94a0b5e7ef3201d2f99282d68bb1f438bba339b8 Mon Sep 17 00:00:00 2001 From: omit-test Date: Sun, 24 May 2026 01:06:52 +0200 Subject: [PATCH 5/6] clarify simplify instructions fallback --- docs/bundles/code-review/run.md | 2 +- docs/modules/code-review.md | 7 ++++++- .../TDD_EVIDENCE.md | 2 ++ packages/specfact-code-review/module-package.yaml | 5 ++--- .../src/specfact_code_review/review/commands.py | 5 ++++- packages/specfact-project/module-package.yaml | 5 ++--- tests/unit/specfact_code_review/review/test_commands.py | 3 +++ 7 files changed, 20 insertions(+), 9 deletions(-) diff --git a/docs/bundles/code-review/run.md b/docs/bundles/code-review/run.md index 928494f..767b461 100644 --- a/docs/bundles/code-review/run.md +++ b/docs/bundles/code-review/run.md @@ -107,7 +107,7 @@ When an IDE does not support bundled prompts or skills, print the same guided si specfact code review run --instructions ``` -The output explains how to remove AI bloat and apply clean-code simplifications using SpecFact evidence, including `safe_mechanical`, `needs_tests`, `design_judgment`, and `preserve` handling, patch previews, conservative keep/skip defaults, and per-file validation. +The output explains how to remove AI bloat and apply clean-code simplifications using SpecFact evidence, including `safe_mechanical`, `needs_tests`, `design_judgment`, and `preserve` handling, patch previews, conservative keep/skip defaults, and per-file validation. It also tells assistants how to handle clean PR branches where `--scope changed` has no worktree files: find branch-delta Python files with a base-ref diff such as `git diff --name-only origin/dev...HEAD -- '*.py' '*.pyi'`, review those files as explicit positional files, and treat findings without `guidance_kind` as unguided advisories rather than auto-fix input. ### Positional files (explicit Python paths) diff --git a/docs/modules/code-review.md b/docs/modules/code-review.md index cd52af2..7c0bd4b 100644 --- a/docs/modules/code-review.md +++ b/docs/modules/code-review.md @@ -126,7 +126,12 @@ specfact code review run --instructions Paste that output into any AI coding assistant and ask it to simplify or remove AI bloat with SpecFact. The instructions explain the expected report file, `guidance_kind` handling, patch-preview decision cards, conservative defaults -for `design_judgment`, and per-file validation. +for `design_judgment`, and per-file validation. They also cover clean PR +branches where `--scope changed` has no worktree files: the assistant should +find branch-delta Python files with a base-ref diff such as +`git diff --name-only origin/dev...HEAD -- '*.py' '*.pyi'`, review those files +as explicit positional files, and treat findings without `guidance_kind` as +unguided advisories, not auto-fix input. Positional `FILES...` cannot be mixed with **`--scope`** or **`--path`** (see **Invalid combinations** above). diff --git a/openspec/changes/code-review-12-guided-simplification-enforcement/TDD_EVIDENCE.md b/openspec/changes/code-review-12-guided-simplification-enforcement/TDD_EVIDENCE.md index 8e8c930..9b4ae49 100644 --- a/openspec/changes/code-review-12-guided-simplification-enforcement/TDD_EVIDENCE.md +++ b/openspec/changes/code-review-12-guided-simplification-enforcement/TDD_EVIDENCE.md @@ -57,6 +57,8 @@ - Result after adding the AI instructions fallback and docs: 6 passed. - `hatch run specfact code review run --instructions` - Result after adding the AI instructions fallback: printed the guided simplify / clean-code workflow and exited successfully without running review analysis. +- Subagent simulation with only `specfact code review run --instructions` guidance + - Result: the assistant followed the conservative decision-card workflow, treated missing `guidance_kind` findings as unguided advisories, and identified the clean-PR branch fallback as actionable after adding a base-ref diff example. - `hatch run contract-test` - Result after PR review fixes: 758 passed, 2 warnings. - `hatch run smart-test` diff --git a/packages/specfact-code-review/module-package.yaml b/packages/specfact-code-review/module-package.yaml index 26b54a6..f66856e 100644 --- a/packages/specfact-code-review/module-package.yaml +++ b/packages/specfact-code-review/module-package.yaml @@ -1,5 +1,5 @@ name: nold-ai/specfact-code-review -version: 0.47.25 +version: 0.47.26 commands: - code tier: official @@ -23,5 +23,4 @@ description: Official SpecFact code review bundle package. category: codebase bundle_group_command: code integrity: - checksum: sha256:668408981debae406bd6758c3b7191e5cbb98976f659069dded90afe04c1a8b3 - signature: 2UX+OBZKN0sxlE9z9U5oA47MocfYqMZM2khZoVMKM0ci3HJvh5/RYSWU1TuT+kbCgZmBeKsaB457KtWIxH+PBQ== + checksum: sha256:87a17f884d717d6def557d2bfc3076288d610e4751a38b12aed4e72e64ed32e2 diff --git a/packages/specfact-code-review/src/specfact_code_review/review/commands.py b/packages/specfact-code-review/src/specfact_code_review/review/commands.py index 2b80b35..959da66 100644 --- a/packages/specfact-code-review/src/specfact_code_review/review/commands.py +++ b/packages/specfact-code-review/src/specfact_code_review/review/commands.py @@ -33,14 +33,17 @@ 1. Generate evidence first: specfact code review run --scope changed --focus simplify --json --out .specfact/code-review-simplify.json + If the worktree is clean on a PR branch and --scope changed finds no files, review the branch-delta Python files as explicit positional files and omit --scope. Find them with the PR base ref, for example: git diff --name-only origin/dev...HEAD -- '*.py' '*.pyi' + 2. Treat guidance_kind as the action contract: - safe_mechanical: apply only after local safety checks pass. - needs_tests: add or identify targeted tests before changing behavior. - design_judgment: inspect intent evidence and ask before editing. - preserve: keep by default and record preserve_reason. + Findings without guidance_kind are unguided advisories: summarize them separately, do not auto-apply them, and ask before using them as refactor input. 3. For vibe-coder or junior users, present each finding as a decision card: - Finding, plain-language issue, why it might need to stay, exact patch preview, validation plan, recommended choice. + Finding, plain-language issue, why it might need to stay, exact patch preview or small before/after proposal, validation plan, recommended choice. 4. For design_judgment findings, check API, callback, framework hook, adapter, public symbol, CLI boundary, compatibility shim, and readability intent. If intent is unclear, default to keep or skip. diff --git a/packages/specfact-project/module-package.yaml b/packages/specfact-project/module-package.yaml index 98b9675..66b4a88 100644 --- a/packages/specfact-project/module-package.yaml +++ b/packages/specfact-project/module-package.yaml @@ -1,5 +1,5 @@ name: nold-ai/specfact-project -version: 0.41.16 +version: 0.41.17 commands: - project - plan @@ -27,5 +27,4 @@ core_compatibility: '>=0.40.0,<1.0.0' description: Official SpecFact project bundle package. bundle_group_command: project integrity: - checksum: sha256:4985bf438fb47aeced3d15a47866909a914492c52701a68e0c1ef5fef07ef8e2 - signature: od7YByTQZRdevYEK7FZjZqY+j6f13gmrnl3iFndP8oSZeNLj1ntdn083WA96hCmadYiKkFpws/kgDwZurGFIAA== + checksum: sha256:98760f97c05a8bb7606931dd50b2c885c8d98309332cdab6bc5b2d5153564cdc diff --git a/tests/unit/specfact_code_review/review/test_commands.py b/tests/unit/specfact_code_review/review/test_commands.py index 96aaf5b..8538af6 100644 --- a/tests/unit/specfact_code_review/review/test_commands.py +++ b/tests/unit/specfact_code_review/review/test_commands.py @@ -37,6 +37,9 @@ def _fail_run_command(_files: list[Path], **_kwargs: object) -> tuple[int, str | assert "remove AI bloat" in result.output assert "safe_mechanical" in result.output assert "design_judgment" in result.output + assert "branch-delta Python files" in result.output + assert "git diff --name-only origin/dev...HEAD" in result.output + assert "Findings without guidance_kind are unguided advisories" in result.output assert "exact patch preview" in result.output assert "default to keep or skip" in result.output assert "specfact code review run --scope changed --focus simplify" in result.output From 56ec0581eef113848bca5df6c08c907f579eff34 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 23 May 2026 23:09:55 +0000 Subject: [PATCH 6/6] chore(modules): ci sign changed modules --- packages/specfact-code-review/module-package.yaml | 1 + packages/specfact-project/module-package.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/specfact-code-review/module-package.yaml b/packages/specfact-code-review/module-package.yaml index f66856e..d3b86b2 100644 --- a/packages/specfact-code-review/module-package.yaml +++ b/packages/specfact-code-review/module-package.yaml @@ -24,3 +24,4 @@ category: codebase bundle_group_command: code integrity: checksum: sha256:87a17f884d717d6def557d2bfc3076288d610e4751a38b12aed4e72e64ed32e2 + signature: uGnmRW920celR5bugIuq8XLjBJ4qdWT/WZmUz/h/Gk7/V0VD7NY+F7O0wB8BgaClL55yxochTTFEjEM18iHmCA== diff --git a/packages/specfact-project/module-package.yaml b/packages/specfact-project/module-package.yaml index 66b4a88..089eec4 100644 --- a/packages/specfact-project/module-package.yaml +++ b/packages/specfact-project/module-package.yaml @@ -28,3 +28,4 @@ description: Official SpecFact project bundle package. bundle_group_command: project integrity: checksum: sha256:98760f97c05a8bb7606931dd50b2c885c8d98309332cdab6bc5b2d5153564cdc + signature: Hp8M0QWi1OO1+gCPil5K0MbUy3tY3xg8R3OzdHRbeJFWnYPaB8GMuEpEkN163OVjzEqToTKYnifLpGqfgJt9CA==