feat(methodology): pre-stall reviewer awareness + re-acceptance signals (4 commits)#145
Open
Leopold-Fitz-AI wants to merge 4 commits intoPolyArch:mainfrom
Conversation
In re-acceptance sessions where the substantive work landed in a
prior session and the current session is just doing a Codex
re-review, the Finalize Phase predictably produces zero edits — the
prior session's exit point was already at minimum complexity.
Today the methodology treats "ran cleanup, no edits" identically to
"ran cleanup, edited X files": both produce a finalize summary, only
the latter contains substantive content. A consumer of the loop's
artifacts cannot tell whether the no-op outcome means the round was
well-shaped (positive signal) or whether the cleanup pass was
ineffective (negative signal).
This change requires the first content line of every finalize
summary to be one of three classifications:
Outcome: no-op (already-minimal)
Outcome: cosmetic (formatting only)
Outcome: substantive (logic edits)
The classification reframes the no-op outcome as positive evidence
that the prior rounds did not ship over-complex artifacts. For
sessions whose Codex review approved COMPLETE before Finalize, a
substantive outcome warrants a one-sentence justification of why
the prior rounds shipped non-minimal artifacts.
Documentation-only change at v1: the prompt template asks for the
classification line and the reviewer/auditor can spot-check it.
A future change can add stop-hook enforcement.
Tests:
- tests/test-finalize-outcome-classification.sh asserts all three
literal classification forms are present in the rendered prompt
template, the rationale clarifying that no-op is NOT failure is
present, and the existing template placeholders are preserved.
Closes part of methodology-improvement issue (re-acceptance PolyArch#5).
…d-artifact findings out-of-loop
When the reviewer's top-priority finding can only be fixed by editing
an artifact the loop forbids in-place edits to (e.g., a plan file
under --track-plan-file byte-lock), today's reviewer prompt has no
mechanism for the reviewer to recognise that and stop demanding the
fix. The loop counts the same critique each round, the implementer
cannot act on it, and after a few iterations the stagnation circuit
breaker fires. By that point the conversation has already produced
the impasse signal in plain text — the loop just cannot read it.
This change makes the reviewer loop-aware:
1. The stop hook builds a SESSION_INVARIANTS string from the existing
state-file fields (`plan_tracked`, `start_branch`) listing the
session's byte-locks and immutables, plus the canonical
resolution path (cancel/amend/restart).
2. Both reviewer prompt templates (`regular-review.md` and
`full-alignment-review.md`) gain a new Session Invariants section
that injects the dynamic invariants list and adds a fourth
finding-classification lane: `out-of-loop`. The static instruction
asks the reviewer to tag findings whose only fix mutates a locked
artifact as `out-of-loop` rather than `must-fix`, and to recommend
the canonical resolution path instead of demanding in-round
action.
This is the high-leverage half of the bottom-line recommendation in
the methodology-improvement issue: pair this change with the
implementer-side `blocked_by_invariant` schema (separate commit) and
the loop catches the impasse one round earlier with no false-positive
risk.
The implementation is purely additive at the prompt-template level;
existing reviewers that ignore the new section behave exactly as
before. The stop-hook change is a new template variable + a new line
in two existing `load_and_render_safe` calls.
Tests:
- tests/test-session-invariants-injection.sh asserts the
`{{SESSION_INVARIANTS}}` placeholder is present in both reviewer
templates, the out-of-loop finding lane is documented, and the
stop hook constructs the invariants string from `PLAN_TRACKED` /
`START_BRANCH` and threads it into both review-prompt renders.
Pre-existing test failures (test-template-loader.sh `load_and_render_safe
with existing template`, test-templates-comprehensive.sh `Real
template used when available`) are unrelated and reproduce on `main`
without this change.
Closes part of methodology-improvement issue (stagnation PolyArch#4).
…n-lineage.md When a new RLCR session opens on a tree that already contains substantive work — landed by a prior session that exited via the stagnation circuit breaker, by max-iteration cap, or cleanly with off-loop commits between sessions — today's setup script captures the new base_commit silently. The fact that the new session is inheriting a non-empty delta is invisible to the loop's tracking machinery; the implementer narrates the continuity in round-summary prose, but no machine-readable record links the two sessions. This change adds first-class detection and a one-screen lineage artifact: 1. After BASE_COMMIT is captured, the setup script looks for the most recent prior session in `.humanize/rlcr/`, reads its base_commit from any of the recognised state files (state.md / finalize-state.md / complete-state.md / methodology-analysis- state.md), and compares to the new session's base_commit. 2. If they differ, INHERITED_DELTA=true is persisted as a new field in state.md so the round-0 contract + reviewer prompt can reason about it directly. 3. session-lineage.md is generated in the new session's loop directory containing the prior session's timestamp + exit-state file + base_commit, the new session's base_commit, the prior_base..current_base git-log range with one-line subjects, and a stub asking the implementer to record why a new session was needed rather than a continuation. The detection is read-only with respect to git state and is gated on directory existence + state-file presence, so sessions opening on a clean repo (no prior `.humanize/rlcr/` history) are unaffected. inherited_delta defaults to "false" for backward compatibility with callers that ignore the field. Tests: - tests/test-session-lineage-inherited-delta.sh asserts the detection block, state.md schema extension, lineage-file generation gate, and existing state.md fields are all present; also runs a bash syntax check on the modified script. The script's logic was end-to-end smoke-tested against a temporary git repo with a fabricated prior session at an earlier base_commit. Closes part of methodology-improvement issue (re-acceptance PolyArch#6).
…viewer recognition Pairs with the loop-aware reviewer prompt change. Together they form the bottom-line recommendation in the methodology-improvement issue: the reviewer learns about session invariants on the prompt side (separate commit), and the implementer gets a schema-level escape channel on the summary side (this commit). When the implementer recognises that a Codex finding can only be fixed by mutating a session-byte-locked artifact, today's free-form summary template gives no machine-readable way to declare that. The declaration ends up as one bullet among many in prose; the reviewer treats it symmetrically with other findings and produces another round of substantive critique. After several iterations the stagnation circuit breaker fires. This change adds an optional `## Blocked By Methodology Invariant` block to the implementer summary schema: ```markdown ## Blocked By Methodology Invariant - Invariant: <name> - Findings blocked: <list> - Canonical resolution: <path> - Why I cannot act in-loop: <one-sentence> ``` Documentation lands on both ends: 1. `next-round-prompt.md` (Claude side): documents when to use the block (locked-artifact impasse only — NOT ordinary follow-up deferral), the four required fields, and a misuse warning so the block does not become an escape hatch for queued cleanup work. 2. `regular-review.md` (Codex side): instructs the reviewer to recognise the block, verify the implementer's claim is genuine (push back if the block is being abused), tag verified-blocked findings as `out-of-loop` rather than `must-fix`, recommend the canonical resolution, and stop re-issuing the listed findings as round-N+1 mainline gaps. This is v1: the block is documented + recognised by the reviewer, but the stop hook does not yet auto-short-circuit the review when the block is present. A future change can layer on stop-hook detection that emits a structured exit-reason recommending cancel/amend/restart without spending a Codex review round. Tests: - tests/test-blocked-by-invariant-block.sh asserts both prompt templates document the block + the four required fields, the next-round prompt warns against block misuse, and the reviewer prompt instructs verify-then-route plus push-back-on-misuse. Closes part of methodology-improvement issue (stagnation PolyArch#3).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c42127150
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+853
to
+854
| if [[ -n "$PRIOR_BASE_COMMIT" ]] && [[ "$PRIOR_BASE_COMMIT" != "$BASE_COMMIT" ]]; then | ||
| INHERITED_DELTA="true" |
There was a problem hiding this comment.
Set INHERITED_DELTA using more than PRIOR_BASE_COMMIT != BASE_COMMIT: both values are base-branch SHAs captured at session start, so this misses the common case where new commits were added on the working branch between sessions while the base branch did not move. In that scenario the comparison stays equal, inherited_delta is incorrectly left false, and session-lineage.md is never generated even though the new session is actually re-accepting inherited work.
Useful? React with 👍 / 👎.
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
Lands four narrow, additive improvements to the RLCR methodology, addressing two distinct failure modes surfaced by the loop's own methodology-analysis phase:
The four commits are independent, additive, and individually revertable. None require breaking changes to existing implementation-mode sessions.
Commits (in branch order)
feat(finalize)— require anOutcome:classification line (no-op (already-minimal)/cosmetic (formatting only)/substantive (logic edits)) at the top of every finalize summary. Reframes the no-op outcome as positive evidence rather than ambient noise. Closes RLCR loop: 7 methodology gaps for re-acceptance / inherited-delta sessions #147 suggestion 5.feat(reviewer-prompt)— build aSESSION_INVARIANTSstring in the stop hook from existing state.md fields and inject it into both reviewer prompt templates. Adds a fourth finding-classification lane:out-of-loop. Reviewers tag findings whose only fix mutates a byte-locked artifact asout-of-looprather thanmust-fixand recommend the canonical resolution path. Closes RLCR loop: 7 methodology improvements for handling locked-artifact impasse and earlier stall detection #146 suggestion 4.feat(setup)— detect inherited-delta sessions at session open by comparing the new session'sbase_committo the most recent prior session's. When they differ, persistinherited_delta: trueto state.md and auto-generate a one-screensession-lineage.mdcontaining the prior session's timestamp + exit-state file + base, the new session's base, the prior_base..current_base git-log range, and a stub asking the implementer to record why a new session was needed. Closes RLCR loop: 7 methodology gaps for re-acceptance / inherited-delta sessions #147 suggestion 6 + lands the v1 of RLCR loop: 7 methodology gaps for re-acceptance / inherited-delta sessions #147 suggestion 2.feat(summary-schema)— add an optional## Blocked By Methodology Invariantblock to the implementer summary schema. Documents the four required fields (Invariant / Findings blocked / Canonical resolution / Why I cannot act in-loop), warns against misuse, and instructs the reviewer to verify-then-route the block contents (push back on false-blocked, accept verified-blocked asout-of-loop). Lands the v1 of RLCR loop: 7 methodology improvements for handling locked-artifact impasse and earlier stall detection #146 suggestion 3 (documentation + reviewer recognition); v2 stop-hook auto-short-circuit deferred to follow-up PR.Together, commits 2 + 4 are the bottom-line recommendation in the analysis report — pairing the reviewer-side awareness with the implementer-side escape channel catches the impasse one round earlier with no false-positive risk.
Origin
The improvements were derived from two methodology-analysis reports generated by the loop's own circuit-breaker / completion phases on a recent two-session arc:
The fixes are sanitized — no project-specific identifiers in the methodology files. The two reports are tracked as companion issues #146 (stagnation) and #147 (re-acceptance); this PR closes part of each.
Test plan
Each commit ships its own dedicated test script under
tests/. All four pass clean:tests/test-finalize-outcome-classification.sh— 9/0tests/test-session-invariants-injection.sh— 6/0tests/test-session-lineage-inherited-delta.sh— 13/0 (includes bash syntax check)tests/test-blocked-by-invariant-block.sh— 9/0tests/test-template-references.shcontinues to pass at 121/0 — the new{{SESSION_INVARIANTS}}placeholder is properly threaded through both reviewer-prompt rendering call sites.The
setup-rlcr-loop.shchange was end-to-end smoke-tested against a temporary git repo with a fabricated prior session at an earlier base commit; the detection correctly classifies inherited-delta and produces the expected commit-range git log.Pre-existing failures in
test-template-loader.sh(load_and_render_safe with existing template) andtest-templates-comprehensive.sh(Real template used when available) reproduce onmainwithout this change and are unrelated.Backwards compatibility
Outcome:line in finalize summaries continue to function (v1 is documentation-only; no stop-hook enforcement).{{SESSION_INVARIANTS}}placeholder gets a non-empty default value derived from existing state.md fields. Reviewers that ignore the new section behave exactly as before.inherited_deltadefaults tofalsefor fresh sessions; the new field is appended at the end of the state.md schema, so callers that read by field name continue to work.## Blocked By Methodology Invariantblock is optional. Implementers who don't use it produce summaries identical to today's.Out of scope
The original methodology-analysis reports listed several larger structural changes (tracked in the open #146 + #147 issues): a
--modeflag with three session archetypes (#147-7), splitting progress vs. discovery round budgets (#146-6), a semantic stall detector based on cross-round finding-similarity (#146-2), dual byte-lock semantics (#147-3), the v2 stop-hook auto-short-circuit on## Blocked By Methodology Invariant(#146-3 v2), the action-locus classifier (#146-1), the session-outcome synthesizer (#146-5), recovery routing in the exit-reason file (#146-7), the dual-reading-mode reviewer prompt (#147-4), the re-acceptance round mode (#147-1), and the richer inherited-delta state block (#147-2 v2).These would benefit from RFC-style discussion before landing and are deferred to follow-up PRs.
Notes
feat/methodology-pre-stall-and-re-acceptance-signals.