Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 100 additions & 1 deletion src/summarization/__tests__/trigger.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import { shouldTriggerSummarization } from '@/summarization';
import {
shouldTriggerSummarization,
_resetUnrecognizedTriggerWarnings,
} from '@/summarization';

describe('shouldTriggerSummarization', () => {
it('uses pre-prune pressure for token_ratio triggers when messages were pruned', () => {
Expand Down Expand Up @@ -47,4 +50,100 @@ describe('shouldTriggerSummarization', () => {

expect(result).toBe(false);
});

describe('unrecognized trigger type', () => {
let warnSpy: jest.SpyInstance;

beforeEach(() => {
_resetUnrecognizedTriggerWarnings();
warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => undefined);
});

afterEach(() => {
warnSpy.mockRestore();
});

it('does not fire and warns once per unrecognized type', () => {
const baseParams = {
maxContextTokens: 2500,
prePruneContextTokens: 2400,
remainingContextTokens: 100,
messagesToRefineCount: 4,
};

// Cast via `unknown` because the type union guards against this at compile
// time; we are intentionally exercising the runtime fallback.
const result1 = shouldTriggerSummarization({
...baseParams,
trigger: { type: 'token_count', value: 8000 } as unknown as {
type: 'token_ratio';
value: number;
},
});

expect(result1).toBe(false);
expect(warnSpy).toHaveBeenCalledTimes(1);
expect(warnSpy.mock.calls[0][0]).toContain('token_count');
expect(warnSpy.mock.calls[0][0]).toContain('token_ratio');
expect(warnSpy.mock.calls[0][0]).toContain('remaining_tokens');
expect(warnSpy.mock.calls[0][0]).toContain('messages_to_refine');

// Same unrecognized type a second time: no duplicate warning.
shouldTriggerSummarization({
...baseParams,
trigger: { type: 'token_count', value: 8000 } as unknown as {
type: 'token_ratio';
value: number;
},
});
expect(warnSpy).toHaveBeenCalledTimes(1);

// Different unrecognized type: warns again, once.
shouldTriggerSummarization({
...baseParams,
trigger: { type: 'nonsense', value: 1 } as unknown as {
type: 'token_ratio';
value: number;
},
});
expect(warnSpy).toHaveBeenCalledTimes(2);
expect(warnSpy.mock.calls[1][0]).toContain('nonsense');
});

it('does not grow memory unboundedly under a flood of unique types', () => {
const baseParams = {
maxContextTokens: 2500,
prePruneContextTokens: 2400,
remainingContextTokens: 100,
messagesToRefineCount: 4,
};

for (let i = 0; i < 500; i++) {
shouldTriggerSummarization({
...baseParams,
trigger: { type: `bogus-${i}`, value: 1 } as unknown as {
type: 'token_ratio';
value: number;
},
});
}

// Still logged each new type (up to the cap) β€” we never silently dropped
// warnings; we just evicted oldest entries from the dedup set.
expect(warnSpy).toHaveBeenCalledTimes(500);

// Re-warns for a recently-seen type that should still be in the cache
// (last one just inserted). No duplicate warning means the dedup set
// still functions; the size cap did not break the dedup contract.
const beforeRecent = warnSpy.mock.calls.length;
shouldTriggerSummarization({
...baseParams,
trigger: { type: 'bogus-499', value: 1 } as unknown as {
type: 'token_ratio';
value: number;
},
});
expect(warnSpy).toHaveBeenCalledTimes(beforeRecent);
});
});
});
47 changes: 47 additions & 0 deletions src/summarization/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,51 @@
import type { SummarizationTrigger } from '@/types';

const VALID_TRIGGER_TYPES = [
'token_ratio',
'remaining_tokens',
'messages_to_refine',
] as const;

/**
* Upper bound on the dedup set for unrecognized trigger types. Bounds memory in
* case a caller threads dynamic/user-provided strings through `trigger.type`.
* Well above the handful of legit misconfigurations a process would ever see.
*/
const MAX_WARNED_TRIGGER_TYPES = 32;

const warnedUnrecognizedTriggerTypes = new Set<string>();

/**
* Warn (once per process, per unrecognized type) when the configured trigger
* type is something the runtime does not evaluate. Without this, a misconfigured
* `trigger.type` silently disables summarization with no visible signal.
*
* The dedup set is size-capped; on overflow we evict the oldest entry (Set
* preserves insertion order) so we keep bounded memory and still warn on
* recently-seen types.
*/
function warnUnrecognizedTriggerType(type: string): void {
if (warnedUnrecognizedTriggerTypes.has(type)) {
return;
}
if (warnedUnrecognizedTriggerTypes.size >= MAX_WARNED_TRIGGER_TYPES) {
const oldest = warnedUnrecognizedTriggerTypes.values().next().value;
if (oldest !== undefined) {
warnedUnrecognizedTriggerTypes.delete(oldest);
}
}
warnedUnrecognizedTriggerTypes.add(type);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Cap unknown-trigger cache growth

warnedUnrecognizedTriggerTypes is process-global and every previously unseen trigger.type is retained forever via warnedUnrecognizedTriggerTypes.add(type). Because SummarizationTrigger.type accepts arbitrary strings and this path runs on normal request flow, deployments that ingest dynamic or user-provided agent configs can accumulate unbounded unique keys over time, creating a memory-growth vector in long-lived workers. Consider bounding this cache (size cap/LRU) or using a time-based eviction policy.

Useful? React with πŸ‘Β / πŸ‘Ž.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in d60ec1e β€” the dedup set is now capped at 32 entries with FIFO eviction of the oldest key on overflow (Set preserves insertion order). Added a regression test that floods the function with 500 unique unrecognized types and verifies the dedup contract still holds.

console.warn(
`[shouldTriggerSummarization] Unrecognized trigger.type: "${type}". ` +
`Summarization will not fire. Valid values: ${VALID_TRIGGER_TYPES.join(', ')}.`
);
}

/** For tests only. Resets the dedup set so warnings can be observed again. */
export function _resetUnrecognizedTriggerWarnings(): void {
warnedUnrecognizedTriggerTypes.clear();
}

/**
* Determines whether summarization should be triggered based on the configured trigger
* and current context state.
Expand Down Expand Up @@ -98,5 +144,6 @@ export function shouldTriggerSummarization(params: {
}

// Unrecognized trigger type: cannot evaluate, do not fire.
warnUnrecognizedTriggerType(trigger.type);
return false;
}
Loading