Show the working: every recorded length behind the repaired distance

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) {

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',
   };
 }
 

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]);

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);
+});