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. 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.