diff --git a/src/subagent/repetition.ts b/src/subagent/repetition.ts index 1c8c80e1..104f8d08 100644 --- a/src/subagent/repetition.ts +++ b/src/subagent/repetition.ts @@ -6,8 +6,17 @@ * invisible to them. This module watches the accumulated text of the current * inference cycle and flags a trailing window that repeats verbatim past a * threshold, so the run loop can abort the cycle instead of streaming forever. + * + * Also home to the TUI stall-watchdog's character-level tail-repetition + * guard (`detectTailCharLoop`) — a separate, simpler check consolidated + * here from tui/stall-watchdog.ts so the two repetition detectors live in one + * module instead of two. It solves the same "is the tail looping" question + * for a different consumer with different constants; see its own doc comment + * for why it is not merged into `detectRepetition` above. */ +import { detectSequencePeriod, type SequencePeriodCheck } from "../util/period-detection.js"; + /** Tunable thresholds for the trailing-window repetition check. */ export interface RepetitionConfig { /** Smallest normalized window (chars) considered a loop unit. */ @@ -245,3 +254,72 @@ export function trackContentlessGrowth( hit: visibleChars < config.minVisibleChars, }; } + +// 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. Live loops +// repeat units as short as 10 chars ("Groaning. " emitted ~1,363 times), so +// the floor sits at 8 — short structural tics that survive it (a "- item\n" +// bullet is 7 chars) fall below, and the ones at or above it are filtered by +// the distinct-chars floor and the raised repeat bar instead. Still well +// under the ~140-char period of the captured incident's two-sentence cycle. +const CHAR_REPETITION_MIN_PERIOD = 8; +// How many exact repeats of the period are required before it counts as a +// loop rather than a coincidence. Raised 3x in step with the 3x-lower period +// floor so the minimum exactly-periodic span stays at 192 chars (was 24*8, +// now 8*24). 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 far +// under this bar and are not flagged; a genuine degenerate loop repeats +// hundreds of times, so it still clears the bar long before the stream ends. +const CHAR_REPETITION_MIN_REPEATS = 24; +// 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 CHAR_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 CHAR_REPETITION_MIN_DISTINCT_CHARS = 8; + +export type TailCharLoopCheck = SequencePeriodCheck; + +/** + * Whether the tail of `text` is an exact repeat of some short span at least + * `CHAR_REPETITION_MIN_REPEATS` times. Pure text-in, decision-out: the caller + * (the TUI stall watchdog) owns accumulating the buffer across deltas and + * cycles within a turn. + * + * Delegates to the generic detectSequencePeriod over the character array — + * periods longer than `text.length / CHAR_REPETITION_MIN_REPEATS` are skipped + * there, not as an arbitrary cutoff but because they cannot mathematically + * reach the occurrence threshold within the given text. + * + * This is deliberately not merged with `detectRepetition` above: that one + * normalizes whitespace/invisibles and optionally folds digits before + * running a KMP period search tuned for streamed model text, while this is a + * plain per-character search with a distinct-chars floor instead of digit + * folding, tuned for the TUI's live character buffer. Same question ("is the + * tail looping"), different constants and different false-positive shape — + * see the config comments on each for why neither threshold set may be + * changed to match the other. + */ +export function detectTailCharLoop(text: string): TailCharLoopCheck { + return detectSequencePeriod(text.split(""), { + minPeriod: CHAR_REPETITION_MIN_PERIOD, + maxPeriod: CHAR_REPETITION_MAX_PERIOD_CAP, + minRepeats: CHAR_REPETITION_MIN_REPEATS, + minDistinct: () => CHAR_REPETITION_MIN_DISTINCT_CHARS, + }); +} diff --git a/src/tui/stall-watchdog.test.ts b/src/tui/stall-watchdog.test.ts index e0645430..cc1adc80 100644 --- a/src/tui/stall-watchdog.test.ts +++ b/src/tui/stall-watchdog.test.ts @@ -2,7 +2,6 @@ import { describe, expect, test } from "bun:test"; import { applyStallRecovery, - detectRepetition, isStalledForDisplay, repetitionRecoveryMessage, shouldAbortForStall, @@ -12,6 +11,7 @@ import { STALL_RECOVERY_MESSAGE, STALL_TIMEOUT_MS, } from "./stall-watchdog.js"; +import { detectTailCharLoop } from "../subagent/repetition.js"; describe("shouldAbortForStall", () => { // Mid-stream hang: tokens already flowed, then everything went silent — @@ -140,14 +140,14 @@ describe("applyStallRecovery", () => { }); }); -describe("detectRepetition", () => { +describe("detectTailCharLoop", () => { 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); + expect(detectTailCharLoop(text).repeating).toBe(false); }); // The captured incident: the two sentences ran together with no line break @@ -158,7 +158,7 @@ describe("detectRepetition", () => { "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(30).fill(`${line1}${line2}`).join(""); - const check = detectRepetition(text); + const check = detectTailCharLoop(text); expect(check.repeating).toBe(true); expect(check.period).toBe(line1.length + line2.length); }); @@ -168,7 +168,7 @@ describe("detectRepetition", () => { // chars keeps it above REPETITION_MIN_DISTINCT_CHARS. test("flags a short-phrase loop with a 10-char unit", () => { const text = "Groaning. ".repeat(60); - const check = detectRepetition(text); + const check = detectTailCharLoop(text); expect(check.repeating).toBe(true); expect(check.period).toBe("Groaning. ".length); }); @@ -180,35 +180,35 @@ describe("detectRepetition", () => { // 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); + expect(detectTailCharLoop(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); + expect(detectTailCharLoop(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); + expect(detectTailCharLoop(text).repeating).toBe(false); }); test("ignores short recurring fragments", () => { const text = Array(10).fill("ok").join(" "); - expect(detectRepetition(text).repeating).toBe(false); + expect(detectTailCharLoop(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); + expect(detectTailCharLoop("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); + expect(detectTailCharLoop(text).repeating).toBe(false); }); }); diff --git a/src/tui/stall-watchdog.ts b/src/tui/stall-watchdog.ts index d4ea9388..c1f8967c 100644 --- a/src/tui/stall-watchdog.ts +++ b/src/tui/stall-watchdog.ts @@ -1,5 +1,4 @@ import type { TurnStatus } from "./session-chrome.js"; -import { detectSequencePeriod, type SequencePeriodCheck } from "../util/period-detection.js"; // How long the run can be continuously awaiting a response with no new content // before the watchdog fires and aborts the in-flight request. @@ -26,65 +25,6 @@ export interface ShouldAbortForStallArgs { readonly activeToolCalls: readonly string[]; } -// 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. Live loops -// repeat units as short as 10 chars ("Groaning. " emitted ~1,363 times), so -// the floor sits at 8 — short structural tics that survive it (a "- item\n" -// bullet is 7 chars) fall below, and the ones at or above it are filtered by -// the distinct-chars floor and the raised repeat bar instead. Still well -// under the ~140-char period of the captured incident's two-sentence cycle. -const REPETITION_MIN_PERIOD = 8; -// How many exact repeats of the period are required before it counts as a -// loop rather than a coincidence. Raised 3x in step with the 3x-lower period -// floor so the minimum exactly-periodic span stays at 192 chars (was 24*8, -// now 8*24). 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 far -// under this bar and are not flagged; a genuine degenerate loop repeats -// hundreds of times, so it still clears the bar long before the stream ends. -const REPETITION_MIN_REPEATS = 24; -// 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 = SequencePeriodCheck; - -/** - * 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. - * - * Delegates to the generic detectSequencePeriod over the character array — - * periods longer than `text.length / REPETITION_MIN_REPEATS` are skipped - * there, not as an arbitrary cutoff but because they cannot mathematically - * reach the occurrence threshold within the given text. - */ -export function detectRepetition(text: string): RepetitionCheck { - return detectSequencePeriod(text.split(""), { - minPeriod: REPETITION_MIN_PERIOD, - maxPeriod: REPETITION_MAX_PERIOD_CAP, - minRepeats: REPETITION_MIN_REPEATS, - minDistinct: () => REPETITION_MIN_DISTINCT_CHARS, - }); -} - /** * 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 diff --git a/src/tui/turn-state.ts b/src/tui/turn-state.ts index 6d92e3a4..f14e244e 100644 --- a/src/tui/turn-state.ts +++ b/src/tui/turn-state.ts @@ -11,15 +11,15 @@ import { type } from "arktype"; -import { detectRepetition } from "./stall-watchdog.js"; +import { detectTailCharLoop } from "../subagent/repetition.js"; import type { TurnStatus } from "./session-chrome.js"; // Bound on the accumulated stream text kept for repetition checks. Comfortably -// larger than the periods `detectRepetition` can confirm, so trimming never +// larger than the periods `detectTailCharLoop` can confirm, so trimming never // drops content the check still needs. const STREAM_TEXT_BUFFER_CHARS = 8_000; -// `detectRepetition` walks a character-level period search; cheap per call, +// `detectTailCharLoop` walks a character-level period search; cheap per call, // but the reactor loop can emit a delta per token, and running it on every // single one makes it the hottest thing in that loop for no benefit — a // repeating tail does not appear or disappear between two three-character @@ -108,7 +108,7 @@ export interface TurnState { * narrating a similar short line before each of several tool calls is * ordinary and must not accumulate into an apparent loop, whereas a * genuinely degenerate model repeats within one unbroken stream. Bounded to - * `STREAM_TEXT_BUFFER_CHARS`; feeds `detectRepetition`, nothing else. + * `STREAM_TEXT_BUFFER_CHARS`; feeds `detectTailCharLoop`, nothing else. */ readonly streamText: string; /** @@ -117,9 +117,9 @@ export interface TurnState { * throttle below tell "40 more chars arrived" from "the buffer is full." */ readonly streamCharsSeen: number; - /** `streamCharsSeen` as of the last `detectRepetition` call. */ + /** `streamCharsSeen` as of the last `detectTailCharLoop` call. */ readonly repetitionCheckedAt: number; - /** Result of the most recent `detectRepetition` check on `streamText`. */ + /** Result of the most recent `detectTailCharLoop` check on `streamText`. */ readonly repeating: boolean; /** * `streamTokenCount` at the moment repetition was first observed this turn. @@ -138,7 +138,7 @@ export interface TurnState { * Consecutive completed cycles whose fingerprint matched the one before it. * A model repeating the same block every cycle, with a tool call in * between each, builds this streak even though no single cycle's text ever - * gets long enough to trip `detectRepetition` on its own. + * gets long enough to trip `detectTailCharLoop` on its own. */ readonly consecutiveMatchingCycles: number; /** @@ -458,7 +458,7 @@ const streaming = ( // starts empty (see `runningTool`) and would otherwise read back false on // the next check, un-latching a real detection the moment a tool call // interrupts the stream. - const repeating = state.repeating || (due && detectRepetition(streamText).repeating); + const repeating = state.repeating || (due && detectTailCharLoop(streamText).repeating); return { ...state, status: state.status === "blocked" ? "blocked" : "running",