From 5df126e272e14377b2838f87fc746e85ec71b461 Mon Sep 17 00:00:00 2001 From: "Ryan Johnson (ntninja)" Date: Fri, 3 Jul 2026 20:24:13 +0000 Subject: [PATCH] =?UTF-8?q?fix(marshaling):=20no=20rulings=20before=20comm?= =?UTF-8?q?it=20=E2=80=94=20kill=20click-to-insert;=20unified=20re-detecti?= =?UTF-8?q?on=20preview?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live-testing surfaced rulings appearing BEFORE any explicit commit while tuning detection thresholds, plus baffling "Lap inserted" audit entries while removing laps. Root cause: the RSSI trace svg carried an onclick that IMMEDIATELY sent an InsertLap at the cursor time — and the browser synthesizes a click on the svg when a threshold drag (pointerdown → pointerup on a handle) finishes inside it, so EVERY drag planted a phantom lap; stray clicks did the same. Fix 1 — no direct click-to-insert, ever (RssiGraph.svelte): - The svg's click-to-insert path is removed entirely. The ONLY add path is the explicit, labelled "Add lap here" button in the cursor readout under the plot (deliberate, un-misfireable). - The in-plot hover hint now points at that button ("add lap ↓ below"); the crosshair cursor cues the position readout, not a click action. - Lap-marker clicks (select-lap) keep working unchanged. - Tests: a trace click sends NOTHING (component + screen level); a full threshold drag followed by the browser-synthesized svg click sends NOTHING; the readout button still sends the exact heat-tagged InsertLap. Fix 2 — unified re-detection preview (redetect.ts + Marshaling.svelte): - New pure `previewRows(current, detected, tolerance?)`: ONE chronological list telling the whole story — laps derived from the surviving pass chain, each `kept` (closing pass matched an official one) or `added` (new), interleaved with `removed` rows for every official pass the re-detection drops (passes leaving the record — no lap number). Unit-tested: kept/added classification, removed interleaving in time order, pure-kept diffs, tolerance matching. - The tune panel's side-by-side preview-vs-official lap lists are replaced by that single list: kept rows plain, added rows accent-marked "+", removed rows struck/dimmed "− pass at Ts — removed". The "Would be N laps (+A, −R)" summary stays; the exact commit command batch (VoidDetection/InsertLap) is untouched — render-only. Gates: rd-console svelte-check (both passes) clean; vitest 42 files / 544 tests green; frontend eslint + prettier clean. Co-Authored-By: Claude Fable 5 --- .../apps/rd-console/src/lib/RssiGraph.svelte | 54 +++++------- frontend/apps/rd-console/src/lib/redetect.ts | 52 ++++++++++++ .../rd-console/src/screens/Marshaling.svelte | 82 +++++++++++++----- .../rd-console/tests/MarshalingScreen.test.ts | 85 +++++++++++++++++-- .../apps/rd-console/tests/RssiGraph.test.ts | 58 +++++++++---- .../apps/rd-console/tests/redetect.test.ts | 58 ++++++++++++- 6 files changed, 313 insertions(+), 76 deletions(-) diff --git a/frontend/apps/rd-console/src/lib/RssiGraph.svelte b/frontend/apps/rd-console/src/lib/RssiGraph.svelte index 18f5f44..8228cd5 100644 --- a/frontend/apps/rd-console/src/lib/RssiGraph.svelte +++ b/frontend/apps/rd-console/src/lib/RssiGraph.svelte @@ -56,14 +56,16 @@ onselect: (competitor: CompetitorRef, lap: Lap) => void; /** * Add a brand-new lap for a competitor at a source-clock time (the cursor's race-relative - * instant). Wired to a click on the trace / the "Add lap here" affordance at the crosshair. + * instant). Wired ONLY to the explicit, labelled "Add lap here" button in the cursor readout + * below the plot — never to a bare click on the trace: stray clicks (and the click the browser + * synthesizes when a threshold drag ends inside the svg) must not plant phantom laps. * Optional: when absent (or when `canControl` is false) the graph is review-only. */ onaddlap?: (competitor: CompetitorRef, at: number) => void; /** * Whether the session may mutate (the role gate — read-only pilots can't add laps). When false - * the add-lap affordance is hidden and a trace click does nothing, mirroring the parent's - * `canControl` boundary on every other correction. + * the "Add lap here" affordance is hidden, mirroring the parent's `canControl` boundary on + * every other correction. */ canControl?: boolean; /** @@ -316,18 +318,11 @@ hover = null; } - /** Click on the trace → add a lap at the cursor's race-relative time (role-gated). */ - function onTraceClick( - e: MouseEvent, - ct: CompetitorTrace, - span: { from: number; to: number } - ): void { - if (!canControl || !onaddlap) return; - const svg = e.currentTarget as SVGSVGElement; - const px = pointerX(e, svg); - const x = Math.min(PAD_L + plotW, Math.max(PAD_L, px)); - onaddlap(ct.competitor.competitor, Math.round(timeAt(x, span))); - } + // There is deliberately NO click-on-the-trace add-lap path: a bare svg click is un-labelled and + // misfires — every threshold drag that ends inside the svg makes the browser synthesize a click + // on it, and stray clicks land there too, each planting a phantom "Lap inserted" ruling (live + // 2026-07-03). The ONLY add path is the explicit "Add lap here" button in the cursor readout + // below the plot. // ── Draggable enter/exit threshold handles (the RH-style live tuning) ───────────────────────── // Only wired when `onthresholds` is supplied. A pointer drag on a threshold handle maps the @@ -432,21 +427,18 @@

No samples captured for this node.

{:else} {@const isHover = hover != null && hover.ref === ref} - - - + onHover(e, ct, span)} onmouseleave={clearHover} - onclick={(e: MouseEvent) => onTraceClick(e, ct, span)} > @@ -488,11 +480,7 @@ tabindex="0" aria-pressed={isSelected(ref, lap)} aria-label={`Lap ${lap.number} at ${formatMicros(lap.duration_micros)} — select`} - onclick={(e: MouseEvent) => { - // A marker click selects the lap; don't let it bubble to the SVG's add-lap handler. - e.stopPropagation(); - onselect(ref, lap); - }} + onclick={() => onselect(ref, lap)} onkeydown={(e: KeyboardEvent) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); @@ -582,10 +570,10 @@ rssi {Math.round(hover.rssi)} {#if canControl && onaddlap} - + click: add lapadd lap ↓ below {/if} {/if} @@ -696,6 +684,9 @@ display: block; width: 100%; height: 13rem; + /* The crosshair cues the position READOUT only — a click on the plot never acts (adding a lap + is the explicit, labelled button below the plot). */ + cursor: crosshair; } .frame { stroke: rgba(255, 255, 255, 0.12); @@ -829,9 +820,6 @@ } /* Hover crosshair + readout (the "where exactly is this?" guide). */ - .plot.addable { - cursor: crosshair; - } .crosshair { stroke: #ffd24a; stroke-width: 1; diff --git a/frontend/apps/rd-console/src/lib/redetect.ts b/frontend/apps/rd-console/src/lib/redetect.ts index 72335bc..8c1b808 100644 --- a/frontend/apps/rd-console/src/lib/redetect.ts +++ b/frontend/apps/rd-console/src/lib/redetect.ts @@ -155,6 +155,58 @@ export function previewLaps(passTimes: number[]): PreviewLap[] { return laps; } +/** + * One row of the UNIFIED re-detection preview (see {@link previewRows}): a single chronological + * list that tells the whole story — the laps the tuned thresholds would produce (`kept` when the + * lap's closing pass matches a current official one, `added` when it's new), interleaved with the + * official passes the re-detection drops (`removed` — passes leaving the record, so they carry no + * lap number). + */ +export type PreviewRow = + | { + status: 'kept' | 'added'; + /** 1-based lap number in the previewed (post-commit) lap list. */ + number: number; + /** Source-clock time (µs) of the pass that closes this lap. */ + at: number; + /** Lap duration (µs). */ + durationMicros: number; + } + | { + status: 'removed'; + /** Source-clock time (µs) of the official pass the re-detection drops. */ + at: number; + /** The pass's log offset — the `VoidDetection` target a commit sends. */ + ref: number; + }; + +/** + * The unified re-detection preview: ONE chronological row list combining the surviving lap chain + * with the passes that would be removed. Laps derive from the detected passes exactly like + * {@link previewLaps}; each lap is `kept` when its CLOSING pass matched an official pass (within + * `toleranceMicros`, per {@link diffPasses}) and `added` otherwise. Every official pass the + * re-detection no longer sees interleaves as a `removed` row at its own time. Preview-only — + * nothing here sends commands. + */ +export function previewRows( + current: OfficialPass[], + detected: number[], + toleranceMicros: number = DEFAULT_MATCH_TOLERANCE_MICROS +): PreviewRow[] { + const diff = diffPasses(current, detected, toleranceMicros); + const keptDetectedAt = new Set(diff.kept.map((k) => k.detectedAt)); + const rows: PreviewRow[] = previewLaps(detected).map((lap) => ({ + status: keptDetectedAt.has(lap.at) ? 'kept' : 'added', + number: lap.number, + at: lap.at, + durationMicros: lap.durationMicros + })); + for (const pass of diff.removed) rows.push({ status: 'removed', at: pass.at, ref: pass.ref }); + // Chronological (stable, so same-instant rows keep lap-then-removed order). + rows.sort((a, b) => a.at - b.at); + return rows; +} + /** * A competitor's current OFFICIAL passes, reconstructed from their (marshaling-corrected) lap * list: lap 1's opening pass (`start_ref`, at `lap1.at − lap1.duration`) plus every lap's diff --git a/frontend/apps/rd-console/src/screens/Marshaling.svelte b/frontend/apps/rd-console/src/screens/Marshaling.svelte index 661dc02..11efbdb 100644 --- a/frontend/apps/rd-console/src/screens/Marshaling.svelte +++ b/frontend/apps/rd-console/src/screens/Marshaling.svelte @@ -71,7 +71,7 @@ detectPasses, diffPasses, officialPasses, - previewLaps + previewRows } from '../lib/redetect.js'; import { commandForAction } from '../lib/transitions.js'; import type { Session } from '../lib/session.svelte.js'; @@ -338,13 +338,15 @@ // ── Add a brand-new lap (not an edit of an existing one) ── // The `InsertLap` path adds a missed/never-detected crossing for a competitor at a source-clock // time — so it works even when the competitor has ZERO laps. Two entry points feed it the time: - // • the graph: a click on a competitor's trace passes the cursor's race-relative source-time; + // • the graph: the explicit "Add lap here" button in the cursor readout under the trace passes + // the cursor's race-relative source-time (never a bare click on the trace — stray clicks and + // drag-end synthesized clicks must not plant laps); // • the per-competitor control: an "Add lap" button at a typed time (seconds), for sim heats // with no trace/graph. // Role-gated by `canControl` like every other correction (the parent only renders these when the // session may control; the Director re-checks). - /** Add a lap for a competitor at an exact source-clock time (µs) — the graph-click path. */ + /** Add a lap for a competitor at an exact source-clock time (µs) — the graph's button path. */ async function insertLap(competitor: CompetitorRef, at: number): Promise { if (!canControl || !heat) return; const ack = await session.send(insertLapCommand(adapter, competitor, Math.round(at), heat)); @@ -589,7 +591,11 @@ ); const redetectDiff = $derived(diffPasses(officialPasses(shownPilotLaps), detectedPassTimes)); const redetectDirty = $derived(redetectDiff.added.length > 0 || redetectDiff.removed.length > 0); - const previewLapList = $derived(previewLaps(detectedPassTimes)); + // The UNIFIED preview: one chronological row list — kept/added laps interleaved with the + // official passes the re-detection drops (redetect.ts `previewRows`), so the whole story reads + // top-down instead of a confusing side-by-side of preview vs official lists. + const previewRowList = $derived(previewRows(officialPasses(shownPilotLaps), detectedPassTimes)); + const previewLapCount = $derived(previewRowList.filter((r) => r.status !== 'removed').length); function doResetThresholds(): void { if (!tuneTrace) return; @@ -823,19 +829,33 @@

{:else}

- Would be {previewLapList.length} lap{previewLapList.length === 1 ? '' : 's'} + Would be {previewLapCount} lap{previewLapCount === 1 ? '' : 's'} (+{redetectDiff.added.length} added, −{redetectDiff.removed.length} removed)

{#if redetectDirty} +
    - {#each previewLapList as lap (lap.at)} -
  1. - Lap {lap.number} - {formatMicros(lap.durationMicros)} -
    2. + {#each previewRowList as row (row.status === 'removed' ? `x${row.ref}` : `l${row.at}`)} + {#if row.status === 'removed'} +
  2. + + pass at {formatMicros(row.at)}s — removed +
    3. + {:else} +
  3. + + Lap {row.number} + {formatMicros(row.durationMicros)} +
    4. + {/if} {/each}
{/if} @@ -1416,29 +1436,53 @@ background: var(--gf-accent); color: #061018; } - .preview-laps { + /* The unified re-detection preview: one chronological list, big mono rows (sunlit-laptop + readable). Kept rows read plain; added rows carry the accent "+" chip styling; removed + rows read struck/dimmed danger — "this pass leaves the record on commit" at a glance. */ + .preview-rows { margin: var(--gf-space-2) 0 0; padding: 0; list-style: none; display: flex; - flex-wrap: wrap; - gap: var(--gf-space-2); + flex-direction: column; + gap: var(--gf-space-1); + max-width: 26rem; } - .preview-laps li { + .preview-rows li { display: flex; align-items: baseline; gap: var(--gf-space-2); padding: var(--gf-space-1) var(--gf-space-3); - border: 1px dashed color-mix(in srgb, var(--gf-accent) 55%, var(--gf-border)); + border: 1px solid transparent; border-radius: var(--gf-radius-sm); - background: var(--gf-surface-sunken); font-family: var(--gf-font-mono); font-size: var(--gf-font-size-sm); color: var(--gf-text-secondary); } - .preview-laps .lap-num { + .preview-rows .mark { + /* Fixed-width status column so the lap labels align across kept/added/removed rows. */ + min-width: 1ch; font-weight: var(--gf-font-weight-semibold); } + .preview-rows .lap-num { + font-weight: var(--gf-font-weight-semibold); + } + .preview-rows li.added { + border-color: color-mix(in srgb, var(--gf-accent) 55%, var(--gf-border)); + border-style: dashed; + background: var(--gf-surface-sunken); + color: var(--gf-text); + } + .preview-rows li.added .mark { + color: var(--gf-accent); + } + .preview-rows li.removed { + color: color-mix(in srgb, var(--gf-danger) 65%, var(--gf-text-muted)); + opacity: 0.8; + } + .preview-rows li.removed .what { + text-decoration: line-through; + } .result-actions { border-color: color-mix(in srgb, var(--gf-accent) 35%, var(--gf-border)); } diff --git a/frontend/apps/rd-console/tests/MarshalingScreen.test.ts b/frontend/apps/rd-console/tests/MarshalingScreen.test.ts index c0593d1..1f7cfaa 100644 --- a/frontend/apps/rd-console/tests/MarshalingScreen.test.ts +++ b/frontend/apps/rd-console/tests/MarshalingScreen.test.ts @@ -484,7 +484,11 @@ describe('Marshaling (Slice 3)', () => { ); }); - it('clicking the trace adds a NEW lap at the cursor source-time (InsertLap)', async () => { + it('a trace click sends NOTHING — only the explicit "Add lap here" button inserts', async () => { + // No ruling may exist before an explicit action: the old click-to-insert path fired an + // immediate InsertLap on ANY svg click — including the click the browser synthesizes when + // a threshold drag ends inside the svg — planting phantom "Lap inserted" rulings + // (live 2026-07-03). const { session, sendSpy } = makeTestSession({ live: liveRunning, laps: lapList, @@ -507,11 +511,23 @@ describe('Marshaling (Slice 3)', () => { toJSON: () => ({}) } as DOMRect); - // ALICE's trace spans 0..90s over plotW=984 from PAD_L=8 → click at X=500 ≈ 45.0s. + // A bare click on the trace (X=500 ≈ 45.0s) sends no command at all. await fireEvent.click(svg, { clientX: 500 }); + expect(sendSpy).not.toHaveBeenCalled(); + + // The deliberate path still works: hover places the cursor, the labelled readout button + // inserts — the exact heat-tagged InsertLap at the cursor's source-time. + await fireEvent.mouseMove(svg, { clientX: 500 }); + await fireEvent.click( + within(graph).getByRole('button', { name: /Add lap for ALICE at .* seconds/ }) + ); expect(sendSpy).toHaveBeenCalledTimes(1); - const cmd = sendSpy.mock.calls[0][0] as { InsertLap: { competitor: string; at: number } }; + const cmd = sendSpy.mock.calls[0][0] as { + InsertLap: { adapter: string; competitor: string; at: number; heat: string }; + }; + expect(cmd.InsertLap.adapter).toBe('rh-1'); expect(cmd.InsertLap.competitor).toBe('ALICE'); + expect(cmd.InsertLap.heat).toBe('heat-1'); // X=500 → ((500-8)/984)*90s = 45.0s in source micros. expect(cmd.InsertLap.at).toBeGreaterThan(44_500_000); expect(cmd.InsertLap.at).toBeLessThan(45_500_000); @@ -1117,11 +1133,20 @@ describe('Marshaling (Slice 3)', () => { expect(screen.getByTestId('redetect-summary')).toHaveTextContent( 'Would be 3 laps (+1 added, −0 removed)' ); - // The preview lap list renders the would-be laps (the 41→60s split: 19s + 21s). - const previewList = screen.getByLabelText(/Preview laps for ALICE/); - expect(previewList).toHaveTextContent('Lap 2'); - expect(previewList).toHaveTextContent('19.000'); - expect(previewList).toHaveTextContent('21.000'); + // The UNIFIED preview list: one chronological story — the surviving laps with the new lap + // accent-marked. The 41→60s split makes lap 2 the ADDED one (19s), laps 1 + 3 kept. + const previewList = screen.getByLabelText(/Re-detection preview for ALICE/); + const rows = previewList.querySelectorAll('li'); + expect(rows).toHaveLength(3); + expect(rows[0].classList.contains('kept')).toBe(true); + expect(rows[0]).toHaveTextContent('Lap 1'); + expect(rows[0]).toHaveTextContent('40.000'); + expect(rows[1].classList.contains('added')).toBe(true); + expect(rows[1]).toHaveTextContent('+ Lap 2'); + expect(rows[1]).toHaveTextContent('19.000'); + expect(rows[2].classList.contains('kept')).toBe(true); + expect(rows[2]).toHaveTextContent('Lap 3'); + expect(rows[2]).toHaveTextContent('21.000'); // NOTHING was sent while adjusting — commit is explicit. expect(sendSpy).not.toHaveBeenCalled(); expect( @@ -1129,6 +1154,50 @@ describe('Marshaling (Slice 3)', () => { ).toBe(false); }); + it('passes the new levels DROP interleave as struck "removed" rows in the one preview list', async () => { + const { session, sendSpy } = renderTune(); + render(Marshaling, { session }); + + // Raise enter above every peak but the 132 one: only the 41s pass survives — the holeshot + // (1s) and the 81s gate pass leave the record. They render as removed rows, in time order, + // in the SAME list (no side-by-side preview-vs-official confusion). + await fireEvent.input(screen.getByLabelText('Enter threshold'), { + target: { value: '131' } + }); + expect(screen.getByTestId('redetect-summary')).toHaveTextContent( + 'Would be 0 laps (+0 added, −2 removed)' + ); + const previewList = screen.getByLabelText(/Re-detection preview for ALICE/); + const rows = previewList.querySelectorAll('li'); + expect(rows).toHaveLength(2); + expect(rows[0].classList.contains('removed')).toBe(true); + expect(rows[0]).toHaveTextContent('− pass at 1.000s — removed'); + expect(rows[1].classList.contains('removed')).toBe(true); + expect(rows[1]).toHaveTextContent('− pass at 1:21.000s — removed'); + // Still preview-only — nothing sent. + expect(sendSpy).not.toHaveBeenCalled(); + }); + + it('finishing a threshold DRAG on the graph sends nothing (no phantom "Lap inserted")', async () => { + // The live bug (2026-07-03): releasing a threshold drag inside the svg makes the browser + // synthesize a click on it — the old click-to-insert path turned EVERY drag into an + // immediate InsertLap ruling, before any commit. + const { session, sendSpy } = renderTune(); + render(Marshaling, { session }); + + const svg = screen.getByLabelText(/RSSI trace for ALICE/); + const handle = screen.getByRole('slider', { name: 'Enter threshold for ALICE' }); + // The full drag gesture (jsdom has no PointerEvent — a MouseEvent with the pointer type)… + fireEvent(handle, new MouseEvent('pointerdown', { bubbles: true, clientY: 40 })); + fireEvent(handle, new MouseEvent('pointermove', { bubbles: true, clientY: 30 })); + fireEvent(handle, new MouseEvent('pointerup', { bubbles: true })); + // …then the click the browser synthesizes on the svg where the pointer was released. + await fireEvent.click(svg, { clientX: 500, clientY: 30 }); + + // Rulings exist only after an explicit action — the drag+click sent NOTHING. + expect(sendSpy).not.toHaveBeenCalled(); + }); + it('flags equal/inverted thresholds instead of detecting', async () => { const { session } = renderTune(); render(Marshaling, { session }); diff --git a/frontend/apps/rd-console/tests/RssiGraph.test.ts b/frontend/apps/rd-console/tests/RssiGraph.test.ts index 67e3d89..5a8c81c 100644 --- a/frontend/apps/rd-console/tests/RssiGraph.test.ts +++ b/frontend/apps/rd-console/tests/RssiGraph.test.ts @@ -182,12 +182,14 @@ describe('RssiGraph hover readout', () => { }); /** - * Click-to-add-a-lap (gap 2): a click on a competitor's trace calls `onaddlap` with the cursor's - * source-time (the px→source-time mapping), role-gated by `canControl`. The explicit "Add lap here" - * button at the crosshair does the same for trackpad/keyboard users. + * Add-a-lap is EXPLICIT-ONLY: a bare click on the trace must never add a lap — the browser + * synthesizes a click on the svg when a threshold drag ends inside it, and stray clicks land there + * too; each used to plant a phantom "Lap inserted" ruling (live 2026-07-03). The only add path is + * the labelled "Add lap here" button in the cursor readout below the plot, role-gated by + * `canControl`. */ describe('RssiGraph add-lap', () => { - it('clicking the trace at a known X calls onaddlap with the matching source-time', async () => { + it('a click on the trace sends NOTHING — the trace has no click-to-insert path', async () => { const onaddlap = vi.fn(); render(RssiGraph, { trace: signalTrace, @@ -200,17 +202,12 @@ describe('RssiGraph add-lap', () => { const svg = screen.getByLabelText(/RSSI trace for ALICE/); pinSvgBox(svg); - // Click at the X mapping to 30.0s. + // Click at the X mapping to 30.0s — the old click-to-insert misfire surface. await fireEvent.click(svg, { clientX: xForTime(30_000_000) }); - expect(onaddlap).toHaveBeenCalledTimes(1); - const [ref, at] = onaddlap.mock.calls[0]; - expect(ref).toBe('ALICE'); - // The source-time is the cursor's race-relative instant (≈30.0s), within a rounding tolerance. - expect(at).toBeGreaterThan(29_900_000); - expect(at).toBeLessThan(30_100_000); + expect(onaddlap).not.toHaveBeenCalled(); }); - it('the "Add lap here" button adds at the cursor time', async () => { + it('the "Add lap here" button adds at the cursor time (the ONLY add path)', async () => { const onaddlap = vi.fn(); render(RssiGraph, { trace: signalTrace, @@ -231,7 +228,7 @@ describe('RssiGraph add-lap', () => { expect(onaddlap.mock.calls[0][1]).toBeLessThan(50_100_000); }); - it('clicking a lap marker selects (not adds) — the add-lap handler does not fire', async () => { + it('clicking a lap marker still selects (not adds) — the add-lap handler does not fire', async () => { const onaddlap = vi.fn(); const onselect = vi.fn(); render(RssiGraph, { @@ -257,7 +254,7 @@ describe('RssiGraph add-lap', () => { expect(svg.querySelector('.enter-line')).not.toBeNull(); }); - it('is review-only when canControl is false: a trace click does nothing', async () => { + it('is review-only when canControl is false: no "Add lap here" affordance is offered', async () => { const onaddlap = vi.fn(); render(RssiGraph, { trace: signalTrace, @@ -271,7 +268,7 @@ describe('RssiGraph add-lap', () => { pinSvgBox(svg); await fireEvent.click(svg, { clientX: xForTime(30_000_000) }); expect(onaddlap).not.toHaveBeenCalled(); - // …and no "Add lap here" affordance is offered. + // Hovering shows the readout but never the add button. await fireEvent.mouseMove(svg, { clientX: xForTime(30_000_000) }); expect(screen.queryByRole('button', { name: /Add lap for/ })).toBeNull(); }); @@ -349,6 +346,37 @@ describe('RssiGraph threshold tuning + preview', () => { expect(onthresholds).toHaveBeenLastCalledWith('ALICE', 110, 85); }); + it('finishing a threshold DRAG never adds a lap (the browser-synthesized click is inert)', async () => { + // The live bug (2026-07-03): pointerdown→pointerup on a handle inside the svg makes the + // browser synthesize a CLICK on the svg — with the old click-to-insert path, EVERY threshold + // drag planted a phantom "Lap inserted". The synthesized click must send nothing. + const onaddlap = vi.fn(); + const onthresholds = vi.fn(); + render(RssiGraph, { + trace: signalTrace, + laps: lapList, + selected: null, + onselect: () => {}, + onaddlap, + canControl: true, + onthresholds + }); + const svg = screen.getByLabelText(/RSSI trace for ALICE/); + pinSvgBox(svg); + + // The full drag gesture on the enter handle… + const handle = screen.getByRole('slider', { name: 'Enter threshold for ALICE' }); + await firePointer(handle, 'pointerdown', { clientY: yForValue(110) }); + await firePointer(handle, 'pointermove', { clientY: yForValue(120) }); + await firePointer(handle, 'pointerup', {}); + // …then the click the browser synthesizes on the svg where the pointer was released. + await fireEvent.click(svg, { clientX: xForTime(45_000_000), clientY: yForValue(120) }); + + // The drag tuned the thresholds — but NOTHING was inserted. + expect(onthresholds).toHaveBeenLastCalledWith('ALICE', 120, 95); + expect(onaddlap).not.toHaveBeenCalled(); + }); + it('arrow keys nudge the focused handle ±1 (keyboard access)', async () => { const onthresholds = vi.fn(); render(RssiGraph, { diff --git a/frontend/apps/rd-console/tests/redetect.test.ts b/frontend/apps/rd-console/tests/redetect.test.ts index c2af8e9..cd71d31 100644 --- a/frontend/apps/rd-console/tests/redetect.test.ts +++ b/frontend/apps/rd-console/tests/redetect.test.ts @@ -6,7 +6,8 @@ import { detectPasses, diffPasses, officialPasses, - previewLaps + previewLaps, + previewRows } from '../src/lib/redetect.js'; /** A uniform-grid trace: sample `i` at `i·period` (1s cadence by default). */ @@ -148,6 +149,61 @@ describe('previewLaps (consecutive-pass lap derivation)', () => { }); }); +describe('previewRows (the unified re-detection preview)', () => { + // The canonical official chain: holeshot at 1s, gate passes at 41s + 81s (two 40s laps). + const current = [ + { at: 1 * S, ref: 10 }, + { at: 41 * S, ref: 12 }, + { at: 81 * S, ref: 14 } + ]; + + it('classifies each preview lap by its CLOSING pass: matched official → kept, new → added', () => { + // The re-detection keeps the official chain and finds one NEW pass at 60s: the 41→60s lap + // closes on the new pass (added); the 1→41s and 60→81s laps close on matched passes (kept). + const rows = previewRows(current, [1 * S, 41 * S, 60 * S, 81 * S]); + expect(rows).toEqual([ + { status: 'kept', number: 1, at: 41 * S, durationMicros: 40 * S }, + { status: 'added', number: 2, at: 60 * S, durationMicros: 19 * S }, + { status: 'kept', number: 3, at: 81 * S, durationMicros: 21 * S } + ]); + }); + + it('interleaves dropped official passes as `removed` rows in time order (no lap number)', () => { + // The re-detection no longer sees the 41s pass: one long 1→81s lap remains (closing on the + // matched 81s pass → kept), and the dropped 41s pass interleaves BEFORE it at its own time. + const rows = previewRows(current, [1 * S, 81 * S]); + expect(rows).toEqual([ + { status: 'removed', at: 41 * S, ref: 12 }, + { status: 'kept', number: 1, at: 81 * S, durationMicros: 80 * S } + ]); + }); + + it('a pure-kept diff yields all-kept rows (nothing added, nothing removed)', () => { + const rows = previewRows(current, [1 * S, 41 * S, 81 * S]); + expect(rows).toEqual([ + { status: 'kept', number: 1, at: 41 * S, durationMicros: 40 * S }, + { status: 'kept', number: 2, at: 81 * S, durationMicros: 40 * S } + ]); + expect(rows.every((r) => r.status === 'kept')).toBe(true); + }); + + it('matches within tolerance like diffPasses — a re-timed pass still reads as kept', () => { + // The 41s pass re-detects 300ms later (within the 500ms default tolerance): same pass, so + // the lap closing on it stays `kept` (durations follow the DETECTED times, like previewLaps). + const rows = previewRows(current, [1 * S, 41 * S + 300_000, 81 * S]); + expect(rows.map((r) => r.status)).toEqual(['kept', 'kept']); + expect(rows[0]).toMatchObject({ at: 41 * S + 300_000, durationMicros: 40 * S + 300_000 }); + }); + + it('detecting nothing turns every official pass into a removed row (and no laps)', () => { + expect(previewRows(current, [])).toEqual([ + { status: 'removed', at: 1 * S, ref: 10 }, + { status: 'removed', at: 41 * S, ref: 12 }, + { status: 'removed', at: 81 * S, ref: 14 } + ]); + }); +}); + describe('officialPasses (lap list → boundary passes)', () => { it('reconstructs lap 1’s opening pass plus every closing pass', () => { const laps: Lap[] = [