Fix #2308: LLM facade drops opts.op before providers — per-op provider switches (e.g. think - #2309
Conversation
`LlmClient.buildCallInput()` dropped `opts.op` when constructing the
`ProviderCallInput` object handed to `provider.complete()` / `stream()`.
Callers set `op` for every logical call site (e.g. `capture.summarize`,
`retrieval.filter`, `skill.evolve`) but only the audit / metrics paths
inside the facade saw the tag — providers never did.
As a result any per-op provider behavior (request-body tweaks, routing
overrides, reasoning kill-switches keyed on
`opts.op === "capture.summarize"`) silently could never fire. The
issue-reported symptom: OpenRouter DeepSeek reasoning models burn full
token budget and double latency on `capture.summarize` because the
recommended `thinking: { type: "disabled" }` switch is unreachable.
Fix:
- extend `ProviderCallInput` with optional `op?: string`
- copy `opts?.op` in `buildCallInput()`
Field stays optional so providers must not assume it is set. No public
LlmClient surface change; consumers keep calling `complete` /
`completeJson` / `stream` unchanged.
Added 4 unit tests in `tests/unit/llm/client.test.ts` under an
"op propagation (issue MemTensor#2308)" describe block covering complete /
completeJson / stream forwarding and the "no op supplied" case.
Memtensor-AI
added
ai:generated
🤖 Open Code ReviewTarget: PR #2309 ✅ OpenCodeReview: Review complete: 0 finding(s) across 2 selected item(s). Generated by cloud-assistant via Open Code Review. |
✅ Automated Test Results: PASSEDAll tests passed (29/29 executed). memos_local_plugin/unit: 29/29. Duration: 3s [advisory, non-gating] AI-generated tests on branch test/auto-gen-aca74652f3c40919-20260830053736: 17/17 passed — these do NOT affect the PR verdict; review the branch manually. Branch: |
Memtensor-AI
added
status:ready
|
Merged into Verification on the updated branch:
The provider-specific per-operation reasoning policy remains intentionally separate from this transport/plumbing fix. |
3 participants
Description
Fix #2308: the LLM facade in
apps/memos-local-plugin/core/llm/client.tsno longer dropsopts.opwhen it handsProviderCallInputtoprovider.complete()/provider.stream(). Two-line change: extendProviderCallInputincore/llm/types.tswith an optionalop?: string, and copyopts?.opinsidebuildCallInput().opstays optional so providers must not assume it is set; no publicLlmClientsurface change and no new error codes.This unblocks the OpenRouter/DeepSeek reasoning kill-switch reported in the issue (
opts.op === "capture.summarize"inside a provider's request builder can now actually evaluate true). Landing the per-op switch itself — e.g.body.thinking = { type: "disabled" }inproviders/openai.ts— is a routing-policy decision left as a follow-up so operators can review it independently.Tests: added 4 new unit tests under a new "op propagation (issue #2308)" describe block in
tests/unit/llm/client.test.tscoveringcomplete/completeJson/streamforwarding plus the "no op supplied" case. Verification: all 84 LLM unit tests green (5 files: client / providers / json-mode / prompts / fetcher),tsc -p tsconfig.json --noEmitclean. The 46 pre-existing failing test files in the broader unit suite (memory/l3/subscriber etc.) were verified unchanged against the base branch viagit stashand are unrelated to this fix.Confidence: 0.85 — root cause and fix were both explicit in the issue with local verification by the reporter.
Related Issue (Required): Fixes #2308
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Added four focused unit tests covering
complete,completeJson, streaming fallback, and calls without an explicitop. Existing PR checks passed before retargeting and will be rerun against the updated base.Checklist
@whipser030, @hijzy please review this PR.
Reviewer Checklist