Make sample 318 stdin-runnable for the daily Rig sampler - #303
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@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 — no blocking issues, but two readability suggestions.
📋 Key Themes & Highlights
Key Themes
- Readability regression: the instructions, tool definition, and output schema were compressed into one-liners. For a sample file that exists to teach, this is a net negative — multi-line form is the convention throughout the rest of the sample directory.
- Root contract fix is correct: removing
inputand switching top.writeOutputwith a fixed temp path is exactly the right way to satisfy the stdin-launcher contract.
Positive Highlights
- ✅ Root cause correctly identified and fixed (input-free root required by stdin launcher)
- ✅
barrelContentadded to output so the generated barrel is both returned and written - ✅
p.writeInput→p.writeOutputswitch is semantically accurate - ✅
repair()addon retained
Not Addressed (out of scope, but worth noting)
- No regression test guards against the sampler failure recurring — this is an existing gap in the test suite, not introduced by this PR.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 29.5 AIC · ⌖ 4.28 AIC · ⊞ 6.3K
Comment /matt to run again
| totalExports: s.int, | ||
| filesScanned: s.int, | ||
| }), | ||
| instructions: p`Generate a TypeScript barrel for skills/rig/engines from ${p.bash("find skills/rig/engines -name '*.ts' -not -name 'index.ts' -not -name '*.d.ts' | head -20")}. Use extractExportedSymbols for each file, set outputFile to /tmp/rig-barrel.ts, write barrelContent to ${p.writeOutput("barrelContent", "/tmp/rig-barrel.ts")}, and return the declared output.`, |
There was a problem hiding this comment.
[/codebase-design] The instructions are now a single dense line, sacrificing the readability that made the original multi-line template literal easy to scan.
💡 Suggested refactor
Split the template literal across lines:
instructions: p`Generate a TypeScript barrel for skills/rig/engines
from ${p.bash("find skills/rig/engines -name '*.ts' -not -name 'index.ts' -not -name '*.d.ts' | head -20")}.
Use extractExportedSymbols for each file, set outputFile to /tmp/rig-barrel.ts,
write barrelContent to ${p.writeOutput("barrelContent", "/tmp/rig-barrel.ts")},
and return the declared output.`,This matches the style of other samples and keeps each prompt step scannable at a glance.
| const symbols = [...content.matchAll(/^export\s+(?:(?:default\s+)?(?:function|class|const|let|var|type|interface|enum)\s+(\w+)|(\{[^}]+\}))/gm)].flatMap((match) => match[1] ? [match[1]] : match[2]!.replace(/[{}]/g, "").split(",").map((name) => name.trim().split(/\s+as\s+/)[0].trim()).filter(Boolean)); | ||
| return { filePath, symbols }; | ||
| }, | ||
| })], |
There was a problem hiding this comment.
[/codebase-design] The tool definition and output schema are now one-liners, which keeps file size down but harms readability in a sample whose purpose is to teach. Samples are documentation first.
💡 Suggested refactor
Expand the output schema and tool definition to multi-line form, consistent with every other sample in the directory:
output: s.object({
barrelContent: s.string,
exports: s.array(s.string),
outputFile: s.path,
totalExports: s.int,
filesScanned: s.int,
}),Similarly, break the tool handler body out of the one-liner .flatMap chain for legibility.
The daily Rig sample report failed on
318-ts-barrel-re-exporter.mdbecause the exported root agent required structured input, while markdown samples are executed through the stdin launcher and must expose an input-free root. This updates the sample to match the launcher contract without changing runtime behavior.Root contract
318-ts-barrel-re-exporter.mdSample behavior
skills/rig/enginesp.writeInput(...)to a fixed temp path viap.writeOutput(...)barrelContentto the declared output so the generated barrel is both returned and writtenSample shape