Add a sweep-low-value-tests skill

[warp-agent-staging]

Summary

Adds a sweep-low-value-tests skill: the ad-hoc, whole-suite counterpart to the test-utility review gate going into the factory's code-review skill (warpdotdev/factory-dev#39).

Work item: QUALITY-1548

Why a second skill rather than one document

From the requester:

along those lines, i'll want to do a pass over the entirety of the client and server codebases, removing these sorts of low-value tests, so if my desire to do "ad-hoc" runs of this sort of "low-value test sweeper" affects how you implement it, take that under consideration

A review gate and a sweep are different jobs. A review gate reads a diff and reacts to what an author just added. A sweep reads a suite merged months ago with no diff to anchor on: it has to find candidates at scale, rank them, prove a deletion did not drop the only guard on a behavior, and chase the production seams the deletions leave dead. One document doing both would serve neither.

The two skills share the criteria vocabulary and overlap by roughly ten lines. Each now names the other as its counterpart so the next person to edit one knows to check the other. The factory skill stays self-contained — a factory review agent works inside a target repo checkout and must not depend on this repository being cloned.

What the skill covers beyond the shared criteria

• Candidate discovery at scale. A sweep cannot read every test.
• The git co-change signal. This is the sharpest tool a sweep has and it comes straight from the requester's quoted tweet: "tests that require changes whenever the underlying source changes are not good tests since they assert implementation, not behavior." That is directly measurable. For each test file and its production sibling, commits(test) + commits(prod) - commits(either) is the number of commits that touched both, by inclusion-exclusion; divided by the test's commit count it gives a co-change ratio that ranks the whole suite with plain git log.
• Two translated categories. "Out-of-scope" and "scaffolding" do not survive the move from a diff to a suite — there is no change to be out of the scope of, and no author present to have written it. They become orphaned (the subject no longer exists, or the only caller of the production code is the test) and never-load-bearing (decided on history evidence plus the safety check, not on the absent author's intent).
• Confidence tiers and batching: delete / rewrite / ask a human / keep, landing in batches a human can read rather than one enormous deletion PR.
• A real safety check. Deleting a test always makes the suite pass, so a green run proves nothing. Break the production behavior, confirm the candidate fails, remove the candidate, and confirm something else still fails. If nothing does, the test was load-bearing and the answer is to rewrite it, not delete it. Then revert the break.
• The seam pass. The half of the request that usually gets skipped: the production indirection that only existed to serve the deleted tests is now dead, and removing it is the actual payoff.
• Guardrails. A sweep that drops real coverage is worse than no sweep. Deleting a test is not the goal; a suite you trust is.

Revised after a dry run

The criteria this skill shares with the review gate were pointed at tests already merged in warpdotdev/warp and warpdotdev/warp-server before either PR merged. Read-only; nothing was deleted. The consolidated evidence lives on factory-dev#39. Four corrections landed here too, because a sweep is where they would actually be executed at scale:

• The seam pass now has a cost side. Removing a single-call-site seam together with its tests contradicts warp's own rust-unit-tests skill and would have deleted eight real behavior tests in edit_state.rs. The pass now asks whether the behavior is reachable at comparable cost without the seam, and never removes a seam alongside tests that have not cleared the safety check.
• Contract-pin tests are never deleted - telemetry keys, wire formats, CLI surfaces, the absence of an interface implementation. This exemption explicitly outranks the tautological and trivial-code categories, because it is the collision a mechanical sweep would otherwise resolve the wrong way.
• A call assertion is a behavior assertion when the collaborator is the unit's only observable output.
• Real IO is not wrong-level when the IO is the subject. A log rotator's tests belong on a real filesystem.

Six species the runs found that a diff review cannot see are now named: assertion-free, quarantined, harness, config-mirror, frozen incidental output, and third-party-shape. The production half of an orphaned deletion is routed to a human.

Two things the runs could not measure are now recorded rather than glossed: the client checkout is a tree:0 partial clone where a path-filtered git log lazily fetches trees and hung at 120s, so the git signal needs a full clone; and the 0.8 co-change threshold is explicitly labelled unvalidated. Both runs argued for keeping the safety check expensive, so the skill now says outright that it has no cheap substitute and that a coverage tool is not one.

The name-heuristic example is replaced with the sharper version the client run produced: the four Debug-representation tests look duplicative, each in fact covers a distinct hand-written match arm, and the real defect is the arm that has no test at all.

Round two: seven more fixes, and the sweeper got the worst of them

The criteria were re-run over both codebases. Seven further corrections landed, all of which matter more here than at review time because a sweep executes them at scale.

Carried over from the review gate: the seam pass now leads with the cost question rather than the merits test and gates removal on naming a replacement; the contract-pin exemption scopes to cases, requires the test to observe the value where consumers do, and treats "another system" as outside the unit's compile-time reach rather than outside the repository; change-detector gains the headless-DOM carve-out and distinguishes a shipped test id from one the test planted; duplicative gains "and same boundary role", since two rows can share a branch and still differ at the extremum.

Framework-enforced now has its own scoping section, because it is simultaneously the largest deletable surface in a mock-heavy suite and the most dangerous thing here to automate. A negative call assertion restates the framework's guarantee only when no live expectation exists on that mock instance in that subtest; an optional expectation flips it to load-bearing; a call-count assertion essentially never qualifies. Ignoring subtest scope produced 149 false leads in one automated scan, so the skill now says a candidate list is not reviewable until it has been scoped that way.

The deletion bias

The requester asked how many dry-run findings wanted the test fixed rather than removed. Weighted by site the answer is overwhelmingly "fixed", but the catalog read delete-first and two independent agents produced deletion-flavoured reports because of it. In a review that is a bad habit; in a sweep it is the exact failure mode the guardrails already exist to prevent.

So: a remediation ladder near the decision question, the catalog re-verbed to match, and the rank tiers reordered so fix or narrow comes first and is stated to be the expected largest tier, with delete demoted to rung 4. Assertion-free moved off the delete tier entirely - when the behavior a test names is real and uncovered, the correction is to make the test assert it, not to remove it. The safety check now says the same thing. And the skill states that a sweep reporting mostly deletions has probably stopped reading carefully.

Also folded in: a deletion must carry the coverage gap it exposes, and the deference clause now says to sanity-check a repo skill's factual claims before enforcing it, since a stale local rule is a finding to report rather than a rule to sweep by.

197 lines, up from 170: the framework-enforced scoping section and the ladder account for nearly all of it, and the ladder itself replaced correction verbs rather than adding prose. Still under the 200-line threshold that would require a references/ split, but with little headroom left - the next addition should displace something.

Round three: the shared catalog is cut here too

The review gate's Test value section was A/B tested against 27 known-correct verdicts with the long version withheld: 0 misses, 0 regressions, and the shared eight-name catalog never came up - every verdict came from the decision question first. That evidence transfers, so the six shared entries here collapse into the same one-sentence list of common answers, framed as conclusions to arrive at rather than patterns to match.

What did not get cut, deliberately:

• The sweep-only species. Orphaned, never-load-bearing, assertion-free, quarantined, harness, config-mirror, frozen incidental output, third-party-shape. These are not restatement of anything - they are the shapes a whole suite exposes and a diff cannot, and each maps to a mechanical discovery step earlier in the document.
• Step 5 and the seam pass, which are procedure rather than catalog.
• The framework-enforced scoping rules, which exist because getting them wrong at 457 sites is the most expensive mistake available here.

The three restores land here as well, and they matter more in a sweep, because a sweep acts on its verdicts rather than proposing them to a human. The deference restore is the significant one: "enforce those in place of these" pointed at a skill that lists Default impls and near-identical tests as delete-worthy, which would have authorised deleting the exact tests the never-delete list protects — including the four Debug tests this document already uses as its own worked example of a false positive. That was a live contradiction sitting inside one file.

The never-delete list absorbed the carve-outs the deleted catalog entries were carrying, so nothing protective went with them: rendering assertions where the class is all the unit exposes, real IO where the IO is the subject, and a near-identical case that pins a boundary the others do not.

197 lines, unchanged. The catalog cut and the restores came out even; the document is not shorter, but the half that was restatement is now the half that is evidence.

Verification

No build or test suite applies — the skill is prose. Instead:

• Every command in the skill was run before it was written down. The co-change math was validated against a synthetic repository with a known commit history: a test file co-changing on 4 of its 5 commits scores 0.80 and one co-changing on 1 of 2 scores 0.50, both matching the hand-computed values. The pairing sed was checked against _tests.rs, _test.go, .test.ts, and _test.py naming.
• The shape heuristics were run against warpdotdev/warp and rewritten after the first drafts misfired: an earlier rg-based form is gone (rg is not universally installed) and the name heuristic was tightened after it matched production pub fn new() declarations instead of test names. Its residual false-positive rate is called out in the skill itself, with the real example that motivated it — test_debug_representation_no_secrets, a valuable security test whose name looks trivial.
• Frontmatter follows the update-skill conventions: kebab-case name, third-person description leading with an action verb, trigger phrasings included. 197 lines, under the 200-line threshold that would require a references/ split.
• Re-read whole after the revision; no rule contradicts another or contradicts the factory skill. 197 lines after round two, still under the 200-line threshold.
• The two new commands added in the revision (quarantined-test discovery, and AssertNotCalled in the interaction grep) were run against warpdotdev/warp before being written down.

Reviewer notes

Two judgment calls worth a second opinion:

• The co-change ratio's 0.8 threshold is a starting point, not a measured value. It nominates files to read and never convicts one, which the skill says explicitly, but the number itself is a guess.
• The safety check costs a real test run per behavior. That is deliberate — it is the only thing standing between a sweep and silently deleted coverage — but it does put a floor on how fast a sweep can go.

---

Conversation: https://staging.warp.dev/conversation/145b2a27-ac12-4631-9345-25f0402b4c7e
Run: https://oz.staging.warp.dev/runs/019fec38-643b-72b2-8334-dfd4d6abf0f2

This PR was generated with Oz.