Skip to content

Commit 22336e8

Browse files
authored
Merge pull request #159 from shinpr/fix/code-verifier-review-convergence
Simplify review status convergence
2 parents 4a931f4 + d8a29d4 commit 22336e8

130 files changed

Lines changed: 299 additions & 433 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.claude-plugin/marketplace.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
"name": "dev-workflows",
1313
"source": "./dev-workflows",
1414
"strict": true,
15-
"version": "0.24.1",
15+
"version": "0.24.2",
1616
"description": "Skills + Subagents for backend development - Use skills for coding guidance, or run recipe workflows for full orchestrated agentic coding with specialized agents",
1717
"author": {
1818
"name": "Shinsuke Kagawa",
@@ -82,7 +82,7 @@
8282
"name": "dev-workflows-frontend",
8383
"source": "./dev-workflows-frontend",
8484
"strict": true,
85-
"version": "0.24.1",
85+
"version": "0.24.2",
8686
"description": "Skills + Subagents for React/TypeScript - Use skills for coding guidance, or run recipe workflows for full orchestrated agentic coding with specialized agents",
8787
"author": {
8888
"name": "Shinsuke Kagawa",
@@ -155,7 +155,7 @@
155155
"name": "dev-workflows-fullstack",
156156
"source": "./dev-workflows-fullstack",
157157
"strict": true,
158-
"version": "0.24.1",
158+
"version": "0.24.2",
159159
"description": "Skills + Subagents for fullstack development (backend + React/TypeScript) - Use skills for coding guidance, or run recipe workflows for full orchestrated agentic coding with specialized agents",
160160
"author": {
161161
"name": "Shinsuke Kagawa",
@@ -244,7 +244,7 @@
244244
"name": "dev-skills",
245245
"source": "./dev-skills",
246246
"strict": true,
247-
"version": "0.24.1",
247+
"version": "0.24.2",
248248
"description": "Lightweight skills for users with existing workflows - coding best practices, testing principles, and design guidelines without recipe workflows or agents",
249249
"author": {
250250
"name": "Shinsuke Kagawa",

agents/acceptance-test-generator.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ Operates in an independent context, executing autonomously until task completion
1919

2020
### Implementation Approach Compliance
2121
- **Test Code Generation**: MUST strictly comply with Design Doc implementation patterns (function vs class selection)
22-
- **Contract Safety**: MUST enforce testing-principles skill mock creation and contract definition rules without exception
22+
- **Contract Safety**: Apply the testing-principles skill mock creation and contract definition rules to every generated skeleton
2323

2424
## Input Parameters
2525

@@ -50,15 +50,15 @@ Test type definitions, budgets, and ROI calculations are specified in **integrat
5050
| **System Context** | Requires full system integration? | Skip | [UNIT_LEVEL] |
5151
| **Upstream Scope** | In Include list? | Skip | [OUT_OF_SCOPE] |
5252

53-
**AC Include/Exclude Criteria**:
53+
**AC Selection Criteria**:
5454

5555
**Include** (High automation ROI):
5656
- Business logic correctness (calculations, state transitions, data transformations)
5757
- Data integrity and persistence behavior
5858
- User-visible functionality completeness
5959
- Error handling behavior (what user sees/experiences)
6060

61-
**Exclude** (Low ROI in LLM/CI/CD environment):
61+
**Use alternative verification** (Low ROI in LLM/CI/CD environment):
6262
- External service real connections → Use contract/interface verification instead
6363
- Performance metrics → Non-deterministic in CI, defer to load testing
6464
- Implementation details → Focus on observable behavior
@@ -116,7 +116,7 @@ ROI calculation formula and cost table are defined in **integration-e2e-testing
116116
- A downstream service receives a real event/message (e.g., topic publish, queue enqueue, webhook call)
117117
- An external service receives a real API call with the expected payload
118118
- Transactional consistency across services (e.g., two-phase commit, saga compensation)
119-
5. **Sort by ROI** within each lane using the score and tie-break order from integration-e2e-testing skill — this is the single ranking step; Phase 4 budget enforcement consumes this ranked list directly without re-sorting.
119+
5. **Sort by ROI** within each lane using the score and tie-break order from integration-e2e-testing skill — this is the single ranking step, and Phase 4 budget enforcement processes the ranked list in the order produced here.
120120

121121
**Output**: Ranked, deduplicated candidate list with lane assigned per E2E candidate.
122122

agents/code-reviewer.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ When `prior_feedback` is present, complete the correction re-review here:
7171

7272
For each acceptance criterion extracted in Step 1:
7373
- Search implementation files for the corresponding code
74-
- Determine status: fulfilled / partially fulfilled / unfulfilled
74+
- Mark it `fulfilled` only when evidence covers every governing boundary path; otherwise mark it `unfulfilled` and name each uncovered path in `gap`
7575
- Record the file path and relevant code location
7676
- Note any deviations from the Design Doc specification
7777
- For behavior-changing ACs, confirm the evidence covers the boundary paths, not only the main path: where a distinct branch, state, input class, lifecycle step, or fallback governs the behavior, verify it is exercised. Compare the source/referenced behavior and the implemented behavior at the same granularity; an unsupported change in a boundary dimension is a `dd_violation`
@@ -124,7 +124,7 @@ For each function/method in implementation files, check against coding-principle
124124
#### 3-3. Test Coverage for Acceptance Criteria
125125
- For each AC marked fulfilled: Glob/Grep for corresponding test cases
126126
- Record which ACs have test coverage and which do not
127-
- For each test claimed as AC coverage, inspect the test body and confirm at least one assertion exercises the AC's observable behavior. Tests that are `skip`/`xit`-marked (on tests that should run), contain only TODO/placeholder bodies, or use always-true assertions (e.g., `expect(true).toBe(true)`, `expect(arr.length).toBeGreaterThanOrEqual(0)`) do not count as AC coverage even when grep finds them; record those as `coverage_gap` with rationale explaining the substance issue. Tests verifying intentional absence (e.g., empty list, null result) are substantive when the absence is the AC's expectation.
127+
- For each test claimed as AC coverage, inspect the test body and count it as coverage only when at least one assertion exercises the AC's observable behavior. Record `skip`/`xit`-marked tests that should run, TODO/placeholder-only bodies, and always-true assertions (e.g., `expect(true).toBe(true)`, `expect(arr.length).toBeGreaterThanOrEqual(0)`) as `coverage_gap` even when grep finds them, with rationale explaining the substance issue. Tests verifying intentional absence (e.g., empty list, null result) are substantive when the absence is the AC's expectation.
128128
- Beyond substance, confirm each AC test exercises the claimed boundary and would turn red if the promised behavior regressed. When a task file is in scope, verify its Operation Verification Methods and optional Verification Focus. Missing required evidence is a `coverage_gap`.
129129

130130
#### Finding Classification
@@ -185,13 +185,13 @@ Verify against the Design Doc architecture:
185185
verdict: string ("pass" | "needs-improvement" | "needs-redesign")
186186
187187
acceptanceCriteria[].item: string
188-
acceptanceCriteria[].id: string (required only when status is not fulfilled; stable within this review chain)
189-
acceptanceCriteria[].status: string ("fulfilled" | "partially_fulfilled" | "unfulfilled")
188+
acceptanceCriteria[].id: string (required only when status is unfulfilled; stable within this review chain)
189+
acceptanceCriteria[].status: string ("fulfilled" | "unfulfilled")
190190
acceptanceCriteria[].confidence: string ("high" | "medium" | "low")
191191
acceptanceCriteria[].location: string (file:line; null if unimplemented)
192192
acceptanceCriteria[].evidence: string[] (each "source: file:line")
193-
acceptanceCriteria[].gap: string (null when fully fulfilled)
194-
acceptanceCriteria[].suggestion: string (null when fully fulfilled)
193+
acceptanceCriteria[].gap: string (null when status is fulfilled)
194+
acceptanceCriteria[].suggestion: string (null when status is fulfilled)
195195
196196
identifierVerification[].identifier: string
197197
identifierVerification[].id: string (required only when match is false; stable within this review chain)

agents/code-verifier.md

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,21 +54,21 @@ Stop expanding the search when additional evidence cannot change a discrepancy o
5454
- `conflict`: Observed behavior or a governing contract contradicts the document.
5555
- `unverified`: Available evidence cannot establish the claim; state the exact limitation and effect.
5656

57-
Use `critical` only when the issue makes approved scope incorrect, non-executable, or non-verifiable. Use `major` for a material correction and `minor` for non-blocking precision. Group locations that share one cause and correction into one discrepancy.
57+
Emit a discrepancy only when leaving it unresolved can change scope, feasibility, implementation, a contract, or verification. Group locations that share one cause and correction into one discrepancy.
5858

59-
Use `unverified` only for a specific material document claim whose unresolved truth can change scope, feasibility, implementation, a contract, or verification; assign it `critical` or `major`. Use `limitations` only for an evidence-access or coverage constraint that does not itself identify a material document claim. Record a fact in one place, not both; a material limitation becomes an `unverified` discrepancy.
59+
Use `unverified` only for a specific material document claim whose unresolved truth can change scope, feasibility, implementation, a contract, or verification. Use `limitations` only for an evidence-access or coverage constraint that does not itself identify a material document claim. Record a fact in one place, not both; a material limitation becomes an `unverified` discrepancy.
6060

6161
## Output
6262

6363
Return exactly one JSON object:
6464

6565
```json
6666
{
67-
"summary": {"docType": "design-doc", "documentPath": "docs/design/example.md", "status": "consistent|mostly_consistent|needs_review|inconsistent|blocked"},
67+
"summary": {"docType": "design-doc", "documentPath": "docs/design/example.md", "status": "consistent|needs_review|inconsistent|blocked"},
6868
"blockingReason": null,
6969
"inventoryCoverage": null,
7070
"discrepancies": [
71-
{"id": "D001", "status": "drift|gap|conflict|unverified", "severity": "critical|major|minor", "claim": "document claim", "documentLocation": "section or line", "codeLocation": "file:line or null", "relatedLocations": ["other location with the same cause"], "evidence": "observed fact", "effect": "why this changes scope, feasibility, implementation, contract, or verification"}
71+
{"id": "D001", "status": "drift|gap|conflict|unverified", "claim": "document claim", "documentLocation": "section or line", "codeLocation": "file:line or null", "relatedLocations": ["other location with the same cause"], "evidence": "observed fact", "effect": "why this changes scope, feasibility, implementation, contract, or verification"}
7272
],
7373
"limitations": ["exact evidence-access or coverage constraint and its verification effect"]
7474
}
@@ -86,12 +86,11 @@ When `unit_inventory` is supplied, replace `inventoryCoverage: null` with this o
8686

8787
For each inventory category, `accountedCount + excluded.length + unaccounted.length` equals `inputCount`. Report every unaccounted item as one cause-grouped `gap` discrepancy.
8888

89-
An unaccounted inventory item makes `consistent` and `mostly_consistent` invalid; use at least `needs_review`. When the supplied inventory cannot be parsed or the counts cannot be made balanced from it, use `blocked` and state the exact input defect.
89+
An unaccounted inventory item makes `consistent` invalid; use at least `needs_review`. When the supplied inventory cannot be parsed or the counts cannot be made balanced from it, use `blocked` and state the exact input defect.
9090

9191
Status rules:
9292

93-
- `consistent`: no discrepancy or material limitation exists;
94-
- `mostly_consistent`: only minor precision issues or non-material coverage limitations remain, and inventory has no unaccounted item;
93+
- `consistent`: no discrepancy exists;
9594
- `needs_review`: a repairable material discrepancy, including any `unverified` discrepancy, exists;
9695
- `inconsistent`: governing evidence contradicts the selected outcome or contract;
9796
- `blocked`: required input or repository evidence is unusable for the requested verification.

agents/integration-test-reviewer.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ Evaluate each test for:
7878
- Clear Arrange section (setup)
7979
- Single Act (action)
8080
- Meaningful Assert (verification)
81-
- Substantive assertion: each test must execute at least one assertion that observes the AC's behavior. Always-true assertions (e.g., `expect(true).toBe(true)`, `expect(arr.length).toBeGreaterThanOrEqual(0)`), TODO-only bodies, or leftover `skip`/`xit` markers on tests that should run do not count as substantive evidence. Tests verifying intentional absence (e.g., `expect(queryAllBy*).toHaveLength(0)`) are substantive when the absence is the AC's expectation
81+
- Substantive assertion: classify a test as substantive only when it executes at least one assertion that observes the AC's behavior. Classify always-true assertions (e.g., `expect(true).toBe(true)`, `expect(arr.length).toBeGreaterThanOrEqual(0)`), TODO-only bodies, and leftover `skip`/`xit` markers on tests that should run as insufficient evidence. Tests verifying intentional absence (e.g., `expect(queryAllBy*).toHaveLength(0)`) are substantive when the absence is the AC's expectation
8282
- Isolated state per test (reset in beforeEach)
8383
- Deterministic execution (mock time/random sources when needed)
8484

agents/investigator.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
---
22
name: investigator
3-
description: Comprehensively collects problem-related information and creates evidence matrix. Use PROACTIVELY when bug/error/issue/defect/not working/strange behavior is reported. Reports only observations without proposing solutions.
3+
description: Comprehensively collects problem-related information and creates evidence matrix. Use PROACTIVELY when bug/error/issue/defect/not working/strange behavior is reported. Reports observations and evidence for downstream cause verification.
44
tools: Read, Grep, Glob, LS, Bash, WebSearch, TaskCreate, TaskUpdate
55
skills:
66
- ai-development-guide
@@ -79,7 +79,7 @@ For each node listed in the path map, check whether there is a fault. A node is
7979
- It contains an inconsistency that can explain the user-reported symptom
8080

8181
If a fault is found, record it as a failure point with the required fields (see Output Format).
82-
- **Do NOT stop after finding the first fault** — check all remaining nodes on all mapped paths
82+
- After finding a fault, continue through every remaining node on all mapped paths
8383
- A single symptom can have multiple failure points at different layers
8484

8585
For each failure point found:
@@ -129,7 +129,7 @@ Disclose unexplored areas and investigation limitations.
129129
{
130130
"type": "code|history|dependency|config|document|external",
131131
"location": "Location investigated",
132-
"findings": "Facts discovered (without interpretation)"
132+
"findings": "Observed facts"
133133
}
134134
],
135135
"externalResearch": [

agents/quality-fixer-frontend.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ Use the indicators below for this review.
5252
- Functions with TODO comments but whose current logic is functionally correct
5353
- Legitimate empty returns or default values that match the expected behavior
5454

55-
**If any incomplete implementation is found**: Stop immediately. Return `status: "stub_detected"` without proceeding to quality checks (see Output Format).
55+
**If any incomplete implementation is found**: Stop at Step 1 and return `status: "stub_detected"` (see Output Format).
5656

5757
**If no incomplete implementation is found**: Proceed to Step 2.
5858

agents/quality-fixer.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ Use the indicators below for this review.
4646

4747
**Legitimate patterns** (treat as complete; proceed to Step 2): intentionally minimal implementations, functions with TODO comments but functionally correct logic, and legitimate empty/default returns that match the expected behavior.
4848

49-
**If any incomplete implementation is found**: Stop immediately. Return `status: "stub_detected"` without proceeding to quality checks (see Output Format).
49+
**If any incomplete implementation is found**: Stop at Step 1 and return `status: "stub_detected"` (see Output Format).
5050

5151
**If no incomplete implementation is found**: Proceed to Step 2.
5252

agents/scope-discoverer.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ Includes additional fields:
247247
Run each item below before producing the final JSON. When any item is unsatisfied, return to the relevant Step and complete it before producing the JSON output.
248248

249249
- [ ] Output is limited to scope discovery (no PRD or Design Doc content generated)
250-
- [ ] Every discovery is backed by evidence (no assumptions without sources)
250+
- [ ] Every discovery cites its source evidence
251251
- [ ] Low-confidence discoveries are reported with appropriate confidence markers
252252
- [ ] Triangulation strength reflects actual source count (weak noted when single-source)
253253
- [ ] Saturation check was performed before concluding discovery

0 commit comments

Comments
 (0)