Skip to content

Fix DashScope rerank path and optimize batch embedding#566

Open
greatyue6-byte wants to merge 5 commits intoCortexReach:masterfrom
greatyue6-byte:ayu/dashscope-p0-p2-memory-integration
Open

Fix DashScope rerank path and optimize batch embedding#566
greatyue6-byte wants to merge 5 commits intoCortexReach:masterfrom
greatyue6-byte:ayu/dashscope-p0-p2-memory-integration

Conversation

@greatyue6-byte
Copy link
Copy Markdown

Summary

  • fix DashScope rerank routing to use the correct endpoint/model path
  • fix batch embedding alignment to use returned index
  • optimize DashScope batch embedding to use a single batch API call
  • confirm dashscope-embedding.ts no longer contains the old rerank residue

Included commits

  • Fix DashScope rerank routing and embedding index alignment
  • Optimize DashScope batch embedding path

Validation

  • node test/retriever-rerank-regression.mjs
  • node test/embedder-batch-index-alignment.test.mjs
  • node test/smart-memory-lifecycle.mjs

Notes

  • the dashscope-embedding.ts cleanup item was re-checked and required no code change because the residue was already gone
  • this PR keeps scope limited to DashScope integration fixes and the batch-path optimization

Copy link
Copy Markdown
Collaborator

@AliceLJY AliceLJY left a comment

Choose a reason for hiding this comment

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

代码审查通过,变更逻辑清晰,结构合理。

审查了以下内容:

  1. DashScope rerank 路径修复(P0):新增 dashscope-rerank.ts provider,endpoint 路由正确(/services/rerank/text-rerank/text-rerank),qwen3-vl-rerank 的请求体格式与通用 rerank model 区分处理,fetchDashScopeRerank 按 index 排序保证对齐——逻辑正确。

  2. Batch embedding index 对齐修复(P1)embedMany 现在使用 item.index 而非数组位置来映射结果,当 API 返回顺序与输入不一致时不会错位。fallback 到 validIndices[indexOf] 保证向后兼容。

  3. DashScope batch embedding 优化(P2)embedBatchQuery/embedBatchPassageembedMany 单次 API 调用,不再逐条 Promise.all,减少请求数。dashscope-embedding.ts 的 multimodal embedding 格式(contents 数组 + enable_fusion)与 DashScope 文档一致。

  4. 其他:provider 类型扩展、CLI 模式跳过 service 注册、darwin-x64 LanceDB fallback 错误提示、config schema 同步——均无问题。

小建议(非阻塞)isDashScopeProvider() 每次调用都重新执行 detectEmbeddingProviderProfile(),结果对同一 Embedder 实例是不变的,可以在构造函数中缓存。不影响正确性,仅供后续优化参考。

@rwmjhb
Copy link
Copy Markdown
Collaborator

rwmjhb commented Apr 9, 2026

The underlying bugs are real: wrong DashScope rerank endpoint and silent
embedding misalignment when the API returns results out of order. Both are worth
fixing. A few blockers need to be resolved first.

Must fix

  1. logReg and isCliMode are undefined.
    index.ts uses logReg(...) (replacing api.logger.info) and wraps service
    registration in if (isCliMode()), but neither identifier appears in the diff or
    anywhere in the repo. This is the most likely cause of the build failure — please
    add the definitions or revert to api.logger.info and remove the CLI-mode guard
    (it's also unrelated to the stated PR scope).

  2. Batch index alignment — empty-string case is still broken.
    The P1 fix remaps embeddings using item.index, but that index is relative to
    the filtered validTexts array, not the original texts input. For an input like
    ['', 'hello', 'world'], the provider returns indices 0, 1 for hello/world,
    but the code writes to results[0] and results[1] instead of results[1] and
    results[2]. The new test only covers out-of-order responses on non-empty inputs,
    so it doesn't catch this. The fix: track the original position through
    validIndices and use that for the write-back, not item.index directly.

  3. Abort/timeout signal is not wired through embedMany.
    embedBatchQuery and embedBatchPassage now accept and forward a signal
    argument, but embedMany(texts, task?) still ignores any signal parameter and
    passes only validTexts and task to embedWithRetry. Batch requests can't be
    cancelled.

  4. New regression test isn't wired into the test suite.
    test/embedder-batch-index-alignment.test.mjs is added but not referenced in
    the package.json test script, so CI won't run it.

Nice to have

  • buildRerankRequest still has a case 'dashscope' arm that's now unreachable
    (the new if (provider === 'dashscope') branch handles it first and returns).
    Safe to remove.
  • openclaw.plugin.json schema description references gte-rerank-v2, but the
    runtime defaults to qwen3-vl-rerank. Should align.
  • embedding.timeoutMs is parsed and passed through but never populated by
    parsePluginConfig and not exposed in the manifest schema — currently dead config.
  • The CLI-mode and logReg changes read like unrelated refactoring bundled into
    this PR. If they're intentional, they'd be easier to review in a separate PR.

Fix the blockers above and this will be in good shape.

Copy link
Copy Markdown
Collaborator

@rwmjhb rwmjhb left a comment

Choose a reason for hiding this comment

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

感谢这次修复——DashScope rerank 路径错误和 batch embedding 位置错乱都是真实 bug。有几个阻塞项:

Must Fix

EF1 — TypeScript build 失败,阻塞 CI merge

build_status=fail,怀疑根因是 src/embedder.ts 新增的 any 类型参数在 strict mode 下触发编译错误。所有运行时测试通过,说明是 tsc 类型检查失败而非逻辑错误。

F2(核心 bug)— P1 fix 在输入含空字符串时仍错位src/embedder.ts:1015-1022

当前修复用 item.index(在 validTexts 中的位置)写入 results,但正确位置应该是 validIndices[item.index](在原始 texts 中的位置)。例如:

texts = ['', 'hello', 'world']  →  validIndices = [1, 2]
DashScope 返回 index:0 for 'hello'
当前代码:results[0] = embedding  ✗
正确应为:results[validIndices[0]] = results[1] = embedding  ✓

现有测试 ['first', 'second'] 两个都非空,validIndices=[0,1] 与位置重合,无法检出此 bug。请在测试中加入空字符串用例验证。

EF4 — 新增对齐测试未覆盖其声称修复的空字符串场景

测试用 ['first', 'second'],两个都非空,测试能通过但不验证任何对齐逻辑(见 F2)。

MR1 — logReg()isCliMode() 在 diff 中无定义

index.ts 引入这两个函数但 diff 中未见定义。如果是已有内部工具函数,请确认它们在 PR 修改后仍可访问;如果是新增的,需要包含定义。

MR3 — batch embedding 的 abort/timeout 管道不完整

AbortController 被创建但未传递给底层 fetch,timeout 触发后请求仍在进行。


Nice to Have

  • F6 (src/retriever.ts:369-382): buildRerankRequest 中的 dashscope case 是死代码——DashScope 在 line 1213 已经走独立的 fetchDashScopeRerank 分支提前 return,不会进入 buildRerankRequest。建议移除或加注释说明。
  • F7 (openclaw.plugin.json:359): Schema 描述写的是 gte-rerank-v2,但代码默认用 qwen3-vl-rerank,两者请求格式不同,容易误导用户配置。
  • EF2: embedWithDashScopeRetrypayload: any 参数可能是 EF1 build failure 的根因,建议改为具体类型。

方向正确,修复 build error 和 F2 对齐 bug 后可以合并。

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