From 06738bedcf522d8ab064160fba7e427ea1b38a70 Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Tue, 11 Aug 2026 10:58:19 -0400 Subject: [PATCH 1/2] [jwies/autofix-readability-finding-shapes] autofix: handle the documentation reviewer's readability finding shapes The review release that adds prose-readability clauses changes what a documentation thread can contain, and three of the fixer's Step 4 rules were calibrated for the old shape only. Batched instances: the reviewer caps readability at one thread per review and quotes up to three further instances in that thread's body, possibly in other files. The do-not-touch-other-files rule gains a second exception: a quoted instance is part of the finding wherever it lives; an instance alluded to without a quote is not. Without this the fixer could only fix the anchored instance and the thread could never fully resolve. Rewrite fidelity: a readability finding carries its plain rewrite, and that rewrite passed claim validation (meaning preservation checked). The fixer's improvised paraphrase passed nothing, so the rule is apply the reviewer's words verbatim, or leave the item unfixed and say so when the text has drifted out from under the quote. Duplicate paragraphs: the rewrite-before-delete bias exists for restated comments on unexplained code. Applied to a says-the-same-thing-twice finding it would mint a third phrasing of content that already exists twice, which is the defect the finding names. Deletion is the fix. The README documents the shape that stays out of reach by design: a PR title/description readability finding folds into the review body, never becomes a thread, and the thread-driven worklist cannot see it. The description is the author's voice; the review body already offers them the rewrite. --- .../autofix-readability-finding-shapes.md | 35 +++++++++++++++++++ workflows/autofix/README.md | 8 +++++ workflows/autofix/autofix.md | 26 ++++++++++++-- 3 files changed, 66 insertions(+), 3 deletions(-) create mode 100644 .changeset/autofix-readability-finding-shapes.md diff --git a/.changeset/autofix-readability-finding-shapes.md b/.changeset/autofix-readability-finding-shapes.md new file mode 100644 index 00000000..259f2175 --- /dev/null +++ b/.changeset/autofix-readability-finding-shapes.md @@ -0,0 +1,35 @@ +--- +"autofix": minor +--- + +The fixer learns the documentation reviewer's two new readability finding +shapes (review's prose-readability release: metaphor in place of the +mechanism, says-the-same-thing-twice, undefined coinage). + +Three rule changes in `autofix.md` Step 4, no code changes: + +- **Batched instances are in scope.** The reviewer caps readability at one + thread per review and enumerates up to three further instances, verbatim, in + that thread's body. The do-not-touch-other-files rule gains a second + exception for them: each *quoted* instance is part of the finding wherever + it lives, while an instance merely alluded to without a quote is not. + Without this, the fixer could only ever fix the anchored instance, the + thread could never fully resolve, and the batching (which exists to spare + the author five separate threads) would trade author attention for fixer + blindness. +- **Readability rewrites are applied verbatim.** A readability finding carries + its plain rewrite, and that rewrite passed claim validation, which checked + it preserves the original sentence's meaning. The fixer's improvised + paraphrase passed nothing, so improvising is now off the table: apply the + reviewer's words, or (when the file has drifted and the quote no longer + fits) leave the item unfixed and say so. +- **Duplicate paragraphs are deleted, never re-worded.** The existing + rewrite-before-delete bias is calibrated for a restated comment sitting on + unexplained code. Applied to a says-the-same-thing-twice finding it would + produce a third phrasing of content that already exists twice, which is the + defect the finding names. Deletion of the quoted copy is the fix. + +The README documents the one shape that stays out of reach by design: a PR +title/description readability finding folds into the review body and never +becomes a thread, so the thread-driven worklist cannot see it. The description +is the author's voice; the review body already offers them the rewrite. diff --git a/workflows/autofix/README.md b/workflows/autofix/README.md index ad63f7c6..e71f8d7a 100644 --- a/workflows/autofix/README.md +++ b/workflows/autofix/README.md @@ -136,6 +136,14 @@ arming `nits` to clear three stale comments also invites the fixer into every other cosmetic thread on the PR. A flat namespace cannot show that, so it is written down here and in `scope.ts`. +One class of documentation finding never reaches this workflow at all: a PR +title/description readability finding posts PR-level, folded into the review +body rather than opened as a thread, so the worklist (which reads threads and +parses their labels) never sees it. That is deliberate, not a gap: the +description is the author's voice, the review body already carries the plain +rewrite for the author to take or leave, and a bot editing PR metadata is a +different trust decision than a bot editing comment text on a branch. + Read this before adding a token to the vocabulary. `nits` and `loop` look like peers and are not, and the day both are requested the rule that resolves them has to already exist. diff --git a/workflows/autofix/autofix.md b/workflows/autofix/autofix.md index 10fcb0a7..3c3427f4 100644 --- a/workflows/autofix/autofix.md +++ b/workflows/autofix/autofix.md @@ -494,6 +494,21 @@ Edit the files directly in the workspace. Rules, all hard: a rationale.** A plausible-sounding reason you cannot source is worse than no comment, because the next reader will trust it and the reviewer cannot tell the difference. +- **A readability item is fixed with the reviewer's own words.** A documentation + finding may flag prose readability (a metaphor that hides the mechanism, a + paragraph restating an earlier one, shorthand the document never defines) + rather than comment content, and it carries the plain rewrite, quoted in its + body or as a suggestion block. Apply that rewrite verbatim: it survived claim + validation, which checked that it preserves the original sentence's meaning, + and a paraphrase you improvise did not. If the file has drifted and the quoted + rewrite no longer fits the text, leave the item unfixed and say so in Step 7 + rather than composing your own. +- **On a says-the-same-thing-twice item, delete; never mint a third phrasing.** + The rewrite-before-delete bias above is calibrated for a restated comment + sitting on unexplained code. A duplicated paragraph is the opposite case: the + content already exists in the copy that stays, so the fix is deleting the copy + the finding names, and rewording the duplicate into different words is the + failure mode the finding exists to stop, not a fix for it. - **Say when a deletion left a hole.** If you delete a comment off a constant, magic value, or workaround without being able to state why the value is what it is, the thread is fixed and the code is still unexplained. Report both in @@ -501,9 +516,14 @@ Edit the files directly in the workspace. Rules, all hard: can write (`staleAfter`'s restating comment is gone; nothing records why the window is that long). Reporting it as plainly fixed hides the one thing a human still needs to do. -- **Do not touch files no item points at.** The one exception is a change that - is mechanically forced by a fix (a caller that must be updated for a changed - signature); note any such file in Step 7. +- **Do not touch files no item points at.** Two exceptions. First, a change + mechanically forced by a fix (a caller that must be updated for a changed + signature). Second, the instances a batched documentation item quotes: the + documentation reviewer caps readability at one thread per review and + enumerates up to three further instances, verbatim, in that thread's body, so + each **quoted** instance is part of the finding wherever it lives; fix those + too. An instance the body merely alludes to without quoting is not part of + the finding. Note any file either exception led you into in Step 7. - **Do not amend, rebase, or force-push.** You produce working-tree changes; the push is a safe output. - If a fix would require a design decision the reviewer did not make for you, From 092c73304ad74def576e2753958c663312233310 Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Thu, 13 Aug 2026 10:25:48 -0400 Subject: [PATCH 2/2] [jwies/autofix-readability-finding-shapes] autofix: bound the batched-instances exception by the verbatim-rewrite rule Review feedback on #335: - The changeset said two new finding shapes; there are three. - The batched-instances exception and the verbatim-rewrite rule conflicted for a quoted instance without its own rewrite: the reviewer's batching instruction (review.md, the readability cap) requires quoting further instances, not rewriting them. The exception now defers to the rewrite rule: fix a quoted instance only when the fix needs no improvised words (deleting a duplicated paragraph) or the thread body carries a rewrite for that instance; otherwise leave it and report in Step 7. --- .changeset/autofix-readability-finding-shapes.md | 9 +++++++-- workflows/autofix/autofix.md | 8 +++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/.changeset/autofix-readability-finding-shapes.md b/.changeset/autofix-readability-finding-shapes.md index 259f2175..416dc452 100644 --- a/.changeset/autofix-readability-finding-shapes.md +++ b/.changeset/autofix-readability-finding-shapes.md @@ -2,7 +2,7 @@ "autofix": minor --- -The fixer learns the documentation reviewer's two new readability finding +The fixer learns the documentation reviewer's three new readability finding shapes (review's prose-readability release: metaphor in place of the mechanism, says-the-same-thing-twice, undefined coinage). @@ -16,7 +16,12 @@ Three rule changes in `autofix.md` Step 4, no code changes: Without this, the fixer could only ever fix the anchored instance, the thread could never fully resolve, and the batching (which exists to spare the author five separate threads) would trade author attention for fixer - blindness. + blindness. The verbatim-rewrite rule below still governs each quoted + instance: the reviewer quotes it but need not rewrite it, so the fixer + touches it only when the fix needs none of its own words (deleting a + duplicated paragraph) or the thread body carries a rewrite for that + instance; a quoted metaphor or coinage without one is left unfixed and + reported. - **Readability rewrites are applied verbatim.** A readability finding carries its plain rewrite, and that rewrite passed claim validation, which checked it preserves the original sentence's meaning. The fixer's improvised diff --git a/workflows/autofix/autofix.md b/workflows/autofix/autofix.md index 3c3427f4..865d9d56 100644 --- a/workflows/autofix/autofix.md +++ b/workflows/autofix/autofix.md @@ -523,7 +523,13 @@ Edit the files directly in the workspace. Rules, all hard: enumerates up to three further instances, verbatim, in that thread's body, so each **quoted** instance is part of the finding wherever it lives; fix those too. An instance the body merely alludes to without quoting is not part of - the finding. Note any file either exception led you into in Step 7. + the finding. The verbatim-rewrite rule above still governs each quoted + instance: the reviewer is only required to quote it, not to rewrite it, so + fix a quoted instance when its fix needs no words of yours (a duplicated + paragraph: delete the quoted copy) or when the thread body carries a rewrite + for that instance; a quoted metaphor or coinage with no rewrite of its own is + left unfixed and reported in Step 7, exactly like a drifted quote. Note any + file either exception led you into in Step 7. - **Do not amend, rebase, or force-push.** You produce working-tree changes; the push is a safe output. - If a fix would require a design decision the reviewer did not make for you,