Carry a multi-line string in every syntax, and answer the router-split items

[ptr727]

Closes #466. Closes #471.

#466 - a multi-line string carries in every syntax

strip_strings carried state past the end of a line for the C# verbatim form only. Every other syntax was masked a line at a time, so a string that spans lines left its markers readable and comment-wrap / comment-case reported on string content, which is the false-positive direction: the rule asks a reader to edit text that is data.

Each syntax now declares two things: raw, the quotes whose strings take no backslash escape, and carry, the forms that survive a newline. The whole-line forms (a heredoc, a here-string, a block scalar) are resolved before the line is scanned for a marker, and the line that closes one gives back whatever follows the closer, the way a closing quote already did.

Syntax	Carries
shell, Dockerfile, makefile, git hook	an unclosed quote, and a heredoc (<<EOF, <<-EOF, <<'EOF', stacked on one line)
PowerShell	an unclosed quote, and a here-string (@" to "@, @' to '@)
YAML	a block scalar (|, >, with chomping and indent indicators), ended by dedent
C#	the verbatim string, unchanged

Two decisions worth review:

• A YAML run: scalar is deliberately not opaque. It holds a script whose # lines are exactly the comments this rule governs, so treating every block scalar as data would have silently dropped coverage of every workflow script in the fleet. A data key (files:, tags:, build-args:) is opaque.
• A form only carries where the language has it. A YAML plain scalar's apostrophe is not a string, so an ordinary quote does not carry there. Carrying it would blank the rest of the file, and a rule that reads nothing reports nothing, which reads exactly like a clean pass. There is a case for each syntax asserting this.

Also fixed while implementing: in shell a backslash outside a string escapes the next character, which is how '\'' embeds a quote. Without that the idiom leaves one quote open, and with carrying now on, that would have gone quiet for the rest of the file.

Verified no behavior change elsewhere: the tree-wide comment-wrap / comment-case finding set is identical before and after, apart from the files this PR edits.

#471 - the seven source-side items

1. The no-coordination-references rule justified itself with a fact that is no longer true. It said a link to the template repo 404s because the repo is private. It is public. The justification is recast onto the two grounds that hold: the coordination flow is machinery a consumer should never see, and derived-from framing is already forbidden by the present-tense rule.
2. OPERATIONS.md is in the section model. A new "Where repo-specific content goes" section names the three topical docs and what each takes, so extraction has a predictable target instead of a per-repo judgment call. The README backlog item that asked for this is retired.
3. The reference-style-links rule contradicted the file it is written in. GOVERNANCE.md has 51 inline links and no reference definitions, so it is added to the exception list, which is what its actual shape reflects. The exception is now stated as a closed list rather than a category, so the next repo does not have to re-derive whether its file qualifies.
4. OPERATIONS.md joins that list, as agent-instruction content extracted out of AGENTS.md.
5. The semicolon class is swept out of the verbatim sections. All 32 in GOVERNANCE.md; AGENTS.md was already clean. The governing rule is recorded with it: correct-as-you-next-edit assumes someone able to edit the file, and a downstream repo cannot edit a verbatim section, so the hub sweeps the class rather than waiting for an edit that can never happen there. Scoped explicitly to verbatim content, so intent-fidelity files are still corrected as they are next edited.
6. spec/audit.py --branch REF reads that ref instead of the registry groundTruthBranch, so a convergence is verifiable before it is promoted, with no registry edit. The hand-rolled argv parse is replaced with argparse because the old one took every non--- argument as a repo name, so --branch develop would have audited a repo named "develop". A ref that does not resolve is one error naming it, rather than every ?ref= read 404ing into a baseline's worth of file-absent letters.
7. The Financial-Modeling driftNotes are reconciled against a live audit run, with each named deliverable confirmed present on main. One correction to the report: status staying cataloged is not stale. The registry schema offers only cataloged and backlog, and all 21 repos are cataloged, so there is no converged state to move to.

Both offered rules are promoted into "Verification Discipline", where the other silent-failure rules live: config with a uniqueness rule is validated on read, and validate-and-read on the same normalized key. Neither was present in the canonical (grepped for uniqueness, duplicate, normalize, lookup, precedence across all four rule books before adding).

The triage-method note is recorded in the section model as "Migrating a repo onto the split": probe the canonical for each local rule's distinctive phrase, because a word-overlap heuristic scores a repo-specific rule written in ordinary governance vocabulary as a reworded duplicate, and it is confidently wrong in the direction that loses content.

Coverage and tests

• scripts/test_pr_review.py is new. pr_review.py had no test module at all while scripts/README.md claimed every script had one. Its failure mode is a wrong answer rather than a crash, so the cases feed crafted GraphQL payloads: a review attributed to the wrong login, a review counted against a stale head, a maintainer's own thread read as a finding, and a wait that returns success while nothing landed. One case reads the reviewer login out of the runbook rather than restating it, and one asserts no mutation has crept into a read-only script.
• changed_lines had zero tests. It is the whole mechanism behind --diff scoping, and a parse that returns too little makes the warn-only CI step silently stop reporting.
• The sentence-split rule had zero tests.
• spec/audit.py --selftest is offline and was never run by CI. It runs there now, with new cases for the CLI parse, the branch override, and the unresolvable-ref path.
• Coverage over scripts/: 87% to 99%, 104 to 150 cases.

Not in scope, deliberately

• The dash class. Item 5's argument applies to it identically, but it is roughly 350 instances across GOVERNANCE.md, CODESTYLE.md, WORKFLOW.md, and AUDIT.md, which is a prose rewrite that wants its own review rather than riding along here. Recorded in the README backlog.
• .gitattributes' own comment backlog. The change to that file is a single data line (the LF pin for the new test module), and reworking its 14 comment findings would bury the diff. The standing correct-as-you-edit policy covers it.
• The .markdownlint-cli2.jsonc drift the Financial-Modeling audit reports. That is a downstream re-vendor, not a hub change.

Verification

Every gate run locally and green: the 150-case suite, spec/audit.py --selftest, spec/validate.py, scripts/repo_gate.py, prose_lint --check charset --check dupword, markdownlint over **/*.md, cspell, actionlint, editorconfig-checker, mypy, and ruff (unchanged finding count on every touched file). CRLF preserved on every CRLF file, LF on every pinned one, verified by byte scan.

Note that merging this re-vendors 12 GOVERNANCE.md verbatim sections across the fleet. That drift wave is the mechanism working.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR updates the repo’s prose/comment linting and governance/audit tooling to (1) correctly treat multi-line string forms as opaque across all supported syntaxes (avoiding false-positive comment findings inside string data), and (2) incorporates the remaining source-side improvements identified during the router split convergence, including stronger verification rules and substantially expanded test coverage.

Changes:

• Extend prose_lint’s string masking to carry multi-line string state across lines for shell (quotes + heredocs), PowerShell (quotes + here-strings), YAML (block scalars, excluding run:), and C# (verbatim strings).
• Add spec/audit.py --branch REF and tighten audit behavior for unresolvable refs (single error instead of baseline-wide noise), with corresponding self-tests.
• Add/expand unit tests across scripts/ and wire them into CI validation; reconcile and refine governance/docs/registry prose per the router-split findings.

Reviewed changes

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

Show a summary per file

File	Description
spec/section-model.md	Records repo-specific extraction destinations (including OPERATIONS.md) and migration guidance for the router split.
spec/audit.py	Adds --branch override, replaces ad-hoc argv parsing with argparse, resolves branch refs early, and expands self-tests.
scripts/test_repo_gate.py	Adds CLI-oriented coverage and additional harness assertions for repo gate behavior.
scripts/test_prose_lint.py	Adds extensive multi-line string, syntax-dispatch, sentence-split, discovery, and diff-scope tests.
scripts/test_pr_review.py	New test module covering pr_review.py parsing, polling correctness, and transport/read-only contract.
scripts/README.md	Updates script documentation and test invocation list; aligns link style with updated governance exceptions.
scripts/prose_lint.py	Implements carried multi-line string masking across syntaxes; extends syntax metadata; improves rule comments.
scripts/pr_review.py	Makes CLI entrypoint testable by accepting argv; clarifies liveness-query comments.
registry/repos.json	Reconciles drift notes for Financial-Modeling to reflect the completed convergence.
README.md	Updates backlog item to reflect follow-on “dash” sweep scope as intentionally separate.
GOVERNANCE.md	Applies router-split source-side rule fixes, clarifications, and promoted verification-discipline rules.
AUDIT.md	Documents spec/audit.py --branch as a supported verification workflow for pre-promotion convergence checks.
.github/workflows/validate-task.yml	Runs the expanded script self-tests and spec/audit.py --selftest in CI.
.gitattributes	Pins LF for the new scripts/test_pr_review.py module.

Suppressed comments (1)

scripts/prose_lint.py:400

• PowerShell here-strings require the closing token ("@ / '@) to start at column 0. Using line.lstrip() allows an indented closer to terminate the here-string early, which can re-enable comment scanning on lines that are still part of the here-string body.

    if carry.kind == 'here':
        body = line.lstrip()
        if not body.startswith(carry.text):
            return carry, None
        return CLEAR, len(line) - len(body) + len(carry.text)

[ptr727]

Round 1 answered, including the suppressed finding

Both inline threads are fixed in b1a2cc8 and replied to on the thread. The suppressed one has no thread to reply on, so it is answered here.

Suppressed: scripts/prose_lint.py:400, an indented here-string closer

PowerShell here-strings require the closing token ("@ / '@) to start at column 0. Using line.lstrip() allows an indented closer to terminate the here-string early.

Valid, and fixed in the same commit. resume_at now tests line.startswith(carry.text) against the raw line, so an indented "@ is here-string content. A case covers it: a body line beginning "@ is not the closer, followed by the bait sentence, followed by the real closer at column 0.

Worth noting that this and the inline heredoc finding are one class, which is why they are fixed together: a terminator matched after stripping indentation ends the string on a line that is still its content, and the rule then reports on data, which is the false-positive direction the issue set out to remove. The heredoc case had the extra wrinkle that the two openers genuinely differ, so the <<- flag is carried from the opener rather than the terminator being matched leniently for both. Both directions are now pinned: the near-misses that must not terminate, and the tab-indented terminator that must.

For the record, the third round-1 finding (the mutable default in the test payload helper) is fixed too, and the whole suite, spec/audit.py --selftest, spec/validate.py, scripts/repo_gate.py, the prose gate, mypy, and the linter set are green locally on the new head.

[Copilot]

Pull request overview

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

Suppressed comments (1)

scripts/prose_lint.py:211

• For YAML, single-quoted scalars treat backslash as an ordinary character (no backslash escapes). With raw left empty here, strip_strings() will treat a backslash before the closing ' as escaping it, which can incorrectly keep the string “open” and mask any # comment marker later on the line.

Set YAML’s raw to include the single quote so single-quoted YAML strings use the doubled-quote escape rule instead of backslash escaping.

# A YAML block scalar is the multi-line form.
# A plain scalar's apostrophe is not a string, so an ordinary quote must not carry here.
YAML: Syntax = {**HASH, 'carry': frozenset({'block'})}

[ptr727]

Round 2 answered

No new inline comments. One suppressed finding, which has no thread, so it is answered here.

Suppressed: scripts/prose_lint.py:211, YAML single-quoted scalars are raw

For YAML, single-quoted scalars treat backslash as an ordinary character. With raw left empty here, strip_strings() will treat a backslash before the closing ' as escaping it, which can incorrectly keep the string "open" and mask any # comment marker later on the line.

Valid, and fixed in 74a6a0c. YAML's single-quoted form spells its escape as a doubled quote and takes no backslash escape, so key: 'C:\tmp\' consumed its own closing quote and blanked the rest of the line, hiding a trailing comment. raw is now ' for YAML.

The class, not just the instance. TOML's literal string is the same construct, and .toml was sharing the plain hash spec, so it had the same defect. It takes a syntax entry of its own now with the same raw. .tf, .gitattributes, and .gitignore stay on the shared spec: HCL does use backslash escapes, and the last two have no string form to get wrong.

Both directions are pinned, because the two quote forms are read differently in the same file:

• key: 'C:\tmp\', key: 'it''s', and the TOML equivalents each keep their trailing comment visible, which only a raw read produces.
• key: "a\"b" and s = "a\"b" also keep theirs, which only a backslash-escaping read produces. A raw read would close at the escaped quote, reopen at the last one, and swallow the comment.

Verified no collateral change: the tree-wide comment-wrap / comment-case finding set is unchanged apart from the two validate-task.yml comments this PR deliberately fixed. Full local gate run green on the new head: 154 cases, spec/audit.py --selftest, spec/validate.py, scripts/repo_gate.py, the prose gate, mypy, markdownlint, cspell, actionlint, editorconfig-checker.

[Copilot]

Pull request overview

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

Suppressed comments (1)

scripts/prose_lint.py:320

• strip_strings() does not handle PowerShell’s backtick escape inside double-quoted strings. With the new POWERSHELL spec marking " as a raw (doubled-quote) form, an escaped quote like "a`"b" will be misread as closing the string at the escaped ", which can re-expose comment markers that are still inside string content.

        elif doubled:
            if ch == quote and line[i + 1:i + 2] == quote:   # a doubled quote is one character
                out[i] = out[i + 1] = ' '
                i += 2
                continue

[ptr727]

Round 3 answered

No new inline comments. One suppressed finding, answered here since it has no thread.

Suppressed: scripts/prose_lint.py:320, PowerShell's backtick escape

strip_strings() does not handle PowerShell's backtick escape inside double-quoted strings. With the new POWERSHELL spec marking " as a raw (doubled-quote) form, an escaped quote like "a`"b" will be misread as closing the string at the escaped ", which can re-expose comment markers that are still inside string content.

Valid, and fixed in 659304e. It is also the sharpest of the three rounds, because it names a modeling error rather than an off-by-one.

What was wrong. I had one flag doing two jobs. raw meant both "embeds its delimiter by doubling it" and "has no character escape", which held for every form I had in front of me: a C# verbatim string, a shell single-quoted string, a YAML or TOML literal scalar. PowerShell's double-quoted string is the counter-example that breaks the conflation - it doubles the delimiter and escapes with a backtick. So marking it raw silently dropped the backtick, and "a`"b" closed on the escaped quote, leaving b" to open a string that then carried into every line below it.

The fix reads the two as independent facts, decided per string as it opens:

• doubled - from raw, or from the @ prefix for the C# verbatim form.
• escaped - from escape (the character), escape_in (the quotes it works inside), and escape_out (whether it works outside a string too).

That also subsumes the shell outside-a-string backslash that the b1a2cc8 commit had bolted on as a one-off field, so the model got smaller rather than larger. Where each language lands:

Syntax	Doubling	Escape
C-like, XML, CSS, INI	none	\ inside either quote
C# verbatim (@")	""	none
shell ' / "	'' on the single	\ inside the double, and outside a string
PowerShell '	''	none
PowerShell "	""	` inside, and outside a string
YAML, TOML ' / "	'' on the single	\ inside the double

Both failure directions are pinned, per language, because they fail oppositely: reading an escape a string does not have consumes its closing quote and blanks the rest of the line, while missing one it does have ends the string early on the escaped quote. The PowerShell cases now cover "a`"b", "a""b"``, ``'a'``, 'a''b', and `"C:\tmp"`, and the existing C# verbatim cases still hold the other side of the table. Two table assertions were added as well: the escape is one character, it is never also a quote, and a syntax cannot declare an escape with nowhere for it to apply.

Verified no collateral change: the tree-wide comment-wrap / comment-case finding set is identical outside the two files this commit touches. Full local gate run green on the new head: 155 cases, spec/audit.py --selftest, spec/validate.py, scripts/repo_gate.py, the prose gate, mypy, ruff (unchanged count), markdownlint, cspell, actionlint, editorconfig-checker.

Documented in scripts/README.md alongside the carrying rules, since the next person adding a syntax needs both.

[Copilot]

Pull request overview

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

[Copilot]

Pull request overview

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

Suppressed comments (2)

scripts/prose_lint.py:213

• raw is documented/used as the set of quote delimiters that escape an embedded delimiter by doubling it (handled via the doubled branch in strip_strings). Shell single quotes do not escape a delimiter by doubling, so treating ' as raw makes balanced constructs like 'it'' look unterminated and then carry across lines, which can incorrectly suppress comment scanning for the rest of the file.

# A shell single-quoted string takes no escape at all, and either quote form spans lines.
# Outside a string a backslash escapes the next character, which is how `'\''` embeds a quote.
# A heredoc runs from its label to the line that repeats it.
SHELL: Syntax = {**HASH, 'raw': "'", 'escape_in': '"', 'escape_out': True,
                 'carry': frozenset({'quote', 'label'})}

scripts/prose_lint.py:375

• SCRIPT_SCALAR doesn’t account for YAML block-scalar chomping/indent indicators (e.g. run: |-, run: |2). Those headers match BLOCK_SCALAR but not SCRIPT_SCALAR, so they’ll be treated as opaque data even though the intent is that run: blocks remain script-comment-scanned.

BLOCK_SCALAR = re.compile(r'(?::|^\s*-)\s*[|>][+-]?\d?\s*$')

# `run:` holds a script rather than data, so its `#` lines are comments this rule governs.
SCRIPT_SCALAR = re.compile(r'(?:^|\s)run:\s*[|>]')

[ptr727]

Round 5 answered: one accepted, one declined with evidence

No inline comments. Two suppressed findings, both against scripts/prose_lint.py.

1. Accepted, with the reasoning corrected: shell single quotes declared as raw

Shell single quotes do not escape a delimiter by doubling, so treating ' as raw makes balanced constructs like 'it'' look unterminated and then carry across lines.

The declaration was wrong and is removed in b4ca858. The failure it predicts does not occur. Both halves matter, so both are stated.

Why the declaration was wrong. After round 3, raw means exactly one thing: the string embeds its delimiter by repeating it. A shell single-quoted string cannot embed its delimiter at all - '\'' closes the string, escapes a bare quote outside it, and reopens. So raw: "'" asserted something false about the language. What it was standing in for, that a backslash is not an escape inside '...', is already carried by escape_in: '"', which is why removing the entry changes no behavior. The entry is gone and the comment now says what shell actually does.

Why the predicted failure does not occur. The two readings always agree on whether a run of quotes leaves a string open, so neither one can carry where the other does not. Doubling consumes quotes in pairs and plain toggling consumes them one at a time, and both leave the string open on an odd count. Checked exhaustively rather than argued:

0 disagreement(s) over 87380 shell-quote strings

That is every string of length 1 to 8 over ', a, b, and #, masked both ways, comparing whether each leaves a string open. 'it'' in particular carries under both readings, because it is genuinely unbalanced shell.

The readings do differ in which quotes survive masking, which is the observable the new case pins: echo 'a''b' keeps 4 quotes as two adjacent strings, where a doubling read would keep 2.

2. Declined: SCRIPT_SCALAR and the chomping and indent indicators

SCRIPT_SCALAR doesn't account for YAML block-scalar chomping/indent indicators (e.g. run: |-, run: |2). Those headers match BLOCK_SCALAR but not SCRIPT_SCALAR.

It already matches all of them. BLOCK_SCALAR is anchored to end-of-line and so has to spell the indicators out. SCRIPT_SCALAR is r'(?:^|\s)run:\s*[|>]' with no $ anchor and is used with .search(), so it stops at the | or > and whatever follows is irrelevant:

run: |         BLOCK=True SCRIPT=True   -> ['comment-wrap']
run: |-        BLOCK=True SCRIPT=True   -> ['comment-wrap']
run: |2        BLOCK=True SCRIPT=True   -> ['comment-wrap']
run: >-        BLOCK=True SCRIPT=True   -> ['comment-wrap']
  run: |+      BLOCK=True SCRIPT=True   -> ['comment-wrap']
files: |-      BLOCK=True SCRIPT=False  -> []

Each row is the real extractor on a two-line file whose second line is a # comment inside the block, so ['comment-wrap'] means the block stayed script and [] means it went opaque. Every run: form is scanned, and a data key with the same indicator is not.

The finding is still worth something: nothing pinned that, and a later $ added to SCRIPT_SCALAR for symmetry with BLOCK_SCALAR would have broken it silently. The case now covers run: |, |-, |+, |2, >, >-, and an indented run: |, against files: |, files: |-, and tags: >-.

---

Two prose_lint.py comment-wrap findings were cleared along the way, so the file's own backlog is down by two rather than up. Full local gate run green on the new head: 156 cases, spec/audit.py --selftest, spec/validate.py, scripts/repo_gate.py, the prose gate, mypy, markdownlint, cspell, actionlint, editorconfig-checker. The tree-wide comment finding set is unchanged apart from those two removals.

[Copilot]

Pull request overview

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

Suppressed comments (1)

scripts/prose_lint.py:580

• head is captured only once as masked[:at] (the code before the first comment marker on the line). If the line contains a closed block comment and then code that opens a carried multi-line form (notably PowerShell <# ... #> @" / @' here-strings), opened_string(spec, head, line) never sees the opener, so the carry state is dropped and markers inside the here-string can be misclassified as comments.

Consider deriving head as a length-preserving “code-only” view of the entire line (blanking comment spans, but retaining code after same-line closed block comments) before calling opened_string at the end of the loop.

            if not scanned:
                head, scanned = masked[:at], True    # the code this line opens a string from
            if found is None:
                carry = tail_state                   # the rest of the line is code

[ptr727]

Round 7 clean, and verifying that turned up a runbook defect

Round 7 reviewed the current head and generated no inline comments and no suppressed block. That is the first round of the seven with nothing in either channel.

The runbook could not see the suppressed findings it exists to catch

Checking the round-7 body the documented way returned nothing for every round, including the five that each carried a real suppressed finding:

sha=f1136d34 suppressed=no    <- had 1
sha=b1a2cc83 suppressed=no    <- had 1
sha=74a6a0c9 suppressed=no    <- had 1
sha=28729fa6 suppressed=no    <- had 2
sha=b4ca8584 suppressed=no    <- had 1

.github/copilot-instructions.md said to find them with contains("low confidence"). That phrase does not appear in the current review format at all - the block is headed Suppressed comments (N). So an agent following the runbook literally gets a clean answer on a review with findings in it, which is the same false clean the rule directly above it warns about, reappearing one level up: not in the thread poll this time, but in the command written to work around the thread poll.

Fixed in 3d13948. The filter is a test alternation over both phrasings, and a second form scopes the check to the current head so an answered finding from an earlier round does not read as still open:

gh api repos/<owner>/<repo>/pulls/<N>/reviews --jq \
  '.[] | select(.body | test("Suppressed comments|low confidence")) | .body'

The prose now says to match on more than one phrasing and why, rather than naming one heading that will move again.

I caught this only because I had been reading every review body in full rather than trusting the filter. Worth stating plainly: the findings on this PR were reached by reading, not by the documented command. Had I relied on the command, rounds 1 through 6 would have looked clean and five real defects would have shipped, including the two the earlier rounds found in code written to fix the round before it.

This is the seventh push, so a review on this head is what closes the loop. Full local gate run green: 157 cases, spec/audit.py --selftest, spec/validate.py, scripts/repo_gate.py, the prose gate, mypy, markdownlint, cspell, actionlint, editorconfig-checker.

[Copilot]

Pull request overview

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

Suppressed comments (1)

scripts/prose_lint.py:396

• run: YAML block scalars are treated as “script, not data” by skipping the YAML block carry, but the lines inside are still scanned using the YAML string rules (YAML['carry'] is {'block'}), so shell multi-line quotes/heredocs inside a run: | script won’t carry across lines. That can still produce comment-wrap/comment-case findings on string content inside workflow scripts (e.g., an unclosed shell quote containing a # ... line). Consider adding a dedicated “run-script” mode that parses run: scalar bodies with shell string/carry rules (quote + heredoc) while still treating # as the comment marker, instead of falling back to YAML string semantics line-by-line.

    if 'block' in spec['carry'] and BLOCK_SCALAR.search(head) and not SCRIPT_SCALAR.search(head):
        return Carried('block', '', len(line) - len(line.lstrip()))

[Copilot]

Pull request overview

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

Suppressed comments (1)

spec/audit.py:511

• When the configured ground-truth branch does not exist, the function returns early before emitting the already-known branch-fact findings (e.g., branch: main does not exist, or hasDevelop drift). Those are still useful diagnostics even if no content can be read, and they were already fetched above.

    ground_head = {"main": branch_main, "develop": branch_dev}.get(ground)
    if ground_head is None:
        ground_head = gh(f"repos/{slug}/branches/{ground}", ok404=True)
    if ground_head is None:
        return findings + [("ERROR", f"branch: ground-truth branch {ground} does not exist, so nothing could be read")], ""

[ptr727]

Round 8 answered

No inline comments. One suppressed finding, and it is right.

Suppressed: spec/audit.py:511, the early return discards findings already read

When the configured ground-truth branch does not exist, the function returns early before emitting the already-known branch-fact findings (e.g., branch: main does not exist, or hasDevelop drift). Those are still useful diagnostics even if no content can be read, and they were already fetched above.

Valid, and fixed in 174a452. The early return sat above the two branch-fact appends, so a repo that tripped it reported one error and nothing else, losing a hasDevelop registry drift and a missing-main defect that the branch reads had already answered.

The point of that return is to skip the content reads that would all 404 and report the whole baseline as absent. It was never to discard what the branch reads already established. The two appends now come first, and the return is findings + [ERROR], so it carries them.

The case I wrote would not have caught it, which is the more useful half of this finding. It asserted the error count and the empty SHA, and both held while two findings silently vanished. Its fixture also declared hasDevelop: false against a repo with no branches, so the drift it should have been watching for never fired. The fixture now declares hasDevelop: true and the assertion is on the exact finding sequence:

ok   missing ground branch: the branch facts, then one error naming the ref, no letters

That is the shape the repo's own verification rule names: a case that passes for an incidental reason is worse than no case, because it is later cited as evidence. This one was cited as evidence in my round-6 answer.

Full local gate run green on the new head: 157 cases, spec/audit.py --selftest, spec/validate.py, scripts/repo_gate.py, the prose gate, mypy.

[Copilot]

Pull request overview

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

Suppressed comments (1)

scripts/prose_lint.py:351

• In YAML (and similar), apostrophes inside plain scalars (e.g. key: don't # comment) are not string delimiters, but strip_strings treats any ' as opening a (raw) string. This masks everything after the apostrophe and can hide real trailing # comments on the same line.

        elif ch in quotes:
            quote = ch
            # An interpolated one is spelled either way round, so read the whole prefix.
            # Only the double-quoted form has a verbatim spelling, so a char literal is ordinary.
            start = i

[ptr727]

Round 9 answered

No inline comments. One suppressed finding, and it is right.

Suppressed: scripts/prose_lint.py:351, a YAML plain scalar's apostrophe

In YAML, apostrophes inside plain scalars (e.g. key: don't # comment) are not string delimiters, but strip_strings treats any ' as opening a (raw) string. This masks everything after the apostrophe and can hide real trailing # comments on the same line.

Valid, and fixed in 1288243. Reproduced first:

key: don't  # Two things. Here.            -> []            <- the comment is gone
name: Don't do this  # Two things. Here.   -> []            <- and here
key: 'quoted'  # Two things. Here.         -> ['comment-wrap']

This is not something this PR introduced - .yml carried the same quotes: '"\'' before it, so the masking ran to end of line either way. Worth saying plainly, because it is also not a reason to leave it: the finding is about the syntax model, this PR is the change that gave YAML a syntax entry of its own, and the rule this repo applies to a reviewer citing one instance is to fix the class.

The rule is that YAML delimits with a quote only at the start of a value. quote_after now names what a quote must follow for that: the :, the sequence dash, or a flow indicator. Mid-word it is text, which is what YAML says it is.

It is empty for every other syntax, and deliberately so. A C# string opens after =, (, or ,, none of which YAML's set contains, so applying the restriction everywhere would stop masking C# strings altogether. The case asserts that with the masked span, since no C# comment marker fits inside a char literal to hide.

Covered both ways: key: don't, name: Don't do this, and key: [don't, 'x'] keep their trailing comment, while key: 'quoted', - 'item', key: {k: 'v'}, and key: "don't" still mask theirs.

It is a live pattern, not a hypothetical. This repo's own catalog/snippets/workflows/check-upstream-version-task.yml:30 reads must match the merge-bot's upstream-version-<base> head refs in a plain scalar. Nothing on that line was being hidden today, but the next # after an apostrophe would have been.

Full local gate run green on the new head: 159 cases, spec/audit.py --selftest, spec/validate.py, scripts/repo_gate.py, the prose gate, mypy, markdownlint, cspell, actionlint, editorconfig-checker. Coverage over scripts/ holds at 99%. The tree-wide comment finding set is unchanged apart from line shifts in spec/audit.py from the round-8 move.

[Copilot]

Pull request overview

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

Suppressed comments (1)

scripts/prose_lint.py:399

• SCRIPT_SCALAR is too broad: re.compile(r'(?:^|\s)run:\s*[|>]') will also match keys that merely contain run: after whitespace (e.g. dry run: |), causing those block scalars to be treated as scripts instead of opaque data. That can reintroduce false positives inside non-run block scalars.

Consider anchoring to the start of the YAML key (with optional list-item dash) so only the actual run: field is exempted.

# `run:` holds a script rather than data, so its `#` lines are comments this rule governs.
SCRIPT_SCALAR = re.compile(r'(?:^|\s)run:\s*[|>]')

[ptr727]

Round 10 answered

No inline comments. One suppressed finding, and it is right.

Suppressed: scripts/prose_lint.py:399, SCRIPT_SCALAR is too broad

re.compile(r'(?:^|\s)run:\s*[|>]') will also match keys that merely contain run: after whitespace (e.g. dry run: |), causing those block scalars to be treated as scripts instead of opaque data.

Valid, and fixed in f6c8b2b. dry run: is a legitimate YAML key - plain scalar keys may contain spaces - and the \s alternative matched its last word:

before                          after
run: |             True         True
      - run: |     True         True
dry run: |         True   <-    False
first run: |-      True   <-    False
post-run: |        False        False

Anchored to the key position now, r'^\s*(?:-\s+)?run:\s*[|>]', which keeps both real forms - the bare field and the list-item - run: that a workflow step actually uses - and drops the accidental ones.

This is the second finding against this one regex, and the pairing is why. Round 5 said it was too narrow, claiming it missed run: |- and run: |2; that was wrong, because it has no $ anchor and so already matched every indicator form. Round 10 says the same missing anchor makes it too broad at the other end, and that one is right. The two together describe the regex correctly: unanchored at the front is the defect, unanchored at the back is the feature. Nothing pinned either end, which is how one true and one false finding both landed on the same line.

The cases now cover both directions in one place: run: |, |-, |+, |2, >, >-, an indented run: |, and - run: | stay script, while files: |, files: |-, tags: >-, dry run: |, first run: |-, and post-run: | stay data.

Full local gate run green on the new head: 159 cases, spec/audit.py --selftest, spec/validate.py, scripts/repo_gate.py, the prose gate, mypy. The tree-wide comment finding set is one smaller than before, from a wrapped comment of my own that this commit tightened.

[Copilot]

Pull request overview

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

[ptr727]

Round 11 clean in both channels: the loop is closed

Round 11 reviewed head f6c8b2b and returned no inline comments and no suppressed block. Merge gate on that head:

Condition	State
Formal Copilot review on the head SHA	1
Suppressed findings on the head	0
Unresolved threads	0
Merge state	MERGEABLE / CLEAN
Required checks	both pass

Ready for review and squash. Not merged, deliberately - that is the maintainer's call.

What the eleven rounds cost and bought

Round	Inline	Suppressed	Outcome
1	2	1	heredoc and here-string terminators matched after stripping indentation; mutable default
2	0	1	YAML and TOML single-quoted scalars read with a backslash escape they do not have
3	0	1	doubling and escaping conflated behind one flag, breaking PowerShell's backtick
4	0	0	clean
5	0	2	a raw declaration false about shell; one finding declined with evidence
6	0	1	a line's code read only up to its first comment, so a here-string never opened
7	0	0	clean, and verifying it exposed the runbook defect below
8	0	1	the unresolvable-ref early return discarded branch facts already fetched
9	0	1	a YAML plain scalar's apostrophe read as a string delimiter, hiding trailing comments
10	0	1	the run: exemption matched any key ending in the word (`dry run:
11	0	0	clean

Eleven defects fixed, one finding disproved. Only round 1 produced an inline thread. Everything after it came out of the suppressed block, so a loop that polls reviewThreads would have called this done after round 1 with ten defects in it.

Three of the eleven were in code written to fix the round before it. One (round 8) was in a case I had cited in an earlier answer as evidence, which passed while two findings silently vanished.

The finding that mattered most was about the process, not the code

Round 7 was clean, and checking it the way .github/copilot-instructions.md documented returned nothing for every round - including the five that each carried a real suppressed finding. The documented filter keyed on contains("low confidence"), a phrase absent from the current review format, whose heading is Suppressed comments (N). That is the same false clean the rule directly above it warns about, reappearing in the command written to work around it. Fixed in 3d13948 with a test alternation over both phrasings plus a head-scoped form.

I found the ten defects by reading every review body in full rather than trusting that filter. Had I trusted it, this PR would have shipped them.

Two contradictory findings on one line

SCRIPT_SCALAR drew a finding in round 5 saying it was too narrow and one in round 10 saying it was too broad. The first was wrong (no trailing anchor, so every indicator form already matched) and the second was right (no leading anchor, so dry run: | matched). Both landed there because neither end was pinned. Both ends are now, in one case that asserts the script forms and the data forms together.

Final state

11 commits, 15 files. 159 test cases over scripts/, up from 104, at 99% coverage, up from 87%. spec/audit.py --selftest now runs in CI. Every gate green locally and in CI: the suite, the audit selftest, spec/validate.py, scripts/repo_gate.py, the prose gate, markdownlint, cspell, actionlint, editorconfig-checker, mypy, and ruff at an unchanged finding count on every touched file.

Merging re-vendors 12 GOVERNANCE.md verbatim sections across the fleet. That drift wave is the mechanism working, not a regression.