Repository navigation
feat(context): report why a preparation lost its semantic recall path (#2039) - #2144
jasondeng1997 wants to merge 3 commits into
Conversation
6e1ce0d to
b50385e
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Existing strict host decoders reject the expanded response and discard prepared context, including healthy results.
1 open issue
What changed in this PR
Adds a preparation-level reason for lost semantic recall, extending the configurable embedding fallback work from #2042.
Changes:
- Carries embedding timeout and unavailability causes through recall into
PreparedContext. - Publishes an optional nullable reason in the HTTP contract.
- Adds regression coverage and bilingual documentation.
| File | Description |
|---|---|
| tests/test_api_contract.py | Checks optional, nullable reason contract. |
| tests/e2e/test_recall_embedding_timeout.py | Verifies degraded and healthy preparation reasons. |
| tests/builtin/runtime/test_prepared_context_degradation.py | Tests precedence, serialization, and validation. |
| src/powercontext/server/mapping.py | Maps reason into HTTP responses. |
| src/powercontext/http/_generated/schema.py | Updates generated schema. |
| src/powercontext/http/_generated/models.py | Adds generated reason enum and field. |
| src/powercontext/builtin/runtime/topic_memory_search.py | Reports Topic Memory fallback causes. |
| src/powercontext/builtin/runtime/prepared_context.py | Carries degradation in build results. |
| src/powercontext/builtin/runtime/models.py | Adds runtime preparation reason. |
| src/powercontext/builtin/runtime/atomic_memory.py | Propagates Atomic Memory fallback causes. |
| src/powercontext/builtin/runtime/application.py | Aggregates and publishes degradation reasons. |
| src/powercontext/builtin/artifacts/topic_memory/models.py | Adds internal fallback metadata. |
| openapi/powercontext.yaml | Defines the public reason contract. |
| docs/zh/docs/workflows/prepare-context-text.md | Documents reasons in Chinese. |
| docs/en/docs/workflows/prepare-context-text.md | Documents reasons in English. |
Review effort: Balanced
| "content_bytes": value.content_bytes, | ||
| # Field by field rather than a dump, so a new runtime field has to be named here on | ||
| # purpose; omitting it would publish a preparation that cannot say why it degraded. | ||
| "reason": value.reason, |
There was a problem hiding this comment.
Addressed in 1a3db93 — the field is no longer emitted unconditionally.
reason is now gated on PrepareContextRequest.explain (default false), and the two prepare routes register with response_model_exclude_unset=True, so a default response is still exactly {schema, status, content, content_bytes}. mapping.py builds the payload field by field rather than dumping the model, precisely so the key is absent rather than null — a null value is not equivalent to omission for an exact-key validator.
Both shapes are now covered on the wire: tests/test_server.py asserts the default HTTP response has exactly four keys, and tests/e2e/test_handoff_hints.py compares with exclude_unset=True because the hint route performs no retrieval.
No shipped decoder needs to change, because no official Host sets explain, and RFC 1435 requires automatic recall not to opt in. The remaining question from the human review is whether the top-level field should exist at all rather than living in the Receipt; that thread is the one to follow.
There was a problem hiding this comment.
复核 head 9ea45fe5:这条在代码层面已闭合,记录证据以便决定是否 resolve。
- 发送侧不再无条件带
reason:server/mapping.py::prepared_context_response只在value.reason is not None时写入
payload["reason"];_prepare也只在request.explain为真且确有通道丢失时model_copy该字段。 - 路由层两个入口(
PREPARE_CONTEXT、PREPARE_HANDOFF_HINT)改走response_model_exclude_unset=True,
所以未设置的字段不会以null出现,四字段封套保持原样。 - 本机实测(Windows / Python 3.14.3,
PYTHONPATH指向镜像 src):
tests/builtin/runtime/test_prepared_context_degradation.py+
tests/test_server.py::test_prepare_context_keeps_the_four_field_envelope_unless_the_request_explains共 15 passed;
其中 HTTP 级用例在无 embedding 的部署上断言set(body) == {"schema","status","content","content_bytes"},
explain: true也不多出第五个键。 - 生成物一致性:
_generated/models.py的reason带默认None且非 required,reason也不在
PreparedContext.required里(tests/test_api_contract.py新增断言),因此旧客户端解码不炸;
prepare_context不在_MCP_OPERATION_IDS内,不存在 MCP 侧多一个null的分叉。
唯一建议不在本线程:reason 的 description 把词表写成「RFC 1435 减 none」,RFC 该字段还有
reused_fts_fallback,已在文档行另开线程说明(仅措辞,不改实现)。
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
b50385e to
3c151ce
Compare
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
3c151ce to
0739b99
Compare
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
0739b99 to
a4577e0
Compare
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
Fengzdadi
left a comment
There was a problem hiding this comment.
Thanks for scoping this tightly and for catching the host-validator problem — the four-key envelope test on the wire is exactly the right guard.
My main concern is the contract shape: RFC 1435 (merged, tracked in #1356, not yet implemented) already specifies how a caller asks why a preparation came out the way it did, and this PR adds a second, parallel surface for a subset of the same question.
| this PR | RFC 1435 | |
|---|---|---|
| opt-in | PrepareContextRequest.explain |
PrepareContextRequest.include_receipt |
| where the cause lives | top-level PreparedContext.reason |
receipt.retrieval.searches[].fallback_reason |
| vocabulary | embedding_timeout / embedding_unavailable |
inference_timeout / inference_unavailable (plus none, reused_fts_fallback) |
| granularity | one value per preparation, timeout outranks unavailable | one group per (round, family, mode, outcome, fallback_reason) |
The RFC's vocabulary also matches the error_code values memory.search.embedding_fallback already logs (inference_timeout / inference_unavailable), so the API value and the log value currently have two spellings for one classification.
Once the Receipt is implemented, the public API would carry two opt-in flags and two enums for the same fallback, and removing explain / reason would then be a breaking change. I'd rather settle that now than after a release.
Options I can see:
- Fold this into the Receipt work. Keep the plumbing you added (carrying the fallback marker from
_resolve_query_embeddingand the Topic Memory searcher into the preparation) — it is whatSearchGroup.fallback_reasonneeds anyway — but expose it throughinclude_receipt/receipt.retrieval.searchesrather than a new field. If a full Receipt is too large for one PR, it is worth asking on #1356 whether a first slice is acceptable and which Receipt fields may be deferred, since the RFC's schema lists the other fields as required. - Keep a top-level field, but amend the RFC. If you think a single per-preparation cause is genuinely different from the Receipt's per-search groups, say so in an RFC 1435 amendment so the two are designed together, and at least reuse the
inference_*vocabulary.
Either way, could a maintainer weigh in on which direction is preferred before more review effort goes into the current shape?
A smaller point: no official Host sends explain: true (and per RFC 1435 automatic Host recall must not opt in), so this PR does not change what the Hook injects. The user-visible regression in #2039 — Hook recall dropping to lexical and the agent paying an extra pc_search round trip — is still governed by the recall_embedding_timeout_seconds default, which remains open there. That's not a reason to block this PR, just worth keeping in mind when deciding what to close #2039 against.
…oceanbase#2039) `prepare_context` bounded the query embedding, then said nothing when the bound expired: `status: "empty"` read the same whether nothing matched or the semantic path never ran, and oceanbase#2042's WARNING only reached the server log. A request can now ask with `"explain": true`, and the response then carries `reason`: `embedding_timeout` when `recall_embedding_timeout_seconds` expired, `embedding_unavailable` when the provider failed. - The field is opt-in, and absent even for an explaining request when nothing was lost. Every Host validates the injected envelope -- most by exact key set (zcode, opencode, dsh, pi, claude-code, codex, workbuddy, kimi, openclaw) -- so a fifth key on an ordinary preparation would break all of them for a fact none of them reads. RFC 1435 states the same rule for the injection value. - The marker is filled from the same classification that emits the fallback event, carried from `_resolve_query_embedding` and the Topic Memory searcher through the recall round into the preparation, so the log and the response cannot disagree about the cause. - A preparation that never wanted vectors stays silent: a keyword-only deployment is not degraded, only one that lost a channel it asked for. - When both channels fail, a timeout outranks an unavailable provider, because the budget is the one an operator can raise.
a4577e0 to
1a3db93
Compare
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
`PreparedContext.reason` said `embedding_timeout` / `embedding_unavailable` while the fallback events and RFC 1435's `SearchGroup.fallback_reason` already said `inference_timeout` / `inference_unavailable`. One classification ended up with two spellings on two surfaces of the same Server, which is exactly the divergence the Receipt work would have to reconcile later. Both now come from one function: the event `error_code` and the response `reason` are the same value, and the two translation tables that only existed to paper over the split are gone. The test that used to assert the two spellings separately now asserts they are equal. The failure belongs to the inference provider, not to the channel it was going to feed, so the response names the provider's problem too.
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
|
Thanks — the RFC collision is real, and the vocabulary split was the part of it that had no defence. That part is fixed in 49526f7. What changed now
The two dicts that existed only to translate between the surfaces are deleted, the e2e test that asserted the two spellings side by side now asserts they are equal, and the contract description states that the values are RFC 1435's On the shape I would take option 2 for this bug, and I would rather be told I am wrong than guess:
So if a maintainer picks option 1, my next push drops Agreed on the smaller point. This does not change what any Hook injects, and it does not close the user-visible half of #2039: the |
`Acceptance (oceanbase)` failed this revision on `test_atomic_memory_read_keeps_content_and_state_in_one_snapshot[oceanbase-get]` with `(1205, 'Lock wait timeout exceeded')`, and the OceanBase migration job then sat for 40 minutes. Both predate this branch and are unrelated to a vocabulary change that touches no storage path; the same jobs passed on 1a3db93. An author cannot re-run a fork PR's jobs, so this commit only retriggers CI.
| Add `"explain": true` to the request to also learn whether the preparation lost a retrieval channel it would | ||
| otherwise have used: the response then carries a `reason`, `inference_timeout` when the | ||
| `recall_embedding_timeout_seconds` budget expired and `inference_unavailable` when the provider failed. These are | ||
| the same spellings RFC 1435 gives `SearchGroup.fallback_reason` and the same ones the fallback event logs, so the |
There was a problem hiding this comment.
[P2] reason 的词表不是「RFC 1435 减 none」,还少了 reused_fts_fallback
:57(中文同段 docs/zh/docs/workflows/prepare-context-text.md:56,以及 openapi/powercontext.yaml 中 reason
的 description、integrations/dify/plugin/powercontext_dify/contract.json、tools/pc_prepare_context.yaml、
desktop/ui/src/generated/api.d.ts 的同一句)把 PreparedContextReason 描述成 RFC 1435
SearchGroup.fallback_reason 的词表去掉 none。但 RFC 1435 该字段是四个值:
none、inference_unavailable、inference_timeout、reused_fts_fallback
(docs/en/rfcs/1435_explainable_prepared_context_receipts.md:「If a later round reuses the FTS-only outcome of that
failure, it uses reused_fts_fallback」)。本 PR 的枚举是其中两个 ⇒ 准确说法是「四个里的两个错误类别」,
不是「减 none」。
这不是纯措辞:reused_fts_fallback 描述的正是本仓库确实会走到的路径——round 0 因推理错误丢掉向量后,
扩充轮 _recall_round 以 allow_embedding=False 复用 FTS 结果(_resolve_query_embedding 返回 (... , None)),
按 RFC 该记 reused_fts_fallback,而 reason 侧是 None(折叠到 round 0 的原因,本身可以接受,
_degradation 的注释也这么写)。按当前措辞,读者会认为该拼写在 reason 侧「不可能出现」而不是「被折叠」,
做 reason × fallback_reason 关联的评估代码也不会预留这个分支。补一句说明,或直接写「RFC 1435 四个拼写中的
两个错误类别」,两者都可,无需改实现。
另:本机复核了折叠优先级与两处归一化——tests/builtin/runtime/test_prepared_context_degradation.py 与
tests/test_server.py::test_prepare_context_keeps_the_four_field_envelope_unless_the_request_explains 共 15 passed;
prepare_context 不在 _MCP_OPERATION_IDS 内,因此不存在「HTTP 四字段、MCP 多一个 null」的分叉。
hidb4ai
left a comment
There was a problem hiding this comment.
The default SDK request now includes the new opt-in flag, which breaks preparation against an unupgraded Server. Details and reproduction are inline.
| description: Opt into current-Scope native code evidence. No index is built during preparation. | ||
| explain: | ||
| type: boolean | ||
| default: false |
There was a problem hiding this comment.
[P2] Omit explain from SDK requests when diagnostics are disabled
The Python client serializes all defaults for PREPARE_CONTEXT (src/powercontext/client/client.py:1376-1377), so adding explain=False makes even prepare_context(PrepareContextRequest(scope_id=..., query=...)) send "explain": false. When the client is upgraded before the Server, this breaks ordinary preparation: I ran the head SDK against a localhost Server at target 30e3f1b1 and got HTTP 422 (invalid_request); replaying the same payload with only explain removed returned HTTP 200.
Please omit the new flag when diagnostics are disabled, or negotiate support before sending it. Preserve the existing omitted-versus-null handling for assembly when adjusting serialization.
hireview001
left a comment
There was a problem hiding this comment.
- Resolve merge conflicts with the target branch.

What this changes
prepare_contextbounds the query embedding, and when that bound expires the preparation continues with lexical retrieval only.statuscannot say so:status: "empty"looks identical whether nothing matched or the semantic path never ran. #2042 added the runtime setting and a server-side WARNING; this PR closes the remaining item from that issue — the preparation can now say why it degraded.A request asks with
"explain": true, and the response then carriesreason:inference_timeoutrecall_embedding_timeout_secondsbudget expiredinference_unavailableThe values are RFC 1435's
SearchGroup.fallback_reasonvocabulary minusnone, which an absent field already carries. One classification has one spelling across the API, the fallback events, and the Receipt the RFC describes.Why opt-in rather than a fifth field on every response
Every Host validates the prepared-context envelope, and most compare the key set exactly (
zcode,opencode,dsh,pi,claude-code,codex,workbuddy,kimi-code,openclaw). An unconditionalreason— evenreason: null— makes all of them reject every preparation, which drops the injected context entirely. I found this the hard way: the first revision of this PR failedcontrolled-clionfresh_session_failed_to_recall, which is the ZCode validator refusing the fifth key. RFC 1435 states the same rule for the injection value:status,content,content_bytes, and explainability behind an opt-in.So the field is opt-in and omitted when it would say nothing:
PrepareContextRequest.explain(boolean, defaultfalse, not required) is the opt-in;PreparedContext.reasonis nullable, outside the spec'srequiredlist, and published only when a channel was actually lost — a request that does not explain gets a byte-identical four-field envelope;/v1/context/prepareand/v1/handoff/hint, which runs no retrieval) serialize unset fields only, so the default response keeps exactly the keys a Host validates.tests/test_server.pyasserts that on the wire,tests/e2e/test_handoff_hints.pyasserts the hint envelope and its MCP delivery, andtests/builtin/runtime/test_prepared_context_degradation.pyassertscontent: nullis still present on an empty preparation — dropping that key would break the same validators from the other side.The marker itself
reasonand theerror_codeofmemory.search.embedding_fallback/topic_memory.search.embedding_fallbackcome from one function, carried from_resolve_query_embeddingand the Topic Memory searcher through the recall round into the preparation. The event and the field cannot disagree about the cause, and an operator keeps the signal for a Host that never opts in.recall_embedding_timeout_secondsfixes the first and cannot fix the second, so reporting the other one would point an operator at a knob that does not help.Relationship to the Receipt
RFC 1435 publishes the same cause per search as
receipt.retrieval.searches[].fallback_reason. What this PR carries through recall is that value: the per-preparation fold only decides which single cause to publish when several searches lost the channel. Two open questions are being settled with maintainers rather than assumed here — whether a top-level field should exist before the Receipt, and whether the opt-in flag should beexplainor reused frominclude_receipt. Neither is frozen: no official Host sendsexplain, so either resolution is a non-breaking change before a release.Tests
test_prepared_context_degradation.py: the precedence fold, the default, serialization, the transport mapping in both directions (reason present when set, exactly four keys when not), and rejection of causes outside the enum;test_recall_embedding_timeout.py: an expired budget reportsinference_timeout; a failing provider reports the matching cause, and the test asserts the value is the same one the fallback event logs; a budget that fits reports nothing; a deployment without an embedding model reports nothing; and a preparation that did not ask reports nothing while still logging the fallback;tests/test_server.py: the HTTP response for a plain and for an explaining request on a healthy deployment both carry exactlyschema,status,content,content_bytes;tests/test_api_contract.py:reasonis published, nullable, not required;explaindefaults tofalseand is not required.ruff check,ruff format --check,ty check,scripts/generate_api.py --checkandnode desktop/scripts/generate.mjs --checkare clean. The spec and both generated surfaces (src/powercontext/http/_generated/, the Desktopapi.d.tsand contract digests) are regenerated in this PR.Out of scope
recall_embedding_timeout_secondsat 0.25 s, and the issue's table shows a 2 s budget makes the same requestready10/10. That is a product decision about the Hook budget, not a field; changing it in this diff would make the PR argue with itself. Still open on bug:prepare_contextabandons the query embedding after 0.25 s, so Hook recall silently degrades to lexical search with a remote embedding provider #2039.reasonanswers it for a caller that asks, and the statistics side needs a schema decision of its own.Closes part of #2039. Refs #2042, RFC 1435.