fix: validate what goes inside criteria, not just the container

CHANGELOG.md

@@ -1,5 +1,18 @@
 # Changelog
 
+## Unreleased
+
+- `Questions.noul` validates `criteria`, which it previously passed through
+  untouched while `choice` and `score` validated theirs. It must be a Hash
+  keyed by `true`/`false` (booleans or the strings, normalized to strings),
+  and anything else raises `ArgumentError` at the call site instead of a 422
+  from the API.
+- `Questions.choice` raises when two labels stringify to the same key rather
+  than silently collapsing them into one option with the later description.
+  Blank labels are rejected.
+- Descriptions in all three builders must be a String, Hash, Array, or nil,
+  matching the `EntryType` the official SDKs accept.
+
 ## 0.1.0 - 2026-09-18
 
 Provider-neutral release. One `Client`, two providers behind it.

README.md

@@ -118,6 +118,22 @@ Questions.choice("Which team?", criteria: { "billing" => "...", "auth" => "..."
 Questions.score("How severe?", criteria: ["cosmetic", "minor", "major"])            # 2 to 10 levels
 ```
 
+A noul takes optional `criteria` describing what a yes and a no mean. The keys
+are `true` and `false` — Ruby's booleans or the strings — and anything else
+raises `ArgumentError`:
+
+```ruby
+Questions.noul(
+  "Has the customer contacted support about this before?",
+  criteria: { true => "Mentions a prior attempt or ticket", false => "No sign of previous contact" }
+)
+```
+
+Every description — a noul outcome, a choice option, a score level — is text,
+a Hash, an Array, or `nil` for undescribed. Choice labels are stringified for
+the wire, and two that stringify the same (`:billing` and `"billing"`) raise
+rather than silently becoming one option with the second description.
+
 Answers come back typed: `Answers::Noul` (`noul`, `probabilities`),
 `Answers::Choice` (`choice`, `confidence`, `probabilities`), and
 `Answers::Score` (`score`, `confidence`, `probabilities`, `legend`).

lib/ruby_decision_model/questions.rb

@@ -5,11 +5,16 @@ module RubyDecisionModel
   module Questions
     module_function
 
+    # The keys a noul's criteria may carry. The wire wants "true" and
+    # "false"; Ruby's booleans are accepted and stringified, since that is
+    # how the hash reads best at a call site.
+    NOUL_CRITERIA_KEYS = %w[true false].freeze
+
     def noul(instructions, criteria: nil)
       validate_instructions!(instructions)
 
       question = { "type" => "noul", "instructions" => instructions }
-      question["criteria"] = criteria unless criteria.nil?
+      question["criteria"] = validate_noul_criteria!(criteria) unless criteria.nil?
       question
     end
 
@@ -19,10 +24,8 @@ def choice(instructions, criteria:)
         raise ArgumentError, "criteria must be a Hash with 1..255 entries"
       end
 
-      stringified = {}
-      criteria.each { |k, v| stringified[k.to_s] = v }
-
-      { "type" => "choice", "instructions" => instructions, "criteria" => stringified }
+      { "type" => "choice", "instructions" => instructions,
+        "criteria" => stringify_labels(criteria) }
     end
 
     def score(instructions, criteria:)
@@ -31,9 +34,58 @@ def score(instructions, criteria:)
         raise ArgumentError, "criteria must be an Array with 2..10 entries"
       end
 
+      criteria.each_with_index { |level, i| validate_description!(level, "criteria[#{i}]") }
+
       { "type" => "score", "instructions" => instructions, "criteria" => criteria }
     end
 
+    # Option labels go over the wire as JSON object keys, so they are
+    # stringified. Two labels that stringify the same would silently become
+    # one option, and the model would be offered a rubric the caller did not
+    # write, so say so instead.
+    def stringify_labels(criteria)
+      criteria.each_with_object({}) do |(label, description), stringified|
+        key = label.to_s
+        if stringified.key?(key)
+          raise ArgumentError, "criteria labels collide once stringified: two entries are both #{key.inspect}"
+        end
+        raise ArgumentError, "criteria labels must not be blank" if key.strip.empty?
+
+        validate_description!(description, "criteria[#{key.inspect}]")
+        stringified[key] = description
+      end
+    end
+
+    # noul criteria describe what a yes and a no mean. Anything else is a
+    # 422 from the API, and choice and score have always said so at the call
+    # site rather than over the wire.
+    def validate_noul_criteria!(criteria)
+      unless criteria.is_a?(Hash)
+        raise ArgumentError, "criteria must be a Hash with true and false descriptions, got #{criteria.class}"
+      end
+
+      criteria.each_with_object({}) do |(outcome, description), stringified|
+        key = outcome.to_s
+        unless NOUL_CRITERIA_KEYS.include?(key)
+          raise ArgumentError,
+                "criteria keys must be #{NOUL_CRITERIA_KEYS.join(' and ')}, got #{outcome.inspect}"
+        end
+        raise ArgumentError, "criteria has #{key.inspect} twice" if stringified.key?(key)
+
+        validate_description!(description, "criteria[#{key.inspect}]")
+        stringified[key] = description
+      end
+    end
+
+    # A description is an EntryType: text, structure, or nil for undescribed.
+    def validate_description!(description, where)
+      return if description.nil?
+      return if description.is_a?(String) || description.is_a?(Hash) || description.is_a?(Array)
+
+      raise ArgumentError, "#{where} must be a String, Hash, Array, or nil, got #{description.class}"
+    end
+    private_class_method :stringify_labels, :validate_noul_criteria!, :validate_description!
+
     def validate_instructions!(instructions)
       case instructions
       when String

test/questions_test.rb

@@ -57,6 +57,88 @@ def test_choice_accepts_255_options
     assert_equal 255, question["criteria"].length
   end
 
+  # --- criteria contents, not just their container ---
+
+  def test_noul_accepts_true_and_false_descriptions
+    question = RubyDecisionModel::Questions.noul(
+      "Has the customer contacted support before?",
+      criteria: { true => "Mentions a prior ticket", false => "No sign of previous contact" }
+    )
+
+    assert_equal({ "true" => "Mentions a prior ticket", "false" => "No sign of previous contact" },
+                 question["criteria"])
+  end
+
+  def test_noul_accepts_string_keys_too
+    question = RubyDecisionModel::Questions.noul("Urgent?", criteria: { "true" => "yes means", "false" => nil })
+    assert_equal({ "true" => "yes means", "false" => nil }, question["criteria"])
+  end
+
+  def test_noul_accepts_one_sided_criteria
+    question = RubyDecisionModel::Questions.noul("Urgent?", criteria: { true => "time-sensitive" })
+    assert_equal({ "true" => "time-sensitive" }, question["criteria"])
+  end
+
+  def test_noul_rejects_criteria_that_are_not_a_hash
+    error = assert_raises(ArgumentError) { RubyDecisionModel::Questions.noul("Urgent?", criteria: %w[yes no]) }
+    assert_match(/must be a Hash/, error.message)
+  end
+
+  def test_noul_rejects_keys_that_are_not_true_or_false
+    error = assert_raises(ArgumentError) do
+      RubyDecisionModel::Questions.noul("Urgent?", criteria: { "yes" => "means urgent" })
+    end
+    assert_match(/must be true and false/, error.message)
+  end
+
+  def test_noul_rejects_a_duplicate_outcome
+    assert_raises(ArgumentError) do
+      RubyDecisionModel::Questions.noul("Urgent?", criteria: { true => "a", "true" => "b" })
+    end
+  end
+
+  def test_noul_rejects_a_description_that_is_not_text_or_structure
+    assert_raises(ArgumentError) { RubyDecisionModel::Questions.noul("Urgent?", criteria: { true => 1 }) }
+  end
+
+  def test_noul_without_criteria_is_unchanged
+    refute RubyDecisionModel::Questions.noul("Urgent?").key?("criteria")
+  end
+
+  def test_choice_rejects_labels_that_collide_once_stringified
+    # Both become "billing" as a JSON key, so one option would silently
+    # replace the other and the model would see a rubric nobody wrote.
+    error = assert_raises(ArgumentError) do
+      RubyDecisionModel::Questions.choice("Which team?", criteria: { :billing => "a", "billing" => "b" })
+    end
+    assert_match(/collide/, error.message)
+  end
+
+  def test_choice_rejects_a_blank_label
+    assert_raises(ArgumentError) { RubyDecisionModel::Questions.choice("Which?", criteria: { "  " => "a" }) }
+  end
+
+  def test_choice_rejects_a_description_that_is_not_text_or_structure
+    assert_raises(ArgumentError) { RubyDecisionModel::Questions.choice("Which?", criteria: { "a" => 3 }) }
+  end
+
+  def test_choice_accepts_structured_descriptions
+    criteria = { "billing" => { "covers" => %w[refunds invoices] }, "auth" => nil }
+    question = RubyDecisionModel::Questions.choice("Which team?", criteria: criteria)
+
+    assert_equal criteria, question["criteria"]
+  end
+
+  def test_score_rejects_a_level_that_is_not_text_or_structure
+    error = assert_raises(ArgumentError) { RubyDecisionModel::Questions.score("How bad?", criteria: ["low", 2]) }
+    assert_match(/criteria\[1\]/, error.message)
+  end
+
+  def test_score_accepts_structured_levels
+    criteria = [{ "label" => "calm" }, nil, "very angry"]
+    assert_equal criteria, RubyDecisionModel::Questions.score("How frustrated?", criteria: criteria)["criteria"]
+  end
+
   def test_score_rejects_too_many_levels
     assert_raises(ArgumentError) { RubyDecisionModel::Questions.score("Rate severity", criteria: (1..11).map(&:to_s)) }
   end