[rig-tasks] Add 10 rig samples — 2026-08-05 - #356
Conversation
Samples: npm-lifecycle-script-analyzer, ts-jsdoc-coverage-checker, git-hook-installer, ts-type-guard-generator, json-fixture-anonymizer, csv-to-markdown-table, ts-interface-method-counter, git-tag-annotation-extractor, jsonl-file-analyzer, npm-peer-dep-conflict-checker All 10/10 typecheck passed. 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 /grill-with-docs — requesting changes on API misuse and a correctness bug.
📋 Key Themes & Findings
Blocking Issues
- Missing
p.writeInputin 3 samples (374, 375, 376): Each has anoutputFileinput and a generated-content output field, but no write intent in the template. The samples imply file persistence but silently omit it — readers will copy the pattern and get no file on disk. p.writereferenced in prose, not as an intent (373): The instructions text says to "write ... using p.write" but nop.write(...)call is embedded in theptemplate. The hooks will never be written to disk.- Semver parsing bug in 380:
.replace(/[^0-9]/, "")strips only the first non-digit, turning"18.2.0"into182, not18. All major-version comparisons against real package versions will produce spurious conflicts.
Positive Highlights
- ✅ Good variety of patterns across the batch: async tool handlers,
s.record/s.enumschemas,repair()andsteering()addons,p.glob,p.readInput - ✅
s.int,s.path,s.optionalused consistently and appropriately - ✅ All 10 samples pass typecheck per the PR description
- ✅ Tool handlers are deterministic and well-scoped;
parametersschemas are explicit
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 56.7 AIC · ⌖ 7.4 AIC · ⊞ 6.3K
Comment /matt to run again
| } | ||
| // Simple semver major check | ||
| const expectedMajor = parseInt(expectedRange.replace(/[^0-9]/, ""), 10); | ||
| const foundMajor = parseInt(foundVersion.replace(/[^0-9]/, ""), 10); |
There was a problem hiding this comment.
[/grill-with-docs] foundVersion.replace(/[^0-9]/, "") strips only the first non-digit, so "18.2.0" becomes "182.0" → parseInt → 182, not 18. This makes the major-version comparison produce false conflicts for virtually all real version strings.
💡 Fix: use `.match(/\d+/)` instead
const expectedMajor = parseInt(expectedRange.match(/\d+/)?.[0] ?? "", 10);
const foundMajor = parseInt(foundVersion.match(/\d+/)?.[0] ?? "", 10);This correctly extracts 18 from both "^18.0.0"and"18.2.0"`.
| instructions: p`Install git hooks into .git/hooks from the provided input. | ||
|
|
||
| First call checkHooksDir to verify the hooks directory is accessible. | ||
| For each hook in input.hooks, write the script to .git/hooks/<name> using p.write. |
There was a problem hiding this comment.
[/grill-with-docs] The instruction text says "write the script to .git/hooks/<name> using p.write" but no p.write(...) intent is embedded in the p...`` template. The LLM will see prose mentioning p.write but won't receive the actual file-write instruction that the intent generates — hooks will not be persisted to disk.
💡 Fix
Embed the intent in the template or extend the checkHooksDir tool to also perform the write. For a static-path demonstration:
instructions: p`Check hooks dir: ${p.write(".git/hooks/pre-commit", script)}`For dynamic paths, p.writeInput resolves a path from an input field — use one field per hook, or rethink the approach to use a tool-based write instead.
| tools: [extractInterfaces], | ||
| output: s.object({ | ||
| generatedGuards: s.array(s.string), | ||
| outputFile: s.path, |
There was a problem hiding this comment.
[/grill-with-docs] outputFile appears in both the input and output schemas but no p.writeInput or p.writeOutput intent is present in the instructions template — the generated source is captured in generatedSource but never written to disk. This sample appears to demonstrate file persistence but silently omits it.
💡 Fix: add p.writeInput to persist the output
Add the intent to the instructions template:
instructions: p`Generate TypeScript type guards...
Source file:
${p.readInput("sourceFile")}
${p.writeInput("outputFile", "generatedSource")}
...`p.writeInput("outputFile", "generatedSource") writes the generatedSource output field to the path supplied in input.outputFile after the model produces valid output. Without it, callers must write the file themselves — which the sample doesn't document.
| output: s.object({ | ||
| fieldsAnonymized: s.int, | ||
| totalRecords: s.int, | ||
| outputPath: s.path, |
There was a problem hiding this comment.
[/grill-with-docs] Same missing-write pattern as 374: outputFile is in the input and outputPath echoes it in the output, but the anonymized JSON in result is never written to disk — no p.writeInput intent is in the template. The sample implies file-level output but doesn't deliver it.
💡 Fix
Add ${p.writeInput("outputFile", "result")} to the instructions template so the anonymized JSON is persisted after generation. Drop outputPath from the output schema (it adds nothing if the write intent already handles it).
| output: s.object({ | ||
| rowCount: s.int, | ||
| columnCount: s.int, | ||
| outputFile: s.path, |
There was a problem hiding this comment.
[/grill-with-docs] outputFile is in both input and output schemas but the generated markdownTable is never written to disk — no p.writeInput in the template. Three samples in this batch (374, 375, 376) share this same pattern; consider it a systematic gap.
💡 Fix
Add ${p.writeInput("outputFile", "markdownTable")} to the instructions template. This is the idiomatic way to persist LLM-generated content to a caller-supplied path in Rig.
1 participant
Summary
Added 10 new rig sample files to
skills/rig/samples/.Typecheck failures
No failures — all 10 programs passed typecheck.
Tasks run