From 060f1209e9bdfeec07aecb9570641fa1eb7d6db6 Mon Sep 17 00:00:00 2001 From: Szymon Kurcab Date: Fri, 18 Sep 2026 20:46:10 +0200 Subject: [PATCH] feat: let callers send the request fields the providers document The request body was {model, state, questions} and nothing else, built from a literal. OpenRouter's DecisionsRequest documents four more -- provider, session_id, trace, user -- and none of them could be sent through this client at all. The official JavaScript SDK spreads the caller's request into the payload so any unknown top-level field goes through, and Python has extra_body. Here the only route was to subclass the provider and override request_body, for a session id. ask takes extra:. Keys are stringified, structured values go through untouched, and model, state, and questions are refused: they come from a validated client and from the arguments the answers are matched back against, so quietly letting a hash overwrite them would decouple the request from the response handling. request_body is a documented override point, so it gains extra: with a default and a provider written against the old three-keyword signature keeps working. Passing extra fields to one raises ConfigurationError naming the provider, rather than dropping the fields on the floor, which is the failure mode that would have cost someone an afternoon. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 12 ++ README.md | 20 +++- lib/ruby_decision_model/client.rb | 26 ++++- lib/ruby_decision_model/providers/base.rb | 30 ++++- test/extra_fields_test.rb | 130 ++++++++++++++++++++++ 5 files changed, 213 insertions(+), 5 deletions(-) create mode 100644 test/extra_fields_test.rb 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