ADR-319: Implement harvest-playbook skill and /harvest command dispatcher - #69
Conversation
…cher Adds plugins/dev-team/skills/harvest-playbook/SKILL.md, the core six-step harvest procedure (gather inputs from paths -> build candidate method content via Method markers/litmus-test classification/exemplar diffing -> interview -> author the playbook directory -> replace consumed Method markers with provenance links -> present the TODO list and replay-and-diff validation procedure), and plugins/dev-team/commands/harvest.md, a thin dispatcher documenting the argument contract and forwarding to the skill. Key decisions: - The Testable component (harvest-playbook) was implemented directly rather than through the tdd-tester/tdd-implementer/tdd-refactorer trio: the component is pure agent-skill prose with no AAA-testable logic of its own, verified instead by the fixture-driven dry-run harness (component-taxonomy's explicit "agent-skill prose" example of a Testable component verified by whatever mechanism fits). This mirrors ADR-318's own precedent of bypassing the trio for its hand-authored-fixture content. - Treated the dry-run checklist (authored before the skill prose, red) as this task's TDD-red analogue in place of Gherkin E2E scenarios, since no Gherkin/BDD harness exists anywhere in this repo and missing-test-harness prohibits inventing one; the checklist is included in the work summary as reviewable content per the brief. - Verified green by materializing the fixture exemplar repos, spawning a subagent blind to authoring context to run harvest-playbook to completion against the fixture spec/exemplars/template-output/interview-answer-key, and grading its output against the checklist (all mechanical and judgment-shaped items passed, including the derived strip/replace step and the surfaced logging-format divergence).
build-and-test: Python test resultsStatus: ✅ Passed Test log |
jodavis-claude
left a comment
jodavis-claude
left a comment
There was a problem hiding this comment.
Reviewed against the ADR-319 task brief and _spec_PlaybookHarvesting.md's Task 3 exit criteria. The six-step harvest-playbook procedure and the commands/harvest.md dispatcher both look solid: they match the investigate-bug/fix.md style precedents, correctly cite playbook-contract instead of restating it (directory contract, TODO semantics, marker format/lifecycle, vendor-neutrality rules), and I independently cross-checked the described dry-run results (marker count, strip/replace tokens, logging-format divergence) against the actual fixture files (fixture-spec.md, template-output/service.yaml, interview-answer-key.md) — they're consistent. One requirements-level gap below (checklist not landed as reviewable PR content), otherwise no blocking issues found. Style/content of the prose itself is clean.
|
Sign-off review: confirmed the dry-run checklist is now present in the PR description (added in the prior fix pass) and CI (build-and-test, gate) is green. No new code changes since the last review pass — approving. |
jodavis-claude
left a comment
jodavis-claude
left a comment
There was a problem hiding this comment.
Sign-off review — ADR-319 (harvest-playbook / /harvest)
Prior review thread status:
- Requirements-gap thread (dry-run checklist missing from PR description): resolved satisfactorily. The full checklist (mechanical checks with grep counts, judgment-shaped checks with cited evidence, argument-contract prose-inspection note) is now present in the PR description under "Dry-run checklist (authored before the skill prose — red artifact) and grading (green)". No code changes were needed for this fix, consistent with the fix summary. Replied on the thread confirming this; could not mark the GitHub thread itself as resolved due to a tooling permission restriction in this session — flagging for the human owner to resolve manually if desired.
Scan of modified files since last review: no code files changed since the last review pass — the fix was a PR-description-only edit (single commit 770d16d, unchanged). Re-inspected harvest-playbook/SKILL.md and commands/harvest.md against playbook-contract/SKILL.md and CONTRIBUTING.md/_doc_Projects.md: six-step procedure present and correctly ordered, argument-contract rules (durable-artifact requirement, update-mode detection) stated in step 1, Method marker provenance-link format matches playbook-contract's example exactly, vendor-neutrality rules cited (not restated), TODO semantics cited with manual-fallback requirement preserved, commands/harvest.md follows the fix.md/spec.md thin-dispatcher pattern with a missing---out guard. No new Priority 1-4 issues found.
CI: build-and-test and gate both green.
Approving sign-off.
- Support partial-spec harvests: scope can be given conversationally in
step 1, or asked explicitly at the start of step 3, instead of assuming
the whole artifact is always in scope.
- Add a review step (step 5) that invokes spec-discussion against the
authored playbook's SKILL.md before Method markers are replaced, so the
user can leave REVIEW comments the same way they would on a spec.
- Reword the Method provenance line per suggestion ("see" instead of
"harvested into").
- Drop the replay-and-diff validation procedure from harvest-playbook's
presented output — it's this feature's own one-time acceptance test
(see _spec_PlaybookHarvesting.md), not a standard step for every
harvested playbook. Updated the spec's decision record and Task 3 exit
criteria to match.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RBnDBvr47h4fVqNte6feE2
3 participants
Work item
ADR-319 — Implement the
harvest-playbookskill (the core harvest procedure: gather inputs from paths, classify method content via markers/litmus-test/exemplar-diffing, interview the user, author a vendor-neutral playbook directory, replace consumed Method markers with provenance links, and present the TODO list plus replay-and-diff validation instructions) and its thin/harvestcommand dispatcher, so a user can turn a validated spec/exemplar-repo set into a reusable playbook.Changes
plugins/dev-team/skills/harvest-playbook/SKILL.md(new) — the six-step harvest procedure: gather inputs from paths → build candidate method content via Method markers / litmus-test classification / template-output diffing / plan-vs-history divergence mining → interview the user → author the playbook directory → replace consumed Method markers with provenance links → present the TODO list and replay-and-diff validation procedure. Citesplaybook-contractfor the directory contract, TODO semantics, Method marker format/lifecycle, vendor-neutrality rules, and bare-name resolution rather than restating any of it.plugins/dev-team/commands/harvest.md(new) — thin dispatcher on thefix.md/implement.mdpattern:description+argument-hintfrontmatter documenting/harvest <spec-path | none> [--exemplar <repo-path>]... [--template-output <path>] --out <playbook-directory> [--name <playbook-name>], and a body that parses arguments (with a missing---outguard) and forwards toharvest-playbook.No fixtures,
playbook-contract, orRUN.mdwere modified — both are owned by earlier tasks (ADR-317, ADR-318) and are cited, not restated.Design decisions
harvest-playbook) directly rather than through the tdd-tester/tdd-implementer/tdd-refactorer trio: it is pure agent-skill prose with no AAA-testable logic, matchingcomponent-taxonomy's explicit callout of agent-skill prose as a Testable component verified by whatever mechanism actually fits.missing-test-harnessforbids inventing one. The checklist was authored before theSKILL.mdprose (red), then the finished prose was graded against it (green).--outis written to directly (not into a--name-named subdirectory), matching the fixture dry run's scripted answer key.argument-hintmirrors the spec's illustrative multi-flag syntax on bothharvest-playbookandcommands/harvest.md.Testing completed
Fixture-driven dry run per
RUN.md's Step 2/3 procedure: materializedexemplar-repo-1/exemplar-repo-2viamaterialize.py, ran a subagent blind to authoring context throughharvest-playbookagainst the fixture spec, exemplars,template-output/, andinterview-answer-key.md. All mechanical checks (file existence, vendor-neutrality grep, marker-replacement count, TODO manual-fallback presence) and judgment-shaped checks (executable-knowledge steps, correctly derived strip/replace step, surfaced plan-vs-history divergence) passed. Argument-contract behavior (refusal on zero durable-artifact sources, update-mode detection for an existing--outplaybook) verified by prose inspection.No pytest unit tests or Gherkin E2E scenarios were written — both components are agent-skill prose with no executable/AAA-testable logic, per
component-taxonomyandmissing-test-harness.Dry-run checklist (authored before the skill prose — red artifact) and grading (green)
Authored per
_spec_PlaybookHarvesting.md's "Verifying the Testable prose components" andRUN.md's Step 3 mechanical/judgment split, against a dry run driven withfixture-spec.md, materializedexemplar-repo-1/exemplar-repo-2,template-output/, andinterview-answer-key.md.Mechanical checks — all passed:
test -f <playbook-out>/SKILL.md && test -f <playbook-out>/spec-template.md→ pass (both files present).grep -riE "dev-team|spawn|invoke skill|use skill|subagent" <playbook-out>/SKILL.md <playbook-out>/spec-template.md→ pass (no matches, exit 1).grep -c "harvested into" <scratch-fixture-spec.md>→ pass (count 2, matching the fixture spec's two Method markers).grep -B2 "TODO" <playbook-out>/SKILL.md→ pass (each of the two TODOs — the{{SERVICE_NAME}}substitution script and theservice.yamlschema validator — is preceded by a concrete manual fallback:grep/sedcommands for the first, a manual field-by-field checklist for the second).Judgment-shaped checks — all passed:
5. Steps are executable knowledge, not delegation — every step in the produced
SKILL.mdnames concrete commands (grep,sed,git diff) or manual checklists; no skill/agent references.6. The strip/replace step is correctly derived from the
template-output/diff — the produced playbook's Steps 2–3 name the exact tokens seen intemplate-output/service.yaml/README.md({{SERVICE_NAME}},{{LOG_FORMAT}}) and what they resolve to, matchingexemplar-repo-1/02-strip-replace.7. The plan-vs-history divergence mined from the exemplars surfaces — the logging-format split (
exemplar-repo-1/orders-service moves tojson;exemplar-repo-2/billing-service keepstext"for legacy compatibility") appears both as the playbook's canonical-plus-exception step 3 and in the dry run's interview transcript (its question 4, answered from the fixture answer key's row 4).Argument-contract checks — verified by prose inspection (not re-exercised via a second live dry run, since the fixture set is built for a full harvest, not an empty-input or already-existing-playbook scenario):
SKILL.mdstep 1 states the refusal for zero durable-artifact sources and the update-mode detection for an existing playbook at--out, matching the brief's argument-contract exit criterion.commands/harvest.md— visual inspection (Wrapper tier, no dedicated test expected):description+argument-hintfrontmatter document the full flag syntax; the body parses arguments and forwards toharvest-playbookwith no duplicated logic.