Skip to content

fix(core): storage hardening — Buffer safety, embedding abort, atomic instanceId#461

Open
rainforest888 wants to merge 2 commits into
TencentCloud:mainfrom
rainforest888:fix/core-storage
Open

fix(core): storage hardening — Buffer safety, embedding abort, atomic instanceId#461
rainforest888 wants to merge 2 commits into
TencentCloud:mainfrom
rainforest888:fix/core-storage

Conversation

@rainforest888

Copy link
Copy Markdown

fix(offload): Tracing & Token Hardening

拆分自 #391,per YOMXXX review

修复范围

聚焦 src/offload/ 层:token estimate、local LLM、Opik tracing。不涉及 store、runtime、gateway 层。

具体修复

src/offload/fast-token-estimate.ts — Astral-平面 Token 修正

问题 修复
astral-plane 字符(emoji / CJK extension B,U+10000+)编码为 surrogate pair,原实现把 high surrogate 和 low surrogate 各计为一个 token high surrogate 检测后跳过低代理(i += 2),修正 ~2× token 高估

src/offload/local-llm/index.ts — L1.5 Fallback 修正

问题 修复
解析失败返回 {taskCompleted: false, ...} — normalizeJudgment 把 false 当有效判断,永远不触发 "LLM 不可用" 重试 改为 {taskCompleted: null, ...}== null 检查触发重试路径

src/offload/opik-tracer.ts — ESM 兼容 + Trace 截断

问题 修复
ESM 下 require("opik")undefined,tracer 永久静默关闭 改为 createRequire(import.meta.url)
未脱敏 tool 输出(文件内容、环境变量)完整写入 trace,泄露到 Opik backend 统一截断到 2000 字符

验证

@Maxwell-Code07

Copy link
Copy Markdown
Collaborator

Thank you for your contribution! We will review your code and get back to you as soon as possible.

@YOMXXX YOMXXX left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the focused core hardening split. I verified the branch locally with:

  • pnpm test — passed: 4 files, 67 tests
  • pnpm build — passed
  • git diff --check HEAD~2..HEAD — passed
  • a local Float32Array subarray Buffer check — passed

I think this still needs one change before approval: the new EmbeddingCallOptions.abortSignal is wired into the embedding HTTP fetch, but the actual auto-recall timeout path does not pass an abort signal. performAutoRecall() still uses Promise.race with a timer that resolves after recall.timeoutMs; once that race resolves, performAutoRecallInner() and the embedding HTTP request can continue running until the embedding request's own timeout. With defaults, recall.timeoutMs is 5000ms while embedding.timeoutMs is 10000ms and embedding.recallTimeoutMs may be unset, so the PR does not fully deliver the documented "recall timeout aborts the in-flight embedding request" behavior.

Please create an AbortController for the recall timeout path and pass its signal through searchMemories() / EmbeddingCallOptions so the same timeout that skips memory injection also cancels the embedding fetch. The Buffer byteOffset/byteLength fixes and the instance-id in-flight guard look directionally good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants