Skip to content

Commit b7f2e0f

Browse files
committed
Gate thinking repetition folding to short counter periods
1 parent 2286857 commit b7f2e0f

3 files changed

Lines changed: 61 additions & 7 deletions

File tree

docs/ARCHITECTURE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ Two directors, selected by role:
107107

108108
- **ChatDirector** (interactive, `src/agent/director.ts`) — Extends `DefaultDirector` with task list tracking, workflow nudges, LSP auto-activation, and multi-turn chat semantics. It never terminates the session: operator declines are surfaced as replies and the reactor stays alive for the next message. Auto mode is toggled by CLI flags (`--auto` / `--no-auto`); there is currently no in-session key to toggle it (default on; constrained envelope — workspace writes and unconstrained shell auto-allow; installs, recursive rm, force/uncontained worktree changes, sensitive-path and opaque-wrapper shell still ask; contained non-force `git worktree add`/`remove`/`prune` and `list` auto-allow; shell file-mutation denied). It is not a separate edit/plan mode.
109109
- **SubAgentDirector** (delegated work, `src/subagent/index.ts`) — Drives a dispatched worker until a turn arrives with no tool calls, then replies with the final assistant text and ends the run. A tool-less turn **after tools** completes only with the four-heading envelope (Summary, Findings, Blockers, Paths); a missing envelope nudges once then salvages as **incomplete-report**. A tool-less completion with **zero tool calls in the entire run** is returned as a **never-acted** salvage report (not a successful implement). When `task(intent="implement")` is set, a tool-using run that never wrote/edited/deleted a file is returned as **never-edited** instead of complete — so a pure-explore "plan" cannot look shipped to the parent (tracked via `thrashState.editedPaths` from `edit_file` / `write_file` / `delete_file`). Explore/read-only workers that used tools then replied with findings remain normal completes. Hard stops also fire after 2 consecutive identical tool-call fingerprints (**no-progress**), on progressive re-read pressure (**thrash** — the same path re-read past a limit amid enough tool volume, tracked by `src/subagent/thrash.ts`), or after the leaf turn budget (**turn-budget**, default 30, overridable via `task(maxTurns)`, agent profile `maxTurns`, or `settings.subagentMaxTurns`, capped at 100), each returning a structured salvage report (reason, partial findings, blockers) so a thrashing child cannot burn tokens indefinitely. Before hard thrash, a one-shot **re-read-nudge** fires when re-read pressure crosses a soft threshold (default 3 same-path reads with enough tool volume, still below the hard re-read limit of 4): the director injects an ephemeral redirect — implement leaves are asked to edit or wrap up; explore leaves are asked to expand findings / change approach / report, never forced into edit — then keeps running so hard thrash remains reachable if the leaf ignores it. A fourth hard stop, **repetition**, is detected outside the director entirely:
110-
`runSubAgent`'s stream sink watches the streamed text of the in-flight cycle for degenerate token loops (`src/subagent/repetition.ts`) — format chars (ZWSP, BOM, bidi marks, soft hyphen, …) stripped then whitespace-collapsed raw text, a smallest-period KMP check over the probe tail, default window >= 16 chars repeated >= 8 times, evaluated every 256 streamed chars — and on a hit aborts the run controller mid-cycle, returning a `repetition` salvage report that leads with the looped window and warns the parent against re-dispatching the identical brief. `inference.thinking.delta` is sampled the same way on its own buffer, but with digit runs folded to one placeholder and a shorter window (>= 4 chars repeated >= 32 times): thinking is never rendered to the user, so a monotonic counter (e.g. `0/1 1/2 2/3 …`, which stays non-periodic and escapes the raw-text check) can be caught without risking a false positive on a legitimate numbered list or table — that risk only applies to visible text, which keeps the digit-preserving check. Because directors only see completed turns, this is the only stop that can catch a loop inside a single turn that never finishes. A one-shot **report-forced** signal fires a few turns before the cap while the leaf is still tooling — it is not a stop: the director injects a wrap-up nudge and lets the leaf finish on its own, so turn-budget stays reachable for a leaf still making progress. When both report-forced and re-read-nudge apply, report-forced wins (near-budget wrap-up is more urgent than a mid-run redirect). Operator/parent cancel after any progress likewise returns a **cancelled** salvage report (partial findings + tool activity) instead of a bare cancel string; cancel before progress still surfaces as cancelled-by-operator.
110+
`runSubAgent`'s stream sink watches the streamed text of the in-flight cycle for degenerate token loops (`src/subagent/repetition.ts`) — format chars (ZWSP, BOM, bidi marks, soft hyphen, …) stripped then whitespace-collapsed raw text, a smallest-period KMP check over the probe tail, default window >= 16 chars repeated >= 8 times, evaluated every 256 streamed chars — and on a hit aborts the run controller mid-cycle, returning a `repetition` salvage report that leads with the looped window and warns the parent against re-dispatching the identical brief. `inference.thinking.delta` is sampled the same way on its own buffer, but with digit runs folded to one placeholder and a shorter window (>= 4 chars repeated >= 32 times), gated to periods <= 16 chars once folded: thinking is never rendered to the user, so a monotonic counter (e.g. `0/1 1/2 2/3 …`, which stays non-periodic and escapes the raw-text check) can be caught, but folding still erases real information — a healthy templated enumeration line becomes byte-identical to its neighbors once digits are erased, so the period-length cap only lets counter-shaped folded periods (a handful of chars) through and refuses the much longer periods a folded prose line produces. Because directors only see completed turns, this is the only stop that can catch a loop inside a single turn that never finishes. A one-shot **report-forced** signal fires a few turns before the cap while the leaf is still tooling — it is not a stop: the director injects a wrap-up nudge and lets the leaf finish on its own, so turn-budget stays reachable for a leaf still making progress. When both report-forced and re-read-nudge apply, report-forced wins (near-budget wrap-up is more urgent than a mid-run redirect). Operator/parent cancel after any progress likewise returns a **cancelled** salvage report (partial findings + tool activity) instead of a bare cancel string; cancel before progress still surfaces as cancelled-by-operator.
111111
Optional `task(tier=)` (`fast` | `standard` | `clever`) overrides profile inference, profile tier, and the parent provider for that spawn only, and fails closed when the tier is unconfigured. The parent `task` tool keeps a session-scoped brief-dispatch ledger (`src/subagent/brief-dispatch.ts`): fingerprints cover prompt + agent + intent + success_criteria + do_not (not maxTurns/description/tier). After thrash / no-progress / repetition / never-acted / never-edited salvage, an identical re-dispatch is hard-blocked for the rest of the parent chat; change at least one fingerprint field to force a re-run. Turn-budget salvage still invites a higher maxTurns for a few same-brief retries without a successful complete, then flips the parent hint to stop and change approach (soft — further identical dispatches are still admitted). A successful complete resets the same-brief retry budget.
112112

113113

src/subagent/repetition.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,37 @@ describe("detectRepetition", () => {
103103
).join("");
104104
expect(detectRepetition(`Migration progress:\n${items}`)).toBeNull();
105105
});
106+
107+
test("does not flag templated enumeration in thinking after digit folding", () => {
108+
// Regression: folding digits collapses a healthy templated line to a
109+
// byte-identical ~40+ char unit once its digits are erased. 200 lines
110+
// (~10KB) would trip windowMinChars 4 / repeatThreshold 32 without the
111+
// maxFoldedPeriodChars gate, aborting a healthy worker mid-reasoning.
112+
const items = Array.from(
113+
{ length: 200 },
114+
(_, i) => `${i + 1}. Ran batch ${i + 1} and verified ${i * 3} records migrated\n`,
115+
).join("");
116+
const hit = detectRepetition(items, DEFAULT_THINKING_REPETITION_CONFIG, {
117+
normalizeDigits: true,
118+
});
119+
expect(hit).toBeNull();
120+
});
121+
122+
test("still catches the monotonic counter with thousands of pairs", () => {
123+
const text = monotonicCounterStream(4000);
124+
const hit = detectRepetition(text, DEFAULT_THINKING_REPETITION_CONFIG, { normalizeDigits: true });
125+
expect(hit).not.toBeNull();
126+
expect(hit?.repeats).toBeGreaterThanOrEqual(DEFAULT_THINKING_REPETITION_CONFIG.repeatThreshold);
127+
});
128+
129+
test("a near-counter with a short prose wrapper still folds to a short period and trips", () => {
130+
// "step N/N done. " folds to "step 0/0 done. " — a 15-char period, still
131+
// within maxFoldedPeriodChars (16), so this shape is (deliberately) still
132+
// caught: it reads as a stalled step counter, not templated enumeration.
133+
const text = Array.from({ length: 100 }, (_, i) => `step ${i}/${i + 1} done. `).join("");
134+
const hit = detectRepetition(text, DEFAULT_THINKING_REPETITION_CONFIG, { normalizeDigits: true });
135+
expect(hit).not.toBeNull();
136+
});
106137
});
107138

108139
describe("repetition check accounting at the cycle-text cap", () => {

src/subagent/repetition.ts

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,17 @@ export type RepetitionConfig = {
1616
repeatThreshold: number;
1717
/** How much normalized tail text is examined per check. */
1818
probeChars: number;
19+
/**
20+
* Largest normalized window (chars) the digit-folded path may fire on.
21+
* Only meaningful with opts.normalizeDigits — folding digit runs to one
22+
* placeholder can turn a healthy templated enumeration line into a
23+
* byte-identical period once its digits are erased. A true oscillating- or
24+
* monotonic-counter loop folds to a tiny period (a few chars); a templated
25+
* prose line folds to a much longer one. Capping the folded period length
26+
* lets the short, counter-shaped periods through while refusing to fire on
27+
* the long, prose-shaped ones. Ignored when normalizeDigits is false.
28+
*/
29+
maxFoldedPeriodChars?: number;
1930
};
2031

2132
// windowMinChars * repeatThreshold = 128 chars of exactly periodic text —
@@ -33,16 +44,25 @@ export const DEFAULT_REPETITION_CONFIG: RepetitionConfig = {
3344
export const REPETITION_CHECK_INTERVAL_CHARS = 256;
3445

3546
// Thinking streams are never shown to the user, so unlike text (see
36-
// normalize() below) they can safely fold digit runs into one placeholder —
37-
// there is no numbered-list or table false-positive risk to protect against.
38-
// That normalization collapses a monotonic counter ("0/1 1/2 2/3 …") to a
39-
// short constant-length unit ("0/0 0/0 0/0 …"), so the window threshold can
40-
// drop accordingly while the repeat threshold stays high enough that a loop
41-
// still needs a long sustained run to trip.
47+
// normalize() below) they can fold digit runs into one placeholder without
48+
// risking a numbered-list or table rendering complaint. But folding still
49+
// erases real information: a healthy templated enumeration line (a worker
50+
// narrating "N. Ran batch N and verified N*3 records migrated" once per
51+
// iteration) is only distinct because of its digits, so once folded, many
52+
// such lines become one repeating ~40+ char unit and look exactly like a
53+
// loop. The discriminator that keeps that safe is period length: a true
54+
// oscillating- or monotonic-counter loop ("0/1 1/2 2/3 …") folds to a tiny
55+
// period (a handful of chars — the counter digits and their separators),
56+
// while a templated prose line folds to a much longer one (the surrounding
57+
// sentence survives folding intact). maxFoldedPeriodChars caps the folded
58+
// path to short periods so it only ever catches counter-shaped loops, never
59+
// prose-shaped enumeration; the repeat threshold on top of that still
60+
// requires a long sustained run before it trips.
4261
export const DEFAULT_THINKING_REPETITION_CONFIG: RepetitionConfig = {
4362
windowMinChars: 4,
4463
repeatThreshold: 32,
4564
probeChars: 8192,
65+
maxFoldedPeriodChars: 16,
4666
};
4767

4868
export type RepetitionHit = {
@@ -109,6 +129,9 @@ export function detectRepetition(
109129
const suffixLen = i + 1;
110130
const period = suffixLen - (pi[i] ?? 0);
111131
if (period < config.windowMinChars) continue;
132+
if (opts.normalizeDigits && config.maxFoldedPeriodChars !== undefined) {
133+
if (period > config.maxFoldedPeriodChars) continue;
134+
}
112135
if (suffixLen < period * config.repeatThreshold) continue;
113136
const repeats = Math.floor(suffixLen / period);
114137
if (best === null || repeats > best.repeats) {

0 commit comments

Comments
 (0)