fix: refuse unsendable questions before paying for them

CHANGELOG.md

@@ -1,5 +1,16 @@
 # Changelog
 
+## Unreleased
+
+- `ask` validates the questions map before sending it. A value that is not a
+  Hash, an unknown or missing `type`, missing or empty `instructions`, missing
+  or wrongly sized `criteria`, a blank id, or two ids that collide once
+  stringified all raise `RequestError` naming the id. Previously any of these
+  was serialized, sent, and billed, and came back as a 422 or a
+  `MissingAnswers`.
+- Added `Questions.validate!(question, id:)`, the same check as a public
+  method.
+
 ## 0.1.0 - 2026-09-18
 
 Provider-neutral release. One `Client`, two providers behind it.

README.md

@@ -124,6 +124,15 @@ 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`.
 
+`ask` validates the whole questions map before it sends anything: every value
+must be a Hash with a known `type`, `instructions`, and the `criteria` its type
+requires, and two ids must not collide once stringified (`:a` and `"a"` would
+be one answer for two questions). A question that cannot be answered raises
+`RequestError` naming its id, rather than being billed as input tokens and
+coming back as a 422 or a missing answer. Hand-rolled question hashes are held
+to the same rules as the builders'; `Questions.validate!(question, id:)` runs
+them on its own.
+
 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.
@@ -172,7 +181,7 @@ return is still accepted and treated as having no headers, which means no
 | Error | Meaning |
 | --- | --- |
 | `ConfigurationError` | No provider could be resolved, missing api_key, unknown provider, or bad `retry:` value |
-| `RequestError` | Questions hash was empty |
+| `RequestError` | Questions hash was empty, or a question could not be sent as written |
 | `TransportError` (`TimeoutError`) | Network or timeout failure after retries, carries `#cause_error` |
 | `ApiError` | Non-2xx response, carries `#status`, `#body`, and `#headers` |
 | `Unauthorized` | 401 |

lib/ruby_decision_model/client.rb

@@ -54,7 +54,7 @@ def base_url
     end
 
     def ask(state:, questions:)
-      raise RequestError, "questions must not be empty" if questions.nil? || questions.empty?
+      validate_questions!(questions)
 
       body = @provider.request_body(model: @model, state: state, questions: questions)
       status, response_body, response_headers = perform_with_retry(
@@ -65,6 +65,33 @@ def ask(state:, questions:)
 
     private
 
+    # A question the API will refuse is worth catching here: the request is
+    # billed by input tokens, so sending one that cannot be answered costs
+    # money to be told what the client already knew.
+    def validate_questions!(questions)
+      raise RequestError, "questions must not be empty" if questions.nil? || questions.empty?
+
+      unless questions.is_a?(Hash)
+        raise RequestError, "questions must be a Hash of id => question, got #{questions.class}"
+      end
+
+      seen = {}
+      questions.each_key do |raw_id|
+        id = raw_id.to_s
+        raise RequestError, "question id #{raw_id.inspect} is blank" if id.strip.empty?
+
+        # Answers come back keyed by the stringified id, so :a and "a" would
+        # be one answer for two questions.
+        if seen.key?(id)
+          raise RequestError,
+                "question ids #{seen[id].inspect} and #{raw_id.inspect} are both #{id.inspect} on the wire"
+        end
+
+        seen[id] = raw_id
+        Questions.validate!(questions[raw_id], id: id)
+      end
+    end
+
     def resolve_provider(provider, api_key:, base_url:)
       case provider
       when Providers::Base

lib/ruby_decision_model/questions.rb

@@ -34,6 +34,61 @@ def score(instructions, criteria:)
       { "type" => "score", "instructions" => instructions, "criteria" => criteria }
     end
 
+    TYPES = %w[noul choice score].freeze
+
+    # Checks one question -- built here or hand-rolled -- against the shape
+    # the API documents, and raises RequestError naming the offending id.
+    # Client#ask runs this over the whole map before it sends anything, so a
+    # question that cannot be answered costs nothing.
+    def validate!(question, id:)
+      raise RequestError, "question #{id.inspect} must be a Hash, got #{question.class}" unless question.is_a?(Hash)
+
+      type = field(question, "type")
+      unless TYPES.include?(type.to_s)
+        raise RequestError, "question #{id.inspect} has type #{type.inspect}; expected one of #{TYPES.join(', ')}"
+      end
+
+      validate_question_instructions!(question, id)
+      validate_question_criteria!(question, id, type.to_s)
+    end
+
+    def field(question, name)
+      question.fetch(name) { question[name.to_sym] }
+    end
+
+    def validate_question_instructions!(question, id)
+      has_key = question.key?("instructions") || question.key?(:instructions)
+      raise RequestError, "question #{id.inspect} is missing instructions" unless has_key
+
+      instructions = field(question, "instructions")
+      # nil is what the official SDKs call an undescribed entry; anything the
+      # builders would reject as empty is rejected here too.
+      return if instructions.nil?
+
+      case instructions
+      when String, Hash, Array
+        raise RequestError, "question #{id.inspect} has empty instructions" if instructions.empty?
+      else
+        raise RequestError, "question #{id.inspect} has instructions of type #{instructions.class}"
+      end
+    end
+
+    def validate_question_criteria!(question, id, type)
+      criteria = field(question, "criteria")
+
+      case type
+      when "choice"
+        unless criteria.is_a?(Hash) && criteria.size.between?(1, 255)
+          raise RequestError, "choice question #{id.inspect} needs criteria: a Hash with 1..255 entries"
+        end
+      when "score"
+        unless criteria.is_a?(Array) && criteria.size.between?(2, 10)
+          raise RequestError, "score question #{id.inspect} needs criteria: an Array with 2..10 entries"
+        end
+      end
+    end
+    private_class_method :field, :validate_question_instructions!, :validate_question_criteria!
+
     def validate_instructions!(instructions)
       case instructions
       when String

test/client_test.rb

@@ -282,21 +282,16 @@ def test_malformed_score_missing_confidence_raises_invalid_response
     assert_includes error.message, "severity"
   end
 
-  def test_unsupported_question_type_raises_invalid_response
+  def test_unsupported_question_type_is_refused_before_the_request
     weird_questions = { "mystery" => { "type" => "unknown", "instructions" => "huh" } }
-    body = JSON.generate(
-      "id" => "resp_9",
-      "model" => "typesafe/jev-1.13",
-      "answers" => { "mystery" => { "type" => "unknown", "value" => "x" } },
-      "usage" => {}
-    )
-    transport = FakeTransport.new([[200, body]])
+    transport = FakeTransport.new([[200, success_body]])
     client = build_client(transport)
 
-    error = assert_raises(RubyDecisionModel::InvalidResponse) do
+    error = assert_raises(RubyDecisionModel::RequestError) do
       client.ask(state: {}, questions: weird_questions)
     end
     assert_includes error.message, "mystery"
+    assert_empty transport.calls
   end
 
   def test_non_hash_probabilities_fall_back_to_empty_hash

test/question_validation_test.rb

@@ -0,0 +1,155 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+# Requests are billed by input tokens, so a question the API is going to
+# refuse should never leave the process.
+class QuestionValidationTest < Minitest::Test
+  def setup
+    @transport = FakeTransport.new([[200, success_body]])
+    @client = RubyDecisionModel::Client.new(api_key: "test-key", transport: @transport, sleeper: no_sleep)
+  end
+
+  def success_body
+    JSON.generate(
+      "answers" => { "q" => { "type" => "noul", "noul" => 0.5 } },
+      "usage" => { "input_tokens" => 1, "output_tokens" => 1 }
+    )
+  end
+
+  def refuse(questions)
+    error = assert_raises(RubyDecisionModel::RequestError) { @client.ask(state: {}, questions: questions) }
+    assert_empty @transport.calls, "the request should not have been sent"
+    error
+  end
+
+  # --- the map itself ---
+
+  def test_empty_and_nil_question_maps_are_refused
+    refuse({})
+    refuse(nil)
+  end
+
+  def test_a_questions_argument_that_is_not_a_hash_is_refused
+    error = refuse([RubyDecisionModel::Questions.noul("Is this urgent?")])
+    assert_match(/must be a Hash/, error.message)
+  end
+
+  def test_a_blank_question_id_is_refused
+    error = refuse({ "  " => RubyDecisionModel::Questions.noul("Is this urgent?") })
+    assert_match(/is blank/, error.message)
+  end
+
+  def test_ids_that_collide_once_stringified_are_refused
+    # Both arrive as "a" on the wire, so one answer would come back for two
+    # questions and the other would look like it went missing.
+    error = refuse({ :a => RubyDecisionModel::Questions.noul("First?"),
+                     "a" => RubyDecisionModel::Questions.noul("Second?") })
+    assert_match(/both "a" on the wire/, error.message)
+  end
+
+  def test_symbol_ids_on_their_own_are_fine
+    response = @client.ask(state: {}, questions: { q: RubyDecisionModel::Questions.noul("Is this urgent?") })
+    assert_in_delta 0.5, response["q"].noul
+  end
+
+  # --- each question ---
+
+  def test_a_question_that_is_not_a_hash_is_refused
+    error = refuse({ "q" => "is this urgent?" })
+    assert_match(/must be a Hash/, error.message)
+    assert_match(/"q"/, error.message)
+  end
+
+  def test_an_unknown_type_is_refused
+    error = refuse({ "q" => { "type" => "ranking", "instructions" => "Rank these" } })
+    assert_match(/expected one of noul, choice, score/, error.message)
+  end
+
+  def test_a_missing_type_is_refused
+    refuse({ "q" => { "instructions" => "Is this urgent?" } })
+  end
+
+  def test_missing_instructions_are_refused
+    error = refuse({ "q" => { "type" => "noul" } })
+    assert_match(/missing instructions/, error.message)
+  end
+
+  def test_empty_instructions_are_refused
+    refuse({ "q" => { "type" => "noul", "instructions" => "" } })
+    refuse({ "q" => { "type" => "noul", "instructions" => [] } })
+  end
+
+  def test_instructions_of_the_wrong_type_are_refused
+    error = refuse({ "q" => { "type" => "noul", "instructions" => 42 } })
+    assert_match(/instructions of type Integer/, error.message)
+  end
+
+  def test_nil_instructions_are_allowed
+    # EntryType in the official SDKs covers null for state, instructions, and
+    # criteria alike.
+    response = @client.ask(state: {}, questions: { "q" => { "type" => "noul", "instructions" => nil } })
+    assert_in_delta 0.5, response["q"].noul
+  end
+
+  # --- criteria each type requires ---
+
+  def test_a_choice_question_without_criteria_is_refused
+    error = refuse({ "q" => { "type" => "choice", "instructions" => "Which team?" } })
+    assert_match(/needs criteria/, error.message)
+  end
+
+  def test_a_choice_question_with_too_many_options_is_refused
+    criteria = (1..256).to_h { |i| [i.to_s, nil] }
+    refuse({ "q" => { "type" => "choice", "instructions" => "Which?", "criteria" => criteria } })
+  end
+
+  def test_a_score_question_with_one_level_is_refused
+    refuse({ "q" => { "type" => "score", "instructions" => "How bad?", "criteria" => ["only one"] } })
+  end
+
+  def test_a_score_question_with_eleven_levels_is_refused
+    refuse({ "q" => { "type" => "score", "instructions" => "How bad?", "criteria" => (1..11).map(&:to_s) } })
+  end
+
+  def test_a_noul_question_needs_no_criteria
+    response = @client.ask(state: {}, questions: { "q" => { "type" => "noul", "instructions" => "Urgent?" } })
+    assert_in_delta 0.5, response["q"].noul
+  end
+
+  def test_symbol_keyed_question_hashes_are_accepted
+    response = @client.ask(state: {}, questions: { "q" => { type: "noul", instructions: "Urgent?" } })
+    assert_in_delta 0.5, response["q"].noul
+  end
+
+  def test_questions_from_the_builders_pass
+    transport = FakeTransport.new([[200, JSON.generate(
+      "answers" => {
+        "a" => { "type" => "noul", "noul" => 0.5 },
+        "b" => { "type" => "choice", "choice" => "x", "confidence" => 0.5, "probabilities" => {} },
+        "c" => { "type" => "score", "score" => 1.0, "confidence" => 0.5, "probabilities" => {}, "legend" => {} }
+      },
+      "usage" => {}
+    )]])
+    client = RubyDecisionModel::Client.new(api_key: "k", transport: transport, sleeper: no_sleep)
+
+    response = client.ask(state: {}, questions: {
+                            "a" => RubyDecisionModel::Questions.noul("Urgent?"),
+                            "b" => RubyDecisionModel::Questions.choice("Which?", criteria: { "x" => nil }),
+                            "c" => RubyDecisionModel::Questions.score("How bad?", criteria: %w[low high])
+                          })
+
+    assert_equal 3, response.answers.size
+  end
+
+  # --- the validator on its own ---
+
+  def test_validate_is_callable_directly
+    assert_nil RubyDecisionModel::Questions.validate!(
+      RubyDecisionModel::Questions.noul("Urgent?"), id: "q"
+    )
+    assert_raises(RubyDecisionModel::RequestError) do
+      RubyDecisionModel::Questions.validate!({ "type" => "noul" }, id: "q")
+    end
+  end
+end