Enforce en-GB-oxendict spelling in source (#249) - #259
Conversation
|
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:
SummaryEnforce the repository-wide en-GB-oxendict spelling policy across identifiers, source prose, comments, docstrings, fixtures, and tracked Markdown, Python, and Rust files.
Formatting, tests, type checking, linting, documentation checks, Makefile validation, and diff checks pass. WalkthroughThe change standardizes repository spelling to en-GB-oxendict, extends the pinned ChangesOxford spelling enforcement
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 19 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (19 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideThis PR widens the repository spelling gate to cover Markdown, Python, and Rust, normalizes internal spellings and identifiers to en-GB-oxendict (Oxford -ize/-yse) conventions, and documents the policy and exception workflow, including narrowly scoped ignores for external contracts and test fixtures. Flow diagram for widened spelling gate and exception workflowflowchart TD
Dev[Run make spelling]
Dev --> MSpelling[Makefile spelling recipe]
MSpelling --> GenTypos[generate_typos_config.py]
GenTypos --> SharedDict[Shared estate dictionary]
GenTypos --> LocalOverlay[typos.local.toml]
GenTypos --> TyposToml[Generated typos.toml]
MSpelling --> TyposCheck[typos spelling check]
TyposCheck --> Files[Tracked *.md, *.py, *.rs]
LocalOverlay --> Ignores[patterns.ignore for external contracts and fixtures]
Ignores --> TyposCheck
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Link the Unreleased changelog entry to draft PR #259 and close the living ExecPlan with final validation, review, branch, and publication evidence.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eed2e13566
ℹ️ 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 review |
✅ Action performedReview finished.
|
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 `@cuprum/_pipeline_internals.py`:
- Line 7: Update the module docstring near the ``CommandResult`` assembly
description to use the repository-required “centralize” spelling instead of
“centralise”; leave the surrounding wording unchanged.
In `@docs/developers-guide.md`:
- Around line 1303-1304: Update the exception sentence in the documentation near
the existing docstrings, string fixtures, and prose guidance to match ADR-008:
exempt spellings required by external contracts and deliberate spelling-test
fixtures, rather than limiting exemptions to external APIs.
In `@scripts/tests/test_typos_rollout.py`:
- Line 32: Add diagnostic messages to the new assertions in the test covering
spelling targets and Oxford correction mappings. Update the assertion involving
spelling_recipe and the assertion involving mappings["organize"] to use the
Python assert message form, preserving the proposed failure-context messages and
existing conditions.
🪄 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: d764fa9d-7f15-40f1-bdf6-6d1d35d40253
📒 Files selected for processing (57)
AGENTS.mdCHANGELOG.mdMakefilebenchmarks/_benchmark_type_validators.pybenchmarks/_benchmark_types.pybenchmarks/fetch_main_benchmark_baseline.pybenchmarks/profile_tee_hotpath.pybenchmarks/sinks.pybenchmarks/summarize_folded.pybenchmarks/tee_profile_driver.pybenchmarks/tee_profile_scenarios.pybenchmarks/tee_profile_worker.pycuprum/_backend.pycuprum/_pipeline_config.pycuprum/_pipeline_internals.pycuprum/adapters/metrics_adapter.pycuprum/adapters/tracing_adapter.pycuprum/builders/args.pycuprum/context/registration.pycuprum/sh.pycuprum/unittests/_adapter_test_support.pycuprum/unittests/_tee_profile_concurrency_support.pycuprum/unittests/test_args_validators_property.pycuprum/unittests/test_backend_resolver_property.pycuprum/unittests/test_builder_property_based.pycuprum/unittests/test_env_context.pycuprum/unittests/test_fetch_main_benchmark_baseline.pycuprum/unittests/test_folded_summary.pycuprum/unittests/test_line_splitting.pycuprum/unittests/test_maturin_build.pycuprum/unittests/test_pipeline.pycuprum/unittests/test_profile_driver.pycuprum/unittests/test_rust_splice.pycuprum/unittests/test_rust_streams.pycuprum/unittests/test_safe_cmd_context.pycuprum/unittests/test_safe_cmd_run.pycuprum/unittests/test_safe_cmd_stdin.pycuprum/unittests/test_sh.pycuprum/unittests/test_sh_property_based.pycuprum/unittests/test_stream_drain.pycuprum/unittests/test_tee_profile_worker_concurrency.pycuprum/unittests/test_tee_profile_worker_core.pycuprum/unittests/test_tee_profile_worker_selector_metrics.pycuprum/unittests/test_tracing_span_concurrency.pycuprum/unittests/test_tracing_span_stateful.pydocs/adr-008-enforce-oxford-spelling-in-source.mddocs/contents.mddocs/developers-guide.mddocs/execplans/enforce-ize-identifiers.mdrust/cuprum-rust/src/splice/mod.rsscripts/tests/test_typos_rollout.pytests/behaviour/test_rust_streams_behaviour.pytests/behaviour/test_telemetry_adapters.pytests/helpers/execution.pytests/helpers/maturin.pytypos.local.tomltypos.toml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/shared-actions(auto-detected)leynos/pylint-pypy-shim(auto-detected)leynos/whitaker(auto-detected)
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@benchmarks/fetch_main_benchmark_baseline.py`:
- Around line 42-49: Expand the public NumPy-style docstrings at
benchmarks/fetch_main_benchmark_baseline.py:42-49 for ArtefactQuery, documenting
every query field and api_base_url; at
benchmarks/fetch_main_benchmark_baseline.py:210-216, document payload inputs,
selection order, and the None result; at
benchmarks/fetch_main_benchmark_baseline.py:247-252, document extraction results
and archive failure conditions; at
benchmarks/fetch_main_benchmark_baseline.py:270-275, document query, token,
result, and GitHub-response validation failures; and at
benchmarks/fetch_main_benchmark_baseline.py:357-358, document CLI arguments,
exit codes, and token-failure behavior. Also expand the public helper docstring
at tests/helpers/maturin.py:221-232 to document root, out_dir, the wheel result,
and include a useful example call, adding appropriate Parameters, Returns, and
Raises sections where applicable.
In `@docs/contents.md`:
- Around line 42-43: Update the ADR-008 entry in docs/contents.md to use the
existing [adr-008] reference label instead of an inline destination, ensuring it
reuses the single link definition already declared later in the document and
matches the surrounding reference-style entries.
In `@docs/developers-guide.md`:
- Around line 1306-1310: Update the documentation’s “shared estate dictionary”
wording to use the generator’s established terminology: shared/base
en-GB-oxendict dictionary and local cache. Keep the description aligned with the
implementation’s refresh and merge behavior.
In `@Makefile`:
- Around line 129-132: Update the Makefile spelling targets so configuration
generation is a shared prerequisite that runs before both spelling-helper-test
and the final scan. Ensure _run_spelling_gate and the commands under spelling
use the freshly generated typos.toml, and add or update relevant tests to verify
generation precedes validation.
In `@scripts/tests/test_typos_rollout.py`:
- Around line 79-91: Extend test_spelling_gate_preserves_documented_exceptions
with parameterized fixtures covering every local spelling-gate exception: the
/artifacts path, teh and ises fixture pairs, and the byte-splitting case. Feed
each fixture through _run_spelling_gate and assert success, while retaining
existing coverage for --artifact-name, artifacts, and mappings["organise"].
In `@typos.local.toml`:
- Around line 14-18: Update the typos.local.toml exceptions for "artifacts",
/artifacts, and --artifact-name to use boundaries that match only the intended
external API or CLI literals, not longer repository-owned values. Add near-miss
cases covering longer values to scripts/tests/test_typos_rollout.py, then
regenerate typos.toml from the updated configuration.
In `@typos.toml`:
- Line 34: Move the fenced-code and inline-code ignore patterns from
[default].extend-ignore-re into [type.markdown] so they apply only to Markdown
scanning, updating the generator or typos.local.toml as the source of truth and
regenerating typos.toml. Add fixtures verifying backtick-wrapped “organise” is
rejected in both Python and Rust files.
🪄 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: a2cb2f5e-44d5-4b0e-b501-1b657212ea41
📒 Files selected for processing (58)
AGENTS.mdCHANGELOG.mdMakefilebenchmarks/_benchmark_type_validators.pybenchmarks/_benchmark_types.pybenchmarks/fetch_main_benchmark_baseline.pybenchmarks/profile_tee_hotpath.pybenchmarks/sinks.pybenchmarks/summarize_folded.pybenchmarks/tee_profile_driver.pybenchmarks/tee_profile_scenarios.pybenchmarks/tee_profile_worker.pycuprum/_backend.pycuprum/_pipeline_config.pycuprum/_pipeline_internals.pycuprum/adapters/metrics_adapter.pycuprum/adapters/tracing_adapter.pycuprum/builders/args.pycuprum/context/registration.pycuprum/sh.pycuprum/unittests/__snapshots__/test_fetch_main_benchmark_baseline.ambrcuprum/unittests/_adapter_test_support.pycuprum/unittests/_tee_profile_concurrency_support.pycuprum/unittests/test_args_validators_property.pycuprum/unittests/test_backend_resolver_property.pycuprum/unittests/test_builder_property_based.pycuprum/unittests/test_env_context.pycuprum/unittests/test_fetch_main_benchmark_baseline.pycuprum/unittests/test_folded_summary.pycuprum/unittests/test_line_splitting.pycuprum/unittests/test_maturin_build.pycuprum/unittests/test_pipeline.pycuprum/unittests/test_profile_driver.pycuprum/unittests/test_rust_splice.pycuprum/unittests/test_rust_streams.pycuprum/unittests/test_safe_cmd_context.pycuprum/unittests/test_safe_cmd_run.pycuprum/unittests/test_safe_cmd_stdin.pycuprum/unittests/test_sh.pycuprum/unittests/test_sh_property_based.pycuprum/unittests/test_stream_drain.pycuprum/unittests/test_tee_profile_worker_concurrency.pycuprum/unittests/test_tee_profile_worker_core.pycuprum/unittests/test_tee_profile_worker_selector_metrics.pycuprum/unittests/test_tracing_span_concurrency.pycuprum/unittests/test_tracing_span_stateful.pydocs/adr-008-enforce-oxford-spelling-in-source.mddocs/contents.mddocs/developers-guide.mddocs/execplans/enforce-ize-identifiers.mdrust/cuprum-rust/src/splice/mod.rsscripts/tests/test_typos_rollout.pytests/behaviour/test_rust_streams_behaviour.pytests/behaviour/test_telemetry_adapters.pytests/helpers/execution.pytests/helpers/maturin.pytypos.local.tomltypos.toml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/shared-actions(auto-detected)leynos/pylint-pypy-shim(auto-detected)leynos/whitaker(auto-detected)
Link the Unreleased changelog entry to draft PR #259 and close the living ExecPlan with final validation, review, branch, and publication evidence.
7743a0b to
9a4851f
Compare
Link the Unreleased changelog entry to draft PR #259 and close the living ExecPlan with final validation, review, branch, and publication evidence.
9a4851f to
a71b7d5
Compare
|
@coderabbitai Please investigate the cause of the following issue using codegraph exploration and research, identify a fix and provide an AI coding agent prompt for the fix: |
This comment was marked as resolved.
This comment was marked as resolved.
|
@coderabbitai review |
✅ Action performedReview finished.
|
da02b0f to
b852fee
Compare
Link the Unreleased changelog entry to draft PR #259 and close the living ExecPlan with final validation, review, branch, and publication evidence.
b852fee to
eee86ab
Compare
`test_adapter_fail_fast` and `test_tracing_span_concurrency` used the -ise forms in module docstrings, method docstrings, and comments, which the en-GB-oxendict house rule spells -ize; `test_pipeline_wait_state_machine` had the same in "initialises". `test_an_unrecognised_phase_still_raises` is renamed with them. The rule that preserves existing -ise identifiers exists to protect names other code depends on, and this one has no consumers at all: it is a test method, named nowhere but its own definition. Leaving it would plant a fresh counterexample just as PR #259 lands the identifier policy.
… telemetry (#73, #285) (#243) * Extract pure completion transition; state-machine test it (#73) Pipeline completion ordering lived inline in _process_completed_task, which mutated _PipelineWaitState, read the clock, and terminated stages in one async method — untestable at the state-transition level. Extract the pure decision into _PipelineWaitState.record_completion( completed_idx, exit_code, *, ended_at): it stamps the exit code and end time (clock injected), latches the first non-zero exit in completion order as failure_index, and returns whether the remaining downstream stages must be terminated. _process_completed_task now calls it and keeps the clock read and the async termination side effect. Add cuprum/unittests/test_pipeline_wait.py: a Hypothesis RuleBasedStateMachine drives random completion orders (restarting fresh pipelines when drained) and asserts first-failure semantics, timing-slot population, and the termination decision; example tests pin the boundaries (first-completed failure wins, final-stage failure requests no termination, all-success records no failure, fail-fast fires once). Regenerate the maturin wheel-manifest snapshot for the new test file. Closes #73 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Segregate the completion command from the fail-fast query Review findings on the completion transition: - record_completion both mutated the wait state and returned the termination decision, so a query was inseparable from a command, against the command/query segregation rule in AGENTS.md. Split it: record_completion is now a command returning None, and the new should_terminate_others(completed_idx) query reports the fail-fast decision from state without changing it, so it is repeatable and order-independent. _process_completed_task calls the command, then the query. - AGENTS.md requires function documentation to demonstrate usage and outcome; both methods gained worked examples. - The docstring described terminating the "remaining downstream stages". _terminate_pipeline_remaining_stages stops every still-running stage except the failed one, upstream included, so the wording now says "every other still-running stage". - The suite only covered the pure transition, so it could not catch inverted or omitted termination, or a mis-forwarded clock or index. Add TestProcessCompletedTask, which drives the real async boundary with a recording termination double and an injected perf_counter. Verified non-vacuous: omitting the termination call and forwarding the wrong clock each fail these tests. - Every bare assertion in the suite gained a diagnostic message. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Document the pipeline completion-ordering seam The pre-merge Developer Documentation check flagged that the new record_completion / should_terminate_others seam was described only in docstrings, with the design doc covering fail-fast behaviour generally. Add a "Completion ordering seam" subsection to the fail-fast policy in the design doc: what each half of the command/query split does, that the clock is injected so the transition is deterministic, that completion order rather than stage order decides which failure latches, and that _process_completed_task is the sole caller joining the two. Also correct the adjacent policy wording, which said fail-fast terminates "the remaining stages": it terminates every other still-running stage, upstream included. That is the same inaccuracy already corrected in the docstrings on this branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Verify the completion transition with CrossHair; observe fail-fast Completes issue #73's CrossHair criterion and the two unresolved warnings on this PR, without weakening the command/query split. CrossHair (test_pipeline_wait_crosshair.py): PEP 316 contracts confirm, over a bounded symbolic model, that a completion writes only its own slot, that the first non-zero completion latches failure_index and a later one never replaces it, that should_terminate_others holds exactly for a non-final first failure (covering final-stage and single-stage pipelines), and that repeating the query changes nothing. The model caps the pipeline at three stages, exit codes at -2..2, and timestamps at 0.0..4.0, and builds the state directly, so no asyncio task, subprocess, or clock enters the symbolic space. Verified genuine rather than assumed: dropping the final-stage exclusion and letting a later failure re-latch each yield POST_FAIL instead of CONFIRMED. The import-time probe follows test_line_splitting.py and degrades to a skip only for a missing dependency (ImportError) or a tracer that cannot handle the interpreter (TraceException); every other failure is re-raised, so a supported interpreter runs the verification rather than warning past it. That probe logic existed already, so per the AGENTS.md abstraction policy it moves to the shared cuprum/unittests/_crosshair_support.py that both modules import, instead of being duplicated. test_line_splitting.py's eleven harness tests cover it unchanged. Observability: _process_completed_task now emits two structured records through logging.getLogger(__name__) — pipeline_stage_first_failure when a completion newly latches, and pipeline_fail_fast_termination immediately before termination is awaited — sharing cuprum_stage_index, cuprum_exit_code, and cuprum_duration_s, and distinguished by a stable cuprum_action. Neither fires for a success, a later failure, or a final-stage or single-stage failure. Logging stays in the async caller; moving it into the command or query would break the determinism the symbolic verification depends on. Five log-capture tests pin those cases, and are non-vacuous: logging unconditionally, or dropping the termination record, each fail them. Documents the seam, the record fields, and both verification commands in the developers guide, with a matching note in the design doc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Group completion pairs to clear the argument-count finding CodeScene flagged Excess Number of Function Arguments on both _latches_first_failure and _first_failure_latch_contract, each taking five scalars. Replace the parallel index/exit-code arguments with one (stage_index, exit_code) tuple per completion, so both take three: stage_count plus the two completion pairs, in completion order. Grouping per completion rather than per kind keeps the two values that describe a single event together, which is what the arguments already meant. The helper unpacks both tuples immediately, leaving the state construction, the ordered record_completion calls with their 1.0 and 2.0 timestamps, and the latch assertions untouched. The symbolic domain is preserved exactly: the contract's preconditions now bound each tuple element individually — two distinct valid stage indexes and two exit codes in -2..2 — rather than the former scalars. Plain typed tuples keep CrossHair's inputs primitive and bounded; no dataclass, NamedTuple, or runtime construction is introduced. Verified the refactor did not weaken the exploration rather than assuming it: letting a later failure re-latch, and latching a hard-coded index, each still yield POST_FAIL instead of CONFIRMED. CodeScene health for the file goes from 9.68 to 10.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Document the fail-fast records in the users guide The two records this PR adds are emitted at WARNING on a default logging configuration, so users see them whether or not they went looking. They were documented only in the developers guide, which is the wrong audience for output that arrives unbidden. Add a fail-fast diagnosis section under pipeline execution covering both records, their shared fields, and the two behaviours that otherwise read as bugs: only the first failure is reported, because fail-fast makes the remaining stages fail too and reporting each would bury the cause; and a final-stage failure emits no termination record because nothing was left running. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Say which stages fail-fast terminates The users guide said fail-fast terminates "the remaining stages", which reads as the stages after the failure. It terminates every *other* still-running stage: `_stages_to_terminate` selects every index that is neither the failed stage nor already settled, so an upstream producer is stopped just as a downstream consumer is. `should_terminate_others` and the design document already say this explicitly; the guide was the one place left implying downstream-only, which matters because a user reasoning about a slow upstream producer would draw the wrong conclusion. Correct the pre-existing bullet as well as the new table row, so the two statements a few lines apart cannot disagree. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Describe the completion seam in the module docstring "Pipeline waiting logic with fail-fast semantics" named the subject but not the structure, so a reader had to reconstruct the command/query split from the code and could not tell which neighbouring module owns what. State the seam: the command latches the first failure in completion order, the query reports whether to stop the others, and `_process_completed_task` is the only place the two are joined. Name what deliberately lives elsewhere — process termination and cleanup in `_process_lifecycle`, pipe-task collection in `_pipeline_streams` — so the boundary is explicit rather than inferred from the imports. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Split the pipeline-wait tests by concern `test_pipeline_wait.py` had reached 539 lines, past the 400-line cap, and mixed four distinct concerns: a Hypothesis state machine, pinned boundary cases, the async wiring, and the structured records. Split it along those lines into four modules, each named for what it covers and none above 191 lines. The state machine and the examples now say in their docstrings how they relate — one generalises what the other pins — which the single file left implicit. Extract the shared scaffolding into `_pipeline_wait_support.py`. The two `fake_terminate` doubles were near-identical but had drifted: one recorded `(index, cancel_grace)` pairs, the other only indices. `record_terminations` keeps the richer form, so a test that needs the grace period no longer has to reintroduce its own copy to get it. Update the developers guide, which pointed at the single file. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Make simultaneous completions deterministic, and pin the clamp Four review findings on the pipeline-wait observability work. `_wait_for_pipeline` iterated the set `asyncio.wait` returns, so stages completing in the same batch had no completion order left to observe and `failure_index` fell out of set iteration — the module docstring's "first non-zero exit in completion order" was a claim the code could not keep. Sort each batch by stage index. That is both deterministic and the more useful answer: in a pipeline the upstream failure is what caused the downstream ones it triggered, so it is the one worth naming. Add the two tests that pin it, driving real settled stages through `_wait_for_pipeline`. Recording the processing order rather than only the outcome makes the check deterministic instead of relying on set order happening to differ from stage order. Mutation-verified: reverting to the bare set fails the ordering test on every run. Add the duration-clamp case. Every existing observability test starts a stage at zero and reads a later clock, so all of them hold whether or not `max(0.0, ...)` is there; this one inverts the pair and would publish -87.5 seconds without it. Mutation-verified. Add a syrupy snapshot over the whole record shape — logger, level, rendered message, and every `cuprum_` field. The field assertions each cover the part their test is about; the snapshot catches a change to any part no test happened to assert on. Derive the log fields only when a record will carry them. Every stage passes through this function and most emit nothing, so a success no longer allocates fields it discards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Pin completion order at the async boundary, not just in the transition Every case in `TestProcessCompletedTask` drove a single completion, so none of them could tell completion order from stage order: a boundary that latched the lowest-indexed failure instead of the first one to complete passed all three. The Hypothesis state machine does catch that mutant, but only against the pure transition — it says nothing about the index `_process_completed_task` actually passes to `record_completion`. Add a two-completion case where stage 2 fails before stage 0, and a `_run_completions` driver to sequence it. The clock advances per completion rather than staying pinned, so each stage's `ended_at` is distinguishable and a boundary that reused an earlier reading is visible. This is the counterpart to `TestSimultaneousCompletions`: stage order is the tie-break *within* one `asyncio.wait` batch, where completion order cannot be observed, and completion order decides across batches. Neither test alone pins that boundary. Mutation-verified three ways — latching the lowest index, terminating on every non-final failure, and reusing an earlier end time each fail it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Say which stage wins when a batch settles together The guide and the design record described completion order as the sole rule for latching `failure_index`, but `_wait_for_pipeline` also sorts each `asyncio.wait` batch by stage index. That sort was undocumented outside the module, so a reader could not tell whether it encoded a priority or merely made an unobservable order deterministic. Document it as the tie-break it is: real completion order decides across batches, and ascending stage index only orders stages *within* one batch, where `asyncio.wait` returns an unordered set and the alternative is a `failure_index` that varies run to run. Record why the tie is broken towards the earliest stage — an upstream failure causes the downstream ones it triggers — and name the test that would catch the sort's removal, since tests that fail stages at distinct times cannot. Align roadmap 2.1.2 with the same wording: it said "terminate downstream" where the implementation terminates every other still-running stage, upstream producers included. * Let a fail-fast record say which run it came from The fail-fast records carried a stage index, an exit code, and a duration. That describes a failure but does not locate it: two pipelines running concurrently in one process both report a stage 0, and nothing joined a record to the span or lifecycle events the observe hooks publish for that same stage. Carry the stage's existing `ExecId` as `cuprum_exec_id`, plus `cuprum_stage_count` so a record is self-describing. The token reaches the wait path as `_PipelineSpawnResult.stages`, a `_StageWaitContext` grouping the per-stage data the wait already needed, so `started_at` and the tokens travel together rather than as parallel arguments. `_PipelineWaitState` defaults it to empty and reports `None`, because the pure transition never reads it and the symbolic model must not carry it. Report the teardown as well as its start. `pipeline_fail_fast_termination` is emitted *before* termination is awaited, so on its own it cannot distinguish a teardown that finished from one still blocked on a stage that will not die. A new `pipeline_fail_fast_terminated` record closes that gap once termination returns, carrying how many stages were stopped — `_terminate_pipeline_remaining_stages` now returns that count rather than having the caller recompute the selection — and how long it took. Move the record payload into `cuprum/_pipeline_wait_records.py`: it is a published contract the users' guide documents, and keeping it beside the verified ordering decision invited editing one while meaning the other. The logger name is hard-coded there rather than following `__name__`, because operators are told to attach handlers to `cuprum._pipeline_wait`. No metric and no trace event. The library emits neither on its own — `MetricsHook` is an opt-in adapter over `ExecEvent`, and its phase match is fail-closed, so a fail-fast phase would make it raise for every user who has already registered it. Both would need a new observability surface rather than a use of an existing one; `cuprum_action` is a stable low-cardinality event name a log-based counter can key on meanwhile. * Tell the hooks when a pipeline gives up on itself A fail-fast teardown was, until now, reported only as log records on `cuprum._pipeline_wait`. That is fine for a human reading logs and useless for the metrics and tracing integrations, which would have had to parse log text to notice that a pipeline had been torn down early. Add a `pipeline_fail_fast` `ExecPhase`, emitted once per pipeline for the completion that *newly latches* `failure_index` and still leaves stages to stop — so not for a success, not for a failure that follows the latch, and not for a final-stage or single-stage failure, none of which trigger a teardown. It is published *before* termination is requested, so a consumer sees the decision even when the teardown then blocks on a stage that will not die. The event reuses the failing stage's existing `exec_id`. That is what lets a trace show the teardown on the span the stage's own `start` opened, and it is why `_StageWaitContext` now carries the stage observations alongside the tokens: publishing an `ExecEvent` needs the stage's program, argv, and tags, not merely its token. Both are derived from one observation tuple at the single construction site, so a log record and the matching event cannot disagree about which stage failed. `record_completion` and `should_terminate_others` stay a pure command and query; emission stays at the async boundary. `stage_index` and `stage_count` are typed fields rather than tags because caller-supplied tags are merged last and may legitimately shadow `pipeline_stage_index`; the decision has to report the index the coordinator acted on. The adapters render it three ways: `MetricsHook` increments `cuprum_pipeline_fail_fast_total`, labelled by `program` and `project` and nothing else — `exec_id` would make the series unbounded, and stage index and exit code would multiply it for no aggregate a dashboard needs; `TracingHook` adds a `cuprum.pipeline_fail_fast` span event to the failing stage's open span, starting and ending none; the logging adapter renders it at `LogLevels.fail_fast_level`, WARNING by default, rather than falling through to a generic DEBUG message that reports only the program. Adding a phase is a contract change, recorded as such in the changelog. `MetricsHook`'s match is fail-closed and hook failures are re-raised, so a hook that rejects unknown phases will raise until updated. In this repository that is one adapter, updated here; `TracingHook` and the structured logging hook are both fail-open. Third-party fail-closed hooks need an explicit arm, which is a versioning consideration rather than a reason not to emit. The `Span` and `Tracer` protocols move to `tracing_protocols.py`, re- exported unchanged. This is a precondition rather than a drive-by: the hook module was at the 400-line ceiling, and the contract reads better without the correlation machinery around it. * Give the silent-completion cases a name each The parametrized test took its stage count, completion sequence, and reason as three separate arguments, which CodeScene flags as excess arguments and which reads as a bare tuple at the call site. Wrap them in a `_SilentCase` record, matching the `_RecordCase` pattern the sibling observability module already uses for the same shape. Behaviour is unchanged; all three cases still run and still fail when the emission condition is widened to cover final-stage or single-stage failures. * Teach the merged oracles about the eighth phase The rebase onto main brought in the metrics reducer and the two adapter property suites from #247, which enumerate `ExecPhase` and the counter table independently of production so a wrong name cannot pass by construction. Those enumerations stopped at `exit` and so never saw `pipeline_fail_fast`, leaving the new arm and its counter unexercised by the very oracles written to catch a bad mapping. Widen both enumerations and give the logging level property its own `fail_fast_level` expectation, sampled rather than defaulted so the mapping is checked and not merely observed. Adding the phase to the level suite failed until the expected-level map gained an arm, which is the point of deriving it independently. Also reconcile the documented lists the two branches each half-updated: the metrics counter set, the tracing drop list, the configurable log levels, and the `cuprum_phase` values. * Say what fail-fast really terminates, and who projects it Three published descriptions of the fail-fast telemetry disagreed with the code they describe. The termination scope was written as "the surviving stages", which reads as the stages downstream of the failure. `should_terminate_others` stops *every other still-running stage*, upstream producers included, so all five prose sites now use that one phrase. The design document still claimed fail-fast emits neither a metric nor a trace event and that both would need a new observability surface. Section 8.4 of the same document has documented the opposite since the adapters landed: the wait path publishes records and one event, and `MetricsHook` and `TracingHook` project that event onto the counter and the failing stage's open span. The users' guide claimed the metrics adapter publishes `exec_id`. It does not — `cuprum_pipeline_fail_fast_total` is labelled by `program` and `project` alone — so a counter spike is joined to a stage through the matching log record or event, never through a label. Its tracing drop list also omitted `stdin_error`, which `TracingHook` drops like the rest. * Pin the exit race, and drop a suppression instead of moving it `Span` and `Tracer` each carried a `# pylint: disable=unnecessary-ellipsis`, which the project's suppression policy does not allow. The rule is enabled deliberately and it is the docstring-plus-ellipsis pair it objects to, so the suppression could not simply be narrowed. `MetricsCollector` — the sibling adapter protocol, next door — has always written its stubs as `raise NotImplementedError`. The tracing protocols now do the same, and the suppressions are gone rather than relocated. `_handle_exit` pops a span under the lifecycle lock and ends it outside, so there is a window where the span object exists, is not yet ended, and is no longer reachable through the active map. Two tests now park a `Span.end` to hold that window open and assert what must happen inside it: a `pipeline_fail_fast` for the same execution finds nothing and is dropped, as any uncorrelatable event is; and one for a *different* execution is recorded without waiting on the parked end. Both bite. Ending the span before detaching it records the fail-fast on a span the backend has already closed; holding the lock across `end()` stalls the unrelated fail-fast until the backend returns. * Give the clock its own seam, and the helpers their prefix `_pipeline_wait_records` exports three helpers into one private module, and all three read as public API. Their neighbours across the wait path — `_terminate_pipeline_remaining_stages`, `_collect_pipe_results`, `_stages_to_terminate` — and the dataclass in this very file are all prefixed. These now are too. Pinning the clock reached through `_pipeline_wait.time`, which is the stdlib `time` module object, so every test that pinned it replaced `time.perf_counter` for the whole process while it ran. The wait module now binds `perf_counter` itself, which gives the tests an attribute of their own to patch. The advancing variant that `test_pipeline_wait_async` had inline moves to the support module as `AdvancingClock`, so both clock helpers patch the one seam and a future change to it lands in one place. * Stop the tests grading their own homework `test_the_helper_counts_the_stages_it_stopped` computed its expectation from `_stages_to_terminate`, which is the very selection the helper it checks derives its targets from. Any change that moved both together passed. The counts are now stated per case, and a case where the failed stage's wait task is still running has been added — without it, sparing the failed stage and sparing an already-settled stage are indistinguishable. `test_first_nonzero_exit_latches_failure_index` claimed to pin a lower-indexed stage failing after a higher-indexed one, but had stage 2 *succeed* first, so a rule that latched the lowest failing index agreed with it throughout. Stage 2 now fails first, and stage 0's later, lower-indexed failure must not displace it. Two modules hand-rolled the create-task/settle/assign-index/call sequence that `apply_completions` already provides, and a third copy of the select-one-`cuprum_`-field comprehension sat in the correlation module. Both now come from the support module, so a change to the boundary signature is one edit rather than three. `drive_completions` documented a `completions` parameter it does not take, and the CrossHair module pointed at a `test_pipeline_wait.py` that does not exist. * Say what fail-fast really terminates, and what a raising hook does Two user-facing claims did not match the code they describe. The pipeline notes said a stage exiting non-zero terminates every other still-running stage. The coordinator is narrower than that: only the *first* failure latches, and `should_terminate_others` answers `True` only when that failure is also not the final stage. A failing final stage, and any single-stage pipeline, terminate nothing — there is nothing left running to stop — and later failures are usually consequences of the first rather than new causes. The design document's decision list carried the same unqualified claim, phrased as "the remaining stages"; it now uses the wording the other five prose sites settled on. The guide also said nothing about what happens when an observe hook raises, which matters now that `pipeline_fail_fast` is a new phase a fail-closed hook will reject. Cuprum does not swallow the failure: it logs, then re-raises the hook's own exception type out of `run()` / `run_sync()`. The two hook kinds differ only in when. A synchronous hook raises inline, so emission of that event stops and later hooks do not see it; an awaitable hook raises inside its scheduled task, so every hook still receives the event and the failure surfaces at the drain before the run returns. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Stop the tests restating what the helpers already say Four review points, none of which change behaviour: - `_UNIT_COUNTER_PHASES` was a plain list beside a `MappingProxyType` sibling. Make it a tuple so neither of the two views a test can take of the same pairs is mutable. - The fail-fast wiring tests ran three real subprocesses each, three times over, for one pipeline's worth of events. `scoped` is entered and left inside the helper and the tests only read what it returned, so a module-scoped fixture serves all three. It returns a tuple, so no test can hand the next one a shortened sequence. - Two assertions re-implemented the `vars(record)` comprehension that `field_values` and `record_actions` already provide. Use them. - Drop the `hook._active_spans == {}` assertion. It reaches into private state to restate what the two public assertions above it already pin: no event recorded, and the span ended. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Do not announce a teardown that has nothing to tear down `should_terminate_others` reasons from stage positions alone, so it answers `True` for any non-final latched failure — including one whose sibling stages have all already exited. That happens whenever a whole pipeline settles into a single `asyncio.wait` batch: the batch is handled in stage order, so an upstream failure can be reached after every other stage is done. The fail-fast event, both termination records, and therefore `cuprum_pipeline_fail_fast_total` all fired for that case, reporting a termination decision with no subject and a closing `cuprum_terminated_stage_count` of zero that contradicted the record which opened it. `_process_completed_task` now asks `_has_stages_to_terminate` — which wraps the same `_stages_to_terminate` reducer the teardown picks its targets with, so the announcement and the teardown cannot disagree — and withholds the event, both records, and the termination call when nothing is left running. `pipeline_stage_first_failure` still fires: the failure latched, and `failure_index` reports it either way; what is absent is the termination decision, not the failure. `record_completion` and `should_terminate_others` stay pure, and the event still precedes the termination request. Two supporting changes the above needed: - `_StageWaitContext` was frozen but held `started_at` as a list, which `_PipelineWaitState.from_processes` then aliased — the "immutable snapshot" and the live bookkeeping were one object. The field is now a tuple and the wait state copies it. - `apply_completions` gave the state a single wait task, so every sibling read as settled. It now builds one task per stage and swaps in a settled one per completion, as `_wait_for_pipeline` does, and takes a `before_each` hook so the clock-advancing driver in `test_pipeline_wait_async` can use it instead of duplicating it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> The docs record the new case alongside three wording fixes from the same review round: the design doc's error-propagation policy is no longer hedged as provisional, `failure_index` is described as the stage that failed first rather than the one that triggered fail-fast, and the determinism claim is scoped to stages settling in one wait batch. * Say which failures have nothing left to stop The developers' guide listed the completions that emit no termination record as "a final-stage or single-stage failure". That set grew when `_process_completed_task` started checking whether any sibling stage was still running: a batch is handled in stage order, so an upstream failure can be reached after every other stage has exited, and that case is now silent too. Name the real condition, point at the reducer that decides it, and say that the first-failure record is exempt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Leave the fail-fast path once there is no teardown Adding the live-stage check pushed `_process_completed_task` to a cyclomatic complexity of 9, on CodeScene's threshold, and dropped the module's code health to 9.68. The two trailing branches both asked whether termination was happening, so return once instead. Everything after that point is the teardown, and the latch is re-tested on its own rather than paired with `terminate_others` in a compound condition. Same behaviour, two fewer decision points, code health back to 10.00. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Say what the lifecycle module owns, once `_process_lifecycle` carried a one-line docstring over 397 lines that spawn processes, tear them down on two separate routes, and shield that teardown from caller cancellation. None of that was stated where a reader arrives. Give the module a docstring covering spawn and cleanup-on-error, the fail-fast selection trio, the timed-out teardown, the shielding rationale, and the division of labour with `_pipeline_wait`, which decides while this module executes. The shielding rationale was previously repeated verbatim in three helper docstrings. Now that the module owns it, condense those three to the detail local to each, keeping the file within the 400-line ceiling. Also clarify in the users' guide that the fixed `WARNING` of the Table-1 records is not the configurable `fail_fast_level`, which belongs to the opt-in adapter, and note in the security note that `pipeline_fail_fast` inherits the same `cuprum_argv` projection as every other phase. * Make _BatchRun immutable in fact, not just in name `_BatchRun` is `frozen=True`, which stops a field being rebound but not the four lists behind those fields being appended to. A holder of the record could still change what the run reported. Hold tuples instead, snapshotting the collectors at construction, once the run that fills them is over. `field_values` and `record_actions` take a `Sequence` so they still accept both the tuples and the raw `caplog.records` list. * Say when the fail-fast event stays silent The release note and the normative event contract both described `pipeline_fail_fast` as firing whenever a non-final stage is the first to fail. That is only half the rule: the emission is gated on there being another stage still running. A non-final failure processed after every sibling has already settled still latches `failure_index`, but emits no event and no termination records, because there is nothing left to stop. Both documents now state that condition, so a reader building a metrics or tracing integration against them is not surprised by a run that fails fast-ish and publishes nothing. * Hold teardown against every cancellation `_await_teardown_shielded` shielded the first await and then re-awaited the teardown bare. A shield protects only the await it wraps, so the retry was unprotected: a second cancellation aimed at the caller landed on the teardown itself, cancelling the SIGTERM/grace/SIGKILL escalation mid-flight. The `suppress` around it then swallowed the evidence, and the docstring's claim that teardown "completes regardless" was false — a SIGTERM-immune stage could outlive the run that spawned it. The wait now loops on a fresh `asyncio.shield` until the teardown future is actually done, recording each cancellation and re-raising the first, so the caller still sees exactly one. `cuprum.sh._execute_with_hooks` keeps its single shield: it drains bookkeeping tasks rather than owning process lifetimes, so a second cancellation there orphans nothing. The new case in `test_pipeline_teardown_cancellation` cancels the waiter twice inside an event-coordinated grace window and asserts the stage was still reaped; it also asserts the escalation had not yet run when the second cancellation landed, so it cannot pass by racing the clock. Restoring the unshielded retry fails it. The cancellation cases move out of `test_pipeline_timeouts` into that new module: adding them there would have pushed the file past the 400-line cap, and "what the caller sees when a deadline expires" and "what survives when the caller cancels mid-teardown" are two concerns anyway. * Spell test prose the way the house spells it `test_adapter_fail_fast` and `test_tracing_span_concurrency` used the -ise forms in module docstrings, method docstrings, and comments, which the en-GB-oxendict house rule spells -ize; `test_pipeline_wait_state_machine` had the same in "initialises". `test_an_unrecognised_phase_still_raises` is renamed with them. The rule that preserves existing -ise identifiers exists to protect names other code depends on, and this one has no consumers at all: it is a test method, named nowhere but its own definition. Leaving it would plant a fresh counterexample just as PR #259 lands the identifier policy. * Keep downstream stages alive past the failing one The wiring test's three stages could all settle before the waiter was resumed. `asyncio.wait(..., FIRST_COMPLETED)` returns every task that finished, not just the first, so one batch could hold all three: stage 0 is then processed after its siblings, the settled-batch gate this PR added finds nothing left to terminate, and the run emits no `pipeline_fail_fast` event. The fixture's `assert len(fail_fast) == 1` would fail, on a run that behaved exactly as specified. The downstream stages now sleep briefly once their stdin closes, which puts them in a later batch than the failing stage. The fixture docstring records why, so the delay is not read as an arbitrary sleep and removed. It is module-scoped, so it is paid once. * Give the remaining frozen test cases tuple fields `_Driven`, `_SilentCase`, and `_RecordCase` are `frozen=True`, which blocks rebinding a field but not mutating a list held in one. `_BatchRun` was moved to tuples earlier in this PR for that reason; these three were left behind. Their list fields become tuples, `_drive` snapshots both collectors as it builds its result, and the assertions compare against `()` and a tuple of actions. The shared `CompletionPlan` still takes a list, so the two call sites that feed it convert explicitly rather than widening a helper the rest of the suite shares. * Time the cancellations against the signal, not the clock Two teardown cancellation cases waited a fixed 0.02s against a 0.2s grace window before cancelling. A slow scheduler could deliver the cancellation before the teardown task took its first turn, failing for a reason unrelated to the shield. Both now wait on the process double's `signalled` event, as the second-cancellation case already did, so the cancellation lands inside the grace window by construction. Bound the readiness-marker poll in the end-to-end case with a deadline and a `pytest.fail`, mirroring `wait_for_process_death`, so a child that never starts fails the test instead of hanging the session. Narrow the suppression around the cancelled run from `BaseException` to the two outcomes the await can actually produce, so `KeyboardInterrupt` and unrelated failures still propagate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Pull the stage tag apart from the stage index Every fail-fast test kept `tags["pipeline_stage_index"]` and the typed `ExecEvent.stage_index` field in lock-step, so an implementation that read the tag instead of the field would have passed the whole suite. The two can legitimately disagree: `_build_pipeline_observations` merges `ExecutionContext.tags` last, so a caller may shadow the coordinator's own index — which is precisely why the typed fields exist. Add `test_pipeline_fail_fast_tag_shadowing.py`, which sets the shadowing tags to an impossible 99 and pins both channels at both levels: - End to end, through `run_sync` with a real three-stage pipeline, so the merge in `_build_pipeline_observations` is the one under test. - At the emission seam, driving one chosen completion (stage 1 of 4) through `_process_completed_task`, which also pins the `cuprum_stage_index` log field against the same regression. Each level also asserts the caller's tag arrives unaltered, so neither can pass on a run where the shadowing tags never took effect. Verified by mutation: making `_emit_fail_fast_event` read `observation.tags["pipeline_stage_index"]` fails both new assertions with (99, 99) while the rest of the suite stays green. The real-pipeline runner moves to `_fail_fast_pipeline_support.py` so the wiring module and this one share one definition of the failing pipeline, and `make_stage_observations` gains a `tag_overrides` argument that is merged last for the same reason the production builder merges the caller's tags last. Refs #73, #285. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Route fail-fast telemetry through adapters (#73) Publish one typed fail-fast event through observe hooks and let the logging adapter render its warning. Omit raw arguments and caller tags from that warning, while preserving metric and tracing projections. Replace timing-based test coordination with FIFO event gates, ensure cancellation tests reap reported PIDs on every path, and update the completion transition, documentation, and snapshots to match the boundary. * Sanitize pipeline fail-fast telemetry (#73) Restore the canonical fail-fast WARNING records while emitting an observe event stripped of argv, environment, working directory, and caller tags. Read the failing-stage UUID directly from its observation, and cover the sanitized event, negative-duration boundary, and documented correlation. * Fix post-rebase pipeline wait integration (#73) Preserve the lost-wakeup fallback while passing immutable stage context through the updated wait API. Align the legacy and synthetic test doubles, stabilise timeout teardown startup, and regenerate the wheel manifest. * Align pipeline docs with rebase lint rules (#73) Preserve the rebased fail-fast and cancellation behaviour while applying the formatter and NumPy return documentation required by the new main branch lint policy. * Split adapter projection property assertions Extract the independent logging, tracing, and metrics assertion groups so the Hypothesis property remains a concise coordinator without weakening its projection contracts. * Harden pipeline fail-fast observability (#73) Route pipeline-wait warnings through the structured logging adapter, keep the sanitised event's trusted project label, and serialise same-stage trace callbacks against exit. Cover record ordering, process-exit polling isolation, and the fail-fast tracing race while updating the user and design contracts. * Split pipeline-wait observability tests (#73) Move the record-ordering contract out of the async-boundary suite so each pipeline-wait test module stays below the repository's size limit. Document the complete split suite and regenerate the wheel-manifest snapshot for the new test module. * Report confirmed pipeline fail-fast terminations (#73) Count one verified outcome for every selected teardown target, excluding stages that settle after selection before they can be terminated. Cover the race directly, split batch completion coverage into its own module, and clarify the completion-record and event channels. * Group pipeline completion report fields (#73) Carry each record's action, message, and optional fields in one immutable report value so the adapter boundary no longer accepts an excessive argument surface. Keep the published fields unchanged and assert that teardown outcome fields remain exclusive to the termination-outcome record. * Align replayed pipeline expectations Match target-era pipeline tests to the current wait context and final-stage fail-fast policy. Restore formatter-required spacing and remove the replayed documentation blank line. * Unify tracing protocol boundary (#73) Keep legacy tracing imports as compatibility re-exports while making the public module the sole protocol definition. Document the separate direct pipeline-wait record contract and extract the verified-teardown race setup. * Refine pipeline fail-fast review contracts (#73) Preserve the direct pipeline-wait records while adding their execution correlation token to the structured fail-fast log projection. Clarify the associated telemetry and protocol boundaries, and improve failure diagnostics without changing teardown semantics. * Inject pipeline event clocks (#285) Make lifecycle and fail-fast event timestamps deterministic at the observation boundary, and avoid rebuilding logging level maps for every event. Keep the reporter and structured-event channels distinct in the user documentation. * Repair rebase merge artefacts * Repair rebase documentation and lint gates Restore target-branch timeout observability documentation that was lost during the rebase, while retaining the pipeline fail-fast additions. Keep `TracingHook` below the module-size limit without weakening its documented attribute contract, and resolve the remaining Markdown and spelling gate findings. * Close retried HTTP errors Close handled `HTTPError` instances before retrying or propagating them, preventing deferred resource-cleanup warnings in CI. Make CrossHair fallback tests select their own warning so unrelated warnings cannot change their assertions. --------- Co-authored-by: leynos <leynos@rohga> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rename private, test-local, and function-local `-ise` identifiers to their Oxford `-ize` forms without changing the public API. Keep the CrossHair contract and diagnostic references aligned with the renamed helpers. Add the living execution plan that governs source-corpus cleanup, spelling gate expansion, policy documentation, validation, and publication.
Extend the spelling gate to tracked Python and Rust source and lock the Makefile pathspec with a regression test. Normalize the existing source corpus so the widened gate starts green. Keep GitHub Actions wire vocabulary, the established CLI option, and deliberate spelling-test fixtures through narrowly anchored local patterns. Regenerate the tracked spelling configuration from that overlay.
Make identifiers, comments, docstrings, fixtures, and prose explicit parts of the en-GB-oxendict policy while retaining narrowly documented external contract exceptions. Record the decision in ADR-008, index it for maintainers, and note the new contributor-facing source gate in the Unreleased changelog.
Link the Unreleased changelog entry to draft PR #259 and close the living ExecPlan with final validation, review, branch, and publication evidence.
Align the documented exception scope with ADR-008 and add actionable failure context to the spelling-policy regression assertions.
Exercise the configured spelling scanner across Markdown, Python, and Rust fixtures, including the documented external-contract exceptions. Snapshot the benchmark baseline CLI help and token error while retaining semantic assertions for the stable option and Oxford wording.
Exercise the real Make spelling target, generate configuration before all consumers, and keep Markdown code ignores scoped to Markdown. Tighten local contract exceptions and cover their accepted and near-miss forms. Document ADR-008 in the design, complete the affected public docstrings, and preserve `main`'s maturin test refactor while applying the source spelling policy.
Update newly added upstream prose to satisfy the widened source spelling gate and keep the Maturin helper reference aligned with its renamed symbol.
ADR-008 is already claimed by the Rust pump observation channel decision in an older, further-advanced pull request. Take the next free number so the two can land in either order without a filename collision. Updates the contents index, the developers' guide, the design document, and the ExecPlan alongside the file rename.
The stored Syrupy transcript pinned argparse's line wrapping, not the CLI's contract. CPython 3.12 wraps `--artifact-name ARTEFACT_NAME` across two usage lines where 3.13 and 3.14 keep it whole, so the snapshot failed the 3.12 job while every other version passed. Replace it with assertions on what the interface actually promises: the help exit code, the description, every option name and its help sentence, the optional-argument defaults, rejection of each required option when omitted, and the exact missing-token message. Whitespace is collapsed before matching so rewrapping cannot break the assertions again. Move the command-line surface tests into their own module, with the shared argument builder in a support module, keeping both files inside the 400-line limit.
`test_cli_help_documents_every_option` now asserts that the usage text identifies `fetch_main_benchmark_baseline.py`. Patching `sys.argv` no longer achieves that: Python 3.14 derives argparse's default prog from the actual invocation, so under `python -m pytest` the usage line read `python3 -m pytest`. The parser now pins `prog` explicitly — the tool's name is part of its contract in a way the wrapping never was — and the redundant argv patch and its `sys` import are gone. Verified 9/9 on 3.12, 3.13, and 3.14. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Address the review findings on the spelling-policy branch. `benchmarks/fetch_main_benchmark_baseline.py` derived its argparse description from `__doc__`, so any documentation the module needed would have landed in `--help`. Give the parser its own short description literal, then expand the module docstring with the script's purpose, the `benchmark-ratchet` CI boundary that is its only caller, the meaning of exit code 3, and a usage example. `--help` is unchanged. `render_typos_config` is public but carried a one-line docstring. Give it Parameters, Returns and Raises sections, and state the partition it performs: patterns in `_MARKDOWN_IGNORE_PATTERNS` describe Markdown syntax and are emitted under `[type.markdown]` so they exempt only `*.md`, while every other pattern stays global. The inline-code exemption restored in #294 had no end-to-end cover. Add two cases to `test_spelling_gate_preserves_documented_exceptions` that run the real pinned scanner over `.md` fixtures: an `-ise` term in an inline code span, and one in a fenced block. Both must pass the gate, while plain `.md` prose stays gated by the existing detection test. Give the remaining bare assertions in both touched test modules the diagnostic messages the test-path rule requires. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Retain target-side benchmark and pipeline refactors while removing duplicate rebase fragments. Keep GitHub's public `artifacts` protocol values intact, but apply the repository spelling policy to private identifiers and prose. Update the CLI parser destination and wheel snapshot so the split benchmark tests continue to exercise the current command-line contract.
Apply the source spelling policy to observability prose introduced by the rebased target branch, so the expanded spelling gate remains green.
Keep the rebased unreleased entries distinct from the historical release so Markdown lint recognizes the intended list and heading boundaries.
Restore the target branch's pipeline timing support, GitHub redirect handler, and line-splitting contract after the rebase duplicated their definitions. Keep the branch's private `-ize` spelling policy corrections and apply it to two newly introduced test docstrings.
64378ca to
a658e4c
Compare
Summary
This branch applies the repository en-GB-oxendict policy to code identifiers
and source prose, closing the gap that allowed Python
-iseidentifiers todrift without a failing gate. It normalizes the source corpus, widens the
pinned spelling check to tracked Markdown, Python and Rust files, and records
the policy and exception process in ADR-009.
Closes #249
ExecPlan: enforce-ize-identifiers.md
Review walkthrough
Makefileandscripts/tests/test_typos_rollout.pyforconfiguration ordering, the widened pathspec, and the real-target
integration test.
typos.local.tomlfor bounded external-contract exceptions andtypos.tomlfor the regenerated Markdown code-span policy.AGENTS.md, the developers guide, ADR-009, and the design documentfor the governing rule and its architectural reference.
corrections. GitHub
artifactswire keys,/artifactsURLs, and theestablished
--artifact-nameoption remain unchanged.Validation
make check-fmt: passed.make test: passed (1,147 Python tests, 54 skipped; 104 Rust tests).make typecheck: passed.make lint: passed, including the Python, Rust, and spelling gates.make markdownlint: passed.make nixie: passed.git diff --check: passed.Adoption update
Rebased onto
origin/main, preserving target-side refactors while resolvingall conflicts. The CLI help contract now uses semantic assertions rather than
CPython-version-sensitive wrapping snapshots. The post-rebase repair restores
the target HTTP split, corrects private spelling identifiers, and reconciles
the native-wheel file-list snapshot.
Summary by Sourcery
Extend automated en-GB-oxendict enforcement to source code and identifiers,
normalize existing spelling drift, and document the governing policy and
exception process.
Enhancements:
Python, and Rust source, including identifiers, comments, docstrings,
fixtures, and prose.
documented external GitHub API and CLI spellings.
ADR-009 and contributor guidance.
Build:
scanning all tracked Markdown, Python, and Rust files.
Documentation:
design document, developers guide, and changelog.
Tests:
path coverage, rejected spellings, and documented exceptions.
related native-wheel snapshot updates.
References