Repository navigation
fix(sdk): bound withMemWal recall and auto-save turns to 16 KiB UTF-8 (Fixes #1037) - #1329
Conversation
|
@ducnmm Ready for review — bounds |
HoangDucBach
left a comment
There was a problem hiding this comment.
Issue counts by severity
bugs: 1
suggestions: 2
nits: 0
The recall fix meets #1037, but the auto-save change goes beyond the ticket and drops facts that /api/analyze accepts today. Fix the analyze() bound first.
Verified: Built the PR head, ran test/withMemWal-long-turn.test.mjs (4/4 pass) and probed truncateUtf8Bytes with 2-, 3- and 4-byte codepoints and leading whitespace (all within 16,384 bytes, no U+FFFD). Not verified: behavior against a live relayer, and the full 373-test suite from the PR description.
| let cachedRecall: { key: string; context: string } | null = null; | ||
|
|
||
| function saveInBackground(userMessage: string): void { | ||
| const boundedMessage = truncateUtf8Bytes(userMessage, MAX_EMBED_INPUT_BYTES); |
There was a problem hiding this comment.
[bug]: saveInBackground truncates the auto-save payload to 16,384 bytes, but /api/analyze accepts up to MAX_ANALYZE_TEXT_BYTES = 64 KiB (services/server/src/routes/analyze.rs L100) and already handles 16-64 KiB input by skipping only the pre-extract embed (L133-142). #1037 is about recall() only (the 400 comes from the embedder). A 20 KiB turn that analyze handled before now loses every fact after byte 16,384, silently.
Fix: remove the truncation here, or bound at 64 KiB with a separate constant, and keep MAX_EMBED_INPUT_BYTES for the recall query. Reasoned from the server code, not run against a relayer. The #1037 comment citing MAX_ANALYZE_TEXT_BYTES as 16,384 is also wrong.
| * Bound `text` to at most `maxBytes` UTF-8 bytes without splitting a | ||
| * multi-byte UTF-8 codepoint across the boundary. (#1037) | ||
| */ | ||
| export function truncateUtf8Bytes( |
There was a problem hiding this comment.
[suggestion]: truncateUtf8Bytes keeps only the head of the turn. For "paste 20 KB, then ask a question" the question is cut and the recall query (L201) is the start of the paste (probed: question kept = false for a 20,000-byte paste followed by a short question).
Keep the tail, or head + tail, for the recall query. Reasoned from the probe, not run with a live recall.
| - `MemWal.listNamespaces` and `MemWal.listMemories` reject a `limit` that is not a positive integer before signing the query string, with the same `limit must be a positive integer, got <limit>` `TypeError` as `recall` and `recallManual`. (#1213) | ||
| - `analyze()` accepts an optional `signal` in `AnalyzeOptions` and aborts the in-flight `/api/analyze` HTTP request when the caller's `AbortSignal` fires. (#1095) | ||
| - `MemWal.rememberManual`, `MemWal.recallManual`, `MemWalManual.rememberManual`, and `MemWalManual.recallManual` reject an empty or non-finite `vector` (including `NaN`, `±Infinity`, non-number elements, and sparse arrays) before signing, and `rememberManual` rejects a missing or blank `encryptedData` string with `TypeError`. (#1211) | ||
| - `withMemWal` bounds the recall query and auto-save `analyze()` input derived from the last user message to 16,384 UTF-8 bytes without splitting multi-byte UTF-8 codepoints, while keeping the full user turn in the language model prompt. User turns exceeding the relayer's 16 KiB embedding limit still inject relevant memories and auto-save facts instead of failing with HTTP 400. (#1037) |
There was a problem hiding this comment.
[suggestion]: This entry is under 0.1.11, which is already published on npm (2026-10-28), so the changelog describes a release that does not contain this fix. The same applies to docs/sdk/changelog.mdx L47. Move it to the next version section if the release verifier allows it; I did not check how verify-manual-sdk-release.mjs treats the version header.
8c860ab to
ff4b135
Compare
|
@HoangDucBach @ducnmm Updated to address review feedback (commit ff4b135):
|
Summary
Fixes #1037
MAX_EMBED_INPUT_BYTES(16_384) and an isomorphictruncateUtf8Byteshelper (TextEncoder/TextDecoder, zero Node builtins) inpackages/sdk/src/ai/middleware.tsthat bounds UTF-8 strings to at most 16,384 bytes without splitting multi-byte codepoints (2-, 3-, or 4-byte UTF-8 sequences) across the boundary.memwal.recall(...)query intransformParamsand the backgroundmemwal.analyze(...)payload insaveInBackground(wrapGenerateandwrapStream) to16_384UTF-8 bytes while preserving the full untruncated user turn in the prompt sent to the language model.packages/sdk/CHANGELOG.mdanddocs/sdk/changelog.mdxunder0.1.11.Verification
pnpm --filter @mysten-incubation/memwal buildpnpm --filter @mysten-incubation/memwal test(373/373 passing, including newtest/withMemWal-long-turn.test.mjs)node scripts/verify-manual-sdk-release.mjs --base origin/dev --head HEADnode scripts/check-docs-code-sync.mjsnode scripts/check-docs-freshness.mjs