Skip to content

[rig-sampler] feat(validation): improve missing-field error descriptions for constrained string schemas#198

Merged
pelikhan merged 1 commit into
mainfrom
rig-sampler/57-complex-integration-sonnet-8f575dd0a1be2695
Jul 26, 2026
Merged

[rig-sampler] feat(validation): improve missing-field error descriptions for constrained string schemas#198
pelikhan merged 1 commit into
mainfrom
rig-sampler/57-complex-integration-sonnet-8f575dd0a1be2695

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Samples run

Five consecutive samples were run through the stub Copilot SDK runner (npm run sample):

Sample Result Notes
57-complex-integration-sonnet.ts ✅ pass Complex schema with s.enum("low","medium","high") for riskLevel, s.optional(s.string) for audience, multi-agent delegation, and tools. All fields conformed on first turn.
58-async-tool-node-builtins.ts ✅ pass Uses s.string for pid and s.enum("running","idle","unknown") for status. Exposed that s.string in missing-field errors gives no hint about expected format.
59-bash-each.ts ✅ pass Uses s.url in the input array endpoints; a missing url field would only report "expected string" — unhelpful when the model should know it needs a URI.
01-single-agent-haiku.ts ✅ pass Simple baseline; no constrained strings.
02-review-git-diff.ts ✅ pass Uses s.optional(s.number) for line; plain strings elsewhere.

What the runs revealed

  • Samples 57–59 all define schemas using constrained string helpers (s.url, s.nonEmptyString, s.path, s.date).
  • When a required field of one of these types is missing, the repair prompt previously said "missing required field (expected string)" — the same message for s.string, s.nonEmptyString, s.url, s.path, and s.date.
  • This forces the model to infer constraints from context rather than from the error itself, which can lead to unnecessary repair turns.

The improvement

describeSchemaType (used only in missing-field error messages) now returns a specific label for constrained string schemas:

Schema helper Before After
s.nonEmptyString "string" "non-empty string"
s.url "string" "URL string"
s.date "string" "date string (YYYY-MM-DD)"
s.path "string" "path string"
s.string "string" "string" (unchanged)

This is a non-breaking, internal-only change: describeSchemaType is a private helper, and the public ValidationResult type is unchanged. Four new unit tests cover the new messages.

Generated by Daily Rig Sampler · sonnet46 91.5 AIC · ⌖ 8.3 AIC · ⊞ 5.4K ·

…ained string schemas

describeSchemaType now returns human-readable hints for s.nonEmptyString,
s.url, s.date, and s.path when used as required object fields. Previously
all string variants reported 'expected string'; now they report
'non-empty string', 'URL string', 'date string (YYYY-MM-DD)', or
'path string' so repair prompts give the model clearer guidance.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@pelikhan
pelikhan marked this pull request as ready for review July 26, 2026 20:52
@pelikhan
pelikhan merged commit 138fb9d into main Jul 26, 2026
1 check passed
@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skills-Based Review 🧠

Applied /tdd and /grill-with-docs — approving. The change is focused, well-motivated, and fully covered by tests.

📋 Key Themes & Highlights

Positive Highlights

  • ✅ Four unit tests — one per constrained type — match the four new code branches exactly
  • format checks take precedence over minLength, matching the specificity hierarchy of the schema helpers
  • ✅ Non-breaking: describeSchemaType is private; ValidationResult public surface unchanged
  • ✅ PR body includes a clear before/after table grounded in real sampler runs

Minor Observations

  • The minLength > 0 guard is redundant given that s.nonEmptyString always sets minLength: 1, but it makes the intent explicit and causes no harm.
  • No existing test covers a schema with both format and minLength set simultaneously (not a real helper today). Low risk given the private scope.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 27.4 AIC · ⌖ 5.98 AIC · ⊞ 6.3K
Comment /matt to run again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant