CONS-B1: Palette & Bolt Suite (form submission, accessibility, SVG & CSV optimizations) + movement-optimizer motion-view extraction - #4438
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91ea71b931
ℹ️ 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".
| src/chat | ||
| src/contracts.py | ||
| src/python/src/utils | ||
| src/shared | ||
| src/sidekick |
There was a problem hiding this comment.
Restore UpstreamDrift's top-level sparse roots
When this matrix entry checks out UpstreamDrift, these src/-prefixed paths omit its top-level release packages (chat, contracts.py, python/src/utils, shared, and sidekick). The repository explicitly locks those required roots in tests/ops/test_cross_repo_python_integration.py:16-24 and :66-84; both corresponding tests now fail, and the subsequent editable install/contract run will lack the packages it is intended to validate. Keep the original top-level sparse paths unless the downstream layout and its contract are migrated together.
Useful? React with 👍 / 👎.
|
CI format correction published at exact head |
|
Module-size correction published at exact head �0d6e457606b8c63dcbf76a05d6736e967595bc9 as a normal fast-forward from |
The prior consolidation ran a pre-commit ruff pass that first re-wrapped `timestamp=datetime(2026, 1, 2, tzinfo=timezone.utc) # noqa: UP017` onto multiple lines. That moved the `noqa: UP017` off the offending line, so ruff then "fixed" UP017 and rewrote the import to `from datetime import UTC`. `datetime.UTC` does not exist on Python 3.10, and the CI test matrix still includes 3.10, so this would have landed a `tests (3.10)` failure on main. Restore main's version of the file verbatim; it is lint- and format-clean and carries no palette/bolt content. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two conflicts, one of them a genuine divergent refactor of the same file. **src/movement_optimizer/gui/motion_tabs.py** — both branches split this module to satisfy the module-size budget, but differently. This branch extracted `motion_helpers.py` (`build_motion_colors`, `chain_path_length`); #4438 extracted `motion_view.py` (`MotionViewMixin`) and kept a local `_build_motion_colors`. Took main's version wholesale and deleted the now-orphaned `motion_helpers.py`, which nothing else imported. That reproduces main's exact, reviewed, green state for a vendored sub-app that CI excludes from every ruff, mypy and changed-test lane -- i.e. a place where a mis-resolution would not have been caught. Worth recording: git presented the third hunk as main *deleting* the "Playback speed" control, and it does not. Main keeps it at line 682 as a single-line `_add_control` call while this branch had it line-wrapped. Both branches' `test_motion_tabs.py` are byte-identical and both reference `chain._controls["speed"]`, so dropping it would have broken the sub-app silently. Verified before resolving rather than trusting the conflict markers. **torque_utils.py** — rerere replayed this branch's resolution, which routes polynomial evaluation through the shared `TorquePolynomial` instead of `np.polyval` on reversed coefficients. Since the two use opposite coefficient orders, I checked equivalence numerically rather than by reading: ascending `TorquePolynomial(coeffs).evaluate(t)` matches `np.polyval(coeffs[::-1], t)` to 1e-12 across t in {0, 0.13, 0.5, 1.0, 2.5}. Re-verified on the new base: mypy clean over CI's 418 files, ruff check and format clean over 596, module size budget, changed-test assertion and minimum test contract gates all pass, no conflict markers, no gitlinks, and CI's exact 210 changed-test files now collect cleanly -- 2572 tests, exit 0.
…solidation Three conflicts, all in the Bolt/Palette surface #4438 just landed: * motion_tabs.py — the two branches extracted *different* things from the same module: #4438 pulled the view code into motion_view.MotionViewMixin, while this branch pulled the palette and geometry helpers into motion_helpers. Kept both. The file is now based on main's post-extraction version, imports build_motion_colors/chain_path_length from motion_helpers instead of redefining them inline, and drops the rendering import that became unused. 1093 -> 1074 lines, so no duplicated helper and no module-budget pressure. * torque_utils.py and test_sidekick_ux_hardening.py — resolved to main's versions. The two torque implementations are behaviourally identical (both evaluate ascending coefficients; the merged pendulum test pins 2 + 3t), so main's Bolt form wins rather than re-imposing this branch's TorquePolynomial call. shared/python/swing_sim/torque_profiles still ships and is still consumed by the rate-of-closure torque workspace features. test_action_audit keeps timezone.utc with the noqa anchored on its own line, and the anchored grep for a top-level datetime UTC import is empty.
…rder deterministic Two CI-only failures on this branch. Neither was a stale table, and in both cases the easy fix would have cemented a real defect. 1. `test_table_has_no_stale_rows` reported all 19 power-supply / temperature / tuning rows as unserved. They ARE served. Proof from inside the same failing CI run, single process (`-n 0`): the 19 matching `test_gated_route_rejects_anonymous_caller` cases drove those exact paths through `TestClient(app)` -- the same module-level `app` object the stale check iterates -- and got 401/403. `_DENIED = (401, 403)`, and a route the app does not serve answers 404, which would have failed. So the routes were registered and reachable and the enumeration was under-reporting them. Deleting the rows (as an earlier slice did, which is why `test_every_route_is_classified` was failing on the merged tree) would have turned the check green while the authz matrix agreed that acknowledge_trip, permissive, setpoint, burnout_mode, tc_type and the PID tuning lifecycle did not exist -- the plant's entire control surface, in the batch whose review theme was "nothing de-energizes". `test_table_has_no_stale_rows` now asks `Route.matches()` -- the function Starlette itself calls during dispatch -- instead of string-matching `route.path` over `app.routes`. A check phrased as "would a request reach a handler" cannot disagree with a request. Verified it still discriminates: real routes and parameterised routes match, a bogus path does not, and a wrong method on a real path does not. `test_every_route_is_classified` deliberately stays enumeration-based -- it must catch a NEW route nobody classified, which is the direction that ships something ungated. Root cause of the enumeration/dispatch divergence is not established; it reproduces only under CI's environment (`--import-mode=importlib`, Linux, py3.11) and not locally across the identical 101-file selection and flags. Phrasing the assertion in dispatch terms makes the test correct regardless. 2. `test_desktop.py::test_event_logger_basic` was flaky, failing roughly one run in three locally with `assert 'alarm_trip' == 'alarm_acknowledgment'`. Not caused by this batch -- that test file is byte-identical to `main` -- but `fetch_logs` is, and it ordered by `timestamp DESC` alone. `datetime.now()` reports microseconds while the clock granularity is ~15.6 ms on Windows, so a burst of events shares one timestamp string and SQLite may return them in any order. Beyond the flake, that means an operator could see an alarm trip listed above the acknowledgement that answered it. Now `ORDER BY timestamp DESC, id DESC`, using the existing autoincrement primary key, which is monotonic in insertion order. `test_fetch_logs_orders_same_timestamp_events_newest_first` pins it by writing an explicit identical timestamp so the tie is forced rather than hoped for. Verified as a real regression test: with the tiebreaker removed it fails, returning `['first', 'second', 'third']`. Also merged `origin/main` (CONS-B1 #4438, 221 files) -- no conflicts. Confirmed `git grep -n '^from datetime import UTC' -- '*.py'` is empty, so the py3.10 revert #4438 made is intact. CI-equivalent selection (CI's own 101 files, `-m "not live_simulation" --import-mode=importlib -n 0`) run three consecutive times: 1949 passed, 6 skipped, 0 failed each time. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four conflicts, resolved on their merits rather than by picking a side. `.github/workflows/cross-repo-python-integration.yml` -- the whole-file CRLF/LF conflict. Taking `main` verbatim here would have SILENTLY DISCARDED two of this branch's own changes, so both sides were diffed on content with line endings normalised, the real changes unioned, and the result written back in `main`'s line endings (uniformly CRLF, 262 of 262). Kept from `main`: the `CI_RUNNER_MODE || LOCAL_RUNNER_MODE` convergence, the `REPOSITORY_PRIVATE` input, the public-repo hosted-runner fallback, and the `src/`-prefixed sparse paths. Kept from this branch: the `vendor/ud-tools` sparse-scope entry and the entire "Initialize pinned Tools submodule" step. Verified afterwards that the result differs from `main` by exactly those two additions and nothing else, that every element of both sides is present, that the YAML parses, that UpstreamDrift now carries 8 sparse entries, and that the submodule step still sits directly after the downstream checkout. `src/movement_optimizer/gui/motion_tabs.py` -- #4438 extracted `MotionViewMixin` into a new `motion_view.py` while this branch extracted the motion palette and chain-length helpers into `motion_helpers.py`. Both extractions are kept: the file imports from both modules, retains #4438's `_MotionViewMixin` alias, and both modules exist. `rerere` proposed this resolution and it was checked rather than trusted. `src/pendulum_simulator/src/double_pendulum_golf/torque_utils.py` -- `main`'s only change was wrapping one `raise ValueError(...)` across three lines, which this branch's `TorquePolynomial` refactor already contains identically, so `main`'s change is fully subsumed and nothing is lost. `.secrets.baseline` -- regenerated from `main`'s side with CI's exact invocation, then path keys normalised back to POSIX form because a Windows rescan rewrites them with backslashes that no Linux scan can match. Result is a superset of `main`'s 35 audited entries with none missing, 43 entries over 33 files, LF endings, and stable under a separator-normalised rescan. Verified after resolution against the NEW merge-base: no `<<<<<<< HEAD` outside `.github`, no mode-160000 gitlinks, and `.gitignore` still byte-identical to `main` so #4475's and #4445's rules survive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CONS-B1 — Palette & Bolt suite (single surviving consolidation of #4429)
What this PR actually contains
git log origin/main..HEAD:1. The full Palette & Bolt suite (
91ea71b93, a squash of #4429). Verified to be a complete, strict semantic superset of #4429's 16-commit branch: after normalising formatting on both sides with a singleruff formatpass (identical settings, all 93 differing.pyfiles), the only files that differ are the two motion-view files this branch adds. The substantive content:useId()label binding,role="alert"src/rotation_converter/web/src/components/RotationConverter.tsx,src/ode_solver/web/src/components/ODESolverCalculator.tsx.../data_explorer/plots/LinePlot.tsx,ScatterPlot.tsx,SpectrumPlot.tsx.../frontend/src/lib/chartSnapshot.ts,.../frontend/src/lib/explorer/csv.ts.jules/palette.md,.jules/bolt.md,SPEC.md(v1.1.205).github/workflows/cross-repo-python-integration.yml— sparse-checkout paths corrected to besrc/-relative2.
refactor(movement-optimizer): extract motion view scaffolding—motion_tabs.pysplit, newsrc/movement_optimizer/gui/motion_view.py(net −9 lines). This is the one piece of content that existed only on this branch and not on #4429, and is the reason this branch was chosen as the survivor.3.
fix(ci): format consolidated Python changes— re-runs the CI-pinnedruff==0.14.10formatter. #4429's own pre-commit commits re-wrapped 86 files with a ruff build that disagrees with the pin, which is why #4429'squality-gatefails at Format Check (ruff-format) and this branch's passes.main's formatting is the correct formatting and this commit restores it.4.
fix(tests): restore timezone.utc …— new, added during this repair. The prior consolidation's pre-commit pass re-wrappedtimestamp=datetime(2026, 1, 2, tzinfo=timezone.utc) # noqa: UP017, which moved thenoqaoff the offending line; ruff then applied UP017 and rewrote the import tofrom datetime import UTC.datetime.UTCdoes not exist on Python 3.10 and the test matrix still includes 3.10, so this would have landed atests (3.10)failure onmain. Reverted tomain's verbatim content.Known cosmetic churn
194 of the 222 changed files are pure
ruff-formatre-wrapping of files undersrc/pendulum_simulator/,src/movement_optimizer/andsrc/data_processing/, inherited from the prior consolidation's repo-wide pre-commit run. Those paths are in[tool.ruff]'s exclude list so CI does not format-check them. Each of the 194 was verified content-neutral: it normalises byte-identical tomainunder a singleruff formatpass. They are left in place rather than reverted, because reverting them would also have to revertsrc/movement_optimizer/tests/test_main_window.pyalongside the motion-view refactor and would invalidate the already-greenquality-gate.Supersedes
Closed as containing nothing this branch lacks:
91ea71b93here.7fc2ce419), both being91ea71b93+ a lint commit. Neither contained any of the flight / rate-of-closure work its title advertised.What this PR is not
None of the 22 agent PRs named in the original description (#4431, #4428, #4417, #4416, #4415, #4414, #4412, #4407, #4405, #4404, #4403, #4401, #4400, #4399, #4398, #4397, #4396, #4395, #4394, #4393, #4343, #4340) were ever merged onto this branch, and none of their work is in this diff. They all remain open and still require consolidation (CONS-B3 Variation/Morris family in the consolidation plan). Merging this PR does not close them.
Issues carried
Palette / Bolt work items satisfied by this PR: #4421, #4420, #4419, #4418, #4314, #4313, #4311, #4310.