fix(render): ignore favicon probe noise

[miguel-heygen]

What

Ignore favicon 404s when the render probe summarizes browser asset-load failures, while preserving real missing assets and network failures.

Why

Browsers may request /favicon.ico automatically. The file server and validator already treat that miss as benign, but the probe warning path promoted every buffered 404 to [Render] Asset load failure, adding noise to otherwise clean renders.

Verification

• failing-then-passing focused test covers favicon 404 vs real image 404 and script net::ERR_FAILED
• bun test packages/producer/src/services/render/stages/probeFailures.test.ts
• oxfmt --check on changed files
• git diff --check
• pre-commit lint/format/fallow/tracked-artifact checks passed; monorepo typecheck could not resolve workspace packages in the isolated worktree node_modules setup

[vanceingalls]

R1 — RIGHT (approve, three findings-track nits)

Position: the fix is correct. Tight URL-segment-bounded predicate, extracted helper, failing-then-passing regression test, no interaction with retry/counter/gating (the surrounding comment confirms the warn path "doesn't block the render"). Raw browserConsoleBuffer is preserved intact and returned as lastBrowserConsole — the filter shapes the summary surface only, so downstream diagnostics still see the favicon line if anyone dumps the buffer. That's the safe pattern.

Adversarial pass on the predicate — I probed the shape and found it well-bounded:

• FAVICON_PATTERN = /(?:^|[/?])favicon(?:\.[a-z0-9]+)?(?:[?#\s]|$)/i
• Head anchor (?:^|[/?]) and tail anchor (?:[?#\s]|$) prevent false-positives on user paths like /api/favicon-service/probe.ico, ?favicon=true, favicon-16x16.png (all correctly still surface as errors).
• Handles .ico, .png, .svg, .webp, and query strings (favicon.ico?v=2, favicon.ico#frag). The extension is optional too (GET /favicon ).
• Won't mis-drop a real render asset — if a composition legitimately loads /assets/favicon.png as an image, it would be silenced, but favicons don't render into video output so that's an acceptable tradeoff for the probe surface.
• Consistent with existing favicon carve-outs elsewhere: fileServer.ts:766 (/favicon\.ico$/i), validate.ts:455/469 (url.includes("favicon")), useConsoleErrorCapture.ts:53 (text.includes("favicon")). Your new predicate is tighter than the two .includes sites and broader than the fileServer one — the right calibration for a warn-noise filter.
• No metric/counter reads "Asset load failure" — grep is single-site. Pure warn surface.

PR envelope: clean — Miguel-authored, single commit, no Co-Authored-By: Claude or 🤖 Generated with [Claude Code] leak.

CI: the Tests on windows-latest failure Magi flagged was on the first run (87402784580, 16:05Z). After the 16:06Z push the same job at 87406050060 went SUCCESS. Windows render + tests are both green at head; mergeStateStatus=BLOCKED is required-review only, not a CI signal.

Nits (non-blocking)

1. Test parameterization gap. The regression exercises favicon.ico only. A one-line table extension would harden against future regex regressions to favicon\.ico$:

   • /favicon.svg, /favicon.png, /favicon.webp (extension variants the pattern claims to handle)
   • /favicon.ico?v=2 (query-string tail-boundary)
   • /favicon-16x16.png and /apple-touch-icon.png as negative cases confirming the pattern doesn't silently suppress them either (scope-check assertion).

2. Sibling filter untouched. probeStage.ts:625 (the duration <= 0 diagnostic path) still uses the inline /\[Browser:ERROR\]|\[Browser:PAGEERROR\]|404|net::ERR_/i without the favicon carve-out. Plausibly intentional — that path only fires on hard failure and wants all noise as evidence — but a one-line comment noting the deliberate divergence would prevent a future reviewer from "unifying" it and re-introducing render-failure diagnostic gaps.

3. Out-of-scope siblings. Browser auto-probes /apple-touch-icon*.png, /favicon-16x16.png, /site.webmanifest, /browserconfig.xml on some clients. FAVICON_PATTERN correctly doesn't touch these — but if the noise turns out to have more heads than one, this is the ideal helper to grow. Track as follow-up if the noise recurs.

— Via

[james-russo-rames-d-jusso]

Reviewed at cbf646a5. Windows-latest CI failure is an unrelated one-off — ECONNREFUSED 127.0.0.1:3000 + a happy-dom Google Fonts fetch abort — Magi's rerun should clear it. Not a review-holder.

Blockers

(none)

Concerns

(none)

Nits

(none)

Green notes

🟢 Helper extraction is clean. isActionableProbeFailure(line) isolates the pattern into a colocated probeFailures.ts + unit test; probeStage.ts swaps the inline regex for the helper — one-line callsite change, no other behavior modified.

🟢 FAVICON_PATTERN bounds hold. Walked a few candidates:

• /favicon.ico?v=123 — leading /, favicon, optional .ico consumed, trailing ?. Matches (filtered).
• /favicon.ico resource=image (probe line format) — trailing \s. Matches (filtered).
• /favicon — trailing $. Matches (filtered).
• /favicon-designer/tool.png (false-positive candidate) — leading /, favicon, no optional ext, next char - — doesn't satisfy [?#\s]|$. Correctly NOT matched.
• /assets/some/favicon.png.bak — matches favicon after /, .png optional ext consumed, trailing char is . (of .bak). Doesn't satisfy bound → NOT matched (an actual real-asset failure would still surface). Fine — unusual filename shape.

🟢 NETWORK_FAILURE_PATTERN — net::ERR_ catches all Chromium net error codes, plus explicit ERR_NAME_NOT_RESOLVED and ERR_CONNECTION_REFUSED for the non-net::-prefixed variants that DevTools sometimes emits. If another exotic browser probe pattern surfaces later, it's a two-line extension in one file rather than an inline-in-caller update.

🟢 Test asserts three cases: favicon 404 filtered, real image 404 preserved, net::ERR_FAILED on a script preserved. Covers the three shapes that mattered to the reporter of the noise.

Verdict framing

Small, surgical probe-noise reduction with a well-scoped extraction and a red-first regression test. LGTM from my side.

— Review by Rames D Jusso