feat(rate-of-closure, #4103): land the second React web slice (components, hooks, release) - #4533
Merged
Merged
Conversation
Adds the 45 component, hook and release files that #4530's model-only slice left behind: the camera control bar, club canvas viewport and playback controls, the ground-playback panels, regional execution evidence and ledger tables, the regional surface plan panel, the scalar ensemble and wind-strategy scatters, the wind-strategy panel, the view compositor, seven hooks, the webRuntime descriptor, authorityProxyConfig, the release artifact generator and contract, and the ground-tee visual Playwright spec. Of 125 candidates only 45 could land. Ten symbols the rest need are absent from main and live in five modules that diverged in both directions -- flight, capabilityOptimizer, variationRegistry, mesh and flightPlaybackDrawing. None can be taken wholesale; every one deletes content relative to main, and flight's addition re-types FlightResult.trajectory and rewrites simulateFlight to delegate through an options-based integrator. That is a reimplementation on main's newer core, not a file move, and is tracked separately. PrimaryViewTabs.test.tsx is reverted to main's version: this slice's copy was additive by line count yet failed against main's unchanged component. The prune script is hardened to refuse to remove any file present on origin/main -- it deleted TorqueProfilePanel.test.tsx on this pass. Verified: tsc clean, eslint clean, 1,399 tests across 170 files, and the production Vite build succeeds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
dieterolson
enabled auto-merge (squash)
August 18, 2026 09:47
dieterolson
added a commit
that referenced
this pull request
Aug 18, 2026
…4538) Adds 9 docs/specs contracts, 5 docs/release artefacts, the clubhead tensor contract, golf_club's CAD and STL validation modules with its handoff doc, tests/ops/test_maturin_swing_core_workflow.py, and two CI workflows. maturin-swing-core.yml gains PYTEST_DISABLE_PLUGIN_AUTOLOAD on its parity step, which the new ops test asserts: that Rust-only lane must not import plugins cached on a self-hosted runner, such as pytest-qt without PyQt6. rate-of-closure-windows-state-security.yml is deliberately NOT landed, nor the ops test asserting its shape. It requests [self-hosted, Windows, X64, d-sorg-windows-security] and no runner in the org carries that label -- the only online Windows self-hosted runner is labelled matlab. It triggers on pull_request for src/rate_of_closure/web/** and web_authority/**, so landing it would queue a job forever on exactly the PRs this migration produces. It needs a labelled runner first. Also repairs .secrets.baseline, which was broken two ways: 30 result keys held Windows backslash separators, failing tests/ops on main (that suite is changed-file scoped, so the debt stayed invisible until a PR touched tests/ops/), and three digest fixtures added by #4533 were never recorded, which fails the detect-secrets gate on every subsequent PR. Ordering matters: detect_secrets scan writes native separators, so on Windows it must run before normalising, not after. Verified: 141 ops tests pass; 3,160 across the touched packages. Co-authored-by: codex-scheduled <codex-scheduled@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 18, 2026
dieterolson
pushed a commit
that referenced
this pull request
Aug 18, 2026
browser_companion_harness.py is a Playwright harness, not a test: it has no assertions of its own, so the changed-test-assertion gate flagged it. Added to scripts/test_assertion_allowlist.txt, the gate's documented escape hatch for fixture-only modules, with a comment matching the file's convention. Also records two digest fixtures added by #4541 (clubAssemblyBinding.test.ts, clubEngineeringSidecar.test.ts) that were never written to .secrets.baseline, which fails detect-secrets on every subsequent PR -- the same omission #4538 fixed for #4533's fixtures. The baseline is rescanned and then re-normalised to forward slashes, in that order, because the scan writes native separators on Windows. Verified: ruff clean, CI's exact mypy invocation clean across 15 changed source files, the assertion gate passes, and tests/ops is green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dieterolson
added a commit
that referenced
this pull request
Aug 18, 2026
…14 modules, 5 tests) (#4536) * feat(rate-of-closure, #4103): land web_companion and web_distribution Adds 14 modules and 5 tests. web_companion brings the local companion app, bundle, CLI, contracts, response contract, runtime, and the single-flight AuthoritySupervisor that owns one restartable authority child and serialises its short-lived HTTP requests without automatic replay. web_distribution brings the asset manifest and resolver, asset packaging, runtime descriptor and install verification. test_regional_ground_real_loopback.py is not optional here: the three companion gateway tests pass create_cancellable_authority_app from that module as their authority_app_factory, so without it the spawned child dies on import and the only symptom is "local Python authority exited before readiness". Reaching that diagnosis needed the child's stderr, which web_authority/runtime.py discards with stderr=subprocess.DEVNULL -- the same blind spot already fixed in the Morris runtime. Widens that test's own poll budgets, which nothing asserts on. At poll_timeout_s=15 and a 1s per-request transport timeout it failed intermittently under -n auto, where a dozen workers each spawn a real authority subprocess: the job completes but the client gives up first and reports poll_timeout. Raised to 120s and 10s, both well inside the 300s production default. Two no-any-return findings in response_contract are narrowed with str() at the boundary, matching the surrounding branches. Verified: 3,074 tests pass across two consecutive full-suite runs; CI's exact mypy invocation clean across all 14 changed source files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rate-of-closure): satisfy the assertion gate and secrets baseline browser_companion_harness.py is a Playwright harness, not a test: it has no assertions of its own, so the changed-test-assertion gate flagged it. Added to scripts/test_assertion_allowlist.txt, the gate's documented escape hatch for fixture-only modules, with a comment matching the file's convention. Also records two digest fixtures added by #4541 (clubAssemblyBinding.test.ts, clubEngineeringSidecar.test.ts) that were never written to .secrets.baseline, which fails detect-secrets on every subsequent PR -- the same omission #4538 fixed for #4533's fixtures. The baseline is rescanned and then re-normalised to forward slashes, in that order, because the scan writes native separators on Windows. Verified: ruff clean, CI's exact mypy invocation clean across 15 changed source files, the assertion gate passes, and tests/ops is green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rate-of-closure): add the packaging set the distribution workflow needs The rate-of-closure-web-distribution workflow landed by #4538 installs '.[rate-of-closure-web]' and builds a wheel, but neither the extra nor the build hook existed on main, so both its jobs failed: the contracts job on ModuleNotFoundError for scipy and fastapi, and the wheel job on the same. Adds what the workflow assumes, all of which belongs with web_companion: - the rate-of-closure-web extra (fastapi, filelock, scipy, uvicorn) - the rate-of-closure-web console script -> web_companion.cli:main - build_hooks.RateWebBuildPy wired as the build_py cmdclass - MANIFEST.in, which pruned the web dist and node_modules from sdists The build hook is fail-closed but a no-op for ordinary builds: with no web/dist and no ROC_RELEASE_REVISION it only clears build staging. It raises only when the two disagree -- a built dist without an exact revision, or a revision that does not match a clean checkout HEAD -- which is the point, since it decides whether web assets may enter a wheel. Verified locally: pyproject parses with the extra, script and cmdclass present; build_hooks imports; and the console-script target rate_of_closure.web_companion.cli:main resolves and is callable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rate-of-closure): pin the ground-tee spec's own viewport The visual-evidence workflow landed by #4538 ran e2e/ground-tee-visual.spec.ts for the first time and it failed on expect(page.viewportSize()).toEqual({ width: 1600, height: 1200 }) The spec asserts that viewport but never sets it. That held under the source branch's Playwright config; main's projects (chromium-desktop and friends, from #4473) carry their own viewports, so the assertion could not pass. Declares the viewport with test.use so the captured evidence stays comparable under whichever project runs it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rate-of-closure): include the web dist in wheel package data The wheel build reached the build hook and then failed closed with "installed web asset manifest is unavailable": resolve_packaged_web_assets reads the manifest out of the installed package, so the built frontend has to be declared as package data or it never enters the wheel. main's [tool.setuptools.package-data] rate_of_closure list and the source branch's are disjoint -- main carries #4473's visual baselines, the branch carries the web dist globs -- so this takes the union rather than either side, which would have dropped the other's assets. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rate-of-closure): separate the release build from the everyday build The wheel job failed on a missing dist/rate-of-closure-assets.v1.json. Only release/generateReleaseArtifacts.mjs writes it -- along with the runtime descriptor and the index.html embed -- and nothing on main called that script. Wiring it into `npm run build` unconditionally does not work either: the release artifact contract rejects .map files outright, because a distributed static bundle must not ship sourcemaps and the integrity manifest cannot list what it refuses to serve. main builds with sourcemap: true, so every ordinary build would have started failing. So the two builds are now distinct: - `build` stays tsc && vite build, sourcemaps on, used by the visual-evidence workflow which only needs a served app - `build:release` adds the artifact generator, and vite drops sourcemaps when ROC_RELEASE_REVISION is set, which only the distribution and browser-qualification jobs do Verified locally in both modes: ordinary build emits 4 maps and no release JSON; release build emits 0 maps and both release JSON artifacts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rate-of-closure): run visual evidence only on the engine it installs The visual-evidence job installs Chromium and is named for Ground/Tee evidence capture, but ran `npm run test:e2e`, a bare `playwright test`, which also selects the firefox-desktop and webkit-desktop projects from main's config. Those failed on browserType.launch: Executable doesn't exist at .../ms-playwright/firefox-1538/firefox/firefox which is a missing browser binary, not anything about the app -- worth stating plainly, because the failure reads like a cross-engine bug in variation-crossbrowser.spec.ts and it is not. Scopes the run to chromium-desktop and chromium-narrow. Making the cross-browser specs genuinely run is a separate decision about installing three engines on the fleet; they were never running before this workflow existed either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rate-of-closure): scope the package-data contract test to its own claim Adding the web dist to [tool.setuptools.package-data] broke test_all_visualization_authorities_are_declared_as_package_data, which asserted set equality against the six visualization entries. That assertion checks two different things: that every visualization authority is declared -- the contract its name states -- and that nothing else is, which is not. The built web distribution is legitimate package data, and pinning the whole list to one feature's entries makes any other feature's packaging a failure. Now asserts the authorities are a subset, and separately that nothing outside them and web/dist/ appears, so drift is still caught. Verified a stray entry is still rejected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: ignore Playwright output and drop the artifacts I committed by accident detect-secrets flagged two findings in src/rate_of_closure/web/playwright-report/index.html. That file should never have been in the repo: my previous commit picked it up through `git add -A` after I ran a Playwright listing in this worktree, along with test-results/browser-qualification.json. Neither directory was gitignored, which also matters beyond my mistake -- self-hosted runners reuse work directories, so Playwright output survives between jobs and detect-secrets scans it as if it were source. Both are now ignored and untracked. Verified: the PR diff against main contains no playwright-report or test-results entry, and still has zero deletions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: codex-scheduled <codex-scheduled@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Continues the React migration where #4530 (
modelonly) stopped, adding the component, hook and release layers. Part of the slice-by-slice migration ofconsolidated/rate-closure-remainder-2026-08-13.What this adds — 45 files
CameraControlBar,ClubCanvasViewport,ClubCanvasPlaybackControls,GroundPlaybackToolbar,GroundPlaybackComparisonSummary,GroundPlaybackResultEvidence,RegionalExecutionEvidencePanel,RegionalExecutionLedgerTables,RegionalSurfacePlanPanel,ScalarEnsembleScatter,WindStrategyPanel,WindStrategyScatter,ViewCompositor, plusclubCanvasRendering,plotLayout,pointerCoordinates,torqueProfilePanelSupport,useViewportClampedPopoverand testsuseRegionalGroundAuthority,useSimulationBallSetup,useSimulationTorqueWorkspace,useVariationWorkspace,useWorkspaceFilesand testswebRuntime+ test,groundPlaybackWorkspaceV2.testgenerateReleaseArtifacts.mjs,releaseArtifactContract.mjs,releaseArtifactContract.node-test.mjsauthorityProxyConfig+ test,vite-env.d.ts,index.htmlruntime tag,tsconfig.json("types": ["vite/client"]),e2e/ground-tee-visual.spec.tsWhy only 45 of 125 candidates
This is a hard boundary, not a budget. Ten symbols the remaining files need are absent from
main, and they live in five modules that diverged in both directions:flightAngularFlightPoint,FlightSimulationOptions,simulateFlightWithOptions+31/−115capabilityOptimizerCapabilityOptimizationCancelled,CapabilitySampleObservation,capabilitySampleObservationWire+190/−38variationRegistryGROUND_NORMAL_RESTITUTION_KEY,GROUND_ROLLING_RESISTANCE_KEY+11/−25meshwriteBinaryStl+71/−89flightPlaybackDrawingdrawGroundPlaybackNone can be taken wholesale — every one deletes content relative to
main. Andflight's addition is not additive in substance either: it re-typesFlightResult.trajectoryto carry angular state and rewritessimulateFlightto delegate through an options-based integrator. Porting it means reimplementing onmain's newer core, which already carries the #4518 ground-crossing guard. That is a feature port, tracked separately — not something to smuggle into a migration slice.64 files were dropped by iterative typecheck pruning (6 rounds), 8 more by test outcome:
main'sApp,PrimaryViewTabs,chipForgivenessEnsembleandworkspaceVariationSessiondo not havetests/browserPlaywright harness belongs to theweb_companionslicetests/e2e/*.pw.tshas no configured runner (playwright.config.tssetstestDir: "./e2e") and no counterpart onmain, so it would land as dead filesTwo things worth flagging
PrimaryViewTabs.test.tsxis explicitly reverted tomain's version. This slice's copy was purely additive by line count — the usual signal that a file is safe to take — yet it failed againstmain's unchanged component. Taking it would have shipped a red main-owned test.The prune script now refuses to remove any file present on
origin/main. On this pass it deletedTorqueProfilePanel.test.tsx, a main-owned file this slice had modified. That would have reverted shipped work rather than dropping an unlandable addition. The staged diff is verified to contain zero deletions.Verification
tsc --noEmit— cleaneslint .— cleanvitest run— 1,399 passed across 170 files, 0 failednpm run build— production Vite build succeeds1.17.32)No Python or Rust changes.
🤖 Generated with Claude Code