Repository navigation
fix(catalog): expose real thinking efforts to omp's effort picker - #42
Merged
Merged
Conversation
omp populates its effort picker from `model.thinking` (not the `thinkingLevelMap` that earendil pi-ai exposes). Without it, omp falls back to a generic `minimal`..`high` ladder, so `DeepSeek-V4-Pro` offers `low`/`medium` it does not supported and hides `max`. Map `thinkint_config` into omp's `ThinkingConfig` shape (mode "effort", ordered efforts, `defaultLevel`, `requiresEffort = !disabled`) and attach it to each registered model. earendil pi keeps reading `thinkingLevelMap` and ignores the new field.
Owner
|
Merged — thank you! Squash-merged as Before merging I installed OMP 18.12.6 and verified this end-to-end against the live catalog, A/B against v0.6.3 (which has no
One tiny note: on this OMP version the fallback for DeepSeek-V4-Pro included |
jischeng
added a commit
to jischeng/pi-provider-qoder
that referenced
this pull request
Oct 23, 2026
Sync upstream v0.6.2..v0.6.5 while keeping this fork's features, then fix the bugs that surfaced during the review. The upstream changes and the fixes touch the same functions, so they land together rather than as artificially split commits that would not pass on their own. Upstream taken: tool-call aggregation by ID instead of stream index (simonsmh#39), combined thinking-disable signals (simonsmh#40) with the effort-less catalog exception, bounded model-busy recovery (simonsmh#41), omp effort metadata (simonsmh#42), service-enabled Auto retention (simonsmh#43), COSY version tracking, and wire-format tool-call IDs. Fork behaviour preserved: multi-account providers, per-account entitlement pre-flight, price factors, keep-alive SSE, premature-disconnect surfacing, XML <tool_call> recovery, and always-on-thinking effort fallback. Fixes: 1. Cross-region credential leak. The global provider prefix ("qoder") is a strict prefix of the CN one ("qoder-cn"), so `id.startsWith(prefix + "-")` matched CN slots when listing global accounts. A global catalogue refresh then called /model/list with CN credentials against the GLOBAL endpoint and wrote that account into the global cache. Added isProviderOfMode() in region.ts and routed both account discovery and re-registration through it. 2. System-message folding was computed and discarded. The request body still prepended systemText to the unfiltered list, so it carried the top-level prompt plus every system turn instead of one merged message. 3. Orphan `tool` messages. A result whose id no preceding assistant turn declared was passed through (the "no assistant to pair with" case returned null, which read as permission rather than a veto). Upstreams reject that with "tool must follow a message with tool_calls", failing every later request in the session. Affected: resumed mid-turn, a user turn between call and result, and calls the lookahead trimmed. 4. Startup could hang forever. The default export awaits automatic login, and the PAT exchange / token refresh had no deadline, so an upstream that accepted the connection and never answered blocked extension load with no models, no UI, and no error. Bounded via authFetchSignal() (15s, merged with the caller's signal); the usage fetch defaults to a 10s timeout too. 5. Multi-pane torn reads of the COSY version cache. Plain writeFileSync let a second process parse a half-written body, throwing and leaving that pane on the bundled fallback version for the rest of the day. Now write-then-rename, matching the model catalogue. 6. Non-object tool arguments. `<tool_call>` XML whose arguments were an array, scalar, null, or a non-JSON string produced a value violating `ToolCall.arguments: JsonObject`, so argument validation failed instead of the tool running with no arguments. 7. Content order shuffled around tool calls. When a stream produced text, a tool call, then more text, the trailing text was appended to the block BEFORE the call: content read ["BEFOREAFTER", toolCall] and events addressed indexes that no longer matched. Root cause was a cached text-block index in ThinkingTagParser.emitText (and the equivalent branch in stream.ts) that was never re-checked once tool calls appended to `content`. Present upstream in v0.6.5 as well; its test asserted the shuffled order, so that assertion is corrected and a contentIndex-consistency check added. Also fixes provider re-registration across session replacement (issue #1): the registered-provider set was module-level, and ESM modules stay cached, so /resume, /new and /fork built a ModelRuntime with zero Qoder providers and the picker fell back to the default model. Now a per-instance WeakMap cleared on session_shutdown. Deviating points from upstream are marked `// fork:` in the source so the next sync can enumerate them mechanically.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
omp's effort picker reads
model.thinking(its ownThinkingConfig), not thethinkingLevelMapthis plugin provides. Whenthinkingis absent, omp falls back to a genericminimal..highladder, so a model likeDeepSeek-V4-Pro(which only supportshigh/max) showslow/mediumit cannot honor and hidesmax.Fix
Add
buildThinkingConfig(entry)incatalog.ts, mapping Qoder'sthinking_configinto omp'sThinkingConfig:mode: "effort"- Qoder encodes effort as areasoning_effortstring.efforts- thethinking_config.enabled.effortskeys, in canonical order (minimal..max).defaultLevel- the effort markedis_default.requiresEffort: !tc.disabled- mirrors the catalog'sdisabledbranch.Attach it as
model.thinkingon each registered model. earendil pi ignores the field (it readsthinkingLevelMap), so this only affects omp's picker.Tests
Four
buildThinkingConfigunit cases: effort-based with/without adisabledbranch, effort ordering, andundefinedfor toggle-only or non-reasoning entries.Notes
QoderThinkingConfigis defined inline (not imported from omp) because the plugin compiles against@earendil-works/pi-ai, which has no such type. Same pattern as the existingcompat as Record<string, unknown>OMP shim inindex.ts.requiresEffortonly mirrors the catalog'sdisabledbranch; it is not currently consumed on omp's qoder path (omp reads it for anthropic/bedrock/budget models), so it has no request-side effect for Qoder today.efforts) still returnundefined, keeping omp's generic behavior for those.off/auto/inheritregardless of metadata; this change only corrects the concrete effort list, not the always-on selectors.