diff --git a/packages/fitfix/src/cli.mjs b/packages/fitfix/src/cli.mjs index 65d318b..4ab7264 100755 --- a/packages/fitfix/src/cli.mjs +++ b/packages/fitfix/src/cli.mjs @@ -169,9 +169,43 @@ if (flags['dry-run']) { `pool ${info.poolM} m, timer ${(info.timerMs / 60000).toFixed(1)} min`, ); for (const f of info.findings) console.log(' -', f.type, JSON.stringify(f)); + printWorking(info); process.exit(0); } +/** + * The per-lap breakdown: what the watch recorded, what the repair will leave, + * and each recorded length on its own. + * + * The findings say *that* a lap was merged; this says why it was believable. + * "47 + 44" against a one-length reference of ~81 s is visibly one length the + * watch split in two, and "81 + 84 + 95" is visibly three real ones -- the + * totals alone cannot tell those apart, and that is the question someone is + * asking when a repaired distance looks wrong. Mirrors "Show the working" in + * the browser. + */ +function printWorking(info) { + if (!info.swimLaps.length) return; + console.log(''); + console.log( + info.lengthUnitS + ? ` working -- one length reads as ~${Math.round(info.lengthUnitS)} s` + : info.autoLengths + ? ' working -- auto, but no usable durations: each lap taken as one length' + : ' working -- lengths per lap was fixed, not inferred', + ); + // Lap numbers skip wherever the swimmer rested: a rest lap holds no lengths, + // so it has nothing to show. Said here so the gaps do not read as a bug. + 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(' + '); + console.log( + ` ${String(l.lap + 1).padStart(3)} ${String(l.lengths).padStart(5)} -> ${String(l.target).padEnd(5)} ${seen}${merged ? ' merged' : ''}`, + ); + } +} + const { bytes, summary, info } = repair(u8, opts); if (opts.lengthsPerLap === 'auto' && info.lengthUnitS) { diff --git a/packages/fitfix/src/swim-repair.js b/packages/fitfix/src/swim-repair.js index f656d72..a2d9a90 100644 --- a/packages/fitfix/src/swim-repair.js +++ b/packages/fitfix/src/swim-repair.js @@ -144,9 +144,18 @@ export function mergeToTarget(indices, target, durationOf) { * real swim anyway. */ if (size > 256) { - const chunk = Math.ceil(size / n); + // Exactly n groups, sizes differing by at most one. The first version cut + // fixed chunks of ceil(size / n), which yields *fewer* than n: 257 lengths + // at a target of 64 came out as 52 groups, while every caller -- the + // findings, the lap-structure guard and "Show the working" -- reported 64. + const base = Math.floor(size / n); + const extra = size % n; const groups = []; - for (let i = 0; i < size; i += chunk) groups.push(indices.slice(i, i + chunk)); + for (let g = 0, i = 0; g < n; g++) { + const len = base + (g < extra ? 1 : 0); + groups.push(indices.slice(i, i + len)); + i += len; + } return groups; } @@ -406,6 +415,12 @@ export function analyze(u8, opts = {}) { lapActive.forEach((act, li) => { const target = lapTargets[li]; if (!act.length) return; + // The groups repair() will produce for this lap, computed once and used + // for both the stroke classification below and the reported target. The + // target used to be min(recorded, lapTargets[li]) -- a second answer to + // the same question, and it did disagree with the merge (see the size + // fallback in mergeToTarget). Counting the groups cannot. + const groups = mergeToTarget(act, target, (k) => getField(lengths[k], F.length.elapsed)); const durMs = act.reduce((a, k) => a + getField(lengths[k], F.length.elapsed), 0); const strokes = act.reduce((a, k) => a + (getField(lengths[k], F.length.strokes) ?? 0), 0); const stroke = strokes >= strokeSplit ? 'breaststroke' : 'freestyle'; @@ -415,10 +430,28 @@ export function analyze(u8, opts = {}) { swimLaps.push({ lap: li, lengths: act.length, + /** What the repair will leave in this lap -- never more than `lengths`. */ + target: groups.length, durS: durMs / 1000, strokes, stroke, deviceStroke, + /* + * Each recorded length on its own, in order. The lap totals above cannot + * tell "47 s + 44 s" (one length the watch split in two) from "81 s + + * 84 s" (two real lengths) -- and that difference is the whole question + * a swimmer is asking when a repaired distance looks wrong. Exposed so a + * front end can show the evidence rather than only the verdict. + */ + // null where the watch recorded no duration, rather than durationOf's 0: + // a length that was never timed must not read as a zero-second one. + lengthsS: act.map((k) => { + const ms = getField(lengths[k], F.length.elapsed); + return ms === null ? null : ms / 1000; + }), + // 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)), }); if (act.length > target) @@ -440,7 +473,7 @@ export function analyze(u8, opts = {}) { * as "breaststroke, 61 strokes" and then written as freestyle for both * lengths. Reporting one thing and doing another is worse than either. */ - for (const group of mergeToTarget(act, target, (k) => getField(lengths[k], F.length.elapsed))) { + for (const group of groups) { const gDurMs = group.reduce((a, k) => a + getField(lengths[k], F.length.elapsed), 0); const gStrokes = group.reduce((a, k) => a + (getField(lengths[k], F.length.strokes) ?? 0), 0); const byStrokes = gStrokes >= strokeSplit; @@ -567,6 +600,13 @@ export function analyze(u8, opts = {}) { lapTargets, /** Seconds one real length takes, when it was inferred rather than given. */ lengthUnitS: unit, + /** + * Whether lengths per lap was left to be worked out. Not the same as + * `lengthUnitS` being set: 'auto' with no usable durations infers no unit + * and falls back to one length per lap, and a front end that took a null + * unit to mean "fixed" told the swimmer they had set a number they had not. + */ + autoLengths: o.lengthsPerLap === 'auto', }; } diff --git a/packages/fitfix/test/cli.test.mjs b/packages/fitfix/test/cli.test.mjs index e20988c..3c8fc8a 100644 --- a/packages/fitfix/test/cli.test.mjs +++ b/packages/fitfix/test/cli.test.mjs @@ -63,6 +63,18 @@ test('--dry-run reports without writing anything', async () => { await assert.rejects(readFile(out), /ENOENT/, '--dry-run wrote a file'); }); +test('--dry-run shows the working, lap by lap', async () => { + // swim-07 has exactly one phantom split among real multi-length blocks, so + // the breakdown has to show both the merge and the blocks it left alone. + const swim07 = join(dir, 'swim-07.fit'); + await writeFile(swim07, await readFixture('swim-07.fit')); + const { stdout } = await cli([swim07, '--dry-run']); + + assert.match(stdout, /working -- one length reads as ~81 s/); + assert.match(stdout, /^\s+2\s+2 -> 1\s+47 \+ 44\s+merged$/m, 'the phantom split, merged'); + assert.match(stdout, /^\s+18\s+3 -> 3\s+81 \+ 84 \+ 95$/m, 'a real block, kept'); +}); + test('repairs a file and honours the assumptions', async () => { const fixed = join(dir, 'fixed.fit'); const { stdout } = await cli([input, fixed]); diff --git a/packages/fitfix/test/working.test.mjs b/packages/fitfix/test/working.test.mjs new file mode 100644 index 0000000..0ccfba0 --- /dev/null +++ b/packages/fitfix/test/working.test.mjs @@ -0,0 +1,168 @@ +/** + * The per-lap breakdown ("show the working") that analyze() hands the front + * ends. + * + * It exists for one question: a swimmer looking at a repaired distance and not + * sure it is right. That makes its only real obligation honesty -- a preview + * that disagrees with the file it previews is worse than none, because it is + * exactly what the swimmer will trust. So the central test is not about the + * table's shape but that it predicts, lap by lap and in total, what repair() + * then actually writes. + */ + +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { getField, patchFrame, readFit, writeFit } from '../src/fit-patch.js'; +import { analyze, mergeToTarget, repair } from '../src/swim-repair.js'; +import { ORIGINALS, readFixture } from './fixtures.mjs'; + +const EPSILON = 1e-6; + +for (const name of ORIGINALS) { + test(`working: ${name} predicts the length count repair() writes`, async () => { + const u8 = await readFixture(name); + const info = analyze(u8); + const { summary } = repair(u8); + + const promised = info.swimLaps.reduce((a, l) => a + l.target, 0); + assert.equal(promised, summary.lengths, 'the preview must add up to the file'); + }); +} + +test('working: holds under a fixed lengths-per-lap as well as auto', async () => { + // A fixed target is the case most likely to merge away real distance, and so + // the one where the preview matters most. + for (const name of ORIGINALS) { + const u8 = await readFixture(name); + for (const lengthsPerLap of [1, 2, 3]) { + const opts = { lengthsPerLap }; + const promised = analyze(u8, opts).swimLaps.reduce((a, l) => a + l.target, 0); + assert.equal( + promised, + repair(u8, opts).summary.lengths, + `${name}, lengthsPerLap ${lengthsPerLap}`, + ); + } + } +}); + +test('working: each lap lists every recorded length, and they add up', async () => { + for (const name of ORIGINALS) { + for (const lap of analyze(await readFixture(name)).swimLaps) { + const where = `${name} lap ${lap.lap + 1}`; + assert.equal(lap.lengthsS.length, lap.lengths, `${where}: one duration per length`); + assert.equal(lap.lengthStrokes.length, lap.lengths, `${where}: one stroke count per length`); + + const total = lap.lengthsS.reduce((a, s) => a + (s ?? 0), 0); + assert.ok(Math.abs(total - lap.durS) < EPSILON, `${where}: durations sum to the lap`); + const strokes = lap.lengthStrokes.reduce((a, s) => a + (s ?? 0), 0); + assert.equal(strokes, lap.strokes, `${where}: strokes sum to the lap`); + + // Merging only ever goes down; nothing is split. + assert.ok(lap.target >= 1 && lap.target <= lap.lengths, `${where}: 1 <= target <= recorded`); + } + } +}); + +test('working: a phantom split reads as fragments of one length', async () => { + /* + * swim-07's second lap is the fixture's one true phantom turn: the watch saw + * two lengths of roughly half the inferred unit each. The breakdown has to + * show both fragments and the merge, because that pairing -- two halves + * against a one-length reference -- is what lets a swimmer see it was right. + */ + const info = analyze(await readFixture('swim-07.fit')); + const lap = info.swimLaps.find((l) => l.lengths > l.target); + assert.ok(lap, 'swim-07 has one merged lap'); + assert.equal(lap.lengths, 2); + assert.equal(lap.target, 1); + for (const s of lap.lengthsS) { + assert.ok(s < info.lengthUnitS * 0.75, `a fragment (${s}s) is well under one length`); + } + + // And the genuine blocks are left alone, which is the other half of trusting it. + const blocks = info.swimLaps.filter((l) => l.lengths > 1 && l.lengths === l.target); + assert.deepEqual( + blocks.map((l) => l.lengths), + [3, 2], + 'the 3- and 2-length blocks from swimming through the turn survive', + ); +}); + +test('working: a lap too big to partition exactly still yields the promised count', () => { + /* + * Past 256 lengths mergeToTarget gives up on the optimal partition and cuts + * equal chunks. It used to cut fixed chunks of ceil(size / n), which is + * *fewer* than n groups -- 257 lengths at a target of 64 came out as 52 -- + * while the table, the findings and the lap-structure guard all said 64. + * No real swim has a 257-length lap; a crafted file does, and the fixtures + * never reach this path, which is how it went unnoticed. + */ + const dur = () => 50; + for (const [size, n] of [ + [257, 64], + [257, 2], + [300, 225], + [1000, 7], + [257, 256], + ]) { + const indices = Array.from({ length: size }, (_, k) => k); + const groups = mergeToTarget(indices, n, dur); + assert.equal(groups.length, n, `${size} lengths at ${n}: group count`); + assert.ok( + groups.every((g) => g.length >= 1), + `${size} at ${n}: no empty group`, + ); + assert.deepEqual(groups.flat(), indices, `${size} at ${n}: every length once, in order`); + const sizes = groups.map((g) => g.length); + assert.ok(Math.max(...sizes) - Math.min(...sizes) <= 1, `${size} at ${n}: even split`); + } +}); + +/** swim-03 with the duration fields of the first `count` active lengths removed. */ +async function untimed(count) { + const [MSG_LENGTH, F_TYPE, F_ELAPSED, F_TIMER, ACTIVE] = [101, 12, 3, 4, 1]; + const { header, frames } = readFit(await readFixture('swim-03.fit')); + let left = count; + const out = frames.map((f) => { + if ( + left > 0 && + f.kind === 'data' && + f.globalNum === MSG_LENGTH && + getField(f, F_TYPE) === ACTIVE + ) { + left--; + return patchFrame(f, { [F_ELAPSED]: null, [F_TIMER]: null }); + } + return f.bytes; + }); + return writeFit(header, out); +} + +test('working: a length with no recorded duration is null, not zero seconds', async () => { + // Shown as "—" by both front ends. As 0 it read as a real, impossibly fast + // length -- exactly the kind of thing someone checking a merge would trust. + const info = analyze(await untimed(1)); + const all = info.swimLaps.flatMap((l) => l.lengthsS); + assert.equal(all.filter((s) => s === null).length, 1, 'exactly the one untimed length'); + assert.ok( + all.filter((s) => s !== null).every((s) => s > 0), + 'the rest are real durations', + ); +}); + +test('working: auto with nothing to measure says auto, not fixed', async () => { + // With every duration gone no unit can be inferred and auto falls back to + // one length per lap. `lengthUnitS` is null exactly as it is for a fixed + // number, so the front ends need `autoLengths` to tell the two apart. + const info = analyze(await untimed(Number.POSITIVE_INFINITY)); + assert.equal(info.lengthUnitS, null); + assert.equal(info.autoLengths, true); + assert.ok( + info.swimLaps.every((l) => l.target === 1), + 'falls back to one per lap', + ); + + const fixed = analyze(await readFixture('swim-03.fit'), { lengthsPerLap: 2 }); + assert.equal(fixed.autoLengths, false); +}); diff --git a/web/app.js b/web/app.js index e683b72..99a79a1 100644 --- a/web/app.js +++ b/web/app.js @@ -27,6 +27,10 @@ const busy = el('busy'); const chooser = el('chooser'); const chooserTitle = el('chooserTitle'); const chooserList = el('chooserList'); +const working = el('working'); +const workingBadge = el('workingBadge'); +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 }; @@ -231,6 +235,10 @@ function render() { statCard({ label: 'Swim time', value: fmtSeconds(summary.swimS) }), ].join(''); + // Before the findings, which return early when there are none -- and a swim + // with nothing wrong in it is still one whose working someone may want to see. + renderWorking(info); + const findings = info.findings; if (!findings.length) { findingsEl.innerHTML = '

✅ Nothing looks wrong in this file.

'; @@ -261,6 +269,68 @@ function render() { `; } +/** + * The per-lap breakdown under "Show the working": what the watch recorded, + * what the repair leaves, and every recorded length on its own. + * + * Everything here comes from analyze(), and the lap targets are the ones + * repair() is handed rather than recomputed, so the table cannot promise a + * distance the file does not get -- working.test.mjs holds the two to the same + * total. The open/closed state is left alone across re-renders: changing an + * assumption re-runs the repair, and snapping the panel shut each time would + * hide the very rows the change was meant to affect. + */ +function renderWorking(info) { + const laps = info.swimLaps; + working.hidden = !laps.length; + 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`; + + if (info.lengthUnitS) { + workingUnit.textContent = + `One length in this swim reads as about ${Math.round(info.lengthUnitS)} seconds. ` + + 'Two short lengths that add up to about one look like a turn the watch imagined, ' + + 'and are merged; lengths that are each about one look real, and are kept. That is ' + + 'the rule, not proof — check it against what you remember swimming.'; + } else if (info.autoLengths) { + workingUnit.textContent = + 'This file has no usable length durations to measure a length against, so ' + + 'each lap is treated as a single length. If that is wrong, set a number under ' + + 'Assumptions.'; + } else { + workingUnit.textContent = + 'Lengths per lap is set to a fixed number under Assumptions, so each lap is ' + + 'cut to that many rather than measured.'; + } + + workingRows.innerHTML = laps + .map((l) => { + const isMerged = l.lengths > l.target; + // 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) => + // A length the watch never timed is "—", not a confident "0 s". + s === null + ? '—' + : `${esc(Math.round(s))} s`, + ) + .join(' + '); + return ` + + ${esc(l.lap + 1)} + ${esc(l.lengths)} + ${isMerged ? '' : ''}${esc(l.target)} + ${seen}${isMerged ? ' merged' : ''} + `; + }) + .join(''); +} + function runRepair() { const opts = readOptions(); try { @@ -331,6 +401,8 @@ function withBusy(work) { } function loadBytes(name, label, bytes) { + // A new swim starts closed. Only re-runs of the same file keep it open. + working.open = false; state.name = name; filenameEl.textContent = label; state.input = bytes; @@ -488,6 +560,10 @@ function reset() { findingsEl.innerHTML = ''; removedList.innerHTML = ''; chooserList.innerHTML = ''; + workingRows.innerHTML = ''; + // Closed again for the next file: its working is a different swim's. + working.open = false; + working.hidden = true; result.hidden = true; errorBox.hidden = true; share.hidden = true; diff --git a/web/index.html b/web/index.html index 412b0e1..6ab7be2 100644 --- a/web/index.html +++ b/web/index.html @@ -232,6 +232,49 @@

🌊 phantomturn

+ + + diff --git a/web/style.css b/web/style.css index 7af442b..8368f47 100644 --- a/web/style.css +++ b/web/style.css @@ -535,6 +535,96 @@ body.dragging::after { padding-top: 1.1rem; } +/* ---------------------------------------------------------------- working */ +/* "Show the working": the same disclosure as Assumptions, holding a table. */ +.working-body { + padding: 1rem 1.1rem 1.1rem; + border-top: 1px solid var(--border); +} + +.working-unit, +.working-note { + margin: 0; + font-size: 0.875rem; + line-height: 1.55; + color: var(--muted); +} + +.working-note { + margin-top: 0.85rem; + font-size: 0.8rem; +} + +.working-table { + width: 100%; + margin-top: 0.85rem; + border-collapse: collapse; + font-size: 0.875rem; + /* Durations line up digit under digit, which is what makes "47 + 44" + against "81 + 84" readable at a glance. */ + font-variant-numeric: tabular-nums; +} + +.working-table th { + padding: 0 0.6rem 0.45rem; + border-bottom: 1px solid var(--border); + text-align: left; + font-size: 0.7rem; + font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.08em; + color: var(--muted); +} + +.working-table td { + padding: 0.45rem 0.6rem; + border-bottom: 1px solid var(--border); + vertical-align: top; +} + +.working-table tr:last-child td { + border-bottom: 0; +} + +.working-table th:first-child, +.working-table td:first-child { + padding-left: 0.2rem; +} + +/* Clear of the 3px tint bar, which it otherwise sat flush against. */ +.working-table tr.is-merged td:first-child { + padding-left: 0.6rem; +} + +.working-table .num { + white-space: nowrap; +} + +.working-table td:first-child { + color: var(--muted); +} + +/* The rows the repair changed, in the same tone as a phantom-turn finding -- + the two describe the same event, so they should look like it. */ +.working-table tr.is-merged td { + background: color-mix(in srgb, var(--warn-soft) 45%, var(--surface)); +} + +.working-table tr.is-merged td:first-child { + box-shadow: inset 3px 0 0 var(--warn); +} + +.working-table .dur { + white-space: nowrap; +} + +.working-tag { + margin-left: 0.35rem; + font-size: 0.75rem; + font-weight: 600; + color: var(--warn); +} + .option { display: grid; gap: 0.35rem;