fix(web): design-mode requests stop naming levers that do nothing

.fork/customizations.yaml

@@ -1715,9 +1715,60 @@
     in favor of their own tsconfig island; the engine is bundled to an
     injectable IIFE string by the fork:design-mode-engine Vite plugin as
     virtual:fork-design-mode-engine.
+
+    Two accuracy rules govern what a change request is allowed to assert, both
+    added after a request cost more time than doing the edit by hand, and both
+    since corrected for how the underlying APIs actually behave.
+
+    First, a css bullet may only say "change `px-2.5` -> `px-1`" once
+    cssOrigin.ts has PROVED that class is the lever, by removing it and
+    re-measuring while the element shows its original cascade. Removal alone
+    cannot decide a TIE — if something else declares the same value, removing
+    the class moves nothing — so a tie gets a second probe: the utility's own
+    declared value is applied inline and re-measured, which resolves
+    `calc(var(--spacing) * N)` in the element's context. A declared value that
+    computes to something OTHER than the measured one means the utility
+    provably lost (the motivating `px-2.5`-is-10px-but-measured-8px case is
+    decided, not hedged); only a genuine same-value tie is reported as
+    ambiguous and names BOTH. Culprit naming excludes exactly ONE selector —
+    the probed utility's own single-class rule, a tautology — and nothing
+    broader: a plain-CSS project's `.composer-chip` is the likeliest culprit
+    there even though the element carries the class, and a competing utility on
+    the element is a finding. When the class provably is not the lever the
+    bullet names the winning rule and its file (Vite's data-vite-dev-id is what
+    still carries the authored filename). Ranking is layer-aware: unlayered
+    beats layered above
+    specificity, because Tailwind v4 utilities are in `@layer utilities` and an
+    unlayered fork rule of trivial specificity beats them. Two CSSOM traps are
+    load-bearing here: since CSS Nesting a CSSStyleRule HAS a (truthy, empty)
+    `cssRules`, so style rules must be handled before any grouping check or
+    every declaration is skipped; and `matchMedia` never throws, so only
+    CSSMediaRule may be condition-gated — `@supports`/`@container` must be
+    descended into unconditionally.
+
+    Second, a source location is only forwarded when react-grab reports the
+    reporting frame as `isSymbolicated === true`. Not `!== false`: react-grab
+    0.1.44 / bippy 0.5.41 return a frame UNTOUCHED when symbolication fails, so
+    there is no `false` to test for and `!== false` passed every failure. The
+    frame is paired by position first, since the context's filePath is
+    normalized while the frame's fileName is a raw served URL. Rejection costs
+    the LINE, not the file: `data-t3-source-file` keeps the authored path and
+    `data-t3-component` the fiber's component name, rendered as "Rendered by
+    `<X>` in path (line not resolvable)" and only when no location resolved.
+    Hint results are TTL-evicted from the preload cache like nulls — they are
+    not successes, and caching them would pin a transient failure for the
+    element's lifetime.
+
+    Relatedly, the NO_PREVIEW guardrail must not promise upstream's automatic
+    verification while this fork has not vendored client/verifier.ts; a guard
+    test pins that wording.
   tier: 4
   files:
     - apps/web/src/custom/designMode/protocol.ts
+    # Outside engine/ on purpose: like protocol.ts it crosses the TS-island fence
+    # (the engine tsconfig includes it by path) so the web project can type-check
+    # and unit-test it, which files under engine/ cannot be.
+    - apps/web/src/custom/designMode/cssOrigin.ts
     - apps/web/src/custom/designMode/designModeStore.ts
     - apps/web/src/custom/designMode/designModeBridge.ts
     - apps/web/src/custom/designMode/designChangeDraftStore.ts
@@ -1819,3 +1870,4 @@
     - apps/web/package.json
   verify:
     - apps/web/src/__fork_guards__/forkDesignMode.test.ts
+    - apps/web/src/__fork_guards__/forkDesignModeCssOrigin.test.ts

apps/desktop/src/preview/DesignSourceResolver.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, it } from "vite-plus/test";
 
-import { normalizeResolvedSource } from "./DesignSourceResult.ts";
+import { describeResolvedSource, normalizeResolvedSource } from "./DesignSourceResult.ts";
 
 const VALID = {
   filePath: "/src/components/Button.tsx",
@@ -21,13 +21,27 @@ describe("normalizeResolvedSource", () => {
     expect(normalizeResolvedSource({ ...VALID, columnNumber: 0 })?.column).toBe(1);
   });
 
-  it("carries only the source location — extra react-grab context never crosses", () => {
+  it("carries the source location and component name — nothing else crosses", () => {
     const result = normalizeResolvedSource({
       ...VALID,
       componentName: "SubmitButton",
       selector: "button.primary",
+      snippet: "<button/>",
     } as typeof VALID);
-    expect(result).toEqual({ file: VALID.filePath, line: 12, column: 4 });
+    expect(result).toEqual({
+      file: VALID.filePath,
+      line: 12,
+      column: 4,
+      componentName: "SubmitButton",
+    });
+  });
+
+  it("drops a component name that is not identifier-shaped", () => {
+    expect(normalizeResolvedSource({ ...VALID, componentName: "a b\nc" } as typeof VALID)).toEqual({
+      file: VALID.filePath,
+      line: 12,
+      column: 4,
+    });
   });
 
   it("rejects contexts without a usable source location", () => {
@@ -49,4 +63,96 @@ describe("normalizeResolvedSource", () => {
     expect(normalizeResolvedSource({ ...VALID, filePath: "/src/\tApp.tsx" })).toBeNull();
     expect(normalizeResolvedSource({ ...VALID, filePath: "/src/App.tsx\u007f" })).toBeNull();
   });
+
+  // The failure this guards: a 71-line ComposerControl.tsx reported "at line 135" — a real
+  // position in the SERVED module, which reads as authoritative and sends the agent hunting.
+  //
+  // The shape below is what react-grab 0.1.44 ACTUALLY emits on failure: the frame comes back
+  // untouched, with no `isSymbolicated` key at all. There is no `isSymbolicated: false` in the
+  // dist, which is why the gate tests `=== true`. Asserting on a hand-written `false` would
+  // pass against a shape the library never produces.
+  it("rejects a frame that carries no symbolication flag", () => {
+    const stack = [{ fileName: VALID.filePath, lineNumber: 12, columnNumber: 4 }];
+    expect(normalizeResolvedSource({ ...VALID, stack } as typeof VALID)).toBeNull();
+  });
+
+  it("accepts only an explicitly symbolicated frame", () => {
+    const stack = [
+      { fileName: VALID.filePath, lineNumber: 12, columnNumber: 4, isSymbolicated: true },
+    ];
+    expect(normalizeResolvedSource({ ...VALID, stack } as typeof VALID)?.line).toBe(12);
+  });
+
+  it("pairs the frame by position, not by raw fileName", () => {
+    // react-grab reports a served URL on the frame and a normalized path on the context, so a
+    // raw string compare misses and the verdict used to fall to frames[0].
+    const stack = [
+      { fileName: "http://localhost:5173/src/other.tsx", lineNumber: 99, columnNumber: 1 },
+      {
+        fileName: "http://localhost:5173/src/components/Button.tsx?t=1",
+        lineNumber: 12,
+        columnNumber: 4,
+        isSymbolicated: true,
+      },
+    ];
+    expect(normalizeResolvedSource({ ...VALID, stack } as typeof VALID)?.line).toBe(12);
+  });
+
+  it("pairs by comparable path when position is ambiguous", () => {
+    const stack = [
+      { fileName: "http://localhost:5173/src/other.tsx", lineNumber: 12, columnNumber: 4 },
+      {
+        fileName: "http://localhost:5173/src/components/Button.tsx?t=1",
+        lineNumber: 12,
+        columnNumber: 4,
+        isSymbolicated: true,
+      },
+    ];
+    expect(normalizeResolvedSource({ ...VALID, stack } as typeof VALID)?.line).toBe(12);
+  });
+
+  it("fails closed when the reporting frame cannot be identified", () => {
+    const stack = [{ fileName: "/src/unrelated.tsx", lineNumber: 500, columnNumber: 9 }];
+    expect(normalizeResolvedSource({ ...VALID, stack } as typeof VALID)).toBeNull();
+  });
+
+  it("still trusts a bare location with no stack at all", () => {
+    expect(normalizeResolvedSource({ ...VALID, stack: [] } as typeof VALID)?.line).toBe(12);
+    expect(normalizeResolvedSource(VALID)?.line).toBe(12);
+  });
+});
+
+describe("describeResolvedSource", () => {
+  it("returns the full location when there is one", () => {
+    expect(describeResolvedSource({ ...VALID, componentName: "Btn" } as typeof VALID)).toEqual({
+      file: VALID.filePath,
+      line: 12,
+      column: 4,
+      componentName: "Btn",
+    });
+  });
+
+  // A rejected location loses the LINE, not the file: react-grab reads the path off the
+  // fiber's module, and only the position inside it needed symbolication.
+  it("keeps the component name and the file when the line is rejected", () => {
+    const stack = [{ fileName: VALID.filePath, lineNumber: 12, columnNumber: 4 }];
+    expect(
+      describeResolvedSource({ ...VALID, stack, componentName: "Btn" } as typeof VALID),
+    ).toEqual({ componentName: "Btn", file: VALID.filePath });
+  });
+
+  it("never emits a line or column alongside a rejected location", () => {
+    const stack = [{ fileName: VALID.filePath, lineNumber: 12, columnNumber: 4 }];
+    const result = describeResolvedSource({ ...VALID, stack } as typeof VALID);
+    expect(result).toEqual({ file: VALID.filePath });
+    expect(result).not.toHaveProperty("line");
+    expect(result).not.toHaveProperty("column");
+  });
+
+  it("returns null when nothing usable survives", () => {
+    expect(describeResolvedSource(null)).toBeNull();
+    expect(describeResolvedSource({})).toBeNull();
+    const stack = [{ fileName: "/a.tsx" }];
+    expect(describeResolvedSource({ filePath: "/src/App.tsx\u0000", stack } as never)).toBeNull();
+  });
 });

apps/desktop/src/preview/DesignSourceResolver.ts

@@ -17,8 +17,8 @@ import { getElementContext } from "react-grab/primitives";
 
 import {
   DESIGN_SOURCE_RESOLVER_GLOBAL,
-  normalizeResolvedSource,
-  type DesignSourceResult,
+  describeResolvedSource,
+  type ResolvedDesignSource,
 } from "./DesignSourceResult.ts";
 
 /** react-grab resolution symbolicates through source maps — cap concurrent work so a
@@ -48,19 +48,23 @@ function releaseSlot(): void {
 
 /** Settled AND in-flight results share one promise per element — concurrent callers
  * (hover prefetch racing a click promotion) never trigger duplicate react-grab work.
- * Successes cache for the element's lifetime; null results expire after a short TTL so
- * an element resolved before React mounted its dev metadata (hydration, a lazy chunk)
- * can succeed on a later ask instead of staying selector-only forever (PR #54 review).
- * The TTL also bounds retry cost: at most one react-grab attempt per element per TTL. */
-const resolutionCache = new WeakMap<Element, Promise<DesignSourceResult | null>>();
+ *
+ * Three tiers, not two. Only a FULL location caches for the element's lifetime. Both a null
+ * result and a hint (`{file?, componentName?}` with no position) expire after a short TTL, so
+ * an element asked before React mounted its dev metadata — hydration, a lazy chunk, a source
+ * map still in flight — can succeed on a later ask instead of being pinned at "(line not
+ * resolvable)" forever. Caching hints as successes would have quietly cancelled the retry
+ * behaviour PR #54 added, since a hint is non-null. The TTL also bounds retry cost: at most
+ * one react-grab attempt per element per TTL. */
+const resolutionCache = new WeakMap<Element, Promise<ResolvedDesignSource | null>>();
 
-const NULL_RESULT_TTL_MS = 5000;
+const RETRYABLE_RESULT_TTL_MS = 5000;
 
-async function resolveElement(element: Element): Promise<DesignSourceResult | null> {
+async function resolveElement(element: Element): Promise<ResolvedDesignSource | null> {
   await acquireSlot();
   try {
     const context = await getElementContext(element);
-    const normalized = normalizeResolvedSource(context);
+    const normalized = describeResolvedSource(context);
     // Recheck after the await — an element replaced mid-resolution must not hand the
     // engine a location for a node that no longer exists.
     if (!normalized || !element.isConnected || element.ownerDocument !== document) return null;
@@ -72,7 +76,7 @@ async function resolveElement(element: Element): Promise<DesignSourceResult | nu
   }
 }
 
-function resolve(element: unknown): Promise<DesignSourceResult | null> {
+function resolve(element: unknown): Promise<ResolvedDesignSource | null> {
   if (!(element instanceof Element) || element.ownerDocument !== document || !element.isConnected) {
     return Promise.resolve(null);
   }
@@ -81,10 +85,11 @@ function resolve(element: unknown): Promise<DesignSourceResult | null> {
   const resolution = resolveElement(element);
   resolutionCache.set(element, resolution);
   void resolution.then((result) => {
-    if (result !== null) return;
+    // A hint is not a success — `line` is what distinguishes the two arms of the union.
+    if (result !== null && "line" in result && result.line !== undefined) return;
     window.setTimeout(() => {
       if (resolutionCache.get(element) === resolution) resolutionCache.delete(element);
-    }, NULL_RESULT_TTL_MS);
+    }, RETRYABLE_RESULT_TTL_MS);
   });
   return resolution;
 }