feat(asset): WP-020 스케치 에셋 매니페스트 시스템 구현 (#47) - #53

tests/location-recents-insertion.test.tsx (1)

64-69: Unify provider wrapping across all tests in this suite.

Only the first case uses SketchManifestProvider; the other LocationPage renders use a different harness. Consider extracting one shared render helper and reusing it for all three cases to avoid setup drift.

Suggested refactor

+function renderLocationPageWithProviders(
+  LocationPage: (props: { resolvedLocationId: string }) => JSX.Element
+) {
+  return render(
+    <SketchManifestProvider manifest={BASELINE_MANIFEST}>
+      <MemoryRouter>
+        <LocationPage resolvedLocationId="aaa000000001" />
+      </MemoryRouter>
+    </SketchManifestProvider>
+  );
+}
@@
-    render(
-      <SketchManifestProvider manifest={BASELINE_MANIFEST}>
-        <MemoryRouter>
-          <LocationPage resolvedLocationId="aaa000000001" />
-        </MemoryRouter>
-      </SketchManifestProvider>
-    );
+    renderLocationPageWithProviders(LocationPage);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/location-recents-insertion.test.tsx` around lines 64 - 69, The tests
diverge in provider/wrapper setup causing setup drift; unify them by extracting
a shared render helper that wraps LocationPage with SketchManifestProvider
(using BASELINE_MANIFEST) and MemoryRouter. Create a helper function (e.g.,
renderLocationPage) used across all three tests to render <LocationPage
resolvedLocationId="...">, replacing the inline wrappers in each test so all
cases use the same manifest/provider harness (refer to SketchManifestProvider,
MemoryRouter, LocationPage, and BASELINE_MANIFEST to locate the code). Ensure
the helper accepts the resolvedLocationId (and any optional overrides) so tests
remain flexible.

frontend/entities/asset/api/fetch-remote-manifest.ts (1)

7-19: Consider adding a timeout for resilience.

The fetch has no timeout configured. While the PR notes that the server endpoint doesn't exist yet (404 silent fallback), for production hardening consider adding an AbortController timeout to prevent indefinite hangs on slow/unresponsive servers.

♻️ Optional: Add timeout with AbortController

 export const fetchRemoteManifest: RemoteManifestFetcher = async () => {
+  const controller = new AbortController();
+  const timeoutId = setTimeout(() => controller.abort(), 5000);
+
-  const response = await fetch(REMOTE_MANIFEST_URL, {
-    headers: { Accept: 'application/json' },
-  });
+  try {
+    const response = await fetch(REMOTE_MANIFEST_URL, {
+      headers: { Accept: 'application/json' },
+      signal: controller.signal,
+    });
+    clearTimeout(timeoutId);
     if (!response.ok) {
       throw new Error(`remote manifest fetch failed: ${response.status}`);
     }
     // ... rest of validation
+  } catch (err) {
+    clearTimeout(timeoutId);
+    throw err;
+  }
 };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/entities/asset/api/fetch-remote-manifest.ts` around lines 7 - 19,
fetchRemoteManifest currently calls fetch(REMOTE_MANIFEST_URL) with no timeout;
add an AbortController and a short configurable timeout (e.g. via a constant) to
avoid hanging: create an AbortController, start a setTimeout that calls
controller.abort() after the timeout, pass controller.signal to fetch, and clear
the timeout after a successful response; ensure errors from aborted fetch are
handled or rethrown as a descriptive timeout Error so callers of
fetchRemoteManifest can distinguish timeout vs other failures.

frontend/pages/home/ui/home-dashboard.tsx (1)

117-166: LGTM! Optional: Extract duplicated weather summary content.

The SketchBackground integration is correct — it receives the required location and condition props and is properly layered behind the card content. The provider wiring at the app level (per app-providers.tsx) ensures the context is available.

The weather summary content (temperature, condition text, high/low) is duplicated between the Link (lines 130-143) and div (lines 150-163) branches. Consider extracting into a shared component for DRY, though this is optional given the small scope.

♻️ Optional: Extract shared weather summary

// Could extract to a shared component:
function WeatherSummaryContent({ weather }: { weather: CoreWeather }) {
  return (
    <>
      <p className="font-display text-7xl font-extrabold text-foreground">
        {Math.round(weather.current.temperatureC)}°
      </p>
      <p className="font-body text-base text-muted-foreground">
        {weather.current.condition.text}
      </p>
      <div className="flex gap-4">
        <span className="font-body text-sm text-muted-foreground">
          H {Math.round(weather.today.maxC)}°
        </span>
        <span className="font-body text-sm text-muted-foreground">
          L {Math.round(weather.today.minC)}°
        </span>
      </div>
    </>
  );
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/pages/home/ui/home-dashboard.tsx` around lines 117 - 166, The
weather summary JSX is duplicated between the Link and div branches; extract it
into a small shared component (e.g., WeatherSummaryContent) that accepts the
weather prop (signature like WeatherSummaryContent({ weather }: { weather:
CoreWeather }) ) and returns the temperature, condition text, and H/L spans,
then replace the duplicated blocks in home-dashboard.tsx by rendering
<WeatherSummaryContent weather={weather} /> inside both the Link and the div so
the UI and props (weather.current.temperatureC, weather.current.condition.text,
weather.today.maxC/minC) remain unchanged.

tests/asset/load-session-manifest.test.ts (1)

85-97: Consider using vi.waitFor for async failure handling.

The setTimeout(resolve, 10) on line 93 is a timing-based wait that could be flaky on slow CI runners. Consider using vi.waitFor with a condition or fake timers for more deterministic behavior.

♻️ Optional: Use vi.waitFor with flush approach

   it('remoteFetcher 실패 시 기존 pending을 건드리지 않는다', async () => {
     const existing = JSON.stringify({
       'hub/seoul/clear-day': 'https://cdn.example.com/keep.webp',
     });
     window.localStorage.setItem(PENDING_MANIFEST_STORAGE_KEY, existing);
     const remoteFetcher = vi.fn().mockRejectedValue(new Error('network'));
     loadSessionManifest({ remoteFetcher });
-    // 실패가 비동기적으로 처리될 시간을 준다.
-    await new Promise((resolve) => setTimeout(resolve, 10));
+    // 실패가 비동기적으로 처리될 때까지 대기.
+    await vi.waitFor(() => {
+      expect(remoteFetcher).toHaveBeenCalled();
+    });
+    // 마이크로태스크 flush 후에도 pending이 유지되는지 확인.
+    await Promise.resolve();
     expect(window.localStorage.getItem(PENDING_MANIFEST_STORAGE_KEY)).toBe(
       existing
     );
   });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/asset/load-session-manifest.test.ts` around lines 85 - 97, Replace the
timing-based sleep with a deterministic wait: remove the setTimeout-based pause
and use vi.waitFor to assert the expected state after loadSessionManifest
completes. Specifically, call loadSessionManifest({ remoteFetcher }) and then
vi.waitFor(() =>
expect(window.localStorage.getItem(PENDING_MANIFEST_STORAGE_KEY)).toBe(existing))
(or waitFor(() => expect(remoteFetcher).toHaveBeenCalled()) first) so the test
waits for the async failure to be observed deterministically; reference
PENDING_MANIFEST_STORAGE_KEY, loadSessionManifest, and remoteFetcher when making
the change.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

frontend/entities/asset/model/manifest.ts (1)

14-18: ⚠️ Potential issue | 🟡 Minor

Trim validated URLs before storing them.

Line 14 and Line 36 validate url.trim().length, but Line 17 and Line 37 still persist the untrimmed value. This can leave leading/trailing whitespace in manifest URLs.

Suggested patch

 function freezeManifest(raw: Record<string, string>): SketchManifest {
   const entries: Array<[SemanticKey, string]> = [];
   for (const [key, url] of Object.entries(raw)) {
     if (!isSemanticKey(key)) {
       throw new Error(`baseline manifest has invalid semantic key: ${key}`);
     }
     if (typeof url !== 'string' || url.trim().length === 0) {
       throw new Error(`baseline manifest has invalid url for key: ${key}`);
     }
-    entries.push([key, url]);
+    entries.push([key, url.trim()]);
   }
   return Object.freeze(Object.fromEntries(entries)) as SketchManifest;
 }
@@
   const merged: Record<string, string> = { ...baseline };
   for (const [key, url] of Object.entries(override)) {
     if (typeof url === 'string' && url.trim().length > 0 && isSemanticKey(key)) {
-      merged[key] = url;
+      merged[key] = url.trim();
     }
   }

Also applies to: 36-37

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@frontend/entities/asset/model/manifest.ts` around lines 14 - 18, The manifest
URL validation trims the input but stores the original untrimmed string; update
the storage to persist the trimmed URL by replacing occurrences where you push
or assign url into entries (e.g., entries.push([key, url])) with the trimmed
value (e.g., use url.trim()) and do the same for the other occurrence mentioned
(lines around the second validation), ensuring every place that writes the
manifest entry uses the trimmed url.

scripts/stitch/process-sketch.ts (1)

105-114: Validate sourceSize against actual input dimensions before processing.

At Line 105–Line 114, the script checks key existence and image metadata, but it does not verify that --input matches the key’s expected sourceSize. A mismatched file can still overwrite the canonical output and hash for that semantic key.

🔧 Proposed patch

   const map = await loadAssetMap();
-  if (!map[key]) {
+  const entry = map[key];
+  if (!entry) {
     throw new Error(`asset-map.json has no entry for key: ${key}`);
   }

   const meta = await sharp(absInput).metadata();
   if (!meta.width || !meta.height) {
     throw new Error('could not read source image dimensions');
   }
+  const expectedSize = /^(\d+)x(\d+)$/.exec(entry.sourceSize);
+  if (expectedSize) {
+    const expectedWidth = Number(expectedSize[1]);
+    const expectedHeight = Number(expectedSize[2]);
+    if (meta.width !== expectedWidth || meta.height !== expectedHeight) {
+      throw new Error(
+        `source size mismatch for key ${key}: expected ${entry.sourceSize}, got ${meta.width}x${meta.height}`
+      );
+    }
+  }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/stitch/process-sketch.ts` around lines 105 - 114, The script
currently loads the asset map with loadAssetMap(), finds map[key], and reads
image metadata via sharp(absInput).metadata(), but it doesn't verify that the
actual image dimensions match the expected sourceSize for that semantic key; add
a validation step that reads expected sourceSize from map[key] (e.g.,
map[key].sourceSize or similar), compare it to meta.width and meta.height, and
if they differ throw an Error (or exit) before calling computeBottomCrop and
continuing so a mismatched input cannot overwrite the canonical output/hash for
that key.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes