diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 010dad10..c245fa5f 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -37,6 +37,12 @@ Auto-review on push is configured (via the branch ruleset's `copilot_code_review **A review with no inline comments is still a completed review - not a failure, and not a reason to ask the maintainer to re-trigger.** Copilot very often posts a single formal review (GraphQL `state: COMMENTED`) whose body ends with "...reviewed N of N changed files ... and generated no comments" and adds **zero** inline threads. That review carries the head `commit.oid` and fully satisfies the loop - it is the clean-pass success case. Never read "no inline comments" as "the review didn't run," and never re-request or escalate to the maintainer because comments are absent. +**Read the low-confidence findings, which are not inline threads.** A review body can carry a collapsed `
` block headed "Comments suppressed due to low confidence", and those findings appear nowhere in `reviewThreads`, so a loop that polls threads alone never sees them and reports a clean pass. They have been right repeatedly, including a rule stated more broadly than its check enforced and a check that skipped fenced blocks in every rule but one. Read the body of every review, investigate each suppressed finding on the same footing as an inline one, and answer it in the PR conversation, since a suppressed finding has no thread to reply on or resolve. + +```sh +gh api repos///pulls//reviews --jq '.[] | select(.body | contains("low confidence")) | .body' +``` + **Round 1 is normally auto-seeded - poll for it before trying to self-trigger.** Auto-review-on-open supplies the first review with no `botIds` call needed, but it can lag one to three minutes. After opening a PR (or the first push), **poll** for a Copilot review on the head SHA (see [Verify Review Covered Current Head](#verify-review-covered-current-head)) before concluding none ran. The `requestReviews` mutation below is for **re-requesting on later pushes** (a new head SHA); by then a prior review exists, so its bot node id is readable. A missing bot node id on round 1 therefore means "the auto-review has not landed yet - wait and poll," **not** "ask the maintainer to kick it off." > **The reviewer login differs by API.** In **GraphQL** (`gh api graphql` and `gh pr view --json reviews`, which is GraphQL-backed) the `Bot.login` is `copilot-pull-request-reviewer` - **no `[bot]` suffix**. In the **REST** API (`gh api repos/.../issues|pulls/...`) the same account's `user.login` is `copilot-pull-request-reviewer[bot]` - **with** the suffix. Each query below uses the correct form for its API; match the API, not a single spelling, when adapting them. diff --git a/.github/workflows/validate-task.yml b/.github/workflows/validate-task.yml index eabb7228..d6555e1c 100644 --- a/.github/workflows/validate-task.yml +++ b/.github/workflows/validate-task.yml @@ -25,8 +25,10 @@ jobs: with: globs: '**/*.md' - # cspell gate = README + HISTORY only; all-*.md would mean endlessly padding cspell.json for technical terms - # (broad live spell-check is the editor extension's job). See CODESTYLE.md "Markdown and Spelling". + # The cspell gate covers README + HISTORY only. + # Gating all *.md would mean endlessly padding cspell.json for technical terms. + # Broad live spell-check is the editor extension's job. + # See CODESTYLE.md "Markdown and Spelling". - name: Spell check step uses: streetsidesoftware/cspell-action@de2a73e963e7443969755b648a1008f77033c5b2 # v8.4.0 with: @@ -60,7 +62,13 @@ jobs: - name: Check repo gates step run: python3 scripts/repo_gate.py - # The charset and duplicate-word rules are clean tree-wide, so they gate. The semicolon - # rule stays warn-only: going green needs the sweep the prose rule itself forbids. + # The charset and duplicate-word rules are clean tree-wide, so they gate. + # Every other prose rule reports in the step below without gating. - name: Check prose step - run: python3 scripts/prose_lint.py . --check ascii --check dupword + run: python3 scripts/prose_lint.py . --check charset --check dupword + + # Warn-only, and visible rather than absent: an unrun check is one nobody acts on. + # The backlog is corrected as each file is next edited, never swept. + - name: Report prose backlog step + continue-on-error: true + run: python3 scripts/prose_lint.py . --check charset-unknown --check semicolon --check dash --check comment-wrap --check comment-case --summary diff --git a/.markdownlint-cli2.jsonc b/.markdownlint-cli2.jsonc index 01b4059c..40f5d762 100644 --- a/.markdownlint-cli2.jsonc +++ b/.markdownlint-cli2.jsonc @@ -1,9 +1,12 @@ { "config": { - // Prose paragraphs and data-heavy tables/URLs are intentionally long. - // Reflowing at 80 cols hurts readability and churns diffs. + // Prose paragraphs and data-heavy tables or URLs are intentionally long. + // Reflowing at 80 columns hurts readability and churns diffs. "MD013": false, - // MD033 (inline HTML) stays enabled so native markdown wins - HTML comments (reference-link dividers) pass it, and details/summary are allowed for GitHub collapsibles, which have no markdown equivalent. Every other element still flags. + // MD033 (inline HTML) stays enabled so native markdown wins. + // HTML comments, used as reference-link dividers, pass it. + // The details and summary elements are allowed for GitHub collapsibles, which have no markdown equivalent. + // Every other element still flags. "MD033": { "allowed_elements": ["details", "summary"] }, // Require fenced code blocks over the legacy 4-space-indented style. "MD046": { "style": "fenced" }, diff --git a/GOVERNANCE.md b/GOVERNANCE.md index 8fffad9d..1dbf8b84 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -149,6 +149,7 @@ Applies to code and workflow (`#`) comments alike. - Write for the human reading *this* project's code now: state only the non-obvious *why*. No cross-project references (do not name other repos), no historic or design narrative, no rule citations - governance lives in this file, not echoed inline. - **Keep it short.** One line is the default. A comment earns a second line only by carrying a constraint the code cannot. Most comments are one sentence, and never restate *what* the code does - a well-named symbol already says it. - **Structured, not prose: one sentence per line, and never wrap a sentence across lines.** No block paragraphs and no multi-sentence run-ons. A comment that genuinely needs several sentences is several lines, each a single sentence. A sentence too long for one sensible line is too long - split the thought. +- **A comment line that opens prose starts with a capital.** A line opening in lowercase reads as the continuation of the one above it, so a sentence that genuinely starts there is capitalized. A trailing fragment that annotates the code on its line is a label rather than a sentence, and the version pin the action-pinning rule requires is one, so neither takes a capital. Where the first word is a tool or identifier whose own casing is lowercase, restructure so the sentence does not open on it rather than capitalizing the name against its official spelling. - **A multi-line comment shows whether it is a continuation or a list.** A continuation of the same topic stays unindented, one sentence per line. Mark a sub-topic with a `-` after the comment marker (`# -`, `// -`), and only for genuine sub-topics - parallel items hanging off a lead line, never a continuation of one thought. - **No class-, type-, or file-header summary comment blocks.** A type or file gets a comment only for a specific non-obvious point, kept terse - never a block summarizing what the file contains or what the class is for. A summary restates the declaration below it, goes stale as the file grows, and is the file-scope form of the design narrative and verbosity creep this section already bans. A license or provenance header a tool or policy requires is not a summary and is unaffected. - **Do not grow a comment across edits.** When you touch code near an existing comment, the comment must come out **same length or shorter** - never append "one more clause" of rationale. If a block comment has crept to multiple sentences of prose, cut it back to its single load-bearing point as part of your change. Verbosity creep is the specific regression to prevent: every iteration that adds a clause is a regression, not an improvement. @@ -171,15 +172,20 @@ Sub-topics take a `-` after the comment marker, each elaborating a distinct item ### Character Set -- **Write ASCII in all agent-authored text** - documentation, code, comments, commit messages, and PR descriptions. The agent does not introduce non-ASCII characters. Replace typographic Unicode with its ASCII equivalent on sight: - - em dash (U+2014) and en dash (U+2013) -> hyphen `-` (use a spaced ` - ` for an em-dash-style clause break) - - right arrow (U+2192) -> `->`; double arrow (U+21D2) -> `=>` - - less-than-or-equal (U+2264) -> `<=`; greater-than-or-equal (U+2265) -> `>=` - - curly quotes (U+2018/U+2019/U+201C/U+201D) -> straight `'` and `"`; ellipsis (U+2026) -> `...` -- **No semicolon joining two independent clauses in agent-authored prose** - documentation, comments, commit messages, and PR descriptions. Recast as a comma or as two sentences: "the check runs on push; it gates the merge" becomes "the check runs on push and gates the merge", or two sentences. A semicolon separating items in a list that already contains commas keeps its standard use, and a statement terminator in **code** is untouched by this rule. This bans the semicolon splice only - a colon introducing an explanation, elaboration, or list keeps its standard use and is not a splice. Existing prose is corrected as each file is next edited, not swept. -- **Allowed non-ASCII (two narrow exceptions):** - - **Scientific or technical symbols with no clean ASCII equivalent** - e.g. ohm, micro, degree, pi. Keep the symbol; do not approximate it away. - - **Unicode the developer deliberately typed** - emoji used for emphasis or as callout markers (for example the warning/info markers a maintainer placed in `README.md`). Preserve it; never strip the developer's own characters. This carve-out is for developer-authored text, not a license for the agent to add emoji. +Agent-authored text is ASCII by default: documentation, code, comments, commit messages, and PR descriptions. A non-ASCII character is read against three tiers, because whether one is typography or meaning depends on where it sits. A character in no tier is a finding rather than a silent pass. + +- **Tier 1, never legitimate.** Typography carrying no meaning its ASCII form loses. Remove on sight: + - em dash (U+2014) and en dash (U+2013) -> **restructure the sentence**. Two sentences, or a comma. Do not substitute a spaced hyphen. + - right arrow (U+2192) -> `->`, double arrow (U+21D2) -> `=>` + - curly quotes (U+2018/U+2019/U+201C/U+201D) -> straight `'` and `"` + - ellipsis (U+2026) -> `...`, bullet (U+2022) -> `-` + - no-break space (U+00A0) -> a space, non-breaking hyphen (U+2011) -> `-` +- **Tier 2, legitimate only next to a number.** Relational and arithmetic operators: U+2264, U+2265, U+2260, U+00B1, U+2212, U+00D7, U+00F7, U+00B7. Keep one when an adjacent non-space token is a number, a tier-3 symbol, or another tier-2 operator, so a threshold table or a measured range reads as the range it is. In flowing prose write the ASCII form: `<=`, `>=`, `!=`, `+/-`, `-`, `x`, `/`. A U+2264 directly before `35` in a table of sensor thresholds is the range it describes and stays. The same character between two words, as in a sentence about one check running before another, is prose and takes `<=`. +- **Tier 3, always legitimate.** Scientific and unit symbols whose ASCII form would be a lie: micro (U+00B5), degree (U+00B0), ohm (U+2126), pi (U+03C0), superscript two and three (U+00B2, U+00B3), section (U+00A7). Keep the symbol. Do not approximate it away, and do not spell it out. +- **Unicode the developer deliberately typed** stays regardless of tier, such as 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. The carve-out governs what an agent may rewrite rather than what the gate reports, so an un-tiered character of this kind is still a `charset-unknown` finding until someone classifies it. It covers developer-authored text, and is 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". +- **No spaced hyphen joining or interrupting a sentence.** The em-dash-style clause break ` - `, and the paired aside ` - x - `, are both recast: a comma where the clauses are short and closely linked, two sentences where they are not, or parentheses for a genuine aside. This is the same construction the tier-1 em dash is restructured into, so allowing its ASCII spelling would keep the shape and only change the character. A hyphen inside a compound word, a leading list marker, a range, and the `- **Label** - explanation` separator that opens a governed bullet keep their standard use: the last is structurally a colon, and flagging it would restructure the document format rather than the prose. Existing prose is corrected as each file is next edited, not swept. +- **No semicolon in agent-authored prose.** A mid-sentence semicolon joining clauses is recast as a comma or as two sentences. A semicolon separating items in a list that already contains commas keeps its standard use, and a statement terminator in **code** is untouched. A colon introducing an explanation, elaboration, or list is not a semicolon and is unaffected. Existing prose is corrected as each file is next edited, not swept. ### Line Endings @@ -203,10 +209,12 @@ Sub-topics take a `-` after the comment marker, each elaborating a distinct item The checks that separate work actually done from work that merely reports success. Their unifying property: **every failure below is green.** A skipped job and a passing job are indistinguishable in the aggregated required check, a pattern that matches less still exits zero, and a gate that stops gating still reports success. No linter, status check, or review layer catches any of them. -- **A test must assert the mechanism it names.** Label each case by the behavior it proves, and satisfy yourself it would fail if that mechanism broke. A case that passes for an incidental reason - the right answer reached by the wrong path - is worse than no case, because it is later cited as evidence. +- **A test must assert the mechanism it names, and a gate has to be watched failing.** Label each case by the behavior it proves, then write the case that reintroduces the fault and confirm the gate objects to it. A case that passes for an incidental reason, the right answer reached by the wrong path, is worse than no case, because it is later cited as evidence. A proof that restates the gated data instead of reading it proves only that the function works, so drive the real table or the real config. And a gate that finds nothing is indistinguishable from a gate with nothing to find, so assert a floor on what a healthy run covers. - **Gates, filters, and gate-like watchers fail loud, never narrow quietly.** A pattern that silently matches less, an allowlist that silently stops matching, or a gate that silently stops gating all report success while doing nothing. When a construct exists to notice something, make the not-noticing case produce an error or an annotation. An identity allowlist used as a gate, for one, must raise an error when its list stops matching, not silently pass everything through. - **Run the repo's whole lint gate before every push, not the parts that look relevant.** CI runs all of them, so a partial local run only defers the failure - and the tool most likely to catch a given change is often the one it seems least about (an edit that manipulates line endings is exactly when `editorconfig-checker` matters). The repo documents each linter's known-working invocation - this rule is that **all** of them run. - **Editing CRLF files programmatically: `.` matches `\r` in a regex**, so a captured line keeps its carriage return and rejoining with `\r\n` yields `CRCRLF`. A text-mode rewrite has the mirror failure, silently flattening CRLF to LF. Prefer line-based edits (`splitlines(keepends=True)`) or literal replacement over regex reassembly. This is the mechanism behind the Line Endings warning above, and it is worth naming because the corruption is invisible in a rendered diff. +- **Scope a check by what the project declares, not by the file that prompted it.** A check written while editing one file tends to cover that file's language and stop, and then reports success on every other surface the rule governs. Read the declared types, or the config that enumerates them, and cover each one, then assert a floor per surface so a table that narrows fails loudly instead of passing quietly. A rule about comments means every comment syntax the project ships, and a format that carries comments in practice counts even where its specification says otherwise. +- **Never edit source through a shell heredoc when the text carries backslash escapes.** The shell consumes the escape and writes an invisible control character in its place, so a `\b` inside a regex becomes a backspace and the pattern silently matches nothing while every test still passes. Use a file-editing tool for such text. When a check inspects text for control characters, use `str.isprintable()` rather than a codepoint floor, since DEL and the Unicode format characters sit above 32 and are equally invisible in a diff. - **Never edit an active `.code-workspace` file.** A workspace file rewritten on disk can make VS Code reload the window, and a reload destroys the running agent session's context - the work in flight is lost with nothing to catch it, and the trigger is not fully characterized (an agent's edit has caused the reload where a human's identical edit did not). Surface the needed change for the maintainer to apply by hand. - **A green check is not evidence the work happened.** A skipped job and a passing job are indistinguishable in the aggregated required check. When a job exists to exercise something, confirm from its log that it ran and produced the output it promises. - **A workflow change is only fully exercised by CI.** Extracting a `run:` block and executing it locally validates the script and nothing else - `secrets: inherit`, `permissions:`, `needs:` wiring, and reusable-workflow inputs resolve only in a real run. @@ -224,7 +232,7 @@ The repo runs a review loop on every PR: local agent iteration plus remote autom 1. Required status checks are green (`mergeStateStatus: CLEAN`), **and** 2. A Copilot review is confirmed on the **current head SHA** by matching the review's commit SHA to the head, not an earlier push - a push makes required checks go green **before** the re-review lands, so a green merge-state can precede the current-head review and never signals readiness on its own, **and** -3. **Every** Copilot finding on that head SHA is closed out - all review threads resolved, **and** any issue-level Copilot comments (which have no resolve action) triaged and replied to - so zero outstanding findings remain, **and** +3. **Every** Copilot finding on that head SHA is closed out - all review threads resolved, **and** any issue-level Copilot comments (which have no resolve action) triaged and replied to, **and** the low-confidence findings collapsed in the review body investigated and answered, since those appear in no thread and a loop that polls threads alone reports a clean pass while they stand - so zero outstanding findings remain, **and** 4. The maintainer has given **explicit** permission to merge. `mergeStateStatus: CLEAN` reflects **only** required statuses - it never reflects open bot review comments, so `CLEAN` alone is **never** sufficient to merge. A green/`CLEAN` PR with an unresolved Copilot finding fails this gate; treat it as "not mergeable" no matter what the merge-state field says. The agent never merges on its own (consistent with "default to staging"; merging is maintainer-authorized). diff --git a/catalog/snippets/configs/vscode-tasks-python.json b/catalog/snippets/configs/vscode-tasks-python.json index 6d6a554c..137c627b 100644 --- a/catalog/snippets/configs/vscode-tasks-python.json +++ b/catalog/snippets/configs/vscode-tasks-python.json @@ -1,19 +1,24 @@ { "version": "2.0.0", "tasks": [ - // Python language group. Every command-executing task is `type: process` (like the .NET snippet; the - // aggregators are dependsOn-only) so the command is - // executed directly, never through a shell - it avoids `&&`/`;` chaining, which is not portable - // (Windows PowerShell 5.1, still the default VS Code task shell on many setups, rejects `&&`). - // Sequencing is expressed with `dependsOrder: sequence` + `dependsOn`, not shell operators. + // Python language group. + // Every command-executing task is `type: process`, as in the .NET snippet. + // The aggregators are dependsOn-only. + // The command runs directly rather than through a shell, which avoids `&&` and `;` chaining. + // That chaining is not portable, since Windows PowerShell 5.1 rejects `&&` and is still the default + // VS Code task shell on many setups. + // Sequencing is expressed with `dependsOrder: sequence` and `dependsOn`, never shell operators. // - // The first tasks are the Python clean-compile set (CODESTYLE.md "Local Development Loop"): - // `ruff format` -> `ruff check` -> the type checker. A repo whose CI type checker is mypy (pyright - // editor-only) swaps the "Python Types" command to `uv run mypy src`; a pyright-strict repo keeps it - // as `uv run pyright`. Adapt the target paths to the repo's package layout. - // A lint-only Scripts-profile subtree (no uv project, no `uv.lock`) runs every tool via `uvx ` not `uv run `. - // The type-checker swap above composes with it: `uvx mypy src` for a mypy repo, `uvx pyright` otherwise. - // It omits the pytest, coverage, and `uv sync` tasks (CODESTYLE.md Python 'Two profiles'). + // The first tasks are the Python clean-compile set, per CODESTYLE.md "Local Development Loop": + // `ruff format`, then `ruff check`, then the type checker. + // A repo whose CI type checker is mypy, with pyright editor-only, swaps "Python Types" to + // `uv run mypy src`. + // A pyright-strict repo keeps `uv run pyright`. + // Adapt the target paths to the repo's package layout. + // A lint-only Scripts-profile subtree, with no uv project and no `uv.lock`, runs every tool via + // `uvx ` rather than `uv run `. + // The type-checker swap composes with it: `uvx mypy src` for a mypy repo, `uvx pyright` otherwise. + // It omits the pytest, coverage, and `uv sync` tasks, per CODESTYLE.md Python "Two profiles". { "label": "Ruff Format", "type": "process", @@ -59,8 +64,9 @@ } }, { - // The clean-compile aggregator - formats in place, then lints, then type-checks, in order (no shell - // chaining). Named "Format" (it mutates, like the sibling ".NET Format" task), not "Verify". + // The clean-compile aggregator formats in place, then lints, then type-checks, in order. + // No shell chaining is involved. + // It is named "Format" rather than "Verify" because it mutates, like the sibling ".NET Format". "label": "Python Format", "dependsOrder": "sequence", "dependsOn": [ @@ -114,9 +120,11 @@ "clear": false } }, - // Lint group - run on demand via Docker (--pull=always forces each to re-pull the current :latest); mirrors the CI - // lint gate. Broad live spell-checking is the cspell extension's job. Pre-commit does formatting only. - // These tasks are language-agnostic - identical to the .NET snippet's Lint group. + // Lint group, run on demand via Docker, where --pull=always forces each to re-pull :latest. + // It mirrors the CI lint gate. + // Broad live spell-checking is the cspell extension's job. + // Pre-commit does formatting only. + // These tasks are language-agnostic, identical to the .NET snippet's Lint group. { "label": "Lint: EditorConfig", "type": "process", diff --git a/catalog/snippets/configs/vscode-tasks.json b/catalog/snippets/configs/vscode-tasks.json index 75f177ba..ee1998c8 100644 --- a/catalog/snippets/configs/vscode-tasks.json +++ b/catalog/snippets/configs/vscode-tasks.json @@ -1,9 +1,10 @@ { "version": "2.0.0", "tasks": [ - // .NET language group. A non-.NET repo drops this group and adds its own - // language's tasks. The first three tasks are the .NET clean-compile set - // (CODESTYLE.md) carried verbatim; the rest are convenience/project-specific. + // .NET language group. + // A non-.NET repo drops this group and adds its own language's tasks. + // The first three tasks are the .NET clean-compile set (CODESTYLE.md), carried verbatim. + // The rest are convenience or project-specific. { "label": ".NET Build", "type": "process", @@ -120,8 +121,10 @@ "clear": false } }, - // Lint group - run on demand via Docker (--pull=always pins each to the current :latest); mirrors the CI - // lint gate. Broad live spell-checking is the cspell extension's job. Pre-commit does formatting only. + // Lint group, run on demand via Docker, where --pull=always pins each to the current :latest. + // It mirrors the CI lint gate. + // Broad live spell-checking is the cspell extension's job. + // Pre-commit does formatting only. { "label": "Lint: EditorConfig", "type": "process", diff --git a/catalog/snippets/devcontainer/dotnet/devcontainer.json b/catalog/snippets/devcontainer/dotnet/devcontainer.json index dc5e36bc..7210018d 100644 --- a/catalog/snippets/devcontainer/dotnet/devcontainer.json +++ b/catalog/snippets/devcontainer/dotnet/devcontainer.json @@ -30,13 +30,14 @@ "remoteUser": "vscode", - // The bind-mount on macOS hosts surfaces /home/vscode/.ssh as root-owned; - // chown it back so writes from inside the container (known_hosts updates - // by gh / git) land cleanly. Idempotent on Linux/WSL2. + // The bind-mount on macOS hosts surfaces /home/vscode/.ssh as root-owned. + // Chown it back so writes from inside the container land cleanly. + // Those include the known_hosts updates gh and git make. + // Idempotent on Linux and WSL2. "onCreateCommand": "sudo install -d -m 700 -o vscode -g vscode /home/vscode/.ssh", - // Restore .NET local tools (csharpier, dotnet-outdated). No git hooks are - // installed by default - see README "Optional: enable git hooks locally". + // Restore the .NET local tools, csharpier and dotnet-outdated. + // No git hooks are installed by default, per README "Optional: enable git hooks locally". "postCreateCommand": ".devcontainer/dotnet/post-create.sh", "customizations": { diff --git a/catalog/snippets/devcontainer/python/devcontainer.json b/catalog/snippets/devcontainer/python/devcontainer.json index a36be913..05a466f8 100644 --- a/catalog/snippets/devcontainer/python/devcontainer.json +++ b/catalog/snippets/devcontainer/python/devcontainer.json @@ -30,13 +30,14 @@ "remoteUser": "vscode", - // The bind-mount on macOS hosts surfaces /home/vscode/.ssh as root-owned; - // chown it back so writes from inside the container (known_hosts updates - // by gh / git) land cleanly. Idempotent on Linux/WSL2. + // The bind-mount on macOS hosts surfaces /home/vscode/.ssh as root-owned. + // Chown it back so writes from inside the container land cleanly. + // Those include the known_hosts updates gh and git make. + // Idempotent on Linux and WSL2. "onCreateCommand": "sudo install -d -m 700 -o vscode -g vscode /home/vscode/.ssh", - // Install pinned uv and pre-warm the PyPiLibrary venv. No git hooks are - // installed by default - see README "Optional: enable git hooks locally". + // Install pinned uv and pre-warm the PyPiLibrary venv. + // No git hooks are installed by default, per README "Optional: enable git hooks locally". "postCreateCommand": ".devcontainer/python/post-create.sh", "customizations": { diff --git a/catalog/snippets/vscode/base.jsonc b/catalog/snippets/vscode/base.jsonc index 7a1d1625..be058dc8 100644 --- a/catalog/snippets/vscode/base.jsonc +++ b/catalog/snippets/vscode/base.jsonc @@ -1,8 +1,8 @@ -// Standard workspace fragment carried by every fleet repo: the shared editor -// settings and the extension set common to all repos. A repo's -// `.code-workspace` is `{ "folders": [{ "path": "." }] }` plus this base, -// merged with the per-type fragments (dotnet, python, docker) for the languages -// and targets it ships. +// Standard workspace fragment carried by every fleet repo. +// It holds the shared editor settings and the extension set common to all repos. +// A repo's `.code-workspace` is `{ "folders": [{ "path": "." }] }` plus this base. +// That is merged with the per-type fragments for the languages and targets it ships. +// The fragments are dotnet, python, and docker. { "settings": { "markdown.extension.toc.levels": "2..3", diff --git a/catalog/snippets/vscode/docker.jsonc b/catalog/snippets/vscode/docker.jsonc index 5657ae12..1ccce349 100644 --- a/catalog/snippets/vscode/docker.jsonc +++ b/catalog/snippets/vscode/docker.jsonc @@ -1,5 +1,5 @@ -// Docker additions: merge into the base fragment for a repo that ships a Docker -// image. The Docker extension adds authoring support only; no settings of its own. +// Docker additions, merged into the base fragment for a repo that ships a Docker image. +// The Docker extension adds authoring support only, with no settings of its own. { "extensions": { "recommendations": [ diff --git a/docs/token-cost.md b/docs/token-cost.md index 6104fbde..a9493915 100644 --- a/docs/token-cost.md +++ b/docs/token-cost.md @@ -89,7 +89,7 @@ This inverts the obvious fix. The `gh` output was already tiny at 574 bytes aver - **Model and effort tiering is unquantified.** The mix was 17,851 Opus requests, 390 Haiku, and zero Sonnet, with a global high effort setting, so the headroom is obvious. The saving is not: establishing it needs deliberate paired runs, and no number is claimed here. - **Fresh-context self-review is unproven.** An estimate put 44% of findings within reach of a reviewer reading the diff against the docs, but that was an estimate, and a same-model no-context review was tried and found far less than Copilot did. Removing context does not remove shared priors. Treat it as an experiment, and give any retest the finding taxonomy rather than a generic instruction to be adversarial. -- **The prose gate is warn-first.** Compare its hits against the next batch of review findings before enforcing it, and do not claim the mechanical share until that comparison exists. +- **The semicolon and dash rules are warn-first, and the charset and duplicate-word rules gate, being clean tree-wide.** Compare their hits against the next batch of review findings before enforcing them, and do not claim the mechanical share until that comparison exists. ## Re-measuring diff --git a/host-setup/agent-safety/.markdownlint-cli2.jsonc b/host-setup/agent-safety/.markdownlint-cli2.jsonc index 8eb5a70a..4e090637 100644 --- a/host-setup/agent-safety/.markdownlint-cli2.jsonc +++ b/host-setup/agent-safety/.markdownlint-cli2.jsonc @@ -1,7 +1,8 @@ { - // claude-md-safety.md is a fragment the installer appends into ~/.claude/CLAUDE.md (which already - // has its own H1), so it intentionally opens at H2. MD041 (first line must be a top-level heading) - // does not apply to an appended snippet. This nested config affects only this directory. + // The claude-md-safety.md fragment is appended by the installer into ~/.claude/CLAUDE.md. + // That file already has its own H1, so the fragment intentionally opens at H2. + // MD041 requires the first line to be a top-level heading, which an appended snippet is not. + // This nested config affects only this directory. "config": { "MD041": false } diff --git a/scripts/README.md b/scripts/README.md index 6ed3b1ed..913a1c74 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -17,7 +17,11 @@ uvx coverage@latest run -m unittest discover -s scripts && uvx coverage@latest r ## `prose_lint.py` -Enforces the [`GOVERNANCE.md`](../GOVERNANCE.md) "Documentation Style Conventions" rules that no linter checks: typographic Unicode where an ASCII equivalent exists, a semicolon joining two independent clauses, and a duplicated consecutive word. The two documented non-ASCII exceptions, scientific symbols and developer-typed characters, are deliberately not flagged. +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, and an un-tiered one is still reported as `charset-unknown` until it is classified. + +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. Run it scoped to changed lines, matching the standing rule that existing prose is corrected as each file is next edited rather than swept: @@ -25,13 +29,23 @@ Run it scoped to changed lines, matching the standing rule that existing prose i python3 scripts/prose_lint.py . --diff origin/develop ``` -Whole-tree (`python3 scripts/prose_lint.py .`) reports the legacy semicolon backlog as well, which is informational rather than a gate. `ascii` and `dupword` are clean tree-wide, so CI gates those two and leaves `semicolon` warn-only. +Whole-tree (`python3 scripts/prose_lint.py .`) reports the legacy backlog as well, which is informational rather than a gate. `charset` and `dupword` are clean tree-wide, so CI gates those two and reports the rest warn-only. **Scope** is every text file git tracks, binaries skipped by a NUL-byte check, with no extension allowlist: an allowlist covers what its author thought of and silently stops covering whatever is added next, which is the same reason the line-endings rule already requires `git ls-files` over a raw `find`. `--list-files` prints the discovered set for auditing. A double-quoted span in markdown is treated as a quotation and not scanned for prose rules, so a rule that states its own counter-example does not report the document that documents it. Outside markdown a double quote is structural, so the prose inside it still counts. -**Known recall gap:** the splice detector keys on a pronoun or article after the semicolon, so an imperative splice ("Delegate exploration; keep synthesis") is missed. Reading the semicolons in a diff by eye still catches what it cannot. +The `semicolon` and `dash` rules ban a construction rather than a detectable subset of it, so each flags by default and the exceptions are the ones the rule names: a semicolon inside a list that already carries commas, and for the dash a compound word, a leading list marker, a range, and the `- **Label** - explanation` separator that opens a governed bullet. + +**Both are markdown-only for now.** A shell script carries 78 statement separators that are not prose at all, so telling a comment from code is a precondition for reaching source files. Until then a semicolon or dash in a code comment is missed, which reading the diff by eye still catches. + +The `comment-wrap` rule covers comments in every syntax the fleet's project types carry, not only the hash ones: `//` and `/* */` for C#, C, C++ and JSONC, `/* */` alone for CSS, `` for XML, csproj and markdown, `<# #>` for PowerShell, `;` for INI, and `#` for Python, shell, YAML and TOML. + +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. Blanking is per line, which suits a string that ends on the line it starts. The C# verbatim string is the one form carried across lines. Every other syntax is scanned a line at a time, so any string that spans lines leaves its markers readable, and an ordinary quoted string in shell, a PowerShell here-string, a YAML block scalar, and a heredoc all report a marker inside them as a comment. A documentation comment (`///`, `/**`, a docstring) is left to CODESTYLE, which permits the paragraphs this rule forbids. + +A comment sentence also has to start with a capital, which `comment-case` checks. A lowercase opening reads as the continuation of the line above it, so the two rules are read together: a wrapped sentence reports as `comment-wrap`, and a lowercase opening that is not a continuation reports as `comment-case`. Where the first word is a tool whose own casing is lowercase, the fix is to restructure rather than to capitalize the name against CODESTYLE's tooling-casing rule. + +`charset` and `dupword` are clean tree-wide and gate CI. `charset-unknown`, `semicolon`, `dash`, `comment-wrap`, and `comment-case` run as one warn-only CI step, so the backlog is visible without blocking and is corrected as each file is next edited. ## `repo_gate.py` diff --git a/scripts/prose_lint.py b/scripts/prose_lint.py index cdff6cfa..cc9bd8d7 100644 --- a/scripts/prose_lint.py +++ b/scripts/prose_lint.py @@ -3,26 +3,34 @@ markdownlint, cspell, actionlint, and editorconfig-checker all pass on prose that breaks these rules, so nothing enforced them before this script. Rules implemented: - ascii Write ASCII in all agent-authored text. - semicolon No semicolon joining two independent clauses. + charset Non-ASCII judged against the three tiers the charset rule defines. + charset-unknown A non-ASCII character in no tier, so it is classified rather than assumed. + semicolon No semicolon in prose, outside a list that already carries commas. + dash No spaced hyphen joining or interrupting a sentence. + comment-wrap One sentence per comment line, never wrapped and never two on a line. + comment-case A comment sentence starts with a capital, not a lowercase word. dupword No duplicated consecutive word. sentence-split A sentence must not wrap across lines (one sentence per line). Exit 1 if any violation is found. Read-only, never edits. """ from __future__ import annotations -import argparse, re, subprocess, sys, unicodedata +import argparse, io, re, subprocess, sys, tokenize, unicodedata from pathlib import Path +from typing import TypedDict -# One source of truth for the rule names, so the CLI choices cannot drift from what check_file -# implements. Writing them out separately is how a rule exists in one place and not another. +# One source of truth for the rule names, so the CLI choices cannot drift from check_file. RULES = { - 'ascii': 'typographic Unicode where an ASCII equivalent exists', - 'semicolon': 'a semicolon joining two independent clauses', + 'charset': 'a non-ASCII character its tier does not permit here', + 'charset-unknown': 'a non-ASCII character in no tier', + 'semicolon': 'a semicolon in prose, outside a list that already carries commas', + 'dash': 'a spaced hyphen joining or interrupting a sentence', + 'comment-wrap': 'a comment sentence wrapped across lines, or two on one line', + 'comment-case': 'a comment sentence opening in lowercase', 'dupword': 'a duplicated consecutive word', 'sentence-split': 'a sentence wrapping across lines', } -DEFAULT_RULES = frozenset({'ascii', 'semicolon', 'dupword'}) +DEFAULT_RULES = frozenset({'charset', 'charset-unknown', 'semicolon', 'dash', 'dupword'}) # Produced rather than authored trees, consulted only on the no-git fallback path. # Where git can answer, its own ignore rules are the better answer. @@ -131,30 +139,174 @@ def discover(paths: list[str], excludes: tuple[str, ...] = ()) -> list[Path]: return sorted(set(keep)) -# Typographic Unicode the rule says to replace with its ASCII equivalent on sight. -# GOVERNANCE.md allows two narrow exceptions that are deliberately NOT flagged: -# scientific/technical symbols with no clean ASCII equivalent (ohm, micro, degree, -# pi, superscripts, section sign) and developer-typed Unicode such as emoji. -# Only substitutable typography appears here. -# Escapes, never literals - this file is scanned by the rule below, and a literal is invisible. -SUGGEST = { - '\u2014': '-', '\u2013': '-', '\u2018': "'", '\u2019': "'", - '\u201c': '"', '\u201d': '"', '\u2026': '...', '\u00a0': ' ', - '\u2022': '-', '\u2011': '-', '\u2192': '->', '\u21d2': '=>', - '\u2264': '<=', '\u2265': '>=', +# A non-ASCII character is typography in one place and meaning in another, so it is read by tier. +# Escapes, never literals: this file is scanned by the rule it implements. +# +# Tier 1 carries no meaning its ASCII form loses, so it always flags. +TIER1 = { + '\u2014': 'restructure', '\u2013': 'restructure', '\u2018': "'", + '\u2019': "'", '\u201c': '"', '\u201d': '"', + '\u2026': '...', '\u2022': '-', '\u00a0': ' ', + '\u2011': '-', '\u2192': '->', '\u21d2': '=>', } -# A semicolon splice: "; ..." -# Deliberately conservative - only flags a lowercase word after "; " that starts -# a clause with a following finite verb. List semicolons and code are not matched. -SPLICE = re.compile( - r';\s+(?Pit|this|that|they|he|she|we|you|the|a|an|there|these|those)\s+\w+', - re.IGNORECASE) +# Tier 2 is an operator, and only its use between two words is a finding. +TIER2 = { + '\u2264': '<=', '\u2265': '>=', '\u2260': '!=', + '\u00b1': '+/-', '\u2212': '-', '\u00d7': 'x', + '\u00f7': '/', '\u00b7': '.', +} + +# Tier 3 is a unit symbol whose ASCII form would be a lie, so it never flags. +TIER3 = frozenset({ + '\u00b5', '\u00b0', '\u2126', '\u03c0', '\u00b2', '\u00b3', '\u00a7', +}) + +# A digit, unit, or operator on either side makes a tier-2 character the range it describes. +NUMERIC = re.compile(r'[0-9]') + +# The rule bans the construction, not a detectable subset, so a prose semicolon flags by default. +# A pronoun-keyed pattern found 170 of 493 and missed every imperative splice. +SEMICOLON = re.compile(r';') + +# A spaced hyphen, the em-dash-style clause break and the paired aside alike. +# A compound word carries no spaces, a list marker nothing before it, and a range is digit-bounded. +DASH = re.compile(r'(?<=[^\s\d])\s+-\s+(?=[^\s\d])') + +# `- **Label** - explanation` is a definition separator, structurally a colon. +# Flagging it would restructure the document format rather than the prose. +# The first dash on such a line is skipped, and any later one still counts. +LABEL_DASH = re.compile(r'^\s*[-*]\s+\*\*[^*]+\*\*[.:]?\s+-\s+') # The negative lookbehind keeps a word-joining character from starting a repetition: # "either/or or must-pair" is one phrase followed by a conjunction, not a doubled word. DUPWORD = re.compile(r'(?'),), 'doc': (), 'quotes': '"', + 'verbatim': False} +POWERSHELL: Syntax = {'line': ('#',), 'block': (('<#', '#>'),), 'doc': (), 'quotes': '"\'', + 'verbatim': False} +INI: Syntax = {'line': ('#', ';'), 'block': (), 'doc': (), 'quotes': '"\'', 'verbatim': False} +LISP_LIKE: Syntax = {'line': ('#',), 'block': (), 'doc': (), 'quotes': '"', 'verbatim': False} +# CSS has block comments only, so a `//` in it is the scheme separator of a URL. +CSS: Syntax = {'line': (), 'block': (('/*', '*/'),), 'doc': (), 'quotes': '"\'', 'verbatim': False} + +SYNTAX: dict[str, Syntax] = { + # Python, shell, and the hash-commented configs + '.py': HASH, '.sh': HASH, '.bash': HASH, '.yml': HASH, '.yaml': HASH, + '.toml': HASH, '.tf': HASH, '.gitattributes': HASH, '.gitignore': HASH, + # C#, C, and C++ + '.cs': CSHARP, '.c': C_LIKE, '.cpp': C_LIKE, '.cc': C_LIKE, '.cxx': C_LIKE, + '.h': C_LIKE, '.hpp': C_LIKE, '.jsonc': C_LIKE, '.json5': C_LIKE, + '.js': C_LIKE, '.ts': C_LIKE, '.css': CSS, '.scss': CSS, + # JSON carries comments in practice, which is what JSONC names. + # VS Code tasks, launch, devcontainer, and workspace files ship them under a plain .json name. + '.json': C_LIKE, '.code-workspace': C_LIKE, + # Markup and project files + '.md': XML_LIKE, '.html': XML_LIKE, '.xml': XML_LIKE, '.csproj': XML_LIKE, + '.props': XML_LIKE, '.targets': XML_LIKE, '.slnx': XML_LIKE, '.resx': XML_LIKE, + # PowerShell, INI, and EDA + '.ps1': POWERSHELL, '.psm1': POWERSHELL, + '.ini': INI, '.cfg': INI, '.conf': INI, '.editorconfig': INI, + '.kicad_sch': LISP_LIKE, '.kicad_pcb': LISP_LIKE, '.kicad_mod': LISP_LIKE, +} + +# Extensionless files whose name fixes the syntax. +BY_NAME = { + 'dockerfile': HASH, 'makefile': HASH, 'pre-commit': HASH, 'gemfile': HASH, + 'caddyfile': HASH, '.gitattributes': HASH, '.editorconfig': INI, '.gitignore': HASH, +} + +# JSON proper carries no comments, so a `//` in one is data. +NO_COMMENTS = frozenset({'.lock', '.csv', '.tsv', '.txt', '.svg', '.min'}) + + +def syntax_for(path: Path) -> Syntax | None: + """The comment syntax for this file, or None when it carries no comments.""" + name = path.name.lower() + if name in BY_NAME: + return BY_NAME[name] + suffix = path.suffix.lower() + if suffix in NO_COMMENTS: + return None + if suffix in SYNTAX: + return SYNTAX[suffix] + return HASH if not suffix else None + + +def strip_strings(line: str, quotes: str, verbatim: bool = False, + carried: bool = False) -> tuple[str, bool]: + """Blank quoted spans so a comment marker inside a string is not read as one. + + Length-preserving, so an offset into the result is an offset into the line. + A verbatim string takes its own rules where the syntax has one. + There the backslash is an ordinary character and a doubled quote is the escape, so reading a + backslash as an escape consumes the closing quote and blanks the rest of the line. + It also spans lines, so `carried` opens one and the second return says it is still open. + """ + out = list(line) + quote = '"' if carried else '' + inside_verbatim = carried + escaped = False + i = 0 + while i < len(line): + ch = line[i] + if escaped: + escaped = False + out[i] = ' ' + elif inside_verbatim: + if ch == quote and line[i + 1:i + 2] == quote: # a doubled quote is one character + out[i] = out[i + 1] = ' ' + i += 2 + continue + out[i] = ' ' if ch != quote else ch + if ch == quote: + quote, inside_verbatim = '', False + elif ch == '\\' and quote: + escaped = True + out[i] = ' ' + elif quote: + out[i] = ' ' if ch != quote else ch + if ch == quote: + quote = '' + 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 + while start > 0 and line[start - 1] in '@$': + start -= 1 + inside_verbatim = verbatim and ch == '"' and '@' in line[start:i] + i += 1 + return ''.join(out), inside_verbatim + + +# A pragma, shebang, or divider is machinery rather than prose. +NOT_PROSE = re.compile(r'^(!|\s*[-=#*/<>]+\s*$)|noqa|type:\s*ignore|pylint|ruff:|mypy:|shellcheck' + r'|cSpell|markdownlint|omit from toc|prettier|eslint|SPDX|Copyright' + r'|^v\d+(\.\d+)*$') + +# Two sentences on one line, guarded against an abbreviation, an initial, or a dotted identifier. +# The initial guard anchors on a word boundary: `J. Smith` is one name, where a sentence ending in +# an acronym such as CI is two sentences and has to be caught. +# The second sentence may open in either case, since a lowercase opening is still a second sentence. +RUN_ON = re.compile(r'(? str: return re.sub(r'"[^"\n]*"', '""', s) +def in_numeric_context(line: str, pos: int) -> bool: + """Whether the character at `pos` sits in an expression rather than in a sentence. + + The discriminator is what flanks it once spaces are skipped. A digit, a tier-3 unit, or + another operator on either side makes it the range it describes. A word on both sides makes + it prose, which is the case the ASCII form is for. + """ + def neighbor(step: int) -> str: + j = pos + step + while 0 <= j < len(line) and line[j].isspace(): + j += step + return line[j] if 0 <= j < len(line) else '' + + return any(c and (NUMERIC.match(c) or c in TIER3 or c in TIER2) + for c in (neighbor(-1), neighbor(1))) + + +def charset_findings(lineno: int, line: str) -> list[tuple[int, str, str]]: + """Every non-ASCII character on the line, judged against its tier. + + An unrecognized character is reported rather than passed. A gate that allows whatever it does + not recognize stops gating as the character set grows. + """ + out: list[tuple[int, str, str]] = [] + for pos, ch in enumerate(line): + if ch.isascii(): + continue + name = unicodedata.name(ch, f'U+{ord(ch):04X}') + if ch in TIER3: + continue + if ch in TIER1: + fix = TIER1[ch] + hint = 'restructure the sentence' if fix == 'restructure' else f"use '{fix}'" + out.append((lineno, 'charset', f'{name} (U+{ord(ch):04X}) -> {hint}')) + elif ch in TIER2: + if not in_numeric_context(line, pos): + out.append((lineno, 'charset', + f"{name} (U+{ord(ch):04X}) in prose -> use '{TIER2[ch]}'")) + else: + out.append((lineno, 'charset-unknown', + f'{name} (U+{ord(ch):04X}) is in no tier - classify it in GOVERNANCE.md')) + return out + + +def python_comments(raw: str) -> list[tuple[int, str, bool]] | None: + """Every comment in Python source as (line, text, starts-the-line), or None if it will not parse. + + `tokenize` rather than a regex because a `#` inside a string literal is not a comment, and a + trailing comment is one a line-anchored pattern never sees. + """ + out: list[tuple[int, str, bool]] = [] + try: + for tok in tokenize.generate_tokens(io.StringIO(raw).readline): + if tok.type == tokenize.COMMENT: + leading = not tok.line[:tok.start[1]].strip() + out.append((tok.start[0], tok.string.lstrip('#').strip(), leading)) + except (tokenize.TokenError, IndentationError, SyntaxError, ValueError): + return None + return out + + +def extracted_comments(path: Path, lines: list[str]) -> list[tuple[int, str, bool]]: + """Every comment in the file as (line, text, starts-the-line), for any syntax the fleet uses. + + A marker inside a string literal is not a comment, so each line is scanned with quoted spans + blanked first. A documentation comment is skipped: CODESTYLE governs those and permits the + paragraphs this rule forbids. + """ + spec = syntax_for(path) + if spec is None: + return [] + out: list[tuple[int, str, bool]] = [] + closing = '' + doc_closing = '' + in_string = False + for n, raw in enumerate(lines, 1): + line = raw.rstrip('\r') + pos = 0 + if doc_closing: # CODESTYLE owns every line until it closes + end = line.find(doc_closing) + if end < 0: + continue + pos, doc_closing = end + len(doc_closing), '' + elif closing: # carried in from an unclosed block + end = line.find(closing) + body = (line if end < 0 else line[:end]).strip() + # Only `/* */` continues a line with a leading `*`, and only on a line it continues. + # Taking it off anywhere else edits the prose the rules then judge. + # The marker is one `*` against whitespace, so `**bold**` and `*emphasis*` keep theirs. + if closing == '*/' and body.startswith('*') and body[1:2].isspace(): + body = body[1:].strip() + if body: + out.append((n, body, True)) + if end < 0: + continue + pos, closing = end + len(closing), '' + # Scan left to right and take whichever marker comes first. + # A ceiling can only describe the first comment, so a later one was unreachable. + while pos < len(line): + # Mask from here rather than once per line, so comment text never sets string state. + # A quote in a comment is prose, and reading it as a string blanks the markers after it. + tail, tail_state = strip_strings(line[pos:], spec['quotes'], spec['verbatim'], in_string) + masked = ' ' * pos + tail + found: str | tuple[str, str] | None = None + at = len(line) + for marker in spec['line']: + where = masked.find(marker, pos) + if 0 <= where < at: + at, found = where, marker + for opener, closer in spec['block']: + where = masked.find(opener, pos) + if 0 <= where < at: + at, found = where, (opener, closer) + if found is None: + in_string = tail_state # the rest of the line is code + break + # Only the code before the marker advances the string state. + _, in_string = strip_strings(line[pos:at], spec['quotes'], spec['verbatim'], in_string) + # CODESTYLE owns a documentation comment, so this rule skips over it. + # A line one runs to end of line, while a closed block one gives the rest back. + if any(line[at:].startswith(d) for d in spec['doc']): + if isinstance(found, str): + break + end = line.find(found[1], at + len(found[0])) + if end < 0: + doc_closing = found[1] # it carries on into the lines below + break + pos = end + len(found[1]) + continue + leading = not line[:at].strip() + if isinstance(found, str): # a line comment runs to end of line + body = line[at + len(found):].strip() + if body: + out.append((n, body, leading)) + break + 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() + if body: + out.append((n, body, leading)) + if end < 0: + closing = closer + break + pos = end + len(closer) + return out + + +def fenced_lines(lines: list[str]) -> set[int]: + """Line numbers inside a fenced block, which every rule skips. + + A fenced example is quoted code rather than this file's own prose, so a comment in one belongs + to whatever is being shown. + """ + out: set[int] = set() + in_fence = False + for n, raw in enumerate(lines, 1): + if CODE_FENCE.match(raw.rstrip('\r')): + in_fence = not in_fence + out.add(n) + continue + if in_fence: + out.add(n) + return out + + +def comment_wrap_findings(path: Path, raw: str, lines: list[str]) -> list[tuple[int, str, str]]: + """Comment lines whose sentence wraps into the next, or that carry two sentences. + + The rule is one sentence per comment line. A wrapped sentence is the common failure, and a + run-on is the other half of the same rule, so both are reported. + """ + comments = python_comments(raw) if path.suffix == '.py' else None + if comments is None: + comments = extracted_comments(path, lines) + skip = fenced_lines(lines) + comments = [c for c in comments if c[0] not in skip] + + out: list[tuple[int, str, str]] = [] + prev_body = '' + prev_no = 0 + for n, body, leading in comments: + if not body or NOT_PROSE.search(body): + prev_body = '' + continue + if RUN_ON.search(strip_inline_code(body)): + out.append((n, 'comment-wrap', 'two sentences on one comment line -> split them')) + # A continuation is the very next line: two comments with code between them are separate. + adjacent = n == prev_no + 1 + continuation = (adjacent and leading and prev_body + and not SENT_END.search(prev_body) and body[:1].islower()) + if continuation: + out.append((prev_no, 'comment-wrap', + 'comment sentence wraps into the next line -> one sentence per line')) + # A lowercase opening that is not a continuation is a sentence that failed to start. + elif leading and body[:1].islower(): + out.append((n, 'comment-case', + 'comment sentence opens in lowercase -> capitalize, or restructure so it ' + 'does not open on a lowercase name')) + # A trailing comment can start a sentence the next full-line comment continues, so it is + # remembered. The continuation itself still has to be a full-line comment, since a + # trailing one annotates its own line rather than continuing the line above. + prev_body = body + prev_no = n + return out + + def check_file(path: Path, rules: set[str]) -> list[tuple[int, str, str]]: out: list[tuple[int, str, str]] = [] try: @@ -182,6 +540,8 @@ def check_file(path: Path, rules: set[str]) -> list[tuple[int, str, str]]: except (UnicodeDecodeError, OSError): return out lines = raw.split('\n') + if {'comment-wrap', 'comment-case'} & rules: + out.extend(f for f in comment_wrap_findings(path, raw, lines) if f[1] in rules) in_fence = False prev_txt = '' prev_no = 0 @@ -194,21 +554,30 @@ def check_file(path: Path, rules: set[str]) -> list[tuple[int, str, str]]: if in_fence: continue - if 'ascii' in rules: - for ch in set(line): - fix = SUGGEST.get(ch) - if fix is None: - continue - name = unicodedata.name(ch, f'U+{ord(ch):04X}') - out.append((i, 'ascii', f"typographic {name} -> use '{fix}'")) + if 'charset' in rules or 'charset-unknown' in rules: + out.extend(f for f in charset_findings(i, line) if f[1] in rules) txt = strip_inline_code(line) prose = strip_quoted(txt) if path.suffix == '.md' else txt - if 'semicolon' in rules: - m = SPLICE.search(prose) - if m: - out.append((i, 'semicolon', f"semicolon splice before '{m.group('w')}'")) + # Both prose rules are markdown-only until a comment can be told from code. + # A shell script carries 78 statement separators that are not prose at all. + if path.suffix == '.md': + if 'semicolon' in rules: + listish = prose.count(';') > 1 or ':' in prose.split(';')[0] + for m in SEMICOLON.finditer(prose): + # A list keeps its semicolons, and a list announces itself with a colon or by + # having more than one separator. + if listish and ',' in prose[:m.start()]: + continue + out.append((i, 'semicolon', 'semicolon in prose -> a comma or two sentences')) + if 'dash' in rules: + skip = LABEL_DASH.match(prose) + for m in DASH.finditer(prose): + if skip and m.start() < skip.end(): + continue + out.append((i, 'dash', + 'spaced hyphen -> a comma, two sentences, or parentheses')) if 'dupword' in rules: for m in DUPWORD.finditer(prose): @@ -222,7 +591,7 @@ def check_file(path: Path, rules: set[str]) -> list[tuple[int, str, str]]: and not re.match(r'^\s*\[[^\]]+\]:', stripped)) if prev_txt and is_prose: p = prev_txt.strip() - # previous prose line ended mid-sentence and this line continues it + # The previous prose line ended mid-sentence, and this line continues it. if (p and not SENT_END.search(p) and not p.endswith((':', '-', '|')) and stripped[0].islower()): out.append((prev_no, 'sentence-split', diff --git a/scripts/test_prose_lint.py b/scripts/test_prose_lint.py index bb66db02..179d9fcc 100644 --- a/scripts/test_prose_lint.py +++ b/scripts/test_prose_lint.py @@ -17,9 +17,8 @@ REPO = Path(__file__).resolve().parent.parent GOVERNANCE = REPO / 'GOVERNANCE.md' -# Bait for the duplicate-word rule, assembled so this module does not itself hold the pattern it -# feeds the gate. A test file full of rejected input would otherwise report itself, which is the -# self-flagging problem the quoted-span exemption solves for prose. +# Bait assembled from two literals, so this module never holds the pattern it feeds the gate. +# A file full of rejected input would otherwise report itself. DUP = 'the ' + 'the' SPLICE_BAIT = 'It runs on push; ' + 'it gates the merge' @@ -36,19 +35,53 @@ def kinds(self, text: str, rules: set[str], name: str = 'bait.md') -> list[str]: return [kind for _, kind, _ in prose_lint.check_file(path, rules)] -class TestSuggestTable(BaitCase): - def test_every_entry_is_caught(self) -> None: - """Each table key, placed in a line, produces one ascii finding.""" - for ch, fix in prose_lint.SUGGEST.items(): +class TestTierTables(BaitCase): + def test_every_tier_one_character_is_always_caught(self) -> None: + """Tier 1 carries no meaning its ASCII form loses, so context never excuses it.""" + for ch, fix in prose_lint.TIER1.items(): with self.subTest(codepoint=f'U+{ord(ch):04X}', fix=fix): - self.assertEqual(['ascii'], self.kinds(f'left {ch} right\n', {'ascii'})) + self.assertEqual(['charset'], self.kinds(f'left {ch} right\n', {'charset'})) + self.assertEqual(['charset'], self.kinds(f'8 {ch} 9\n', {'charset'})) + + def test_a_tab_separates_an_operator_from_its_neighbor(self) -> None: + """Any whitespace, not only a literal space, sits between an operator and its figure.""" + for gap in (' ', '\t', ' '): + with self.subTest(gap=repr(gap)): + self.assertEqual([], self.kinds(f'threshold{gap}{chr(0x2264)}{gap}35\n', {'charset'})) + + def test_every_tier_two_character_turns_on_its_neighbors(self) -> None: + """The same operator is the range it describes next to a figure, and prose between words.""" + for ch in prose_lint.TIER2: + with self.subTest(codepoint=f'U+{ord(ch):04X}'): + self.assertEqual([], self.kinds(f'threshold {ch} 35 units\n', {'charset'})) + self.assertEqual([], self.kinds(f'0 {ch} x\n', {'charset'})) + self.assertEqual(['charset'], + self.kinds(f'the check {ch} the threshold\n', {'charset'})) + + def test_every_tier_three_character_is_left_alone(self) -> None: + """A unit symbol whose ASCII form would be a lie is kept in prose and in a table alike.""" + for ch in prose_lint.TIER3: + with self.subTest(codepoint=f'U+{ord(ch):04X}'): + self.assertEqual([], self.kinds(f'reads 35 {ch} today\n', {'charset'})) + self.assertEqual([], self.kinds(f'the {ch} value\n', {'charset'})) + + def test_an_unclassified_character_is_reported_not_passed(self) -> None: + """A gate that allows whatever it does not recognize stops gating as the set grows.""" + unknown = chr(0x2603) + self.assertNotIn(unknown, prose_lint.TIER1) + self.assertNotIn(unknown, prose_lint.TIER2) + self.assertNotIn(unknown, prose_lint.TIER3) + self.assertEqual(['charset-unknown'], + self.kinds(f'a {unknown} here\n', {'charset-unknown'})) def test_keys_are_single_non_ascii_characters(self) -> None: """A key is a substitution target, so an ASCII key would flag text that is already fine.""" - for ch in prose_lint.SUGGEST: - with self.subTest(codepoint=f'U+{ord(ch):04X}'): - self.assertEqual(1, len(ch)) - self.assertFalse(ch.isascii()) + for label, table in (('1', prose_lint.TIER1), ('2', prose_lint.TIER2), + ('3', prose_lint.TIER3)): + for ch in table: + with self.subTest(tier=label, codepoint=f'U+{ord(ch):04X}'): + self.assertEqual(1, len(ch)) + self.assertFalse(ch.isascii()) def test_replacements_are_printable_ascii(self) -> None: """The suggested form has to be typeable. @@ -57,10 +90,11 @@ def test_replacements_are_printable_ascii(self) -> None: above 32 and are just as invisible in a diff. Applied to the replacements only, since the keys are non-ASCII by construction and two of them (U+00A0, U+2011) are not printable. """ - for ch, fix in prose_lint.SUGGEST.items(): - with self.subTest(codepoint=f'U+{ord(ch):04X}', fix=fix): - self.assertTrue(fix.isascii()) - self.assertTrue(fix.isprintable()) + for label, table in (('1', prose_lint.TIER1), ('2', prose_lint.TIER2)): + for ch, fix in table.items(): + with self.subTest(tier=label, codepoint=f'U+{ord(ch):04X}', fix=fix): + self.assertTrue(fix.isascii()) + self.assertTrue(fix.isprintable()) def test_the_source_carries_no_literal_non_ascii(self) -> None: """The table is written as escapes. @@ -74,24 +108,61 @@ def test_the_source_carries_no_literal_non_ascii(self) -> None: if not c.isascii()] self.assertEqual([], bad) - def test_the_table_covers_a_plausible_number_of_characters(self) -> None: + def test_each_tier_covers_a_plausible_number_of_characters(self) -> None: """A table that shrank to nothing would satisfy every case above by having no entries.""" - self.assertGreaterEqual(len(prose_lint.SUGGEST), 14) + for label, table, floor in (('1', prose_lint.TIER1, 12), + ('2', prose_lint.TIER2, 8), + ('3', prose_lint.TIER3, 7)): + with self.subTest(tier=label): + self.assertGreaterEqual(len(table), floor) class TestGovernanceCoupling(unittest.TestCase): - def test_every_codepoint_the_charset_rule_names_is_covered(self) -> None: - """The rule text drives the table, rather than a copy of the rule driving it. + """The rule text drives the tables, rather than a copy of the rule driving them. - This is the case that catches an incomplete table, which no bait built from the table - itself can do. Three characters in SUGGEST are deliberately not named by the rule - (U+00A0, U+2022, U+2011), so the relation is one-directional. - """ - named = {int(m, 16) for m in re.findall(r'U\+([0-9A-Fa-f]{4})', - GOVERNANCE.read_text(encoding='utf-8'))} - self.assertGreaterEqual(len(named), 8, 'the doc parse found almost nothing, the anchor moved') - missing = sorted(f'U+{cp:04X}' for cp in named if chr(cp) not in prose_lint.SUGGEST) - self.assertEqual([], missing) + These are the cases that catch an incomplete or mis-tiered table, which no bait built from the + tables themselves can do: bait proves the matching works, not that the data is right. + """ + + def setUp(self) -> None: + self.doc = GOVERNANCE.read_text(encoding='utf-8') + section = re.search(r'^### Character Set$(.*?)^### ', self.doc, re.M | re.S) + if section is None: + self.fail('the Character Set heading moved, so the parse is blind') + self.section = section.group(1) + + def tier_codepoints(self, label: str) -> set[int]: + """Codepoints named in one tier's bullet, read out of the rule text itself.""" + m = re.search(rf'^- \*\*Tier {label},(.*?)(?=^- \*\*)', self.section, re.M | re.S) + if m is None: + self.fail(f'the Tier {label} bullet moved, so the parse is blind') + return {int(h, 16) for h in re.findall(r'U\+([0-9A-Fa-f]{4})', m.group(1))} + + def test_every_tier_names_a_plausible_number_of_characters(self) -> None: + """A tier bullet that stopped parsing would make every case below it pass vacuously.""" + for label, floor in (('1', 10), ('2', 8), ('3', 7)): + with self.subTest(tier=label): + self.assertGreaterEqual(len(self.tier_codepoints(label)), floor) + + def test_tier_one_and_two_are_in_the_gate_tables(self) -> None: + for label, table in (('1', prose_lint.TIER1), ('2', prose_lint.TIER2)): + for cp in sorted(self.tier_codepoints(label)): + with self.subTest(tier=label, codepoint=f'U+{cp:04X}'): + self.assertIn(chr(cp), table) + + def test_tier_three_is_allowed_rather_than_replaced(self) -> None: + """A tier-3 symbol in a replacement table would flag the character the rule protects.""" + for cp in sorted(self.tier_codepoints('3')): + with self.subTest(codepoint=f'U+{cp:04X}'): + self.assertIn(chr(cp), prose_lint.TIER3) + self.assertNotIn(chr(cp), prose_lint.TIER1) + self.assertNotIn(chr(cp), prose_lint.TIER2) + + def test_the_tiers_do_not_overlap(self) -> None: + t1, t2, t3 = set(prose_lint.TIER1), set(prose_lint.TIER2), set(prose_lint.TIER3) + self.assertEqual(set(), t1 & t2) + self.assertEqual(set(), t1 & t3) + self.assertEqual(set(), t2 & t3) class TestDupword(BaitCase): @@ -119,11 +190,6 @@ def test_a_quoted_counter_example_is_exempt_in_markdown(self) -> None: """A rule that states its counter-example quotes the construction it bans.""" self.assertEqual([], self.kinds(f'Recast "{SPLICE_BAIT}" as two.\n', {'semicolon'})) - def test_a_quoted_span_is_not_exempt_outside_markdown(self) -> None: - """In data and code a double quote is structural, so the prose inside it still counts.""" - self.assertEqual(['semicolon'], - self.kinds(f'{{ "note": "{SPLICE_BAIT}" }}\n', {'semicolon'}, name='bait.json')) - def test_a_list_semicolon_is_not_a_splice(self) -> None: self.assertEqual([], self.kinds('Inputs: a, b, and c; outputs: d and e.\n', {'semicolon'})) @@ -131,6 +197,335 @@ def test_a_fenced_block_is_skipped(self) -> None: self.assertEqual([], self.kinds('```sh\nrun; it exits\n```\n', {'semicolon'})) +class TestDash(BaitCase): + def test_a_clause_break_is_flagged(self) -> None: + self.assertEqual(['dash'], self.kinds('It is capability, not permission - a token is not.\n', + {'dash'})) + + def test_a_paired_aside_is_flagged_at_both_ends(self) -> None: + self.assertEqual(['dash', 'dash'], + self.kinds('The router - a thin file - holds the map.\n', {'dash'})) + + def test_a_label_separator_is_exempt(self) -> None: + """`- **Label** - explanation` is structurally a colon and the shape every bullet uses.""" + self.assertEqual([], self.kinds('- **Bug** - wrong behavior, missing coverage\n', {'dash'})) + + def test_a_later_dash_on_a_label_line_still_counts(self) -> None: + """Exempting the separator must not exempt the rest of the line.""" + self.assertEqual(['dash'], + self.kinds('- **Bug** - wrong behavior - and worse besides\n', {'dash'})) + + def test_compound_words_and_ranges_are_left_alone(self) -> None: + for text in ('A well-named must-pair input.\n', 'Sections D1 - D9 apply.\n', + '- a plain list item\n'): + with self.subTest(text=text.strip()): + self.assertEqual([], self.kinds(text, {'dash'})) + + +class TestSemicolon2(BaitCase): + def test_any_prose_semicolon_is_flagged(self) -> None: + """The rule bans the construction, so the default is to flag rather than to detect a subset.""" + self.assertEqual(['semicolon'], self.kinds(f'{SPLICE_BAIT}.\n', {'semicolon'})) + + def test_the_imperative_splice_the_old_pattern_missed_is_caught(self) -> None: + """A pronoun-keyed pattern found 170 of 493, and this shape was the documented gap.""" + self.assertEqual(['semicolon'], + self.kinds('Delegate exploration; keep synthesis.\n', {'semicolon'})) + + def test_a_list_that_already_carries_commas_keeps_its_semicolon(self) -> None: + self.assertEqual([], self.kinds('Inputs: a, b, and c; outputs: d and e.\n', {'semicolon'})) + + def test_a_splice_whose_clause_carries_a_comma_is_still_a_splice(self) -> None: + """A comma earlier on the line is not a list, so it cannot excuse the semicolon.""" + self.assertEqual(['semicolon'], + self.kinds('It runs on push, always; it gates the merge.\n', {'semicolon'})) + + def test_prose_rules_do_not_reach_code_files(self) -> None: + """A shell script carries statement separators, not prose, until comments can be extracted.""" + for name in ('bait.sh', 'bait.py', 'bait.yml'): + with self.subTest(name=name): + self.assertEqual([], self.kinds('a=1; b=2; it runs\n', {'semicolon', 'dash'}, + name=name)) + + +class TestCommentWrap(BaitCase): + """The comment rule reaches every syntax the fleet's project types carry, not only the hash ones.""" + + RUN_ON = 'One thing happens. Another thing happens.' + + def flag(self, name: str, text: str) -> list[str]: + """Both comment kinds, so a case cannot pass by asking for the rule it does not test.""" + return self.kinds(text, {'comment-wrap', 'comment-case'}, name=name) + + def test_a_run_on_is_caught_in_every_comment_syntax(self) -> None: + """One case per syntax, so a failure names the language whose extractor broke.""" + 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'), + ('a.py', f'x = 1 # {self.RUN_ON}\n'), + ('a.sh', f'# {self.RUN_ON}\n'), + ('a.ps1', f'<# {self.RUN_ON} #>\n'), + ('a.yml', f'# {self.RUN_ON}\n'), + ('a.toml', f'# {self.RUN_ON}\n'), + ('a.ini', f'; {self.RUN_ON}\n'), + ('a.jsonc', f'// {self.RUN_ON}\n'), + ('a.json', f'// {self.RUN_ON}\n'), + ('a.code-workspace', f'// {self.RUN_ON}\n'), + ('a.xml', f'\n'), + ('a.csproj', f'\n'), + ): + with self.subTest(file=name, syntax=text.strip()[:12]): + self.assertEqual(['comment-wrap'], self.flag(name, text)) + + def test_json_carries_comments_because_jsonc_is_what_ships(self) -> None: + """VS Code tasks, devcontainer, and workspace files ship comments under a plain .json name.""" + for name in ('a.json', 'a.code-workspace', 'a.jsonc'): + with self.subTest(file=name): + self.assertEqual(['comment-wrap'], self.flag(name, f'// {self.RUN_ON}\n')) + + def test_a_marker_inside_a_string_is_not_a_comment(self) -> None: + for name, text in (('a.cs', 'var s = "// no. Really.";\n'), + ('a.sh', 'echo "# no. Really."\n'), + ('a.json', '{"url": "https://x/y. Z"}\n'), + ('a.py', 'u = "http://x/#f. G"\n')): + with self.subTest(file=name): + self.assertEqual([], self.flag(name, text)) + + def test_a_documentation_comment_is_left_to_codestyle(self) -> None: + """An XML doc comment and a docstring may run to paragraphs, which CODESTYLE governs.""" + self.assertEqual([], self.flag('a.cs', f'/// {self.RUN_ON}\n')) + self.assertEqual([], self.flag('a.py', f'"""{self.RUN_ON}"""\n')) + + def test_a_closed_block_doc_gives_the_rest_of_the_line_back(self) -> None: + """CODESTYLE owns the documentation comment, not the line it happens to sit on. + + A line doc comment does run to end of line, so only the block form gives anything back. + """ + self.assertEqual([], self.flag('a.cs', f'/** {self.RUN_ON} */\n')) + self.assertEqual([], self.flag('a.cs', f'/// {self.RUN_ON} // and more\n')) + self.assertEqual(['comment-wrap'], + self.flag('a.cs', '/** Docs. */ // Two things. Here.\n')) + + def test_a_multi_line_doc_block_owns_every_line_until_it_closes(self) -> None: + """A marker in documentation text is prose, so scanning those lines invents comments. + + The closing line still gives back what follows the closer, which is the one finding here. + """ + self.assertEqual(['comment-wrap'], self.flag('a.cs', '/** Docs start\n' + ' * // Two things. Here.\n' + ' * /* not an opener\n' + ' */ // Two things. Here.\n')) + + def test_verbatim_rules_apply_to_the_double_quoted_form_only(self) -> None: + """C# spells a verbatim string with double quotes, so `@` on a char literal is ordinary. + + Under verbatim rules the doubled quote is one escaped character and both are blanked, + so counting what survives tells the two readings apart. + """ + masked, _ = prose_lint.strip_strings("var c = @'a''b'; // t", '"\'', True) + self.assertEqual(4, masked.count("'")) + + def test_a_verbatim_string_spans_lines(self) -> None: + """It is the one string form here that carries, so masking per line invents comments. + + The line that closes it still gives back what follows the quote. + """ + # The marker on the second line is string content, so nothing is reported. + self.assertEqual([], self.flag('a.cs', 'var s = @"line one\n' + '// Two things. Here.\n' + 'line three";\n')) + # The line that closes it still gives back the comment after the quote. + self.assertEqual(['comment-wrap'], self.flag('a.cs', 'var s = @"line one\n' + 'line two"; // Two things. Here.\n')) + # A plain string ends on its own line, so the next line is ordinary code. + self.assertEqual(['comment-wrap'], self.flag('a.cs', 'var s = "line one";\n' + '// Two things. Here.\n')) + # Closing one and opening another leaves real code between them, which is not string content. + self.assertEqual(['comment-wrap'], + self.flag('a.cs', 'var s = @"start\n' + 'end"; /* Two things. Here. */ var t = @"open again\n' + 'still string";\n')) + + def test_a_quote_in_comment_text_is_prose_rather_than_a_string(self) -> None: + """Masking the comment too lets its quote open a string that blanks the markers after it. + + Within the line that costs the block its closer, and across lines the state carries and + blanks every marker below until something closes it. + """ + self.assertEqual(['comment-wrap'], + self.flag('a.cs', 'code(); /* note @"x */ code2(); // Two things. Here.\n')) + # Each block line is its own sentence, so the two findings are the recovered comments. + self.assertEqual(['comment-wrap', 'comment-wrap'], + self.flag('a.cs', '/* A note about @"paths.\n' + ' And more. */ // Two things. Here.\n' + 'var x = 1; // Two things. Here.\n')) + + def test_only_a_c_style_continuation_loses_its_leading_asterisk(self) -> None: + """The `*` continuing a `/* */` line is punctuation, and anywhere else it is prose. + + Taking it off an emphasis marker leaves a lowercase opening that the case rule reports, + which is the rule judging text the extractor damaged. + """ + for name, text in (('a.md', '\n'), + ('a.ps1', '<# *emphasis* leads here #>\n'), + ('a.cs', '/* *emphasis* leads here */\n')): + with self.subTest(file=name): + self.assertEqual([], self.flag(name, text)) + # The convention still holds on the lines it was written for. + self.assertEqual([(1, 'Start here.', True), (2, 'Still going.', True)], + prose_lint.extracted_comments(Path('a.cs'), + ['/* Start here.', ' * Still going. */'])) + # The marker is one `*` against whitespace, so a continuation keeps its own emphasis. + for text, body in ((' * **bold** here */', '**bold** here'), + (' **bold** here */', '**bold** here'), + (' *emphasis* here */', '*emphasis* here')): + with self.subTest(line=text): + self.assertEqual([(1, 'Start.', True), (2, body, True)], + prose_lint.extracted_comments(Path('a.cs'), ['/* Start.', text])) + + def test_a_format_with_no_comment_syntax_is_skipped(self) -> None: + for name in ('a.lock', 'a.csv', 'a.txt'): + with self.subTest(file=name): + self.assertEqual([], self.flag(name, f'// {self.RUN_ON}\n')) + + def test_a_wrapped_sentence_is_caught_and_adjacency_is_required(self) -> None: + """Two comments with code between them are separate, not one wrapped sentence.""" + self.assertEqual(['comment-wrap'], + self.flag('a.py', '# A sentence that keeps\n# going onto the next line.\n')) + self.assertEqual([], self.flag('a.py', '# A label here\nx = 1\n# Another label\n')) + + def test_machinery_and_abbreviations_are_not_prose(self) -> None: + for text in ('#!/usr/bin/env python3\n', '# ------------\n', '# noqa: S603 - fixed argv\n', + '# Uses e.g. Docker and i.e. Podman here.\n', '# Bump to 3.13 for the runner.\n', + '# See audit.py and validate.py for this.\n'): + with self.subTest(text=text.strip()[:30]): + self.assertEqual([], self.flag('a.py', text)) + + def test_a_sentence_opening_in_lowercase_is_flagged(self) -> None: + """A lowercase opening reads as the continuation of the line above it.""" + self.assertEqual(['comment-case'], self.flag('a.py', '# details are allowed here.\n')) + + def test_a_genuine_continuation_is_not_a_case_error(self) -> None: + """A wrapped sentence is one finding, not two: the lowercase start is expected there.""" + self.assertEqual(['comment-wrap'], + self.flag('a.py', '# A sentence that keeps\n# going onto the next line.\n')) + + def test_a_capitalized_opening_and_a_code_token_are_both_accepted(self) -> None: + """A backticked identifier does not open in lowercase, so it needs no restructuring.""" + for text in ('# The details element is allowed.\n', '# `ruff format` runs first.\n'): + with self.subTest(text=text.strip()): + self.assertEqual([], self.flag('a.py', text)) + + def test_a_sentence_ending_in_an_acronym_is_still_two_sentences(self) -> None: + """The initial guard anchored on any capital, and this codebase ends sentences in acronyms.""" + for text in ('# The check runs in CI. Another thing happens.\n', + '# Pinned by SHA. Dependabot still bumps it.\n'): + with self.subTest(text=text.strip()[:40]): + self.assertEqual(['comment-wrap'], self.flag('a.py', text)) + + def test_a_second_sentence_may_open_in_either_case(self) -> None: + """A lowercase opening is still a second sentence on the line.""" + self.assertEqual(['comment-wrap'], + self.flag('a.py', '# One thing happens. another thing happens.\n')) + + def test_an_initial_is_one_name_rather_than_two_sentences(self) -> None: + """`J. Smith` is the case the guard exists for, and it must survive the widening.""" + self.assertEqual([], self.flag('a.py', '# Reviewed by J. Smith today.\n')) + + def test_a_trailing_comment_can_start_a_wrapped_sentence(self) -> None: + """Clearing the predecessor on a trailing comment reported the wrong rule, not merely fewer. + + The pair below is a wrapped sentence, and it was reported as a capitalization error, whose + advice would have been to capitalize the continuation rather than to un-wrap it. + """ + self.assertEqual(['comment-wrap'], + self.flag('a.py', 'x = 1 # a sentence that keeps\n# going onto the next line.\n')) + + def test_a_trailing_annotation_does_not_continue_the_line_above(self) -> None: + """A trailing comment annotates its own line, so it cannot be a continuation.""" + self.assertEqual([], self.flag('a.py', 'x = 1 # a thing that\ny = 2 # continues\n')) + self.assertEqual([], self.flag('a.py', 'x = 1 # count of items\n# Another thing entirely.\n')) + + def test_a_comment_inside_a_fenced_block_is_skipped(self) -> None: + """A fenced example is quoted code, so its comments belong to whatever is being shown.""" + self.assertEqual([], self.flag('a.md', + 'Prose.\n\n```html\n\n```\n')) + self.assertEqual(['comment-wrap'], + self.flag('a.md', 'Prose.\n\n\n')) + + def test_a_block_opener_inside_a_line_comment_is_text(self) -> None: + """Read as a real opener it opens a block, and the code lines below are linted as prose. + + The documentation form is the same case: exempting it from linting must not leave the + ceiling unbounded, or the syntax whose doc marker is a line comment reopens the defect. + """ + for name, text in (('a.cs', '// Match a /* opener here\nvar x = 1; // Two things. Here.\n'), + ('a.cs', '/// See a /* opener here\nvar x = 1; // Two things. Here.\n'), + ('a.ps1', '# Match a <# opener here\n$x = 1 # Two things. Here.\n')): + with self.subTest(file=name, line=text.split('\n')[0]): + self.assertEqual(['comment-wrap'], self.flag(name, text)) + + def test_every_comment_on_a_line_is_read_not_just_the_first(self) -> None: + """A ceiling can only describe the first comment, so a later one was unreachable. + + Each case puts the offending sentence in the second comment, which a scan that stops at + the first reports as clean. + """ + for name, text in ( + ('a.cs', 'var x = 1; /* Note. */ // Two things. Here.\n'), + ('a.cs', '/* Note. */ /* Two things. Here. */\n'), + ('a.cs', '/* Start here.\n Still going. */ // Two things. Here.\n'), + ): + with self.subTest(line=text.split('\n')[0]): + self.assertEqual(['comment-wrap'], self.flag(name, text)) + + 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')) + + def test_css_has_block_comments_only(self) -> None: + """A `//` in CSS is the scheme separator of a URL, not a comment marker.""" + self.assertEqual([], self.flag('a.css', 'a { background: url(http://x/y. Z); }\n')) + self.assertEqual(['comment-wrap'], self.flag('a.css', '/* One thing. Another thing. */\n')) + + def test_a_version_pin_is_machinery_rather_than_prose(self) -> None: + """The action-pinning rule requires a trailing `# vX.Y.Z`, which is a label, not a sentence.""" + self.assertEqual([], self.flag('a.yml', ' uses: x@sha # v7.0.0\n')) + self.assertEqual([], self.flag('a.yml', ' uses: x@sha # v3\n')) + + def test_the_syntax_table_covers_a_plausible_number_of_extensions(self) -> None: + """A table that shrank would make every case above pass by having nothing to dispatch on.""" + self.assertGreaterEqual(len(prose_lint.SYNTAX), 25) + for label in ('.cs', '.cpp', '.py', '.sh', '.yml', '.json', '.jsonc', '.xml', '.ps1', '.ini'): + with self.subTest(ext=label): + self.assertIsNotNone(prose_lint.syntax_for(Path(f'x{label}'))) + + class TestDiscovery(unittest.TestCase): def setUp(self) -> None: self.tmp = Path(self.enterContext(tempfile.TemporaryDirectory())) @@ -196,7 +591,7 @@ def test_a_binary_file_is_not_scanned(self) -> None: class TestCli(unittest.TestCase): def setUp(self) -> None: self.tmp = Path(self.enterContext(tempfile.TemporaryDirectory())) - # main() reports findings on stdout/stderr, which would read as real findings in a CI log. + # The main() call prints findings, which would read as real ones in a CI log. self.enterContext(contextlib.redirect_stdout(io.StringIO())) self.enterContext(contextlib.redirect_stderr(io.StringIO()))