mirror of
https://github.com/we-promise/sure.git
synced 2026-05-29 23:39:03 +00:00
fix(ai): address PR review on Anthropic provider foundation
Surface fixes raised by Codex + CodeRabbit on PR 1/5:
- Provider::Anthropic#chat_response now accepts (and ignores) a
`messages:` kwarg. Assistant::Responder passes both `messages:`
(OpenAI-shape) and `conversation_history:` (raw Message records) for
cross-provider parity, so the previous signature raised
ArgumentError on the first chat turn through the Anthropic provider.
- Provider::Anthropic#supports_model? bypasses the `claude` prefix
gate when a custom base_url is configured, mirroring the OpenAI
provider. Bedrock-shaped IDs like
`anthropic.claude-sonnet-4-5-20250929-v1:0` and
`claude-opus-4@20250514` are otherwise rejected by
Assistant::Provided#get_model_provider and the chat dies.
- Setting.anthropic_access_token is now in
EncryptedSettingFields::ENCRYPTED_FIELDS so the Anthropic API key
is encrypted at rest like every other provider secret. Previously
plaintext while siblings (openai_access_token, twelve_data_api_key,
external_assistant_token) were ciphertext.
- Chat.default_model falls back to whichever provider is actually
configured. Previously, with LLM_PROVIDER=anthropic but no
Anthropic credentials, the default model resolved to a Claude ID
that no registered provider supported, so chats failed even when
OpenAI was fully configured. Adds Provider::{Anthropic,Openai}#configured?
class methods for the readable callsite.
- Provider::Anthropic.effective_model uses
`ENV["ANTHROPIC_MODEL"].presence || Setting.anthropic_model` so the
Setting lookup is only performed when the env var is absent — the
previous `ENV.fetch(KEY, default)` evaluated the default arg
eagerly on every call.
- Provider::Anthropic::ChatConfig#anthropic_input_schema strips both
`:strict` and `"strict"` keys so JSON-decoded schemas with string
keys cannot leak the OpenAI-only flag through to Anthropic.
Test coverage added: supports_model? bypass on custom endpoints,
chat_response messages: kwarg compatibility, default_model fallback
in the three credential combinations, configured? against ENV +
Setting, strict-flag stripping for both key types, and a
`Setting.expects(:anthropic_model).never` assertion proving the
ENV-precedence test now exercises the lazy path.
All 4365 tests pass (1 pre-existing libvips env error unrelated).
This commit is contained in:
@@ -53,9 +53,18 @@ class Chat < ApplicationRecord
|
||||
|
||||
# Returns the default AI model to use for chats.
|
||||
# Resolved from the configured llm_provider so installs that swap providers
|
||||
# don't have to manually update every chat default.
|
||||
# don't have to manually update every chat default. Falls through to a
|
||||
# provider that actually has credentials configured, otherwise the chosen
|
||||
# provider's classes would later raise "no LLM provider supports model …"
|
||||
# even when the other provider is configured.
|
||||
def default_model
|
||||
if Setting.llm_provider == "anthropic"
|
||||
prefers_anthropic = Setting.llm_provider == "anthropic"
|
||||
|
||||
if prefers_anthropic && Provider::Anthropic.configured?
|
||||
Provider::Anthropic.effective_model.presence || Setting.anthropic_model
|
||||
elsif Provider::Openai.configured?
|
||||
Provider::Openai.effective_model.presence || Setting.openai_model
|
||||
elsif Provider::Anthropic.configured?
|
||||
Provider::Anthropic.effective_model.presence || Setting.anthropic_model
|
||||
else
|
||||
Provider::Openai.effective_model.presence || Setting.openai_model
|
||||
|
||||
@@ -12,10 +12,19 @@ class Provider::Anthropic < Provider
|
||||
VISION_CAPABLE_MODEL_PREFIXES = %w[claude].freeze
|
||||
|
||||
def self.effective_model
|
||||
configured_model = ENV.fetch("ANTHROPIC_MODEL", Setting.anthropic_model)
|
||||
# Use ENV[].presence rather than ENV.fetch(KEY, default) so the Setting
|
||||
# lookup is only performed when the ENV var is actually absent — otherwise
|
||||
# the default arg is evaluated eagerly on every call.
|
||||
configured_model = ENV["ANTHROPIC_MODEL"].presence || Setting.anthropic_model
|
||||
configured_model.presence || DEFAULT_MODEL
|
||||
end
|
||||
|
||||
def self.configured?
|
||||
ENV["ANTHROPIC_ACCESS_TOKEN"].present? ||
|
||||
ENV["ANTHROPIC_API_KEY"].present? ||
|
||||
Setting.anthropic_access_token.present?
|
||||
end
|
||||
|
||||
def initialize(access_token, base_url: nil, model: nil)
|
||||
client_options = { api_key: access_token }
|
||||
client_options[:base_url] = base_url if base_url.present?
|
||||
@@ -27,6 +36,12 @@ class Provider::Anthropic < Provider
|
||||
end
|
||||
|
||||
def supports_model?(model)
|
||||
# Custom endpoints (Bedrock, Vertex, or other Anthropic-compatible proxies)
|
||||
# use their own model-ID conventions — e.g. Bedrock IDs look like
|
||||
# `anthropic.claude-sonnet-4-5-20250929-v1:0`. Mirror the OpenAI provider
|
||||
# and bypass the prefix gate when the caller has wired a custom base_url.
|
||||
return true if custom_endpoint?
|
||||
|
||||
DEFAULT_ANTHROPIC_MODEL_PREFIXES.any? { |prefix| model.to_s.start_with?(prefix) }
|
||||
end
|
||||
|
||||
@@ -78,6 +93,7 @@ class Provider::Anthropic < Provider
|
||||
instructions: nil,
|
||||
functions: [],
|
||||
function_results: [],
|
||||
messages: nil,
|
||||
conversation_history: [],
|
||||
streamer: nil,
|
||||
previous_response_id: nil,
|
||||
|
||||
@@ -74,10 +74,14 @@ class Provider::Anthropic::ChatConfig
|
||||
|
||||
# OpenAI strict schemas frequently include `additionalProperties: false`, which
|
||||
# Anthropic also accepts. The shapes are otherwise JSON Schema 2020-12 compatible.
|
||||
# `strict` is OpenAI-only and must not be forwarded.
|
||||
# `strict` is OpenAI-only and must not be forwarded — strip both symbol and
|
||||
# string keys so we don't leak it when a caller hands us a JSON-decoded hash.
|
||||
def anthropic_input_schema(schema)
|
||||
schema = schema.deep_dup
|
||||
schema.delete(:strict) if schema.is_a?(Hash)
|
||||
if schema.is_a?(Hash)
|
||||
schema.delete(:strict)
|
||||
schema.delete("strict")
|
||||
end
|
||||
schema
|
||||
end
|
||||
end
|
||||
|
||||
@@ -14,6 +14,10 @@ class Provider::Openai < Provider
|
||||
ENV.fetch("OPENAI_MODEL") { Setting.openai_model }.presence || DEFAULT_MODEL
|
||||
end
|
||||
|
||||
def self.configured?
|
||||
ENV["OPENAI_ACCESS_TOKEN"].present? || Setting.openai_access_token.present?
|
||||
end
|
||||
|
||||
def initialize(access_token, uri_base: nil, model: nil)
|
||||
client_options = { access_token: access_token }
|
||||
llm_uri_base = uri_base.presence
|
||||
|
||||
@@ -74,6 +74,7 @@ class Setting < RailsSettings::Base
|
||||
eodhd_api_key
|
||||
alpha_vantage_api_key
|
||||
openai_access_token
|
||||
anthropic_access_token
|
||||
external_assistant_token
|
||||
].freeze
|
||||
|
||||
|
||||
@@ -62,6 +62,29 @@ class ChatTest < ActiveSupport::TestCase
|
||||
end
|
||||
end
|
||||
|
||||
test "default_model returns claude when LLM_PROVIDER=anthropic and Anthropic is configured" do
|
||||
Provider::Anthropic.stubs(:configured?).returns(true)
|
||||
Setting.stubs(:llm_provider).returns("anthropic")
|
||||
|
||||
assert_equal Provider::Anthropic::DEFAULT_MODEL, Chat.default_model
|
||||
end
|
||||
|
||||
test "default_model falls back to OpenAI when Anthropic is preferred but unconfigured" do
|
||||
Provider::Anthropic.stubs(:configured?).returns(false)
|
||||
Provider::Openai.stubs(:configured?).returns(true)
|
||||
Setting.stubs(:llm_provider).returns("anthropic")
|
||||
|
||||
assert_equal Provider::Openai::DEFAULT_MODEL, Chat.default_model
|
||||
end
|
||||
|
||||
test "default_model uses Anthropic when OpenAI is unconfigured" do
|
||||
Provider::Anthropic.stubs(:configured?).returns(true)
|
||||
Provider::Openai.stubs(:configured?).returns(false)
|
||||
Setting.stubs(:llm_provider).returns("openai")
|
||||
|
||||
assert_equal Provider::Anthropic::DEFAULT_MODEL, Chat.default_model
|
||||
end
|
||||
|
||||
test "creates with configured model when OPENAI_MODEL env is set" do
|
||||
prompt = "Test prompt"
|
||||
|
||||
|
||||
@@ -65,4 +65,30 @@ class Provider::Anthropic::ChatConfigTest < ActiveSupport::TestCase
|
||||
# Anthropic schemas must not carry the OpenAI-specific `strict` flag.
|
||||
req[:tools].each { |t| assert_not t[:input_schema].key?(:strict) }
|
||||
end
|
||||
|
||||
test "strips both symbol and string-keyed `strict` flags from input_schema" do
|
||||
config = Provider::Anthropic::ChatConfig.new(
|
||||
prompt: "hi",
|
||||
functions: [
|
||||
{
|
||||
name: "fn_with_string_strict",
|
||||
description: "schema arrived from JSON.parse with string keys",
|
||||
params_schema: {
|
||||
"type" => "object",
|
||||
"properties" => {},
|
||||
"required" => [],
|
||||
"additionalProperties" => false,
|
||||
"strict" => true
|
||||
},
|
||||
strict: true
|
||||
}
|
||||
]
|
||||
)
|
||||
|
||||
req = config.build_request(model: "claude-sonnet-4-6")
|
||||
|
||||
schema = req[:tools].first[:input_schema]
|
||||
assert_not schema.key?(:strict)
|
||||
assert_not schema.key?("strict")
|
||||
end
|
||||
end
|
||||
|
||||
@@ -31,6 +31,20 @@ class Provider::AnthropicTest < ActiveSupport::TestCase
|
||||
assert_not @subject.supports_model?("gpt-4.1")
|
||||
end
|
||||
|
||||
test "supports_model? bypasses the prefix gate for custom endpoints" do
|
||||
custom = Provider::Anthropic.new(
|
||||
"test-token",
|
||||
base_url: "https://bedrock.example.com/anthropic",
|
||||
model: "anthropic.claude-sonnet-4-5-20250929-v1:0"
|
||||
)
|
||||
|
||||
# Bedrock-shaped IDs start with "anthropic", not "claude" — would fail the
|
||||
# default prefix check, but custom endpoints must accept any model.
|
||||
assert custom.supports_model?("anthropic.claude-sonnet-4-5-20250929-v1:0")
|
||||
assert custom.supports_model?("claude-opus-4@20250514")
|
||||
assert custom.supports_model?("any-string-the-endpoint-accepts")
|
||||
end
|
||||
|
||||
test "supported_models_description returns prefixes for standard provider" do
|
||||
assert_equal "models starting with: claude", @subject.supported_models_description
|
||||
end
|
||||
@@ -40,12 +54,28 @@ class Provider::AnthropicTest < ActiveSupport::TestCase
|
||||
assert_not @subject.supports_pdf_processing?(model: "gpt-4o")
|
||||
end
|
||||
|
||||
test "effective_model defers to ENV when set" do
|
||||
test "effective_model defers to ENV when set without consulting Setting" do
|
||||
ClimateControl.modify("ANTHROPIC_MODEL" => "claude-haiku-4-5") do
|
||||
Setting.expects(:anthropic_model).never
|
||||
assert_equal "claude-haiku-4-5", Provider::Anthropic.effective_model
|
||||
end
|
||||
end
|
||||
|
||||
test "configured? reflects ENV and Setting presence" do
|
||||
ClimateControl.modify("ANTHROPIC_ACCESS_TOKEN" => nil, "ANTHROPIC_API_KEY" => nil) do
|
||||
Setting.stubs(:anthropic_access_token).returns(nil)
|
||||
assert_not Provider::Anthropic.configured?
|
||||
|
||||
Setting.stubs(:anthropic_access_token).returns("sk-ant-x")
|
||||
assert Provider::Anthropic.configured?
|
||||
end
|
||||
|
||||
ClimateControl.modify("ANTHROPIC_API_KEY" => "sk-ant-y") do
|
||||
Setting.stubs(:anthropic_access_token).returns(nil)
|
||||
assert Provider::Anthropic.configured?
|
||||
end
|
||||
end
|
||||
|
||||
test "effective_model falls back to default when nothing set" do
|
||||
ClimateControl.modify("ANTHROPIC_MODEL" => nil) do
|
||||
Setting.stubs(:anthropic_model).returns(nil)
|
||||
@@ -67,6 +97,31 @@ class Provider::AnthropicTest < ActiveSupport::TestCase
|
||||
assert_match(/rate limit/i, response.error.message)
|
||||
end
|
||||
|
||||
test "chat_response accepts messages: kwarg passed by Responder without raising" do
|
||||
# The OpenAI-shaped `messages:` array is passed alongside `conversation_history:`
|
||||
# for cross-provider parity. Anthropic ignores it but must still accept it as
|
||||
# a keyword argument — historical regression that broke the first chat turn.
|
||||
fake_client = stub_anthropic_client_with(
|
||||
build_anthropic_message(
|
||||
id: "msg_kw",
|
||||
model: @subject_model,
|
||||
text_blocks: [ "ok" ],
|
||||
tool_use_blocks: [],
|
||||
usage: { input_tokens: 1, output_tokens: 1 }
|
||||
)
|
||||
)
|
||||
@subject.instance_variable_set(:@client, fake_client)
|
||||
|
||||
response = @subject.chat_response(
|
||||
"hi",
|
||||
model: @subject_model,
|
||||
messages: [ { role: "user", content: "hi" } ],
|
||||
conversation_history: []
|
||||
)
|
||||
|
||||
assert response.success?
|
||||
end
|
||||
|
||||
test "chat_response returns parsed ChatResponse on success" do
|
||||
fake_client = stub_anthropic_client_with(
|
||||
build_anthropic_message(
|
||||
|
||||
Reference in New Issue
Block a user