fix(ai-chat): сброс lastDegenerationCheckLen в onStepFinish (#486)
onStepFinish обнулял inProgressText, но НЕ lastDegenerationCheckLen — а это смещение В аккумулятор. После первого длинного шага протухшая (большая) отметка делала throttle-условие отрицательным, и детектор токен-лупов молчал весь следующий шаг, пока текст не перерастал старую отметку. Обнуляем отметку в onStepFinish. Throttle-предикат вынесен в output-degeneration (shouldCheckDegeneration + DEGENERATION_CHECK_STEP), чтобы юнит гонял ту же логику, что и стрим. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -56,6 +56,7 @@ import {
|
||||
import {
|
||||
isDegenerateOutput,
|
||||
truncateDegeneratedTail,
|
||||
shouldCheckDegeneration,
|
||||
} from './output-degeneration';
|
||||
|
||||
// Max agent steps per turn. One step = one model generation; a step that calls
|
||||
@@ -1101,7 +1102,6 @@ export class AiChatService implements OnModuleInit {
|
||||
const degenerationController = new AbortController();
|
||||
let degenerationDetected = false;
|
||||
let lastDegenerationCheckLen = 0;
|
||||
const DEGENERATION_CHECK_STEP = 2000;
|
||||
|
||||
// Step-granular durability (#183): create the assistant row UPFRONT in the
|
||||
// 'streaming' state (before any token), then UPDATE it as each step finishes
|
||||
@@ -1275,8 +1275,10 @@ export class AiChatService implements OnModuleInit {
|
||||
// trigger, abort the run ONCE with a distinguishable reason.
|
||||
if (
|
||||
!degenerationDetected &&
|
||||
inProgressText.length - lastDegenerationCheckLen >=
|
||||
DEGENERATION_CHECK_STEP
|
||||
shouldCheckDegeneration(
|
||||
inProgressText.length,
|
||||
lastDegenerationCheckLen,
|
||||
)
|
||||
) {
|
||||
lastDegenerationCheckLen = inProgressText.length;
|
||||
if (isDegenerateOutput(inProgressText)) {
|
||||
@@ -1296,6 +1298,13 @@ export class AiChatService implements OnModuleInit {
|
||||
// the in-progress accumulator for the next step.
|
||||
capturedSteps.push(step as StepLike);
|
||||
inProgressText = '';
|
||||
// Reset the degeneration-check watermark too (#486): it tracks a byte
|
||||
// offset INTO inProgressText, so once that resets to '' a stale (large)
|
||||
// mark makes `inProgressText.length - lastDegenerationCheckLen` go
|
||||
// negative and the throttled detector stays silent until a later step's
|
||||
// text re-grows past the old offset — a whole degenerate step could slip
|
||||
// through undetected. Zeroing it re-arms the check from the next byte.
|
||||
lastDegenerationCheckLen = 0;
|
||||
// Step-granular durability (#183): persist this finished step (its text +
|
||||
// tool calls + tool RESULTS) the moment it ends, so a process death after
|
||||
// this point still recovers the step. Not awaited here (never block the
|
||||
|
||||
@@ -3,6 +3,8 @@ import {
|
||||
hasPeriodicTail,
|
||||
isDegenerateOutput,
|
||||
truncateDegeneratedTail,
|
||||
shouldCheckDegeneration,
|
||||
DEGENERATION_CHECK_STEP,
|
||||
REPEATED_LINES_THRESHOLD,
|
||||
MIN_PERIOD_REPEATS,
|
||||
} from './output-degeneration';
|
||||
@@ -180,3 +182,42 @@ describe('truncateDegeneratedTail', () => {
|
||||
expect(truncateDegeneratedTail(text)).toBe(text);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Throttle + step-boundary reset (#486). The stream keeps a watermark
|
||||
* (`lastDegenerationCheckLen`) that is an OFFSET into the accumulated step text.
|
||||
* On a step boundary the accumulator resets to '', so the watermark MUST reset to
|
||||
* 0 too — otherwise the throttle goes silent for the whole next step. These tests
|
||||
* pin the pure decision AND the reset property that ai-chat.service.onStepFinish
|
||||
* now enforces.
|
||||
*/
|
||||
describe('shouldCheckDegeneration (throttle) + step-boundary reset (#486)', () => {
|
||||
it('fires once the text grows a full DEGENERATION_CHECK_STEP past the mark', () => {
|
||||
expect(shouldCheckDegeneration(DEGENERATION_CHECK_STEP, 0)).toBe(true);
|
||||
expect(shouldCheckDegeneration(DEGENERATION_CHECK_STEP - 1, 0)).toBe(false);
|
||||
expect(shouldCheckDegeneration(5000, 3000)).toBe(true); // grew 2000 since mark
|
||||
expect(shouldCheckDegeneration(4000, 3000)).toBe(false); // grew only 1000
|
||||
});
|
||||
|
||||
it('BUG (no reset): a stale large watermark silences the next step', () => {
|
||||
// End of a long step: the watermark sits at 5000. The step ends and the
|
||||
// accumulator resets to '' — but if the watermark is NOT reset, a fresh short
|
||||
// degenerate burst (length 2000) never triggers a check: 2000 - 5000 < STEP.
|
||||
const staleWatermark = 5000;
|
||||
const nextStepLen = DEGENERATION_CHECK_STEP; // a fresh 2KB burst
|
||||
expect(shouldCheckDegeneration(nextStepLen, staleWatermark)).toBe(false);
|
||||
});
|
||||
|
||||
it('FIX (reset to 0): the same short degenerate burst IS checked and detected', () => {
|
||||
// onStepFinish now zeroes the watermark, so the fresh burst re-arms the check.
|
||||
const resetWatermark = 0;
|
||||
const degenerateBurst = 'loadTools.\n'.repeat(300); // real degeneration
|
||||
expect(degenerateBurst.length).toBeGreaterThanOrEqual(DEGENERATION_CHECK_STEP);
|
||||
// The throttle now fires...
|
||||
expect(
|
||||
shouldCheckDegeneration(degenerateBurst.length, resetWatermark),
|
||||
).toBe(true);
|
||||
// ...and the detector catches the loop that would otherwise stream unchecked.
|
||||
expect(isDegenerateOutput(degenerateBurst)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -131,6 +131,32 @@ export function isDegenerateOutput(text: string): boolean {
|
||||
return hasRepeatedLineRun(text) || hasPeriodicTail(text);
|
||||
}
|
||||
|
||||
/**
|
||||
* How many bytes the in-progress text must grow before the (amortized) tail
|
||||
* heuristics are re-run. Shared with ai-chat.service so the throttle the stream
|
||||
* applies is the SAME one the unit test drives.
|
||||
*/
|
||||
export const DEGENERATION_CHECK_STEP = 2000;
|
||||
|
||||
/**
|
||||
* Throttle decision for the degeneration guard (#444/#486). Returns true when
|
||||
* the accumulated text has grown at least DEGENERATION_CHECK_STEP bytes past the
|
||||
* last-checked offset, so the pure rules only fire every ~2KB. Pure; the caller
|
||||
* updates its watermark to `textLen` when this returns true.
|
||||
*
|
||||
* The watermark is an offset INTO the accumulator, so when the accumulator is
|
||||
* reset to '' on a step boundary the caller MUST reset the watermark to 0 too
|
||||
* (#486). Otherwise `textLen - lastCheckLen` goes negative after the reset and
|
||||
* this returns false until a later step re-grows past the stale offset — a whole
|
||||
* degenerate step could stream unchecked.
|
||||
*/
|
||||
export function shouldCheckDegeneration(
|
||||
textLen: number,
|
||||
lastCheckLen: number,
|
||||
): boolean {
|
||||
return textLen - lastCheckLen >= DEGENERATION_CHECK_STEP;
|
||||
}
|
||||
|
||||
/**
|
||||
* Truncate a degenerated tail before persist so hundreds of KB of garbage never
|
||||
* reach the DB / replay (#444). Keeps everything up to and including the FIRST
|
||||
|
||||
Reference in New Issue
Block a user