ADR-320: Adopt Method-marker and Design doc header conventions across spec pipeline - #71
Conversation
… spec has Method markers
build-and-test: Python test resultsStatus: ✅ Passed Test log |
jodavis-claude
left a comment
There was a problem hiding this comment.
Reviewed against the ADR-320 task brief exit criteria and playbook-contract (the normative source for the Method-marker and Design-doc conventions). All five exit criteria are met and the implementation matches the reference model precisely:
spec-discussionguard — added under step 2, correctly distinguishes Method markers (> [!NOTE]/> **Method:**) from> **Review:**markers and states they must never be resolved or removed.spec-first-draftreview-pass message — step 3's pause message now teaches the Method marker callout alongside the existing> **Review:**instruction, with a clear contrast ("it's not a review comment and won't be resolved or removed").spec-first-draftheader template —> **Will become:**replaced with> **Design doc:**, wording matches_spec_PlaybookHarvesting.md's reference header verbatim.final-sign-offpassive note — new step 5 locates the spec viadocumentation.specs.searchsubstitutingwork-item-id(matches the documentedget-project-configurationconvention and thefind-repo-documentationprecedent exactly), skips silently when no spec is found, and only passively mentions/harvestavailability in the Jira comment — never auto-invokesharvest-playbook.- Existing spec retrofits —
_spec_AgentOrchestration.md,_spec_ConcurrentDevelopment.md, and_spec_TddForImplementation.mdall correctly retrofitted, with theEpic:/Feature-work-item:lines' positions preserved. Confirmed via repo-wide grep that no staleWill becomeheader lines remain anywhere (_spec_PlaybookHarvesting.md's historical body prose is intentionally untouched)._spec_ProposedSkillsBreakdown.mdcorrectly left alone — it has no standard header to retrofit.
No correctness, security, performance, or documentation issues found — these are all Wrapper-tier prose/header edits with no executable code paths, consistent with the task brief's assessment. No stale references to the old "Will become" convention were left anywhere in the repo. No inline comments needed.
One minor, non-blocking observation for future consideration (not introduced by this PR — it's inherited from the already-merged documentation.specs.search config convention): if a work-item-id happens to match multiple _spec_*.md files via the grep search, final-sign-off step 5 has no explicit tie-breaking rule. This mirrors an existing ambiguity in find-repo-documentation's analogous search and isn't something this task's scope should fix.
Approving.
- Revert final-sign-off's passive /harvest mention (commit 3fce0a6) — not worth the added complexity for a reminder comment, per review feedback. - spec-first-draft: name spec-task-breakdown as the explicit owner of the "final documentation task" the Design doc header line refers to, and state the invariant that every task breakdown must append it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RBnDBvr47h4fVqNte6feE2
Work item
ADR-320 — Adopt the Method-marker and "Design doc" header conventions defined by
playbook-contract(ADR-317, already merged) across the four spec-pipeline touchpoints that must recognize or teach them, and retrofit existing_spec_*.mdfiles in this repo to the new header form.Changes
plugins/dev-team/skills/spec-discussion/SKILL.md— added a guard note under step 2 ("Collect review markers") stating that> [!NOTE]/> **Method:**callouts are not review comments and must never be resolved or removed by this skill.plugins/dev-team/skills/spec-first-draft/SKILL.md— two edits: (1) step 3's pause message now also teaches the> [!NOTE]/> **Method:**callout as the way to record methodology rationale, distinct from> **Review:**; (2) step 2's header template replaces the> **Will become:**line with a> **Design doc:**line matching_spec_PlaybookHarvesting.md's reference form, using_doc_<FeatureName>.mdas the generic placeholder.plugins/dev-team/skills/final-sign-off/SKILL.md— added a new step 5 ("Check for a Method marker to mention") that locates the spec viadocumentation.specs.search(substituting thework-item-idargument already passed to this skill), checks it for a Method marker, and — if found — has the renumbered step 6 ("Add a Jira comment") append a passive sentence mentioning/harvestavailability. Skips silently if no spec is found. Never invokesharvest-playbook. Renumbered the former steps 5-6 to 6-7._spec_AgentOrchestration.md— header retrofit:> **Will become:**line replaced with the> **Design doc:**form._spec_ConcurrentDevelopment.md— same retrofit; the> **Epic:**line's position was preserved, only the Will-become line was replaced._spec_TddForImplementation.md— same retrofit; the> **Feature-work-item:**line's position was preserved, only the Will-become line was replaced._spec_PlaybookHarvesting.md— not modified; already uses the new header form (it is the reference model for the other retrofits)._spec_ProposedSkillsBreakdown.md— not modified; has no> **Status:**/> **Will become:**header to retrofit (raw planning notes, not a standard spec), consistent with the task brief's recommendation to leave it out of scope.Design decisions
final-sign-off's new Method-marker check as its own step 5, immediately before the Jira-comment step, and had the Jira-comment step (renumbered to 6) conditionally append the/harvestmention — rather than folding the whole check inline into the comment step — so the spec-lookup logic and skip-silently behavior are easy to read in isolation, per the brief's "implementation judgment" latitude on placement.spec-first-draft's pause message to explicitly contrast it with> **Review:**("it's not a review comment and won't be resolved or removed") since that distinction is the entire point of the guard added elsewhere in this task._spec_ProposedSkillsBreakdown.mdwas left untouched per the brief's own recommendation — it has no standard header to mechanically retrofit.Testing completed
None required. All five commits are Wrapper-tier edits to skill prose or mechanical spec-header retrofits; per
component-taxonomy, Wrapper components get no dedicated test (visual inspection is sufficient), and this repo has no test or E2E harness of any kind (confirmed viamissing-test-harness's discovery command).