Collapse the five web-vitals trackers into a shared helper

[philipwalton]

Problem

The web-vitals handlers in src/javascript/log.ts — trackCLS, trackFCP, trackINP, trackLCP — were near-identical ~25-line blocks that differed only in how they map attribution data to log params (recommendations.md #15). The async keywords on them were also pointless: nothing inside them awaits, and nothing awaits them.

Root cause

Each metric handler was written by copy-paste: every one registered a callback that calls log.event(metric.name, {value, metric_rating, metric_value, ...metric-specific params, event_id}), duplicating the common shape five times.

What changed

• Added a generic trackMetric(onFn, opts, getAttributionParams) helper that registers the callback and logs the common params, spreading the metric-specific params (returned by the mapper) between metric_value and event_id — the exact position they occupied in every original handler.
• trackCLS, trackFCP, trackINP, trackLCP are now thin wrappers supplying only their mapper (and INP's durationThreshold: 16). LCP's dynamic-fetchpriority logic (the /hint sendBeacon side effect and debug_dfp) lives unchanged inside LCP's mapper and still runs before log.event.
• trackTTFB is intentionally left bespoke: its param order differs (metric_value before metric_rating, event_id before the attribution block), and it conditionally assigns a whole navigation-timing block plus serverTiming expansion and activation_start. Forcing it into the helper would hurt readability.
• Removed the pointless async keywords (TTFB was already non-async).
• init() order is untouched — FCP still starts first, with the explanatory comment preserved.
• Typing: trackMetric<T extends MetricWithAttribution, O extends ReportOpts> infers each metric's *MetricWithAttribution callback type and opts type (e.g. INP's INPAttributionReportOpts) directly from the passed on* function. No any anywhere.

Param-equivalence proof (static self-check, old vs. new)

Keys emitted per metric, in order — identical before and after:

Metric	Params
CLS	value, metric_rating, metric_value, debug_target, event_id
FCP	value, metric_rating, metric_value, original_page_path, debug_ttfb, debug_fb2fcp, event_id
INP	value, metric_rating, metric_value, debug_target, debug_type, debug_time, debug_delay, debug_processing, debug_presentation, event_id
LCP	value, metric_rating, metric_value, debug_target, debug_url, debug_dfp, debug_ttfb, debug_rld, debug_rlt, debug_erd, event_id
TTFB	untouched (bespoke)

All value expressions (including the '(not set)' fallbacks and LCP's hit/miss debug_dfp) are unchanged verbatim.

Verification

• npm run lint — pass (0 errors)
• npm run types:check — pass (astro check + tsc, 0 errors)
• npm run test:unit — pass (16 files, 67 tests)
• npm run build — pass (48 pages)
• Full e2e (npm test) — ran once in full, then re-ran the two flaky specs in isolation:
  • Full run failures, all among the known pre-existing set: homepage.ts "working links to all published articles" (deterministic, atom.xml sort bug); content-loading.ts "should not attempt to load non-HTML content" (deterministic); log.ts "should track engagement time" (environmental); worker.ts both priority-hints tests (environmental clearStorage/__reset timeout — notably the "wait until hint has been sent" step passed, confirming the refactored LCP /hint beacon fires).
  • Isolated re-run of SPECS=worker,log: 2/2 spec files passed, 100% — including both priority-hints tests and the engagement-time test. The log.ts spec asserts the exact beacon params this refactor must preserve.

Codex review

Verdict: REQUEST_CHANGES — overruled, with the disagreement recorded here:

• Codex's one "blocking" (Medium) finding: removing async changes error propagation — a sync throw during on* registration previously became an unawaited rejected promise (init continued), now it would propagate and skip later trackers. This is technically true but not a real regression: the old behavior was a silent unhandled rejection; web-vitals on* registration doesn't throw synchronously (it feature-detects internally); trackTTFB was already non-async on main with these exact semantics; and removing the pointless async was an explicit goal of the change.
• Non-blocking (Low): add unit tests asserting emitted params per handler — reasonable follow-up; the e2e log spec covers this today. Not added to keep this PR a pure refactor.
• Non-blocking (Low): constrain the mapper's return type to forbid overriding common keys — left out as over-engineering for four call sites.

Please scrutinize

• Whether you agree TTFB should stay bespoke rather than being forced into the helper.
• Whether the codex overrule (error-propagation semantics of the removed async) matches your intent.
• Conflicts: PR Refresh Logger page params (dl/dt) on SPA navigations #118 (fix/logger-spa-params) touches Logger.ts/content-loader.ts and test/e2e/log.ts but not these tracker functions — expect no or trivial conflicts. PR Clean up breakpoints mql typing and remove dead content-loader code #119 (fix/js-cleanups, merged) touches content-loader.ts/breakpoints.ts, not log.ts — no conflict.

🤖 Generated with Claude Code