Strip the continuation asterisk only where it is one - #467
Conversation
A leading `*` was taken off every block comment body. That is the `/* */` convention for continuing a line, and it is prose everywhere else, so an emphasis marker in an HTML or PowerShell comment lost its opening character. The rule then read the damaged text: `<!-- *emphasis* leads here -->` became `emphasis* leads here`, a lowercase opening, and `comment-case` reported a finding the extractor had created. Stripping now happens only on a line continuing a `/* */` block, which is the line the convention describes. The opening line keeps its text in every syntax, C included, where a leading `*` is content rather than a continuation. Reported by Copilot on promotion #460, in the low-confidence block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a comment-extraction defect in scripts/prose_lint.py where leading * characters were being stripped from block comment bodies even when the * was part of the actual prose (e.g., Markdown emphasis markers), which could then trigger downstream prose lint findings that the extractor itself introduced.
Changes:
- Restrict leading-asterisk stripping to carried
/* */block continuation lines (rather than all block comment bodies and syntaxes). - Stop stripping leading
*from same-line block comment bodies (e.g.,/* *emphasis* */) so emphasis markers remain intact. - Add a regression test covering the previously-reported false positives and pinning the intended continuation behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/prose_lint.py | Narrows where leading * stripping is applied during block comment extraction to avoid damaging prose. |
| scripts/test_prose_lint.py | Adds regression coverage for emphasis-marker preservation and /* */ continuation handling. |
Scoping the strip to `/* */` continuation lines fixed where it ran and left
`lstrip('*')` doing the stripping, which takes every leading `*` it finds. A
continuation line carrying its own emphasis lost it: `**bold** here` became
`bold** here`, and `*emphasis* here` became `emphasis* here`.
The marker is one `*` followed by a space, so matching that shape exactly leaves
prose alone. A line opening with `**` or with `*` against a word is not the
marker and keeps every character.
Reported by Copilot on #467.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
scripts/prose_lint.py:418
- The comment says the continuation marker is "one
*and a space", but the code intentionally accepts any whitespace viabody[1:2].isspace(). Consider updating the comment to say "whitespace" so it matches the implementation (and avoids implying tabs or other whitespace would behave differently).
# Only `/* */` continues a line with a leading `*`, and only on a line it continues.
# Taking it off anywhere else edits the prose the rules then judge.
# The marker is one `*` and a space, so `**bold**` and `*emphasis*` keep theirs.
if closing == '*/' and body.startswith('*') and body[1:2].isspace():
body = body[1:].strip()
scripts/test_prose_lint.py:380
- This test comment says the continuation marker is "one
*and a space", but the implementation strips when the*is followed by any whitespace (isspace()). Update the comment to match the actual behavior.
# The marker is one `*` and a space, so a continuation line keeps its own emphasis.
for text, body in ((' * **bold** here */', '**bold** here'),
(' **bold** here */', '**bold** here'),
(' *emphasis* here */', '*emphasis* here')):
Both comments described the continuation marker as one `*` and a space, while `isspace()` accepts a tab as well. A tab-indented continuation is stripped, and the comment said it would not be. The behavior is the one worth keeping, so the words move to it rather than the other way round. From the low-confidence block of the round-two review (#467). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Answering the two round-two low-confidence findings. They are the same point against both copies of the comment, and both are right. Fixed in 33d3379. The comments said the marker is one A tab-indented continuation is stripped, and the comment said it would not be. Accepting whitespace is the behavior worth keeping, since a continuation indented with a tab is a real thing to write and there is no reason to treat it differently, so the words moved to the code rather than the code to the words. 71 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 fifth review of promotion #460, reported twice against the two places that did it.
The defect
A leading
*was taken off every block comment body. That is the/* */convention for continuing a line, and it is ordinary prose in every other block syntax.The extractor removed the emphasis marker, which left a lowercase opening, and
comment-casethen reported a finding the extractor had created. A false positive built out of damaged text, which is worse than a plain false positive because the reported prose is not what the file contains.The fix
Stripping happens only on a line that continues a
/* */block, which is the line the convention actually describes. The opening line keeps its text in every syntax, C included, since a*right after/*is content rather than a continuation. The/**documentation form is unaffected, being skipped before this point.Verification
Four assertions, the first three watched failing against
develop, each reporting a spuriouscomment-case:<!-- *emphasis* leads here -->['comment-case'][]<# *emphasis* leads here #>['comment-case'][]/* *emphasis* leads here */['comment-case'][]/* Start here.+* Still going. */The fourth pins the convention where it does apply, so the fix cannot be read as removing it.
This is pre-existing rather than introduced by the promotion's earlier fixes.
.lstrip('*')dates to d4c4085, the commit being promoted, so it reachesmainfor the first time with it.71 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