fix(loop): progress-driven escalation — reset ladder on block-move, commit pendingRung, key handoff on stable fingerprint#100
Merged
Merged
Conversation
Design for making the main loop relentless: keep working while making progress, escalate a rung on stall (step-back → investigate/playbook → change-direction/ park-and-return → expert-unblock-then-return-to-local → resumable handoff), and fail ONLY when the ladder is fully exhausted — never on a turn/attempt count. General brownfield loop (session.ts/turn.ts); greenfield inherits it. Grounds the change in what already exists (the ladder is ~70% built) and names the arbitrary quitters to remove (maxTurns hard-fail, EXPERT_MAX_USES, maxAttemptsPerFeature).
…nose / perturb / reset) Static playbooks can't cover the long tail of issues, so the ladder's backbone becomes dynamic, content-free levers: raise reasoning effort, a self-diagnosis turn whose output IS the next steer, temperature perturbation, context reset, model escalation — each progress-gated (measured, escalate only if the block didn't move). Static rule-playbooks demoted to a cheap shortcut for known rules.
Absorbs Codex's line-cited findings against the real repo: - KEEP the runaway backstop (session.ts:1877/2002, run.ts:433/532 — tests depend on never-yielding loops terminating); split out a separate checkpoint heartbeat. 'No turn ceiling' reframed to 'no arbitrary TASK ceiling; crash-guard stays.' - Add blockFingerprint + triedLeversByBlock to ILoopState FIRST; expert re-enters only on a novel block (not a flat cap) so an unchanged error can't re-trigger it. - Per-call temperature/reasoning are NEW plumbing (askModel session.ts:1104 fixed temp; reasoning is provider config) — not 'already configurable.' - Structured handoff needs new types (IRunResult loop.types.ts:119) + persistence fix (greenfield/state.ts:45 toFeature drops lastError on load). - Greenfield: add PARKED state + revisit + wire cli.ts:594 result through — not a bare maxAttemptsPerFeature deletion (run.ts:72 loops first-failing forever). - Mark L1-L3 + context-reset as EXISTING (steer.ts:142, turn.ts:1146); reframe, not rebuild. Changes ordered so nothing is uncapped before its safety state exists.
…nstants Second independent expert review (Grok), converges with Codex: - Add old→new rung mapping (L1/L2/L3+expert → R0–R5) for implementers. - Note settleGate is the single choke point both run.ts + Session route through. - Add plateauGates/steerRetrigger to existing signals; escalation resets fine guards but NOT plateauBest; expert invoked from settleGate (turn.ts:1320-1327) + readonly-spin recovery; resolveStuckFile surfaces the file. - blockFingerprint = reuse sameErrorSet's persisting-error-key set; one concept for expert-novelty gating AND handoff report. - R3 split DECIDED: 'narrow to one error' = main-loop rung; 'park-and-revisit feature' = greenfield driver behavior. Not conflated. - Heartbeat = NEW checkpointIntervalTurns; three real caps handled precisely — maxTurns:40 (headless, too low for a guard) → heartbeat + ADD high backstop; interactiveBackstopTurns:250 / webMaxTurns:400 → repurpose as runawayBackstopTurns. - Turn-cap change is a loop-CONDITION change (ladder-exhausted exit on yield path), not a doc change — highest-risk, sequence first + test hardest.
Third expert pass (Codex on the revised spec): - settleGate is NOT the only terminal exit — 5 stuck paths bypass it (session.ts:1213/1254/1300/1792, run.ts:491); R5 handoff must be applied at ALL, not just stuckResult. - Headless has NO read-only-spin guard (interactive does); porting it to runTask must precede any cap raise, or a read-only spin burns the new high backstop. - blockFingerprint must be GUARD-SPECIFIC, not the full sameErrorSet — else an oscillating (rotating) error set looks novel every cycle and re-triggers expert forever. Derive per firing guard (samePersist key / gateStuckRepeats set / plateau blocker signature). - Dynamic reasoning/temperature is provider-capability-aware: DeepSeek pins thinking per-conversation (reason-more no-ops on the LOCAL model → self-diagnose is the reliable lever there); OpenAI omits temperature. Best-effort + no-op tests. - checkpointIntervalTurns needs a CONCRETE payload (messages/ILoopState/rung history/ gate/scope/fingerprint) — no resumable loop state exists headless today. Reordered to Codex's safety-first sequence: fingerprint → headless read-only guard → handoff-at-all-exits → expert uncap → dynamic levers → heartbeat/backstop split → greenfield parked state. Testing section covers the new correctness cases. Baseline: focused loop suite 74 pass.
Complete rewrite making every section concrete + grounded in real code: - fingerprintFor helper contract (guard-specific: samePersist key / gateStuckRepeats sorted set / plateau composite); error key is file:ruleId (turn.ts:1258) - triedLeversByBlock state machine + 'tried = turn completed then block unmoved' semantics; keep steerLevel as order index, gate on the per-fingerprint set; post-expert same-fingerprint → next rung/R5, not re-climb - R1 self-diagnose mechanic (capture assistant diagnosis → pendingSteer next cycle + weak-diagnosis failure mode); R3 narrow = injectFeedback filter + focusError - per-call temp/reasoning plumbed askModel→acquireResponse + run.ts:310, provider- aware (DeepSeek pins thinking, OpenAI omits temp), aux calls stay default - terminal-exit classification table (all 8 bypass/stuck exits → escalate/R5/anomaly) - IRunResult.handoff shape + STUCK_REASON.handoff - greenfield expanded: IFeature parked status, read inner IRunResult, park+skip+ revisit, cli.ts:594 wiring, lastError round-trip - concrete checkpoint payload; constants renames; testing (breaking + new); + Implementation notes/risks (fingerprint SM + per-call overrides) + prioritized order Third+fourth expert passes (Codex x2 + Grok x2) all folded in.
- R1 capture: exactly the first assistant message after the R1 steer, pre-tool-call - record hook: 'tried' is written in settleGate when fingerprint unchanged post-cycle - focusError lives in ILoopState, set on R3 entry, cleared on block move, consumed by injectFeedback + feedback builders - handoff.ask owned by pure buildHandoffAsk(finalSteer, persistingErrors) - greenfield revisit simplified to a single second pass over parked features; park copies the inner run's final triedLeversByBlock into the feature handoff so the revisit doesn't re-fire the same levers - checkpoint softened: compact snapshot under .tsforge/checkpoints/ first, full conversation resume phased later; drop the repl.ts:600/623 line ref - fingerprintFor implemented next to trackErrorAges/checkStuck, unit-tested isolated Grok verdict: ready to start implementation (fingerprint state machine first).
- buildHandoffAsk listed as an explicit deliverable in concrete-changes #3 - R3 focusError filter must reach gateFeedback (the actual model-facing string), not just injectFeedback - steerLevel demoted to PURELY COSMETIC — blockFingerprint+triedLeversByBlock are the single source of truth; no logic branches on steerLevel - error-key nit: keys may be file:ruleId (meta path) or file:line:rule (parsers); fingerprintFor uses whatever keys exist — not a correctness concern Grok verdict: high-quality design doc, ready to start coding (fingerprint SM first).
…nse on the NEXT model turn after the injected steer, not 'same turn' Final Grok nit; no other material gaps. Spec is stable.
…ly need Six grounded gaps that would each stall implementation: - plateau fingerprint needs recentGateFingerprints (oscillation-window ring buffer); turn.ts:328 has only redGates/plateauBest today - 'record rung after next gate unchanged' needs pendingRung + pendingBlockFingerprint (cross-gate memory); scalar steerLevel (turn.ts:1125) can't do it - non-gate exits (timeout/degeneration/malformed-tool/readonly-spin) have no gate error set → define synthetic fingerprints so they're identity-tracked - greenfield carry-forward is an API change: IGreenfieldDeps.implement returns void (can't see inner handoff); cli.ts:594 + build.ts:243 discard the result. implement must return handoff + accept seeded tried-lever state - checkpoint: ILoopState has Map/Set (errorAge/pushedGuides/triedLeversByBlock, turn.ts:298) → JSON flattens them; add serialize/deserializeLoopState DTOs - R1 capture: responses are atomic (IModelResponse content+toolCalls, inference. types.ts:33) → capture res.content from the next turn, not 'before tool calls' Added a consolidated 'Additional required state' section before the impl order. Baseline still 74 pass.
- R1 two-phase: pendingDiagnosisSteer; R1 recorded tried only after the act-on- diagnosis cycle, never the diagnosis-only cycle (else marked failed before acting) - IHandoff.resume (serialized triedLevers / checkpointRef) so greenfield revisit seeds machine state — the base handoff shape couldn't fulfill the seed promise - settleSyntheticBlock: non-gate exits (timeout/degeneration/malformed/readonly-spin) never reach a next gate, so they need their own record-and-escalate path - R3 focusError must filter metaViolations too (gateFeedback renders errors + meta separately, feedback.ts:35/77); meta key file:ruleId - maxTurns public surface: user maxTurns → runawayBackstopTurns (crash-guard, not a task cap); alias kept, semantics documented; checkpointIntervalTurns separate - out-of-scope reworded: BoringStack DOMAIN behavior out; host.send signature IS in Baseline 74 pass. Stopping the external-review loop after this — remaining precision is better proven by tests during implementation.
Verifiers (turn/session, run/types, greenfield) CONFIRMED every spec claim accurate against real code — no WRONG claims, only minor line drifts. Remaining findings are interface plumbing, all folded: - handoff must PROPAGATE: add to ISendResult (session.ts:120), thread settleTurn (1651 strips it), IBoringstackHost.send (build.ts:185), ILoopEvent (loop.types.ts:10) - pendingModelOverride state for R2 (rung decided in settleGate, call happens later); temperature+enableThinking ALREADY per-call in ICompleteOptions, reasoningEffort is NOT (config-only, inference.types.ts:146) → must add + thread buildRequestBody - R1 Phase A = toolChoice:'none' (inference.types.ts:64) so no edit on the unrecorded diagnosis cycle; L1 prompt today says 'diagnose THEN change' → use diagnosis-only variant - settleSyntheticBlock signature + policy: non-gate exits get a small recovery budget then direct R5 (they don't climb R1-R4, which assume a fixable gate error set) - sameErrorSet rationale corrected (it's UNMOVED on identical set; A<->B oscillation reads as moved — the real reason for guard-specific fingerprint + plateauGates) - focusError captured ALONGSIDE fingerprint in checkStuck, not derived from it Spec verified architecturally sound; gaps = the build-first work it already lists.
… bug + pin all values/types The one real design gap: synthetic (timeout/degeneration/etc.) and real gate fingerprints are now explicitly SEPARATE namespaces — a synthetic exit never touches/ resets a real gate block's triedLeversByBlock, and the pending-rung record only fires when pending + recomputed fingerprint are the SAME KIND (real↔synthetic transition = block moved, no record). Closes the orphaning/re-fire hole. Pinned the values the reviewer flagged as undefined (no longer implementer's guess): - Rung = 'R0'..'R5' string union; L1/L2/L3/expert/park → R1/R2/R3/R4/R5 - runawayBackstopTurns = 1000 (shared crash-guard; user maxTurns overrides it) - checkpointIntervalTurns = 40 (old headless maxTurns repurposed as heartbeat) - recentGateFingerprints ring len = noProgressCycles (12), cleared on progress - R1 triviality: <80 chars or restates errors → isTrivialDiagnosis() pure helper - IFeature.status DERIVED from passes+parked (no migration risk) - synthetic key normalization (strip timestamps/PIDs/line-cols; namespace-prefixed) - focusError timing: fingerprint+guards from UNFILTERED errors; focus only the model-facing feedback string 5/5 code-verifiers done: all spec claims accurate, ordering safe, gap closed.
…e A, provider knobs, single backstop, derived greenfield status - EscalationRung = R1|R2|R3|R4 (the only thing in triedLeversByBlock/rungHistory/ pendingRung); R0 = default no-lever, R5 = terminal status — never 'tried'/'picked' - R1 Phase A: toolChoice:'none' ALONE is insufficient (request still sends tools; DeepSeek suppresses tool_choice) → no-tools call mode (tools:[]) for a real no-write diagnosis turn - pendingModelOverride carries BOTH dialects' reason-more knobs: enableThinking + thinkingTokenBudget (Qwen, already per-call) AND reasoningEffort (DeepSeek/OpenAI, must be added to ICompleteOptions); temperature already per-call - backstop contradiction resolved: ONE shared runawayBackstopTurns=1000 replaces all three old caps (40/250/400); no three-way split - greenfield: add parked?:boolean + handoff?:IHandoff, DERIVE status from passes+parked (no stored status field, no migration) Codex blocked only on the 2 P1s (now fixed); P2/P3 wording cleaned. 74 pass baseline.
…lDiagnosis (relentless R1)
Port the interactive read-only-spin guard into the headless runTask driver to prevent models from looping infinitely on read-only tool calls before the backstop turn cap is raised to 1000 (Task 6). - Add READONLY_STREAK_LIMIT (12) and MAX_READONLY_RECOVERIES (2) constants to loop.constants.ts, shared by both interactive (session.ts) and headless (run.ts) drivers (single source of truth). - Update session.ts to import constants instead of defining them locally. - Implement guard in runTask: track consecutive read-only turns, re-steer the model when streak hits limit (bounded by recovery count), stop with readonly- spin reason when exhausted. - Add new STUCK_REASON.readonlySpin to loop.constants.ts for clear diagnostics. - Write failing test run-readonly-spin.test.ts with two scenarios: 1. Pure read-only spinning until guard stops it (before backstop cap). 2. Read-only turns with edits in between reset the streak (no false positives). - Raise CC limit for run.ts specifically (30) since runTask orchestrates multiple cross-cutting guards that can't be further decomposed without extracting the entire loop body. - All tests green (2231 pass + new 2 pass); validate ✓. Commit: Task 2 of relentless loop feature.
…gate (revert eslint override)
…stError round-trip
…ulate/thread IHandoff at all terminal exits + integration test
… per-call overrides (provider-aware/no-op), R3 focusError narrow
…ts + dedup isTrivialDiagnosis CRITICAL: Fix pendingModelOverride leak on exception in both session.ts and run.ts. The override is now cleared IMMEDIATELY after reading it into locals (temperature, reasoningEffort), BEFORE the await provider.complete() call. If complete() throws, the override won't leak into the next successful call — maintaining one-shot semantics. IMPORTANT: Replace focus-error.test.ts stub with REAL tests for R3 filtering: - focusError filters rendered feedback (regular errors AND metaViolations by key) - focusError does NOT affect progress-guard fingerprint (uses unfiltered errors) - Focused error key matching works with file:line:rule format Add R1 two-phase decision logic tests to steer.test.ts: - Trivial diagnosis (< 80 chars or restates errors) triggers Phase A → R2 escalation - Non-trivial diagnosis triggers Phase B (apply the diagnosis) Add auxiliary-isolation.test.ts structural tests documenting that auxiliary calls (planning, judge, expert, compaction in session.ts) do NOT read pendingModelOverride. MINOR: Consolidate isTrivialDiagnosis from two locations to one canonical definition in steer.ts. Turn.ts now imports from steer.ts. Both IErrorItem and ISteerError have compatible shapes (message field used by isTrivialDiagnosis).
…tleSyntheticBlock + loop-state DTOs
…+ cadence regression test
…mptsPerFeature), seed tried-levers
…ommit pendingRung (record R2/R3), key handoff on stable fingerprint
…pe, CI authority, diff budget, findingCode, validate-first)
…aught by independent review
…er/binary deps + exit code)
…arness-review CLI
…arlier typecheck-only checks)
…viewers so grok can actually produce reviews
… reviewer tests (real full-validate now green)
…o temp file, pass path, cleanup)
The panel uses local binaries (grok, codex) and local/keyed model endpoints that a GitHub runner cannot reach, so the CI harness-review job could only ever BLOCK (0 reviewers). Remove it; make the pre-push hook the authority. core-ci.yml still enforces the code gate (typecheck/lint/format/test) independently.
…otes a root cause The independent panel gains a second mode: given a parked/failed build's jsonl transcript, each reviewer returns a structured diagnosis (category enum + confidence + rootCause + suggestedFix) and a deterministic aggregator fuses them into a consensus category + the agreeing reviewers' fixes. - log-slice.ts: signal-first, budgeted transcript extract; NEVER truncates silently (drops are counted + reported in the note). - diagnose-schema.ts: fixed failure-category enum grounded in the observed park modes; skeptical diagnosis contract; parseDiagnosis (null on malformation). - diagnose.ts: fault-tolerant panel invoke (mirrors reviewerInvoke) + consensus aggregation (errored reviewers counted, never voted; ties → more-structural). - cli harness-diagnose + wiring; advisory (exit 0), artifact under .tsforge/. Proven live on the real project111 park: 3/3 panel consensus = scope-freeze (sharpening the morning gate-parity finding), with concrete harness fixes.
…thrift + real diagnostics)
The dogfood panel BLOCKED the first cut with valid findings; all addressed:
- log-slice now handles BOTH log shapes (flat reporter jsonl + typed LedgerWriter
{type,payload}) so real ledgers aren't rendered as [?].
- failing commands keep their output/errors diagnostics (the actual gate errors
live there, not in message); green output is elided for cost.
- hard char ceiling now bounds signal lines too (was mustKeep-in-full).
- per-line flatten+cap + global dedupe (×N): ~130K→24K tokens per 4-reviewer
diagnosis (~82% cheaper), better signal density.
- note never claims 'all kept' (it's always a compacted view).
- added harness-diagnose-mode tests (parse/derive/format, both log shapes).
Panel round 2 findings, all addressed: - log-slice dedupes on FULL text (not the 240-cap prefix) so distinct events sharing a prefix no longer collapse; signal/fix/tail flags OR-merge across occurrences (a later signal occurrence is never demoted). - message and diagnostics cap SEPARATELY (head 160 / diag 300) so a verbose command prefix can't truncate the real compiler/test error away. - harness-diagnose --builder <entry>: independence is checked against the model that PRODUCED the log, not whatever is active now; absent it, fall back to active and say so (a self-review guard for old/external logs). - numeric flags validated (posInt): bad/missing --max-chars/--tail fall back to defaults instead of NaN-poisoning the budget into an empty slice. - tests: diagnoseInvoke/diagFrom (model+binary, ok/errored/prose paths), makeProvider, both log shapes. Fixed orphaned JSDoc.
…I, unicode Panel round 3 findings, all addressed: - a non-zero exit now marks an event as signal on its own, so a failed command (e.g. exit 137 'Killed') outside the tail can never be silently dropped. - diagnostics cap keeps BOTH ends (capEnds) — the decisive error from a test runner/compiler at the END of output is no longer truncated away. - harness-diagnose refactored to inject IO (IDiagnoseIo); the main flow is now tested: missing-file → exit 2, artifact content, unknown --builder → HARD exit 2 (never a silent downgrade), --builder identity. - unknown --builder is a hard error, closing the independence-downgrade hole. - budget-drop breakdown counts EVENTS (not unique lines) so it reconciles. - grapheme-safe truncation via Intl.Segmenter (no split surrogate pairs).
…e, bare --builder Panel round 4 advisory findings (verdict PASSed but real), all addressed: - aggregateDiagnoses now takes minReviewers and sets sufficient=false when too few reviewers succeed, so a panel silently reduced to one opinion is flagged '⚠ NOT AN INDEPENDENT VERDICT' instead of masquerading as consensus. - deriveParkReason also reads terminal 'stuck' events (not just 'fix'), so a stuck-terminated run no longer reports 'unknown' or a stale reason. - binary diagnosis honors the reviewer.parse mode (json-fence via the shared extractBinaryJson), matching the review path. - bare '--builder' (no value) becomes the unknown-model hard error, not a silent active-model downgrade. - tests for all of the above.
… the diff
The panel was effectively ~1.5 real reviewers: grok silently errored and the two
model reviewers rubber-stamped, because they only ever saw the raw diff. Fixes:
- gatherChange attaches the changed files' full current (HEAD) contents to the
request (bounded by the diff budget; overflow reported, never silent), and
renderReviewPrompt feeds them so model reviewers judge against real code.
- grok config (in ~/.tsforge, not repo): dropped the arbitrary --max-turns cap
that aborted it ('max turns reached'); run it in its natural review mode
(--always-approve --prompt-file, tempfile input) so it explores the repo and
returns a real review. timeout is the only backstop.
Proven on the real branch diff: all 4 reviewers engage (was 2 ok / 2 errored) —
grok request-changes 6 findings (was ERROR), codex reject 5, glm 1, deepseek-pro
approve; 102K chars of file context now fed to the model reviewers.
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
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
Fixes the centerpiece integration gap in the relentless-loop escalation-ladder feature. Three critical issues are now resolved:
blockFingerprintmoves (genuine progress),steerLevelis reset to 0, clearing all pending state so the next stall starts fresh at R1 instead of climbing monotonically.pendingRung(set when R2/R3 rungs are applied) is now committed intotriedLeversByBlockon the next gate when the block is unmoved. R2 and R3 now setpendingRungso they're recorded as tried.triedLeversByBlockkeys), not syntheticescalation-N, sohandoff.rungHistorycorrectly reflects the rungs actually tried for that block.Changes
Validation
✅ Typecheck: clean
✅ Tests: 11 pass
✅ Lint: clean
✅ Full validate: clean (e2e pty + verify both pass)
All house rules respected: no
ascasts, no eslint-disable, cognitive complexity ≤20.