fix(review): thickness band derived from thickness_in (#88); MultiSelect shows selected names + commits on toggle settle (#90)

#88 — thickness band is now display-only, derived deterministically from
thickness_in against the synced Woo thickness terms (TV-9 / QUALITY-95).
Replaces the <select> with a read-only div. New pure function
deriveThicknessTermId + 6 unit tests. Docs (UAT-PRACTICE, contracts/UX)
updated to reflect the band is not an owner control.

#90 — MultiSelect closed state now shows the selected option names
(truncated to 2 + +N) instead of just a count. onCommit prop wired so
multi-select changes persist + re-review immediately (250ms debounce,
flush on Done, cleanup on destroy). Single-select binds use string vars
(speciesSel/edgeSel/gradeSel) to fix the number/string mismatch with
<option value={id}>; a tick()-deferred syncSelects() keeps the DOM
option in sync after programmatic writes (AI prefill, reload hydrate)
that Svelte's own select_option would otherwise reset.
This commit is contained in:
Ty
2026-09-21 09:56:44 -07:00
parent 502314b338
commit bdd03afce4
6 changed files with 244 additions and 32 deletions
+79 -25
View File
@@ -1,5 +1,5 @@
<script lang="ts">
import { onMount, onDestroy } from 'svelte';
import { onMount, onDestroy, tick } from 'svelte';
import {
getSlab, updateSlab, reviewSlab, getTaxonomy, getSettings, inferTaxon, inferContent,
ApiError,
@@ -7,6 +7,7 @@
} from '../../api/slabs';
import type { CalibratedCapture } from '../flow';
import MultiSelect from './MultiSelect.svelte';
import { deriveThicknessTermId } from './thicknessBand';
// Props handed off by App (one-column flow, UX.md §1).
export let slabId: string;
@@ -21,7 +22,7 @@
let loading = true;
// Picker option types (id + name + whether the AI recommended it).
interface Opt { id: number; name: string; ai: boolean }
interface Opt { id: number; name: string; ai: boolean; lowIn?: number }
const byId = <T extends { id: number }>(arr: T[]) => {
const m = new Map<number, T>();
for (const x of arr) m.set(x.id, x);
@@ -54,7 +55,13 @@
const attr = tax!.attributes.find(
(a) => a.name.toLowerCase() === name.toLowerCase(),
);
return (attr?.terms ?? []).map((t) => ({ id: t.id, name: t.name, ai: false }));
return (attr?.terms ?? []).map((t) => {
// Carry the band's numeric lower bound for thickness (TV-9 round-up); other
// attributes ignore it. "2-4 in" → 2; a name with no leading number → undefined.
const m = t.name.match(/\d+(?:[./]\d+)?/);
const lowIn = m ? Number(m[0]) : undefined;
return { id: t.id, name: t.name, ai: false, lowIn: Number.isFinite(lowIn) ? lowIn : undefined };
});
};
speciesOpts = childrenOf(speciesGrouper).map((c) => ({ id: c.id, name: c.name, ai: false }));
// Wood categories: parent/child grouped (METADATA-10 — parents group only, never
@@ -82,12 +89,33 @@
}
// --- Editable model (mirrors the live slab) ---
let speciesId: number | '' = '';
// #90: single-selects bind a STRING var to the native <select>, because
// <option value={id}> coerces the option value to a string. Binding a
// `number | ''` var directly to those string values is the classic
// number/string mismatch that leaves the selected option blank after AI
// prefill or reload. `Number(...)` at commit normalizes back.
let speciesSel: string = '';
let edgeSel: string = '';
let gradeSel: string = '';
let woodCatIds: number[] = [];
let edgeTypeId: number | '' = '';
let figureIds: number[] = [];
let gradeId: number | '' = '';
let thicknessId: number | '' = '';
// #88: thickness band is NOT an owner control — it is derived deterministically
// from thickness_in (TV-9 / QUALITY-95) against the synced Woo thickness terms.
// The Review screen shows it read-only; the owner changes thickness upstream, and
// the band follows.
$: thicknessId = deriveThicknessTermId(
slab?.thickness_in ?? null,
thicknessOpts.map((o) => ({ id: o.id, name: o.name })),
);
$: thicknessName = thicknessOpts.find((o) => o.id === thicknessId)?.name ?? '';
// #88 display text: the derived band name, or an explicit "no band" note when a
// thickness is set but nothing in the synced set encloses it.
$: thicknessDisplay =
thicknessId !== ''
? thicknessName
: (slab?.thickness_in ?? 0) > 0
? `— no band for ${fmt1(slab!.thickness_in!)}″`
: '—';
let moistureId: number | '' = '';
let figTagIds: number[] = [];
let featTagIds: number[] = [];
@@ -129,12 +157,13 @@
const fmt2 = (v: number) => v.toFixed(2);
function hydrate(s: Slab, t: Taxonomy | null) {
speciesId = s.species_id ?? '';
speciesSel = s.species_id != null ? String(s.species_id) : '';
woodCatIds = [...(s.wood_category_ids ?? [])];
edgeTypeId = s.edge_type_term_id ?? '';
edgeSel = s.edge_type_term_id != null ? String(s.edge_type_term_id) : '';
figureIds = [...(s.figure_term_ids ?? [])];
gradeId = s.grade_term_id ?? '';
thicknessId = s.thickness_term_id ?? '';
gradeSel = s.grade_term_id != null ? String(s.grade_term_id) : '';
// thicknessId is derived reactively from slab.thickness_in (#88) — not hydrated
// from the stored term, so a stale stored band can't shadow the derived value.
moistureId = s.moisture_term_id ?? '';
figTagIds = [...(s.fig_tag_ids ?? [])];
featTagIds = [...(s.feat_tag_ids ?? [])];
@@ -165,11 +194,11 @@
reviewError = '';
try {
const patch = {
species_id: speciesId === '' ? null : Number(speciesId),
species_id: speciesSel === '' ? null : Number(speciesSel),
wood_category_ids: woodCatIds,
edge_type_term_id: edgeTypeId === '' ? null : Number(edgeTypeId),
edge_type_term_id: edgeSel === '' ? null : Number(edgeSel),
figure_term_ids: figureIds,
grade_term_id: gradeId === '' ? null : Number(gradeId),
grade_term_id: gradeSel === '' ? null : Number(gradeSel),
thickness_term_id: thicknessId === '' ? null : Number(thicknessId),
moisture_term_id: moistureId === '' ? null : Number(moistureId),
fig_tag_ids: figTagIds,
@@ -218,13 +247,13 @@
const r = await inferTaxon(slabId);
const s = r.suggestions;
const recs: number[] = [];
if (s.species_id) { speciesId = s.species_id; recs.push(s.species_id); }
if (s.species_id) { speciesSel = String(s.species_id); recs.push(s.species_id); }
if (s.wood_category_ids) {
woodCatIds = [...s.wood_category_ids];
if (!primaryCatId) primaryCatId = woodCatIds[0];
recs.push(...s.wood_category_ids);
}
if (s.edge_type_term_id) edgeTypeId = s.edge_type_term_id;
if (s.edge_type_term_id) edgeSel = String(s.edge_type_term_id);
if (s.figure_term_ids) { figureIds = [...s.figure_term_ids]; recs.push(...s.figure_term_ids); }
if (s.feature_tag_ids?.length) {
featTagIds = [...s.feature_tag_ids];
@@ -234,6 +263,9 @@
aiRecommendations = recs;
aiNote = recs.length ? `AI suggested ${recs.length} field${recs.length === 1 ? '' : 's'} — review & edit below.` : 'No confident AI suggestions.';
await commitReview();
// #90: force the <select> DOM to reflect the prefilled values after Svelte's
// own select_option(reset) has run.
syncSelects();
} catch (e) {
aiNote = e instanceof ApiError ? e.message : 'Inference unavailable.';
} finally {
@@ -272,6 +304,22 @@
return arr.includes(id) ? arr.filter((x) => x !== id) : [...arr, id];
}
// #90: keep the native <select> DOM option in sync when the bound value is set
// programmatically (AI prefill, reload hydrate). Svelte 4's update() re-runs
// select_option(select, val) on every val change, which resets el.value to the
// bound value — so a $: sync in the same flush gets overwritten. We defer with
// tick() so the write lands AFTER Svelte's DOM update, guaranteeing the visible
// option matches the bound value. Idempotent: only writes when they differ.
async function syncSelects() {
await tick();
if (speciesEl && speciesEl.value !== speciesSel) speciesEl.value = speciesSel;
if (edgeEl && edgeEl.value !== edgeSel) edgeEl.value = edgeSel;
if (gradeEl && gradeEl.value !== gradeSel) gradeEl.value = gradeSel;
}
let speciesEl: HTMLSelectElement | undefined;
let edgeEl: HTMLSelectElement | undefined;
let gradeEl: HTMLSelectElement | undefined;
onMount(async () => {
try {
const [s, t, set] = await Promise.all([getSlab(slabId), getTaxonomy(), getSettings()]);
@@ -279,6 +327,8 @@
deriveOptions();
hydrate(s, t);
canGenerateContent = !!set.content_llm_enabled;
// #90: sync the <select> DOM to the hydrated values (same race as AI prefill).
syncSelects();
await commitReview();
} catch (e) {
loadError = e instanceof ApiError ? e.message : 'Could not load the slab.';
@@ -333,10 +383,10 @@
<!-- Species (single, AI-assist) -->
<div class="field">
<label class="lbl" for="species">Species <span class="ai">AI assist</span></label>
<select id="species" class="sel" data-testid="species" bind:value={speciesId} on:change={commitReview}>
<select id="species" class="sel" data-testid="species" bind:this={speciesEl} bind:value={speciesSel} on:change={commitReview}>
<option value="">— select —</option>
{#each speciesOpts as o (o.id)}
<option value={o.id} data-ai={o.ai ? 'true' : ''}>{o.name}{o.ai ? ' (AI)' : ''}</option>
<option value={o.id} data-ai={aiRecommendations.includes(o.id) ? 'true' : ''}>{o.name}{aiRecommendations.includes(o.id) ? ' (AI)' : ''}</option>
{/each}
</select>
</div>
@@ -349,12 +399,13 @@
placeholder="Choose wood categories"
testid="wood-cat"
onToggle={(id) => (woodCatIds = toggleMulti(woodCatIds, id))}
onCommit={commitReview}
/>
<!-- Edge type (single, AI-assist) -->
<div class="field">
<label class="lbl" for="edge">Edge type <span class="ai">AI assist</span></label>
<select id="edge" class="sel" data-testid="edge-type" bind:value={edgeTypeId} on:change={commitReview}>
<select id="edge" class="sel" data-testid="edge-type" bind:this={edgeEl} bind:value={edgeSel} on:change={commitReview}>
<option value="">— select —</option>
{#each edgeOpts as o (o.id)}
<option value={o.id}>{o.name}</option>
@@ -372,13 +423,14 @@
placeholder="Choose figure"
testid="figure"
onToggle={(id) => (figureIds = toggleMulti(figureIds, id))}
onCommit={commitReview}
/>
</div>
<!-- Grade (single, owner judgment — NOT AI-assisted per METADATA-40) -->
<div class="field">
<label class="lbl" for="grade">Grade</label>
<select id="grade" class="sel" data-testid="grade" bind:value={gradeId} on:change={commitReview}>
<select id="grade" class="sel" data-testid="grade" bind:this={gradeEl} bind:value={gradeSel} on:change={commitReview}>
<option value="">— select —</option>
{#each gradeOpts as o (o.id)}<option value={o.id}>{o.name}</option>{/each}
</select>
@@ -394,6 +446,7 @@
placeholder="Choose fig-* tags"
testid="fig-tag"
onToggle={(id) => (figTagIds = toggleMulti(figTagIds, id))}
onCommit={commitReview}
/>
</div>
@@ -407,15 +460,14 @@
placeholder="Choose feat-* tags"
testid="feat-tag"
onToggle={(id) => (featTagIds = toggleMulti(featTagIds, id))}
onCommit={commitReview}
/>
</div>
<!-- #88: thickness band is deterministic from thickness_in — display-only, no owner pick. -->
<div class="field">
<label class="lbl" for="thickness">Thickness band</label>
<select id="thickness" class="sel" data-testid="thickness-band" bind:value={thicknessId} on:change={commitReview}>
<option value="">— select —</option>
{#each thicknessOpts as o (o.id)}<option value={o.id}>{o.name}</option>{/each}
</select>
<label class="lbl" for="thickness">Thickness band <span class="sug">from thickness</span></label>
<div id="thickness" class="ro" data-testid="thickness-band">{thicknessDisplay}</div>
</div>
<div class="field">
<label class="lbl" for="moisture">Moisture</label>
@@ -504,6 +556,8 @@
.lbl { display: block; font-size: .8rem; color: #bbb; margin: 0 0 4px; }
.lbl .ai { margin-left: 6px; font-size: .58rem; font-weight: 700; padding: 1px 5px; border-radius: 2px; background: #00e5ff; color: #002; }
.inp, .sel { width: 100%; padding: 9px; background: #1d1d1d; border: 1px solid #444; border-radius: 3px; color: #eee; font-size: .9rem; text-align: left; }
/* #88 read-only thickness band: same chrome as .sel but non-interactive. */
.ro { width: 100%; padding: 9px; background: #141414; border: 1px solid #333; border-radius: 3px; color: #9ad0d8; font-size: .9rem; text-align: left; }
.sel { appearance: none; background-image: linear-gradient(45deg,transparent 50%,#777 50%),linear-gradient(135deg,#777 50%,transparent 50%); background-position: calc(100% - 16px) 55%,calc(100% - 11px) 55%; background-size: 5px 5px; background-repeat: no-repeat; }
.ta { resize: vertical; }
.field { margin-bottom: 12px; }