[rig-tasks] Add 10 rig samples — 2026-07-25 - #114
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 — requesting changes primarily for duplicate samples and a correctness issue in sample 95.
📋 Key Themes & Highlights
Key Issues
- Duplicate samples (samples 90 and 91):
90-workspace-config-drift.mdis verbatim identical to82-workspace-config-drift.md, and91-commit-format-suggester.mdis near-identical to83-commit-format-suggester.md. The sample library's value comes from showing distinct patterns — duplicates undermine that. - Correctness gap in sample 95:
parseWorktreePorcelaincan only return"locked","bare", or"clean"— the"dirty"enum value in the output schema is unreachable. The LLM is instructed to detect dirty worktrees but has no tool mechanism to do so. - Unused parameter in sample 97:
nameis declared inclassifyDependency's parameters but never destructured in the handler, making the schema misleading. steering()with no message in sample 91: A no-op addon that wastes a turn.
Positive Highlights
- ✅ Samples 88, 92, 93, 94, 95, 96 cover genuinely new patterns (
s.record(s.object(...)), typed tools with threshold validation, asyncdefineToolwithnode:child_process, porcelain parsing) - ✅ Consistent use of
model: "small"andmaxTurnswhere appropriate - ✅ Good use of
s.optional(s.string)for the branch field in sample 95 - ✅ All 10 samples pass typecheck
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 48.2 AIC · ⌖ 4.81 AIC · ⊞ 6.3K
Comment /matt to run again
| @@ -0,0 +1,34 @@ | |||
| # 90 - Workspace Config Drift | |||
There was a problem hiding this comment.
[/codebase-design] This sample is a verbatim duplicate of 82-workspace-config-drift.md — same parseJson tool, same agent config, same instructions, same output schema, and same repair() addon. It adds no new pattern to the sample library.
💡 Suggestion
Either delete this and replace with a genuinely distinct sample, or differentiate it meaningfully (e.g. add steering() guidance, extend to YAML configs, or add a maxTurns rationale comment).
Duplicate samples dilute the sample library and confuse users learning by example.
| @@ -0,0 +1,22 @@ | |||
| # 91 - Commit Format Suggester | |||
There was a problem hiding this comment.
[/codebase-design] This sample is a near-verbatim duplicate of 83-commit-format-suggester.md — same instructions, same output schema, same [steering(), repair()] addon combo, same maxTurns: 5. The import order differs (repair, steering vs steering, repair) but produces identical runtime behaviour.
💡 Suggestion
Replace this with a distinct sample that covers a new concept not already in the library. If the intent was to refine the commit-format pattern, update 83 in place and document what changed.
| inDevDependencies: s.boolean, | ||
| inPeerDependencies: s.boolean, | ||
| }), | ||
| handler({ inDependencies, inDevDependencies, inPeerDependencies }) { |
There was a problem hiding this comment.
[/codebase-design] The name parameter is declared in the tool's parameters schema but never used in the handler body — only inDependencies, inDevDependencies, and inPeerDependencies are destructured.
💡 Fix
handler({ name, inDependencies, inDevDependencies, inPeerDependencies }) {
// use name if you want to log or return it, otherwise remove it from parameters
}Either remove name from parameters (since the handler doesn't use it), or destructure and use it in the return value so the LLM can correlate the classification back to a package name. As-is, the tool silently discards an input the LLM is instructed to pass.
| category: s.enum("feat", "fix", "chore", "docs", "test", "refactor", "style"), | ||
| })), | ||
| maxTurns: 5, | ||
| addons: [steering(), repair()], |
There was a problem hiding this comment.
[/codebase-design] steering() is called with no message argument, making it a no-op stub — the steering addon needs a message to steer the model toward a goal.
💡 Fix
Either remove steering() entirely, or provide a meaningful message like the existing sample 76-commit-msg-rewriter.md does:
addons: [steering({ message: "Ensure each suggested commit message uses conventional commit format: type(scope): description." }), repair()],A steering() with no message wastes a turn without providing guidance.
| path: s.string, | ||
| branch: s.optional(s.string), | ||
| state: s.enum("locked", "bare", "clean", "dirty"), | ||
| })), |
There was a problem hiding this comment.
[/codebase-design] The output schema declares state: s.enum("locked", "bare", "clean", "dirty") including "dirty", but the parseWorktreePorcelain tool never sets state to "dirty" — it only checks for locked and bare flags, defaulting to "clean". The instructions say "Determine the state ... dirty if there are uncommitted changes" but the tool returns before any dirty-check can happen.
💡 Fix
Either:
- Remove
"dirty"from the enum and the instructions (since the tool can't detect it), or - Extend the tool to run
git status --porcelain "<path>"inside the handler and set state to"dirty"when it returns output.
As-is, the LLM is told to classify dirty worktrees but has no tool mechanism to detect them — it will guess.
| const markdownDocSummarizer = agent({ | ||
| model: "small", | ||
| instructions: p`Read the README: ${p.readOptional("README.md", "No README found.")}. Identify each top-level heading (##) and its content. Delegate summarization of each section to the sectionSummarizer agent. Compile all summaries into a report and write it to summaries/README-summary.md via ${p.writeOutput("reportPath", "summaries/README-summary.md")}.`, | ||
| output: s.object({ |
There was a problem hiding this comment.
[/codebase-design] p.readOptional is used here, but README.md is inlined directly into the main agent's instructions. This means the entire file content is injected into the prompt at construction time — not at delegation time — which could balloon token usage for large READMEs before the agent even begins section extraction.
💡 Suggestion
Consider passing README.md as input to the orchestrator agent instead, so the caller controls what document is summarized and the sample is more reusable:
input: s.object({ docPath: s.string }),
instructions: p`Read the document: ${p.readInput("docPath")}. Identify each top-level heading (##)...`,This also makes the sample demonstrate p.readInput — a distinct pattern not yet shown by surrounding samples.
| // Agent role: extract and classify direct dependencies from package.json, then describe the overall tree shape. | ||
| const pkgDependencyGraph = agent({ | ||
| model: "small", | ||
| instructions: p`Read the project manifest: ${p.read("package.json")}. Get the resolved dependency tree using ${p.bash("npm ls --json --depth=1 2>/dev/null || echo '{}'")}. Use the classifyDependency tool to classify each direct dependency as runtime, dev, or peer. Also list all devDependency names. Estimate the treeShape as flat (<5 deps), shallow (5–20), or deep (>20) and set depthScore to the total direct dependency count.`, |
There was a problem hiding this comment.
[/codebase-design] The classifyDependency tool returns a plain string ("peer", "dev", or "runtime"), but the output schema wraps each dep in s.object({ name, version, type }). The tool result only provides type — name and version must come entirely from the LLM's recall of the npm ls output. If the LLM hallucinates or drops a package, the tool gives no corrective signal.
💡 Suggestion
Return a richer object from the tool so the LLM can use it to build the output without needing to remember package names independently:
handler({ name, inDependencies, inDevDependencies, inPeerDependencies }) {
const type = inPeerDependencies ? "peer" : inDevDependencies ? "dev" : inDependencies ? "runtime" : "dev";
return { name, type };
}This makes the tool a reliable anchor rather than a pure classification oracle.
1 participant
Summary
Added 10 new rig sample files to
skills/rig/samples/.Typecheck failures
No failures — all 10 samples passed typecheck on the first attempt.
Tasks run