fix(review): Create title no silent fail; gate enables content LLM

Create title used to look enabled while review echoed a pre-typed title when species/primary were missing from taxonomy (no error). Persist wood cats before the deterministic build, return title_built / title_build_missing from review, and surface a Create-title nudge like Create desc. WooSetupGate now turns on content_llm_enabled when inference URL+key are set (Call 2 / Create desc). Call 1 stays out. Create-desc gate also requires inference URL+key so empty Settings show a clear missing list.
2026-09-26 22:50:15 -07:00
parent f54cc37dcc
commit fca32396c0
10 changed files with 187 additions and 19 deletions
@@ -184,12 +184,23 @@ async def review_slab(slab_id: str, payload: dict[str, Any] | None = None) -> di
    # they are produced by the deterministic build below, not persisted from the
    # payload (so the review POST can't clobber the owner's Create workflow).
    cats, terms = _taxonomy_lookup(conn)
-    primary_id = payload.get("primary_wood_category_id")
+
+    def _as_cat_id(v):
+        if v is None or v == "":
+            return None
+        try:
+            return int(v)
+        except (TypeError, ValueError):
+            return None
+
+    primary_id = _as_cat_id(payload.get("primary_wood_category_id"))
    wood_cats = slab.get("wood_category_ids") or []
    if not primary_id:
-        primary_id = wood_cats[0] if wood_cats else None
-    primary_leaf = cats.get(primary_id, {}) if primary_id else {}
-    species_name = cats.get(slab.get("species_id"), {}).get("name", "") if slab.get("species_id") else ""
+        primary_id = _as_cat_id(wood_cats[0]) if wood_cats else None
+    # Taxonomy rows use integer PKs; coerce so string JSON ids still resolve.
+    primary_leaf = cats.get(primary_id, {}) if primary_id is not None else {}
+    species_id = _as_cat_id(slab.get("species_id"))
+    species_name = cats.get(species_id, {}).get("name", "") if species_id is not None else ""
    figure_term_ids = slab.get("figure_term_ids") or []
    figure = terms.get(figure_term_ids[0], {}).get("name") if figure_term_ids else None

@@ -210,14 +221,22 @@ async def review_slab(slab_id: str, payload: dict[str, Any] | None = None) -> di
    # Deterministic title / short_description, re-derived from the current
    # metadata on every review (the Create button's source of truth).
    # #98: pass synced terms so Live Edge comes from the edge term, not any id.
+    # title_built tells the client whether the Create-title path actually ran
+    # (vs echoing a pre-typed title when species/primary are missing from taxonomy).
+    title_build_missing: list[str] = []
+    if not species_name:
+        title_build_missing.append("species")
+    if not primary_leaf:
+        title_build_missing.append("primary_wood_category")
+    title_built = not title_build_missing
    title = (
        build_title(slab, figure, species_name, primary_leaf, terms)
-        if (primary_leaf and species_name)
+        if title_built
        else slab.get("title") or None
    )
    short_desc = (
        build_short_description(slab, figure, species_name, primary_leaf, terms)
-        if (primary_leaf and species_name)
+        if title_built
        else slab.get("short_description") or None
    )

@@ -227,6 +246,8 @@ async def review_slab(slab_id: str, payload: dict[str, Any] | None = None) -> di
        "ready": ready, "missing": missing, "title": title, "short_description": short_desc,
        "price": price, "price_suggestion": price_suggestion(conn, slab),
        "primary_wood_category_id": primary_id,
+        "title_built": title_built,
+        "title_build_missing": title_build_missing,
    }


@@ -62,6 +62,8 @@ def test_title_live_edge_no_dims(env):
    body = r.json()
    assert body["title"] == "Figured Black Walnut Live Edge Slab"
    assert body["short_description"].startswith("Figured Black Walnut Live Edge Slab, 9ft 6in x 24.3in x 1.75in")
+    assert body["title_built"] is True
+    assert body["title_build_missing"] == []
    assert "18.4 bdft" in body["short_description"]


@@ -220,3 +222,25 @@ def test_ready_requires_figure_term_ids(env):
    })
    # May still miss other fields depending on fixture; figure must not be listed
    assert "figure_term_ids" not in missing2
+
+
+def test_review_title_built_false_without_species_or_primary(env):
+    """Create-title silent-fail fix: report title_built=false when taxonomy cannot resolve."""
+    client, conn = env
+    conn.execute(
+        "INSERT INTO slabs(id,sku,length_in,thickness_in,sqft,bdft,width_min_in,width_max_in,width_avg_in,"
+        "species_id,wood_category_ids,figure_term_ids,price,price_source,title,short_description,description,"
+        "content_source,status,server_rev,client_rev,created_at,updated_at) "
+        "VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)",
+        ("s1", None, 114.0, 1.75, 30.0, 18.4, 18.0, 24.3, 24.3, None, "[]", "[]", None, None,
+         "Typed Title", None, None, "manual", "calibrated", 1, 1, "t", "t"),
+    )
+    conn.commit()
+    r = client.post("/api/v1/slabs/s1/review", json={})
+    assert r.status_code == 200, r.text
+    body = r.json()
+    assert body["title_built"] is False
+    assert "species" in body["title_build_missing"]
+    assert "primary_wood_category" in body["title_build_missing"]
+    assert body["title"] == "Typed Title"
+    assert body["short_description"] is None
@@ -306,6 +306,9 @@ export interface ReviewResult {
  price: number | null;
  price_suggestion: number | null;
  primary_wood_category_id: number | null;
+  /** True when deterministic title/short_desc were built from taxonomy. */
+  title_built?: boolean;
+  title_build_missing?: string[];
 }

 export interface ReviewInput {
@@ -7,7 +7,7 @@
    type Call2Prompt,
  } from '../../api/slabs';
  import type { CalibratedCapture } from '../flow';
-  import { canCreateTitleFromSpecies, effectiveSpeciesId } from './speciesFromCapture';
+  import { effectiveSpeciesId } from './speciesFromCapture';
  import MultiSelect from './MultiSelect.svelte';
  import { decodeName } from '../decodeName';
  import { deriveFigTagIds, scopedFeatTags } from './figTags';
@@ -17,6 +17,7 @@
  } from './lineup';
  import { nextPrimary } from './woodPrimary';
  import { missingCreateDescLabels } from './createDescGate';
+  import { missingCreateTitleLabels, titleBuildMissingLabels } from './createTitleGate';
  import { orderWoodCatGroups } from './woodCatOrder';
  import { applyMoistureDefault } from './moistureDefault';

@@ -130,6 +131,7 @@
    ready: boolean; missing: string[]; title: string | null;
    short_description: string | null; price: number | null;
    price_suggestion: number | null; primary_wood_category_id: number | null;
+    title_built?: boolean; title_build_missing?: string[];
  }
  let review: ReviewOut | null = null;
  let reviewBusy = false;
@@ -142,11 +144,17 @@
  let createError = '';
  // Prefer slab.species_id; fall back to Capture handoff if createSlab omitted it.
  $: resolvedSpeciesId = effectiveSpeciesId(slab?.species_id, capture?.speciesId ?? null);
-  $: canCreateTitle = canCreateTitleFromSpecies(resolvedSpeciesId, woodCatIds.length);
+  $: createTitleMissing = missingCreateTitleLabels({
+    speciesId: resolvedSpeciesId,
+    woodCategoryCount: woodCatIds.length,
+  });
+  $: canCreateTitle = createTitleMissing.length === 0;
  // Create-desc gate: recompute on every field change (no blur required).
  // Short/description are Call 2 outputs; price is publish-only.
+  // Needs content_llm_enabled + inference URL/key (Call 2); Call 1 stays out.
  $: createDescMissing = missingCreateDescLabels({
    contentLlmEnabled: canGenerateContent,
+    inferenceConfigured,
    title,
    speciesId: resolvedSpeciesId,
    woodCategoryIds: woodCatIds,
@@ -185,6 +193,12 @@
  let genBusy = false;
  let genError = '';
  let canGenerateContent = false;
+  /** Inference URL + API key both configured in Settings (Call 2 creds). */
+  let inferenceConfigured = false;
+  const secretConfigured = (s: { configured: boolean } | string | null | undefined): boolean => {
+    if (s == null) return false;
+    return typeof s === 'string' ? s.length > 0 : !!s.configured;
+  };
  const fmt1 = (v: number) => { const s = v.toFixed(1); return s.endsWith('.0') ? s.slice(0, -2) : s; };
  const fmt2 = (v: number) => v.toFixed(2);

@@ -487,22 +501,35 @@
  // auto-fills only empty fields; this is the explicit "force overwrite" action.
  async function createTitle(): Promise<void> {
    if (!slab || createBusy) return;
+    if (!canCreateTitle) {
+      createError = `Create title needs: ${createTitleMissing.join(', ')}`;
+      return;
+    }
    createBusy = true;
    createError = '';
    try {
-      // 0. Repair: if Measure chose a species but createSlab omitted it, write it
-      // before the deterministic build (review reads species_id from the row).
-      if (resolvedSpeciesId != null && slab.species_id == null) {
-        slab = await updateSlab(slabId, {
-          species_id: resolvedSpeciesId,
-          client_rev: slab.server_rev,
-        });
-      }
+      // 0. Persist species + wood categories before the deterministic build.
+      // Review reads the row / taxonomy; UI-only wood cats used to make Create
+      // title look enabled while the server echoed a pre-typed title (silent no-op).
+      const prePatch: Parameters<typeof updateSlab>[1] = {
+        wood_category_ids: woodCatIds,
+        client_rev: slab.server_rev,
+        ...(resolvedSpeciesId != null ? { species_id: resolvedSpeciesId } : {}),
+      };
+      slab = await updateSlab(slabId, prePatch);
      // 1. Run the deterministic review to get title / short_description.
      const res = await reviewSlab(slabId, {
        primary_wood_category_id: primaryCatId === '' ? undefined : Number(primaryCatId),
      });
      if (res.primary_wood_category_id != null) primaryCatId = res.primary_wood_category_id;
+      // 1b. Surface build failure instead of silently keeping a typed title.
+      if (res.title_built === false) {
+        const why = titleBuildMissingLabels(res.title_build_missing);
+        createError = why.length
+          ? `Create title needs: ${why.join(', ')}. Sync taxonomy via Settings → Test Woo if leaves look empty.`
+          : 'Could not build title from species + primary wood category. Sync taxonomy in Settings.';
+        return;
+      }
      // 2. Write the deterministic values into the bound fields — always overwrite.
      if (res.title) title = res.title;
      if (res.short_description) shortDescription = res.short_description;
@@ -663,6 +690,8 @@
      deriveOptions();
      hydrate(s, t);
      canGenerateContent = !!set.content_llm_enabled;
+      inferenceConfigured =
+        secretConfigured(set.inference_base_url) && secretConfigured(set.inference_api_key);
      // #90: sync the <select> DOM to the hydrated values (same race as AI prefill).
      syncSelects();
      await loadPhotos();
@@ -872,6 +901,11 @@
          {createBusy ? 'Building…' : 'Create short desc'}
        </button>
      </div>
+      {#if !canCreateTitle}
+        <p class="nudge" data-testid="create-title-missing">
+          Create title needs: {createTitleMissing.join(', ')}
+        </p>
+      {/if}
    </div>
    <div class="field">
      <label class="lbl" for="desc">Description</label>
@@ -8,6 +8,7 @@ import {

 const full = {
  contentLlmEnabled: true,
+  inferenceConfigured: true,
  title: 'Figured Walnut Live Edge Slab',
  speciesId: 11,
  woodCategoryIds: [21],
@@ -24,7 +25,6 @@ test('Create desc gate: full set passes; does not require short or description',
 });

 test('Create desc gate: empty price still passes when other fields full', () => {
-  // Price is publish-only; Create desc must not gate on it.
  assert.equal(canCreateDesc(full), true);
  assert.deepEqual(missingCreateDesc(full), []);
 });
@@ -33,6 +33,7 @@ test('Create desc gate: lists each missing field; no short/description/price key
  const miss = missingCreateDesc({
    ...full,
    contentLlmEnabled: false,
+    inferenceConfigured: false,
    title: '',
    speciesId: null,
    woodCategoryIds: [],
@@ -44,6 +45,7 @@ test('Create desc gate: lists each missing field; no short/description/price key
  });
  assert.deepEqual(miss, [
    'content_llm',
+    'inference_creds',
    'title',
    'species',
    'wood_categories',
@@ -64,6 +66,13 @@ test('Create desc gate: lists each missing field; no short/description/price key
  assert.equal(labels.includes('price'), false);
 });

+test('Create desc gate: inference URL+key required even when content LLM on', () => {
+  assert.equal(canCreateDesc({ ...full, inferenceConfigured: false }), false);
+  assert.deepEqual(missingCreateDesc({ ...full, inferenceConfigured: false }), [
+    'inference_creds',
+  ]);
+});
+
 test('Create desc gate: recomputes on field change (reactive-ready)', () => {
  assert.equal(canCreateDesc({ ...full, figureTermIds: [] }), false);
  assert.equal(canCreateDesc({ ...full, figureTermIds: [1] }), true);
@@ -2,10 +2,15 @@
 * Create-desc gate (Call 2): disable Create desc unless these are present.
 * Does NOT require short_description, description, or price — Call 2 writes
 * short + long; price stays publish-only.
+ *
+ * content_llm_enabled is the Settings flag; inference URL+key must also be
+ * configured (gate / Settings). Call 1 stays out — this gate is Call 2 only.
 */

 export type CreateDescInput = {
  contentLlmEnabled: boolean;
+  /** True when inference_base_url and inference_api_key are both configured. */
+  inferenceConfigured: boolean;
  title: string;
  speciesId: number | null | undefined;
  woodCategoryIds: number[];
@@ -19,6 +24,7 @@ export type CreateDescInput = {
 /** Human labels shown under Create desc when the gate fails. */
 export const CREATE_DESC_LABELS: Record<string, string> = {
  content_llm: 'content LLM on',
+  inference_creds: 'inference URL + API key',
  title: 'title',
  species: 'species',
  wood_categories: '≥1 wood category',
@@ -39,6 +45,7 @@ function presentId(v: number | string | '' | null | undefined): boolean {
 export function missingCreateDesc(input: CreateDescInput): string[] {
  const missing: string[] = [];
  if (!input.contentLlmEnabled) missing.push('content_llm');
+  if (!input.inferenceConfigured) missing.push('inference_creds');
  if (!String(input.title ?? '').trim()) missing.push('title');
  if (input.speciesId == null || !Number.isFinite(Number(input.speciesId))) missing.push('species');
  if (!input.woodCategoryIds?.length) missing.push('wood_categories');
@@ -0,0 +1,23 @@
+import assert from 'node:assert/strict';
+import { test } from 'node:test';
+import {
+  missingCreateTitle,
+  missingCreateTitleLabels,
+  titleBuildMissingLabels,
+} from './createTitleGate';
+
+test('Create title gate: species + wood category', () => {
+  assert.deepEqual(missingCreateTitle({ speciesId: 11, woodCategoryCount: 1 }), []);
+  assert.deepEqual(missingCreateTitle({ speciesId: null, woodCategoryCount: 1 }), ['species']);
+  assert.deepEqual(missingCreateTitle({ speciesId: 11, woodCategoryCount: 0 }), [
+    'wood_categories',
+  ]);
+  assert.ok(missingCreateTitleLabels({ speciesId: null, woodCategoryCount: 0 }).includes('species (from Measure)'));
+});
+
+test('titleBuildMissingLabels maps server keys', () => {
+  assert.deepEqual(titleBuildMissingLabels(['species', 'primary_wood_category']), [
+    'species in synced taxonomy',
+    'primary wood category in synced taxonomy',
+  ]);
+});
@@ -0,0 +1,38 @@
+/**
+ * Create-title / Create-short-desc gate: deterministic template build (#89).
+ * Needs species (Measure) + ≥1 wood category. Does NOT need the content LLM —
+ * that is Call 2 / Create desc only.
+ */
+
+export type CreateTitleInput = {
+  speciesId: number | null | undefined;
+  woodCategoryCount: number;
+};
+
+export const CREATE_TITLE_LABELS: Record<string, string> = {
+  species: 'species (from Measure)',
+  wood_categories: '≥1 wood category',
+};
+
+/** Keys for review.title_build_missing when taxonomy cannot resolve leaf names. */
+export const TITLE_BUILD_LABELS: Record<string, string> = {
+  species: 'species in synced taxonomy',
+  primary_wood_category: 'primary wood category in synced taxonomy',
+};
+
+export function missingCreateTitle(input: CreateTitleInput): string[] {
+  const missing: string[] = [];
+  if (input.speciesId == null || !Number.isFinite(Number(input.speciesId))) {
+    missing.push('species');
+  }
+  if (!(input.woodCategoryCount > 0)) missing.push('wood_categories');
+  return missing;
+}
+
+export function missingCreateTitleLabels(input: CreateTitleInput): string[] {
+  return missingCreateTitle(input).map((k) => CREATE_TITLE_LABELS[k] ?? k);
+}
+
+export function titleBuildMissingLabels(keys: string[] | null | undefined): string[] {
+  return (keys ?? []).map((k) => TITLE_BUILD_LABELS[k] ?? k);
+}
@@ -38,6 +38,8 @@ test('Review UI: View prompt + Create desc gate wiring', () => {
  assert.match(reviewSrc, /getCall2Prompt/);
  assert.match(reviewSrc, /createDescMissing/);
  assert.match(reviewSrc, /missingCreateDescLabels/);
+  assert.match(reviewSrc, /create-title-missing/);
+  assert.match(reviewSrc, /title_built/);
  assert.match(reviewSrc, /orderWoodCatGroups/);
  assert.match(reviewSrc, /applyMoistureDefault/);
  assert.match(reviewSrc, /onCommit=\{commitReview\}/);
@@ -74,9 +74,16 @@
    statusOk = false;
    try {
      // Inference only — never write device Woo into Settings.
-      const inferPatch: Record<string, string> = {};
+      // When URL+key are (or become) configured, turn on content_llm_enabled so
+      // Create desc / Call 2 works. Call 1 stays out (do not set inference_enabled).
+      const inferPatch: Record<string, string | boolean> = {};
      if (inferenceUrl.trim()) inferPatch.inference_base_url = inferenceUrl.trim();
      if (inferenceKey.trim()) inferPatch.inference_api_key = inferenceKey.trim();
+      const willHaveUrl = !!(inferenceUrl.trim() || inferenceUrlConfigured());
+      const willHaveKey = !!(inferenceKey.trim() || inferenceKeyConfigured());
+      if (willHaveUrl && willHaveKey) {
+        inferPatch.content_llm_enabled = true;
+      }
      if (Object.keys(inferPatch).length) {
        settings = await updateSettings(inferPatch);
        inferenceUrl = secretValue(settings.inference_base_url as Secret);
@@ -207,7 +214,7 @@
              data-testid="woo-gate-inference-key"
            />
          </div>
-          <p class="hint">Only filled values are saved to shared Settings. Device Woo is never written there.</p>
+          <p class="hint">Only filled values are saved to shared Settings. When both are set, Content LLM (Create desc / Call 2) turns on. Device Woo is never written there.</p>
        </div>

        <button