diff --git a/.changeset/autofix-readability-finding-shapes.md b/.changeset/autofix-readability-finding-shapes.md new file mode 100644 index 00000000..416dc452 --- /dev/null +++ b/.changeset/autofix-readability-finding-shapes.md @@ -0,0 +1,40 @@ +--- +"autofix": minor +--- + +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). + +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. 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 + 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..865d9d56 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,20 @@ 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. 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,