fix(retriever): PR746 signal extraction + useLightweightAutoRecall#779
fix(retriever): PR746 signal extraction + useLightweightAutoRecall#779jlin53882 wants to merge 3 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 279289c444
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| if (tagTokens.length > 0) { | ||
| if (tagTokens.length > 0 || useLightweightAutoRecall) { | ||
| mode = "bm25"; |
There was a problem hiding this comment.
When retrieveWithTrace is called in any branch (including the new source: "auto-recall" path used by the added test), this assignment writes to mode before it has been declared; because the package is an ES module, that raises ReferenceError before the search runs. The subsequent calls also pass an undeclared diagnostics, so the memory_debug trace path cannot return results until this method gets its own mode declaration and either creates diagnostics or stops passing it.
Useful? React with 👍 / 👎.
| category, | ||
| trace, | ||
| diagnostics, | ||
| query, safeLimit, scopeFilter, category, trace, diagnostics, signal, |
There was a problem hiding this comment.
For manual/CLI retrieval in vector mode or stores without FTS, this new signal argument is passed to vectorOnlyRetrieval, but that helper still only accepts parameters through diagnostics and calls this.embedder.embedQuery(query) without a signal. At runtime the extra argument is ignored, so an aborted RetrievalContext.signal does not cancel the embedding request on these paths; the same mismatch exists for the hybrid helper below.
Useful? React with 👍 / 👎.
PR 739 重建（原本 PR 有 merge conflict）： - 新增 src/redis-lock.ts（340行） - dynamic import 避免 ioredis 未安裝時 crash - RedisUnavailableError + Symbol.for marker（跨 module ESM-safe） - isRedisConnectionError 含 wrapped error 遞迴檢查 - parseRedisUrl() 正確解析 URL（含密碼、DB selection） - RedisLockManager + createRedisLockManager 工廠 - dbPath hash namespace 避免跨 store lock interference - 修改 src/store.ts（+50行） - import RedisLockManager/createRedisLockManager/RedisUnavailableError - runWithFileLock() 新增 Redis lock 優先實作 - runWithFileLockCore() 保留既有 file lock 實作 - getRedisLockManager() 模組層級 cache（Option E） - 修正 CortexReach#670：realpath:false 避免 stale lock cleanup 造成 ENOENT - 新增 3 個測試檔（40 個測試，全部通過） - test/redis-lock-error-types.test.mjs - test/redis-lock-url-parse.test.mjs - test/redis-lock-concurrent-init.test.mjs - package.json：新增 ioredis devDependency 注意：batch accumulator（CortexReach#690）完整保留，不受影響。
1 participant
Summary
Fix PR746: thread AbortSignal through retrieval pipeline and use BM25-only for auto-recall source.
Changes
signal?: AbortSignaltoRetrievalContextinterfacesource === "auto-recall", use BM25-only mode (lightweight, no embedding cost)modeonce inretrieve()/retrieveWithTrace()and reuse in trace finalization (previously declared twice)test/retriever-auto-recall-signal.test.mjswith 7 test casesTest Registration
test/retriever-auto-recall-signal.test.mjsregistered inscripts/ci-test-manifest.mjs(core-regression group)Closes #746