[rig-tasks] Add 10 rig samples — 2026-08-03 - #343
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design — commenting with non-blocking observations.
📋 Key Themes & Highlights
Issues Found
- Dead
inputparameter (369):rootDiris declared in the input schema butp.bashhardcodesfind . ...— the value is never threaded through. - Redundant
addons: [](362, 363, 365): The field is optional; an empty array is noise that contradicts the idiomatic style of other samples. - Magic constant in job-count heuristic (365):
Math.max(1, jobMatches.length - 3)subtracts 3 with no explanation, which reads as a bug to learners. - Regex limitations not called out (368): The GraphQL type extractor's single-line regex silently misses types with nested braces or multiline bodies.
- Unbounded
linesoutput (370): Emitting all log lines (includinginfo) into the output array risks context exhaustion on real CI logs.
Positive Highlights
- ✅ Consistent use of
s.optionalin tool parameters (367) is a nice pattern for nullable lockfile fields. - ✅
repair()addon is correctly applied on agents whose output schemas are complex records (367, 369, 370). - ✅ All 10 samples typecheck cleanly on first attempt — a good signal of generation quality.
- ✅ Good variety of patterns:
s.recordroot output (368),p.readInput(363, 370), binary sampling vianode:fs/promises(366).
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 48.3 AIC · ⌖ 4.46 AIC · ⊞ 6.3K
Comment /matt to run again
| instructions: p`Analyze the directory structure of the given root path. | ||
|
|
||
| Directories found (up to depth 3): | ||
| ${p.bash("find . -maxdepth 3 -type d -not -path '*/node_modules/*' -not -path '*/.git/*' 2>/dev/null | sort || echo ''")} |
There was a problem hiding this comment.
[/codebase-design] rootDir input is declared but the p.bash command ignores it — find . ... always scans CWD regardless of what the caller passes.
This makes the input field dead code and will surprise users who pass a non-default root path.
💡 Suggestion
Use p.bash with a shell variable or a separate p.readInput approach, or remove the input field if the agent is always expected to run in the target directory. For example, thread the value through:
instructions: p`...
Directories found:
${p.bash(`find "${input.rootDir}" -maxdepth 3 ...`)}`,| }), | ||
| tools: [checkLicenseHeader], | ||
| maxTurns: 8, | ||
| addons: [], |
There was a problem hiding this comment.
[/codebase-design] addons: [] is redundant noise — the field is optional and an empty array has no effect. Most other samples in this repo simply omit it.
Remove the key to keep the sample canonical and uncluttered.
| try { | ||
| const content = await readFile(filePath, "utf8"); | ||
| const jobMatches = content.match(/^\s{0,2}[\w-]+:\s*$/gm) ?? []; | ||
| const jobCount = Math.max(1, jobMatches.length - 3); |
There was a problem hiding this comment.
[/codebase-design] Math.max(1, jobMatches.length - 3) is an unexplained magic constant — subtracting 3 is not documented and will confuse readers learning from this sample.
The regex /^\s{0,2}[\w-]+:\s*$/gm matches any top-level YAML key (including on:, name:, env:), not just job entries, hence the correction. Consider a more targeted match like /^ [\w-]+:/gm (two-space indent) or at least add an inline comment explaining why 3 is subtracted.
| handler: async ({ filePath }: { filePath: string }) => { | ||
| try { | ||
| const content = await readFile(filePath, "utf8"); | ||
| const typeRegex = /(type|input|enum|interface|union)\s+(\w+)[^{]*\{([^}]*)\}/g; |
There was a problem hiding this comment.
[/codebase-design] The regex [^{]*\{([^}]*)\} fails on types with nested braces or multi-line bodies — it will miss fields in any type whose body contains { (e.g. field default values like field: String = "{").
This is a known limitation worth calling out in the sample description so readers know to use a proper GraphQL parser for production use.
💡 Suggestion
Add a comment like:
// Note: regex-based extraction; does not handle nested braces or multi-line type bodies.
// For production use, prefer a real GraphQL parser (e.g. graphql-js).| 4. Count errorCount (severity="error") and warningCount (severity="warning"). | ||
| 5. Set dominantError to the most common errorClass among error-severity lines, or omit if none.`, | ||
| output: s.object({ | ||
| lines: s.array(s.object({ |
There was a problem hiding this comment.
[/codebase-design] The lines output includes every log line regardless of severity — for large CI logs this can produce an enormous output array that exhausts the model's context or causes schema validation failures.
Consider filtering output to only error/warning lines, or adding a head -500 cap on the p.readInput step and noting the truncation in the description.
1 participant
Summary
Added 10 new rig sample files to
skills/rig/samples/.Typecheck failures
None — all 10 tasks passed typecheck on the first attempt.
Tasks run