diff --git a/AGENTS.md b/AGENTS.md index be31a0e..3ce28c3 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 @@ -297,20 +300,55 @@ for screenshots and for checking a deploy. `resolveLapTargets()` produces one target per lap; `analyze()` returns them as `lapTargets` and `repair()` uses those rather than recomputing. A fixed number still overrides. Merging is downwards only — a lap with fewer lengths - than its target is untouched and nothing is ever split, so a *missed* turn is - out of reach either way. -- **The unit estimate is `max()` of two estimators on purpose.** Both fail - small, in opposite directions: the recorded-length estimator is useless when - every length was split (swim-01 has no intact length anywhere), and the - lap-total estimator is dragged down by long continuous blocks. Do not - "simplify" it to one of them — `auto-lengths.test.mjs` pins the swimmer's - confirmed count for each fixture. + than its target is untouched. A *missed* turn is out of its reach; that is + what the separate, opt-in split below is for. +- **The unit is whichever of two estimates is more uniform.** One comes from + the recorded lengths, one from the lap totals; `estimateLengthUnit()` takes + the set with the lower coefficient of variation, as described under "How the + length unit is chosen" above. This bullet used to describe the `max()` of the + two, the rule that section explains was discarded after it scored 4-for-6 — + do not bring it back. `auto-lengths.test.mjs` pins the swimmer's confirmed + count for each fixture. - **The `lap-structure` finding is the guard against a wrong target.** It fires when more than half the laps hold multiple lengths, or any lap holds four or more — which means the swimmer did not lap once per length and the merge will delete 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, 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 + 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. +- **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 index in the file *as recorded* — not its start time, which two + lengths can share, and not its index in a split file, which shifts), and keep + the made-up lengths marked as such (`swimLaps[].split`) in every front end. + Anything that reports what the *watch* did counts from `recorded` and + `info.lengths`, never from the split file — the Watch column, the + phantom-turn and lap-structure findings, the "was" figures. A fixed + lengths-per-lap can merge the made-up parts straight back; the finding's + `gained` says how many lengths a split really added, and a merged group + containing a made-up length keeps the watch's stroke, or its doubled stroke + count reads as breaststroke all over again. `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 + 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..de6cbd2 100644 --- a/README.md +++ b/README.md @@ -160,8 +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, nothing here will recover 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 4ab7264..a94760f 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,28 @@ if (isZip(raw)) { const dst = pos[1] ?? join(dirname(src), `${label.replace(/\.fit$/i, '')}_fixed.fit`); +if (splitLaps === true) { + opts.splitMissedTurns = true; + // Said, not silently skipped: "I asked for a split and nothing happened" + // should never have to be worked out from the distance alone. + if ( + !analyze(u8, { ...opts, splitMissedTurns: false }).findings.some( + (f) => f.type === 'missed-turn', + ) + ) + console.warn(`${label}: --split-missed-turns: no possible missed turn to split`); +} 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( @@ -199,9 +245,14 @@ 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] ? '!' : ''}${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' : ''}`, + ` ${String(l.lap + 1).padStart(3)} ${String(l.recorded).padStart(5)} -> ${String(l.target).padEnd(5)} ${seen}${merged ? ' merged' : ''}${missed ? ' possible missed turn' : ''}${l.split.some(Boolean) ? ' split (* made up)' : ''}`, ); } } @@ -218,6 +269,40 @@ if (opts.lengthsPerLap === 'auto' && info.lengthUnitS) { ); } +/* + * Also loud, for the opposite reason: the file comes out short. The tool only + * splits when asked -- a split invents a turn the watch never recorded -- so + * by default the most it does is say so, and how to ask. + */ +for (const f of info.findings.filter((f) => f.type === 'missed-turn')) { + console.warn(''); + if (f.split && f.gained === 0) { + console.warn( + ` !! lap ${f.lap + 1}: split as asked, but the fixed --lengths-per-lap merges the parts`, + ); + console.warn( + ' !! straight back -- the distance is unchanged by it. Use auto for it to count.', + ); + continue; + } + 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. --split-missed-turns=${f.lap + 1} splits it.`, + ); +} + // 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..27c15a9 100644 --- a/packages/fitfix/src/swim-repair.js +++ b/packages/fitfix/src/swim-repair.js @@ -92,6 +92,36 @@ 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, + /** + * 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, }; @@ -201,6 +231,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 it only reports unless the swimmer asks for a split. + */ +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; @@ -320,7 +365,128 @@ function resolveLapTargets(lapGroups, durationOf, lengthsPerLap) { */ export function analyze(u8, opts = {}) { const o = { ...DEFAULTS, ...opts }; + return prepare(u8, o).info; +} + +/** + * The split pre-pass and the analysis of its result, in one place. + * + * analyze() and repair() each used to call the two in sequence, and the copies + * drifted: repair() stopped passing the recorded-index map, so the page -- which + * goes through repair() -- counted made-up lengths as recorded while every test, + * which went through analyze(), passed. One helper, so there is no second copy + * to fall behind. + */ +function prepare(u8, o) { + const { bytes, invented, applied, origin } = applySplits(u8, o); + return { src: bytes, info: analyzeFile(bytes, o, invented, applied, origin) }; +} + +/** + * 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: [], origin: null }; + if (!o.splitMissedTurns) return none; + + // Detection runs on the file as recorded: the lengths to split are the ones + // the swimmer was shown. + const base = analyzeFile(u8, o); + const found = base.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(); + // origin[i]: the index, in the file as recorded, of length i of the split + // file. Everything that reports what the *watch* did counts through it, so a + // made-up length is never mistaken for a recorded one. + const origin = []; + let index = 0; + let recordedIndex = -1; + for (const fr of frames) { + if (fr.kind !== 'data' || fr.globalNum !== MSG.length) { + out.push(fr.bytes); + continue; + } + recordedIndex++; + const f = byKey.get(recordedIndex); + if (!f) { + out.push(fr.bytes); + origin.push(recordedIndex); + 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; + // Floored, not rounded: rounding can go up on every part but the last + // and leave it a negative remainder -- 2 strokes in 4 parts came out as + // 1, 1, 1, -1, which an unsigned field turns into the invalid marker. + const partMs = last ? ms - usedMs : Math.floor(ms / parts); + const partStrokes = last ? strokes - usedStrokes : Math.floor(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++); + origin.push(recordedIndex); + 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. `baseTarget` is what the lap came + // to without the split, so the analysis can say what the split changed. + applied: chosen.map((f) => ({ + ...f, + split: true, + baseTarget: base.swimLaps.find((l) => l.lap === f.lap)?.target ?? 0, + note: + 'one recorded length ran as long as two, in both time and strokes, and was ' + + 'split on request: the turn is put halfway and the strokes divided with ' + + 'the time. The lengths it makes are made up, not recorded.', + })), + origin, + }; +} + +function analyzeFile(u8, o, invented = new Set(), applied = [], origin = null) { const { frames } = readFit(u8); + /** A length's index in the file as the watch recorded it. */ + const recordedIndexOf = (k) => (origin ? origin[k] : k); + /** Recorded lengths among these split-file indices: made-up parts count once. */ + const recordedCount = (ks) => new Set(ks.map(recordedIndexOf)).size; const lengths = frames.filter((f) => f.kind === 'data' && f.globalNum === MSG.length); const laps = frames.filter((f) => f.kind === 'data' && f.globalNum === MSG.lap); const session = frames.find((f) => f.kind === 'data' && f.globalNum === MSG.session); @@ -410,7 +576,46 @@ export function analyze(u8, opts = {}) { alternative, } = resolveLapTargets(lapActive, durationOf, o.lengthsPerLap); - const findings = []; + /* + * 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(); + + // 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]; @@ -430,6 +635,8 @@ export function analyze(u8, opts = {}) { swimLaps.push({ lap: li, lengths: act.length, + /** What the watch recorded: `lengths` less any made up by a split. */ + recorded: recordedCount(act), /** What the repair will leave in this lap -- never more than `lengths`. */ target: groups.length, durS: durMs / 1000, @@ -452,17 +659,49 @@ 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)), + /** Per length: made up by splitting a missed turn, not recorded by the watch. */ + split: act.map((k) => invented.has(k)), }); - if (act.length > target) + for (const k of act) { + if (!missed.has(k)) continue; + const durS = durationOf(k); + findings.push({ + type: 'missed-turn', + lap: li, + /* + * The length's index in the file as recorded: unique, and stable across + * re-analysis -- including of a file where another missed turn has + * already been split, which shifts every later index of the split file. + * (The start time was the first choice and is not unique: two lengths + * can share a second.) + */ + key: recordedIndexOf(k), + split: false, + 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.', + }); + } + + // A merge that only folds a split back together is not a phantom turn the + // watch made -- the missed-turn finding reports that it undid the split. + if (recordedCount(act) > groups.length) findings.push({ type: 'phantom-turn', lap: li, - detected: act.length, + detected: recordedCount(act), assumed: target, durS: durMs / 1000, strokes, - note: `lap split into ${act.length} lengths`, + note: `lap split into ${recordedCount(act)} lengths`, }); /* @@ -483,7 +722,20 @@ export function analyze(u8, opts = {}) { (k) => SWIM_STROKE[k] === getField(lengths[group[0]], F.length.swimStroke), ); - if (byStrokes !== byTime) + const ambiguous = byStrokes !== byTime; + // A made-up length merged with anything is as unreliable as the missed + // turn it came from: under a fixed target the halves fold back together + // and the doubled stroke count reads as breaststroke again. + const holdsMissedTurn = + group.some((k) => missed.has(k)) || + (group.length > 1 && group.some((k) => invented.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, @@ -517,8 +769,20 @@ export function analyze(u8, opts = {}) { * The data alone cannot tell the two apart, so this reports rather than * decides -- but it has to be impossible to miss. */ + /* + * What each requested split actually added. A fixed lengths-per-lap can fold + * the made-up lengths straight back together, and the swimmer who ticked + * "I remember turning here" has to be told that the file did not change, + * rather than read "now split into 2" beside a distance that did not move. + */ + for (const f of findings) { + if (f.type !== 'missed-turn' || !f.split) continue; + const lap = swimLaps.find((l) => l.lap === f.lap); + f.gained = Math.max(0, (lap?.target ?? 0) - f.baseTarget); + } + if (swimLaps.length) { - const lengthsBefore = swimLaps.reduce((a, l) => a + l.lengths, 0); + const lengthsBefore = swimLaps.reduce((a, l) => a + l.recorded, 0); const lengthsAfter = swimLaps.reduce((a, l) => a + Math.min(l.lengths, lapTargets[l.lap]), 0); const split = swimLaps.filter((l) => l.lengths > lapTargets[l.lap]); const maxPerLap = Math.max(...swimLaps.map((l) => l.lengths)); @@ -590,7 +854,10 @@ export function analyze(u8, opts = {}) { elapsedMs, pauses, laps: laps.length, - lengths: lengths.length, + // As recorded: the made-up lengths are not something the watch counted. + lengths: lengths.length - (invented.size - new Set([...invented].map(recordedIndexOf)).size), + /** Lengths in the output that a split made up rather than the watch recorded. */ + madeUpLengths: invented.size - new Set([...invented].map(recordedIndexOf)).size, swimLaps, findings, // Handed to repair() rather than recomputed there. The two copies of this @@ -607,15 +874,18 @@ 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, }; } /** 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 { src, info } = prepare(u8, o); 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 @@ -675,11 +945,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 +996,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/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/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 0000000..24049ec Binary files /dev/null and b/packages/fitfix/test/fixtures/swim-08.fit differ diff --git a/packages/fitfix/test/missed-turn.test.mjs b/packages/fitfix/test/missed-turn.test.mjs new file mode 100644 index 0000000..780700d --- /dev/null +++ b/packages/fitfix/test/missed-turn.test.mjs @@ -0,0 +1,167 @@ +/** + * Missed turns: two lengths the watch recorded as one. + * + * The other direction from a phantom turn, and the one this tool cannot fix -- + * it merges, it never splits, because splitting means inventing a turn time and + * a stroke split the watch never recorded. What it can do is notice, say so, + * and not make the file worse. Before this, it did neither: swim-08 came out + * 50 m short with nothing said, and the length's doubled stroke count was read + * as breaststroke and *written* into a swim that was freestyle throughout. + * + * swim-08's truth is confirmed by the swimmer: all freestyle, 22 lengths, + * 1100 m. Lap 12 was three lengths recorded as two, one of them 151 s with 51 + * strokes against a unit of ~79 s and a median of ~28 strokes. + */ + +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { getField, readFit } from '../src/fit-patch.js'; +import { analyze, repair } from '../src/swim-repair.js'; +import { POOL_OVERRIDE, readFixture } from './fixtures.mjs'; + +const MSG_LENGTH = 101; +const [F_TYPE, F_STROKE] = [12, 7]; +const [ACTIVE, FREESTYLE] = [1, 0]; + +const activeStrokes = (u8) => + 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/packages/fitfix/test/split.test.mjs b/packages/fitfix/test/split.test.mjs new file mode 100644 index 0000000..7fb7f41 --- /dev/null +++ b/packages/fitfix/test/split.test.mjs @@ -0,0 +1,291 @@ +/** + * 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, + ); +}); + +// --- from review: the split file is not what the watch recorded ----------- + +/** swim-08 with its frames passed through `edit` (a frame -> patch map, or undefined). */ +async function doctored(edit) { + const { patchFrame, writeFit } = await import('../src/fit-patch.js'); + const { header, frames } = readFit(await readFixture('swim-08.fit')); + const act = frames.filter( + (f) => f.kind === 'data' && f.globalNum === MSG_LENGTH && getField(f, F_TYPE) === ACTIVE, + ); + return writeFit( + header, + frames.map((f) => { + const patch = edit(f, act); + return patch ? patchFrame(f, patch) : f.bytes; + }), + ); +} + +const breaststrokes = (u8) => active(u8).filter((f) => getField(f, F_STROKE) !== 0).length; + +test('split: never makes the stroke worse, whatever lengths per lap is', async () => { + /* + * The missed-turn guard is computed on the split file, where each half looks + * like one ordinary length. A fixed target then merged the halves back and + * the doubled stroke count read as breaststroke again: ticking "I remember + * turning here" with lengths-per-lap 1 wrote four breaststroke lengths into + * this all-freestyle swim, against three without it. + */ + const u8 = await readFixture('swim-08.fit'); + for (const lengthsPerLap of ['auto', 1, 2, 3]) { + const off = breaststrokes(repair(u8, { lengthsPerLap }).bytes); + const on = breaststrokes(repair(u8, { lengthsPerLap, splitMissedTurns: true }).bytes); + assert.ok( + on <= off, + `lengthsPerLap ${lengthsPerLap}: ${on} breaststroke with the split, ${off} without`, + ); + } +}); + +test('split: says when a fixed target merges it straight back', async () => { + // "Now split into 2" beside a distance that did not move is a claim the file + // does not back; `gained` is what the page and CLI read to say so. + const u8 = await readFixture('swim-08.fit'); + const gained = (opts) => + analyze(u8, { ...opts, splitMissedTurns: true }).findings.find((f) => f.type === 'missed-turn') + .gained; + assert.equal(gained({}), 1, 'auto: the split adds its length'); + assert.equal(gained({ lengthsPerLap: 1 }), 0, 'fixed 1: merged straight back'); + assert.equal(gained({ lengthsPerLap: 2 }), 0, 'fixed 2: merged straight back'); +}); + +test('split: everything that reports the watch reports it as recorded', async () => { + /* + * Only the stat cards subtracted the made-up length at first. The Watch + * column said 3 for a lap the watch recorded as 2, the lap-structure guard + * said "24 lengths down to 16" beside a card saying the watch recorded 23, + * and --dry-run counted 41 lengths in a 40-length file. + */ + // Through both entry points: repair() runs its own copy of the pre-pass, and + // the first version of this fix only threaded the recorded-index map through + // analyze() -- which is what the tests used and the page does not. + const u8 = await readFixture('swim-08.fit'); + const key = (await keyOf()).valueOf(); + for (const [via, run] of [ + ['analyze', (o) => analyze(u8, o)], + ['repair', (o) => repair(u8, o).info], + ]) + for (const lengthsPerLap of ['auto', 1]) { + const off = run({ lengthsPerLap }); + const on = run({ lengthsPerLap, splitMissedTurns: [key] }); + assert.equal(on.lengths, off.lengths, `${via}, ${lengthsPerLap}: info.lengths`); + assert.deepEqual( + on.swimLaps.map((l) => l.recorded), + off.swimLaps.map((l) => l.lengths), + `${lengthsPerLap}: per-lap recorded counts`, + ); + const pick = (i, type, field) => + i.findings.filter((f) => f.type === type).map((f) => f[field]); + assert.deepEqual(pick(on, 'phantom-turn', 'detected'), pick(off, 'phantom-turn', 'detected')); + assert.deepEqual( + pick(on, 'lap-structure', 'lengthsBefore'), + pick(off, 'lap-structure', 'lengthsBefore'), + ); + assert.equal(on.madeUpLengths, 1); + } +}); + +test('split: a length sharing the missed turn start second is left alone', async () => { + // Keys were start times at first, and a start time is not unique. + const u8 = await doctored((f, act) => { + const missed = act.find((x) => Math.round(getField(x, F_ELAPSED) / 1000) === 151); + return f === act[act.indexOf(missed) + 1] + ? { [F_START]: getField(missed, F_START) } + : undefined; + }); + const lap = analyze(u8, { splitMissedTurns: true }).swimLaps.find((l) => l.lap === 11); + assert.deepEqual(lap.split, [true, true, false], 'only the missed turn, not its neighbour'); +}); + +test('split: keys stay put after another missed turn has been split', async () => { + /* + * The page ticks one, re-analyses the split file, and takes the next key + * from that. Every later index of the split file is shifted by the made-up + * length, so a key taken from it would point at the wrong length -- keys are + * indices into the file as recorded. + */ + const u8 = await doctored((f, act) => + f === act[act.length - 2] ? { [F_ELAPSED]: 160_000, 4: 160_000, [F_STROKES]: 56 } : undefined, + ); + const keys = analyze(u8) + .findings.filter((f) => f.type === 'missed-turn') + .map((f) => f.key); + assert.equal(keys.length, 2, 'the doctored file has two'); + const second = analyze(u8, { splitMissedTurns: [keys[0]] }).findings.find( + (f) => f.type === 'missed-turn' && !f.split, + ); + assert.equal(second.key, keys[1], 'the second key survives splitting the first'); + assert.deepEqual( + repair(u8, { splitMissedTurns: [keys[0], second.key] }).bytes, + repair(u8, { splitMissedTurns: true }).bytes, + ); +}); + +test('split: a part never goes negative, however few strokes there are', async () => { + /* + * From review. Non-last parts were rounded, so every one could round up and + * leave the last with a negative remainder: 2 strokes in 4 parts came out as + * 1, 1, 1 and -1. Written into an unsigned field, -1 is the invalid marker, + * read back as 0, and total strokes rose from 2 to 3 -- exactly the "adds + * nothing else" the split promises not to break. Needs a swim with about one + * stroke per length, so it is doctored: every length 1 stroke, and one of + * them four lengths long with 2. + */ + const u8 = await doctored((f, act) => { + if (!act.includes(f)) return undefined; + return f === act[5] ? { [F_ELAPSED]: 320_000, 4: 320_000, [F_STROKES]: 2 } : { [F_STROKES]: 1 }; + }); + const found = analyze(u8).findings.find((f) => f.type === 'missed-turn'); + assert.ok(found, 'the doctored length reads as a missed turn'); + assert.ok(found.looksLike >= 3, `split into ${found.looksLike}, enough parts to round badly`); + + const sum = (b) => active(b).reduce((a, f) => a + (getField(f, F_STROKES) ?? 0), 0); + const out = repair(u8, { splitMissedTurns: true }).bytes; + assert.equal(sum(out), sum(repair(u8).bytes), 'total strokes unchanged'); + assert.ok( + active(out).every((f) => getField(f, F_STROKES) !== null), + 'no part written as the invalid marker', + ); +}); diff --git a/web/app.js b/web/app.js index 99a79a1..45bd62d 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. @@ -89,6 +89,37 @@ 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) => + f.split && f.gained === 0 + ? `One recorded length took ${fmtSeconds(f.durS)} with ${f.strokes} strokes. It is ` + + 'split as you asked, but lengths per lap is set to a fixed number under ' + + 'Assumptions, which merges the parts straight back together — so the distance ' + + 'is unchanged by it. Set lengths per lap to auto for the split to count.' + : 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: '🤔', name: 'Two readings are possible', @@ -98,7 +129,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); /** @@ -165,6 +196,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; } @@ -221,7 +254,10 @@ 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 lengthsBefore = info.swimLaps.reduce((a, l) => a + l.recorded, 0); const distanceBefore = lengthsBefore * info.poolM; statsEl.innerHTML = [ @@ -258,6 +294,7 @@ function render() {
${esc(meta.name)}${lap}
${esc(meta.detail(f))}
+ ${meta.control ? meta.control(f) : ''}
`; }) @@ -286,9 +323,15 @@ 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; + 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(' · '); if (info.lengthUnitS) { workingUnit.textContent = @@ -313,19 +356,21 @@ 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); + const wasSplit = l.split.some(Boolean); return ` - + ${esc(l.lap + 1)} - ${esc(l.lengths)} - ${isMerged ? '' : ''}${esc(l.target)} - ${seen}${isMerged ? ' merged' : ''} + ${esc(l.recorded)} + ${l.target !== l.recorded ? '' : ''}${esc(l.target)} + ${seen}${isMerged ? ' merged' : ''}${missed ? ' possible missed turn' : ''}${wasSplit ? ' split — made up, not recorded' : ''} `; }) .join(''); @@ -401,6 +446,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; @@ -554,6 +601,7 @@ function reset() { state.input = null; state.output = null; state.anonymized = null; + state.splits = new Set(); picker.value = ''; filenameEl.textContent = ''; statsEl.innerHTML = ''; @@ -603,6 +651,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 = () => { @@ -622,6 +684,9 @@ for (const key of Object.keys(CONTROLS)) { el('resetOptions').addEventListener('click', () => { applyDefaults(); + // The default is no split. A switch that invents data does not survive a + // "reset to defaults". + state.splits = new Set(); if (state.input) runRepair(); else renderOptionState(readOptions()); }); 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.