Expand database with new sources and single-monitor aggregate ingestion - #10
Conversation
Extend the build pipeline with a new optional input, data/studies/aggregate_observations.csv, so redistributable group-mean statistics from single-monitor studies can join metric_observations alongside the paired TrackMan/FlightScope comparisons. Adds a cohort dimension, allows missing standard deviations, and fail-closed validation for provenance, metrics, and unit convertibility. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RRnmawXtzrcRJatcGNYVZ
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RRnmawXtzrcRJatcGNYVZ
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bfe2f0cdd6
ℹ️ 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 _observation_id( | ||
| source_id: str, club: str, metric: str, vendor: str, cohort: str = "" | ||
| ) -> str: |
There was a problem hiding this comment.
Include the monitor model in observation IDs
When one source reports the same metric and cohort for two models from the same vendor, the duplicate check accepts both rows because its key includes monitor_model, but _observation_id omits that field and generates identical primary keys. The subsequent SQLite insert therefore fails with a uniqueness error for otherwise distinct observations; either include the model in the aggregate ID or align the validation key with the actual identity.
Useful? React with 👍 / 👎.
| "monitor_model": model, | ||
| "software_version": row["software_version"], | ||
| "environment": row["environment"], | ||
| "cohort": "single_participant", |
There was a problem hiding this comment.
Rebuild the committed database artifacts
When users consume the committed artifacts advertised by the repository instead of running the builder, they still receive the old schema: both observation CSV headers omit cohort, and the committed SQLite table also omits cohort while retaining NOT NULL constraints on both SD columns. Rebuild and commit these artifacts so the published database matches the schema produced by this code.
Useful? React with 👍 / 👎.
| float(row["reported_mean"]) | ||
| if row.get("reported_sd", "").strip(): | ||
| float(row["reported_sd"]) |
There was a problem hiding this comment.
Reject invalid aggregate numeric values
When an aggregate row contains NaN, infinity, or a negative standard deviation, these float calls accept it and validation reports success. Non-finite means can then fail the SQLite build because Python binds NaN as NULL, while infinities and negative SDs can be persisted as invalid statistics and corrupt downstream results; require finite means and finite, non-negative SDs.
Useful? React with 👍 / 👎.
| for column in ("monitor_vendor", "monitor_model", "cohort", "club"): | ||
| if not row.get(column, "").strip(): | ||
| errors.append(f"{prefix}: missing {column}") |
There was a problem hiding this comment.
Validate all required aggregate metadata
When an aggregate CSV omits or leaves blank software_version, environment, or measurement_status, this required-field loop does not detect it. Missing columns consequently pass validation and later raise KeyError during normalization, while blank values are silently written into the supposedly required provenance columns; include these fields in the validation checks.
Useful? React with 👍 / 👎.
Catalog additions from a structured web sweep: - 12 official vendor documentation sources and 166 new vendor-field records covering Uneekor EYE XO2 and EYE MINI, SkyTrak+, Bushnell Launch Pro, Full Swing KIT, Voice Caddie SC4 PRO, Rapsodo MLM2PRO, Garmin Approach R10 and R50, Square Golf, TrackMan iO, and Foresight Falcon; Rapsodo MLM2PRO rows upgraded from gist-observed evidence to the official specification page. - 15 public shot-level datasets (TrackMan, Mevo+, Garmin R10/R50, SkyTrak, Rapsodo MLM2PRO, Square Golf, Awesome Golf, OpenFlight, CaddieSet) cataloged reference-only with byte-level SHA-256 pins and commit pins where GitHub-hosted; CaddieSet MIT and OpenFlight AGPL licenses verified from repository LICENSE files. - 14 open-access study candidates (MDPI, Frontiers, IUSCA, IJGS, ISBS, and journal reliability studies) cataloged reference-only pending direct license and table verification. Code review fixes: aggregate observation IDs now include monitor model and cohort; aggregate validation rejects non-finite means, negative or non-finite standard deviations, and missing provenance metadata; tests cover the new ingestion path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019RRnmawXtzrcRJatcGNYVZ
What Changed
data/studies/aggregate_observations.csvlets redistributable single-monitor group statistics joinmetric_observations, with acohortdimension, nullable SDs for mean-only tables, and fail-closed validation (unknown/non-redistributable sources, unknown metrics, non-convertible units, non-finite means, negative SDs, missing provenance metadata, duplicate keys).Source and Rights
All new sources are
reference_only. No new observation rows were added, because the collection environment's egress policy blocks publisher and vendor domains, so numeric tables could not be verified byte-level — and this repository does not ingest numbers nobody has actually seen. Licenses that could be verified byte-level were: CaddieSet (MIT, from the repo LICENSE) and OpenFlight (AGPL-3.0-or-later). Vendor pages and studies cataloged from search-index snapshots say so explicitly in their limitations and record their expected licenses (for example MDPI/Frontiers CC BY 4.0 defaults) pending direct verification. Several datasets contain player names; their limitations note this and they are not redistributed. The Blackmore 10,169-shot TrackMan pin was re-verified today (SHA-256 unchanged).Monitor, Conditions, Units, and Measurement Status
New vendor-field records use source language for measurement claims (measured versus calculated versus derived), record package gating (for example Bushnell Club Data package, Foresight Club Analysis license, Voice Caddie app-only metrics), and note ball/sticker requirements (Titleist RCT, Callaway RPT, club markers). The aggregate ingestion schema records monitor vendor/model, software version, environment, cohort, club, reported unit, sample count, measurement status, and matched-shot flag per row, converted deterministically with
to_canonical.Validation
ruff check .python -m pytest(14 tests)launch-monitor-data validate(56 sources, 57 comparisons, 261 vendor fields)🤖 Generated with Claude Code
https://claude.ai/code/session_019RRnmawXtzrcRJatcGNYVZ