Conversation
…very language (#459) The flat non-ASCII ban 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. Three tiers give it that vocabulary. Tier 1 carries no meaning its ASCII form loses and always flags. Tier 2 is an operator, kept beside a number or another operator and replaced between words. Tier 3 is a unit symbol whose ASCII form would be a lie and never flags. A character in no tier is reported rather than passed. 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 replacement becomes "restructure the sentence", and the spaced hyphen is banned in its own right. Comments get a gate covering every syntax the fleet types carry: // and /* */, <!-- -->, <# #>, ;, and #. JSON is read as JSONC because that is what ships, which recovered four invisible files including the task and devcontainer snippets downstream repos copy. Their 37 malformed comments are fixed here. Python uses tokenize so a trailing comment is seen exactly, and every other language masks quoted spans. Two comment rules follow: one sentence per line, never wrapped and never two on a line, and a line that opens prose starts with a capital. The Verification Discipline mechanisms land too, including that a gate has to be watched failing and that a check is scoped by what the project declares rather than by the file that prompted it. Six review rounds. The last three had empty inline threads and real findings only in the low-confidence block: the comment rules ignored fenced blocks, a trailing comment could not start a wrapped sentence and reported the wrong rule, a sentence ending in an acronym was invisible, and a lowercase second sentence escaped both rules. The runbook and the merge gate now require investigating that block, since a loop polling review threads reports a clean pass while those stand. 61 self-test cases. Warn-only backlog reported without gating: dash 963, comment-wrap 454, semicolon 388, comment-case 56.
There was a problem hiding this comment.
Pull request overview
Promotes governance and linting changes to main by replacing the flat “ASCII-only” prose lint with a tiered charset model, expanding prose/comment lint coverage, and wiring the updated gates into CI and repo guidance docs.
Changes:
- Replace the prior
asciirule with tieredcharset/charset-unknownhandling, and add new prose/comment rules (dash,comment-wrap,comment-case) with expanded test coverage. - Update CI (
validate-task.yml) to gate oncharset+dupwordand report remaining prose/comment backlogs as warn-only. - Refresh governance + snippets documentation/comments to reflect the new rules and review-loop requirements.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
scripts/prose_lint.py |
Implements tiered charset checks plus new dash/semicolon/comment linting and comment extraction across file syntaxes. |
scripts/test_prose_lint.py |
Updates and significantly expands unit tests for tiered charset behavior, dash/semicolon policy, and comment linting. |
scripts/README.md |
Documents updated prose_lint.py rule set, tiers, scope, and CI posture (gate vs warn-only). |
.github/workflows/validate-task.yml |
Switches gating checks to charset + dupword and adds a warn-only backlog reporting step. |
GOVERNANCE.md |
Updates comment-style guidance and replaces the charset/semicolon/dash rules with tiered charset language + new constructions. |
.github/copilot-instructions.md |
Adds instruction to review “low confidence” suppressed findings in Copilot review bodies. |
.markdownlint-cli2.jsonc |
Comment wording improvements in markdownlint config. |
host-setup/agent-safety/.markdownlint-cli2.jsonc |
Comment wording improvements in nested markdownlint config. |
docs/token-cost.md |
Updates statement about which prose rules are warn-first vs gating. |
catalog/snippets/vscode/base.jsonc |
Reformats snippet comments to comply with the new comment-shape rules. |
catalog/snippets/vscode/docker.jsonc |
Reformats snippet comments to comply with the new comment-shape rules. |
catalog/snippets/devcontainer/python/devcontainer.json |
Reformats snippet comments to comply with the new comment-shape rules. |
catalog/snippets/devcontainer/dotnet/devcontainer.json |
Reformats snippet comments to comply with the new comment-shape rules. |
catalog/snippets/configs/vscode-tasks.json |
Reformats snippet comments to comply with the new comment-shape rules. |
catalog/snippets/configs/vscode-tasks-python.json |
Reformats snippet comments to comply with the new comment-shape rules. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
scripts/prose_lint.py:399
- In extracted_comments(), block comment openers are searched before line comment markers. As written, a
/*(or<#) that appears inside a// ...(or# ...) line comment can be misinterpreted as starting a real block comment, causing subsequent code lines to be linted as comment prose until a*/(or#>) appears.
body = (line[at + len(opener):end if end >= 0 else None]).strip().lstrip('*').strip()
if body:
out.append((n, body, leading))
closing = '' if end >= 0 else closer
if closing:
scripts/test_prose_lint.py:295
- There is no regression test covering comment-marker precedence (e.g., a
/*sequence inside a// ...line comment). Without a test, extracted_comments() can regress back into treating that/*as a real block opener and linting subsequent code lines as comment prose.
def test_a_marker_inside_a_string_is_not_a_comment(self) -> None:
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](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
GOVERNANCE.md:186
- The docs say developer-typed Unicode (e.g. emoji) is preserved regardless of tier, but the implementation and tests treat an un-tiered emoji (U+2603) as a charset-unknown finding. Clarify the doc so it matches the current behavior (preserved in-place but still reportable as charset-unknown).
- **Unicode the developer deliberately typed** stays regardless of tier - emoji used for emphasis or as callout markers, for example the warning markers a maintainer placed in `README.md`. Never strip the developer's own characters. This carve-out is for developer-authored text, not a license for the agent to add emoji.
- **An unrecognized non-ASCII character is reported, not allowed.** Classify it into a tier above before using it. A gate that passes whatever it does not recognize stops gating as the character set grows, which is the silent-narrowing failure named under "Verification Discipline".
scripts/README.md:24
- This sentence says developer-typed characters such as emoji are preserved regardless of tier, but prose_lint currently reports an un-tiered emoji (U+2603) as charset-unknown. Consider rewording to reflect that developer Unicode is preserved in-place but can still be surfaced as charset-unknown when not tiered.
Enforces the [`GOVERNANCE.md`](../GOVERNANCE.md) "Documentation Style Conventions" rules that no linter checks: non-ASCII judged against the charset rule's three tiers, a semicolon in prose, a spaced hyphen joining or interrupting a sentence, a duplicated consecutive word, and the shape of a comment's prose.
The tiers decide by context rather than by a flat ban. Tier 1 carries no meaning its ASCII form loses and always flags. Tier 2 is an operator, kept next to a figure or another operator and replaced between words, so a threshold table reads as the range it is. Tier 3 is a unit or scientific symbol whose ASCII form would be a lie and never flags. Developer-typed characters such as emoji are preserved regardless of tier.
A character in no tier is a `charset-unknown` finding rather than a silent pass, since a gate that allows whatever it does not recognize stops gating as the character set grows. Classifying one is a fleet-law edit, so CI surfaces it without blocking on it.
scripts/prose_lint.py:270
- strip_strings() treats backslashes as escapes in all quoted spans, which breaks C# verbatim strings (e.g. @"C:\tmp") where backslash is literal. That can cause the closing quote to be masked and make trailing // comments invisible to the comment-wrap/comment-case extractor.
if ch == '\\' and quote:
escaped = True
out[i] = ' '
continue
if quote:
scripts/test_prose_lint.py:278
- Comment extraction has a regression risk for C# verbatim strings: strip_strings() currently treats backslash as an escape inside any quoted span, which can hide a trailing // comment after a verbatim string ending in a backslash (e.g. @"C:\tmp"). Adding a case here would lock in the intended behavior for C# syntax.
for name, text in (
('a.cs', f'// {self.RUN_ON}\n'),
('a.cs', f'/* {self.RUN_ON} */\n'),
('a.cpp', f'// {self.RUN_ON}\n'),
('a.c', f'/* {self.RUN_ON} */\n'),
Copilot's finding on promotion PR #460, fixed at the source. That PR's head is `develop`, so it picks this up when this squashes in. ## The contradiction Two adjacent bullets in "Documentation Style Conventions" disagreed on the emoji case: > **Unicode the developer deliberately typed** stays regardless of tier [...] Never strip the developer's own characters. > **An unrecognized non-ASCII character is reported, not allowed.** An emoji is both developer-typed and un-tiered, so the two bullets pointed opposite ways. The implementation already had a coherent answer: ``` un-tiered emoji: [(1, 'charset-unknown', 'SNOWMAN (U+2603) is in no tier - classify it in GOVERNANCE.md')] ``` It is preserved in place and never rewritten, and it is still reported until classified. The doc simply never said the second half, so the first bullet read as an exemption from the gate rather than from rewriting. ## The fix The carve-out is about what an agent may rewrite, not about what the gate reports. Saying that leaves both bullets true and needs no change to either rule. `scripts/README.md` restated the same claim in one sentence, so it takes the same clause. The spaced hyphen in the edited bullet is recast at the same time, under the rule three bullets below it that existing prose is corrected as each file is next edited rather than swept. ## Why it blocks a promotion This is fleet law on its way to `main`, and `main` is what the audit reads as ground truth. Shipping a self-contradicting rule is what leaves downstream repositories arguing the point, and the contradiction already produced one 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. No behavior change, so no test accompanies it. 62 self-tests and 19 repo_gate tests pass, `repo_gate` is clean, and `charset` plus `dupword` exit 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Answering the four findings in the round-two review's low-confidence block, since a suppressed finding has no thread to reply on. All four are right. They are two defects seen twice each, one fixed here and one deferred with a reason. The carve-out contradiction (
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
scripts/prose_lint.py:270
- strip_strings() only treats backslash as an escape. For C# verbatim strings (e.g. @"..."), embedded quotes are doubled (""), so this loop will treat the first quote in the pair as closing the string and can mis-read later comment markers on the same line as real comments.
if quote:
out[i] = ' ' if ch != quote else ch
if ch == quote:
quote = ''
elif ch in quotes:
scripts/prose_lint.py:399
- extracted_comments() can only extract at most one block comment per line, and it cannot see a line comment that appears after a block comment on the same line (e.g.
code /* ... */ // tail). Because marker searches always start at column 0 (find()with no start index) andcutis set to the block opener position, any later comment on the line is skipped, reducing comment-wrap/case coverage.
leading = True
for opener, closer in spec['block']:
at = masked.find(opener)
if 0 <= at < min(cut, line_at):
if any(line[at:].startswith(d) for d in spec['doc']):
) 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>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
scripts/test_prose_lint.py:452
- The PR description says the C# verbatim-string comment-masking defect (#462) is deferred, but this change set appears to include verbatim-string handling already (SYNTAX['.cs']['verbatim'] plus the new strip_strings() verbatim logic and the associated tests). This is a discrepancy between the PR description and the actual diff; please update the PR description (or clarify what part remains deferred) so reviewers and downstream consumers don’t misinterpret the promotion scope.
def test_a_verbatim_string_keeps_its_own_closing_quote(self) -> None:
"""A backslash is ordinary inside one and a doubled quote is the escape.
Read with C escape rules the string never closes, so the masker blanks the rest of the
line and the trailing comment goes unseen.
"""
# Ending in a backslash, the string swallows its closing quote and hides a real comment.
self.assertEqual(['comment-wrap'],
self.flag('a.cs', 'var p = @"C:\\tmp\\"; // Two things. Here.\n'))
# Reading a doubled quote as a close then a reopen puts string content outside the string.
self.assertEqual([],
self.flag('a.cs', 'var s = @"a""// One thing. Another thing.""b"; // ok\n'))
# An interpolated one is spelled either way round, and only one of them abuts the quote.
for text in ('var s = $@"C:\\tmp\\"; // Two things. Here.\n',
'var s = @$"C:\\tmp\\"; // Two things. Here.\n'):
with self.subTest(line=text.strip()):
self.assertEqual(['comment-wrap'], self.flag('a.cs', text))
def test_only_the_syntax_that_has_verbatim_strings_gets_them(self) -> None:
"""C shares the C-like spec without the form, so `@` there is an ordinary character."""
self.assertTrue(prose_lint.SYNTAX['.cs']['verbatim'])
self.assertFalse(prose_lint.SYNTAX['.c']['verbatim'])
self.assertFalse(prose_lint.SYNTAX['.json']['verbatim'])
# The C escape still hides a marker, which is what the verbatim rule must not undo.
self.assertEqual(['comment-wrap'],
self.flag('a.cs', 'var s = "a\\"b"; // Two things. Here.\n'))
scripts/README.md:44
- The README states broadly that “A marker inside a string literal is not a comment”, but the implementation blanks quoted spans on a per-line basis (see scripts/prose_lint.py:253-295). For syntaxes that allow multi-line strings (notably C# verbatim strings), comment markers on subsequent lines inside the string can still be misread as comments. Tighten the wording here to reflect the line-scoped behavior so consumers don’t assume full multi-line string tracking.
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. A documentation comment (`///`, `/**`, a docstring) is left to CODESTYLE, which permits the paragraphs this rule forbids.
|
Answering the two findings in the round-four low-confidence block.
#462 is closed. Nothing to change, and I am noting it rather than silently ignoring it, since a stale finding is still worth answering.
That is a false positive, the direction that is worse than a miss, since it asks a reader to edit text that is data. Fixed in #465 rather than reworded around: The README takes the wording fix too, and goes further than the finding asked. 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. Naming the remaining gap beats a claim a reader would have to disprove. This PR picks both up when #465 squashes in. |
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: ``` var s = @"line one // Two things. Here. <- string content line three"; ``` ``` before : [(2, 'Two things. Here.', True)] after : [] ``` 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_strings` now 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 --strict` is clean, `repo_gate` is clean, `charset` and `dupword` exit 0, and the warn-only backlog holds at comment-wrap 454, semicolon 388, comment-case 56. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
scripts/prose_lint.py:464
- When extracting block comments, the body is normalized with .lstrip('') for all block syntaxes. This is appropriate for C-style '/ ... /' where interior lines are often prefixed with '', but it should not be applied to other block comment syntaxes (e.g., ''), since it can alter the actual comment text and affect comment-wrap/comment-case detection.
opener, closer = found
end = line.find(closer, at + len(opener)) # a quote in the comment is prose
body = (line[at + len(opener):end if end >= 0 else None]).strip().lstrip('*').strip()
if body:
out.append((n, body, leading))
scripts/prose_lint.py:417
- extracted_comments() unconditionally strips leading '' from block-comment bodies. That behavior is specific to C-style block comments (lines commonly prefixed with ''), but it will also mutate non-C block comment syntaxes like HTML comments ('') and PowerShell blocks ('<# ... #>'), which can change what the comment-prose rules see and lead to incorrect lint results.
This issue also appears on line 460 of the same file.
elif closing: # carried in from an unclosed block
end = line.find(closing)
body = (line if end < 0 else line[:end]).strip().lstrip('*').strip()
if body:
out.append((n, body, True))
|
Answering the two findings in the round-five low-confidence block. They are the same defect reported against both places that do it, and both are right. Fixed in #467. A leading The extractor removed the emphasis marker, that left a lowercase opening, and Stripping now happens only on a line continuing a This one is pre-existing. This PR picks it up when #467 squashes in. |
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. ``` <!-- *emphasis* leads here --> -> 'emphasis* leads here' -> ['comment-case'] <# *emphasis* leads here #> -> 'emphasis* leads here' -> ['comment-case'] /* *emphasis* leads here */ -> 'emphasis* leads here' -> ['comment-case'] ``` The extractor removed the emphasis marker, which left a lowercase opening, and `comment-case` then 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 spurious `comment-case`: | case | before | after | | ---- | ------ | ----- | | `<!-- *emphasis* leads here -->` | `['comment-case']` | `[]` | | `<# *emphasis* leads here #>` | `['comment-case']` | `[]` | | `/* *emphasis* leads here */` | `['comment-case']` | `[]` | | `/* Start here.` + ` * Still going. */` | stripped | stripped | 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 reaches `main` for the first time with it. 71 self-tests and 19 repo_gate tests pass, `uvx mypy --strict` is clean, `repo_gate` is clean, `charset` and `dupword` exit 0, and the warn-only backlog holds at comment-wrap 454, semicolon 388, comment-case 56. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three things that cost a wrong turn while driving promotion #460, none of them written down anywhere. Each is recorded as the general rule plus the concrete symptom, so the next agent recognizes it rather than rediscovering it. ## A closing keyword never fires under this branching model GitHub closes a referenced issue only on a merge into the repository's **default** branch, and every feature pull request here targets `develop`. #464 carried `Closes #462` and merged, and #462 stayed open, reading as unfixed until it was closed by hand. The promotion that later reaches `main` carries the commit rather than the keyword, so it does not close it either. This is fleet law rather than a Copilot mechanic, and it binds any agent regardless of provider, so per this file's own rule it belongs in [`GOVERNANCE.md`](../GOVERNANCE.md) rather than the runbook. It is a **top-level** branching-model rule rather than a third entry under "Executing a `develop -> main` promotion safely", because the pull request it bites is a feature one merging into `develop`, not the promotion itself. That parent says "two traps" and means it. ## `gh pr edit` is broken by the classic-Projects sunset It fails with `GraphQL: Projects (classic) is being deprecated ... (repository.pullRequest.projectCards)` and mutates nothing. That matters during a long review loop, where the promotion body goes stale as fixes land under it. The runbook now names `gh api -X PATCH \"repos/<owner>/<repo>/pulls/<N>\" -F body=@<file>` and says to read the body back afterwards, since a failed call is not evidence the pull request is untouched. Filed as its own list rather than added to the existing one, which is specifically about **review request** paths. This is an edit the loop makes between rounds. ## A poll that tests a captured result against `!= \"0\"` reads empty as success An empty string is exactly what a mis-written `--jq` filter returns, so the two cases that must be distinguished, a query finding nothing and a query running wrong, both satisfy the test. One such poll during #460 reported a review that had not landed, and the next message said so before the correction. Counting matches and testing `-gt 0` makes them read alike. The specific trap named with it: `--arg` is a `gh api graphql` flag. Passing it to a plain `gh api --jq` call fails with `accepts 1 arg(s), received 4` while the surrounding `$(...)` still yields the empty string. Placed in "Verify Review Covered Current Head", one paragraph above the existing warning about exiting on `mergeStateStatus`, since both are ways a poll exits early on a false signal. ## Verification Documentation only, no behavior change, so no test accompanies it. 71 self-tests and 19 repo_gate tests pass, `repo_gate` is clean, `charset` and `dupword` exit 0, and the warn-only backlog is unchanged at dash 962 and semicolon 388, so the new prose adds no findings of its own. ## Fidelity note Both files carry verbatim sections, so downstream copies are **stale** until re-vendored. That is already true of them from #460. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
From the low-confidence block of Copilot's second review of promotion #469. It flagged one duplicate. Checking the other two additions from #468 found a second, and the second is worse than a duplicate. ## What #468 got wrong That pull request said it was recording three mechanics "none of them written down before". I did not grep the files before writing to them. **`gh pr edit`** was already covered at `.github/copilot-instructions.md` "PR Edits and Merge-State Gotchas", which gives **both** the GraphQL and the REST form and says to verify the edit took. The existing entry is better than the one I added beside it, so the addition goes and the original stays untouched. **Issue-closing keywords** were already covered in `GOVERNANCE.md` under the release model, and that entry is not merely earlier but **correct where mine was not**: > Issue-closing keywords (`Closes #N`, `Fixes #N`) go in the `develop -> main` promotion PR, not the feature -> develop PR. That is a working mechanism. My branching-model bullet said to close the issue by hand instead. Two bullets, one topic, **different procedures**, which is a contradiction in fleet law rather than a repetition, and exactly the drift Copilot warned the duplicate would cause. It also means my reply on #470, that the promotion mechanism was untested and so the rule should stay silent on it, was answering a question this repository had already answered. #462 could have been closed by putting the keyword on promotion #460. ## What survives The release-model rule gains the one thing mine had that it lacked: the fallback for a develop pull request that already merged with a keyword on it. That is the case #462 actually hit, and without it a reader who has already made the mistake finds no instruction. The polling guard from #468 **stays**. Guarding an empty bot node id before a mutation is documented in two places already, but the **exit test of a poll loop** is not, and the numeric comparison is what stops an empty result reading as a landed review. Kept as the one genuinely new thing in that commit. ## Why this happened, and what would prevent it The closing-keyword rule lives under "Release Model" while the question I was answering was a branching one, so reading the Branching Model section did not surface it. That is a findability problem in a 400-line governance file rather than an excuse: a `grep` for the topic would have found it in either section, and adding to a rules file without grepping it first is the actual failure. Not proposing a reorganization here. Moving committed rule text between sections changes two verbatim sections at once and every downstream copy with them, which is not something to ride along with a fix. ## Verification ``` gh pr edit mentions -> 1 (copilot-instructions.md:209) closing-keyword bullets -> 1 (GOVERNANCE.md:83) ``` 71 self-tests and 19 repo_gate tests pass, `repo_gate` is clean, `charset` and `dupword` exit 0. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Promotes `f35e8c7` (#468), `a5e12a2` (#470), and `7477c42` (#472). Conflict-free, three commits ahead. The net change against `main` is **three lines in two files**. Reviewing this promotion is what reduced it to that, so the history and the result are described separately below. ## What actually lands **A polling guard, in the Copilot review runbook.** A poll that captures a `gh api --jq` result and exits on `[ "$found" != "0" ]` treats an **empty** string as a landed review, and an empty string is what a mis-written filter returns. Counting matches and testing `-gt 0` makes a query that finds nothing and a query that ran wrong read alike. One such poll during #460 reported a review that had not landed. **A fallback on the existing issue-closing rule, in `GOVERNANCE.md`.** The rule already said to put `Closes #N` on the promotion pull request rather than the feature one. It did not say what to do once a develop pull request has already merged carrying the keyword. It now does: move the keyword to the promotion body, and close by hand only when the promotion has merged without it. That is the case #462 hit. ## What was reverted, and why that matters more #468 opened claiming three mechanics "none of them written down before". Two of them **were**, in the files it edited. `gh pr edit` being broken by the classic-Projects sunset was already documented under "PR Edits and Merge-State Gotchas", with both the GraphQL and the REST form. The addition was a plain duplicate and is gone. Issue-closing keywords were already documented under the release model, and that entry is **correct where the addition was wrong**. It gives a working mechanism, the keyword on the promotion pull request. The added branching-model bullet said to close the issue by hand instead. Same topic, two sections, different procedures, which is a contradiction in fleet law rather than a repetition, and is the drift a duplicate is supposed to risk only later. That bullet is gone and the surviving rule absorbed the one thing it lacked. Two further corrections landed on the way. The closing-keyword bullet first said a keyword "never fires under this model", which is false because a pull request merging **into** `main` closes what it references. The polling note called `--arg` a `gh api graphql` flag, which is false because `--arg` belongs to `jq` and `gh api graphql` takes `-f` and `-F`. Both files are vendored verbatim across the fleet, so either statement would have propagated on the next re-vendor. ## Why promote now What survives is small and true. The polling guard is the only new mechanic in the set, and it is the one that produced a wrong report during #460 rather than a hypothetical. The `GOVERNANCE.md` sentence completes a rule that was already right by covering the state a reader reaches only after getting it wrong. `main` is what a newly scaffolded or realigning repository carries, and what the fleet audit reads as ground truth, so a rule that is complete on `develop` and partial on `main` is a rule that argues with itself across the fleet. ## Fidelity note `GOVERNANCE.md` and `.github/copilot-instructions.md` both carry verbatim sections, so downstream copies stay **stale** until re-vendored. They were already stale from #460 and this does not change that state, only its size, which is now three lines rather than the eleven #468 first proposed. Documentation only, no behavior change. 71 self-tests and 19 repo_gate tests pass, `repo_gate` is clean, `charset` and `dupword` exit 0, and the warn-only backlog is unchanged at dash 962, comment-wrap 454, semicolon 388, comment-case 56. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Promotes
d4c4085(#459),adaa068(#461),e52c68a(#463),7bf8a12(#464),269629a(#465), and036e468(#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-unknownwhile 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
mainis 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 -->becameemphasis* leads hereandcomment-casereported 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 tod4c4085rather than to any fix made during this review, so the promotion is where it would first reachmain.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
maincarries 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
reviewThreadswould have reported a clean pass on both rounds.Fidelity note
GOVERNANCE.mdand.github/copilot-instructions.mdsections declared verbatim changed, so downstream copies are stale until re-vendored.spec/fidelity_honesty.pyseparates 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