Harden shared spelling rollout - #49
Merged
Merged
Conversation
Scope conditional HTTP validators to the source that supplied them so a changed authority cannot reuse stale freshness metadata. Reject generated `typos.toml` drift in the spelling gate and add the generic Oxford `italicize` family harvested from the estate.
|
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:
WalkthroughUpdate Oxford spelling mappings, enforce generated configuration freshness, and prevent HTTP validators from being reused after a source URL changes. ChangesSpelling and refresh safeguards
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 18 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (18 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideHardens the shared en-GB Oxford dictionary rollout by scoping HTTP freshness metadata per source, enforcing that the Makefile spelling gate fails on stale generated configuration, and expanding the Oxford-based dictionary with a new italicization spelling family plus corresponding regression tests. Sequence diagram for source-scoped HTTP metadata in _refresh_httpsequenceDiagram
participant Caller
participant typos_rollout as typos_rollout
participant RemoteSource
Caller->>typos_rollout: _refresh_http(source, metadata, opener)
typos_rollout->>typos_rollout: _read_metadata(metadata)
typos_rollout->>typos_rollout: [saved.source != source]
alt source_mismatch
typos_rollout->>typos_rollout: saved = {}
else source_match
typos_rollout->>typos_rollout: saved unchanged
end
typos_rollout->>typos_rollout: _conditional_headers(saved)
typos_rollout->>RemoteSource: Request(source, headers)
RemoteSource-->>typos_rollout: HTTP response
typos_rollout-->>Caller: RefreshResult
Flow diagram for strengthened Makefile spelling gateflowchart LR
A[spelling target] --> B[generate typos config via typos_rollout_cli.py]
B --> C[git diff --exit-code -- typos.toml]
C -->|no drift| D[run typos with typos.toml]
C -->|drift detected| E[spelling target fails]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
leynos
marked this pull request as ready for review
July 13, 2026 19:11
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f197327927
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@tests/test_typos_rollout.py`:
- Around line 282-288: In the test setup, annotate the captured requests
variable as list[object], and update the Response class to initialize headers in
an __init__ method instead of defining a mutable class-level dictionary.
Preserve the existing status attribute and response behavior.
- Around line 326-332: Extend the refresh assertions in the relevant test to
verify that the persisted cached dictionary contains the replacement content and
that its metadata references the replacement source. Keep the existing
request-header and result.status assertions, and use the test’s established
cache-reading or metadata symbols to validate the stored state after refresh.
🪄 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: 8bbec71e-3338-4279-896e-5493c9aa4252
📒 Files selected for processing (5)
Makefiledata/typos-oxendict-base.tomlscripts/typos_rollout.pytests/test_typos_rollout.pytypos.toml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tests/test_typos_rollout.py`:
- Around line 506-508: Replace the textual assertion in the relevant test with a
behavioral test that creates or uses stale generated configuration, runs the
spelling target, and asserts a non-zero exit status. Reuse the existing test
helpers and spelling-target invocation patterns in tests/test_typos_rollout.py
rather than checking Makefile contents directly.
🪄 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: 570c6adf-4a06-40f8-8b40-8913a4020c8f
📒 Files selected for processing (5)
Makefiledata/typos-oxendict-base.tomlscripts/typos_rollout.pytests/test_typos_rollout.pytypos.toml
Require `typos.toml` to be indexed before checking generated drift, and exercise both stale and untracked configurations through the real Makefile target. Strengthen the source-switch regression to verify persisted state.
Owner
Author
|
@coderabbitai review |
Owner
Author
|
@codex review |
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Harvest the generic `underutilize` Oxford family from `rstest-bdd` so consumers preserve the correct form without repository-local exceptions.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@tests/test_typos_rollout.py`:
- Around line 520-578: Split tests/test_typos_rollout.py into domain-focused
test modules, placing the HTTP-refresh tests and Makefile spelling-gate tests in
separate modules and moving shared setup into conftest.py fixtures. Preserve
existing test behavior and ensure each resulting module stays under the
400-logical-line limit.
- Around line 544-554: Update test_spelling_gate_rejects_stale_generated_config
so it verifies the expected 'mold' entry was present and replaced before writing
the mutated policy. Assert that the replacement changed the file content, then
proceed with the existing fixture setup and assertions.
- Around line 533-541: Add timeouts to the subprocess calls in run_spelling_gate
and the nearby git invocations, using a suitable test-level duration so stalled
dependency resolution or repository commands fail promptly while preserving
existing command behavior.
🪄 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: 05885ffe-ef61-413e-9320-7ef6358fb48e
📒 Files selected for processing (2)
Makefiletests/test_typos_rollout.py
Keep refresh, rendering, and spelling-gate behaviour in focused test modules below the repository's line limit. Bound subprocess fixtures, verify stale policy mutation, and retain deliberate spelling samples outside enforcement.
This was referenced Jul 14, 2026
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
1 participant
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.
Summary
This branch hardens the shared en-GB-oxendict rollout after downstream review found that HTTP freshness validators could cross source boundaries and that consumer spelling gates did not reject stale generated configuration. It also adds the generic Oxford
italicizefamily harvested frommxd.No matching roadmap task, implementation ExecPlan in this repository, or open issue was found for this follow-up.
Review walkthrough
Validation
make ci: 129 tests passed, including 3 snapshots; syntax, shell syntax, home-phase boundary, type checking, generated-config drift and pinned spelling all passed.mbake validate Makefile: passed.git diff --check: passed.Notes
A standalone whole-repository ShellCheck still reports findings in unchanged legacy scripts
notdeadyetandpython-setup; ShellCheck is not part of this repository'smake cigate and this branch does not modify those files.Summary by Sourcery
Harden shared spelling rollout by scoping HTTP cache metadata to each source, enforcing freshness of generated spelling configuration, and extending the shared dictionary with Oxford italicize mappings.
Bug Fixes:
Enhancements:
Tests: