pi: enable thinking levels for system.ai models#194
Open
elyssazyne wants to merge 1 commit into
Open
Conversation
ucode wrote pi's models.json with bare {"id": ...} entries, so every
model defaulted to reasoning: false and pi's TUI reported "Current model
does not support thinking" for every family.
Advertise per-model thinking support instead:
- MODEL_THINKING_LEVELS maps each model to the effort levels Databricks
accepts; _pi_model_entry emits reasoning + a thinkingLevelMap (openai
disables via "none", xhigh must be explicit, unsupported levels hidden).
- Claude uses adaptive thinking (compat.forceAdaptiveThinking): the
system.ai Anthropic route requires thinking.type:"adaptive" +
output_config.effort and rejects the legacy budget format.
- Gate thinking on the system.ai. prefix. The legacy AI-Gateway route
(databricks-*, deprecating) 400s on the new thinking params across all
families, so those models ship without thinking and still respond.
- Unknown/new models ship without thinking until added to the table.
Cover both routes in test_launch_pi_per_model. The e2e fixture only
discovered via the AI-Gateway route, so the system.ai path (the one that
actually sends thinking params) was never exercised. Add an e2e_uc_models
fixture (discover_model_services) and launch each model on both routes,
tagged by route.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
What
ucode piwrites each model intomodels.jsonas a bare{"id": ...}, sopidefaults them toreasoning: falseand reports "Current model does not support thinking" for every family — no way to enable thinking or choose an effort level. Naively enablingreasoningisn't enough either: supported effort levels differ per model, so a uniform level set gets rejected.Enabling thinking also differs by hosting method. Unity Catalog (UC) models (
system.ai.*) use the current route; the legacy AI Gateway route (databricks-*) uses an older, incompatible thinking format and is being deprecated. This PR enables thinking for UC models only — legacy-route models are left as-is (they still respond, without thinking controls).The e2e tests only discover models through the legacy AI Gateway route, so the UC path — the one that actually sends thinking params — was never exercised. This PR extends
test_launch_pi_per_modelto cover UC models too. The same gap exists for the other agents' launch tests; I kept this PR scoped topi, but I'm happy to extend the others if desired.Change
MODEL_THINKING_LEVELS, mapping every modelucode picurrently exposes to the effort levels supported by that model.system.ai.prefix; AI Gateway and unknown/new models getreasoning: false.compat.forceAdaptiveThinkingpilimitation: itsgoogle-generative-ailayer detects Gemini 3 vs 2.5 with regexes expecting dotted minor versions (gemini-3.1-pro), but Databricks serves dashed ids (gemini-3-1-pro), so only the unversionedgemini-3-pro/-flashare recognized; the rest fall through to a dynamic budget that ignores the requested effort. They still think at their default.e2e_uc_modelsfixture (tests/conftest.py) that discovers UC models viadiscover_model_services, and updatedtest_launch_pi_per_model/_all_modelsto launch every model on both the AI Gateway (existing) and UC (new) routes, tagged by route.Test plan
uv run pytest— passed.uv run ruff check .+ruff format --check— passed.UCODE_TEST_WORKSPACE=<your-workspace-url> uv run pytest tests/test_e2e.py -v— passed