Skip to content

Commit bb61e76

Browse files
fix: place user overrides first in deconfliction + add comprehensive tests
Fix a bug where a hash-based run sorting before a user-overridden run would not be deconflicted against the user's explicit color choice. The algorithm now has three phases: - Phase 0: Place ALL user-overridden colors as immovable landmarks first, regardless of sort position. This ensures every hash-based run is always checked against every user-chosen color. - Phase 1: Place cached non-user runs (preserving their deconflictions). - Phase 2: Process remaining new non-user runs against the full set. Add comprehensive test suite (oklch_colors_test.ts) covering: - Basic: no runs, single run, distant colors (no deconfliction) - Clash detection: identical colors, very similar colors, multiple clashes - User overrides: never deconflicted, respected as landmarks, hash runs checked against user colors regardless of sort order - Mixed scenarios: user override in middle, user override sorting later - Replacement quality: 3-axis search verified, sRGB-valid output - Incremental/cache: cached runs preserved, new runs checked against all - Determinism: same inputs → same outputs, Map insertion order independent - Dark mode: different lightness ranges - Stress: 30 identical runs, realistic 12-run scenario Co-authored-by: Samuel <samuel@knutsen.co>
1 parent ed775f5 commit bb61e76

3 files changed

Lines changed: 639 additions & 17 deletions

File tree

tensorbored/webapp/util/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ tf_ts_library(
110110
"matcher_test.ts",
111111
"memoize_test.ts",
112112
"ngrx_test.ts",
113+
"oklch_colors_test.ts",
113114
"string_test.ts",
114115
"ui_selectors_test.ts",
115116
"value_formatter_test.ts",
@@ -121,6 +122,7 @@ tf_ts_library(
121122
":matcher",
122123
":memoize",
123124
":ngrx",
125+
":oklch_colors",
124126
":string",
125127
":ui_selectors",
126128
":value_formatter",

tensorbored/webapp/util/oklch_colors.ts

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -332,32 +332,45 @@ export function computeDeconfliction(
332332
const deconflictions = new Map<string, string>();
333333
const assignedColors: string[] = [];
334334
const assignedLabs: Lab[] = [];
335+
const placed = new Set<string>();
335336

336-
// Phase 1: Collect effective colors for all cached runs (preserving order).
337+
function placeColor(hex: string) {
338+
assignedColors.push(hex);
339+
assignedLabs.push(hexToOklab(hex));
340+
}
341+
342+
// Phase 0: Place all user-overridden colors as immovable landmarks.
343+
// These go first regardless of sort position so that every hash-based
344+
// run is always checked against every user-chosen color.
345+
for (const runId of sortedRunIds) {
346+
if (!userOverriddenRuns.has(runId)) continue;
347+
placeColor(runIdToBaseColor.get(runId)!);
348+
placed.add(runId);
349+
}
350+
351+
// Phase 1: Place cached non-user runs (preserving their deconflictions).
337352
for (const runId of sortedRunIds) {
338-
if (!cachedRunIds.has(runId)) continue;
353+
if (placed.has(runId) || !cachedRunIds.has(runId)) continue;
339354

340-
const baseColor = runIdToBaseColor.get(runId)!;
341355
const cached = cachedDeconflictions.get(runId);
342-
const effective = cached ?? baseColor;
356+
const effective = cached ?? runIdToBaseColor.get(runId)!;
343357

344358
if (cached) {
345359
deconflictions.set(runId, cached);
346360
}
347-
assignedColors.push(effective);
348-
assignedLabs.push(hexToOklab(effective));
361+
placeColor(effective);
362+
placed.add(runId);
349363
}
350364

351-
// Phase 2: Process new runs against all cached + previously-deconflicted colors.
365+
// Phase 2: Process remaining (new, non-user) runs.
352366
for (const runId of sortedRunIds) {
353-
if (cachedRunIds.has(runId)) continue;
367+
if (placed.has(runId)) continue;
354368

355369
const baseColor = runIdToBaseColor.get(runId)!;
356370
const baseLab = hexToOklab(baseColor);
357371

358-
if (assignedLabs.length === 0 || userOverriddenRuns.has(runId)) {
359-
assignedColors.push(baseColor);
360-
assignedLabs.push(baseLab);
372+
if (assignedLabs.length === 0) {
373+
placeColor(baseColor);
361374
continue;
362375
}
363376

@@ -368,17 +381,14 @@ export function computeDeconfliction(
368381
}
369382

370383
if (minDist >= MIN_DELTA_E) {
371-
assignedColors.push(baseColor);
372-
assignedLabs.push(baseLab);
384+
placeColor(baseColor);
373385
} else {
374386
const replacement = findDistantColor(assignedColors, darkMode);
375387
if (replacement) {
376388
deconflictions.set(runId, replacement);
377-
assignedColors.push(replacement);
378-
assignedLabs.push(hexToOklab(replacement));
389+
placeColor(replacement);
379390
} else {
380-
assignedColors.push(baseColor);
381-
assignedLabs.push(baseLab);
391+
placeColor(baseColor);
382392
}
383393
}
384394
}

0 commit comments

Comments
 (0)