fix(pi): omit unsupported GPT off-reasoning effort - #223
Conversation
a6359d3 to
9e6b87d
Compare
9e6b87d to
f2fbb83
Compare
|
@rohita5l I refreshed this stacked PR: Claude capability/version policy is now shared by Pi and Claude Code, the Sonnet 4.5 Claude Code behavior is explicit in the description, and both threads have detailed replies and are resolved. Could you re-review and approve the fork CI run when convenient? |
f2fbb83 to
636c3b6
Compare
|
Post-review update: shared capability policy now reflects Sonnet 4.6 at 1M/64k and Fable 5 at native 1M/128k with adaptive thinking but no |
|
Pushed
else if (model.provider !== "github-copilot" && model.thinkingLevelMap?.off !== null) {
params.reasoning = { effort: (model.thinkingLevelMap?.off ?? "none") };
}
Measured against
Fix is Also bisected the rest of Pi's Responses payload against the gateway; all 200, no other compat flags needed: Added two regression tests in
Two things for reviewers:
Worth noting this was painful to diagnose because the gateway returns |
8d66253 to
15ad5c0
Compare
|
Rebased this PR stack onto current Stack dependencyThis is 2 of 3 and must land after #217:
The rebase preserves main's #282 contract: Pi pins
If #217 is squash-merged, please rebase this branch onto the updated Validation
Managed allowlist × live-discovery intersection is deliberately outside this PR and tracked in #290. |
Expose the GLM and Kimi coding-model cohort through Pi and OpenCode with shared token limits and reasoning metadata. Keep unsupported chat models out of discovery, including Inkling until gateway issue databricks#215 is fixed, and retain the GPT-OSS Responses API routing guard.
Centralize Claude family/version parsing so Pi metadata, adaptive-thinking compatibility, and Claude Code's [1m] selector cannot drift. Cover Sonnet 4.5, Opus 4.6, future major versions, Fable fallback, and prefixed model IDs.
`_pi_gpt_model_entry` declared `reasoning: True` without an off-state, so for
the thinking-off case Pi's Responses builder fell back to
`reasoning: {effort: "none"}` (pi-ai openai-responses.js, the
`thinkingLevelMap?.off !== null` branch). `"none"` is only valid on gpt-5.1+,
so every request to gpt-5, gpt-5-mini, gpt-5-nano and gpt-5-5-pro was rejected:
BAD_REQUEST: Unsupported value: 'none' is not supported with the 'gpt-5'
model. Supported values are: 'minimal', 'low', 'medium', and 'high'.
Setting `thinkingLevelMap: {"off": None}` makes Pi omit `reasoning` entirely,
which the gateway accepts for all 14 codex ids. Verified against
/ai-gateway/codex/v1/responses: effort="none" 400s on gpt-5/-mini/-nano/-5-5-pro
and 200s on gpt-5-1..-5-6; omitting `reasoning` is 200 everywhere.
`{"off": "minimal"}` was rejected as an alternative because gpt-5-5-pro 400s on
it too. Same pattern already used for the Gemini 3.x entries.
The rest of Pi's Responses payload was bisected against the gateway and is
fine: store:false, prompt_cache_key, prompt_cache_retention:"24h",
prompt_cache_options, include:["reasoning.encrypted_content"], developer role,
flat tool schemas, and the session_id / x-client-request-id affinity headers.
Regression was hard to spot because the gateway returns
{"error_code","message"} rather than OpenAI's {"error":...}, so Pi's
error-body.js recovery no-ops and every 400 renders as
"OpenAI API error (400): 400 status code (no body)". Reported upstream as
earendil-works/pi#7748.
Refs databricks#286
15ad5c0 to
94fe107
Compare
Issue and stack
Closes #286.
Depends on #217 and #333. Review only the concern-isolated two-file incremental range:
dgokeeffe/ucode@f299e50...94fe107
Do not merge before #333. This PR was previously described as the context-window PR; #333 now owns that concern, while this existing head is retained for the GPT thinking-off compatibility commit without rewriting history.
Unit 02 — Pi GPT reasoning compatibility
Objective and user-visible behavior
Prevent Pi's thinking-off mode from emitting unsupported
reasoning.effort: "none"for GPT gateway models. ExplicitthinkingLevelMap: {"off": null}makes Pi omit the reasoning field when thinking is off.Candidate:
review/pi-gpt-reasoning(94fe107), mandatory PR targetreview/shared-context-policy(f299e50). This is a concern-isolated two-file stacked unit and must not remain conflated with context-policy PR #223.Exact scope
Production:
src/ucode/agents/pi.py.Tests:
tests/test_agent_pi.py.Non-goals: token/context windows (unit 03), OpenCode, OSS models, MLflow proxying, cache retention.
Before / after reproduction
Focused validation:
Full suite at the stacked candidate:
uv run --frozen pytest -q # 2 failed, 1616 passed, 36 skippedFailures are the installed Claude and Pi User-Agent capture tests. This older stack does not contain unit 01; the integrated
devbaseline passes the Pi capture test, while independently reproducing the Claude capture failure.Live Databricks evidence
Retained prior redacted gateway evidence: GPT-5-family requests with explicit effort
nonereturnedBAD_REQUEST; omittingreasoningwas accepted on/ai-gateway/codex/v1. No bearer tokens or workspace identifiers are stored. The deterministic tests cover GPT-5, mini, nano, 5.5-pro, and 5.6 IDs plus a non-GPT control.Impact map
databricks-openai/ Responses only.models.json.reasoning; non-GPT Responses entries remain unchanged.Rollback and residual risk
Revert
94fe107; rollback risks restoring gateway 400s when users select thinking off. Residual risk: future Pi semantics for a null off mapping could change, so the generated model-entry contract should remain covered by tests.Hygiene
git diff --check review/shared-context-policy..review/pi-gpt-reasoningpasses. The PR must targetreview/shared-context-policy; its target-relative diff is the review contract. Two-file range; no generated files, credentials,uv.lock,.pi-subagents/, orgoal.md; no merge markers or unresolved index entries.