Adopt shared Oxford spelling enforcement - #190
Conversation
There was a problem hiding this comment.
Sorry @leynos, your pull request is larger than the review limit of 150000 diff characters
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 6 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (39)
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe change introduces a shared spelling-policy rollout with deterministic dictionary generation, cache refresh, phrase checking, Makefile integration, expanded tests, updated documentation, and consistent en-GB artefact terminology across release workflows. ChangesSpelling rollout
Possibly related PRs
Suggested reviewers: Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 3 warnings)
✅ Passed checks (16 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Sorry @leynos, your pull request is larger than the review limit of 150000 diff characters
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32ce772a05
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release.yml (1)
89-106: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winAdd explicit workflow permissions Declare a workflow-level
permissions:block withcontents: readso this job does not inherit the default token scope.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml around lines 89 - 106, Declare a workflow-level permissions block in release.yml with contents set to read, applying explicitly to the build-macos workflow and preventing inheritance of broader default token permissions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/documentation-style-guide.md`:
- Around line 632-633: Update the “Checkboxes” guideline in the documentation
style guide to use the repository’s en-GB-oxendict spelling, changing “GitHub
Flavored Markdown” to “GitHub-flavoured Markdown” while preserving the
surrounding wording and formatting.
In `@docs/execplans/phase-1-implement-the-socket-listener-in-weaverd.md`:
- Line 257: Update the “Artefacts and Notes” heading under “Artefacts and Notes”
to sentence case: capitalize only the first word, changing “Notes” to “notes”.
In `@scripts/generate_typos_config.py`:
- Around line 20-23: Update DEFAULT_BASE_URL to reference an immutable upstream
release tag or commit SHA instead of refs/heads/main. Preserve the existing
typos-oxendict-base.toml resource path so regeneration remains reproducible and
requires an explicit URL change for future dictionary updates.
In `@scripts/tests/test_typos_rollout_contract.py`:
- Line 1: Expand the module docstring in test_typos_rollout_contract.py to
describe the test’s purpose and utility—verifying Python syntax and Makefile
generated-configuration drift checks—and state that it should be run with pytest
from the repository root.
- Around line 19-25: Update the test around the Makefile assertions in
test_typos_rollout_contract.py to extract the spelling-config target recipe
before validating it. Assert that the indexed generated-config check and drift
check both occur within that recipe and appear in the required order, rather
than searching the entire Makefile.
In `@scripts/tests/test_typos_rollout.py`:
- Around line 18-23: Replace the RolloutModules tuple alias with a PEP 695 type
statement using the existing four-module tuple structure. Keep the alias name
and member types unchanged while adopting the Python 3.13 `type` syntax.
- Around line 26-42: Remove the module-local rollout_modules_fixture and reuse
the shared rollout_modules fixture from scripts/tests/conftest.py. Update tests
that need generate_typos_config to compose with the shared fixture or import
that module within the specific test, without redefining the fixture or
duplicating its path and cache setup.
In `@scripts/tests/typos_rollout_test_support.py`:
- Around line 1-11: Expand the docstring for the public dictionary_text helper
into numpydoc format, retaining its summary and adding Parameters and Returns
sections that document stem and the returned dictionary document string. Do not
change the function’s behavior or generated text.
In `@scripts/typos_rollout_check.py`:
- Line 15: Update the Sequence import in scripts/typos_rollout_check.py to use
collections.abc instead of typing, matching the established ABC type-hint import
pattern and satisfying Ruff UP035.
- Around line 226-237: In the main CLI flow, wrap
generator.dictionary_from_cache(repository) with handling for its OSError and
emit a concise actionable message directing the user to run the dictionary
generator first, then exit with the command’s existing failure status instead of
exposing a traceback. Preserve the current findings output and return behavior
when the cache loads successfully.
---
Outside diff comments:
In @.github/workflows/release.yml:
- Around line 89-106: Declare a workflow-level permissions block in release.yml
with contents set to read, applying explicitly to the build-macos workflow and
preventing inheritance of broader default token permissions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9a81a4e8-c0e1-45ff-95aa-15a8e5982f88
📒 Files selected for processing (37)
.github/workflows/build-and-package.yml.github/workflows/ci.yml.github/workflows/release.yml.gitignoreAGENTS.mdMakefilecrates/weaver-cli/src/tests/unit/help_output.rsdocs/developers-guide.mddocs/documentation-style-guide.mddocs/execplans/13-3-3-archive-3-2-2-daemon-start-help-guidance.mddocs/execplans/2-2-1-act-apply-patch-sub-command.mddocs/execplans/3-1-1-weaver-plugins-crate.mddocs/execplans/3-1-1a-plugin-for-rope.mddocs/execplans/4-1-1-scaffold-sempai-core-and-sempai.mddocs/execplans/4-1-2-structured-diagnostics-with-stable-error-codes.mddocs/execplans/4-1-5-normalization-into-canonical-formula-model.mddocs/execplans/5-2-1-define-the-rename-symbol-capability-contract.mddocs/execplans/5-2-5-unit-behavioural-and-end-to-end-coverage-for-rename-symbol.mddocs/execplans/7-1-1-stable-jsonl-schemas-for-observe-get-card.mddocs/execplans/7-1-4-cache-integration-for-card-extraction-keyed-by-uri.mddocs/execplans/phase-1-implement-the-socket-listener-in-weaverd.mddocs/execplans/phase-4-weaver-graph.mddocs/reliable-testing-in-rust-via-dependency-injection.mdscripts/generate_typos_config.pyscripts/tests/conftest.pyscripts/tests/test_typos_rollout.pyscripts/tests/test_typos_rollout_check.pyscripts/tests/test_typos_rollout_contract.pyscripts/tests/test_typos_rollout_policy.pyscripts/tests/test_typos_rollout_refresh.pyscripts/tests/typos_rollout_test_support.pyscripts/typos_rollout.pyscripts/typos_rollout_cache.pyscripts/typos_rollout_check.pyscripts/typos_rollout_http.pytypos.local.tomltypos.toml
|
Addressed the outside-diff least-privilege request from the latest review. The release workflow now defaults |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Use an explicit `let-else` branch for the missing argument diagnostic so the test preserves its failure message while satisfying Whitaker's panic-closure rule.
Replace Weaver's repository-local spelling generator with the shared dictionary refresh, cache and phrase-enforcement infrastructure. Preserve narrow upstream identifiers while correcting maintained prose, including the required `handwritten` form. Pin Nixie 1.1.0 in CI and install Merman 0.7.0 before Mermaid validation.
Extract policy validation, correction merging and phrase scanning into focused helpers shared with the hosted-green estate implementation. Add exact characterization coverage while keeping the helper gate aware of the new policy test module.
Fail before phrase checking when the shared dictionary authority is unavailable and no valid local cache exists. Keep validated stale-cache operation, improve checker diagnostics, and cover the cache-only contract. Address the remaining documentation, type, fixture, Makefile scope, and release-workflow review findings without weakening official names or the moving shared-authority policy.
Exercise the cache-backed generator success path and assert that rendered ignore and exclusion policy survives deterministic TOML generation.
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile`:
- Around line 72-89: The Markdown lint dependency chain currently runs
spelling-helper-test and its full test suite before every markdownlint
invocation. Update the Makefile targets around markdownlint, spelling-config,
spelling-phrase-check, and spelling-helper-test so routine Markdown linting uses
a fast Markdown-only path, while spelling policy generation still has an
explicit helper-test validation gate or an equivalent change-based cache/skip
mechanism.
In `@scripts/tests/conftest.py`:
- Around line 14-23: Expand the docstring of rollout_modules_fixture to use the
numpy style for this public fixture, retaining its summary and adding a Returns
section that documents the returned tuple of cache, HTTP refresh, and rollout
modules.
In `@scripts/tests/test_typos_rollout_policy.py`:
- Line 57: Define one module-level type alias for the three-module tuple,
following the accepted alias pattern in test_typos_rollout.py, and replace the
repeated tuple annotations at the affected rollout_modules declarations with
that alias. Keep the alias outside any TYPE_CHECKING block so runtime annotation
references remain valid.
- Line 17: Immediately above PROHIBITED_PHRASE, add a concise one-line comment
explaining that the split string prevents the phrase checker under test from
flagging its own prohibited phrase; leave the concatenation unchanged.
In `@scripts/typos_rollout_check.py`:
- Around line 50-124: Replace the full NumPy-style docstrings in the private
functions _tracked, _excluded, and _masked with concise single-line summary
docstrings, matching the existing pattern used by _phrase_findings and
_file_findings. Leave each function’s implementation unchanged.
In `@scripts/typos_rollout_http.py`:
- Around line 257-266: Remove the unreachable HTTP_NOT_MODIFIED branch from
_remote_response_result and preserve its valid-cache behavior through the
existing general _valid_cache/_remote_is_not_newer handling. Keep 304 processing
in _http_error_result, where production urllib responses are handled.
In `@scripts/typos_rollout.py`:
- Around line 43-66: Document the public Dictionary.phrase_corrections field in
the class docstring’s NumPy-style Attributes section, describing its
phrase-replacement pairs and matching the existing documentation style. Do not
alter the dataclass fields or related behavior.
- Around line 181-220: The merge_dictionaries function currently unions local
ignore_patterns and excluded_files without enforcing the non-weakening overlay
contract. Validate local.ignore_patterns and local.excluded_files against a
narrow-scope policy before merging, rejecting broad expressions such as “.*” or
“**/*”; alternatively, remove local authority and use only the shared dictionary
values, while preserving the existing sorted-union behavior for permitted
entries.
- Around line 104-133: Update _dictionary_from_text to reject Boolean and
floating-point schema values, accepting only an actual integer matching
SCHEMA_VERSION rather than relying on equality alone. Require all base-policy
tables (oxford, words, phrases, patterns, and files) and their expected keys to
be present and valid before constructing Dictionary; preserve sparse defaults
only for explicitly identified local overlays, and add rejection tests for
Boolean, floating-point, and incomplete dictionaries.
In `@typos.local.toml`:
- Around line 15-35: Remove the redundant anchored artifact-name and
artifact-suffix patterns from the ignore list in typos.local.toml, specifically
the entries matching those keys and their interpolated forms. Retain the
should-upload-workflow-artifacts patterns and the existing word-boundary entries
for artifact-name and artifact-suffix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f131c331-168a-4e4f-b983-bccdca45a2ee
📒 Files selected for processing (37)
.github/workflows/build-and-package.yml.github/workflows/ci.yml.github/workflows/release.yml.gitignoreAGENTS.mdMakefilecrates/weaver-cli/src/tests/unit/help_output.rsdocs/developers-guide.mddocs/documentation-style-guide.mddocs/execplans/13-3-3-archive-3-2-2-daemon-start-help-guidance.mddocs/execplans/2-2-1-act-apply-patch-sub-command.mddocs/execplans/3-1-1-weaver-plugins-crate.mddocs/execplans/3-1-1a-plugin-for-rope.mddocs/execplans/4-1-1-scaffold-sempai-core-and-sempai.mddocs/execplans/4-1-2-structured-diagnostics-with-stable-error-codes.mddocs/execplans/4-1-5-normalization-into-canonical-formula-model.mddocs/execplans/5-2-1-define-the-rename-symbol-capability-contract.mddocs/execplans/5-2-5-unit-behavioural-and-end-to-end-coverage-for-rename-symbol.mddocs/execplans/7-1-1-stable-jsonl-schemas-for-observe-get-card.mddocs/execplans/7-1-4-cache-integration-for-card-extraction-keyed-by-uri.mddocs/execplans/phase-1-implement-the-socket-listener-in-weaverd.mddocs/execplans/phase-4-weaver-graph.mddocs/reliable-testing-in-rust-via-dependency-injection.mdscripts/generate_typos_config.pyscripts/tests/conftest.pyscripts/tests/test_typos_rollout.pyscripts/tests/test_typos_rollout_check.pyscripts/tests/test_typos_rollout_contract.pyscripts/tests/test_typos_rollout_policy.pyscripts/tests/test_typos_rollout_refresh.pyscripts/tests/typos_rollout_test_support.pyscripts/typos_rollout.pyscripts/typos_rollout_cache.pyscripts/typos_rollout_check.pyscripts/typos_rollout_http.pytypos.local.tomltypos.toml
Reject incomplete authority dictionaries and broad local exceptions while retaining an explicit sparse-overlay path for repository-specific policy. Keep production HTTP 304 handling on the real urllib error path. Complete the remaining review documentation, type, fixture, and generated configuration clean-ups without weakening the spelling gate order.
Extract small predicates from the shared-authority and local-exception validators. This preserves the strict policy contracts while keeping the new validation code below the hosted code-health thresholds.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Dismissed as stale after all findings were addressed and all 21 review threads resolved. Current head 291ca72 passes the complete local gate, hosted build-test and release matrix, CodeScene, and CodeRabbit checks.
Summary
This branch replaces Weaver's repository-local spelling generator with the
shared en-GB-oxendict dictionary, cache and exact-phrase enforcement used
across the code estate. It updates maintained prose, enforces
handwritten,preserves narrow upstream identifiers and brings Mermaid CI to
nixie-cli==1.1.0withmerman-cli0.7.0.The refresh path is fail-closed: an unavailable HTTPS authority may use a
validated stale untracked cache, but cannot treat tracked
typos.tomlas apolicy-cache fallback. Shared authorities require an actual integer schema and
every policy table and key; only the explicitly identified local overlay may
use sparse input. Local regexes and file globs that could disable enforcement
broadly are rejected before policy merge.
Production HTTP 304 handling remains at urllib's
HTTPErrorboundary, and theunreachable response-object branch is removed. The release workflow defaults
to read-only contents access while retaining write access only for the
publishing job.
No linked issue, roadmap task or repository ExecPlan was found for this
estate-wide maintenance change.
Review walkthrough
CI workflow
and release workflow
for the strict helper, configuration, phrase and Typos order, the
Nixie/Merman pins and least-privilege token permissions.
rollout core,
policy validation,
cache handling,
HTTP refresh boundary
and phrase checker.
Together they refresh a newer complete shared authority, permit sparse input
only for
typos.local.toml, retain validated stale-cache operation, rejectbroad local exceptions and fail before phrase checking when no valid cache
exists.
semantic security tests,
generator tests,
checker tests
and refresh tests
for strict and sparse parsing, non-weakening local policy, successful
cache-backed generation, rendered ignore/exclude arrays and production
HTTPError 304 handling.
regenerated Typos configuration
and developers' guide.
removes an unchanged default-branch Whitaker violation required for a green
PR.
Validation
make check-fmt: passed.make lint: Rustdoc, Clippy and Whitaker passed.make test: 1,465 nextest cases passed; four skipped. Doctests: 93 passed,eight ignored.
make spelling: generated configuration current; phrase policy and Typos1.48.0 clean.
make markdownlint: 91 files, zero errors.make nixie: all diagrams valid.checkmake Makefile,mbake validate Makefile, Actionlint and sixworkflow-contract tests: passed.
leynos/agent-helper-scripts/refs/heads/mainauthority produced cache SHA-256c6d18b10a1671405bd6c6de758ec214dd54c412e38cb2a64067718c38815ad84;the generator contains zero tracked-configuration fallback paths.
sparse-overlay isolation, non-weakening exceptions, generated-overlay drift,
function-arity, file-length and whitespace audits passed.
df5a170base dependency surface is preserved.Notes
Hosted checks for
bcf7e096c845b244c8e84187307a0158eaa8ffd9and the tenevidence-mapped review-thread replies and resolutions are pending. The
untracked shared-dictionary cache remains local by design. The reconciliation
safety stash is retained until this pull request merges.