From ebb1b40f87d12bb86403c3cb9f5ef12eee55175c Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sun, 18 Oct 2026 08:57:19 -0700 Subject: [PATCH 1/2] State the Fleet Standard for Script and Utility File Names (#3198) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Adds a General subsection "Script and Utility File Names" to `CODESTYLE.md` carrying the standard agreed in the issue: the per-type table, the installed-commands, wrapper, externally-fixed, and directories clauses, and the stated departures from the Google Shell Style Guide and Microsoft's approved verbs. - Adds a "Repo-Scoped Secrets" bullet to `GOVERNANCE.md` stating that a secret file's stem is lowercase snake_case, pointing at the new section. - Points the `python-codestyle` Skill's module naming at the new section, so the rule has one home, and regenerates the Skill distributions. Wording departs from the issue's text only where a gate or a carried reader needs it: long sentences are split for the sentence-length check, and the shell departure no longer says hyphens match "the existing scripts", since `CODESTYLE.md` is carried to repositories whose scripts do not yet follow it. ## Local-review findings The precedence question this section listed was answered on #3199: any executable or launcher on `PATH` or in a system slot takes the command name, and the wrapper exemption covers only a launcher run by its path. That landed in e1685da7 and the two commits before it. The one `introduced` finding the next pass raised, the "Wrapper exemption" restating the installed names incompletely (no `fetch-report.ps1`), is **Fixed in b59b663a**: the bullet now gives one example and leaves the naming to "Installed commands". The pass over b59b663a recorded no findings, so no local-review finding is open. ## Verification - `prose_lint.py --diff` and `--check sentence-length`, `build_dist.py --check`, `spec/validate.py`, `repo_gate.py --check eol`: clean. - `docker_lint.py` markdownlint, cspell, editorconfig-checker: clean. - Full `tests/` unittest suite: OK. Closes on promotion: #3140 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit * **Documentation** * Clarified file-naming conventions for scripts and utilities, including casing by file type, command names, and applicable exceptions. * Added examples and guidance for naming secret files with lowercase `snake_case` stems. * Updated related references and aligned the Python style guidance with the repository’s filename rules. --------- Co-authored-by: Claude Opus 5.5 --- .../python-codestyle/references/code-style.md | 3 ++- .../.source-digests/python-codestyle | 2 +- .../python-codestyle/references/code-style.md | 3 ++- .../python-codestyle/references/code-style.md | 3 ++- CODESTYLE.md | 27 +++++++++++++++++++ GOVERNANCE.md | 1 + 6 files changed, 35 insertions(+), 4 deletions(-) diff --git a/.agents/skills/python-codestyle/references/code-style.md b/.agents/skills/python-codestyle/references/code-style.md index 45f355bc..1941e0f3 100644 --- a/.agents/skills/python-codestyle/references/code-style.md +++ b/.agents/skills/python-codestyle/references/code-style.md @@ -64,7 +64,8 @@ ## Naming -- `snake_case` for functions, methods, variables, modules, package directories. +- `snake_case` for functions, methods, variables, modules, package directories. A module's file + name follows CODESTYLE.md's "Script and Utility File Names". - `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, diff --git a/.claude-plugin/fleet-skills/.source-digests/python-codestyle b/.claude-plugin/fleet-skills/.source-digests/python-codestyle index 6859deaa..07ed7b54 100644 --- a/.claude-plugin/fleet-skills/.source-digests/python-codestyle +++ b/.claude-plugin/fleet-skills/.source-digests/python-codestyle @@ -1 +1 @@ -047e89b805ed646f +bba583635a70530a diff --git a/.claude-plugin/fleet-skills/skills/python-codestyle/references/code-style.md b/.claude-plugin/fleet-skills/skills/python-codestyle/references/code-style.md index 45f355bc..1941e0f3 100644 --- a/.claude-plugin/fleet-skills/skills/python-codestyle/references/code-style.md +++ b/.claude-plugin/fleet-skills/skills/python-codestyle/references/code-style.md @@ -64,7 +64,8 @@ ## Naming -- `snake_case` for functions, methods, variables, modules, package directories. +- `snake_case` for functions, methods, variables, modules, package directories. A module's file + name follows CODESTYLE.md's "Script and Utility File Names". - `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, diff --git a/.github/skills/python-codestyle/references/code-style.md b/.github/skills/python-codestyle/references/code-style.md index 45f355bc..1941e0f3 100644 --- a/.github/skills/python-codestyle/references/code-style.md +++ b/.github/skills/python-codestyle/references/code-style.md @@ -64,7 +64,8 @@ ## Naming -- `snake_case` for functions, methods, variables, modules, package directories. +- `snake_case` for functions, methods, variables, modules, package directories. A module's file + name follows CODESTYLE.md's "Script and Utility File Names". - `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, diff --git a/CODESTYLE.md b/CODESTYLE.md index fa23048c..da7f86f5 100644 --- a/CODESTYLE.md +++ b/CODESTYLE.md @@ -41,6 +41,27 @@ A command-line script is one a person or an agent runs by name. A hook its host - **Every command-line script answers `--help` and `-h`** with the same text on stdout, then exits 0, changing nothing. That text opens on a usage line naming the script. An agent learns a script's interface by asking it, so a script that reads `--help` as an input acts in a way nobody predicted. - **An argument the script does not recognize is refused**, with a non-zero exit before any read or write. The refusal names it as an unknown or unrecognized option, argument, flag, or parameter, and PowerShell's own refusal of a parameter it cannot bind counts. An unrecognized first argument is never taken as a default mode or as a target. +### Script and Utility File Names + +| Type | Rule | Basis | +| --- | --- | --- | +| Python `.py` | lowercase snake_case | [PEP 8][pep8-names-link] module naming. A hyphenated module also cannot be named in an `import` statement | +| Shell `.sh` and PowerShell `.ps1` | lowercase-hyphenated | The Linux command convention. A cross-platform pair keeps one name, `sync-tools.sh` beside `sync-tools.ps1` | +| Windows batch `.cmd` and `.bat` | lowercase-hyphenated | The same convention as shell and PowerShell. A shim that puts a command on `PATH` takes the name of the command it provides, `sync-tools.cmd` launching `sync-tools.ps1` | +| PowerShell functions inside a `.ps1` | Verb-Noun with an approved verb (`Get-Verb`) | PSScriptAnalyzer `PSUseApprovedVerbs`. This is a command rule, not a file rule | +| Env and secret files | lowercase snake_case stem | `mail_relay.env`, `db_root_password` | +| Env variable names and CI secret names | UPPER_SNAKE_CASE | The POSIX and GitHub convention | + +- **Installed commands.** An executable or launcher on `PATH` or in a system slot (systemd, cron, run-parts, motd) is named as a command, whatever its language. That name is lowercase-hyphenated. It carries no extension unless the platform needs one to run it, such as `.cmd` or `.ps1` on Windows. +- **Wrapper exemption.** A thin launcher whose only job is to run a Python tool keeps that tool's stem, so `fetch_report.py`, `fetch_report.sh`, and `fetch_report.ps1` stay grouped together. It covers only a launcher run by its path. A launcher installed onto `PATH` or into a system slot is an installed command instead, such as `fetch-report` rather than `fetch_report`. +- **Externally fixed names are exempt.** That covers Git hook names, `__init__.py`, `conftest.py`, and Pester's `.Tests.ps1` suffix, whose stem mirrors the script under test. It covers a PowerShell data file (`.psd1`) whose name a tool fixes, such as a module manifest named for its module. `PSScriptAnalyzerSettings.psd1` is another, since PSScriptAnalyzer finds it by that name. It also covers any name a platform defines. +- **Directories are out of scope.** + +Departures, stated on purpose so a reader does not "fix" them back: + +- The [Google Shell Style Guide][google-shell-link] asks for underscores in shell file names. The fleet uses hyphens instead, matching installed Linux commands. +- Microsoft's [approved verbs][ps-approved-verbs-link] govern command names. A `.ps1` run by path is a file rather than an exported command, so the file takes the shell convention and its functions take Verb-Noun. + ### Markdown and Spelling These apply repo-wide, in every directory: Markdown lints clean via `markdownlint-cli2` against the shared config, spelling is US English via CSpell against the shared `cspell.json`, the CI spelling gate covers `README.md` and `HISTORY.md` only, `HISTORY.md` mirrors the README's opening, and "Markdown" is a proper noun in prose. A repo excluding a subtree of its own that it does not treat as authored prose, a committed data archive, a vendored theme, or a hand-maintained record, puts a `.markdownlint-cli2.jsonc` carrying its own `ignores` beside that content rather than editing the shared root config, whose contents are fleet-fixed. Those `ignores` patterns resolve against the directory holding them rather than against the repo root, so a repo-root-relative entry there matches nothing and reports no error saying so, and excluding through the CI workflow's own negated Markdown glob input instead is a CI-only fix that leaves the same files flagged for anyone running the linter locally. The full rules are in the `comment-and-doc-style` Skill referenced above. @@ -71,3 +92,9 @@ Bash, and only where a program cannot be Python: a bootstrap that installs the i [governance-verification-discipline]: ./GOVERNANCE.md#verification-discipline [readme]: ./README.md [root]: ./.editorconfig + + + +[google-shell-link]: https://google.github.io/styleguide/shellguide.html +[pep8-names-link]: https://peps.python.org/pep-0008/#package-and-module-names +[ps-approved-verbs-link]: https://learn.microsoft.com/en-us/powershell/scripting/developer/cmdlet/approved-verbs-for-windows-powershell-commands diff --git a/GOVERNANCE.md b/GOVERNANCE.md index 9038d571..e71c9501 100644 --- a/GOVERNANCE.md +++ b/GOVERNANCE.md @@ -109,6 +109,7 @@ Line-ending governance for an operational repo is in [Line Endings](#line-ending A repo whose own stacks or scripts read local runtime credentials from disk, most commonly an operational repo's Docker Compose stack, keeps them in a dotted `.secrets/` directory at the repo root. This is the repo-scoped counterpart to the host-scoped `~/.secrets/` convention a repo's own `OPERATIONS.md` may document, and it is a different thing from `spec/secrets.json`, the CI/GitHub Actions secret-name registry `spec/audit.py` cross-checks. `spec/secrets.json` governs what a workflow reads from GitHub Actions. This convention governs what a repo's own process reads from its own checkout. - **The directory is named `.secrets/`, dotted, never a bare `secrets/`.** +- **A secret file's stem is lowercase snake_case** (`homeassistant_db_password`, `mail_relay.env`), per [`CODESTYLE.md` "Script and Utility File Names"](./CODESTYLE.md#script-and-utility-file-names). - **A single opaque credential file carries no extension** (`homeassistant_db_password`, not `homeassistant_db_password.txt`), the same reason `README` and `LICENSE` carry none. It is a security property, read but never sourced, not a formatting preference. - **A structured credential keeps its format's extension** (`.json` for structured config). - **The shared env file is named for what it configures**, not a bare `.env` (`docker.env` for a repo whose stacks are Docker Compose), so a second env-shaped file added later stays unambiguous. From 3b1fe0471d272ae0d202ad8c54652a459119a77b Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Sun, 18 Oct 2026 10:07:09 -0700 Subject: [PATCH 2/2] Report a Vetted Suppressed Summary Its Reader Misses in pr_review.py (#3202) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary `unrecognized_in` compared vetted summaries against what a block reader actually read only for the previously-missed section. The vetted suppressed spellings, `Suppressed comments (N)` and `Comments suppressed due to low confidence (N)`, got no such check against `suppressed_blocks`. A layout that reader misses therefore reported `shapes=ok` while `suppressed=` came out short. - The suppressed spellings move into a `SUPPRESSED_SUMMARIES` set, and the one existing comparison runs over both readers, rather than a copy being added beside it. - The comparison now counts only the summary opening each block. Counted over every summary a block holds, a block that ran on into the next section stood in for that section, so the over-run form of the defect stayed silent too. The local strict review found this. ## Tests - `test_a_vetted_suppressed_summary_its_reader_misses_is_reported` stubs `suppressed_blocks` to drop its last block over three constructed bodies. It fails on the base in all three cases. - `test_a_block_running_into_the_next_section_does_not_stand_in_for_it` patches `sibling_heading` to never bound. It fails with the first-line counting reverted. - Full suite: 2968 tests OK, 7 skipped. Ruff, mypy, prose gate, eol, `spec/validate.py`, and `build_dist.py --check` are clean. ## Local review Three recorded passes at the `frontier` tier. The first two raised findings that were fixed in this branch, and the third raised none. One finding outside this diff, which predates the branch, is filed as #3201. Refs #3193 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit * **Bug Fixes** * Improved detection of unread sections in pull request reviews, including suppressed sections and sections whose boundaries are unclear. * Review results now identify vetted section headings that were missed, helping make review coverage more accurate. --- scripts/pr_review.py | 43 ++++++++++++++++++---------------- tests/test_pr_review.py | 52 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 20 deletions(-) diff --git a/scripts/pr_review.py b/scripts/pr_review.py index e0256edd..a7bb7b6c 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -734,6 +734,7 @@ def strip_fences( "(N) previously missed issues", "(N) previously missed issue", } +SUPPRESSED_SUMMARIES = {"Suppressed comments (N)", "Comments suppressed due to low confidence (N)"} VETTED_SUMMARIES = ( set(OVERVIEW_SECTIONS) | { @@ -742,9 +743,8 @@ def strip_fences( "Show a summary per file", "File summaries", "Review details", - "Suppressed comments (N)", - "Comments suppressed due to low confidence (N)", } + | SUPPRESSED_SUMMARIES | PREVIOUSLY_MISSED_SUMMARIES ) VETTED_LABELS = {"Files reviewed", "Comments generated", "Review effort level", "Review effort"} @@ -3734,11 +3734,13 @@ def unrecognized_in(body: str) -> list[str]: quoting a heading is not a review carrying one, and this script's own pull requests quote these lists in full. - A previously-missed summary that no reader finds as a section is reported too. Its spelling is - vetted wherever it appears, while the reader keys on how its line opens. A layout the reader - misses would otherwise hand its findings to nobody under `shapes=ok`. The check compares which - summaries the blocks hold rather than how many blocks there are. Compared by count, a bare - `(N)` line read in one region stood in for a summary missed in another. + A previously-missed or suppressed summary that its reader does not find as a section is + reported too. Its spelling is vetted wherever it appears, while the reader keys on how its line + opens and where the section around it ends. A layout the reader misses would otherwise hand its + findings to nobody under `shapes=ok`. The check compares which summaries open the blocks rather + than how many blocks there are. Compared by count, a bare `(N)` line read in one region stood in + for a summary missed in another. Compared by every summary a block holds, a block that ran on + into the next section stood in for that section. A body carrying no heading at all is reported rather than passed, since every one of the 333 measured opens on a heading and a body with none is a format nothing here has seen. A refusal @@ -3762,19 +3764,20 @@ def unrecognized_in(body: str) -> list[str]: for marker in (normal(s) for s in dict.fromkeys(SUMMARY.findall(plain))) if unvetted(marker, VETTED_SUMMARIES) and not finding_entry(marker) ] - missed = Counter( - marker - for marker in (normal(s) for s in SUMMARY.findall(plain)) - if not unvetted(marker, PREVIOUSLY_MISSED_SUMMARIES) - ) - read = Counter( - normal(s) - for block in previously_missed_blocks(body) - for s in SUMMARY.findall(CODE_SPAN.sub(" ", strip_fences(block))) - ) - unread = missed - read - if unread: - found.append(f"summary: {next(iter(unread))}, vetted while no reader found its section") + summaries = [normal(s) for s in SUMMARY.findall(plain)] + for vetted, reader in ( + (PREVIOUSLY_MISSED_SUMMARIES, previously_missed_blocks), + (SUPPRESSED_SUMMARIES, suppressed_blocks), + ): + expected = Counter(marker for marker in summaries if not unvetted(marker, vetted)) + read = Counter( + normal(s) + for block in reader(body) + for s in SUMMARY.findall(CODE_SPAN.sub(" ", strip_fences(block)).split("\n", 1)[0])[:1] + ) + unread = expected - read + if unread: + found.append(f"summary: {next(iter(unread))}, vetted while no reader found its section") found += [ f"metadata label: {la}" for la in dict.fromkeys(labels) if unvetted(la, VETTED_LABELS) ] diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index 1f3a4206..ed181979 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -1705,6 +1705,58 @@ def test_the_vetted_but_unread_check_compares_which_summaries_were_read(self) -> pr_review.unrecognized_in(stand_in), ) + def test_a_vetted_suppressed_summary_its_reader_misses_is_reported(self) -> None: + """The suppressed spellings are vetted wherever they appear, as the previously missed are. + + A layout that `suppressed_blocks` misses therefore reported `shapes=ok` while `suppressed=` + came out short. Each case reads correctly first, so the stub dropping a block is what + changes the answer. + """ + both = ( + f"{OVERVIEW}\n{COVERED}\n\n
\nReview details\n\n" + + entries_section("Suppressed comments (1)", "Constructed finding 1") + + entries_section("Previously missed (1)", "Constructed finding 2") + + entries_section("Suppressed comments (2)", "Constructed finding 3", "Constructed 4") + + "
\n" + ) + cases = { + "low confidence": collapsed(), + "suppressed": collapsed(heading="Suppressed comments (2)"), + "alternating in one wrapper": both, + } + for name, body in cases.items(): + with self.subTest(name): + blocks = pr_review.suppressed_blocks(body) + self.assertEqual([], pr_review.unrecognized_in(body)) + with mock.patch.object(pr_review, "suppressed_blocks", return_value=blocks[:-1]): + found = pr_review.unrecognized_in(body) + self.assertEqual(1, len(found)) + self.assertIn("vetted while no reader found its section", found[0]) + self.assertTrue(found[0].startswith("summary: ")) + self.assertIn("uppressed", found[0]) + + def test_a_block_running_into_the_next_section_does_not_stand_in_for_it(self) -> None: + """Unbounded, the first section's block holds every section after it in the wrapper. + + Counted by every summary a block holds, the sections it ran into read as found, so an + unbounded block reported `shapes=ok`. Only the summary opening a block counts. + """ + body = ( + f"{OVERVIEW}\n{COVERED}\n\n
\nReview details\n\n" + + entries_section("Suppressed comments (1)", "Constructed finding 1") + + entries_section("Suppressed comments (2)", "Constructed finding 2", "Constructed 3") + + "
\n" + ) + self.assertEqual([], pr_review.unrecognized_in(body)) + with mock.patch.object( + pr_review, "sibling_heading", side_effect=lambda lines, _: len(lines) + ): + self.assertEqual(1, len(pr_review.suppressed_blocks(body))) + found = pr_review.unrecognized_in(body) + self.assertEqual( + ["summary: Suppressed comments (N), vetted while no reader found its section"], found + ) + def test_every_no_thread_render_drops_the_same_presentation_markup(self) -> None: """All of these sections come from one format, so a badge in a suppressed block buries its title exactly as one in a previously-missed block does.