feat(rate-of-closure, #4103): third React web slice + two additive symbol ports - #4541
Merged
Conversation
…mbol ports
Adds 23 net-new files: the regional-ground execution job/result/presentation
and their file IO, the authority client, job-preparation and variation
request wires, the variation workspace, three regional-ground hooks, the
imported-job panel, club STL export, club engineering sidecar, and flight
preparation launch.
Two of the ten symbols blocking the previous slice are ported additively
onto main's versions rather than by taking the branch's files, which delete
content:
- mesh.writeBinaryStl with binaryHeader and two byte constants; main
already had BINARY_HEADER_BYTES, BINARY_RECORD_BYTES, triangleNormals.
- the two regional-ground variation keys, checked byte-for-byte against
the already-landed regional_ground_variation_dataset.py.
Three candidate ports were investigated and deliberately not made. The
capabilityOptimizer re-exports are dead here -- the only consumer imports
straight from capabilityObservationContract. drawGroundPlayback is built on
branch-local helper signatures whose arities differ from main's, so porting
it means reworking main's renderer to unblock two files. And flight's
angular-state trio would require either duplicating the integrator or
replacing the one carrying #4518's ground-crossing guard.
PrimaryViewTabs.test.tsx and TorqueProfilePanel.test.tsx are both restored
to main's versions: each was purely additive by line count yet failed
against main's unchanged component. The second was caught only because the
prune guard now refuses to delete files present on origin/main.
Verified: tsc clean, eslint clean, 1,420 tests across 175 files, production
build succeeds, zero deletions in the staged diff.
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 14:20
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.
Unblocks and lands 23 more React files by porting two of the ten symbols that gated #4533's remainder — additively onto
main's versions, never by taking the branch's files.What lands
23 net-new files: the regional-ground execution job/result/presentation and their file IO, the authority client, job-preparation and variation request wires, the variation workspace, three regional-ground hooks (
useRegionalGroundExecutionController,useRegionalGroundExecutionWorkspace,useRegionalGroundVariationWorkspace), the imported-job panel, club STL export, club engineering sidecar, and flight preparation launch.The two ports (67 lines, zero deletions)
mesh.writeBinaryStlbinaryHeader, andBINARY_PREFIX_BYTES/MAX_UINT32mainalready hadBINARY_HEADER_BYTES,BINARY_RECORD_BYTES,triangleNormals,Vec3,TriangleGROUND_NORMAL_RESTITUTION_KEY,GROUND_ROLLING_RESISTANCE_KEYvariationRegistry.tsandvariation.tsVARIABLE_REGISTRYentries. Key strings checked byte-for-byte against the already-landedregional_ground_variation_dataset.py;CATEGORY_LAUNCHisswing_sim.flight.launchon both sidesBoth are consumed by files landing here (
clubStlExport,regionalGroundVariationWorkspace) — no dead surface.Three ports investigated and deliberately not made
capabilityOptimizerre-exports — I wrote them, then found they're dead: the only consumer,capabilityObservationEnsemble, imports straight fromcapabilityObservationContract. Reverted rather than shipped.drawGroundPlayback— built on branch-local helper signatures whose arities differ frommain's, plus two constantsmainlacks. Porting it means reworkingmain's renderer to unblock two files. Not worth the blast radius.flight'sAngularFlightPoint/FlightSimulationOptions/simulateFlightWithOptions— the branch moved the integrator into a newflightIntegrator.tsand re-typedFlightResult.trajectorytoAngularFlightPoint[].main's in-filesimulateFlightcannot satisfy that without either duplicating the integrator or replacing the one carrying feat(swing-sim): land the flight slice of #4466 with its React parity counterpart #4518's ground-crossing guard.Worth recording for whoever does take it on: the branch integrator does cover that bug, structurally rather than by a time check. It requires
currentGap > 0strictly, so a launch starting at ground level never registers a crossing — which is exactly what&& t > dtachieves inmain's guard.Two main-owned tests restored
PrimaryViewTabs.test.tsxandTorqueProfilePanel.test.tsxare both reverted tomain's versions. Each was purely additive by line count — the usual signal a file is safe to take — yet failed againstmain's unchanged component.The second was caught only because the prune script now refuses to delete any file present on
origin/main; before that hardening it silently removed it, which would have reverted shipped work.Verification
tsc --noEmit— cleaneslint .— cleanvitest run— 1,420 passed across 175 files, 0 failednpm run build— production Vite build succeeds1.17.35)One process note recorded in the SPEC row:
pathlib.write_textrewrites LF files as CRLF on Windows, which turned a four-line edit into an 834-line whole-file diff until the endings were restored.🤖 Generated with Claude Code