Skip to content

Commit dbd0428

Browse files
clauderuvnet
andcommitted
fix(memory): validate weights + deterministic tie-break; port #3104 lockfile fix
Per ruvnet's review on #3119: - weighted RRF fusion previously had undefined ranking semantics for malformed weights (NaN poisons the sort comparator, negative/Infinity invert or swamp the intended ordering). Each weight component now validates independently (finite, >= 0) and falls back to its own documented default otherwise, same degradation as an omitted weights field already had. - rrf ties now break deterministically by entry.id (explicit secondary sort key) instead of relying on Array.sort's stability plus Map insertion order as an undocumented side effect. - 4 new tests: 3 malformed-weight fallback cases + 1 genuine-tie (weights: {semantic:0, structured:0}) deterministic-order case. Also ports PR #3104's fix verbatim (v3/pnpm-lock.yaml: @claude-flow/mcp specifier ^3.0.0-alpha.9 -> 3.0.0-alpha.10, matching the CLI manifest's exact pin) to unblock the install-dependent CI gates this PR needs to actually execute (issue #3101, also flagged in the review). Root-level npm ETARGET (issue #3095, package-lock.json) is a separate, still-open, maintainer-flagged issue with no proposed fix yet -- not addressed here. Validated: `corepack pnpm install --frozen-lockfile --lockfile-only` and `--ignore-scripts` both succeed from v3/; full @claude-flow/memory suite 465/466 (same 1 pre-existing unrelated env failure); tsc --noEmit clean. Co-Authored-By: RuFlo <ruv@ruv.net> Claude-Session: https://claude.ai/code/session_01AjuAxfhZi6eZXnLbLC14Ay
1 parent b901326 commit dbd0428

3 files changed

Lines changed: 59 additions & 4 deletions

File tree

v3/@claude-flow/memory/src/hybrid-backend.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,42 @@ describe('HybridBackend - ADR-009', () => {
318318
const keys = results.map((r) => r.key).sort();
319319
expect(keys).toEqual(['fusion-sem-fav', 'fusion-struct-fav']);
320320
});
321+
322+
// Review (PR #3119): malformed weights previously had undefined ranking
323+
// semantics (NaN poisons the sort comparator, negative/Infinity invert
324+
// or swamp the intended ordering). Each malformed component now falls
325+
// back to its own default (0.7 semantic / 0.3 structured) — same
326+
// ranking as the semantic-heavy default, since 0.7 > 0.3.
327+
it.each([
328+
['NaN semantic weight', { semantic: NaN, structured: 0.3 }],
329+
['negative semantic weight', { semantic: -5, structured: 0.3 }],
330+
['Infinity structured weight', { semantic: 0.7, structured: Infinity }],
331+
])('falls back to default weighting for a malformed weight (%s)', async (_label, weights) => {
332+
expect(await fusedTopKey(weights)).toBe('fusion-sem-fav');
333+
});
334+
335+
it('breaks exact rrf ties deterministically by entry id, not iteration order', async () => {
336+
// weights of 0/0 make every entry's rrf contribution 0 — a genuine
337+
// tie between both entries, not just a coincidentally-close score.
338+
const results = await backend.queryHybrid({
339+
semantic: {
340+
content: QUERY_CONTENT,
341+
k: 5,
342+
threshold: 0.01,
343+
filters: { namespace: 'fusion-test' },
344+
},
345+
structured: {
346+
namespace: 'fusion-test',
347+
keyPrefix: 'fusion-',
348+
limit: 5,
349+
},
350+
combineStrategy: 'union',
351+
weights: { semantic: 0, structured: 0 },
352+
});
353+
expect(results.map((r) => r.key)).toEqual(
354+
['fusion-sem-fav', 'fusion-struct-fav'].sort((a, b) => a.localeCompare(b))
355+
);
356+
});
321357
});
322358

323359
describe('CRUD Operations', () => {

v3/@claude-flow/memory/src/hybrid-backend.ts

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -689,22 +689,41 @@ export class HybridBackend extends EventEmitter implements IMemoryBackend {
689689
): MemoryEntry[] {
690690
const RRF_K = 60; // matches smart-retrieval.ts's applyRRF default
691691

692+
// Review (PR #3119): an unvalidated malformed weight (NaN, negative,
693+
// Infinity) gives RRF undefined ranking semantics — NaN poisons every
694+
// sum it touches (breaking the sort comparator below), a negative
695+
// weight silently inverts that arm's intended preference, and
696+
// Infinity swamps the other arm outright. Sanitize per-component: any
697+
// non-finite or negative value falls back to that component's own
698+
// documented default rather than failing the whole hybrid query (this
699+
// mirrors `weights` itself already being optional with a default —
700+
// a malformed field degrades the same way a missing one does).
701+
const safeWeight = (value: number, fallback: number): number =>
702+
Number.isFinite(value) && value >= 0 ? value : fallback;
703+
const safeWeights = {
704+
semantic: safeWeight(weights.semantic, 0.7),
705+
structured: safeWeight(weights.structured, 0.3),
706+
};
707+
692708
const fused = new Map<string, { entry: MemoryEntry; rrf: number }>();
693709

694710
semanticScored.forEach(({ entry }, rank) => {
695-
const contribution = weights.semantic * (1 / (RRF_K + rank + 1));
711+
const contribution = safeWeights.semantic * (1 / (RRF_K + rank + 1));
696712
const existing = fused.get(entry.id);
697713
fused.set(entry.id, { entry, rrf: (existing?.rrf ?? 0) + contribution });
698714
});
699715

700716
structuredResults.forEach((entry, rank) => {
701-
const contribution = weights.structured * (1 / (RRF_K + rank + 1));
717+
const contribution = safeWeights.structured * (1 / (RRF_K + rank + 1));
702718
const existing = fused.get(entry.id);
703719
fused.set(entry.id, { entry, rrf: (existing?.rrf ?? 0) + contribution });
704720
});
705721

722+
// Review (PR #3119): break rrf ties deterministically by entry.id
723+
// rather than relying on Array.sort's stability + Map insertion order
724+
// (semantic-arm-first) as an undocumented accident of iteration order.
706725
return Array.from(fused.values())
707-
.sort((a, b) => b.rrf - a.rrf)
726+
.sort((a, b) => b.rrf - a.rrf || a.entry.id.localeCompare(b.entry.id))
708727
.map((r) => r.entry);
709728
}
710729

v3/pnpm-lock.yaml

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)