Skip to content

Commit 9cbc333

Browse files
neo-opus-adatobiu
andauthored
fix(ai): stabilize backpressure deferral dedup key against volatile counters (#14156) (#14157)
* fix(ai): stabilize backpressure deferral dedup key against volatile counters (#14156) The backpressure deferral log floods (~8% of orchestrator-log lines): recordDeferral dedups via deferralLogKeys, but the dedup key embeds the full reasonText, and memorySummaryBackfill emits 'pending-memory-minisummary:${backlog}' whose count changes every poll — so the key churns and the dedup never fires while kbSync holds the heavy-maintenance lease for hours. Strip a trailing ':<digits>' from the dedup key only; the full reasonText (with count) stays in the log message + the recordTaskOutcome payload. Recurring deferrals now collapse to one log per episode (clearDeferralLogState still re-logs a new episode). No change to deferral/lease behavior. Unit: 4 changing-counter polls -> 1 log, post-clear -> re-logs; 28/28 specs pass. * refactor(ai): key deferral dedup on stable (task, blocker, reasonCode), not regex-stripped reasonText (#14156) Follow-up to the #14157 depth-floor point (broad trailing-number normalization also collapsed constant interval-style reasons like periodic-sync:1800000). Drop the volatile reasonText from the dedup key entirely and key on the deferred task + blocker + reasonCode — stable, declared identifiers. The deferred task's source-class is already implied by taskName and the count is volatile noise, so the key gains no distinction from reasonText. Full reasonText stays in the log message + the recordTaskOutcome payload. 28/28 specs pass. --------- Co-authored-by: tobiu <tobiasuhlig78@gmail.com>
1 parent b03aa1e commit 9cbc333

2 files changed

Lines changed: 48 additions & 3 deletions

File tree

ai/daemons/orchestrator/services/MaintenanceBackpressureService.mjs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -237,9 +237,14 @@ export function recordDeferral({
237237
}) {
238238
const isLeaseHeld = reasonCode === 'heavy-maintenance-lease-held';
239239
const holderOwner = holdingLease?.owner || 'unknown';
240-
const dedupKey = isLeaseHeld
241-
? `${taskName}:lease-held-by-${holderOwner}:${reasonText}`
242-
: `${taskName}:${blockingTaskName}:${reasonText}`;
240+
// Dedup on STABLE identifiers only — the deferred task, its blocker, and the deferral reasonCode —
241+
// never the volatile `reasonText`: its `:<count>` / `:<interval>` suffix changes every poll and its
242+
// source-class is already implied by `taskName`, so including it churns the key (the dedup never
243+
// fires) without adding a real distinction. The full reasonText (with count) stays in the log
244+
// message + the recordTaskOutcome payload below.
245+
const dedupKey = isLeaseHeld
246+
? `${taskName}:lease-held-by-${holderOwner}`
247+
: `${taskName}:${blockingTaskName}:${reasonCode}`;
243248

244249
if (!deferralLogKeys.has(dedupKey)) {
245250
const taskLabel = taskDefinitions?.[taskName]?.label || taskName;

test/playwright/unit/ai/daemons/orchestrator/services/MaintenanceBackpressureService.spec.mjs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,46 @@ test.describe('Neo.ai.daemons.orchestrator.services.MaintenanceBackpressureServi
289289
expect(outcomeCalls[0].payload.deferredAt).toMatch(/^\d{4}-\d{2}-\d{2}T/);
290290
});
291291

292+
test('recordDeferral dedupes across a CHANGING reasonText counter (the volatile-backlog flood fix)', () => {
293+
const deferralLogKeys = new Set();
294+
const logCalls = [];
295+
const writeLog = (level, message) => logCalls.push({level, message});
296+
const taskDefinitions = {kbSync: {label: 'KB sync'}, 'memory-summary-backfill': {label: 'memory miniSummary backfill'}};
297+
298+
// The memorySummaryBackfill backlog counter changes every poll. The dedup key is keyed on
299+
// (task, blocker, reasonCode) — NOT the volatile reasonText — so the changing count never
300+
// churns it and the deferral logs once per episode (not the live ~8%-of-lines flood).
301+
for (const backlog of [47, 48, 49, 50]) {
302+
recordDeferral({
303+
deferralLogKeys,
304+
taskName : 'memory-summary-backfill',
305+
reasonCode : 'heavy-maintenance-backpressure',
306+
reasonText : `pending-memory-minisummary:${backlog}`,
307+
blockingTaskName: 'kbSync',
308+
taskDefinitions,
309+
writeLog
310+
});
311+
}
312+
313+
// Four polls with a changing counter → ONE log line (the dedup now holds on the stable key).
314+
expect(logCalls.length).toBe(1);
315+
expect(logCalls[0].message).toContain('Deferring memory miniSummary backfill');
316+
expect(logCalls[0].message).toContain('pending-memory-minisummary:47'); // first episode's live count stays in the message
317+
318+
// A new deferral episode after the prior one clears → re-logs exactly once.
319+
clearDeferralLogState({deferralLogKeys, taskName: 'memory-summary-backfill'});
320+
recordDeferral({
321+
deferralLogKeys,
322+
taskName : 'memory-summary-backfill',
323+
reasonCode : 'heavy-maintenance-backpressure',
324+
reasonText : 'pending-memory-minisummary:51',
325+
blockingTaskName: 'kbSync',
326+
taskDefinitions,
327+
writeLog
328+
});
329+
expect(logCalls.length).toBe(2);
330+
});
331+
292332
test('recordDeferral (cross-daemon lease-held) uses holdingLease owner in key + payload', () => {
293333
const deferralLogKeys = new Set();
294334
const logCalls = [];

0 commit comments

Comments
 (0)