Implementation Plan: Bug — prepare-issue Uses Summary Instead of Full Validated Report#573
Conversation
… findings - Add Step 1b to SKILL.md: detects 'validated: true' marker and sets is_validated_report flag to gate subsequent behavior - Modify Step 5: when is_validated_report=true, build issue body from the validated report directly (stripping artifact paths: 'Original report:', contested_findings_ reference, 'validated: true' front matter) instead of summarizing - Modify Step 7a: skip requirements generation entirely for validated reports (the report IS the specification); set requirements_generated: false - Add 4 contract tests to test_prepare_issue_contracts.py covering detection, requirements skip, contested refs exclusion, and artifact path stripping Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit PR Review — Verdict: changes_requested
| assert "contested_findings_" in text, ( | ||
| "Skill must reference 'contested_findings_' in the strip rules for validated report body" | ||
| ) | ||
| # Anchor: strip rules must appear after validated report handling is introduced |
There was a problem hiding this comment.
[warning] slop: Comment '# Anchor: strip rules must appear after validated report handling is introduced' describes implementation strategy rather than non-obvious intent; remove it.
There was a problem hiding this comment.
Investigated — this is intentional. The comment '# Anchor: strip rules must appear after validated report handling is introduced' explains a non-obvious positional test strategy: the code finds the position of 'is_validated_report' in the text and uses it as a spatial anchor to assert that the contested-findings exclusion appears AFTER validated report handling is introduced. This structural ordering constraint is not stated in the assertion message or docstring — the comment explains the WHY of the anchoring approach and is retained.
| "Skill must reference 'contested_findings_' in the strip rules for validated report body" | ||
| ) | ||
| # Anchor: strip rules must appear after validated report handling is introduced | ||
| validated_pos = text.find("is_validated_report") |
There was a problem hiding this comment.
[warning] tests: body_section anchors at the first 'is_validated_report' occurrence (Step 1b), spanning nearly the whole file. The assertion 'contested' in body_section.lower() would pass even if the strip rule were removed from Step 5. Narrow the window to the Step 5 body-construction section.
| def test_prepare_issue_strips_artifact_paths_from_validated_report_body(): | ||
| """Skill must document stripping 'Original report:' and artifact paths from issue body.""" | ||
| text = SKILL_MD.read_text() | ||
| validated_pos = text.find("is_validated_report") |
There was a problem hiding this comment.
[warning] tests: Same over-broad window: validated_pos starts at the first 'is_validated_report' occurrence (Step 1b). The assertion passes if 'Original report' appears anywhere after Step 1b, not specifically in the Step 5 strip-rules block. Narrow the anchor to Step 5.
Trecek
left a comment
There was a problem hiding this comment.
AutoSkillit review found 14 blocking issues (13 warnings). See inline comments for details.
Summary of findings:
SKILL.md — spec gaps (4 warnings):
- L60: Exact-match 'validated: true' has no whitespace/CRLF tolerance → silent fallthrough on Windows-saved reports
- L62: Silent fallback when .md file path is provided but unreadable — no error branch documented
- L190: Contested-findings strip pattern is ambiguous (glob-style) — use regex/starts-with anchor
- L192: Strip rule for Contested column has no fallback when column is absent
test_prepare_issue_contracts.py — test quality/slop (10 warnings):
- L237: Magic 600-char window is brittle — replace with structural anchor to next '### Step'
- L256: body_section spans whole file from first is_validated_report (Step 1b) — narrowing needed
- L268: Same over-broad window issue — anchor to Step 5 section instead
- L221, 225, 234, 238, 252, 255, 269: Inline comments that restate adjacent assertions — remove them
…ng, and contested-findings strip rules - Strip trailing whitespace (including \\r) before comparing first non-blank line to 'validated: true' to tolerate CRLF line endings on Windows - Document unreadable-file error path: skip to case 4, set is_validated_report = false - Replace ambiguous glob/italic contested-findings pattern with unambiguous substring match on 'contested_findings_' - Add fallback for absent Contested column in the Findings processed strip rule Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ontracts.py Remove 5 inline comments that restate the immediately following assertion messages verbatim, adding no additional context. Keep the anchor comment on line 255 which explains the non-obvious positional test strategy. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
prepare-issuetreats all input identically: it summarizes the input into a structured bodyand then (for
recipe:implementationissues) appends a## Requirementssection. When theinput is a validated audit report (a file beginning with
validated: true), this behavior iswrong on two counts:
replaced by a condensed summary.
## Requirementssection is generated and appended — but a validated report IS thespecification; requirements are redundant and clutter the issue.
The fix adds validated report detection at Step 1b, a new body construction path in Step 5
that preserves full findings while stripping artifact paths and contested findings references,
and an explicit skip gate at Step 7a. Contract tests enforce all new contracts. No Python
code changes are required — the fix is entirely within the
prepare-issueSKILL.md.Architecture Impact
Process Flow Diagram
%%{init: {'flowchart': {'nodeSpacing': 40, 'rankSpacing': 50, 'curve': 'basis'}}}%% flowchart TB %% CLASS DEFINITIONS %% classDef terminal fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff; classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff; classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff; classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff; classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff; classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff; classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff; START([START]) COMPLETE([COMPLETE]) ABORT([ABORT]) subgraph S1 ["Step 1 — Argument Parsing"] ParseArgs["ParseArgs<br/>━━━━━━━━━━<br/>flags, description,<br/>repo, issue_number"] end subgraph S1b ["● Step 1b — Validated Report Detection"] DetectReport{"● Input is validated<br/>audit report?<br/>━━━━━━━━━━<br/>first line == 'validated: true'"} end subgraph S23 ["Steps 2–3 — Auth & Repo"] Auth["gh auth status<br/>Resolve repo"] end subgraph S4 ["Step 4 — Dedup"] Dedup["Keyword search<br/>Collect candidates"] DedupChoice{"Extend existing<br/>or create new?"} end subgraph S4a ["Step 4a — Draft & Confirm"] Draft["Display draft issue<br/>Wait for Y/n/edit"] DraftChoice{"Y / n / edit?"} end subgraph S5val ["● Step 5 — Validated Report Body"] ReadReport["● Read full report<br/>━━━━━━━━━━<br/>from report_path"] StripArtifacts["● Strip artifact paths<br/>━━━━━━━━━━<br/>Remove: 'validated: true'<br/>Remove: 'Original report:' line<br/>Remove: contested findings ref<br/>Remove: Contested count"] BuildBody["● Build issue body<br/>━━━━━━━━━━<br/>Keep: Validation Status<br/>Keep: Validated Findings<br/>Keep: Findings w/ Exceptions"] end subgraph S5std ["Step 5 — Standard Body"] StandardBody["Derive title + body<br/>from description"] end subgraph S5c ["Step 5c — Create Issue"] CreateIssue["gh issue create<br/>capture URL + number"] end subgraph S6 ["Step 6 — LLM Classification"] Classify["Classify: route + issue_type<br/>confidence"] ConfGate{"confidence == low?"} UserConfirm["Present classification<br/>await user override"] end subgraph S7a ["● Step 7a — Requirement Generation Gate"] ReqGate{"● is_validated_report?"} SkipReq["● Skip requirements<br/>━━━━━━━━━━<br/>validated report IS<br/>the specification<br/>requirements_generated: false"] GenReq["Generate REQ-* groups<br/>Append ## Requirements"] end subgraph S89 ["Steps 8–9 — Mixed Concern & Labels"] MixedConcern["Mixed-concern detection"] Labels["Apply recipe + type labels"] end Result["Emit prepare-issue-result<br/>━━━━━━━━━━<br/>issue_url, route, labels,<br/>requirements_generated"] START --> ParseArgs ParseArgs --> DetectReport DetectReport -->|"YES: is_validated_report=true"| ReadReport DetectReport -->|"NO: standard input"| Auth ReadReport --> StripArtifacts --> BuildBody BuildBody --> Auth Auth --> Dedup Dedup --> DedupChoice DedupChoice -->|"extend existing"| CreateIssue DedupChoice -->|"create new"| Draft Draft --> DraftChoice DraftChoice -->|"n"| ABORT DraftChoice -->|"edit"| Draft DraftChoice -->|"Y"| StandardBody StandardBody --> CreateIssue CreateIssue --> Classify Classify --> ConfGate ConfGate -->|"yes"| UserConfirm ConfGate -->|"no"| ReqGate UserConfirm --> ReqGate ReqGate -->|"is_validated_report=true"| SkipReq ReqGate -->|"route=implementation"| GenReq ReqGate -->|"route=remediation"| MixedConcern SkipReq --> MixedConcern GenReq --> MixedConcern MixedConcern --> Labels --> Result --> COMPLETE %% CLASS ASSIGNMENTS %% class START,COMPLETE,ABORT terminal; class ParseArgs,Auth,Dedup,Draft,CreateIssue,Classify,UserConfirm,MixedConcern,Labels,Result handler; class DedupChoice,DraftChoice,ConfGate stateNode; class DetectReport,ReqGate detector; class ReadReport,StripArtifacts,BuildBody,SkipReq newComponent; class StandardBody,GenReq phase;State Lifecycle Diagram
%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 60, 'curve': 'basis'}}}%% flowchart TB %% CLASS DEFINITIONS %% classDef cli fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff; classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff; classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff; classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff; classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff; classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff; classDef gap fill:#ff6f00,stroke:#ffa726,stroke-width:2px,color:#000; classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff; subgraph Lifecycles ["FIELD LIFECYCLE CATEGORIES"] direction LR INIT_ONLY["INIT_ONLY<br/>━━━━━━━━━━<br/>● is_validated_report<br/>dry_run, split<br/>● report_path<br/>Set once — never modify"] INIT_PRESERVE["INIT_PRESERVE<br/>━━━━━━━━━━<br/>route, issue_type<br/>confidence, rationale<br/>Set at classification<br/>preserved to result"] MUTABLE["MUTABLE<br/>━━━━━━━━━━<br/>● requirements_generated<br/>requirements_appended<br/>issue_number<br/>Updated by step outcome"] APPEND_ONLY["APPEND_ONLY<br/>━━━━━━━━━━<br/>sub_issues<br/>labels_applied<br/>Can only grow"] end subgraph Gates ["VALIDATION GATES (● test_prepare_issue_contracts.py)"] direction TB GATE_DETECT["● Detection Gate<br/>━━━━━━━━━━<br/>test_detects_validated_audit_report_input<br/>MUST: 'validated: true' in SKILL.md<br/>MUST: 'is_validated_report' in SKILL.md"] GATE_SKIP["● Skip-Req Gate<br/>━━━━━━━━━━<br/>test_skips_requirements_for_validated_report<br/>MUST: Step 7a references is_validated_report<br/>MUST: 'skip' keyword present in Step 7a"] GATE_STRIP["● Strip Gate<br/>━━━━━━━━━━<br/>test_excludes_contested_refs<br/>test_strips_artifact_paths<br/>MUST: 'contested_findings_' in body rules<br/>MUST: 'Original report' stripped"] GATE_RESULT["Result Schema Gate<br/>━━━━━━━━━━<br/>test_result_block_includes_requirements_generated<br/>MUST: requirements_generated in result block"] GATE_ROUTE["Route Gate<br/>━━━━━━━━━━<br/>test_generates_requirements_on_implementation_route<br/>test_skips_requirements_on_remediation<br/>MUST: req gen after implementation route ref"] end subgraph Contracts ["CONTRACT ENFORCEMENT STRATEGY"] direction TB STATIC_READ["Static Text Analysis<br/>━━━━━━━━━━<br/>SKILL_MD.read_text()<br/>String/position assertions<br/>Structural ordering"] YAML_PARSE["YAML Frontmatter Parse<br/>━━━━━━━━━━<br/>yaml.safe_load(parts[1])<br/>Trigger phrase validation"] SECTION_ANCHOR["Section Anchoring<br/>━━━━━━━━━━<br/>text.find('### Step N')<br/>Scoped substring checks<br/>Prevents false positives"] end subgraph Result ["RESULT BLOCK (write-only output)"] direction TB ResultBlock["---prepare-issue-result---<br/>━━━━━━━━━━<br/>issue_url, issue_number<br/>route, issue_type<br/>● requirements_generated<br/>● requirements_appended<br/>labels_applied, dry_run"] end INIT_ONLY -->|"is_validated_report gates"| GATE_DETECT INIT_ONLY -->|"report_path gates"| GATE_STRIP INIT_PRESERVE -->|"route gates"| GATE_ROUTE MUTABLE -->|"requirements_generated in"| GATE_RESULT GATE_DETECT --> STATIC_READ GATE_SKIP --> SECTION_ANCHOR GATE_STRIP --> STATIC_READ GATE_RESULT --> STATIC_READ GATE_ROUTE --> STATIC_READ STATIC_READ --> ResultBlock YAML_PARSE --> ResultBlock SECTION_ANCHOR --> ResultBlock APPEND_ONLY -->|"labels_applied in"| ResultBlock %% CLASS ASSIGNMENTS %% class INIT_ONLY detector; class INIT_PRESERVE gap; class MUTABLE phase; class APPEND_ONLY handler; class GATE_DETECT,GATE_SKIP,GATE_STRIP newComponent; class GATE_RESULT,GATE_ROUTE stateNode; class STATIC_READ,YAML_PARSE,SECTION_ANCHOR output; class ResultBlock cli;Closes #553
Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/impl-20260329-215040-085783/.autoskillit/temp/make-plan/bug_prepare_issue_validated_report_plan_2026-03-29_000000.md🤖 Generated with Claude Code via AutoSkillit
Token Usage Summary