chore(agents): drop testing-agent and testing-reviewer - #323
Merged
Conversation
Neither earned its keep: both carried evidence:'none' in the benchmark catalog with no suite behind them, and neither is wired into the review gate's reviewer registry, so nothing measured or invoked them automatically. Removes the canonical pair plus the .claude/.cursor/dist copies, their two agents-manifest entries, and their two catalog subjects; dashboards re-rendered (shipped-agents-without-evidence drops 4 -> 2). The CLI tests used testing-agent.md as a stand-in for 'a devkit-owned agent' in six places (conflict detection, init surface assertions, the codex TOML output path). Deleting it would have left those assertions vacuous rather than red, so they now use correctness-reviewer.md — a permanent agent — and still test the same behaviour. The 'testing' skill is left in place: it is invocable on its own and does not depend on either agent. Known limitation, unchanged by this commit: sync-agents has no prune path, so a consumer that already installed these keeps the files until they are removed by hand. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (12)
💤 Files with no reviewable changes (9)
📝 WalkthroughWalkthroughThe pull request removes testing-agent and testing-reviewer definitions, manifest entries, and benchmark catalog entries. It updates README coverage data and CLI tests to reference correctness-reviewer assets. ChangesTesting agent removal
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes
testing-agentandtesting-reviewer— owner call: not needed.Neither was earning its keep. Both carried
evidence: "none"in the benchmark catalog with no suite behind them, and neither appears in the review gate's reviewer registry (gate-engine/review/reviewers.mts), so nothing measured them and nothing invoked them automatically. They were manually-invocable subagents only.What changed
agents/) plus the installed.claude/,.cursor/, anddist/copies — 8 files..devkit/agents-manifest.jsonentries. The manifest is exactly consistent afterward: 11 files, 11 entries, nothing missing or orphaned.docs/benchmarks/catalog.jsonsubjects and re-rendered the dashboards — "shipped agents with no benchmark evidence" goes 4 → 2.The test fixtures needed care
The CLI tests used
testing-agent.mdin six places as a stand-in for "a devkit-owned agent", not because they were testing anything about a testing agent: conflict detection (detectAgentConflicts), the non-devkit-collision preserve/--forcepath, the init surface assertions, and the codex TOML output path.Deleting the file would have left those assertions vacuous rather than red —
detectAgentConflictsonly reports a path it owns, so a seeded file at a no-longer-owned path would simply stop being a conflict and the test would keep passing while testing nothing. They now usecorrectness-reviewer.md, a permanent agent, and assert the same behaviour.Full CLI suite green: 1189 tests across 85 files.
Deliberately not included
testingskill stays. It is invocable on its own and does not depend on either agent. It now has no referencing agent brief, so say the word if it should go too.sync-agentshas no prune path, and this PR does not add one. A consumer that already installed these keeps.claude/agents/testing-agent.mdafter upgrading, and since Claude Code discovers agents by scanning that directory, the removed agents will still appear in their agent list until deleted by hand. A general "remove manifest-owned files no longer in the bundle" step is its own change, not something to smuggle in here.Unrelated pre-existing drift, left alone
devkit doctorreportsagents: devkit source ahead of manifest (2). I verified this is not caused by this change — the drift is incommit-guard.mdandfeature-critique.md(content differs from their manifest hashes), which are another session's in-flight edits. My removal left the manifest fully consistent.Follow-up filed
While auditing which agents actually feed the telemetry loop, feature-completeness turned out to emit no verdict rows at all — zero in
commit_reviews,commit_gate_results, andcommit_gate_timings, across every repo including frink — despite running on straight opus and blocking commits. Filed as sc-1423 under epic 1399.Summary by CodeRabbit
Changes
Documentation
Tests