Skip to content

feat(models): enable reasoning for deepseek-v4.1-flash - #91

Closed
newCman1 wants to merge 1 commit into
patlux:mainfrom
newCman1:feat/catalog-reasoning-overrides
Closed

newCman1 wants to merge 1 commit into
patlux:mainfrom
newCman1:feat/catalog-reasoning-overrides

Conversation

@newCman1

Copy link
Copy Markdown

Fixes #90.

Problem

src/commandcode-catalog.ts is generated from the bundled Command Code CLI, so a model published upstream after the last sync has no entry in MODEL_REASONING or MODEL_EFFORTS until npm run sync:commandcode-catalog runs again. deepseek/deepseek-v4.1-flash is in that state, and three things follow from the missing flag:

  • src/core.ts returns early from mappedReasoningEffort on !model.reasoning, so reasoning_effort is never put on the wire
  • index.ts derives compat.supportsReasoningEffort: MODEL_EFFORTS[model.id] !== undefined from the effort list
  • src/models.ts builds no thinking metadata at all, so Pi offers no thinking levels

commandcode-catalog-overrides.ts only exposes an efforts hook, so it cannot fix this on its own: efforts without the flag still hit the core.ts guard.

Evidence

Command Code CLI 1.53.0 (npm pack command-code@1.53.0, dist/cli.mjs):

DEEPSEEK_V4_1_FLASH:{id:"deepseek/deepseek-v4.1-flash",inputModalities:["text","image"],
  description:"V4.1 hybrid-attention reasoning with vision",
  reasoning:!0,reasoningEfforts:["low","high","max"],contextWindow:1e6}

The pinned catalog (generated from command-code@1.44.0) contains neither the reasoning flag nor an effort list for that id, which is why pi --list-models reports thinking: no for a model the endpoint accepts reasoning_effort for.

Change

  • add MODEL_REASONING_OVERRIDES next to the existing effort overrides, documented the same way
  • merge it over the generated MODEL_REASONING in src/models.ts, mirroring how MODEL_EFFORT_OVERRIDES is merged over MODEL_EFFORTS
  • carry the flag and the upstream efforts (low, high, max) for deepseek/deepseek-v4.1-flash

Verification

npm run test:unit (15 suites), npm run typecheck and npm run format:check all pass.

  • tests/test-models.ts adds a case for the merge itself. It fails while the override is missing and also fails once the generated catalog ships the flag, so the entry gets dropped rather than silently duplicated.
  • tests/test-stream.ts adds a request-shape case for a catalog-gap model, asserting the field on the emitted body. Reverting the source change fails exactly that case and nothing else.
  • Wire check against the live endpoint through a local echo proxy: --thinking low|high|max sends reasoning_effort: "low"|"high"|"max", --thinking off sends no field, and the responses come back with matching reasoning tokens.

Existing tests already cover both hosts, so the new model is included in the pi/OMP metadata assertions (thinkingLevelMap tristate, effort map, canonical metadata) without extra cases.

Not included

Both are the same root cause — the pinned catalog predates the model — but they live in different files and are deliberately left out to keep this PR to one problem:

@newCman1 newCman1 changed the title feat(models): enable reasoning for catalog-gap models feat(models): enable reasoning for deepseek-v4.1-flash Sep 10, 2026
@newCman1

Copy link
Copy Markdown
Author

The typecheck job is red on this PR, but it is not caused by this change — it looks like a pre-existing failure on main:

  • npm run typecheck itself passes (step 5, green). The red step is 8, npm test.
  • The failure is tests/test-pi-local.mjs:944 (assert.equal(claudePrint.code, 0, ...)1 !== 0), for the "Claude request through Anthropic Messages endpoint" case.
  • I checked out upstream main (adea658) in a scratch clone without this PR's commits and ran the same file against the same pi: identical failure at the same line.
  • Cause: the mock server matches on the exact URL, and pi 0.85 appends a query string to the Anthropic endpoint.
    • pi 0.84.4: "/v1/messages"
    • pi 0.85.0: "/v1/messages?beta=true"
    • The mock hits its fallback res.writeHead(404) and the CLI exits non-zero.
    • Reproduced locally with pi 0.85.1 (pi --version0.85.1), the version CI installs via npm install -g @earendil-works/pi-coding-agent@latest.
  • So CI has presumably been red on main since 0.85.0 shipped (2026-09-04); the other jobs on this PR are green.

Making the mock match on the path instead of the full URL fixes it — one const requestPath = (req.url ?? "").split("?")[0] plus the three comparisons that follow (/provider/v1/models, /provider/v1/chat/completions, /provider/v1/messages). With that, node tests/test-pi-local.mjs prints [pi-local] PASS locally on pi 0.85.1. tests/test-omp-compat.mjs has the same exact-match pattern for two of those routes, but its requests did not fail here, so I left it alone.

Happy to add that as a second commit on this branch, or to open it as a separate PR if you would rather keep this one to the reasoning change — whichever you prefer.

I did not touch src/pricing.ts in this PR on purpose; the pricing/inputModalities gaps for the same model are noted under "Not included" above and can be follow-ups.

@patlux

patlux commented Sep 15, 2026

Copy link
Copy Markdown
Owner

Thanks for the diagnosis and regression coverage. Superseded by merged #98, which brings the upstream reasoning flag and low/high/max efforts for DeepSeek V4.1 Flash into the generated catalog, together with the self-healing sync in #95. No additional reasoning-override layer is needed for data already published upstream. Closed only after the replacement passed CI and merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Catalog pinned to CLI 1.44.0: models added upstream (deepseek-v4.1-flash, etc.) can never have reasoning enabled

2 participants