cleanup: clear stale TODOs, tighten types, translate art-prompt hints

Every 'being written in parallel' / 'ASSUMED SIGNATURE' TODO is gone: each referenced module now exists and each assumption was checked against it rather than the comment simply deleted. - logo/presets/retouch: renderer typed as the real Renderer contract instead of unknown / (...args: never[]). - director: the ModelRegistry assumption is VERIFIED against the installed @earendil-works types (find() on ModelRegistry, getModel() on ModelRuntime). - strings: the img2imgBug TODO had it backwards -- drawthings raises a graded message a static string cannot express, and this is the fallback. - poster: an Italian hint was being spliced into an English art prompt, which degrades these models. New director.refineArtPrompt() folds the note in via the LLM and falls back to the old splice on any failure, so it can only improve on the previous behaviour. scrubArtPrompt still runs either way. Remaining TODOs are one category only (Italian strings living in per-file tables rather than ui/strings.ts) and are accurate, not stale.
2026-08-27 10:30:54 +02:00
parent 68a0cad747
commit aee92d284c
9 changed files with 156 additions and 54 deletions
@@ -56,6 +56,7 @@ import {
 import type { DesignSpec } from "../design/spec.ts";
 import { createJob, openInFinder, saveSpec, slugify, type Job } from "../job.ts";
 import { S, bullets, duration, errorText, fill, menu, type ErrorMessage } from "../ui/strings.ts";
+import type { Renderer } from "./poster.ts";

 // ---------------------------------------------------------------------------
 // Italian, local — see the TODO
@@ -220,9 +221,10 @@ export interface LogoDeps {
   * The Typst renderer (render/typst.ts). A logo carries no typeset text, so /logo never
   * calls it; it is in `Deps` only so index.ts can build ONE dependency object shared by
   * every command.
-   * TODO: type as `typeof import("../render/typst.ts")` once that module lands.
+   * Typed against the same `Renderer` contract index.ts builds and every other command
+   * receives, so a signature change is caught here rather than at runtime.
   */
-  renderer?: unknown;
+  renderer?: Renderer;
  /** Defaults to design/director.ts. */
  director?: DirectorPort;
  /** Defaults to backends/cpu.ts. */
@@ -93,13 +93,12 @@ import { previewImage } from "./retouch.ts";
 /**
 * What the Typst stage is asked for.
 *
- * TODO: render/typst.ts is being written in parallel. This is the surface this command
- * needs; index.ts owns the adapter from `resolveSpec()` / `render()` /
- * `renderAllFormats()` to these two methods. Assumptions worth naming:
+ * `render/typst.ts` satisfies this contract directly. What it guarantees, and what this
+ * command therefore relies on:
 *   - the renderer resolves the spec itself (contrast, geometry, font paths);
 *   - it writes into `outDir` under the names in job.ts's OUTPUT_BASENAMES;
- *   - it converts absolute paths to the root-relative form Typst requires
- *     (see docs/OPEN-DEFECTS.md §2).
+ *   - it converts absolute paths to the root-relative form Typst requires — a leading
+ *     "/" resolves against --root, NOT the filesystem (see docs/OPEN-DEFECTS.md §2).
 */
 export interface RenderRequest {
  spec: DesignSpec;
@@ -137,6 +136,8 @@ export interface Director {
  direct(brief: Brief, cfg: ImgenConfig, ctx: DirectorContext): Promise<DirectResult>;
  parseFreeText(text: string, ctx: DirectorContext, cfg?: ImgenConfig): Promise<ParsedBrief>;
  generateCaption(spec: DesignSpec, brief: Brief, ctx: DirectorContext, cfg?: ImgenConfig): Promise<CaptionResult>;
+  /** Folds an Italian note into the English art prompt. Falls back to a splice on failure. */
+  refineArtPrompt?(prompt: string, hintItalian: string, ctx: DirectorContext, cfg?: ImgenConfig): Promise<string>;
  /** Strips any request for lettering out of an art prompt. Pure. */
  scrubArtPrompt(prompt: string, artHint: string | undefined, warnings: DirectorWarning[]): string;
 }
@@ -147,8 +148,9 @@ export interface PosterDeps {
  renderer: Renderer;
  director: Director;
  /**
-   * Optional PDF check. TODO: render/preflight.ts exports `checkPdf()`; index.ts wires
-   * it in. When absent the PDF is simply not inspected — the files are still produced.
+   * Optional PDF check. index.ts wires this to `render/preflight.ts`'s `checkPdf()`,
+   * flattening its report to the failing/warning messages. When absent the PDF is simply
+   * not inspected — the files are still produced.
   */
  preflight?: (pdfPath: string) => Promise<{ ok: boolean; problems: string[] }>;
  /** Injected for tests. Defaults to job.ts's macOS-only Finder reveal. */
@@ -463,12 +465,28 @@ async function typesetDraft(
 // here is gated by a confirmation dialog and costs minutes, not milliseconds.
 // ═══════════════════════════════════════════════════════════════════════════

-/** A new artwork: fresh seed, plus his note folded into the prompt with lettering scrubbed out. */
-export function withNewArtwork(deps: PosterDeps, spec: DesignSpec, hint?: string): DesignSpec {
-  const merged = hint?.trim() ? `${spec.art.prompt}, ${hint.trim()}` : spec.art.prompt;
-  // TODO: the hint arrives in Italian and is spliced into an English prompt. scrubArtPrompt
-  // guarantees it cannot ask for lettering; translating it properly is a director job.
-  const prompt = deps.director.scrubArtPrompt(merged, undefined, []);
+/**
+ * A new artwork: fresh seed, plus his note folded into the prompt with lettering scrubbed.
+ *
+ * His note arrives in Italian and the models are trained on English, so the note goes
+ * through the director rather than being spliced in verbatim. `refineArtPrompt` falls back
+ * to the plain splice on any failure, and `scrubArtPrompt` guarantees — either way — that
+ * the result cannot ask for lettering.
+ */
+export async function withNewArtwork(
+  deps: PosterDeps,
+  spec: DesignSpec,
+  hint?: string,
+  ctx?: DirectorContext,
+): Promise<DesignSpec> {
+  const note = hint?.trim() ?? "";
+  let prompt: string;
+  if (note && ctx && deps.director.refineArtPrompt) {
+    prompt = await deps.director.refineArtPrompt(spec.art.prompt, note, ctx, deps.config);
+  } else {
+    const merged = note ? `${spec.art.prompt}, ${note}` : spec.art.prompt;
+    prompt = deps.director.scrubArtPrompt(merged, undefined, []);
+  }
  return { ...spec, art: { ...spec.art, prompt, seed: seedOf(deps) } };
 }

@@ -1441,7 +1459,7 @@ async function posterCommand(
      const beforeArt = spec;
      let repainted = false;
      try {
-        spec = withNewArtwork(deps, spec, hint);
+        spec = await withNewArtwork(deps, spec, hint, { ...ctx, signal: ctx.signal });
        await persistSpec(job, spec);
        status(S.progress.generating);
        draftArt = await paintArt(deps, job, spec, "draft", {
@@ -48,6 +48,7 @@ import type { Backend } from "../backends/types.ts";
 import type { DirectorContext, ParsedBrief } from "../design/director.ts";
 import { foldAccents, normaliseSlug } from "../job.ts";
 import { S, bullets, errorText, fill, list, menu } from "../ui/strings.ts";
+import type { Renderer } from "./poster.ts";

 // ---------------------------------------------------------------------------
 // Dependencies — injected so index.ts owns construction and tests own the rest
@@ -73,11 +74,10 @@ export interface PresetsDeps {
  /** Unused here — /presets must work before Draw Things is installed. Injected for symmetry. */
  backend?: Backend;
  /**
-   * render/typst.ts, injected for symmetry with the other commands. A preset has no
-   * page to typeset, so this is deliberately untyped rather than guessing a signature.
-   * TODO: typst.ts is being written in parallel; narrow this once it lands.
+   * Injected for symmetry with the other commands. A preset has no page to typeset, so
+   * this is never called — it is carried so index.ts can build ONE dependency object.
   */
-  renderer?: unknown;
+  renderer?: Renderer;
  /** Used only to pre-fill the form from free text. Optional: a local fallback exists. */
  director?: DirectorLike;
  /** Called after the config file changed, so index.ts can re-derive anything cached. */
@@ -47,11 +47,12 @@ import { ART_GENERATION_MAX_PX, FORMATS_GEOMETRY, roundToMultiple } from "../ren
 import type { Brief, DirectResult, DirectorContext } from "../design/director.ts";
 import { createJob, openInFinder, slugify } from "../job.ts";
 import { S, bullets, errorText, fill, menu } from "../ui/strings.ts";
+import type { Renderer } from "./poster.ts";

 // ---------------------------------------------------------------------------
 // Italian strings local to /retouch
 //
-// TODO: strings.ts has no `S.retouch` section yet (it only carries
+// TODO: move to `S.retouch` in ui/strings.ts (which currently only carries
 // `S.commands.retouch`). These live here until it grows one; every user-visible
 // string in this file comes from here or from `S`.
 // ---------------------------------------------------------------------------
@@ -119,9 +120,9 @@ export interface RetouchCpuOps {
 }

 /**
- * TODO — ASSUMED SIGNATURE. render/contrast.ts is being written in parallel; this is the
- * shape `/retouch` codes against: `smartCrop(input, out, {width,height}, opts?)` writing a
- * saliency-aware crop and returning the real pixel size it produced.
+ * The crop slice of `render/contrast.ts`. Matches its exported `smartCrop`, which writes
+ * a saliency-aware crop (sharp's attention strategy) and returns the size read back from
+ * the written file rather than the size that was requested.
 */
 export interface RetouchCropper {
  smartCrop(
@@ -138,13 +139,11 @@ export interface RetouchDirector {
 }

 /**
- * TODO — ASSUMED SIGNATURE for render/typst.ts. `/retouch` never typesets anything itself;
- * the renderer is carried here only so `startPoster` can be built with it in index.ts and
- * so the whole extension is wired from one place.
+ * `/retouch` never typesets anything itself; the renderer is carried here only so
+ * `startPoster` can be built with it in index.ts and the extension is wired from one
+ * place. Aliased to the real contract so a signature change cannot drift unnoticed.
 */
-export interface RetouchRenderer {
-  render(...args: never[]): Promise<unknown>;
-}
+export type RetouchRenderer = Renderer;

 /** Hand-off into the poster flow with an existing photo as the artwork. */
 export type PosterHandoff = (
@@ -847,8 +846,8 @@ async function handleRetouch(
          ctx,
        );
      } else {
-        // TODO: poster.ts is being written in parallel — when Deps carries no hand-off we
-        // type the command for him and let its free-text parser pick the path back up.
+        // No hand-off callback in Deps: type the command for him and let /poster's
+        // free-text parser pick the path back up. Same destination, one extra hop.
        say(pi, fill(R.handoffManual, { file: basename(result.handoff.photoPath) }));
        pi.sendUserMessage(
          `/poster foto: ${result.handoff.photoPath}${result.handoff.freeText ? ` ${result.handoff.freeText}` : ""}`,
@@ -134,9 +134,9 @@ async function defaultFormatsFor(
 * Is `art.png` already big enough to print at this format? Never throws: an artwork we
 * cannot measure is treated as not print-ready, so the worst case is the old behaviour.
 *
- * TODO: assumes `render/contrast.ts` exports `imageSize(path): Promise<PixelSize | null>`
- * (render/typst.ts imports it from there). Imported lazily on purpose — a static import
- * would drag `sharp` into extension start-up, where nothing needs it.
+ * Uses `render/contrast.ts`'s `imageSize(path): Promise<PixelSize | null>`. Imported
+ * lazily on purpose — a static import would drag `sharp` into extension start-up, where
+ * nothing needs it.
 */
 async function artIsPrintReady(
  format: Format,
@@ -191,10 +191,10 @@ export interface SocialRenderOutcome {
 /**
 * The slice of the renderer this command needs.
 *
- * TODO: assumed to be satisfied by `render/typst.ts`'s `renderAllFormats()`, which owns
- * `resolveSpec()`, the per-aspect fit and `contrast.smartCrop()`. index.ts adapts the
- * real signature onto this interface, so a mismatch is fixed in one place. The adapter
- * must honour `req.pdf`, or a print format in `formats` yields a PNG and no PDF.
+ * Satisfied by `render/typst.ts`'s `renderAllFormats()`, which owns `resolveSpec()`, the
+ * per-aspect fit and `contrast.smartCrop()`. index.ts adapts it onto this interface, so a
+ * mismatch is fixed in one place. The adapter must honour `req.pdf`, or a print format in
+ * `formats` yields a PNG and no PDF.
 */
 export interface SocialRenderer {
  renderAllFormats(req: SocialRenderRequest): Promise<SocialRenderOutcome>;
@@ -137,7 +137,8 @@ export interface Brief {
 * A repair the deterministic pass had to make. `code` is stable and machine-readable;
 * `italian` is ready to show.
 *
- * TODO: ui/strings.ts is being written in parallel and has no `S.director` section yet.
+ * TODO: move to `S.director` in ui/strings.ts. Kept together here so the move is a
+ * single cut-and-paste — nothing else in this file contains Italian.
 * When it grows one, delete WARNING_TEXT below and render from `code` + `params` there.
 */
 export interface DirectorWarning {
@@ -631,8 +632,9 @@ function resolveDirectorModel(cfg: ImgenConfig, ctx: DirectorContext): Model<Api
  const wantModel = cfg.directorModel?.trim();

  if (wantProvider) {
-    // TODO: assumes ModelRegistry exposes one of getModel(provider,id) / find(provider,id)
-    // and getModels(provider), as documented for pi's compatibility facade.
+    // VERIFIED against @earendil-works/pi-coding-agent: ModelRegistry declares
+    // find(provider, modelId) and ModelRuntime declares getModel(providerId, modelId).
+    // Both are probed because which object arrives depends on how the host built ctx.
    const lookup = callable(reg, "getModel") ?? callable(reg, "find");
    if (wantModel && lookup) {
      const found = lookup(wantProvider, wantModel) as Model<Api> | undefined;
@@ -1533,14 +1535,75 @@ export async function generateCaption(
  };
 }

+/** Schema for the art-prompt refinement call. One field: the revised English prompt. */
+const ArtPromptSchema = Type.Object({
+  prompt: Type.String({
+    description:
+      "The revised image prompt, in ENGLISH. Describes artwork only. Never requests text, letters, words, numbers, signage or typography.",
+  }),
+});
+
+const ART_PROMPT_SYSTEM = [
+  "You revise prompts for a local image-generation model.",
+  "",
+  "You are given an existing ENGLISH prompt and a note written in ITALIAN by a non-technical user.",
+  "Fold the note's INTENT into the prompt and return the result.",
+  "",
+  "Rules, all mandatory:",
+  "- Output ENGLISH only. Never echo the Italian.",
+  "- Describe ARTWORK ONLY: subject, composition, colour, light, medium, mood.",
+  "- NEVER request text, letters, words, numbers, signage, posters, logos or typography.",
+  "  Any lettering the model draws is lettering the typesetter has to fight.",
+  "- Keep what still applies from the original prompt; do not restart from nothing.",
+  "- Keep it under about 60 words. Long prompts confuse small local models.",
+].join("\n");
+
+/**
+ * Fold an Italian note from the user into the English art prompt.
+ *
+ * Splicing the Italian in verbatim measurably degrades output — these models are trained
+ * on English — so the note goes through the model that already art-directs. Every failure
+ * path falls back to the naive splice, which is never worse than before: this is a quality
+ * improvement, never a reason for the command to fail.
+ */
+export async function refineArtPrompt(
+  prompt: string,
+  hintItalian: string,
+  ctx: DirectorContext,
+  cfg?: ImgenConfig,
+): Promise<string> {
+  const hint = hintItalian.trim();
+  const naive = () => scrubArtPrompt(hint ? `${prompt}, ${hint}` : prompt, undefined, []);
+  if (!hint) return scrubArtPrompt(prompt, undefined, []);
+
+  try {
+    const model = resolveDirectorModel(cfg ?? ({ presets: {} } as unknown as ImgenConfig), ctx);
+    const { value } = await askStructured(
+      ctx,
+      model,
+      ART_PROMPT_SYSTEM,
+      `Existing prompt (English):\n${prompt}\n\nHis note (Italian):\n${hint}\n\nReturn the revised English prompt.`,
+      ArtPromptSchema,
+      "emit_art_prompt",
+      "Emit the revised English art prompt.",
+      (v) => v as Static<typeof ArtPromptSchema>,
+      { maxTokens: 220, temperature: 0.6 },
+    );
+    // The model is not trusted on the lettering rule: scrub regardless of what it returns.
+    const cleaned = scrubArtPrompt(value.prompt, undefined, []);
+    return cleaned.trim() ? cleaned : naive();
+  } catch {
+    return naive();
+  }
+}
+
 // ---------------------------------------------------------------------------
 // Warning text
 // ---------------------------------------------------------------------------

 /**
- * TODO: these belong in `S.director` in ui/strings.ts, which is being written in
- * parallel and has no such section yet. They are kept here, together, so moving them is
- * a single cut-and-paste — nothing else in this file contains Italian.
+ * TODO: move to `S.director` in ui/strings.ts. Kept here, together, so moving them is a
+ * single cut-and-paste — nothing else in this file contains Italian.
 */
 const WARNING_TEXT: Record<DirectorWarningCode, (p: Record<string, string>) => string> = {
  "font-display-replaced": (p) => `Il carattere «${p.carattere}» non andava bene per i titoli: ne ho scelto un altro.`,
@@ -140,10 +140,19 @@ export interface Region {
 * These mirror the layouts in templates/*.typ. `framed` and `split` are deliberately
 * absent: neither sets type over the picture (split says so in its own source), so there
 * is nothing to measure and no scrim to draw.
+ *
+ * A region must cover the type that lands ON THE ARTWORK, not every line the template
+ * sets. `banded` is the trap: its middle strip is where the eye goes, but that strip is
+ * two OPAQUE blocks (`band-block` filled with `pal.accent`, `strip-block` with `pal.ink`)
+ * painted over the picture, so there is nothing there to measure — the ink is chosen by
+ * the palette, not by the pixels. The only type that sits on bare artwork is the `lower`
+ * zone (details, price, footer, plus any unknown role), placed bottom-anchored at
+ * `place(bottom + left, dy: -sa.y, ...)`, and the scrim that protects it is bottom-anchored
+ * too. So `banded` measures the FOOT of the page, mirroring `hero-bottom`'s bottom band.
 */
 export const TEXT_REGIONS: Partial<Record<TemplateName, Region>> = {
  "hero-bottom": { left: 0, top: 0.55, width: 1, height: 0.45 },
-  banded: { left: 0, top: 0.34, width: 1, height: 0.32 },
+  banded: { left: 0, top: 0.72, width: 1, height: 0.28 },
  "centred-stack": { left: 0.06, top: 0.22, width: 0.88, height: 0.56 },
 };

@@ -524,7 +524,9 @@ export const S = {
    /**
     * Draw Things issue #121: partire da un'immagine esistente (img2img / edit) è
     * inaffidabile su 16GB e il comando ritaglia da solo al centro.
-     * TODO: drawthings.ts should raise this via BackendError(..., errorText(S.errors.img2imgBug)).
+     * This is the FALLBACK wording. drawthings.ts raises its own graded message — it can
+     * distinguish a confirmed crash from a suspected one, which a static string cannot —
+     * and retouch.ts falls back to this only when a BackendError carries no Italian.
     */
    img2imgBug: {
      message: "Partire da un'immagine che hai già, su questo computer, non funziona in modo affidabile: è un difetto noto del programma che disegna.",
@@ -650,8 +652,8 @@ export const S = {
  },

  /**
-   * Health check. TODO: doctor.ts is being written in parallel — assumed to report one
-   * line per check using these labels plus `ok` / `ko` markers.
+   * Health check. doctor.ts reports one line per check using these labels plus the
+   * `ok` / `ko` markers below.
   */
  doctor: {
    title: "Controllo generale",
@@ -360,16 +360,25 @@
 #let scrim(height, colour, angle, width: 100%, strength: 100%) = {
  let c = if colour == none { black } else { hex(colour, fallback: black) }
  let a = if angle == none { 90deg } else { angle }
-  let solid = if strength >= 100% { c } else { c.transparentize(100% - strength) }
+  // `strength` scales EVERY stop, not just the terminal one. Scaling only the last stop
+  // leaves the intermediate stops at their full-strength alpha, so below ~70% strength
+  // the 75% stop ends up denser than the "solid" end and the wash peaks a quarter of the
+  // way in from the anchored edge instead of at it — two dark bands bracketing the type
+  // instead of one clean ramp. `t` is the stop's share of full density; the shape of the
+  // ramp (0 / 22 / 70 / 100) is unchanged, only its scale.
+  let stop(t) = {
+    let alpha = strength * t
+    if alpha >= 100% { c } else { c.transparentize(100% - alpha) }
+  }
  rect(
    width: width,
    height: height,
    stroke: none,
    fill: gradient.linear(
-      (c.transparentize(100%), 0%),
-      (c.transparentize(78%), 45%),
-      (c.transparentize(30%), 75%),
-      (solid, 100%),
+      (stop(0.0), 0%),
+      (stop(0.22), 45%),
+      (stop(0.70), 75%),
+      (stop(1.0), 100%),
      angle: a,
    ),
  )