Carry a verbatim string across the lines it spans - #465
Conversation
Blanking quoted spans per line suits a language whose strings end on the line they start. The C# verbatim string does not, so a marker on any later line of one was read as a comment and its prose linted as code. `strip_strings` now reports whether it ended inside a verbatim string, and the extractor carries that state as it does an unclosed block. A line wholly inside one is skipped, and the line that closes it still gives back whatever follows the quote. The README claimed markers in string literals are never comments, which was true only within a line. It now says what is carried and names the heredoc and the block scalar as forms that are not, rather than leaving the reader to find out. Reported by Copilot on promotion #460 as a wording gap. The wording was the smaller half. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes the prose/comment extractor so C# verbatim strings (@"...") are tracked across line boundaries, preventing comment markers inside verbatim string content from being misread as real comments, and updates documentation/tests to reflect and verify the behavior.
Changes:
- Update
strip_stringsto return whether a C# verbatim string remains open at end-of-line, enabling cross-line carry. - Teach
extracted_commentsto skip lines that are entirely verbatim-string content while still scanning the closing line for trailing comments. - Add regression tests and clarify the README’s description of which multi-line string-ish constructs are (and are not) carried.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/prose_lint.py | Carries verbatim-string state across lines during comment extraction. |
| scripts/test_prose_lint.py | Adds/adjusts tests to validate verbatim-string multi-line behavior. |
| scripts/README.md | Updates documentation to describe per-line blanking and the verbatim-string carry exception. |
`was_inside and in_string` was read as "the whole line is string content", and it is not. A line can close the carried string, hold real code and comments, and open another that stays unclosed. Both ends are inside a string while the middle is not, so the skip dropped whatever sat between them. The skip was also redundant. A line genuinely inside a carried string is blanked whole by the masker, so the scan below it finds nothing there either way. Taking it out fixes the one case and changes neither of the others. Reported by Copilot on #465. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
scripts/prose_lint.py:409
extracted_commentsupdatesin_stringby callingstrip_strings(...)before handlingdoc_closing/closing. This means an@"sequence inside a documentation/block comment can flipin_stringto True and then blank out real comment markers that occur after the comment closes (or on later lines), causing missed findings. To avoid string-state leaking from comment text, only runstrip_strings(and updatein_string) on the portion of the line that is actually scanned for markers (i.e., after the doc/block closer when you’re inside a carried comment).
for n, raw in enumerate(lines, 1):
line = raw.rstrip('\r')
# A line inside a carried string is blanked whole, so the scan below finds nothing in it.
# Skipping the line instead would drop a string that closes and reopens around real code.
masked, in_string = strip_strings(line, spec['quotes'], spec['verbatim'], in_string)
A quote in comment text is prose. Masking the line in one pass read it as a string opener, which blanked every marker after it. Within a line that cost the block its closer: `/* note @"x */ code(); // tail` found no `*/`, so the comment ran to end of line and carried into the lines below. Across lines the carried string state made it worse, because the opener stayed open and blanked markers until something closed it. Masking now runs from the scan position, and only the code before a marker advances the string state, so a comment contributes nothing to it. Block closers are found in the raw line for the same reason. The within-line half predates the string carry and reproduces on develop. The cross-line half arrived with the carry in this branch. The second assertion was written expecting two findings where the input has three, the extra one being the block comment's own wrapped sentence. Rephrased so each block line is a sentence and the two findings are the recovered comments, rather than fitting the number to the output. Reported by Copilot on #465, in the low-confidence block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Answering the round-two low-confidence finding, since a suppressed finding has no thread to reply on. It is right, and it is two defects rather than one. Fixed in 0a6f6b8. A quote in comment text is prose, and masking the line in one pass read it as a string opener, which blanked every marker after it. Within a line, that cost the block its closer: No Across lines, the carry you flagged made it worse. The phantom opener stayed open and blanked markers on every line below until something closed it, losing two real comments in the case above. The fix is the shape you suggested. Masking runs from the scan position, and only the code before a marker advances the string state, so a comment contributes nothing to it. Block closers are found in the raw line for the same reason: inside a comment, a quote is not a string delimiter. One correction worth recording against myself. The second assertion was written expecting two findings where the input has three, the extra being the block comment's own sentence wrapping across its two lines. I rephrased the bait so each block line is a self-contained sentence and the two findings are exactly the recovered comments, rather than editing the expected number to match the output. That is the second time this session I have written an expectation from the shape of a neighbouring test instead of deriving it from the input. 70 self-tests and 19 repo_gate tests pass, both cases watched failing against ff4eca9, |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
scripts/README.md:44
- The README implies the only multi-line string form needing special handling is the C# verbatim string, with the remaining gap limited to heredocs/YAML block scalars. However, the extractor only carries state for C# verbatim strings (
spec['verbatim'] == True); other syntaxes here (shell/YAML#, PowerShell#, etc.) still scan each line independently even though quoted strings / here-strings can also span lines. This wording can mislead readers about when markers inside multi-line string content may still be treated as comments.
JSON is treated as JSONC, because that is what ships: VS Code tasks, launch, devcontainer and workspace files all carry comments under a plain `.json` name. A marker inside a string literal is not a comment, so each line is scanned with quoted spans blanked first, and Python uses `tokenize` so a trailing comment is seen exactly. Blanking is per line, which suits a language whose strings end on the line they start. The C# verbatim string is carried across lines because it does not, while a heredoc or a block scalar in the shell and YAML syntaxes is not yet, so a marker inside one of those still reads as a comment. A documentation comment (`///`, `/**`, a docstring) is left to CODESTYLE, which permits the paragraphs this rule forbids.
The previous wording put the C# verbatim string on one side and a heredoc or a block scalar on the other, which reads as a list of two remaining cases. The carry is keyed on the syntax, and C# is the only syntax that sets it, so every other one scans a line at a time and any string spanning lines leaks its markers. Checked rather than reasoned: a shell quoted string, a PowerShell here-string, and a YAML block scalar each report a `#` inside them as a comment. Naming the rule rather than two instances of it means the sentence stays true when a syntax is added. Reported by Copilot on #465, in the low-confidence block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Answering the round-three low-confidence finding. Right, and my wording was worse than imprecise. Putting the C# verbatim string on one side and a heredoc or block scalar on the other reads as a list of two remaining cases. The carry is keyed on the syntax and C# is the only syntax that sets it, so every other syntax scans a line at a time and any string spanning lines leaks. Checked rather than reasoned: Corrected in b997508 by naming the rule rather than two instances of it, so the sentence stays true when a syntax is added. The behavior gap is filed as #466 rather than fixed here. The remaining forms each need their own opener and closer rules, and they do not share one: a shell or PowerShell quote that simply does not close on its line, a here-string with 70 self-tests and 19 repo_gate tests pass, |
Promotes `d4c4085` (#459), `adaa068` (#461), `e52c68a` (#463), `7bf8a12` (#464), `269629a` (#465), and `036e468` (#467). Conflict-free, six commits ahead. ## What lands The charset rule gets three tiers instead of a flat non-ASCII ban, which could not express context - an audit report classified U+2264 and U+2265 as the scientific carve-out while the rule named both must-replace. Tier 1 never survives ASCII, tier 2 is an operator kept beside a number and replaced between words, tier 3 is a unit symbol whose ASCII form would be a lie. A character in no tier is reported rather than passed, and the rule is clean tree-wide, so it gates. Two prose rules invert from detecting a subset to banning the construction. The pronoun-keyed splice pattern found 170 of 493 and missed every imperative one; the em-dash rule now says restructure the sentence, and the spaced hyphen is banned in its own right. Comments get a gate covering every syntax the fleet types carry - `//`, `/* */`, `<!-- -->`, `<# #>`, `;`, `#` - with JSON read as JSONC because that is what ships. That recovered four files the old discovery never saw, including the VS Code task and devcontainer snippets downstream repos copy; their 37 malformed comments are fixed here. The Verification Discipline mechanisms land too: a gate has to be watched failing, and a check is scoped by what the project declares rather than by the file that prompted it. ## The comment-extractor fix (#461) Copilot's review of this promotion found a defect in the extractor #459 adds, so it was fixed at the source and this PR now carries it. Block openers were searched before line markers against a ceiling of the whole line, so a `/*` inside a `//` comment opened a real block: the line's own comment was truncated at the opener, the closer carried into the lines below, and the code there was handed to the comment rules as prose. PowerShell failed identically with `<#` inside a `#` comment. Those are the two fleet syntaxes carrying both marker kinds. Tree-wide counts are unchanged - dash 963, comment-wrap 454, semicolon 388, comment-case 56 - because nothing in this repository nests the markers that way. The exposure is downstream, in the C# and PowerShell repositories the extractor is aimed at, and it would surface the moment the comment rules gate rather than warn. That is the argument for fixing before promotion rather than after. ## The carve-out contradiction (#463) Copilot's second round found the same review's own subject matter contradicting itself. Two adjacent bullets disagreed on whether a developer-typed but un-tiered character is exempt from the gate or reported by it. The implementation already reported it as `charset-unknown` while the doc read as an exemption. The carve-out now states that it governs what an agent may rewrite rather than what the gate reports, which leaves both bullets true and changes neither rule. Worth blocking a promotion for, because `main` is what the audit reads as ground truth, and this contradiction had already produced a real misreading - the audit report that classified U+2264 and U+2265 as the scientific carve-out, which is what motivated tiering the rule in #459 to begin with. ## The scanner rewrite (#464) The third round found a fourth defect in the same function, so the structure went rather than the instance. The extractor searched each marker kind from column 0 against a ceiling, and a ceiling can only describe the first comment on a line. That one shape produced every extractor defect in this promotion: a marker quoted inside the first comment read as real, a doc marker excluded from the ceiling unbounded it, and anything after the first comment was unreachable. A single left-to-right pass replaces it. Several blocks on one line, a comment trailing the line where a block closes, and a multi-line documentation block that no longer leaks its prose into the scan all come with it. #462 is fixed there too rather than deferred, since a defect left in the promoted diff keeps being found and the loop cannot reach a clean round while it stands. C# verbatim strings read correctly in both directions and in all three spellings, and C# took its own syntax entry so the rest of the C-like family does not inherit a form it lacks. ## Multi-line strings (#465) The fourth round found that masking runs per line while a C# verbatim string spans them, so a marker on any later line of one was reported as a comment. A false positive, and the direction that is worse than a miss, since it asks a reader to edit text that is data. Reviewing that fix found two more in the same area: a quote in comment text opened a phantom string that blanked the markers after it, within a line and then across lines, and a line-skip meant as an optimization dropped a string that closed and reopened around real code. Masking now runs from the scan position and only code advances the string state. #466 records what is still not carried. C# is the only syntax whose strings are tracked across lines, and the README says so rather than leaving a reader to find out. ## The continuation asterisk (#467) The fifth round found the extractor damaging the prose it then judged. A leading `*` was taken off every block comment body, which is the `/* */` convention for continuing a line and ordinary text everywhere else, so `<!-- *emphasis* leads here -->` became `emphasis* leads here` and `comment-case` reported the lowercase opening it had just created. Worse than a plain false positive, since the prose reported is not what the file holds. Stripping is now one `*` against whitespace, on a line continuing a `/* */` block. This defect dates to `d4c4085` rather than to any fix made during this review, so the promotion is where it would first reach `main`. ## Why promote now The snippets fixed here are the ones a new or realigning repo copies, and the reviewer-facing rules - the merge gate and the runbook now require investigating the low-confidence block - only bind downstream once `main` carries them. That requirement earned itself twice more on #461. Round one's single inline comment read as a wording nit and was a live defect; round two had no inline comments at all and both findings sat in the suppressed block, one of them the reopened bug. A loop polling `reviewThreads` would have reported a clean pass on both rounds. ## Fidelity note `GOVERNANCE.md` and `.github/copilot-instructions.md` sections declared verbatim changed, so downstream copies are **stale** until re-vendored. `spec/fidelity_honesty.py` separates stale from modified by hash, so the audit reports it correctly. Warn-only backlog, reported but not gating: dash 963, comment-wrap 454, semicolon 388, comment-case 56. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
From the low-confidence block of Copilot's fourth review of promotion #460, where it was raised as a README wording gap. The wording was the smaller half.
The defect
Blanking quoted spans per line suits a language whose strings end on the line they start. The C# verbatim string does not:
The marker on the second line is prose inside a string, and the linter read it as a comment and reported on it. A false positive, and the one direction that is worse than a miss, since it asks a reader to edit text that is data.
The fix
strip_stringsnow reports whether it ended inside a verbatim string, and the extractor carries that state as it already does for an unclosed block or documentation block. A line wholly inside one is skipped, and the line that closes it still gives back whatever follows the quote.The wording
The README claimed a marker inside a string literal is never a comment, which held only within a line. It now says blanking is per line, that the verbatim string is carried because it spans lines, and names the heredoc and the YAML block scalar as forms that are not carried, so a marker inside one of those still reads as a comment. Naming the remaining gap is better than a claim a reader would have to disprove.
Verification
Three cases: the marker inside the string reports nothing, the closing line still yields its trailing comment, and a plain string does not carry into the next line.
The first assertion was wrong when written - it expected a finding on a snippet that contains no comment - and the suite caught it before the commit. Corrected to assert the absence, which is what actually distinguishes the two readings.
69 self-tests and 19 repo_gate tests pass,
uvx mypy --strictis clean,repo_gateis clean,charsetanddupwordexit 0, and the warn-only backlog holds at comment-wrap 454, semicolon 388, comment-case 56.🤖 Generated with Claude Code