feat(config): consolidate all time knobs into settings.timeouts; make --timeout a real run watchdog - #409
feat(config): consolidate all time knobs into settings.timeouts; make --timeout a real run watchdog#409viraatc wants to merge 45 commits into
Conversation
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
There was a problem hiding this comment.
Code Review
This pull request refactors the configuration schema by centralizing all global durations, deadlines, and timeouts into a new frozen Pydantic model Timeouts (accessible via settings.timeouts). This separates workload durations from failure-handling deadlines. Additionally, a whole-run watchdog (run_timeout_s) has been introduced to gracefully abort stuck runs, signaling managed subprocesses via SIGTERM to write an interrupted final snapshot before exiting non-zero. All configuration templates, examples, and tests have been updated to align with this new schema, and new integration tests have been added to verify the watchdog behavior. No review comments were provided, so there is no feedback to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Findings from the review council (Codex review + adversary council), all verified against the code before fixing: - Watchdog now stays armed through the unbounded metrics drain: it was cancelled right after session.run, so run_timeout_s could not bound a stuck aggregator drain (wait_for_exit(None)). Cancelled after services exit instead. - Watchdog SIGTERMs only the metrics aggregator (ServiceLauncher.terminate with module suffix, replacing terminate_all): SIGTERMing the event logger dropped its buffered events.jsonl tail; the logger flushes on the ENDED event, which session.stop() still delivers. - A timed-out run skips accuracy scoring in finalize: phases that never started KeyError in scorer init and partial phases would yield misleading subset scores. Artifacts are still salvaged. - Teardown race no longer skips finalization: if session.run raises after the watchdog fired, fall through with an empty SessionResult so result_summary.json (INTERRUPTED, complete=false) is always written; run_benchmark raises the timeout ExecutionError after finalize. - run_audit maps a watchdog fire to ExecutionError naming the timeout instead of the Ctrl-C KeyboardInterrupt path (exit 130). - MetricsConfig gets cyclopts.Parameter(name='*') matching sibling settings blocks (flat --tokenizer-workers + --metrics-tokenizer-workers). - Stale drain-key name fixed in session.py docstring. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
arekay-nv
left a comment
There was a problem hiding this comment.
I think the schema breakdown make sense and is a lot cleaner.
Regarding the timeouts - two suggestions, and feedback is welcome:
- Remove the duration field - makes it simpler especially since we are mostly going to be doing concurrency based runs.
- Modularize the phases with an explicit type of phases and dependencies, but move the per-phase timeouts/drains etc there.
So a global timeout for everything - and a per-phase config for controlling how a phase behaves. We can have some explicit dependencies such aswarmupalways goes beforeperformance,reportingcomes afteraccuracyetc.
…s; make --timeout a real run watchdog Reworked from PR #409 review feedback, rebuilt on latest main: - New frozen Timeouts model at settings.timeouts holds every give-up deadline: run_timeout_s (--timeout, whole-run watchdog), service-ready, per-phase drains (absorbs DrainConfig), metrics drain (0-sentinel killed; None = unlimited), and the worker lifecycle waits (moved off settings.client; carriers renamed *_s, excluded from dumps and CLI). - --timeout was consumed nowhere; it now aborts the run: session.stop() then SIGTERM the aggregator (INTERRUPTED final snapshot, first-wins), ExecutionError after finalization - a fired watchdog can never yield a COMPLETE result_summary.json. Deadline is captured before setup; the timer stays armed through the metrics drain. Timed-out runs skip accuracy scoring; audit phases map a fired watchdog to ExecutionError. - publish_final serialized with an asyncio.Lock: a SIGTERM racing the ENDED-driven finalize can no longer abandon a half-written snapshot. - runtime.min_duration_ms/--duration deleted: sample count is explicit (--num-samples) or the dataset issued once. max_duration_ms stays in runtime as the perf-phase workload cap (int|None, gt 0); reaching it is a normal end. MLPerf ruleset path (RuntimeSettings/UserConfig) keeps its internal duration fields. - ServiceLauncher.terminate(module): exact-match SIGTERM; MetricsPipeline.terminate_metrics_aggregator() is the narrow public face. - config/schema.py split into enums/audit/model_params/datasets/settings/ timeouts modules; schema.py keeps the root aggregate + re-export hub. SystemDefaults and TEMPLATE_TYPE_MAP deleted. - Examples, templates, and docs migrated; docs gain a YAML<->CLI time-knob table. Stale inert timeout: values dropped, warmup drain removed from examples. Breaking: bare configs (no --num-samples) now run the dataset once instead of deriving QPS x 10min samples; old YAML keys hard-error via extra=forbid.
…n spellings Real-world report: a run with a 2.2M-item tokenization backlog wedged in the unbounded metrics drain after ^C, and every further ^C was suppressed. Second distinct ^C now cancels the run task — the unwind kills the service children (the SIGTERMed aggregator writes a best-effort INTERRUPTED snapshot without finishing its drain), salvages tmpfs, exits 130. Re-deliveries within 1s remain one keystroke (uv run & co. forward the group SIGINT), so graceful ^C keeps working under wrappers. Cleanups: warmup/accuracy phase settings use the canonical min_duration_ms=None; the e2e concurrency perf test no longer passes the poisson-only min-duration flag; the standalone httpclient bench renames its unrelated --duration to --duration-s.
…euristic First ^C stops gracefully; every further SIGINT cancels the run task (teardown/metrics drain abandoned, service children killed, best-effort INTERRUPTED snapshot, exit 130). No timing state. Known caveat, documented: wrappers that forward the terminal's group SIGINT (uv run) deliver one keystroke twice, so a single ^C under them force-quits immediately.
Line-profiler stats are dumped by shutdown() called from the CLI run() finally, worker_main's finally, and pytest_sessionfinish. The plugin's stderr-suppression hook is gone too: with the profiler torn down before interpreter exit there are no shutdown errors to hide.
…review-round fixes Force-quit (second distinct ^C) now abandons everything without grace: the pipeline teardown SIGKILLs the service children on every unwind path (force_quit predicate), HTTP workers are SIGKILLed with no graceful wait, and the tmpfs salvage is skipped. First ^C keeps the graceful path: report for whatever is already computed, normal drain budget, exit 130. Runners that forward the terminal's group SIGINT (uv run) deliver a single ^C twice, the forwarded copy ~200ms later; deliveries within 1s of the last accepted one are dropped as duplicates so one keystroke never force-quits. Locked by integration tests: wedged-aggregator force quit, pre-session ^C, and a real uv-run one-keystroke run. The audit runner installs its own SigintGovernor around the phase loop and passes it to every phase, so a ^C during a long audit phase takes the graceful path instead of a raw KeyboardInterrupt that skips finalize; a ^C between phases refuses to start another phase. Review-round fixes: split-brain guard keys on report.state (catches the drain-timeout subcase of an aborted run); governor binds beside the watchdog at session creation; SIGINT-handler restore uses a sentinel (None is a restorable C-installed handler); whole-run deadline derivation shared via _run_deadline; run_benchmark fails loudly when no usable report exists; the metrics-drain failure message names the tokenizer-failure cause; signal frame typed FrameType | None; run_timeout_s description states the setup-boundary limitation; tokenizer-workers doc default is 4.
… the loop thread; profiler teardown unconditional Three correctness holes from review: 1. After run_until_complete returns, the SIGINT governor's session/task/loop stay bound but the loop is stopped - a first ^C during sync finalization (or audit inter-phase setup) queued session.stop on the dead loop and was silently swallowed. The graceful path now requires a live task on a running loop; otherwise KeyboardInterrupt is raised immediately (salvage still runs via run_benchmark's finally, exit 130). 2. ProfileController.start/stop ran sequential blocking urlopen calls (2s each) on the event-loop thread, so a queued force-quit task.cancel could not land until every POST finished. The POSTs now run in worker threads (asyncio.to_thread), awaited sequentially - identical ordering and records, but cancellation lands between POSTs. The session's on_phase_start hook is awaitable to carry this, and the perf cap is armed AFTER the awaited profile arming: _run_phase clears the phase-stop flag at entry, so a one-shot cap firing during the await would be erased and the phase would run uncapped. 3. Line-profiler shutdown() early-returned when print_stats() had already run, and a failing output destination skipped teardown - both left the C profiler enabled at exit. Teardown now runs unconditionally in a finally; _stats_printed only suppresses a duplicate dump. Regression tests: SigintGovernor state machine (no-live-run raise, duplicate-delivery drop, force cancel), hook awaited before issuing, cap-shorter-than-hook still bounds the phase, print_stats-then-shutdown and failing-destination teardown.
…ion matrix Test-quality pass over the PR's test surface: - SigintGovernor: replace overlapping named scenario tests with one exhaustive delivery-sequence matrix (every bind-state x distinct/dup sequence up to length 3, against a dedicated child task), keeping only the finished-loop cases the matrix cannot express. PerfPhaseTimeout tests move next to the governor and run against the real event loop (cap fires, accuracy start disarms, never-armed cases parametrized, idempotent cancel) instead of a fake-loop scheduling recorder. - test_sigint.py: launch/readiness/teardown/leftover-scan boilerplate extracted into helpers; four distinct scenarios stay. - test_run_timeout.py: shared _make_config builder + big-prompts dataset writer replace five hand-rolled config blocks. - test_schema.py: drop max_duration 0/negative rejection duplicates (covered parametrized in test_timeouts.py, same validator). - test_benchmark.py: audit-dispatch tests get a shared config-double helper and hoisted module imports; the two audit-FAIL tests collapse into one parametrized case; class renamed TestRunBenchmarkAuditDispatch. - test_profiler.py: file-level unit pytestmark (was invisible to -m unit), enabled_profiler fixture replaces repeated env/singleton setup, singleton restored after every test; drop the conditional no-op prefix test. - line_profiler._teardown_profiler tolerates an already-disabled profiler (line_profiler raises ValueError on double sys.monitoring release) so shutdown teardown is truly unconditional.
…dow divergence nv-alicheng's review flagged that a SIGINT reaching only the main process can leave final_snapshot.json COMPLETE while result_summary.json is corrected to INTERRUPTED. This is deliberate - the snapshot is the aggregator's own observation, the summary is run-level truth - but the contract was undocumented and the divergence window untested, and three docs still claimed a ^C never leaves COMPLETE artifacts. - CLI_QUICK_REFERENCE, AGENTS.md, snapshot.py SessionState docstring, and the test_sigint module contract now state the precedence: result_summary + exit code are authoritative; final_snapshot can legitimately read state=complete when the abort lands in the post-ENDED metrics-drain window (and the aggregator ignores SIGINT - the parent's ENDED path is authoritative for ^C, with SIGTERM/marker-event as the INTERRUPTED entries). - New integration test pins the window deterministically: session ends, tokenization backlog drains, ^C to the main process only -> exit 130, summary state=interrupted complete=false, snapshot state=complete, no orphaned children. - execute.py's phase-start hook extracted to _make_phase_start_hook and its profiler-before-perf-cap ordering asserted directly (a production reorder now fails the test); the timing-based session test that re-implemented the ordering locally is deleted.
A ^C landing during post-measurement accuracy scoring (run task done, governor raises KeyboardInterrupt mid-score_accuracy) exits 130, and finalize_benchmark's finally still writes the genuinely completed perf report as complete:true. That is the honest outcome - the measurement finished; only post-measurement scoring was aborted. Documented as the one exception to 'a ^C'd run's summary is never complete' in CLI_QUICK_REFERENCE, the finalize comment, and the sigint test module contract; pinned by a unit regression (score_accuracy raising KeyboardInterrupt -> summary complete:true, interrupt propagates). Shared complete-report builder extracted for the two finalize guards.
Single import style for the cli module (string-target monkeypatch, _run imported directly); explicit awaited cancellation via wait_for in the governor matrix teardown.
a71a187 to
bffa94d
Compare
…rdown grace Simplification pass modeled on aiperf's interrupt design: a ^C has one meaning. It stops the session gracefully and arms a 30s teardown grace; if the metrics drain has not finished when the grace expires, the service children are SIGTERMed (the aggregator's handler writes a best-effort INTERRUPTED snapshot) then SIGKILLed, so a wedged drain can never hang the abort - escalation is timeout-driven, not keystroke-driven. Repeat ^C is a logged no-op, which also makes group-SIGINT-forwarding runners (uv run) need no special handling. Deleted with the second-^C force-quit semantics: the duplicate-delivery window and forced state in SigintGovernor, the force_quit predicate and kill_now fast path in MetricsPipeline, the forced-quit exception branch in _run_benchmark_async, and http_client.kill_workers / worker_manager.kill_now (endpoint_client files are back to main verbatim; main's worker teardown is already bounded). Also deferred to a follow-up (files back to main verbatim): the line-profiler explicit-shutdown refactor and the threaded profile POSTs - with no force-cancel to unblock, the sync POSTs are fine again, and the session's on_phase_start hook returns to a plain sync callback (no await window, so the perf-cap arming order is no longer load-bearing). A ^C during post-measurement accuracy scoring now rewrites the report to interrupted/complete:false before it is persisted (an interrupted run is an invalid run; artifacts only expose the partial metrics) and the interrupt propagates for exit 130. Tests follow the model: governor suite covers graceful+grace arming, repeat no-op, grace expiry, disarm-on-drain, and the no-live-run raise; the wedged-drain integration test now needs only a single ^C (grace shrunk to 3s via the class constant); the uv one-keystroke and drain-window divergence tests are unchanged in spirit.
…GINT restore Two holes found by post-simplification review: 1. Grace expiry SIGKILLs a wedged aggregator, so the report is built from the subscriber's last LIVE pub/sub snapshot - and the split-brain guard only rewrote state=="complete", letting an aborted run persist result_summary.json with state:"live". The guard now rewrites any aborted non-interrupted state; the wedge integration test asserts the summary lands interrupted/complete:false and the unit guard test is parametrized over both snapshot states. 2. signal.signal(SIGINT, None) raises TypeError, so the sentinel-based restore was broken for exactly the case it existed for (a C-installed previous handler, getsignal()->None). Restoring SIG_DFL instead would destroy the host's handler, so the governor now probes getsignal() first and refuses to install over an unrepresentable C handler (stays passive, same stance as off-main-thread); the restore path only ever sees Python-representable handlers. The _SIGINT_NOT_INSTALLED sentinel, its object-typed local, and the type:ignore are gone (both run_benchmark and run_audit).
…edence, shard reaping Council findings (two parallel reviewers, addressed locally): Correctness: - finalize_benchmark writes the split-brain-rewritten report back onto BenchmarkResult, so the audit runner's post-phase state check sees the honest interrupted state instead of the aggregator's stale COMPLETE (a drain-window ^C during the final audit phase could previously certify a PASS). - run_benchmark refreshes user_interrupted from the governor after the loop returns: a ^C landing between the coroutine's flag snapshot and task completion can no longer produce COMPLETE artifacts on an exit-130 run. - A drain/report failure on an interrupted run no longer re-raises as ExecutionError (exit 4): the user's abort outranks a drain error its own grace escalation may have caused - exit stays 130. - The finalization KeyboardInterrupt handler now covers every post-measurement write (scoring, summary log, profiling.json, accuracy_results), rewriting and RE-persisting the summary as interrupted if it was already written COMPLETE. - Tokenizer shard workers arm PR_SET_PDEATHSIG=SIGKILL in their initializer: a SIGKILLed aggregator (watchdog / teardown-grace escalation) can no longer orphan the non-daemon ProcessPool shards that are outside every launcher PID list. Behavioral test reads PR_GET_PDEATHSIG back from a real pool worker. - SIGINT install/restore is one shared context manager (watchdog.sigint_policy) used by run_benchmark and run_audit: probes getsignal() first (an unrepresentable C handler stays untouched, governor passive), restores after the caller's finally so a repeat ^C during tmpfs salvage still hits the governor's no-op. Quality: _on_global_timeout renamed _on_perf_phase_timeout (it only caps the perf phase), Timeouts docstring no longer promises None for service_ready_timeout_s, sigint test helpers deduplicate the /proc scan and name timeout_s, grace-expiry unit test asserts exactly one fire, timeouts validation tests pin the failing field (extra=forbid would mask typos), dict annotations typed.
…able-code false positive)
|
|
||
| def test_restores_on_exception(self): | ||
| gov = SigintGovernor() | ||
| prev = signal.getsignal(signal.SIGINT) |
Cut the whole-finalize KeyboardInterrupt re-persist wrapper (guarded a millisecond window between artifact writes; the exit code is the documented run-outcome verdict) and the tokenizer-shard pdeathsig guard (pre-existing risk - main's terminate_all already SIGKILLs - and CF workers self-reap on queue EOF; follow-up material). The scoring-window invalidation, split-brain write-back, user_interrupted refresh, and exit-130 precedence stay: each enforces the documented contract in 1-3 lines.
…s a config knob Size pass: PerfPhaseTimeout returns to its original execute.py location (verbatim, as _PerfPhaseTimeout) and SigintGovernor/sigint_policy/ RunWatchdog live beside it - the separate watchdog.py module double- counted the moved code in the diff. Multi-line rationale comments compressed to their load-bearing core. The teardown grace is now settings.timeouts.teardown_grace_s (default 30; None = never abandon; 0 = abandon immediately) consumed by both the ^C governor and the run watchdog; the wedged-drain integration test shrinks it via YAML instead of a python -c constant override. The five per-drain CLI alias flags are dropped - the auto-generated dotted flags remain and the docs table now shows those spellings; --timeout stays. Non-test churn: 1694 -> 1514.
| ) from e | ||
| raise | ||
| finally: | ||
| watchdog.cancel() |
There was a problem hiding this comment.
[review-council: Claude+Codex+Grok] medium — accuracy scoring / finalize is not covered by run_timeout_s; a hung external scorer hangs an otherwise-clean run. RunWatchdog is cancelled here (watchdog.cancel()), then run_benchmark calls finalize_benchmark → score_accuracy → scorer.score() with no live deadline. For SKIP_ENDPOINT_PHASE scorers that delegates whole agent execution + grading to an external service (SWE-bench swebench_service_url), and VBench/LCB spawn uv run subprocesses — any can block indefinitely on a clean run, and only a manual ^C escapes. Acknowledged in CLI_QUICK_REFERENCE.md:166 ("not deadline-bounded") but contradicted by this field's own docstring (schema.py:838 "through every phase and drain") and CLI_QUICK_REFERENCE.md:188 ("the only total-wall-time bound"). Either bound scoring with a dedicated scorer timeout, or reconcile the three statements.
| Priority: | ||
| 1. If `n_samples_to_issue` is set, return it (explicit override) | ||
| 2. If min_duration_ms=0, return all dataset samples (new CLI default) | ||
| 2. If no duration target is set, return all dataset samples |
There was a problem hiding this comment.
nit: min_duration to make clear where it should be (duration now applies to
Every bare 'duration' the MR added is now explicit: the sizing input is min_duration_ms (poisson-only, target_qps x min_duration_ms, sizing not a timer), the runtime cap is max_duration_ms, and the give-up deadline is run_timeout_s. total_samples_to_issue's priority docstring names the actual rule per step; offline/concurrency runs are called out as purely count-driven (min_duration_ms with them is a config error).
The grace applies exactly to runs being marked INTERRUPTED (^C or the run watchdog); the name now says so. Config field, constructor params, docs table, and templates renamed together.
…_duration_ms docstring The interrupt machinery (SigintGovernor, sigint_policy, RunWatchdog) returns to commands/benchmark/watchdog.py - the module reviewers reviewed; the perf-phase cap (_PerfPhaseTimeout) stays at its original execute.py location. RuntimeSettings.min_duration_ms now documents the full contract: sizing input (target_qps x min_duration_ms), never a runtime timer, n_samples_to_issue wins, populated from settings.runtime.min_duration_ms or directly by rulesets. Reverts the gratuitous 'duration floor' -> 'min_duration_ms floor' renames in the compliance docs (the original text was already precise).
… public boundary run_benchmark_async gives callers without a governor (audit phases get one from run_audit; embedded/test callers) a passive SigintGovernor - never installed as a signal handler, interrupted stays False - so _run_benchmark_async requires it and the five scattered 'sigint is not None' guards collapse into plain attribute access. The adjacent report-persist guards in finalize merge into one block. Also restores the compliance-plan min_duration blockquote verbatim from main (the branch's condensed rewrite had dropped the 'merely derives a count' explanation; the file is now untouched by this MR).
…run_timeout_s is a hard bound arekay's review round: - HIGH: a ^C during service launch / endpoint connect (task bound, session not yet) previously raised raw KeyboardInterrupt out of the loop, skipping the pipeline __aexit__ and orphaning the aggregator/event-logger children. The governor now cancels the run task - exactly like the watchdog's pre-session fire - so the unwind kills the children; _run_benchmark_async maps the cancellation back to KeyboardInterrupt for exit 130. Unit test pins the cancel. - MED: interrupted_teardown_grace_s: null no longer softens run_timeout_s - the watchdog's SIGKILL escalation always arms (null grace applies to the ^C path only, as documented). - nits: redundant cyclopts help= strings that duplicated the field description are dropped (cyclopts falls back to the description); templates regenerated.
What does this PR do?
Consolidates every global time knob into
settings.timeoutsand makes--timeouta real whole-run watchdog.Changes
--timeout/ top-leveltimeout:was consumed nowhere (silent no-op) -> nowsettings.timeouts.run_timeout_s, a whole-run watchdog armed from setup through the metrics drain; firing writes INTERRUPTED artifacts, skips scoring, exits non-zero.settings.drainblock, flatsettings.service_ready_timeout_s, and every0 = unlimitedsentinel deleted; hard cutover (extra=forbid), no back-compat shims; templates/examples/docs/tests migrated. One convention everywhere:null= unlimited; drain budgets additionally accept an honest0(zero budget, give up immediately);run_timeout_srejects0.min_duration_ms/max_duration_msare workload durations onsettings.runtime, not timeouts. The--durationalias is deleted —--runtime.min-duration-msis the one spelling (poisson only, requires an explicittarget_qps; explicit--num-sampleswins; both unset = one dataset pass). The MLPerf ruleset path keeps its internal duration fields.metrics_drain_timeout_snow fails the run instead of exiting 0 with partial ISL/OSL buried in the summary; artifacts written first.SigintGovernor, shared by compliance-audit runs) stops the session gracefully — the aggregator drains, artifacts landstate: interrupted/complete: false,events.jsonlflushed, exit 130. Teardown is bounded by a fixed 30 s grace: if the metrics drain has not finished, the service children are SIGTERMed (the aggregator writes a best-effort INTERRUPTED snapshot) then SIGKILLed — a wedged drain can never hang the abort, and no second keystroke is needed. Repeat ^C is a logged no-op, so runners that forward the terminal's group SIGINT (uv rundelivers a single ^C twice) need no special handling. Escalation is timeout-driven, not keystroke-driven (design cue: aiperf).interrupted/complete: falsebefore it is persisted — an interrupted run is an invalid run; its artifacts only expose the partial metrics).result_summary.json+ the exit code are the run-level truth;metrics/final_snapshot.jsonrecords what the aggregator itself observed and can legitimately readstate: completewhen the abort lands after the session's terminal ENDED (post-ENDED drain window). The aggregator ignores SIGINT (the parent's ENDED path is authoritative); INTERRUPTED is entered via the session's marker event or SIGTERM (run watchdog / teardown grace).result_summary.jsonis nevercomplete: true(split-brain rewrite guard keyed onreport.state, covering the drain-timeout subcase too).--durationdeleted, above).uv run, drain-window artifact divergence — exit 130, honest artifacts, no orphans), governor unit suite (graceful + grace arming/expiry/disarm, repeat no-op, no-live-run raise), drain-timeout failure, finalization-window regression, config validation + removed-key tripwires.Exit codes (interrupt/timeout contract)
state: complete,complete: trueExecutionError: watchdog (run_timeout_s) fired,metrics_drain_timeout_sexpired, or session aborted (e.g. transport closure)state: interruptedorcomplete: falseevents.jsonl; grace expiry: whatever was already written + best-effortinterruptedsnapshotInvariant: an aborted run never exits 0 and never ships a
complete: truesummary — an interrupted run is an invalid run; its artifacts exist only to expose the partial metrics. One documented, tested edge:final_snapshot.jsonmay readstate: completefor a post-ENDED-drain-window abort (the summary stays authoritative).Follow-ups: #449 (promote warmup to a first-class phase type), #459 (consolidate run-outcome state flags into one abort/outcome model), profiling lifecycle hardening (explicit line-profiler shutdown instead of atexit; non-blocking profile POSTs) as a separate PR after this merges. schema.py module split also deferred to a follow-up MR.
Type of change
Testing
uv runkeystroke forwarding)Checklist