From e188e0a994136f36a4e3e057cf3cbc88665afaf0 Mon Sep 17 00:00:00 2001 From: Max Winterstein <5927148+MaxWinterstein@users.noreply.github.com> Date: Fri, 25 Sep 2026 08:30:24 +0000 Subject: [PATCH 1/4] Report a missed turn instead of hiding it, and stop writing its stroke A swim came back 50 m short and the page said nothing. Lap 12 held a 151 s length with 51 strokes against a unit of ~79 s and a median of 28 strokes -- two lengths the watch recorded as one. The per-lap estimate saw three lengths there and was capped, correctly, at the two recorded, because this tool merges and never splits. The problem was the silence, and what followed from it: 51 strokes in "one length" cleared the breaststroke threshold, so the repair wrote breaststroke into a swim the swimmer confirms was freestyle throughout. The file came out worse than the watch left it. A single length now reads as a missed turn when it runs at least 1.75x the unit in duration *and* 1.75x the median in strokes. Both halves are needed. Duration alone fires on an 18 m pool, which reaches 1.61x on its own, and on a kick set or a pause at the wall, which are long without the strokes; the tempting "rounds to two lengths" at 1.5x would have flagged both short-pool fixtures. It is reported, never split -- splitting means inventing a turn time and a stroke split nobody recorded -- as a finding that accounts for the shortfall exactly (21 written + 1 reported = the confirmed 22) and as a marked row in Show the working. It works under a fixed lengths-per-lap too, since how the lap button was pressed has nothing to do with a missed turn. Fixing the stroke found an older bug of the same shape. The page has always said an ambiguous stroke -- count and duration disagreeing -- keeps the watch's label. It did not: repair() re-derived the stroke from the count and wrote that, and two ambiguous groups in the fixtures had the watch's breaststroke overwritten with freestyle. analyze() now decides the stroke per merged group once, including when to leave it alone, and repair() writes exactly that, the way it already takes lapTargets instead of recomputing them. keepStrokeWhenUnsure (default on) covers both cases; the Python reference writes the verdict unconditionally, so AS_REFERENCE turns it off and the goldens stay byte-exact. Default: 12 of 12 ambiguous groups keep the watch's label; as the reference: the same 2 overwrites as before. The swim is fixture swim-08, anonymized outside the tree and renamed on the way in. Each part of the rule has a test that fails without it: detection off, the ratio lowered to 1.5, the label kept off, and the stroke half of the rule removed -- the last needed a doctored kick-set length, since no real fixture exercises it. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 17 +++ README.md | 5 +- packages/fitfix/src/cli.mjs | 23 ++- packages/fitfix/src/swim-repair.js | 121 ++++++++++++++-- packages/fitfix/test/fixtures.mjs | 10 +- packages/fitfix/test/fixtures/swim-08.fit | Bin 0 -> 13171 bytes packages/fitfix/test/missed-turn.test.mjs | 167 ++++++++++++++++++++++ web/app.js | 32 +++-- web/style.css | 18 +++ 9 files changed, 373 insertions(+), 20 deletions(-) create mode 100644 packages/fitfix/test/fixtures/swim-08.fit create mode 100644 packages/fitfix/test/missed-turn.test.mjs diff --git a/AGENTS.md b/AGENTS.md index be31a0e..1a271f5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -158,6 +158,8 @@ PYTHONPATH=/tmp/pylibs pkgx +python.org -- python3 \ **The reference hardcodes its assumptions, and the JS has since moved past several of them.** `AS_REFERENCE` in `fixtures.mjs` pins them all back: +`keepStrokeWhenUnsure: false` (it writes the stroke-count verdict even when +the duration disagrees, where the JS now keeps the watch's label), `lengthsPerLap: 1` (it merges every active length in a lap, full stop), `strokeSplit: 40` and `durationSplit: 100` (per-length constants fitted at 50 m, where the JS's scaled defaults happen to land on exactly the same @@ -187,6 +189,7 @@ assumption is worth a lot. | swim-05 | **18 m**, recorded as 20 m | short pool, **wrong pool size**, and the first file to break the unit estimator | **open** — watch 64, time 63.7, strokes 63.8 | | swim-06 | 18 m, recorded correctly | short pool where nearly every recorded length is already a real length, so *any* merging is wrong | **open** — watch 63, time 61.4, strokes 64.5 | | swim-07 | 50 m | nearly clean: one true phantom split among thirty single-length laps, plus two genuine blocks (3 and 2 lengths) from swimming through the turn — laps end at rests, not button presses, so multi-length laps are normal use and a blanket merge destroys real distance | 17 / 850 m | +| swim-08 | 50 m | the first **missed turn**: lap 12 holds a 151 s length with 51 strokes, twice a normal length in both. Nothing merges it -- the tool cannot split -- so the file comes out one short and the `missed-turn` finding has to account for the difference exactly | 22 / 1100 m, all freestyle (21 written + 1 reported) | ### How the length unit is chosen, and why it changed twice @@ -311,6 +314,20 @@ for screenshots and for checking a deploy. real distance. It reports rather than refuses, because the data alone cannot distinguish "many phantom turns" from "a different lapping habit". Fixture 1 trips it legitimately. Keep it loud in every front end. +- **A missed turn is reported, never repaired.** Two lengths recorded as one + show up as a single length at ~2x the unit *and* ~2x the median stroke count + (`MISSED_TURN_RATIO`, 1.75). Both halves matter: a kick set or a pause at the + wall is long without the strokes, and an 18 m pool reaches 1.61x on duration + alone. The threshold rests on one positive example (swim-08) and seven + negatives -- do not lower it without a second real one. And the stroke of + such a length is left as the watch said: its count covers two lengths, so + measured against a per-length threshold it reads as breaststroke, which is + exactly what used to get written into an all-freestyle swim. +- **Stroke decisions are made once, in `analyze()`.** `strokeWrite` maps each + merged group to the stroke to write, or `null` to keep the watch's label, and + `repair()` writes that rather than re-deriving it. It re-derived it once, and + the page promised the watch's label was kept for ambiguous strokes while the + file got the stroke count's verdict regardless. - **Second vs millisecond resolution.** `length.start_time` is in whole seconds, `lap.total_elapsed_time` in milliseconds. Lap boundaries are derived from the *next* lap's `start_time` to stay in whole seconds throughout. A diff --git a/README.md b/README.md index 6dd05bc..ec35c42 100644 --- a/README.md +++ b/README.md @@ -161,7 +161,10 @@ scrubbing step that is not optional. heuristic tuned on seven files — check the before/after numbers, and put a number in if you disagree. - **It only ever merges, never splits.** If the watch *missed* a turn and - recorded two lengths as one, nothing here will recover it. + recorded two lengths as one, the file stays one length short — splitting it + would mean inventing a turn the watch never recorded. It does now *say* so: a + length that runs as long as two, in both time and strokes, gets a "possible + missed turn" finding, and its stroke is left as the watch recorded it. - The freestyle/breaststroke split defaults to `auto`, which scales it from the pool length — 40 strokes per length at 50 m, proportionally fewer in a short pool. The constant behind it was still fitted on one swimmer, so a fixed diff --git a/packages/fitfix/src/cli.mjs b/packages/fitfix/src/cli.mjs index 4ab7264..daacd62 100755 --- a/packages/fitfix/src/cli.mjs +++ b/packages/fitfix/src/cli.mjs @@ -199,9 +199,12 @@ function printWorking(info) { console.log(' lap watch fixed the lengths it saw (s) rest laps not shown'); for (const l of info.swimLaps) { const merged = l.lengths > l.target; - const seen = l.lengthsS.map((s) => (s === null ? '?' : Math.round(s))).join(' + '); + const seen = l.lengthsS + .map((s, i) => (s === null ? '?' : `${Math.round(s)}${l.missedTurns[i] ? '!' : ''}`)) + .join(' + '); + const missed = l.missedTurns.some(Boolean); console.log( - ` ${String(l.lap + 1).padStart(3)} ${String(l.lengths).padStart(5)} -> ${String(l.target).padEnd(5)} ${seen}${merged ? ' merged' : ''}`, + ` ${String(l.lap + 1).padStart(3)} ${String(l.lengths).padStart(5)} -> ${String(l.target).padEnd(5)} ${seen}${merged ? ' merged' : ''}${missed ? ' possible missed turn' : ''}`, ); } } @@ -218,6 +221,22 @@ if (opts.lengthsPerLap === 'auto' && info.lengthUnitS) { ); } +/* + * Also loud, for the opposite reason: the file comes out short and this tool + * cannot fix it. It merges; it never splits -- that would mean inventing a + * turn the watch never recorded -- so the most it can do is say so. + */ +for (const f of info.findings.filter((f) => f.type === 'missed-turn')) { + console.warn(''); + console.warn( + ` !! lap ${f.lap + 1}: possible missed turn -- one length took ${Math.round(f.durS)} s`, + ); + console.warn( + ` !! with ${f.strokes} strokes, about ${f.looksLike} lengths' worth (one is ~${Math.round(f.unitS)} s).`, + ); + console.warn(` !! The distance is probably ${f.looksLike - 1} length(s) short. Not fixed.`); +} + // Loud, before anything else: this is the case where the output is garbage. const structure = info.findings.find((f) => f.type === 'lap-structure'); if (structure) { diff --git a/packages/fitfix/src/swim-repair.js b/packages/fitfix/src/swim-repair.js index a2d9a90..b635d70 100644 --- a/packages/fitfix/src/swim-repair.js +++ b/packages/fitfix/src/swim-repair.js @@ -92,6 +92,21 @@ export const DEFAULTS = { poolLength: null, /** Overwrite the watch's own stroke classification. */ reclassifyStroke: true, + /** + * Leave the watch's stroke label alone when the evidence for changing it is + * unreliable: the stroke count and the duration disagree, or the length + * looks like two lengths the watch recorded as one (a missed turn), so its + * stroke count covers two lengths and means nothing against a per-length + * threshold. + * + * The page has always told people the watch's label is kept when the two + * criteria disagree; until this option existed it was not -- the stroke + * count's verdict was written anyway, and two ambiguous groups in the + * fixtures had the watch's breaststroke overwritten with freestyle. The + * Python reference writes the verdict unconditionally, so AS_REFERENCE turns + * this off. + */ + keepStrokeWhenUnsure: true, /** Set elapsed time to timer time when the timer was never paused. */ normalizeElapsed: true, }; @@ -201,6 +216,21 @@ export function mergeToTarget(indices, target, durationOf) { const STROKES_PER_100M = 80; const SECONDS_PER_100M = 200; +/** + * How far past one length -- in duration *and* in stroke count, both -- a + * single recorded length has to run before it reads as two lengths the watch + * recorded as one, a missed turn. + * + * Both, because either alone fires on real swims. Across the first seven + * fixtures the longest single length relative to its swim's unit is 1.61x + * (swim-05, an 18 m pool, where turn and push-off are a large share of a + * length) and breaststroke runs to 1.47x; "rounds to two", at 1.5x, would + * flag both short-pool files. swim-08's missed turn is 1.92x the unit with + * 1.9x the median stroke count. 1.75 sits between -- on one positive + * example, which is why this only ever reports and never splits. + */ +const MISSED_TURN_RATIO = 1.75; + /** Resolves a threshold that may be 'auto', scaling it to one length. */ const perLength = (setting, poolM, per100) => Number.isFinite(setting) ? setting : (per100 * poolM) / 100; @@ -410,6 +440,42 @@ export function analyze(u8, opts = {}) { alternative, } = resolveLapTargets(lapActive, durationOf, o.lengthsPerLap); + /* + * Lengths that look like two recorded as one. The reference length has to + * exist even when lengths-per-lap is a fixed number, since a missed turn + * has nothing to do with how the lap button was pressed -- so it is + * estimated here if auto did not already produce one. + */ + const refUnit = unit ?? estimateLengthUnit(lapActive, durationOf).unit; + const knownStrokes = lapActive + .flat() + .map((k) => getField(lengths[k], F.length.strokes)) + .filter((s) => s !== null) + .sort((a, b) => a - b); + const medianStrokes = knownStrokes.length + ? knownStrokes[Math.floor(knownStrokes.length / 2)] + : null; + const missed = new Set(); + if (refUnit && medianStrokes) { + for (const k of lapActive.flat()) { + const s = getField(lengths[k], F.length.strokes); + if ( + durationOf(k) >= MISSED_TURN_RATIO * refUnit && + s !== null && + s >= MISSED_TURN_RATIO * medianStrokes + ) + missed.add(k); + } + } + + /* + * The stroke each merged group gets written as, keyed by the group's first + * length -- the one that survives. null means "leave the watch's label". + * Decided once here and handed to repair(), like lapTargets, so the page + * cannot say a label is kept while the file says otherwise. + */ + const strokeWrite = new Map(); + const findings = []; const swimLaps = []; lapActive.forEach((act, li) => { @@ -452,8 +518,27 @@ export function analyze(u8, opts = {}) { // null where no stroke count was recorded, as lengthsS does for a missing // duration; the lap total above keeps treating it as 0. lengthStrokes: act.map((k) => getField(lengths[k], F.length.strokes)), + /** Per recorded length: does it look like two lengths recorded as one? */ + missedTurns: act.map((k) => missed.has(k)), }); + for (const k of act) { + if (!missed.has(k)) continue; + const durS = durationOf(k); + findings.push({ + type: 'missed-turn', + lap: li, + durS, + strokes: getField(lengths[k], F.length.strokes), + looksLike: Math.round(durS / refUnit), + unitS: refUnit, + note: + 'one recorded length runs as long as two, in both time and strokes -- ' + + 'probably a turn the watch did not see. Nothing is split: that would ' + + 'mean inventing a turn the watch never recorded.', + }); + } + if (act.length > target) findings.push({ type: 'phantom-turn', @@ -483,7 +568,15 @@ export function analyze(u8, opts = {}) { (k) => SWIM_STROKE[k] === getField(lengths[group[0]], F.length.swimStroke), ); - if (byStrokes !== byTime) + const ambiguous = byStrokes !== byTime; + const holdsMissedTurn = group.some((k) => missed.has(k)); + const keep = o.keepStrokeWhenUnsure && (ambiguous || holdsMissedTurn); + strokeWrite.set(group[0], keep ? null : gStroke); + + // A missed turn doubles the stroke count, so a stroke finding about it + // would be reporting on an artefact -- the missed-turn finding covers it. + if (holdsMissedTurn && keep) continue; + if (ambiguous) findings.push({ type: 'ambiguous-stroke', lap: li, @@ -607,6 +700,8 @@ export function analyze(u8, opts = {}) { * unit to mean "fixed" told the swimmer they had set a number they had not. */ autoLengths: o.lengthsPerLap === 'auto', + /** Group's first length -> stroke to write, or null to keep the watch's. */ + strokeWrite, }; } @@ -675,11 +770,21 @@ export function repair(u8, opts = {}) { [F.length.avgSpeed]: durS > 0 ? (poolM / durS) * 1000 : 0, [F.length.cadence]: durS > 0 ? (strokes * 60) / durS : 0, }; - if (o.reclassifyStroke) - spec[F.length.swimStroke] = - SWIM_STROKE[strokes >= strokeSplit ? 'breaststroke' : 'freestyle']; + // analyze() decided the stroke per group, including when to leave the + // watch's label alone; recomputing it here is how the two used to + // disagree. + const decided = info.strokeWrite.get(k); + const watchSaid = Object.keys(SWIM_STROKE).find( + (n) => SWIM_STROKE[n] === getField(lengths[k], F.length.swimStroke), + ); + if (o.reclassifyStroke && decided) spec[F.length.swimStroke] = SWIM_STROKE[decided]; newSpecs.set(k, spec); - newInfo.push({ durMs, active: true, strokes }); + newInfo.push({ + durMs, + active: true, + strokes, + stroke: decided ?? watchSaid ?? (strokes >= strokeSplit ? 'breaststroke' : 'freestyle'), + }); mine.push(idx); } } @@ -716,9 +821,9 @@ export function repair(u8, opts = {}) { [F.lap.avgCadence]: ratio(strokes * 60, swimMs / 1000), }; if (act.length && o.reclassifyStroke) { - const kinds = new Set( - act.map((k) => (newInfo[k].strokes >= strokeSplit ? 'breaststroke' : 'freestyle')), - ); + // From what each surviving length now says, so a lap never claims a + // stroke its own lengths do not. + const kinds = new Set(act.map((k) => newInfo[k].stroke)); p[F.lap.swimStroke] = SWIM_STROKE[kinds.size === 1 ? [...kinds][0] : 'mixed']; } return p; diff --git a/packages/fitfix/test/fixtures.mjs b/packages/fitfix/test/fixtures.mjs index ae0e0ea..ada0596 100644 --- a/packages/fitfix/test/fixtures.mjs +++ b/packages/fitfix/test/fixtures.mjs @@ -24,6 +24,7 @@ export const ORIGINALS = [ 'swim-05.fit', 'swim-06.fit', 'swim-07.fit', + 'swim-08.fit', ]; /** @@ -67,7 +68,14 @@ export const POOL_OVERRIDE = { 'swim-05.fit': 18 }; * `strokeSplit: 40` matters only for swim-05: at 50 m the scaled default works * out to exactly 40, so the older fixtures never noticed. */ -const AS_REFERENCE = { lengthsPerLap: 1, strokeSplit: 40, durationSplit: 100 }; +const AS_REFERENCE = { + lengthsPerLap: 1, + strokeSplit: 40, + durationSplit: 100, + // The reference writes the stroke-count verdict even when it disagrees with + // the duration; the JS now keeps the watch's label there by default. + keepStrokeWhenUnsure: false, +}; export const GOLDEN = { 'swim-02.fit': { diff --git a/packages/fitfix/test/fixtures/swim-08.fit b/packages/fitfix/test/fixtures/swim-08.fit new file mode 100644 index 0000000000000000000000000000000000000000..24049ec0bf8b255bbc79340b72fd016b32e8e25f GIT binary patch literal 13171 zcmcIrX^>sTbw1}zzj-rHGnyIAs%11IiILF89!Y=@5{p29B)}*{5H<-dAOVs9tw>i` z{Vrf+ynsPfR1kJd+29|Jmrz-Z4aT9c30^~0>{KeLR4PuY5)84D$`9Kw-|2gMrtj0c z?@h^~tIq4b_bk1f)7|HGpEt4d^&2;yPjuGGRX4r$ugeLk*kq0x6BkX)Vnu90EU_Yx zesz4Lg2eRs&Pv6sAUc-{(o-a{%+kbUc$73@NPpK)0M}(iGpY1j6O)MxCT>|?)C!mq zzGfCYi&in~=Yupn;l+`Rh*Nk#tEaH5_b9DKy<*Burd%-PW>YSja<3^5nDP`;o@&a| zOnK0hhfR5wiO(?ch$+uD%^|sp%e_2VQFEWz|_Xn&eFv^iKUyR zhoz6DpJg)3bmk%E87woI<}%G=Ig907mh)K7XS#st4_KD4T+DPS%W{?#ENfXVCz?bD zU1wu-)@V=T78AFcxMX7Vak?oFxt=B+oNHoq@M06MG4cI}i#tsGkdcRt>^8E;$X;Xj z8T*Kd51ROxiN9>hKQr+!O#C~s6U0sw+ab0`Y_HfpvHfBvi=85NhS-^6XNf&S?1kJ#NI0QUa{N6ZWp^#tQA`p8;gBT?4O8zLF~(7UlIE?v0oSa z4YA)8`pI#Q< zmRk8M=*P3gq`s@TUHjw`(Qih}`hsW%^^d+Gdc3<*>8Ib(#?f~PU-Ik5CVb(_7f=fq zx^_I)Tj{7!g`#MTH;R;k3i<}GG0(@+6-0ke?K%mj#f9OxwbSYf<*c>2SxQV3SlU=H z^CmK3S=vNhe#XRD`e5A!vCYoL6T5~{V_}2E6`5KY62MMwtx|38?m_F94O~g2(3~fUKR7<}FXu5S6w#(fLP@=;r0YSn?j|OM z7Me!OLZa@J=u04ab{>;LD~O&7iF}(*fM{d`kwPi2P3<2ZA31k6lfneF=~yjEe*~hP zi-;83@+55r(NEV1DYVo1bnU36L3cfA!+#5+ZTp!NCSsQ}VRe28ZvXG&BOfj&Qs~I5 z?g0?3?`2Zxqyc(8l;o>>6+|nR5h--#NqP%JFJPxon1mrX5K8iG>izZck%cRn6uL3u zH-|*Nx~(8Ob`g<6PhNFuZTC**6K^o_W+PjSY&CL^k$a8Z zX6*eYe!#@=qjs8jmx({`ouxLh?apQD4qT?;z*&N)bf$9};W5HdT;rT2xJh@4y-Vyy zvAf0Y5xZCHL9vI#J}&kNv7ZwA8L`j%{i%ax)E(+)8i3tn!(o}tG>7Rt=JT1aX1bo` z29_IHZe_~*R25=6)Kgk#AR?)NinKa;4hEgRdD;XZsw2y-ZAi- z`{UiZhUAZg(t$5Gy)Ojk;N<@3o|$Dz){JgWS?Wqf;J0jjsNanQ)f|aY zCj_if2;M14hCRP?im$FVd=B`-*JxVS-SLXWI5>%3s?Rgb?t##`9lKC@u9{uv)OiAZ z6oPlARC7~Zby>WVj~}J;U33X}dt`s}5uI3Iu)=+cm^r50C z1n&guIBE)UX&IT0_sI=+i1bJCn!=r>a2n~ z3c)+Cjpm%vfiE|`3au$r6BKeZS3u`f;)ax?{+v5_xX*R2&YV*x8^1JFkyy!*8#Qc!>LV@U@x zc+k3}G`?Ib-wv%kRYP;ds%^efN#y89tCqH>QhRgfTr)bi`*3d~&FQBI8Wx4%KTU#> z&)`AplHz^2Rvz$Y&@DN0uH+cTqdEPA-Le;z*VmkWLZRj;1pi6ZXth+jOslINjzMb* zRj(_pRbQEU)TlHQ&^aw_PC4qYmUnJ$IOp`!4|1ar{HI_r@);ZXoM)PVr+Eo?Hs}GsuZKn(J~5_5e|N*xQV-lvQ51qd zfD1-Gg9oikiudJOdBC4RPs-6;pJN!W=C(hK%Ij-R4+>Fp6oNlERL$uM7UI$}vPwni zTp{P@J!nm#8r_|&6ol08BIVy8!D0&bM<|u@JIxM6xyoP8= z(zpyBv@R))FW1W7#JPdqELP+CtL_T);6Z8hyhcllQ;wr2ae7I*&pp)GL(vm;utXvF z6LuX(O(8BVBRh4?4diHUD6P#8htFMQnIF~Mmd5*>-UNUz3c=qD=$z9PEX1W{WR;52 zxkAp*3CuTzYV_oseQp?asl@du$G@G#=_To$+uQg{TyHSI5{2MzG~7Ve>73$yb+z(< zKZBl~CTkt(HocX?4}ZqtKc{)oaYH?2V{jEoY*0 zDpA*XP4!pHJ%<{8iR+CK+T!rJ&oX)u&jXe~-=}H?M z`P|=pHJWou2cF#YOK44@nqV+z&YgkIsl;1Sj{0+M&!L9!IAecffV7j&G?qqNqFy!QYU6RGJ%31}x}vG648Cb{Ib#J`V`fqRu(=td8a` zBN}F-H@SKrstMP7S8)s8G#QQTLD*pufF~@NURMyUpY(@~E3?ncytScNC9lAzI=`~=}#bBy~=>*`n;3b__ z@$7;gPOErrXYxgibB&TdomTNof7cTbELc*mGSx>BVZiX*6wt zxs}O9tfjG{Np;RflDmCE9MW{AAw*>`%|b*4LLt(KQH_FJfZ!e%3wbsZd;u3XN{g5l zGc93S&a{H*a!dtAWD{M7KoN}&AcRcb$n-HL4cTJ4nF+CzcQW0@r43A*5b4CUo#{TN z` zk?Fsf-baiJ)32FMU_kW7XznzPoQ)CpG0()8|E7pZJR~-W-9XgFZ0Kupku{K6oKWk+$8okj3_x%omaFHZsHlLmDa4Y}}?5X>L&vljoXUS{Rs<#v?b6#Uo$DbTP{^ zmP;AyF0IYjlHA1N42kfaxR%bwOwrZRlqZ>Tw+qMaGv&!9p5jJ(x`|yZHlna&65Ave z6QtF3HVLIc+;P7gNn(vh}1-fs|g*1H2g1tCc<1`@rfxQ@*D;YWkXNSl*3h0gL-@1 zT8uf_&iok5<1BCn{sSwkX~({Q&|$@>G>E~rj@^H&dx-!r7@S+^89qRjM}b|wj#p6p zB3vugz1v3ga02_sN8}Z>gmr0sK8?rrl(xQCa9YKm>=pdp; + readFit(u8) + .frames.filter((f) => f.kind === 'data' && f.globalNum === MSG_LENGTH) + .filter((f) => getField(f, F_TYPE) === ACTIVE) + .map((f) => getField(f, F_STROKE)); + +test('missed turn: swim-08 reports exactly the one it has', async () => { + const info = analyze(await readFixture('swim-08.fit')); + const missed = info.findings.filter((f) => f.type === 'missed-turn'); + assert.equal(missed.length, 1); + assert.equal(missed[0].lap + 1, 12, 'lap 12, as the swimmer said'); + assert.equal(missed[0].looksLike, 2); + assert.equal(Math.round(missed[0].durS), 151); + + const lap = info.swimLaps.find((l) => l.lap === missed[0].lap); + assert.deepEqual(lap.missedTurns, [true, false], 'the 151 s length, not the 97 s one'); +}); + +test('missed turn: the report accounts for the shortfall exactly', async () => { + /* + * The repaired file is still short -- that is the point of reporting rather + * than fixing -- but what it writes plus what it reports as missing has to + * come to the confirmed truth. If it does not, the finding is noise. + */ + const u8 = await readFixture('swim-08.fit'); + const { summary, info } = repair(u8); + const reportedMissing = info.findings + .filter((f) => f.type === 'missed-turn') + .reduce((a, f) => a + f.looksLike - 1, 0); + + assert.equal(summary.lengths, 21, 'still one short: nothing is split'); + assert.equal(summary.lengths + reportedMissing, 22, 'confirmed: 22 lengths'); + assert.equal((summary.lengths + reportedMissing) * info.poolM, 1100, 'confirmed: 1100 m'); +}); + +test('missed turn: its stroke is left as the watch said', async () => { + // 51 strokes in "one length" clears the breaststroke threshold, so the + // classifier used to write breaststroke into an all-freestyle swim. + const u8 = await readFixture('swim-08.fit'); + assert.ok( + activeStrokes(u8).every((s) => s === FREESTYLE), + 'the watch said freestyle throughout', + ); + + const { bytes, info } = repair(u8); + assert.ok( + activeStrokes(bytes).every((s) => s === FREESTYLE), + 'and the repaired file still does', + ); + assert.equal( + info.findings.filter((f) => f.type === 'stroke-mismatch').length, + 0, + 'no stroke finding about an artefact of the missed turn', + ); +}); + +test('missed turn: none of the other fixtures trip it', async () => { + /* + * The threshold sits between swim-08's 1.92x and the 1.61x an 18 m pool + * reaches on its own. This is the test that would fail first if it were + * lowered to the tempting "rounds to two lengths" at 1.5x. + */ + for (let n = 1; n <= 7; n++) { + const name = `swim-0${n}.fit`; + const pool = POOL_OVERRIDE[name] ? { poolLength: POOL_OVERRIDE[name] } : {}; + for (const lengthsPerLap of ['auto', 1, 2]) { + const found = analyze(await readFixture(name), { ...pool, lengthsPerLap }).findings.filter( + (f) => f.type === 'missed-turn', + ); + assert.equal(found.length, 0, `${name}, lengthsPerLap ${lengthsPerLap}`); + } + } +}); + +test('missed turn: still detected when lengths per lap is a fixed number', async () => { + // How the lap button was pressed has nothing to do with a missed turn, so a + // fixed target must not switch the check off. + const info = analyze(await readFixture('swim-08.fit'), { lengthsPerLap: 1 }); + assert.equal(info.findings.filter((f) => f.type === 'missed-turn').length, 1); +}); + +test('unsure strokes keep the watch label by default, and not as the reference', async () => { + /* + * The page has always said an ambiguous stroke keeps the watch's label. It + * did not: in swim-04 lap 17 and swim-05 lap 1 the watch said breaststroke + * and the repair wrote freestyle. keepStrokeWhenUnsure makes the promise + * true; turning it off reproduces the Python reference, which is how the + * goldens stay byte-exact. + */ + for (const [name, opts, lap] of [ + ['swim-04.fit', {}, 17], + ['swim-05.fit', { poolLength: 18 }, 1], + ]) { + const u8 = await readFixture(name); + const info = analyze(u8, opts); + const decided = [...info.strokeWrite.entries()]; + const firstOfLap = new Set(info.lapLengths[lap - 1]); + const kept = decided.filter(([k, s]) => firstOfLap.has(k) && s === null); + assert.ok(kept.length >= 1, `${name} lap ${lap}: an ambiguous group keeps the watch label`); + + const asReference = analyze(u8, { ...opts, keepStrokeWhenUnsure: false }); + const forced = [...asReference.strokeWrite.entries()].filter( + ([k, s]) => firstOfLap.has(k) && s === null, + ); + assert.equal(forced.length, 0, `${name} lap ${lap}: the reference overwrites it`); + } +}); + +test('missed turn: a long length without the strokes to match is not one', async () => { + /* + * Duration alone is not enough. A kick set, or a pause at the wall without + * pressing lap, makes a length run long with an ordinary or near-zero stroke + * count -- and that is one length, not two. No fixture has one, so without + * this test the stroke half of the rule could be deleted and every test + * above would still pass; this doctors one into swim-07. + */ + const { patchFrame, writeFit } = await import('../src/fit-patch.js'); + const [F_ELAPSED, F_TIMER, F_STROKES] = [3, 4, 5]; + const { header, frames } = readFit(await readFixture('swim-07.fit')); + + let doctored = false; + const out = frames.map((f) => { + if ( + !doctored && + f.kind === 'data' && + f.globalNum === MSG_LENGTH && + getField(f, F_TYPE) === ACTIVE + ) { + doctored = true; + // 200 s is ~2.5 lengths at swim-07's ~81 s unit -- well past the ratio -- + // while its strokes stay one length's worth. (Scaling the first active + // length instead picks a 47 s fragment and stays under the ratio for the + // wrong reason, which is how an earlier version of this test passed with + // the stroke check deleted.) + const ms = 200_000; + return patchFrame(f, { [F_ELAPSED]: ms, [F_TIMER]: ms, [F_STROKES]: getField(f, F_STROKES) }); + } + return f.bytes; + }); + const info = analyze(writeFit(header, out)); + assert.equal(info.findings.filter((f) => f.type === 'missed-turn').length, 0); +}); diff --git a/web/app.js b/web/app.js index 99a79a1..ee61646 100644 --- a/web/app.js +++ b/web/app.js @@ -89,6 +89,17 @@ const FINDINGS = { detail: (f) => `${f.note} That would take ${f.lengthsBefore} lengths down to ${f.lengthsAfter}.`, }, + 'missed-turn': { + icon: '🔁', + name: 'Possible missed turn', + tone: 'danger', + detail: (f) => + `One recorded length took ${fmtSeconds(f.durS)} with ${f.strokes} strokes — about ` + + `${f.looksLike} lengths' worth, where one reads as ${fmtSeconds(f.unitS)}. The watch ` + + `probably missed a turn, so the distance is likely ${f.looksLike - 1} length` + + `${f.looksLike - 1 === 1 ? '' : 's'} short. Not fixed: splitting it would mean ` + + `inventing a turn the watch never recorded, and its stroke is left as the watch said.`, + }, 'uncertain-lengths': { icon: '🤔', name: 'Two readings are possible', @@ -98,7 +109,7 @@ const FINDINGS = { }; /** Things that make the whole result untrustworthy sort to the top. */ -const TOP = new Set(['lap-structure', 'uncertain-lengths']); +const TOP = new Set(['lap-structure', 'uncertain-lengths', 'missed-turn']); const findingOrder = (f) => (TOP.has(f.type) ? 0 : 1); /** @@ -286,9 +297,13 @@ function renderWorking(info) { if (!laps.length) return; const merged = laps.filter((l) => l.lengths > l.target).length; - workingBadge.textContent = merged - ? `${merged} lap${merged === 1 ? '' : 's'} merged` - : `${laps.length} laps, none merged`; + const missedLaps = laps.filter((l) => l.missedTurns.some(Boolean)).length; + workingBadge.textContent = [ + merged ? `${merged} lap${merged === 1 ? '' : 's'} merged` : `${laps.length} laps, none merged`, + missedLaps ? `${missedLaps} possible missed turn${missedLaps === 1 ? '' : 's'}` : '', + ] + .filter(Boolean) + .join(' · '); if (info.lengthUnitS) { workingUnit.textContent = @@ -313,19 +328,20 @@ function renderWorking(info) { // Each duration unbreakable, so a narrow screen wraps at the "+" and never // strands the unit: "95" on one line and "s" on the next was the result. const seen = l.lengthsS - .map((s) => + .map((s, i) => // A length the watch never timed is "—", not a confident "0 s". s === null ? '—' - : `${esc(Math.round(s))} s`, + : `${esc(Math.round(s))} s`, ) .join(' + '); + const missed = l.missedTurns.some(Boolean); return ` - + ${esc(l.lap + 1)} ${esc(l.lengths)} ${isMerged ? '' : ''}${esc(l.target)} - ${seen}${isMerged ? ' merged' : ''} + ${seen}${isMerged ? ' merged' : ''}${missed ? ' possible missed turn' : ''} `; }) .join(''); diff --git a/web/style.css b/web/style.css index 8368f47..a6ccd67 100644 --- a/web/style.css +++ b/web/style.css @@ -618,6 +618,24 @@ body.dragging::after { white-space: nowrap; } +/* A length that looks like two: the other direction from a merge, and the one + the repair cannot fix -- so it takes the danger tone of its finding. */ +.working-table tr.is-missed td:first-child { + box-shadow: inset 3px 0 0 var(--danger); + padding-left: 0.6rem; +} + +.working-table .dur-missed { + font-weight: 700; + color: var(--danger); +} + +/* Compound selector: .working-tag below sets the warn colour and would win a + tie on specificity by coming later. */ +.working-tag.working-tag-missed { + color: var(--danger); +} + .working-tag { margin-left: 0.35rem; font-size: 0.75rem; From 6fffd94fcbe12df9723976e1704158744149e4bb Mon Sep 17 00:00:00 2001 From: Max Winterstein <5927148+MaxWinterstein@users.noreply.github.com> Date: Fri, 25 Sep 2026 08:42:01 +0000 Subject: [PATCH 2/4] Split a missed turn when the swimmer says so The missed-turn finding says a swim is short and by how much; this lets the swimmer act on it. Each finding gets a "Split into 2 lengths" switch, off by default, and --split-missed-turns[=LAPS] does the same on the command line. swim-08 then comes out at the confirmed 1100 m. It is the one place this tool adds data, which is why it is opt-in and per length. A merge only discards: every number it writes is still one the watch measured. A split has to make up two -- where the turn fell, and how the strokes divide -- and on a threshold resting on a single real example the worst it could do is add distance nobody swam. The choices are the neutral ones: the turn goes halfway and the strokes divide with the time, the remainder to the last part so the totals are exact. It runs as a pre-pass. applySplits() writes the extra length as a copy of the original frame placed right after it, so the definition in force is the same one, and returns a new file; analyze() and repair() then run unchanged on that. Lap targets, stroke decisions and the guarantee that the working table adds up to what is written all hold on the split file with no special case. The finding is carried across marked `split`, or the switch that turned it on would vanish the moment it did. What the tests pin is the narrowest honest claim: it adds a length and nothing else. Total time and strokes are unchanged; the two parts sum to the original and start back to back; files without a missed turn come out byte-identical with it on; the output repairs to the same result again; it commutes with anonymizing. Every one of those fails when the split is deliberately broken -- strokes duplicated, parts started at the same second, the finding dropped, the parts left unmarked, or the split turned on by default. Two details the browser caught. The stat cards' "was" figures were counted from the split file and showed 1200 m for a swim the watch recorded as 1150 m -- counting made-up data as "before" -- so the made-up lengths come back out. And the working table's note still said nothing is ever split. The made-up lengths are italic and labelled "split -- made up, not recorded" in the table and starred in the CLI, and the switch keeps keyboard focus when the findings are rebuilt under it. Whether Garmin Connect accepts a file with an added length message is untested from here. Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 14 ++- README.md | 13 ++- packages/fitfix/src/cli.mjs | 56 ++++++++++- packages/fitfix/src/swim-repair.js | 116 ++++++++++++++++++++++- packages/fitfix/test/cli.test.mjs | 17 ++++ packages/fitfix/test/split.test.mjs | 141 ++++++++++++++++++++++++++++ web/app.js | 62 ++++++++++-- web/index.html | 8 +- web/style.css | 29 ++++++ 9 files changed, 431 insertions(+), 25 deletions(-) create mode 100644 packages/fitfix/test/split.test.mjs diff --git a/AGENTS.md b/AGENTS.md index 1a271f5..a65ce82 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -314,7 +314,7 @@ for screenshots and for checking a deploy. real distance. It reports rather than refuses, because the data alone cannot distinguish "many phantom turns" from "a different lapping habit". Fixture 1 trips it legitimately. Keep it loud in every front end. -- **A missed turn is reported, never repaired.** Two lengths recorded as one +- **A missed turn is reported, and repaired only on request.** Two lengths recorded as one show up as a single length at ~2x the unit *and* ~2x the median stroke count (`MISSED_TURN_RATIO`, 1.75). Both halves matter: a kick set or a pause at the wall is long without the strokes, and an 18 m pool reaches 1.61x on duration @@ -323,6 +323,18 @@ for screenshots and for checking a deploy. such a length is left as the watch said: its count covers two lengths, so measured against a per-length threshold it reads as breaststroke, which is exactly what used to get written into an all-freestyle swim. +- **A split is a pre-pass, and it is opt-in on purpose.** `splitMissedTurns` + runs `applySplits()` before anything else sees the file, so every other + guarantee -- lap targets, stroke decisions, the working table adding up to + what is written -- holds on the split file unchanged. It is the one place + this tool adds data: a merge only discards, so every number it writes was + measured, but a split has to make up where the turn fell and how the strokes + divide. Keep it off by default, keyed per length (the finding's `key`, the + length's start time), and keep the made-up lengths marked as such + (`swimLaps[].split`) in every front end. `split.test.mjs` holds it to adding + nothing but a length: total time and strokes are unchanged. Before/after + figures must subtract the made-up lengths, or "before" includes data that + never existed. - **Stroke decisions are made once, in `analyze()`.** `strokeWrite` maps each merged group to the stroke to write, or `null` to keep the watch's label, and `repair()` writes that rather than re-deriving it. It re-derived it once, and diff --git a/README.md b/README.md index ec35c42..de6cbd2 100644 --- a/README.md +++ b/README.md @@ -160,11 +160,14 @@ scrubbing step that is not optional. you lapped inconsistently, which no single number can. It is still a heuristic tuned on seven files — check the before/after numbers, and put a number in if you disagree. -- **It only ever merges, never splits.** If the watch *missed* a turn and - recorded two lengths as one, the file stays one length short — splitting it - would mean inventing a turn the watch never recorded. It does now *say* so: a - length that runs as long as two, in both time and strokes, gets a "possible - missed turn" finding, and its stroke is left as the watch recorded it. +- **It merges on its own, and splits only when you ask.** If the watch *missed* + a turn and recorded two lengths as one, you get a "possible missed turn" + finding: a length that runs as long as two, in both time and strokes, with its + stroke left as the watch recorded it. The file stays short unless you tick + *Split into 2 lengths* on that finding (`--split-missed-turns=LAP` on the + command line). The split lengths are made up — the turn is put halfway and + the strokes are divided with the time — and are marked that way in *Show the + working*. Total time and strokes don't change; only the length count does. - The freestyle/breaststroke split defaults to `auto`, which scales it from the pool length — 40 strokes per length at 50 m, proportionally fewer in a short pool. The constant behind it was still fitted on one swimmer, so a fixed diff --git a/packages/fitfix/src/cli.mjs b/packages/fitfix/src/cli.mjs index daacd62..d62c480 100755 --- a/packages/fitfix/src/cli.mjs +++ b/packages/fitfix/src/cli.mjs @@ -30,7 +30,14 @@ Assumptions, calibrated on a Forerunner 265 and scaled to your pool: --duration-split=N|auto seconds per length, cross-checks --stroke-split (default ${DEFAULTS.durationSplit}, scaled the same way) --keep-stroke leave the watch's stroke classification alone - --keep-elapsed leave total elapsed time as recorded`; + --keep-elapsed leave total elapsed time as recorded + + --split-missed-turns[=LAPS] + split lengths that look like two the watch + recorded as one -- all of them, or only those in + the listed laps (e.g. =12,20). Off by default: it + invents where the turn fell, so use it for a turn + you actually remember.`; const args = process.argv.slice(2); if (!args.length || args.includes('--help') || args.includes('-h')) { @@ -67,6 +74,7 @@ const KNOWN = new Set([ 'dry-run', 'entry', 'help', + 'split-missed-turns', ]); const unknown = Object.keys(flags).filter((f) => !KNOWN.has(f)); if (unknown.length) { @@ -93,6 +101,22 @@ for (const [flag, { key, auto }] of Object.entries(NUMERIC)) { } for (const [flag, key] of Object.entries(BOOLEAN)) if (flags[flag]) opts[key] = false; +/* + * Laps are what a swimmer can name from the table, so that is what the flag + * takes; the library wants finding keys, which are resolved once the file is + * read. Validated here, like every other flag, rather than silently ignored. + */ +let splitLaps = null; +if (flags['split-missed-turns'] !== undefined) { + const v = flags['split-missed-turns']; + if (v === true) splitLaps = true; + else if (/^\d+(,\d+)*$/.test(v)) splitLaps = new Set(v.split(',').map(Number)); + else { + console.error(`--split-missed-turns takes lap numbers like =12,20, got "${v}"`); + process.exit(2); + } +} + const raw = new Uint8Array(await readFile(src)); /* @@ -162,6 +186,19 @@ if (isZip(raw)) { const dst = pos[1] ?? join(dirname(src), `${label.replace(/\.fit$/i, '')}_fixed.fit`); +if (splitLaps === true) opts.splitMissedTurns = true; +else if (splitLaps) { + const found = analyze(u8, { ...opts, splitMissedTurns: false }).findings.filter( + (f) => f.type === 'missed-turn', + ); + const unknown = [...splitLaps].filter((n) => !found.some((f) => f.lap + 1 === n)); + if (unknown.length) { + console.error(`--split-missed-turns: no possible missed turn in lap ${unknown.join(', ')}`); + process.exit(2); + } + opts.splitMissedTurns = found.filter((f) => splitLaps.has(f.lap + 1)).map((f) => f.key); +} + if (flags['dry-run']) { const info = analyze(u8, opts); console.log( @@ -200,11 +237,13 @@ function printWorking(info) { for (const l of info.swimLaps) { const merged = l.lengths > l.target; const seen = l.lengthsS - .map((s, i) => (s === null ? '?' : `${Math.round(s)}${l.missedTurns[i] ? '!' : ''}`)) + .map((s, i) => + s === null ? '?' : `${Math.round(s)}${l.missedTurns[i] ? '!' : ''}${l.split[i] ? '*' : ''}`, + ) .join(' + '); const missed = l.missedTurns.some(Boolean); console.log( - ` ${String(l.lap + 1).padStart(3)} ${String(l.lengths).padStart(5)} -> ${String(l.target).padEnd(5)} ${seen}${merged ? ' merged' : ''}${missed ? ' possible missed turn' : ''}`, + ` ${String(l.lap + 1).padStart(3)} ${String(l.lengths).padStart(5)} -> ${String(l.target).padEnd(5)} ${seen}${merged ? ' merged' : ''}${missed ? ' possible missed turn' : ''}${l.split.some(Boolean) ? ' split (* made up)' : ''}`, ); } } @@ -228,13 +267,22 @@ if (opts.lengthsPerLap === 'auto' && info.lengthUnitS) { */ for (const f of info.findings.filter((f) => f.type === 'missed-turn')) { console.warn(''); + if (f.split) { + console.warn( + ` !! lap ${f.lap + 1}: split a ${Math.round(f.durS)} s length into ${f.looksLike}, as asked.`, + ); + console.warn(' !! The turn is put halfway -- those lengths are made up, not recorded.'); + continue; + } console.warn( ` !! lap ${f.lap + 1}: possible missed turn -- one length took ${Math.round(f.durS)} s`, ); console.warn( ` !! with ${f.strokes} strokes, about ${f.looksLike} lengths' worth (one is ~${Math.round(f.unitS)} s).`, ); - console.warn(` !! The distance is probably ${f.looksLike - 1} length(s) short. Not fixed.`); + console.warn( + ` !! The distance is probably ${f.looksLike - 1} length(s) short. --split-missed-turns=${f.lap + 1} splits it.`, + ); } // Loud, before anything else: this is the case where the output is garbage. diff --git a/packages/fitfix/src/swim-repair.js b/packages/fitfix/src/swim-repair.js index b635d70..3ba96e0 100644 --- a/packages/fitfix/src/swim-repair.js +++ b/packages/fitfix/src/swim-repair.js @@ -107,6 +107,21 @@ export const DEFAULTS = { * this off. */ keepStrokeWhenUnsure: true, + /** + * Split lengths that look like missed turns back into the lengths they were. + * + * `false` (the default) splits nothing: the file stays as short as the watch + * made it, and the missed-turn finding says by how much. `true` splits every + * one found. An array splits only those whose finding `key` it lists -- how + * the page lets a swimmer confirm them one at a time. + * + * Off by default because it invents data. A merge only discards: every + * number it writes is still one the watch measured. A split has to make up + * where the turn fell (halfway) and how the strokes divide (with the time), + * so the worst it can do is add distance nobody swam -- on a threshold that + * rests on a single real example. The swimmer is the only one who knows. + */ + splitMissedTurns: false, /** Set elapsed time to timer time when the timer was never paused. */ normalizeElapsed: true, }; @@ -350,6 +365,91 @@ function resolveLapTargets(lapGroups, durationOf, lengthsPerLap) { */ export function analyze(u8, opts = {}) { const o = { ...DEFAULTS, ...opts }; + const { bytes, invented, applied } = applySplits(u8, o); + return analyzeFile(bytes, o, invented, applied); +} + +/** + * Splits the chosen missed-turn lengths into equal parts and returns the new + * file, before any analysis or repair sees it. + * + * A pre-pass rather than a step inside repair(): everything downstream -- lap + * assignment, lap targets, the stroke decisions, the working table and the + * guarantee that it adds up to what is written -- then runs unchanged on a file + * that simply has the right number of lengths in it. `invented` lists the + * resulting lengths by index, so the front ends can say which ones were made up. + * + * Each part is a copy of the original frame, placed immediately after it, so + * the definition in force is the same one. Durations and strokes divide + * equally, the remainder going to the last part so the totals are exact, and + * each part starts where the one before ended, in the whole seconds FIT uses. + * repair() then rewrites the derived fields (speed, cadence, index) of every + * surviving length, as it already does after a merge. + */ +function applySplits(u8, o) { + const none = { bytes: u8, invented: new Set(), applied: [] }; + if (!o.splitMissedTurns) return none; + + // Detection runs on the file as recorded: the lengths to split are the ones + // the swimmer was shown. + const found = analyzeFile(u8, o).findings.filter((f) => f.type === 'missed-turn'); + const wanted = o.splitMissedTurns === true ? null : new Set(o.splitMissedTurns); + const chosen = found.filter((f) => wanted === null || wanted.has(f.key)); + if (!chosen.length) return none; + + const byKey = new Map(chosen.map((f) => [f.key, f])); + const { header, frames } = readFit(u8); + const out = []; + const invented = new Set(); + let index = 0; + for (const fr of frames) { + if (fr.kind !== 'data' || fr.globalNum !== MSG.length) { + out.push(fr.bytes); + continue; + } + const f = + getField(fr, F.length.lengthType) === LENGTH_TYPE.active + ? byKey.get(getField(fr, F.length.startTime)) + : undefined; + if (!f) { + out.push(fr.bytes); + index++; + continue; + } + const parts = f.looksLike; + const ms = getField(fr, F.length.elapsed); + const strokes = getField(fr, F.length.strokes) ?? 0; + const start = getField(fr, F.length.startTime); + let usedMs = 0; + let usedStrokes = 0; + for (let i = 0; i < parts; i++) { + const last = i === parts - 1; + const partMs = last ? ms - usedMs : Math.round(ms / parts); + const partStrokes = last ? strokes - usedStrokes : Math.round(strokes / parts); + out.push( + patchFrame(fr, { + [F.length.startTime]: start + Math.round(usedMs / 1000), + [F.length.elapsed]: partMs, + [F.length.timer]: partMs, + [F.length.strokes]: partStrokes, + }), + ); + invented.add(index++); + usedMs += partMs; + usedStrokes += partStrokes; + } + } + return { + bytes: writeFit(header, out), + invented, + // Carried into the analysis of the split file, which no longer contains + // the long length -- without it, the finding and its switch would vanish + // the moment the switch was turned on. + applied: chosen.map((f) => ({ ...f, split: true })), + }; +} + +function analyzeFile(u8, o, invented = new Set(), applied = []) { const { frames } = readFit(u8); const lengths = frames.filter((f) => f.kind === 'data' && f.globalNum === MSG.length); const laps = frames.filter((f) => f.kind === 'data' && f.globalNum === MSG.lap); @@ -476,7 +576,10 @@ export function analyze(u8, opts = {}) { */ const strokeWrite = new Map(); - const findings = []; + // Splits already applied keep their finding, marked split, so the switch + // that turned them on is still there to turn them off. Lap indices carry + // over unchanged: splitting adds lengths, never laps. + const findings = [...applied]; const swimLaps = []; lapActive.forEach((act, li) => { const target = lapTargets[li]; @@ -520,6 +623,8 @@ export function analyze(u8, opts = {}) { lengthStrokes: act.map((k) => getField(lengths[k], F.length.strokes)), /** Per recorded length: does it look like two lengths recorded as one? */ missedTurns: act.map((k) => missed.has(k)), + /** Per length: made up by splitting a missed turn, not recorded by the watch. */ + split: act.map((k) => invented.has(k)), }); for (const k of act) { @@ -528,6 +633,9 @@ export function analyze(u8, opts = {}) { findings.push({ type: 'missed-turn', lap: li, + /** Stable across re-analysis of the same file: the length's start, in seconds. */ + key: getField(lengths[k], F.length.startTime), + split: false, durS, strokes: getField(lengths[k], F.length.strokes), looksLike: Math.round(durS / refUnit), @@ -708,9 +816,11 @@ export function analyze(u8, opts = {}) { /** Applies the repair and returns the new file. */ export function repair(u8, opts = {}) { const o = { ...DEFAULTS, ...opts }; - const info = analyze(u8, o); + // The same pre-pass analyze() does, so both work on the same file. + const { bytes: src, invented, applied } = applySplits(u8, o); + const info = analyzeFile(src, o, invented, applied); const { poolM, recordedPoolM, strokeSplit, timerMs, pauses, lapLengths } = info; - const { header, frames } = readFit(u8); + const { header, frames } = readFit(src); const lengths = frames.filter((f) => f.kind === 'data' && f.globalNum === MSG.length); // lapLengths comes from analyze() rather than being recomputed. The frames diff --git a/packages/fitfix/test/cli.test.mjs b/packages/fitfix/test/cli.test.mjs index 3c8fc8a..f1fc9a4 100644 --- a/packages/fitfix/test/cli.test.mjs +++ b/packages/fitfix/test/cli.test.mjs @@ -182,6 +182,23 @@ test('a zip with no activity file in it says so', async () => { assert.match(stderr, /no \.fit file inside/); }); +test('--split-missed-turns splits only when asked, and only a real one', async () => { + const swim08 = join(dir, 'swim-08.fit'); + await writeFile(swim08, await readFixture('swim-08.fit')); + + const plain = await cli([swim08, join(dir, 'plain.fit')]); + assert.match(plain.stdout, /1050 m, 21 lengths/, 'off by default'); + assert.match(plain.stderr, /--split-missed-turns=12 splits it/, 'and it says how'); + + const split = await cli([swim08, join(dir, 'split.fit'), '--split-missed-turns=12']); + assert.match(split.stdout, /1100 m, 22 lengths/); + assert.match(split.stderr, /made up, not recorded/); + + const wrong = await cli([swim08, '--split-missed-turns=3']); + assert.equal(wrong.code, 2, 'a lap with no missed turn is refused, not ignored'); + assert.match(wrong.stderr, /no possible missed turn in lap 3/); +}); + test('a file it cannot safely repair fails loudly', async () => { const truncated = join(dir, 'truncated.fit'); await writeFile(truncated, (await readFixture('swim-02.fit')).subarray(0, 4000)); diff --git a/packages/fitfix/test/split.test.mjs b/packages/fitfix/test/split.test.mjs new file mode 100644 index 0000000..12b41d8 --- /dev/null +++ b/packages/fitfix/test/split.test.mjs @@ -0,0 +1,141 @@ +/** + * Splitting a missed turn: the one place this tool adds a length. + * + * Everywhere else it only merges, so every number it writes is one the watch + * measured. A split has to make two up -- where the turn fell, and how the + * strokes divide -- so it is off by default, opt-in per length, and these + * tests hold it to the narrowest thing it can honestly claim: that it + * redistributes what was recorded, and adds nothing else. Total time and total + * strokes are unchanged; only the count of lengths, and so the distance, moves. + * + * swim-08's truth is confirmed by the swimmer: 22 lengths, 1100 m, freestyle. + */ + +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { anonymize } from '../src/anonymize.js'; +import { checkIntegrity, getField, readFit } from '../src/fit-patch.js'; +import { analyze, repair } from '../src/swim-repair.js'; +import { ORIGINALS, POOL_OVERRIDE, readFixture } from './fixtures.mjs'; + +const MSG_LENGTH = 101; +const [F_START, F_ELAPSED, F_STROKES, F_TYPE, F_STROKE] = [2, 3, 5, 12, 7]; +const ACTIVE = 1; + +const active = (u8) => + readFit(u8) + .frames.filter((f) => f.kind === 'data' && f.globalNum === MSG_LENGTH) + .filter((f) => getField(f, F_TYPE) === ACTIVE); + +const keyOf = async () => + analyze(await readFixture('swim-08.fit')).findings.find((f) => f.type === 'missed-turn').key; + +test('split: off unless asked for', async () => { + const u8 = await readFixture('swim-08.fit'); + assert.equal(repair(u8).summary.lengths, 21, 'the default still writes what the watch recorded'); + assert.deepEqual(repair(u8, { splitMissedTurns: false }).bytes, repair(u8).bytes); +}); + +test('split: swim-08 comes out at the confirmed 1100 m', async () => { + const { summary, bytes, info } = repair(await readFixture('swim-08.fit'), { + splitMissedTurns: true, + }); + assert.equal(summary.lengths, 22); + assert.equal(summary.distanceM, 1100); + assert.ok(checkIntegrity(bytes), 'a valid FIT file'); + assert.ok( + active(bytes).every((f) => getField(f, F_STROKE) === 0), + 'still freestyle throughout', + ); + + // The finding survives, marked, so the switch that turned it on stays. + const f = info.findings.filter((x) => x.type === 'missed-turn'); + assert.equal(f.length, 1); + assert.equal(f[0].split, true); +}); + +test('split: redistributes what was recorded and adds nothing else', async () => { + const u8 = await readFixture('swim-08.fit'); + const before = active(u8); + const after = active(repair(u8, { splitMissedTurns: true }).bytes); + const plain = active(repair(u8).bytes); + + const sum = (frames, field) => frames.reduce((a, f) => a + (getField(f, field) ?? 0), 0); + assert.equal(sum(after, F_STROKES), sum(plain, F_STROKES), 'total strokes unchanged'); + assert.equal(sum(after, F_ELAPSED), sum(plain, F_ELAPSED), 'total swim time unchanged'); + + // The two parts, exactly: halves of the original, back to back, in order. + const original = before.find((f) => Math.round(getField(f, F_ELAPSED) / 1000) === 151); + const i = after.findIndex((f) => getField(f, F_START) === getField(original, F_START)); + const [a, b] = [after[i], after[i + 1]]; + assert.equal(getField(a, F_ELAPSED) + getField(b, F_ELAPSED), getField(original, F_ELAPSED)); + assert.equal(getField(a, F_STROKES) + getField(b, F_STROKES), getField(original, F_STROKES)); + assert.ok(Math.abs(getField(a, F_ELAPSED) - getField(b, F_ELAPSED)) <= 1, 'split halfway'); + assert.equal( + getField(b, F_START), + getField(a, F_START) + Math.round(getField(a, F_ELAPSED) / 1000), + 'the second starts where the first ends', + ); +}); + +test('split: the made-up lengths are marked as made up', async () => { + // The page and the CLI both tag them; this is where that flag comes from. + const info = analyze(await readFixture('swim-08.fit'), { splitMissedTurns: true }); + const lap = info.swimLaps.find((l) => l.lap === 11); + assert.deepEqual(lap.split, [true, true, false], 'the two halves, not the real 97 s length'); + assert.equal( + info.swimLaps.flatMap((l) => l.split).filter(Boolean).length, + 2, + 'and nothing else in the swim', + ); +}); + +test('split: chosen by key, one length at a time', async () => { + const u8 = await readFixture('swim-08.fit'); + const all = repair(u8, { splitMissedTurns: true }).bytes; + assert.deepEqual(repair(u8, { splitMissedTurns: [await keyOf()] }).bytes, all); + assert.deepEqual( + repair(u8, { splitMissedTurns: [123] }).bytes, + repair(u8).bytes, + 'a key that matches nothing splits nothing', + ); +}); + +test('split: the working table still adds up to the file', async () => { + const u8 = await readFixture('swim-08.fit'); + for (const lengthsPerLap of ['auto', 1, 2]) { + const opts = { splitMissedTurns: true, lengthsPerLap }; + const promised = analyze(u8, opts).swimLaps.reduce((a, l) => a + l.target, 0); + assert.equal(promised, repair(u8, opts).summary.lengths, `lengthsPerLap ${lengthsPerLap}`); + } +}); + +test('split: repairing the output again changes nothing', async () => { + const once = repair(await readFixture('swim-08.fit'), { splitMissedTurns: true }); + const twice = repair(once.bytes, { splitMissedTurns: true }); + assert.equal(twice.summary.lengths, once.summary.lengths); + assert.equal(twice.summary.distanceM, once.summary.distanceM); +}); + +test('split: files without a missed turn come out byte-identical', async () => { + for (const name of ORIGINALS.filter((n) => n !== 'swim-08.fit')) { + const u8 = await readFixture(name); + const pool = POOL_OVERRIDE[name] ? { poolLength: POOL_OVERRIDE[name] } : {}; + assert.deepEqual( + repair(u8, { ...pool, splitMissedTurns: true }).bytes, + repair(u8, pool).bytes, + name, + ); + } +}); + +test('split: commutes with anonymizing, like the rest of the repair', async () => { + // The fixture is already anonymized, so this checks the split's timestamps + // are relative -- a start time the dates' rebase would not carry along. + const u8 = await readFixture('swim-08.fit'); + const opts = { splitMissedTurns: true }; + assert.deepEqual( + anonymize(repair(u8, opts).bytes).bytes, + repair(anonymize(u8).bytes, opts).bytes, + ); +}); diff --git a/web/app.js b/web/app.js index ee61646..2d1fa8f 100644 --- a/web/app.js +++ b/web/app.js @@ -33,7 +33,7 @@ const workingUnit = el('workingUnit'); const workingRows = el('workingRows'); /** Loaded file, the most recent repair output, and the anonymized copy. */ -const state = { name: null, input: null, output: null, anonymized: null }; +const state = { name: null, input: null, output: null, anonymized: null, splits: new Set() }; /** * Escapes text destined for innerHTML. @@ -94,11 +94,26 @@ const FINDINGS = { name: 'Possible missed turn', tone: 'danger', detail: (f) => - `One recorded length took ${fmtSeconds(f.durS)} with ${f.strokes} strokes — about ` + - `${f.looksLike} lengths' worth, where one reads as ${fmtSeconds(f.unitS)}. The watch ` + - `probably missed a turn, so the distance is likely ${f.looksLike - 1} length` + - `${f.looksLike - 1 === 1 ? '' : 's'} short. Not fixed: splitting it would mean ` + - `inventing a turn the watch never recorded, and its stroke is left as the watch said.`, + f.split + ? `One recorded length took ${fmtSeconds(f.durS)} with ${f.strokes} strokes, and is ` + + `now split into ${f.looksLike} lengths of about ${fmtSeconds(f.durS / f.looksLike)} ` + + 'each. Those lengths are made up, not recorded: the turn is put halfway and the ' + + 'strokes are divided with the time. Untick to put it back as the watch recorded it.' + : `One recorded length took ${fmtSeconds(f.durS)} with ${f.strokes} strokes — about ` + + `${f.looksLike} lengths' worth, where one reads as ${fmtSeconds(f.unitS)}. The watch ` + + `probably missed a turn, so the distance is likely ${f.looksLike - 1} length` + + `${f.looksLike - 1 === 1 ? '' : 's'} short. Its stroke is left as the watch said.`, + /* + * Opt-in, one length at a time, because a split invents data and only the + * swimmer knows whether there was a turn there. The value is the finding's + * key -- the length's start time -- which survives re-analysis, so the + * choice sticks while the other assumptions change. + */ + control: (f) => ` + `, }, 'uncertain-lengths': { icon: '🤔', @@ -176,6 +191,8 @@ function readOptions() { // get clobbered halfway through at "1". if (!ok && document.activeElement !== input) input.value = value; } + // Not a form control: the switches live on the findings they belong to. + opts.splitMissedTurns = state.splits.size ? [...state.splits] : false; return opts; } @@ -232,7 +249,13 @@ function render() { const { info, summary } = state.output; // What the watch claimed, reconstructed from the pre-merge length counts. - const lengthsBefore = info.swimLaps.reduce((a, l) => a + l.lengths, 0); + // What the watch recorded -- so the lengths a split made up come back out. + // Counted from the split file they are not "before" anything, and the old + // figure showed 1200 m for a swim the watch had recorded as 1150. + const madeUp = info.findings + .filter((f) => f.type === 'missed-turn' && f.split) + .reduce((a, f) => a + f.looksLike - 1, 0); + const lengthsBefore = info.swimLaps.reduce((a, l) => a + l.lengths, 0) - madeUp; const distanceBefore = lengthsBefore * info.poolM; statsEl.innerHTML = [ @@ -269,6 +292,7 @@ function render() {
${esc(meta.name)}${lap}
${esc(meta.detail(f))}
+ ${meta.control ? meta.control(f) : ''}
`; }) @@ -298,9 +322,11 @@ function renderWorking(info) { const merged = laps.filter((l) => l.lengths > l.target).length; const missedLaps = laps.filter((l) => l.missedTurns.some(Boolean)).length; + const splitLaps = laps.filter((l) => l.split.some(Boolean)).length; workingBadge.textContent = [ merged ? `${merged} lap${merged === 1 ? '' : 's'} merged` : `${laps.length} laps, none merged`, missedLaps ? `${missedLaps} possible missed turn${missedLaps === 1 ? '' : 's'}` : '', + splitLaps ? `${splitLaps} split` : '', ] .filter(Boolean) .join(' · '); @@ -332,16 +358,17 @@ function renderWorking(info) { // A length the watch never timed is "—", not a confident "0 s". s === null ? '—' - : `${esc(Math.round(s))} s`, + : `${esc(Math.round(s))} s`, ) .join(' + '); const missed = l.missedTurns.some(Boolean); + const wasSplit = l.split.some(Boolean); return ` ${esc(l.lap + 1)} ${esc(l.lengths)} ${isMerged ? '' : ''}${esc(l.target)} - ${seen}${isMerged ? ' merged' : ''}${missed ? ' possible missed turn' : ''} + ${seen}${isMerged ? ' merged' : ''}${missed ? ' possible missed turn' : ''}${wasSplit ? ' split — made up, not recorded' : ''} `; }) .join(''); @@ -417,6 +444,8 @@ function withBusy(work) { } function loadBytes(name, label, bytes) { + // Splits belong to one swim; a key from another would match nothing, or worse. + state.splits = new Set(); // A new swim starts closed. Only re-runs of the same file keep it open. working.open = false; state.name = name; @@ -570,6 +599,7 @@ function reset() { state.input = null; state.output = null; state.anonymized = null; + state.splits = new Set(); picker.value = ''; filenameEl.textContent = ''; statsEl.innerHTML = ''; @@ -619,6 +649,20 @@ window.addEventListener('drop', (e) => { loadFile(e.dataTransfer?.files); }); +/* + * A split switch re-runs the repair at once. The findings are rebuilt from + * scratch by that, which would drop keyboard focus on the page -- so it is put + * back on the same switch afterwards. + */ +findingsEl.addEventListener('change', (e) => { + const key = e.target.dataset?.splitKey; + if (key === undefined) return; + if (e.target.checked) state.splits.add(Number(key)); + else state.splits.delete(Number(key)); + runRepair(); + findingsEl.querySelector(`[data-split-key="${CSS.escape(key)}"]`)?.focus(); +}); + /** Re-running on every keystroke is wasteful on a large file. */ let pending; const scheduleRepair = () => { diff --git a/web/index.html b/web/index.html index 6ab7be2..609b295 100644 --- a/web/index.html +++ b/web/index.html @@ -262,9 +262,11 @@

🌊 phantomturn

Rest laps hold no lengths and are left out, so lap numbers skip - where you stopped. Nothing is ever split, so a lap can only lose - lengths — and if the target is wrong for how you actually lapped, - the ones it loses are real. This table is where that shows. + where you stopped. Lengths are only ever merged, never split — + unless you switch on a split for a possible missed turn, and then + only that one, marked as made up. So if the target is wrong for + how you actually lapped, the lengths it loses are real, and this + table is where that shows.