Skip to content

Bound block-comment scanning by the earliest line-comment marker - #461

Merged
ptr727 merged 3 commits into
developfrom
fix/comment-precedence-line-before-block
Jul 30, 2026
Merged

Bound block-comment scanning by the earliest line-comment marker#461
ptr727 merged 3 commits into
developfrom
fix/comment-precedence-line-before-block

Conversation

@ptr727

@ptr727 ptr727 commented Jul 30, 2026

Copy link
Copy Markdown
Owner

Copilot's finding on the promotion PR (#460), fixed at the source rather than on the promotion. That PR's head is develop, so it picks this up when this squashes in.

The defect

extracted_comments() searched block openers before line markers, against a ceiling of the whole line. A /* inside a // comment therefore opened a real block:

// Match a /* opener in the parser   ->  (1, 'opener in the parser', False)
int x = 1;                           ->  (2, 'int x = 1;', True)
// Done here                         ->  (3, '// Done here', True)

Line 1's own comment is truncated at the opener, line 2 is a code line handed to the comment rules as prose, and line 3 keeps its marker as block interior. PowerShell fails identically with <# inside a # comment. Those are the two fleet syntaxes carrying both marker kinds; CSS, XML, and the hash-only syntaxes carry one each and never reach the case.

The fix

Locate the earliest real line-comment marker first, then bound opener scanning by it. min(cut, line_at) keeps the reverse intact, so a // inside /* ... */ still belongs to the block, and the doc-marker skip is applied when locating the line marker so /// does not become the ceiling.

Verification

The case is watched failing against the old extractor before the fix goes in, where the bare assertion reports ['comment-wrap'] != ['comment-wrap', 'comment-wrap'] - the extra finding is exactly the swallowed code line, which is the defect stated as an assertion rather than as prose. Both affected syntaxes are covered.

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, and repo_gate plus the charset and dupword gates stay clean. That is also why the fix is worth taking rather than deferring: the false positives land downstream, in the C# and PowerShell repositories this extractor was written for, and they would surface the moment the comment rules gate rather than warn.

🤖 Generated with Claude Code

A line comment runs to end of line, so a block opener after one is text. The
extractor searched openers first against an unbounded ceiling, 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 fails the same way with `<#` inside a `#` comment. Those are the two
fleet syntaxes carrying both marker kinds; CSS, XML, and the hash-only syntaxes
carry one each and never reach the case.

`min(cut, line_at)` keeps the reverse intact, so a `//` inside a block still
belongs to the block, and the doc-marker skip is applied when locating the line
marker so `///` does not become the ceiling.

Tree-wide counts are unchanged - dash 963, comment-wrap 454, semicolon 388,
comment-case 56 - because nothing here nests the markers that way. The exposure
is downstream, in the C# and PowerShell repositories this extractor was written
for, and it would surface the moment the comment rules gate rather than warn.

The case is watched failing against the old extractor first, where it reports
the swallowed code line as a second finding.

Reported by Copilot on the promotion PR (#460).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 30, 2026 21:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a comment-extraction edge case in the prose linter where a block-comment opener appearing inside a line comment (e.g., /* inside //, or <# inside #) could incorrectly start a block comment and cause subsequent code lines to be linted as prose.

Changes:

  • Update extracted_comments() to locate the earliest real line-comment marker first and bound block-opener scanning to that position.
  • Add a regression test covering the affected C-like and PowerShell syntaxes.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
scripts/prose_lint.py Bounds block-opener scanning by the earliest line-comment marker to prevent false block-comment starts inside line comments.
scripts/test_prose_lint.py Adds a regression test ensuring block openers inside line comments are treated as text (not as real block openers).

Comment thread scripts/prose_lint.py
Excluding doc markers from the ceiling reopened the defect for the syntax whose
doc marker is itself a line comment. `/// See a /* opener here` left the ceiling
unbounded, so the opener started a block and the code below it was linted as
prose again - the same three-line failure, one marker along.

The doc check belongs to what gets emitted, not to what counts as a comment: a
`///` line runs to end of line like any other. Dropping it from the ceiling loop
leaves the emit-side skip in place, so a doc comment still yields no findings.

The case is watched failing against the previous commit before the change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 30, 2026 21:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:388

  • The new comment suggests documentation comments are "skipped as prose rather than as a comment", but in this function they still act as line comments for bounding while being skipped from emission/linting. Rewording would avoid implying they’re treated as prose.
        # A line comment runs to end of line, so a block opener after one is text.
        # Left unbounded it opens a block that swallows the code lines below.
        # A documentation comment bounds it too, being skipped as prose rather than as a comment.

scripts/test_prose_lint.py:374

  • Grammar: "must not unbound" should be "must not unbind".
        The documentation form is the same case: skipping it as prose must not unbound the

"Skipped as prose" reads as though a doc comment were prose, when the point is
the opposite: it is a comment here like any other, and only the linting exempts
it. The ceiling loop is the place that distinction matters, so the comment on it
should not invite the reading the code just stopped taking.

The test docstring said "must not unbound the ceiling", which is not a verb.

Both from the low-confidence block of the round-two review (#461).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 30, 2026 21:52
@ptr727

ptr727 commented Jul 30, 2026

Copy link
Copy Markdown
Owner Author

Answering the two findings in the round-two review's low-confidence block here, since a suppressed finding has no thread to reply on. Both are right and both are fixed in f8037ab.

prose_lint.py:388 - "skipped as prose" implies a doc comment is prose. Correct, and it inverts the point the commit before it just made. A doc comment is a line comment here, which is exactly why it bounds the ceiling; only the linting exempts it. Writing "skipped as prose" on the ceiling loop invites the reading the code had just stopped taking, which makes it the worst place to leave the ambiguity. Now reads "being exempt from linting rather than from here."

test_prose_lint.py:374 - "must not unbound" should be a verb. Correct. Reworded to "must not leave the ceiling unbounded", which also states the condition in the terms the code uses.

Worth noting that this block has now been load-bearing on both rounds of this PR: round one's single inline comment read as a wording nit and turned out to be a live defect, and round two had no inline comments at all. A loop polling reviewThreads would have reported a clean pass twice over. That is the case the merge gate and the runbook were changed to require, in the commit this branch is fixing.

62 self-tests and 19 repo_gate tests pass, repo_gate is clean, and charset plus dupword exit 0.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@ptr727
ptr727 merged commit adaa068 into develop Jul 30, 2026
7 checks passed
@ptr727
ptr727 deleted the fix/comment-precedence-line-before-block branch July 30, 2026 22:17
ptr727 added a commit that referenced this pull request Jul 31, 2026
)

Two findings from the low-confidence block of Copilot's third review of
promotion PR #460, fixed at the source. That PR's head is `develop`, so
it picks these up when this squashes in.

Closes #462.

## Every comment on a line, not just the first

The extractor searched each marker kind from column 0 against a ceiling.
A ceiling can only describe the *first* comment on a line, which is why
the same structure produced three defects in a row across this
promotion:

1. a marker quoted inside the first comment read as real (#461)
2. a doc marker excluded from the ceiling unbounded it (#461, second
commit)
3. anything after the first comment was unreachable (this PR)

```
var x = 1; /* Note. */ // Two things. Here.   ->  [(1, 'Note.', False)]
```

The trailing comment is dropped, so every comment rule is blind to it.

Rather than patch the third instance, this replaces the ceiling with a
single left-to-right pass over a cursor. Whichever marker comes first
wins, a line comment ends the line, and a closed block resumes the scan
after its closer. Two cases nobody asked for come with it: several
blocks on one line, and a comment trailing the line where a multi-line
block closes.

## Verbatim strings, both directions

The masker treated a backslash as an escape inside every quoted span. In
a C# verbatim string the backslash is ordinary and a doubled quote is
the escape, so it was wrong both ways:

```
var p = @"C:\tmp\"; // Two things. Here.        ->  masked to `var p = @"` , comment hidden
var s = @"a""// One thing. Another thing.""b";  ->  string content read as a comment
```

The first is a false negative, the second a false positive. C# takes its
own syntax entry for this, since the rest of the C-like family shares
the markers but has no verbatim form and `.js`, `.ts`, `.json`, and
`.jsonc` would be wrong to inherit it.

## Verification

Six new assertions, each watched failing against the extractor currently
on `develop`:

| case | before | after |
| ---- | ------ | ----- |
| `code /* Note. */ // run-on` | `[]` | flagged |
| `/* Note. */ /* run-on */` | `[]` | flagged |
| block closing mid-line, then `// run-on` | `[]` | flagged |
| `@"C:\tmp\"; // run-on` | `[]` | flagged |
| `@"a""// run-on""b"` | flagged | `[]` |
| `SYNTAX['.cs']['verbatim']` | `KeyError` | `True` |

65 self-tests and 19 repo_gate tests pass, `repo_gate` is clean, and
`charset` plus `dupword` exit 0.

The warn-only backlog is unchanged at comment-wrap 454, semicolon 388,
comment-case 56. The wider scan first reported comment-wrap 457, and the
three extra were this change's own new comments wrapping across lines,
which the rule forbids. They are rewritten one sentence per line rather
than left standing in the linter's own source.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
ptr727 added a commit that referenced this pull request Jul 31, 2026
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants