feat(rate-of-closure, #4103): land docs, workflows and support files (+ repair .secrets.baseline) - #4538
Merged
Merged
Conversation
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: 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:48
dieterolson
pushed a commit
that referenced
this pull request
Aug 18, 2026
…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>
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
pushed a commit
that referenced
this pull request
Aug 18, 2026
… 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>
dieterolson
pushed a commit
that referenced
this pull request
Aug 18, 2026
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>
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
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.
The documentation, workflow and support tail of
consolidated/rate-closure-remainder-2026-08-13, plus a repair to.secrets.baselinethat this slice surfaced.What lands
docs/specscontracts — flight-to-ground, ground impact bounce, ground material profiles, ground reference execution, ground result studies, ground skid-roll, chip forgiveness analysis, calculation runtime manifest, camera viewport controlsdocs/releaseartefacts — four-surface capability (doc,v1.json,v1.schema.json) and the campaign manifest (doc +v1.json)docs/development/rate_of_closure_clubhead_tensor_contract.mdsrc/shared/python/golf_club/—cad_validation.py,stl_validation.py,AGENT_HANDOFF.mdtests/ops/test_maturin_swing_core_workflow.pyand the 3-linematurin-swing-core.ymlchange it asserts:PYTEST_DISABLE_PLUGIN_AUTOLOAD: "1"on the parity step, so that Rust-only lane cannot import plugins cached on a self-hosted runner (e.g.pytest-qtwith no PyQt6 installed)rate-of-closure-visual-evidenceandrate-of-closure-web-distributionOne workflow is deliberately withheld
rate-of-closure-windows-state-security.ymlis not landed, and neither istests/ops/test_rate_of_closure_windows_state_workflow.pywhich asserts its shape.It requests
runs-on: [self-hosted, Windows, X64, d-sorg-windows-security]. Querying the org runner API: no runner carriesd-sorg-windows-security. The only online Windows self-hosted runner is labelledself-hosted,X64,Windows,matlab.It triggers on
pull_requestfor paths includingsrc/rate_of_closure/web/**andsrc/rate_of_closure/web_authority/**— exactly the paths this migration keeps touching. Landing it would queue a job forever on every one of these PRs. It needs a labelled runner provisioned first; that is infrastructure work, not a file move.The two workflows that do land were checked the same way — both are path-filtered and target pools that exist:
d-sorg-fleet(23 runners online) and hostedubuntu-24.04..secrets.baselinewas broken two waysAdding a file under
tests/ops/pulls that suite into the run, which is how both of these surfaced:tests/ops/test_detect_secrets_baseline.pyonmaintoday. That suite is changed-file scoped, so the debt stayed invisible until a PR touchedtests/ops/.RegionalSurfacePlanPanel.test.tsx,groundPlaybackWorkspaceV2.test.ts,webRuntime.test.ts— were never recorded in the baseline, which fails thedetect-secretsgate on every subsequent PR.Both are fixed here. Worth recording the ordering trap:
detect_secrets scanwrites native separators, so on Windows it must be run before normalising, not after — doing it the other way silently re-introduces the backslashes.All flagged values are content digests (
input_sha256, fixture hashes), not credentials. The workflow's own comparison normalises separators on both sides before diffing, so rewriting the keys cannot change which findings are recognised.Verification
pytest tests/ops— 141 passed, 3 skipped (was 3 failed before the baseline repair)pytest tests/ops tests/rate_of_closure src/shared/python/golf_club src/shared/python/swing_sim— 3,160 passedruff check .— clean1.17.34)One unrelated flake was observed and ruled out:
test_club_view_camera.py::test_worst_library_mesh_uses_bounded_playback_cadenceis a wall-clock assertion on a main-owned test and module this slice does not touch; it passes 3/3 in isolation.🤖 Generated with Claude Code