skills/docs/worklogs/2026-01-23-ralph-wiggum-iteration-counter-fix.md

5.5 KiB

title date keywords commits compression_status
Fix ralph-wiggum iteration counter stuck at 1 2026-01-23
ralph-wiggum
pi-extension
iteration-counter
bug-fix
code-review
0 uncompressed

Session Summary

Date: 2026-01-23 Focus Area: Fixing ralph-wiggum extension iteration counter bug

Accomplishments

  • Diagnosed root cause of iteration counter staying stuck at 1
  • Fixed ralph_done tool to increment iteration before hasPendingMessages() check
  • Applied code review findings (4 issues fixed)
  • Closed issue skills-s984

Key Decisions

Decision 1: Always increment iteration, defer prompt delivery

  • Context: The hasPendingMessages() guard was blocking iteration increment entirely, causing counter to stay at 1
  • Options considered:
    1. Remove the guard entirely - Risk of duplicate prompts
    2. Increment first, only skip prompt delivery if pending - Counter accurate, prompts still controlled
    3. Check for pending USER messages only - Would require API change
    4. Use re-entry flag - More complex, doesn't solve root cause
  • Rationale: Option 2 keeps counter accurate while still preventing duplicate prompt delivery
  • Impact: Iteration counter now reflects actual progress; UI stays accurate; reflection checkpoints work

Decision 2: Move reflection calculation before pending check

  • Context: If reflection was due but pending messages blocked, reflection was skipped entirely (not deferred)
  • Rationale: Reflection state should be recorded regardless of prompt delivery
  • Impact: lastReflectionAt is now set even when prompt is deferred; message indicates "(reflection pending)"

Problems & Solutions

Problem Solution Learning
hasPendingMessages() always true during normal operation Move increment before the check, save state in both branches Guards can block more than intended; order matters
Reflection tracking skipped when prompt deferred Calculate needsReflection before pending check, update lastReflectionAt in both paths Side effects should happen before conditional returns
Error message "not active" was ambiguous Split into two checks: null state vs wrong status, with specific messages Better error messages help debugging
Magic expression state.iteration - 1 repeated Extract to const completedIteration DRY applies to expressions too

Technical Details

Code Changes

  • Total files modified: 1
  • Key files changed:
    • ~/.pi/agent/extensions/ralph-wiggum/index.ts - Fixed ralph_done tool execute function

Before (broken)

if (ctx.hasPendingMessages()) {
  return { content: [{ type: "text", text: "Pending messages already queued. Skipping ralph_done." }], details: {} };
}

// Increment iteration - NEVER REACHED when pending messages exist
state.iteration++;

After (fixed)

// Always increment iteration to keep counter accurate
state.iteration++;
const completedIteration = state.iteration - 1;

// Check max iterations before anything else
if (state.maxIterations > 0 && state.iteration > state.maxIterations) {
  completeLoop(ctx, state, ...);
  return { ... };
}

// Calculate reflection status before pending check (so we don't skip it)
const needsReflection = state.reflectEvery > 0 && completedIteration % state.reflectEvery === 0;
if (needsReflection) state.lastReflectionAt = state.iteration;

// If there are pending messages, record progress but skip prompt delivery
if (ctx.hasPendingMessages()) {
  saveState(ctx, state);
  updateUI(ctx);
  const reflectNote = needsReflection ? " (reflection pending)" : "";
  return {
    content: [{ type: "text", text: `Iteration ${completedIteration} complete.${reflectNote} Prompt deferred - pending messages.` }],
    details: {},
  };
}

Code Review Findings Fixed

  1. [ERROR] MED - Split null/inactive state checks with specific error messages
  2. [SMELL] LOW - Extracted completedIteration constant
  3. [SMELL] LOW - Standardized message format ("complete" in both paths)
  4. [STRUCTURAL] - Moved reflection calculation before pending-messages check

Process and Workflow

What Worked Well

  • Issue had detailed bug report with root cause analysis already done (docs/work/2026-01-22-ralph-iteration-counter-bug.md)
  • Code review lenses caught additional issues beyond the primary fix
  • Fixing all issues in one pass

What Was Challenging

  • Extension not in git repo (deployed separately), couldn't easily diff changes
  • No TypeScript compiler readily available to validate syntax

Learning and Insights

Technical Insights

  • ctx.hasPendingMessages() returns true whenever tool responses are queued - essentially always during multi-tool agent operation
  • Guards that block early can prevent important state updates from happening
  • The order of operations matters: increment → check limits → calculate side effects → check guards → deliver

Architectural Insights

  • The "always save state, optionally defer prompt" pattern is cleaner than "guard blocks everything"
  • Error messages should distinguish between different failure modes

Context for Future Work

Open Questions

  • Should there be a mechanism to force prompt delivery even with pending messages?
  • Should deferred reflections trigger on the next non-deferred iteration?

Next Steps

  • Test the fix in a real ralph loop session
  • Consider adding this pattern to pi extension documentation

Session Metrics

  • Commits made: 0 (not yet committed)
  • Files touched: 1
  • Lines added/removed: ~30/~20
  • Issues closed: 1 (skills-s984)