fix(memory): use field-level guards in applyMemorySearchDefaults
Addresses review feedback: the all-or-nothing opt-out guard was inconsistent with sibling apply* functions and would silently suppress all smart defaults for users who set any unrelated memorySearch field (e.g. just changing their embedding provider). Now mirrors applyContextPruningDefaults pattern: each sub-feature is only skipped when its specific controlling field is already set. - Setting memorySearch.provider still gets session indexing + hybrid defaults - Setting memorySearch.sources skips session indexing but still gets hybrid - Setting query.hybrid skips hybrid defaults but still gets session indexing Added three partial-config test cases to cover this behavior.
This commit is contained in:
parent
bf282b5055
commit
ada970a097
@ -8,24 +8,47 @@ describe("config memory search defaults", () => {
|
||||
const cfg = loadConfig();
|
||||
const ms = cfg.agents?.defaults?.memorySearch;
|
||||
|
||||
// Session memory indexing should be on
|
||||
expect((ms?.experimental as Record<string, unknown>)?.sessionMemory).toBe(true);
|
||||
expect(ms?.sources).toContain("sessions");
|
||||
expect(ms?.sources).toContain("memory");
|
||||
|
||||
// Hybrid search with temporal decay
|
||||
const hybrid = ms?.query?.hybrid;
|
||||
expect(hybrid?.enabled).toBe(true);
|
||||
expect(hybrid?.temporalDecay?.enabled).toBe(true);
|
||||
expect(hybrid?.temporalDecay?.halfLifeDays).toBe(30);
|
||||
|
||||
// MMR re-ranking
|
||||
expect(hybrid?.mmr?.enabled).toBe(true);
|
||||
expect(hybrid?.mmr?.lambda).toBe(0.7);
|
||||
});
|
||||
});
|
||||
|
||||
it("does not overwrite explicit memorySearch config", async () => {
|
||||
it("still applies hybrid defaults when user only sets an unrelated field like provider", async () => {
|
||||
await withTempHomeConfig(
|
||||
{
|
||||
agents: {
|
||||
defaults: {
|
||||
memorySearch: {
|
||||
provider: "openai",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
async () => {
|
||||
const cfg = loadConfig();
|
||||
const ms = cfg.agents?.defaults?.memorySearch;
|
||||
|
||||
// User's explicit field preserved
|
||||
expect(ms?.provider).toBe("openai");
|
||||
|
||||
// Smart defaults still applied — user only set provider, not these
|
||||
expect((ms?.experimental as Record<string, unknown>)?.sessionMemory).toBe(true);
|
||||
expect(ms?.sources).toContain("sessions");
|
||||
expect(ms?.query?.hybrid?.temporalDecay?.enabled).toBe(true);
|
||||
expect(ms?.query?.hybrid?.mmr?.enabled).toBe(true);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("does not overwrite explicit sources config", async () => {
|
||||
await withTempHomeConfig(
|
||||
{
|
||||
agents: {
|
||||
@ -40,12 +63,42 @@ describe("config memory search defaults", () => {
|
||||
const cfg = loadConfig();
|
||||
const ms = cfg.agents?.defaults?.memorySearch;
|
||||
|
||||
// User explicitly set sources — respect it, don't inject sessions
|
||||
// User explicitly restricted sources — respect it
|
||||
expect(ms?.sources).toEqual(["memory"]);
|
||||
expect(ms?.sources).not.toContain("sessions");
|
||||
|
||||
// No defaults injected since user provided explicit config
|
||||
expect((ms?.experimental as Record<string, unknown>)?.sessionMemory).toBeUndefined();
|
||||
// Hybrid defaults still applied since query.hybrid wasn't set
|
||||
expect(ms?.query?.hybrid?.temporalDecay?.enabled).toBe(true);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it("does not overwrite explicit query.hybrid config", async () => {
|
||||
await withTempHomeConfig(
|
||||
{
|
||||
agents: {
|
||||
defaults: {
|
||||
memorySearch: {
|
||||
query: {
|
||||
hybrid: {
|
||||
enabled: false,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
async () => {
|
||||
const cfg = loadConfig();
|
||||
const ms = cfg.agents?.defaults?.memorySearch;
|
||||
|
||||
// User explicitly disabled hybrid — respect it, don't inject our defaults
|
||||
expect(ms?.query?.hybrid?.enabled).toBe(false);
|
||||
expect(ms?.query?.hybrid?.temporalDecay).toBeUndefined();
|
||||
expect(ms?.query?.hybrid?.mmr).toBeUndefined();
|
||||
|
||||
// Session indexing defaults still applied since sources/experimental weren't set
|
||||
expect((ms?.experimental as Record<string, unknown>)?.sessionMemory).toBe(true);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
@ -558,8 +558,39 @@ export function applyMemorySearchDefaults(cfg: OpenClawConfig): OpenClawConfig {
|
||||
return cfg;
|
||||
}
|
||||
|
||||
// Don't touch anything if the user has explicitly configured memorySearch
|
||||
if (defaults.memorySearch !== undefined) {
|
||||
const existing = defaults.memorySearch ?? {};
|
||||
let next = { ...existing };
|
||||
let mutated = false;
|
||||
|
||||
// Only apply session indexing defaults if the user hasn't touched either field
|
||||
if (existing.experimental?.sessionMemory === undefined && existing.sources === undefined) {
|
||||
next = {
|
||||
...next,
|
||||
experimental: { ...existing.experimental, sessionMemory: true },
|
||||
sources: ["memory", "sessions"],
|
||||
};
|
||||
mutated = true;
|
||||
}
|
||||
|
||||
// Only apply hybrid search defaults if the user hasn't configured query.hybrid at all
|
||||
if (existing.query?.hybrid === undefined) {
|
||||
next = {
|
||||
...next,
|
||||
query: {
|
||||
...existing.query,
|
||||
hybrid: {
|
||||
enabled: true,
|
||||
vectorWeight: 0.7,
|
||||
textWeight: 0.3,
|
||||
temporalDecay: { enabled: true, halfLifeDays: 30 },
|
||||
mmr: { enabled: true, lambda: 0.7 },
|
||||
},
|
||||
},
|
||||
};
|
||||
mutated = true;
|
||||
}
|
||||
|
||||
if (!mutated) {
|
||||
return cfg;
|
||||
}
|
||||
|
||||
@ -569,25 +600,7 @@ export function applyMemorySearchDefaults(cfg: OpenClawConfig): OpenClawConfig {
|
||||
...cfg.agents,
|
||||
defaults: {
|
||||
...defaults,
|
||||
memorySearch: {
|
||||
experimental: { sessionMemory: true },
|
||||
sources: ["memory", "sessions"],
|
||||
query: {
|
||||
hybrid: {
|
||||
enabled: true,
|
||||
vectorWeight: 0.7,
|
||||
textWeight: 0.3,
|
||||
temporalDecay: {
|
||||
enabled: true,
|
||||
halfLifeDays: 30,
|
||||
},
|
||||
mmr: {
|
||||
enabled: true,
|
||||
lambda: 0.7,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
memorySearch: next,
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user