[rig-sampler] fix(validation): improve minLength error message to include actual string length

[github-actions]

Samples Run

The following five samples were executed via RIG_SAMPLE=N npm run sample (stub Copilot SDK):

#	File	Result	Notes
01	01-single-agent-haiku.ts	✅ stub ran	Single string output, 1 turn, no repair needed
02	02-review-git-diff.ts	✅ stub ran	Structured object output with findings array, 2 ask events (initial + sub-agent), no repair
03	03-diagnose-test-failure.ts	✅ stub ran	Object with enum fields (risk, severity), 2 ask events, no repair
04	04-generate-readme.ts	✅ stub ran	Object with rootCause, confidence (number), arrays; 2 ask events
05	05-write-readme-intent.ts	✅ stub ran	Object with path constrained to s.literal("README.md"), contents string; uses p.read + p.bash intents

Analysis

• Repair turns: No repair loops were triggered in any sample during the stub run. All samples produced schema-conforming output on the first turn.
• Schema fit: Samples 03 and 04 use s.enum for risk/severity fields — appropriate and tight. Sample 05 uses s.literal("README.md") for path — good use of a single-value constraint.
• Error message clarity: During analysis of the validation code, a misleading error message was found in validateSchema. When a string value fails the minLength check, the error always reported "got empty string" — even when the actual value was non-empty but shorter than the minimum length. In a repair loop, this confuses the model: it may be told the string was empty when it actually provided a short non-empty value.

Change

In skills/rig/rig.ts, the minLength validation branch now distinguishes:

• An empty string: "got empty string" (unchanged behavior for the common case)
• A non-empty but too-short string: "got string of length M" (new, more accurate message)

This improves repair prompt quality when s.nonEmptyString or a custom minLength constraint rejects a short value — the model now sees the actual length rather than a misleading "empty string" label.

A new test case in src/rig.test.ts covers the non-empty too-short path alongside the existing empty-string test.

Generated by Daily Rig Sampler · sonnet46 87 AIC · ⌖ 8.22 AIC · ⊞ 5.4K · ◷

[github-actions]

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

[github-actions]

Skills-Based Review 🧠

Applied /tdd — one observation on the new test coverage; no blocking issues.

📋 Key Themes & Highlights

Key Themes

• Test type-safety: The new test for non-empty too-short strings uses a type cast (as ReturnType<typeof s.string>) to inject a raw schema object that the public API can't produce. The comment inside the test acknowledges this. The test works, but it validates an unreachable code path from the caller's perspective — worth either exposing minLength as a proper s.string option or testing validateSchema directly.

Positive Highlights

• ✅ Fix in rig.ts is minimal, correct, and well-scoped: one conditional, no side effects
• ✅ Existing empty-string test updated to assert on the specific message token ("empty string"), tightening coverage
• ✅ PR description clearly explains the motivation (repair-loop confusion) and the before/after behaviour
• ✅ The two-branch ternary (value.length === 0 ? ... : ...) reads cleanly and matches the intent exactly

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 32.7 AIC · ⌖ 7.04 AIC · ⊞ 6.3K
_{Comment /matt to run again}

[github-actions]

[/tdd] The test uses minLen5Schema as ReturnType<typeof s.string> to force a raw schema object past TypeScript — a type-unsound workaround that the inline comment itself flags as a limitation.

If s.string does not support a minLength argument through the public API, that gap is worth addressing. Using a type cast here means the test validates behaviour that callers cannot legally trigger through the public API.

💡 Cleaner alternatives

Option A — expose a minLength option on s.string (or a s.string({ minLength: N }) overload) so the test is type-safe:

const schema = s.object({ code: s.string({ minLength: 5 }) });

Option B — if this path is intentionally internal, test validateSchema directly (if exported) without going through analyzeResponse with a cast:

const result = validateSchema("ab", { type: "string", minLength: 5 }, "code", false);
expect(result).toEqual({ ok: false, error: "code: expected string with minLength 5, got string of length 2" });

Either option removes the cast, keeps the test type-safe, and makes the test read as a specification of real, reachable behaviour.