Skip to content

Commit 7a4e0bd

Browse files
jlin53882Review Claw
andauthored
fix: skip applyRecencyBoost when decayEngine is active in vectorOnlyRetrieval (#500)
* fix: guard applyRecencyBoost with decayEngine check in vectorOnlyRetrieval Bug 7 fix: when decayEngine is active, skip applyRecencyBoost here because decayEngine already handles temporal scoring; avoid double-boost. * test: add regression for Bug 7 - skip applyRecencyBoost when decayEngine active in vectorOnlyRetrieval Co-authored-by: Review Claw <review-claw@openclaw.ai> --------- Co-authored-by: Review Claw <review-claw@openclaw.ai>
1 parent de1ba95 commit 7a4e0bd

File tree

2 files changed

+201
-1
lines changed

2 files changed

+201
-1
lines changed

src/retriever.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -759,7 +759,11 @@ export class MemoryRetriever {
759759
);
760760

761761
failureStage = "vector.postProcess";
762-
const recencyBoosted = this.applyRecencyBoost(mapped);
762+
// Bug 7 fix: when decayEngine is active, skip applyRecencyBoost here because
763+
// decayEngine already handles temporal scoring; avoid double-boost.
764+
const recencyBoosted = this.decayEngine
765+
? mapped
766+
: this.applyRecencyBoost(mapped);
763767
if (diagnostics) diagnostics.stageCounts.afterRecency = recencyBoosted.length;
764768
const weighted = this.decayEngine
765769
? recencyBoosted
Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
import { describe, it } from "node:test";
2+
import assert from "node:assert/strict";
3+
import jitiFactory from "jiti";
4+
5+
const jiti = jitiFactory(import.meta.url, { interopDefault: true });
6+
7+
const { MemoryRetriever, DEFAULT_RETRIEVAL_CONFIG } = jiti("../src/retriever.ts");
8+
const { createDecayEngine, DEFAULT_DECAY_CONFIG } = jiti("../src/decay-engine.ts");
9+
10+
// ============================================================
11+
// Test helpers
12+
// ============================================================
13+
14+
function makeEntry(id, text, daysAgo, tier = "working") {
15+
const now = Date.now();
16+
const age = daysAgo * 86_400_000;
17+
return {
18+
id,
19+
text,
20+
vector: new Array(384).fill(0.1),
21+
category: "fact",
22+
scope: "global",
23+
importance: 0.8,
24+
timestamp: now - age,
25+
metadata: JSON.stringify({
26+
tier,
27+
confidence: 0.9,
28+
accessCount: 1,
29+
createdAt: now - age,
30+
lastAccessedAt: now - age,
31+
}),
32+
};
33+
}
34+
35+
function createMockStore(entries) {
36+
const map = new Map(entries.map(e => [e.id, e]));
37+
return {
38+
hasFtsSupport: true,
39+
async vectorSearch() {
40+
return entries.map((entry, index) => ({ entry, score: 0.9 - index * 0.05 }));
41+
},
42+
async bm25Search() { return []; },
43+
async hasId(id) { return map.has(id); },
44+
};
45+
}
46+
47+
function createMockEmbedder() {
48+
return {
49+
async embedQuery() { return new Array(384).fill(0.1); },
50+
};
51+
}
52+
53+
// ============================================================
54+
// Bug 7: decayEngine recency double-boost regression
55+
// ============================================================
56+
57+
describe("MemoryRetriever - decayEngine recency double-boost regression (Bug 7)", () => {
58+
it("should NOT double-boost recency when decayEngine is active in vector-only mode", async () => {
59+
// Two entries: very recent (1 day) vs old (60 days)
60+
const recentEntry = makeEntry("recent-1", "Recent decision about API design", 1);
61+
const oldEntry = makeEntry("old-1", "Old decision about database schema", 60);
62+
63+
const entries = [recentEntry, oldEntry];
64+
const store = createMockStore(entries);
65+
const embedder = createMockEmbedder();
66+
67+
// WITHOUT decayEngine: applyRecencyBoost boosts recent entries
68+
const retrieverWithoutDecay = new MemoryRetriever(store, embedder, {
69+
...DEFAULT_RETRIEVAL_CONFIG,
70+
mode: "vector",
71+
recencyHalfLifeDays: 14,
72+
recencyWeight: 0.1,
73+
filterNoise: false,
74+
hardMinScore: 0,
75+
});
76+
77+
// WITH decayEngine: recency is handled by decayEngine, NOT applyRecencyBoost
78+
const decayEngine = createDecayEngine({
79+
...DEFAULT_DECAY_CONFIG,
80+
recencyHalfLifeDays: 30,
81+
recencyWeight: 0.4,
82+
});
83+
const retrieverWithDecay = new MemoryRetriever(store, embedder, {
84+
...DEFAULT_RETRIEVAL_CONFIG,
85+
mode: "vector",
86+
filterNoise: false,
87+
hardMinScore: 0,
88+
}, { decayEngine });
89+
90+
const [withoutDecay, withDecay] = await Promise.all([
91+
retrieverWithoutDecay.retrieve({ query: "decision", limit: 5 }),
92+
retrieverWithDecay.retrieve({ query: "decision", limit: 5 }),
93+
]);
94+
95+
assert.equal(withoutDecay.length, 2, "withoutDecay should return 2 results");
96+
assert.equal(withDecay.length, 2, "withDecay should return 2 results");
97+
98+
const recentWithDecay = withDecay.find(r => r.entry.id === "recent-1");
99+
const oldWithDecay = withDecay.find(r => r.entry.id === "old-1");
100+
101+
// With decayEngine, recent entry should score >= old entry (decayEngine boosts recency)
102+
assert.ok(recentWithDecay, "recent entry should be in withDecay results");
103+
assert.ok(recentWithDecay.score >= oldWithDecay.score,
104+
"with decayEngine: recent entry should score >= old entry");
105+
106+
// Without decayEngine, recent entry should also be boosted (applyRecencyBoost)
107+
const recentWithoutDecay = withoutDecay.find(r => r.entry.id === "recent-1");
108+
const oldWithoutDecay = withoutDecay.find(r => r.entry.id === "old-1");
109+
assert.ok(recentWithoutDecay.score >= oldWithoutDecay.score,
110+
"without decayEngine: recent entry should score >= old entry");
111+
});
112+
113+
it("should produce comparable scores regardless of which recency path is used (no extreme double-boost)", async () => {
114+
// If Bug 7 existed, applying BOTH boosts would make recent entries score MUCH higher.
115+
// With the fix, only one recency mechanism fires, so scores stay comparable.
116+
const recentEntry = makeEntry("recent-2", "Very recent memory about auth", 1);
117+
const oldEntry = makeEntry("old-2", "Very old memory about auth", 90);
118+
119+
const store = createMockStore([recentEntry, oldEntry]);
120+
const embedder = createMockEmbedder();
121+
const decayEngine = createDecayEngine(DEFAULT_DECAY_CONFIG);
122+
123+
const retrieverWithDecay = new MemoryRetriever(store, embedder, {
124+
...DEFAULT_RETRIEVAL_CONFIG,
125+
mode: "vector",
126+
filterNoise: false,
127+
hardMinScore: 0,
128+
}, { decayEngine });
129+
130+
const retrieverWithoutDecay = new MemoryRetriever(store, embedder, {
131+
...DEFAULT_RETRIEVAL_CONFIG,
132+
mode: "vector",
133+
recencyHalfLifeDays: 30,
134+
recencyWeight: 0.1,
135+
filterNoise: false,
136+
hardMinScore: 0,
137+
});
138+
139+
const [withDecay, withoutDecay] = await Promise.all([
140+
retrieverWithDecay.retrieve({ query: "auth", limit: 5 }),
141+
retrieverWithoutDecay.retrieve({ query: "auth", limit: 5 }),
142+
]);
143+
144+
assert.ok(withDecay.length >= 1, "withDecay should return results");
145+
assert.ok(withoutDecay.length >= 1, "withoutDecay should return results");
146+
147+
const recentWithDecay = withDecay.find(r => r.entry.id === "recent-2");
148+
const recentWithoutDecay = withoutDecay.find(r => r.entry.id === "recent-2");
149+
150+
// Scores should be in the same ballpark (different formulas, but not wildly different).
151+
// If double-boost existed, withDecay would be >> withoutDecay.
152+
if (recentWithoutDecay && recentWithDecay) {
153+
const ratio = recentWithDecay.score / recentWithoutDecay.score;
154+
assert.ok(ratio > 0.3 && ratio < 3.0,
155+
`Scores should be comparable (ratio=${ratio.toFixed(2)}); extreme ratio suggests double-boost bug`);
156+
}
157+
});
158+
159+
it("should skip applyRecencyBoost when decayEngine is active (bm25-only path for comparison)", async () => {
160+
// Verify the bm25-only path also correctly skips recencyBoost when decayEngine is active.
161+
// This is a consistency check across retrieval modes.
162+
const recentEntry = makeEntry("recent-3", "Recent decision about caching", 2);
163+
const oldEntry = makeEntry("old-3", "Old decision about caching strategy", 45);
164+
165+
const store = {
166+
hasFtsSupport: true,
167+
async vectorSearch() { return []; },
168+
async bm25Search() {
169+
return [
170+
{ entry: recentEntry, score: 0.85 },
171+
{ entry: oldEntry, score: 0.82 },
172+
];
173+
},
174+
async hasId(id) { return id === recentEntry.id || id === oldEntry.id; },
175+
};
176+
const embedder = createMockEmbedder();
177+
const decayEngine = createDecayEngine(DEFAULT_DECAY_CONFIG);
178+
179+
const retriever = new MemoryRetriever(store, embedder, {
180+
...DEFAULT_RETRIEVAL_CONFIG,
181+
mode: "hybrid", // use hybrid to exercise bm25-only path when vector returns nothing
182+
filterNoise: false,
183+
hardMinScore: 0,
184+
}, { decayEngine });
185+
186+
const results = await retriever.retrieve({ query: "caching", limit: 5 });
187+
188+
assert.ok(results.length >= 1, "should return at least one result");
189+
// The recent entry should be present and scored appropriately
190+
const recentResult = results.find(r => r.entry.id === "recent-3");
191+
assert.ok(recentResult, "recent entry should be in results");
192+
assert.ok(recentResult.score > 0, "score should be positive");
193+
});
194+
});
195+
196+
console.log("OK: retriever decay recency double-boost regression test passed");

0 commit comments

Comments
 (0)