Repository navigation
fix: disable deepseek thinking via extra_body in codex workflows - #19
Merged
Merged
Conversation
Roll forward the two manual-codex-judge*.yml files on main to match what feat/codex-eval-backend currently carries. Workflow_dispatch parses the YAML from main, so the iterative fixes on the feat branch weren't visible to the Actions tab until this lands. Cumulative changes since the initial registration in #13: - Install LiteLLM proxy via `uv tool install` (was pip+setup-python). - Drop a `cp .../litellm_proxy_hooks.py /tmp/litellm/` step so LiteLLM can resolve the hook module (it looks relative to the config file's directory, not PYTHONPATH). - Register the hook via `litellm_settings.callbacks` so non-function tools (codex's namespace / MCP groupings) are stripped before forwarding to DeepSeek. - Set `reasoning_effort: none` on both deepseek-v4-* models so thinking mode is disabled and codex doesn't have to round-trip `reasoning_content` (which it doesn't know about). - Move `setup-uv` above the litellm step so `uv tool` is available for the proxy install; remove the obsolete `setup-python` step. Workflows remain `workflow_dispatch:` only and depend on skills-evals code (driver, hook, tests, conftest changes) that lives only on feat/codex-eval-backend. Dispatch them with ref=feat/codex-eval-backend until #11 merges. Signed-off-by: Tanzim Saqib <[email protected]>
Same fix as on the feat branch — reasoning_effort=none was ignored
(DeepSeek's schema only accepts "high" or "max"). The actual
disable is `thinking: { type: "disabled" }` passed via extra_body
so LiteLLM merges it into the upstream request body.
Signed-off-by: Tanzim Saqib <[email protected]>
#18 landed reasoning_effort=none on main. This branch's commit replaces it with the correct thinking.type=disabled. Take ours wholesale — main's value is the broken one. Signed-off-by: Tanzim Saqib <[email protected]> # Conflicts: # .github/workflows/manual-codex-judge-user-scope.yml # .github/workflows/manual-codex-judge.yml
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.
Summary
Follow-up to #18 — that PR shipped
reasoning_effort: noneas the supposed disable for DeepSeek v4 thinking mode, but the DeepSeek schema only accepts"high"or"max"forreasoning_effort(per https://api-docs.deepseek.com/api/create-chat-completion), so the field was silently ignored and the workflows kept 400ing on:The actual documented disable is a separate
thinkingobject:Passed via
extra_bodyin the LiteLLMmodel_listso it merges into the upstream HTTP body LiteLLM forwards to DeepSeek. Applied to bothdeepseek-v4-proanddeepseek-v4-flashin both workflow files.Why this is safe to merge immediately
workflow_dispatch:only — merging cannot trigger anything.feat/codex-eval-backend(test: add openai codex cli eval backend #11). Dispatch withUse workflow from: feat/codex-eval-backendas before.After merge
Re-dispatch
manual / codex cli <- llm judgefrom the Actions tab with the feat branch ref. Smoke should still pass; scenarios should now make it past turn 1 (which was the wall the wrong fix left them at).