product-pipeline-probe: first end-to-end product-pipeline probe (+ CF-E fix, L10)#25
Conversation
…(vehicle + audit trail) Push ONE trivial throwaway vehicle (features/probe-greeting/greet.mjs) through the four product stages /pharn-spec -> /pharn-plan -> /pharn-grill -> /pharn-build to measure whether they integrate as a chain. The vehicle is meaningless by design (P7) and is scheduled for revert in a follow-up; the deliverable is the measured run (uncommitted PROBE.md). Commits the process/audit artifacts + vehicle, leaving PROBE.md uncommitted so /pharn-dev-regress's scope partition sees inside == declared (## Files) — the commit discipline the approved PLAN specified to avoid the CF-1-amplified false escape. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…VIEW/SHIP + PROBE) Completes the gated /pharn-dev-ship run: the grill-log, the regress + verify machine reports and human renders, the review (GREEN, 0 blocking), the ship roll-up, and the PROBE.md deliverable (the measured hand-off matrix + findings CF-A/B/C/E + G3). Also folds in the prettier formatting the verify style gates (G3) required. Floor verdicts this run: validate exit 0 (GREEN) · regress no-regressions · verify PASS. The product pipeline runs as a chain (first evidence) — NOT a proof it is bug-free (P0). The vehicle (features/probe-greeting/) is a throwaway slated for revert in a follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ncate scope The --from-plan writes-scope setter broke its `## Files` scan at the first non-path line matching its head-less-exclusion cue regex — so an explanatory blockquote (`> … not touched …`, e.g. referencing the `### Explicitly not touched` subsection) ABOVE the path items zeroed the scope → fail-closed exit 1, blocking a valid plan. Surfaced by the product-pipeline-probe (CF-E). Fix: exempt blockquote lines (`> …`) from the Boundary-2 cue in pathsFromPlanFiles — blockquotes are explanatory commentary, never exclusion-section intros. Only blockquotes are exempted, so a NON-blockquote head-less intro (`Files NOT written:`) and an exclusion-only `## Files` still fail closed (both pinned by new tests). +2 tests, 167 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…greeting/) Per the human-approved disposition (GATE 1), the probe's vehicle was always a throwaway — removed now that the measurement is captured. The dev audit trail (.dev/features/product-pipeline-probe/PROBE.md etc.) is retained as the historical record of the run; only the meaningless product code + its product-pipeline artifacts are reverted. Floor GREEN, npm test PASS after removal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ce (CF-A) Promotes the durable CF-A insight from the product-pipeline-probe: validate.mjs scans root features/ but excludes .dev/, so finding-bearing PRODUCT artifacts face CHECK 5 that the equivalent .dev/-excluded DEV artifacts never do. Extends L9 (the style-gate half) with the validate-CHECK-5 half. Provenance valid + id unique (check-provenance GREEN pre-write); human-accepted via gated /pharn-dev-memory-promote. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR fixes a bug in set-writes-scope.cjs where blockquote lines containing exclusion wording incorrectly truncated authorized Files paths, adding regression tests and a fix plan. It also adds extensive documentation (PLAN, PROBE, GRILL, REVIEW, VERIFY, REGRESSION, SHIP, JSON reports) for a product-pipeline-probe dev run, plus a lessons-learned entry. ChangesSetter Cue Fix (CF-E)
Product Pipeline Probe Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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 |
![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
🧹 Nitpick comments (4)
.dev/features/product-pipeline-probe/PLAN.md (2)
125-130: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHistorical note: CF-E fix is in this PR, not a separate increment.
The blockquote comment states "fix is a SEPARATE increment," but this PR actually includes the
set-writes-scope.cjsblockquote-exemption fix (CF-E). This is understandable as a frozen historical observation from the probe run, but readers may be confused since the fix ships together with these documents. Consider adding a post-script or inline note clarifying that the fix was later bundled into the same PR.🤖 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/product-pipeline-probe/PLAN.md around lines 125 - 130, The historical note in the PLAN.md blockquote is misleading because it says the CF-E fix is a separate increment even though this PR includes the set-writes-scope.cjs blockquote-exemption fix. Update the note near the “Files” section to clearly state that the CF-E fix was bundled into this PR, and keep the existing reference to PROBE.md while adding a brief clarifying post-script or inline note so readers understand the scope.
175-176: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHistorical note: vehicle revert is in this PR, not a follow-up increment.
The plan states "revert the vehicle … in a follow-up increment," but the PR objectives and commit history indicate the
features/probe-greeting/vehicle was reverted in this same PR. Like the CF-E note, this is a frozen historical expectation that may confuse readers since the revert ships together. Consider a clarifying note.🤖 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/product-pipeline-probe/PLAN.md around lines 175 - 176, The PLAN.md note about reverting the vehicle is stale and conflicts with the PR’s actual scope; update the `features/probe-greeting/` historical note to clearly state that the vehicle revert happened in this same PR, not in a later follow-up. Keep the rest of the `validate.mjs` and floor-check guidance unchanged, but clarify the expectation in the relevant plan section so readers are not misled..dev/features/product-pipeline-probe/PROBE.md (2)
154-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHistorical note: vehicle revert is bundled in this PR.
PROBE.md states "revert in a follow-up — human-approved 2026-06-30." The PR objectives indicate the
features/probe-greeting/revert is part of this same PR. A clarifying footnote would help readers.🤖 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/product-pipeline-probe/PROBE.md around lines 154 - 156, Add a clarifying footnote in PROBE.md near the vehicle note to state that the features/probe-greeting/ revert is bundled in this PR, and keep the existing guidance tied to greet.mjs so readers understand the revert is no longer a follow-up. Update the wording in the “vehicle is meaningless by design” section to reference the bundled revert explicitly without changing the surrounding probe-policy context.
62-82: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHistorical note: CF-E parser fix ships in this PR.
PROBE.md records that "The parser fragility itself is NOT fixed here — it is a candidate for a separate increment." The
set-writes-scope.cjsblockquote-exemption fix (CF-E) is actually included in this PR. This is accurate as a frozen run-time observation, but consider a post-hoc footnote for readers since the fix is bundled.🤖 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/product-pipeline-probe/PROBE.md around lines 62 - 82, The PROBE.md note is outdated because it says the CF-E parser fragility is not fixed here, but this PR actually includes the set-writes-scope.cjs blockquote-exemption change. Update the historical note to reflect that the CF-E fix is bundled in this PR, and add a short post-hoc footnote if needed so readers understand the observation is frozen while the parser change itself shipped here.
🤖 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 @.claude/hooks/set-writes-scope.cjs:
- Around line 175-179: The blockquote exemption in set-writes-scope.cjs is too
broad and lets quoted exclusion headers like “> Files NOT written:” bypass
Boundary 2. Narrow the isBlockquote handling in the scope-detection logic so
only explanatory quoted notes are exempt, while quoted intro lines that precede
normal path items still fail closed; update the condition around the
line-scanning check and add a regression test for the quoted-intro exclusion
case.
In @.dev/features/product-pipeline-probe/GRILL.md:
- Around line 123-127: The advisory verdict count in the GRILL.md summary is
inconsistent with the documented findings. Update the summary near the verdict
so it matches the actual YAML blocks produced by the grill, or add the missing
finding if one was omitted; use the “ADVISORY VERDICT” section and the
structured findings list as the source of truth when reconciling the total.
---
Nitpick comments:
In @.dev/features/product-pipeline-probe/PLAN.md:
- Around line 125-130: The historical note in the PLAN.md blockquote is
misleading because it says the CF-E fix is a separate increment even though this
PR includes the set-writes-scope.cjs blockquote-exemption fix. Update the note
near the “Files” section to clearly state that the CF-E fix was bundled into
this PR, and keep the existing reference to PROBE.md while adding a brief
clarifying post-script or inline note so readers understand the scope.
- Around line 175-176: The PLAN.md note about reverting the vehicle is stale and
conflicts with the PR’s actual scope; update the `features/probe-greeting/`
historical note to clearly state that the vehicle revert happened in this same
PR, not in a later follow-up. Keep the rest of the `validate.mjs` and
floor-check guidance unchanged, but clarify the expectation in the relevant plan
section so readers are not misled.
In @.dev/features/product-pipeline-probe/PROBE.md:
- Around line 154-156: Add a clarifying footnote in PROBE.md near the vehicle
note to state that the features/probe-greeting/ revert is bundled in this PR,
and keep the existing guidance tied to greet.mjs so readers understand the
revert is no longer a follow-up. Update the wording in the “vehicle is
meaningless by design” section to reference the bundled revert explicitly
without changing the surrounding probe-policy context.
- Around line 62-82: The PROBE.md note is outdated because it says the CF-E
parser fragility is not fixed here, but this PR actually includes the
set-writes-scope.cjs blockquote-exemption change. Update the historical note to
reflect that the CF-E fix is bundled in this PR, and add a short post-hoc
footnote if needed so readers understand the observation is frozen while the
parser change itself shipped here.
🪄 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: 33a7f0ad-6a5e-4a0f-a3ca-7a9f7e3010df
📒 Files selected for processing (13)
.claude/hooks/set-writes-scope.cjs.claude/hooks/set-writes-scope.test.cjs.dev/features/product-pipeline-probe/GRILL.md.dev/features/product-pipeline-probe/PLAN.md.dev/features/product-pipeline-probe/PROBE.md.dev/features/product-pipeline-probe/REGRESSION.md.dev/features/product-pipeline-probe/REVIEW.md.dev/features/product-pipeline-probe/SHIP.md.dev/features/product-pipeline-probe/VERIFY.md.dev/features/product-pipeline-probe/regression-report.json.dev/features/product-pipeline-probe/verify-report.json.dev/features/setter-cue-fix/PLAN.md.dev/memory-bank/lessons-learned.md
| const isBlockquote = /^\s*>/.test(line); | ||
| if ( | ||
| !isPathItem && | ||
| !isBlockquote && | ||
| /\bnot\W*(touch|writ|modif|edit|chang)|\bexplicitly\W*excluded|\bout\W*of\W*scope|\boff\W*limits/i.test(line) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Don't blanket-exempt quoted exclusion intros.
This now suppresses Boundary 2 for any > ... line, so a shape like > Files NOT written: followed by normal - \path`items is treated as authorized scope instead of failing closed. Sincewrites-scope.jsonis the write-authorization boundary, that widens permissions for an exclusion-only## Files` block. Keep the exemption narrow to explanatory notes, and add a regression for the quoted-intro case.
🤖 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 @.claude/hooks/set-writes-scope.cjs around lines 175 - 179, The blockquote
exemption in set-writes-scope.cjs is too broad and lets quoted exclusion headers
like “> Files NOT written:” bypass Boundary 2. Narrow the isBlockquote handling
in the scope-detection logic so only explanatory quoted notes are exempt, while
quoted intro lines that precede normal path items still fail closed; update the
condition around the line-scanning check and add a regression test for the
quoted-intro exclusion case.
| **ADVISORY VERDICT: 7 concerns raised (0 blocking-severity, 4 important, 3 minor) — for the human to weigh | ||
| before `/pharn-dev-build`.** The spec→plan chain is intact (content-hash verified). The grill gates nothing; the | ||
| only floor-grade results this run produced are the writes-scope pin on this log and the spec-hash match above. | ||
| "Grill produced a GRILL.md" does **not** mean the plan is good — it means the chain held and these concerns were | ||
| surfaced for the human (P0). |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Finding count mismatch: claims 7 concerns but only 6 are documented.
The advisory verdict states "7 concerns raised (0 blocking-severity, 4 important, 3 minor)," but the structured findings contain only 6 YAML blocks: 3 important (P0 fix-#7, P6 timing, P6 RED-risk) and 3 minor (P0 taint, P6 floor-conflation, P1 AC). Either one concern is missing from the document or the count should read "6 concerns (3 important, 3 minor)."
🤖 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/product-pipeline-probe/GRILL.md around lines 123 - 127, The
advisory verdict count in the GRILL.md summary is inconsistent with the
documented findings. Update the summary near the verdict so it matches the
actual YAML blocks produced by the grill, or add the missing finding if one was
omitted; use the “ADVISORY VERDICT” section and the structured findings list as
the source of truth when reconciling the total.
Reviews the CF-E fix that landed under "do everything" without a review pass. All four lenses clean: P0 (the "more permissive, never removes a path" claim verified), P1 (hook ships 2 binding tests), P2 (exemption can't inject a path — blockquote lines are never collected), P3 (one axis). Advisory: Boundary-2's content-cue heuristic remains fragile for non-blockquote pre-path prose; proposed candidate lesson (prefer structural anchors over prose cues, relates to L6). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PrzemekGalarowicz
force-pushed
the
product-pipeline-probe
branch
from
609876c to
6139e20
Compare
1 participant
What this PR does
Bundles the product-pipeline integration probe and its approved follow-ups (per a "do everything, merge everything" request — multiple axes in one PR, separated by commit):
product-pipeline-probe— the first measured end-to-end run of the PRODUCT pipeline (/pharn-spec → /pharn-plan → /pharn-grill → /pharn-build) on a throwaway vehicle, wrapped in the gated/pharn-dev-shiploop. Deliverable:.dev/features/product-pipeline-probe/PROBE.md(the measured hand-off matrix + findings).setter-cue-fix(CF-E fix) —set-writes-scope.cjs --from-planno longer truncates## Filesscope on an explanatory blockquote containing an exclusion cue. +2 regression tests.features/probe-greeting/removed (approved throwaway disposition); the.dev/audit trail is retained.validate-SCANNED surface (rootfeatures/) that.dev/-excluded dev artifacts don't (CF-A). Human-accepted via gated/pharn-dev-memory-promote.Probe result
The product pipeline runs as a chain (first evidence): every hand-off produced exactly the shape the next stage consumed; the
spec→plan→grill→buildcontent-hash chain held across all four stages; fix #7 bounded the build (observed an exit-2 deny); the SPEC approval gate halted. This is evidence, not a proof of bug-freedom (P0).Findings surfaced: CF-E (new bug — fixed here), CF-A (validate scan asymmetry — promoted as L10), CF-B/CF-C (nested gate; scope thrash — documented), CF-1-amplified (regress conflation — mitigated by commit discipline), G3 (verify style gates — = L9, resolved by formatting). CF-A/G3 were deliberately not "fixed" by loosening the floor (that would let a laundered finding escape CHECK 5).
Floor verdicts (all GREEN)
validateexit 0 ·/pharn-dev-regressno-regressions·/pharn-dev-verifyPASS·/pharn-dev-reviewGREEN (0 blocking) ·npm run checkgreen (167 tests).🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation