perf(memory): copy only requested embedding dimensions by ly-wang19 · Pull Request #96952 · openclaw/openclaw

ly-wang19 · 2026-06-26T05:47:05Z

What Problem This Solves

Local embedding normalization truncated vectors with Array.from(vector).slice(0, outputDimensionality). That copies the full embedding vector before discarding the tail, so large vectors still pay the full read/copy cost even when a smaller output dimensionality is requested.

Why This Change Was Made

This adds a bounded vector copy helper that copies only the requested dimensions before normalization. The existing no-truncation path still copies the full vector into a normal number array for sanitize/normalize behavior.

User Impact

Local memory embeddings with outputDimensionality configured avoid unnecessary tail-coordinate reads and full-vector copies before normalization. Normal embedding values and normalization semantics are intended to stay unchanged.

Evidence

node scripts/run-vitest.mjs packages/memory-host-sdk/src/host/embeddings.test.ts
.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main --parallel-tests "node scripts/run-vitest.mjs packages/memory-host-sdk/src/host/embeddings.test.ts"
autoreview clean: no accepted/actionable findings reported; tests exit 0

clawsweeper · 2026-06-26T05:53:11Z

Codex review: needs real behavior proof before merge. Reviewed June 26, 2026, 5:50 PM ET / 21:50 UTC.

Summary
The PR changes local memory embedding normalization to copy only the requested output dimensions before sanitizing and normalizing, with a regression test for not reading tail coordinates.

Reproducibility: yes. by source inspection: current main converts the full local embedding vector before truncating to a positive outputDimensionality. I did not run a live local embedding setup in this read-only review.

Review metrics: 1 noteworthy metric.

Potential merge surface: 2 files, +33/-4. The live PR file list is stale-base noisy, but the potential merge commit against current main is the narrow memory-host change reviewers should judge.

Stored data model
Persistent data-model change detected: migration/backfill/repair: docs/cli/update.md, persistent cache schema: extensions/memory-core/src/memory/qmd-manager.ts, persistent cache schema: extensions/memory-core/src/memory/qmd-runtime-cache.test.ts, persistent cache schema: extensions/memory-core/src/memory/qmd-runtime-cache.ts, serialized state: extensions/deepinfra/image-generation-provider.test.ts, serialized state: extensions/google/image-generation-provider.test.ts, and 37 more. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P1] Add redacted terminal output, copied live output, logs, or a recording showing a real local embedding run with outputDimensionality after this change.

Proof guidance:

[P1] Needs real behavior proof before merge: The PR body lists a unit test and autoreview, but it does not include redacted terminal output, copied live output, logs, a recording, or a linked artifact from a real local embedding run; after adding proof, updating the PR body should trigger a fresh review, or a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P1] The contributor has not supplied after-fix real behavior proof from a real local embedding setup; unit tests and autoreview output are supplemental only for this external PR gate.

Maintainer options:

Decide the mitigation before merge
Keep the focused bounded-copy optimization, but require redacted real local embedding output or logs before merge.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] Manual review remains because the blocker is contributor-supplied real behavior proof, not a code defect an automated repair worker can provide.

Security
Cleared: The actual potential merge diff changes only local memory embedding TypeScript code and a colocated test, with no dependency, workflow, credential, package, or install surface changes.

Review details

Best possible solution:

Keep the focused bounded-copy optimization, but require redacted real local embedding output or logs before merge.

Do we have a high-confidence way to reproduce the issue?

Yes by source inspection: current main converts the full local embedding vector before truncating to a positive outputDimensionality. I did not run a live local embedding setup in this read-only review.

Is this the best way to solve the issue?

Yes for the code shape: copying the requested prefix before sanitize/normalize is the narrowest fix on the in-process local provider path. It is not merge-ready until the contributor adds real behavior proof.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against d4a01e48bcc5.

Label changes

Label justifications:

P3: This is a narrow local memory embedding performance cleanup with limited blast radius and no urgent user-facing outage.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body lists a unit test and autoreview, but it does not include redacted terminal output, copied live output, logs, a recording, or a linked artifact from a real local embedding run; after adding proof, updating the PR body should trigger a fresh review, or a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

What I checked:

Root policy applied: Root AGENTS.md was read fully; the review applied its proof, best-fix, compatibility, and current-main requirements. (AGENTS.md:31, d4a01e48bcc5)
Current main copies the full vector before truncating: Current main calls Array.from(vector) before slice when outputDimensionality is set, so the full ArrayLike vector is read before the tail is discarded. (packages/memory-host-sdk/src/host/embeddings.ts:148, d4a01e48bcc5)
Actual merge result is narrow: The potential merge commit against current main changes only packages/memory-host-sdk/src/host/embeddings.ts and packages/memory-host-sdk/src/host/embeddings.test.ts, for +33/-4 total. (b2c869bdd99a)
Patch copies only the requested prefix: The merge patch adds copyEmbeddingVector and passes outputDimensionality to it before sanitizeAndNormalizeEmbedding, avoiding tail reads on the truncation path. (packages/memory-host-sdk/src/host/embeddings.ts:31, b2c869bdd99a)
Positive dimension contract preserves semantics: The config schema accepts outputDimensionality only as a positive integer, so the helper's zero-length behavior does not change a valid shipped configuration path. (src/config/zod-schema.agent-runtime.ts:895, d4a01e48bcc5)
Runtime caller path checked: The worker child resolves createLocalEmbeddingProviderInProcess, so the helper is on the local embedding worker runtime path used by local memory embeddings. (packages/memory-host-sdk/src/host/embeddings-worker-child.ts:49, d4a01e48bcc5)

Likely related people:

liuhao1024: Introduced local GGUF outputDimensionality truncation in the memory-host embedding path that this PR optimizes. (role: feature introducer; confidence: high; commits: 63d5820bebc9; files: packages/memory-host-sdk/src/host/embeddings.ts)
vincentkoc: Preserved outputDimensionality through the local embedding worker boundary and also authored the current PR head commit. (role: adjacent repair contributor; confidence: high; commits: 500d765ea402, 46822906a02c; files: packages/memory-host-sdk/src/host/embeddings-worker.ts, packages/memory-host-sdk/src/host/embeddings.ts, packages/memory-host-sdk/src/host/embeddings.test.ts)
Alix-007: Current blame on the local embedding normalization lines points to a recent memory-host snapshot commit. (role: recent area contributor; confidence: medium; commits: bfc33ac11405; files: packages/memory-host-sdk/src/host/embeddings.ts)
bek91: Recently merged QMD memory diagnostics and runtime-cache work that appears in the stale-base PR file list but not in the actual merge result. (role: adjacent memory runtime contributor; confidence: medium; commits: 9636bea901d0; files: extensions/memory-core/src/memory/qmd-manager.ts, packages/memory-host-sdk/src/host/types.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

vincentkoc · 2026-06-26T21:56:08Z

Merged via squash.

Prepared head SHA: 46822906a02cc1fbc73c4f9a174ff187d1153f7c
Landed commit: 4c4396c4c2505e8c59ef73b1aa2a1e674042deec

* perf(memory): copy only requested embedding dimensions * perf(memory): copy only requested embedding dimensions --------- Co-authored-by: ly-wang19 <[email protected]> Co-authored-by: Vincent Koc <[email protected]>

openclaw-barnacle Bot added the size: XS label Jun 26, 2026

perf(memory): copy only requested embedding dimensions

7fde731

ly-wang19 force-pushed the codex/high-quality-algo-4 branch from 905c66c to 7fde731 Compare June 26, 2026 06:03

perf(memory): copy only requested embedding dimensions

4682290

vincentkoc merged commit 4c4396c into openclaw:main Jun 26, 2026
211 of 218 checks passed

github-actions Bot mentioned this pull request Jun 26, 2026

📡 Upstream Digest — 2026-06-26 22:59 UTC curtismercier/openclaw-mods#1156

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

perf(memory): copy only requested embedding dimensions#96952

perf(memory): copy only requested embedding dimensions#96952
vincentkoc merged 2 commits into
openclaw:mainfrom
ly-wang19:codex/high-quality-algo-4

ly-wang19 commented Jun 26, 2026

Uh oh!

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

vincentkoc commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

ly-wang19 commented Jun 26, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

vincentkoc commented Jun 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

clawsweeper Bot commented Jun 26, 2026 •

edited

Loading