Skip to content

feat(rate-of-closure, #4103): land docs, workflows and support files (+ repair .secrets.baseline) - #4538

Merged
dieterolson merged 1 commit into
mainfrom
feat/roc-docs-ops-slice
Aug 18, 2026
Merged

feat(rate-of-closure, #4103): land docs, workflows and support files (+ repair .secrets.baseline)#4538
dieterolson merged 1 commit into
mainfrom
feat/roc-docs-ops-slice

Conversation

@dieterolson

Copy link
Copy Markdown
Collaborator

The documentation, workflow and support tail of consolidated/rate-closure-remainder-2026-08-13, plus a repair to .secrets.baseline that this slice surfaced.

What lands

  • 9 docs/specs contracts — 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 controls
  • 5 docs/release artefacts — four-surface capability (doc, v1.json, v1.schema.json) and the campaign manifest (doc + v1.json)
  • docs/development/rate_of_closure_clubhead_tensor_contract.md
  • src/shared/python/golf_club/cad_validation.py, stl_validation.py, AGENT_HANDOFF.md
  • tests/ops/test_maturin_swing_core_workflow.py and the 3-line maturin-swing-core.yml change 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-qt with no PyQt6 installed)
  • 2 CI workflowsrate-of-closure-visual-evidence and rate-of-closure-web-distribution

One workflow is deliberately withheld

rate-of-closure-windows-state-security.yml is not landed, and neither is tests/ops/test_rate_of_closure_windows_state_workflow.py which asserts its shape.

It requests runs-on: [self-hosted, Windows, X64, d-sorg-windows-security]. Querying the org runner API: no runner carries d-sorg-windows-security. The only online Windows self-hosted runner is labelled self-hosted,X64,Windows,matlab.

It triggers on pull_request for paths including src/rate_of_closure/web/** and src/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 hosted ubuntu-24.04.

.secrets.baseline was broken two ways

Adding a file under tests/ops/ pulls that suite into the run, which is how both of these surfaced:

  1. 30 result keys held Windows backslash separators, failing tests/ops/test_detect_secrets_baseline.py on main today. That suite is changed-file scoped, so the debt stayed invisible until a PR touched tests/ops/.
  2. Three digest fixtures added by feat(rate-of-closure, #4103): land the second React web slice (components, hooks, release) #4533RegionalSurfacePlanPanel.test.tsx, groundPlaybackWorkspaceV2.test.ts, webRuntime.test.ts — were never recorded in the baseline, which fails the detect-secrets gate on every subsequent PR.

Both are fixed here. Worth recording the ordering trap: detect_secrets scan writes 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/ops141 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_sim3,160 passed
  • ruff check . — clean
  • mypy via CI's exact invocation — clean
  • File-size budget — clean
  • SPEC.md: exactly one insertion (row 1.17.34)

One unrelated flake was observed and ruled out: test_club_view_camera.py::test_worst_library_mesh_uses_bounded_playback_cadence is 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

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>
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@dieterolson
dieterolson enabled auto-merge (squash) August 18, 2026 10:48
@dieterolson
dieterolson merged commit d59c806 into main Aug 18, 2026
29 of 34 checks passed
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant