Name SNIPPET_END on the Script Reference Too - #1705
Conversation
The paragraph read "that set cannot drop the full stop", and the nearest set a reader has by then is `SENT_END`'s, introduced in the same sentence that contrasts the two. Read that way the claim is about the wrong set and is false, since dropping the full stop from `SENT_END` has no bearing on this guard. The code and the test docstring already name `SNIPPET_END` outright. All three surfaces now state it the same way, which a grep for the sentence confirms.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Repository: ptr727/ProjectTemplate/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The edited paragraph still contains a misleading/incorrect guard description and an overstated claim about SNIPPET_END that should be tightened for documentation accuracy.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Updates the prose-gate script documentation to explicitly name SNIPPET_END as the terminator set referenced in the configuration-snippet exemption discussion, aligning the README with the code comment and test narrative.
Changes:
- Replaces the ambiguous “that set” reference with an explicit
SNIPPET_ENDreference inscripts/README.md.
| File | Description |
|---|---|
| scripts/README.md | Clarifies which terminator set the configuration-snippet documentation is referring to. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
"`SNIPPET_END` cannot drop the full stop without dropping the guard" is not true of the pattern. Dropping `.` leaves `[!?]`, so `conclusion: wrong!` stays refused and the guard still works. What dropping it would do is re-exempt the wrapped sentence the guard exists for, `hand: correctly.`, which is the same shape as the FQDN it refuses. A maintainer reading the old claim would take the full stop for load-bearing in a way it is not, and read narrowing as impossible when narrowing is the trade on offer. All three surfaces carried the sentence and all three now state what the change would cost instead. One clause earlier the script reference attributed the choice of set to the guard `is_tool_directive` pairs with, which is `SENT_END` itself, so the sentence said `SENT_END` reads a set other than itself. It names the exemption and `SNIPPET_END` at the contrast point, as the code comment does. A grep for the old sentence across the tree reports none.
The paragraph said the exemption pairs the guard `is_tool_directive` pairs, and that guard is `SENT_END`, which the sentence after it denies by naming `SNIPPET_END` instead. The two are not interchangeable: `address: fd00::` is exempt under one and reported under the other. It now says the exemption pairs a guard with its anchor the same way and for the same reason, which leaves the contrast standing. The code comment also left the full stop's owner to be inferred, and the nearest candidate was the value rather than the set. Dropping a value's full stop exempts it outright, which is a different outcome from the one the sentence describes, so the set is now named there as the other two surfaces name it. And the round before this left a comma where it removed a clause, reading "being the case,.". No gate reads punctuation, so nothing but a reader was going to find it.
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The changes consistently correct documentation and commentary to match the actual regex constants and behavior, with no functional impact.
Review effort: Lite
Findings: None
Resolved since last review (1)
ptr727
added
the
comments
A verbless line takes its subject from the line above it, and the line above this one names `is_tool_directive`, so "`SNIPPET_END` rather than `SENT_END`" read as that function's choice. It is the exemption's. The script reference already named the subject there, so the three surfaces disagreed on the one sentence an earlier round rewrote them all to align. "It re-exempts the wrapped sentence" bound to the cost case above it, which is a body carrying no terminator, and the claim is false of that one. Measured both ways: with the full stop in the set a continuation ending in one reports `comment-wrap` and an unterminated one reports nothing, and with it dropped both report nothing. So what dropping it re-exempts is a wrapped sentence whose continuation ends in a full stop, which is what the line now says. The pull request also needed the `comments` label, added before this push rather than at open, since the first commit was a one-line reference change that added no comment. That is the second shape of the label trap this chain has hit.
|
Pushed 4f482ac, which names the subject of the block's last two fragments. Two open, recorded rather than fixed, and I have stopped here.
Why I stopped. This is the seventh round on one paragraph. Each round has fixed what the last one raised and introduced a comparable number of new referent defects, and the measured trend is flat rather than converging. The code has been correct since the colon fix several rounds ago; every finding since has been in prose describing it. The cause is structural rather than a lapse in care. One claim is written out on three surfaces, so each correction is three edits in three different contexts, and each context offers a pronoun a different nearest noun. Four of the last five rounds included a defect of the form "fixed on one surface, not the others", which is the signature of that shape rather than of inattention. I would rather fix the duplication than keep correcting the copies, and that is a change to how the gate is documented rather than something to fold into this pull request. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The changes are documentation/comment-only, internally consistent with the current implementation (`SNIPPET_END = re.compile(r"[.!?]["')]]?\s*$") and do not introduce behavioral risk.
Review effort: Lite
Findings: None
Promotes the configuration-snippet exemption from `develop` to `main`. The delta is one squash commit, `1fa927a`, three files, 141 insertions. ## What it carries `comment-case` reports a comment line whose sentence opens in lowercase, and a configuration snippet pasted into a comment opens on a lowercase key, so no form of one could pass. The rule and a copy-pasteable snippet were mutually exclusive, and the only spellings that passed each changed the line a reader copies. A comment body of one key, a colon and one value is now configuration rather than a sentence, exempt from `comment-case` and `comment-wrap`, sitting beside the lone-key exemption that already covered the `# packages:` line above the one that failed. Clearing the wrap with it is what a multi-line snippet needs. The guard reads its own terminator set rather than `SENT_END`, whose set carries the colon. A colon ends configuration rather than a sentence here, which is why the lone-key exemption exists, and reading it as a terminator refused `address: fd00::`, `image: ghcr.io/o/n:` and `root: C:`. ## What it costs, stated where the pattern is - **A marker comment**, the common loss: `# todo: refactor` is the same shape as a config line. - **A sentence wrapping through such a body**, which loses its wrap. - **A refusal that loses no detection**: a value ending in a full stop is judged as before, an FQDN being the case, and the terminator set cannot drop the full stop without dropping the guard. `is_comment_prose` deliberately does not carry the exemption, unlike its three siblings, so `comment-added` still prices an added `# key: value` line. An earlier round added it for symmetry and that was reverted, since `comment-added` reads a body of any case and the exemption there would have freed `Owner: alice` from the label as well. Measured over the tree the exemption changes no finding and newly reaches one body, a command to paste. ## Review record Two Copilot rounds on this promotion. The first stated `reviewed=3 changed=3 findings=1`, full coverage of the three-file diff. The second, on the current head, states no coverage, which is the nondeterminism recorded on #1692. The file set is the same three files at both heads, so the first round's statement covers exactly the paths being promoted, and only their content moved in between. One thread, answered and resolved. Eight local adversarial passes on the feature branch, finding 9, 7, 7, 3, 2, 2, 1 and 1. The one item left open when this promotion was opened is now fixed rather than carried. In `scripts/README.md` the phrase "that set" bound to `SENT_END`'s set under a nearest-antecedent reading, where the code and the test docstring both name `SNIPPET_END` outright. #1705 names it explicitly and merged as `4f0418b6`, which is this promotion's current head. The follow-up that record called for is filed as #1706: a rule's reach is written out on three hand-authored surfaces, and the churn measured here came from that rather than from any difficulty in the two-line fix. Fixes #1700
Answers the one review thread on the promotion #1704.
scripts/README.mdended a sentence with "that set cannot drop the full stop without dropping the guard". The nearest prior set in that paragraph isSENT_END's, introduced by the same sentence that contrasts the two, so the claim read as being about the wrong set and is false of it: dropping the full stop fromSENT_ENDhas no bearing on this guard.It now names
SNIPPET_END, which the code comment and the test docstring already did. A grep for the sentence confirms all three surfaces state it the same way.Two open, recorded rather than fixed
A local pass over this commit found two more in the same paragraph. Neither affects behavior and neither is what the promotion thread raised, so they are written down here rather than taken in a further round.
is_tool_directivepairs with its own anchor and for the same reason. That guard reads its own terminator set rather thanSENT_END". The nearest referent for "That guard" is the guardis_tool_directivepairs with, and that guard isSENT_END, so as written the sentence saysSENT_ENDreads a set other than itself. The code comment avoids it by namingSNIPPET_ENDat the contrast point; the README names it four sentences later.SNIPPET_ENDcannot drop the full stop without dropping the guard" is not true of the pattern. Dropping.leaves[!?]["')\]]?\s*$, which still refusesconclusion: wrong!, one of the bodies the suite pins as judged. The guard would be narrowed rather than dropped. The true claim is narrower: dropping the full stop would re-exempt the wrapped-sentence case the guard exists for,hand: correctly., which is the same shape as the refusedorigin: example.com..The second is the one worth fixing, and it is a three-surface change rather than a one-line one.
Closes on promotion: nothing of its own. This exists to unblock #1704.