fix(phase-6-finalize): derive head-dependent step set for re-fire - #1073
Merged
cuioss-oliver merged 8 commits intoAug 1, 2026
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @cuioss-oliver, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Review limit reached
Next review available in: 21 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: Repository: cuioss/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (18)
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 |
PR Reviewer Guide 🔍
|
Collaborator
Author
Triage dispositionsIn reply to comment_id:
|
…spatch SSOT
`standards/dispatch-inline-split.md` classified `default:architecture-refresh`
as dispatched while its own executor doc (`standards/architecture-refresh.md`)
states it runs inline because the Tier-1 `prompt` mode requires an
`AskUserQuestion`, which a dispatched leaf cannot fire. Move the row to the
`## Inline steps` roster so the SSOT matches the executor doc. The closure
invariant holds by construction: one row leaves the dispatched roster and one
enters the inline roster.
Collapse the five hand-maintained inline-only membership lists in
`phase-6-finalize/SKILL.md` into pointers at that roster rather than
re-synchronising five copies: the "Inline-only built-in steps" bullet, the
"HEAD-dependent step set" note, the BUILT-IN inline-only dispatch branch, item
5c's metrics gate, and item 5d's completion-guard skip class. Each site keeps
only the detail genuinely local to it (record-metrics/archive-plan sequencing,
the push barrier's parity-driven re-entry, and 5d's "record their own mark
synchronously" reason). All five lists were also incomplete, so the pointer
fixes a correctness gap as well as the duplication.
Add the (f) cross-document consistency regression to
`test_dispatch_roster_closure.py`: over the bounded step-doc population this
plan touches, every doc that self-classifies must agree with the roster. The
population and the claims are derived — the check reads each doc's own
classification sentence instead of asserting an `architecture-refresh` literal,
which would pass vacuously the instant the pair is fixed. A mutation guard
drives the detector with the exact pre-fix roster row and the executor doc's
inline sentence, and negative controls keep narrative prose ("runs as inline
orchestration") from being read as a classification claim — that wider
roster-vs-step-doc population belongs to the sibling epic's PLAN-121.
Migrate `test_architecture_refresh.py`'s inline-roster assertion to read the
SSOT through the new pointer instead of pinning the SKILL.md bullet layout the
pointer replaces; its old `'architecture-refresh' in skill_md_text` substring
check was never evidence of inline classification.
Co-Authored-By: Claude <noreply@anthropic.com>
…atter Membership in the HEAD-dependent set was governed by HEAD_DEPENDENT_STEPS, a six-member set hand-maintained as prose in phase-6-finalize/SKILL.md. Because membership was hand-listed rather than derived, a step that IS head-dependent by the governing discriminator kept a stale `done` record across a loop-back commit, standing as green for a diff it never examined. Declare head-dependence per step doc instead, as a `head_dependent` frontmatter fact — the same model `mutates_source` already uses. - ext-point-finalize-step.md: add the `head_dependent` contract row (type bool, Required Conditional) with the governing discriminator stated verbatim, plus the missing `advances_main_via_rebase` row that SKILL.md already read as a frontmatter fact but the contract table never declared. Record the per-step vs whole-set consumption contract so the derivation stays as lazy as the literal it replaces. - Declare `head_dependent: true` on the 9 head-dependent step docs. Three were omitted by the hand-maintained literal and are live instances of the defect: pre-submission-self-review (reviews the plan's diff), finalize-step-plugin-doctor (structural-lint gate over the tree), and finalize-step-era-stamp-fill (its record asserts tracked source carries no unresolved PR-PENDING sentinel). - Add the --head-at-completion persistence obligation to those three docs, and reconcile the step docs' stale prose onto the declared fact — including two "one of the three HEAD-dependent steps" cardinality claims in sonar-roundtrip and automatic-review that were staler than SKILL.md's own "six". Correct a ride-along drift found against the live registry: the Current Implementations table recorded default:architecture-refresh at order 25 while its authoritative frontmatter and the registry both report order 9. Add the D4c derivation guard: population derived via find_implementors() and the registry's own frontmatter extractor (no second parser), asserting non-emptiness, the two known members, all three previously-omitted members, and the --head-at-completion obligation per member. Deliberately asserts no cardinality literal — a hardcoded count is the drift shape being removed. A per-member mutation guard proves the predicate isolates each required member independently, so a derivation that omitted one while satisfying the others cannot read as green. Surveyed all 16 steps the derivation resolves to false and confirmed each recorded verdict against the live 25-step registry. No contradictions found.
Replace the hand-maintained HEAD_DEPENDENT_STEPS literal with the derived `head_dependent: true` frontmatter fact, and tighten the stale-verdict branch so a verdict computed against a superseded HEAD is never left standing as green. SKILL.md: - One authoritative statement of head-dependence, phrased as the derived predicate plus the two qualifying shapes. Every other site cross-references it; no site re-lists or counts the members. - Remove every cardinality claim, including the two spelled-out "six steps" claims that sat outside all previously-named regions. - Strengthen the absent-SHA re-entry row: it re-fires AND reports the prior verdict UNVERIFIED, so a record that was never anchored to a SHA stays distinguishable from one that was validated and later superseded. - State that the `!= HEAD` inequality covers all three supersession mechanisms in scope — loop-back commit, force-push, rebase — so no separate force-push or rebase detector is introduced. - Leave the push step's parity-driven carve-out intact; the item-5f re-stamp now resolves the step's own fact instead of naming four steps inline. Sweep (enumerated, not sampled — see decision.log for the full report). Grep was not available in this envelope, so two complete enumerators replaced the textual sweep: a whole-suite run surfaced the single test pinning the removed literal, and a full read of SKILL.md surfaced three further literal occurrences plus a fourth drift — the Resumability section still scoped the HEAD comparison to pre-push-quality-gate alone, contradicting the derived membership. Regressions: - D4a: a loop-back commit advancing HEAD past a recorded done on pre-submission-self-review re-fires it, asserted against the persisted record. - D4b: a force-push that replaces the SHA a ci-verify green was computed against invalidates that green, asserted against the persisted head_at_completion rather than a re-run of CI. - The count-claim detector now accepts spelled-out cardinals (two..twenty), with the two removed sentences added to its mutation guard and a negative control proving the widening does not fire on ordinary cardinal prose. - A new sweep covers the HEAD-dependent prose region inside Step 3, which no previous sweep reached. Scoped to that region rather than all of Step 3, since the widened pattern would otherwise flag legitimate prose.
…NING runnable - a literal semicolon in its --message tripped the one-command-per-Bash-call hook, so the command as written could never execute Co-Authored-By: Claude <noreply@anthropic.com>
…registry table The ride-along order correction reconciled the Order column 25 to 9 against architecture-refresh.md frontmatter but left default_on reading false on the same row while the frontmatter declares true. Caught by pre-submission-self-review over the loop-back diff. Co-Authored-By: Claude <noreply@anthropic.com>
…nd stop inferring head-dependence from the inline roster push.md still spoke the retired literal - the one unconverted site, and the closure test guards only SKILL.md. SKILL.md item-1 had replaced a six-step enumeration with a reference to the whole inline roster, inverting the claim's truth value for pre-push-quality-gate, ci-verify and era-stamp-fill, all made head_dependent by this plan. Rescoped to the declared-fact predicate and added an explicit orthogonality warning. Co-Authored-By: Claude <noreply@anthropic.com>
…isambiguate the head-dependence discriminator Three findings from self-review pass 3. One: three sibling manage-logging messages in finalize-step-plugin-doctor still carried a literal semicolon and were unrunnable by the same premise as the earlier line-161 fix. Two: my own pass-2 fix reintroduced a hand-maintained membership fragment, contradicting the same paragraph's do-NOT-re-list directive - replaced with the resolve-each-step-s-own-fact form. Three: the discriminator exclusion clause negated shape 2, so a settle-stage editing step classified both ways - scoped the exclusion to steps matching neither shape, in SKILL.md and in the authoritative ext-point row. Co-Authored-By: Claude <noreply@anthropic.com>
…st and fix a false universal in the orthogonality note The removed assertion compared two distinct literals in the same function, so it reduced to a-times-40 not-equals b-times-40 with zero detection power. The orthogonality sentence added in the prior commit read as a universal over the inline roster, which is false for push, branch-cleanup, record-metrics, archive-plan and architecture-refresh - qualified to some-and-some. Co-Authored-By: Claude <noreply@anthropic.com>
cuioss-oliver
force-pushed
the
feature/gates-do-not-refire-over-the-loop-back-diff
branch
from
August 1, 2026 19:15
3dcfab2 to
43ed04c
Compare
cuioss-oliver
deleted the
feature/gates-do-not-refire-over-the-loop-back-diff
branch
August 1, 2026 19:29
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
Makes
phase-6-finalize's head-dependence gating self-consistent, so a re-fire decision is never made against a stale HEAD, and corrects the one confirmed classification defect in the same document.Two problems were load-bearing:
HEAD_DEPENDENT_STEPSwas a hand-maintained prose list. Membership was restated inphase-6-finalize/SKILL.mdand inferred, in places, from the inline/dispatched roster — two unrelated axes. A step whose verdict depends on the HEAD it was computed against could silently drop out of the set, leaving a stale green (a loop-back commit, a force-push, or a rebase superseding the recorded HEAD) standing as if it had been verified against the current diff.default:architecture-refreshwas classified contradictorily.standards/dispatch-inline-split.mdclassified it dispatched, whilestandards/architecture-refresh.mdstates it is inline because its Tier-1 mode requires anAskUserQuestionthat a dispatched leaf cannot fire. The SSOT contradicted itself.Head-dependence is now derived from finalize-step frontmatter rather than hand-listed, the derived set is consumed by the finalize dispatcher, and the last
HEAD_DEPENDENT_STEPSliteral is retired. The dispatch SSOT now classifiesarchitecture-refreshinline, and the duplicated inline-only enumerations link to the SSOT instead of re-stating it.Changes
Derived head-dependence (D1/D2/D3)
marketplace/bundles/plan-marshall/skills/extension-api/standards/ext-point-finalize-step.md— finalize-step HEAD-dependence is declared in step frontmatter and derived from it; also reconciles thearchitecture-refreshdefault_onvalue in the registry table.marketplace/bundles/plan-marshall/skills/phase-6-finalize/SKILL.md— consumes the derived head-dependent set; retires the lastHEAD_DEPENDENT_STEPSliteral, removes the re-listed membership fragment, stops inferring head-dependence from the inline roster, and disambiguates the head-dependence discriminator ("would this verdict change if HEAD changed?").standards/ci-verify.md,standards/pre-push-quality-gate.md,standards/push.md,standards/finalize-step-security-audit.md,standards/finalize-step-simplify.md,workflow/pre-submission-self-review.md,workflow/sonar-roundtrip.md, andskills/automatic-review/SKILL.md.Dispatch/inline SSOT correction (D5a/D5d)
marketplace/bundles/plan-marshall/skills/phase-6-finalize/standards/dispatch-inline-split.md—default:architecture-refreshis classified inline; the dispatched rationale is deleted, not reworded.Tests (D4/D5e)
test/plan-marshall/phase-6-finalize/test_head_dependence_derivation.py— the derived head-dependent set is asserted non-empty and contains both known members.test/plan-marshall/phase-6-finalize/test_loop_back_outcome.py— a loop-back commit re-firespre-submission-self-review.test/plan-marshall/phase-6-finalize/test_ci_verify.py— a force-push invalidates a recordedci-verifygreen.test/plan-marshall/phase-6-finalize/test_architecture_refresh.pyandtest/plan-marshall/phase-6-finalize/test_dispatch_roster_closure.py— cross-document consistency over the inline/dispatched classification.Incidental fixes (Boy Scout)
.claude/skills/finalize-step-plugin-doctor/SKILL.md— the documented cross-skill WARNING carried a literal;in its--message, which trips the one-command-per-Bash-call hook; the command as written could never execute..claude/skills/finalize-step-era-stamp-fill/SKILL.md— reconciled alongside.Each new test was verified to fail before its fix landed.
Test Plan
python3 .plan/execute-script.py plan-marshall:build-pyproject:pyproject_build run --command-args "verify plan-marshall")Related Issues
None.
Generated by plan-finalize skill