Skip to content

Commit 0f1ae2f

Browse files
committed
Detect repetition by character period, not by line
Line-splitting missed the captured incident outright: the looping sentences ran together with no newline between them, so the whole span collapsed into one line and never reached the occurrence check. Detection now finds the smallest period the streamed tail exactly repeats, which catches the no-newline shape the same way it catches a line-level loop. Line counting also flagged ordinary structure — a repeated markdown table row, a few identical code lines — as degeneration. The period search requires more repeats and enough character variety in the repeating unit to rule those out, verified against both plus a monochrome run (a repeated rule or the same character streamed many times), which is trivially periodic at every length and would otherwise be the easiest false trigger of all. The per-delta check is now throttled to run once per chunk of new text rather than once per token, since a repeating tail cannot appear or disappear between two three-character tokens. The idle notice also now checks the repetition flag directly, so a stream that is looping but not silent can no longer be labeled a silent hang.
1 parent 893336b commit 0f1ae2f

6 files changed

Lines changed: 197 additions & 67 deletions

File tree

src/tui-opentui/runtime-bridge.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -924,6 +924,14 @@ export function attachSessionBridge(
924924
// Content-based, not time-based: a repeating line means the model is
925925
// stuck regardless of how fast it is producing it, so this is checked
926926
// before the silence clock rather than folded into it.
927+
//
928+
// Gated on `status === "running"` because every turn-ending transition
929+
// (interrupt, connector.reply with no tools outstanding, reactor.done /
930+
// reactor.error) routes through `initialTurnState`, which clears
931+
// `repeating`. If a future settle path changes `isProcessing` without
932+
// also resetting `status` and `repeating` through that same reset, this
933+
// guard would no longer mean "the turn is actually live" and could fire
934+
// on an already-settled turn — recheck this alongside any such change.
927935
if (bag.turn.status === "running" && bag.turn.repeating) {
928936
const repeatedTokens =
929937
bag.turn.streamTokenCount - (bag.turn.repeatingSinceTokenCount ?? 0)
@@ -954,7 +962,13 @@ export function attachSessionBridge(
954962

955963
// Notice only — the phase still paints below, because a ramp that stops
956964
// moving is the very thing that reads as a hang.
957-
if (shouldNoticeStall({ ...stallArgs, stallNoticeMs })) {
965+
if (
966+
shouldNoticeStall({
967+
...stallArgs,
968+
stallNoticeMs,
969+
repeating: bag.turn.repeating,
970+
})
971+
) {
958972
setStatusFlash(shell, STALL_NOTICE_MESSAGE)
959973
}
960974

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

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -112,26 +112,54 @@ describe("detectRepetition", () => {
112112
expect(detectRepetition(text).repeating).toBe(false)
113113
})
114114

115-
test("flags a line repeated past the occurrence threshold", () => {
115+
// The captured incident: the two sentences ran together with no line break
116+
// at all. A line-splitting detector never sees this; the period search
117+
// does not care where (or whether) the lines break.
118+
test("flags the captured incident string verbatim, with no newlines", () => {
116119
const line1 =
117120
"I'll verify callId emission and remaining edges, then write the ranked findings."
118121
const line2 = "Confirming callId emission, then writing the ranked findings."
119-
const text = Array(4).fill([line1, line2]).flat().join("\n")
122+
const text = Array(10).fill(`${line1}${line2}`).join("")
120123
const check = detectRepetition(text)
121124
expect(check.repeating).toBe(true)
122-
expect(check.repeatedLine).toBe(line1)
123-
expect(check.occurrences).toBeGreaterThanOrEqual(3)
125+
expect(check.period).toBe(line1.length + line2.length)
124126
})
125127

126-
test("ignores short recurring lines", () => {
127-
const text = Array(6).fill("Checking...").join("\n")
128+
test("does not flag the same cycle a handful of times", () => {
129+
const line1 =
130+
"I'll verify callId emission and remaining edges, then write the ranked findings."
131+
const line2 = "Confirming callId emission, then writing the ranked findings."
132+
// Fewer than the occurrence threshold: a model can legitimately restate
133+
// a step once or twice across tool-call cycles without looping.
134+
const text = Array(4).fill(`${line1}${line2}`).join("")
128135
expect(detectRepetition(text).repeating).toBe(false)
129136
})
130137

131-
test("does not flag two occurrences", () => {
132-
const line =
133-
"I'll verify callId emission and remaining edges, then write the ranked findings."
134-
const text = [line, "some other progress here.", line].join("\n")
138+
test("does not flag a repeated markdown table separator row", () => {
139+
const row = "| ---------------------- | ---------------------- |"
140+
const text = Array(6).fill(row).join("\n")
141+
expect(detectRepetition(text).repeating).toBe(false)
142+
})
143+
144+
test("does not flag a few identical code lines", () => {
145+
const line = " const result = await fetchData(request, options, context)"
146+
const text = Array(3).fill(line).join("\n")
147+
expect(detectRepetition(text).repeating).toBe(false)
148+
})
149+
150+
test("ignores short recurring fragments", () => {
151+
const text = Array(10).fill("ok").join(" ")
152+
expect(detectRepetition(text).repeating).toBe(false)
153+
})
154+
155+
// A monochrome run is periodic at every period by construction — the
156+
// easiest thing to false-trigger on if entropy is not checked.
157+
test("does not flag a long run of the same character", () => {
158+
expect(detectRepetition("x".repeat(500)).repeating).toBe(false)
159+
})
160+
161+
test("does not flag a repeated horizontal rule", () => {
162+
const text = Array(10).fill("----------------------------").join("\n")
135163
expect(detectRepetition(text).repeating).toBe(false)
136164
})
137165
})
@@ -154,8 +182,13 @@ describe("shouldNoticeStall", () => {
154182
stallNoticeMs: STALL_NOTICE_MS,
155183
isProcessing: true,
156184
streamingType: null,
185+
repeating: false,
157186
}
158187

188+
test("stays quiet while repeating, even if also silent by the clock", () => {
189+
expect(shouldNoticeStall({ ...base, repeating: true })).toBe(false)
190+
})
191+
159192
test("speaks up long before the abort backstop", () => {
160193
expect(STALL_NOTICE_MS).toBeLessThan(STALL_TIMEOUT_MS)
161194
expect(shouldNoticeStall(base)).toBe(true)

src/tui-opentui/stall-watchdog.ts

Lines changed: 78 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -21,41 +21,88 @@ export type ShouldAbortForStallArgs = {
2121
readonly streamingType: "text" | "thinking" | "tool" | null
2222
}
2323

24-
// A repeated line has to be long enough that short, legitimately-recurring
25-
// fragments ("Let me check.") do not trip the guard.
26-
const REPETITION_MIN_LINE_LENGTH = 20
27-
// How far back into the streamed text to look for repeats. Bounded so the
28-
// check stays cheap however long the turn runs.
29-
const REPETITION_LOOKBACK_LINES = 12
30-
// Three verbatim repeats of the same substantial line is not a coincidence
31-
// of phrasing — it is the model looping.
32-
const REPETITION_MIN_OCCURRENCES = 3
24+
// The captured incident looped two sentences with no line break between them
25+
// ("...ranked findings.Confirming callId emission...") — degeneration is a
26+
// character-level loop, not a line-level one. Splitting on "\n" misses it
27+
// entirely, so the tail is treated as a plain string and checked for the
28+
// smallest period it exactly repeats: the shortest span p such that the last
29+
// several hundred characters equal p repeated.
30+
//
31+
// A period below this is more likely a short structural tic (indentation, a
32+
// repeated bullet or table-cell divider) than a looping phrase. Chosen well
33+
// under the ~140-char period of the captured incident's two-sentence cycle,
34+
// with headroom for shorter degenerate loops (a single repeated sentence).
35+
const REPETITION_MIN_PERIOD = 24
36+
// How many exact repeats of the period are required before it counts as a
37+
// loop rather than a coincidence. Verified against real non-degenerate
38+
// repetition: a 6-row markdown table separator (period ~51 chars, 6 exact
39+
// repeats) and 3 identical code lines (period ~60 chars, 3 exact repeats)
40+
// both land under this bar and are not flagged; the captured incident's
41+
// sentence pair comfortably clears it well before the stream ends.
42+
const REPETITION_MIN_REPEATS = 8
43+
// Hard ceiling on the period search regardless of buffer size, purely to cap
44+
// worst-case work per check — token-level degeneration loops on a phrase or
45+
// two, never on multi-paragraph spans.
46+
const REPETITION_MAX_PERIOD_CAP = 2_000
47+
// A monochrome run ("x".repeat(500), a "----" rule, a wall of spaces) is
48+
// trivially periodic at *every* period, which would otherwise make it the
49+
// single easiest thing to false-trigger on — verified by execution against
50+
// `thinking-reveal.test.ts`'s burst-of-"x" fixture, which tripped the guard
51+
// before this floor existed. Requiring the repeating unit itself to contain
52+
// this many distinct characters keeps single-character and low-variety runs
53+
// out without weakening the sentence-level case: the captured incident's
54+
// cycle spans two full sentences, comfortably above it.
55+
const REPETITION_MIN_DISTINCT_CHARS = 8
3356

3457
export type RepetitionCheck = {
3558
readonly repeating: boolean
36-
readonly repeatedLine: string | null
37-
readonly occurrences: number
59+
readonly period: number | null
60+
readonly repeats: number
3861
}
3962

4063
/**
41-
* Whether the tail of the streamed text is dominated by a line repeated
42-
* verbatim. Pure text-in, decision-out: the caller owns accumulating the
43-
* buffer across deltas and cycles within a turn.
64+
* Length of the exact-period run ending at the last character of `text`,
65+
* including the base period itself. `text[i] === text[i - period]` walked
66+
* backwards from the end; stops at the first mismatch or the start of the
67+
* string.
68+
*/
69+
function periodicSuffixLength(text: string, period: number): number {
70+
let i = text.length - 1
71+
let j = i - period
72+
let matched = 0
73+
while (j >= 0 && text[i] === text[j]) {
74+
matched++
75+
i--
76+
j--
77+
}
78+
return matched + period
79+
}
80+
81+
/**
82+
* Whether the tail of `text` is an exact repeat of some short span at least
83+
* `REPETITION_MIN_REPEATS` times. Pure text-in, decision-out: the caller owns
84+
* accumulating the buffer across deltas and cycles within a turn.
85+
*
86+
* Periods longer than `text.length / REPETITION_MIN_REPEATS` are skipped, not
87+
* as an arbitrary cutoff but because they cannot mathematically reach the
88+
* occurrence threshold within the given text — a loop with a longer period
89+
* needs a longer buffer to confirm, which is a buffer-size trade-off owned by
90+
* the caller, not a second detection path here.
4491
*/
4592
export function detectRepetition(text: string): RepetitionCheck {
46-
const lines = text
47-
.split("\n")
48-
.map((line) => line.trim())
49-
.filter((line) => line.length >= REPETITION_MIN_LINE_LENGTH)
50-
const tail = lines.slice(-REPETITION_LOOKBACK_LINES)
51-
const counts = new Map<string, number>()
52-
for (const line of tail) counts.set(line, (counts.get(line) ?? 0) + 1)
53-
for (const [line, occurrences] of counts) {
54-
if (occurrences >= REPETITION_MIN_OCCURRENCES) {
55-
return { repeating: true, repeatedLine: line, occurrences }
56-
}
93+
const maxPeriod = Math.min(
94+
REPETITION_MAX_PERIOD_CAP,
95+
Math.floor(text.length / REPETITION_MIN_REPEATS),
96+
)
97+
for (let period = REPETITION_MIN_PERIOD; period <= maxPeriod; period++) {
98+
const matched = periodicSuffixLength(text, period)
99+
const repeats = matched / period
100+
if (repeats < REPETITION_MIN_REPEATS) continue
101+
const unit = text.slice(text.length - period)
102+
if (new Set(unit).size < REPETITION_MIN_DISTINCT_CHARS) continue
103+
return { repeating: true, period, repeats }
57104
}
58-
return { repeating: false, repeatedLine: null, occurrences: 0 }
105+
return { repeating: false, period: null, repeats: 0 }
59106
}
60107

61108
/**
@@ -87,14 +134,18 @@ export function shouldAbortForStall(args: ShouldAbortForStallArgs): boolean {
87134

88135
export type ShouldNoticeStallArgs = ShouldAbortForStallArgs & {
89136
readonly stallNoticeMs: number
137+
/** Whether the repetition guard currently sees a looping tail. */
138+
readonly repeating: boolean
90139
}
91140

92141
/**
93142
* Returns true while the run has been silent long enough to say so but not yet
94143
* long enough to abort. False once the abort takes over, so the two never
95-
* paint at the same time.
144+
* paint at the same time, and false while repeating — that run is producing
145+
* output, just not useful output, and "no response" would misdescribe it.
96146
*/
97147
export function shouldNoticeStall(args: ShouldNoticeStallArgs): boolean {
148+
if (args.repeating) return false
98149
if (shouldAbortForStall(args)) return false
99150
return silentPastThreshold(args, args.stallNoticeMs)
100151
}

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -377,15 +377,18 @@ describe("repetition guard", () => {
377377
t.bridge.submit("build it", "immediate")
378378
t.port.clear()
379379

380+
// The captured incident shape: the two sentences run together with
381+
// no line break between cycles.
380382
const line1 =
381-
"I'll verify callId emission and remaining edges, then write the ranked findings.\n"
382-
const line2 = "Confirming callId emission, then writing the ranked findings.\n"
383+
"I'll verify callId emission and remaining edges, then write the ranked findings."
384+
const line2 = "Confirming callId emission, then writing the ranked findings."
385+
const cycle = `${line1}${line2}`
383386

384387
// Tokens keep landing every tick — a real stall would never fire here.
385-
for (let i = 0; i < 6; i++) {
388+
for (let i = 0; i < 10; i++) {
386389
t.bridge.handle({
387390
type: "inference.text.delta",
388-
data: { token: i % 2 === 0 ? line1 : line2 },
391+
data: { token: cycle },
389392
})
390393
t.advance(10)
391394
t.tick()

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

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -193,8 +193,10 @@ describe("turn transitions", () => {
193193
describe("repetition tracking", () => {
194194
const line1 =
195195
"I'll verify callId emission and remaining edges, then write the ranked findings."
196-
const line2 =
197-
"Confirming callId emission, then writing the ranked findings."
196+
const line2 = "Confirming callId emission, then writing the ranked findings."
197+
// The captured incident shape: the two sentences run together with no
198+
// separator, each delta landing as one full cycle.
199+
const cycle = `${line1}${line2}`
198200

199201
const textDelta = (text: string) => ({
200202
type: "inference.text.delta",
@@ -212,23 +214,27 @@ describe("repetition tracking", () => {
212214
expect(s.repeatingSinceTokenCount).toBeNull()
213215
})
214216

215-
test("a line looping past the threshold flips repeating and latches the token count", () => {
216-
const deltas = Array(4)
217-
.fill([line1, line2])
218-
.flat()
219-
.map((line) => textDelta(`${line}\n`))
217+
test("the captured incident shape (no separator between cycles) flips repeating", () => {
218+
const deltas = Array(10)
219+
.fill(cycle)
220+
.map((text) => textDelta(text))
220221
const s = fold([{ type: "inference.start" }, ...deltas])
221222
expect(s.repeating).toBe(true)
222-
// Three text deltas land before the third `line1` repeat crosses the
223-
// occurrence threshold and latches the count.
224-
expect(s.repeatingSinceTokenCount).toBe(5)
223+
expect(s.repeatingSinceTokenCount).not.toBeNull()
224+
})
225+
226+
test("a couple of restated cycles across tool calls is not a loop", () => {
227+
const deltas = Array(3)
228+
.fill(cycle)
229+
.map((text) => textDelta(text))
230+
const s = fold([{ type: "inference.start" }, ...deltas])
231+
expect(s.repeating).toBe(false)
225232
})
226233

227234
test("repetition tracked across a tool cycle survives connector.reply with tools outstanding", () => {
228-
const deltas = Array(4)
229-
.fill([line1, line2])
230-
.flat()
231-
.map((line) => textDelta(`${line}\n`))
235+
const deltas = Array(10)
236+
.fill(cycle)
237+
.map((text) => textDelta(text))
232238
const withTool = turnStateFromEvent(
233239
fold([{ type: "inference.start" }, ...deltas]),
234240
{ type: "tool.start", data: { call: { id: "c1", name: "grep" } } },
@@ -244,10 +250,9 @@ describe("repetition tracking", () => {
244250
})
245251

246252
test("a fresh submit clears the repetition state", () => {
247-
const deltas = Array(4)
248-
.fill([line1, line2])
249-
.flat()
250-
.map((line) => textDelta(`${line}\n`))
253+
const deltas = Array(10)
254+
.fill(cycle)
255+
.map((text) => textDelta(text))
251256
const looping = fold([{ type: "inference.start" }, ...deltas])
252257
const restarted = turnStateOnSubmit(looping, 200)
253258
expect(restarted.repeating).toBe(false)

0 commit comments

Comments
 (0)