From f85e34fe1c921572bd8cfd65486fe9bf42b93565 Mon Sep 17 00:00:00 2001 From: Noah Lindner Date: Sun, 19 Jul 2026 10:47:28 -0400 Subject: [PATCH] spec: be quiet on tests unless they lie --- .review/REVIEW-PROMPT.md | 9 +++++++-- src/review-sweep.test.ts | 9 +++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/.review/REVIEW-PROMPT.md b/.review/REVIEW-PROMPT.md index d5874cd..53d54f8 100644 --- a/.review/REVIEW-PROMPT.md +++ b/.review/REVIEW-PROMPT.md @@ -32,7 +32,12 @@ Run these steps: (reinvents-primitive / slop). "Slop" is code RELATIVE to the simpler or already-existing way: does it reinvent a corpus primitive, or is it bigger / more abstract / more speculative than the corpus pattern for the same job? When you cite a fix, name the actual corpus file/primitive it should use. -5. **Be precise — the corpus IS the filter.** Surface only a real bug or a genuine corpus/rubric violation. +5. **Be quiet on tests unless they lie.** Test diffs are support evidence, not a place to dump harness taste. + Do NOT flag harmless arrangement, naming, snapshot style, broad-vs-narrow harness choice, or missing edge + cases you merely wish existed. Raise a test finding only when the test can pass while the product bug remains, + asserts the wrong behavior, removes meaningful coverage, relies on nondeterminism/flaky external state, or + hides a production footgun behind test-only branching. +6. **Be precise — the corpus IS the filter.** Surface only a real bug or a genuine corpus/rubric violation. (This precision rule governs the LINE-LEVEL findings; it does NOT gate the whole-change `overbuilt` / `wrong-premise` / `confident-noop` finding from step 3, which is judged against the simplest version, not a primitive.) SUPPRESS generic best-practice nitpicks, style preferences, and low-confidence guesses: a reviewer that cries @@ -40,7 +45,7 @@ Run these steps: can't tie a finding to a real defect or a specific corpus primitive, drop it. And verify anything you *can* check against the checkout (an import, a definition, a type) by opening the file before you assert it, rather than inferring a defect from the diff surface. Then format per the **Comment format** below. -6. Write the review to the output file you were given — the runner posts it for you. Do NOT run `gh` (you have none). +7. Write the review to the output file you were given — the runner posts it for you. Do NOT run `gh` (you have none). ## Prior reviews on this PR (your memory) diff --git a/src/review-sweep.test.ts b/src/review-sweep.test.ts index 4ec615c..2f77a0e 100644 --- a/src/review-sweep.test.ts +++ b/src/review-sweep.test.ts @@ -47,6 +47,7 @@ const pr = (number: number, sha: string): Pr => ({ const sha256 = (s: string) => new Bun.CryptoHasher('sha256').update(s).digest('hex') const prefixOf = (prompt: string) => prompt.slice(0, prompt.indexOf(THIS_PR)) +const reviewStepsOf = (prompt: string) => prompt.split('Run these steps:')[1]?.split('## Prior reviews on this PR')[0] ?? '' // Three different PRs: different numbers, different head SHAs, and (crucially) one mid-thread with memory — // the hardest case, since "continuing a review" must STILL not perturb the prefix. @@ -219,6 +220,14 @@ test('the no-op token is instructed in the prompt, and adding it kept the prefix expect(prefixes[0]).toBe(prefixes[2]) }) +test('the review spec suppresses noisy test-only nits', () => { + const steps = reviewStepsOf(prompts[0] ?? '') + expect(steps).toContain('Be quiet on tests unless they lie') + expect(steps).toContain('Do NOT flag harmless arrangement, naming, snapshot style') + expect(steps).toContain('only when the test can pass while the product bug remains') + expect(steps).toContain('nondeterminism/flaky external state') +}) + // Inline review comments can only anchor to RIGHT-side lines the diff actually touches — get this wrong and the // whole review 422s. Added + context lines count; the removed line's number does not. test('diffRightLines: anchorable lines are the new-file added/context lines', () => {