Skip to content

Commit faa847c

Browse files
rwmjhbHeng Xia
andauthored
[codex] fix requestDimensions schema sizing and request payload alignment (#572)
* fix: align requestDimensions schema sizing and request payloads * test: reset singleton plugin state between scenarios --------- Co-authored-by: Heng Xia <pope@Hengs-Mac-mini.local>
1 parent a583e32 commit faa847c

File tree

6 files changed

+130
-23
lines changed

6 files changed

+130
-23
lines changed

index.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ const isCliMode = () => process.env.OPENCLAW_CLI === "1";
2121

2222
// Import core components
2323
import { MemoryStore, validateStoragePath } from "./src/store.js";
24-
import { createEmbedder, getVectorDimensions } from "./src/embedder.js";
24+
import {
25+
createEmbedder,
26+
getEffectiveVectorDimensions,
27+
} from "./src/embedder.js";
2528
import { createRetriever, DEFAULT_RETRIEVAL_CONFIG } from "./src/retriever.js";
2629
import { createScopeManager, resolveScopeFilter, isSystemBypassId, parseAgentIdFromSessionKey } from "./src/scopes.js";
2730
import { createMigrator } from "./src/migrate.js";
@@ -91,6 +94,7 @@ interface PluginConfig {
9194
model?: string;
9295
baseURL?: string;
9396
dimensions?: number;
97+
requestDimensions?: number;
9498
omitDimensions?: boolean;
9599
taskQuery?: string;
96100
taskPassage?: string;
@@ -1647,7 +1651,7 @@ const pluginVersion = getPluginVersion();
16471651
// WeakSet keyed by API instance — each distinct API object tracks its own initialized state.
16481652
// Using WeakSet instead of a module-level boolean avoids the "second register() call skips
16491653
// hook/tool registration for the new API instance" regression that rwmjhb identified.
1650-
const _registeredApis = new WeakSet<OpenClawPluginApi>();
1654+
let _registeredApis = new WeakSet<OpenClawPluginApi>();
16511655

16521656
// ============================================================================
16531657
// Hook Event Deduplication (Phase 1)
@@ -1730,9 +1734,10 @@ function _initPluginState(api: OpenClawPluginApi): PluginSingletonState {
17301734
);
17311735
}
17321736

1733-
const vectorDim = getVectorDimensions(
1737+
const vectorDim = getEffectiveVectorDimensions(
17341738
config.embedding.model || "text-embedding-3-small",
17351739
config.embedding.dimensions,
1740+
config.embedding.requestDimensions,
17361741
);
17371742
const store = new MemoryStore({ dbPath: resolvedDbPath, vectorDim });
17381743
const embedder = createEmbedder({
@@ -1741,6 +1746,7 @@ function _initPluginState(api: OpenClawPluginApi): PluginSingletonState {
17411746
model: config.embedding.model || "text-embedding-3-small",
17421747
baseURL: config.embedding.baseURL,
17431748
dimensions: config.embedding.dimensions,
1749+
requestDimensions: config.embedding.requestDimensions,
17441750
omitDimensions: config.embedding.omitDimensions,
17451751
taskQuery: config.embedding.taskQuery,
17461752
taskPassage: config.embedding.taskPassage,
@@ -4033,6 +4039,8 @@ export function parsePluginConfig(value: unknown): PluginConfig {
40334039
// Accept number, numeric string, or env-var string (e.g. "${EMBED_DIM}").
40344040
// Also accept legacy top-level `dimensions` for convenience.
40354041
dimensions: parsePositiveInt(embedding.dimensions ?? cfg.dimensions),
4042+
// Intentionally no top-level fallback: requestDimensions is request-only.
4043+
requestDimensions: parsePositiveInt(embedding.requestDimensions),
40364044
omitDimensions:
40374045
typeof embedding.omitDimensions === "boolean"
40384046
? embedding.omitDimensions
@@ -4255,10 +4263,9 @@ export { getDefaultMdMirrorDir };
42554263
* @public
42564264
*/
42574265
export function resetRegistration() {
4258-
// Note: WeakSets cannot be cleared by design. In test scenarios where the
4259-
// same process reloads the module, a fresh module state means a new WeakSet.
4260-
// For hot-reload scenarios, the module is re-imported fresh.
4261-
// (WeakSet.clear() does not exist, so we do nothing here.)
4266+
_registeredApis = new WeakSet<OpenClawPluginApi>();
4267+
_singletonState = null;
4268+
_hookEventDedup.clear();
42624269
}
42634270

42644271
export default memoryLanceDBProPlugin;

openclaw.plugin.json

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,17 @@
4747
},
4848
"dimensions": {
4949
"type": "integer",
50-
"minimum": 1
50+
"minimum": 1,
51+
"description": "Internal vector dimensions for LanceDB schema sizing and local embedding validation"
52+
},
53+
"requestDimensions": {
54+
"type": "integer",
55+
"minimum": 1,
56+
"description": "Optional dimensions/output_dimension value to send to embedding providers that support variable output sizes"
5157
},
5258
"omitDimensions": {
5359
"type": "boolean",
54-
"description": "When true, omit the dimensions parameter from embedding requests even if dimensions is configured"
60+
"description": "When true, omit dimensions/output_dimension from embedding requests even if requestDimensions is configured"
5561
},
5662
"taskQuery": {
5763
"type": "string",
@@ -892,14 +898,20 @@
892898
"advanced": true
893899
},
894900
"embedding.dimensions": {
895-
"label": "Vector Dimensions",
901+
"label": "Schema Dimensions",
896902
"placeholder": "auto-detected from model",
897-
"help": "Override vector dimensions for custom models not in the built-in lookup table",
903+
"help": "Internal vector dimensions used for LanceDB schema sizing and local embedding validation. Override this for custom models not in the built-in lookup table.",
904+
"advanced": true
905+
},
906+
"embedding.requestDimensions": {
907+
"label": "Request Dimensions",
908+
"placeholder": "omit by default",
909+
"help": "Optional dimensions/output_dimension value to send to the embedding API. If unset, no request-side dimensions field is sent.",
898910
"advanced": true
899911
},
900912
"embedding.omitDimensions": {
901913
"label": "Omit Request Dimensions",
902-
"help": "Do not send the dimensions parameter to the embedding API even if embedding.dimensions is configured. Useful for local models like Qwen3-Embedding that reject the field.",
914+
"help": "Do not send dimensions/output_dimension to the embedding API even if embedding.requestDimensions is configured. Useful for local models like Qwen3-Embedding that reject the field.",
903915
"advanced": true
904916
},
905917
"embedding.taskQuery": {

src/embedder.ts

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,10 @@ export interface EmbeddingConfig {
105105
apiKey: string | string[];
106106
model: string;
107107
baseURL?: string;
108+
/** Internal vector dimension for schema sizing and local validation. */
108109
dimensions?: number;
110+
/** Optional API request output dimension for providers that support it. */
111+
requestDimensions?: number;
109112

110113
/** Optional task type for query embeddings (e.g. "retrieval.query") */
111114
taskQuery?: string;
@@ -434,6 +437,14 @@ export function getVectorDimensions(model: string, overrideDims?: number): numbe
434437
return dims;
435438
}
436439

440+
export function getEffectiveVectorDimensions(
441+
model: string,
442+
dimensions?: number,
443+
requestDimensions?: number,
444+
): number {
445+
return getVectorDimensions(model, requestDimensions ?? dimensions);
446+
}
447+
437448
// ============================================================================
438449
// Embedder Class
439450
// ============================================================================
@@ -471,7 +482,7 @@ export class Embedder {
471482
this._taskQuery = config.taskQuery;
472483
this._taskPassage = config.taskPassage;
473484
this._normalized = config.normalized;
474-
this._requestDimensions = config.dimensions;
485+
this._requestDimensions = config.requestDimensions;
475486
this._omitDimensions = config.omitDimensions === true;
476487
// Enable auto-chunking by default for better handling of long documents
477488
this._autoChunk = config.chunking !== false;
@@ -515,7 +526,11 @@ export class Embedder {
515526
console.log(`[memory-lancedb-pro] Initialized ${this.clients.length} API keys for round-robin rotation`);
516527
}
517528

518-
this.dimensions = getVectorDimensions(config.model, config.dimensions);
529+
this.dimensions = getEffectiveVectorDimensions(
530+
config.model,
531+
config.dimensions,
532+
config.requestDimensions,
533+
);
519534
this._cache = new EmbeddingCache(256, 30); // 256 entries, 30 min TTL
520535
}
521536

test/embedder-error-hints.test.mjs

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ async function run() {
130130
installMockEmbeddingClient(jinaEmbedder, async (payload) => {
131131
assert.equal(payload.task, "retrieval.passage");
132132
assert.equal(payload.normalized, true);
133-
assert.equal(payload.dimensions, 1024);
133+
assert.equal(payload.dimensions, undefined, "jina should not send dimensions unless requestDimensions is set");
134134
return createEmbeddingResponse(1024);
135135
});
136136
await jinaEmbedder.embedPassage("hello");
@@ -144,7 +144,7 @@ async function run() {
144144
});
145145
installMockEmbeddingClient(genericEmbedder, async (payload) => {
146146
assert.equal(payload.encoding_format, "float");
147-
assert.equal(payload.dimensions, 384);
147+
assert.equal(payload.dimensions, undefined, "generic profile should not send dimensions unless requestDimensions is set");
148148
return createEmbeddingResponse(384);
149149
});
150150
await genericEmbedder.embedPassage("hello");
@@ -196,7 +196,7 @@ async function run() {
196196
apiKey: "test-key",
197197
model: "voyage-4-lite",
198198
baseURL: "https://api.voyageai.com/v1",
199-
dimensions: 512,
199+
requestDimensions: 512,
200200
});
201201
installMockEmbeddingClient(voyageDimEmbedder, async (payload) => {
202202
assert.equal(payload.output_dimension, 512, "voyage should send output_dimension");
@@ -211,7 +211,7 @@ async function run() {
211211
await withEmbeddingCaptureServer(
212212
(payload) => {
213213
assert.equal(payload.encoding_format, "float", "generic profile should send encoding_format");
214-
assert.equal(payload.dimensions, 384, "generic profile should send dimensions");
214+
assert.equal(payload.dimensions, undefined, "generic profile should not send dimensions by default");
215215
assert.equal(payload.task, undefined, "generic profile should not send task");
216216
assert.equal(payload.normalized, undefined, "generic profile should not send normalized");
217217
return { body: createEmbeddingResponse(384) };
@@ -228,6 +228,24 @@ async function run() {
228228
},
229229
);
230230

231+
await withEmbeddingCaptureServer(
232+
(payload) => {
233+
assert.equal(payload.encoding_format, "float", "generic profile should send encoding_format");
234+
assert.equal(payload.dimensions, 384, "generic profile should send dimensions when requestDimensions is set");
235+
return { body: createEmbeddingResponse(384) };
236+
},
237+
async ({ baseURL }) => {
238+
const embedder = new Embedder({
239+
provider: "openai-compatible",
240+
apiKey: "test-key",
241+
model: "text-embedding-3-small",
242+
baseURL,
243+
requestDimensions: 384,
244+
});
245+
await embedder.embedPassage("hello world");
246+
},
247+
);
248+
231249
await withJsonServer(
232250
403,
233251
{ error: { message: "Invalid API key", code: "invalid_api_key" } },

test/plugin-manifest-regression.mjs

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ Module._initPaths();
1717

1818
const jiti = jitiFactory(import.meta.url, { interopDefault: true });
1919
const plugin = jiti("../index.ts");
20+
const resetRegistration = plugin.resetRegistration ?? (() => {});
2021

2122
const manifest = JSON.parse(
2223
readFileSync(new URL("../openclaw.plugin.json", import.meta.url), "utf8"),
@@ -109,6 +110,15 @@ assert.equal(
109110
"boolean",
110111
"embedding.omitDimensions should be declared in the plugin schema",
111112
);
113+
assert.equal(
114+
manifest.configSchema.properties.embedding.properties.requestDimensions?.type,
115+
"integer",
116+
"embedding.requestDimensions should be declared in the plugin schema",
117+
);
118+
assert.ok(
119+
Object.prototype.hasOwnProperty.call(manifest.uiHints, "embedding.requestDimensions"),
120+
"uiHints should expose embedding.requestDimensions",
121+
);
112122
assert.equal(
113123
manifest.configSchema.properties.sessionMemory.properties.enabled.default,
114124
false,
@@ -149,6 +159,7 @@ try {
149159
},
150160
{ services },
151161
);
162+
resetRegistration();
152163
plugin.register(api);
153164
assert.equal(services.length, 1, "plugin should register its background service");
154165
assert.equal(typeof api.hooks.agent_end, "function", "autoCapture should remain enabled by default");
@@ -171,6 +182,7 @@ try {
171182
dimensions: 1536,
172183
},
173184
});
185+
resetRegistration();
174186
plugin.register(sessionDefaultApi);
175187
// selfImprovement registers command:new by default (#391), independent of sessionMemory config
176188
assert.equal(
@@ -192,6 +204,7 @@ try {
192204
dimensions: 1536,
193205
},
194206
});
207+
resetRegistration();
195208
plugin.register(sessionEnabledApi);
196209
assert.equal(
197210
typeof sessionEnabledApi.hooks.before_reset,
@@ -265,6 +278,7 @@ try {
265278
chunking: false,
266279
},
267280
});
281+
resetRegistration();
268282
plugin.register(chunkingOffApi);
269283
const chunkingOffTool = chunkingOffApi.toolFactories.memory_store({
270284
agentId: "main",
@@ -293,6 +307,7 @@ try {
293307
chunking: true,
294308
},
295309
});
310+
resetRegistration();
296311
plugin.register(chunkingOnApi);
297312
const chunkingOnTool = chunkingOnApi.toolFactories.memory_store({
298313
agentId: "main",
@@ -320,21 +335,57 @@ try {
320335
dimensions: 4,
321336
},
322337
});
338+
resetRegistration();
323339
plugin.register(withDimensionsApi);
324340
const withDimensionsTool = withDimensionsApi.toolFactories.memory_store({
325341
agentId: "main",
326342
sessionKey: "agent:main:test",
327343
});
328344
const requestCountBeforeWithDimensions = embeddingRequests.length;
329345
await withDimensionsTool.execute("tool-3", {
330-
text: "dimensions should be sent by default",
346+
text: "dimensions should stay internal by default",
331347
scope: "global",
332348
});
333349
const withDimensionsRequest = embeddingRequests.at(requestCountBeforeWithDimensions);
334350
assert.equal(
335-
withDimensionsRequest?.dimensions,
351+
Object.prototype.hasOwnProperty.call(withDimensionsRequest ?? {}, "dimensions"),
352+
false,
353+
"embedding.dimensions should be used for local schema sizing, not forwarded by default",
354+
);
355+
356+
const withRequestDimensionsApi = createMockApi({
357+
dbPath: path.join(workDir, "db-with-request-dimensions"),
358+
autoCapture: false,
359+
autoRecall: false,
360+
embedding: {
361+
provider: "openai-compatible",
362+
apiKey: "dummy",
363+
model: "text-embedding-3-small",
364+
baseURL: embeddingBaseURL,
365+
requestDimensions: 4,
366+
},
367+
});
368+
resetRegistration();
369+
plugin.register(withRequestDimensionsApi);
370+
const withRequestDimensionsTool = withRequestDimensionsApi.toolFactories.memory_store({
371+
agentId: "main",
372+
sessionKey: "agent:main:test",
373+
});
374+
const requestCountBeforeRequestDimensions = embeddingRequests.length;
375+
const withRequestDimensionsResult = await withRequestDimensionsTool.execute("tool-3b", {
376+
text: "requestDimensions should drive both request payload and local schema size",
377+
scope: "global",
378+
});
379+
assert.equal(
380+
withRequestDimensionsResult.details.action,
381+
"created",
382+
"requestDimensions-only config should still create memories end-to-end",
383+
);
384+
const withRequestDimensionsRequest = embeddingRequests.at(requestCountBeforeRequestDimensions);
385+
assert.equal(
386+
withRequestDimensionsRequest?.dimensions,
336387
4,
337-
"embedding.dimensions should be forwarded by default",
388+
"embedding.requestDimensions should be forwarded to embedding requests",
338389
);
339390

340391
const omitDimensionsApi = createMockApi({
@@ -346,10 +397,11 @@ try {
346397
apiKey: "dummy",
347398
model: "text-embedding-3-small",
348399
baseURL: embeddingBaseURL,
349-
dimensions: 4,
400+
requestDimensions: 4,
350401
omitDimensions: true,
351402
},
352403
});
404+
resetRegistration();
353405
plugin.register(omitDimensionsApi);
354406
const omitDimensionsTool = omitDimensionsApi.toolFactories.memory_store({
355407
agentId: "main",
@@ -364,7 +416,7 @@ try {
364416
assert.equal(
365417
Object.prototype.hasOwnProperty.call(omitDimensionsRequest, "dimensions"),
366418
false,
367-
"embedding.omitDimensions=true should omit dimensions from embedding requests",
419+
"embedding.omitDimensions=true should omit dimensions from embedding requests even when requestDimensions is set",
368420
);
369421
} finally {
370422
await new Promise((resolve) => embeddingServer.close(resolve));

test/reflection-bypass-hook.test.mjs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ const jiti = jitiFactory(import.meta.url, {
1717

1818
const pluginModule = jiti("../index.ts");
1919
const memoryLanceDBProPlugin = pluginModule.default || pluginModule;
20+
const resetRegistration = pluginModule.resetRegistration ?? (() => {});
2021
const { MemoryStore } = jiti("../src/store.ts");
2122
const { storeReflectionToLanceDB } = jiti("../src/reflection-store.ts");
2223

@@ -134,9 +135,11 @@ describe("reflection hooks tolerate bypass scope filters", () => {
134135

135136
beforeEach(() => {
136137
workDir = mkdtempSync(path.join(tmpdir(), "reflection-bypass-hook-"));
138+
resetRegistration();
137139
});
138140

139141
afterEach(() => {
142+
resetRegistration();
140143
rmSync(workDir, { recursive: true, force: true });
141144
});
142145

0 commit comments

Comments
 (0)