fix(pi): extend forceAdaptiveThinking to Claude 5+ models#244
Closed
dgokeeffe wants to merge 10 commits into
Closed
fix(pi): extend forceAdaptiveThinking to Claude 5+ models#244dgokeeffe wants to merge 10 commits into
dgokeeffe wants to merge 10 commits into
Conversation
…h reasoning/limits Builds on the merged OpenCode OSS support (kimi/glm) to close the remaining gaps: - Pi had NO OSS provider. Add databricks-mlflow (api openai-completions -> /ai-gateway/mlflow/v1) so Pi can select OSS chat models, with a local SSE-repair proxy (_mlflow_proxy) for models like inkling whose gateway stream omits finish_reason (Pi's parser rejects that; OpenCode's @ai-sdk/openai tolerates it, so it needs no proxy). - Broaden _OSS_MODEL_FAMILIES from kimi/glm to the full chat-completions cohort (inkling, llama, qwen, gpt-oss, gemma), excluding non-chat services (embeddings/rerankers) that share a family substring (e.g. qwen3-embedding). - Correct + extend _MODEL_TOKEN_LIMITS from probed gateway caps: glm is 1M context / 65536 output (was 200k/25k); per-family output ceilings for the cohort. Add model_is_reasoning() from probed openai_reasoning capability; Pi sets reasoning:true so it renders streamed reasoning_content as thinking. Both agents read the shared model_token_limits/model_is_reasoning tables. Verified live: glm-5-2 answers through Pi (6*7 -> 42). (cherry picked from commit 23d71a5)
Pi databricks-mlflow OSS provider + broadened OSS cohort with reasoning/limits. (cherry picked from commit 3c9503e)
gpt-oss-* contains 'gpt-' so it matched the codex bucket, but it's a chat-completions-only OSS model (served via /ai-gateway/mlflow/v1), not an openai-responses model. Broadening _OSS_MODEL_FAMILIES to include gpt-oss put it in BOTH codex_models and oss_models, so it was offered under Pi's databricks-openai provider too — where the openai-responses route 400s for it. Exclude gpt-oss from the codex bucket so it's OSS-only. (cherry picked from commit acd2d06)
Proxy (_mlflow_proxy.py): - Close the connection at stream end (self.close_connection = True) so HTTP/1.1 clients detect the message boundary instead of hanging after [DONE] (the proxy sends no Content-Length/chunking). [critical] - Only repair text/event-stream responses; relay non-streaming and error bodies verbatim with their real Content-Type instead of forcing SSE. [high] - Catch URLError/OSError around urlopen and return a clean 502 instead of a dead socket on connection-level failures. [high] - Restrict forwarded paths to /ai-gateway/mlflow/ (404 otherwise) so the localhost relay can't be used as an authenticated arbitrary-path workspace client. [security] - Move terminator-repair writes inside the OSError guard so a client disconnect doesn't traceback on the daemon thread. [medium] - Parse SSE data lines with or without the optional space after the colon. [low] Discovery (databricks.py): - OSS chat models matching a broad family (qwen, llama-) but lacking a specific _MODEL_TOKEN_LIMITS entry now fall back to a conservative floor (128k/8192) so they're never offered uncapped (which 400s). Non-OSS and embedding ids still return None. Verified live: glm-5-2 streams content (42) + finish_reason + [DONE] through the proxy; non-stream/error/404/502 paths covered by tests. 898 passed. (cherry picked from commit ededa9c)
(cherry picked from commit a6359d3)
… reach OpenCode The OSS branch (fix/1m-context-claude-gpt-pi) rebuilt opencode around a databricks-oss bucket but dropped the databricks-openai (GPT/Codex) wiring that only lived on fix/opencode-codex-provider. Neither branch had both, so on the latest work GPT-5.x/5.6 was unreachable from OpenCode. Reconcile the two efforts across all three layers: - databricks.build_opencode_base_urls: add the 'openai' -> /ai-gateway/codex/v1 base URL - cli.configure_shared_state: discover codex models for opencode and populate opencode_models['openai'] - agents.opencode: emit the databricks-openai provider (per-model useResponsesApi=true) alongside oss Now OpenCode gets anthropic + google + openai + oss together. Ports databricks#97 wiring onto the OSS/1M-context work. Adds TestOpenAIProvider + base-url coverage. Co-authored-by: fix/opencode-codex-provider
OSS model discovery only queried UC model-services for `system.ai.*` ids and, unlike the claude/gemini/codex families, had no AI-Gateway fallback. On workspaces that expose OSS foundation models as regular `databricks-*` serving endpoints (rather than `system.ai.*` UC model-services), `oss_models` came back empty and pi/opencode never offered GLM, Qwen, Gemma, inkling, etc. Add `discover_oss_models`, which lists foundation-model endpoints advertising `mlflow/v1/chat/completions` and keeps the OSS chat families via the existing `_is_oss_chat_model` filter (the family filter is required because Claude/Gemini endpoints can advertise the same dialect on some workspaces). Wire it as the fallback in cli.py, mirroring the other three families. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…directing HOME Pi honors the PI_CODING_AGENT_DIR env var to resolve its config directory (~/.pi/agent), so redirecting /Users/david.okeeffe to APP_DIR/pi-home was unnecessary. The HOME redirect broke macOS keychain default resolution under ucode: the Security framework looks for the login keychain under the redirected HOME, finds none, and security default-keychain returns 'A default keychain could not be found'. As a result gh auth, the git credential helper, and any keychain-backed tool failed inside pi. Setting PI_CODING_AGENT_DIR to the existing PI_CONFIG_DIR preserves config isolation (models.json/settings.json/sessions still land under APP_DIR/pi-home/.pi/agent) while leaving /Users/david.okeeffe as the user's real home, so the login keychain stays discoverable. Tests: the two pi e2e sites monkeypatched PI_UCODE_HOME/PI_CONFIG_PATH to redirect pi at a tmp home. They now also patch PI_CONFIG_DIR (read by build_runtime_env) and PI_SETTINGS_PATH/PI_SETTINGS_BACKUP_PATH (previously masked because the HOME redirect made pi read settings from the un-patched real APP_DIR path).
Claude Opus 5 (and Sonnet 5) use the adaptive thinking API (thinking.type.adaptive) instead of the legacy enabled/disabled toggle (thinking.type.enabled). The old regex only matched 4.6+, so opus-5 models were missing forceAdaptiveThinking and the gateway 400ed with: "thinking.type.enabled" is not supported for this model. Extend the regex to match any major version >= 5 (with any minor), keeping the existing 4.6+ behavior unchanged. Tests: - test_claude_opus_5_gets_force_adaptive_thinking - test_claude_sonnet_4_5_does_not_get_force_adaptive_thinking
Author
|
Closing in favor of #245 — the fix has been folded into the consolidated branch () which now includes the Claude 5+ adaptive thinking fix alongside the other Pi/OpenCode model routing changes. |
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
Claude Opus 5 (e.g.
databricks-claude-opus-5-0) rejects the legacythinking.type.enabledfield and requiresthinking.type.adaptive:Pi's
forceAdaptiveThinkingcompat flag tells Pi to sendthinking.type.adaptiveinstead. The regex that gates this flag only matched Claude 4.6+, so Opus 5 was missing it and 400ed at the gateway.Fix
Extend the regex in
_pi_claude_model_entryto also match any major version ≥ 5 (with any minor version), while keeping the existing 4.6+ behavior unchanged:This correctly:
claude-opus-5-0,claude-sonnet-5-0, and future majors ≥ 5claude-opus-4-6throughclaude-opus-4-99(4.6+)claude-sonnet-4-5(predates the adaptive thinking API)Tests
test_claude_opus_5_gets_force_adaptive_thinking— verifies opus-5 and sonnet-5 both getforceAdaptiveThinking: Truewith correct 1M/128k token limitstest_claude_sonnet_4_5_does_not_get_force_adaptive_thinking— regression guard that sonnet 4.5 does not get the flagAll 53 tests in
test_agent_pi.pypass. ruff format clean.Validation