feat(putting-monitor): recorded-putt replay corpus regression test suite (#5222) - #5256
dieterolson wants to merge 2 commits into
Conversation
…ite (#5222) - Add 7 short MJPEG clips under src/putting_launch_monitor/tests/data/ with manifest.json and calibration.json - Add test_replay_corpus.py asserting 5 putts match launch speed and HLA within tolerance, and 2 negative controls (hand placement, unarmed roll) assert 0 accepted putts - Replay tests marked slow so default test lane excludes them while slow lane verifies the full regression suite - Add scripts/generate_putting_replay_corpus.py to reproducibly construct and calibrate corpus clips - Document corpus replay and procedure to add clips in README.md - Refresh AGENT_HANDOFF.md, handoff-manifest.json, SPEC.md, and DEVELOPMENT_LOG.md
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3698b3cc73
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| def generate_all() -> None: | ||
| DATA_DIR.mkdir(parents=True, exist_ok=True) | ||
| cam = SyntheticCamera() |
There was a problem hiding this comment.
When this suite is used as evidence of real-camera readiness, it exercises no camera recordings: generate_all() constructs a SyntheticCamera, and every clip is rendered with the same synthetic helpers already used by the unit tests. The existing contract in docs/putting_launch_monitor/TURNOVER.md:39,93-97 explicitly says #5222 needs recordings from the #5221 operator session. Consequently, regressions involving real lighting, compression, backgrounds, hands, clubs, or ball detection can pass this entire corpus; retain these synthetic cases if useful, but populate the promised corpus with the operator clips, their calibration, and independently measured references.
Useful? React with 👍 / 👎.
| "expected_speed_mph": round(putt.speed_mph, 2), | ||
| "expected_hla_deg": round(putt.hla_deg, 2), |
There was a problem hiding this comment.
When this reproducible generator is rerun after a tracker or calculation regression, it executes the current PuttingMonitor and writes that same implementation's measured speed and HLA back as the expected values. The regenerated regression test will therefore bless systematic errors instead of detecting them; expected values should come from the prescribed synthetic inputs or, for the recorded corpus, independent operator reference measurements.
Useful? React with 👍 / 👎.
| return frames | ||
|
|
||
|
|
||
| def generate_all() -> None: |
There was a problem hiding this comment.
generate_all() spans 138 lines and combines specification construction, video rendering, replay execution, result validation, and manifest serialization, making changes to any one concern risky and difficult to test independently. Extract the positive-case generation, negative-case generation, and manifest writing into focused functions as required by the repository's function-responsibility limit.
AGENTS.md reference: AGENTS.md:L203-L207
Useful? React with 👍 / 👎.
1 participant
Summary
Resolves #5222 (part of epic #5218).
Builds the recorded-putt replay regression corpus and test suite for the Putting Launch Monitor tool:
src/putting_launch_monitor/tests/data/with 7 short MJPEG clips (< 750 KB each, ~2.8 MB total),calibration.json, andmanifest.json.src/putting_launch_monitor/tests/test_replay_corpus.pyreplaying each clip throughVideoFileSourceintoPuttingMonitor. Asserts 5 putts match expected speed (mph) and HLA (deg) within tolerance, and 2 negative controls (hand placing ball, ball rolling through unarmed) assert 0 accepted putts.@pytest.mark.slowso the fast default CI lane (-m \ not slow...\) runs the manifest schema/clip-existence test in milliseconds, while the slow lane (-m slow) exercises the full end-to-end replay suite.scripts/generate_putting_replay_corpus.pyto reproducibly render and calibrate corpus clips.src/putting_launch_monitor/README.mdwith step-by-step instructions on running the suite and adding new clips.src/putting_launch_monitor/AGENT_HANDOFF.md, refreshedmanuals/tools/handoff-manifest.json, added PR entry inSPEC.md, and updatedDL-#5218indocs/development/DEVELOPMENT_LOG.md.Verification
pytest src/putting_launch_monitor/tests(fast lane: 56 passed)pytest -m slow src/putting_launch_monitor/tests/test_replay_corpus.py(slow lane: 7 passed)ruff check src/putting_launch_monitor scripts/generate_putting_replay_corpus.py(passed)ruff format --check src/putting_launch_monitor scripts/generate_putting_replay_corpus.py(passed)black --check src/putting_launch_monitor scripts/generate_putting_replay_corpus.py(passed)mypy src/putting_launch_monitor scripts/generate_putting_replay_corpus.py(passed)python -m scripts.check_tools_handoff --check(passed)python -m scripts.build_tools_module_inventory --check(passed)python scripts/generate_tools_json.py --check(passed)python scripts/check_tools_manifest_layout.py(passed)python shared_scripts/spec_changelog.py validate --spec SPEC.md(passed)python shared_scripts/fleet_hooks.py fast(passed)