From e741a53a96ed1b550580da68d90e8b2599f059a2 Mon Sep 17 00:00:00 2001 From: Max Winterstein <5927148+MaxWinterstein@users.noreply.github.com> Date: Fri, 25 Sep 2026 07:50:55 +0000 Subject: [PATCH 1/3] Show the working: every recorded length behind the repaired distance A repaired distance you are not sure about has no way to be checked today. The page shows the before and after totals and a finding per merged lap, but the lap totals cannot tell the two cases that matter apart: "47 s + 44 s" against a one-length reference of about 81 s is one length the watch split in two, and "81 s + 84 s" is two real ones. The swimmer can tell them apart at a glance -- given the numbers. analyze() now carries, per swim lap, the duration and stroke count of each recorded length and the target the repair will cut it to. Both front ends use it. In the browser it is a "Show the working" panel under the findings, collapsed by default because for most swims a twenty-row table is noise, with the merged laps tinted the same as their phantom-turn finding. Under --dry-run the CLI prints the same breakdown. The preview's one real obligation is honesty: a table that promises a distance the file does not get is worse than no table, since it is exactly what the swimmer will trust. working.test.mjs holds it to that -- the per-lap targets must sum to the length count repair() actually writes, on every fixture, under auto and under fixed lengths-per-lap of 1, 2 and 3. The targets are the ones repair() is handed rather than recomputed, so the two cannot drift. Two narrow-screen details: counts are bare numbers under headers that say what they count, because "1 length" in every cell wrapped and doubled the height of each row at 390px; and each duration is unbreakable, so a line wraps at the "+" rather than stranding a lone "s" -- the same orphaned-unit bug the stat tiles had. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/fitfix/src/cli.mjs | 32 ++++++++++ packages/fitfix/src/swim-repair.js | 11 ++++ packages/fitfix/test/cli.test.mjs | 12 ++++ packages/fitfix/test/working.test.mjs | 89 +++++++++++++++++++++++++++ web/app.js | 61 ++++++++++++++++++ web/index.html | 36 +++++++++++ web/style.css | 85 +++++++++++++++++++++++++ 7 files changed, 326 insertions(+) create mode 100644 packages/fitfix/test/working.test.mjs diff --git a/packages/fitfix/src/cli.mjs b/packages/fitfix/src/cli.mjs index 65d318b..4cb4223 100755 --- a/packages/fitfix/src/cli.mjs +++ b/packages/fitfix/src/cli.mjs @@ -169,9 +169,41 @@ 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` + : ' 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) => 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..695dc3d 100644 --- a/packages/fitfix/src/swim-repair.js +++ b/packages/fitfix/src/swim-repair.js @@ -415,10 +415,21 @@ 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: Math.min(act.length, target), 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. + */ + lengthsS: act.map(durationOf), + lengthStrokes: act.map((k) => getField(lengths[k], F.length.strokes) ?? 0), }); if (act.length > target) 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..b372bff --- /dev/null +++ b/packages/fitfix/test/working.test.mjs @@ -0,0 +1,89 @@ +/** + * 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 { analyze, 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); + assert.ok(Math.abs(total - lap.durS) < EPSILON, `${where}: durations sum to the lap`); + const strokes = lap.lengthStrokes.reduce((a, s) => a + s, 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', + ); +}); diff --git a/web/app.js b/web/app.js index e683b72..3cbd362 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,53 @@ 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`; + + workingUnit.textContent = info.lengthUnitS + ? `One length in this swim reads as about ${Math.round(info.lengthUnitS)} seconds. ` + + 'Two short lengths that add up to about one are a turn the watch imagined; ' + + 'lengths that are each about one are real, however many there are.' + : '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) => `${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 +386,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 +545,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..198cf80 100644 --- a/web/index.html +++ b/web/index.html @@ -232,6 +232,42 @@

🌊 phantomturn

+ + + diff --git a/web/style.css b/web/style.css index 7af442b..c00e63d 100644 --- a/web/style.css +++ b/web/style.css @@ -535,6 +535,91 @@ 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; +} + +.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; From 9f4e619ff2036773140c561f7ed3a042841604d2 Mon Sep 17 00:00:00 2001 From: Max Winterstein <5927148+MaxWinterstein@users.noreply.github.com> Date: Fri, 25 Sep 2026 08:01:44 +0000 Subject: [PATCH 2/3] Address the review: make the table's count the merge's own count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From an adversarial review of this branch. The one that mattered broke the guarantee the branch exists to make. Past 256 lengths in a lap, mergeToTarget gives up on the optimal partition and cuts equal chunks -- of ceil(size / n), which yields fewer than n groups. 257 lengths at a target of 64 came out as 52. The table said 64, and so did the phantom-turn finding and the lap-structure guard, which read the same target; all of them promised a distance the file did not get. The fallback now makes exactly n groups, sizes differing by at most one. And the table no longer computes its own answer at all: analyze() runs the merge once per lap and reports the number of groups it produced, so the figure shown is the figure written by construction rather than by agreement. No real swim holds a 257-length lap and no fixture reaches this path, which is how it survived. Two smaller ones: - A length the watch never timed reached the table as 0 s, because getField returns null and null / 1000 is 0. It is null now, shown as "—" in the page and "?" in the CLI -- a zero-second length is exactly the kind of number someone checking a merge would take at face value. - `auto` with no usable durations infers no unit and falls back to one length per lap. Both front ends read a null unit as "fixed", and told the swimmer they had set a number while the field said auto. analyze() now reports `autoLengths`, and the wording says what actually happened. And the "→" is aria-hidden rather than announced as "right arrow", the table is described by its explanatory sentence, and the lap number on a merged row is clear of the tint bar. The size fallback test fails on the old code and passes on this one; the reviewer's own reproductions (86 promised vs 74 written; 247 vs 172) now agree. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/fitfix/src/cli.mjs | 6 +- packages/fitfix/src/swim-repair.js | 37 ++++++++++-- packages/fitfix/test/working.test.mjs | 83 ++++++++++++++++++++++++++- web/app.js | 26 +++++++-- web/index.html | 2 +- web/style.css | 5 ++ 6 files changed, 143 insertions(+), 16 deletions(-) diff --git a/packages/fitfix/src/cli.mjs b/packages/fitfix/src/cli.mjs index 4cb4223..4ab7264 100755 --- a/packages/fitfix/src/cli.mjs +++ b/packages/fitfix/src/cli.mjs @@ -190,14 +190,16 @@ function printWorking(info) { console.log( info.lengthUnitS ? ` working -- one length reads as ~${Math.round(info.lengthUnitS)} s` - : ' working -- lengths per lap was fixed, not inferred', + : 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) => Math.round(s)).join(' + '); + 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' : ''}`, ); diff --git a/packages/fitfix/src/swim-repair.js b/packages/fitfix/src/swim-repair.js index 695dc3d..f34af48 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'; @@ -416,7 +431,7 @@ export function analyze(u8, opts = {}) { lap: li, lengths: act.length, /** What the repair will leave in this lap -- never more than `lengths`. */ - target: Math.min(act.length, target), + target: groups.length, durS: durMs / 1000, strokes, stroke, @@ -428,7 +443,12 @@ export function analyze(u8, opts = {}) { * a swimmer is asking when a repaired distance looks wrong. Exposed so a * front end can show the evidence rather than only the verdict. */ - lengthsS: act.map(durationOf), + // 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; + }), lengthStrokes: act.map((k) => getField(lengths[k], F.length.strokes) ?? 0), }); @@ -451,7 +471,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; @@ -578,6 +598,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/working.test.mjs b/packages/fitfix/test/working.test.mjs index b372bff..629f932 100644 --- a/packages/fitfix/test/working.test.mjs +++ b/packages/fitfix/test/working.test.mjs @@ -12,7 +12,8 @@ import assert from 'node:assert/strict'; import test from 'node:test'; -import { analyze, repair } from '../src/swim-repair.js'; +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; @@ -52,7 +53,7 @@ test('working: each lap lists every recorded length, and they add up', async () 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); + 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); assert.equal(strokes, lap.strokes, `${where}: strokes sum to the lap`); @@ -87,3 +88,81 @@ test('working: a phantom split reads as fragments of one length', async () => { '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 3cbd362..118a4b4 100644 --- a/web/app.js +++ b/web/app.js @@ -290,12 +290,21 @@ function renderWorking(info) { ? `${merged} lap${merged === 1 ? '' : 's'} merged` : `${laps.length} laps, none merged`; - workingUnit.textContent = info.lengthUnitS - ? `One length in this swim reads as about ${Math.round(info.lengthUnitS)} seconds. ` + + 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 are a turn the watch imagined; ' + - 'lengths that are each about one are real, however many there are.' - : 'Lengths per lap is set to a fixed number under Assumptions, so each lap is ' + + 'lengths that are each about one are real, however many there are.'; + } 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) => { @@ -303,13 +312,18 @@ 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) => `${esc(Math.round(s))} s`) + .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)} + ${isMerged ? '' : ''}${esc(l.target)} ${seen}${isMerged ? ' merged' : ''} `; }) diff --git a/web/index.html b/web/index.html index 198cf80..41b3f98 100644 --- a/web/index.html +++ b/web/index.html @@ -246,7 +246,7 @@

🌊 phantomturn

- +
diff --git a/web/style.css b/web/style.css index c00e63d..8368f47 100644 --- a/web/style.css +++ b/web/style.css @@ -591,6 +591,11 @@ body.dragging::after { 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; } From ad9afaf2121141d33a3fbe586343d1c79f0f6cb5 Mon Sep 17 00:00:00 2001 From: Max Winterstein <5927148+MaxWinterstein@users.noreply.github.com> Date: Fri, 25 Sep 2026 08:12:24 +0000 Subject: [PATCH 3/3] Say what the working shows, not more than it can From CodeRabbit on #6, and right. The panel's text claimed a certainty the tool does not have: two short lengths "are a turn the watch imagined", and a lap "only ever loses lengths it counted twice". The second is false under a wrong fixed target -- lengths-per-lap 1 on a swim with real multi-length blocks merges those blocks away, and this branch's own browser test does exactly that. Both contradict AGENTS.md, which is explicit that the data cannot distinguish phantom turns from a different lapping habit. That matters more here than anywhere else on the page: this panel exists for someone who doubts the repaired distance, and reassurance is the one thing it must not substitute for evidence. It now says the durations are the rule it applied, not proof, and that a wrong target loses real lengths -- which is what the table is there to reveal. Also: lengthStrokes keeps null where no stroke count was recorded, as lengthsS already does for a missing duration, instead of reporting it as zero. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/fitfix/src/swim-repair.js | 4 +++- packages/fitfix/test/working.test.mjs | 2 +- web/app.js | 5 +++-- web/index.html | 11 +++++++++-- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/fitfix/src/swim-repair.js b/packages/fitfix/src/swim-repair.js index f34af48..a2d9a90 100644 --- a/packages/fitfix/src/swim-repair.js +++ b/packages/fitfix/src/swim-repair.js @@ -449,7 +449,9 @@ export function analyze(u8, opts = {}) { const ms = getField(lengths[k], F.length.elapsed); return ms === null ? null : ms / 1000; }), - lengthStrokes: act.map((k) => getField(lengths[k], F.length.strokes) ?? 0), + // 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) diff --git a/packages/fitfix/test/working.test.mjs b/packages/fitfix/test/working.test.mjs index 629f932..0ccfba0 100644 --- a/packages/fitfix/test/working.test.mjs +++ b/packages/fitfix/test/working.test.mjs @@ -55,7 +55,7 @@ test('working: each lap lists every recorded length, and they add up', async () 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); + 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. diff --git a/web/app.js b/web/app.js index 118a4b4..99a79a1 100644 --- a/web/app.js +++ b/web/app.js @@ -293,8 +293,9 @@ function renderWorking(info) { 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 are a turn the watch imagined; ' + - 'lengths that are each about one are real, however many there are.'; + '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 ' + diff --git a/web/index.html b/web/index.html index 41b3f98..6ab7be2 100644 --- a/web/index.html +++ b/web/index.html @@ -262,8 +262,15 @@

🌊 phantomturn

Lap

Rest laps hold no lengths and are left out, so lap numbers skip - where you stopped. Nothing is ever split — a lap only ever loses - lengths it counted twice. + 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. +