From a2b0e2347a5d4c6ea44feb93a6e4042198a49df7 Mon Sep 17 00:00:00 2001 From: Gareth-Rouse Date: Fri, 31 Jul 2026 11:52:23 +0100 Subject: [PATCH] fix(catalog): keep Synthetic's wire-off reasoning through the manager merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P2 on b88869dc6 (reproduced live): the mapper's `reasoning: false` for a `none`-only route only held for the raw fetcher. The production path normalizes through `createModelManager`, where `mergeDynamicModel` merged the dynamic row over the bundled reference with `existingModel.reasoning || dynamicModel.reasoning` — so a stale bundled `reasoning: true` (e.g. `hf:zai-org/GLM-5.2`) won again and `buildModel` fabricated a `[minimal,low,medium,high,xhigh]` ladder for a route that advertised only the `none` off-state. `mergeDynamicModel` now treats Synthetic's discovered `reasoning` as authoritative (both-sides-synthetic guard inside the per-id merge), mirroring the existing Copilot `dynamicInputAuthoritative` precedent in the same function. This loses nothing: the Synthetic mapper already folds the reference's reasoning vote into the dynamic row whenever the wire is silent on reasoning, so the reference still gets its say — at the mapper, where the wire vocabulary can be consulted, instead of via a blind OR at the merge. The new regression test drives the full production path (`createModelManager.refresh("online")`) rather than calling the fetcher and `buildModel` directly, closing the gap Codex called out. --- packages/catalog/CHANGELOG.md | 2 +- packages/catalog/src/model-manager.ts | 14 +++++++++- .../catalog/test/synthetic-provider.test.ts | 27 +++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/packages/catalog/CHANGELOG.md b/packages/catalog/CHANGELOG.md index 05aaaf4f9..e42a2972d 100644 --- a/packages/catalog/CHANGELOG.md +++ b/packages/catalog/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed Synthetic models losing their thinking selector, vision input, output cap and pricing: the discovery mapper read `supports_reasoning`, `supports_vision` and `max_tokens`, none of which Synthetic sends. It advertises `supported_features`, `reasoning_parameters.efforts`, `input_modalities`, `max_output_length` and `$`-prefixed `pricing`, so any route without a bundled reference (the `syn:*` router aliases, newly added routes such as `hf:moonshotai/Kimi-K3`) resolved to `reasoning: false` — hiding the effort dial and dropping `reasoning_effort` from every request — plus text-only input, zero cost, and an 8k output cap low enough to end turns on `length` and trigger recovery compaction. Effort ladders now come from the per-model wire vocabulary, with the router's `none` tier mapped onto `minimal`; a route whose wire vocabulary contains no known tier reports non-reasoning rather than fabricating an unadvertised ladder, the wire vocabulary overrides stale bundled reference ladders, and a populated `supported_features` list without `tools` no longer downgrades routes a bundled reference already marked tool-capable. An advertised effort vocabulary is authoritative over the bundled reference's reasoning flag, a route with at least one named tier reasons even if that tier stands alone (only a `none`-only vocabulary is the off-switch), and a present-but-empty `supported_features` list now marks the route tool-less rather than falling back to the reference default. +- Fixed Synthetic models losing their thinking selector, vision input, output cap and pricing: the discovery mapper read `supports_reasoning`, `supports_vision` and `max_tokens`, none of which Synthetic sends. It advertises `supported_features`, `reasoning_parameters.efforts`, `input_modalities`, `max_output_length` and `$`-prefixed `pricing`, so any route without a bundled reference (the `syn:*` router aliases, newly added routes such as `hf:moonshotai/Kimi-K3`) resolved to `reasoning: false` — hiding the effort dial and dropping `reasoning_effort` from every request — plus text-only input, zero cost, and an 8k output cap low enough to end turns on `length` and trigger recovery compaction. Effort ladders now come from the per-model wire vocabulary, with the router's `none` tier mapped onto `minimal`; a route whose wire vocabulary contains no known tier reports non-reasoning rather than fabricating an unadvertised ladder, the wire vocabulary overrides stale bundled reference ladders, and a populated `supported_features` list without `tools` no longer downgrades routes a bundled reference already marked tool-capable. An advertised effort vocabulary is authoritative over the bundled reference's reasoning flag, a route with at least one named tier reasons even if that tier stands alone (only a `none`-only vocabulary is the off-switch), and a present-but-empty `supported_features` list now marks the route tool-less rather than falling back to the reference default. The production model-manager merge (`mergeDynamicModel`) now treats Synthetic's discovered `reasoning` as authoritative instead of OR-ing the stale bundled flag back over a wire-advertised off-state — matching the existing Copilot `input` precedent — so a `none`-only route stays non-reasoning through `createModelManager`, not just in the raw fetcher. ## [17.2.1] - 2026-07-30 diff --git a/packages/catalog/src/model-manager.ts b/packages/catalog/src/model-manager.ts index ca7230aac..00a8a7c75 100644 --- a/packages/catalog/src/model-manager.ts +++ b/packages/catalog/src/model-manager.ts @@ -484,13 +484,25 @@ function mergeDynamicModel(existingModel: Model, dynamic const supportsImage = dynamicInputAuthoritative ? dynamicModel.input.includes("image") : existingModel.input.includes("image") || dynamicModel.input.includes("image"); + // Synthetic's discovery is authoritative (`dynamicModelsAuthoritative`) and + // its per-model `reasoning_parameters.efforts` vocabulary is the route's + // whole truth: when the wire advertises only the `none` off-state the + // mapper emits `reasoning: false`, and OR-ing the bundled reference's + // stale `reasoning: true` back would re-arm an effort dial the route + // doesn't expose. Other providers keep the OR so a bundled reasoning flag + // survives a discovery row that simply omits the capability. + const dynamicReasoningAuthoritative = + existingModel.provider === "synthetic" && dynamicModel.provider === "synthetic"; + const reasoning = dynamicReasoningAuthoritative + ? dynamicModel.reasoning + : existingModel.reasoning || dynamicModel.reasoning; // Re-build from spec stage: sparse compat comes from `compatConfig` (the // verbatim override vocabulary), never the resolved `compat` record. return buildModel({ ...existingModel, ...dynamicModel, name: preferDiscoveryName(dynamicModel.name, existingModel.name, dynamicModel.id), - reasoning: existingModel.reasoning || dynamicModel.reasoning, + reasoning, input: supportsImage ? ["text", "image"] : ["text"], cost: { input: preferDiscoveryCost(dynamicModel.cost.input, existingModel.cost.input), diff --git a/packages/catalog/test/synthetic-provider.test.ts b/packages/catalog/test/synthetic-provider.test.ts index 374dc85a2..209e7e7b3 100644 --- a/packages/catalog/test/synthetic-provider.test.ts +++ b/packages/catalog/test/synthetic-provider.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test"; import { buildModel } from "@oh-my-pi/pi-catalog/build"; import { Effort } from "@oh-my-pi/pi-catalog/effort"; +import { createModelManager } from "@oh-my-pi/pi-catalog/model-manager"; import { syntheticModelManagerOptions } from "@oh-my-pi/pi-catalog/provider-models/openai-compat"; import type { FetchImpl } from "@oh-my-pi/pi-catalog/types"; @@ -247,6 +248,32 @@ describe("Synthetic provider discovery", () => { expect(bare?.reasoning).toBe(false); }); + test("keeps the wire-off state authoritative through the production manager merge", async () => { + // The CLI resolves models through `createModelManager`, which merges the + // dynamic row over the bundled reference. `hf:zai-org/GLM-5.2` has a baked + // `reasoning: true` reference; without the wire-vocabulary override the + // merge would OR that flag back and `buildModel` would fabricate a ladder + // for a route that advertised only `none`. + const { fetch } = syntheticModelsFetch([ + { + id: "hf:zai-org/GLM-5.2", + object: "model", + name: "zai-org/GLM-5.2", + reasoning_parameters: { efforts: ["none"] }, + input_modalities: ["text"], + context_length: 202752, + max_output_length: 32768, + supported_features: ["tools"], + }, + ]); + const manager = createModelManager(syntheticModelManagerOptions({ apiKey: "syn-test-key", fetch })); + const { models } = await manager.refresh("online"); + + const glm = models.find(model => model.id === "hf:zai-org/GLM-5.2"); + expect(glm?.reasoning).toBe(false); + expect(glm?.thinking).toBeUndefined(); + }); + test("serves no dynamic models without an API key", () => { expect(syntheticModelManagerOptions().fetchDynamicModels).toBeUndefined(); });