build-stage: add /pharn-build product command and plan-scope gates#22
Merged
Conversation
Introduce the product build stage (hash-chain re-check + fix #7 writes-scope) with build-loop audit trail and a fail-closed --from-plan scope test. Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (29)
✅ Files skipped from review due to trivial changes (17)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds build-stage command and verification docs, restructures ChangesPipeline command and scope docs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Complete the build-loop pipeline artifacts for the /pharn-build increment and apply minor markdownlint/grill follow-up formatting fixes. Co-authored-by: Cursor <cursoragent@cursor.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.dev/features/build-stage/PLAN.md:
- Around line 78-79: The “no parseable scope → refuse” entry in the build-stage
plan is mislabeling the floor boundary by attributing fail-closed behavior to
both the setter and /pharn-build. Update the wording in the PLAN.md section so
the unique symbol set from set-writes-scope.cjs is the actual floor signal (exit
1), and describe /pharn-build as advisory orchestration that obeys it rather
than the floor itself. Keep the two-clocks distinction explicit and align this
line with the other floor/advisory terminology used in the build-stage docs.
In @.dev/features/build-stage/VERIFY.md:
- Line 23: The verifier count output description is using a stale field name and
must be aligned with the authoritative JSON shape. Update VERIFY.md to match the
actual output from count-verifiers.mjs and verify-report.json, using the same
key name consistently (either verifiers or findings), and reference the
count-verifiers.mjs output text so the documented example reflects the real
producer format.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 068e8a96-79d9-4ba6-b48f-12de857331dd
📒 Files selected for processing (10)
.claude/commands/pharn-build.md.claude/hooks/set-writes-scope.test.cjs.dev/features/build-stage/GRILL.md.dev/features/build-stage/PLAN.md.dev/features/build-stage/REGRESSION.md.dev/features/build-stage/REVIEW.md.dev/features/build-stage/SHIP.md.dev/features/build-stage/VERIFY.md.dev/features/build-stage/regression-report.json.dev/features/build-stage/verify-report.json
Comment on lines
+78
to
+79
| - the build record `features/<name>/BUILD.md` is itself fix #7-scoped (Phase-2 `--from-frontmatter … --target`) → **floor: hook (fix #7)**; its **content** (the advisory roll-up) is model work, never a self-issued "correct" / seal → **advisory** (the §6 ship-stage seal is the human's GATE-2 decision, not `/pharn-build`'s). | ||
| - no parseable scope → refuse → **floor: fail-closed** (`set-writes-scope.cjs` exit 1; `/pharn-build` refuses rather than fall through to the hook's absent-scope default-safe-set). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The "floor: fail-closed" label overstates the floor boundary.
The setter's exit 1 is floor, but /pharn-build's refusal to stop is advisory command orchestration (the two-clocks split). The GRILL.md finding correctly notes this is unlabeled here. Consider: no parseable scope → refuse → **floor signal: setter exit 1**; **advisory: command obeys it** to match the two-clocks discipline applied elsewhere.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.dev/features/build-stage/PLAN.md around lines 78 - 79, The “no parseable
scope → refuse” entry in the build-stage plan is mislabeling the floor boundary
by attributing fail-closed behavior to both the setter and /pharn-build. Update
the wording in the PLAN.md section so the unique symbol set from
set-writes-scope.cjs is the actual floor signal (exit 1), and describe
/pharn-build as advisory orchestration that obeys it rather than the floor
itself. Keep the two-clocks distinction explicit and align this line with the
other floor/advisory terminology used in the build-stage docs.
|
|
||
| ## ADVISORY layer — verifiers | ||
|
|
||
| **No verifiers registered — floor gates only.** `node .dev/floor/count-verifiers.mjs .` → `{"registered":0,"verifiers":[]}` (deterministic frontmatter membership over `role: verifier`, never a prose grep — P5). Step 2 is a no-op; the verdict is the floor gates alone. No verifier was authored speculatively (P7). |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Inconsistent field name in verifier count output description.
VERIFY.md claims count-verifiers.mjs outputs {"registered":0,"verifiers":[]}, but verify-report.json uses "findings":[] (not "verifiers":[]). If the report is authoritative, the prose has a stale field name; if count-verifiers.mjs truly emits "verifiers", then verify-report.json has a mismatch with its upstream producer. Align one to the other.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.dev/features/build-stage/VERIFY.md at line 23, The verifier count output
description is using a stale field name and must be aligned with the
authoritative JSON shape. Update VERIFY.md to match the actual output from
count-verifiers.mjs and verify-report.json, using the same key name consistently
(either verifiers or findings), and reference the count-verifiers.mjs output
text so the documented example reflects the real producer format.
Close the spec→plan→grill→build chain. /pharn-build derives its fix #7 writes-scope via set-writes-scope.cjs --from-plan, which needs a `## Files` heading with back-tick paths; /pharn-plan emitted a free-text `## Steps / Files` instead, so a stock product PLAN failed-closed and the product pipeline could not build end-to-end (the #22-named `plan-files-scope` follow-up). - .claude/commands/pharn-plan.md: split the Step-4 PLAN template's `## Steps / Files` into an advisory `## Steps` + a parseable `## Files` (leading back-tick paths, angle-bracket placeholders that fail closed when unfilled, an `### Explicitly not touched` subsection), with guidance citing the set-writes-scope.cjs --from-plan contract + ARCHITECTURE §6 (P4). - .claude/hooks/set-writes-scope.test.cjs: add the closing-the-loop test (a filled /pharn-plan-shaped plan → exit 0, scope = exactly the `## Files` paths, exclusion/steps-prose paths absent) and a producer-faithfulness test (the real pharn-plan.md template → exit 1, locking the placeholder discipline against a bare-word regression). Reuses set-writes-scope.cjs + the fix #7 hooks unchanged (the producer is matched to the parser, not the reverse). No new floor primitive. Floor GREEN (1 capability); npm test 165; npm run check green. Gated /pharn-dev-ship run; audit trail under .dev/features/plan-files-scope/ (PLAN/GRILL/REGRESSION/VERIFY/REVIEW/SHIP + reports): regress no-regressions; verify PASS (6 gates); review GREEN (0 floor-gate findings). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…es /regress + /verify) to canon L9 records the gate-coverage gap surfaced by the plan-files-scope dogfood: an increment's own markdown style is gated by neither /pharn-dev-regress (style gates skip absent a shared-config change) nor /pharn-dev-verify's canonical gate map (test/validate/lint/structural) — so it can redden `npm run check` yet pass both stages. Remedy recorded: add format:check + lint:md to /pharn-dev-verify's gate map. Promoted via gated /pharn-dev-memory-promote: check-provenance GREEN (valid provenance, id L9 unique), human-accepted. Provenance points at a5de975. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… gate map (L9 remedy) Implement L9's remedy: /pharn-dev-verify's canonical FLOOR gate set now includes format:check + lint:md, so the verify verdict tracks the full `npm run check` and closes L9's style-gate coverage hole AT VERIFY (an increment's own markdown style is caught at verify, not only at the full npm run check / CI). check-verify.mjs is UNCHANGED — it is generic over gate keys, so this is a pure command-prose widening of .claude/commands/pharn-dev-verify.md (Step-1 gate runs + results map + the gate enumeration + granularity/devDeps/Live-integration notes + a new advisory-orchestration bullet + the verify-report.json example). No new floor primitive; no new test (OQ1: P7 — check-verify is generic + already tested; the gate-set lives in advisory command prose, labelled honestly, not floor-locked). /pharn-dev-regress untouched. Gated /pharn-dev-ship run; the verify stage was a DOGFOOD (the new six-gate set ran, all exit 0). Audit trail under .dev/features/verify-style-gates/: regress no-regressions; verify PASS; review GREEN (0 floor-gate findings, 1 advisory residual the increment names itself, no new lesson — it is L5 applied). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Doc-sync: the Step-0 "Scope-source caveat (a current, honest limit — LIMITS.md)" in pharn-build.md described /pharn-plan's free-text `## Steps / Files` as current and the plan-files-scope follow-up as pending — both stale since plan-files-scope landed (a5de975: /pharn-plan now emits a parseable `## Files`). - Rewrote the block to "Scope-source note (resolved — plan-files-scope)": /pharn-plan now emits a parseable `## Files`; PRESERVED the fail-closed point — a malformed/ incomplete plan with no parseable `## Files` still makes /pharn-build refuse rather than guess a scope (correct, not a bug). - Re-anchored the inline example from "## Steps / Files" to a malformed/hand-written plan generally. - Dropped the now-pointless LIMITS.md reference (grill finding: LIMITS.md carries no matching entry to keep in sync). Pure prose, no behavioral change, no new floor primitive, no test (P7). Grep confirmed pharn-build.md was the only file still calling plan-files-scope a pending follow-up. Gated /pharn-dev-ship run; audit under .dev/features/build-caveat-sync/: regress no-regressions; verify PASS (6 gates); review GREEN (0 findings). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/pharn-build, the product pipeline's fourth stage (spec → plan → grill → build → …) — the first stage that writes the user's implementation files, gated by two reused floor mechanisms (no new primitive):check-plan-spec-agree.mjs(spec→plan hash chain re-verified at build time) and fix Add /pharn-eval orchestration and structural variance runner (increment 3c) #7 writes-scope (set-writes-scope.cjs --from-plan+enforce-writes-scope.cjs)..dev/features/build-stage/(PLAN, GRILL audit trail, regression report showing no regressions vsmain).--from-planreceives a PLAN with no parseable## Filesheading (the/pharn-plan## Steps / Filesgap).Test plan
npm test— new--from-planfail-closed test passes; no hook regressionsnode .dev/floor/validate.mjs .— GREEN (floor capability count stays 1)/pharn-dev-regress—regression-report.jsonverdictno-regressions## Filesscope-source gap /plan-files-scopefollow-upMade with Cursor
Summary by CodeRabbit
## Filesscope.## Filesand added coverage for invalid/placeholder inputs.