feat(rate-of-closure, #4103): land web_companion + web_distribution (14 modules, 5 tests) - #4536
Merged
Conversation
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>
|
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 10:26
…ation The rate-of-closure-web-distribution workflow landed by #4538 targets test files by name and was failing with pytest exit 5 -- no tests collected -- on every PR touching src/rate_of_closure/**. This adds what it names: scripts/check_rate_web_wheel.py, test_web_asset_distribution.py, the browser_companion_harness and its test, and the four web/tests/browser Playwright specs with their four support modules. That suite needs testDir "./tests/browser", but main's playwright.config.ts is testDir "./e2e" carrying the project matrix from #4473, so repointing it would silently disable every e2e spec. A separate playwright.browser.config.ts is added instead, and the three test:browser* scripts pass it with --config. Vitest's exclude gains tests/** for the same reason it already excludes e2e/**: it would otherwise collect Playwright specs and fail them for want of a served app. tests/e2e/*.pw.ts is still withheld -- no config in either tree matches it. Verified: 3,312 Python tests, 1,420 React tests across 175 files, tsc clean, ruff clean, file-size budget clean, and CI's exact mypy invocation clean across 15 changed source files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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>
… 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>
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>
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>
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>
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>
… 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>
…ccident 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>
dieterolson
pushed a commit
that referenced
this pull request
Aug 18, 2026
#4536 landed while this PR was open, so the table would have merged already stale. Re-measured against origin/main: 52 files, not ~60, and the split moved -- 19 tests, 18 web/src, 15 elsewhere. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dieterolson
added a commit
that referenced
this pull request
Aug 18, 2026
…s mypy advice) (#4546) * docs(rate-of-closure, #4103): bring the handoff doc back to current state Twenty slices have merged since this was last accurate. The remaining-work table is re-measured against origin/main and now names the real blocker: the ~60 files left are not blocked on effort but on the camera-controls cluster, which is a reimplementation rather than a migration -- wiring CameraViewportMixin into simulation_view and flight_view passes 20 of 20 camera GUI tests while regressing three main-owned ones, and reproducing the branch's Face-On behaviour needs ~20 further ui/pyqt6 files that delete shipped work. Corrects one entry that was actively harmful. The doc told the next agent to check mypy files individually, which is exactly what made #4531 fail quality-gate: CI passes every changed file to one invocation with MYPYPATH=src, so per-file checking invents no-any-return findings CI does not have and hides the redundant-cast findings it does. The replacement gives the exact command, including that Python 3.12 is required. Adds the traps found since -- pathlib.write_text rewriting LF as CRLF, the detect_secrets scan/normalise ordering, and purely-additive files that still fail against main -- and balances the "branch is not uniformly newer" rule with its converse, since applying it blindly cost two good ports. Deletions are not disqualifying; the bar is main's whole suite green against the swapped-in version first, as #4542 and #4545 both cleared. 149 lines, inside the 150-line policy. No code changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(rate-of-closure): re-measure the remaining-work table after #4536 #4536 landed while this PR was open, so the table would have merged already stale. Re-measured against origin/main: 52 files, not ~60, and the split moved -- 19 tests, 18 web/src, 15 elsewhere. 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.
Lands the local companion server and the web asset distribution layer — the two Python packages that the deferred
web_companion/web_distributiontests have been waiting on. Part of the slice-by-slice migration ofconsolidated/rate-closure-remainder-2026-08-13.What this adds
web_companion(8 modules) —app,bundle,cli,contracts,response_contract,runtime,supervisor,__init__. TheAuthoritySupervisorowns one restartable authority child and serialises its short-lived HTTP requests, deliberately without automatic request replay.web_distribution(6 modules) —asset_manifest,asset_resolver,package_assets,runtime_descriptor,verify_install,__init__.5 tests — the four
test_web_companion_*files plustest_regional_ground_real_loopback.py.Why the loopback test comes along
It isn't padding. The three companion gateway tests pass
as their
authority_app_factory. Without that module on disk the spawned child dies on import, and the only symptom the parent reports is:Getting from that message to the actual cause required capturing the child's stderr, because
web_authority/runtime.pyspawns withstderr=subprocess.DEVNULL. That is the same diagnostic blind spot already fixed in the Morris runtime, and it is worth fixing here too — flagged rather than folded into this slice.One test-robustness fix
test_regional_ground_real_loopback.pysetpoll_timeout_s=15.0with a 1s per-request transport timeout. Under-n auto— a dozen xdist workers each spawning a real authority subprocess — the job completes but the client gives up first and raisespoll_timeout. No test asserts timeout behaviour; these bounds exist only to keep the suite snappy. Raised to 120s / 10s, both well inside the 300s production default and the suite's own--timeout=300.This was reproducible, not a one-off: it failed on two separate full-suite runs before the change and is clean on two after.
What stays deferred
The remaining 25 branch tests were re-measured against this tree and still cannot land. They either need
ui/pyqt6modules still in flight (#4531), or assert APIsmain's shared modules do not have (SimulationView.camera_controls,SimulationEnsembleResult.runs, extraSimulationConfigkwargs).Verification
pytest tests/rate_of_closure src/shared/python/swing_sim— 3,074 passed, 7 skipped, 0 failed, on two consecutive runsMYPYPATH=src:src/python/src) — clean across 14 source filesruff check/ruff format --check— cleandetect-secrets— no new findings (baseline left untouched; only its timestamp would have moved)1.17.33)🤖 Generated with Claude Code