[rig-tasks] Add 10 rig samples — 2026-07-25 - #122
Conversation
Samples added: - 110: workspace-config-drift-3 (config drift w/ repair addon) - 111: conventional-commit-suggester (steering+repair addons) - 112: runtime-env-health (p.bash + defineTool thresholds) - 113: git-hotspot-analyzer (steering addon, s.record output) - 114: loc-statistics-gatherer (defineTool complexity classifier) - 115: import-cycle-detector (repair addon, s.boolean+s.array) - 116: barrel-file-generator (async defineTool, p.write) - 117: git-hook-inventory (defineTool classification, s.record) - 118: pr-review-checklist (multi-enum output, repair addon) - 119: source-map-analyzer (async defineTool with fs/promises) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design — four correctness issues found across the new samples. Overall batch is well-structured; the patterns (steering + repair, defineTool with async handlers, s.record root output) are good teaching examples.
📋 Key Themes & Highlights
Issues Found
- 113:
\${input.topN}is a backslash-escaped literal — thetopNinput value is never injected into the prompt; the model always sees the string${input.topN}. - 119: Tool handler returns
{ density }but the output schema declaresmappingDensity— the LLM must bridge this key mismatch itself. - 116: Instructions say "Write each barrel file using p.write" but no
p.writeappears in the agent;barrelFilesWrittenimplies writes that never happen. - 117:
classifyHookhandler omitssummaryfrom its return value, yetsummaryis required in the output schema — the LLM must fabricate it every time.
Positive Highlights
- ✅ Good variety of
defineToolpatterns (sync, async, withnode:fs/promisesandnode:child_process) - ✅ Appropriate use of
repair()on agents with complex outputs (110, 111, 115, 118) - ✅ Consistent use of
s.recordas root output shape for per-file analysis agents (113, 117, 119) - ✅
p.readInputused correctly in 110 for dynamic file path from input
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 36.8 AIC · ⌖ 4.51 AIC · ⊞ 6.3K
Comment /matt to run again
| Select the top \${input.topN} files by churn score. For each file compute a churnScore | ||
| (number of commits) and list topContributors. Return only the declared output as a record | ||
| keyed by file path.`, | ||
| output: s.record( |
There was a problem hiding this comment.
[/codebase-design] \${input.topN} renders as a literal string in the prompt — the caller-supplied topN limit is never injected into the model's instructions, so it is silently ignored.
💡 Suggested fix
Inject the value through the steering message or via an s.int description. Since topN is a number (not a path), p.readInput won't work here — instead, include it explicitly in the steering addon message or in the agent description:
addons: steering({ message: `Focus only on the top ${input.topN} files by change frequency.` }),Or pass topN through the instructions string by having the harness resolve it, if that pattern is supported by the runtime. Until then the backslash escape means the model always sees the literal text ${input.topN} rather than e.g. 5.
| const mappingLen: number = typeof map.mappings === "string" ? map.mappings.length : 0; | ||
| const density = mappingLen < 100 ? "sparse" : mappingLen < 1000 ? "medium" : "dense"; | ||
| const hasSourcesContent = Array.isArray(map.sourcesContent) && map.sourcesContent.length > 0; | ||
| return { sourceCount, density, hasSourcesContent }; |
There was a problem hiding this comment.
[/codebase-design] The tool handler returns { density } but the output schema field is named mappingDensity. This key mismatch means the LLM must bridge the gap itself rather than the tool result mapping cleanly into the declared output shape.
💡 Suggested fix
Rename the returned key to match the output schema:
return { sourceCount, mappingDensity: density, hasSourcesContent };
// and in the error branch:
return { sourceCount: 0, mappingDensity: "sparse", hasSourcesContent: false };Aligning tool return keys with the output schema reduces ambiguity and makes the sample a cleaner teaching example.
| ${p.bash("find . -name '*.ts' ! -name 'index.ts' ! -name '*.test.ts' ! -name '*.spec.ts' -not -path '*/node_modules/*' 2>/dev/null | head -100")} | ||
|
|
||
| Use the classifyExports tool to identify exported symbols in files. Then generate barrel | ||
| content for each directory that has exported symbols. Write each barrel file using p.write. |
There was a problem hiding this comment.
[/codebase-design] The instructions reference p.write ("Write each barrel file using p.write") but no p.write call appears in the agent definition. The barrelFilesWritten output field implies actual writes, but nothing will be written — the agent can only report what it would write.
💡 Suggested fix
Either:
- Add a
p.writeintent to actually write barrel files (if the runtime supports it for this use case), or - Rename
barrelFilesWrittentobarrelFilesToWriteand update the instructions to say "report the barrel content" rather than "write" — making it a dry-run analyser, consistent with what the agent actually does.
As a sample, misleading prose about side effects can confuse users trying to learn from the pattern.
| const isAsync = content.includes(" &") || content.includes("async"); | ||
| return { status: isSample ? "stub" : "active", isAsync }; | ||
| }, | ||
| }), |
There was a problem hiding this comment.
[/codebase-design] The classifyHook handler declares name as a required parameter but never uses it — the returned object also omits summary, which is a required field in the output schema. The LLM must invent a summary value since the tool never provides one.
💡 Suggested fix
Either include summary in the handler's return value or remove it from the output schema. For example:
handler({ name, content }) {
if (!content || content === "missing") return { status: "missing", isAsync: false, summary: "not installed" };
const isSample = content.includes("sample") || content.trim() === "#!/bin/sh";
const isAsync = content.includes(" &") || content.includes("async");
return { status: isSample ? "stub" : "active", isAsync, summary: isSample ? "sample hook" : name };
}
Summary
Added 10 new rig sample files to
skills/rig/samples/.Typecheck failures
No typecheck failures this run. All 10 programs passed on the first attempt.
Tasks run