Skip to content

Commit f311041

Browse files
5queezerclaude
andcommitted
fix: address review round 3 (3 medium, 1 low)
- migrate-memories.mjs: pass apiKey for LanceDB Cloud URIs (medium #1) - Throw on old schema without scope column instead of silent warn (medium #2) - Log hint when rerank API key is present but RERANK_PROVIDER unset (medium qwibitai#3) - Validate vectorDim early for custom providers (low qwibitai#4) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4e67441 commit f311041

3 files changed

Lines changed: 33 additions & 7 deletions

File tree

container/agent-runner/src/memory-store.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -204,16 +204,20 @@ export class MemoryStore {
204204
try {
205205
table = await db.openTable(TABLE_NAME);
206206

207-
// Check if we need to add scope column for backward compatibility
207+
// Check if existing data has scope column (backward compatibility)
208208
try {
209209
const sample = await table.query().limit(1).toArray();
210210
if (sample.length > 0 && !("scope" in sample[0])) {
211-
console.warn(
212-
"Adding scope column for backward compatibility with existing data",
211+
throw new Error(
212+
`Existing LanceDB table at "${this.config.dbPath}" uses old schema without "scope" column.\n` +
213+
` Fix: Re-embed your memories using: node scripts/migrate-memories.mjs <backup.jsonl> "${this.config.dbPath}"\n` +
214+
` Or delete the table directory and let it be recreated on first store.`,
213215
);
214216
}
215-
} catch (err) {
216-
console.warn("Could not check table schema:", err);
217+
} catch (err: any) {
218+
// Re-throw schema migration errors, only swallow probe failures
219+
if (err?.message?.includes('old schema')) throw err;
220+
console.warn("Could not check table schema:", err instanceof Error ? err.message : String(err));
217221
}
218222
} catch (_openErr) {
219223
// Table doesn't exist yet — create it

container/agent-runner/src/memory.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,19 @@ const RERANK_DEFAULTS: Record<string, RerankDefaults> = {
147147
function resolveRetrievalConfig(): Partial<RetrievalConfig> {
148148
const provider = RERANK_PROVIDER.toLowerCase();
149149
if (!provider || provider === 'none') {
150+
// Hint: if a rerank-capable API key is present but RERANK_PROVIDER is unset
151+
const hintKeys: Array<[string, string]> = [
152+
['JINA_API_KEY', 'jina'],
153+
['VOYAGE_API_KEY', 'voyage'],
154+
['PINECONE_API_KEY', 'pinecone'],
155+
['SILICONFLOW_API_KEY', 'siliconflow'],
156+
];
157+
for (const [envKey, providerName] of hintKeys) {
158+
if (process.env[envKey]) {
159+
console.log(`[memory] Hint: ${envKey} is set but RERANK_PROVIDER is not — set RERANK_PROVIDER=${providerName} to enable cross-encoder reranking`);
160+
break;
161+
}
162+
}
150163
return { rerank: 'none' };
151164
}
152165

@@ -182,9 +195,15 @@ let _retriever: MemoryRetriever | null = null;
182195
function getStore(): MemoryStore {
183196
const embeddingConfig = resolveEmbeddingConfig();
184197
if (!_store) {
198+
const vectorDim = embeddingConfig.dimensions || 0;
199+
if (!vectorDim) {
200+
throw new Error(
201+
`Cannot determine embedding dimensions. Set EMBEDDING_DIM when using a custom provider.`,
202+
);
203+
}
185204
_store = new MemoryStore({
186205
dbPath: LANCEDB_URI || LOCAL_DB_DIR,
187-
vectorDim: embeddingConfig.dimensions || 0,
206+
vectorDim,
188207
apiKey: LANCEDB_URI ? (LANCEDB_API_KEY || undefined) : undefined,
189208
});
190209
}

scripts/migrate-memories.mjs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,10 @@ async function main() {
7171
console.log(`Embedding model: ${EMBEDDING_MODEL}`);
7272
console.log(`Embedding endpoint: ${BASE_URL}`);
7373

74-
const db = await lancedb.connect(lancedbDir);
74+
const connectOpts = lancedbDir.startsWith('db://') && process.env.LANCEDB_API_KEY
75+
? { apiKey: process.env.LANCEDB_API_KEY }
76+
: undefined;
77+
const db = await lancedb.connect(lancedbDir, connectOpts);
7578

7679
// Stream lines to avoid loading entire file into memory
7780
const rl = createInterface({

0 commit comments

Comments
 (0)