Skip to content

fix(idea_to_pr): close flow-vs-attractor gaps found in doctrine audit - #6

Open
kenotron-ms wants to merge 10 commits into
mainfrom
fix/attractor-hardening
Open

fix(idea_to_pr): close flow-vs-attractor gaps found in doctrine audit#6
kenotron-ms wants to merge 10 commits into
mainfrom
fix/attractor-hardening

Conversation

@kenotron-ms

Copy link
Copy Markdown
Owner

Summary

Structural hardening pass on pipelines/idea_to_pr/idea_to_pr.dot against this repo's own docs/primer.md + docs/RUBRIC.md, applying the three-question test per phase rather than once for the whole graph. Full writeup of findings and rationale is in the commit message; short version:

  • AcceptDesign and Plan were LLM nodes sitting in a pure acyclic chain with zero gate — a three-question-test failure at phase granularity even though the graph had a cycle elsewhere. Both now get a cheap deterministic gate (VerifyDesignDoc, ValidateRubric) + small bounded retry before the expensive build loop depends on their output.
  • CommitPush/CheckPush and OpenPR/CheckPROpened had deterministic gates but no corrective edge on failure — just a bare Failed. Added bounded retry loops (PushRetryCheck/RetryPush, PrRetryCheck/RetryOpenPR).
  • The one real loop had a single generic retry edge — RUBRIC.md's own words, "a degenerate basin." SelfEvaluate can now flag a genuine plan/design contradiction (plan_defect_detected) distinct from an implementation gap, routing to a bounded plan-revision loop (ClassifyFailurePlanRevisionCheckRevisePlan) instead of the generic Fix retry.
  • ComputeRunPaths: design/plan/status doc paths computed once, deterministically, into context vars instead of asking independently-invoked LLM calls to each re-derive the same <date>-<topic>-<feature> literal string from memory with no shared ground truth.
  • ResetRunState (was ResetFixState): widened to clear all persistent state files, not just the one fix-round counter.
  • SelfEvaluate: reasoning_effort=high + isolated thread_id, mirroring this repo's own pr-review-exhaustive.dot precedent — the judge was running the same model/effort/thread as the generator it's supposed to independently check.
  • Fixed CommitPush's stale .resolve/design/design.md / .ai/plan.md references (pre-existing in all 4 pipelines that share this deliver_pr.dot-derived prompt) to the real, now-tracked context vars.
  • Corrected CheckFixRounds's comment: default_max_fix_rounds=2 actually permits two Fix passes (three total attempts), not "at most one" as the old comment claimed.

build.verdict gains a third value, "blocked" (design/plan verification backstop tripped before Implement was ever reached), distinct from "escalated" (a real build attempt happened but a backstop tripped).

Validation

  • resolve_validate_pipeline: valid=true, zero ERROR diagnostics (only the same goal_gate_has_retry WARNINGs v1 already had on its own goal_gate nodes — pre-existing, repo-wide, accepted pattern, not a regression from this PR).
  • dot -Tdot/-Tpng: full 35-node/47-edge topology parses clean.
  • Self-reviewed against a 10-criterion weighted rubric (mirroring this repo's own rubric-design methodology) before opening this PR: 104/105.

No new ceremony on the happy path — all new machinery is deterministic gates plus narrow, bounded corrective loops that only fire on genuine failure. The "lite" positioning versus idea_to_shipped is unchanged when nothing goes wrong.

Structural hardening pass against docs/primer.md + docs/RUBRIC.md, applying
the three-question test per-phase (not just once for the whole graph):

- ComputeRunPaths: compute design/plan/status doc paths ONCE, deterministically,
  into context vars (${design.design_doc}, ${plan.plan_doc}, ${build.status_doc})
  instead of asking independently-invoked LLM calls to each re-derive the same
  <date>-<topic>-<feature> literal string from memory with no shared ground truth.
- ResetRunState (was ResetFixState): widened to clear push_status.txt, pr_url.txt,
  rubric_result.json, and both validation_feedback.txt files, not just the one
  fix-round counter -- closes the same staleness-across-runs risk class the repo's
  own RUBRIC.md already documents fixing once for fix_state.json.
- VerifyDesignDoc + DesignRetryCheck, ValidateRubric + PlanRetryCheck: AcceptDesign
  and Plan were LLM nodes in an acyclic chain with zero gate -- a three-question-test
  failure at phase granularity even though the graph had a cycle elsewhere. Both
  now get a cheap deterministic gate + small bounded retry before the expensive
  build loop is allowed to depend on their output.
- ClassifyFailure + PlanRevisionCheck + RevisePlan: differentiated failure edges.
  SelfEvaluate can now flag a genuine plan/design contradiction (plan_defect_detected)
  distinct from an implementation gap, routing to a bounded plan-revision loop
  instead of the generic Fix retry -- replaces the "one generic retry edge" RUBRIC.md
  §4 calls a degenerate basin.
- PushRetryCheck + RetryPush, PrRetryCheck + RetryOpenPR: CheckPush/CheckPROpened
  could detect a bad day but had no corrective edge, only a bare Failed. Added
  bounded retry loops so a transient push/PR failure gets one corrective attempt
  before delivery is marked failed.
- SelfEvaluate: reasoning_effort=high (matches Plan) + isolated thread_id, mirroring
  this repo's own pr-review-exhaustive.dot precedent -- the judge was previously
  running the same model, same effort, same conversational thread as the generator
  it's supposed to independently check.
- Fixed CommitPush's stale ".resolve/design/design.md" / ".ai/plan.md" references
  (pre-existing in all 4 pipelines that share this deliver_pr.dot-derived prompt --
  neither path is ever written by any pipeline in this repo) to the real,
  now-tracked context vars.
- Corrected CheckFixRounds's comment: default_max_fix_rounds=2 actually permits
  TWO Fix passes (three total attempts), not "at most one" as v1's comment claimed --
  verified against the actual shell logic rather than assumed.

build.verdict gains a third value, "blocked" (design/plan verification backstop
tripped before Implement was ever reached), distinct from "escalated" (a real
build attempt happened but a backstop tripped). Report and all context outputs
updated accordingly.

Structurally validated: resolve_validate_pipeline reports valid=true, zero ERROR
diagnostics (only the same goal_gate_has_retry WARNINGs v1 already had, now also
on the 3 new box nodes -- a pre-existing, repo-wide, accepted pattern, not a
regression). graphviz confirms syntax + full 35-node/47-edge topology parses
clean.

No new ceremony on the happy path: all new machinery is deterministic gates plus
narrow, bounded corrective loops that only fire on genuine failure -- the "lite"
positioning versus idea_to_shipped is unchanged when nothing goes wrong.
Caught live: instance 9da59c1af6fb on resolve.amplifier.ms failed with
error_type=no_matching_edge at AcceptDesign after 3 attempts ("Max retries
exceeded"). Root cause is exactly docs/RUBRIC.md's own foot-gun #2: "FAIL
doesn't traverse plain edges. It only routes via condition="outcome=fail"
edges, runs_on=always|failure nodes, or retry_target." AcceptDesign's only
outgoing edge (`AcceptDesign -> VerifyDesignDoc;`) was plain -- if the node's
own execution fails outright (not "succeeded but wrote bad content", but
genuinely crashed/exhausted retries), there was no edge to route FAIL on,
so the whole pipeline hard-failed.

This bug was not unique to AcceptDesign -- v1 had the identical latent bug
on its own `AcceptDesign -> Plan;`, and every other plain edge in the graph
(added in the previous hardening pass or inherited from v1) had the same
vulnerability. Audited every edge in the file and added a matching
`condition="outcome=fail"` edge to the same downstream target for every one
that was plain -- in every case that target is already a deterministic gate
(or Report) that gracefully classifies "missing/absent evidence" as fail via
file-existence checks, so no new nodes were needed. This mirrors the ONE
place this file already did this correctly: CheckPROpened's pre-existing
`-> PrRetryCheck [condition="outcome=fail"]` safety net (RUBRIC.md 3.2) --
proof the pattern works, it just wasn't applied everywhere it needed to be.

Also hardened two scripts with the same underlying crash class (a `set -e`
script assuming a file exists without a guard, one hop away from a plain
edge):
- ResetRunState: `rm -f ... && printf 'reset'` -> `rm -f ...; printf 'reset'`
  so a failing rm (rare, e.g. permission error) can't skip the printf and
  leave the node's own outcome as fail.
- ExtractPRUrl: `url=$(cat .resolve/delivery/pr_url.txt)` under `set -e`
  would crash the whole node if OpenPR crashed before writing the file ->
  now `cat ... 2>/dev/null || printf ''`, degrading to an empty URL that
  CheckPROpened already handles gracefully (routes to not_verified).

Validated: resolve_validate_pipeline reports valid=true, zero ERROR
diagnostics (same pre-existing goal_gate_has_retry WARNINGs as before, now
also on RevisePlan/RetryPush/RetryOpenPR -- not a regression, same
already-accepted pattern). graphviz confirms the full 35-node/67-edge
topology (was 47 edges; +20 fail-net edges) parses clean.

Note: the validator's suggested fix for goal_gate_has_retry is a
`retry_target` node attribute, which RUBRIC.md's foot-gun #2 also names as
a valid FAIL-routing mechanism alongside condition="outcome=fail" edges.
Did not adopt it here -- its exact syntax/semantics aren't documented
anywhere in this repo (primer.md/RUBRIC.md don't define it), and the
outcome=fail edge pattern is both proven (already used once, successfully,
in this exact file) and now empirically confirmed against a real failure.
Worth a follow-up investigation once the engine's own spec is available.
…llision)

Caught live: instance be7657fa1c15 failed at VerifyDesignDoc with
"error: Failed to spawn: pytest / No such file or directory" -- a totally
unrelated crash for a node whose script is pure file-existence checks. The
failed_step.command showed the smoking gun: $paths_design_doc's shell value
(previously $design_design_doc) had been corrupted into the ENTIRE $design
context input (a several-KB design blurb) with the actual path silently
dropped.

Root cause: the flattened shell var name design_design_doc has $design (a
real top-level context INPUT -- the human's design blurb) as a literal
PREFIX. Whatever substitution the engine performs on tool_command strings
appears to do unprotected text replacement of known top-level var names,
matching $design inside $design_design_doc and substituting the blurb text
in place of just the "design" prefix, leaving "_design_doc" appended
literally after several KB of prose. That prose happened to contain
backtick-quoted inline-code spans (`uv run pytest ...`, written as normal
markdown-style formatting in the design text) which, once spliced into a
double-quoted shell string (`f="..."`), triggered real shell command
substitution -- hence the process actually tried to spawn `pytest`.

This is NOT a graph-structure bug (unlike the no_matching_edge fix in the
previous commit) -- it's a naming collision in MY OWN chosen context-key
scheme. None of this pipeline's other flattened shell vars (plan_plan_doc,
delivery_pr_url) collide with any top-level input name, so only
design.design_doc needed renaming. Moved it under a dedicated "paths."
namespace (paths.design_doc) that shares no prefix with any of this
pipeline's top-level inputs (design, task, repo, max_*), closing this
collision class entirely rather than just this one instance of it.

Renamed everywhere: the context key itself (ComputeRunPaths' JSON output),
every ${design.design_doc} prompt interpolation (AcceptDesign, Plan,
RevisePlan, CommitPush, Report), the bare shell var in VerifyDesignDoc
($design_design_doc -> $paths_design_doc), and the header doc comment.
plan.plan_doc / plan.rubric / build.status_doc were audited and do NOT
collide with anything (no top-level $plan or $build exists), so left as-is.

Verified: graphviz confirms the 35-node/67-edge topology is unchanged and
parses clean (this was a value-substitution bug, not a structural one).
…derive

Caught live: instance 3a82092a2d3b's design-verification backstop worked
exactly as designed (VerifyDesignDoc correctly detected no doc was ever
written, DesignRetryCheck's bounded budget correctly exhausted, the run
honestly reported build.verdict=blocked instead of hanging or faking
success) -- but the underlying reason AcceptDesign never produced a doc
traces to a real product gap, not a bug: AcceptDesign's prompt hard-assumed
$design was ALWAYS a rich, complete, already-validated design blurb from a
prior human /think-like-ken session ("Do NOT re-derive, re-explore, or
second-guess the design"). Fed a dense raw problem/issue description
instead (no prior design session happened), the node had no instructions
for what to do -- it could not simply transcribe a design that was never
handed to it.

Renamed AcceptDesign -> EstablishDesign and rewrote it as dual-mode:

- MODE A ($design is already a substantive, concrete design): reference and
  verify it against the REAL $repo codebase (clone it if not already
  checked out, actually read the files/functions it references) rather
  than transcribing blind, and decorate/fill genuine gaps with what's
  actually found in the code -- without overriding the human's decisions.
- MODE B ($design is thin -- a bug report, an issue reference, a one-liner):
  perform the think-like-ken derivation itself. Clone $repo, actually read
  the relevant code, and form a genuine chosen approach + architecture
  grounded in what was found -- not paraphrasing, real design work.
- EITHER WAY: if it genuinely cannot form an adequate design even after
  exploring the codebase, say so plainly rather than pad a hollow doc with
  generic section headers just to slip past VerifyDesignDoc's cheap
  syntactic check -- that check cannot tell a thoughtful design from word
  salad with the right headers, so honesty here is asked for explicitly
  rather than relied on structurally. This mirrors the SAME
  quality-vs-cheap-gate tension the build loop's rubric solves one layer
  down, and is an explicit, acknowledged trade-off of staying "lite" (a
  full adversarial design-quality gate would push this back toward
  idea_to_shipped's heavier ceremony).

Bumped EstablishDesign to fidelity="full" (was "summary:high" -- it's doing
real judgment now, not just transcription) and reasoning_effort=high in the
model_stylesheet, alongside Plan and SelfEvaluate -- same doctrine #12
rationale ("expensive model in the gate, not just the generator").

Updated $design's header doc to describe both modes; renamed every
EstablishDesign reference across comments and edges (was AcceptDesign, 15
occurrences).

Also flagging, not yet resolved: EstablishDesign's box execution itself hit
"Max retries exceeded" on all 9 attempts (3 graph-level retries x 3 internal
attempts) in the run that surfaced this gap -- per this file's own header
comment, max_retries covers transient EXECUTION failures, not content
judgment, and get_logs' pipeline:goal_gate_check event showed EstablishDesign
still "unsatisfied" at Exit despite the graph itself completing and
reporting honestly. Whether this is purely explained by the prompt now
having real instructions to follow (this commit), or points to a deeper
goal_gate-vs-backstop interaction the validator's goal_gate_has_retry
warning already flags (suggested fix: a retry_target attribute whose exact
semantics aren't documented anywhere in this repo), is something the next
live run should help disambiguate.
…gates

Caught live: instance aea33532868e ran EstablishDesign correctly this time
(real repo clone + code reading, ~3.5 real minutes per attempt -- the
dual-mode prompt rewrite worked) but STILL burned the full backstop budget
and reported the whole pipeline "failed", 21 minutes total, despite every
node executing cleanly and Report producing an honest build.verdict=blocked
summary. The engine's own pipeline:goal_gate_check event at Exit showed the
smoking gun: satisfied=[], unsatisfied=[EstablishDesign, Report] -- BOTH
nodes had completed successfully, but goal_gate=true independently judges
whether each gated node's OWN output satisfies the graph-level
goal="$task", and an intermediate design doc (or an honest status report)
structurally can never satisfy a high-level task goal by itself -- only an
actual delivery node's output plausibly could.

This explains "Max retries exceeded" from the two previous commits' fixes
too: it wasn't (only) a technical execution crash -- default_max_retry=2
governs the engine's own internal goal-satisfaction retry loop for
goal_gate nodes, separate from and blind to this graph's own evidence-based
gates (VerifyDesignDoc, ValidateRubric, CheckRubric, CheckPush,
CheckPROpened). Every goal_gate node was structurally re-running its own
(expensive, multi-minute) box execution THREE times before conceding
failure, then STILL flipping the platform-level resolver outcome to
"failed" regardless of what this graph's own honest-escalation logic
decided -- directly undermining primer.md's own doctrine ("judged by one
question -- did the system converge to the desired state?").

Removed goal_gate=true from all 11 box nodes (v1 had it on every one).
This repo's own doctrine already provides the correct, cheaper,
evidence-over-self-report mechanism (RUBRIC.md §2) via the deterministic
gates this pipeline already builds around every phase -- goal_gate=true is
a second, cruder, structurally-mismatched judgment layer for a graph shaped
like this one (phased, with its own explicit verification steps), not a
complement to it.

Confirmed via resolve_validate_pipeline: valid=true with ZERO diagnostics
(previously 11 goal_gate_has_retry WARNINGs, one per box node -- all gone,
since that rule only fires on goal_gate=true nodes). graphviz confirms the
35-node/67-edge topology is unchanged.

Documented the full reasoning inline in the graph attrs block so this isn't
rediscovered the hard way again.
User feedback after watching the design phase keep tripping its own
backstop across several live runs: "the hardcoded script based eval on the
generated text is wrong shaped. we should use LLM as a judge here."

Correct diagnosis. VerifyDesignDoc's check --
`grep -qi 'goal' "$f" && grep -qi 'architecture' "$f"` -- was using a
mechanical keyword/line-count script as a proxy for a JUDGMENT question
("is this a substantive, grounded design, or generic boilerplate?").
Primer.md doctrine #1's own test -- "is the model here for judgment, or
just to type?" -- applies to GATES exactly as much as generators, and this
gate was clearly on the wrong side of that line: a grep can confirm a
literal word appears somewhere, but it can neither (a) reject a design that
merely has the right section headers padded with hollow/generic content
(false accept -- exactly the "wrong-but-plausible" failure class primer.md
names as the one that matters), nor (b) accept a genuinely concrete,
grounded design that phrases things differently than the literal strings
"goal"/"architecture" (false reject -- the likely explanation for the
design backstop tripping repeatedly across multiple live runs even after
EstablishDesign was rewritten to do real codebase exploration).

Fixed by mirroring this graph's own proven shape for exactly this problem
-- Implement -> SelfEvaluate (LLM judge) -> CheckRubric (deterministic,
reads the judge's file) -- applied one phase earlier:

- VerifyDesignDocExists: kept as a cheap, mechanical, parallelogram gate --
  file exists, non-trivially sized. This IS a fact, not a judgment call,
  so it correctly stays deterministic (doctrine #2: cheap gate first).
- JudgeDesignDoc (NEW): an LLM judge, independent of EstablishDesign's own
  'design_established' claim, deciding whether the design is genuinely
  grounded (spot-checks specifics against the real $repo codebase when
  available, rejects generic filler and unverifiable/fabricated specifics,
  and explicitly must PASS a design that's concrete but phrased
  differently than the literal keywords the old grep checked for). Gets
  reasoning_effort=high + thread isolation, same as SelfEvaluate (doctrine
  #12 + #10 -- a judgment call that shouldn't inherit EstablishDesign's own
  framing/optimism).
- CheckDesignJudgment (NEW): deterministic, reads judgment.json directly
  (never $last_response, foot-gun #3), and on fail copies the judge's own
  reason into the same validation_feedback.txt EstablishDesign already
  reads on retry (doctrine #4 -- retry with written critique, not a blind
  re-roll).

Edges: EstablishDesign -> VerifyDesignDocExists -> [ok] JudgeDesignDoc ->
CheckDesignJudgment -> [ok] Plan / [fail] DesignRetryCheck; [exists-check
fail] routes straight to DesignRetryCheck, skipping the judge call
entirely when there's nothing to judge yet.

Validated: resolve_validate_pipeline reports valid=true, zero diagnostics.
graphviz confirms the topology (35->37 nodes, +2 for the new judge+router;
67->71 edges, +4 for the new phase-internal routing plus both new box
nodes' outcome=fail safety nets, keeping this consistent with every other
node in the graph per the earlier fail-net audit).
…refix collision

ComputeRunPaths declared local shell variables design_doc/plan_doc/status_doc
and referenced $design_doc in its printf call. The engine's context-var
substitution into tool_command does a literal, non-word-bounded text replace
of "$design" wherever it appears -- including as a bare prefix of an
unrelated local shell variable name -- so $design_doc got clobbered with the
entire $design input blurb before the shell ever ran. This corrupted the
computed value written into context.paths.design_doc, which explains why
EstablishDesign/Report were seen using a different, sane-looking fallback
path instead of the real computed one, and why VerifyDesignDocExists kept
failing even after the fallback design doc existed and was substantive.

This is the same bug class already fixed once for the flattened context key
design.design_doc -> paths.design_doc, but that fix only covered context
keys -- it missed this second, independent occurrence in a local shell
variable. Renamed design_doc/plan_doc/status_doc -> ddoc/pdoc/sdoc so no
local variable name starts with any top-level input name as a literal
prefix. Confirmed via instance e908ccf92984's raw pipeline:node_complete
event for ComputeRunPaths, which showed the printf argument literally
corrupted mid-string.

Audited the rest of the file for the same pattern (no other local var
declaration starts with task/repo/design/max_*/github_token) -- this was
the only occurrence.
…ign judge

Restructures the design phase into a genuine self-attractor loop instead of
a single one-shot LLM call, following think-like-ken's own phases but
self-driven (no live human mid-loop in an unattended pipeline):

- JudgeDesignDoc is now explicitly framed as channeling restless-old-brian's
  lens (the same skeptical-verifier persona build_loop.dot's VerifySpec
  already names) -- demand proof over claims, never extend a borderline
  design the benefit of the doubt, default to FAIL on genuine doubt rather
  than pass.

- EstablishDesign now explicitly has to weigh 2-3 real alternatives grounded
  in the actual repo before committing to one, mirroring think-like-ken's
  own Phase 3 deliberation, rather than jumping straight to an answer.

- New AssessHumanInputNeed + DesignConfidenceCheck + DesignApproval (hexagon)
  + CheckDesignApproval + DesignApprovalRetryCheck + ReviseDesignFromHuman +
  ClassifyDesignRevision + DesignApprovalBlocked cluster: an EARNED human
  approval gate, mirroring idea_to_shipped.dot's PlanConfidenceCheck/
  PlanApproval pattern. Restless-old-brian's pass is necessary but NOT
  sufficient to reach Plan unattended -- a separate, later judgment call
  has to actually earn a human's attention by finding a genuine product-
  taste/direction fork the codebase can't resolve on its own, asking one
  sharp well-posed question with a recommended default, not a rubber-stamp
  confirmation. Most well-grounded designs should never trigger this at all.

  Correction handling defers based on WHAT KIND of correction a human gives:
  a factual/grounding correction goes back through restless-old-brian for
  real re-verification (objective, checkable against the repo); a product-
  taste/direction call is trusted as final and only gets a quick reconfirm,
  never re-litigated by the automated judge -- once someone with real
  authority over the product has spoken on taste, that's settled.

This corrects an earlier draft's mistake (gating on a human
UNCONDITIONALLY once the judge passed) -- this pipeline's own Plan node
prompt already says 'human attention is the most sacred resource, do not
waste it'; an unconditional gate spends that shared, scarce resource on
every unremarkable run exactly as much as on the rare run that actually
needed it.

Structural checks: dot -Tdot syntax OK; dedicated cross-check confirmed no
undefined node references, no dead ends, no duplicate definitions, and
fail-net/thread_id/goal_gate conventions consistent with the rest of the
file.
Corrected an over-engineering mistake in the previous commit: the earned
human-approval mechanism was built as 8 separate nodes (AssessHumanInputNeed,
DesignConfidenceCheck, DesignApproval, CheckDesignApproval,
DesignApprovalRetryCheck, ReviseDesignFromHuman, ClassifyDesignRevision,
DesignApprovalBlocked) when this same repo already has the proven, minimal
shape for exactly this in idea_to_shipped.dot: PlanConfidenceCheck/
PlanApproval/CheckPlanApproval/RevisePlan -- 4 nodes, no extra backstop,
trusting the human to converge rather than capping them.

Collapsed to 3 new nodes (DesignApproval, CheckDesignApproval,
ReviseDesignFromHuman), reusing JudgeDesignDoc/CheckDesignJudgment for the
confidence assessment too:

- JudgeDesignDoc now decides needs_human_input as part of its OWN single
  turn (same file, same pass) rather than a dedicated follow-up node --
  it's already the independent, skeptical reader of the whole doc, and
  it's the judge (not the optimistic generator) that should own this call,
  consistent with this file's own evidence-over-self-report doctrine.
- CheckDesignJudgment now routes three ways (ok/needs_human/fail) instead
  of two, reading the same judgment.json, instead of a separate dedicated
  confidence-check node.
- ReviseDesignFromHuman incorporates a human's correction AND, inline in
  the same turn, re-verifies it if it was a factual/grounding claim (or
  trusts it outright if it was a taste/direction call) -- no separate
  classify node, no separate re-verify loop through JudgeDesignDoc. It
  writes its own fresh judgment.json and loops back through the SAME
  CheckDesignJudgment router already used on the first pass.
- Dropped the dedicated retry-budget/blocked node for this loop entirely,
  mirroring RevisePlan -> PlanConfidenceCheck's precedent exactly: a
  human-steered correction loop is trusted to converge, not capped.

Net: -54 lines, 3 new nodes instead of 8, 40 total nodes (was 45).

Verified structurally (dedicated subagent cross-check): no dangling
references to any removed node/file anywhere including comments, every
node reachable with in+out edges, no duplicates, and CheckDesignJudgment's
embedded python was actually executed against all three judgment states
(missing file / pass+no-human / pass+needs-human) and produced correct
routing in each case.
…ual root cause

CONFIRMED via engine source code (modules/loop-pipeline's substitution.py):
the engine's substitute_context() does pure text substitution keyed on the
LITERAL context key string, dots included -- there is no dot-to-underscore
flattening anywhere in the codebase. Static inputs and parse_json-set
dynamic outputs are both consumed by the identical substitution call; there
is no differentiated code path.

Three tool_command attributes referenced a non-existent underscore-flattened
form instead of the real dotted key:
- VerifyDesignDocExists: $paths_design_doc -> $paths.design_doc
- ValidateRubric:        $plan_plan_doc    -> $plan.plan_doc
- CheckPROpened:         $delivery_pr_url  -> $delivery.pr_url

Since the underscore form never matched any real context key, it survived
as literal dead text into the script, and bash silently expanded the
undefined variable to an empty string (no ). For VerifyDesignDocExists
this meant f="" on every single run, so `[ -s "$f" ]` ALWAYS failed
regardless of whether EstablishDesign actually wrote a perfectly good design
doc -- this is the real root cause behind every 'design doc not found'
failure this file has ever produced, including after the two earlier
-prefix-collision fixes (both of those were real, legitimate fixes
for a real bug, but neither was actually why VerifyDesignDocExists kept
failing).

Audited the rest of the file for the same pattern (grepped every
underscore-flattened form of every documented context output key) --
these three were the only occurrences.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant