Files
rohitg00__agentmemory/test/diagnostic-followup-rate.test.ts
Rohit Ghumare 9a4bdbe0e8 feat(diag): smart-search followup-rate proxy + reader-failure counter (#786)
* feat(diag): smart-search followup-rate proxy + reader-failure counter

Closes #771.

Disambiguates 'retrieval bug' vs 'reader bug'. Today every retrieval
regression looks identical to a reader-behavior regression because we
have no signal for 'the right memory was in the context but the agent
didn't use it'. This change ships the live-use proxy half end-to-end
plus the placeholder counter the benchmark scorer will increment.

Live-use proxy
--------------

mem::smart-search now detects follow-up searches: when an agent-initiated
call (sessionId present, source !== 'viewer') is followed by another
search from the same session inside AGENTMEMORY_FOLLOWUP_WINDOW_SECONDS
(default 30s) AND the new result set has zero overlap with the prior
one AND the queries differ, we count it as a directional signal that
the first results didn't satisfy the agent.

  - New KV scope mem:recent-searches holds the most recent search per
    session: { sessionId, query, resultIds, at }. sessionId is
    duplicated into the row so the sweep can delete by it (StateKV.list
    returns values only).
  - mem::diagnostic::recent-searches-sweep runs hourly, drops rows
    older than 24h.
  - OTEL counter agentmemory.smart_search.followup_within_window_total
    fires on every detected follow-up. Mirrored to a module-scope
    counter for read-back without an OTEL collector.
  - mem::diagnostic::followup-stats exposes the live counts;
    GET /agentmemory/diagnostics/followup returns them with a
    'directional / may overcount on refinement' caveat.
  - agentmemory status surfaces 'Followup rate: H/T (P%) within Ws'.

Benchmark-mode counter
----------------------

Counter agentmemory.reader_failure_with_evidence_total is registered
but never incremented by core in live use. The benchmark scorer
(separate from this PR) increments it when judge_correct === false AND
the gold-evidence IDs are a subset of the retrieved-context IDs.

Exclusions
----------

  - Viewer-originated searches (X-Agentmemory-Source: viewer or body
    field source: 'viewer') skip the diagnostic entirely. The API
    trigger reads the header and threads it into the smart-search
    payload; the body field wins if both are set.
  - Direct sdk.trigger callers without a sessionId are skipped.
  - Identical re-query is a retry, not a follow-up (skipped).
  - Result-set overlap means the prior results were used (skipped).

Edge cases covered by tests
---------------------------

  - First call records but never flags.
  - Disjoint results within window: flagged.
  - Overlapping results within window: not flagged.
  - Identical query (retry): not flagged.
  - Prior search outside window: not flagged.
  - Viewer source: skipped, scope unchanged so subsequent agent calls
    don't see the viewer search as 'prior'.
  - No sessionId: skipped.
  - Sweep deletes stale (>24h) rows, keeps fresh ones.

Endpoint count bumped 125 -> 126 across README / AGENTS / index banner.

* fix(diag): whitelist smart-search payload, off-critical-path followup detection

Address CodeRabbit review on PR #786.

src/triggers/api.ts: api::smart-search was spreading req.body into the
sdk.trigger payload, which violates the AGENTS.md security rule that
REST endpoints must whitelist fields. Replace the spread with an
explicit field list (query, expandIds, limit, project, includeLessons,
agentId, sessionId, source). Unknown fields from the client are now
dropped before reaching mem::smart-search.

src/functions/smart-search.ts: followup detection used to await
kv.get + kv.set on the critical response path, adding one round-trip
of state I/O per agent-initiated search. Move detection into a
withKeyedLock('recent-searches:<sessionId>', ...) call that runs
fire-and-forget. The lock serializes per session so the next call's
detectFollowup still reads the prior call's row (lock body queues
behind), and other sessions run in parallel. flushPendingFollowups()
exposes a drain hook for tests and graceful-shutdown paths.

Empty-result calls now skip detection entirely. A search that returns
zero hits is a retrieval failure, not a reader-failure signal — the
old code treated the empty-vs-prior comparison as 'disjoint' and would
inflate the rate every time search returned nothing.

src/state/schema.ts unchanged. RecentSearch now exported from
smart-search.ts and consumed by recent-searches-sweep.ts so the row
shape stays consistent across producer and sweeper.

Audit logging on every recent-searches write was suggested but
skipped — high-frequency diagnostic writes don't belong in the audit
log (analogous flows like recordAccessBatch and dedup don't audit
either; it would 10x audit volume without operator-actionable value).

Tests: added an empty-results case (sub-point b) that asserts the
empty-skip guard. Mock sdk.trigger now flushes pending followups
after each smart-search call so test assertions see consistent state.

122 test files / 1336 tests pass.
2026-06-02 20:18:30 +01:00

301 lines
9.5 KiB
TypeScript

import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
vi.mock("../src/logger.js", () => ({
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
}));
import {
registerSmartSearchFunction,
getFollowupStats,
resetFollowupStatsForTests,
flushPendingFollowups,
} from "../src/functions/smart-search.js";
import { registerRecentSearchesSweepFunction } from "../src/functions/recent-searches-sweep.js";
import { KV } from "../src/state/schema.js";
import type { HybridSearchResult } from "../src/types.js";
function mockKV() {
const store = new Map<string, Map<string, unknown>>();
return {
get: async <T>(scope: string, key: string): Promise<T | null> =>
(store.get(scope)?.get(key) as T) ?? null,
set: async <T>(scope: string, key: string, data: T): Promise<T> => {
if (!store.has(scope)) store.set(scope, new Map());
store.get(scope)!.set(key, data);
return data;
},
delete: async (scope: string, key: string): Promise<void> => {
store.get(scope)?.delete(key);
},
list: async <T>(scope: string): Promise<T[]> =>
Array.from(store.get(scope)?.values() ?? []) as T[],
getStore: () => store,
};
}
function mockSdk(kv: ReturnType<typeof mockKV>) {
const functions = new Map<string, Function>();
const sdk = {
registerFunction: (
idOrOpts: string | { id: string },
handler: Function,
) => {
const id = typeof idOrOpts === "string" ? idOrOpts : idOrOpts.id;
functions.set(id, handler);
},
registerTrigger: () => {},
trigger: async (
idOrInput: string | { function_id: string; payload?: unknown },
data?: unknown,
) => {
const id = typeof idOrInput === "string" ? idOrInput : idOrInput.function_id;
const payload =
typeof idOrInput === "string" ? data : (idOrInput as any).payload;
const fn = functions.get(id);
if (!fn) {
if (id === "mem::lesson-recall") return { success: true, lessons: [] };
throw new Error(`No function: ${id}`);
}
const result = await fn(payload);
// smart-search now runs followup detection off the critical
// response path; drain it before returning so test assertions
// see consistent state.
if (id === "mem::smart-search") await flushPendingFollowups();
return result;
},
} as any;
void kv;
return sdk;
}
function makeHit(obsId: string, sessionId = "ses_1"): HybridSearchResult {
return {
observation: {
id: obsId,
sessionId,
timestamp: new Date().toISOString(),
title: `obs ${obsId}`,
narrative: "n",
type: "pattern",
concepts: [],
files: [],
} as any,
sessionId,
combinedScore: 0.8,
} as HybridSearchResult;
}
describe("Smart-search followup-rate diagnostic (#771)", () => {
let sdk: any;
let kv: ReturnType<typeof mockKV>;
let searchResults: HybridSearchResult[];
beforeEach(() => {
delete process.env.AGENTMEMORY_FOLLOWUP_WINDOW_SECONDS;
resetFollowupStatsForTests();
kv = mockKV();
sdk = mockSdk(kv);
searchResults = [];
registerSmartSearchFunction(sdk, kv as any, async () => searchResults);
registerRecentSearchesSweepFunction(sdk, kv as any);
});
afterEach(() => {
delete process.env.AGENTMEMORY_FOLLOWUP_WINDOW_SECONDS;
});
it("records the first agent-initiated search but does not flag it as a followup", async () => {
searchResults = [makeHit("obs_a"), makeHit("obs_b")];
await sdk.trigger("mem::smart-search", {
query: "auth flow",
sessionId: "ses_1",
});
const stats = getFollowupStats();
expect(stats.agentInitiatedSearches).toBe(1);
expect(stats.followupWithinWindow).toBe(0);
const stored = await kv.get(KV.recentSearches, "ses_1");
expect(stored).not.toBeNull();
expect((stored as any).sessionId).toBe("ses_1");
expect((stored as any).query).toBe("auth flow");
});
it("flags a follow-up when the second search inside the window returns a disjoint set", async () => {
searchResults = [makeHit("obs_a"), makeHit("obs_b")];
await sdk.trigger("mem::smart-search", {
query: "auth flow",
sessionId: "ses_1",
});
searchResults = [makeHit("obs_c"), makeHit("obs_d")];
await sdk.trigger("mem::smart-search", {
query: "token expiry handling",
sessionId: "ses_1",
});
const stats = getFollowupStats();
expect(stats.agentInitiatedSearches).toBe(2);
expect(stats.followupWithinWindow).toBe(1);
expect(stats.rate).toBeCloseTo(0.5);
});
it("does not flag a follow-up when result sets overlap", async () => {
searchResults = [makeHit("obs_a"), makeHit("obs_b")];
await sdk.trigger("mem::smart-search", {
query: "auth flow",
sessionId: "ses_1",
});
searchResults = [makeHit("obs_b"), makeHit("obs_c")];
await sdk.trigger("mem::smart-search", {
query: "auth token",
sessionId: "ses_1",
});
const stats = getFollowupStats();
expect(stats.followupWithinWindow).toBe(0);
});
it("does not flag a follow-up on an identical re-query (retry, not follow-up)", async () => {
searchResults = [makeHit("obs_a")];
await sdk.trigger("mem::smart-search", {
query: "auth flow",
sessionId: "ses_1",
});
// Different result set on the retry (e.g. flaky search index), but
// same query — still not a follow-up.
searchResults = [makeHit("obs_b")];
await sdk.trigger("mem::smart-search", {
query: "auth flow",
sessionId: "ses_1",
});
expect(getFollowupStats().followupWithinWindow).toBe(0);
});
it("does not flag a follow-up when prior search is outside the window", async () => {
process.env.AGENTMEMORY_FOLLOWUP_WINDOW_SECONDS = "1";
searchResults = [makeHit("obs_a")];
await sdk.trigger("mem::smart-search", {
query: "first",
sessionId: "ses_1",
});
// Backdate the stored search so the window has elapsed.
const stored = (await kv.get(KV.recentSearches, "ses_1")) as any;
stored.at = Date.now() - 5_000;
await kv.set(KV.recentSearches, "ses_1", stored);
searchResults = [makeHit("obs_b")];
await sdk.trigger("mem::smart-search", {
query: "second",
sessionId: "ses_1",
});
expect(getFollowupStats().followupWithinWindow).toBe(0);
});
it("skips viewer-originated searches (source === 'viewer')", async () => {
searchResults = [makeHit("obs_a")];
await sdk.trigger("mem::smart-search", {
query: "from viewer",
sessionId: "ses_1",
source: "viewer",
});
expect(getFollowupStats().agentInitiatedSearches).toBe(0);
// Viewer call shouldn't write to recent-searches either, otherwise
// a subsequent agent call would treat the viewer search as prior.
expect(await kv.get(KV.recentSearches, "ses_1")).toBeNull();
});
it("skips searches without a sessionId (direct sdk callers)", async () => {
searchResults = [makeHit("obs_a")];
await sdk.trigger("mem::smart-search", { query: "no session" });
expect(getFollowupStats().agentInitiatedSearches).toBe(0);
});
it("recent-searches sweep deletes rows older than 24h, keeps fresh ones", async () => {
const fresh = {
sessionId: "ses_fresh",
query: "x",
resultIds: [],
at: Date.now() - 1_000,
};
const stale = {
sessionId: "ses_stale",
query: "x",
resultIds: [],
at: Date.now() - 25 * 60 * 60 * 1000,
};
await kv.set(KV.recentSearches, fresh.sessionId, fresh);
await kv.set(KV.recentSearches, stale.sessionId, stale);
const result = (await sdk.trigger(
"mem::diagnostic::recent-searches-sweep",
{},
)) as { swept: number };
expect(result.swept).toBe(1);
expect(await kv.get(KV.recentSearches, "ses_fresh")).not.toBeNull();
expect(await kv.get(KV.recentSearches, "ses_stale")).toBeNull();
});
it("skips detection when current results are empty (retrieval failure, not reader failure)", async () => {
searchResults = [makeHit("obs_a"), makeHit("obs_b")];
await sdk.trigger("mem::smart-search", {
query: "first",
sessionId: "ses_1",
});
// Empty result set on the next call. Without the empty-skip guard
// the empty-vs-prior comparison would be vacuously "disjoint" and
// inflate the rate. Skip detection entirely.
searchResults = [];
await sdk.trigger("mem::smart-search", {
query: "second",
sessionId: "ses_1",
});
const stats = getFollowupStats();
// Only the first call counts as agent-initiated; the empty-result
// second call is skipped entirely.
expect(stats.agentInitiatedSearches).toBe(1);
expect(stats.followupWithinWindow).toBe(0);
});
it("followup-stats function returns the configured window and live counts", async () => {
process.env.AGENTMEMORY_FOLLOWUP_WINDOW_SECONDS = "45";
searchResults = [makeHit("obs_a")];
await sdk.trigger("mem::smart-search", {
query: "q1",
sessionId: "ses_1",
});
searchResults = [makeHit("obs_b")];
await sdk.trigger("mem::smart-search", {
query: "q2",
sessionId: "ses_1",
});
const stats = (await sdk.trigger(
"mem::diagnostic::followup-stats",
{},
)) as {
success: boolean;
windowSeconds: number;
agentInitiatedSearches: number;
followupWithinWindow: number;
rate: number;
};
expect(stats.success).toBe(true);
expect(stats.windowSeconds).toBe(45);
expect(stats.agentInitiatedSearches).toBe(2);
expect(stats.followupWithinWindow).toBe(1);
expect(stats.rate).toBeCloseTo(0.5);
});
});