Public: anyone can read this; the crew can change it.
Automate a review finding as a lint or test: Smaller, but please fix in the same round: #8843
Check · Review · Land · Live done — Landed and live in production.
torque/forge:torque/8838-automate-a-review-finding-as-a-lint-or-t → main 2 commits fork 1 thread · 1 unresolved
2 files
changed +32
−3
src/flow/repeats.ts +17 −3
| @@ -58,12 +58,26 @@ export function wordsOf(text: string): string[] { | ||
| 58 | 58 | return [...out].sort(); |
| 59 | 59 | } |
| 60 | 60 | |
| 61 | /** The findings in one review body: its list items or lines, each a request. */ | |
| 61 | /** | |
| 62 | * Rule "a lead-in is not a finding" (#8838): a line whose text, bare of | |
| 63 | * bold/italic markers and trailing space, ends in a colon introduces the list | |
| 64 | * that follows and asks for nothing: `Smaller points:`, `**Smaller points:**`, | |
| 65 | * `_Fix in the same round:_`. | |
| 66 | */ | |
| 67 | export const isLeadIn = (line: string): boolean => | |
| 68 | line.replace(/^[\s*_]+|[\s*_]+$/g, "").endsWith(":"); | |
| 69 | ||
| 70 | /** | |
| 71 | * The findings in one review body: its list items or lines, each a request. | |
| 72 | * A line that ends in a colon introduces the list that follows ("Smaller | |
| 73 | * points, fix in the same round:"), asks for nothing, and is dropped: as a | |
| 74 | * finding it repeated across every review that used it (#8838). | |
| 75 | */ | |
| 62 | 76 | export function linesOf(body: string): string[] { |
| 63 | 77 | return body |
| 64 | 78 | .split("\n") |
| 65 | .map((l) => l.replace(/^\s*(?:[-*]|\d+[.)])\s*/, "").trim()) | |
| 66 | .filter((l) => l.length >= 12) | |
| 79 | .map((l) => l.replace(/^\s*(?:[-*]|\d+[.)])\s+/, "").trim()) | |
| 80 | .filter((l) => l.length >= 12 && !isLeadIn(l)) | |
| 67 | 81 | .slice(0, MAX_LINES) |
| 68 | 82 | .map((l) => l.slice(0, MAX_LINE)); |
| 69 | 83 | } |
test/review-repeats.test.ts +15
| @@ -178,3 +178,18 @@ describe("the web door", () => { | ||
| 178 | 178 | expect(draw("agent")).toContain("public repositories:"); |
| 179 | 179 | }); |
| 180 | 180 | }); |
| 181 | ||
| 182 | describe("a lead-in line is not a finding (#8838)", () => { | |
| 183 | test("linesOf drops the line that only introduces a list", async () => { | |
| 184 | const { linesOf } = await import("../src/flow/repeats.ts"); | |
| 185 | expect( | |
| 186 | linesOf("Smaller, but please fix in the same round:\n- Move these two lines below the spread.\n- Add a test for the agent admin case."), | |
| 187 | ).toEqual(["Move these two lines below the spread.", "Add a test for the agent admin case."]); | |
| 188 | // reviews opening the same way are not a repeated ask | |
| 189 | expect(linesOf("Smaller points, fix in the same round:")).toEqual([]); | |
| 190 | for (const bare of ["**Smaller points:**", "_Fix in the same round:_", "**Smaller points, fix in the same round:** ", "*__Fix them in the same round:__*"]) | |
| 191 | expect(linesOf(bare)).toEqual([]); | |
| 192 | // a real ask that merely contains a colon stays | |
| 193 | expect(linesOf("**High:** `flowOnly` is missing from the agent refusal.")).toEqual(["**High:** `flowOnly` is missing from the agent refusal."]); | |
| 194 | }); | |
| 195 | }); |
new above
offline
!l.endsWith(":")missesSmaller points:and_..:_. Strip*/_wrappers before testing, and cover it in test/review-repeats.test.ts.