Skip to content

Read dupword from the comments, not from the source lines - #499

Merged
ptr727 merged 1 commit into
developfrom
feature/dupword-comment-scope
Aug 1, 2026
Merged

Read dupword from the comments, not from the source lines#499
ptr727 merged 1 commit into
developfrom
feature/dupword-comment-scope

Conversation

@ptr727

@ptr727 ptr727 commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Finding 3 of #489, reported by the Blog agent while carrying the baseline into the first Hugo repo.

The defect

dupword is one of three rules in the blocking CI step (--check charset --check dupword --check spelling). Outside markdown it read the raw source line, so it judged code as prose and rejected correct work.

The reported case has no legal fix:

<div class="gallery gallery-cols-1">

Two class names sharing a prefix is the ordinary CSS idiom. Editing it changes the rendered page, so the only way to pass the gate was to break the styling.

The fix

spelling already solved this in the same function: outside markdown it reads the extracted comments rather than the source line. dupword now selects its text the same way, reusing the same extraction.

Narrowing to the comment is preferred over exempting the class attribute, since rel, srcset, sizes and the data-* attributes all take value lists of the same shape, and an exemption list covers only what its author thought of.

Each comment on a line is judged on its own rather than joined with its neighbors, because two comments are two sentences and joining them reads the second's opening word as a repeat of the first's last.

Measured, both directions

Every case below was run against the rule before and after. Nothing else changed.

Case Before After
<div class="gallery gallery-cols-1"> FIRES quiet
<a rel="nofollow nofollow-ugc" href="#"> FIRES quiet
x = "the the" (Python string literal) FIRES quiet
key: the the (YAML scalar) FIRES quiet
{ "a": "the the" } (JSON string value) FIRES quiet
<p>the the</p> (HTML body text) FIRES quiet
<!-- The the thing. --> (HTML comment) FIRES FIRES
# The the thing. (Python, shell, YAML) FIRES FIRES
// The the thing. and /* ... */ (C#) FIRES FIRES
x = 1 # The the thing. (trailing comment) FIRES FIRES
the the thing (markdown prose) FIRES FIRES

The cost, stated rather than hidden

A duplicated word in HTML body text, or in a YAML or JSON string value, is no longer caught. That trade was accepted deliberately when the approach was chosen. The narrower alternative, stripping only HTML attribute values, was considered and declined for the reason above.

Asserting the floor

This narrows a blocking gate, so the floor is asserted rather than assumed.

  • Tree-wide dupword was clean before the change and is clean after.
  • Six new test cases fail against the old rule and pass against the new one, so they are not vacuous. The comment-syntax cases pass against both, which is the floor holding.
  • test_the_repo_is_clean_of_duplicated_words pins the tree-wide floor the way the spelling rule already does, so the gate cannot silently stop gating.
  • DUP_ALLOW (that that, had had) is untouched. It is the pre-existing band-aid, still needed for markdown prose, and now the only remaining allowlist in the rule.

Test bait for the attribute cases is assembled from two literals, following the convention this module already states, so the file never holds the pattern it feeds the gate.

Verification

Every step of the CI job run locally:

python3 scripts/test_prose_lint.py     136 tests, OK  (130 before, 6 added)
python3 scripts/test_repo_gate.py       23 tests, OK
python3 scripts/test_pr_review.py       27 tests, OK
python3 spec/audit.py --selftest        SELFTEST PASS
python3 scripts/repo_gate.py            eol 0, sha-pin 0
prose_lint --check charset --check dupword --check spelling    clean
prose_lint . --diff HEAD                clean, all default rules

The warn-only step reports the same 5 pre-existing comment-wrap findings in prose_lint.py as before, and no new ones. Two comments this PR touches were rewritten to one sentence per line so the change adds no backlog.

Docs

scripts/README.md documented the comments-only scope for spelling alone. It now covers both rules, states why an attribute value is not prose, and states the cost.

🤖 Generated with Claude Code

`dupword` sits in the blocking CI step, and outside markdown it read the raw
source line, so it rejected correct code. A repeated token is not repeated
prose: `class="gallery gallery-cols-1"` is the ordinary way two CSS class
names share a prefix, and no edit satisfies the rule without changing the
rendered page.

The fix already existed in the same function for `spelling`, which reads the
extracted comments outside markdown. `dupword` now selects its text the same
way. Each comment on a line is judged on its own rather than joined with its
neighbors, since joining reads the second's opening word as a repeat of the
first's last.

Narrowing to the comment is preferred over exempting an attribute, because
`rel`, `srcset`, `sizes` and `data-*` share the property and an exemption list
covers only what its author thought of.

The cost, accepted deliberately: a duplicated word in HTML body text, or in a
YAML or JSON string value, is no longer caught.

Reported by the Blog agent as finding 3 of #489.

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

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 false positive in the blocking dupword prose gate by aligning it with the existing spelling rule behavior: outside Markdown, both rules now analyze extracted comments rather than raw source lines. This prevents valid code/content constructs (notably repeated tokens in HTML attribute values and other non-prose syntax) from being incorrectly rejected by CI, while preserving enforcement for actual prose in Markdown and in comments across supported syntaxes.

Changes:

  • Update dupword to scan extracted comments (per-comment) instead of scanning raw source lines in non-Markdown files.
  • Add targeted tests that prove the prior false positives (attributes / literals) are fixed while comment-based enforcement remains intact.
  • Update documentation to describe the shared “comments-only outside Markdown” scope for both spelling and dupword, including the intentional tradeoff.

Reviewed changes

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

File Description
scripts/prose_lint.py Changes dupword to read extracted comments outside Markdown (per comment), avoiding false positives on code/attribute values.
scripts/test_prose_lint.py Adds regression tests covering the reported attribute cases and the new comments-only semantics for dupword.
scripts/README.md Documents the updated scope and its deliberate cost for the CI-gating dupword rule.

@ptr727
ptr727 merged commit d68b2ca into develop Aug 1, 2026
7 checks passed
@ptr727
ptr727 deleted the feature/dupword-comment-scope branch August 1, 2026 13:23
ptr727 added a commit that referenced this pull request Aug 1, 2026
Finding 4 of #489, reported by the Blog agent. Same class as #499, which
fixed finding 3.

## The defect

A comment whose entire content is a documentation URI is not a sentence,
and it cannot be capitalized or restructured without corrupting the
address it exists to carry. `comment-case` reported it anyway:

```
.github/dependabot.yml:1: comment-case: comment sentence opens in lowercase -> capitalize, or restructure so it does not open on a lowercase name
```

That line is `# https://docs.github.com/...`, the reference the file
opens with. A reference block opening a config file is the ordinary
shape, so every repo carrying one inherited a finding no edit could
answer. The hub's own `.editorconfig` opens with a block of eight.

## The fix

`BARE_URI` joins the existing `NOT_PROSE` exemption at the one place
both comment rules consult, so a body that is only a URI is skipped by
`comment-case` and `comment-wrap` alike.

Skipping it also stops the line below a URI from reading as its
continuation. That mattered more than expected: consecutive reference
lines are separate addresses, and the old rule was reporting
`comment-wrap` ("sentence wraps into the next line") on a URL that
wrapped into nothing.

A URI inside a sentence is still prose, so the exemption requires the
whole body to be the address and nothing else.

## Verdict diff, tree-wide

Ten findings removed, none added. Counted with `prose_lint.py . --check
comment-wrap --check comment-case`, ignoring pure line-number shifts
inside `prose_lint.py` itself.

```
before 526  ->  after 516
```

| Removed | Rule |
| --- | --- |
| `.github/dependabot.yml:1` | comment-case (the reported case) |
| `.editorconfig:1`, `:3`, `:7`, `:10` | comment-case |
| `.editorconfig:3`, `:4`, `:7`, `:10` | comment-wrap (the false
continuation) |
| `catalog/snippets/configs/dependabot.yml:2` | comment-case |

## One finding this exposed, fixed rather than left

Removing the false continuation on `.editorconfig:10` revealed
`.editorconfig:11`, a bare command (`dotnet format style
--verify-no-changes ...`) that the old rule had been hiding inside a
`comment-wrap` on the URL above it. It is a genuine lowercase opening,
not a URI, so the exemption does not cover it.

Rather than ship a net-new finding, it is restructured to `# Verify
with: dotnet format ...`, which is what GOVERNANCE's own guidance
prescribes for a comment opening on a lowercase tool name.
`.editorconfig` is `intent` fidelity, so this does not force a
downstream re-vendor.

This is one line of the larger `.editorconfig` comment-shape sweep still
owed in `TODO.md`. It is fixed here only because this change is what
surfaced it.

## Asserting the floor

- Six new test cases fail against the old rule and pass against the new
one, so they are not vacuous.
- `test_a_uri_inside_a_sentence_is_still_prose` proves the other
direction, that the exemption did not swallow prose that merely mentions
a URL.
- `test_a_uri_block_does_not_make_the_next_line_a_continuation` pins the
continuation behavior, which is the subtle half.
- Cases cover `#`, `;`, `//` and `<!-- -->` syntaxes, plus the
angle-bracketed and `ftp://` forms.

## Verification

Every step of the CI job run locally:

```
python3 scripts/test_prose_lint.py     139 tests, OK  (136 before, 3 added)
python3 scripts/test_repo_gate.py       23 tests, OK
python3 scripts/test_pr_review.py       27 tests, OK
python3 spec/audit.py --selftest        SELFTEST PASS
python3 scripts/repo_gate.py            eol 0, sha-pin 0
prose_lint --check charset --check dupword --check spelling    clean
prose_lint . --diff HEAD                clean, all default rules
```

🤖 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 Aug 1, 2026
Promotes the three prose-gate fixes to `main`, so a repo carrying the
baseline from `main` gets the corrected gates and the corrected
`configure.sh` rather than the versions that reject correct work.

This is the unblocking step for the `Blog` onboarding in #456: `Blog`
carries from `main`, and today's `main` hands it two false-positive
gates and 54 unfixable findings.

| PR | Squash | What |
| --- | --- | --- |
| #499 | `d68b2ca` | `dupword` reads the comments outside markdown, not
the raw source line (#489 finding 3) |
| #500 | `c3ad6db` | A URI-only comment is a reference, not a sentence
(#489 finding 4) |
| #501 | `67aaa6c` | The `verbatim`-carried `repo-config/configure.sh`
swept to one sentence per line (#489 finding 6) |

## What this changes for a repo carrying from `main`

- `class="gallery gallery-cols-1"` and any other repeated token outside
a comment stops being a **blocking** CI failure. That was the reported
case with no legal fix in the HTML.
- A comment whose whole body is a URI stops producing a `comment-case`
finding no edit can answer. Every config file opening with a reference
block inherited one.
- `repo-config/configure.sh` arrives clean. It is `verbatim` with
`appliesTo: "*"`, so a downstream copy is byte-matched and could never
have been fixed locally.

## Consequence, stated plainly

`configure.sh` is `verbatim`, so **every repo already carrying it now
differs from `main` until re-vendored**. A fleet re-vendor pass is owed
after this merges. A repo onboarding after this point carries the
corrected file from the start.

## Verification

Run on `develop` at `67aaa6c`, which is what this promotes:

```
python3 scripts/test_prose_lint.py     141 tests, OK
python3 scripts/test_repo_gate.py       23 tests, OK
python3 scripts/test_pr_review.py       27 tests, OK
python3 spec/audit.py --selftest        SELFTEST PASS
python3 scripts/repo_gate.py            eol 0, sha-pin 0
prose_lint --check charset --check dupword --check spelling    clean
```

Warn-only backlog moved 526 -> 516 on the comment rules from #500, and
`configure.sh` went 54 -> 0 from #501.

🤖 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