Merge pull request #562 from corbitsdev/cl-6939-consolidate-the-duplicate-repetition-detector

Consolidate stall-watchdog's repetition detector into subagent/repetition.ts

Author: TheGreatAxios

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,
+  });
+}

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);
   });
 });
 

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

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",