Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .vibe/skills/specfact-code-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,15 @@ allowed-tools: []

# House Rules - AI Coding Context (v4)

Updated: 2026-03-30 | Module: nold-ai/specfact-code-review
Updated: 2026-05-22 | Module: nold-ai/specfact-code-review

## DO

- 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`
- 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
- 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)
Expand Down
1 change: 1 addition & 0 deletions openspec/CHANGE_ORDER.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ The architecture pillar remains active because `architecture-02-well-architected
| codebase + project-runtime | 02 | codebase-import-runtime-hardening | [#235](https://github.com/nold-ai/specfact-cli-modules/issues/235) | Parent Feature: [#234](https://github.com/nold-ai/specfact-cli-modules/issues/234); Epic: [#162](https://github.com/nold-ai/specfact-cli-modules/issues/162); no known blockers |
| code-review + project | 03 | code-review-ai-bloat-detection | [#269](https://github.com/nold-ai/specfact-cli-modules/issues/269) | Parent Feature: [#175](https://github.com/nold-ai/specfact-cli-modules/issues/175); Epic: [#162](https://github.com/nold-ai/specfact-cli-modules/issues/162); no known blockers |
| code-review + project | 04 | code-review-11-simplification-feedback-loop | [#276](https://github.com/nold-ai/specfact-cli-modules/issues/276) | Parent Feature: [#275](https://github.com/nold-ai/specfact-cli-modules/issues/275); Epic: [#162](https://github.com/nold-ai/specfact-cli-modules/issues/162); blocked by `code-review-ai-bloat-detection` / [#269](https://github.com/nold-ai/specfact-cli-modules/issues/269) |
| code-review + project | 05 | code-review-12-guided-simplification-enforcement | [#286](https://github.com/nold-ai/specfact-cli-modules/issues/286) | Parent Feature: [#275](https://github.com/nold-ai/specfact-cli-modules/issues/275); Epic: [#162](https://github.com/nold-ai/specfact-cli-modules/issues/162); blocked by `code-review-11-simplification-feedback-loop` / [#276](https://github.com/nold-ai/specfact-cli-modules/issues/276) |

### Documentation restructure

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-05-22
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
# TDD Evidence: code-review-12-guided-simplification-enforcement

## Failing Before

- `hatch run pytest tests/unit/specfact_code_review/run/test_findings.py tests/unit/specfact_code_review/tools/test_ai_bloat_runner.py tests/unit/specfact_code_review/run/test_runner.py tests/unit/test_guided_simplify_resources.py -q`
- Result: failed as expected before implementation.
- Evidence: 18 failed, 64 passed.
- Missing contract areas: guided finding fields, preserve validation, schema 1.2 summary, classifier guidance kinds, simplify enforce behavior, and prompt/skill walkthrough policy.
- `hatch run pytest tests/unit/specfact_code_review/run/test_findings.py tests/unit/specfact_code_review/run/test_commands.py tests/unit/specfact_code_review/rules/test_updater.py tests/unit/test_guided_simplify_resources.py -q`
- Result: failed as expected before PR review fixes.
- Evidence: 6 failed, 87 passed.
- Missing contract areas: orphan guided-field validation, failed safe-mechanical blocking counts, missing deterministic safe-mechanical fixers, bottom-up rewrite ordering, and headless action-table defaults.
- `hatch run pytest tests/unit/specfact_code_review/run/test_commands.py::test_apply_simplification_fixes_keeps_dead_branch_with_else tests/unit/specfact_code_review/run/test_findings.py::test_review_finding_rejects_guided_evidence_fields_without_guidance_kind -q`
- Result: failed as expected before follow-up PR review fixes.
- Evidence: 4 failed.
- Missing contract areas: dead-branch fixer skipped no-else guard and guided evidence fields required `guidance_kind`.

## Passing After

- `hatch run pytest tests/unit/specfact_code_review/run/test_commands.py tests/unit/specfact_code_review/run/test_findings.py tests/unit/specfact_code_review/tools/test_ai_bloat_runner.py tests/unit/specfact_code_review/run/test_runner.py tests/unit/specfact_code_review/rules/test_updater.py::test_default_skill_content_stays_within_line_budget tests/unit/specfact_code_review/rules/test_updater.py::test_load_bundled_skill_content_returns_valid_structure_when_available tests/unit/test_guided_simplify_resources.py -q`
- Result: 116 passed.
- `hatch run pytest tests/unit/specfact_code_review/run/test_findings.py tests/unit/specfact_code_review/run/test_commands.py tests/unit/specfact_code_review/rules/test_updater.py tests/unit/test_guided_simplify_resources.py -q`
- Result after PR review fixes: 93 passed.
- `hatch run pytest tests/unit/specfact_code_review/run/test_findings.py tests/unit/specfact_code_review/run/test_commands.py tests/unit/specfact_code_review/run/test_runner.py tests/unit/specfact_code_review/rules/test_updater.py tests/unit/test_guided_simplify_resources.py -q`
- Result after strict metadata fallout fix: 125 passed.
- `hatch run pytest tests/unit/specfact_code_review/run/test_commands.py tests/unit/specfact_code_review/tools/test_ai_bloat_runner.py -q`
- Result after final cleanup: 50 passed.
- `hatch run pytest tests/unit/specfact_code_review/run/test_commands.py tests/unit/specfact_code_review/run/test_findings.py -q`
- Result after follow-up PR review fixes: 77 passed.
- `hatch run contract-test`
- Result after PR review fixes: 758 passed, 2 warnings.
- `hatch run smart-test`
- Result: 742 passed, 2 warnings.
- `hatch run type-check`
- Result: 0 errors, 0 warnings, 0 notes.
- `hatch run lint`
- Result: 10.00/10.
- `hatch run yaml-lint`
- Result: 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.
- `hatch run specfact code review run --bug-hunt --json --out .specfact/code-review.json --scope changed`
- Result after follow-up PR review fixes: PASS, CI exit 0, score 120, 0 findings.
- `openspec validate code-review-12-guided-simplification-enforcement --strict`
- Result: valid.

## Local Dev-Link Validation

- Linked live modules with:
- `hatch run link-dev-module specfact-code-review --force`
- `hatch run link-dev-module specfact-project --force`
- Verified runtime precedence through CLI output: project-scope `.specfact/modules/specfact-code-review` and `.specfact/modules/specfact-project` shadow user-scope copies.
- Current changed-scope mode checks:
- Default: PASS, schema `1.0`, score 115, 0 findings.
- Bug-hunt: PASS, schema `1.0`, score 115, 0 findings.
- Simplify shadow: PASS, schema `1.0`, score 115, 0 findings.
- Simplify enforce: PASS, schema `1.0`, score 115, 0 findings.
- Guided fixture checks:
- Simplify shadow on fixture: PASS, schema `1.2`, 3 findings, summary counts `safe_mechanical=1`, `needs_tests=1`, `preserve=1`.
- Simplify enforce on same fixture: FAIL, schema `1.2`, `ci_exit_code=1`, blocked only by the unresolved safe-mechanical recommendation.
- Simplify enforce with `--fix` on safe-mechanical fixture: rewrote `return sum(values)`, then PASS. Remaining Semgrep wrapper finding now carries `design_judgment` guidance and schema `1.2`.
- Prompt/skill dry run with subagent:
- Confirmed walkthrough levels, `guidance_kind` policy, no batch edits, and action status requirements.
- Found and fixed gaps: missing-report explanation, headless batching language, and concrete action-log shape.
- Final dev-link bug-hunt with extended targeted-test timeout:
- `SPECFACT_ALLOW_UNSIGNED=1 SPECFACT_CODE_REVIEW_TARGETED_TEST_TIMEOUT=300 hatch run specfact code review run --bug-hunt --json --out .specfact/tmp-local-dev-link-review/changed-bughunt-clean-final.json --scope changed`
- Result: PASS, CI exit 0, score 115, 0 findings.

## Signing Note

`hatch run sign-modules --changed-only --payload-from-filesystem --bump-version patch --base-ref origin/dev` failed locally because no private signing key was available. I reran with `--allow-unsigned`, which bumped affected module versions and refreshed filesystem checksums. Cryptographic signature restoration remains an approval-time signing step.
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
## Context

The current simplify flow emits useful candidates but does not always tell an LLM what to do safely. The most important missing distinction is between accidental structure and meaningful structure. Examples:

- `@require(lambda ...)` and `@ensure(lambda ...)` are contract expressions, not pass-through bloat.
- Optional parameters on abstract adapters may preserve protocol compatibility even if a concrete implementation does not use them.
- Small wrappers can be domain predicates, public compatibility boundaries, or CLI affordances.
- Long low-complexity functions may be readable orchestration rather than bloat.

The review output must encode these distinctions deterministically so an AI IDE or headless agent can act without guessing.

## Goals / Non-Goals

**Goals:**

- Add action-oriented guidance to simplify findings while keeping JSON backward-compatible.
- Make safe mechanical cleanup enforceable and optionally fixable.
- Keep judgment-heavy and preserve-worthy patterns out of automatic cleanup.
- Make `/specfact.08-simplify` interactive and adaptive to user experience level.
- Make the `specfact-code-review` skill give LLMs the same cleanup policy in IDE and CLI contexts.

**Non-Goals:**

- No LLM or embedding classifier inside the CLI.
- No automatic refactor for design-judgment findings.
- No breaking removal of existing simplification metadata fields.
- No claim that findings prove AI authorship.

## Decisions

### Decision 1: Extend finding guidance instead of adding a second artifact

The JSON report remains the source of truth. Each simplification finding can carry:

- `guidance_kind`: `safe_mechanical`, `needs_tests`, `design_judgment`, or `preserve`;
- `recommended_action`: `remove`, `inline`, `collapse`, `deduplicate`, `make_required`, `keep`, or `inspect`;
- `clean_code_principle`: `kiss`, `dry`, `yagni`, `contracts`, `api_stability`, or `readability`;
- `rationale`, `safety_checks`, `preserve_reason`, and `action_status`;
- optional before/after evidence and improvement metrics after an auto-applied safe fix.

Existing fields such as `confidence`, `rewrite_hint`, `canonical_pattern`, `intent_key`, and `related_locations` remain valid.

### Decision 2: Classify before enforcing

`--focus simplify` should emit all relevant guidance kinds, but only `safe_mechanical` findings may become enforceable. `needs_tests`, `design_judgment`, and `preserve` remain non-blocking and non-autofix.

### Decision 3: Preserve meaningful patterns explicitly

False-positive-prone contexts are not merely suppressed; they should produce `preserve` findings when that helps the user understand why cleanup is not recommended. Contract lambdas, abstract/protocol adapter params, public compatibility wrappers, CLI boundary wrappers, and domain-named predicates should include a `preserve_reason`.

### Decision 4: Prompt and skill are part of the product

The slash prompt and skill are the interactive delivery surface for the target audience. They must ask for or infer walkthrough level and adjust wording:

- vibe coder: teaching flow, one finding at a time;
- junior developer: principle, risk, test, proposed edit;
- senior/pro: concise grouped triage;
- headless agent: deterministic JSON-first behavior with no broad refactors.

### Decision 5: Evidence must show outcome, not just recommendation

When `--fix` applies a safe rewrite, the report should record what was recommended, what changed, what remains, and the improvement. For manual prompt flows, the prompt should summarize accepted, skipped, kept, and still-present findings after rerunning review.

## Risks / Trade-offs

- **Over-enforcement:** Limit blocking to `safe_mechanical` only.
- **Autofix risk:** Restrict `--fix` to small AST-safe rewrites with post-review evidence.
- **Prompt overwhelm:** Start with walkthrough level and group by guidance kind and intent.
- **Schema churn:** Keep all fields optional and validate legacy reports.
- **False-positive confusion:** Prefer explicit `preserve` with reason over silent disappearance where a finding would otherwise be tempting to fix.
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
## Why

`code-review-11-simplification-feedback-loop` made simplification findings richer, but the output is still closer to a senior developer's radar than a junior-safe workflow. Users and LLM agents need findings that say whether a cleanup is safe to apply, needs tests, requires design judgment, or should be preserved because it encodes a contract, public API boundary, compatibility shim, or domain intent.

This follow-up turns `specfact code review run --focus simplify` into a guided clean-code workflow for interactive IDE users and headless AI agents. It keeps meaningful contracts and extension points intact while making truly mechanical AI-bloat cleanup clear enough for non-senior developers and LLMs to interpret correctly.

## What Changes

- Extend simplification findings with senior-readable guidance: guidance kind, recommended action, rationale, clean-code principle, safety checks, preserve reason, action status, and before/after improvement evidence.
- Classify simplification findings into `safe_mechanical`, `needs_tests`, `design_judgment`, and `preserve` so agents do not blindly remove meaningful patterns.
- Make `--focus simplify --mode enforce` fail only on unresolved `safe_mechanical` findings.
- Make `--focus simplify --fix` apply only deterministic safe-mechanical rewrites and record what was applied, still recommended, kept, skipped, or failed.
- Upgrade `/specfact.08-simplify` and the `specfact-code-review` skill into adaptive walkthrough surfaces for vibe-coder, junior, senior/pro, and headless-agent usage.
- Keep review JSON backward-compatible and use it as the authoritative evidence artifact.

## Capabilities

### New Capabilities

- `guided-simplification-review`: Senior-grade guidance and evidence for simplify-focused code review workflows.

### Modified Capabilities

- `review-finding-model`: Add optional action-oriented simplification guidance fields.
- `review-run-command`: Add guided enforce/fix behavior for simplify focus.
- `code-review-simplification-feedback`: Upgrade metadata from advisory hints to actionable, LLM-safe guidance.
- `house-rules-skill`: Align the installed skill with the new simplify decision policy.

## Impact

- **Affected bundles:** `packages/specfact-code-review` and `packages/specfact-project`.
- **Affected interfaces:** `.specfact/code-review.json` receives additive optional guidance metadata and report summary fields; existing required fields remain compatible.
- **Affected prompt resources:** `packages/specfact-project/resources/prompts/specfact.08-simplify.md`.
- **Affected skill resources:** `packages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.md`, `skills/specfact-code-review/SKILL.md`, and IDE skill copies.
- **Release impact:** module package version bumps and signature refresh are required when packaged resources or manifests change.

## Source Tracking

<!-- source_repo: nold-ai/specfact-cli-modules -->
- **Modules Epic:** [#162](https://github.com/nold-ai/specfact-cli-modules/issues/162)
- **Parent Feature:** [#275](https://github.com/nold-ai/specfact-cli-modules/issues/275)
- **GitHub Issue:** [#286](https://github.com/nold-ai/specfact-cli-modules/issues/286)
- **Repository:** nold-ai/specfact-cli-modules
- **Prior Baseline:** [#276](https://github.com/nold-ai/specfact-cli-modules/issues/276) / `code-review-11-simplification-feedback-loop`
- **Last Synced Status:** proposed
- **Sanitized:** false
Loading