Skip to content

fix(carry): commit carry once in finalization and use it everywhere - #276

Merged
jewbetcha merged 5 commits into
open-flight:mainfrom
cvrt-jh:fix/ballistics-carry-precedence
Sep 23, 2026
Merged

jewbetcha merged 5 commits into
open-flight:mainfrom
cvrt-jh:fix/ballistics-carry-precedence

Conversation

@cvrt-jh

@cvrt-jh cvrt-jh commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Makes shot finalization in server.py the single writer of carry_spin_adjusted, and sends that committed carry to the simulator connectors.

Three related changes, one story:

  1. server.py: run the RK4 simulator whenever ballistics is enabled and a launch angle exists, replacing whatever is already on the shot; use the spin table only when the simulator cannot run. The old guard ran the simulator only if shot.carry_spin_adjusted is None.
  2. rolling_buffer/monitor.py: stop pre-filling carry_spin_adjusted with the spin-table estimate in _create_shot. That pre-fill was what made the guard above skip the simulator on every shot with a measured spin. The table helper itself stays; the server's fallback uses it.
  3. sim/resolver.py: prefer the committed carry_spin_adjusted over the bare launch-angle table when building the shot for GSPro and friends, so the connectors and the kiosk report the same carry.

Why was this required?

Reported in #openflight-help today: spin-adjusted carry on the kiosk reads far shorter than both the debug (launch-angle table) number and a commercial simulator on the same swings, while club speed and ball speed match closely. Reproduced with the reported 7-iron (104.2 mph ball, 19.1 deg launch, 5164 rpm, 83.7 mph club):

path carry (yd)
spin table, what the kiosk showed 119.2
launch-angle table, the debug view 145.0
RK4 simulator, skipped by the guard 144.2
commercial simulator on that swing 141.8

The monitor filled carry with a ball-speed table plus a spin penalty that never sees the launch angle; the server then declined to overwrite it. The guard predates the simulator (added for the table path in April); when the simulator was wired inside it in June the short-circuit was born. #266 improved the model itself, but the model was never reached on exactly the shots where spin is measured, which is why merging it did not change what people see on the kiosk.

Rigs without a vertical launch angle are unchanged. The monitor's pre-fill gate was processed.has_spin, which is SpinResult.is_reliable: confidence at or above 0.6 with medium or high quality. The server fallback already used its own literal 0.6. In the processor those two always agree (every quality demotion also caps confidence at 0.5), so the table carry those rigs see is the same number as before. The two literals are now one constant, SPIN_CONFIDENCE_RELIABLE in launch_monitor.py, used by both SpinResult.is_reliable and the server fallback, with a test pinning them to each other at the boundary.

Automated tests

  • TestBallisticCarryPrecedence in tests/test_server.py (7 cases) drives _finalize_shot_detected directly: simulator replaces a pre-filled 119.2 yd with 144.2 yd; the table fallback replaces a pre-filled value when ballistics is off or no launch angle exists; the fallback fills an empty carry; the fallback's spin gate agrees with SpinResult.is_reliable on both sides of the boundary; and the carry handed to _forward_shot_to_simulators resolves to the same number the kiosk gets.
  • test_create_shot_leaves_carry_to_server_finalization in tests/test_rolling_buffer.py: a clean high-confidence spin no longer pre-fills carry.
  • Two resolver tests in tests/test_sim_resolver.py: committed carry is preferred, table is the fallback.

Eight of the ten fail on main and pass here (checked by checking out main's src/ under this branch's tests; two of the eight fail there only because the shared constant does not exist yet). The boundary test also goes red when the server floor is moved 0.05 off the constant while SpinResult stays put, so it detects drift, not just the current value. Full suite: 1596 passed, 7 skipped. Pylint 9.58 (was 9.52). Ruff clean on src/openflight; the one I001 in tests/test_sim_resolver.py is on an untouched import line and exists on main. Strict docs build clean.

Manual (human) testing

No hardware run for this PR. What I verified by hand:

  • Ran the reported shot through each estimator in a REPL against this branch and confirmed the table in the "Why" section; the only path that lands on the commercial-simulator number is the one the old guard skipped.
  • Read every writer of carry_spin_adjusted and every reader in ui/src (DisplayMode.tsx, liveMetrics.ts) to confirm the kiosk and the live panel both prefer it over estimated_carry_yards, so this is the number users see.
  • Confirmed _forward_shot_to_simulators is called inside finalization after carry is committed, so the resolver change cannot see a stale value.

Someone with an IWR6843 or KLD7 rig hitting balls next to a reference unit would be the real confirmation. The log makes it visible: measured-spin shots should now print Ballistic carry: where before they printed nothing.

AI assistance

Worked with Claude Code throughout: tracing the carry paths, writing the tests, and drafting this text. The diagnosis matches the one the reporter shared in Discord. The decision to consolidate into a single writer rather than only fix the guard, the check that the two spin gates were already equal, the test cases, and the numbers are mine.

Checklist

  • Single feature/fix - this PR is scoped to one thing with a clear story above
  • Automated tests included - new or updated tests cover this change
  • Manual testing described - I documented what I verified by hand above
  • Python tests pass (uv run pytest tests/ -v)
  • Pylint passes (uv run pylint src/openflight/ --fail-under=9)
  • Ruff passes (uv run ruff check src/openflight/)
  • UI builds (cd ui && npm run build) - no UI files touched
  • UI lint passes (cd ui && npm run lint) - no UI files touched
  • Updated docs or CHANGELOG if needed
  • No unrelated changes mixed in

The rolling buffer monitor pre-fills carry_spin_adjusted with the
spin-table estimate for every shot whose spin is not rejected. Shot
finalization only ran the RK4 simulator when that field was still None,
so any shot with a measured spin kept the table number and never reached
the physics model. The table never sees the launch angle, which is why
kiosk carry read 20-30 yards short of the debug estimate and of
commercial launch monitors on the same swings.

Run the simulator whenever ballistics is enabled and a launch angle is
available, and keep the table fallback only for shots nothing else has
filled. Rigs without a vertical launch angle are unchanged.
Remove the rolling buffer monitor's spin-table pre-fill of
carry_spin_adjusted so finalization is the single writer: the ballistic
simulator when it can run, the spin table otherwise. Shots without a
vertical launch angle now go through the server's fallback, which trusts
measured spin only at confidence 0.6 or above and otherwise uses the
club-optimal spin.

Route the committed carry to the simulator connectors as well. The
resolver sent the bare launch-angle table to GSPro even when the
simulator had produced a carry for the kiosk, so the two disagreed.
SpinResult.is_reliable and the server's table-carry fallback each carried
their own 0.6 literal. They must agree, because the monitor used to gate
its carry pre-fill on is_reliable and the server fallback now stands in
for it on rigs without a launch angle. One constant, one boundary test.
@cvrt-jh
cvrt-jh requested a review from jewbetcha as a code owner September 23, 2026 08:18
@jewbetcha
jewbetcha merged commit 958bc00 into open-flight:main Sep 23, 2026
14 checks passed
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.

2 participants