feat(hy3): parse reaction and relay takeoff times (E2 col 83-87, F2 cols 83-102) - #27
Open
fsalum wants to merge 9 commits into
Open
feat(hy3): parse reaction and relay takeoff times (E2 col 83-87, F2 cols 83-102)#27fsalum wants to merge 9 commits into
fsalum wants to merge 9 commits into
Conversation
Hy-Tek uses three diving stroke chars, not one: F = 1-metre springboard, G = 3-metre springboard, H = platform. The prior DIVING = "F", "6", 6 treated F as the only diving code and extrapolated numeric aliases "6"/6 from the FREESTYLE..MEDLEY pattern (1..5) with no corpus evidence Hy-Tek ever writes those in the stroke column for diving -- a wrong alias could silently mis-map some other numeric code onto diving. Measured on a real corpus: 2012 PAC-12 (F=26 G=26 H=26), 2007 SEC (F=60 G=60 H=54), 2005 PAC-10 (F=15 G=15 H=12) all show three parallel diving events. Across 13 multi-diving-event files, G never appears without F and H never appears without G (strict nesting). Dive counts match within a meet across chars, and median scores ascend F < G < H, tracking increasing degree of difficulty. HS meets emit only F, consistent with NFHS being 1-metre only. G and H previously fell through select_from_enum() to UNKNOWN, silently dropping all 3-metre and platform diving results. Drop the numeric aliases; use the character form only, since select_from_enum(Stroke, ...) is only ever called with a single extracted stroke-column char (hy3) or event_stroke field (hyv), never a numeric value, in the whole codebase.
Two hy3 line parsers raised on real-world Meet Manager exports instead of degrading gracefully: - f3_parser: an F3 relay leg referencing a swimmer meet-id with no D1 roster record raised KeyError. Skip the unrostered leg (the relay keeps its other legs), matching the existing tolerance for the empty-swimmers and absent leg-1 cases. - h1_parser: an H1 DQ-detail line whose last_entry carries no DQ slot raised an AssertionError. Make it a no-op, mirroring h2_parser -- an H1 can resolve to a non-DQ entry (e.g. a relay DQ, or a non-DQ entry emitted between the DQ result and its H1). The DQ result itself is unaffected; only the reason string is skipped. Adds regression unit tests for both.
Reaction and relay-takeoff columns need their own coercion. parse_time_or_none
requires a positive value, but a takeover slot records an early exchange as a
negative number -- 7,868 such values across a 33,008-file corpus -- so reusing
it would drop every one of them.
Sentinels are plural and slot-dependent: blank, 0.00 in any sign spelling, and
the literal NRT ("No Reaction Time") that Meet Manager writes into unmeasured
takeover slots in 513 files. All map to None.
Values above the plausible reaction range are passed through unchanged. A
minority of files put something else in these columns; filtering here would
make that population permanently invisible.
The parser read backup_4 through col 82 and then jumped to the date at col 88, dropping the five columns between. They hold the swimmer's start reaction time: 2.56% of 53.7M E2 rows corpus-wide carry a value in the plausible range. Includes an offset regression test. A one-column error still produces plausible-looking floats, so the neighbouring date is the tripwire that catches a misread.
The code already noted a '15-column gap' before F2's date field without
identifying it. That gap is relay legs 2, 3 and 4 (3 x 5 chars); leg 1 sits in
the 5-char slot F2 shares with E2.
The columns are reaction times, and the file format says so itself: 513 files
write the literal NRT ('No Reaction Time') into unmeasured takeover slots, and
leg 1 in those same rows carries a signed +0.00.
The physics corroborates it across the corpus. Leg 1 is a block start: 23
negatives out of 87,069, median 0.61. Legs 2-4 are exchanges: 2,702 / 2,282 /
2,861 negatives, median ~0.24. Nothing but a takeoff time behaves that way.
Exposed as a positional 4-list rather than four named attributes, matching the
file's uniform column layout.
Two PII-redacted slices of real meets, covering the two opposite shapes the reaction columns take. The dense fixture is a modern fully-automatic-timing meet: 35 of its 37 E2 rows carry a start reaction time (0.53-0.90), all four F2 takeoff slots are populated on every relay, and one relay has three negative takeovers. That last one is the regression guard -- a >0.0 filter on these columns would drop every early exchange and the tests would still look green without it. The NRT fixture is the opposite: an older generation that wrote the literal NRT into every takeover slot and a signed +0.00 into every leadoff, with the E2 reaction column blank throughout. Everything must parse to None. A raw-file tripwire asserts the sentinels are still physically present, so a regenerated fixture that lost them cannot pass by vacuous truth. Redaction extends the documented rules to two fields they had missed: the 5-char last-name prefix that E1 and each F3 relay-leg slot carry. No parser reads them, but they are real surname fragments. Verified by diffing every emitted line against its source line and asserting the differing character positions fall only inside the redacted column ranges.
E1 cols 9-13, and the same columns in each 13-char F3 relay-leg slot, hold the first five characters of the swimmer's last name. The documented redaction rule set did not cover them, so the five fixtures added before the rule existed still carried 109 distinct real surname fragments between them. No parser reads either field -- e1_parser takes the swimmer id at cols 4-8 and the event gender at col 14, and f3_parser takes the id and the leg number at col 15 -- so blanking them to Swimm, matching the D1 last_name placeholder Swimmer<meet_id>, is parse-safe. All 106 tests pass unchanged. Only those columns were touched: every file is byte-for-byte the same length, and a character-level diff against the previous revision confirms every changed position falls inside an E1 or F3 name-prefix range. Line checksums and all other record types are untouched. A full re-audit of all seven fixtures against the complete documented rule set now reports no unredacted personal field of any kind.
…ture Completes the previous commit, which covered five of the six fixtures that predate the E1/F3 name-prefix rule and missed this one. 221 occurrences, 142 distinct real surname fragments, all in F3 leg slots -- this fixture exists to exercise relays with alternates, so it is almost entirely F3 content. Two of its F3 lines carry a fifth leg, so anything checking only the first four slots will under-report it; all eight slots are covered here, matching f3_parser's own range. No parser reads cols 9-13 of a leg slot -- f3_parser takes the swimmer id at cols 4-8 and the leg number at col 15 -- so blanking them to Swimm is parse-safe. The file is byte-for-byte the same length, a character-level diff against the previous revision puts every changed position inside an F3 name-prefix range, and test_bug2b_relay_entries_with_alternates_preserve_leg_numbers, which reads this fixture, still passes along with the rest of the 106. Every fixture in the directory now satisfies the documented rule set.
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
1 participant
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.
Summary
e2_parserreadbackup_4_timethrough col 82 and then jumped straight to the dateat col 88, silently dropping the five columns in between.
f2_parserhad the samehole, plus a 15-column gap the code acknowledged in a comment but never identified:
Those columns are reaction times.
E2F2That accounts for the gap exactly: E2 carries one 5-char slot and its date follows
at col 88; F2 carries four, so its date sits 15 columns later at col 103 and its
alt_time_codeat col 111. No offsets are guessed — the previously-known fields onboth sides now abut the new ones with nothing left over.
Evidence that these are reaction times
The format says so itself. 513 files in a 33,008-file corpus write the literal
string
NRT— Meet Manager's "No Reaction Time" — into F2 takeover slots 2-4, andin those same rows slot 1 carries a signed
+0.00. A field that is named "noreaction time" when unmeasured is a reaction time when measured.
The physics corroborates it, and distinguishes the slots. Slot 1 is a block
start off the leadoff; slots 2-4 are relay exchanges, where a swimmer may leave the
block before the incoming touch. Across the corpus:
Leg 1 behaves like a start (essentially never negative, median 0.61s — a human
reacting to a signal). Legs 2-4 behave like exchanges (frequently negative, median
0.24s — a swimmer already moving). Nothing but a takeoff time produces that split,
and it independently confirms the slot ordering.
Density. 2.56% of 53.7M E2 rows corpus-wide carry a value in the plausible
reaction range — consistent with fully-automatic timing being available at a
minority of meets rather than the column being noise.
Design notes
Negatives are meaningful and must not be filtered. A relay takeover slot records
an early exchange as a negative number — 7,868 such values corpus-wide. The existing
parse_time_or_nonerequires> 0.0and would silently destroy every one of them,so these columns get their own coercion,
parse_reaction_time, rather than reusingit. The docstring says this in place, because the bug it prevents is invisible: a
> 0.0filter leaves a green test suite and just loses the data.Sentinels are plural and slot-dependent. Blank,
0.00in any sign spelling(
0.00/+0.00/-0.00all round-trip throughfloat()to zero), and theliteral
NRT. All map toNone. Unparseable junk (a bare+sign, stray highbytes — both observed) also maps to
None.Out-of-range values are passed through unchanged, deliberately. A minority of
files put something above the plausible reaction range in these columns: 80,706
values above 2.0 across the corpus, concentrated in 117 files that are 100%
implausible — i.e. a whole-file property, not scattered corruption. Its meaning is
unresolved. Clamping or dropping here would make that population permanently
invisible to anyone who wants to work it out, so the parser reports what the file
says and leaves the judgement to the caller.
Exposed positionally for F2.
finals_reaction_timesis a 4-list rather thanfour named attributes, matching the file's uniform column layout, and keeping the
list index equal to the relay leg number minus one.
New attributes
Per course prefix (
prelim/swimoff/finals), so six in total:<prefix>_reaction_time: Optional[float]— set bye2_parser<prefix>_reaction_times: Optional[List[Optional[float]]]— set byf2_parserBoth default to
None, so nothing changes for existing callers.Tests
Two new PII-redacted fixtures covering the opposite shapes these columns take:
mm_reaction_times_dense.hy3— a modern fully-automatic-timing meet. 35 of 37 E2rows carry a reaction time (0.53-0.90), all four F2 slots populated on every relay,
and one relay with three negative takeovers. That relay is the regression guard: a
> 0.0filter would drop every early exchange, and without this fixture the suitewould still look green.
mm_relay_nrt_sentinel.hy3— the opposite generation:NRTin every takeover slot,+0.00in every leadoff, E2 reaction column blank throughout. Everything must parseto
None. A raw-file tripwire asserts the sentinels are still physically present,so a regenerated fixture that lost them cannot pass by vacuous truth.
Plus unit tests for
parse_reaction_timeand an offset regression test on each ofE2 and F2. A one-column misread still yields plausible-looking floats, so the tests
assert the neighbouring date field parses correctly — that is the tripwire that
actually catches a shifted offset.
Full suite: 106 passing.
Also included: fixture redaction fix
While building the new fixtures I re-audited the existing ones against the documented
rule set in
tests/hy3/fixtures/README.mdand found the rules had missed two fields:E1cols 9-13, and the same columns within each 13-charF3relay-leg slot, both ofwhich hold the first five characters of the swimmer's last name. Six fixtures added
before the rule set existed still carried real surname fragments there.
No parser reads either field —
e1_parsertakes the swimmer id at cols 4-8 and theevent gender at col 14, and
f3_parsertakes the id and the leg number at col 15 —so they are blanked to
Swimm, matching the existingD1last-name placeholder.Verified non-destructive: every file is byte-for-byte the same length, and a
character-level diff against the previous revision confirms every changed position
falls inside an E1 or F3 name-prefix range. All other record types and all line
checksums are untouched, and the suite passes unchanged.
The README now documents both fields and states that the rule applies to every
fixture in the directory.
Merge order
This is stacked on
fix/relay-parse-robustness(#26), which is itself stacked onfeat/diving-stroke(#25). Please merge #25, then #26, then this one.It targets
masteronly because a cross-fork PR's base must be a branch that existsin this repo, and
fix/relay-parse-robustnesslives on the fork — the same reason#26 targets
masterdespite sitting on top of #25. As a result the diff shown herealso contains the three commits belonging to #25 and #26:
47fdc13,87c4f76— feat: add board-specific diving stroke codes (F/G/H) #25b259ef5— fix(hy3): tolerate malformed relay F3 legs and orphaned H1 DQ lines #26Only the six commits from
3856af2onward are new in this PR, and they are theones described above. Once #25 and #26 land, this PR's diff collapses to just those
six with no rebase needed.