Add Lighthouse audit system with build tools and dashboard - #3

src/data-extract.ts-133-133 (1)

133-133: ⚠️ Potential issue | 🟡 Minor

Mode fallback loses granularity and may mis-group legacy files.

When summary.mode is missing, this collapses every non-desktop variant to 'mobile', so mobile-4g and mobile-wifi files (which are first-class in config.emulations) get merged together in extracted.urls[url].modesRaw. If any legacy report lacks the mode field, the averaged dashboard series will silently blend two different network profiles.

Prefer decoding from the filename when possible, or skip entries without a resolvable mode. At minimum, log a warning when the fallback fires.

🤖 Prompt for AI Agents

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

In `@src/data-extract.ts` at line 133, The fallback that sets mode to 'mobile'
when summary.mode is missing is collapsing distinct emulation profiles; update
the logic in the data extraction where mode is assigned (currently using
summary.mode or file.includes('-desktop-')) to attempt decoding the full mode
token from the filename (e.g., detect "mobile-4g", "mobile-wifi", "desktop" by
parsing the filename pattern) and only fallback to a single generic value if you
cannot resolve any token; if you still must fallback, do not merge profiles
silently—either skip the entry or emit a warning via the logger that the
filename could not resolve a mode so maintainers know why an entry was
dropped/merged; ensure changes affect how entries are placed into
extracted.urls[url].modesRaw and reference config.emulations to validate allowed
mode strings.

src/data-extract.ts-138-149 (1)

138-149: ⚠️ Potential issue | 🟡 Minor

timestamp fallback to Date.now() corrupts historical ordering.

If a summary is written without timestamp, this assigns the current extraction time, which then drives both the temporal-proximity grouping (LEGACY_GROUP_GAP_MS) and the final result.sort((a, b) => a.timestamp - b.timestamp). The net effect is that a historical report silently jumps to "now", clusters with unrelated current runs, and appears as the most recent data point on the dashboard.

Safer options:

• Fall back to the file's mtime (fs.statSync(...).mtimeMs).
• Parse the timestamp out of the filename if your naming scheme encodes it.
• Skip the file with a warning instead of fabricating a timestamp.

🤖 Prompt for AI Agents

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

In `@src/data-extract.ts` around lines 138 - 149, The runData construction
currently assigns timestamp: summary.timestamp || Date.now(), which fabricates
“now” when a summary lacks a timestamp; change this to derive the timestamp from
the file's modification time (e.g., use fs.statSync(filePath).mtimeMs) or, if
stat fails, log a warning and skip the file rather than using Date.now(); update
the timestamp assignment in the runData object (where summary.timestamp is read)
and add an fs import and error handling around stat to avoid corrupting ordering
and grouping (references: runData, summary.timestamp).

tsconfig.json-19-19 (1)

19-19: ⚠️ Potential issue | 🟡 Minor

Stale reference to build.ts.

The build entry is build.js (per package.json scripts and build.js itself), not build.ts. TypeScript silently ignores the nonexistent path, so it's not a hard error, but the include is misleading. Either drop "build.ts" from include, or—if you want to typecheck the build script—rename it to build.ts and add allowJs accordingly.

-  "include": ["src/**/*", "build.ts"],
+  "include": ["src/**/*"],

🤖 Prompt for AI Agents

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

In `@tsconfig.json` at line 19, The tsconfig include array contains a stale
reference "build.ts" that doesn't exist; update the tsconfig.json include (or
rename the script) so it's accurate: either remove "build.ts" from the "include"
array, or convert the build script to TypeScript (rename to build.ts) and enable
allowJs/adjust compilerOptions accordingly so the build script is actually
type-checked; target the "include" entry in tsconfig.json and the build script
file referenced by package.json scripts to make the change.

src/index.ts-47-47 (1)

47-47: ⚠️ Potential issue | 🟡 Minor

Exit non-zero on failure.

main().catch(console.error) swallows the error and the process exits with code 0, so CI (e.g., the new release.yml or any wrapper script) cannot detect Lighthouse runner failures.

-main().catch(console.error);
+main().catch((err) => {
+  console.error(err);
+  process.exit(1);
+});

🤖 Prompt for AI Agents

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

In `@src/index.ts` at line 47, Current use of main().catch(console.error) logs
errors but leaves the process exit code as 0; update the top-level invocation
(the main() call in src/index.ts) to handle promise rejection by logging the
error and exiting non‑zero — e.g., replace the simple catch with a handler that
calls console.error with the error and then process.exit(1) so CI detects
failures.

src/index.ts-16-16 (1)

16-16: ⚠️ Potential issue | 🟡 Minor

args.runIterations can be NaN.

In src/config.ts, runIterations is set via parseInt(arg.split('=')[1]) — passing --runs=abc (or --runs=) yields NaN, not undefined, so this branch keeps NaN. Downstream iterations > 0 evaluates false and the loop is silently skipped, which is confusing for the user.

Consider guarding with Number.isFinite:

-  const iterations = args.runIterations !== undefined ? args.runIterations : config.iterations;
+  const iterations = Number.isFinite(args.runIterations as number)
+    ? (args.runIterations as number)
+    : config.iterations;

🤖 Prompt for AI Agents

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

In `@src/index.ts` at line 16, The current assignment to iterations uses
args.runIterations which may be NaN; change the guard to check for a finite
number (e.g. use Number.isFinite(args.runIterations)) and fall back to
config.iterations when it's not a valid number, and emit a clear warning or
error via the existing logger if fallback occurs; update the assignment that
sets iterations (the line referencing args.runIterations and config.iterations)
so it only accepts finite numeric values.

src/dashboard/index.ts-7-12 (1)

7-12: ⚠️ Potential issue | 🟡 Minor

Ensure config.baseDir exists before writing, and note duplicate path logic.

• If config.baseDir does not exist (e.g., fresh checkout where no iterations were run before generateVisualReport is invoked), fs.writeFileSync will throw ENOENT. Create the directory first.
• The same path.join(config.baseDir, 'visual-summary.html') is also computed inside generateHtml in src/dashboard/template.ts:88. The value computed there appears unused in the returned HTML, so consider removing one of the duplicates to keep a single source of truth.

🛡️ Proposed hardening

 export function generateVisualReport(statsData: StatsData): void {
   const reportPath = path.join(config.baseDir, 'visual-summary.html');
   const htmlContent = generateHtml(statsData);
+  fs.mkdirSync(config.baseDir, { recursive: true });
   fs.writeFileSync(reportPath, htmlContent);
   console.log(`\n✨ Refined Premium Dashboard updated: ${reportPath}`);
 }

🤖 Prompt for AI Agents

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

In `@src/dashboard/index.ts` around lines 7 - 12, generateVisualReport currently
writes to path.join(config.baseDir, 'visual-summary.html') without ensuring
config.baseDir exists and duplicates the same join inside generateHtml; fix by
ensuring the directory exists (create config.baseDir with fs.mkdirSync(..., {
recursive: true }) or similar) before calling fs.writeFileSync in
generateVisualReport, and deduplicate the path logic by removing the unused
path.join in generateHtml (or refactor generateHtml to accept the fully-resolved
reportPath) so the file path is computed in one place (referencing
generateVisualReport, config.baseDir and generateHtml to locate the changes).

src/dashboard/client.ts-449-456 (1)

449-456: ⚠️ Potential issue | 🟡 Minor

Tooltip can render undefined for out-of-range url indices.

urlLabels[d.x] has no fallback here (unlike the axis callback on line 440). If d.x ever exceeds urlLabels.length - 1 (e.g., data/label mismatch), the tooltip prints "Score: 82 undefined (Click for details)".

Details

-            return 'Score: ' + d.y + ' ' + urlLabels[d.x] + ' (Click for details) ';
+            return 'Score: ' + d.y + ' ' + (urlLabels[d.x] || '') + ' (Click for details) ';

🤖 Prompt for AI Agents

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

In `@src/dashboard/client.ts` around lines 449 - 456, The tooltip label callback
currently uses urlLabels[d.x] without bounds checking, causing "undefined" if
d.x is out of range; update the tooltip's label callback (the function assigned
under tooltip.callbacks.label that reads const d = ctx.raw) to guard against
invalid indices and use the same fallback as the axis callback (e.g., check d.x
is a number within 0..urlLabels.length-1 and if not use a default like 'Unknown
URL' or an empty string) so the returned string never contains "undefined".

justfile-40-41 (1)

40-41: ⚠️ Potential issue | 🟡 Minor

lighthouse-runner.mjs.map is never produced.

build.js sets sourcemap: false (line 15), so there's no map file to remove. Either enable sourcemaps in the build or drop the stale filename from clean.

🧹 Proposed fix

 # Clean build artifacts
 clean:
-    rm -f lighthouse-runner.mjs lighthouse-runner.mjs.map
+    rm -f lighthouse-runner.mjs

🤖 Prompt for AI Agents

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

In `@justfile` around lines 40 - 41, The clean target in the justfile references
lighthouse-runner.mjs.map which is never generated because build.js sets
sourcemap: false; update either build.js (change the bundler option sourcemap:
false to sourcemap: true in the build configuration) to produce the .map file,
or remove the stale lighthouse-runner.mjs.map entry from the clean recipe in the
justfile so the clean target only removes files actually produced.

src/lighthouse.ts-182-194 (1)

182-194: ⚠️ Potential issue | 🟡 Minor

Missing runId breaks run-group correlation in data-extract.

runLighthouse(url, i, mode) is called without the optional runId argument. src/data-extract.ts groupAndAverage first groups by explicit runId and only falls back to temporal proximity for legacy entries. Without a runId per pass, every new run is grouped by timestamp alone (LEGACY_GROUP_GAP_MS), which is fragile for slow audits or interleaved URLs. Pass i (or a monotonically increasing counter) as runId to make grouping deterministic.

Details

-          await runLighthouse(url, i, mode);
+          await runLighthouse(url, i, mode, i);

🤖 Prompt for AI Agents

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

In `@src/lighthouse.ts` around lines 182 - 194, The loop calls runLighthouse(url,
i, mode) without supplying a runId, so groupAndAverage in src/data-extract.ts
(which groups by explicit runId before falling back to LEGACY_GROUP_GAP_MS)
cannot deterministically correlate runs; modify the invocation in the Round
Robin loop to pass a stable runId (e.g., the pass counter i or a dedicated
monotonically increasing counter) to runLighthouse (ensure the runLighthouse
signature/forwarding is updated where necessary) so each pass across config.urls
and config.emulations is recorded with that runId for deterministic grouping.

src/lighthouse.ts-43-77 (1)

43-77: ⚠️ Potential issue | 🟡 Minor

Chrome instance can leak on early failure.

chromeLauncher.launch happens before the try block. If any synchronous code between the launch (line 47) and the try on line 77 throws (e.g., getUrlHash(url) on an invalid URL), chrome.kill() will never run and the headless process will linger. Move the launch inside the try, or put an immediate try/finally around the whole body.

🛡️ Proposed fix

 export async function runLighthouse(url: string, iteration: number, mode: string, runId?: number): Promise<void> {
   const chrome = await chromeLauncher.launch({
     chromePath: 'chromium',
     chromeFlags: ['--headless', '--no-sandbox', '--disable-gpu'],
   });
-
-  const isDesktop = mode === 'desktop';
-  ...
-  console.log(`\n🚀 [${mode.toUpperCase()}] Audit: ${url} (ID: ${urlHash})`);
-
   try {
+    const isDesktop = mode === 'desktop';
+    ...
+    console.log(`\n🚀 [${mode.toUpperCase()}] Audit: ${url} (ID: ${urlHash})`);
     const runnerResult = await lighthouse(url, options, configObj);

🤖 Prompt for AI Agents

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

In `@src/lighthouse.ts` around lines 43 - 77, The chromeLauncher.launch call in
runLighthouse is outside the try/finally so a thrown error (e.g., from
getUrlHash) can leak the Chrome process; move the launch into the try block or
wrap the entire function body in a try/finally so that
chromeLauncher.launch(...) (and the returned chrome instance) is created within
the try and chrome.kill() is always invoked in the finally; ensure references to
chrome from chromeLauncher.launch and chrome.kill are preserved and handle the
case where chrome may be undefined (e.g., check chrome before calling kill).

src/dashboard/client.ts-391-412 (1)

391-412: ⚠️ Potential issue | 🟡 Minor

Zoom/pan callbacks assigned after chart creation won't take effect immediately.

Setting trendsChart.options.plugins.zoom.zoom.onZoom and .pan.onPan after the chart is instantiated requires calling trendsChart.update('none') afterward for the changes to register in chartjs-plugin-zoom v2.2.0. Without this, the callbacks won't fire on the user's first direct zoom/pan interaction. Recommend either declaring the callbacks in the initial new Chart(...) options object, or explicitly calling trendsChart.update('none') after both callback assignments (after line 412).

🤖 Prompt for AI Agents

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

In `@src/dashboard/client.ts` around lines 391 - 412, The zoom/pan callbacks
assigned to trendsChart after instantiation
(trendsChart.options.plugins.zoom.zoom.onZoom and
trendsChart.options.plugins.zoom.pan.onPan) won't register until the chart is
updated; after assigning both callbacks, call trendsChart.update('none') so they
take effect immediately, or alternatively move these handlers into the initial
new Chart(...) options object; keep using the same variables (isUpdatingChart,
isUpdatingSlider, timelineSlider, updateVisibleLabels) in the handlers.

lighthouse-runner.mjs-300-300 (1)

300-300: ⚠️ Potential issue | 🟡 Minor

readdirSync(runsDir) will throw if the directory is missing.

compressAllReports is invoked from the --compress branch before any guard. Because runsDir creation is gated by fs.existsSync at config init, it should exist, but a user running --compress on a fresh checkout without ./reports/runs/ (or with a custom baseDir that was never audited) will get an unhandled ENOENT. Guard with fs.existsSync(runsDir) and print a friendly "no reports to compress" message.

🤖 Prompt for AI Agents

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

In `@lighthouse-runner.mjs` at line 300, compressAllReports currently calls
fs3.readdirSync(runsDir) which will throw ENOENT if runsDir doesn't exist;
update compressAllReports to first check fs3.existsSync(runsDir) and if false
log a friendly "no reports to compress" message (or similar) and return early.
Locate the call site using the function name compressAllReports and the variable
runsDir (where readdirSync is used) and add the existsSync guard and early
return so running with --compress on a fresh checkout or custom baseDir does not
crash.

lighthouse-runner.mjs-484-486 (1)

484-486: ⚠️ Potential issue | 🟡 Minor

Falsy-check on metric values hides 0 as "-".

metrics.tbt || "-" (and the (... || 0) / 1e3 for FCP/LCP) treat a legitimate 0 value as missing data. TBT = 0 is a real, desirable outcome; logging it as "-" is misleading. Same pattern should be fixed in buildMetricsRow at lines 1684-1689 and in getAggAvg at line 1266 (see separate comment). Prefer metrics.tbt != null ? ... : "-".

🤖 Prompt for AI Agents

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

In `@lighthouse-runner.mjs` around lines 484 - 486, The log and helper functions
incorrectly use falsy-checks that convert legitimate zero metric values into "-"
(e.g., the console.log line using metrics.tbt || "-", and similar patterns in
buildMetricsRow and getAggAvg); update those checks to explicitly test for
null/undefined (e.g., use metrics.tbt != null ? metrics.tbt : "-" and for
fcp/lcp use (metrics.fcp != null ? metrics.fcp : 0) before dividing) so 0 is
preserved; locate the console.log statement shown and the functions
buildMetricsRow and getAggAvg and replace all "||" fallbacks used to detect
missing metrics with null/undefined checks (!= null) to ensure zeros are treated
as real values.

lighthouse-runner.mjs-1820-1828 (1)

1820-1828: ⚠️ Potential issue | 🟡 Minor

External scripts loaded without Subresource Integrity.

Six <script> and one <link> pull from cdn.jsdelivr.net / cdnjs.cloudflare.com with no integrity= nor crossorigin=. If the dashboard HTML is ever hosted/shared, a compromised CDN could inject arbitrary code. Since the CDN URLs already pin exact versions, adding SRI hashes is straightforward and free. Consider generating SRI hashes at build time or, better, bundling these dependencies locally alongside the generated HTML so the report is fully self-contained and offline-viewable.

🤖 Prompt for AI Agents

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

In `@lighthouse-runner.mjs` around lines 1820 - 1828, The external CDN <script>
and <link> tags in the generated HTML (the Chart.js, chartjs-plugin-zoom,
chartjs-chart-boxplot, noUiSlider CSS/JS, Luxon and chartjs-adapter-luxon tags
shown in the diff) lack Subresource Integrity and crossorigin attributes; fix
this by either (a) adding correct integrity="sha384-..." and
crossorigin="anonymous" attributes for each CDN URL (generate SRI hashes at
build time and inject them into the template that produces these tags), or (b)
bundle these assets locally and update the template to reference local files
instead so the report is self-contained/offline; update the HTML generation
logic that emits those <script> and <link> tags accordingly.

lighthouse-runner.mjs-1265-1268 (1)

1265-1268: ⚠️ Potential issue | 🟡 Minor

getAggAvg coerces missing categories to 0, skewing downward.

d.map(r => (r.categories[key] || 0)) counts a missing or 0 category as a real 0 in the average, pulling the radar chart toward the origin whenever some runs don't have a given category. averageGroup (line 107-108) already filters null/NaN correctly; mirror that here:

 const getAggAvg = (data, key) => {
-  const vals = data.flatMap(d => d.map(r => (r.categories[key] || 0)));
-  return vals.length ? vals.reduce((a, b) => a + b, 0) / vals.length : 0;
+  const vals = data
+    .flatMap(d => d.map(r => r.categories[key]))
+    .filter(v => v != null && !Number.isNaN(v));
+  return vals.length ? vals.reduce((a, b) => a + b, 0) / vals.length : 0;
 };

🤖 Prompt for AI Agents

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

In `@lighthouse-runner.mjs` around lines 1265 - 1268, getAggAvg currently coerces
missing category values to 0 via (r.categories[key] || 0), which skews the
average; update getAggAvg to mirror averageGroup by extracting the values for
key from each run, converting to Number, and filtering out null/undefined/NaN
(or using Number.isFinite) before computing the mean so only real numeric values
are averaged; reference getAggAvg and averageGroup when making the change.

lighthouse-runner.mjs-1487-1488 (1)

1487-1488: ⚠️ Potential issue | 🟡 Minor

Math.min(...arr) / Math.max(...arr) will throw on large datasets.

allTimestamps grows with audit history; once it exceeds the engine's argument limit (~65-125k in V8 depending on stack), the spread invocation will throw RangeError: Maximum call stack size exceeded. Given this dashboard is intended to aggregate all historical runs, that ceiling is reachable. Use a linear reduce:

-    const fullMin = Math.min(...allTimestamps);
-    const fullMax = Math.max(...allTimestamps);
+    let fullMin = allTimestamps[0];
+    let fullMax = allTimestamps[0];
+    for (const t of allTimestamps) {
+      if (t < fullMin) fullMin = t;
+      if (t > fullMax) fullMax = t;
+    }

🤖 Prompt for AI Agents

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

In `@lighthouse-runner.mjs` around lines 1487 - 1488, The spread calls
Math.min(...allTimestamps) and Math.max(...allTimestamps) will throw for very
large allTimestamps arrays; replace them with a linear scan (e.g., using
Array.prototype.reduce or a simple for loop) to compute fullMin and fullMax so
it works for arbitrarily large datasets. In the block where fullMin and fullMax
are computed, iterate over allTimestamps (or reduce) to update min/max starting
from Infinity/-Infinity and handle the empty-array case (return undefined or a
sensible default) instead of using the spread. Ensure you update the variables
named fullMin and fullMax where they are used elsewhere in the file so behavior
remains identical.

lighthouse-runner.mjs-1837-1854 (1)

1837-1854: ⚠️ Potential issue | 🟡 Minor

Invalid HTML: <ul> cannot have <a> as direct children.

The <ul class="nav-links"> contains four <a> elements directly, with no wrapping <li>. Browsers render it, but it fails HTML validation and can break accessibility tooling/screen readers that rely on proper list semantics. Wrap each link in an <li>:

         <ul class="nav-links">
-            <a href="#overview" class="nav-item active">
-                <svg .../>
-                <span>Overview</span>
-            </a>
+            <li><a href="#overview" class="nav-item active">
+                <svg .../>
+                <span>Overview</span>
+            </a></li>
             ...
         </ul>

🤖 Prompt for AI Agents

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

In `@lighthouse-runner.mjs` around lines 1837 - 1854, The <ul class="nav-links">
uses <a> elements as direct children which is invalid HTML; wrap each anchor in
a list item to restore proper list semantics and accessibility. Update the
markup where <ul class="nav-links"> is defined (the nav-links block containing
the four <a href="#overview" class="nav-item">, <a href="#metrics"
class="nav-item">, <a href="#trends" class="nav-item">, and <a href="#runs"
class="nav-item">) so each anchor is enclosed within an <li> element, preserving
existing classes and SVG/span children.

lighthouse-runner.mjs-1683-1706 (1)

1683-1706: ⚠️ Potential issue | 🟡 Minor

buildMetricsRow displays 0 as "-" for every metric.

Same bug pattern as in lighthouse.ts log output: metrics.tbt ? metrics.tbt.toFixed(0) + "ms" : "-" treats 0 as missing. For CLS this is the most visible — a perfect cls: 0 score is rendered as "-", not "0.000". Use nullish checks:

-  const fcp = metrics.fcp ? (metrics.fcp / 1e3).toFixed(2) + "s" : "-";
-  const lcp = metrics.lcp ? (metrics.lcp / 1e3).toFixed(2) + "s" : "-";
-  const tbt = metrics.tbt ? metrics.tbt.toFixed(0) + "ms" : "-";
-  const cls = metrics.cls ? metrics.cls.toFixed(3) : "-";
-  const si = metrics.si ? (metrics.si / 1e3).toFixed(2) + "s" : "-";
-  const tti = metrics.tti ? (metrics.tti / 1e3).toFixed(2) + "s" : "-";
+  const fmt = (v, f) => (v != null && !Number.isNaN(v)) ? f(v) : "-";
+  const fcp = fmt(metrics.fcp, v => (v / 1e3).toFixed(2) + "s");
+  const lcp = fmt(metrics.lcp, v => (v / 1e3).toFixed(2) + "s");
+  const tbt = fmt(metrics.tbt, v => v.toFixed(0) + "ms");
+  const cls = fmt(metrics.cls, v => v.toFixed(3));
+  const si  = fmt(metrics.si,  v => (v / 1e3).toFixed(2) + "s");
+  const tti = fmt(metrics.tti, v => (v / 1e3).toFixed(2) + "s");

Also note getGrade at line 1690-1695 has the same if (!val) pitfall — a 0 value returns "none" instead of "good".

🤖 Prompt for AI Agents

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

In `@lighthouse-runner.mjs` around lines 1683 - 1706, The formatting and grading
logic in buildMetricsRow (and its inner getGrade) treats 0 as missing because it
uses truthy checks like `metrics.tbt ? ... : "-"` and `if (!val) return "none"`,
so change those checks to nullish checks (e.g., `val == null` or `val === null
|| val === undefined`) wherever you test metric presence (fcp, lcp, tbt, cls,
si, tti) and in getGrade; this ensures real zero values (like cls === 0) are
formatted and graded correctly while still treating null/undefined as missing.

package.json (1)

18-18: Declaring "type": "commonjs" while shipping .mjs is a minor smell.

The bundle output is ESM (format: 'esm' in build.js); the .mjs extension forces Node to load it as ESM regardless of the package-level "type", so it works by accident. If there is no real CJS consumer in this repo, switching to "type": "module" (and then you could also drop the .mjs extension in favor of .js) would make the intent explicit and remove the contradiction with tsconfig.json's "module": "CommonJS" — which is only used for tsc --noEmit type-checking, but is still confusing.

🤖 Prompt for AI Agents

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

In `@package.json` at line 18, The package.json currently declares "type":
"commonjs" which contradicts the bundle output (build.js uses format: 'esm') and
shipping .mjs files; change the package manifest to "type": "module" to make ESM
intent explicit, then update build outputs (drop the .mjs extension in the
bundler/output config so artifacts are .js) and reconcile build/tsconfig
settings by noting that tsconfig.json's "module": "CommonJS" is only for tsc
type-checking—if desired, update tsconfig.json "module" to "ESNext"/"ES2020" to
avoid confusion; ensure any code that imports these artifacts is compatible with
ESM.

src/data-extract.ts (1)

35-39: Defaulting missing category averages to 0 is a misleading score.

When vals.length === 0, the code emits avgCategories[key] = 0, which is indistinguishable from a real "failing" Lighthouse score and will render as such on the dashboard. Since IterationEntry.categories is Record<string, number> you cannot use null without a type change, but the practical choices are:

• Widen the type to Record<string, number | null> (consistent with metrics) and emit null for "no data"; let the template handle the missing case.
• Or simply skip the key rather than emitting a zero so the dashboard shows "n/a".

🤖 Prompt for AI Agents

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

In `@src/data-extract.ts` around lines 35 - 39, The current loop computes
avgCategories for each key but assigns 0 when vals.length === 0, which hides "no
data"; change the avgCategories type and assignment so missing averages are
represented explicitly: update the declaration avgCategories: Record<string,
number> to avgCategories: Record<string, number | null>, and inside the loop
where you compute vals set avgCategories[key] = vals.length ? vals.reduce((a, b)
=> a + b, 0) / vals.length : null; reference avgCategories, catKeys, group and
IterationEntry.categories when making these changes so downstream code/template
can display "n/a" for nulls.

src/utils.ts (1)

1-17: LGTM — minor note on MD5 and truncation.

The utilities are clean. A couple of optional considerations:

• getUrlHash uses MD5 truncated to 8 hex chars (~32-bit space). That is fine for non-security bucketing/labels, but beware of collisions if the URL set grows large. Consider sha1/sha256 or a longer slice if collisions would corrupt grouping downstream in data-extract.ts.
• The crypto import works, but since the output is bundled as ESM (esbuild format: 'esm'), prefer the node-prefixed form import { createHash } from 'node:crypto' for clearer Node builtin resolution.

🤖 Prompt for AI Agents

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

In `@src/utils.ts` around lines 1 - 17, Change the crypto import to use the Node
builtin prefix (import { createHash } from 'node:crypto') and update getUrlHash
to use a stronger hash and longer truncation to reduce collisions (e.g., use
'sha256' via createHash('sha256') and slice the hex to 16 chars instead of 8);
keep the same return type and behavior otherwise (function name getUrlHash) and
leave getUrlLabel/sleep untouched.

justfile (1)

31-33: Empty iterations default passes bare --run to the runner.

When invoked as just run with the default empty string, the command expands to node lighthouse-runner.mjs --run with nothing after. src/config.ts only parses --run when arr[i+1] !== undefined, so it silently falls back to config.iterations. Consider guarding the flag in the recipe to avoid surprising no-op behavior.

♻️ Proposed fix

 # Build and run with custom iterations
 run iterations='':
-    node build.js && node lighthouse-runner.mjs --run {{iterations}}
+    node build.js && node lighthouse-runner.mjs {{ if iterations != "" { "--run " + iterations } else { "" } }}

🤖 Prompt for AI Agents

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

In `@justfile` around lines 31 - 33, The justfile recipe "run" currently passes a
bare "--run" when iterations is the empty default, which causes src/config.ts's
parser (checking arr[i+1] !== undefined for "--run") to ignore it; update the
recipe so it only includes the "--run {{iterations}}" flag when iterations is
non-empty (or set a sensible default like '1') to avoid emitting a trailing flag
with no value; modify the "run" recipe in justfile to guard inclusion of the
flag (or change iterations='' to iterations='1') so lighthouse-runner.mjs
receives either no flag or a valid value that src/config.ts can parse.

src/lighthouse.ts (1)

158-158: Persisting the full stats file after every audit is I/O-heavy.

fs.writeFileSync(dataPath, JSON.stringify(statsData, null, 2)) is called on every single run inside a nested urls × emulations × iterations loop. As the dataset grows, each write re-serializes the entire accumulated history. Consider writing once per iteration pass (or only in main() after each round), keeping the per-run side effect only for the individual *.summary.json / LHR files.

🤖 Prompt for AI Agents

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

In `@src/lighthouse.ts` at line 158, The current code calls
fs.writeFileSync(dataPath, JSON.stringify(statsData, null, 2)) inside the nested
urls × emulations × iterations loop, causing expensive repeated full-file
serializations; move the full stats persist out of the inner loop so only
per-run artifacts (e.g., *.summary.json / LHR files) are written inside the loop
and the aggregated statsData is written once per iteration pass or at the end of
main(); locate the write call (fs.writeFileSync with dataPath and statsData) and
change its placement to after the iteration/emulation loops (or in main() after
each round) while keeping the existing per-run write behavior for summaries
intact.

src/dashboard/client.ts (1)

58-76: themeBtn.addEventListener will throw if the button is missing.

If the template ever changes or the script loads before the DOM button exists, themeBtn is null and this line throws, halting the rest of the dashboard init (all charts below never render). A simple null guard avoids cascading UI breakage.

Details

-themeBtn.addEventListener('click', () => setTheme(!body.classList.contains('light-mode')));
+themeBtn?.addEventListener('click', () => setTheme(!body.classList.contains('light-mode')));

🤖 Prompt for AI Agents

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

In `@src/dashboard/client.ts` around lines 58 - 76, The code currently assumes
themeBtn and themeText exist which can throw if DOM elements are missing; add
null guards: only call themeBtn.addEventListener when themeBtn is non-null, and
in setTheme guard use themeText (and body if needed) before accessing
innerText/classList; also ensure the initial localStorage check that calls
setTheme(true) can handle missing themeText by making setTheme tolerant of null
elements; keep references to themeBtn, setTheme, themeText, and
updateChartThemes when implementing these guards.

src/config.ts (1)

70-84: Silent fallback for invalid --run/--quality values.

• --run without a numeric value (e.g. --run --compress) silently falls back to config.iterations rather than erroring — easy to miss in CI.
• --quality 150 is silently discarded (stays at 30) with no warning.
• No validation of the urls.json payload: if it isn't an array of strings (loadedUrls: string[]), the runner will blindly iterate over unexpected values or for…of-throw on a non-iterable.

Minimal hardening would log a warning and exit non-zero on invalid input rather than pretending the flag wasn't there.

Details

   const loaded = JSON.parse(fs.readFileSync(urlsPath, 'utf8'));
-  loadedUrls = loaded;
+  if (!Array.isArray(loaded) || !loaded.every((u) => typeof u === 'string')) {
+    console.error('❌ Fatal: urls.json must be a JSON array of strings.');
+    process.exit(1);
+  }
+  loadedUrls = loaded;

🤖 Prompt for AI Agents

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

In `@src/config.ts` around lines 70 - 84, The CLI parsing currently silently
ignores invalid or missing values for flags in args (runIterations, quality,
compress) and the urls payload; update the parsing and loading logic to validate
inputs and fail loudly: when parsing args, detect cases where --run or --quality
are provided but the next token is missing, non-numeric, or out of range
(quality not in 1–100) and emit a clear console.error warning and
process.exit(1) instead of leaving defaults; likewise, when loading URLs
(loadedUrls), validate that the parsed value is an Array of strings (each item
typeof === 'string') and if not, log an error and exit non-zero. Reference the
existing symbols args, runIterations, quality, compress and loadedUrls to locate
where to add these checks. Ensure error messages include the offending
flag/value so CI logs are actionable.

src/dashboard/template.ts (1)

88-88: Unused reportPath constant.

Declared but never referenced; the actual write happens in src/dashboard/index.ts. Remove it.

Details

 export function generateHtml(statsData: StatsData): string {
-  const reportPath = `${config.baseDir}/visual-summary.html`;
   const urls = Object.keys(statsData.urls);

🤖 Prompt for AI Agents

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

In `@src/dashboard/template.ts` at line 88, Remove the unused constant reportPath
declared as reportPath = `${config.baseDir}/visual-summary.html` in the template
file (the const named reportPath in src/dashboard/template.ts); delete the
declaration line and ensure no other code in the file depends on it (the actual
file write is handled in src/dashboard/index.ts), leaving no leftover references
or unused imports.

src/dashboard/styles.ts (1)

586-588: Empty CSS rule.

The .noUi-handle-upper .noUi-tooltip rule block is empty — either add the intended positioning or remove the rule to keep the stylesheet clean.

🤖 Prompt for AI Agents

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

In `@src/dashboard/styles.ts` around lines 586 - 588, The CSS contains an empty
rule for .noUi-handle-upper .noUi-tooltip; either remove that empty selector
block to keep the stylesheet clean or implement the intended tooltip positioning
by adding the appropriate positioning properties (e.g., offsets/transform or
left/right) inside the .noUi-handle-upper .noUi-tooltip rule so the right-handle
tooltip is positioned to the left as intended.

build.js (1)

28-34: Remove unreachable error-check branch.

esbuild.build() rejects the promise on build errors and throws a BuildFailure exception; it never resolves with a BuildResult containing populated errors. Lines 29-32 are unreachable dead code — all build failures will be caught by the .catch() handler at line 37. Remove the redundant error-check and simplify to directly log success on successful build completion.

🤖 Prompt for AI Agents

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

In `@build.js` around lines 28 - 34, The code checks result.errors after calling
esbuild.build(buildOptions) but esbuild.build rejects with a BuildFailure on
errors and never returns a BuildResult with populated errors; remove the
unreachable branch that inspects result.errors and the result variable
altogether, so after await esbuild.build(buildOptions) simply log success (e.g.,
"Built successfully!") on completion and let the existing .catch() handler
handle failures; update any references to result to avoid unused variable
warnings.

lighthouse-runner.mjs (3)

44-46: mkdirSync runs as an import-time side effect.

runsDir is created eagerly during module initialization (init_config), before any argument parsing or command dispatch. That means even node lighthouse-runner.mjs --help-style invocations (or tests importing config) will silently create ./reports/runs/ on disk. Prefer lazy creation inside the code paths that actually write into the directory (Lighthouse runs; extract), and skip it for the --compress path which only reads existing files.

🤖 Prompt for AI Agents

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

In `@lighthouse-runner.mjs` around lines 44 - 46, The module currently creates the
runsDir at import time in init_config using fs.mkdirSync(runsDir, { recursive:
true }), causing side effects on simple imports; move that creation lazily into
the actual code paths that write files (the Lighthouse run handler function(s)
and the extract/write flow), and remove the import-time mkdirSync call so the
--compress/read-only path only reads existing files; search for runsDir,
init_config, and any functions that perform Lighthouse runs or extract/write
operations and call fs.mkdirSync there just before the first write.

---

69-69: MD5 truncated to 8 hex chars will collide in practice.

getUrlHash keeps only 8 hex chars (32 bits). By the birthday bound, collisions become likely at ~65k URLs — but more pragmatically, IDs of the form ${urlHash}-${mode}-${timestamp} (line 371/447) rely on urlHash being unique across the user's URL set. Two distinct URLs sharing a prefix can silently overwrite each other's JSON/summary files because filenames are deterministic per (urlHash, mode, timestamp). Either keep the full digest, or use 12-16 hex chars to get a comfortable safety margin. MD5 itself is fine here since this is non-cryptographic, but consider crypto.createHash("sha1") to quiet future FIPS warnings.

🤖 Prompt for AI Agents

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

In `@lighthouse-runner.mjs` at line 69, getUrlHash currently truncates the MD5
digest to 8 hex chars causing likely collisions; update getUrlHash to return a
longer hex slice (recommend 12–16 chars) or the full digest to reduce collision
risk, and if desired switch the hash algorithm to "sha1" to avoid potential FIPS
warnings—then ensure any code that builds filenames using the urlHash (e.g., the
`${urlHash}-${mode}-${timestamp}` pattern) continues to work with the longer
hash.

---

1-8: Committing a bundled output file is fragile.

This file carries all of esbuild's __esm/__commonJS scaffolding and re-declares every src/* module inline. Any bug fix must be made in the TypeScript sources and re-bundled, but the diff is being reviewed against the generated artifact, which makes it easy to drift, hard to diff meaningfully, and prone to overwriting fixes on the next rebuild.

Consider either:

• Keeping lighthouse-runner.mjs out of the repo (generate it at install/release time, e.g. via a prepare/build npm script) and distributing via an npm package or a release tarball, or
• At minimum, adding a note (and a CI check) that this file must only be regenerated via ./build.js and never hand-edited.

All subsequent findings apply to the corresponding TypeScript sources; please fix there and re-bundle.

🤖 Prompt for AI Agents

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

In `@lighthouse-runner.mjs` around lines 1 - 8, The committed bundle contains
build scaffolding (__esm, __commonJS) and inlined compiled modules which should
not be edited directly; instead, update the original TypeScript sources and
re-run the bundler. Remove manual edits to the generated symbols (__esm,
__commonJS) and ensure the build pipeline regenerates lighthouse-runner.mjs via
the proper build step (e.g., ./build.js or the project's prepare/build npm
script); add a repository note and CI check enforcing that lighthouse-runner.mjs
is only generated by the build script, not hand-edited.

src/dashboard/template.ts (2)

88-88: Unused reportPath.

reportPath is computed but never returned or referenced; writing the HTML to disk is handled by src/dashboard/index.ts. Safe to delete.

♻️ Proposed change

 export function generateHtml(statsData: StatsData): string {
-  const reportPath = `${config.baseDir}/visual-summary.html`;
   const urls = Object.keys(statsData.urls);

🤖 Prompt for AI Agents

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

In `@src/dashboard/template.ts` at line 88, The variable reportPath declared as
const reportPath = `${config.baseDir}/visual-summary.html` in template.ts is
unused and should be removed; delete the reportPath declaration to eliminate
dead code and ensure no other code references it (verify that only reportPath is
removed and not any config usage), since writing the HTML is handled elsewhere
(src/dashboard/index.ts) and nothing returns or uses reportPath.

---

13-22: calcAvg is dead code.

Only calcWeightedAvg is invoked by generateHtml. The simple calcAvg helper has no callers and can be removed.

🤖 Prompt for AI Agents

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

In `@src/dashboard/template.ts` around lines 13 - 22, The calcAvg function is
unused dead code; remove the calcAvg function declaration from template.ts and
any related exports or references so only calcWeightedAvg remains used by
generateHtml, ensuring no remaining imports or tests refer to calcAvg and
running tests/build to confirm no references remain.

build.js (1)

15-15: Sourcemap generation disabled, but npm run clean removes lighthouse-runner.mjs.map.

The clean script in package.json (line 13) removes a .map file that this build never produces. Either enable sourcemap (recommended for distributable binaries / stack traces) or drop the .map entry from the clean script.

🤖 Prompt for AI Agents

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

In `@build.js` at line 15, The build config currently sets sourcemap: false while
the npm "clean" script deletes lighthouse-runner.mjs.map; either enable source
maps by setting the sourcemap option to true in the build config (so the
produced artifact matches the clean script) or remove the .map filename from the
"clean" script in package.json; update the sourcemap setting (symbol: sourcemap)
or adjust the "clean" npm script to stop removing lighthouse-runner.mjs.map so
build and cleanup are consistent.

src/lighthouse.ts (1)

49-51: Unused local isThrottled.

isThrottled is assigned but never read. Either drop it or use it in the throttlingMethod branch for readability.

♻️ Proposed change

   const isDesktop = mode === 'desktop';
   const isWifi = mode === 'mobile-wifi';
-  const isThrottled = mode === 'mobile-4g';

🤖 Prompt for AI Agents

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

In `@src/lighthouse.ts` around lines 49 - 51, The local variable isThrottled is
declared but never used; either remove its declaration or use it to set
throttlingMethod for clarity. Locate the mode checks around "const isDesktop =
mode === 'desktop'; const isWifi = mode === 'mobile-wifi'; const isThrottled =
mode === 'mobile-4g';" and either delete the isThrottled line, or reference
isThrottled when computing throttlingMethod (the branch that currently uses
mode) so throttlingMethod uses isThrottled instead of directly comparing mode
for readability and to eliminate the unused variable warning.

src/data-extract.ts (1)

132-132: Dead computation: urlHash is never used.

urlHash is computed on every iteration but never referenced. getUrlHash is imported only for this unused assignment — both can be removed.

♻️ Proposed change

-import { getUrlHash, getUrlLabel, RunData, StatsData, IterationEntry } from './utils';
+import { getUrlLabel, RunData, StatsData, IterationEntry } from './utils';

       const url = summary.url || '';
-      const urlHash = getUrlHash(url);
       const mode: string = summary.mode || (file.includes('-desktop-') ? 'desktop' : 'mobile');

🤖 Prompt for AI Agents

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

In `@src/data-extract.ts` at line 132, Remove the dead computation of urlHash by
deleting the unused assignment "const urlHash = getUrlHash(url)" in
src/data-extract.ts and eliminating the now-unused getUrlHash import; before
removing the import, search for other uses of getUrlHash in the module to avoid
breaking consumers, and if getUrlHash has side effects ensure those are no
longer required or refactored into an explicit call where needed.

lighthouse-runner.mjs (1)

1-2126: Bundled artifact committed to git—recommend gitignoring and producing via CI only.

lighthouse-runner.mjs is a build output tracked in git with no .gitignore entry. Since the release workflow already runs npm run build:minify to generate it, committing this artifact creates drift risk: every source fix requires a matching rebuild to keep the bundled version in sync, and reviewers must verify both the source and artifact. Instead, add to .gitignore and let CI produce and release it.

Also, fixDoublePrefixesInReport (src/compression.ts) is exported but never called in the codebase. Either wire it into the compress pipeline or remove the unused export.

🤖 Prompt for AI Agents

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

In `@lighthouse-runner.mjs` around lines 1 - 2126, The repo contains a committed
build artifact (lighthouse-runner.mjs) that should be gitignored and generated
by CI; remove the tracked file from the repo (git rm --cached), add its filename
or the build output directory to .gitignore, and update CI/build docs to ensure
npm run build:minify produces it during release; additionally, address the
unused export fixDoublePrefixesInReport from src/compression.ts by either
removing the exported function if not needed or integrating it into the
compression pipeline (call it from compressReportImages or compressAllReports)
so it is exercised by the existing compressAllReports flow.

package.json (1)

13-13: clean script is not cross-platform.

rm -f is a POSIX command — it will fail on Windows CMD and plain PowerShell. For cross-platform compatibility, replace with rimraf:

♻️ Proposed change

-    "clean": "rm -f lighthouse-runner.mjs lighthouse-runner.mjs.map"
+    "clean": "rimraf lighthouse-runner.mjs lighthouse-runner.mjs.map"

Then add rimraf to devDependencies. Note: lighthouse-runner.mjs.map is never generated because build.js sets sourcemap: false.

🤖 Prompt for AI Agents

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

In `@package.json` at line 13, The package.json "clean" npm script currently uses
the POSIX command "rm -f" which breaks on Windows; replace the "clean" script
entry named "clean" to use a cross-platform tool (e.g., "rimraf") and add
"rimraf" to devDependencies, and while updating the script remove or stop
referencing "lighthouse-runner.mjs.map" if your build.js sets sourcemap:false so
that the script only deletes actually generated artifacts (e.g., target files
such as "lighthouse-runner.mjs").

Fix all unresolved CodeRabbit comments on this PR:

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

src/dashboard/template.ts (3)

88-88: Unused reportPath — dead code.

reportPath is computed here but never referenced in generateHtml. The actual write is done in src/dashboard/index.ts (generateVisualReport). Remove this line to avoid confusion.

♻️ Proposed fix

 export function generateHtml(statsData: StatsData): string {
-  const reportPath = `${config.baseDir}/visual-summary.html`;
   const urls = Object.keys(statsData.urls);

🤖 Prompt for AI Agents

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

In `@src/dashboard/template.ts` at line 88, Remove the unused dead variable
reportPath (const reportPath = `${config.baseDir}/visual-summary.html`) from
src/dashboard/template.ts: locate the declaration near the generateHtml function
and delete it, since the actual write path is handled by generateVisualReport in
src/dashboard/index.ts and reportPath is never referenced.

---

10-22: calcAvg is unused.

calcAvg is defined but never called in this module (only calcWeightedAvg is used downstream). Either remove it or, if it's intended for future use, mark/export it explicitly.

🤖 Prompt for AI Agents

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

In `@src/dashboard/template.ts` around lines 10 - 22, calcAvg is defined but never
used; either remove the function or export it if it's intended for
external/future use. Find the calcAvg function in this file and either delete
its definition or add an explicit export (e.g., export function calcAvg(...)) so
it is available to downstream modules; ensure no other internal references (like
calcWeightedAvg) depend on it before removal. If exporting, also update any
index exports or documentation as needed to expose calcAvg to consumers.

---

302-307: overallModePerf[mode].toFixed(0) can read undefined if a mode has no averaged data.

allModes is derived from statsData.urls[...].modes keys so every mode in the list has at least one URL entry — however, if modeDataAvg[mode].flat() is empty (e.g., all arrays are [] because only modesRaw was populated for that mode), calcWeightedAvg returns 0 and this is safe. Still, consider guarding against missing keys defensively ((overallModePerf[mode] ?? 0).toFixed(0)) since this HTML is rendered from potentially-stale persistent_stats.json.

🤖 Prompt for AI Agents

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

In `@src/dashboard/template.ts` around lines 302 - 307, The template can throw
when overallModePerf[mode] is undefined; update the rendering to defensively
default missing mode values to 0 before formatting. Locate the map over allModes
in template.ts that uses overallModePerf[mode].toFixed(0) and change the
expression to use a nullish/default guard (e.g., overallModePerf[mode] ?? 0) so
undefined entries from stale persistent_stats.json or missing calcWeightedAvg
results are rendered as 0; keep the rest of the card markup and references to
modeDataAvg/calcWeightedAvg intact.

src/config.ts (2)

25-39: process.exit(1) at module load is an ergonomic hazard.

Any importer of ./config — including tests, IDE type-checks, or downstream tools that simply want a type — will hard-kill the process if urls.json is missing. Prefer throwing an error and letting the CLI entrypoint translate it into a friendly message + process.exit:

♻️ Proposed fix

   try {
     loadedUrls = JSON.parse(fs.readFileSync('./urls.json', 'utf8'));
   } catch {
-    console.error(
-      '❌ Fatal: Could not load urls.json. Please create it with an array of URLs.',
-    );
-    process.exit(1);
+    throw new Error('Could not load urls.json. Please create it with an array of URLs.');
   }

Have the top-level entry (src/index.ts) catch and translate to console.error + exit(1).

🤖 Prompt for AI Agents

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

In `@src/config.ts` around lines 25 - 39, The module-level config loader currently
calls process.exit(1) when urls.json cannot be read; change this to throw a
descriptive Error instead so importing modules (and tests) aren't killed: in the
config loader that builds loadedUrls (refer to variables loadedUrls and urlsPath
and the try/catch blocks) remove process.exit and rethrow or throw a new Error
with a clear message about missing/invalid urls.json; then update the
application entrypoint (e.g., src/index.ts) to catch that Error and perform
console.error plus process.exit(1) there so only the CLI terminates while
imports remain safe.

---

41-50: Hardcoded runtime config discourages reuse.

iterations, delay, baseDir, reportSubDir, dataFile, emulations are all baked in. Users cannot change them via env or a config file without editing source and rebuilding. Consider sourcing overridable fields from env vars (LHR_ITERATIONS, LHR_BASE_DIR, etc.) or a lighthouse-runner.config.json alongside urls.json.

🤖 Prompt for AI Agents

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

In `@src/config.ts` around lines 41 - 50, The config object currently hardcodes
runtime values (fields iterations, delay, baseDir, reportSubDir, dataFile,
emulations, chromePath) which prevents overrides; modify the export const config
in src/config.ts to load these fields from environment variables (e.g.,
LHR_ITERATIONS, LHR_DELAY, LHR_BASE_DIR, LHR_REPORT_SUBDIR, LHR_DATA_FILE,
LHR_EMULATIONS, CHROME_PATH) with sensible defaults falling back to the existing
literals, and add support to read an optional lighthouse-runner.config.json
(merging/overriding the env/defaults) so users can override values without
rebuilding; ensure the unique symbol config and loadedUrls remain used and
validate/parses emulations (CSV/JSON) and numeric fields (iterations, delay)
when constructing the final Config object.

lighthouse-runner.mjs (1)

1-10: lighthouse-runner.mjs is a build artifact — consider not committing it.

This entire ~2000-line file is the esbuild bundle of src/*.ts (exact same sources, just mangled/reconciled). Committing it duplicates every source change in the diff, causes noisy merge conflicts, and creates risk that the .mjs drifts out of sync with src/. The release workflow already builds it on demand (per the PR description) and package.json has build / clean scripts for it.

Recommend:

1. Add lighthouse-runner.mjs (and *.mjs.map) to .gitignore.
2. Remove it from version control.
3. Let the release workflow be the sole producer.

If you deliberately want to ship a prebuilt binary for npm consumers (since main points at this file), consider using prepublishOnly to build on publish instead of committing the artifact.

🤖 Prompt for AI Agents

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

In `@lighthouse-runner.mjs` around lines 1 - 10, The committed
lighthouse-runner.mjs file is a generated esbuild bundle (and likely its
.mjs.map) that should not be tracked; remove lighthouse-runner.mjs (and any
*.mjs.map) from the repository, add patterns for them to .gitignore, and update
the repo so the file is deleted from version control (git rm --cached or
equivalent) so the release workflow remains the sole producer; if you intend to
ship a prebuilt artifact for npm consumers instead of relying on the CI build,
configure package.json to build on publish (e.g., add a prepublishOnly script)
and ensure package.json "main" continues to point to the built artifact.

src/lighthouse.ts (2)

82-87: || 0 on category scores misses the "audit not run" case.

Unlike the keyMetrics block below (which correctly switched to ??), safeGet(lhr.categories.xxx, 'score') || 0 still uses ||. A Lighthouse score of exactly 0 (category completely failed) coalesces to 0 too so the user-visible behavior is identical — but if score is ever null (category present but not scored) you also get 0, which is a legitimate Lighthouse score. For consistency with lines 90-99, switch to ?? 0, or propagate null and render it as "-" like the metrics do.

🤖 Prompt for AI Agents

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

In `@src/lighthouse.ts` around lines 82 - 87, The categories object is using
fallback with || which conflates a real 0 score and a null "not scored" value;
update the expressions in the categories block (the keys performance,
accessibility, 'best-practices', seo using safeGet(lhr.categories.X, 'score'))
to use the nullish-coalescing operator (?? 0) or alternatively propagate null
and render it as "-" to match the keyMetrics behavior — e.g., replace each
"safeGet(..., 'score') || 0" with "safeGet(..., 'score') ?? 0" (or return null
and handle display downstream).

---

49-51: isThrottled is computed but unused.

isThrottled = mode === 'mobile-4g' is assigned but never referenced (the throttlingMethod ternary uses isDesktop || isWifi directly). Harmless but confusing — drop it or use it in place of the ternary for readability.

🤖 Prompt for AI Agents

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

In `@src/lighthouse.ts` around lines 49 - 51, The variable isThrottled (const
isThrottled = mode === 'mobile-4g') is defined but not used; either remove that
unused declaration or use it to make the throttlingMethod decision clearer:
replace the inline ternary that now checks (isDesktop || isWifi) with a
condition that uses isThrottled (or refactor to use all three flags
consistently: isDesktop, isWifi, isThrottled) in the throttlingMethod
computation so the intent is explicit and no unused variable remains (look for
throttlingMethod and the consts isDesktop/isWifi/isThrottled in
src/lighthouse.ts).

package.json (2)

1-28: Missing engines field.

With target: 'node24' in build.js, consumers or CI on older Node versions (e.g., Node 20 LTS) may silently get runtime errors on transpiled-to-node24 syntax (e.g., newer syntactic features, Promise helpers). Add an engines entry so npm/pnpm warn on install:

♻️ Proposed fix

   "license": "ISC",
   "type": "commonjs",
+  "engines": {
+    "node": ">=24"
+  },
   "dependencies": {

🤖 Prompt for AI Agents

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

In `@package.json` around lines 1 - 28, The package.json is missing an engines
field while build.js targets 'node24', so add an "engines" entry to package.json
(e.g., set node to a minimum version compatible with build.js output such as
">=24.0.0") to surface warnings on install; update package.json's top-level
metadata (near name/version/description) to include the engines object so CI and
consumers running older Node (e.g., Node 20) get a clear warning.

---

23-27: Update @types/node to match runtime target.

build.js targets node24 but @types/node is pinned to ^20.10.0, which lags significantly behind. The typecheck script will miss newly-added Node 22/24 APIs and provide inaccurate type information. Bump @types/node to ^25 (currently at 25.6.0) to match the runtime target.

🤖 Prompt for AI Agents

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

In `@package.json` around lines 23 - 27, The `@types/node` devDependency is pinned
to ^20.10.0 while build.js targets node24, causing typechecks (the typecheck
script) to miss Node 22/24 APIs; update the devDependencies entry for
"@types/node" to a Node 25 range (e.g., ^25 or the current 25.6.0) in
package.json so TypeScript sees the correct runtime types and rerun the
typecheck/build to verify no type errors.

build.js (1)

22-35: Watch mode never reports build errors.

In --watch mode, any initial or subsequent rebuild errors will be emitted by esbuild on its internal logger but this script never inspects them and never exits non-zero. If a CI job is somehow invoked with --watch, or if a developer relies on process exit codes, silent failures are possible. Also note result.errors.length > 0 is largely redundant because esbuild.build throws on error by default (caught below), so the branch is effectively dead code. Consider replacing with a thrown-error assertion or removing the manual check.

🤖 Prompt for AI Agents

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

In `@build.js` around lines 22 - 35, The watch branch never inspects rebuild
errors and never exits non-zero; change the watch logic in build() to use
esbuild's watch API with an onRebuild handler (e.g., call
esbuild.build({...buildOptions, watch:{onRebuild(error,result){ if (error) {
console.error('Build error:', error); process.exit(1); } else {
console.log('Rebuilt successfully'); }}}})) instead of relying on
esbuild.context(). Also remove the redundant manual check of result.errors after
esbuild.build (or replace it with proper try/catch that logs the thrown error
and exits non-zero) so initial build failures are surfaced.

src/dashboard/client.ts (3)

349-352: Math.min(...allTimestamps) / Math.max(...) is fragile for large trend datasets.

Spread into Math.min/Math.max will blow the call stack at engine‑dependent argument limits (≈100k entries in most browsers). Unlikely today, but this array grows with runs × urls × modes, so a simple reducer avoids the ceiling entirely.

♻️ Safer reduction

-    const fullMin = Math.min(...allTimestamps);
-    const fullMax = Math.max(...allTimestamps);
+    let fullMin = Infinity, fullMax = -Infinity;
+    for (const t of allTimestamps) {
+      if (t < fullMin) fullMin = t;
+      if (t > fullMax) fullMax = t;
+    }

🤖 Prompt for AI Agents

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

In `@src/dashboard/client.ts` around lines 349 - 352, The current computation of
fullMin/fullMax uses Math.min(...allTimestamps) and Math.max(...allTimestamps),
which can exceed argument limits for large arrays; replace these spread calls
with safe reductions over allTimestamps (e.g., reduce to compute min and max) so
the engine never receives a giant argument list; update the block that creates
allTimestamps and the assignments to fullMin and fullMax (referenced as
trendData, allTimestamps, fullMin, fullMax) to use Array.prototype.reduce to
derive the minimum and maximum values instead of using spread with
Math.min/Math.max.

---

381-389: Nit: use the values argument instead of re‑querying the slider.

noUiSlider passes the numeric values (already via your format.to) to the update handler. Re‑reading via timelineSlider.get()[0] is redundant and forces an extra object allocation per drag tick. Also, handle is unused.

♻️ Proposed tweak

-    timelineSlider.on('update', (values, handle) => {
+    timelineSlider.on('update', (values) => {
       if (isUpdatingSlider) return;
       isUpdatingChart = true;
-      const min = timelineSlider.get()[0];
-      const max = timelineSlider.get()[1];
+      const min = Number(values[0]);
+      const max = Number(values[1]);
       trendsChart.zoomScale('x', { min, max });
       updateVisibleLabels(min, max);
       isUpdatingChart = false;
     });

🤖 Prompt for AI Agents

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

In `@src/dashboard/client.ts` around lines 381 - 389, The update handler for
timelineSlider re-queries the slider instead of using the provided values and
also ignores the handle param; change the handler registered in
timelineSlider.on('update', ...) to early-return on isUpdatingSlider, set
isUpdatingChart, read min = values[0] and max = values[1] (using the values
argument passed into the handler), call trendsChart.zoomScale('x', { min, max })
and updateVisibleLabels(min, max), then clear isUpdatingChart; remove the
redundant timelineSlider.get() calls and the unused handle variable to avoid
extra allocations per tick.

---

391-412: Move onZoom/onPan callbacks into initial chart options — assigning them after construction is not supported by chartjs-plugin-zoom.

The callbacks are assigned to trendsChart.options.plugins.zoom.zoom.onZoom and ...pan.onPan after new Chart(...) completes (lines 391–412), with no trendsChart.update() call. With chartjs-plugin-zoom v2.2.0, callbacks must be in the initial options object at chart creation; modifying them after construction is not directly supported and may not be picked up by the plugin. If they silently fail, the slider↔chart sync becomes one-way (slider → chart only).

Move these callbacks into the options.plugins.zoom object during chart initialization at line 246, alongside zoom.wheel, zoom.pinch, pan.enabled, etc.

🤖 Prompt for AI Agents

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

In `@src/dashboard/client.ts` around lines 391 - 412, The onZoom/onPan handlers
are being assigned to trendsChart.options.plugins.zoom.zoom.onZoom and
trendsChart.options.plugins.zoom.pan.onPan after chart construction which
chartjs-plugin-zoom doesn't pick up; move these callbacks into the initial
options.plugins.zoom object passed to new Chart(...) (alongside existing
zoom.wheel, zoom.pinch and pan.enabled) so they are registered at creation,
keeping their logic that checks isUpdatingChart/isUpdatingSlider, sets
isUpdatingSlider, reads chart.scales.x.min/max, updates
timelineSlider.set([min,max], true, true) if timelineSlider, calls
updateVisibleLabels(min, max), and resets isUpdatingSlider.

Fix all unresolved CodeRabbit comments on this PR:

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

lighthouse-runner.mjs (1)

469-469: ⚠️ Potential issue | 🟠 Major

jsExecTime is wired to the wrong Lighthouse audit.

third-party-summary's numericValue represents main-thread blocking time caused by third-party code, not overall JavaScript execution time. The correct audit for main-thread JS execution time (parse + compile + execute) is bootup-time.

🔧 Fix (in `src/lighthouse.ts`, then rebuild)

-      jsExecTime: safeGet(lhr.audits["third-party-summary"], "numericValue") ?? null,
+      jsExecTime: safeGet(lhr.audits["bootup-time"], "numericValue") ?? null,

🤖 Prompt for AI Agents

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

In `@lighthouse-runner.mjs` at line 469, jsExecTime is reading the wrong audit;
replace the reference to lhr.audits["third-party-summary"] with
lhr.audits["bootup-time"] so jsExecTime uses the bootup-time audit's
numericValue (e.g., jsExecTime: safeGet(lhr.audits["bootup-time"],
"numericValue") ?? null), keeping the same safeGet fallback pattern and ensuring
the change is applied where jsExecTime is set.

src/data-extract.ts (1)

35-44: Minor: use avgValues consistently for categories.

Lines 37–38 reimplement the same null/NaN-aware averaging that avgValues already provides (used for metrics on line 43). Reusing it keeps the two code paths in sync if the filter ever changes (e.g., clamping 0–100, treating NaN differently).

♻️ Proposed tweak

   const avgCategories: Record<string, number | null> = {};
   for (const key of catKeys) {
-    const vals = group.map((e) => e.categories[key]).filter((v) => v != null && !isNaN(v as number)) as number[];
-    avgCategories[key] = vals.length ? vals.reduce((a, b) => a + b, 0) / vals.length : null;
+    avgCategories[key] = avgValues(group.map((e) => e.categories[key]));
   }

🤖 Prompt for AI Agents

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

In `@src/data-extract.ts` around lines 35 - 44, The category averaging block
duplicates null/NaN-aware logic; replace the manual loop that builds
avgCategories using the ad-hoc filter with calls to avgValues like the metrics
path does: for each key in catKeys call avgValues(group.map(e =>
e.categories[key])) and assign to avgCategories[key] (preserving the
Record<string, number | null> type), so categories and metrics use the same
null/NaN handling implemented by avgValues.

src/compression.ts (1)

88-95: Skip the rewrite when nothing was replaced.

content.replace(imgRegex, …) always runs and fs.writeFileSync(filePath, content) is called even when replacements is empty / imagesProcessed === 0. Rewriting an untouched multi‑MB HTML report is wasteful I/O and also pointlessly bumps the file’s mtime on repeated --compress runs over already-processed reports.

♻️ Suggested tweak

-  content = content.replace(imgRegex, (m) => replacements.get(m) ?? m);
-
-  fs.writeFileSync(filePath, content);
-  const compressedSize = Buffer.byteLength(content, 'utf8');
+  if (replacements.size > 0) {
+    content = content.replace(imgRegex, (m) => replacements.get(m) ?? m);
+    fs.writeFileSync(filePath, content);
+  }
+  const compressedSize = Buffer.byteLength(content, 'utf8');

🤖 Prompt for AI Agents

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

In `@src/compression.ts` around lines 88 - 95, The current code always runs
content = content.replace(imgRegex, ...) and fs.writeFileSync(filePath, content)
even when no images were replaced; modify the logic in the function that uses
imgRegex, replacements and imagesProcessed so that if imagesProcessed === 0 (or
replacements.size === 0) you skip running the write and avoid updating the file
mtime—return { originalSize, compressedSize: originalSize, imagesProcessed: 0 }
(or compute compressedSize only when writing) and only call
fs.writeFileSync(filePath, content) when at least one replacement occurred; keep
existing return shape and use the same variables (content, imgRegex,
replacements, imagesProcessed, filePath) to gate the write.

src/index-logic.ts (1)

8-42: LGTM.

Flow control (compress short-circuit → optional iterations → extract → visualize) is clear, and runLighthouse handles its own failures so a single bad URL doesn't abort the round-robin.

One tiny nit you can ignore: the await sleep(config.delay * 1000) on line 32 also fires after the very last (url, mode) combination of the last pass, adding a gratuitous delay before extraction/dashboard rendering. Easy fix if you care: skip the sleep when i === iterations && url === lastUrl && mode === lastMode.

🤖 Prompt for AI Agents

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

In `@src/index-logic.ts` around lines 8 - 42, The final sleep after the last run
causes an unnecessary delay; update the main() loop so it skips calling await
sleep(config.delay * 1000) when about to finish all work (i.e., when i ===
iterations and the current url is the last entry of config.urls and the current
mode is the last entry of config.emulations). Modify the nested loops in main()
to compute or check the lastUrl/lastMode (from config.urls and
config.emulations) and only call sleep(...) when not on that final combination;
keep runLighthouse(...) behavior unchanged.

src/config.ts (1)

54-56: LHR_EMULATIONS values are not validated against the Config.emulations union.

process.env.LHR_EMULATIONS.split(',').map(s => s.trim()) returns arbitrary strings, and the as any cast silently bypasses the narrow 'mobile-4g' | 'mobile-wifi' | 'desktop' union. A typo like LHR_EMULATIONS=desktops,mobile-4g passes through and later hits src/lighthouse.ts where isDesktop/isWifi/isThrottled are all false, producing a misconfigured audit rather than a clear error.

Consider filtering to known values (and erroring on an empty result) instead of casting away the type.

🛠️ Sketch

-  emulations: (process.env.LHR_EMULATIONS 
-    ? process.env.LHR_EMULATIONS.split(',').map(s => s.trim()) 
-    : fileConfig.emulations || ['mobile-4g', 'mobile-wifi', 'desktop']) as any,
+  emulations: (() => {
+    const allowed = ['mobile-4g', 'mobile-wifi', 'desktop'] as const;
+    const raw = process.env.LHR_EMULATIONS
+      ? process.env.LHR_EMULATIONS.split(',').map((s) => s.trim())
+      : fileConfig.emulations || allowed;
+    const filtered = raw.filter((m: string): m is typeof allowed[number] =>
+      (allowed as readonly string[]).includes(m),
+    );
+    if (filtered.length === 0) {
+      throw new Error(`No valid emulations. Allowed: ${allowed.join(', ')}`);
+    }
+    return filtered;
+  })(),

🤖 Prompt for AI Agents

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

In `@src/config.ts` around lines 54 - 56, The emulations assignment accepts
arbitrary strings and uses "as any", bypassing the Config.emulations union;
update the logic that builds emulations from process.env.LHR_EMULATIONS to
split/trim then filter against the allowed set
['mobile-4g','mobile-wifi','desktop'] (reference the emulations property and
LHR_EMULATIONS env var), remove the unsafe "as any" cast, and if the filtered
result is empty (and no fileConfig.emulations present) throw a clear error so
mis-typed values fail fast rather than producing a misconfigured run used later
by lighthouse checks (e.g., isDesktop/isWifi/isThrottled).

src/dashboard/client.ts (1)

505-522: inp is listed in metricLabels but never produced upstream.

metricLabels includes inp: 'INP', but src/lighthouse.ts doesn’t extract INP into summary.metrics, so the filter filter(([k]) => metricLabels[k]) will never surface it, and the thresholds ternary on line 515 has no inp branch either. Either add an inp audit extraction in lighthouse.ts (Lighthouse exposes experimental-interaction-to-next-paint) and a threshold case here, or drop the inp entry to avoid confusion.

🤖 Prompt for AI Agents

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

In `@src/dashboard/client.ts` around lines 505 - 522, metricLabels includes "inp"
but summary.metrics never contains INP (so the UI never shows it) and the
thresholds branch in the metrics rendering (in the metrics.map block) has no
case for "inp"; either (A) add INP extraction in src/lighthouse.ts (pull the
"experimental-interaction-to-next-paint" audit into summary.metrics as "inp")
and then add an INP thresholds branch in the metrics rendering logic (treating
it like other timing metrics with appropriate ms-to-s conversion and
thresholds), or (B) remove "inp: 'INP'" from the metricLabels object to avoid
advertising a metric that isn’t produced; update the rendering logic (the
metrics.map block that references metricLabels and the thresholds ternary)
accordingly to keep branches consistent.

src/dashboard/template.ts (1)

13-22: Remove the unused calcAvg function.

calcAvg is exported but has no callers in the codebase — only calcWeightedAvg is used for aggregations. This is dead code that can be safely removed to reduce the module size.

🤖 Prompt for AI Agents

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

In `@src/dashboard/template.ts` around lines 13 - 22, Remove the dead exported
function calcAvg from the module: locate the exported function named calcAvg in
template.ts and delete its declaration and export so it is no longer part of the
public API; ensure no other code references calcAvg (the review states only
calcWeightedAvg is used) and run a quick compile/typecheck to verify there are
no remaining references.

Fix all unresolved CodeRabbit comments on this PR:

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

package.json (3)

16-16: clean script is not cross-platform.

rm -f fails on Windows (cmd/PowerShell) where contributors may run npm run clean. Consider a Node-based approach or rimraf to keep the script portable.

♻️ Proposed change

-    "clean": "rm -f lighthouse-runner.mjs lighthouse-runner.mjs.map",
+    "clean": "node -e \"require('node:fs').rmSync('lighthouse-runner.mjs',{force:true});require('node:fs').rmSync('lighthouse-runner.mjs.map',{force:true})\"",

🤖 Prompt for AI Agents

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

In `@package.json` at line 16, The "clean" npm script uses a POSIX-only command
("rm -f") which breaks on Windows; replace it with a cross-platform approach by
updating the "clean" script in package.json (the "clean" script entry) to use a
portable tool such as "rimraf lighthouse-runner.mjs lighthouse-runner.mjs.map"
and add rimraf as a devDependency (or alternatively implement a small Node-based
script and point the "clean" script to it), ensuring npm scripts and
package.json dependencies are updated accordingly.

---

8-17: Consider declaring bin and files for a CLI-oriented package.

build.js prepends a #!/usr/bin/env node shebang to lighthouse-runner.mjs, strongly suggesting CLI usage — but no bin entry is exposed, so npx lighthouse-runner / global install won't work. Additionally, without a files whitelist, npm publish will ship src/, build.js, TS configs, etc., bloating the tarball. Both are optional, but worth considering before the next publish (prepublishOnly already guarantees a fresh build).

♻️ Proposed change

   "main": "lighthouse-runner.mjs",
+  "bin": {
+    "lighthouse-runner": "lighthouse-runner.mjs"
+  },
+  "files": [
+    "lighthouse-runner.mjs"
+  ],
   "scripts": {

🤖 Prompt for AI Agents

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

In `@package.json` around lines 8 - 17, Add a "bin" entry and a "files" whitelist
to package.json so the package installs as a CLI and only ships the built
artifacts: add a "bin" mapping pointing the CLI name (e.g., "lighthouse-runner")
to "lighthouse-runner.mjs" (ensuring build.js still writes the shebang and
output file is executable) and add a "files" array that includes the runtime
build outputs (e.g., "lighthouse-runner.mjs", "lighthouse-runner.mjs.map") plus
relevant docs like "README.md" to avoid publishing source/fixtures/tsconfigs;
keep "prepublishOnly" as-is to ensure build before publish.

---

22-22: "type": "commonjs" contradicts the ESM build output.

build.js emits format: 'esm' and the TS sources use ESM import/export. The .mjs extension currently forces ESM regardless of this field, so things work today — but the declaration is misleading and brittle: any future change to emit .js (or a consumer inspecting type) will silently flip the artifact to CJS and break execution. Recommend aligning the field with the actual module system.

♻️ Proposed change

-  "type": "commonjs",
+  "type": "module",

Note: build.js uses require() and would need to be renamed to build.cjs or converted to ESM if this change is made.

🤖 Prompt for AI Agents

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

In `@package.json` at line 22, package.json declares "type": "commonjs" which
contradicts your ESM outputs (build.js sets format: 'esm' and TS uses
import/export); change the package.json "type" field to "module" to align with
emitted ESM artifacts and avoid silently shipping CJS. After changing to
"module", update the build script: either rename the existing build.js (which
uses require()) to build.cjs or rewrite build.js as ESM (use import instead of
require) so the build tooling remains runnable under the new package "type".
Ensure references to build.js in npm scripts or CI are updated to the new
filename or ESM module entry.

src/compression.ts (1)

88-93: Avoid rewriting the report when no images were replaced.

When replacements is empty (every converted image turned out larger, or all matches were small-WebP continues), content is unchanged yet fs.writeFileSync(filePath, content) still rewrites the file, bumping mtime and doing unnecessary I/O per-report. Gate the write on whether anything actually changed:

♻️ Proposed fix

-  content = content.replace(imgRegex, (m) => replacements.get(m) ?? m);
-
-  fs.writeFileSync(filePath, content);
-  const compressedSize = Buffer.byteLength(content, 'utf8');
+  if (replacements.size > 0) {
+    content = content.replace(imgRegex, (m) => replacements.get(m) ?? m);
+    fs.writeFileSync(filePath, content);
+  }
+  const compressedSize = Buffer.byteLength(content, 'utf8');

🤖 Prompt for AI Agents

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

In `@src/compression.ts` around lines 88 - 93, The code always calls
fs.writeFileSync(filePath, content) and recalculates compressedSize even when no
image replacements occurred, causing unnecessary rewrites; change the logic
around the content = content.replace(imgRegex, (m) => replacements.get(m) ?? m)
to detect whether any replacement actually occurred (e.g., compare the new
string to the original or track a flag while replacing) and only call
fs.writeFileSync(filePath, content) and compute compressedSize when content
changed; reference the existing variables imgRegex, replacements, content,
filePath, and compressedSize to implement this gate.

src/dashboard/template.ts (1)

140-148: DRY opportunity: calcWeightedAvg is called six times per (url, mode) pair in two places.

The weighted-metrics computation is duplicated verbatim between the modeMetrics build (lines 140‑147) and metricsRows (lines 224‑230). Each invocation re-derives newestTs and weights from scratch on the same entries array, so you do 12 passes when 2 would do. A small helper tightens both call sites and fixes the repetition:

♻️ Suggested helper

const METRIC_KEYS = ['fcp', 'lcp', 'tbt', 'cls', 'si', 'tti'] as const;
const weightedMetrics = (entries: IterationEntry[]) =>
  Object.fromEntries(METRIC_KEYS.map((k) => [k, calcWeightedAvg(entries, 'metrics', k)]));

Then modeMetrics[mode] = urls.map(u => weightedMetrics(statsData.urls[u].modes[mode] || [])) and the metricsRows branch becomes const avg = weightedMetrics(entries).

Also applies to: 218-234

🤖 Prompt for AI Agents

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

In `@src/dashboard/template.ts` around lines 140 - 148, The code repeatedly calls
calcWeightedAvg six times per (url, mode) in both modeMetrics and metricsRows
causing redundant passes; introduce a helper (e.g., METRIC_KEYS constant and a
weightedMetrics(entries) function) that maps the six metric keys to their
calcWeightedAvg results and returns an object, then replace the inline repeated
calls in modeMetrics assignment and in the metricsRows branch (where entries is
used) with a single call to weightedMetrics(statsData.urls[url].modes[mode] ||
[]) / weightedMetrics(entries) so each entries array is scanned once and both
call sites reuse the helper.

src/lighthouse.ts (1)

52-58: logLevel: 'info' produces very noisy Lighthouse output.

Lighthouse's 'info' log level prints a continuous stream of status lines for every audit — across N URLs × M emulations × K iterations that's a large volume of terminal noise that buries the per-run summary on line 129. Consider 'error' (or 'warn') as the default and exposing it via CLI flag if verbose output is ever wanted.

♻️ Proposed change

-    logLevel: 'info',
+    logLevel: 'error',

🤖 Prompt for AI Agents

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

In `@src/lighthouse.ts` around lines 52 - 58, The default Lighthouse log level is
too verbose; change the options object so options.logLevel uses a quieter
default (e.g., 'error' or 'warn') instead of 'info', and wire a CLI flag or
environment variable (e.g., --log-level or LIGHTHOUSE_LOG_LEVEL) into the code
that builds the options so callers can opt into verbose output; update the
construction of the options constant (the options object where logLevel is set)
to read the flag/env and fall back to the quieter default.

src/data-extract.ts (1)

22-58: averageGroup early return loses iterationCount: 1 intent only for single-entry groups, and unconditionally resets iteration.

Two small consistency items in this helper:

1. Line 24 returns { ...group[0], iterationCount: 1 } — but group[0] was built upstream with iteration: 1, so that value is already 1 (fine). However, for multi-entry groups (line 50) you hardcode iteration: 1 into the averaged output. If iteration numbers carry meaning elsewhere, consider preserving the range (e.g. iterationCount) rather than masquerading it as iteration 1.
2. Line 47 computes the "last entry" via reduce((a, b) => a.timestamp > b.timestamp ? a : b) which picks the max-timestamp entry, not the last by position. Since inputs to groupAndAverage are sorted by timestamp (line 94 for legacy; runId groups are insertion-ordered), the result is correct today, but group[group.length - 1] after sort would be cheaper and more obviously intentional.

Non-blocking; just flagging since the iteration: 1 assignment is load-bearing for grouping elsewhere.

🤖 Prompt for AI Agents

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

In `@src/data-extract.ts` around lines 22 - 58, averageGroup currently hardcodes
iteration: 1 for multi-entry groups and finds the last entry via reduce; change
it to preserve the original iteration baseline and use a clearer/cheaper
last-element lookup: remove the hardcoded iteration: 1 and instead set iteration
to group[0].iteration (or another intended baseline) while keeping
iterationCount: group.length, and replace lastEntry = group.reduce(...) with
lastEntry = group[group.length - 1] so the function (averageGroup) preserves
iteration semantics and uses the actual last item by position.

Fix all unresolved CodeRabbit comments on this PR:

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

src/index-logic.ts (1)

35-37: Unconditional sleep after the final audit wastes config.delay seconds.

The await sleep(config.delay * 1000) runs after every iteration, including the last URL/mode of the last pass, so every CLI invocation pays an extra config.delays before exit. Cheap fix is to skip the wait when there's nothing left to do.

🤖 Prompt for AI Agents

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

In `@src/index-logic.ts` around lines 35 - 37, The unconditional await
sleep(config.delay * 1000) after console.log(`Waiting ${config.delay}s...`)
causes an extra delay after the final audit; change the loop to compute whether
this is the last iteration and only call sleep when there are more audits to
run. Locate the loop that logs and sleeps (references: console.log,
config.delay, sleep) and add an isLastIteration check (e.g., based on loop
indices or remaining urls/modes/passes) so that sleep is skipped when true;
ensure the console.log still prints only when you will actually wait.

src/dashboard/client.ts (1)

33-36: transparentize will break on uppercase or non‑integer HSL components.

hsl.match(/\d+/g) only captures integer digit runs, so e.g. 'hsl(210.5, 100%, 50.2%)' would return ['210', '5', '100', '50', '2'] and silently produce garbage colors. Today all colors.* and modeColors.* are integers, so this is fine — but if anyone later adds a fractional HSL anywhere this fails silently. Either guard with a stricter regex, or just use hsl.replace('hsl(', 'hsla(').replace(')', ', '+alpha+')').

🤖 Prompt for AI Agents

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

In `@src/dashboard/client.ts` around lines 33 - 36, transparentize currently
parses HSL with hsl.match(/\d+/g) which breaks on uppercase, decimals, signs, or
spaces; update the transparentize function to robustly handle HSL strings by
either (a) switching to a safer string transform that replaces the leading
"hsl(" with "hsla(" and appends ", <alpha>)" to preserve the original content
and formatting (handle case-insensitive "hsl" by normalizing to lower/upper as
needed), or (b) use a stricter regex that captures optional decimals, optional
spaces and the percent sign for the S/L components (e.g., a pattern for numbers
like [-+]?\d*\.?\d+ and percent tokens) and then build the hsla string with
those captured groups; locate and update the transparentize function to
implement one of these fixes so fractional and uppercase HSL values are handled
safely.

src/lighthouse.ts (1)

105-116: Dead runData object — never read or written.

runData is built but only summary (from createSummary at line 123) is persisted. The iteration field captured in runData never reaches any consumer; src/data-extract.ts even hardcodes iteration: 1 because the summary file lacks this key. Either remove runData entirely, or include iteration in createSummary (and summary.json) so the extractor can recover the original iteration index.

♻️ Proposed cleanup

-    const runData: RunData = {
-      id: `${urlHash}-${mode}-${timestamp}`,
-      url,
-      urlLabel: getUrlLabel(url),
-      mode,
-      iteration,
-      timestamp,
-      fileName,
-      categories,
-      metrics,
-      ...(runId != null ? { runId } : {}),
-    };
-
-
     const reportFilePath = path.join(runsDir, fileName);

Also drop the now‑unused RunData import on line 6 (and StatsData if it remains unused here).

🤖 Prompt for AI Agents

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

In `@src/lighthouse.ts` around lines 105 - 116, The constructed runData object
(variable runData) is never persisted so iteration is dropped; update the
createSummary call (and the produced summary.json) to include the iteration
field from runData so downstream extractor can read the real iteration index
(ensure createSummary signature and its output JSON schema include iteration),
and remove any now-unused RunData and StatsData imports if you end up deleting
runData entirely; alternatively, if you prefer removal, delete the runData
construction and its import(s) instead of adding iteration to the summary.

Fix all unresolved CodeRabbit comments on this PR:

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

src/lighthouse.ts (2)

7-7: Unused args import.

args is imported from ./config.js but never referenced in this module. Drop it to keep imports tidy.

♻️ Proposed fix

-import { config, runsDir, args } from './config.js';
+import { config, runsDir } from './config.js';
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

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

In @src/lighthouse.ts at line 7, The import list in src/lighthouse.ts includes
an unused symbol args; remove args from the import so only config and runsDir
are imported (update the import statement that currently names config, runsDir,
args). Verify there are no remaining references to args in the file and run a
quick linter/test to ensure the unused-import warning is resolved.

</details>

---

`13-25`: **Prefer `structuredClone` over `JSON.parse(JSON.stringify(...))`.**

`JSON.parse(JSON.stringify(lhr))` is the slowest deep-clone option and silently drops `undefined`, functions, and non-enumerable properties. Lighthouse reports can be several MB, and this is the hot path before each report write. `structuredClone` (built into Node 17+, well within your `engines.node >= 25` target) is faster and preserves more types.

<details>
<summary>♻️ Proposed change</summary>

```diff
-  const cleaned = JSON.parse(JSON.stringify(lhr));
+  const cleaned = structuredClone(lhr);

Additionally, stripScreenshots only removes audits whose details.type === 'screenshot'. Lighthouse also emits screenshot-bearing details like filmstrip and screenshot-thumbnails; if your goal is to keep report files small, consider extending the filter. Optional.

🤖 Prompt for AI Agents

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

In `@src/lighthouse.ts` around lines 13 - 25, Replace the slow JSON-based deep
clone in stripScreenshots with structuredClone: in function
stripScreenshots(lhr) use structuredClone(lhr) instead of
JSON.parse(JSON.stringify(lhr)) to preserve types and speed up cloning; keep the
rest of the logic but also extend the audit-filtering condition in the loop (the
checked symbol is stripScreenshots and the audits iteration over cleaned.audits)
to drop audits where a.details.type is 'screenshot' or other screenshot-bearing
types like 'filmstrip' and 'screenshot-thumbnails' so the resulting cleaned
object omits all screenshot-related details.

src/dashboard/template.ts (2)

4-4: Unused config import.

config is imported from '../config.js' but never referenced in this module.

♻️ Proposed fix

-import { StatsData, IterationEntry } from '../utils.js';
-import { config } from '../config.js';
+import { StatsData, IterationEntry } from '../utils.js';

🤖 Prompt for AI Agents

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

In `@src/dashboard/template.ts` at line 4, The import of config from
'../config.js' in src/dashboard/template.ts is unused; remove the unused import
statement (the "config" import) from the top of the module or, if configuration
values are actually required, update the template module to reference the config
symbols instead; ensure no other references to "config" remain before
committing.

---

91-96: getGrade parameter widened by callers may receive undefined.

The signature is (val: number | null, ...) but callers like getGrade(metrics.fcp, ...) pass values from metrics: any, which in practice can be undefined for missing keys. val == null already covers both, but tightening the type to number | null | undefined documents the contract and helps if metrics is later strictly typed.

♻️ Proposed change

-  const getGrade = (val: number | null, thresholds: number[]) => {
+  const getGrade = (val: number | null | undefined, thresholds: number[]) => {
     if (val == null) return 'none';

🤖 Prompt for AI Agents

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

In `@src/dashboard/template.ts` around lines 91 - 96, The getGrade function's
first parameter can be undefined when callers pass dynamic metrics (e.g.,
getGrade(metrics.fcp, ...)), so update its signature from (val: number | null,
thresholds: number[]) to (val: number | null | undefined, thresholds: number[])
and keep the existing null/undefined guard (if (val == null) return 'none');
also search for other usages of getGrade and update any type annotations or
overloads to accept undefined so callers with possibly-missing metric keys
compile cleanly.

build.js (2)

11-11: target: 'node24' is inconsistent with package.json engines.node: ">=25.0.0".

esbuild is told to compile for Node 24, but package.json declares the minimum supported Node version as >=25.0.0. Either align by bumping the esbuild target to node25, or relax engines.node back to >=24.0.0. Today this means esbuild will conservatively transpile/avoid Node-25-only features even when only Node 25 is supported, which is harmless but misleading.

♻️ Proposed change

-  target: 'node24',
+  target: 'node25',

🤖 Prompt for AI Agents

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

In `@build.js` at line 11, The esbuild config sets target: 'node24' while
package.json declares engines.node: ">=25.0.0", causing an inconsistency; update
the esbuild target to 'node25' to match package.json (or alternatively relax
engines.node to ">=24.0.0") so the build target and declared engine align—locate
the target: 'node24' entry in the build.js esbuild config and change it to
'node25', or adjust package.json's engines.node accordingly.

---

41-44: Dead branch: esbuild rejects on errors by default.

esbuild.build(buildOptions) throws on errors with the default logLevel, so result.errors.length > 0 essentially never triggers — control will land in the outer catch. This is harmless, but if you want a single error-reporting path you can drop the result.errors check, or set logLevel: 'silent' and rely on result.errors exclusively.

🤖 Prompt for AI Agents

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

In `@build.js` around lines 41 - 44, esbuild.build(buildOptions) throws on build
errors, so the if (result.errors.length > 0) branch is dead; remove that
conditional and rely on the outer catch to handle failures (i.e., keep the await
esbuild.build(buildOptions) call and process.exit(1) in the catch), or
alternatively set buildOptions.logLevel = 'silent' and explicitly check
result.errors after esbuild.build(buildOptions) if you prefer a single-path
error-reporting via the result object.

src/compression.ts (2)

5-5: Unused args import.

args is imported from ./config.js but never referenced in this module. Consider removing it to keep imports clean.

♻️ Proposed fix

-import { runsDir, args } from './config.js';
+import { runsDir } from './config.js';

🤖 Prompt for AI Agents

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

In `@src/compression.ts` at line 5, The import statement currently brings in both
runsDir and args from config (symbol: args) but args is not used in this module;
remove args from the import so the line only imports runsDir (i.e., change the
import that references runsDir, args to import only runsDir) and run a quick
search for any later references to args in functions in this file (e.g., any
uses inside compression-related helpers) to ensure you don't accidentally remove
needed logic.

---

90-93: Reusing imgRegex for both .exec() and .replace() is correct, but consider readability.

The same global regex is iterated in the while loop (line 47) and then re-used in content.replace(imgRegex, ...) (line 91). Per spec this is safe — String.prototype.replace resets lastIndex internally — but it can mislead readers. A small clarification: since replacements is keyed by fullMatch, you could replace by direct string concatenation in the loop or use a freshly cloned regex to make intent explicit. Optional cleanup.

🤖 Prompt for AI Agents

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

In `@src/compression.ts` around lines 90 - 93, The code reuses the global imgRegex
for both the while loop (using RegExp.prototype.exec) and later in
content.replace(imgRegex, ...) which is valid but unclear; change to explicitly
avoid reusing the same RegExp by either (a) performing the replacement inside
the existing loop using the replacements Map and string concatenation (use the
same fullMatch keys populated into replacements) and write filePath once, or (b)
create a fresh RegExp instance (e.g., new RegExp(imgRegex.source,
imgRegex.flags)) and pass that into content.replace so the replace call uses a
reset regex; update code around imgRegex, replacements, content, and filePath
accordingly.

tsconfig.json (1)

3-16: lib: ["ES2020"] is below target: "ES2022".

With target set to ES2022 but lib pinned to ES2020, ES2022 globals such as Array.prototype.at, Object.hasOwn, Error.cause, and Object.hasOwn won't be in the type-check library, causing legitimate ES2022 code to fail typechecking. Bump lib to match the target (or drop the explicit lib entry to fall back to the target's default libs).

♻️ Proposed change

-    "lib": ["ES2020"],
+    "lib": ["ES2022"],

🤖 Prompt for AI Agents

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

In `@tsconfig.json` around lines 3 - 16, The tsconfig currently sets "target":
"ES2022" but pins "lib": ["ES2020"], which omits ES2022 runtime typings; update
the tsconfig so the "lib" entry matches the target (e.g., change "lib" to
["ES2022"] or remove the "lib" key entirely to let the compiler pick defaults) —
edit the tsconfig JSON to either replace the "lib" array with ["ES2022"] or
delete the "lib" property, ensuring "target": "ES2022" and "module": "NodeNext"
remain unchanged.

src/data-extract.ts (1)

13-15: Prefer Number.isNaN over global isNaN.

Global isNaN performs type coercion (isNaN("foo") === true) and is footgun-prone. Since these arrays are typed (number | null)[], Number.isNaN is both safer and idiomatic.

♻️ Suggested change

-  const valid = values.filter((v): v is number => v != null && !isNaN(v));
+  const valid = values.filter((v): v is number => v != null && !Number.isNaN(v));

(Same for line 37's filter.)

🤖 Prompt for AI Agents

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

In `@src/data-extract.ts` around lines 13 - 15, Update the type-guarding filters
to use Number.isNaN instead of the global isNaN: change expressions like
values.filter((v): v is number => v != null && !isNaN(v)) to use
!Number.isNaN(v) so the filter on variable valid (and the similar filter around
line 37) uses Number.isNaN for accurate, non-coercing NaN checks while
preserving the null check and type guard.

src/dashboard/client.ts (1)

296-322: Consider try/finally for the sync-guard flags so an exception can't permanently lock zoom↔slider sync.

isUpdatingSlider / isUpdatingChart are reset on the last line of each handler. If timelineSlider.set, trendsChart.zoomScale, or updateVisibleLabels ever throws, the flag stays true for the rest of the page's lifetime and the corresponding direction of sync silently stops working (no error visible to the user — the chart and slider just drift apart). The same pattern applies to the timelineSlider.on('update', …) callback at lines 415‑423. Wrapping the bodies in try { … } finally { flag = false; } makes the recovery deterministic.

♻️ Example for `onZoom` (apply analogously to `onPan` and the slider `update` handler)

           onZoom: ({ chart }) => {
             if (isUpdatingChart || isUpdatingSlider) return;
             isUpdatingSlider = true;
-            const min = chart.scales.x.min;
-            const max = chart.scales.x.max;
-            if (timelineSlider) {
-              timelineSlider.set([min, max], true, true);
-            }
-            updateVisibleLabels(min, max);
-            isUpdatingSlider = false;
+            try {
+              const min = chart.scales.x.min;
+              const max = chart.scales.x.max;
+              if (timelineSlider) {
+                timelineSlider.set([min, max], true, true);
+              }
+              updateVisibleLabels(min, max);
+            } finally {
+              isUpdatingSlider = false;
+            }
           }

🤖 Prompt for AI Agents

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

In `@src/dashboard/client.ts` around lines 296 - 322, The sync-guard flags
(isUpdatingSlider / isUpdatingChart) in the chart handlers can be left true if
an exception is thrown; wrap the body of the onZoom and onPan handlers (and the
timelineSlider.on('update') callback) in try { ... } finally { isUpdatingSlider
= false; } (or isUpdatingChart as appropriate) so the flag is always reset even
if timelineSlider.set, trendsChart.zoomScale, updateVisibleLabels or other calls
throw; keep the early-return guard (if (isUpdatingChart || isUpdatingSlider)
return) and set the flag to true before the try block to preserve the intended
mutual-exclusion semantics.

Fix all unresolved CodeRabbit comments on this PR:

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

src/lighthouse.ts (1)

13-25: Optional: avoid cloning screenshots before discarding them.

JSON.parse(JSON.stringify(lhr)) deep-clones the entire report — including fullPageScreenshot and any base64 screenshot audits — only to delete them right after. For the typical Lighthouse report this is the heaviest part of the payload. Since runnerResult.lhr is not reused after this call, you can either delete in place, or use structuredClone (Node ≥17) and then prune. Not blocking; just avoids a meaningful per-run allocation.

♻️ Proposed refactor

-function stripScreenshots(lhr: any): any {
-  const cleaned = JSON.parse(JSON.stringify(lhr));
-  delete cleaned.fullPageScreenshot;
-  if (cleaned.audits) {
-    for (const [key, audit] of Object.entries(cleaned.audits)) {
-      const a = audit as any;
-      if (a.details && a.details.type === 'screenshot') {
-        delete cleaned.audits[key];
-      }
-    }
-  }
-  return cleaned;
-}
+function stripScreenshots(lhr: any): any {
+  // Mutates the input — caller does not reuse runnerResult.lhr after this.
+  delete lhr.fullPageScreenshot;
+  if (lhr.audits) {
+    for (const key of Object.keys(lhr.audits)) {
+      const a = lhr.audits[key];
+      if (a?.details?.type === 'screenshot') {
+        delete lhr.audits[key];
+      }
+    }
+  }
+  return lhr;
+}

🤖 Prompt for AI Agents

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

In `@src/lighthouse.ts` around lines 13 - 25, The stripScreenshots function
currently does a heavy JSON.parse(JSON.stringify(lhr)) clone; instead mutate the
passed-in object (or use structuredClone if you require immutability on Node
≥17) and remove screenshot data in place: delete lhr.fullPageScreenshot and
iterate lhr.audits (use Object.entries on lhr.audits) deleting any audit where
audit.details?.type === 'screenshot' (reference function name stripScreenshots
and variable cleaned/audit entries) so you avoid cloning large base64 payloads
before discarding them.

src/config.ts (2)

67-81: Invalid CLI flag values are silently dropped.

--run abc, --quality 200, or --quality 0 are accepted by the parser and then discarded — the user gets the default with no indication their intent was ignored. For a CI-friendly CLI this is easy to mis-debug. Consider warning on stderr (or exiting non-zero) when a flag is present but its value fails validation.

♻️ Proposed refactor

     if (arg === '--run' && arr[i + 1] !== undefined) {
       const val = parseInt(arr[i + 1], 10);
-      if (!isNaN(val)) acc.runIterations = val;
+      if (!isNaN(val) && val >= 0) acc.runIterations = val;
+      else console.warn(`⚠️  Ignoring --run "${arr[i + 1]}" (must be a non-negative integer)`);
     }
     if (arg === '--compress') acc.compress = true;
     if (arg === '--quality' && arr[i + 1] !== undefined) {
       const val = parseInt(arr[i + 1], 10);
       if (!isNaN(val) && val >= 1 && val <= 100) acc.quality = val;
+      else console.warn(`⚠️  Ignoring --quality "${arr[i + 1]}" (must be 1-100)`);
     }

🤖 Prompt for AI Agents

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

In `@src/config.ts` around lines 67 - 81, The CLI parser in args (reduce over
process.argv.slice(2)) silently ignores invalid values for --run and --quality;
update the logic in the args reducer to detect when a flag is present but its
value is invalid (e.g., non-numeric for --run, non-numeric or outside 1..100 for
--quality) and handle it by writing a clear error to stderr (console.error) and
exiting with a non-zero code (process.exit(1)) or returning an explicit parse
error; keep the existing assignments for valid values (runIterations, quality,
compress) and ensure the check references the same symbols (args, runIterations,
quality, compress) so users are notified when provided values are rejected.

---

53-56: Unrecognised emulation strings are silently accepted.

The as any cast lets LHR_EMULATIONS=mobile-3g,foo (or a typo in lighthouse-runner.config.json) flow through unchanged. Downstream, src/lighthouse.ts only matches the three documented strings, so an unknown value falls into the "non-desktop, non-throttled mobile" branch — i.e., it runs but with throttling disabled, and the dashboard later groups results under that arbitrary mode label. Worth validating against the union and warning/throwing on unknown entries.

♻️ Proposed refactor

-  emulations: (process.env.LHR_EMULATIONS 
-    ? process.env.LHR_EMULATIONS.split(',').map(s => s.trim()) 
-    : fileConfig.emulations || ['mobile-4g', 'mobile-wifi', 'desktop']) as any,
+  emulations: (() => {
+    const valid = new Set(['mobile-4g', 'mobile-wifi', 'desktop'] as const);
+    const raw = process.env.LHR_EMULATIONS
+      ? process.env.LHR_EMULATIONS.split(',').map(s => s.trim())
+      : fileConfig.emulations || ['mobile-4g', 'mobile-wifi', 'desktop'];
+    const unknown = raw.filter((m: string) => !valid.has(m as any));
+    if (unknown.length) {
+      throw new Error(`❌ Unknown emulation(s): ${unknown.join(', ')}. Allowed: ${[...valid].join(', ')}`);
+    }
+    return raw as Config['emulations'];
+  })(),

🤖 Prompt for AI Agents

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

In `@src/config.ts` around lines 53 - 56, The emulations value currently uses an
unchecked cast (emulations) allowing unknown strings; change construction to
validate entries from LHR_EMULATIONS/fileConfig.emulations against the allowed
union ['mobile-4g','mobile-wifi','desktop'] (use a const ALLOWED_EMULATIONS set
or array) and replace the "as any" cast with a properly typed string[]; for any
unknown entries either log a warning via the existing logger and drop them
(preferred) or throw an error, and ensure the final emulations value passed to
the rest of the app (referenced by the emulations variable and downstream
matching in src/lighthouse.ts) contains only the allowed tokens so unknown
labels cannot silently propagate.

src/dashboard/template.ts (1)

230-241: Optional: collapse the nine .replace() calls into a single pass.

Each .replace() re-scans the full bundled client. A single regex over all INJECT_* tokens with a replacer dispatch would avoid 9 sequential O(n) scans of clientJs and remove the easy-to-forget pattern of adding a new key in two places.

♻️ Proposed refactor

-  const safeInject = (data: any) => JSON.stringify(data).replace(/<\/script>/g, '<\\/script>');
-
-  const clientInjected = clientJs
-    .replace(/\bINJECT_urlLabels\b/g, () => safeInject(urlLabels))
-    .replace(/\bINJECT_allModes\b/g, () => safeInject(allModes))
-    .replace(/\bINJECT_modePerf\b/g, () => safeInject(modePerf))
-    .replace(/\bINJECT_modeMetrics\b/g, () => safeInject(modeMetrics))
-    .replace(/\bINJECT_trendData\b/g, () => safeInject(trendData))
-    .replace(/\bINJECT_allRunsData\b/g, () => safeInject(allRunsData))
-    .replace(/\bINJECT_categoryKeys\b/g, () => safeInject(categoryKeys))
-    .replace(/\bINJECT_modeDataAvg\b/g, () => safeInject(modeDataAvg))
-    .replace(/\bINJECT_modeDataRaw\b/g, () => safeInject(modeDataRaw));
+  const safeInject = (data: unknown) =>
+    JSON.stringify(data).replace(/<\/script>/g, '<\\/script>');
+
+  const injections: Record<string, unknown> = {
+    INJECT_urlLabels: urlLabels,
+    INJECT_allModes: allModes,
+    INJECT_modePerf: modePerf,
+    INJECT_modeMetrics: modeMetrics,
+    INJECT_trendData: trendData,
+    INJECT_allRunsData: allRunsData,
+    INJECT_categoryKeys: categoryKeys,
+    INJECT_modeDataAvg: modeDataAvg,
+    INJECT_modeDataRaw: modeDataRaw,
+  };
+  const clientInjected = clientJs.replace(
+    /\bINJECT_(?:urlLabels|allModes|modePerf|modeMetrics|trendData|allRunsData|categoryKeys|modeDataAvg|modeDataRaw)\b/g,
+    (match) => safeInject(injections[match]),
+  );

🤖 Prompt for AI Agents

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

In `@src/dashboard/template.ts` around lines 230 - 241, Replace the nine
sequential .replace() calls on clientJs with a single .replace using a regex
like /\bINJECT_([A-Za-z0-9_]+)\b/g and a replacer function that looks up the
matched key in a mapping object (map keys like "urlLabels", "allModes",
"modePerf", "modeMetrics", "trendData", "allRunsData", "categoryKeys",
"modeDataAvg", "modeDataRaw") and returns safeInject(mappedValue); update the
clientInjected assignment to use this single-pass replace, reference the
existing safeInject function for escaping, and ensure the replacer returns the
original match or throws a clear error if an unknown INJECT_* token is
encountered so missing keys are detected.

Fix all unresolved CodeRabbit comments on this PR:

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

src/index-logic.ts (1)

48-52: ⚠️ Potential issue | 🟡 Minor

process.exit(1) skips dashboard regeneration on partial failure.

If even one URL/mode tuple fails, this branch hard-exits before extractDataFromReports() and generateVisualReport(statsData) run on lines 54–56 — so all successful iterations from this run are written to disk but the dashboard never picks them up. CI gets the non-zero exit code as intended, but operators lose visibility into the results that did succeed.

Use process.exitCode = 1 instead so the rest of main() (extract + dashboard) still runs and the process exits non-zero only after Complete. is logged.

🛡️ Proposed change

-    if (failedRuns > 0) {
-      console.error(`\n❌ ${failedRuns} run(s) failed. Exiting with error.`);
-      process.exit(1);
-    }
+    if (failedRuns > 0) {
+      console.error(`\n❌ ${failedRuns} run(s) failed.`);
+      process.exitCode = 1;
+    }

🤖 Prompt for AI Agents

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

In `@src/index-logic.ts` around lines 48 - 52, The code currently calls
process.exit(1) when failedRuns > 0 which aborts main() before
extractDataFromReports() and generateVisualReport(statsData) run; change this to
set process.exitCode = 1 instead so the method still returns and the remaining
cleanup/aggregation (calls to extractDataFromReports() and
generateVisualReport(statsData) inside main()) execute and the process exits
non-zero only after completion. Update the branch that checks failedRuns (the
block referencing failedRuns and process.exit(1)) to assign process.exitCode = 1
and remove the immediate hard exit so main() can continue to run its final
steps.

src/dashboard/template.ts (2)

111-118: Extract the URL-label helper — same try/catch block is duplicated three times.

The exact new URL(...) → host + pathname + search (with url-as-fallback) pattern appears at lines 111–118, 181–188, and 195–202. Pulling it into a single helper at module scope reduces noise and ensures the three sites stay in sync if the labelling rule ever changes.

♻️ Proposed refactor

+function toUrlLabel(url: string): string {
+  try {
+    const u = new URL(url);
+    return u.host + u.pathname + u.search;
+  } catch {
+    return url;
+  }
+}
+
 export function generateHtml(statsData: StatsData): string {
   const urls = Object.keys(statsData.urls);
-  const urlLabels = urls.map((u) => {
-    try {
-      const url = new URL(u);
-      return url.host + url.pathname + url.search;
-    } catch {
-      return u;
-    }
-  });
+  const urlLabels = urls.map(toUrlLabel);

…and use toUrlLabel(url) / toUrlLabel(r.url) at the other two sites.

🤖 Prompt for AI Agents

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

In `@src/dashboard/template.ts` around lines 111 - 118, Extract the duplicated
try/catch URL-label logic into a single module-scoped helper named
toUrlLabel(url) that attempts new URL(url) and returns url.host + url.pathname +
url.search, falling back to the original input on error; then replace the three
inline mappings with calls to toUrlLabel (e.g., const urlLabels = urls.map(u =>
toUrlLabel(u)) and use toUrlLabel(r.url) at the other two sites) so all label
generation is centralized and consistent.

---

45-45: Two micro-perf nits that only matter at scale.

• Line 45: Math.max(...entries.map((e) => e.ts)) spreads every entry into an argument list. Engines blow up around ~100k–1M args (RangeError: Maximum call stack size exceeded). For a Lighthouse dashboard you'll never get there in practice, but a reduce form is just as readable and unbounded.
• Line 194: urls.indexOf(r.url) runs inside statsData.runs.map(...), giving O(runs × urls). Building a Map<string, number> once outside the loop turns it into O(runs + urls).

⚡ Proposed tweaks

-  const newestTs = Math.max(...entries.map((e) => e.ts));
+  const newestTs = entries.reduce((m, e) => (e.ts > m ? e.ts : m), -Infinity);

+  const urlIndexByUrl = new Map(urls.map((u, i) => [u, i]));
   const allRunsData = statsData.runs.map((r) => ({
-    urlIndex: urls.indexOf(r.url),
+    urlIndex: urlIndexByUrl.get(r.url) ?? -1,

Also applies to: 193-194

🤖 Prompt for AI Agents

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

In `@src/dashboard/template.ts` at line 45, The current computation of newestTs
uses Math.max(...entries.map((e) => e.ts)) which can blow up for very large
arrays; replace it with a reduce-based max (scan entries for e.ts) to avoid
spreading into the argument list. Also optimize the mapping of run URLs: instead
of calling urls.indexOf(r.url) inside statsData.runs.map (O(runs × urls)),
prebuild a Map<string, number> from urls to their index once (e.g., urlIndexMap)
and use urlIndexMap.get(r.url) inside the runs loop to make it O(runs + urls);
update the code paths that reference newestTs, urls, and statsData.runs/r.url
accordingly.

package.json (1)

18-27: clean is not run by prepublishOnly/CI, so stale artifacts can ship.

prepublishOnly runs npm run build, which is incremental — if a previous build left an .mjs whose source has since been deleted, esbuild won't remove it and npm publish will pick it up via the files allowlist. Cheap fix: chain clean before build in prepublishOnly.

♻️ Proposed change

-    "prepublishOnly": "npm run build"
+    "prepublishOnly": "npm run clean && npm run build"

🤖 Prompt for AI Agents

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

In `@package.json` around lines 18 - 27, The prepublishOnly script currently runs
only "build" and can leave stale artifacts; update the "prepublishOnly" npm
script to run the existing "clean" script before "build" (e.g., chain "npm run
clean && npm run build") so lighthouse-runner.mjs is removed before building;
modify the "prepublishOnly" entry to invoke "clean" then "build" using the
"clean", "build", and "prepublishOnly" script names.

src/lighthouse.ts (3)

43-46: Consider --headless=new for Chrome.

--headless (the legacy "old headless") is on a slow path to removal and behaves differently from a real Chrome instance for some web platform features that Lighthouse exercises (service workers, extensions, network code paths). Chrome's recommendation for new automation is --headless=new, which uses the same browser implementation as headed Chrome and gives more representative Lighthouse scores. Low priority since legacy headless still works today, but worth flipping to keep results aligned with real users.

♻️ Proposed change

-    chromeFlags: ['--headless', '--no-sandbox', '--disable-gpu'],
+    chromeFlags: ['--headless=new', '--no-sandbox', '--disable-gpu'],

🤖 Prompt for AI Agents

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

In `@src/lighthouse.ts` around lines 43 - 46, Update the chrome launch flags to
use the newer headless mode: in the chromeLauncher.launch call (function/method:
chromeLauncher.launch) where chromeFlags is set (and conditionally using
config.chromePath), replace the legacy '--headless' entry with '--headless=new'
while retaining the other flags ('--no-sandbox', '--disable-gpu') so Lighthouse
runs against the newer headless implementation that matches headed Chrome
behavior.

---

88-99: tti audit (interactive) is deprecated since Lighthouse 10 and hidden from the HTML report.

The numeric value is still emitted in the JSON, but its score weight is 0 and Google treats TBT/INP/LCP as the supported successors. As you're already on Lighthouse 13, surfacing TTI in the dashboard alongside Core Web Vitals is misleading — consider dropping it (and the corresponding column in the dashboard) or labelling it as legacy. Same advisory applies to domSize/server-response-time, which are diagnostics rather than scored metrics, but TTI is the one Google has explicitly deprecated.

🤖 Prompt for AI Agents

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

In `@src/lighthouse.ts` around lines 88 - 99, The keyMetrics object in
src/lighthouse.ts is exposing the deprecated TTI audit ('interactive') and other
diagnostic-only metrics; remove the tti entry
(safeGet(lhr.audits['interactive'], 'numericValue')) or replace it with a
clearly labelled legacy field, and update any dashboard/column logic that reads
keyMetrics. Also mark or move domSize (lhr.audits['dom-size']) and
serverResponse (lhr.audits['server-response-time']) as diagnostics rather than
scored metrics so the UI shows them differently; adjust code that renders
columns to drop or label the TTI column and ensure any downstream consumers of
keyMetrics handle the changed shape.

---

13-24: stripScreenshots mutates the live lhr and is called before persisting both the full and summary outputs.

runLighthouse writes the cleaned lhr to disk and then builds the summary from categories/metrics derived earlier, so the summary write is fine. But the mutation does affect any caller that later inspects runnerResult.lhr (e.g. if you ever expose it in runLighthouse's return type). Two minor concerns:

1. delete lhr.audits[key] while iterating Object.entries(lhr.audits) is safe (entries snapshot is taken upfront), but the API contract is fragile — a future refactor that switches to for...in would silently break it.
2. There's no defence against lhr itself being non-object (covered today by the truthiness check on runnerResult, but lhr could still be null per Lighthouse's loose typing); delete null.fullPageScreenshot would crash outside the per-iteration try/catch if runnerResult.lhr is unexpectedly nullish.

Cheap hardening:

♻️ Proposed change

 function stripScreenshots(lhr: any): any {
+  if (!lhr || typeof lhr !== 'object') return lhr;
   delete lhr.fullPageScreenshot;
   if (lhr.audits) {
-    for (const [key, audit] of Object.entries(lhr.audits)) {
+    for (const key of Object.keys(lhr.audits)) {
+      const audit = lhr.audits[key];
       const a = audit as any;
       if (a.details && a.details.type === 'screenshot') {
         delete lhr.audits[key];
       }
     }
   }
   return lhr;
 }

🤖 Prompt for AI Agents

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

In `@src/lighthouse.ts` around lines 13 - 24, stripScreenshots currently mutates
the passed-in lhr and can throw if lhr is nullish; change stripScreenshots to
defensively handle null/undefined lhr and return a new cleaned copy instead of
mutating the input: first check typeof lhr === 'object' and truthiness,
shallow-clone the top-level lhr (e.g., {...lhr}), then if audits exist build a
new audits object by iterating Object.keys(lhr.audits) or filtering entries and
copying only those where audit.details?.type !== 'screenshot' (do not use delete
while iterating), also drop fullPageScreenshot from the new copy, and return the
new cleaned LHR so callers (and runnerResult.lhr) are not modified in place.

src/compression.ts (2)

90-93: Non-atomic write to the report HTML — a crash mid-write corrupts the file.

fs.writeFileSync(filePath, content) overwrites the report in place. If the process is killed (CI timeout, OOM, SIGINT during a long compression batch) between the truncation and the final write, the user is left with a half-written or empty *.html whose only backup is whatever lives in git. Cheap fix: write to ${filePath}.tmp then fs.renameSync (atomic on POSIX, atomic-ish on Windows for same-volume).

🛡️ Proposed change

-  if (replacements.size > 0) {
-    content = content.replace(imgRegex, (m) => replacements.get(m) ?? m);
-    fs.writeFileSync(filePath, content);
-  }
+  if (replacements.size > 0) {
+    content = content.replace(imgRegex, (m) => replacements.get(m) ?? m);
+    const tmp = `${filePath}.tmp`;
+    fs.writeFileSync(tmp, content);
+    fs.renameSync(tmp, filePath);
+  }

Same applies to fs.writeFileSync(filePath, content) at line 27 inside fixDoublePrefixesInReport.

🤖 Prompt for AI Agents

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

In `@src/compression.ts` around lines 90 - 93, The report HTML is overwritten
in-place causing corruption if the process crashes; change both locations that
call fs.writeFileSync(filePath, content) (the replacement block in the
compression routine and the write inside fixDoublePrefixesInReport) to perform
an atomic write by writing to a temp path (e.g., `${filePath}.tmp` or a unique
temp name), fs.writeFileSync the temp file, then fs.renameSync the temp to
filePath so the final replace is atomic across POSIX/Windows same-volume moves;
ensure the temp write uses the same permissions and that any temp file is
cleaned up on failures.

---

41-46: imgRegex may capture trailing data-URI delimiters in some embedding contexts.

The negated character class [^"'\s>}\]\)]+ permits ,, ;, &, <, =, etc. Standard base64 doesn't emit those (only [A-Za-z0-9+/=]), so today's Lighthouse output is fine — but if Lighthouse ever embeds two data URIs concatenated (a pattern this very file is repairing in fixDoublePrefixesInReport), the regex will greedily match past the first one's terminator. Tightening the class to [A-Za-z0-9+/=]+ matches base64's actual alphabet and is strictly safer.

♻️ Proposed change

-  const imgRegex = /data:image\/(jpeg|png|webp);base64,([^"'\s>}\]\)]+)/g;
+  const imgRegex = /data:image\/(jpeg|png|webp);base64,([A-Za-z0-9+/=]+)/g;

🤖 Prompt for AI Agents

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

In `@src/compression.ts` around lines 41 - 46, The imgRegex currently uses a broad
negated class which can greedily capture beyond a single data URI; update the
regular expression used in imgRegex to restrict the captured data to the Base64
alphabet (A-Za-z0-9+/=) so it matches only valid base64 payloads and avoids
swallowing trailing delimiters (this affects the logic that later manipulates
matches such as in fixDoublePrefixesInReport and the replacements Map). Locate
the const imgRegex declaration and replace its character class with one matching
base64 characters only, then run tests that exercise image data URI handling to
verify double-prefix repairs and replacements still behave correctly.

src/index-logic.ts (1)

29-44: Index-based loops add noise where for...of reads cleaner.

The uIdx/mIdx indices exist only to compute isLast; the rest of the body would be more idiomatic with for...of. You can keep the isLast logic without index bookkeeping by using entries():

♻️ Optional refactor

-      for (let uIdx = 0; uIdx < config.urls.length; uIdx++) {
-        const url = config.urls[uIdx];
-        for (let mIdx = 0; mIdx < config.emulations.length; mIdx++) {
-          const mode = config.emulations[mIdx];
+      for (const [uIdx, url] of config.urls.entries()) {
+        for (const [mIdx, mode] of config.emulations.entries()) {
           try {
             await runLighthouse(url, i, mode, runId);
           } catch (err: any) {
             console.error(`   ⚠️ Failed ${url} (${mode}): ${err.message || err.toString()}`);
             failedRuns++;
           }
           const isLast = i === iterations && uIdx === config.urls.length - 1 && mIdx === config.emulations.length - 1;
           if (!isLast) {
             console.log(`Waiting ${config.delay}s...`);
             await sleep(config.delay * 1000);
           }
         }
       }

🤖 Prompt for AI Agents

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

In `@src/index-logic.ts` around lines 29 - 44, Replace the index-based loops that
use uIdx/mIdx with idiomatic for...of + entries() while preserving behavior:
iterate URLs with for (const [uIdx, url] of config.urls.entries()) and
emulations with for (const [mIdx, mode] of config.emulations.entries()) so you
can compute isLast as i === iterations && uIdx === config.urls.length - 1 &&
mIdx === config.emulations.length - 1; keep the existing try/catch that calls
runLighthouse(url, i, mode, runId) and increments failedRuns on error, keep the
console logging and the sleep(config.delay * 1000) when not isLast, and reuse
the same local variables (url, mode, uIdx, mIdx) to avoid changing other logic.

✅ Autofix completed