diff --git a/CHANGELOG.md b/CHANGELOG.md index 846df76..d1065fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,17 @@ # Changelog +## Unreleased + +- `ask` takes `extra:` for top-level request fields beyond `model`, `state`, + and `questions`. The body was hard-coded to those three, so OpenRouter's + documented `provider`, `session_id`, `trace`, and `user` could not be sent + at all; the official JavaScript SDK forwards any unknown top-level field and + Python has `extra_body`. `model`, `state`, and `questions` cannot be + overridden this way. +- `Providers::Base#request_body` takes `extra:`. A provider overriding it + without that keyword keeps working, and raises `ConfigurationError` if + extra fields are actually passed rather than silently dropping them. + ## 0.1.0 - 2026-09-18 Provider-neutral release. One `Client`, two providers behind it. diff --git a/README.md b/README.md index 79638d6..60399d9 100644 --- a/README.md +++ b/README.md @@ -106,7 +106,8 @@ is whatever the provider returned. Subclass `RubyDecisionModel::Providers::Base` and define `name`, `env_var`, `default_base_url`, `endpoint_path`, `default_model`, and optionally `aliases` and `reports_cost?`. Override `headers`, `request_body`, or `usage` when the -wire format differs. Pass an instance as `provider:`. +wire format differs; a `request_body` override should take `extra:` if callers +are to pass extra fields through it. Pass an instance as `provider:`. ## Questions and answers @@ -124,6 +125,23 @@ Answers come back typed: `Answers::Noul` (`noul`, `probabilities`), `response.nouls`, `response.choices`, and `response.scores` return the answers of one type keyed the same way as `response.answers`. +### Extra request fields + +`ask` takes `extra:` for top-level request fields beyond `model`, `state`, and +`questions`. OpenRouter documents `provider`, `session_id`, `trace`, and `user` +on a decisions request: + +```ruby +client.ask(state: state, questions: questions, + extra: { session_id: conversation_id, user: user_id }) +``` + +`model`, `state`, and `questions` cannot be set this way; they come from the +client and from the arguments that the answers are matched against, and +`RequestError` says so. A provider that overrides `request_body` without an +`extra:` keyword raises `ConfigurationError` when extra fields are passed, +rather than dropping them. + Score `probabilities` and `legend` are keyed by the wire's string level keys (`"0"`, `"1"`, ...), not by the criteria labels. Choice `probabilities` sum to approximately 1; treat them as calibrated, not normalized. diff --git a/lib/ruby_decision_model/client.rb b/lib/ruby_decision_model/client.rb index 3457726..b552650 100644 --- a/lib/ruby_decision_model/client.rb +++ b/lib/ruby_decision_model/client.rb @@ -53,10 +53,12 @@ def base_url @provider.base_url end - def ask(state:, questions:) + # extra: top-level request fields the provider documents beyond model, + # state, and questions. + def ask(state:, questions:, extra: {}) raise RequestError, "questions must not be empty" if questions.nil? || questions.empty? - body = @provider.request_body(model: @model, state: state, questions: questions) + body = build_request_body(state: state, questions: questions, extra: extra) status, response_body, response_headers = perform_with_retry( url: @provider.url, headers: @provider.headers, body: body ) @@ -65,6 +67,26 @@ def ask(state:, questions:) private + # request_body is a documented override point, so a provider written + # against the old three-keyword signature keeps working. It just cannot + # be handed extra fields, and says so rather than dropping them. + def build_request_body(state:, questions:, extra:) + return @provider.request_body(model: @model, state: state, questions: questions) if extra.nil? || extra.empty? + + unless provider_accepts_extra? + raise ConfigurationError, + "#{@provider.name} defines request_body without an extra: keyword, so it cannot send extra fields" + end + + @provider.request_body(model: @model, state: state, questions: questions, extra: extra) + end + + def provider_accepts_extra? + @provider.method(:request_body).parameters.any? do |kind, name| + kind == :keyrest || (%i[key keyreq].include?(kind) && name == :extra) + end + end + def resolve_provider(provider, api_key:, base_url:) case provider when Providers::Base diff --git a/lib/ruby_decision_model/providers/base.rb b/lib/ruby_decision_model/providers/base.rb index 85f7089..71674ac 100644 --- a/lib/ruby_decision_model/providers/base.rb +++ b/lib/ruby_decision_model/providers/base.rb @@ -76,8 +76,34 @@ def headers } end - def request_body(model:, state:, questions:) - JSON.generate({ "model" => model, "state" => state, "questions" => questions }) + # extra: top-level request fields beyond the three the API requires. + # OpenRouter documents provider, session_id, trace, and user on a + # decisions request, and the official JS SDK forwards any unknown + # top-level field, so there has to be a way to send one. The three + # required fields are not overridable: they come from a validated + # client, and a request whose questions do not match the ones the + # answers are matched against is not a request worth making. + RESERVED_BODY_FIELDS = %w[model state questions].freeze + + def request_body(model:, state:, questions:, extra: {}) + body = { "model" => model, "state" => state, "questions" => questions } + JSON.generate(body.merge(stringify_extra(extra))) + end + + def stringify_extra(extra) + return {} if extra.nil? || extra.empty? + + raise RequestError, "extra must be a Hash of request fields, got #{extra.class}" unless extra.is_a?(Hash) + + extra.each_with_object({}) do |(key, value), fields| + name = key.to_s + if RESERVED_BODY_FIELDS.include?(name) + raise RequestError, "extra must not set #{name}; pass it as its own argument" + end + raise RequestError, "extra has #{name.inspect} twice" if fields.key?(name) + + fields[name] = value + end end def usage(parsed) diff --git a/test/extra_fields_test.rb b/test/extra_fields_test.rb new file mode 100644 index 0000000..572829e --- /dev/null +++ b/test/extra_fields_test.rb @@ -0,0 +1,130 @@ +# frozen_string_literal: true + +require "test_helper" + +# Top-level request fields beyond model, state, and questions. +class ExtraFieldsTest < Minitest::Test + def questions + { "urgent" => RubyDecisionModel::Questions.noul("Is this urgent?") } + end + + def success_body + JSON.generate("answers" => { "urgent" => { "type" => "noul", "noul" => 0.5 } }, "usage" => {}) + end + + def transport + @transport ||= FakeTransport.new([[200, success_body]]) + end + + def client(**options) + RubyDecisionModel::Client.new(**{ api_key: "k", transport: transport, sleeper: no_sleep }.merge(options)) + end + + def sent_body + JSON.parse(transport.calls.first[:body]) + end + + def test_no_extra_fields_by_default + client.ask(state: "hello", questions: questions) + + assert_equal %w[model questions state], sent_body.keys.sort + end + + def test_open_routers_documented_fields_can_be_sent + client.ask(state: "hello", questions: questions, + extra: { "session_id" => "conv_1", "user" => "u_42" }) + + assert_equal "conv_1", sent_body["session_id"] + assert_equal "u_42", sent_body["user"] + end + + def test_symbol_keys_are_stringified + client.ask(state: "hello", questions: questions, extra: { session_id: "conv_1" }) + + assert_equal "conv_1", sent_body["session_id"] + end + + def test_structured_values_survive + client.ask(state: "hello", questions: questions, + extra: { "provider" => { "order" => %w[typesafe] }, "trace" => { "enabled" => true } }) + + assert_equal({ "order" => %w[typesafe] }, sent_body["provider"]) + assert_equal({ "enabled" => true }, sent_body["trace"]) + end + + def test_the_required_fields_still_arrive + client.ask(state: "hello", questions: questions, extra: { "user" => "u" }) + + assert_equal "typesafe/jev-1.13", sent_body["model"] + assert_equal "hello", sent_body["state"] + assert_equal %w[urgent], sent_body["questions"].keys + end + + def test_extra_may_not_overwrite_the_required_fields + %w[model state questions].each do |reserved| + error = assert_raises(RubyDecisionModel::RequestError) do + client.ask(state: "hello", questions: questions, extra: { reserved => "hijacked" }) + end + assert_match(/must not set #{reserved}/, error.message) + end + end + + def test_keys_that_collide_once_stringified_are_refused + assert_raises(RubyDecisionModel::RequestError) do + client.ask(state: "hello", questions: questions, extra: { :user => "a", "user" => "b" }) + end + end + + def test_extra_must_be_a_hash + assert_raises(RubyDecisionModel::RequestError) do + client.ask(state: "hello", questions: questions, extra: [%w[user u]]) + end + end + + def test_extra_does_not_leak_between_calls + transport = FakeTransport.new([[200, success_body], [200, success_body]]) + c = RubyDecisionModel::Client.new(api_key: "k", transport: transport, sleeper: no_sleep) + + c.ask(state: {}, questions: questions, extra: { "user" => "u" }) + c.ask(state: {}, questions: questions) + + assert_equal "u", JSON.parse(transport.calls[0][:body])["user"] + refute JSON.parse(transport.calls[1][:body]).key?("user") + end + + # --- providers written against the old signature --- + + def legacy_provider_class + Class.new(RubyDecisionModel::Providers::Base) do + def name = :legacy + def env_var = "LEGACY_API_KEY" + def default_base_url = "https://legacy.example" + def endpoint_path = "/decide" + def default_model = "legacy-1" + + def request_body(model:, state:, questions:) + JSON.generate("m" => model, "s" => state, "q" => questions) + end + end + end + + def test_a_provider_without_the_extra_keyword_still_works_without_extra + c = RubyDecisionModel::Client.new(provider: legacy_provider_class.new(api_key: "k"), + transport: transport, sleeper: no_sleep) + + c.ask(state: "hello", questions: questions) + + assert_equal %w[m q s], sent_body.keys.sort + end + + def test_a_provider_without_the_extra_keyword_says_so_rather_than_dropping_fields + c = RubyDecisionModel::Client.new(provider: legacy_provider_class.new(api_key: "k"), + transport: transport, sleeper: no_sleep) + + error = assert_raises(RubyDecisionModel::ConfigurationError) do + c.ask(state: "hello", questions: questions, extra: { "user" => "u" }) + end + assert_match(/cannot send extra fields/, error.message) + assert_empty transport.calls + end +end