Make workpad.py own the Verification evidence record's field set - #2133
Conversation
|
/prflow:review |
PRFlow Review — PR #2133Status: ✅ Approved with notes Blueprint
Verdict: APPROVE with notes (well-scoped single-source relocation of the
|
![prflow-reviewer[bot]](https://avatars.githubusercontent.com/u/223979750?s=60&v=4){kind=small}
# Conflicts: # CLAUDE.md
…r the degrade arms (PR #2133 review) - `_git_head_or_unestablished` catches `Exception`, not only `OSError`, so its "never raises" docstring holds for a non-UTF-8 git output (UnicodeDecodeError), matching `_git_root_error_suffix`. - Trim the issue-#2131 comment blocks in scripts/workpad.py to the prevention-only standard (each names the wrong change it prevents, at most three lines). - Tests: the could-not-run arm (root is a file), the non-OSError arm, the rc!=0 arm attribution, and the reflection-less CI-degrade stderr breadcrumb. Declined: newline sanitization of --command/--outcome — `_insert_reflection_bullet` already collapses every line break before the row is written. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
/prflow:review |
PRFlow Review — PR #2133Status: ✅ APPROVE with notes Blueprint
Verdict: APPROVE with notes (record correctly relocated into workpad.py; validation fails closed; a few low-value test-coverage gaps)Issue ComplianceReviewed against issue #2131: Make workpad.py own the Verification evidence record's field set — criteria from the Verification Checklist Results15 passed, 0 failed, 0 inconclusive — 1 via lite probe, 14 via agent. ✅ Passed items (15 of 15) — click to expand
Code Review Findings🟡 Suggestion / Minor
over-grade annotation: findings flagged above (shapes 3) are advisory only and do not change the verdict. Verdict Criteria
Repo-policy gates:
Consumer prompt extension: PROMPT-EXTENSION-STATUS: content-present (arrived). |
… not run' (a decode failure means git ran) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
/prflow:review |
PRFlow Review — PR #2133Status: ✅ APPROVE with notes Blueprint
Verdict: APPROVE with notes (4 Suggestions; all checklist items pass, no blocking findings)Issue ComplianceReviewed against issue #2131: Make workpad.py own the Verification evidence field set — criteria from the Verification Checklist Results12 passed, 0 failed, 0 inconclusive — 1 via lite probe, 11 via agent. ✅ Passed items (12 of 12) — click to expand
Code Review Findings🟡 Suggestion / Minor
Advisory note (not a finding): the final-pass reviewer observed that over-grade annotation: no finding flagged Verdict Criteria
→ APPROVE with notes. VerdictAPPROVE with notes — a clean, well-tested consolidation of the |
1 participant
Summary
Verification evidence:completion-evidence record's field set out ofCLAUDE.mdprose and into the tool that writes it: a newscripts/workpad.py update --record-verification-evidenceoption that owns the record, refuses an incomplete one before any PATCH, and stamps the record time and head itself.--record-completion-evidence-ci, on a pass, now appends the sameVerification evidence:row from its validated operands.CLAUDE.md, and the internal docs at the option's--helpas the field set's single source instead of re-listing the fields.Changes
scripts/workpad.py: Adds--record-verification-evidencewith--command,--outcome,--run-root(required;--run-rootrepeatable for the two-root recombination case, and the literalnoneaccepted for a denied or ceiling-terminated launch), plus optional--tallies,--elapsed,--started-at. The tool stampsrecorded-at(UTC) andhead(full 40-chargit rev-parse HEAD, or the literalunestablishedwhen git cannot answer). It refuses, before any body mutation, a call missing a required field, an aggregate outcome (aggregate CLEAN/aggregate FAILED) recorded with--run-root none, and an explicit call over a workpad lacking a## Devflow Reflectionsection. Each call appends onenote-kind reflection row beginningVerification evidence:; rows are appended, never replacing a prior one.--record-completion-evidence-cinow also appends one such row on a pass (command =gh pr checks, outcome = the validated check name/conclusion pairs, run-root = the run URL), degrading to a breadcrumb rather than aborting when the workpad has no reflection section.Implement skill and extension:
skills/implement/phases/phase-4-documentation.mdPhase 4.3 now tells the run to record the launch through--record-verification-evidence;.prflow/prompt-extensions/implement.mdroutes theVerification evidence:marker through the same option.CLAUDE.md: The "Recording a whole-suite launch" subsection no longer enumerates the record's fields — it points at the option's--help, names the degraded arm for an installed plugin that predates the option, and records the no-run-root paths with--run-root none.Internal docs:
docs/internal/claude-md-tiered-suite-rationale.md,docs/internal/implement-skill.md, anddocs/internal/skills/implement-verification.mdnamescripts/workpad.pyas the field-set owner (the rationale page namesstarted-atas the accepted start-time field).Resolves
Resolves #2131
Test Plan
lib/test/test_python_scripts_part3.py— a valid call appends one### ℹ️ Notesrow carryingcommand=,outcome=,run-root=,recorded-at=,head=<40 hex>, with optional fields present only when supplied.lib/test/test_python_scripts_part3.py— missing each of--command/--outcome/--run-rootin turn makes no PATCH and names the flag; an aggregate outcome with--run-root noneis refused before any PATCH naming the conflict.lib/test/test_python_scripts_part3.py— a non-aggregate--run-root nonerecordsrun-root=noneverbatim; a second launch keeps both rows; a two-root call emits exactly two orderedrun-root=fields.lib/test/test_python_scripts_part3.py— an explicit call over a reflection-less workpad refuses (no PATCH); a non-git root recordshead=unestablishedand still PATCHes.lib/test/test_python_scripts_part3.py— a--record-completion-evidence-cipass appends oneVerification evidence:row built from its validated operands; a reflection-less CI pass still records the completion-ci marker and appends no row.lib/test/test_python_scripts_part3.py—_git_head_or_unestablishedabsorbs a could-not-run failure (root is a file) and a non-OSErrorfailure (UnicodeDecodeError) intohead=unestablishedwith a breadcrumb naming the cause; the reflection-less CI pass breadcrumbs the un-appended row on stderr.Visual Changes
N/A
Breaking Changes
None. Existing free-text
Verification evidence:rows on old workpads are unaffected (no reader parses them); the option adds only argument-position operands under an already-granted head.Prompt-surface size
Derived at
08f51eb9f96246d9c64df2bda4f667203b5daf6dagainst merge-basead6da899aacd5e9c83e8b4c30b572fc002d16137(origin/main). Covered: tracked*.mdunderskills/,agents/,.prflow/prompt-extensions/..prflow/prompt-extensions/implement.mdskills/implement/phases/phase-4-documentation.mdReview response
PRFlow review at
08f51eb9f96246d9c64df2bda4f667203b5daf6d(APPROVE with notes, four Suggestions):_git_head_or_unestablishednow catchesException(parity with_git_root_error_suffix), so its "never raises" docstring holds for a non-UTF-8 git output.--command/--outcomenewline sanitization:_insert_reflection_bulletalready collapses every line break (' '.join(text.splitlines())) before the row is written, so a\n/\rin either operand yields one bullet line; verified by driving_append_reflectionwith a multi-line outcome. Revisit only if the row writer stops routing through_insert_reflection_bullet.scripts/workpad.pyare trimmed to the prevention-only standard (three cited sites plus the same-class siblings at the CI rider and the append site).NotADirectoryError), the non-OSErrorarm, the rc≠0 arm attribution, and the reflection-less CI-degrade stderr breadcrumb.Branch merged with
origin/main(one conflict,CLAUDE.md"Recording a whole-suite launch": kept this PR's pointer bullets and main's slimmed wording of the #2008 two-root and single-flight bullets).Generated via /prflow:implement (v2.36.15, claude-opus-4-8, low)