Skip to content

Marshaling: no rulings before commit — kill click-to-insert; unified re-detection preview #354

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ryan-johnson2 merged 1 commit into from
Jul 3, 2026
Merged

Marshaling: no rulings before commit — kill click-to-insert; unified re-detection preview #354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 21 additions & 33 deletions frontend/apps/rd-console/src/lib/RssiGraph.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
/**
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -432,21 +427,18 @@
<p class="empty">No samples captured for this node.</p>
{:else}
{@const isHover = hover != null && hover.ref === ref}
<!-- The pointer handlers drive the hover crosshair + the click-to-add-lap convenience; the
accessible, keyboard-operable add path is the labelled "Add lap here" DOM button below
the plot, so the SVG itself stays a non-interactive `role="img"` figure. -->
<!-- svelte-ignore a11y_click_events_have_key_events -->
<!-- svelte-ignore a11y_no_noninteractive_element_interactions -->
<!-- The pointer handlers drive the hover crosshair/readout ONLY — the svg itself carries no
click action (a bare click must never mutate; see the add-lap note in the script). The
accessible, deliberate add path is the labelled "Add lap here" DOM button below the
plot, so the SVG stays a non-interactive `role="img"` figure. -->
<svg
class="plot"
class:addable={canControl && onaddlap != null}
viewBox={`0 0 ${W} ${H}`}
preserveAspectRatio="none"
role="img"
aria-label={`RSSI trace for ${who} with ${compLaps.length} lap markers`}
onmousemove={(e: MouseEvent) => onHover(e, ct, span)}
onmouseleave={clearHover}
onclick={(e: MouseEvent) => onTraceClick(e, ct, span)}
>
<!-- Plot frame -->
<rect class="frame" x={PAD_L} y={PAD_T} width={plotW} height={plotH} fill="none" />
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -582,10 +570,10 @@
<text class="readout-rssi" x="6" y="28">rssi {Math.round(hover.rssi)}</text>
</g>
{#if canControl && onaddlap}
<!-- The "Add lap here" affordance lives in the DOM readout below so it's a real,
labelled, click-target button; this hint just cues that a click adds a lap. -->
<!-- The add affordance is the real, labelled "Add lap here" button in the readout
BELOW the plot (clicking the trace itself never adds); this hint points at it. -->
<text class="add-hint" x={flip ? hx - 6 : hx + 6} y={PAD_T + plotH - 6}
>click: add lap</text
>add lap ↓ below</text
>
{/if}
{/if}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -829,9 +820,6 @@
}

/* Hover crosshair + readout (the "where exactly is this?" guide). */
.plot.addable {
cursor: crosshair;
}
.crosshair {
stroke: #ffd24a;
stroke-width: 1;
Expand Down
52 changes: 52 additions & 0 deletions frontend/apps/rd-console/src/lib/redetect.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
82 changes: 63 additions & 19 deletions frontend/apps/rd-console/src/screens/Marshaling.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<void> {
if (!canControl || !heat) return;
const ack = await session.send(insertLapCommand(adapter, competitor, Math.round(at), heat));
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -823,19 +829,33 @@
</p>
{:else}
<p class="tune-summary" role="status" data-testid="redetect-summary">
Would be {previewLapList.length} lap{previewLapList.length === 1 ? '' : 's'}
Would be {previewLapCount} lap{previewLapCount === 1 ? '' : 's'}
(+{redetectDiff.added.length} added, −{redetectDiff.removed.length} removed)
</p>
{#if redetectDirty}
<!-- The UNIFIED preview: one chronological list — the lap list the commit would
produce, with each new lap accent-marked (+) and each official pass the new
levels drop struck through (−) at its place in time. Replaces the old
side-by-side preview-vs-official lists (confusing in the field). -->
<ol
class="preview-laps"
aria-label={`Preview laps for ${competitorName(shownPilot)}`}
class="preview-rows"
aria-label={`Re-detection preview for ${competitorName(shownPilot)}`}
>
{#each previewLapList as lap (lap.at)}
<li>
<span class="lap-num">Lap {lap.number}</span>
<span class="lap-time">{formatMicros(lap.durationMicros)}</span>
</li>
{#each previewRowList as row (row.status === 'removed' ? `x${row.ref}` : `l${row.at}`)}
{#if row.status === 'removed'}
<li class="removed">
<span class="mark" aria-hidden="true">−</span>
<span class="what">pass at {formatMicros(row.at)}s — removed</span>
</li>
{:else}
<li class={row.status}>
<span class="mark" aria-hidden="true"
>{row.status === 'added' ? '+' : ''}</span
>
<span class="lap-num">Lap {row.number}</span>
<span class="lap-time">{formatMicros(row.durationMicros)}</span>
</li>
{/if}
{/each}
</ol>
{/if}
Expand Down Expand Up @@ -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));
}
Expand Down
Loading