dev/claude/ADR 388

[jodavis-claude]

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

[github-actions]

build-and-test: Python test results

Status: ✅ Passed

Test log

[jodavis-claude]

Reviewed both researcher-proposal-review (renamed from researcher-design-review) and the new researcher-detailed-design-review.

Verified against the task brief's exit criteria and key design decisions:

• skills/researcher-design-review/ is gone (confirmed via git mv); skills/researcher-proposal-review/SKILL.md exists with matching name: frontmatter and scope trimmed to the four problem/solution-fit bullets.
• skills/researcher-detailed-design-review/SKILL.md exists and checks user-scenario completeness, requirements-table well-formedness, success-metric observability, and deliverable independence, exactly as specified.
• Cross-checked the old skill's step 4/5 bullets against both new files: every concern (falsifiable problem, solution fit, non-goals, alternatives → proposal-review; success-metric observability, user-scenario completeness, deliverable independence → detailed-design-review) moved exactly once, no gap, no duplication. The new requirements-table well-formedness check has no prior-skill equivalent, as expected.
• Section names referenced (## User Scenarios, ## Functional Requirements, ## Success Metrics, ## Deliverables) match detailed_design_template.md exactly, including the "guiding principle in prose, not its own row" and "group by ### As a <type>..." nuances.
• Cross-reference framing between the two skills is correct and mutually consistent ("that's researcher-proposal-review's job on the Proposal" / "that's researcher-detailed-design-review's job" wording added appropriately).
• No stray self-references to the old researcher-design-review name in either new file. Confirmed the explicitly out-of-scope stale references (README.md, agents/researcher.md, commands/write-design-spec.md, commands/add-to-spec.md, document-readiness-review/SKILL.md) were correctly left untouched, per the brief (ADR-395/ADR-394's job).
• No _doc_*.md architecture doc describes this skill directory, so no documentation update was needed.
• No unit/E2E tests expected or needed — both components are Wrapper-tier prompt/config content, consistent with Component Taxonomy.

No Priority 1-4 (correctness, security, performance, documentation) issues found.

Non-blocking notes for awareness (not requesting changes):

• researcher-proposal-review's prose still says "design doc"/"the design" throughout rather than "Proposal" — the developer flagged this as an open question in the brief and made a defensible minimal-rename call; a future terminology pass could tighten this, but it isn't required by the exit criteria.
• The Validate exit criterion's real target (Task 2's ADR-380 Proposal/Detailed Design, produced by ADR-386) doesn't exist in the tree yet, so the developer substituted throwaway stand-in documents — this was pre-authorized by the brief's own "Known ambiguities" fallback. Worth re-running the Validate step against the real ADR-380 artifacts once ADR-386 lands, but not a blocker here since the scoping behavior being validated is structural and the stand-in exercised it equivalently.
• Minor: the stack-generated PR title ("dev/claude/ADR 388") isn't descriptive — cosmetic only, not blocking.

Approved.

[jodavis-claude]

Sign-off review for ADR-388 (PR #238): split researcher-design-review into researcher-proposal-review (renamed) and researcher-detailed-design-review (new).

Checked for unresolved review threads from the prior review pass: none exist — the prior review (submitted 2026-09-03T21:06:08Z, after both of this PR's commits) posted its findings as a single summary comment with no inline threads, and no Priority 1-4 issues were raised. GraphQL reviewThreads query confirms zero threads on this PR.

No commits have landed since that review, so there are no newly modified files to re-scan for regressions. Re-confirmed the diff against the task brief's exit criteria:

• skills/researcher-design-review/ no longer exists (git-mv rename); skills/researcher-proposal-review/SKILL.md exists with matching name: frontmatter, scope correctly trimmed to the four problem/solution-fit bullets.
• skills/researcher-detailed-design-review/SKILL.md exists and checks user-scenario completeness, requirements-table well-formedness, success-metric observability (against ## Success Metrics), and deliverable independence.
• Every concern from the old skill moved exactly once between the two new skills — no gap, no duplication.
• Validation log (scripts/validate-build.sh / scripts/validate-tests.sh) shows a clean pass.

No new Priority 1-4 issues found. Non-blocking notes carried over from the prior review (not required to fix): researcher-proposal-review's prose still says "design doc" rather than "Proposal" throughout, and the Validate exit criterion was exercised against throwaway stand-in documents rather than the real ADR-380 artifacts (ADR-386 not yet landed) — worth re-running once available, but not a blocker.

Signing off: approved.

[jodavis-claude]

Suggested change

	- It provides value entirely on its own, without depending on another deliverable shipping first.
	- It provides value entirely on its own, without another deliverable shipping after it.

This is actually the metric. Deliverables might depend on other deliverables coming first, but there should be some value at the end of this deliverable

[jodavis-claude]

Suggested change

	- It cannot be meaningfully done "partway" — there's no sensible partial version.
	- It cannot be meaningfully done "partway" — there's no value provided without the full implementation.