Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 34 additions & 5 deletions src/tui-opentui/runtime-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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)
}

Expand Down
88 changes: 87 additions & 1 deletion src/tui-opentui/stall-watchdog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { describe, expect, test } from "bun:test"

import {
applyStallRecovery,
detectRepetition,
repetitionRecoveryMessage,
shouldAbortForStall,
shouldNoticeStall,
STALL_NOTICE_MS,
Expand Down Expand Up @@ -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", () => {
Expand All @@ -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)
Expand Down
112 changes: 108 additions & 4 deletions src/tui-opentui/stall-watchdog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
58 changes: 58 additions & 0 deletions src/tui-opentui/turn-monitor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
Loading
Loading