diff --git a/CHANGELOG.md b/CHANGELOG.md index 846df76..f5c8b93 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/README.md b/README.md index 79638d6..d4dc3e8 100644 --- a/README.md +++ b/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`). diff --git a/lib/ruby_decision_model/questions.rb b/lib/ruby_decision_model/questions.rb index 5620901..712f8f0 100644 --- a/lib/ruby_decision_model/questions.rb +++ b/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 diff --git a/test/questions_test.rb b/test/questions_test.rb index 79ae020..26fb2e0 100644 --- a/test/questions_test.rb +++ b/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