Skip to content

[rig-sampler] Improve array type description in missing-field error messages - #132

Merged
pelikhan merged 1 commit into
mainfrom
rig-sampler/21-ci-log-diagnosis-66d787baf6e731c0
Jul 25, 2026
Merged

[rig-sampler] Improve array type description in missing-field error messages#132
pelikhan merged 1 commit into
mainfrom
rig-sampler/21-ci-log-diagnosis-66d787baf6e731c0

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Samples run

# File Turns Notes
21 21-ci-log-diagnosis.ts 1 Structured output with commandsToTry: s.array(s.string). Clean run, no repair needed.
22 22-config-normalizer.ts 1 Uses s.unknown for normalized field — appropriate but schema is effectively unvalidated.
23 23-schema-inference.ts 1 Uses p.bash(...) intent and s.unknown for example output. No repair.
24 24-error-message-improver.ts 1 Uses s.optional(s.string) for context input field. Validated correctly.
25 25-migration-guide.ts 1 Uses s.optional(s.string) for context input field. Clean run.

All five samples ran successfully in a single turn with the stub runner.

What was identified

Samples 21, 22, and 24 all declare array fields in their output schemas (commandsToTry, fields, etc.) using s.array(s.string) or s.array(s.object(...)). When such a required field is missing from the model response, the harness emits:

$.commandsToTry: missing required field (expected array)

The describeSchemaType helper returned just "array" for any array schema — it checked "type" in schema before "items" in schema, so the items type was never surfaced. This meant repair prompts received a less-specific hint: the model knew it needed an array but not what element type to use, increasing the risk of a second repair turn.

Change

describeSchemaType now checks "items" in schema before the generic "type" in schema branch, and returns "array of <itemType>" (e.g. "array of string", "array of object") instead of plain "array". The "missing required field" message in repair prompts becomes:

$.commandsToTry: missing required field (expected array of string)

This is a small, non-breaking internal change — the function is not exported and only affects diagnostic strings in AgentError.message and repair prompts.

Tests

Two tests in src/rig.test.ts were updated/added to cover array of string and array of number in the missing-field error message.

Generated by Daily Rig Sampler · sonnet46 84.1 AIC · ⌖ 8.23 AIC · ⊞ 5.4K ·

When a required array field is missing, the validation error now reads
'missing required field (expected array of string)' instead of
'missing required field (expected array)'. This makes repair prompts
more precise and helps the model produce the correct item type on retry.

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

github-actions Bot commented Jul 25, 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 — approving with one minor suggestion on test completeness.

📋 Key Themes & Highlights

Positive Highlights

  • ✅ Root cause correctly addressed: "items" in schema now checked before "type" in schema, so ArraySchema (which carries both) reaches the richer branch first
  • ✅ Two new tests pin the changed behaviour for array of string and array of number
  • ✅ Non-breaking internal change — describeSchemaType is not exported

Minor Gap

  • The PR description mentions "array of object" as a new output, but no test covers it. Low risk since describeSchemaType falls through to the "properties" in schema branch, but a test would make the guarantee explicit.

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

Comment thread src/rig.test.ts
expect(result.error.message).toContain("$.scores");
expect(result.error.message).toContain("array of number");
}
});

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.

[/tdd] No test covers array of object — the PR description mentions it as an example output but there is no corresponding test case.

💡 Suggested addition
it("reports a missing required array-of-objects field with item type 'array of object'", () => {
  const schema = s.object({ tags: s.array(s.object({ id: s.string })), name: s.string });
  const result = analyzeResponse(JSON.stringify({ name: "x" }), schema, "test", 1);
  expect(result.ok).toBe(false);
  if (!result.ok) {
    expect(result.error.message).toContain("array of object");
  }
});

The PR body explicitly cites "array of object" as a newly supported description, so a test pinning that output would complete the coverage story.

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