Skip to content

Commit 030ae28

Browse files
committed
Fix issues with plan-docs QR being too overzealous
1 parent d4a26e1 commit 030ae28

16 files changed

Lines changed: 1276 additions & 926 deletions

skills/scripts/skills/planner/cli/plan.py

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@
7575
"architect": {"init", "set-milestone", "set-intent", "set-decision",
7676
"set-diagram", "add-diagram-node", "add-diagram-edge"},
7777
"developer": {"set-change"},
78-
"tw": {"set-doc", "set-readme", "translate", "set-diagram-render"},
78+
"tw": {"set-doc", "set-readme", "set-diagram-render"},
7979
"qr": {"validate"},
8080
}
8181

@@ -854,25 +854,6 @@ def run(cls, args: argparse.Namespace) -> None:
854854
# =============================================================================
855855

856856

857-
class TranslateCommand(Command):
858-
name = "translate"
859-
help = "Translate plan.json to Markdown"
860-
role = "tw"
861-
862-
@classmethod
863-
def add_arguments(cls, p: argparse.ArgumentParser) -> None:
864-
p.add_argument("--output", required=True, help="Output file path")
865-
866-
@classmethod
867-
def run(cls, args: argparse.Namespace) -> None:
868-
state_dir = get_state_dir()
869-
plan = load_plan(state_dir)
870-
871-
md = translate_to_markdown(plan)
872-
873-
Path(args.output).write_text(md)
874-
success(f"Translated to {args.output}")
875-
876857

877858
def translate_to_markdown(plan: "Plan") -> str:
878859
"""Generate Markdown from plan.json."""
@@ -1242,7 +1223,6 @@ def run(cls, args: argparse.Namespace) -> None:
12421223
SetDocCommand,
12431224
SetReadmeCommand,
12441225
SetDiagramRenderCommand,
1245-
TranslateCommand,
12461226
ValidateCommand,
12471227
ListMilestonesCommand,
12481228
ListIntentsCommand,

skills/scripts/skills/planner/cli/plan_commands.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -494,8 +494,8 @@ def set_diagram_render(ctx: PlanContext, diagram: str, content_file: str) -> dic
494494
return {"diagram": diagram, "operation": "updated"}
495495

496496

497-
def translate(ctx: PlanContext, output: str) -> dict:
498-
"""Translate plan.json to Markdown."""
497+
def _translate(ctx: PlanContext, output: str) -> dict:
498+
"""Translate plan.json to Markdown. Internal only -- not exposed via CLI."""
499499
from .plan import translate_to_markdown
500500

501501
plan = ctx.load_plan()

skills/scripts/skills/planner/orchestrator/planner.py

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,23 +56,28 @@
5656
MODULE_PATH = "skills.planner.orchestrator.planner"
5757

5858

59-
def _translate_plan(state_dir: str) -> None:
59+
def _translate_plan(state_dir: str) -> str | None:
6060
"""Mechanical translation: plan.json -> plan.md.
6161
62+
Returns path to plan.md on success, None on failure.
63+
6264
Why non-fatal: plan.json is the source of truth (the IR).
6365
plan.md is a convenience rendering. If translation fails,
6466
the workflow should still complete -- QR already approved
6567
the plan.json content.
6668
"""
6769
from pathlib import Path
68-
from skills.planner.cli.plan_commands import PlanContext, translate
70+
from skills.planner.cli.plan_commands import PlanContext, _translate
6971

7072
try:
73+
plan_md = str(Path(state_dir) / "plan.md")
7174
ctx = PlanContext(state_dir=Path(state_dir))
72-
translate(ctx, str(Path(state_dir) / "plan.md"))
75+
_translate(ctx, plan_md)
76+
return plan_md
7377
except Exception as e:
7478
import sys
7579
print(f"Warning: plan.md translation failed: {e}", file=sys.stderr)
80+
return None
7681

7782
_provider = PlannerResourceProvider()
7883

@@ -225,7 +230,7 @@ def handler(ctx):
225230
return handler
226231

227232

228-
def qr_decompose_step(title, phase, script, mode_total_steps, model=None):
233+
def qr_decompose_step(title, phase, script, model=None):
229234
"""Steps 4, 8, 12: QR decomposition dispatch.
230235
231236
Dispatches single QR agent to decompose artifact into verification items.
@@ -451,8 +456,7 @@ def handler(ctx):
451456
"5. CURRENT_UNDERSTANDING: how system works; for bugs: symptom + reproduction",
452457
"6. ASSUMPTIONS: unverified inferences with confidence H/M/L -- or 'none'",
453458
"7. INVISIBLE_KNOWLEDGE: design rationale, invariants, accepted tradeoffs",
454-
"8. USER_QUOTES: verbatim user statements, especially corrections",
455-
"9. REFERENCE_DOCS: paths to project docs sub-agents should read (doc/*.md, specs/*) -- or 'none'",
459+
"8. REFERENCE_DOCS: paths to project docs sub-agents should read (doc/*.md, specs/*) -- or 'none'",
456460
"",
457461
"FORMAT: High signal-to-noise. File refs over content. No ASCII diagrams.",
458462
"",
@@ -473,7 +477,6 @@ def handler(ctx):
473477
' "current_understanding": ["how system works", "bug: symptom + repro"],',
474478
' "assumptions": ["inference (H/M/L confidence)"] or ["none"],',
475479
' "invisible_knowledge": ["design rationale", "invariants", "tradeoffs"],',
476-
' "user_quotes": ["verbatim quote with context"],',
477480
' "reference_docs": ["doc/spec.md - what it specifies"] or ["none"]',
478481
"}",
479482
"",
@@ -485,8 +488,7 @@ def handler(ctx):
485488
"[ ] 3. At least one constraint OR explicit 'none confirmed'",
486489
"[ ] 4. Entry points identified OR 'greenfield'",
487490
"[ ] 5. Someone unfamiliar would understand why we're building this",
488-
"[ ] 6. User corrections/quotes preserved verbatim (if any)",
489-
"[ ] 7. Reference documentation paths captured or explicit 'none'",
491+
"[ ] 6. Reference documentation paths captured or explicit 'none'",
490492
"",
491493
"IF ANY CHECK FAILS: gather missing context via AskUserQuestion or exploration.",
492494
],
@@ -507,7 +509,6 @@ def handler(ctx):
507509
title="plan-design-qr-decompose",
508510
phase="plan-design",
509511
script="quality_reviewer/plan_design_qr_decompose.py",
510-
mode_total_steps=13,
511512
model="opus",
512513
),
513514
5: qr_verify_step(
@@ -537,7 +538,6 @@ def handler(ctx):
537538
title="plan-code-qr-decompose",
538539
phase="plan-code",
539540
script="quality_reviewer/plan_code_qr_decompose.py",
540-
mode_total_steps=13,
541541
model="opus",
542542
),
543543
9: qr_verify_step(
@@ -568,7 +568,6 @@ def handler(ctx):
568568
title="plan-docs-qr-decompose",
569569
phase="plan-docs",
570570
script="quality_reviewer/plan_docs_qr_decompose.py",
571-
mode_total_steps=13,
572571
model="opus",
573572
),
574573
13: qr_verify_step(
@@ -738,9 +737,12 @@ def main():
738737
# Why translate on terminal_pass: plan.json is the IR (modified by
739738
# QR fix cycles). plan.md is a rendered view. Terminal gate approval
740739
# signals plan.json is stable -- safe to regenerate the markdown.
741-
if result.terminal_pass and args.state_dir:
742-
_translate_plan(args.state_dir)
743740
print(result.output)
741+
if result.terminal_pass and args.state_dir:
742+
plan_path = _translate_plan(args.state_dir)
743+
if plan_path:
744+
print(f"\nPlan rendered to: {plan_path}")
745+
print("Copy this file to the user's requested output path.")
744746
else:
745747
print(result)
746748

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,8 @@
1-
"""Quality Reviewer scripts for QR phases.
1+
"""Quality reviewer modules."""
22

3-
This package contains:
4-
- Base classes for decompose and verify workflows
5-
- Phase-specific QR scripts (plan-design, plan-code, plan-docs, impl-code, impl-docs)
6-
7-
Base classes:
8-
- DecomposeBase: Shared logic for QR item generation
9-
- VerifyBase: Shared logic for single-item verification
10-
11-
Scripts extend base classes with phase-specific logic.
12-
"""
13-
14-
from .qr_decompose_base import DecomposeBase, write_qr_state
15-
from .qr_verify_base import VerifyBase
3+
# Export shared utilities from new location
4+
from .prompts.decompose import write_qr_state
165

176
__all__ = [
18-
"DecomposeBase",
197
"write_qr_state",
20-
"VerifyBase",
218
]

skills/scripts/skills/planner/quality_reviewer/impl_code_qr_decompose.py

Lines changed: 99 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,121 @@
11
#!/usr/bin/env python3
22
"""QR decomposition for impl-code phase.
33
4-
INTENT.md requires separate files per phase.
5-
This file contains ONLY impl-code-specific item generation.
6-
Shared logic lives in qr_decompose_base.py.
7-
8-
Impl-code checks:
9-
- Acceptance criteria verification (factored: expectations vs observations)
10-
- Cross-cutting concerns (shared state, error propagation)
11-
- Code quality (all 8 quality documents)
12-
- Intent marker validation
4+
Scope: Implemented code quality.
5+
- Acceptance criteria verification (expectations vs observations)
6+
- Cross-cutting concerns (shared state, error propagation)
7+
- Code quality (all 8 quality documents)
8+
- Intent marker validation
9+
NOT plan structure or documentation (verified in earlier phases).
10+
11+
Severity categories: Same as plan-code (Dev can fix code issues).
1312
"""
1413

15-
from .qr_decompose_base import DecomposeBase
14+
from skills.planner.quality_reviewer.prompts.decompose import dispatch_step
1615

1716

18-
class ImplCodeDecompose(DecomposeBase):
19-
"""QR decomposition for impl-code phase."""
17+
PHASE = "impl-code"
2018

21-
PHASE = "impl-code"
2219

23-
def get_artifact_prompt(self) -> str:
24-
"""Impl-code reads plan.json and modified files from codebase."""
25-
return """Read plan.json from STATE_DIR:
20+
STEP_1_ABSORB = """\
21+
Read plan.json from STATE_DIR:
2622
cat $STATE_DIR/plan.json | jq '.'
2723
2824
Also read MODIFIED_FILES from codebase (paths from milestones).
2925
26+
SCOPE: Implemented code quality.
27+
3028
Focus on:
31-
- milestones[].acceptance_criteria
32-
- Actual implemented code in modified files
33-
- Code quality (structure, patterns, documentation)"""
29+
- milestones[].acceptance_criteria (expectations)
30+
- Actual implemented code in modified files (observations)
31+
- Code quality (structure, patterns, documentation)
32+
- Intent markers in implemented code
33+
34+
OUT OF SCOPE:
35+
- Plan structure (already verified in plan-design)
36+
- Documentation files (impl-docs phase)"""
37+
38+
39+
STEP_2_CONCERNS = """\
40+
Brainstorm concerns specific to IMPLEMENTED CODE:
41+
- Acceptance criteria not met
42+
- Cross-cutting concerns broken (error handling, logging)
43+
- Code quality violations (god objects, god functions)
44+
- Missing or invalid intent markers
45+
- Implementation drift from plan
46+
47+
DO NOT brainstorm plan structure or documentation concerns."""
3448

35-
def get_enumeration_guidance(self) -> str:
36-
"""Return phase-specific enumeration guidance for Step 3."""
37-
return """For impl-code, enumerate:
49+
50+
STEP_3_ENUMERATION = """\
51+
For impl-code, enumerate IMPLEMENTATION ARTIFACTS:
52+
53+
ACCEPTANCE CRITERIA:
3854
- Each milestone with acceptance_criteria (ID, criteria count)
39-
- Files modified per milestone
40-
- Cross-cutting concerns mentioned (error handling, logging, etc.)
41-
- Code quality aspects to verify"""
55+
- Each criterion (ID, expectation text)
56+
57+
FILES:
58+
- Files modified per milestone (path list)
59+
- Actual file content (read from codebase)
60+
61+
CROSS-CUTTING:
62+
- Error handling patterns used
63+
- Logging patterns used
64+
- Shared state access patterns
65+
66+
CODE QUALITY:
67+
- Function sizes (line counts)
68+
- Nesting depths
69+
- Dependency counts"""
70+
71+
72+
STEP_5_GENERATE = """\
73+
SEVERITY ASSIGNMENT (per conventions/severity.md, impl-code scope):
74+
75+
MUST (blocks all iterations):
76+
- Acceptance criterion not met
77+
- MARKER_INVALID: intent marker without valid explanation
78+
79+
SHOULD (iterations 1-4) - STRUCTURE categories:
80+
- GOD_OBJECT: >15 methods OR >10 deps
81+
- GOD_FUNCTION: >50 lines OR >3 nesting
82+
- CONVENTION_VIOLATION: violates documented project convention
83+
- INCONSISTENT_ERROR_HANDLING: mixed exceptions/codes
84+
85+
COULD (iterations 1-3) - COSMETIC:
86+
- DEAD_CODE: unused functions, impossible branches
87+
- FORMATTER_FIXABLE: style issues"""
88+
89+
90+
COMPONENT_EXAMPLES = """\
91+
- A modified file
92+
- A milestone's implementation
93+
- A cross-cutting concern pattern"""
94+
95+
96+
CONCERN_EXAMPLES = """\
97+
- Acceptance criteria compliance
98+
- Error handling consistency
99+
- Code structure quality"""
100+
101+
102+
PHASE_PROMPTS = {
103+
1: STEP_1_ABSORB,
104+
2: STEP_2_CONCERNS,
105+
3: STEP_3_ENUMERATION,
106+
5: STEP_5_GENERATE,
107+
}
108+
109+
GROUPING_CONFIG = {
110+
"component_examples": COMPONENT_EXAMPLES,
111+
"concern_examples": CONCERN_EXAMPLES,
112+
}
42113

43114

44-
def get_step_guidance(step: int, total_steps: int, module_path: str = None, **kwargs) -> dict:
45-
"""Entry point for workflow execution."""
115+
def get_step_guidance(step: int, module_path: str = None, **kwargs) -> dict:
46116
module_path = module_path or "skills.planner.quality_reviewer.impl_code_qr_decompose"
47-
decomposer = ImplCodeDecompose()
48-
return decomposer.get_step_guidance(step, total_steps, module_path=module_path, **kwargs)
117+
state_dir = kwargs.get("state_dir", "")
118+
return dispatch_step(step, PHASE, module_path, PHASE_PROMPTS, GROUPING_CONFIG, state_dir)
49119

50120

51121
if __name__ == "__main__":

0 commit comments

Comments
 (0)