fix(posit): switch API flavor when chat.model crosses model families - #430
Open
cpsievert wants to merge 7 commits into
Open
fix(posit): switch API flavor when chat.model crosses model families#430cpsievert wants to merge 7 commits into
cpsievert wants to merge 7 commits into
Conversation
ChatPosit() picks its provider class from the model name (Claude models get the Anthropic flavor, everything else the OpenAI-compatible one), but the Chat.model setter only updated the model name, so switching to a model from the other family sent requests in the wrong wire format to the wrong gateway endpoint. Adds a Provider.set_model() hook that the Chat.model setter consults; the default returns the provider unchanged. The two Posit providers override it to swap in their sibling class on a family switch, carrying over credentials, the gateway base URL, and the cache setting (stored inertly on PositOpenAIProvider so it survives a round trip). Mirrors tidyverse/ellmer#1139. Also drops unsigned thinking blocks when replaying history to Anthropic-backed providers: reasoning emitted by a non-Claude model has no signature, and the API rejects it with "Invalid signature in thinking block". Family switching makes this reachable; Claude's own (signed) thinking is unaffected. Verified end-to-end against the Posit gateway with family switches in both directions, including a conversation containing GLM thinking content replayed to Claude.
CI's pyright resolves pillow_heif's stubs and flags register_heif_opener as a private re-export; this failure is present on main as well.
Addresses Copilot review feedback on #430: - Chat.model setter now closes the previous provider's HTTP clients when set_model() swaps in a new provider instance (async resources are closed via close_async() when an event loop is running). - Posit set_model() now carries over a custom provider name across Anthropic/OpenAI flavor switches.
Async clients can't be closed from sync code (their transport requires a running event loop), so deterministically closing them from the Chat.model setter would require fire-and-forget task scheduling with its own edge cases. The leak scenario (repeated provider swaps after async usage in a long-lived process) is narrow enough that relying on GC for async resources is an acceptable, documented tradeoff.
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.
Mirrors tidyverse/ellmer#1139.
Summary
ChatPosit()picks its provider class from the model name: Claude models getPositAnthropicProvider(pointing at{base_url}/anthropic), everything else getsPositOpenAIProvider(pointing at{base_url}/openai/v1). But theChat.modelsetter only updated the model name, so switching to a model from the other family sent requests in the wrong wire format to the wrong gateway endpoint.This adds a
Provider.set_model()hook that theChat.modelsetter consults (self.provider = self.provider.set_model(value)). The default implementation returns the provider unchanged, so no other provider is affected.PositAnthropicProviderandPositOpenAIProvideroverride it to swap in their sibling class when the new model belongs to the other family, carrying over credentials and the gateway base URL. The Claude-onlycachesetting survives family switches via an inertcacheattribute onPositOpenAIProvider, andChatPosit()now passes itscacheargument to both flavors so switching to Claude honors it.A second, related fix: Anthropic-backed providers now drop thinking blocks that lack a signature when replaying conversation history. Reasoning emitted by a non-Claude model (e.g., GLM via the OpenAI flavor) has no signature, and the Anthropic API rejects it with
Invalid signature in thinking block— family switching makes this reachable mid-conversation. Claude's own (signed) thinking blocks are unaffected, so interleaved-thinking replay still works. Note: ellmer appears to have the same latent issue.Verification
All three steps verified live against the Posit gateway. New tests in
tests/test_provider_posit.pycover family swaps in both directions, no-op within a family, credential carryover, and cache preservation;tests/test_provider_anthropic.pycovers dropping unsigned thinking blocks. Full suite: 1278 passed.