From 3d5acce4e76676adc773e9f626e7d92f36d18c0e Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Fri, 31 Jul 2026 11:02:38 -0700 Subject: [PATCH] Sweep the semicolon prose class out of the carried docs The semicolon sweep only ever cleared GOVERNANCE.md. The other six carried files still ran the class, and they vendor downstream on the same no-trigger argument the dash sweep ran on. Two parts, because sweeping the corpus first meant restructuring lists the exemption exists to protect. The rule keeps a semicolon separating items in a list that already carries commas, but the check read the comma positionally, so an enumeration whose commas fall in a later item had its openers flagged and its tail exempted, splitting one series in two. The comma now qualifies the list as a whole. A table row is judged one cell at a time, since a row is a record of fields and a comma in one column cannot excuse a semicolon in another, and a bullet's `**Label**:` is dropped before the line is read, because it opens the bullet rather than announcing a list, the same construct the label dash is already exempted for. That leaves 44 genuine clause joins, now recast as a comma or two sentences: CODESTYLE.md 24, .github/copilot-instructions.md 13, AUDIT.md 10, WORKFLOW.md 12, repo-config/README.md 1. The 5A guarantees and the 5B trace table keep their semicolons, which is the standard series punctuation the rule already allows. All six files now report zero, the bar GOVERNANCE.md already meets. The paren-internal asides those files still carry are the same construction GOVERNANCE.md kept through its own sweep, so they stay. Co-Authored-By: Claude Opus 5 (1M context) --- .github/copilot-instructions.md | 26 +++++++++--------- AUDIT.md | 14 +++++----- CODESTYLE.md | 48 ++++++++++++++++----------------- WORKFLOW.md | 20 +++++++------- repo-config/README.md | 2 +- scripts/README.md | 2 ++ scripts/prose_lint.py | 29 +++++++++++++++++--- scripts/test_prose_lint.py | 16 +++++++++++ 8 files changed, 98 insertions(+), 59 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index f3222e1..f7bfe16 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -12,9 +12,9 @@ Do not duplicate language-specific rules here. **Project-specific conventions an Summarized for VS Code's generators. The full rules, rationale, and examples are in [GOVERNANCE.md "Pull Request Title and Commit Message Conventions"](../GOVERNANCE.md#pull-request-title-and-commit-message-conventions). -- Imperative subject, <= 72 characters, no trailing period; optional blank-line-separated body for the non-obvious *why*. -- US English, title case with lowercase short bind words; no vague titles, no `Co-Authored-By:` unless asked, no release-bump magnitude (NBGV handles versioning). Dependabot's `Bump X from Y to Z` titles are fine. -- develop PRs squash-merge (`gh pr merge --squash`), main PRs merge-commit (`--merge`); a mismatched flag is rejected by branch protection. +- Imperative subject, <= 72 characters, no trailing period, with an optional blank-line-separated body for the non-obvious *why*. +- US English, title case with lowercase short bind words. No vague titles, no `Co-Authored-By:` unless asked, no release-bump magnitude (NBGV handles versioning). Dependabot's `Bump X from Y to Z` titles are fine. +- develop PRs squash-merge (`gh pr merge --squash`), main PRs merge-commit (`--merge`). A mismatched flag is rejected by branch protection. ## Reviewing Carried Fleet Content @@ -50,9 +50,9 @@ gh api repos///pulls//reviews --jq \ "[.[] | select(.commit_id==\"$PR_HEAD\") | select(.body | test(\"Suppressed comments|low confidence\"))] | length" ``` -**Round 1 is normally auto-seeded, so 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." +**Round 1 is normally auto-seeded, so 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`, with **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. +> **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`, with **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, so match the API, not a single spelling, when adapting them. ```sh # 1. PR node id + the Copilot reviewer's bot node id (read from any existing @@ -78,7 +78,7 @@ mutation($pr: ID!, $bot: ID!) { }' -F pr="$PR_NODE" -F bot="$BOT_ID" ``` -The bot node id is read from an existing Copilot **formal** review (`pullRequest.reviews`), so step 1 needs at least one prior formal review on the PR, and the auto-review-on-open normally supplies the first one (it may have **no inline comments**; that still counts, and its bot node id is still readable). Poll for it (give auto-review-on-open a few minutes) before deciding it is missing. +The bot node id is read from an existing Copilot **formal** review (`pullRequest.reviews`), so step 1 needs at least one prior formal review on the PR, and the auto-review-on-open normally supplies the first one (it may have **no inline comments**, which still counts, and its bot node id is still readable). Poll for it (give auto-review-on-open a few minutes) before deciding it is missing. **Cold start (round 1 not yet landed): read the id repo-wide, not from this PR.** The Copilot reviewer's bot node id is the reviewer bot *account's* node id and is **stable across every PR in the repo**. So a freshly opened PR that has neither a formal review nor an issue comment yet does **not** need UI seeding to bootstrap the id: read it from any prior Copilot review anywhere in the repo, then feed it into the `requestReviews` mutation to drive round 1. Query the **most recent** PRs (`first: 20` with an explicit newest-first order; plain `last: 20` returns the *oldest* PRs, which may predate Copilot on the repo), and **guard for an empty result**, since an empty `$BOT_ID` means none of the sampled PRs carry a Copilot review. Widen the window (raise the count or paginate) before concluding the repo has never had one and falling back to UI seeding; never feed an empty id into the mutation: @@ -99,7 +99,7 @@ if [ -z "$BOT_ID" ]; then fi ``` -If Copilot posted **only an issue comment** on this PR and no formal review, you can instead read the id from that comment's author (`pullRequest.comments` -> author `... on Bot { id }`). Manual UI seeding is the last resort, needed only for a repo that has **never** had a Copilot review, so no prior id exists anywhere to read; then use the mutation for every subsequent re-request. +If Copilot posted **only an issue comment** on this PR and no formal review, you can instead read the id from that comment's author (`pullRequest.comments` -> author `... on Bot { id }`). Manual UI seeding is the last resort, needed only for a repo that has **never** had a Copilot review, so no prior id exists anywhere to read. Use the mutation for every subsequent re-request. **Do NOT post `@Copilot review` as a PR comment.** That comment triggers the Copilot *coding agent* (`copilot-swe-agent[bot]`), which makes code changes rather than posting a review. @@ -137,14 +137,14 @@ Coverage is confirmed when (1) exits 0, and **a formal review with no inline com ### Bounded Retry Workflow -This path is only for a **genuinely missing** review, meaning no Copilot review (formal *or* issue comment) covers the current head SHA after polling. A review that covered the head but produced no comments is a clean pass, not a missing review; do not enter this retry path for it. +This path is only for a **genuinely missing** review, meaning no Copilot review (formal *or* issue comment) covers the current head SHA after polling. A review that covered the head but produced no comments is a clean pass, not a missing review, so do not enter this retry path for it. **A slow review is pending, not missing, so poll with backoff and never escalate on a timeout alone.** Copilot can lag far beyond the usual one-to-three minutes when it has been re-requested many times in quick succession, because it throttles under load, and a re-review landing tens of minutes after the request is normal. A poll that times out is therefore evidence only that the review has not landed *yet*, not that Copilot is done or unresponsive. Report the status as "review still pending" and keep polling on a widening interval (for example 20s steps, then a few minutes) rather than stopping. Enter the escalation step below only when the `requestReviews` mutation itself no-ops or errors, or after a genuinely long wait with the request confirmed accepted, never merely because one fixed poll window elapsed. If a review did not run on the current head, retry: 1. Wait briefly and check head-SHA coverage (see above). -1. Re-request the review via the `requestReviews` mutation (see "Triggering and Polling"); fall back to the GitHub PR UI only if the mutation no-ops. +1. Re-request the review via the `requestReviews` mutation (see "Triggering and Polling"), falling back to the GitHub PR UI only if the mutation no-ops. 1. Retry up to two more times (three total). 1. If still missing, mark review as blocked and escalate to the user/maintainer with what was attempted. @@ -152,7 +152,7 @@ If a review did not run on the current head, retry: Every id below is captured from a live query into a variable and passed from there, never hand-typed, guessed, or pasted as a `PRRT_...` literal. A node id resolves globally, so a fabricated or stale id does not fail, it writes to a real thread on an unrelated repository. This runbook implements [GOVERNANCE.md "Repository Boundaries and Write Safety"](../GOVERNANCE.md#repository-boundaries-and-write-safety): write only to this repo, capture every id from a live query, and never suppress a mutation's output. -List unresolved threads. Use `first: 100` with cursor-based pagination; if `hasNextPage` is true, re-run with `after: ""` to retrieve the next page: +List unresolved threads. Use `first: 100` with cursor-based pagination, and where `hasNextPage` is true, re-run with `after: ""` to retrieve the next page: ```sh gh api graphql -f query=' @@ -209,14 +209,14 @@ mutation($threadId: ID!) { }' -F threadId="$TID" ``` -Issue-level Copilot comments (those in `issues//comments`) have no resolution action, since GitHub provides no API or UI to resolve them. Reply if the finding warrants it; no resolution step is needed or possible. +Issue-level Copilot comments (those in `issues//comments`) have no resolution action, since GitHub provides no API or UI to resolve them. Reply if the finding warrants it, but no resolution step is needed or possible. ### PR Edits and Merge-State Gotchas - **`gh pr edit --title/--body` is broken here.** It touches the deprecated Projects-classic `projectCards` GraphQL field and **exits non-zero without applying the change** (a stale PR description then survives review rounds). Edit the title/body via the API and verify it took: GraphQL `updatePullRequest(input: { pullRequestId, title, body })`, or REST `gh api -X PATCH repos///pulls/ -F body=@body.md` (the `@` reads the body from a file, so name it explicitly, not the literal `file`). - **`main`/`develop` use rulesets, not classic branch protection.** The classic protection REST endpoint (`repos/.../branches//protection`) 404s, so read the ruleset instead. A `mergeStateStatus` of `BLOCKED` on a green PR is usually just **unresolved review threads** (the ruleset requires thread resolution); resolving them moves it to `CLEAN`. (`BLOCKED` is a `mergeStateStatus` value; don't confuse it with the separate `mergeable` field's `MERGEABLE`/`CONFLICTING`, which reports merge conflicts, not review gates.) -- **Push -> head-SHA read race.** A `headRefOid` read taken immediately after a push can return the **old** head; re-read after the push registers, or a coverage poll evaluates the stale SHA. -- **Copilot is sometimes factually wrong** (e.g. it claimed `actionlint -color` "requires a value" when it is a boolean flag). Verify a finding before fixing; decline with evidence when it is wrong, which is distinct from dismissing a still-present finding as stale. +- **Push -> head-SHA read race.** A `headRefOid` read taken immediately after a push can return the **old** head, so re-read after the push registers, or a coverage poll evaluates the stale SHA. +- **Copilot is sometimes factually wrong** (e.g. it claimed `actionlint -color` "requires a value" when it is a boolean flag). Verify a finding before fixing, and decline with evidence when it is wrong, which is distinct from dismissing a still-present finding as stale. Reply-body conventions: diff --git a/AUDIT.md b/AUDIT.md index 52bc181..bac5e33 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -9,7 +9,7 @@ The verdict vocabulary is [`WORKFLOW.md`][workflow]'s: **operational / not opera This audit is not occasional. Run it whenever you **create, adopt, or materially change** a fleet repo, and on demand for any known repo: -- **Onboarding a repo is complete only when it either passes this audit** (operational on every applicable check) **or carries a committed `reports//audit.md` plus a tracking issue** enumerating every residual delta. A repo that is partially set up but never audited is itself a **defect**, the exact state this process prevents. The create-to-conformance counterpart is [`STANDUP.md`][standup]; because both read the same manifests, a repo stood up by that file passes this audit by construction. +- **Onboarding a repo is complete only when it either passes this audit** (operational on every applicable check) **or carries a committed `reports//audit.md` plus a tracking issue** enumerating every residual delta. A repo that is partially set up but never audited is itself a **defect**, the exact state this process prevents. The create-to-conformance counterpart is [`STANDUP.md`][standup]. Because both read the same manifests, a repo stood up by that file passes this audit by construction. - **Touching a repo** (any conformance-affecting change) ends by re-running the applicable checks and **reconciling the registry entry to reality**: `status`, `types`, `releaseTrigger`, `workflowModel`, `driftNotes`. The registry records reality, not intent. [`spec/validate.py`][validate] proves the catalog is self-consistent, not that it matches the live repo. Closing that gap is this audit's job. The deterministic subset (settings, rulesets, secret names, file presence, per-scope markdown section presence, workflow interface conformance, verbatim content, branch facts) is mechanized in [`spec/audit.py`][audit-runner]: owner-initiated, run on demand when onboarding a repo, on suspected drift, or before fleet-wide changes. A required section missing from a carried markdown file is a **drift finding**, not a letter, because a heading rename reads as missing and equivalence is judged by hand. A carried `interface` workflow (spec/fidelity-model.md) is checked by name and wiring (required jobs, the ruleset-bound check name, the artifact-name handoff, and the forbidden `artifact-ids:` fork), all at **drift**, since the body is owned and a rename is a hint to verify. A carried `verbatim` unit, whether a whole file (`.markdownlint-cli2.jsonc`) or a canonical workflow job region (the `github-release` job), is content-hashed against the hub's canonical after line-ending normalization. A mismatch is classified **stale** (matches a past hub revision, re-vendor) or **modified** (matches none, the repo changed fixed content), both at **drift**, since equivalence is intent-governed and a byte diff is a hint to review. ## 1. Scope and Ground-Truth Branch @@ -22,10 +22,10 @@ This holds for **both workflow models**. An `operational` repo commits directly Look up the repo in [`registry/repos.json`][repos] and read its `types[]`. If the entry is `classificationPending` (a backlog repo), classify it from the tree and propose a registry update: -- `*.csproj` / `*.slnx` -> `csharp`; a `dotnet nuget push` workflow -> `nuget`; a `System.CommandLine` console -> `console`. -- `pyproject.toml` / `setup.py` -> `python`; a `pypa/gh-action-pypi-publish` workflow -> `pypi`. -- `Dockerfile` + a docker build/push workflow -> `docker`; an `upstream-version.json` tracker -> `upstream-wrapper`. -- `custom_components/*/manifest.json` + `hacs.json` -> `homeassistant`; a codegen workflow -> `codegen`; no `build-*` task -> `source-only`; governance-only -> `docs`. +- `*.csproj` / `*.slnx` -> `csharp`, a `dotnet nuget push` workflow -> `nuget`, a `System.CommandLine` console -> `console`. +- `pyproject.toml` / `setup.py` -> `python`, a `pypa/gh-action-pypi-publish` workflow -> `pypi`. +- `Dockerfile` + a docker build/push workflow -> `docker`, an `upstream-version.json` tracker -> `upstream-wrapper`. +- `custom_components/*/manifest.json` + `hacs.json` -> `homeassistant`, a codegen workflow -> `codegen`, no `build-*` task -> `source-only`, governance-only -> `docs`. ## 3. Applicability Gate @@ -43,8 +43,8 @@ For each applicable type in [`spec/project-types.json`][project-types] and every A check with `intentRef`/`workflowRef` points at the prose section that owns the rationale, so read it to judge intent. The dimensions: - **csharp** - `.editorconfig` carries the shared `[*.cs]` rule block (letter), and analyzer severities are enforced, not relaxed (intent). -- **nuget** - publish uses OIDC Trusted Publishing, no `NUGET_API_KEY` (letter+intent); `--skip-duplicate`. -- **pypi** - OIDC publish job with `environment: pypi`, `id-token: write`, `skip-existing: true`; no stored token. +- **nuget** - publish uses OIDC Trusted Publishing with no `NUGET_API_KEY` (letter+intent), and the push carries `--skip-duplicate`. +- **pypi** - OIDC publish job carrying `environment: pypi`, `id-token: write` and `skip-existing: true`, with no stored token. - **python** - ruff and pyright present (intent), canonical in `pyproject.toml` (letter), and a standalone `.ruff.toml` / `pyrightconfig.json` is a drift finding. - **console** - smoke runtime matrix is a strict subset, and per-runtime outputs aggregate to one `release-asset-*`, gated `!smoke`. - **docker** - registry layer cache (`buildcache-`, never `type=gha`), the size-limited Docker Hub README is published via the docker-readme task, and the image always re-pushes on publish. diff --git a/CODESTYLE.md b/CODESTYLE.md index 6a37ebd..4a6420b 100644 --- a/CODESTYLE.md +++ b/CODESTYLE.md @@ -16,9 +16,9 @@ Use each tool's official casing in task labels, docs, and prose: `.NET` (not `.N Each language defines a **clean-compile** verification: the combination of build, formatter, linter, and code-analysis tools that must report clean before a commit. It is exposed as one or more **named** VS Code tasks (or, where a language ships no tasks, documented commands), and those definitions are the same across the fleet. The concrete names live in each language section below. -- **Run it after every code change.** The relevant language's clean-compile must pass before you commit; CI runs the same checks as a backstop. -- **The named task definition is the canonical spec** - its exact command sequence, arguments, and strictness. You may run it through the VS Code task **or** by invoking the equivalent native commands directly; either is fine **only if the sequence, arguments, and strictness match exactly**. No shortcuts and no more-lenient options (for example, never drop `--verify-no-changes` or loosen a `--severity`). -- **A local commit/pre-commit gate is the repo's choice.** No single hook runner fits every language (a `dotnet`-tool runner like Husky.Net suits .NET but not Python), so none is mandated, but that is **not** a recommendation against commit gates. CI is the authoritative backstop regardless; a local gate is an additive convenience a repo may wire and keep: Husky.Net (and `dotnet husky run` as a style step) for .NET, `pre-commit` for Python. Keeping a working gate is not drift. +- **Run it after every code change.** The relevant language's clean-compile must pass before you commit, and CI runs the same checks as a backstop. +- **The named task definition is the canonical spec** - its exact command sequence, arguments, and strictness. You may run it through the VS Code task **or** by invoking the equivalent native commands directly, and either is fine **only if the sequence, arguments, and strictness match exactly**. No shortcuts and no more-lenient options (for example, never drop `--verify-no-changes` or loosen a `--severity`). +- **A local commit/pre-commit gate is the repo's choice.** No single hook runner fits every language (a `dotnet`-tool runner like Husky.Net suits .NET but not Python), so none is mandated, but that is **not** a recommendation against commit gates. CI is the authoritative backstop regardless, and a local gate is an additive convenience a repo may wire and keep: Husky.Net (and `dotnet husky run` as a style step) for .NET, `pre-commit` for Python. Keeping a working gate is not drift. ### Analyzer Diagnostics and Suppressions @@ -34,7 +34,7 @@ Each language defines a **clean-compile** verification: the combination of build These apply repo-wide, in every directory: 1. **Markdown linting**: All `.md` files must be lint-clean (error and warning free) via the VS Code `markdownlint` extension. [`.markdownlint-cli2.jsonc`][markdownlint-cli2] at the repo root is the single source of truth, and the davidanson `markdownlint` extension and a command-line `markdownlint-cli2` run both read it, so the IDE and CLI stay in lock-step. Rules it deliberately disables (e.g. `MD013` line-length) are **intentional**, so do not "fix" them. `MD033` inline HTML stays **enabled**: HTML comments are permitted (markdownlint does not flag them), HTML elements are flagged, and anything with a native markdown equivalent uses the markdown. Fix violations at the source rather than disabling rules. -2. **Spelling**: All spelling must be clean via the CSpell VS Code integration; words must be correctly spelled in **US English** (the repo-wide convention, per [GOVERNANCE.md][governance]). The shared `cspell.json` sets `"language": "en-US"` so British spellings are flagged, where a bare `"en"` accepts both US and British and silently passes the wrong spelling. Project-specific terms go in the shared `cspell.json` `words` list, the single source of truth the extension, CLI, and CI all read. The `.code-workspace` must **not** carry its own `cspell.words`/`cSpell.words` block; when externalizing words into `cspell.json`, delete any word list left in the workspace (a leftover one duplicates the list and silently drifts). +2. **Spelling**: All spelling must be clean via the CSpell VS Code integration, and words must be correctly spelled in **US English** (the repo-wide convention, per [GOVERNANCE.md][governance]). The shared `cspell.json` sets `"language": "en-US"` so British spellings are flagged, where a bare `"en"` accepts both US and British and silently passes the wrong spelling. Project-specific terms go in the shared `cspell.json` `words` list, the single source of truth the extension, CLI, and CI all read. The `.code-workspace` must **not** carry its own `cspell.words`/`cSpell.words` block, and when externalizing words into `cspell.json`, delete any word list left in the workspace (a leftover one duplicates the list and silently drifts). 3. **Spelling CI scope**: The enforced CI spell-check gate covers **`README.md` and `HISTORY.md` only**, because these are the files every repo visitor sees, so they must be clean. It is deliberately **not** all `**/*.md`: repos carry many markdown files full of technical terms, and gating every one of them would mean endlessly padding `cspell.json` just to keep CI green. Broad, live spell-checking across any file (source, markdown, text) is the **cspell editor extension's** job, so typos still surface to whoever is editing. A repo owner **may** widen their own CI file list, but README + HISTORY are the default; keep the CI workflow, the `Lint: Spelling` VS Code task, and the GOVERNANCE.md cspell one-liner on the same file list. The list is explicit (not a glob), so a repo that ships no `HISTORY.md` (e.g. one with no changelog) must drop it from all three surfaces and gate on `README.md` alone, since cspell errors on a listed file that does not exist. Markdown *linting* (item 1) stays repo-wide `**/*.md`, which does not choke on technical terms. ## .NET @@ -59,7 +59,7 @@ This is the style guide for any **.NET projects** in this repo. 3. **CI lint backstop** - CI runs the clean-compile checks on every PR as the authoritative backstop - - Git hooks are optional; a repo may wire a local runner (Husky.Net) for pre-commit enforcement, but CI is the gate that matters + - Git hooks are optional, and a repo may wire a local runner (Husky.Net) for pre-commit enforcement, but CI is the gate that matters #### Central Build and Package Configuration @@ -72,7 +72,7 @@ A repo whose projects still carry per-project analyzer settings or versioned `Pa #### Build Tasks -Available VS Code tasks (run them from VS Code's task runner, **Terminal -> Run Task**, or an agent's task-running tool). The three clean-compile tasks below are carried verbatim; a repo adds its own convenience tasks (tool updates, dependency upgrades, benchmarks) on top: +Available VS Code tasks (run them from VS Code's task runner, **Terminal -> Run Task**, or an agent's task-running tool). The three clean-compile tasks below are carried verbatim, and a repo adds its own convenience tasks (tool updates, dependency upgrades, benchmarks) on top: - `.NET Build`: Build with diagnostic verbosity *(clean-compile)* - `CSharpier Format`: Auto-format code with CSharpier *(clean-compile)* @@ -253,7 +253,7 @@ Follow the scope hierarchy in [Analyzer Diagnostics and Suppressions][analyzer-d #### Error Handling and Logging -1. **Structured logging**: Use structured message templates. Serilog is the **application's** concrete backend; a library never references it (see item 2) +1. **Structured logging**: Use structured message templates. Serilog is the **application's** concrete backend, and a library never references it (see item 2) ```csharp logger.LogError(exception, "{Function}", function); @@ -279,20 +279,20 @@ Follow the scope hierarchy in [Analyzer Diagnostics and Suppressions][analyzer-d } ``` -5. **Exceptions**: Do not swallow exceptions; log and rethrow or translate to a domain-specific exception +5. **Exceptions**: Do not swallow exceptions, and either log and rethrow or translate to a domain-specific exception #### Code Patterns 1. **Guard clauses**: Prefer early returns for validation and error handling -2. **Async all the way**: Avoid blocking calls (`.Result`, `.Wait()`); use `async`/`await` +2. **Async all the way**: Avoid blocking calls (`.Result`, `.Wait()`) and use `async`/`await` 3. **Cancellation tokens**: Accept `CancellationToken` as the last parameter and pass it through 4. **ConfigureAwait**: In library code, use `ConfigureAwait(false)` unless context is required - Do not call `ConfigureAwait(false)` in xUnit tests (see xUnit1030) -5. **Disposables**: Use `await using` for async disposables; prefer `using` declarations +5. **Disposables**: Use `await using` for async disposables, and prefer `using` declarations 6. **LINQ vs loops**: Use LINQ for clarity, loops for hot paths or allocations -7. **HTTP**: Reuse `HttpClient` via factory; avoid per-request instantiation +7. **HTTP**: Reuse `HttpClient` via factory, never per-request instantiation 8. **Collections**: Prefer `IReadOnlyList`/`IReadOnlyCollection` for public APIs -9. **Immutability**: Prefer immutable records; use init-only setters when records are not suitable; prefer immutable or frozen collections for read-only data +9. **Immutability**: Prefer immutable records, use init-only setters when records are not suitable, and prefer immutable or frozen collections for read-only data 10. **Exceptions as control flow**: Avoid using exceptions for expected flow 11. **Sealing classes**: Seal classes that are not designed for inheritance 12. **Read-only data**: Use immutable or frozen collections for read-only data sets @@ -358,8 +358,8 @@ This is the style guide for any **Python project(s)** in this repo. - **Type checker in CI** - `pyright` strict, **`mypy` in CI with `pyright` editor-only** (Pylance), or both. Whichever runs in CI is the one the clean-compile and the CI gate invoke. - **Dependency declaration** - `[dependency-groups]`, or PEP 621 `[project.optional-dependencies]` (dev tools installed with `uv sync --extra `). - **Versioning / publishing** - a published package (`_version.py` + a version source + `uv build` + a PyPI publish step), or a **source-only** repo with a static `version` and no publish step (see [Versioning][versioning-section]). -- **Disabled markdownlint rules** - repo-specific; `.markdownlint-cli2.jsonc` at the repo root is the source of truth, not any example rule named here. -- **VS Code config home** - editor **settings/extensions** may live in `.vscode/*.json` **or** the `.code-workspace`; **tasks / launch / debug** configs can only be external `.vscode/*.json` (they cannot live in the workspace file). A `[vscode-tasks]` reference must point wherever the repo actually keeps `tasks.json`. +- **Disabled markdownlint rules** - repo-specific. `.markdownlint-cli2.jsonc` at the repo root is the source of truth, not any example rule named here. +- **VS Code config home** - editor **settings/extensions** may live in `.vscode/*.json` **or** the `.code-workspace`, while **tasks / launch / debug** configs can only be external `.vscode/*.json` (they cannot live in the workspace file). A `[vscode-tasks]` reference must point wherever the repo actually keeps `tasks.json`. **Two profiles.** A repo's Python is one of two shapes, declared as the `build` or `lint-only` profile and validated against the `pyproject.toml` shape. The rest of this section (uv project, `uv.lock`, `uv run`, `src` layout, pytest coverage) describes the **Project** shape (the `build` profile). The two differ by whether the Python has **third-party runtime dependencies**, which shows up structurally in `pyproject.toml`, so the audit reads the shape there (`python.profile.detect`): @@ -373,11 +373,11 @@ This is the style guide for any **Python project(s)** in this repo. | [uv][uv-link] | env, deps, build, publish (build/publish only where the repo ships a package) | `pyproject.toml` `[dependency-groups]` or `[project.optional-dependencies]`, `uv.lock` | | [hatchling][latest-link] | build backend (published packages) | `pyproject.toml` `[build-system]` | | [ruff][ruff-link] | lint + format + import sort | `pyproject.toml` `[tool.ruff]` | -| [pyright][pyright-link] | type checker (default; strict baseline) | `pyproject.toml` `[tool.pyright]` | -| [mypy][mypy-link] | additional/alternate type checker (optional; the CI checker in a mypy-in-CI repo; required for Home Assistant) | `pyproject.toml` `[tool.mypy]` (or per home-assistant/core) | +| [pyright][pyright-link] | type checker (the default, a strict baseline) | `pyproject.toml` `[tool.pyright]` | +| [mypy][mypy-link] | additional/alternate type checker (optional, the CI checker in a mypy-in-CI repo, required for Home Assistant) | `pyproject.toml` `[tool.mypy]` (or per home-assistant/core) | | [pytest][docs-link] | test runner | `pyproject.toml` `[tool.pytest.ini_options]` | -**Type checking targets strongly typed, deterministic code.** `pyright` in **strict** mode is the default baseline on first-party code (a repo may instead run `mypy` in CI and keep `pyright` editor-only via Pylance, per the next paragraph) (`[tool.pyright]` `strict = ["src"]`, or the integration package for a Home Assistant repo; tests run standard mode). pyright is the anchor because **Pylance embeds it**, so the editor and the CLI/CI (`uv run pyright`) run the *same* engine and never disagree; the standalone `ms-pyright.pyright` extension stays in `unwantedRecommendations` because Pylance covers it. Relax strictness on **third-party** code only when a dependency has no usable types and no alternative (e.g. `pandas`): a targeted, commented `# pyright: ignore[...]` or a scoped `[tool.pyright]` override, never a blanket relaxation. +**Type checking targets strongly typed, deterministic code.** `pyright` in **strict** mode is the default baseline on first-party code (a repo may instead run `mypy` in CI and keep `pyright` editor-only via Pylance, per the next paragraph) (`[tool.pyright]` `strict = ["src"]`, or the integration package for a Home Assistant repo, with tests run in standard mode). pyright is the anchor because **Pylance embeds it**, so the editor and the CLI/CI (`uv run pyright`) run the *same* engine and never disagree. The standalone `ms-pyright.pyright` extension stays in `unwantedRecommendations` because Pylance covers it. Relax strictness on **third-party** code only when a dependency has no usable types and no alternative (e.g. `pandas`): a targeted, commented `# pyright: ignore[...]` or a scoped `[tool.pyright]` override, never a blanket relaxation. **`mypy` is allowed, and required where the ecosystem demands it. It is not banned.** Running more than one checker is normal when each serves a purpose (the .NET side pairs `CSharpier` and `dotnet format` the same way), and pyright's inference and mypy's plugin ecosystem (e.g. `pydantic.mypy`) catch different classes of error. A **Home Assistant** integration runs `mypy --strict` because the platinum `strict-typing` quality-scale tier requires it; a pydantic-heavy library may opt in for the plugin. When a repo uses mypy it runs in **CI and the editor** (the `ms-python.mypy-type-checker` extension) so the two stay consistent, and its mypy command joins the clean-compile; a repo with no such need stays pyright-only, which is lighter and inherently consistent. @@ -421,14 +421,14 @@ The Python clean-compile (see [Clean-Compile Verification][clean-compile-verific #### Formatting and Linting -- **`ruff format` is authoritative.** Don't argue with the formatter; if it reformats your code, that's the final form. Configure (line length, target version) in `pyproject.toml` `[tool.ruff]`, not via inline `# fmt:` directives. -- **Run `ruff check --fix` before committing.** Most ruff lint rules have safe autofixes; let the tool handle them. The configured rule families are listed under `[tool.ruff.lint]` `select`. Add new rule families project-wide rather than scattering inline `# noqa` markers. +- **`ruff format` is authoritative.** Don't argue with the formatter, and if it reformats your code, that's the final form. Configure (line length, target version) in `pyproject.toml` `[tool.ruff]`, not via inline `# fmt:` directives. +- **Run `ruff check --fix` before committing.** Most ruff lint rules have safe autofixes, so let the tool handle them. The configured rule families are listed under `[tool.ruff.lint]` `select`. Add new rule families project-wide rather than scattering inline `# noqa` markers. - **`# noqa` is a last resort.** When you must use one, scope it narrowly (`# noqa: E501`, not bare `# noqa`) and add a short comment on the same line explaining why. False-positive patterns that recur across the codebase belong in `[tool.ruff.lint]` `ignore` or per-file `[tool.ruff.lint.per-file-ignores]`, with a comment. Porting an existing codebase is not a license to add `ignore` / `per-file-ignores` blocks to mute newly surfaced lint. Fix it (see [Analyzer Diagnostics and Suppressions][analyzer-diagnostics-and-suppressions]). #### Comments - **Inline `#` comments**: keep tight and local. One line is preferred, but multi-line is fine when you need to document a non-obvious implementation constraint, a local trade-off, or coupling that future edits could easily break. Keep that rationale next to the affected block so the reviewer/maintainer sees it at edit-time. -- **Don't explain *what* the code does.** Well-named identifiers handle that. Don't reference the current task ("added for X", "used by Y"); that belongs in the PR description. +- **Don't explain *what* the code does.** Well-named identifiers handle that. Don't reference the current task ("added for X", "used by Y"), which belongs in the PR description. #### Docstrings @@ -439,7 +439,7 @@ The Python clean-compile (see [Clean-Compile Verification][clean-compile-verific #### Type Hints -- **All public APIs are typed.** The repo's configured type checker runs on `src/` (pyright strict via `[tool.pyright]` `strict = ["src"]`, or `mypy` where that is the CI checker); tests run in the checker's looser/standard mode. +- **All public APIs are typed.** The repo's configured type checker runs on `src/` (pyright strict via `[tool.pyright]` `strict = ["src"]`, or `mypy` where that is the CI checker), and tests run in the checker's looser/standard mode. - **Use modern syntax**: `list[int]` not `List[int]`, `dict[str, X]` not `Dict[str, X]`, `X | None` not `Optional[X]`, `from __future__ import annotations` only when needed for forward references. - **Don't add `# type: ignore` to silence pyright errors without a comment** explaining the constraint. If a recurring false positive needs suppression, configure it project-wide in `[tool.pyright]`. A new port doesn't change this, so fix freshly surfaced type errors rather than muting them (see [Analyzer Diagnostics and Suppressions][analyzer-diagnostics-and-suppressions]). @@ -448,7 +448,7 @@ The Python clean-compile (see [Clean-Compile Verification][clean-compile-verific - `snake_case` for functions, methods, variables, modules, package directories. - `PascalCase` for classes, type aliases, type vars, enum members. - `UPPER_SNAKE_CASE` for module-level constants. -- Single leading underscore for module-private; double leading underscore for name-mangled (rare, and usually means rethink the design). +- Single leading underscore for module-private, double leading underscore for name-mangled (rare, and usually means rethink the design). #### Imports @@ -459,7 +459,7 @@ The Python clean-compile (see [Clean-Compile Verification][clean-compile-verific #### Patterns to Avoid - **Don't add backward-compat shims, `# removed` markers, or rename-to-`_` for unused vars** - just delete. Git history is the audit trail. -- **Don't add error handling for impossible cases.** Trust internal code; only validate at boundaries (user input, parsed config, external APIs). +- **Don't add error handling for impossible cases.** Trust internal code, and validate only at boundaries (user input, parsed config, external APIs). - **Don't use exceptions for expected control flow.** Exceptions are for *unexpected* states. - **Don't suppress errors silently** (`except Exception: pass`). Either handle the specific exception and document why it's safe, or let it propagate. @@ -476,7 +476,7 @@ The Python clean-compile (see [Clean-Compile Verification][clean-compile-verific **Published packages.** `_version.py` ships with `__version__ = "0.0.0"` as a placeholder. Until you wire `_version.py` to something that increments (the usual options are `hatch-vcs`, a version.json bridge, or manual bumps), no new PyPI versions will land, and publishing with `skip-existing: true` keeps a stuck placeholder version from failing the run. -**Source-only repos** (no PyPI publish; source-release on dispatch, or no release at all) do not need `_version.py`: keep a static `version` in `pyproject.toml` `[project]`, or let the release pipeline's version source (e.g. NBGV + `version.json`) own the tag. There is no publish step to guard, so `skip-existing` does not apply. +**Source-only repos** (no PyPI publish, with a source-release on dispatch or no release at all) do not need `_version.py`: keep a static `version` in `pyproject.toml` `[project]`, or let the release pipeline's version source (e.g. NBGV + `version.json`) own the tag. There is no publish step to guard, so `skip-existing` does not apply. ### Linter Cleanliness diff --git a/WORKFLOW.md b/WORKFLOW.md index c0b0e26..10f2283 100644 --- a/WORKFLOW.md +++ b/WORKFLOW.md @@ -53,7 +53,7 @@ flowchart LR main -.->|no back-merge| develop ``` -`operational` repos (live-service config; `workflowModel: operational`) commit directly to `develop` and promote a known-good snapshot to `main` via an occasional PR: +`operational` repos (live-service config, `workflowModel: operational`) commit directly to `develop` and promote a known-good snapshot to `main` via an occasional PR: ```mermaid flowchart LR @@ -164,7 +164,7 @@ The required behaviors, organized by domain. Each is a **MUST**, stated as input ### D4 - Release / Publish -- **D4.1 Gated single-branch publish.** Output: PRs smoke-test and publish nothing. A **human merge never auto-publishes**. A first `plan` job (`publish-plan-task.yml`) decides once and every job gates on it: publish on a **code-affecting bot push to `main`** (gated to the codegen App / Dependabot `github.actor`; an Actions-only bump matches no release path and publishes nothing), a **dispatch** of `main`/`develop`, or a **main-only weekly schedule** (Docker). A source-only repo publishes on dispatch only. Each run builds one branch. +- **D4.1 Gated single-branch publish.** Output: PRs smoke-test and publish nothing. A **human merge never auto-publishes**. A first `plan` job (`publish-plan-task.yml`) decides once and every job gates on it: publish on a **code-affecting bot push to `main`** (gated to the codegen App / Dependabot `github.actor`, with an Actions-only bump matching no release path and publishing nothing), a **dispatch** of `main`/`develop`, or a **main-only weekly schedule** (Docker). A source-only repo publishes on dispatch only. Each run builds one branch. - **D4.2 Tag the built commit.** Output: the release `target_commitish` is the built commit's SHA (NBGV's `GitCommitId`), never a branch name or a separately re-resolved ref. *Prevents: the tag landing on the default branch instead of the built tree.* - **D4.3 Release contents.** Output: every release is a tag on the built commit plus the auto source zip, README, and LICENSE; file-producing targets attach `release-asset-*`; `prerelease` equals `branch != default`. A no-file-target repo that uses the release task (Docker-only, PyPI-only) reaches the tag-only shape **only** with `expect_release_assets: false` set by the caller (which relaxes `fail_on_unmatched_files` and skips the asset download). With the default `true` and no assets the release-create step fails. A source-only repo reaches the same shape through its inlined `action-gh-release` instead, with no release task or `expect_release_assets`. - **D4.4 No-op republish.** Input: a re-run whose version is unchanged. Output: nothing is re-pushed, because the release-create step is skipped when the tag exists (refreshed only on `workflow_dispatch`), and the paired asset-delete is skipped with it. Registry pushes are no-ops. The NuGet/PyPI publish steps are **not** statically gated on existence. They run and the **server** dedupes (`dotnet nuget push --skip-duplicate` turns a 409 into success; PyPI `skip-existing: true`). **Docker always re-pushes** the image (base-image refresh), independently of the release-create skip, within the same run. *Prevents: duplicate releases and wasted pushes.* @@ -240,23 +240,23 @@ For each *applicable* scenario, evaluate every job's `if:`/`needs:` against the | # | Input | Expected output | Exercises | | --- | --- | --- | --- | | S1 | PR touching a build target | `changes` flags it; validation runs; that target's smoke build runs; no push, **no uploads**; validate-release **skipped (smoke), succeeds**; release **skipped**; aggregator **success**; version = prerelease; no release; no dangling artifacts | D1, D2.2, D3 | -| S2 | PR changing only docs | smoke-build **skipped**; validation runs; aggregator **success** | D1.1, D1.5 | -| S3 | PR changing only `.github/workflows/**` | filter excludes -> smoke-build **skipped**; aggregator **success** | D1.4 | -| S4 | PR base = default branch, carrying a build target | smoke versions as prerelease; validate-release **skipped (smoke)** so the default-branch arm does **not** fire; aggregator **success**; promotion not blocked | D1.3, D2.2 | -| S5 | bot push to `main` not touching a release path (e.g. an Actions bump) | the paths filter excludes it; nothing publishes | D4.1 | -| S6 | code-affecting **bot** push to `main` (a human push/promotion, or any develop push, does not) | the `plan` job gates it to the App/Dependabot actor; `main` publishes a release | D3, D4 | +| S2 | PR changing only docs | smoke-build **skipped**, validation runs, aggregator **success** | D1.1, D1.5 | +| S3 | PR changing only `.github/workflows/**` | filter excludes -> smoke-build **skipped**, aggregator **success** | D1.4 | +| S4 | PR base = default branch, carrying a build target | smoke versions as prerelease, validate-release **skipped (smoke)** so the default-branch arm does **not** fire, aggregator **success**, promotion not blocked | D1.3, D2.2 | +| S5 | bot push to `main` not touching a release path (e.g. an Actions bump) | the paths filter excludes it, so nothing publishes | D4.1 | +| S6 | code-affecting **bot** push to `main` (a human push/promotion, or any develop push, does not) | the `plan` job gates it to the App/Dependabot actor, and `main` publishes a release | D3, D4 | | S7 | publish run (schedule, a bot push to main, or a dispatch) | builds the **one** trigger branch: `main` -> `X.Y.Z`, `prerelease=false`, registry stable, badge/readme run; `develop` -> `X.Y.Z-g`, `prerelease=true`, registry prerelease; `release-asset-*` consumed-then-deleted; PyPI build-artifact deleted after its publish; **no dangling artifacts** | D3, D4, D5, D6, D7 | | S8 | dispatch from a ref other than `main` or `develop` | **fails fast** | D2.3 | | S9 | re-run publish, version unchanged | release-create **skipped**, `release-asset-*` delete **skipped**; NuGet/PyPI pushes no-op (server dedupe); **PyPI build-artifact still deleted** (its publish ran); **Docker still re-pushes** the image; no duplicate release | D4.4, D5.2 | -| S10 | branch/version classification disagree | validate-release **fails loud**; build/publish skip | D2.2 | -| S11 | scheduled upstream-version bump (wrapper) | resolver detects a change -> commits the state file -> opens a `-` PR -> merge-bot auto-merges -> the `main` pin publishes via the gate (a develop pin does not auto-publish; it ships via a develop dispatch or promotion) | D8.3, D3.5 | +| S10 | branch/version classification disagree | validate-release **fails loud**, build/publish skip | D2.2 | +| S11 | scheduled upstream-version bump (wrapper) | resolver detects a change -> commits the state file -> opens a `-` PR -> merge-bot auto-merges -> the `main` pin publishes via the gate (a develop pin does not auto-publish, shipping instead via a develop dispatch or promotion) | D8.3, D3.5 | ### 5C. Live Probe (Where Warranted) - Open a trivial-change PR touching one target and confirm S1. - Drive a `smoke: true` push-probe of the build task for **both** the default and a non-default branch and assert the version classification (clean vs prerelease) and that the gate passes, **without publishing**. *Caveat: the Docker leg logs in to the registry even on smoke and reads the buildcache, so it needs `DOCKER_HUB_*` secrets and cannot run on a fork PR (same-repo only).* - Per registry: after a real publish, query NuGet.org for the expected version + prerelease classification (and the `.snupkg` on the symbol server), and confirm a re-run added no duplicate. For PyPI inspect the `Compute PyPI version step` log and the built `dist/*` filenames for `.dev0` off `develop` vs a plain version on the default branch. -- Inspect the latest real publish's logs for `PublicRelease`/`SemVer2` per leg and confirm the artifact lifecycle (uploaded, consumed, deleted; none left behind). +- Inspect the latest real publish's logs for `PublicRelease`/`SemVer2` per leg and confirm the artifact lifecycle (uploaded, consumed, deleted, with none left behind). ### Assessment diff --git a/repo-config/README.md b/repo-config/README.md index 7625ab4..71fed17 100644 --- a/repo-config/README.md +++ b/repo-config/README.md @@ -15,7 +15,7 @@ Two workflow models share `main.json` but differ on `develop` (registry `workflo `main` (both models) requires merge-commit merges (no linear-history rule), signed commits, a passing `Check pull request workflow status job`, resolved review threads, and Copilot review, and blocks force-pushes and deletion, so a `develop -> main` promotion is always gated even when `develop` takes direct commits. Every ruleset intentionally leaves "Require branches to be up to date before merging" **off**, per [GOVERNANCE.md "Branching Model"][governance-branching-model]. -The result is **exactly two rulesets named `develop` and `main`**, and the names are load-bearing (`GOVERNANCE.md` and the workflows reference them); only the `develop` *content* varies by model. The required check binds by name and only turns green after the repo's PR workflow runs once. +The result is **exactly two rulesets named `develop` and `main`**, and the names are load-bearing (`GOVERNANCE.md` and the workflows reference them). Only the `develop` *content* varies by model. The required check binds by name and only turns green after the repo's PR workflow runs once. ## Secrets diff --git a/scripts/README.md b/scripts/README.md index 567d2dd..5609263 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -42,6 +42,8 @@ A double-quoted span in markdown is treated as a quotation and not scanned for p 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. +**The semicolon rule reads the list where it lives.** The comma qualifies the list as a whole rather than one separator's position, so an enumeration whose commas fall in a later item keeps every semicolon it carries. Reading it positionally split one series in two, flagging the openers of the same list it then exempted the tail of, which would have restructured the enumerated guarantees the exemption exists to protect. A markdown table row is judged one cell at a time, since a row is a record of fields and a comma in one column cannot excuse a semicolon in another, and a bullet's `**Label**:` is dropped before the line is read, because it opens the bullet rather than announcing a list, the same construct the label dash is exempted for. What this misses is a sentence that reads as a list without being one: a colon early in a long line still excuses a splice later on it, which reading the diff catches. + **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. diff --git a/scripts/prose_lint.py b/scripts/prose_lint.py index 55897da..253c25d 100644 --- a/scripts/prose_lint.py +++ b/scripts/prose_lint.py @@ -542,6 +542,21 @@ def strip_quoted(s: str) -> str: return re.sub(r'"[^"\n]*"', '""', s) +# A bullet's `**Label**:` opens the text the same way `- **Label** -` does, so its colon +# introduces the bullet rather than a list, and reading it as one excused the splice after it. +LABEL_COLON = re.compile(r'^\s*(?:[-*]|[0-9]+\.)\s+\*\*[^*]+\*\*\s*:') + + +def list_spans(s: str) -> list[str]: + """Split a line into the spans that each hold their own list. + + A markdown table row is a record of fields rather than one sentence, so judging the row whole + let a comma in one column excuse a semicolon in another. + """ + cells = s.strip().strip('|').split('|') if s.lstrip().startswith('|') else [s] + return [LABEL_COLON.sub('', cell) for cell in cells] + + def in_numeric_context(line: str, pos: int) -> bool: """Whether the character at `pos` sits in an expression rather than in a sentence. @@ -801,12 +816,18 @@ def check_file(path: Path, rules: set[str]) -> list[tuple[int, str, str]]: # 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): + for span in list_spans(prose): # A list keeps its semicolons, announced by a colon or a second separator. - if listish and ',' in prose[:m.start()]: + # The comma is a property of the list rather than of one separator's position, + # so an enumeration whose commas fall in a later item keeps every semicolon it + # carries. Reading it positionally split such a list, flagging the openers of + # the same series it then exempted the tail of. + listish = span.count(';') > 1 or ':' in span.split(';')[0] + if listish and ',' in span: continue - out.append((i, 'semicolon', 'semicolon in prose -> a comma or two sentences')) + for _ in SEMICOLON.finditer(span): + 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): diff --git a/scripts/test_prose_lint.py b/scripts/test_prose_lint.py index c907830..6384a6b 100644 --- a/scripts/test_prose_lint.py +++ b/scripts/test_prose_lint.py @@ -246,6 +246,22 @@ def test_a_splice_whose_clause_carries_a_comma_is_still_a_splice(self) -> None: self.assertEqual(['semicolon'], self.kinds('It runs on push, always; it gates the merge.\n', {'semicolon'})) + def test_a_list_whose_commas_fall_in_a_later_item_keeps_its_semicolons(self) -> None: + """The comma qualifies the list, so reading it positionally split one series in two.""" + self.assertEqual([], self.kinds( + 'It exists; it covers each target, and excludes the rest; it runs.\n', {'semicolon'})) + + def test_a_table_row_judges_each_cell_alone(self) -> None: + """A row is a record of fields, so one column's comma cannot excuse another's semicolon.""" + self.assertEqual(['semicolon'], self.kinds('| S1 | it runs; it gates | D1, D2 |\n', + {'semicolon'})) + + def test_a_bullet_label_colon_does_not_announce_a_list(self) -> None: + """`- **Label**:` opens the bullet, the same construct the label dash is exempted for.""" + self.assertEqual(['semicolon'], + self.kinds('- **Async**: avoid blocking calls; use await, always\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'):