Skip to content

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

Draft
simonx1 wants to merge 1 commit into
obie:mainfrom
simonx1:fix/question-builder-validation
Draft

simonx1 wants to merge 1 commit into
obie:mainfrom
simonx1:fix/question-builder-validation

Conversation

@simonx1

@simonx1 simonx1 commented Sep 18, 2026

Copy link
Copy Markdown

The builders were inconsistent about the same job. choice and score checked the type and size of their criteria; noul took whatever it was given and shipped it.

Questions.noul("Urgent?", criteria: ["yes", "no"])
# => {"type"=>"noul", "instructions"=>"Urgent?", "criteria"=>["yes", "no"]}  -> 422 from the API
Questions.noul("Urgent?", criteria: { "yes" => "means urgent" })
# => also built happily; the documented keys are true/false

None of the three looked at descriptions, so a score level or an option rubric could be an Integer and go over the wire as one.

Choice labels had a quieter problem. They're stringified to become JSON keys, so { billing: "...", "billing" => "..." } collapsed into one option carrying whichever description came last. The question that reached the model was not the question that was written, and nothing said so.

The fix

  • noul criteria must be a Hash keyed by true/false (the booleans read best at a call site and are stringified for the wire, which is what JSON would have done anyway). One-sided criteria are fine.
  • Descriptions in all three builders are an EntryType: String, Hash, Array, or nil for undescribed.
  • Colliding and blank choice labels raise.

The 255-option and 2..10-level caps already matched the docs and are unchanged.

Tests

14 new cases in test/questions_test.rb.


Draft: part of a security and API-coverage audit, opened for reference rather than as a request for immediate review. Independent of the other branches, each off main. Suite green on Ruby 3.2.11, 3.3.8 and 3.4.8.

🤖 Generated with Claude Code

The builders were inconsistent about the same job. choice and score
checked the type and size of their criteria; noul took whatever it was
given and shipped it. `Questions.noul("Urgent?", criteria: ["yes", "no"])`
built happily and came back a 422, and so did a criteria hash keyed
"yes"/"no" instead of true/false. noul criteria now have to be a Hash
keyed by true and false -- the booleans read best at a call site, and are
stringified for the wire, which is what JSON would have done anyway.

None of the three looked at descriptions. A score level or an option
rubric could be an Integer and go over the wire as one. They are an
EntryType now: text, a Hash, an Array, or nil for undescribed.

Choice labels had a quieter problem. They are stringified to become JSON
keys, so `{ billing: "...", "billing" => "..." }` collapsed into one
option carrying whichever description came last. The question that
reached the model was not the question that was written, and nothing said
so. Now it raises.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant