Skip to content

fix(memos-local-openclaw): Viewer API search limit and minScore params not working #1376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zerone0x wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(memos-local-openclaw): Viewer API search limit and minScore params not working #1376

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions apps/memos-local-openclaw/src/viewer/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,9 @@ export class ViewerServer {
const q = url.searchParams.get("q") ?? "";
if (!q.trim()) { this.jsonResponse(res, { results: [], query: q }); return; }

const limit = Math.min(100, Math.max(1, Number(url.searchParams.get("limit")) || 20));
const minScore = Math.max(0.35, Math.min(1, Number(url.searchParams.get("minScore")) || 0.64));
Comment on lines +773 to +774
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The query param parsing uses Number(...) || default, which treats 0 as “missing”. That makes limit=0 default to 20 (instead of clamping to 1) and minScore=0 default to 0.64 (instead of clamping to 0.35). Consider parsing with Number.isFinite and applying the default only when the param is absent/NaN, then clamp the resulting value.

Suggested change
const limit = Math.min(100, Math.max(1, Number(url.searchParams.get("limit")) || 20));
const minScore = Math.max(0.35, Math.min(1, Number(url.searchParams.get("minScore")) || 0.64));
const rawLimit = Number(url.searchParams.get("limit"));
const limit = Math.min(100, Math.max(1, Number.isFinite(rawLimit) ? rawLimit : 20));
const rawMinScore = Number(url.searchParams.get("minScore"));
const minScore = Math.max(0.35, Math.min(1, Number.isFinite(rawMinScore) ? rawMinScore : 0.64));

Copilot uses AI. Check for mistakes.
Comment on lines +773 to +774
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This endpoint change is user-visible (new limit/minScore behavior + response fields) but there doesn’t appear to be coverage for /api/search query-param handling. Given the repo already has integration tests that spin up ViewerServer and call endpoints via fetch, please add a test asserting (a) results are truncated to limit and (b) minScore affects semantic hit inclusion (and that values are clamped to the documented ranges).

Suggested change
const limit = Math.min(100, Math.max(1, Number(url.searchParams.get("limit")) || 20));
const minScore = Math.max(0.35, Math.min(1, Number(url.searchParams.get("minScore")) || 0.64));
const rawLimit = Number(url.searchParams.get("limit"));
const limit = Math.min(100, Math.max(1, Number.isFinite(rawLimit) ? rawLimit : 20));
const rawMinScore = Number(url.searchParams.get("minScore"));
const minScore = Math.max(0.35, Math.min(1, Number.isFinite(rawMinScore) ? rawMinScore : 0.64));

Copilot uses AI. Check for mistakes.

const role = url.searchParams.get("role") ?? undefined;
const session = url.searchParams.get("session") ?? undefined;
const owner = url.searchParams.get("owner") ?? undefined;
Expand Down Expand Up @@ -810,7 +813,7 @@ export class ViewerServer {
}
}

const SEMANTIC_THRESHOLD = 0.64;
const SEMANTIC_THRESHOLD = minScore;
const VECTOR_TIMEOUT_MS = 8000;
let vectorResults: any[] = [];
let scoreMap = new Map<string, number>();
Expand Down Expand Up @@ -849,7 +852,7 @@ export class ViewerServer {
if (!seenIds.has(r.id)) { seenIds.add(r.id); merged.push(r); }
}

const results = merged.length > 0 ? merged : ftsResults.slice(0, 20);
const results = (merged.length > 0 ? merged : ftsResults.slice(0, limit)).slice(0, limit);
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merged always includes ftsResults (the second loop appends them unconditionally), so the merged.length > 0 ? merged : ftsResults.slice(...) branch is effectively redundant. Simplifying this to a single slice of merged would make the intent clearer and avoid implying there’s a distinct fallback path.

Suggested change
const results = (merged.length > 0 ? merged : ftsResults.slice(0, limit)).slice(0, limit);
const results = merged.slice(0, limit);

Copilot uses AI. Check for mistakes.

this.store.recordViewerEvent("search");
this.jsonResponse(res, {
Expand All @@ -858,6 +861,8 @@ export class ViewerServer {
vectorCount: vectorResults.length,
ftsCount: ftsResults.length,
total: results.length,
limit,
minScore,
});
}

Expand Down
Loading