Merge repetition guard

Author: TheGreatAxios

src/tui-opentui/runtime-bridge.ts

@@ -41,10 +41,12 @@ import {
 import { quotaWaitSeconds, shouldAutoRetryQuota } from "./quota-retry.js"
 import {
   applyStallRecovery,
+  repetitionRecoveryMessage,
   shouldAbortForStall,
   shouldNoticeStall,
   STALL_NOTICE_MESSAGE,
   STALL_NOTICE_MS,
+  STALL_RECOVERY_MESSAGE,
   STALL_TIMEOUT_MS,
 } from "./stall-watchdog.js"
 import {
@@ -919,6 +921,27 @@ export function attachSessionBridge(
       return
     }
 
+    // Content-based, not time-based: a repeating line means the model is
+    // stuck regardless of how fast it is producing it, so this is checked
+    // before the silence clock rather than folded into it.
+    //
+    // Gated on `status === "running"` because every turn-ending transition
+    // (interrupt, connector.reply with no tools outstanding, reactor.done /
+    // reactor.error) routes through `initialTurnState`, which clears
+    // `repeating`. If a future settle path changes `isProcessing` without
+    // also resetting `status` and `repeating` through that same reset, this
+    // guard would no longer mean "the turn is actually live" and could fire
+    // on an already-settled turn — recheck this alongside any such change.
+    if (bag.turn.status === "running" && bag.turn.repeating) {
+      const repeatedTokens =
+        bag.turn.streamTokenCount - (bag.turn.repeatingSinceTokenCount ?? 0)
+      applyStallRecovery(
+        { abort: doInterrupt, notify: (message) => setStatusFlash(shell, message) },
+        repetitionRecoveryMessage(repeatedTokens),
+      )
+      return
+    }
+
     const stallArgs = {
       status: bag.turn.status,
       awaitingResponse: bag.turn.awaitingResponse,
@@ -930,16 +953,22 @@ export function attachSessionBridge(
     }
 
     if (shouldAbortForStall(stallArgs)) {
-      applyStallRecovery({
-        abort: doInterrupt,
-        notify: (message) => setStatusFlash(shell, message),
-      })
+      applyStallRecovery(
+        { abort: doInterrupt, notify: (message) => setStatusFlash(shell, message) },
+        STALL_RECOVERY_MESSAGE,
+      )
       return
     }
 
     // Notice only — the phase still paints below, because a ramp that stops
     // moving is the very thing that reads as a hang.
-    if (shouldNoticeStall({ ...stallArgs, stallNoticeMs })) {
+    if (
+      shouldNoticeStall({
+        ...stallArgs,
+        stallNoticeMs,
+        repeating: bag.turn.repeating,
+      })
+    ) {
       setStatusFlash(shell, STALL_NOTICE_MESSAGE)
     }
 

src/tui-opentui/stall-watchdog.test.ts

@@ -2,6 +2,8 @@ import { describe, expect, test } from "bun:test"
 
 import {
   applyStallRecovery,
+  detectRepetition,
+  repetitionRecoveryMessage,
   shouldAbortForStall,
   shouldNoticeStall,
   STALL_NOTICE_MS,
@@ -81,14 +83,93 @@ describe("shouldAbortForStall", () => {
 })
 
 describe("applyStallRecovery", () => {
-  test("aborts then notifies", () => {
+  test("aborts then notifies with the default message", () => {
     const calls: string[] = []
     applyStallRecovery({
       abort: () => calls.push("abort"),
       notify: (m) => calls.push(m),
     })
     expect(calls).toEqual(["abort", STALL_RECOVERY_MESSAGE])
   })
+
+  test("aborts then notifies with a supplied message", () => {
+    const calls: string[] = []
+    applyStallRecovery(
+      { abort: () => calls.push("abort"), notify: (m) => calls.push(m) },
+      "custom message",
+    )
+    expect(calls).toEqual(["abort", "custom message"])
+  })
+})
+
+describe("detectRepetition", () => {
+  test("finds nothing in fresh, varied output", () => {
+    const text = [
+      "I'll check the callId emission path first.",
+      "Running the search now.",
+      "Found three matches across the module.",
+    ].join("\n")
+    expect(detectRepetition(text).repeating).toBe(false)
+  })
+
+  // The captured incident: the two sentences ran together with no line break
+  // at all. A line-splitting detector never sees this; the period search
+  // does not care where (or whether) the lines break.
+  test("flags the captured incident string verbatim, with no newlines", () => {
+    const line1 =
+      "I'll verify callId emission and remaining edges, then write the ranked findings."
+    const line2 = "Confirming callId emission, then writing the ranked findings."
+    const text = Array(10).fill(`${line1}${line2}`).join("")
+    const check = detectRepetition(text)
+    expect(check.repeating).toBe(true)
+    expect(check.period).toBe(line1.length + line2.length)
+  })
+
+  test("does not flag the same cycle a handful of times", () => {
+    const line1 =
+      "I'll verify callId emission and remaining edges, then write the ranked findings."
+    const line2 = "Confirming callId emission, then writing the ranked findings."
+    // Fewer than the occurrence threshold: a model can legitimately restate
+    // a step once or twice across tool-call cycles without looping.
+    const text = Array(4).fill(`${line1}${line2}`).join("")
+    expect(detectRepetition(text).repeating).toBe(false)
+  })
+
+  test("does not flag a repeated markdown table separator row", () => {
+    const row = "| ---------------------- | ---------------------- |"
+    const text = Array(6).fill(row).join("\n")
+    expect(detectRepetition(text).repeating).toBe(false)
+  })
+
+  test("does not flag a few identical code lines", () => {
+    const line = "  const result = await fetchData(request, options, context)"
+    const text = Array(3).fill(line).join("\n")
+    expect(detectRepetition(text).repeating).toBe(false)
+  })
+
+  test("ignores short recurring fragments", () => {
+    const text = Array(10).fill("ok").join(" ")
+    expect(detectRepetition(text).repeating).toBe(false)
+  })
+
+  // A monochrome run is periodic at every period by construction — the
+  // easiest thing to false-trigger on if entropy is not checked.
+  test("does not flag a long run of the same character", () => {
+    expect(detectRepetition("x".repeat(500)).repeating).toBe(false)
+  })
+
+  test("does not flag a repeated horizontal rule", () => {
+    const text = Array(10).fill("----------------------------").join("\n")
+    expect(detectRepetition(text).repeating).toBe(false)
+  })
+})
+
+describe("repetitionRecoveryMessage", () => {
+  test("names degeneration and attributes the looped tokens", () => {
+    const message = repetitionRecoveryMessage(42)
+    expect(message).toContain("repeating itself")
+    expect(message).toContain("42")
+  })
 })
 
 describe("shouldNoticeStall", () => {
@@ -101,8 +182,13 @@ describe("shouldNoticeStall", () => {
     stallNoticeMs: STALL_NOTICE_MS,
     isProcessing: true,
     streamingType: null,
+    repeating: false,
   }
 
+  test("stays quiet while repeating, even if also silent by the clock", () => {
+    expect(shouldNoticeStall({ ...base, repeating: true })).toBe(false)
+  })
+
   test("speaks up long before the abort backstop", () => {
     expect(STALL_NOTICE_MS).toBeLessThan(STALL_TIMEOUT_MS)
     expect(shouldNoticeStall(base)).toBe(true)

src/tui-opentui/stall-watchdog.ts

@@ -21,6 +21,90 @@ export type ShouldAbortForStallArgs = {
   readonly streamingType: "text" | "thinking" | "tool" | null
 }
 
+// The captured incident looped two sentences with no line break between them
+// ("...ranked findings.Confirming callId emission...") — degeneration is a
+// character-level loop, not a line-level one. Splitting on "\n" misses it
+// entirely, so the tail is treated as a plain string and checked for the
+// smallest period it exactly repeats: the shortest span p such that the last
+// several hundred characters equal p repeated.
+//
+// A period below this is more likely a short structural tic (indentation, a
+// repeated bullet or table-cell divider) than a looping phrase. Chosen well
+// under the ~140-char period of the captured incident's two-sentence cycle,
+// with headroom for shorter degenerate loops (a single repeated sentence).
+const REPETITION_MIN_PERIOD = 24
+// How many exact repeats of the period are required before it counts as a
+// loop rather than a coincidence. Verified against real non-degenerate
+// repetition: a 6-row markdown table separator (period ~51 chars, 6 exact
+// repeats) and 3 identical code lines (period ~60 chars, 3 exact repeats)
+// both land under this bar and are not flagged; the captured incident's
+// sentence pair comfortably clears it well before the stream ends.
+const REPETITION_MIN_REPEATS = 8
+// Hard ceiling on the period search regardless of buffer size, purely to cap
+// worst-case work per check — token-level degeneration loops on a phrase or
+// two, never on multi-paragraph spans.
+const REPETITION_MAX_PERIOD_CAP = 2_000
+// A monochrome run ("x".repeat(500), a "----" rule, a wall of spaces) is
+// trivially periodic at *every* period, which would otherwise make it the
+// single easiest thing to false-trigger on — verified by execution against
+// `thinking-reveal.test.ts`'s burst-of-"x" fixture, which tripped the guard
+// before this floor existed. Requiring the repeating unit itself to contain
+// this many distinct characters keeps single-character and low-variety runs
+// out without weakening the sentence-level case: the captured incident's
+// cycle spans two full sentences, comfortably above it.
+const REPETITION_MIN_DISTINCT_CHARS = 8
+
+export type RepetitionCheck = {
+  readonly repeating: boolean
+  readonly period: number | null
+  readonly repeats: number
+}
+
+/**
+ * Length of the exact-period run ending at the last character of `text`,
+ * including the base period itself. `text[i] === text[i - period]` walked
+ * backwards from the end; stops at the first mismatch or the start of the
+ * string.
+ */
+function periodicSuffixLength(text: string, period: number): number {
+  let i = text.length - 1
+  let j = i - period
+  let matched = 0
+  while (j >= 0 && text[i] === text[j]) {
+    matched++
+    i--
+    j--
+  }
+  return matched + period
+}
+
+/**
+ * Whether the tail of `text` is an exact repeat of some short span at least
+ * `REPETITION_MIN_REPEATS` times. Pure text-in, decision-out: the caller owns
+ * accumulating the buffer across deltas and cycles within a turn.
+ *
+ * Periods longer than `text.length / REPETITION_MIN_REPEATS` are skipped, not
+ * as an arbitrary cutoff but because they cannot mathematically reach the
+ * occurrence threshold within the given text — a loop with a longer period
+ * needs a longer buffer to confirm, which is a buffer-size trade-off owned by
+ * the caller, not a second detection path here.
+ */
+export function detectRepetition(text: string): RepetitionCheck {
+  const maxPeriod = Math.min(
+    REPETITION_MAX_PERIOD_CAP,
+    Math.floor(text.length / REPETITION_MIN_REPEATS),
+  )
+  for (let period = REPETITION_MIN_PERIOD; period <= maxPeriod; period++) {
+    const matched = periodicSuffixLength(text, period)
+    const repeats = matched / period
+    if (repeats < REPETITION_MIN_REPEATS) continue
+    const unit = text.slice(text.length - period)
+    if (new Set(unit).size < REPETITION_MIN_DISTINCT_CHARS) continue
+    return { repeating: true, period, repeats }
+  }
+  return { repeating: false, period: null, repeats: 0 }
+}
+
 /**
  * Whether silence of `thresholdMs` counts as stuck at all. Shared by the notice
  * and the abort so they never disagree about which runs are stalled — only
@@ -50,31 +134,51 @@ export function shouldAbortForStall(args: ShouldAbortForStallArgs): boolean {
 
 export type ShouldNoticeStallArgs = ShouldAbortForStallArgs & {
   readonly stallNoticeMs: number
+  /** Whether the repetition guard currently sees a looping tail. */
+  readonly repeating: boolean
 }
 
 /**
  * Returns true while the run has been silent long enough to say so but not yet
  * long enough to abort. False once the abort takes over, so the two never
- * paint at the same time.
+ * paint at the same time, and false while repeating — that run is producing
+ * output, just not useful output, and "no response" would misdescribe it.
  */
 export function shouldNoticeStall(args: ShouldNoticeStallArgs): boolean {
+  if (args.repeating) return false
   if (shouldAbortForStall(args)) return false
   return silentPastThreshold(args, args.stallNoticeMs)
 }
 
-/** Shown while the run is silent; names the state and the way out. */
+/**
+ * Shown while nothing is arriving at all. Never fires while tokens are
+ * flowing — a model looping on repeated content is still producing output,
+ * so it is reported by `repetitionRecoveryMessage` instead, not this one.
+ */
 export const STALL_NOTICE_MESSAGE = "no response for a while — ctrl+c to interrupt"
 
 export const STALL_RECOVERY_MESSAGE =
   "stopped after no response — send again to retry"
 
+/**
+ * Shown once a repeated line aborts the turn. Named as degeneration, not a
+ * generic failure, so a retry reads as the reasonable next step rather than
+ * papering over a suspected hang or network fault.
+ */
+export function repetitionRecoveryMessage(repeatedTokens: number): string {
+  return `stopped after repeating itself — ~${repeatedTokens} tokens looped — send again to retry`
+}
+
 export type ApplyStallRecoveryDeps = {
   /** Abort the in-flight run through the session port. */
   readonly abort: () => void
   readonly notify: (message: string) => void
 }
 
-export function applyStallRecovery(deps: ApplyStallRecoveryDeps): void {
+export function applyStallRecovery(
+  deps: ApplyStallRecoveryDeps,
+  message: string = STALL_RECOVERY_MESSAGE,
+): void {
   deps.abort()
-  deps.notify(STALL_RECOVERY_MESSAGE)
+  deps.notify(message)
 }

src/tui-opentui/turn-monitor.test.ts

@@ -369,6 +369,64 @@ describe("stall watchdog", () => {
   })
 })
 
+describe("repetition guard", () => {
+  test("aborts a looping model without waiting on the stall clock", async () => {
+    await withTestRenderer(async (h) => {
+      const t: Harness = await setup(h)
+      try {
+        t.bridge.submit("build it", "immediate")
+        t.port.clear()
+
+        // The captured incident shape: the two sentences run together with
+        // no line break between cycles.
+        const line1 =
+          "I'll verify callId emission and remaining edges, then write the ranked findings."
+        const line2 = "Confirming callId emission, then writing the ranked findings."
+        const cycle = `${line1}${line2}`
+
+        // Tokens keep landing every tick — a real stall would never fire here.
+        for (let i = 0; i < 10; i++) {
+          t.bridge.handle({
+            type: "inference.text.delta",
+            data: { token: cycle },
+          })
+          t.advance(10)
+          t.tick()
+        }
+
+        expect(t.port.calls).toEqual([{ op: "interrupt" }])
+        expect(t.shell.statusFlash).toContain("repeating itself")
+        expect(t.shell.statusFlash).not.toBe(STALL_NOTICE_MESSAGE)
+      } finally {
+        t.bridge.dispose()
+      }
+    })
+  })
+
+  test("a slow but progressing turn is never killed", async () => {
+    await withTestRenderer(async (h) => {
+      const t: Harness = await setup(h)
+      try {
+        t.bridge.submit("build it", "immediate")
+        t.port.clear()
+
+        for (let i = 0; i < 5; i++) {
+          t.bridge.handle({
+            type: "inference.text.delta",
+            data: { token: `distinct progress update number ${i}\n` },
+          })
+          t.advance(500)
+          t.tick()
+        }
+
+        expect(t.port.calls).toEqual([])
+      } finally {
+        t.bridge.dispose()
+      }
+    })
+  })
+})
+
 describe("reasoning settles to a summary", () => {
   test("a closed thinking row carries its elapsed time", async () => {
     await withTestRenderer(async (h) => {