[rig-tasks] Add 10 rig samples — 2026-08-02 - #336
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 /grill-with-docs — requesting changes on three correctness issues in the new samples.
📋 Key Themes & Highlights
Issues Found
- 357: Decorator regex
@([A-Z][a-zA-Z0-9]*)silently misses all lowercase decorators (@inject,@override, etc.) — a silent correctness bug in the tool handler. - 355:
changedKeysinstruction says "estimate from content" — the tooling only extracts keys (not values), so this field will consistently be unreliable. The schema should either be fixed or the limitation documented. - 360:
staleCountinbranchHealthAgentcounts remote-tracking refs, not stale-by-age branches. This corrupts the downstreamoverallHealthclassifier, which usesstaleCount >= 3as a threshold — any repo with 3+ remotes will be misclassified.
Positive Highlights
- ✅ Clean, consistent agent structure across all 10 samples
- ✅ Good use of
repair()addon throughout - ✅ Sample 360 is a great demonstration of the
workflow+Promise.all+call.jsonpattern - ✅
defineToolhandlers keep logic deterministic and out of the LLM path where possible
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 40.7 AIC · ⌖ 4.48 AIC · ⊞ 6.3K
Comment /matt to run again
| handler: async ({ filePath }: { filePath: string }) => { | ||
| try { | ||
| const content = await readFile(filePath, "utf-8"); | ||
| const decoratorRegex = /@([A-Z][a-zA-Z0-9]*)/g; |
There was a problem hiding this comment.
[/grill-with-docs] The regex @([A-Z][a-zA-Z0-9]*) silently skips lowercase decorators — e.g. @override, @inject, @memoize, @autobind — which are common in Angular, NestJS, and reflection-metadata usage. This produces silent false-negatives with no indication anything was missed.
💡 Suggested fix
const decoratorRegex = /@([a-zA-Z][a-zA-Z0-9]*)/g;Lowercase decorators (e.g. @inject, @readonly) are a standard TypeScript pattern; the current pattern excludes them entirely without any warning in the output.
| 1. Call extractYamlKeys on the base file content to get baseKeys. | ||
| 2. Call extractYamlKeys on the target file content to get targetKeys. | ||
| 3. Call diffKeys with baseKeys and targetKeys to get addedKeys and removedKeys. | ||
| 4. changedKeys: keys present in both but with differing values — estimate from content or leave empty. |
There was a problem hiding this comment.
[/grill-with-docs] The instruction "estimate from content or leave empty" gives the LLM no reliable way to detect value changes — YAML parsing is needed to compare values, not text estimation. Consumers of this agent may assume changedKeys is authoritative when it will frequently be empty or incorrect.
💡 Suggested fix
Since extractYamlKeys only returns keys (not values), the agent cannot reliably detect changed values. Either:
- Remove
changedKeysfrom the output schema and note the limitation in the description, or - Add a
extractYamlKeyValuestool that returns aRecord<string, string>and diffs values explicitly.
Leaving the instruction as "estimate" teaches a bad pattern for agent samples.
| Branch list: | ||
| ${p.bash("git branch -a 2>/dev/null || echo ''")} | ||
|
|
||
| Count totalBranches (all branches listed). List activeBranches as local branch names (lines not starting with "remotes/"). |
There was a problem hiding this comment.
[/grill-with-docs] staleCount = totalBranches - activeBranches.length misuses the term "stale" — branches on remotes/ are not stale, they're remote-tracking refs. This conflates "remote" with "stale", which makes the output misleading and is inconsistent with how sample 353 (stale-branch-detector) defines staleness by commit age.
💡 Suggested fix
Rename staleCount to remoteCount and activeBranches to localBranches to accurately reflect what's being counted:
output: s.object({
totalBranches: s.int,
remoteCount: s.int,
localBranches: s.array(s.string),
}),This keeps vocabulary consistent with sample 353, where "stale" is specifically defined by commit date age.
| phase("Synthesize"); | ||
| const overallHealth = await call.json( | ||
| `Given branchHealth=${JSON.stringify(branchHealth)} and commitFrequency=${JSON.stringify(commitFrequency)}, | ||
| classify overallHealth as "healthy" (averagePerDay >= 1 and staleCount < 3), |
There was a problem hiding this comment.
[/grill-with-docs] The overallHealth thresholds reference staleCount from branchHealth, but that field actually counts remote-tracking refs (not stale-by-age branches). A repo with 10 remotes always classifies as "critical" regardless of actual staleness.
💡 Why this matters
This is a downstream consequence of the staleCount naming in branchHealthAgent. Using a raw remote-branch count as a staleness signal in the health classifier produces unreliable results in any real repo with forks or CI branches. Fix by renaming the field to remoteCount (per the other comment) and removing it from the health thresholds, or replacing it with an actual age-based stale count.
1 participant
Summary
Added 10 new rig sample files to
skills/rig/samples/.Typecheck failures
None — all 10 tasks passed typecheck. Task 10 (parallel branch analysis workflow) required one fix: the initial version incorrectly used
parallel([promise, promise])instead ofPromise.all([...]), and tried to importcallfrom"rig"(it is only available as a workflow body context prop). Both issues were corrected before writing the sample file.Tasks run