Repository navigation
Contributors and coding agents have one written style guide, and CONTRIBUTING sends style questions to it - #982
Merged
jmrplens merged 1 commit intoOct 21, 2026
Conversation
jmrplens
added this pull request to stack #984
October 17, 2026 17:11
|
Sorry @jmrplens, your pull request is larger than the review limit of 150,000 diff characters |
|
Important Review skippedToo many files! This PR contains 107 files, which is 7 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. ⚙️ Run configuration
📒 Files selected for processing: 107
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jmrplens
changed the title
jmrplens
force-pushed
the
docs/contributor-style-guide
branch
from
October 17, 2026 17:12
3dc9b09 to
8bcf21d
Compare
github-actions
Bot
added
documentation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## fix/names-records-and-axes #982 +/- ##
==============================================================
+ Coverage 92.05% 92.06% +0.01%
==============================================================
Files 446 446
Lines 91513 91513
==============================================================
+ Hits 84242 84253 +11
+ Misses 7271 7260 -11 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Numerical conformance
1893/1893 checks pass across 111 domains and 523 standards (227 normative designations, 125 further published sources). Used in the tables below is how much of that clause's published tolerance the deviation consumes: 100 % sits exactly on the limit, 5 % uses a twentieth of the allowance, and a dash means the clause states no two-sided tolerance for the quantity, so there is no budget to spend. It is reported and never used to decide a verdict, which is settled at full precision before any rounding. Renamed (4)
Closest to their published limit (top 5) The rows with the least room left, so the ones a change is most likely to push over.
|
| Python Version | Tests | Failures | Coverage | Status |
|---|---|---|---|---|
| macos-latest-3.12 | 29267 | 0 | 92.1% |  Passed |
| macos-latest-3.13 | 29267 | 0 | 92.1% | Passed |
| macos-latest-3.14 | 29267 | 0 | 92.1% | Passed |
| ubuntu-latest-3.12 | 29267 | 0 | 92.1% | Passed |
| ubuntu-latest-3.13 | 29267 | 0 | 92.1% | Passed |
| ubuntu-latest-3.14 | 29267 | 0 | 92.1% | Passed |
| windows-latest-3.12 | 29267 | 0 | 92.1% | Passed |
| windows-latest-3.13 | 29267 | 0 | 92.1% | Passed |
| windows-latest-3.14 | 29267 | 0 | 92.1% | Passed |
Full report at this commit: docs/CONFORMANCE.md · docs/conformance.json · full CI artifacts
jmrplens
force-pushed
the
docs/contributor-style-guide
branch
from
October 20, 2026 01:45
8bcf21d to
ff8be1f
Compare
 [Low impact] The PR appears safe to merge; the two remaining guide-test gaps are non-blocking. FindingsReviews (2) · Last reviewed commit: "Contributors and coding agents have one ..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+201
to
+206
| config = tomllib.loads((root / "pyproject.toml").read_text(encoding="utf-8")) | ||
| selected = config["tool"]["ruff"]["lint"]["select"] | ||
| return ( | ||
| None | ||
| if any(code.startswith(prefix) for prefix in selected) | ||
| else f"ruff {code} is not selected" |
There was a problem hiding this comment.
enforcer_problem reads Ruff's select list but never its ignore list. For example, ruff D205 passes because D2 is selected, even though D205 is explicitly disabled. The guide can therefore name a check that does not run. Check the effective Ruff rules and add a test for an ignored rule.
Owner
Author
Comment on lines
+128
to
+138
| end = starts[k + 1] if k + 1 < len(starts) else len(prose) | ||
| enforced = [ | ||
| m.group(1) for _, ln in prose[start + 1 : end] if (m := _ENFORCED.match(ln)) | ||
| ] | ||
| if len(enforced) != 1: | ||
| problems.append( | ||
| f"{path.name}:{lineno}: {rule_id} needs exactly one '**Enforced by:**' line, has {len(enforced)}" | ||
| ) | ||
| rules.append( | ||
| Rule(rule_id, path.name, statement, enforced[0] if enforced else "") | ||
| ) |
There was a problem hiding this comment.
read_page checks the anchor and Enforced by line, but never requires Why, Do or Don't. dont_fence_problems also succeeds when those sections are absent. The valid _GOOD_RULE fixture already has no examples. Require these parts and test each missing part so explanations and examples cannot disappear unnoticed.
Owner
Author
There was a problem hiding this comment.
…RIBUTING sends style questions to it
phonometry now has a written style guide in `docs/contributing/`: an index and seven pages (naming, API shape, docstrings and comments, numbers and data, tests, documentation and figures, prose, commits and pull requests) holding 235 rules. Every rule has an identifier a review can cite (N7, F35), says what went wrong without it, shows one example to follow and one to avoid, and names the check that enforces it or says that review does. CONTRIBUTING.md keeps the process and sends every style question to the guide, `AGENTS.md` sends coding agents there before anything else, and the review bot cites the rule identifiers.
## What a contributor gets
Read the index, then the page for the area you are changing. The index lists every rule with its one-line statement and its check, in page order, and closes with the 62 rules no script checks yet, each with the idea of the check that would. It also says what to do where the code around a change breaks a rule: the rule wins, the majority form is not copied, and a rule that looks wrong is raised rather than worked around.
The guide lives only in the repository: English, no site page, no Spanish twin. The style sections of CONTRIBUTING (subscripts, language, style defaults, long numbers, printed limits, arrays, notebook display, code fences, naming, the three verbs of a judgment, module length, deprecations) moved into it, brought up to the final 4.0 names, and every docstring, guard message and workflow comment that pointed at one of them now points at its rule. CONTRIBUTING gains what it was missing as process: the editable install the gates need, what `make api-docs` and `make llms` regenerate and that CI fails on their drift, and the bullets an errata entry carries.
The Tests page also holds the two rules every workflow follows: zizmor finds nothing under `.github/` (T22, held by the `Audit the workflows with zizmor` step of CI), and every action is pinned to a release tag, never a branch or a commit SHA (T23, held by `tests/test_repository_config.py::test_every_action_is_pinned_to_a_release_tag`, with zizmor refusing an action used with no ref at all).
## What a coding agent gets
`AGENTS.md` is one screen: the layout, the setup, the gates as CI runs them (zizmor included), and the reading order (the index, the page for the area, then CONTRIBUTING). It states no rule of its own, and a test keeps it that way. The other agent entry file at the root is one line that imports it, and `.coderabbit.yaml` asks the review bot to check `src`, `tests`, `scripts` and `docs` against the guide and cite the rule, with the contributor guide itself exempt from the docs twin rule it states.
## How the guide stays true
`tests/test_contributing_guide.py` holds the guide to itself: every page is well formed (an anchor per rule, one statement, one Enforced by line, a Don't example that can never run), the index and the pages agree rule by rule, every enforcer the guide names exists (a script, a test, a Makefile target, a selected ruff code, a CI job or one of its steps), every link into the guide resolves, and nothing sends a reader to a CONTRIBUTING section the guide absorbed. The guide's Do examples run with every other documentation snippet, and that CI job now installs the pinned pytest its examples of tests need.
Before the pages existed:
```
FAILED tests/test_contributing_guide.py::test_every_page_is_well_formed[api.md]
FAILED tests/test_contributing_guide.py::test_index_matches_the_pages - FileNotFoundError
FAILED tests/test_contributing_guide.py::test_every_enforcer_exists - FileNotFoundError
FAILED tests/test_contributing_guide.py::test_no_reference_to_a_moved_section
E AssertionError: these sections moved to docs/contributing/ and must leave CONTRIBUTING.md
E Left contains 15 more items, first extra item: 'Setting a subscript'
10 failed, 8 passed in 0.58s
```
And with one index row reworded by hand:
```
E AssertionError: N16
E - Put a unit in a name wherever two customary units compete for the quantity
E + Put a unit in a name exactly where two customary units compete for the quantity
1 failed, 18 deselected in 0.10s
```
And with T22 citing the zizmor step before the check read the name of a step:
```
E Left contains 2 more items, first extra item: "T22: no CI job named 'Audit the workflows with zizmor'"
1 failed, 18 deselected in 0.29s
```
On this branch: `20 passed`.
## What writing the rules down turned up
Every rule was checked against the tree. Where a rule claimed more than was decided or than its check holds, the rule now says what is decided and its Enforced by line says what the check covers. Where the tree broke a rule that is not in question, it is fixed here:
- `environment.sound_pressure_level_history` and `environment.impulsive_sound_adjustment` refuse a stereo signal instead of reading it as one channel twice as long (X8).
- The layered-absorber and slit-resonator plots keep their octave labels; drawing the reflection factor had reset the axis to 10^2 and 10^3, and the one guide figure that relabeled it after the call no longer needs to (F35). The figure is byte-identical.
- Eight `.plot()` methods take `language` by name instead of only through `**kwargs`, and `building.SoundReductionResult.report()` takes `metadata` and `language` (A28, A29). Seven of them, the laboratory, intensity and survey insulation results, hand it to the rating plot they draw, and a new test draws each one in Spanish and reads its frequency axis back.
- The tests of the API reference check put their fake packages on the import path with `monkeypatch`, so the path is restored by pytest rather than by hand.
- Eleven docstrings open with a noun phrase rather than "Return" or "Get", and the last `:type`, `:rtype` and `:returns:` fields are gone (D8, D4).
- 52 `noqa` comments in `scripts` and `tests` carried their reason after the codes; the reasons moved to the line above (C4).
- The errata registers write their powers of ten in LaTeX and put their three one-line display formulas on lines of their own (F58, F14).
- `room.enclosed_space_reverberation` no longer copies a frequency axis its record copies anyway (X11).
Four guards close the class behind those fixes, each seen failing first:
```
test_no_suppression_comment_hides_its_reason_from_the_parser: 52 directives in scripts/ and tests/ (it read src only)
check_markdown_hazards.py: docs/ERRATA.md:9868 '$$u_{L_\mathrm{c}} = ...' puts the formula on the opening '$$' (and five more)
test_a_published_type_variable_or_protocol_is_reported: assert [] == ["fake_typing_domain.__all__ lists 'Proto', a Protocol", ...]
test_every_plot_takes_ax_first_and_the_rest_by_name: ...IntensityReductionResult.plot: takes no 'language' (and seven more)
```
All four pass on this branch, as do the content test of the absorber plot and the two new stereo refusals, which fail on the code before the fix.
Where the tree breaks a rule and the fix would change the 4.0 API or needs a decision of its own (`str`-typed choices, a few defaulted unit-named arguments still positional, two records that still unpack like the tuples they replaced), the rule stays as decided and the break is not written into it as an exception.
## Gates
`ruff check .` and `ruff format --check .` clean; `mypy src scripts stub/src tests/static_typing` no issues in 706 files; `bandit -q -r src` clean. Every `scripts/check_*.py` that runs without a full figure render passes; the three that read the recordings `make graphs` writes, the clip freshness check (the clips live in their own repository), the two release checks that read a built wheel and the stub metadata check (it needs a built stub) were left to CI. `make conformance`, `make api-docs`, `make llms` and `make site-reports` leave nothing to commit. The one figure whose renderer changed was regenerated in all four variants and is byte-identical.
Full suite: `29191 passed, 69 skipped`. Documentation snippets, the guide's examples included: `Documentation snippets pass all checks: 7737 blocks over 740 pages.` Site: install, type check, build and `html-validate` pass.
jmrplens
force-pushed
the
docs/contributor-style-guide
branch
from
October 20, 2026 08:09
ff8be1f to
9015cd5
Compare
|
10 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
phonometry now has a written style guide in
docs/contributing/: an index and seven pages (naming, API shape, docstrings and comments, numbers and data, tests, documentation and figures, prose, commits and pull requests) holding 235 rules. Every rule has an identifier a review can cite (N7, F35), says what went wrong without it, shows one example to follow and one to avoid, and names the check that enforces it or says that review does. CONTRIBUTING.md keeps the process and sends every style question to the guide,AGENTS.mdsends coding agents there before anything else, and the review bot cites the rule identifiers.What a contributor gets
Read the index, then the page for the area you are changing. The index lists every rule with its one-line statement and its check, in page order, and closes with the 62 rules no script checks yet, each with the idea of the check that would. It also says what to do where the code around a change breaks a rule: the rule wins, the majority form is not copied, and a rule that looks wrong is raised rather than worked around.
The guide lives only in the repository: English, no site page, no Spanish twin. The style sections of CONTRIBUTING (subscripts, language, style defaults, long numbers, printed limits, arrays, notebook display, code fences, naming, the three verbs of a judgment, module length, deprecations) moved into it, brought up to the final 4.0 names, and every docstring, guard message and workflow comment that pointed at one of them now points at its rule. CONTRIBUTING gains what it was missing as process: the editable install the gates need, what
make api-docsandmake llmsregenerate and that CI fails on their drift, and the bullets an errata entry carries.The Tests page also holds the two rules every workflow follows: zizmor finds nothing under
.github/(T22, held by theAudit the workflows with zizmorstep of CI), and every action is pinned to a release tag, never a branch or a commit SHA (T23, held bytests/test_repository_config.py::test_every_action_is_pinned_to_a_release_tag, with zizmor refusing an action used with no ref at all).What a coding agent gets
AGENTS.mdis one screen: the layout, the setup, the gates as CI runs them (zizmor included), and the reading order (the index, the page for the area, then CONTRIBUTING). It states no rule of its own, and a test keeps it that way. The other agent entry file at the root is one line that imports it, and.coderabbit.yamlasks the review bot to checksrc,tests,scriptsanddocsagainst the guide and cite the rule, with the contributor guide itself exempt from the docs twin rule it states.How the guide stays true
tests/test_contributing_guide.pyholds the guide to itself: every page is well formed (an anchor per rule, one statement, one Enforced by line, a Don't example that can never run), the index and the pages agree rule by rule, every enforcer the guide names exists (a script, a test, a Makefile target, a selected ruff code, a CI job or one of its steps), every link into the guide resolves, and nothing sends a reader to a CONTRIBUTING section the guide absorbed. The guide's Do examples run with every other documentation snippet, and that CI job now installs the pinned pytest its examples of tests need.Before the pages existed:
And with one index row reworded by hand:
And with T22 citing the zizmor step before the check read the name of a step:
On this branch:
20 passed.What writing the rules down turned up
Every rule was checked against the tree. Where a rule claimed more than was decided or than its check holds, the rule now says what is decided and its Enforced by line says what the check covers. Where the tree broke a rule that is not in question, it is fixed here:
environment.sound_pressure_level_historyandenvironment.impulsive_sound_adjustmentrefuse a stereo signal instead of reading it as one channel twice as long (X8)..plot()methods takelanguageby name instead of only through**kwargs, andbuilding.SoundReductionResult.report()takesmetadataandlanguage(A28, A29). Seven of them, the laboratory, intensity and survey insulation results, hand it to the rating plot they draw, and a new test draws each one in Spanish and reads its frequency axis back.monkeypatch, so the path is restored by pytest rather than by hand.:type,:rtypeand:returns:fields are gone (D8, D4).noqacomments inscriptsandtestscarried their reason after the codes; the reasons moved to the line above (C4).room.enclosed_space_reverberationno longer copies a frequency axis its record copies anyway (X11).Four guards close the class behind those fixes, each seen failing first:
All four pass on this branch, as do the content test of the absorber plot and the two new stereo refusals, which fail on the code before the fix.
Where the tree breaks a rule and the fix would change the 4.0 API or needs a decision of its own (
str-typed choices, a few defaulted unit-named arguments still positional, two records that still unpack like the tuples they replaced), the rule stays as decided and the break is not written into it as an exception.Gates
ruff check .andruff format --check .clean;mypy src scripts stub/src tests/static_typingno issues in 706 files;bandit -q -r srcclean. Everyscripts/check_*.pythat runs without a full figure render passes; the three that read the recordingsmake graphswrites, the clip freshness check (the clips live in their own repository), the two release checks that read a built wheel and the stub metadata check (it needs a built stub) were left to CI.make conformance,make api-docs,make llmsandmake site-reportsleave nothing to commit. The one figure whose renderer changed was regenerated in all four variants and is byte-identical.Full suite:
29191 passed, 69 skipped. Documentation snippets, the guide's examples included:Documentation snippets pass all checks: 7737 blocks over 740 pages.Site: install, type check, build andhtml-validatepass.