From 4f5e97bcfcf664b7cd345ab14cc544eaf30d2d1c Mon Sep 17 00:00:00 2001 From: "zhengxiao.wu" Date: Wed, 5 Aug 2026 16:32:34 +0800 Subject: [PATCH 1/2] fix(retrieval): honor tier ceilings and stop cooling unserved recalls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #3534, from its post-merge review round. - The abstract-to-overview substitute now applies only to categories whose stored abstract is the whole file body. A resource or skill whose abstract is missing (`processing_mode=vectors_only`) or over the per-entry cap read its body and returned an overview instead, which for a short file is the body almost verbatim — crossing the opt-in deepening boundary those categories are documented to have, and doing it even under an explicit `detail="abstract"`. They now degrade to a bare URI and their body is never read. - A digest reporting `no_relevant` blanks `rendered`, so the client injects nothing, yet those URIs still entered the dedup ledger and were cooled for `dedup_turns` turns. That contradicted the ledger's own bare-URI grace rule and held memories back from the later turn they were relevant to. - Flat retrieval reaches built-in memory types outside the four named ones (`cases`, `patterns`, `tools`, `trajectories`, skill-usage memories) and reported them as an undeclared `memories` category that no tier or penalty table covered, so other-peer hits skipped the score penalty and callers could not pin their tier. The catch-all is now a declared category with both; it stays out of `quotas`, whose buckets it would overlap. Skill-usage memories also stop being misread as the `skills` category. - ZCode, OpenCode and pi own an OV session id but did not forward it, so their recalls silently ran without query expansion or cross-turn dedup. - The context-request deadline covered only the server's 30s rewrite fuse, but the pipeline is serial: expansion, retrieval and budgeting all precede it. 45s covers both fuses and the work between them. - `plugin` config scope and the `/recall` successor example now match what the code actually does. --- docs/en/agent-integrations/01-overview.md | 4 +- docs/en/api/06-retrieval.md | 9 +- docs/en/api/16-memory.md | 3 +- docs/zh/agent-integrations/01-overview.md | 4 +- docs/zh/api/06-retrieval.md | 11 +- docs/zh/api/16-memory.md | 3 +- .../scripts/shared/plugin-config.mjs | 14 +- .../scripts/shared/recall-core.mjs | 11 +- .../scripts/shared/plugin-config.mjs | 14 +- .../scripts/shared/recall-core.mjs | 11 +- .../lib/plugin-config.mjs | 14 +- .../memory-plugin-shared/lib/recall-core.mjs | 11 +- .../memory-plugin-shared/recall-core.test.mjs | 10 +- .../recall-session-wiring.test.mjs | 53 +++++++ examples/opencode-plugin/index.mjs | 2 +- .../opencode-plugin/lib/memory-recall.mjs | 8 +- .../lib/shared/plugin-config.mjs | 14 +- .../lib/shared/recall-core.mjs | 11 +- examples/ovcli.conf.example | 6 +- examples/pi-coding-agent-extension/index.ts | 2 +- examples/pi-coding-agent-extension/recall.ts | 13 +- .../shared/plugin-config.mjs | 14 +- .../shared/recall-core.d.mts | 6 +- .../shared/recall-core.mjs | 11 +- .../tests/recall-deferred.test.mjs | 24 +++ .../scripts/shared/plugin-config.mjs | 14 +- .../scripts/shared/recall-core.mjs | 11 +- .../scripts/zcode-hook.mjs | 2 +- .../retrieve/context_assembler/__init__.py | 4 + .../retrieve/context_assembler/budget.py | 21 +-- .../retrieve/context_assembler/gather.py | 15 +- .../retrieve/context_assembler/params.py | 26 +++- .../retrieve/context_assembler/pipeline.py | 13 +- .../retrieve/context_assembler/tiers.py | 19 ++- openviking/server/routers/search.py | 22 ++- .../test_context_assembler_pipeline.py | 141 +++++++++++++++++- 36 files changed, 466 insertions(+), 105 deletions(-) create mode 100644 examples/memory-plugin-shared/recall-session-wiring.test.mjs diff --git a/docs/en/agent-integrations/01-overview.md b/docs/en/agent-integrations/01-overview.md index a346ef5caa..73930ddcda 100644 --- a/docs/en/agent-integrations/01-overview.md +++ b/docs/en/agent-integrations/01-overview.md @@ -56,4 +56,6 @@ The same settings can live in `~/.openviking/ovcli.conf`: Environment variables take precedence over `ovcli.conf`. Restart the Agent after changing these settings so its hook processes reload the configuration. These are plugin-client settings; the server's `ov.conf` does not need to change. -When Claude Code asks the server for a digest, the context request waits longer than an ordinary request: the server's own rewrite fuse is `retrieval.recall_rewrite_timeout_s` (30s by default), and aborting earlier would discard the whole response rather than just the digest. Set `OPENVIKING_RECALL_CONTEXT_TIMEOUT_MS` (or `plugin.recallContextTimeoutMs`) to pin that deadline — keep it above the server's fuse and below the Agent's own hook timeout. +The `plugin` section is read by the Claude Code and Codex plugins. The other harnesses take the same knobs from the environment only, so a `plugin.opencode`-style entry named after them is currently inert. + +When Claude Code asks the server for a digest, the context request waits longer than an ordinary request, and aborting earlier would discard the whole response rather than just the digest. The server pipeline is serial and the rewrite is only its last stage: query expansion (`retrieval.recall_intent_timeout_s`, 5s) runs first, then retrieval, body reads and budgeting, and only then the rewrite (`retrieval.recall_rewrite_timeout_s`, 30s). The default deadline is 45s so that a request where every server stage stayed inside its own fuse is not cut off client-side. Set `OPENVIKING_RECALL_CONTEXT_TIMEOUT_MS` (or `plugin.recallContextTimeoutMs`) to pin it — keep it above the sum of both fuses and below the Agent's own hook timeout. diff --git a/docs/en/api/06-retrieval.md b/docs/en/api/06-retrieval.md index f8f6596784..0c1a23729e 100644 --- a/docs/en/api/06-retrieval.md +++ b/docs/en/api/06-retrieval.md @@ -655,10 +655,11 @@ Injecting context every turn used to mean searching per type, reading each hit b | `events` | overview | full | The one memory type whose body is long enough for `# Summary` extraction to be a real compression | | `entities` / `preferences` / `experiences` | abstract | abstract | Short bodies, and the writer stores the whole body in the abstract scalar, so abstract already is the complete file | | `resources` / `skills` | abstract | abstract | The 256-char abstract from semantic processing; bodies can be large or carry credentials, so deepening is opt-in | + | `memories` | abstract | abstract | Built-in memory types outside the four named ones — `cases`, `patterns`, `tools`, `trajectories`, skill-usage memories. Only quota-free retrieval reaches them; they own no bucket, so `quotas` cannot name them, but `detail` and `other_peer_penalty` can | | Directory hits | overview | overview | A directory has no abstract, so it reads the `.overview.md` sidecar; a full tier is meaningless for a subtree | -- **Floor**: every result carries at least its `uri`. When a category's default tier yields nothing usable — a resource that never went through semantic processing, or an abstract that busts the per-entry cap — the entry falls back to overview instead of degrading to a bare pointer -- **Explicit `detail`**: sets that tier as both the requested start and ceiling; entries that do not fit still step down a tier rather than being truncated +- **Floor**: every result carries at least its `uri`. When a memory abstract is unavailable or busts the per-entry cap, the entry falls back to overview: the memory writer stores the whole body in that scalar, so for memory categories overview sits *below* abstract on the content ladder and the substitute discloses less. A `resources` or `skills` abstract is the short generated summary instead, so the same substitution would read a body the caller never asked for — those two degrade to a bare `uri` rather than deepen +- **Explicit `detail`**: sets that tier as both the requested start and ceiling; entries that do not fit still step down a tier rather than being truncated. The memory overview substitute above is the one case where the served `detail` can outrank the pin, and only because it carries less content than the pinned tier would - **Overview by source type**: memory files use the leading `# Summary` section, code files use class and function signatures (reusing `code_outline`), long documents use the heading tree plus first paragraph - **Per-entry cap**: `max_tokens ÷ candidate_count × 2`, applied to every tier except the bare `uri`; a tier exceeding it falls back to the previous tier rather than being truncated. If budget is still left over, one final deepening pass ignores the cap and is bounded only by `max_tokens` @@ -744,7 +745,7 @@ curl -X POST http://localhost:1933/api/v1/search/search \ | Field | Type | Description | |-------|------|-------------| | `entries[].uri` | string | Entry URI, always present at every tier, expandable with the MCP `read` tool | -| `entries[].category` | string | `events`/`entities`/`preferences`/`experiences`/`resources`/`skills` | +| `entries[].category` | string | `events`/`entities`/`preferences`/`experiences`/`resources`/`skills`, or `memories` for a built-in memory type outside those four | | `entries[].detail` | string | Tier actually served: `full`, `overview`, `abstract` or `uri` | | `entries[].text` | string | Body for that tier; empty at the `uri` tier | | `rendered` | string | Flat XML context block, ready to inject; empty when rewrite reports `no_relevant` | @@ -754,6 +755,8 @@ curl -X POST http://localhost:1933/api/v1/search/search \ When `stats.rewrite` is `no_relevant`, the response keeps `entries` for inspection but returns both `digest` and `rendered` as empty strings. This makes the successful empty result safe for clients that predate the explicit status. +Nothing was served that turn, so those URIs also stay out of the `dedup_turns` +ledger and remain available to the later turn they are relevant to. **Validation rules** diff --git a/docs/en/api/16-memory.md b/docs/en/api/16-memory.md index c7cebf45c5..36552a48db 100644 --- a/docs/en/api/16-memory.md +++ b/docs/en/api/16-memory.md @@ -105,7 +105,8 @@ The response shape matches the context face (flat entries, flat XML in `rendered "origins": {"actor_peer": 0, "self": 1, "other_peer": 0}, "deprecated": { "endpoint": "/api/v1/search/recall", - "successor": "/api/v1/search/search?mode=context", + "successor": "/api/v1/search/search", + "successor_body": {"mode": "context"}, "aliases_used": ["max_chars"] } } diff --git a/docs/zh/agent-integrations/01-overview.md b/docs/zh/agent-integrations/01-overview.md index 625e78451f..9fe7175674 100644 --- a/docs/zh/agent-integrations/01-overview.md +++ b/docs/zh/agent-integrations/01-overview.md @@ -56,4 +56,6 @@ export OPENVIKING_RECALL_COMPRESS=off 环境变量优先于 `ovcli.conf`。修改后重启对应的 Agent,让 hook 进程重新加载配置。上述设置属于插件客户端,不需要修改服务端的 `ov.conf`。 -当 Claude Code 请求服务端生成 digest 时,这次 context 请求的等待时间比普通请求更长:服务端自身的 rewrite 保险丝是 `retrieval.recall_rewrite_timeout_s`(默认 30 秒),客户端提前中断会丢掉整个响应,而不只是 digest。可以用 `OPENVIKING_RECALL_CONTEXT_TIMEOUT_MS`(或 `plugin.recallContextTimeoutMs`)指定这个上限,取值应高于服务端保险丝、低于 Agent 自身的 hook 超时。 +`plugin` 段目前由 Claude Code 和 Codex 插件读取。其余 harness 只从环境变量取这些开关,因此以它们命名的 `plugin.opencode` 之类条目当前不生效。 + +当 Claude Code 请求服务端生成 digest 时,这次 context 请求的等待时间比普通请求更长,客户端提前中断会丢掉整个响应,而不只是 digest。服务端流水线是串行的,rewrite 只是最后一段:先是查询扩展(`retrieval.recall_intent_timeout_s`,5 秒),然后是检索、正文读取和预算规划,最后才是 rewrite(`retrieval.recall_rewrite_timeout_s`,30 秒)。默认上限取 45 秒,这样每个服务端阶段都没超出自身保险丝的请求就不会被客户端切断。可以用 `OPENVIKING_RECALL_CONTEXT_TIMEOUT_MS`(或 `plugin.recallContextTimeoutMs`)指定这个上限,取值应高于两个保险丝之和、低于 Agent 自身的 hook 超时。 diff --git a/docs/zh/api/06-retrieval.md b/docs/zh/api/06-retrieval.md index ba15dff51f..af0aa28eb5 100644 --- a/docs/zh/api/06-retrieval.md +++ b/docs/zh/api/06-retrieval.md @@ -657,10 +657,11 @@ Agent 插件每轮注入上下文时,过去需要按类型逐个检索、再 | `events` | 概览档 | 全文档 | 唯一正文足够长、`# Summary` 抽取能真正压缩的类型 | | `entities` / `preferences` / `experiences` | 摘要档 | 摘要档 | 正文本身很短,且写入侧把整篇正文存进了摘要标量,摘要档即完整内容 | | `resources` / `skills` | 摘要档 | 摘要档 | 语义处理生成的 256 字符摘要;正文可能很大或含凭据,加深需显式指定 | + | `memories` | 摘要档 | 摘要档 | 四个具名类型之外的内置记忆类型——`cases`、`patterns`、`tools`、`trajectories`、技能使用记忆。只有 quota-free 检索会命中它们;它们没有自己的检索桶,`quotas` 不能指定,但 `detail` 和 `other_peer_penalty` 可以 | | 目录命中 | 概览档 | 概览档 | 目录没有摘要,读 `.overview.md` 侧车;全文档对目录无意义 | -- **保底**:每条结果至少给出 `uri`;类别默认档拿不到可用内容时(例如资源尚未跑过语义处理,或摘要本身超出单条上限)自动回落到概览档,而不是退成裸指针 -- **显式 `detail`**:把该档作为全部结果请求的起点和上限;装不下的条目仍逐档退档而不截断 +- **保底**:每条结果至少给出 `uri`。记忆类摘要缺失或超出单条上限时回落到概览档:写入侧把整篇正文存进了摘要标量,所以对记忆类别而言概览档在内容阶梯上位于摘要档*之下*,这次替换披露得更少。而 `resources` / `skills` 的摘要是语义处理生成的短摘要,同样的替换会去读调用方没有请求的正文,因此这两类直接退成裸 `uri`,不向上加深 +- **显式 `detail`**:把该档作为全部结果请求的起点和上限;装不下的条目仍逐档退档而不截断。上述记忆类概览档替换是实际档位唯一可能高于指定档的情况,且仅因为它比指定档携带的内容更少 - **概览档按来源取骨架**:记忆文件取开头的 `# Summary` 段,代码文件取函数与类签名(复用 `code_outline`),长文档取标题树加首段 - **单条上限**:`max_tokens ÷ 候选条数 × 2`,对除裸 `uri` 外的所有档位一律生效;某一档超出该上限时退回上一档,不做截断。预算仍有剩余时,最后一轮加深不受该上限约束,只受 `max_tokens` 约束 @@ -746,7 +747,7 @@ curl -X POST http://localhost:1933/api/v1/search/search \ | 字段 | 类型 | 说明 | |------|------|------| | `entries[].uri` | string | 条目 URI,任何档位都必然存在,可用 MCP `read` 下钻 | -| `entries[].category` | string | `events`/`entities`/`preferences`/`experiences`/`resources`/`skills` | +| `entries[].category` | string | `events`/`entities`/`preferences`/`experiences`/`resources`/`skills`,或 `memories`(四个具名类型之外的内置记忆类型) | | `entries[].detail` | string | 实际档位:`full`、`overview`、`abstract` 或 `uri` | | `entries[].text` | string | 该档位的正文;`uri` 档为空 | | `rendered` | string | 扁平 XML 上下文块,可直接注入;重写返回 `no_relevant` 时为空 | @@ -754,7 +755,9 @@ curl -X POST http://localhost:1933/api/v1/search/search \ | `stats` | object | 预算用量、档位分布、扩展与重写状态(`off`、`ok`、`no_relevant`、`failed` 或 `timeout`)、去重账本状态;某个检索域失败时附带 `retrieval_errors`,用于区分「检索坏了」和「确实没有相关记忆」 | 当 `stats.rewrite` 为 `no_relevant` 时,响应仍保留 `entries` 供检查,但 `digest` 和 -`rendered` 都为空字符串。这样即使客户端尚未识别新状态,也不会回退注入原文。 +`rendered` 都为空字符串。这样即使客户端尚未识别新状态,也不会回退注入原文。本轮 +没有交付任何内容,因此这些 URI 也不会进入 `dedup_turns` 账本,之后真正相关的那一轮 +仍能召回它们。 **校验规则** diff --git a/docs/zh/api/16-memory.md b/docs/zh/api/16-memory.md index f6373180b1..57863bee7a 100644 --- a/docs/zh/api/16-memory.md +++ b/docs/zh/api/16-memory.md @@ -105,7 +105,8 @@ recall( "origins": {"actor_peer": 0, "self": 1, "other_peer": 0}, "deprecated": { "endpoint": "/api/v1/search/recall", - "successor": "/api/v1/search/search?mode=context", + "successor": "/api/v1/search/search", + "successor_body": {"mode": "context"}, "aliases_used": ["max_chars"] } } diff --git a/examples/claude-code-memory-plugin/scripts/shared/plugin-config.mjs b/examples/claude-code-memory-plugin/scripts/shared/plugin-config.mjs index a4d0a300e3..9f68d58bd6 100644 --- a/examples/claude-code-memory-plugin/scripts/shared/plugin-config.mjs +++ b/examples/claude-code-memory-plugin/scripts/shared/plugin-config.mjs @@ -5,7 +5,8 @@ * ovcli.conf carried connection fields only, so every harness had to keep its * tuning knobs in ov.conf's harness section — a server-side file that a * client-side plugin has no business editing. The `plugin` section fixes that: - * shared keys apply to every harness, and a per-harness object overrides them. + * shared keys apply to every harness that reads them, and a per-harness object + * overrides them. * * { * "url": "...", "api_key": "...", @@ -17,6 +18,13 @@ * * Resolution stays env → ovcli.conf plugin. → ovcli.conf plugin → * ov.conf harness section (legacy) → defaults. + * + * Consumers: Claude Code and Codex only. The other harnesses ship this module + * through `sync.mjs` but still read their knobs from the environment, so a + * `plugin` entry named after them is inert. `HARNESS_KEYS` lists what a harness + * loader actually consumes today — add a key here as its loader starts calling + * `loadPluginSettings`, not before, so the section never promises a knob that + * silently does nothing. */ import { readFileSync } from "node:fs"; @@ -28,10 +36,6 @@ const DEFAULT_OVCLI_CONF_PATH = join(homedir(), ".openviking", "ovcli.conf"); export const HARNESS_KEYS = { claudeCode: "claude_code", codex: "codex", - opencode: "opencode", - cursor: "cursor", - trae: "trae", - pi: "pi", }; function tryLoadJson(path) { diff --git a/examples/claude-code-memory-plugin/scripts/shared/recall-core.mjs b/examples/claude-code-memory-plugin/scripts/shared/recall-core.mjs index 395b61249a..d6e53692a9 100644 --- a/examples/claude-code-memory-plugin/scripts/shared/recall-core.mjs +++ b/examples/claude-code-memory-plugin/scripts/shared/recall-core.mjs @@ -138,9 +138,14 @@ export function buildContextSearchBody(cfg = {}, options = {}) { return body; } -// Server default for retrieval.recall_rewrite_timeout_s plus room for the -// retrieval that precedes it, still well inside the 60s prompt-hook budget. -const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 35000; +// The server pipeline is serial, and only its last stage is the rewrite fuse: +// query expansion (retrieval.recall_intent_timeout_s, 5s) runs first, then +// retrieval, body reads and budgeting, and only then the rewrite +// (retrieval.recall_rewrite_timeout_s, 30s). A deadline covering the rewrite +// alone aborts requests where every server stage stayed inside its own fuse, so +// this covers both fuses plus the work between them and still leaves a quarter +// of the 60s prompt-hook budget. +const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 45000; /** * HTTP deadline for one context request, or undefined to keep the caller's own. diff --git a/examples/codex-memory-plugin/scripts/shared/plugin-config.mjs b/examples/codex-memory-plugin/scripts/shared/plugin-config.mjs index a4d0a300e3..9f68d58bd6 100644 --- a/examples/codex-memory-plugin/scripts/shared/plugin-config.mjs +++ b/examples/codex-memory-plugin/scripts/shared/plugin-config.mjs @@ -5,7 +5,8 @@ * ovcli.conf carried connection fields only, so every harness had to keep its * tuning knobs in ov.conf's harness section — a server-side file that a * client-side plugin has no business editing. The `plugin` section fixes that: - * shared keys apply to every harness, and a per-harness object overrides them. + * shared keys apply to every harness that reads them, and a per-harness object + * overrides them. * * { * "url": "...", "api_key": "...", @@ -17,6 +18,13 @@ * * Resolution stays env → ovcli.conf plugin. → ovcli.conf plugin → * ov.conf harness section (legacy) → defaults. + * + * Consumers: Claude Code and Codex only. The other harnesses ship this module + * through `sync.mjs` but still read their knobs from the environment, so a + * `plugin` entry named after them is inert. `HARNESS_KEYS` lists what a harness + * loader actually consumes today — add a key here as its loader starts calling + * `loadPluginSettings`, not before, so the section never promises a knob that + * silently does nothing. */ import { readFileSync } from "node:fs"; @@ -28,10 +36,6 @@ const DEFAULT_OVCLI_CONF_PATH = join(homedir(), ".openviking", "ovcli.conf"); export const HARNESS_KEYS = { claudeCode: "claude_code", codex: "codex", - opencode: "opencode", - cursor: "cursor", - trae: "trae", - pi: "pi", }; function tryLoadJson(path) { diff --git a/examples/codex-memory-plugin/scripts/shared/recall-core.mjs b/examples/codex-memory-plugin/scripts/shared/recall-core.mjs index 395b61249a..d6e53692a9 100644 --- a/examples/codex-memory-plugin/scripts/shared/recall-core.mjs +++ b/examples/codex-memory-plugin/scripts/shared/recall-core.mjs @@ -138,9 +138,14 @@ export function buildContextSearchBody(cfg = {}, options = {}) { return body; } -// Server default for retrieval.recall_rewrite_timeout_s plus room for the -// retrieval that precedes it, still well inside the 60s prompt-hook budget. -const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 35000; +// The server pipeline is serial, and only its last stage is the rewrite fuse: +// query expansion (retrieval.recall_intent_timeout_s, 5s) runs first, then +// retrieval, body reads and budgeting, and only then the rewrite +// (retrieval.recall_rewrite_timeout_s, 30s). A deadline covering the rewrite +// alone aborts requests where every server stage stayed inside its own fuse, so +// this covers both fuses plus the work between them and still leaves a quarter +// of the 60s prompt-hook budget. +const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 45000; /** * HTTP deadline for one context request, or undefined to keep the caller's own. diff --git a/examples/memory-plugin-shared/lib/plugin-config.mjs b/examples/memory-plugin-shared/lib/plugin-config.mjs index 2df180300f..e85f89be6a 100644 --- a/examples/memory-plugin-shared/lib/plugin-config.mjs +++ b/examples/memory-plugin-shared/lib/plugin-config.mjs @@ -4,7 +4,8 @@ * ovcli.conf carried connection fields only, so every harness had to keep its * tuning knobs in ov.conf's harness section — a server-side file that a * client-side plugin has no business editing. The `plugin` section fixes that: - * shared keys apply to every harness, and a per-harness object overrides them. + * shared keys apply to every harness that reads them, and a per-harness object + * overrides them. * * { * "url": "...", "api_key": "...", @@ -16,6 +17,13 @@ * * Resolution stays env → ovcli.conf plugin. → ovcli.conf plugin → * ov.conf harness section (legacy) → defaults. + * + * Consumers: Claude Code and Codex only. The other harnesses ship this module + * through `sync.mjs` but still read their knobs from the environment, so a + * `plugin` entry named after them is inert. `HARNESS_KEYS` lists what a harness + * loader actually consumes today — add a key here as its loader starts calling + * `loadPluginSettings`, not before, so the section never promises a knob that + * silently does nothing. */ import { readFileSync } from "node:fs"; @@ -27,10 +35,6 @@ const DEFAULT_OVCLI_CONF_PATH = join(homedir(), ".openviking", "ovcli.conf"); export const HARNESS_KEYS = { claudeCode: "claude_code", codex: "codex", - opencode: "opencode", - cursor: "cursor", - trae: "trae", - pi: "pi", }; function tryLoadJson(path) { diff --git a/examples/memory-plugin-shared/lib/recall-core.mjs b/examples/memory-plugin-shared/lib/recall-core.mjs index 924efe1647..c17db7fc64 100644 --- a/examples/memory-plugin-shared/lib/recall-core.mjs +++ b/examples/memory-plugin-shared/lib/recall-core.mjs @@ -137,9 +137,14 @@ export function buildContextSearchBody(cfg = {}, options = {}) { return body; } -// Server default for retrieval.recall_rewrite_timeout_s plus room for the -// retrieval that precedes it, still well inside the 60s prompt-hook budget. -const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 35000; +// The server pipeline is serial, and only its last stage is the rewrite fuse: +// query expansion (retrieval.recall_intent_timeout_s, 5s) runs first, then +// retrieval, body reads and budgeting, and only then the rewrite +// (retrieval.recall_rewrite_timeout_s, 30s). A deadline covering the rewrite +// alone aborts requests where every server stage stayed inside its own fuse, so +// this covers both fuses plus the work between them and still leaves a quarter +// of the 60s prompt-hook budget. +const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 45000; /** * HTTP deadline for one context request, or undefined to keep the caller's own. diff --git a/examples/memory-plugin-shared/recall-core.test.mjs b/examples/memory-plugin-shared/recall-core.test.mjs index 4fbe41d655..2e549081fe 100644 --- a/examples/memory-plugin-shared/recall-core.test.mjs +++ b/examples/memory-plugin-shared/recall-core.test.mjs @@ -103,9 +103,15 @@ test("a server-side digest outlasts the ordinary request timeout", async () => { legacyCachePath: await tempPath("context-face.json"), }); - assert.ok(timeouts[0] > 30000, `server rewrite must outlast the 30s fuse, got ${timeouts[0]}`); + // The server pipeline is serial: the 5s expansion fuse, retrieval and body + // reads all run before the 30s rewrite fuse even starts, so covering the + // rewrite alone still aborts requests that stayed inside every server budget. + assert.ok( + timeouts[0] > 35000, + `deadline must outlast both server fuses plus the work between, got ${timeouts[0]}`, + ); assert.equal(timeouts[1], undefined); - assert.equal(contextRequestTimeoutMs({ ...cfg, recallContextTimeoutMs: 45000 }, true), 45000); + assert.equal(contextRequestTimeoutMs({ ...cfg, recallContextTimeoutMs: 50000 }, true), 50000); }); test("buildRecallBlock prefers a cited server digest", async () => { diff --git a/examples/memory-plugin-shared/recall-session-wiring.test.mjs b/examples/memory-plugin-shared/recall-session-wiring.test.mjs new file mode 100644 index 0000000000..5269650732 --- /dev/null +++ b/examples/memory-plugin-shared/recall-session-wiring.test.mjs @@ -0,0 +1,53 @@ +import assert from "node:assert/strict"; +import { readFile } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), "..", ".."); + +// The shared request builder only sends `session_id` — and with it `dedup_turns` +// and server-side query expansion — when the caller supplies one. Every harness +// that already owns an OV session id has to forward it at its recall call site, +// which no harness unit test covers end to end: they each stub the transport +// below the point where the id would have been added. These assertions pin the +// wiring so a harness cannot silently fall back to stateless recall again. +const CALL_SITES = [ + { + name: "ZCode hook forwards its derived session id", + file: join(ROOT, "examples", "zcode-memory-plugin", "scripts", "zcode-hook.mjs"), + pattern: /recallForPrompt\([^)]*\{[^}]*\bsessionId\b/s, + }, + { + name: "OpenCode recall forwards the mapped OV session id", + file: join(ROOT, "examples", "opencode-plugin", "lib", "memory-recall.mjs"), + pattern: /sessionId:\s*sessionID\s*\?\s*sessionManager\.getMappedSessionId\(sessionID\)/, + }, + { + name: "pi recall forwards the sync manager's session id", + file: join(ROOT, "examples", "pi-coding-agent-extension", "recall.ts"), + pattern: /sessionId:\s*this\.sessionId\(\)/, + }, +]; + +for (const { name, file, pattern } of CALL_SITES) { + test(name, async () => { + const source = await readFile(file, "utf-8"); + assert.match(source, pattern); + }); +} + +test("pi wires the sync manager into the recall manager", async () => { + const source = await readFile(join(ROOT, "examples", "pi-coding-agent-extension", "index.ts"), "utf-8"); + assert.match(source, /new RecallManager\(client,\s*config,\s*\(\)\s*=>\s*sync\.sessionId\)/); + // The getter exists because SyncManager has to be constructed first. + assert.ok( + source.indexOf("new SyncManager(") < source.indexOf("new RecallManager("), + "SyncManager must be constructed before RecallManager reads its session id", + ); +}); + +test("OpenCode passes the session manager into the recall component", async () => { + const source = await readFile(join(ROOT, "examples", "opencode-plugin", "index.mjs"), "utf-8"); + assert.match(source, /createMemoryRecall\(\{\s*config,\s*sessionManager\s*\}\)/); +}); diff --git a/examples/opencode-plugin/index.mjs b/examples/opencode-plugin/index.mjs index b6c9e3c3a6..9c1e59eee3 100644 --- a/examples/opencode-plugin/index.mjs +++ b/examples/opencode-plugin/index.mjs @@ -24,7 +24,7 @@ export async function OpenVikingPlugin({ client, directory }) { const repoContext = createRepoContext({ config }) const sessionManager = createMemorySessionManager({ config, pluginRoot: dataDir }) - const recall = createMemoryRecall({ config }) + const recall = createMemoryRecall({ config, sessionManager }) const sessionInject = createSessionInject({ config, sessionManager }) const vikingUriGuard = createVikingUriGuard() diff --git a/examples/opencode-plugin/lib/memory-recall.mjs b/examples/opencode-plugin/lib/memory-recall.mjs index 40cd292eaa..f67d8695d4 100644 --- a/examples/opencode-plugin/lib/memory-recall.mjs +++ b/examples/opencode-plugin/lib/memory-recall.mjs @@ -2,14 +2,15 @@ import { buildRecallBlock } from "./shared/recall-core.mjs" import { isBypassed } from "./shared/session-model.mjs" import { effectivePeerId, fetchJSON, log } from "./utils.mjs" -export function createMemoryRecall({ config }) { +export function createMemoryRecall({ config, sessionManager }) { async function injectRelevantMemories(input, output) { if (!config.autoRecall?.enabled) return const query = extractCurrentUserText(output.parts ?? []) if (!query) return if (query.length < config.minQueryLength) return + const sessionID = input.sessionID ?? output.message?.sessionID if (isBypassed(config, { - sessionId: input.sessionID ?? output.message?.sessionID, + sessionId: sessionID, cwd: input.directory ?? input.cwd, })) return @@ -22,6 +23,9 @@ export function createMemoryRecall({ config }) { query, { actorPeerId: effectivePeerId(config), + // The mapped OV session is what turns on server-side query expansion + // and the cross-turn dedup ledger. + sessionId: sessionID ? sessionManager.getMappedSessionId(sessionID) : "", log: (stage, data) => log("DEBUG", "recall", stage, data), }, ) diff --git a/examples/opencode-plugin/lib/shared/plugin-config.mjs b/examples/opencode-plugin/lib/shared/plugin-config.mjs index a4d0a300e3..9f68d58bd6 100644 --- a/examples/opencode-plugin/lib/shared/plugin-config.mjs +++ b/examples/opencode-plugin/lib/shared/plugin-config.mjs @@ -5,7 +5,8 @@ * ovcli.conf carried connection fields only, so every harness had to keep its * tuning knobs in ov.conf's harness section — a server-side file that a * client-side plugin has no business editing. The `plugin` section fixes that: - * shared keys apply to every harness, and a per-harness object overrides them. + * shared keys apply to every harness that reads them, and a per-harness object + * overrides them. * * { * "url": "...", "api_key": "...", @@ -17,6 +18,13 @@ * * Resolution stays env → ovcli.conf plugin. → ovcli.conf plugin → * ov.conf harness section (legacy) → defaults. + * + * Consumers: Claude Code and Codex only. The other harnesses ship this module + * through `sync.mjs` but still read their knobs from the environment, so a + * `plugin` entry named after them is inert. `HARNESS_KEYS` lists what a harness + * loader actually consumes today — add a key here as its loader starts calling + * `loadPluginSettings`, not before, so the section never promises a knob that + * silently does nothing. */ import { readFileSync } from "node:fs"; @@ -28,10 +36,6 @@ const DEFAULT_OVCLI_CONF_PATH = join(homedir(), ".openviking", "ovcli.conf"); export const HARNESS_KEYS = { claudeCode: "claude_code", codex: "codex", - opencode: "opencode", - cursor: "cursor", - trae: "trae", - pi: "pi", }; function tryLoadJson(path) { diff --git a/examples/opencode-plugin/lib/shared/recall-core.mjs b/examples/opencode-plugin/lib/shared/recall-core.mjs index 395b61249a..d6e53692a9 100644 --- a/examples/opencode-plugin/lib/shared/recall-core.mjs +++ b/examples/opencode-plugin/lib/shared/recall-core.mjs @@ -138,9 +138,14 @@ export function buildContextSearchBody(cfg = {}, options = {}) { return body; } -// Server default for retrieval.recall_rewrite_timeout_s plus room for the -// retrieval that precedes it, still well inside the 60s prompt-hook budget. -const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 35000; +// The server pipeline is serial, and only its last stage is the rewrite fuse: +// query expansion (retrieval.recall_intent_timeout_s, 5s) runs first, then +// retrieval, body reads and budgeting, and only then the rewrite +// (retrieval.recall_rewrite_timeout_s, 30s). A deadline covering the rewrite +// alone aborts requests where every server stage stayed inside its own fuse, so +// this covers both fuses plus the work between them and still leaves a quarter +// of the 60s prompt-hook budget. +const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 45000; /** * HTTP deadline for one context request, or undefined to keep the caller's own. diff --git a/examples/ovcli.conf.example b/examples/ovcli.conf.example index 167ee8d0e8..fba844afb9 100644 --- a/examples/ovcli.conf.example +++ b/examples/ovcli.conf.example @@ -14,10 +14,6 @@ "recallCompress": "auto", "claude_code": {}, - "codex": {}, - "opencode": {}, - "cursor": {}, - "trae": {}, - "pi": {} + "codex": {} } } diff --git a/examples/pi-coding-agent-extension/index.ts b/examples/pi-coding-agent-extension/index.ts index ad8366d29f..a507270c04 100644 --- a/examples/pi-coding-agent-extension/index.ts +++ b/examples/pi-coding-agent-extension/index.ts @@ -30,8 +30,8 @@ export default async function (pi: ExtensionAPI) { // --- Initialize modules --- const client = new OVClient(config); - const recall = new RecallManager(client, config); const sync = new SyncManager(client, config); + const recall = new RecallManager(client, config, () => sync.sessionId); const debugLog = (message: string) => { const file = process.env.OV_DEBUG_LOG; if (!file) return; diff --git a/examples/pi-coding-agent-extension/recall.ts b/examples/pi-coding-agent-extension/recall.ts index 89846fff8e..9c60d7f0da 100644 --- a/examples/pi-coding-agent-extension/recall.ts +++ b/examples/pi-coding-agent-extension/recall.ts @@ -12,10 +12,14 @@ export class RecallManager { private config: OVConfig; private cache: RecallCache = { block: null, promptText: "" }; private pendingPrompt = ""; + // Read lazily: the session manager that owns this id is constructed after the + // recall manager, and the id only exists once a session has been opened. + private sessionId: () => string | null; - constructor(client: OVClient, config: OVConfig) { + constructor(client: OVClient, config: OVConfig, sessionId: () => string | null = () => null) { this.client = client; this.config = config; + this.sessionId = sessionId; } queueSearch(userQuery: string): void { @@ -36,7 +40,12 @@ export class RecallManager { (path: string, init?: any, options?: any) => this.client.fetchJSON(path, init, 10000), this.config as any, userQuery, - { actorPeerId: this.config.peerId }, + { + actorPeerId: this.config.peerId, + // Passing the OV session id is what turns on server-side query + // expansion and the cross-turn dedup ledger. + sessionId: this.sessionId() ?? "", + }, ); this.cache = { block, promptText: userQuery }; return block; diff --git a/examples/pi-coding-agent-extension/shared/plugin-config.mjs b/examples/pi-coding-agent-extension/shared/plugin-config.mjs index a4d0a300e3..9f68d58bd6 100644 --- a/examples/pi-coding-agent-extension/shared/plugin-config.mjs +++ b/examples/pi-coding-agent-extension/shared/plugin-config.mjs @@ -5,7 +5,8 @@ * ovcli.conf carried connection fields only, so every harness had to keep its * tuning knobs in ov.conf's harness section — a server-side file that a * client-side plugin has no business editing. The `plugin` section fixes that: - * shared keys apply to every harness, and a per-harness object overrides them. + * shared keys apply to every harness that reads them, and a per-harness object + * overrides them. * * { * "url": "...", "api_key": "...", @@ -17,6 +18,13 @@ * * Resolution stays env → ovcli.conf plugin. → ovcli.conf plugin → * ov.conf harness section (legacy) → defaults. + * + * Consumers: Claude Code and Codex only. The other harnesses ship this module + * through `sync.mjs` but still read their knobs from the environment, so a + * `plugin` entry named after them is inert. `HARNESS_KEYS` lists what a harness + * loader actually consumes today — add a key here as its loader starts calling + * `loadPluginSettings`, not before, so the section never promises a knob that + * silently does nothing. */ import { readFileSync } from "node:fs"; @@ -28,10 +36,6 @@ const DEFAULT_OVCLI_CONF_PATH = join(homedir(), ".openviking", "ovcli.conf"); export const HARNESS_KEYS = { claudeCode: "claude_code", codex: "codex", - opencode: "opencode", - cursor: "cursor", - trae: "trae", - pi: "pi", }; function tryLoadJson(path) { diff --git a/examples/pi-coding-agent-extension/shared/recall-core.d.mts b/examples/pi-coding-agent-extension/shared/recall-core.d.mts index ceaa6a496a..e470aa2374 100644 --- a/examples/pi-coding-agent-extension/shared/recall-core.d.mts +++ b/examples/pi-coding-agent-extension/shared/recall-core.d.mts @@ -2,7 +2,11 @@ export function buildRecallBlock( fetchJSON: (path: string, init?: any, options?: any) => Promise<{ ok: boolean; status?: number; result?: any; error?: any }>, cfg: Record, query: string, - options?: { actorPeerId?: string; log?: (stage: string, data?: any) => void }, + options?: { + actorPeerId?: string; + sessionId?: string; + log?: (stage: string, data?: any) => void; + }, ): Promise; export function buildRecallEndpointBody(cfg?: Record): Record; diff --git a/examples/pi-coding-agent-extension/shared/recall-core.mjs b/examples/pi-coding-agent-extension/shared/recall-core.mjs index 395b61249a..d6e53692a9 100644 --- a/examples/pi-coding-agent-extension/shared/recall-core.mjs +++ b/examples/pi-coding-agent-extension/shared/recall-core.mjs @@ -138,9 +138,14 @@ export function buildContextSearchBody(cfg = {}, options = {}) { return body; } -// Server default for retrieval.recall_rewrite_timeout_s plus room for the -// retrieval that precedes it, still well inside the 60s prompt-hook budget. -const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 35000; +// The server pipeline is serial, and only its last stage is the rewrite fuse: +// query expansion (retrieval.recall_intent_timeout_s, 5s) runs first, then +// retrieval, body reads and budgeting, and only then the rewrite +// (retrieval.recall_rewrite_timeout_s, 30s). A deadline covering the rewrite +// alone aborts requests where every server stage stayed inside its own fuse, so +// this covers both fuses plus the work between them and still leaves a quarter +// of the 60s prompt-hook budget. +const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 45000; /** * HTTP deadline for one context request, or undefined to keep the caller's own. diff --git a/examples/pi-coding-agent-extension/tests/recall-deferred.test.mjs b/examples/pi-coding-agent-extension/tests/recall-deferred.test.mjs index c15e595750..49a10bb078 100644 --- a/examples/pi-coding-agent-extension/tests/recall-deferred.test.mjs +++ b/examples/pi-coding-agent-extension/tests/recall-deferred.test.mjs @@ -44,6 +44,30 @@ test("queued recall waits for the context phase and still injects current-query assert.equal(calls.length, 1, "later context iterations must reuse the cached recall"); }); +test("recall sends the OV session id so dedup and expansion engage", async () => { + const bodies = []; + const client = { + fetchJSON: async (path, init) => { + bodies.push(JSON.parse(init.body)); + return { ok: true, result: { rendered: "- memory" } }; + }, + }; + // The sync manager owns the id and is constructed after the recall manager, + // so it arrives as a getter and is null until a session has been opened. + let sessionId = null; + const recall = new RecallManager(client, config({ recallDedupTurns: 5 }), () => sessionId); + + recall.queueSearch("first prompt"); + await recall.searchPending(); + assert.equal(bodies[0].session_id, undefined); + + sessionId = "pi-session-1"; + recall.queueSearch("second prompt"); + await recall.searchPending(); + assert.equal(bodies[1].session_id, "pi-session-1"); + assert.equal(bodies[1].dedup_turns, 5); +}); + test("a queued short prompt clears the previous recall block", async () => { const client = { fetchJSON: async () => ({ diff --git a/examples/zcode-memory-plugin/scripts/shared/plugin-config.mjs b/examples/zcode-memory-plugin/scripts/shared/plugin-config.mjs index a4d0a300e3..9f68d58bd6 100644 --- a/examples/zcode-memory-plugin/scripts/shared/plugin-config.mjs +++ b/examples/zcode-memory-plugin/scripts/shared/plugin-config.mjs @@ -5,7 +5,8 @@ * ovcli.conf carried connection fields only, so every harness had to keep its * tuning knobs in ov.conf's harness section — a server-side file that a * client-side plugin has no business editing. The `plugin` section fixes that: - * shared keys apply to every harness, and a per-harness object overrides them. + * shared keys apply to every harness that reads them, and a per-harness object + * overrides them. * * { * "url": "...", "api_key": "...", @@ -17,6 +18,13 @@ * * Resolution stays env → ovcli.conf plugin. → ovcli.conf plugin → * ov.conf harness section (legacy) → defaults. + * + * Consumers: Claude Code and Codex only. The other harnesses ship this module + * through `sync.mjs` but still read their knobs from the environment, so a + * `plugin` entry named after them is inert. `HARNESS_KEYS` lists what a harness + * loader actually consumes today — add a key here as its loader starts calling + * `loadPluginSettings`, not before, so the section never promises a knob that + * silently does nothing. */ import { readFileSync } from "node:fs"; @@ -28,10 +36,6 @@ const DEFAULT_OVCLI_CONF_PATH = join(homedir(), ".openviking", "ovcli.conf"); export const HARNESS_KEYS = { claudeCode: "claude_code", codex: "codex", - opencode: "opencode", - cursor: "cursor", - trae: "trae", - pi: "pi", }; function tryLoadJson(path) { diff --git a/examples/zcode-memory-plugin/scripts/shared/recall-core.mjs b/examples/zcode-memory-plugin/scripts/shared/recall-core.mjs index 395b61249a..d6e53692a9 100644 --- a/examples/zcode-memory-plugin/scripts/shared/recall-core.mjs +++ b/examples/zcode-memory-plugin/scripts/shared/recall-core.mjs @@ -138,9 +138,14 @@ export function buildContextSearchBody(cfg = {}, options = {}) { return body; } -// Server default for retrieval.recall_rewrite_timeout_s plus room for the -// retrieval that precedes it, still well inside the 60s prompt-hook budget. -const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 35000; +// The server pipeline is serial, and only its last stage is the rewrite fuse: +// query expansion (retrieval.recall_intent_timeout_s, 5s) runs first, then +// retrieval, body reads and budgeting, and only then the rewrite +// (retrieval.recall_rewrite_timeout_s, 30s). A deadline covering the rewrite +// alone aborts requests where every server stage stayed inside its own fuse, so +// this covers both fuses plus the work between them and still leaves a quarter +// of the 60s prompt-hook budget. +const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 45000; /** * HTTP deadline for one context request, or undefined to keep the caller's own. diff --git a/examples/zcode-memory-plugin/scripts/zcode-hook.mjs b/examples/zcode-memory-plugin/scripts/zcode-hook.mjs index 5d71a286aa..7422a9542d 100644 --- a/examples/zcode-memory-plugin/scripts/zcode-hook.mjs +++ b/examples/zcode-memory-plugin/scripts/zcode-hook.mjs @@ -115,7 +115,7 @@ async function main() { const block = state.promptHash === promptHash && state.recallBlock ? state.recallBlock - : await recallForPrompt(fetchJSON, cfg, prompt, cwd, log).catch((error) => { + : await recallForPrompt(fetchJSON, cfg, prompt, cwd, log, { sessionId }).catch((error) => { logError("recall", error); return null; }); diff --git a/openviking/retrieve/context_assembler/__init__.py b/openviking/retrieve/context_assembler/__init__.py index e61d312cff..51d592edc2 100644 --- a/openviking/retrieve/context_assembler/__init__.py +++ b/openviking/retrieve/context_assembler/__init__.py @@ -12,7 +12,9 @@ DEFAULT_TIER_BY_CATEGORY, MAX_EXCLUDE_URIS, MEMORY_CATEGORIES, + OTHER_MEMORY_CATEGORY, PURPOSE_PRESETS, + REPORTED_CATEGORY_KEYS, AssembleParams, DetailRequest, normalize_detail, @@ -36,7 +38,9 @@ "DetailRequest", "MAX_EXCLUDE_URIS", "MEMORY_CATEGORIES", + "OTHER_MEMORY_CATEGORY", "PURPOSE_PRESETS", + "REPORTED_CATEGORY_KEYS", "assemble_context", "normalize_detail", "normalize_digest", diff --git a/openviking/retrieve/context_assembler/budget.py b/openviking/retrieve/context_assembler/budget.py index fed66c2b43..9ae627fa3e 100644 --- a/openviking/retrieve/context_assembler/budget.py +++ b/openviking/retrieve/context_assembler/budget.py @@ -22,7 +22,7 @@ normalize_detail, ) from openviking.retrieve.context_assembler.render import fragment_tokens -from openviking.retrieve.context_assembler.tiers import tier_text, tier_window +from openviking.retrieve.context_assembler.tiers import abstract_substitute, tier_text, tier_window SEPARATOR_TOKENS = 1 @@ -46,10 +46,10 @@ def per_entry_cap(max_tokens: int, candidate_count: int) -> int: return max(1, max_tokens // max(1, candidate_count) * 2) -def _tiers_down_from(tier: Tier) -> List[Tier]: +def _tiers_down_from(candidate: Candidate, tier: Tier) -> List[Tier]: order = list(reversed(TIER_ORDER[: TIER_RANK[tier] + 1])) - if tier == "abstract": - # A memory file's stored abstract is its whole body, so overview is a + if tier == "abstract" and abstract_substitute(candidate) == "overview": + # The candidate's stored abstract is its whole body, so overview is a # cheaper substitute here rather than a step up: try it before giving up # on showing any content at all. order.insert(1, "overview") @@ -69,12 +69,15 @@ def _make_entry(candidate: Candidate, tier: Tier, text: str) -> AssembledEntry: return entry -def abstract_over_cap(candidate: Candidate, cap: int) -> bool: - """Whether this candidate's stored abstract exceeds the per-entry cap. +def oversized_abstract_needs_body(candidate: Candidate, cap: int) -> bool: + """Whether an over-cap abstract falls back to a tier that reads the body. - Such a candidate falls back to overview, so its body has to be read even - though its own tier would not need one. + Only the full-body-abstract categories have that fallback. A resource or + skill keeps its short abstract or degrades to a bare URI, so an oversized + abstract there never justifies a read. """ + if abstract_substitute(candidate) != "overview": + return False text = candidate.abstract.strip() return bool(text) and fragment_tokens(_make_entry(candidate, "abstract", text)) > cap @@ -105,7 +108,7 @@ def fits_cap(tier: Tier, tokens: int) -> bool: for candidate in candidates: start, ceiling = tier_window(candidate, pins.for_category(candidate.category)) placed: Optional[_Slot] = None - for tier in _tiers_down_from(start): + for tier in _tiers_down_from(candidate, start): text = tier_text(candidate, tier, contents=contents) if text is None: continue diff --git a/openviking/retrieve/context_assembler/gather.py b/openviking/retrieve/context_assembler/gather.py index c4205dd98f..1330b0ff95 100644 --- a/openviking/retrieve/context_assembler/gather.py +++ b/openviking/retrieve/context_assembler/gather.py @@ -15,10 +15,11 @@ from openviking.core.namespace import AGENT_SHARED_ROOTS, canonical_user_root from openviking.core.retrieval_targets import default_target_directories from openviking.retrieve.context_assembler.params import ( - CATEGORY_KEYS, MEMORY_CATEGORIES, ORIGIN_ORDER, + OTHER_MEMORY_CATEGORY, OTHER_PEER_OVERFETCH, + REPORTED_CATEGORY_KEYS, ) from openviking.server.identity import RequestContext from openviking.utils.search_filters import merge_context_type_filter @@ -136,6 +137,13 @@ def strip_level_suffix(uri: str) -> Tuple[str, bool]: def category_for(item: Any, bucket: Optional[str]) -> str: + """Reported category for one hit; always one of ``REPORTED_CATEGORY_KEYS``. + + Flat retrieval has no owning bucket, so it also reaches the built-in memory + types that own no quota bucket. Those report as ``OTHER_MEMORY_CATEGORY`` + rather than as their directory name, which would be a value the response + contract does not declare and no tier or penalty table covers. + """ if bucket: return bucket uri = _uri(item) @@ -144,14 +152,15 @@ def category_for(item: Any, bucket: Optional[str]) -> str: segment = uri.split(marker, 1)[1].split("/", 1)[0] if segment in MEMORY_CATEGORIES: return segment + return OTHER_MEMORY_CATEGORY declared = str(_get_attr(item, "category", "") or "") - if declared in CATEGORY_KEYS: + if declared in REPORTED_CATEGORY_KEYS: return declared if "/skills/" in uri: return "skills" if "/resources/" in uri or uri.startswith("viking://resources"): return "resources" - return "memories" + return OTHER_MEMORY_CATEGORY def dedupe_keep_best(items: Sequence[Any]) -> List[Any]: diff --git a/openviking/retrieve/context_assembler/params.py b/openviking/retrieve/context_assembler/params.py index 790fbd7cc6..4bf1f72426 100644 --- a/openviking/retrieve/context_assembler/params.py +++ b/openviking/retrieve/context_assembler/params.py @@ -14,7 +14,16 @@ Purpose = Literal["chat", "coding"] MEMORY_CATEGORIES: Tuple[str, ...] = ("events", "entities", "preferences", "experiences") +# Built-in memory types outside MEMORY_CATEGORIES — cases, patterns, tools, +# trajectories and skill-usage memories — cannot own a quota bucket: their +# retrieval scope is the memory root, which every other bucket already covers. +# Flat retrieval still reaches them, so they report under one catch-all category +# that carries a tier default and an other-peer penalty like any named one. +OTHER_MEMORY_CATEGORY = "memories" CATEGORY_KEYS: Tuple[str, ...] = (*MEMORY_CATEGORIES, "resources", "skills") +# Every category an entry may be reported as. Quotas take CATEGORY_KEYS only; +# `detail` and per-category penalties accept the catch-all as well. +REPORTED_CATEGORY_KEYS: Tuple[str, ...] = (*CATEGORY_KEYS, OTHER_MEMORY_CATEGORY) TIER_ORDER: Tuple[Tier, ...] = ("uri", "abstract", "overview", "full") TIER_RANK: Dict[str, int] = {tier: rank for rank, tier in enumerate(TIER_ORDER)} @@ -47,9 +56,21 @@ "experiences": "abstract", "resources": "abstract", "skills": "abstract", + OTHER_MEMORY_CATEGORY: "abstract", } DEFAULT_TIER: Tier = "abstract" +# Categories whose stored ``abstract`` holds the whole file body, because the +# memory writer reuses that scalar as embedding text. For them ``overview`` sits +# *below* ``abstract`` on the content ladder, so standing in for a missing or +# oversized abstract discloses less rather than more. A resource or skill +# abstract is the short generated summary instead, and substituting overview +# there would read a body the caller never asked for — the deepening those two +# categories opt into explicitly. +FULL_BODY_ABSTRACT_CATEGORIES: frozenset[str] = frozenset( + (*MEMORY_CATEGORIES, OTHER_MEMORY_CATEGORY) +) + # How far leftover budget may raise a category above its default tier. A # category absent here never deepens, which is what keeps the default path from # reading resource bodies. @@ -90,6 +111,7 @@ "experiences": 0.02, "resources": 0.02, "skills": 0.02, + OTHER_MEMORY_CATEGORY: 0.1, } @@ -163,7 +185,7 @@ def normalize_penalties(value: Any = None) -> Dict[str, float]: merged[key] = _clamp_penalty(penalty, merged[key]) return merged penalty = _clamp_penalty(value, 0.0) - return dict.fromkeys(CATEGORY_KEYS, penalty) + return dict.fromkeys(REPORTED_CATEGORY_KEYS, penalty) def normalize_exclude_uris(values: Optional[Sequence[str]]) -> set[str]: @@ -199,7 +221,7 @@ def normalize_detail(value: Any) -> DetailPins: by_category={ key: tier for key, tier in value.items() - if key in CATEGORY_KEYS and tier in PINNABLE_TIERS + if key in REPORTED_CATEGORY_KEYS and tier in PINNABLE_TIERS } ) return DetailPins(scalar=value if value in PINNABLE_TIERS else None) diff --git a/openviking/retrieve/context_assembler/pipeline.py b/openviking/retrieve/context_assembler/pipeline.py index ec1568d32a..cbb4e34673 100644 --- a/openviking/retrieve/context_assembler/pipeline.py +++ b/openviking/retrieve/context_assembler/pipeline.py @@ -12,7 +12,7 @@ from typing import Any, Dict, Optional from openviking.retrieve.context_assembler.budget import ( - abstract_over_cap, + oversized_abstract_needs_body, per_entry_cap, plan_entries, ) @@ -100,7 +100,7 @@ async def assemble_context( readable = [ c for c in candidates - if needs_content(c, pins.for_category(c.category)) or abstract_over_cap(c, cap) + if needs_content(c, pins.for_category(c.category)) or oversized_abstract_needs_body(c, cap) ] contents: Dict[str, str] = {} if readable: @@ -128,9 +128,14 @@ async def assemble_context( if rewrite_status == "no_relevant": rendered = "" - if ledger and plan.entries: + # A digest that reports no relevant memory blanks the block, so this turn + # served nothing: recording those URIs would cool them for `dedup_turns` + # turns without the reader ever having seen them, and hold them back from + # the later turn they are relevant to. + served = plan.entries if rewrite_status != "no_relevant" else [] + if ledger and served: try: - await ledger.record(plan.entries) + await ledger.record(served) except Exception as exc: logger.debug("Recall ledger record failed (%s); dedup stays best-effort", exc) diff --git a/openviking/retrieve/context_assembler/tiers.py b/openviking/retrieve/context_assembler/tiers.py index 869456fb90..4194af19a6 100644 --- a/openviking/retrieve/context_assembler/tiers.py +++ b/openviking/retrieve/context_assembler/tiers.py @@ -20,6 +20,7 @@ DEFAULT_TIER, DEFAULT_TIER_BY_CATEGORY, DEPTH_CEILING_BY_CATEGORY, + FULL_BODY_ABSTRACT_CATEGORIES, READ_CONCURRENCY, TIER_RANK, Tier, @@ -193,12 +194,24 @@ def start_tier(candidate: Candidate, pin: Optional[Tier] = None) -> Tier: return "overview" tier = pin or DEFAULT_TIER_BY_CATEGORY.get(candidate.category, DEFAULT_TIER) if tier == "abstract" and not candidate.abstract.strip(): - # Resources that never went through semantic processing have no stored - # abstract; overview keeps them from degrading to a bare URI. - return "overview" + return abstract_substitute(candidate) return tier +def abstract_substitute(candidate: Candidate) -> Tier: + """Tier that stands in when ``abstract`` is unavailable or too expensive. + + Only the full-body-abstract categories can spend ``overview`` here: their + abstract is the whole body, so the substitute discloses strictly less. A + resource or skill whose abstract is missing or oversized degrades to a bare + URI instead — reading its body would cross the opt-in deepening boundary + that keeps generated summaries, not raw content, on the default path. + """ + if candidate.category in FULL_BODY_ABSTRACT_CATEGORIES: + return "overview" + return "uri" + + def tier_window(candidate: Candidate, pin: Optional[Tier] = None) -> Tuple[Tier, Tier]: """``(start, ceiling)``: where the candidate starts and how far it may deepen.""" start = start_tier(candidate, pin) diff --git a/openviking/server/routers/search.py b/openviking/server/routers/search.py index 1eabe1c3ec..c46a72cf4f 100644 --- a/openviking/server/routers/search.py +++ b/openviking/server/routers/search.py @@ -3,7 +3,7 @@ """Search endpoints for OpenViking HTTP Server.""" import math -from typing import Any, Dict, List, Literal, Optional, Union +from typing import Any, Dict, List, Literal, Optional, Sequence, Union from fastapi import APIRouter, Depends from fastapi import Response as FastAPIResponse @@ -16,6 +16,7 @@ DEFAULT_LIMIT, DEFAULT_MAX_TOKENS, MAX_EXCLUDE_URIS, + REPORTED_CATEGORY_KEYS, AssembleParams, DetailRequest, assemble_context, @@ -120,16 +121,23 @@ class FindRequest(BaseModel): telemetry: TelemetryRequest = False -def _reject_unknown_categories(value: Any, label: str) -> None: +def _reject_unknown_categories(value: Any, label: str, allowed: Sequence[str]) -> None: if not isinstance(value, dict): return - unknown = sorted(set(value) - set(CATEGORY_KEYS)) + unknown = sorted(set(value) - set(allowed)) if unknown: raise ValueError( - f"unknown {label} keys: {', '.join(unknown)}; allowed: {', '.join(CATEGORY_KEYS)}" + f"unknown {label} keys: {', '.join(unknown)}; allowed: {', '.join(allowed)}" ) +def _reject_unknown_quota_and_detail(quotas: Any, detail: Any) -> None: + # Quotas name a retrieval bucket, `detail` names a reported category: the + # catch-all memory category is reportable but owns no bucket to size. + _reject_unknown_categories(quotas, "quota", CATEGORY_KEYS) + _reject_unknown_categories(detail, "detail", REPORTED_CATEGORY_KEYS) + + CONTEXT_ONLY_FIELDS = ( "query_expansion", "max_tokens", @@ -200,8 +208,7 @@ def _validate_mode(self) -> "SearchRequest": if self.target_uri: raise ValueError("target_uri is not supported in mode='context'") - _reject_unknown_categories(self.quotas, "quota") - _reject_unknown_categories(self.detail, "detail") + _reject_unknown_quota_and_detail(self.quotas, self.detail) return self @@ -237,8 +244,7 @@ class RecallRequest(BaseModel): @model_validator(mode="after") def _validate_quotas(self) -> "RecallRequest": - _reject_unknown_categories(self.quotas, "quota") - _reject_unknown_categories(self.detail, "detail") + _reject_unknown_quota_and_detail(self.quotas, self.detail) return self diff --git a/tests/retrieve/test_context_assembler_pipeline.py b/tests/retrieve/test_context_assembler_pipeline.py index 6fc78f3ccb..ed00a0e579 100644 --- a/tests/retrieve/test_context_assembler_pipeline.py +++ b/tests/retrieve/test_context_assembler_pipeline.py @@ -6,10 +6,23 @@ from openviking.retrieve.context_assembler import pipeline as pipeline_module from openviking.retrieve.context_assembler import rewrite as rewrite_module +from openviking.retrieve.context_assembler.budget import ( + oversized_abstract_needs_body, + per_entry_cap, + plan_entries, +) +from openviking.retrieve.context_assembler.gather import Candidate, category_for from openviking.retrieve.context_assembler.models import AssembledEntry -from openviking.retrieve.context_assembler.params import AssembleParams, normalize_quotas +from openviking.retrieve.context_assembler.params import ( + OTHER_MEMORY_CATEGORY, + AssembleParams, + normalize_detail, + normalize_penalties, + normalize_quotas, +) from openviking.retrieve.context_assembler.pipeline import assemble_context from openviking.retrieve.context_assembler.render import render_entry +from openviking.retrieve.context_assembler.tiers import tier_window from openviking.server.identity import RequestContext, Role from openviking_cli.session.user_id import UserIdentifier @@ -230,7 +243,7 @@ async def fake_find(**kwargs): { "uri": f"{target_uri}/{index}.md", "score": 0.9 - index * 0.01, - "abstract": f"{target_uri} {index}", + "abstract": f"{target_uri} {index}", "level": 2, } for index in range(count) @@ -446,3 +459,127 @@ async def fake_read(uri, **kwargs): "events": "full", "entities": "abstract", } + + +def _resource_candidate(abstract, category="resources", uri=f"{USER_ROOT}/resources/creds.md"): + return Candidate( + uri=uri, + base_uri=uri, + category=category, + score=0.5, + ranked_score=0.5, + level=2, + abstract=abstract, + origin="actor_peer", + is_directory=False, + read_ctx=_ctx(), + ) + + +def test_resource_without_abstract_never_substitutes_its_body(): + """A missing resource abstract degrades to `uri`, pinned or not. + + Overview extraction of a short file returns the body almost verbatim, so + substituting it would disclose content the `resources` tier ceiling keeps + behind an explicit deepening request. + """ + uri = f"{USER_ROOT}/resources/creds.md" + candidate = _resource_candidate("") + contents = {uri: "# Credentials\n\nTOKEN=secret"} + + for detail in ("abstract", None): + assert tier_window(candidate, normalize_detail(detail).for_category("resources")) == ( + "uri", + "uri", + ) + plan = plan_entries([candidate], contents, max_tokens=1600, detail=detail) + assert [(e.detail, e.text) for e in plan.entries] == [("uri", "")] + + +def test_oversized_resource_abstract_degrades_without_reading_the_body(): + uri = f"{USER_ROOT}/resources/creds.md" + candidate = _resource_candidate("X " * 400) + cap = per_entry_cap(60, 1) + + assert not oversized_abstract_needs_body(candidate, cap) + plan = plan_entries( + [candidate], {uri: "# Credentials\n\nTOKEN=secret"}, max_tokens=60, detail="abstract" + ) + assert [(e.detail, e.text) for e in plan.entries] == [("uri", "")] + + +def test_memory_keeps_overview_as_the_cheaper_abstract_substitute(): + """Memory stores its whole body in `abstract`, so overview discloses less.""" + uri = f"{USER_ROOT}/memories/entities/a.md" + body = "# Summary\n\nA thing happened.\n\n# Detail\n\nlong body" + candidate = _resource_candidate("Y " * 400, category="entities", uri=uri) + + assert oversized_abstract_needs_body(candidate, per_entry_cap(60, 1)) + plan = plan_entries([candidate], {uri: body}, max_tokens=60, detail=None) + assert [(e.detail, e.text) for e in plan.entries] == [("overview", "A thing happened.")] + + +def test_built_in_memory_types_report_one_declared_category(): + """Types outside MEMORY_CATEGORIES own no bucket but stay inside the contract.""" + resolved = { + uri: category_for({"uri": uri}, None) + for uri in ( + f"{USER_ROOT}/memories/trajectories/t.md", + f"{USER_ROOT}/memories/cases/c.md", + f"{USER_ROOT}/memories/skills/s.md", + f"{USER_ROOT}/memories/events/e.md", + f"{USER_ROOT}/skills/real.md", + ) + } + assert resolved == { + f"{USER_ROOT}/memories/trajectories/t.md": OTHER_MEMORY_CATEGORY, + f"{USER_ROOT}/memories/cases/c.md": OTHER_MEMORY_CATEGORY, + f"{USER_ROOT}/memories/skills/s.md": OTHER_MEMORY_CATEGORY, + f"{USER_ROOT}/memories/events/e.md": "events", + f"{USER_ROOT}/skills/real.md": "skills", + } + # The undeclared category used to miss every table: no other-peer penalty + # and no way for a caller to pin its tier. + assert normalize_penalties(None)[OTHER_MEMORY_CATEGORY] > 0 + assert normalize_penalties(0.3)[OTHER_MEMORY_CATEGORY] == 0.3 + assert ( + normalize_detail({OTHER_MEMORY_CATEGORY: "overview"}).for_category(OTHER_MEMORY_CATEGORY) + == "overview" + ) + + +async def test_no_relevant_digest_keeps_uris_out_of_the_dedup_ledger(monkeypatch): + """Nothing was injected, so nothing may enter the cooldown window.""" + recorded = [] + hits = [{"uri": f"{USER_ROOT}/memories/events/a.md", "score": 0.6, "abstract": "abs"}] + monkeypatch.setattr(pipeline_module, "server_rewrite_enabled", lambda mode: True) + + async def empty_rewrite(**kwargs): + del kwargs + return "", "no_relevant", None + + class _Ledger: + status = "on" + turn = 3 + + def cooled_uris(self): + return set() + + async def record(self, entries): + recorded.extend(entry.uri for entry in entries) + + async def fake_load(**kwargs): + del kwargs + return _Ledger() + + monkeypatch.setattr(pipeline_module, "rewrite_context", empty_rewrite) + monkeypatch.setattr(pipeline_module.RecallLedger, "load", staticmethod(fake_load)) + result = await assemble_context( + service=_service(hits=hits, bodies={}), + ctx=_ctx(), + params=AssembleParams(query="unrelated", rewrite=True, session_id="s", dedup_turns=5), + ) + + assert result.rendered == "" + assert len(result.entries) == 1 + assert recorded == [] From 14ccb62bd2ad68fc197e9bb7f05b266f01457683 Mon Sep 17 00:00:00 2001 From: "zhengxiao.wu" Date: Wed, 5 Aug 2026 17:59:05 +0800 Subject: [PATCH 2/2] fix(retrieval): make the context deadline and expansion opt-out reachable Forwarding a session id turns on server-side query expansion, an LLM call with its own 5s fuse, but neither the deadline that was supposed to cover it nor the switch that turns it off reached the two harnesses this PR newly enabled it for. - `contextRequestTimeoutMs()` now derives the deadline from the request body rather than from `cfg` plus a rewrite flag. The body is what states which server stages will run: a session takes the expansion fuse, `rewrite` takes the digest fuse, and a bare retrieval takes neither and keeps the caller's own budget. Reading `cfg` alone could not tell those apart. - OpenCode pinned `timeoutMs: 5000` after spreading the helper's options and pi ignored them entirely, so the helper's deadline was dead code in both. Their own budgets are now defaults rather than ceilings. OpenCode's 5s in particular was shorter than the expansion fuse it had just enabled, so a legal request would have been aborted client-side and dropped back to the path with neither dedup nor expansion. - OpenCode and pi read `OPENVIKING_RECALL_QUERY_EXPANSION` (and `recallQueryExpansion` in their own config files) and set the `configured` flag the shared body builder requires, so the documented opt-out exists where the cost was introduced. - The integration overview no longer implies every harness reads the same environment knobs, and describes the deadline as per-stage rather than rewrite-only. --- docs/en/agent-integrations/01-overview.md | 6 +-- docs/zh/agent-integrations/01-overview.md | 6 +-- .../scripts/shared/recall-core.mjs | 40 ++++++++++------- .../scripts/shared/recall-core.mjs | 40 ++++++++++------- .../memory-plugin-shared/lib/recall-core.mjs | 40 ++++++++++------- .../memory-plugin-shared/recall-core.test.mjs | 23 +++++++++- .../recall-session-wiring.test.mjs | 45 +++++++++++++++++++ examples/opencode-plugin/lib/config.mjs | 10 +++++ .../opencode-plugin/lib/memory-recall.mjs | 6 ++- .../lib/shared/recall-core.mjs | 40 ++++++++++------- examples/pi-coding-agent-extension/config.ts | 12 +++++ examples/pi-coding-agent-extension/recall.ts | 6 ++- .../shared/recall-core.mjs | 40 ++++++++++------- .../scripts/shared/recall-core.mjs | 40 ++++++++++------- 14 files changed, 255 insertions(+), 99 deletions(-) diff --git a/docs/en/agent-integrations/01-overview.md b/docs/en/agent-integrations/01-overview.md index 73930ddcda..27e99f82ae 100644 --- a/docs/en/agent-integrations/01-overview.md +++ b/docs/en/agent-integrations/01-overview.md @@ -27,7 +27,7 @@ Every integration on this page connects to a running OpenViking server. If you d Query expansion and recall-result compression are two independent, optional model calls. Disable both in the Agent plugin when response latency matters most; semantic retrieval, budgeting, tier degradation, and cross-turn dedup continue to work. -The same environment variables apply to both the Claude Code and Codex memory plugins: +The same environment variables apply to both the Claude Code and Codex memory plugins. Query expansion is also switchable in OpenCode and pi; compression is Claude Code and Codex only. ```bash export OPENVIKING_RECALL_QUERY_EXPANSION=off @@ -56,6 +56,6 @@ The same settings can live in `~/.openviking/ovcli.conf`: Environment variables take precedence over `ovcli.conf`. Restart the Agent after changing these settings so its hook processes reload the configuration. These are plugin-client settings; the server's `ov.conf` does not need to change. -The `plugin` section is read by the Claude Code and Codex plugins. The other harnesses take the same knobs from the environment only, so a `plugin.opencode`-style entry named after them is currently inert. +The `plugin` section is read by the Claude Code and Codex plugins, so a `plugin` entry named after another harness is currently inert. OpenCode and pi read `OPENVIKING_RECALL_QUERY_EXPANSION` from the environment (or `recallQueryExpansion` in their own config file) but not `OPENVIKING_RECALL_COMPRESS`, since neither requests a server digest. -When Claude Code asks the server for a digest, the context request waits longer than an ordinary request, and aborting earlier would discard the whole response rather than just the digest. The server pipeline is serial and the rewrite is only its last stage: query expansion (`retrieval.recall_intent_timeout_s`, 5s) runs first, then retrieval, body reads and budgeting, and only then the rewrite (`retrieval.recall_rewrite_timeout_s`, 30s). The default deadline is 45s so that a request where every server stage stayed inside its own fuse is not cut off client-side. Set `OPENVIKING_RECALL_CONTEXT_TIMEOUT_MS` (or `plugin.recallContextTimeoutMs`) to pin it — keep it above the sum of both fuses and below the Agent's own hook timeout. +A context request waits longer than an ordinary request, because aborting it client-side discards the whole response rather than just the stage that ran long. The server pipeline is serial and each optional stage has its own fuse: query expansion (`retrieval.recall_intent_timeout_s`, 5s) runs first, then retrieval, body reads and budgeting, and only then the digest rewrite (`retrieval.recall_rewrite_timeout_s`, 30s). The deadline therefore follows what the request actually asks for — 15s once it carries a session and can spend the expansion fuse, 45s when it also asks for a digest, and the plugin's ordinary timeout when it asks for neither. Set `OPENVIKING_RECALL_CONTEXT_TIMEOUT_MS` (or `plugin.recallContextTimeoutMs`) to pin it — keep it above the fuses the request will spend and below the Agent's own hook timeout. diff --git a/docs/zh/agent-integrations/01-overview.md b/docs/zh/agent-integrations/01-overview.md index 9fe7175674..2d421a4a42 100644 --- a/docs/zh/agent-integrations/01-overview.md +++ b/docs/zh/agent-integrations/01-overview.md @@ -27,7 +27,7 @@ OpenViking 可以作为多种 Agent 运行时的长期记忆与上下文后端 查询扩展和召回结果压缩是两个独立的可选模型调用。需要优先保证响应速度时,可以在 Agent 插件端同时关闭它们;语义检索、预算控制、档位降级和跨轮去重仍会正常工作。 -下面这组环境变量同时适用于 Claude Code 和 Codex: +下面这组环境变量同时适用于 Claude Code 和 Codex。查询扩展在 OpenCode 和 pi 上同样可以关闭;压缩只有 Claude Code 和 Codex 支持。 ```bash export OPENVIKING_RECALL_QUERY_EXPANSION=off @@ -56,6 +56,6 @@ export OPENVIKING_RECALL_COMPRESS=off 环境变量优先于 `ovcli.conf`。修改后重启对应的 Agent,让 hook 进程重新加载配置。上述设置属于插件客户端,不需要修改服务端的 `ov.conf`。 -`plugin` 段目前由 Claude Code 和 Codex 插件读取。其余 harness 只从环境变量取这些开关,因此以它们命名的 `plugin.opencode` 之类条目当前不生效。 +`plugin` 段目前由 Claude Code 和 Codex 插件读取,因此以其他 harness 命名的 `plugin` 条目当前不生效。OpenCode 和 pi 支持用环境变量 `OPENVIKING_RECALL_QUERY_EXPANSION`(或各自配置文件里的 `recallQueryExpansion`)关闭查询扩展,但不支持 `OPENVIKING_RECALL_COMPRESS`——这两个 harness 不会请求服务端 digest。 -当 Claude Code 请求服务端生成 digest 时,这次 context 请求的等待时间比普通请求更长,客户端提前中断会丢掉整个响应,而不只是 digest。服务端流水线是串行的,rewrite 只是最后一段:先是查询扩展(`retrieval.recall_intent_timeout_s`,5 秒),然后是检索、正文读取和预算规划,最后才是 rewrite(`retrieval.recall_rewrite_timeout_s`,30 秒)。默认上限取 45 秒,这样每个服务端阶段都没超出自身保险丝的请求就不会被客户端切断。可以用 `OPENVIKING_RECALL_CONTEXT_TIMEOUT_MS`(或 `plugin.recallContextTimeoutMs`)指定这个上限,取值应高于两个保险丝之和、低于 Agent 自身的 hook 超时。 +context 请求的等待时间比普通请求更长,因为客户端提前中断会丢掉整个响应,而不只是超时的那一段。服务端流水线是串行的,每个可选阶段各有保险丝:先是查询扩展(`retrieval.recall_intent_timeout_s`,5 秒),然后是检索、正文读取和预算规划,最后才是 digest 重写(`retrieval.recall_rewrite_timeout_s`,30 秒)。因此这个上限按请求实际启用的阶段决定——带 session、会走查询扩展时取 15 秒,同时还要 digest 时取 45 秒,两者都不涉及时沿用插件自身的普通超时。可以用 `OPENVIKING_RECALL_CONTEXT_TIMEOUT_MS`(或 `plugin.recallContextTimeoutMs`)指定这个上限,取值应高于该请求会用到的保险丝、低于 Agent 自身的 hook 超时。 diff --git a/examples/claude-code-memory-plugin/scripts/shared/recall-core.mjs b/examples/claude-code-memory-plugin/scripts/shared/recall-core.mjs index d6e53692a9..a6e94eef5a 100644 --- a/examples/claude-code-memory-plugin/scripts/shared/recall-core.mjs +++ b/examples/claude-code-memory-plugin/scripts/shared/recall-core.mjs @@ -138,28 +138,38 @@ export function buildContextSearchBody(cfg = {}, options = {}) { return body; } -// The server pipeline is serial, and only its last stage is the rewrite fuse: -// query expansion (retrieval.recall_intent_timeout_s, 5s) runs first, then -// retrieval, body reads and budgeting, and only then the rewrite -// (retrieval.recall_rewrite_timeout_s, 30s). A deadline covering the rewrite -// alone aborts requests where every server stage stayed inside its own fuse, so -// this covers both fuses plus the work between them and still leaves a quarter -// of the 60s prompt-hook budget. +// The server pipeline is serial and each optional stage has its own fuse. A +// request is aborted client-side unless its deadline covers every stage it +// asked for, and aborting discards the whole response rather than just the +// stage that ran long. +// +// session_id -> query expansion (retrieval.recall_intent_timeout_s, 5s) +// always -> retrieval, body reads, budget planning +// rewrite -> digest (retrieval.recall_rewrite_timeout_s, 30s) +// +// Both budgets stay inside the 60s prompt-hook allowance, the rewrite one with +// a quarter to spare. +const EXPANSION_REQUEST_TIMEOUT_MS = 15000; const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 45000; /** * HTTP deadline for one context request, or undefined to keep the caller's own. * - * The ordinary request timeout is shorter than the server's rewrite fuse, so a - * digest that finishes inside its own fuse would be aborted client-side. That - * loses the whole response rather than just the digest, including the - * uncompressed block the server still returns when a rewrite fails. + * Derived from the request body, because the body is what states which server + * stages will run: reading `cfg` alone cannot tell a bare retrieval from one + * that also spends the expansion or rewrite fuse. */ -export function contextRequestTimeoutMs(cfg = {}, serverRewrite = false) { - if (!serverRewrite) return undefined; +export function contextRequestTimeoutMs(cfg = {}, body = {}) { + const wantsRewrite = body.rewrite !== undefined; + // `query_expansion` defaults to "auto" server-side, so only an explicit "off" + // takes the expansion fuse back out of the budget. + const wantsExpansion = Boolean(body.session_id) && body.query_expansion !== "off"; + if (!wantsRewrite && !wantsExpansion) return undefined; + const configured = Number(cfg.recallContextTimeoutMs); if (Number.isFinite(configured) && configured > 0) return Math.max(1000, Math.floor(configured)); - return Math.max(Number(cfg.timeoutMs) || 0, SERVER_REWRITE_REQUEST_TIMEOUT_MS); + const floor = wantsRewrite ? SERVER_REWRITE_REQUEST_TIMEOUT_MS : EXPANSION_REQUEST_TIMEOUT_MS; + return Math.max(Number(cfg.timeoutMs) || 0, floor); } /** @@ -447,7 +457,7 @@ export async function fetchAssembledContext(fetchJSON, cfg, query, options = {}) const res = await fetchJSON("/api/v1/search/search", { method: "POST", body: JSON.stringify(body), - }, { actorPeerId, timeoutMs: contextRequestTimeoutMs(cfg, body.rewrite !== undefined) }); + }, { actorPeerId, timeoutMs: contextRequestTimeoutMs(cfg, body) }); if (!res.ok) { const status = res.status || 0; diff --git a/examples/codex-memory-plugin/scripts/shared/recall-core.mjs b/examples/codex-memory-plugin/scripts/shared/recall-core.mjs index d6e53692a9..a6e94eef5a 100644 --- a/examples/codex-memory-plugin/scripts/shared/recall-core.mjs +++ b/examples/codex-memory-plugin/scripts/shared/recall-core.mjs @@ -138,28 +138,38 @@ export function buildContextSearchBody(cfg = {}, options = {}) { return body; } -// The server pipeline is serial, and only its last stage is the rewrite fuse: -// query expansion (retrieval.recall_intent_timeout_s, 5s) runs first, then -// retrieval, body reads and budgeting, and only then the rewrite -// (retrieval.recall_rewrite_timeout_s, 30s). A deadline covering the rewrite -// alone aborts requests where every server stage stayed inside its own fuse, so -// this covers both fuses plus the work between them and still leaves a quarter -// of the 60s prompt-hook budget. +// The server pipeline is serial and each optional stage has its own fuse. A +// request is aborted client-side unless its deadline covers every stage it +// asked for, and aborting discards the whole response rather than just the +// stage that ran long. +// +// session_id -> query expansion (retrieval.recall_intent_timeout_s, 5s) +// always -> retrieval, body reads, budget planning +// rewrite -> digest (retrieval.recall_rewrite_timeout_s, 30s) +// +// Both budgets stay inside the 60s prompt-hook allowance, the rewrite one with +// a quarter to spare. +const EXPANSION_REQUEST_TIMEOUT_MS = 15000; const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 45000; /** * HTTP deadline for one context request, or undefined to keep the caller's own. * - * The ordinary request timeout is shorter than the server's rewrite fuse, so a - * digest that finishes inside its own fuse would be aborted client-side. That - * loses the whole response rather than just the digest, including the - * uncompressed block the server still returns when a rewrite fails. + * Derived from the request body, because the body is what states which server + * stages will run: reading `cfg` alone cannot tell a bare retrieval from one + * that also spends the expansion or rewrite fuse. */ -export function contextRequestTimeoutMs(cfg = {}, serverRewrite = false) { - if (!serverRewrite) return undefined; +export function contextRequestTimeoutMs(cfg = {}, body = {}) { + const wantsRewrite = body.rewrite !== undefined; + // `query_expansion` defaults to "auto" server-side, so only an explicit "off" + // takes the expansion fuse back out of the budget. + const wantsExpansion = Boolean(body.session_id) && body.query_expansion !== "off"; + if (!wantsRewrite && !wantsExpansion) return undefined; + const configured = Number(cfg.recallContextTimeoutMs); if (Number.isFinite(configured) && configured > 0) return Math.max(1000, Math.floor(configured)); - return Math.max(Number(cfg.timeoutMs) || 0, SERVER_REWRITE_REQUEST_TIMEOUT_MS); + const floor = wantsRewrite ? SERVER_REWRITE_REQUEST_TIMEOUT_MS : EXPANSION_REQUEST_TIMEOUT_MS; + return Math.max(Number(cfg.timeoutMs) || 0, floor); } /** @@ -447,7 +457,7 @@ export async function fetchAssembledContext(fetchJSON, cfg, query, options = {}) const res = await fetchJSON("/api/v1/search/search", { method: "POST", body: JSON.stringify(body), - }, { actorPeerId, timeoutMs: contextRequestTimeoutMs(cfg, body.rewrite !== undefined) }); + }, { actorPeerId, timeoutMs: contextRequestTimeoutMs(cfg, body) }); if (!res.ok) { const status = res.status || 0; diff --git a/examples/memory-plugin-shared/lib/recall-core.mjs b/examples/memory-plugin-shared/lib/recall-core.mjs index c17db7fc64..5a92081d23 100644 --- a/examples/memory-plugin-shared/lib/recall-core.mjs +++ b/examples/memory-plugin-shared/lib/recall-core.mjs @@ -137,28 +137,38 @@ export function buildContextSearchBody(cfg = {}, options = {}) { return body; } -// The server pipeline is serial, and only its last stage is the rewrite fuse: -// query expansion (retrieval.recall_intent_timeout_s, 5s) runs first, then -// retrieval, body reads and budgeting, and only then the rewrite -// (retrieval.recall_rewrite_timeout_s, 30s). A deadline covering the rewrite -// alone aborts requests where every server stage stayed inside its own fuse, so -// this covers both fuses plus the work between them and still leaves a quarter -// of the 60s prompt-hook budget. +// The server pipeline is serial and each optional stage has its own fuse. A +// request is aborted client-side unless its deadline covers every stage it +// asked for, and aborting discards the whole response rather than just the +// stage that ran long. +// +// session_id -> query expansion (retrieval.recall_intent_timeout_s, 5s) +// always -> retrieval, body reads, budget planning +// rewrite -> digest (retrieval.recall_rewrite_timeout_s, 30s) +// +// Both budgets stay inside the 60s prompt-hook allowance, the rewrite one with +// a quarter to spare. +const EXPANSION_REQUEST_TIMEOUT_MS = 15000; const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 45000; /** * HTTP deadline for one context request, or undefined to keep the caller's own. * - * The ordinary request timeout is shorter than the server's rewrite fuse, so a - * digest that finishes inside its own fuse would be aborted client-side. That - * loses the whole response rather than just the digest, including the - * uncompressed block the server still returns when a rewrite fails. + * Derived from the request body, because the body is what states which server + * stages will run: reading `cfg` alone cannot tell a bare retrieval from one + * that also spends the expansion or rewrite fuse. */ -export function contextRequestTimeoutMs(cfg = {}, serverRewrite = false) { - if (!serverRewrite) return undefined; +export function contextRequestTimeoutMs(cfg = {}, body = {}) { + const wantsRewrite = body.rewrite !== undefined; + // `query_expansion` defaults to "auto" server-side, so only an explicit "off" + // takes the expansion fuse back out of the budget. + const wantsExpansion = Boolean(body.session_id) && body.query_expansion !== "off"; + if (!wantsRewrite && !wantsExpansion) return undefined; + const configured = Number(cfg.recallContextTimeoutMs); if (Number.isFinite(configured) && configured > 0) return Math.max(1000, Math.floor(configured)); - return Math.max(Number(cfg.timeoutMs) || 0, SERVER_REWRITE_REQUEST_TIMEOUT_MS); + const floor = wantsRewrite ? SERVER_REWRITE_REQUEST_TIMEOUT_MS : EXPANSION_REQUEST_TIMEOUT_MS; + return Math.max(Number(cfg.timeoutMs) || 0, floor); } /** @@ -446,7 +456,7 @@ export async function fetchAssembledContext(fetchJSON, cfg, query, options = {}) const res = await fetchJSON("/api/v1/search/search", { method: "POST", body: JSON.stringify(body), - }, { actorPeerId, timeoutMs: contextRequestTimeoutMs(cfg, body.rewrite !== undefined) }); + }, { actorPeerId, timeoutMs: contextRequestTimeoutMs(cfg, body) }); if (!res.ok) { const status = res.status || 0; diff --git a/examples/memory-plugin-shared/recall-core.test.mjs b/examples/memory-plugin-shared/recall-core.test.mjs index 2e549081fe..a1b97ddd0c 100644 --- a/examples/memory-plugin-shared/recall-core.test.mjs +++ b/examples/memory-plugin-shared/recall-core.test.mjs @@ -111,7 +111,28 @@ test("a server-side digest outlasts the ordinary request timeout", async () => { `deadline must outlast both server fuses plus the work between, got ${timeouts[0]}`, ); assert.equal(timeouts[1], undefined); - assert.equal(contextRequestTimeoutMs({ ...cfg, recallContextTimeoutMs: 50000 }, true), 50000); + assert.equal( + contextRequestTimeoutMs({ ...cfg, recallContextTimeoutMs: 50000 }, { rewrite: true }), + 50000, + ); +}); + +test("the deadline follows the stages the request actually asks for", async () => { + const cfg = { timeoutMs: 5000 }; + + // A bare retrieval spends no server fuse, so the caller keeps its own budget. + assert.equal(contextRequestTimeoutMs(cfg, {}), undefined); + assert.equal(contextRequestTimeoutMs(cfg, { session_id: "s", query_expansion: "off" }), undefined); + + // A session engages query expansion, which the server defaults to "auto". + // Without headroom a 5s caller aborts a request the expansion fuse alone may + // consume, then falls back to the path with no dedup and no expansion. + const withSession = contextRequestTimeoutMs(cfg, { session_id: "s" }); + assert.ok(withSession > 5000, `expansion needs headroom over the caller budget, got ${withSession}`); + + // A digest costs the rewrite fuse on top of everything above it. + const withRewrite = contextRequestTimeoutMs(cfg, { session_id: "s", rewrite: true }); + assert.ok(withRewrite > withSession, "a digest must outlast a plain expanded request"); }); test("buildRecallBlock prefers a cited server digest", async () => { diff --git a/examples/memory-plugin-shared/recall-session-wiring.test.mjs b/examples/memory-plugin-shared/recall-session-wiring.test.mjs index 5269650732..1a43de75b6 100644 --- a/examples/memory-plugin-shared/recall-session-wiring.test.mjs +++ b/examples/memory-plugin-shared/recall-session-wiring.test.mjs @@ -51,3 +51,48 @@ test("OpenCode passes the session manager into the recall component", async () = const source = await readFile(join(ROOT, "examples", "opencode-plugin", "index.mjs"), "utf-8"); assert.match(source, /createMemoryRecall\(\{\s*config,\s*sessionManager\s*\}\)/); }); + +// Forwarding a session id makes the server spend the query-expansion fuse, so +// an adapter that pins its own HTTP timeout aborts a request the server is +// still inside and drops back to the path without dedup or expansion. Each +// adapter's own budget has to stay a default, not a ceiling. +const TIMEOUT_PASSTHROUGH = [ + { + name: "OpenCode keeps the per-request deadline the helper hands down", + file: join(ROOT, "examples", "opencode-plugin", "lib", "memory-recall.mjs"), + pattern: /timeoutMs:\s*options\.timeoutMs\s*\?\?\s*\d+/, + }, + { + name: "pi keeps the per-request deadline the helper hands down", + file: join(ROOT, "examples", "pi-coding-agent-extension", "recall.ts"), + pattern: /fetchJSON\(path,\s*init,\s*options\?\.timeoutMs\s*\?\?\s*\d+\)/, + }, +]; + +for (const { name, file, pattern } of TIMEOUT_PASSTHROUGH) { + test(name, async () => { + const source = await readFile(file, "utf-8"); + assert.match(source, pattern); + }); +} + +const QUERY_EXPANSION_OPT_OUT = [ + { + name: "OpenCode exposes the query-expansion opt-out", + file: join(ROOT, "examples", "opencode-plugin", "lib", "config.mjs"), + }, + { + name: "pi exposes the query-expansion opt-out", + file: join(ROOT, "examples", "pi-coding-agent-extension", "config.ts"), + }, +]; + +for (const { name, file } of QUERY_EXPANSION_OPT_OUT) { + test(name, async () => { + const source = await readFile(file, "utf-8"); + // The env var alone is not enough: buildContextSearchBody only emits + // `query_expansion` when the harness also marks it as configured. + assert.match(source, /OPENVIKING_RECALL_QUERY_EXPANSION/); + assert.match(source, /recallQueryExpansionConfigured/); + }); +} diff --git a/examples/opencode-plugin/lib/config.mjs b/examples/opencode-plugin/lib/config.mjs index b1ddd1e987..77f834e117 100644 --- a/examples/opencode-plugin/lib/config.mjs +++ b/examples/opencode-plugin/lib/config.mjs @@ -17,6 +17,9 @@ const DEFAULT_CONFIG = { peerId: "", workspacePeer: true, recallPeerScope: "all", + // Server-side query expansion costs a model call before retrieval starts, so + // it has to be switchable from the client that pays the latency. + recallQueryExpansion: "auto", enabled: true, timeoutMs: 30000, runtime: { @@ -159,9 +162,11 @@ function applyBehaviorConfig(config, fileConfig = {}) { "debugLogPath", "workspacePeer", "recallPeerScope", + "recallQueryExpansion", ]) { if (fileConfig[key] !== undefined) config[key] = fileConfig[key] } + config.recallQueryExpansionConfigured = fileConfig.recallQueryExpansion !== undefined } function applyEnv(config) { @@ -182,6 +187,10 @@ function applyEnv(config) { config.autoRecall.preferAbstract = envBool("OPENVIKING_RECALL_PREFER_ABSTRACT") ?? config.autoRecall.preferAbstract } if (process.env.OPENVIKING_RECALL_PEER_SCOPE) config.recallPeerScope = process.env.OPENVIKING_RECALL_PEER_SCOPE + if (process.env.OPENVIKING_RECALL_QUERY_EXPANSION) { + config.recallQueryExpansion = process.env.OPENVIKING_RECALL_QUERY_EXPANSION + config.recallQueryExpansionConfigured = true + } if (process.env.OPENVIKING_WORKSPACE_PEER !== undefined) { config.workspacePeer = envBool("OPENVIKING_WORKSPACE_PEER") ?? config.workspacePeer } @@ -241,6 +250,7 @@ function normalizeConfig(config) { config.autoRecall.minQueryLength = Math.max(1, Math.min(64, Math.round(Number(config.autoRecall.minQueryLength) || 3))) config.captureMode = config.captureMode === "keyword" ? "keyword" : "semantic" config.recallPeerScope = config.recallPeerScope === "actor" ? "actor" : "all" + config.recallQueryExpansion = config.recallQueryExpansion === "off" ? "off" : "auto" config.captureMaxLength = Math.max(200, Math.min(100000, Math.round(Number(config.captureMaxLength) || 24000))) config.captureToolMaxChars = Math.max(200, Math.min(20000, Math.round(Number(config.captureToolMaxChars) || 2000))) config.commitTokenThreshold = Math.max(1000, Math.round(Number(config.commitTokenThreshold) || 20000)) diff --git a/examples/opencode-plugin/lib/memory-recall.mjs b/examples/opencode-plugin/lib/memory-recall.mjs index f67d8695d4..850ecfbd47 100644 --- a/examples/opencode-plugin/lib/memory-recall.mjs +++ b/examples/opencode-plugin/lib/memory-recall.mjs @@ -18,7 +18,11 @@ export function createMemoryRecall({ config, sessionManager }) { if (!health.ok) return const block = await buildRecallBlock( - (path, init = {}, options = {}) => fetchJSON(config, path, init, { ...options, timeoutMs: 5000 }), + // 5s is this hook's own budget for a bare retrieval; when the request + // also spends a server fuse the helper hands down a longer deadline, and + // overriding it here would abort a request the server was still inside. + (path, init = {}, options = {}) => + fetchJSON(config, path, init, { ...options, timeoutMs: options.timeoutMs ?? 5000 }), config, query, { diff --git a/examples/opencode-plugin/lib/shared/recall-core.mjs b/examples/opencode-plugin/lib/shared/recall-core.mjs index d6e53692a9..a6e94eef5a 100644 --- a/examples/opencode-plugin/lib/shared/recall-core.mjs +++ b/examples/opencode-plugin/lib/shared/recall-core.mjs @@ -138,28 +138,38 @@ export function buildContextSearchBody(cfg = {}, options = {}) { return body; } -// The server pipeline is serial, and only its last stage is the rewrite fuse: -// query expansion (retrieval.recall_intent_timeout_s, 5s) runs first, then -// retrieval, body reads and budgeting, and only then the rewrite -// (retrieval.recall_rewrite_timeout_s, 30s). A deadline covering the rewrite -// alone aborts requests where every server stage stayed inside its own fuse, so -// this covers both fuses plus the work between them and still leaves a quarter -// of the 60s prompt-hook budget. +// The server pipeline is serial and each optional stage has its own fuse. A +// request is aborted client-side unless its deadline covers every stage it +// asked for, and aborting discards the whole response rather than just the +// stage that ran long. +// +// session_id -> query expansion (retrieval.recall_intent_timeout_s, 5s) +// always -> retrieval, body reads, budget planning +// rewrite -> digest (retrieval.recall_rewrite_timeout_s, 30s) +// +// Both budgets stay inside the 60s prompt-hook allowance, the rewrite one with +// a quarter to spare. +const EXPANSION_REQUEST_TIMEOUT_MS = 15000; const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 45000; /** * HTTP deadline for one context request, or undefined to keep the caller's own. * - * The ordinary request timeout is shorter than the server's rewrite fuse, so a - * digest that finishes inside its own fuse would be aborted client-side. That - * loses the whole response rather than just the digest, including the - * uncompressed block the server still returns when a rewrite fails. + * Derived from the request body, because the body is what states which server + * stages will run: reading `cfg` alone cannot tell a bare retrieval from one + * that also spends the expansion or rewrite fuse. */ -export function contextRequestTimeoutMs(cfg = {}, serverRewrite = false) { - if (!serverRewrite) return undefined; +export function contextRequestTimeoutMs(cfg = {}, body = {}) { + const wantsRewrite = body.rewrite !== undefined; + // `query_expansion` defaults to "auto" server-side, so only an explicit "off" + // takes the expansion fuse back out of the budget. + const wantsExpansion = Boolean(body.session_id) && body.query_expansion !== "off"; + if (!wantsRewrite && !wantsExpansion) return undefined; + const configured = Number(cfg.recallContextTimeoutMs); if (Number.isFinite(configured) && configured > 0) return Math.max(1000, Math.floor(configured)); - return Math.max(Number(cfg.timeoutMs) || 0, SERVER_REWRITE_REQUEST_TIMEOUT_MS); + const floor = wantsRewrite ? SERVER_REWRITE_REQUEST_TIMEOUT_MS : EXPANSION_REQUEST_TIMEOUT_MS; + return Math.max(Number(cfg.timeoutMs) || 0, floor); } /** @@ -447,7 +457,7 @@ export async function fetchAssembledContext(fetchJSON, cfg, query, options = {}) const res = await fetchJSON("/api/v1/search/search", { method: "POST", body: JSON.stringify(body), - }, { actorPeerId, timeoutMs: contextRequestTimeoutMs(cfg, body.rewrite !== undefined) }); + }, { actorPeerId, timeoutMs: contextRequestTimeoutMs(cfg, body) }); if (!res.ok) { const status = res.status || 0; diff --git a/examples/pi-coding-agent-extension/config.ts b/examples/pi-coding-agent-extension/config.ts index c822e450c5..82f8a89f9b 100644 --- a/examples/pi-coding-agent-extension/config.ts +++ b/examples/pi-coding-agent-extension/config.ts @@ -16,6 +16,8 @@ export interface OVConfig { userAgent: string; workspacePeer: boolean; recallPeerScope: "actor" | "all"; + recallQueryExpansion: "auto" | "off"; + recallQueryExpansionConfigured: boolean; syncTurns: boolean; recallTokenBudget: number; recallMaxContentChars: number; @@ -53,6 +55,10 @@ const DEFAULT_CONFIG: OVConfig = { userAgent: "", workspacePeer: true, recallPeerScope: "all", + // Server-side query expansion costs a model call before retrieval starts, so + // it has to be switchable from the client that pays the latency. + recallQueryExpansion: "auto", + recallQueryExpansionConfigured: false, syncTurns: true, recallTokenBudget: 2000, recallMaxContentChars: 500, @@ -101,6 +107,7 @@ export function loadConfig(extensionDir: string): OVConfig { peerId: creds.peerId, userAgent: buildUserAgent("pi", EXTENSION_VERSION), recallLimitConfigured: Object.prototype.hasOwnProperty.call(file, "recallLimit"), + recallQueryExpansionConfigured: Object.prototype.hasOwnProperty.call(file, "recallQueryExpansion"), recallTokenBudget: file.recallTokenBudget ?? file.recallBudget ?? DEFAULT_CONFIG.recallTokenBudget, scoreThreshold: file.scoreThreshold ?? file.recallScoreThreshold ?? DEFAULT_CONFIG.scoreThreshold, minQueryLength: file.minQueryLength ?? file.recallMinQueryLength ?? DEFAULT_CONFIG.minQueryLength, @@ -128,6 +135,10 @@ export function loadConfig(extensionDir: string): OVConfig { config.recallLimit = Number(process.env.OPENVIKING_RECALL_LIMIT); config.recallLimitConfigured = true; } + if (process.env.OPENVIKING_RECALL_QUERY_EXPANSION) { + config.recallQueryExpansion = process.env.OPENVIKING_RECALL_QUERY_EXPANSION === "off" ? "off" : "auto"; + config.recallQueryExpansionConfigured = true; + } config.recallLimit = clampInt(config.recallLimit, 1, 50, DEFAULT_CONFIG.recallLimit); config.recallMaxContentChars = clampInt(config.recallMaxContentChars, 100, 5000, DEFAULT_CONFIG.recallMaxContentChars); @@ -148,6 +159,7 @@ export function loadConfig(extensionDir: string): OVConfig { config.captureToolMaxChars = clampInt(config.captureToolMaxChars, 200, 20000, DEFAULT_CONFIG.captureToolMaxChars); config.captureMode = config.captureMode === "keyword" ? "keyword" : "semantic"; config.recallPeerScope = config.recallPeerScope === "actor" ? "actor" : "all"; + config.recallQueryExpansion = config.recallQueryExpansion === "off" ? "off" : "auto"; if (!Array.isArray(config.bypassPatterns)) config.bypassPatterns = []; config.peerId = resolveEffectivePeerId({ cfg: config as any, cwd: process.cwd() }).peerId; return config; diff --git a/examples/pi-coding-agent-extension/recall.ts b/examples/pi-coding-agent-extension/recall.ts index 9c60d7f0da..400033d002 100644 --- a/examples/pi-coding-agent-extension/recall.ts +++ b/examples/pi-coding-agent-extension/recall.ts @@ -37,7 +37,11 @@ export class RecallManager { } const block = await buildRecallBlock( - (path: string, init?: any, options?: any) => this.client.fetchJSON(path, init, 10000), + // 10s is this extension's own budget for a bare retrieval; when the + // request also spends a server fuse the helper hands down a longer + // deadline, and ignoring it would abort a request still inside its fuse. + (path: string, init?: any, options?: any) => + this.client.fetchJSON(path, init, options?.timeoutMs ?? 10000), this.config as any, userQuery, { diff --git a/examples/pi-coding-agent-extension/shared/recall-core.mjs b/examples/pi-coding-agent-extension/shared/recall-core.mjs index d6e53692a9..a6e94eef5a 100644 --- a/examples/pi-coding-agent-extension/shared/recall-core.mjs +++ b/examples/pi-coding-agent-extension/shared/recall-core.mjs @@ -138,28 +138,38 @@ export function buildContextSearchBody(cfg = {}, options = {}) { return body; } -// The server pipeline is serial, and only its last stage is the rewrite fuse: -// query expansion (retrieval.recall_intent_timeout_s, 5s) runs first, then -// retrieval, body reads and budgeting, and only then the rewrite -// (retrieval.recall_rewrite_timeout_s, 30s). A deadline covering the rewrite -// alone aborts requests where every server stage stayed inside its own fuse, so -// this covers both fuses plus the work between them and still leaves a quarter -// of the 60s prompt-hook budget. +// The server pipeline is serial and each optional stage has its own fuse. A +// request is aborted client-side unless its deadline covers every stage it +// asked for, and aborting discards the whole response rather than just the +// stage that ran long. +// +// session_id -> query expansion (retrieval.recall_intent_timeout_s, 5s) +// always -> retrieval, body reads, budget planning +// rewrite -> digest (retrieval.recall_rewrite_timeout_s, 30s) +// +// Both budgets stay inside the 60s prompt-hook allowance, the rewrite one with +// a quarter to spare. +const EXPANSION_REQUEST_TIMEOUT_MS = 15000; const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 45000; /** * HTTP deadline for one context request, or undefined to keep the caller's own. * - * The ordinary request timeout is shorter than the server's rewrite fuse, so a - * digest that finishes inside its own fuse would be aborted client-side. That - * loses the whole response rather than just the digest, including the - * uncompressed block the server still returns when a rewrite fails. + * Derived from the request body, because the body is what states which server + * stages will run: reading `cfg` alone cannot tell a bare retrieval from one + * that also spends the expansion or rewrite fuse. */ -export function contextRequestTimeoutMs(cfg = {}, serverRewrite = false) { - if (!serverRewrite) return undefined; +export function contextRequestTimeoutMs(cfg = {}, body = {}) { + const wantsRewrite = body.rewrite !== undefined; + // `query_expansion` defaults to "auto" server-side, so only an explicit "off" + // takes the expansion fuse back out of the budget. + const wantsExpansion = Boolean(body.session_id) && body.query_expansion !== "off"; + if (!wantsRewrite && !wantsExpansion) return undefined; + const configured = Number(cfg.recallContextTimeoutMs); if (Number.isFinite(configured) && configured > 0) return Math.max(1000, Math.floor(configured)); - return Math.max(Number(cfg.timeoutMs) || 0, SERVER_REWRITE_REQUEST_TIMEOUT_MS); + const floor = wantsRewrite ? SERVER_REWRITE_REQUEST_TIMEOUT_MS : EXPANSION_REQUEST_TIMEOUT_MS; + return Math.max(Number(cfg.timeoutMs) || 0, floor); } /** @@ -447,7 +457,7 @@ export async function fetchAssembledContext(fetchJSON, cfg, query, options = {}) const res = await fetchJSON("/api/v1/search/search", { method: "POST", body: JSON.stringify(body), - }, { actorPeerId, timeoutMs: contextRequestTimeoutMs(cfg, body.rewrite !== undefined) }); + }, { actorPeerId, timeoutMs: contextRequestTimeoutMs(cfg, body) }); if (!res.ok) { const status = res.status || 0; diff --git a/examples/zcode-memory-plugin/scripts/shared/recall-core.mjs b/examples/zcode-memory-plugin/scripts/shared/recall-core.mjs index d6e53692a9..a6e94eef5a 100644 --- a/examples/zcode-memory-plugin/scripts/shared/recall-core.mjs +++ b/examples/zcode-memory-plugin/scripts/shared/recall-core.mjs @@ -138,28 +138,38 @@ export function buildContextSearchBody(cfg = {}, options = {}) { return body; } -// The server pipeline is serial, and only its last stage is the rewrite fuse: -// query expansion (retrieval.recall_intent_timeout_s, 5s) runs first, then -// retrieval, body reads and budgeting, and only then the rewrite -// (retrieval.recall_rewrite_timeout_s, 30s). A deadline covering the rewrite -// alone aborts requests where every server stage stayed inside its own fuse, so -// this covers both fuses plus the work between them and still leaves a quarter -// of the 60s prompt-hook budget. +// The server pipeline is serial and each optional stage has its own fuse. A +// request is aborted client-side unless its deadline covers every stage it +// asked for, and aborting discards the whole response rather than just the +// stage that ran long. +// +// session_id -> query expansion (retrieval.recall_intent_timeout_s, 5s) +// always -> retrieval, body reads, budget planning +// rewrite -> digest (retrieval.recall_rewrite_timeout_s, 30s) +// +// Both budgets stay inside the 60s prompt-hook allowance, the rewrite one with +// a quarter to spare. +const EXPANSION_REQUEST_TIMEOUT_MS = 15000; const SERVER_REWRITE_REQUEST_TIMEOUT_MS = 45000; /** * HTTP deadline for one context request, or undefined to keep the caller's own. * - * The ordinary request timeout is shorter than the server's rewrite fuse, so a - * digest that finishes inside its own fuse would be aborted client-side. That - * loses the whole response rather than just the digest, including the - * uncompressed block the server still returns when a rewrite fails. + * Derived from the request body, because the body is what states which server + * stages will run: reading `cfg` alone cannot tell a bare retrieval from one + * that also spends the expansion or rewrite fuse. */ -export function contextRequestTimeoutMs(cfg = {}, serverRewrite = false) { - if (!serverRewrite) return undefined; +export function contextRequestTimeoutMs(cfg = {}, body = {}) { + const wantsRewrite = body.rewrite !== undefined; + // `query_expansion` defaults to "auto" server-side, so only an explicit "off" + // takes the expansion fuse back out of the budget. + const wantsExpansion = Boolean(body.session_id) && body.query_expansion !== "off"; + if (!wantsRewrite && !wantsExpansion) return undefined; + const configured = Number(cfg.recallContextTimeoutMs); if (Number.isFinite(configured) && configured > 0) return Math.max(1000, Math.floor(configured)); - return Math.max(Number(cfg.timeoutMs) || 0, SERVER_REWRITE_REQUEST_TIMEOUT_MS); + const floor = wantsRewrite ? SERVER_REWRITE_REQUEST_TIMEOUT_MS : EXPANSION_REQUEST_TIMEOUT_MS; + return Math.max(Number(cfg.timeoutMs) || 0, floor); } /** @@ -447,7 +457,7 @@ export async function fetchAssembledContext(fetchJSON, cfg, query, options = {}) const res = await fetchJSON("/api/v1/search/search", { method: "POST", body: JSON.stringify(body), - }, { actorPeerId, timeoutMs: contextRequestTimeoutMs(cfg, body.rewrite !== undefined) }); + }, { actorPeerId, timeoutMs: contextRequestTimeoutMs(cfg, body) }); if (!res.ok) { const status = res.status || 0;