Refactor: cooklang timer unit table, `images-health-check` redundant API calls, shared time-unit normalisation

[Robbie-Palmer]

Context

Fresh audit pass cross-checked against the open refactor backlog (#434, #437,
#445, #471, #479, #485, #491, #497, #504, #506, #508, #509, #510, #511, #514,
#526, #534, #538, #540). Each finding below was hand-verified on the current
branch and dropped if it overlaps an existing issue.

Boundary cases checked:

• loadDomainRepository memoization is covered by Refactoring opportunities: DRY, type-safety, performance, dead code #497 §1 — not re-raised.
• stripMarkdown regex duplication in adrQueries.ts vs mdx.ts is
  covered by Refactoring opportunities: DRY, type safety, error handling, and maintainability #514 §5 — not re-raised.
• ADRRefSchema vs hand-rolled regex at repository.ts:498 is covered by
  Refactoring opportunities: DRY, type safety, error handling, and maintainability #514 §6 — not re-raised.
• validateReferentialIntegrity decomposition is covered by Refactor follow-up: split validateReferentialIntegrity, tighten types in sigma/postHog, kill deprecated date helper #437 §1 — not
  re-raised.
• Domain slug schema duplication in models.ts is covered by Refactor: delete the stale domain/models.ts schema duplicate (and the tests that pin it) #485 — not
  re-raised.
• requireBySlug / getOrThrow API helper is covered by Refactor: search-nav twin, ADR/Tech pagination duplication, API throw helper, technology-resolution wrapper #511 §3 — not
  re-raised.
• compareByDateDesc comparator extraction is covered by Refactor: repository.ts helpers, Fuse threshold constant, date-sort comparator, asset-tracker snapshot dedup #540 §6 and
  Refactoring opportunities: DRY, type-safety, performance, dead code #497 §4 — not re-raised.
• Per-edge tech resolvers (e.g. getTechnologyBadgeViewsForBlog) is
  covered by Refactor: search-nav twin, ADR/Tech pagination duplication, API throw helper, technology-resolution wrapper #511 §4 — not re-raised.
• parseDateString silent-NaN gap in experience.ts is covered by Refactor: command-palette URL hook reuse, sigma memo cleanups, mermaid theme hoist, experience date NaN gap #538 §5
  — not re-raised.
• formatTime recipe duplication is covered by Refactor: dedupe formatTime, extract graph colour maps + recipe formatters, plug use-scaled-recipe test gap #526 §1 — not re-raised
  (§3 below is about a different recipe-time helper).
• NodeId discriminator type safety was a candidate, but
  repository/graph/types.ts:11-16 already defines NodeId as a template
  literal union and isNodeType (:69-74) is a typed predicate that narrows
  correctly. No action.
• JsonLdScript data: object type widening is covered by Refactoring opportunities: DRY, type safety, error handling, and maintainability #514 §7. The
  separate U+2028 / U+2029 escape concern that sometimes accompanies this
  pattern is not relevant here — the content is served as
  type="application/ld+json" (not executable JS), so the existing </>/&
  escape covers the only XSS surface (the </script> sequence). No action.

Each item below is independently shippable. Severity in the header.

---

1. timerDurationSeconds is a 22-line switch where a lookup table is shorter, exhaustive, and reusable [medium]

Category: DRY / Maintainability / Extensibility

File: ui/lib/domain/recipe/cooklangTransform.ts:158-184

function timerDurationSeconds(timer: Timer): number | null {
  const qty = resolveQuantityValue(timer.quantity);
  if (qty === undefined) return null;
  const unit = getQuantityUnit(timer.quantity)?.toLowerCase();
  switch (unit) {
    case "s":
    case "sec":
    case "secs":
    case "second":
    case "seconds":
      return qty;
    case "m":
    case "min":
    case "mins":
    case "minute":
    case "minutes":
      return qty * 60;
    case "h":
    case "hr":
    case "hrs":
    case "hour":
    case "hours":
      return qty * 3600;
    default:
      return null;
  }
}

Three problems with this shape:

1. Three forms of the same data structure. The codebase already uses
   lookup tables for the same kind of "string token → typed value" mapping
   for ingredient units in
   packages/recipe-domain/src/unit.ts:158-189
   (NORMALIZED_UNIT_ALIASES, normalizeUnitToken) and for measurement
   conversion in packages/recipe-domain/src/conversion.ts:20-40
   (CONVERSIONS). The timer path is the odd one out — same problem,
   different idiom.
2. Adding a unit means editing two switches in the same function. The
   case list and the multiplier branch are coupled by position. A typo
   like adding "hrs" under the minutes branch by mistake is not caught
   by the compiler.
3. The aliases aren't reusable. formatTimerDisplay immediately above
   (:149-156) takes the raw unit string from getQuantityUnit and
   renders it verbatim, so an author writing ~{10%min} displays as
   "10 min" but ~{10%minutes} displays as "10 minutes". The two
   functions use different canonicalisation policies on the same input.

Why prior issues don't cover this: #445 covers the cooklang helpers
shared with ml-pipelines (resolveQuantityValue, isIngredientOnlyStep);
#526 §1 covers the formatTime(minutes) helper duplicated across the
recipe components. Neither touches timerDurationSeconds or
formatTimerDisplay in cooklangTransform.ts.

Fix: introduce a single time-unit table (alongside the existing
UNIT_LABELS pattern), and have both functions consume it.

// near the top of cooklangTransform.ts, or in a new
// ui/lib/domain/recipe/timeUnits.ts that mirrors recipe-domain/unit.ts.

type TimeUnit = "second" | "minute" | "hour";

const TIME_UNIT_SECONDS: Record<TimeUnit, number> = {
  second: 1,
  minute: 60,
  hour: 3600,
};

const TIME_UNIT_ALIASES: Record<string, TimeUnit> = {
  s: "second", sec: "second", secs: "second",
  second: "second", seconds: "second",
  m: "minute", min: "minute", mins: "minute",
  minute: "minute", minutes: "minute",
  h: "hour", hr: "hour", hrs: "hour",
  hour: "hour", hours: "hour",
};

function normalizeTimeUnit(token: string | undefined): TimeUnit | undefined {
  if (!token) return undefined;
  return TIME_UNIT_ALIASES[token.trim().toLowerCase()];
}

function timerDurationSeconds(timer: Timer): number | null {
  const qty = resolveQuantityValue(timer.quantity);
  if (qty === undefined) return null;
  const unit = normalizeTimeUnit(getQuantityUnit(timer.quantity));
  return unit ? qty * TIME_UNIT_SECONDS[unit] : null;
}

Three downstream wins:

• formatTimerDisplay (:149-156) can route the unit through
  normalizeTimeUnit and render a consistent canonical form, so
  ~{10%min} and ~{10%minutes} render the same way.
• TimeUnit becomes a discriminated union — adding "day" is one entry
  in TIME_UNIT_SECONDS, a small batch of aliases, and the compiler
  ensures every consumer handles it.
• Direct unit tests on normalizeTimeUnit are now possible without
  going through a Timer fixture (see §3).

Pairs naturally with #526 §1 if both land together — the new
timeUnits.ts module would be the home for the timer-side helpers, and
formatTime(minutes) would be a sibling in timeFormatting.ts.

---

2. images-health-check.ts calls listImages() three times for the same data [low-medium]

Category: Efficiency / Robustness

File: ui/scripts/images-health-check.ts:12, 44, 86

The health-check script runs three sequential steps — API connectivity,
variant check, URL generation — and each step calls listImages()
independently:

// :10-15  (Step 1)
const images = await listImages();
console.log(`   📊 Total images in account: ${images.length}`);

// :43-46  (Step 2)
const images = await listImages();
if (images.length > 0 && images[0]?.variants) { ... }

// :85-88  (Step 3)
const images = await listImages();
if (images.length > 0 && images[0]?.id) { ... }

Three problems:

1. Three round-trips to the Cloudflare Images API for the same data.
   The Cloudflare SDK's client.images.v1.list() walks every image in
   the account (see ui/scripts/lib/cloudflare.ts:9-15, the same call
   the sync script already deduplicates by caching the result locally).
   Each call hits the network and contributes to the API rate limit.
2. The three steps are presented as independent diagnostics ("1️⃣
   Testing API connectivity… 2️⃣ Checking image variants… 3️⃣ Testing
   URL generation…") but two of them silently re-test step 1's
   precondition. If the API is flaky, step 2 or 3 can fail while step 1
   succeeded — the report is misleading because it implies independent
   passes.
3. Each step has its own try/catch so a step-2 failure prints a
   redundant variant-specific error after step-1 succeeded — even
   though the underlying failure is the same single network operation.

Why prior issues don't cover this: #510 §3 / §4 cover the
images-sync.ts efficiency issues (duplicate parseCalVerImageFilename
runs, O(N×M) version lookup). The sibling health-check script is not
mentioned. #506 / #514 don't touch ui/scripts/ at all.

Fix: hoist the fetch out of the steps and reuse its result:

async function main() {
  console.log("Running Cloudflare Images health check...\n");

  console.log("1️⃣  Testing API connectivity...");
  let images: Awaited<ReturnType<typeof listImages>>;
  try {
    images = await listImages();
    console.log("   ✅ API connection successful");
    console.log(`   📊 Total images in account: ${images.length}`);
  } catch (error) {
    console.log("   ❌ API connection failed");
    console.log(`   Error: ${error instanceof Error ? error.message : "Unknown error"}`);
    return; // can't run the rest without the listing
  }
  // ...print featured/embedded breakdown unchanged...

  console.log("\n2️⃣  Checking image variants...");
  if (images.length > 0 && images[0]?.variants) { /* unchanged */ }

  console.log("\n3️⃣  Testing image URL generation...");
  if (images.length > 0 && images[0]?.id) { /* unchanged */ }
}

One API call instead of three; failure of the network call is reported
in step 1 and the downstream steps don't try to re-test it.

---

3. timerDurationSeconds has no direct test coverage [low]

Category: Testing gaps

File: ui/tests/lib/domain/recipe/ (gap)

timerDurationSeconds ships into the SDK at
cooklangTransform.ts:396 (timerDurationSeconds: timerDurations) and
the rendered timer countdown in instructionTokens.ts:139 depends on
it. The function has 15 string cases collapsing into 3 numeric branches,
plus a "null on unknown unit" fallback.

The only existing test that touches it is
ui/tests/lib/domain/recipe/instructionTokens.test.ts:16 which hard-codes
a single timerDurationSeconds: [600] value into a fixture. The aliases
("hrs", "min", "seconds", …) are never exercised, and the
"unknown unit returns null" path has no coverage.

Cousin of #491 §6 (which lists missing direct unit tests for
blog/project/adr/role/tech domain queries) and #526 §4 (which adds
use-scaled-recipe-hook tests). Same gap class — different module —
and the fix gets much smaller if §1 above lands first because
normalizeTimeUnit becomes the natural unit-of-test instead of the
whole Timer-fixture path.

Fix: add ui/tests/lib/domain/recipe/timeUnits.test.ts (or whatever
module §1 lands in) with three tiny cases:

1. normalizeTimeUnit returns the canonical form for every alias in
   TIME_UNIT_ALIASES (loop the table).
2. normalizeTimeUnit returns undefined for an unknown string and for
   undefined input.
3. timerDurationSeconds returns qty * multiplier for each unit
   family and null for unknown units / undefined qty.

The first case becomes a one-line snapshot test against the table; the
overall suite is well under 30 lines.

---

Suggested order of attack

1. §2 — images-health-check.ts cleanup is self-contained and one
   file; ~15 LOC delta.
2. §1 — timerDurationSeconds table extraction. Two functions
   touched, no behaviour change other than formatTimerDisplay gaining
   consistent canonical forms (intentional improvement).
3. §3 — tests for the new table; lands naturally on top of §1.

Happy to split any of these into separate PRs.

_{Audit performed via Claude Code.}