From 9a997fd8d803c2f73b06cf9a79665d58fc5bb898 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 5 Jul 2026 07:46:00 +0000 Subject: [PATCH] =?UTF-8?q?feat(#285):=20TDD=20enforcement=20=E2=80=94=20I?= =?UTF-8?q?ron=20Law,=20reviewers,=20Monitor=20gate,=20tdd.enforce=20confi?= =?UTF-8?q?g?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds Superpowers-style mandatory TDD enforcement when tdd.enforce: true in .map/config.yaml: - MapConfig.tdd_enforce field with tdd.enforce YAML alias (dead-toggle guard) - map-tdd SKILL.md: Iron Law ("NO PRODUCTION CODE WITHOUT A FAILING TEST FIRST"), RED-GREEN-REFACTOR mandatory cycle, 14-item Red Flags list, 11-item Rationalization Table with counters, tdd.enforce config check in Step 0 - map-tdd SKILL.md: Spec Compliance Reviewer (adversarial, SPEC-COMPLIANT verdict) and Code Quality Reviewer (sequential gate: spec passes before code review starts) - monitor.md: TDD Violation Detection section with tdd_violation verdict, criteria for detecting code-before-test, rationalization detection patterns, JSON output schema - Tests: VC7 (6 tests for tdd_enforce field/alias), VC8 (11 tests for template content) - map-tdd body budget bumped to 545 (irreducible enforcement control flow) - All changes go through templates_src → make render-templates pipeline Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_01HZ3wHDow49xBGUwWFPH2mD --- .claude/agents/monitor.md | 44 +++++ .claude/skills/map-tdd/SKILL.md | 151 +++++++++++++++++- src/mapify_cli/config/project_config.py | 10 ++ src/mapify_cli/templates/agents/monitor.md | 44 +++++ .../templates/skills/map-tdd/SKILL.md | 151 +++++++++++++++++- .../templates_src/agents/monitor.md.jinja | 44 +++++ .../skills/map-tdd/SKILL.md.jinja | 151 +++++++++++++++++- tests/test_project_config.py | 144 +++++++++++++++++ tests/test_skills.py | 4 + 9 files changed, 740 insertions(+), 3 deletions(-) diff --git a/.claude/agents/monitor.md b/.claude/agents/monitor.md index c6060426..50334d40 100644 --- a/.claude/agents/monitor.md +++ b/.claude/agents/monitor.md @@ -880,6 +880,50 @@ Include in JSON output when validation_criteria provided: - If `security_critical == true`: set `valid: false` (missing executable enforcement is a release blocker). - Otherwise: add a **testability** issue and require Actor to add tests. +### TDD Violation Detection (when tdd.enforce=true) + +When the project config has `tdd.enforce: true`, apply this check to EVERY Actor submission before the 11-dimension model. A violation sets `valid: false` immediately. + +**TDD violation criteria** — any of the following is a `tdd_violation`: +1. Tests were written AFTER implementation (tests reference implementation details unavailable from the spec alone, or the implementation was clearly written first) +2. Tests pass WITHOUT the implementation (trivial pass, over-mocked, testing structure not behavior) +3. No new test exists for the new behavior introduced by this subtask +4. Actor output claims "TDD" but shows tests written simultaneously with or after code + +**How to check** (examine the diff): +- Test files should introduce assertions on behavior described in the spec, not on internal implementation structure +- Implementation files should be adding code that satisfies pre-existing test contracts +- If tests and implementation appear to be written together (same commit, same session), check whether tests assert behavior or structure +- A test that only checks `isinstance(result, SomeClass)` or a single attribute is structural, not behavioral + +**Rationalization detection** — flag any of these patterns in Actor output as a violation signal: +- "Tests written simultaneously with code" → likely violation; verify manually +- "Tests verify implementation structure" → violation (must verify behavior) +- "I added tests after to ensure coverage" → violation +- "This is too simple to need a test first" → violation +- "The test is obvious so I wrote code first" → violation + +**TDD verdict output** (include in JSON when tdd.enforce is true): + +```json +{ + "tdd_check": { + "enforced": true, + "violation": false, + "test_written_first": true, + "test_fails_without_impl": true, + "test_behavior_not_structure": true, + "verdict": "tdd_compliant" + } +} +``` + +If `violation: true`: +- Set `valid: false` +- Use verdict `tdd_violation` +- Include message: `"TDD violation: [specific reason]. Implementation must be deleted and restarted with failing tests first."` +- Do NOT suggest "just add tests" — the implementation must be deleted and rewritten test-first per the Iron Law. + diff --git a/.claude/skills/map-tdd/SKILL.md b/.claude/skills/map-tdd/SKILL.md index 055ddab9..59fea43a 100644 --- a/.claude/skills/map-tdd/SKILL.md +++ b/.claude/skills/map-tdd/SKILL.md @@ -17,6 +17,58 @@ argument-hint: "[task description]" **Key insight:** If implementation is in context when writing tests, AI writes tests that confirm the implementation — including its bugs. By writing tests FIRST from the spec only, tests become an independent correctness oracle. +## The Iron Law + +``` +NO PRODUCTION CODE WITHOUT A FAILING TEST FIRST +``` + +When `tdd.enforce: true` is set in `.map/config.yaml`, this law is mandatory — not advisory. Code written before a failing test is **deleted**. Not refactored. Not adapted. Deleted. Start over. + +### RED-GREEN-REFACTOR (mandatory cycle) + +| Phase | Action | Verification | +|-------|--------|--------------| +| **RED** | Write a failing test | VERIFY it fails for the right reason — not import error, not typo | +| **GREEN** | Write minimal code to pass | VERIFY all tests pass | +| **REFACTOR** | Clean up | VERIFY tests still pass after each change | + +Never skip the RED phase. A test that is "obviously going to fail" still needs a run to confirm it fails for the right reason. + +### Red Flags (self-detection) + +Stop immediately and restart if you notice any of these: +1. Writing implementation before running any test +2. "Just this once" — there are no exceptions +3. "This is different because..." — it is not different +4. "The test is obvious" — then it takes 30 seconds +5. "I'll add tests after" — you will not, or they will confirm bugs +6. "The spec is clear enough" — tests make it executable +7. "TDD slows me down" — you are paying for it later +8. "This is configuration, not code" — configuration breaks too +9. "I already know it works" — the test proves it +10. "I'll refactor into testability later" — now is later +11. Writing tests AFTER implementation and calling it TDD +12. Fixing tests to match buggy implementation +13. "There's no way to test this" — extract the testable core +14. Checking "tests pass" without checking they fail first + +### Rationalization Table + +| Rationalization | Counter | +|-----------------|---------| +| "Too simple to test" | Simple code breaks. Test takes 30 seconds. | +| "I'll add tests after, I promise" | You won't. Or they'll confirm your bugs. | +| "Deleting N hours of work is wasteful" | Sunk cost fallacy. The waste was writing untested code. | +| "TDD is dogmatic" | TDD IS pragmatic — it forces a debuggable interface. | +| "I don't know how to test this yet" | That's a spec clarity problem. Clarify the spec first. | +| "The test would just mock everything" | Mock the boundary, not the behavior. Restructure. | +| "Tests slow down the deadline" | Debugging untested code slows it down more. | +| "The framework handles this" | Prove it with a test. | +| "I need to spike first" | Spikes are throwaway. Write the test on the real implementation. | +| "This is an integration concern" | Extract the unit. Integration tests come after unit tests. | +| "The existing codebase doesn't have tests" | You're adding tests now. One change at a time. | + **What this command does NOT do:** - Does NOT replace /map-efficient — it augments the Actor/Monitor loop with test-first phases - Does NOT work without a spec or plan — requires spec_.md or clear acceptance criteria @@ -103,6 +155,27 @@ python3 .map/scripts/map_orchestrator.py set_tdd_mode true This inserts TEST_WRITER (2.25) and TEST_FAIL_GATE (2.26) phases before ACTOR (2.3) in the step sequence. +### Check tdd.enforce Configuration + +```bash +TDD_ENFORCE=$(python3 -c " +import sys; sys.path.insert(0,'src') +try: + from mapify_cli.config.project_config import load_map_config + from pathlib import Path + cfg = load_map_config(Path('.map/config.yaml')) + print('true' if cfg.tdd_enforce else 'false') +except Exception: + print('false') +" 2>/dev/null || echo 'false') +``` + +When `TDD_ENFORCE=true`: +- The Iron Law applies: any code written before a failing test is **deleted** +- Spec compliance reviewer and code quality reviewer run after each ACTOR phase (mandatory) +- Monitor enforces the TDD violation check +- The rationalization table above is your reference — no excuse survives it + --- ## Step 1: State Machine Loop @@ -303,7 +376,83 @@ Output: standard Actor output (approach + code + trade-offs) ) ``` -After Actor returns, run the TDD Refactor step below, then call Monitor (2.4). Monitor validation is required before marking the subtask complete, including in TDD workflows. +After Actor returns, run the spec compliance reviewer and code quality reviewer (if `tdd.enforce: true`), then run the TDD Refactor step, then call Monitor (2.4). + +### Spec Compliance Reviewer (adversarial, MANDATORY when tdd.enforce=true) + +Spawn this BEFORE code quality review. The two reviews cannot be swapped. + +**Adversarial framing:** The implementer finished suspiciously quickly. Do NOT trust the Actor summary. Read the actual code. + +```python +Task( + subagent_type="actor", + description="Spec compliance review: subtask [ID]", + prompt=f"""You are an adversarial spec compliance reviewer. + +You have been told the implementer finished subtask [ID]. Do NOT trust their summary. +Read the actual code diff. Compare every requirement in the spec to the actual diff. + + +[paste AAG contract + validation_criteria from decomposition] + + + +[paste git diff for this subtask] + + +Check EVERY requirement: +1. Is each requirement implemented? Show file:line evidence. +2. Is there extra work not in the spec? (Scope creep, gold-plating) +3. Are there misunderstandings? (Implemented the wrong thing) +4. Are all edge cases from the spec covered? + +Output: +- SPEC-COMPLIANT: YES or NO +- If NO: list each gap with exact file:line reference and the spec line it violates +""" +) +``` + +**Gate**: If the reviewer returns `SPEC-COMPLIANT: NO`, route back to ACTOR with the specific gaps listed. Do NOT proceed to code quality review until spec passes. + +### Code Quality Reviewer (runs ONLY after spec reviewer passes) + +```python +Task( + subagent_type="actor", + description="Code quality review: subtask [ID]", + prompt=f"""You are a code quality reviewer. Spec compliance is already verified. + +Review the implementation quality only. + + +[paste git diff for this subtask] + + +Check: +1. **File responsibility**: Does each file do one thing? +2. **Unit decomposition**: Are functions small and independently testable? +3. **Plan conformance**: Does structure match the architectural plan? +4. **Size discipline**: No functions > 40 lines without documented justification +5. **Error handling**: All error paths handled explicitly +6. **Type safety**: No untyped `Any` without justification +7. **Naming**: Names describe behavior, not structure + +Output: +**Strengths**: (what is done well — be specific) +**Issues**: +- CRITICAL: [blocks proceeding — must fix before Monitor] +- IMPORTANT: [should fix in this subtask] +- MINOR: [acceptable to defer] +**Assessment**: PASS | PASS_WITH_MINOR | FAIL +""" +) +``` + +**Sequential gate**: Spec compliance review MUST complete with `SPEC-COMPLIANT: YES` before code quality review starts. Running them in parallel bypasses the gate. + +If code quality review returns `FAIL` (CRITICAL issues), route back to ACTOR. Once it returns `PASS` or `PASS_WITH_MINOR`, proceed to TDD Refactor and then Monitor. ### TDD Refactor: Clean Stale Red-Phase Comments diff --git a/src/mapify_cli/config/project_config.py b/src/mapify_cli/config/project_config.py index 5d19dff3..a3aaa49f 100644 --- a/src/mapify_cli/config/project_config.py +++ b/src/mapify_cli/config/project_config.py @@ -225,6 +225,15 @@ class MapConfig: # it yet. max_wave_retries: int = 3 + # Enforce TDD discipline project-wide (#285). When true, /map-efficient routes + # Actor-phase dispatches through the TEST_WRITER → TEST_FAIL_GATE → ACTOR + # sequence automatically, equivalent to always running /map-tdd. Code written + # before a failing test is treated as a violation: Monitor emits a TDD_VIOLATION + # finding. spec-compliance and code-quality reviewer subagents run after each + # subtask. Default OFF so existing workflows are unaffected. + # Dotted YAML key: `tdd.enforce` (aliased in load_map_config). + tdd_enforce: bool = False + def clamp_max_actors(n: object) -> int: """Clamp max_actors to the valid range [1, 8], or return the default 4. @@ -331,6 +340,7 @@ def load_map_config(project_path: Path) -> MapConfig: ("execution.retry_degraded_once", "retry_degraded_once"), ("execution.concurrent_dispatch", "concurrent_dispatch"), ("execution.max_wave_retries", "max_wave_retries"), + ("tdd.enforce", "tdd_enforce"), ): if dotted in data and field_name not in data: data[field_name] = data.pop(dotted) diff --git a/src/mapify_cli/templates/agents/monitor.md b/src/mapify_cli/templates/agents/monitor.md index c6060426..50334d40 100644 --- a/src/mapify_cli/templates/agents/monitor.md +++ b/src/mapify_cli/templates/agents/monitor.md @@ -880,6 +880,50 @@ Include in JSON output when validation_criteria provided: - If `security_critical == true`: set `valid: false` (missing executable enforcement is a release blocker). - Otherwise: add a **testability** issue and require Actor to add tests. +### TDD Violation Detection (when tdd.enforce=true) + +When the project config has `tdd.enforce: true`, apply this check to EVERY Actor submission before the 11-dimension model. A violation sets `valid: false` immediately. + +**TDD violation criteria** — any of the following is a `tdd_violation`: +1. Tests were written AFTER implementation (tests reference implementation details unavailable from the spec alone, or the implementation was clearly written first) +2. Tests pass WITHOUT the implementation (trivial pass, over-mocked, testing structure not behavior) +3. No new test exists for the new behavior introduced by this subtask +4. Actor output claims "TDD" but shows tests written simultaneously with or after code + +**How to check** (examine the diff): +- Test files should introduce assertions on behavior described in the spec, not on internal implementation structure +- Implementation files should be adding code that satisfies pre-existing test contracts +- If tests and implementation appear to be written together (same commit, same session), check whether tests assert behavior or structure +- A test that only checks `isinstance(result, SomeClass)` or a single attribute is structural, not behavioral + +**Rationalization detection** — flag any of these patterns in Actor output as a violation signal: +- "Tests written simultaneously with code" → likely violation; verify manually +- "Tests verify implementation structure" → violation (must verify behavior) +- "I added tests after to ensure coverage" → violation +- "This is too simple to need a test first" → violation +- "The test is obvious so I wrote code first" → violation + +**TDD verdict output** (include in JSON when tdd.enforce is true): + +```json +{ + "tdd_check": { + "enforced": true, + "violation": false, + "test_written_first": true, + "test_fails_without_impl": true, + "test_behavior_not_structure": true, + "verdict": "tdd_compliant" + } +} +``` + +If `violation: true`: +- Set `valid: false` +- Use verdict `tdd_violation` +- Include message: `"TDD violation: [specific reason]. Implementation must be deleted and restarted with failing tests first."` +- Do NOT suggest "just add tests" — the implementation must be deleted and rewritten test-first per the Iron Law. + diff --git a/src/mapify_cli/templates/skills/map-tdd/SKILL.md b/src/mapify_cli/templates/skills/map-tdd/SKILL.md index 055ddab9..59fea43a 100644 --- a/src/mapify_cli/templates/skills/map-tdd/SKILL.md +++ b/src/mapify_cli/templates/skills/map-tdd/SKILL.md @@ -17,6 +17,58 @@ argument-hint: "[task description]" **Key insight:** If implementation is in context when writing tests, AI writes tests that confirm the implementation — including its bugs. By writing tests FIRST from the spec only, tests become an independent correctness oracle. +## The Iron Law + +``` +NO PRODUCTION CODE WITHOUT A FAILING TEST FIRST +``` + +When `tdd.enforce: true` is set in `.map/config.yaml`, this law is mandatory — not advisory. Code written before a failing test is **deleted**. Not refactored. Not adapted. Deleted. Start over. + +### RED-GREEN-REFACTOR (mandatory cycle) + +| Phase | Action | Verification | +|-------|--------|--------------| +| **RED** | Write a failing test | VERIFY it fails for the right reason — not import error, not typo | +| **GREEN** | Write minimal code to pass | VERIFY all tests pass | +| **REFACTOR** | Clean up | VERIFY tests still pass after each change | + +Never skip the RED phase. A test that is "obviously going to fail" still needs a run to confirm it fails for the right reason. + +### Red Flags (self-detection) + +Stop immediately and restart if you notice any of these: +1. Writing implementation before running any test +2. "Just this once" — there are no exceptions +3. "This is different because..." — it is not different +4. "The test is obvious" — then it takes 30 seconds +5. "I'll add tests after" — you will not, or they will confirm bugs +6. "The spec is clear enough" — tests make it executable +7. "TDD slows me down" — you are paying for it later +8. "This is configuration, not code" — configuration breaks too +9. "I already know it works" — the test proves it +10. "I'll refactor into testability later" — now is later +11. Writing tests AFTER implementation and calling it TDD +12. Fixing tests to match buggy implementation +13. "There's no way to test this" — extract the testable core +14. Checking "tests pass" without checking they fail first + +### Rationalization Table + +| Rationalization | Counter | +|-----------------|---------| +| "Too simple to test" | Simple code breaks. Test takes 30 seconds. | +| "I'll add tests after, I promise" | You won't. Or they'll confirm your bugs. | +| "Deleting N hours of work is wasteful" | Sunk cost fallacy. The waste was writing untested code. | +| "TDD is dogmatic" | TDD IS pragmatic — it forces a debuggable interface. | +| "I don't know how to test this yet" | That's a spec clarity problem. Clarify the spec first. | +| "The test would just mock everything" | Mock the boundary, not the behavior. Restructure. | +| "Tests slow down the deadline" | Debugging untested code slows it down more. | +| "The framework handles this" | Prove it with a test. | +| "I need to spike first" | Spikes are throwaway. Write the test on the real implementation. | +| "This is an integration concern" | Extract the unit. Integration tests come after unit tests. | +| "The existing codebase doesn't have tests" | You're adding tests now. One change at a time. | + **What this command does NOT do:** - Does NOT replace /map-efficient — it augments the Actor/Monitor loop with test-first phases - Does NOT work without a spec or plan — requires spec_.md or clear acceptance criteria @@ -103,6 +155,27 @@ python3 .map/scripts/map_orchestrator.py set_tdd_mode true This inserts TEST_WRITER (2.25) and TEST_FAIL_GATE (2.26) phases before ACTOR (2.3) in the step sequence. +### Check tdd.enforce Configuration + +```bash +TDD_ENFORCE=$(python3 -c " +import sys; sys.path.insert(0,'src') +try: + from mapify_cli.config.project_config import load_map_config + from pathlib import Path + cfg = load_map_config(Path('.map/config.yaml')) + print('true' if cfg.tdd_enforce else 'false') +except Exception: + print('false') +" 2>/dev/null || echo 'false') +``` + +When `TDD_ENFORCE=true`: +- The Iron Law applies: any code written before a failing test is **deleted** +- Spec compliance reviewer and code quality reviewer run after each ACTOR phase (mandatory) +- Monitor enforces the TDD violation check +- The rationalization table above is your reference — no excuse survives it + --- ## Step 1: State Machine Loop @@ -303,7 +376,83 @@ Output: standard Actor output (approach + code + trade-offs) ) ``` -After Actor returns, run the TDD Refactor step below, then call Monitor (2.4). Monitor validation is required before marking the subtask complete, including in TDD workflows. +After Actor returns, run the spec compliance reviewer and code quality reviewer (if `tdd.enforce: true`), then run the TDD Refactor step, then call Monitor (2.4). + +### Spec Compliance Reviewer (adversarial, MANDATORY when tdd.enforce=true) + +Spawn this BEFORE code quality review. The two reviews cannot be swapped. + +**Adversarial framing:** The implementer finished suspiciously quickly. Do NOT trust the Actor summary. Read the actual code. + +```python +Task( + subagent_type="actor", + description="Spec compliance review: subtask [ID]", + prompt=f"""You are an adversarial spec compliance reviewer. + +You have been told the implementer finished subtask [ID]. Do NOT trust their summary. +Read the actual code diff. Compare every requirement in the spec to the actual diff. + + +[paste AAG contract + validation_criteria from decomposition] + + + +[paste git diff for this subtask] + + +Check EVERY requirement: +1. Is each requirement implemented? Show file:line evidence. +2. Is there extra work not in the spec? (Scope creep, gold-plating) +3. Are there misunderstandings? (Implemented the wrong thing) +4. Are all edge cases from the spec covered? + +Output: +- SPEC-COMPLIANT: YES or NO +- If NO: list each gap with exact file:line reference and the spec line it violates +""" +) +``` + +**Gate**: If the reviewer returns `SPEC-COMPLIANT: NO`, route back to ACTOR with the specific gaps listed. Do NOT proceed to code quality review until spec passes. + +### Code Quality Reviewer (runs ONLY after spec reviewer passes) + +```python +Task( + subagent_type="actor", + description="Code quality review: subtask [ID]", + prompt=f"""You are a code quality reviewer. Spec compliance is already verified. + +Review the implementation quality only. + + +[paste git diff for this subtask] + + +Check: +1. **File responsibility**: Does each file do one thing? +2. **Unit decomposition**: Are functions small and independently testable? +3. **Plan conformance**: Does structure match the architectural plan? +4. **Size discipline**: No functions > 40 lines without documented justification +5. **Error handling**: All error paths handled explicitly +6. **Type safety**: No untyped `Any` without justification +7. **Naming**: Names describe behavior, not structure + +Output: +**Strengths**: (what is done well — be specific) +**Issues**: +- CRITICAL: [blocks proceeding — must fix before Monitor] +- IMPORTANT: [should fix in this subtask] +- MINOR: [acceptable to defer] +**Assessment**: PASS | PASS_WITH_MINOR | FAIL +""" +) +``` + +**Sequential gate**: Spec compliance review MUST complete with `SPEC-COMPLIANT: YES` before code quality review starts. Running them in parallel bypasses the gate. + +If code quality review returns `FAIL` (CRITICAL issues), route back to ACTOR. Once it returns `PASS` or `PASS_WITH_MINOR`, proceed to TDD Refactor and then Monitor. ### TDD Refactor: Clean Stale Red-Phase Comments diff --git a/src/mapify_cli/templates_src/agents/monitor.md.jinja b/src/mapify_cli/templates_src/agents/monitor.md.jinja index c6060426..50334d40 100644 --- a/src/mapify_cli/templates_src/agents/monitor.md.jinja +++ b/src/mapify_cli/templates_src/agents/monitor.md.jinja @@ -880,6 +880,50 @@ Include in JSON output when validation_criteria provided: - If `security_critical == true`: set `valid: false` (missing executable enforcement is a release blocker). - Otherwise: add a **testability** issue and require Actor to add tests. +### TDD Violation Detection (when tdd.enforce=true) + +When the project config has `tdd.enforce: true`, apply this check to EVERY Actor submission before the 11-dimension model. A violation sets `valid: false` immediately. + +**TDD violation criteria** — any of the following is a `tdd_violation`: +1. Tests were written AFTER implementation (tests reference implementation details unavailable from the spec alone, or the implementation was clearly written first) +2. Tests pass WITHOUT the implementation (trivial pass, over-mocked, testing structure not behavior) +3. No new test exists for the new behavior introduced by this subtask +4. Actor output claims "TDD" but shows tests written simultaneously with or after code + +**How to check** (examine the diff): +- Test files should introduce assertions on behavior described in the spec, not on internal implementation structure +- Implementation files should be adding code that satisfies pre-existing test contracts +- If tests and implementation appear to be written together (same commit, same session), check whether tests assert behavior or structure +- A test that only checks `isinstance(result, SomeClass)` or a single attribute is structural, not behavioral + +**Rationalization detection** — flag any of these patterns in Actor output as a violation signal: +- "Tests written simultaneously with code" → likely violation; verify manually +- "Tests verify implementation structure" → violation (must verify behavior) +- "I added tests after to ensure coverage" → violation +- "This is too simple to need a test first" → violation +- "The test is obvious so I wrote code first" → violation + +**TDD verdict output** (include in JSON when tdd.enforce is true): + +```json +{ + "tdd_check": { + "enforced": true, + "violation": false, + "test_written_first": true, + "test_fails_without_impl": true, + "test_behavior_not_structure": true, + "verdict": "tdd_compliant" + } +} +``` + +If `violation: true`: +- Set `valid: false` +- Use verdict `tdd_violation` +- Include message: `"TDD violation: [specific reason]. Implementation must be deleted and restarted with failing tests first."` +- Do NOT suggest "just add tests" — the implementation must be deleted and rewritten test-first per the Iron Law. + diff --git a/src/mapify_cli/templates_src/skills/map-tdd/SKILL.md.jinja b/src/mapify_cli/templates_src/skills/map-tdd/SKILL.md.jinja index 055ddab9..59fea43a 100644 --- a/src/mapify_cli/templates_src/skills/map-tdd/SKILL.md.jinja +++ b/src/mapify_cli/templates_src/skills/map-tdd/SKILL.md.jinja @@ -17,6 +17,58 @@ argument-hint: "[task description]" **Key insight:** If implementation is in context when writing tests, AI writes tests that confirm the implementation — including its bugs. By writing tests FIRST from the spec only, tests become an independent correctness oracle. +## The Iron Law + +``` +NO PRODUCTION CODE WITHOUT A FAILING TEST FIRST +``` + +When `tdd.enforce: true` is set in `.map/config.yaml`, this law is mandatory — not advisory. Code written before a failing test is **deleted**. Not refactored. Not adapted. Deleted. Start over. + +### RED-GREEN-REFACTOR (mandatory cycle) + +| Phase | Action | Verification | +|-------|--------|--------------| +| **RED** | Write a failing test | VERIFY it fails for the right reason — not import error, not typo | +| **GREEN** | Write minimal code to pass | VERIFY all tests pass | +| **REFACTOR** | Clean up | VERIFY tests still pass after each change | + +Never skip the RED phase. A test that is "obviously going to fail" still needs a run to confirm it fails for the right reason. + +### Red Flags (self-detection) + +Stop immediately and restart if you notice any of these: +1. Writing implementation before running any test +2. "Just this once" — there are no exceptions +3. "This is different because..." — it is not different +4. "The test is obvious" — then it takes 30 seconds +5. "I'll add tests after" — you will not, or they will confirm bugs +6. "The spec is clear enough" — tests make it executable +7. "TDD slows me down" — you are paying for it later +8. "This is configuration, not code" — configuration breaks too +9. "I already know it works" — the test proves it +10. "I'll refactor into testability later" — now is later +11. Writing tests AFTER implementation and calling it TDD +12. Fixing tests to match buggy implementation +13. "There's no way to test this" — extract the testable core +14. Checking "tests pass" without checking they fail first + +### Rationalization Table + +| Rationalization | Counter | +|-----------------|---------| +| "Too simple to test" | Simple code breaks. Test takes 30 seconds. | +| "I'll add tests after, I promise" | You won't. Or they'll confirm your bugs. | +| "Deleting N hours of work is wasteful" | Sunk cost fallacy. The waste was writing untested code. | +| "TDD is dogmatic" | TDD IS pragmatic — it forces a debuggable interface. | +| "I don't know how to test this yet" | That's a spec clarity problem. Clarify the spec first. | +| "The test would just mock everything" | Mock the boundary, not the behavior. Restructure. | +| "Tests slow down the deadline" | Debugging untested code slows it down more. | +| "The framework handles this" | Prove it with a test. | +| "I need to spike first" | Spikes are throwaway. Write the test on the real implementation. | +| "This is an integration concern" | Extract the unit. Integration tests come after unit tests. | +| "The existing codebase doesn't have tests" | You're adding tests now. One change at a time. | + **What this command does NOT do:** - Does NOT replace /map-efficient — it augments the Actor/Monitor loop with test-first phases - Does NOT work without a spec or plan — requires spec_.md or clear acceptance criteria @@ -103,6 +155,27 @@ python3 .map/scripts/map_orchestrator.py set_tdd_mode true This inserts TEST_WRITER (2.25) and TEST_FAIL_GATE (2.26) phases before ACTOR (2.3) in the step sequence. +### Check tdd.enforce Configuration + +```bash +TDD_ENFORCE=$(python3 -c " +import sys; sys.path.insert(0,'src') +try: + from mapify_cli.config.project_config import load_map_config + from pathlib import Path + cfg = load_map_config(Path('.map/config.yaml')) + print('true' if cfg.tdd_enforce else 'false') +except Exception: + print('false') +" 2>/dev/null || echo 'false') +``` + +When `TDD_ENFORCE=true`: +- The Iron Law applies: any code written before a failing test is **deleted** +- Spec compliance reviewer and code quality reviewer run after each ACTOR phase (mandatory) +- Monitor enforces the TDD violation check +- The rationalization table above is your reference — no excuse survives it + --- ## Step 1: State Machine Loop @@ -303,7 +376,83 @@ Output: standard Actor output (approach + code + trade-offs) ) ``` -After Actor returns, run the TDD Refactor step below, then call Monitor (2.4). Monitor validation is required before marking the subtask complete, including in TDD workflows. +After Actor returns, run the spec compliance reviewer and code quality reviewer (if `tdd.enforce: true`), then run the TDD Refactor step, then call Monitor (2.4). + +### Spec Compliance Reviewer (adversarial, MANDATORY when tdd.enforce=true) + +Spawn this BEFORE code quality review. The two reviews cannot be swapped. + +**Adversarial framing:** The implementer finished suspiciously quickly. Do NOT trust the Actor summary. Read the actual code. + +```python +Task( + subagent_type="actor", + description="Spec compliance review: subtask [ID]", + prompt=f"""You are an adversarial spec compliance reviewer. + +You have been told the implementer finished subtask [ID]. Do NOT trust their summary. +Read the actual code diff. Compare every requirement in the spec to the actual diff. + + +[paste AAG contract + validation_criteria from decomposition] + + + +[paste git diff for this subtask] + + +Check EVERY requirement: +1. Is each requirement implemented? Show file:line evidence. +2. Is there extra work not in the spec? (Scope creep, gold-plating) +3. Are there misunderstandings? (Implemented the wrong thing) +4. Are all edge cases from the spec covered? + +Output: +- SPEC-COMPLIANT: YES or NO +- If NO: list each gap with exact file:line reference and the spec line it violates +""" +) +``` + +**Gate**: If the reviewer returns `SPEC-COMPLIANT: NO`, route back to ACTOR with the specific gaps listed. Do NOT proceed to code quality review until spec passes. + +### Code Quality Reviewer (runs ONLY after spec reviewer passes) + +```python +Task( + subagent_type="actor", + description="Code quality review: subtask [ID]", + prompt=f"""You are a code quality reviewer. Spec compliance is already verified. + +Review the implementation quality only. + + +[paste git diff for this subtask] + + +Check: +1. **File responsibility**: Does each file do one thing? +2. **Unit decomposition**: Are functions small and independently testable? +3. **Plan conformance**: Does structure match the architectural plan? +4. **Size discipline**: No functions > 40 lines without documented justification +5. **Error handling**: All error paths handled explicitly +6. **Type safety**: No untyped `Any` without justification +7. **Naming**: Names describe behavior, not structure + +Output: +**Strengths**: (what is done well — be specific) +**Issues**: +- CRITICAL: [blocks proceeding — must fix before Monitor] +- IMPORTANT: [should fix in this subtask] +- MINOR: [acceptable to defer] +**Assessment**: PASS | PASS_WITH_MINOR | FAIL +""" +) +``` + +**Sequential gate**: Spec compliance review MUST complete with `SPEC-COMPLIANT: YES` before code quality review starts. Running them in parallel bypasses the gate. + +If code quality review returns `FAIL` (CRITICAL issues), route back to ACTOR. Once it returns `PASS` or `PASS_WITH_MINOR`, proceed to TDD Refactor and then Monitor. ### TDD Refactor: Clean Stale Red-Phase Comments diff --git a/tests/test_project_config.py b/tests/test_project_config.py index 22e2f975..1393f0d0 100644 --- a/tests/test_project_config.py +++ b/tests/test_project_config.py @@ -1,5 +1,6 @@ """ST-005: MapConfig fields max_actors + retry_degraded_once, and clamp helper. ST-000: MapConfig fields concurrent_dispatch + max_wave_retries (5b config). +Issue #285: tdd_enforce config flag and template content. Covers: VC1 — dotted-key aliasing and field parsing from YAML @@ -8,6 +9,8 @@ VC4 (ST-000) — concurrent_dispatch and max_wave_retries parsed/clamped correctly VC5 (ST-000) — clamp_max_wave_retries truth table VC6 (ST-000) — concurrent_dispatch + max_wave_retries are ACTIVE in runner/orchestrator (Slice 5b) + VC7 (#285) — tdd.enforce dotted-key alias and field defaults + VC8 (#285) — map-tdd SKILL.md and monitor.md template content """ from __future__ import annotations @@ -440,3 +443,144 @@ def test_per_repo_opt_out_concurrent_dispatch_false(self, tmp_path: Path) -> Non assert cfg.concurrent_dispatch is False, ( "execution.concurrent_dispatch: false in config.yaml must override the Slice 6 default" ) + + +# --------------------------------------------------------------------------- +# VC7 — tdd.enforce config flag (#285) +# --------------------------------------------------------------------------- + + +class TestVc7TddEnforce: + """VC7: tdd_enforce field defaults to False; tdd.enforce YAML alias is live.""" + + def test_vc7_default_is_false(self) -> None: + cfg = MapConfig() + assert cfg.tdd_enforce is False, ( + "MapConfig.tdd_enforce must default to False so existing workflows " + "are unaffected when the field is absent from config.yaml" + ) + + def test_vc7_absent_config_returns_false(self, tmp_path: Path) -> None: + cfg = load_map_config(tmp_path) + assert cfg.tdd_enforce is False + + def test_vc7_dotted_key_true_sets_field(self, tmp_path: Path) -> None: + _write_config(tmp_path, "tdd.enforce: true\n") + cfg = load_map_config(tmp_path) + assert cfg.tdd_enforce is True, ( + "tdd.enforce: true in config.yaml must set tdd_enforce=True — " + "alias is a dead toggle if this fails" + ) + + def test_vc7_dotted_key_false_leaves_field_false(self, tmp_path: Path) -> None: + _write_config(tmp_path, "tdd.enforce: false\n") + cfg = load_map_config(tmp_path) + assert cfg.tdd_enforce is False + + def test_vc7_alias_not_a_dead_toggle(self, tmp_path: Path) -> None: + """Per learned rule: dotted-YAML → snake_case alias must be explicitly verified.""" + _write_config(tmp_path, "tdd.enforce: true\n") + cfg = load_map_config(tmp_path) + assert cfg.tdd_enforce is True, ( + "tdd.enforce alias is a dead toggle — load_map_config sees the dotted key " + "but no snake_case field 'tdd_enforce' matched, so the value was silently " + "discarded. Add ('tdd.enforce', 'tdd_enforce') to the alias loop." + ) + + def test_vc7_field_exists_on_mapconfig(self) -> None: + assert hasattr(MapConfig(), "tdd_enforce"), ( + "MapConfig must have a 'tdd_enforce' field (#285)" + ) + + +# --------------------------------------------------------------------------- +# VC8 — template content: map-tdd SKILL.md and monitor.md (#285) +# --------------------------------------------------------------------------- + +_RENDERED_SKILLS = Path(__file__).parent.parent / ".claude" / "skills" +_RENDERED_AGENTS = Path(__file__).parent.parent / ".claude" / "agents" + + +class TestVc8TddTemplateContent: + """VC8: rendered templates contain required #285 enforcement content.""" + + def _skill_content(self) -> str: + path = _RENDERED_SKILLS / "map-tdd" / "SKILL.md" + assert path.exists(), f"map-tdd/SKILL.md not found at {path}" + return path.read_text(encoding="utf-8") + + def _monitor_content(self) -> str: + path = _RENDERED_AGENTS / "monitor.md" + assert path.exists(), f"monitor.md not found at {path}" + return path.read_text(encoding="utf-8") + + def test_vc8_iron_law_present(self) -> None: + assert "NO PRODUCTION CODE WITHOUT A FAILING TEST FIRST" in self._skill_content(), ( + "map-tdd/SKILL.md must contain the Iron Law enforcement text (#285)" + ) + + def test_vc8_red_green_refactor_cycle_present(self) -> None: + content = self._skill_content() + assert "RED-GREEN-REFACTOR" in content, ( + "map-tdd/SKILL.md must describe the RED-GREEN-REFACTOR mandatory cycle (#285)" + ) + + def test_vc8_rationalization_table_present(self) -> None: + content = self._skill_content() + assert "Rationalization Table" in content, ( + "map-tdd/SKILL.md must include the Rationalization Table (#285)" + ) + assert "Sunk cost fallacy" in content, ( + "Rationalization Table must include the sunk-cost-fallacy counter (#285)" + ) + + def test_vc8_red_flags_list_present(self) -> None: + content = self._skill_content() + assert "Red Flags" in content, ( + "map-tdd/SKILL.md must include the Red Flags self-detection list (#285)" + ) + + def test_vc8_spec_compliance_reviewer_present(self) -> None: + content = self._skill_content() + assert "Spec Compliance Reviewer" in content, ( + "map-tdd/SKILL.md must include the Spec Compliance Reviewer subagent (#285)" + ) + assert "SPEC-COMPLIANT" in content, ( + "Spec compliance reviewer must use SPEC-COMPLIANT verdict language (#285)" + ) + + def test_vc8_code_quality_reviewer_present(self) -> None: + content = self._skill_content() + assert "Code Quality Reviewer" in content, ( + "map-tdd/SKILL.md must include the Code Quality Reviewer subagent (#285)" + ) + + def test_vc8_sequential_gate_present(self) -> None: + content = self._skill_content() + assert "Sequential gate" in content or "sequential gate" in content.lower(), ( + "map-tdd/SKILL.md must document the spec-first/code-second sequential gate (#285)" + ) + + def test_vc8_tdd_enforce_config_check_present(self) -> None: + content = self._skill_content() + assert "tdd.enforce" in content, ( + "map-tdd/SKILL.md must reference tdd.enforce config key (#285)" + ) + + def test_vc8_monitor_tdd_violation_detection_present(self) -> None: + content = self._monitor_content() + assert "TDD Violation Detection" in content, ( + "monitor.md must contain TDD Violation Detection section (#285)" + ) + + def test_vc8_monitor_tdd_violation_verdict_present(self) -> None: + content = self._monitor_content() + assert "tdd_violation" in content, ( + "monitor.md must reference tdd_violation verdict for hard-stop enforcement (#285)" + ) + + def test_vc8_monitor_rationalization_detection_present(self) -> None: + content = self._monitor_content() + assert "Rationalization detection" in content or "rationalization" in content.lower(), ( + "monitor.md must include rationalization detection patterns (#285)" + ) diff --git a/tests/test_skills.py b/tests/test_skills.py index 53fd8fe0..1bfeaf33 100644 --- a/tests/test_skills.py +++ b/tests/test_skills.py @@ -107,6 +107,10 @@ _DEFAULT_SKILL_BODY_BUDGET = 515 HIGH_TRAFFIC_SKILL_BODY_BUDGETS = { "map-review": 560, + # map-tdd carries Iron Law enforcement (rationalization table, Red Flags, + # RED-GREEN-REFACTOR cycle), spec compliance reviewer dispatch, and code + # quality reviewer dispatch — all irreducible active control flow (#285). + "map-tdd": 545, } CLAUDE_MUTATION_BOUNDARY_SURFACES = [