Skip to content

Commit 95445da

Browse files
committed
Address AliceLJY review round 2
Must fix: - Flat memory scan: move before the mdFiles.length===0 early return so it is always reachable (not just when nested workspaces are empty) - Tests: runImportMarkdown now uses embedPassage (not embedQuery) and JSON.stringify(metadata) to match production. Added embedPassage mock. - Tests: setupWorkspace now creates files at workspace/<name>/ to match the actual path structure runImportMarkdown expects Worth considering: - Flat memory scan now skips when workspaceGlob is set, avoiding accidental root flat memory import when user specifies --workspace - Removed dev artifacts: ANALYSIS.md and recall-benchmark.py contained personal absolute paths and are not suitable for repo commit
1 parent ae11de7 commit 95445da

File tree

4 files changed

+38
-473
lines changed

4 files changed

+38
-473
lines changed

cli.ts

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,31 +1138,32 @@ export function registerMemoryCLI(program: Command, context: CLIContext): void {
11381138
} catch { /* not found */ }
11391139
}
11401140

1141+
// Also scan the flat `workspace/memory/` directory directly under workspace root
1142+
// (not inside any workspace subdirectory — supports James's actual structure).
1143+
// This scan runs regardless of whether nested workspace mdFiles were found,
1144+
// so flat memory is always reachable even when all nested workspaces are empty.
1145+
// Skip if a specific workspace was requested (workspaceGlob), to avoid importing
1146+
// root flat memory when the user meant to import only one workspace.
1147+
if (!workspaceGlob) {
1148+
const flatMemoryDir = path.join(workspaceDir, "memory");
1149+
try {
1150+
const stats = await fsPromises.stat(flatMemoryDir);
1151+
if (stats.isDirectory()) {
1152+
const files = await fsPromises.readdir(flatMemoryDir);
1153+
for (const f of files) {
1154+
if (f.endsWith(".md") && /^\d{4}-\d{2}-\d{2}/.test(f)) {
1155+
mdFiles.push({ filePath: path.join(flatMemoryDir, f), scope: workspaceScope || "shared" });
1156+
}
1157+
}
1158+
}
1159+
} catch { /* not found */ }
1160+
}
1161+
11411162
if (mdFiles.length === 0) {
11421163
console.log("No Markdown memory files found.");
11431164
return;
11441165
}
11451166

1146-
// Also scan the flat `workspace/memory/` directory directly under workspace root
1147-
// (not inside any workspace subdirectory — supports James's actual structure)
1148-
const flatMemoryDir = path.join(workspaceDir, "memory");
1149-
try {
1150-
const stats = await fsPromises.stat(flatMemoryDir);
1151-
if (stats.isDirectory()) {
1152-
const files = await fsPromises.readdir(flatMemoryDir);
1153-
let added = 0;
1154-
for (const f of files) {
1155-
if (f.endsWith(".md") && /^\d{4}-\d{2}-\d{2}/.test(f)) {
1156-
mdFiles.push({ filePath: path.join(flatMemoryDir, f), scope: workspaceScope || "shared" });
1157-
added++;
1158-
}
1159-
}
1160-
if (added > 0) {
1161-
console.log(`Found ${added} entries in flat memory directory (scope: memory).`);
1162-
}
1163-
}
1164-
} catch { /* not found */ }
1165-
11661167
const targetScope = options.scope || "global";
11671168
const minTextLength = parseInt(options.minTextLength ?? "5", 10);
11681169
const importanceDefault = parseFloat(options.importance ?? "0.7");

test/import-markdown/ANALYSIS.md

Lines changed: 0 additions & 267 deletions
This file was deleted.

test/import-markdown/import-markdown.test.mjs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,17 @@ const mockEmbedder = {
2323
}
2424
return vec;
2525
}),
26+
embedPassage: jest.fn(async (text) => {
27+
// Use same deterministic vector as embedQuery for test consistency
28+
const dim = 384;
29+
const vec = [];
30+
let seed = hashString(text);
31+
for (let i = 0; i < dim; i++) {
32+
seed = (seed * 1664525 + 1013904223) & 0xffffffff;
33+
vec.push((seed >>> 8) / 16777215 - 1);
34+
}
35+
return vec;
36+
}),
2637
};
2738

2839
const mockStore = {
@@ -63,7 +74,9 @@ import { tmpdir } from "node:os";
6374
let testWorkspaceDir;
6475

6576
async function setupWorkspace(name) {
66-
const wsDir = join(testWorkspaceDir, name);
77+
// Files must be created at: <testWorkspaceDir>/workspace/<name>/
78+
// because runImportMarkdown looks for path.join(openclawHome, "workspace")
79+
const wsDir = join(testWorkspaceDir, "workspace", name);
6780
await mkdir(wsDir, { recursive: true });
6881
return wsDir;
6982
}
@@ -82,6 +95,7 @@ beforeAll(async () => {
8295
afterEach(async () => {
8396
mockStore.reset();
8497
mockEmbedder.embedQuery.mockClear();
98+
mockEmbedder.embedPassage.mockClear();
8599
});
86100

87101
afterAll(async () => {
@@ -373,14 +387,14 @@ async function runImportMarkdown(context, options = {}) {
373387
}
374388

375389
try {
376-
const vector = await context.embedder.embedQuery(text);
390+
const vector = await context.embedder.embedPassage(text);
377391
await context.store.store({
378392
text,
379393
vector,
380394
importance,
381395
category: "other",
382396
scope,
383-
metadata: { importedFrom: filePath, sourceScope: srcScope },
397+
metadata: JSON.stringify({ importedFrom: filePath, sourceScope: srcScope }),
384398
});
385399
imported++;
386400
} catch (err) {

0 commit comments

Comments
 (0)