diff --git a/AGENTS.md b/AGENTS.md index 1a271f5..3ce28c3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -300,21 +300,22 @@ 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, 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 +324,26 @@ 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 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 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..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( @@ -200,11 +246,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.recorded).padStart(5)} -> ${String(l.target).padEnd(5)} ${seen}${merged ? ' merged' : ''}${missed ? ' possible missed turn' : ''}${l.split.some(Boolean) ? ' split (* made up)' : ''}`, ); } } @@ -222,19 +270,37 @@ 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. + * 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. 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..27c15a9 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, }; @@ -227,7 +242,7 @@ const SECONDS_PER_100M = 200; * 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. + * example, which is why it only reports unless the swimmer asks for a split. */ const MISSED_TURN_RATIO = 1.75; @@ -350,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); @@ -476,7 +612,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]; @@ -496,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, @@ -520,6 +661,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 +671,15 @@ export function analyze(u8, opts = {}) { 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), @@ -539,15 +691,17 @@ export function analyze(u8, opts = {}) { }); } - if (act.length > target) + // 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`, }); /* @@ -569,7 +723,12 @@ export function analyze(u8, opts = {}) { ); const ambiguous = byStrokes !== byTime; - const holdsMissedTurn = group.some((k) => missed.has(k)); + // 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); @@ -610,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)); @@ -683,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 @@ -708,9 +882,10 @@ 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 { 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 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..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 ee61646..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. @@ -94,11 +94,31 @@ 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 && 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: '🤔', @@ -176,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; } @@ -232,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 = [ @@ -269,6 +294,7 @@ function render() {
${esc(meta.name)}${lap}
${esc(meta.detail(f))}
+ ${meta.control ? meta.control(f) : ''}
`; }) @@ -298,9 +324,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 +360,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' : ''} + ${esc(l.recorded)} + ${l.target !== l.recorded ? '' : ''}${esc(l.target)} + ${seen}${isMerged ? ' merged' : ''}${missed ? ' possible missed turn' : ''}${wasSplit ? ' split — made up, not recorded' : ''} `; }) .join(''); @@ -417,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; @@ -570,6 +601,7 @@ function reset() { state.input = null; state.output = null; state.anonymized = null; + state.splits = new Set(); picker.value = ''; filenameEl.textContent = ''; statsEl.innerHTML = ''; @@ -619,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 = () => { @@ -638,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.