fix(chat): keep link-preview favicons visible on every theme

[kyleseaman]

Problem

In dark mode, a link preview's favicon can be invisible. The chip renders, the
title renders, and where the site's icon should be there is nothing — the icon is
in the DOM, painted, and indistinguishable from the background behind it.

GitHub is the reported case: its markup declares favicons/favicon.png, a
near-black octocat drawn for a white browser tab, and the favicon box paints no
background of its own. Measured in a browser, that icon sits at 1.02:1
against the dark theme's --bg.

Why it matters

The favicon is the only part of an unfurled chip that identifies the destination
at a glance — unfurling replaced the URL text with a page title, so a reader who
cannot see the icon has lost the "which site is this?" signal the raw link used to
carry. It affects every dark theme (the majority of this dashboard's themes) and
every site that ships one tab-coloured icon, which is most of them.

Fix (symptoms → root cause → change)

The symptom is theme-shaped, but the theme is not the cause: a dark theme is only
a problem for a dark icon, and a light theme only for a light one. What decides
it is the pair — the icon's own colours against the surface it is composited over.

Two hypotheses were ruled out before landing on that:

• Theming the icon box is a coin flip. Any fixed background is wrong for half
  the icons that land on it.
• Honouring <link media="(prefers-color-scheme: dark)"> alone does not reach
  the reported case. GitHub declares no media variant; it swaps its favicon from
  JavaScript (js-site-favicon + data-base-href), which a server-side fetch
  never sees.

So the fix works on the pair, in two layers, preferring the site's own artwork
over anything inferred:

1. Use the site's declared variant when there is one. _HeadParser now reads
each icon link's media attribute (icon_color_scheme()) and splits candidates
into a default lane (icon_candidates — everything except dark-scoped icons) and
a dark lane (dark_icon_candidates). The handler fetches the dark variant only
when a page declares one, with a budget of exactly one attempt, sequentially
after the default icon, and drops it when it resolves to bytes the default icon
already carries. New wire field icon_dark; the client owns the choice because
the theme switches at runtime while the payload is cached for 6h and shared by
every tab. A page with one unscoped icon — the overwhelmingly common case — costs
exactly what it did before (test_no_declared_variant_costs_no_extra_request).

2. Where no variant exists, measure and plate. iconContrast.ts samples the
decoded icon on a shared canvas and the surface behind the box (compositing
translucent ancestors over the first opaque one, --bg as last resort), and the
box paints --text behind the icon only when neither end of the icon's
luminance range — 5th and 95th percentile — clears 1.6:1 against that
surface. --text is the one token every theme guarantees against its own
backgrounds, so a single class is correct in both directions: a light plate under
a dark icon on a dark theme, a dark plate under a light icon on a light one.

Two decisions worth a reviewer's attention:

• Why 1.6:1 and not WCAG's 3:1 for non-text content. This can only ever paint
  over what the site shipped, so it aims at "absent", not "dim". Measured in a
  real browser against real favicons: the failures sit at 1.02 (GitHub's octocat
  on dark) and 1.20 (its white counterpart on light), while Google, Stack
  Overflow, Wikipedia, Amazon, npm, GitLab and Python all clear the threshold
  untouched in both modes. At 3:1 a saturated mid-tone mark trips the test and
  earns a plate it does not need — the fix would be more visible than the bug.
• Why both ends of the range must fail. An icon that ships its own opaque
  backdrop (a dark tile with light lettering) is legible on any surface. A mean
  would call it "dark" and plate it on a dark theme; requiring both the dim and
  the bright end to fail spares it.

The surface is measured from the box's parent, never the box: once a plate is
painted, the box's own background is the plate, so measuring the box would
compare the icon against the plate and undo the decision on the next theme event.

Tests

Backend — test/test_link_unfurl.py (+15, 197 in file):

• lane split: a dark-scoped icon never serves as the default; an unscoped icon
  stays in the default lane; the dark lane gets no /favicon.ico fallback and
  drops an href the default lane already has; dark-scoped apple-touch icons order
  after plain ones.
• icon_color_scheme over untrusted media values, including not all and (prefers-color-scheme: dark) — a negation inverts the match, so the parser
  declines to classify rather than guess, and the icon stays usable as the default.
• endpoint: both icons returned; no extra request without a declared variant;
  identical bytes collapsed to ""; a dark-lane 404 leaves the preview intact;
  the dark href is SSRF-vetted like any other fetch (link-local refused) and an
  SVG variant is dropped; the lane spends exactly one request.

Frontend — iconContrast.test.ts (30), LinkPreview.faviconPlate.test.tsx (16),
linkMeta.test.ts (+4):

• the measurement itself over real pixel data: percentiles, alpha-weighted
  coverage, both plate directions, mid-tone marks left alone, self-contrasting
  icons left alone, and the threshold band recorded so it is not silently retuned.
• colour parsing for rgb(), rgba(), color(srgb …) (how the theme's alpha
  utilities compute) and hex, with unknown forms returning "no opinion".
• wiring: the surface is sampled from the box's parent (the oscillation guard),
  the decision is retaken on a theme change, the subscription is dropped on
  unmount, no plate on a missing measurement, and iconDark is held to the same
  data:-image-only screen as icon.
• variant selection: the site's dark icon on a dark surface, the default on a
  light or unmeasurable one, a re-measure after a swap, and an undecodable variant
  demoting to the default icon rather than to the placeholder.

Every one of these was revert-verified: 11 mutations of the production code were
applied one at a time and each was caught (including a naive pixel-mean instead of
the dual-end rule, an unscreened icon_dark, and a single-slot failure flag that
would bounce between two dead images).

Manual verification

Local gates: isort, flake8, mypy (865 files), tsc -b, eslint 0 errors, jscpd 0
clones, i18n 16/16 PASS, vitest 12223 passed. The full backend suite reports 62
failures on this host; all 62 reproduce on origin/main itself (no OS sandbox
backend, missing jq, gateway port 6776 vs 5476, Node 16 default) — verified by
running the same files in a throwaway worktree at the base commit, not by
inspection.

Beyond unit coverage, the decision was exercised end-to-end in a real browser
against real favicon bytes, because jsdom neither decodes an image nor applies the
stylesheet the surface is read from. website/capture/link-preview.{html,tsx} +
scripts/capture-link-preview.mjs follow the existing capture/path-chips.*
convention: the real MarkdownRenderer mounts against the real stylesheet with
fetch stubbed at the same /api/link-meta seam the real hook uses, so the icons
measure and plate themselves exactly as they do in production. The runner is
self-checking — it asserts which icons were plated and which variant each one
rendered, per theme, before it will emit a screenshot, so it cannot quietly
produce a picture of the unfixed bug.

Screenshots

Inline chips, dark theme — row 1 is the reported case (one near-black icon, now
plated), row 2 is a site that declares its own dark variant (its icon, no plate),
row 3 is a mid-tone mark that must stay untouched:

The same three on the light theme, all untouched — nothing about this change
alters a theme the icons were already legible on:

Block card form, both themes (the same decision at the 32px icon size)

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed e7585d15f0eb09fd5e95188e72e57604d7dfb8d9 — this comment is updated in place on each push.

Review details

This PR is thorough and well-tested. Let me verify the two areas most likely to hide a semantic defect: the sequential dark-icon fetch/dedup logic and the frontend re-measurement on variant swap.

The backend _fetch_icon budget slicing, the dark_icon == icon dedup, SSRF vetting (shared _vet path), and SVG rejection are all confirmed correct and test-covered. The frontend chosen/failed/tone reset flow, parent-surface sampling (oscillation guard), and theme-swap re-measure are consistent with the tests and the intent. The committed temp-screenshots/ follow the repo's existing convention (tracked deliverables with a cleanup workflow), not a stray artifact.

No reachable correctness, security, resource, or contract defect survives falsification on the changed lines.

No findings.

[OPUS-REVIEWED] e7585d1

Verdict parsed from the review's SHA-scoped output markers for commit e7585d15f0eb09fd5e95188e72e57604d7dfb8d9.

False positive or not applicable? A repository writer can comment:
/ai-review override fable e7585d15f0eb09fd5e95188e72e57604d7dfb8d9: <one-sentence reason>

[github-actions]

Design Review (Fable 5) — ✅ PASS

Advisory design-level review of e7585d15f0eb09fd5e95188e72e57604d7dfb8d9 — updated in place on each push; does not block merge.

Design-Verdict: PASS

Measures the actual failing pair (icon pixels vs composited surface) instead of guessing per theme or per site — root-cause fix, fail-safe nulls, additive wire contract.

[DESIGN-REVIEWED] e7585d1

[github-actions]

UX Review (Fable 5) — ✅ PASS

Advisory UX-level review of e7585d15f0eb09fd5e95188e72e57604d7dfb8d9 — updated in place on each push; does not block merge.

UX-Verdict: PASS

Invisible-icon repair with no new strings, no new interactions, and the screenshots prove the plate fires only where the icon actually vanished.

[UX-REVIEWED] e7585d1

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of e7585d15f0eb09fd5e95188e72e57604d7dfb8d9 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] e7585d1

False positive or not applicable? A repository writer can comment:
/ai-review override gpt e7585d15f0eb09fd5e95188e72e57604d7dfb8d9: <one-sentence reason>