chore: add benchmark projects for 'c15t-nextjs', 'c15t-react', and 'c…#7
chore: add benchmark projects for 'c15t-nextjs', 'c15t-react', and 'c…#7BurnedChris wants to merge 5 commits intomainfrom
Conversation
…ookie-control'; update configurations and dependencies. Includes new METHODOLOGY.md for benchmarking guidelines and adjustments to .gitignore and package.json scripts.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new benchmarking system: CLI, core benchmark library, runner/orchestrator, shared utilities, schema updates, 11 standardized benchmark apps/configs, documentation, and numerous tooling/manifest changes to support end-to-end cookie-banner performance measurement. Changes
Sequence Diagram(s)mermaid CLI->>Runner: start benchmark request Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 59
🤖 Fix all issues with AI agents
In `@benchmarks/c15t-nextjs/package.json`:
- Around line 12-24: Update the benchmark app to be compatible with Next.js 16
by (1) auditing server and App Router code to await async request APIs (e.g.,
calls to cookies(), headers(), draftMode(), access to params/searchParams) and
update route handlers/components that use them, (2) updating metadata routes to
handle async params (treat params as Promises in
opengraph-image/twitter-image/icon handlers), (3) ensuring CI/deploy uses Node
>=20.9 and TypeScript >=5.1 (package.json dev/proc configs), (4) adjusting
next.config.js to account for next/image changes (add
images.localPatterns.search or equivalent and review cache TTL/IP optimization
settings), (5) adding explicit default.js files for any Parallel Routes slots,
and (6) removing or replacing any usage of the removed next lint command in
build scripts; search for symbols like cookies(), headers(), draftMode(),
next.config.js, metadata route handlers, and any build scripts invoking "next
lint" to locate and fix the code.
In `@benchmarks/c15t-react/config.json`:
- Around line 13-17: The serviceHosts array is missing the benchmark backend
hostname so network/visibility requests to that backend aren't attributed;
update the "serviceHosts" value in config.json by adding the backend host (the
backend's domain) to the existing array (next to "c15t.com" and "consent.io") so
requests to the configured backend URL are included in telemetry and
viewport/visibility measurements.
- Around line 28-32: The npm metadata in the React benchmark config is
incorrect: update the "npm" field in config.json (currently "@c15t/nextjs") to
match the package.json dependency by setting it to "@c15t/react" and, if this
config supports specifying versions, set it to v1.7.1 to align with the
package.json dependency; ensure you modify the "npm" key in the
benchmarks/c15t-react config.json so it reflects the actual package used by the
project.
In `@benchmarks/cookie-yes/config.json`:
- Line 24: The bundleType value in benchmarks/cookie-yes/config.json is using
"iife" which doesn't match the benchmark-schema enum; update the "bundleType"
property to the correct enum value "iffe" (replace "iife" with "iffe") so it
aligns with the allowed values ('esm', 'cjs', 'iffe', 'bundled'); ensure any
other occurrences (e.g., in cookie-control/config.json) are similarly corrected.
In `@benchmarks/didomi/app/layout.tsx`:
- Around line 14-47: The handlers onDidomiReady and onConsentChanged currently
log sensitive consent info (cwtToken and vendor consent statuses); update these
to only emit such logs when a debug flag is enabled (e.g.,
process.env.DEBUG_DIDOMI or a local DEBUG constant) and mask or redact the
cwtToken (show only last 4 chars or replace with ****) before logging; also
avoid logging full vendor consent objects—log minimal, non-identifying summaries
(e.g., boolean or "granted/denied") using the didomiObject.getUserConsentStatus*
calls gated by that debug flag.
In `@benchmarks/enzuzo/app/layout.tsx`:
- Around line 21-51: Remove the commented-out script block in layout.tsx that
contains the hard-coded API key (the commented CookieControl script with apiKey
'ce6586e299e7333a8c22432a32c1972fd859a49c'); delete the entire commented section
or move its non-secret documentation to a private/internal doc, and if the
CookieControl config is required in code later, load the API key from an
environment variable (e.g., via process.env or Next.js runtime config) rather
than embedding it in source or comments.
In `@benchmarks/enzuzo/config.json`:
- Line 20: The bundleType value is using the incorrect enum string "iife";
update the "bundleType" property to the schema-expected value "iffe" (valid
enums: 'esm', 'cjs', 'iffe', 'bundled') so the config conforms to
cookiebench/benchmark-schema; locate the "bundleType" key in the config JSON and
replace "iife" with "iffe".
In `@benchmarks/iubenda/config.json`:
- Line 25: The "bundleType" value in the JSON is using the invalid enum "iife";
update the config entry for "bundleType" to use the valid enum value "iffe" so
schema validation and bundle strategy mapping work correctly (change the value
at the "bundleType" key from "iife" to "iffe").
In `@benchmarks/ketch/config.json`:
- Around line 18-20: The bundleType value in the JSON is invalid ("iife");
update the "bundleType" key in the config (the JSON entry shown with "bundler",
"bundleType", "frameworks") to the schema-accepted value "iffe" so it matches
the benchmark-schema enum ("esm","cjs","iffe","bundled") and passes validation.
In `@benchmarks/ketch/next-env.d.ts`:
- Around line 1-3: The file next-env.d.ts currently imports a non-existent typed
routes file via the line import "./.next/types/routes.d.ts"; which causes a
TypeScript error because typedRoutes is not enabled; remove that import
statement from next-env.d.ts (the import of "./.next/types/routes.d.ts") so the
file no longer references the missing generated routes types, or alternatively
enable typedRoutes in Next.js config if you intend to use those types (update
next.config.ts to set typedRoutes: true) — the immediate fix is to delete the
import line in next-env.d.ts.
In `@benchmarks/onetrust/config.json`:
- Around line 20-27: The techStack.bundleType value is invalid ("iife"); update
the "bundleType" property inside the techStack object to use the schema-approved
value "iffe" (i.e., change "bundleType": "iife" to "bundleType": "iffe") so the
file conforms to the benchmark schema's allowed enum values.
In `@benchmarks/osano/app/layout.tsx`:
- Around line 14-17: The layout currently injects Osano via a raw <script> tag
in the HTML head which blocks rendering; replace the inline tag with Next's
Script component (import Script from 'next/script') and use an appropriate
loading strategy (e.g., strategy="afterInteractive" or "lazyOnload") in the
layout component to avoid blocking, or if you intentionally want blocking for
benchmarking, keep the tag but add a clear inline comment in layout.tsx
explaining that the synchronous <script src="https://cmp.osano.com/.../osano.js"
/> is intentional for accurate user-perceived performance measurement.
In `@benchmarks/osano/config.json`:
- Line 16: The bundleType value is using the incorrect enum "iife"; update the
JSON key bundleType to use the schema-correct value "iffe" (replace "iife" with
"iffe") so it matches the allowed enum values ('esm', 'cjs', 'iffe', 'bundled');
verify the change in the object that contains the "bundleType" property.
- Line 33: The "website" field value is inconsistent with "source.website";
change the "website" entry ("website": "https://Osano.com") to match the
lowercase, canonical URL used by source.website (e.g., "https://www.osano.com")
so both "website" and "source.website" use the exact same lowercase URL.
In `@benchmarks/usercentrics/config.json`:
- Around line 18-24: The bundleType value is invalid ("iife") and must match the
schema enum; update the JSON so the "bundleType" property uses the correct enum
value "iffe" (i.e., change the value of the "bundleType" key in the config to
"iffe") to pass validation.
In `@benchmarks/usercentrics/next-env.d.ts`:
- Around line 1-3: The import of generated route types
("./.next/types/routes.d.ts") in next-env.d.ts is invalid because Next's
typedRoutes option isn't enabled; either enable typedRoutes: true in your Next
configuration (next.config.ts) so the .next/types/routes.d.ts file is produced,
or remove the import from next-env.d.ts to stop referencing non-existent
generated types. Locate the import statement ("./.next/types/routes.d.ts") in
next-env.d.ts and update next.config.ts to set typedRoutes: true (or delete the
import) to resolve the broken reference.
In `@METHODOLOGY.md`:
- Around line 288-289: Update the "**Last Updated**: 2025-10-31" value to the
current date (replace the string "**Last Updated**: 2025-10-31" in
METHODOLOGY.md with "**Last Updated**: 2026-02-16") so the document reflects
when this new file was added; leave "**Version**: 2.0" unchanged.
In `@packages/benchmark/package.json`:
- Around line 21-27: Add an "engines" field to this package.json mirroring other
packages (e.g., "node": ">=20.9.0") by inserting an "engines" object to enforce
the same Node version constraint, and change the hard-pinned "@c15t/logger":
"1.0.0" dependency to "workspace:*" if `@c15t/logger` is part of the monorepo to
keep dependency declarations consistent with "@consentio/shared": "workspace:*".
In `@packages/benchmark/src/bundle-strategy.ts`:
- Around line 7-20: Bundle type detection misses the schema value "iffe" so
isIIFE remains false; add a BUNDLE_TYPES.IFFE = "iffe" (and optionally keep
BUNDLE_TYPES.IIFE for backward compatibility) and update the isIIFE checks to
test for both IFFE and IIFE strings (both direct equality and in
Array.isArray(bundleType). Also update any array-module detection logic
(isArrayWithModules) and any derived flags that depend on isIIFE/isBundled to
include the new IFFE check so configs using "iffe" are recognized.
In `@packages/benchmark/src/constants.ts`:
- Around line 76-80: The BUNDLE_TYPES constant uses the wrong string for the
IIFE variant; update the value in the exported BUNDLE_TYPES object (symbol:
BUNDLE_TYPES) so the IIFE key maps to "iffe" (not "iife") to match the schema
enum values ("esm", "cjs", "iffe", "bundled"), ensuring any
comparisons/validation against bundleType succeed.
In `@packages/benchmark/src/cookie-banner-collector.ts`:
- Around line 239-264: The current logic delays calling startDetection() when
document.readyState === "loading"; instead always invoke startDetection()
immediately to catch banners that render early, then optionally keep a one-time
DOMContentLoaded listener to call startDetection() again if you want a second
check after the DOM finishes loading; update the block that checks
document.readyState to always call startDetection() (and remove the conditional
wait) while leaving detectCookieBanner(), pollInterval, and detectionTimeout
logic unchanged.
In `@packages/benchmark/src/perfume-collector.ts`:
- Around line 241-253: The firstPaint metric is read from the wrong key casing
(rawMetrics.FP) so it always becomes 0; update the access to use the lowercase
key emitted by Perfume.js (rawMetrics.fp) when constructing the PerfumeMetrics
object (the firstPaint property), keeping all other metric accesses (FCP, LCP,
CLS, TBT, FID, INP, TTFB, navigationTiming) unchanged so that firstPaint is
populated correctly.
In `@packages/benchmark/src/types.ts`:
- Around line 32-34: Replace the loose bundleType typing with a shared, stricter
BundleType union that matches the schema enum ("esm", "cjs", "iffe", "bundled")
and use that union wherever bundleType is declared (e.g., in
Config.techStack.bundleType and BundleStrategy.bundleType); change their types
from string | string[] to BundleType | BundleType[] (or BundleType for singular
fields) and update any imports/exports in packages/benchmark/src/types.ts
accordingly so the enum is centralized and reused.
In `@packages/cookiebench-cli/base.json`:
- Around line 1-19: Rename the configuration file from base.json to a
conventional TypeScript config name (e.g., tsconfig.base.json or tsconfig.json)
so tooling and IDEs auto-discover it; update any other tsconfig files or
"extends" references that point to "base.json" to the new name and ensure the
top-level "compilerOptions" block remains unchanged (declaration, module,
target, strict, etc.) and CI/build scripts that reference the old filename are
updated accordingly.
In `@packages/cookiebench-cli/package.json`:
- Around line 20-31: Update the dependency for `@clack/prompts` in package.json to
use the stable release instead of the alpha: replace the current version
specifier "^1.0.0-alpha.0" with a stable semver like "^1.0.1" in the
"dependencies" block so the CLI uses the published stable 1.0.1 package.
- Around line 9-11: Add a Unix shebang as the very first line of the CLI source
(index.ts) so the compiled binary (dist/index.mjs) is executable; specifically
insert #!/usr/bin/env node at the top of the source file (before any imports or
code) for the cookiebench CLI entry so the "cookiebench" bin mapping works when
installed.
In `@packages/cookiebench-cli/README.md`:
- Around line 99-113: The README contains fenced code blocks (e.g., the
interactive prompt snippet starting with "? Select benchmarks to run:") that
lack surrounding blank lines and a language specifier; update those fenced
blocks to have a blank line before and after the triple backticks and add a
language tag (use text) after the opening ```; apply the same change to the
later example block (the other prompt/output snippet) and ensure the final
example has a blank line immediately preceding its opening ``` so all three
examples are fenced as ```text with blank lines around them.
In `@packages/cookiebench-cli/src/commands/db.ts`:
- Around line 219-270: statusCommand currently builds migrationsPath using
DB_PACKAGE_PATH which can point at the cwd and causes false "No migrations
found" when run from a subdirectory; update the migrationsPath construction to
resolve relative to the resolved project root (use projectRoot as the base).
Concretely, replace the existing const migrationsPath = join(DB_PACKAGE_PATH,
"drizzle") with a path resolved from projectRoot (for example using path.resolve
or join(projectRoot, <relative db package path>, "drizzle") or
path.resolve(projectRoot, DB_PACKAGE_PATH, "drizzle")) so that statusCommand and
findProjectRoot are using the same root for locating migrations. Ensure the
change references migrationsPath and keeps the rest of statusCommand logic
intact.
- Line 1: Replace the unsafe execSync usage: change the import from execSync to
execFileSync (from node:child_process) and update every call that uses
shell-style "cd <path> && drizzle-kit ..." to call execFileSync with the command
as an array (e.g., ['drizzle-kit', 'arg1', ...]) and pass { cwd: targetPath,
stdio: 'inherit' } (or appropriate stdio) so paths with spaces and shell parsing
are avoided; update the import symbol execSync and all invocations around the
previous "cd ... &&" usage (including the occurrences around lines 30-36) to use
execFileSync and cwd instead.
In `@packages/cookiebench-cli/src/commands/results.ts`:
- Around line 215-229: The findResultsFiles function currently calls
readdir(dir) which throws if the benchmarks directory is missing; update
findResultsFiles to handle a missing directory gracefully by catching filesystem
errors (specifically ENOENT) or checking directory existence before calling
readdir, and return an empty array when the directory is absent. Locate the
async function findResultsFiles and modify its use of readdir (or add a
pre-check using stat/exists) so that a missing dir does not bubble an exception
but instead results in an empty string[] being returned.
- Around line 983-1092: calculateScores is receiving a single-run INP and an
undercounted third-party request/domain metric; update the aggregation in the
results loop so interactionToNextPaint is averaged across appResults (replace
appResults[0]?.timing.interactionToNextPaint with the mean of
timing.interactionToNextPaint over appResults), compute thirdPartyRequests by
summing isThirdParty across all resource arrays (scripts, styles, images, fonts,
other) for each run then averaging over appResults, and compute
thirdPartyDomains by, for each run, collecting unique domains from resources
where isThirdParty is true (count the set size) and average those counts across
appResults instead of using the hard-coded 5; keep thirdPartySize as-is (already
averaged).
In `@packages/cookiebench-cli/src/commands/save.ts`:
- Around line 393-401: In saveAppToDatabase, guard against empty result.results
(appResults) before any averaging: check if appResults.length === 0 and handle
it (e.g., log a warning via logger and return early or set safe default scores)
to avoid division-by-zero/NaN when computing averages; update the
averaging/score calculation code paths that reference appResults to assume
non-empty after the check so DB save logic won’t persist invalid data.
- Around line 415-509: The INP (interactionToNextPaint) and third‑party
aggregates in the calculateScores call are incorrectly computed:
interactionToNextPaint uses only appResults[0] and
thirdPartyRequests/thirdPartyDomains undercount third‑party resources by only
checking scripts and hard‑coding domains. Change interactionToNextPaint to
average interactionToNextPaint across appResults (same pattern as fcp/lcp/etc.),
change thirdPartyRequests to sum isThirdParty counts across all resource arrays
(scripts, styles, images, fonts, other) per result and then average, and replace
the hard-coded thirdPartyDomains with an average of the unique third‑party
domain count per result (compute unique domains from all resource types where
isThirdParty is true and average across appResults) so all these metrics follow
the same averaging logic used elsewhere in calculateScores.
- Around line 153-195: The techStack fallback in loadConfigForApp currently sets
bundleType to "unknown", which violates the schema enum; update the function
loadConfigForApp to use a schema-valid default (e.g., set techStack.bundleType
to "bundled" instead of "unknown" or omit bundleType when absent) so JSON parsed
configs with missing bundleType validate and save correctly; ensure the change
is applied where techStack is constructed in loadConfigForApp and that any
downstream code expecting bundleType handles the new default.
- Around line 202-225: transformScoresToContract is mapping detail.score into
both the score and value fields, creating duplicate data because the
BenchmarkScores detail type currently only has name and score (see
types/index.ts). Fix by either (A) extending the source type (BenchmarkScores
detail) to include a distinct value property and ensure the producers of
BenchmarkScores populate that value so transformScoresToContract can map
detail.value to value and detail.score to score, or (B) if no separate value
exists or is intended, remove the redundant value field from the contract
mapping in transformScoresToContract (or set it to undefined/null) so score is
the single authoritative field; update types in types/index.ts and any code that
constructs BenchmarkScores accordingly and keep the mapping in
transformScoresToContract consistent with the chosen change.
In `@packages/cookiebench-cli/src/commands/scores.ts`:
- Around line 186-355: The displayAppScores function currently assumes
appResults has items and will divide by appResults.length, causing
NaN/divide-by-zero if empty; update displayAppScores to guard against empty
appResults (check appResults.length === 0) before computing averages and either
log a warning via logger.warn and return early or pass safe defaults into
calculateScores; ensure the guard also prevents accessing
appResults[0]?.timing.networkInformation and any .reduce calls so
calculateScores is only invoked when appResults has at least one entry.
In `@packages/cookiebench-cli/src/components/intro.ts`:
- Around line 54-81: The gradient position calculation can produce NaN when
figletText yields a single line because position = index / (lines.length - 1)
divides by zero; update the logic in the section that builds coloredLines (using
figletText and lines) to guard the denominator—e.g., compute const denom =
Math.max(1, lines.length - 1) and use position = index / denom or explicitly set
position = 0 when lines.length === 1—so the subsequent comparisons (position <
0.1 ... ) behave deterministically and single-line output maps to the intended
color instead of falling through to cyan100.
In `@packages/cookiebench-cli/src/index.ts`:
- Around line 76-128: Interactive menu omits the "scores" action so interactive
users can't access scores; update the options array to include an entry with
value "scores" and appropriate label/hint, and add a matching case in the switch
to call the existing scoresCommand (similar to benchmarkCommand/resultsCommand);
locate the options definition and the switch block in index.ts (symbols:
options, select, isCancel, onCancel, benchmarkCommand, resultsCommand,
saveCommand, dbCommand) and add the "scores" option and a case that awaits
scoresCommand(logger).
In `@packages/cookiebench-cli/src/utils/auth.ts`:
- Around line 1-11: The isAdminUser function currently compares
process.env.CONSENT_ADMIN raw which fails for values with extra whitespace or
different case; update isAdminUser to normalize the env value first (e.g., read
process.env.CONSENT_ADMIN into a local, default to empty string, then trim() and
toLowerCase() it) and then compare the normalized string to "true", "1", or
"yes" so values like " TRUE " or "true" are accepted; change references inside
isAdminUser (adminFlag / process.env.CONSENT_ADMIN) accordingly.
In `@packages/cookiebench-cli/src/utils/logger.ts`:
- Around line 68-72: Remove the unnecessary type cast in the default branch:
instead of creating levelStr with "logLevel as string" use the existing logLevel
value (or wrap with String(logLevel) for safety) when building the log prefix;
update the default branch in the function that formats log messages (the switch
default using levelStr and messageStr) to assign levelStr directly from logLevel
and eliminate the redundant "as string" cast.
In `@packages/cookiebench-cli/src/utils/scoring.ts`:
- Around line 1381-1422: The printScores function builds overallTable and
detailsTable but never outputs them; update printScores (which accepts a
BenchmarkScores) to actually print the populated tables to the console by
converting each table to a string and logging it (e.g., print overallTable after
you finish pushing rows, then print detailsTable), and also emit any
scores.insights and scores.recommendations (if present) after the tables so
users see the full output.
- Around line 1193-1202: The resourceData array uses new
Array(networkMetrics.thirdPartyRequests).fill({...}) which creates multiple
references to the same object; change the creation to produce distinct objects
(e.g., replace the fill call with Array.from({ length:
networkMetrics.thirdPartyRequests }, () => ({ size:
networkMetrics.thirdPartySize / Math.max(networkMetrics.thirdPartyRequests, 1),
isThirdParty: true }))) so each element is a separate object; update the code
that constructs resourceData (the resourceData variable referencing
bundleMetrics and networkMetrics) accordingly.
In `@packages/runner/src/benchmark-runner.ts`:
- Around line 204-241: The timeout path in runSingleBenchmarkWithRetry currently
races runSingleBenchmark against a timeout but does not cancel or cleanup the
still-running runSingleBenchmark, causing overlapping runs and leaking
collectors/handlers; update runSingleBenchmarkWithRetry so each retry uses a
fresh page/context and the previous in-flight run is properly aborted and
cleaned up on timeout or error — either by propagating an AbortSignal into
runSingleBenchmark (add an abort parameter to runSingleBenchmark and honor it
inside its collectors/route handlers) or by closing the page/context when the
timeout triggers, then creating a new page for the next attempt; ensure cleanup
logic runs on catch (before awaiting the retry delay) to reset collectors/route
handlers and reference MAX_RETRIES, ITERATION_TIMEOUT_MS, CLEANUP_DELAY_MS, and
RETRY_DELAY_MULTIPLIER where relevant.
- Around line 119-121: The network monitoring is initialized before navigation
using page.url() which is about:blank, so update the call to
NetworkMonitor.setupMonitoring to accept the actual target URL (e.g.,
this.config.url or the variable used for navigation) and pass that URL into
networkMonitor.setupMonitoring(page, targetUrl) so the method can derive the
correct first‑party hostname; keep cookieBannerCollector.setupDetection(page)
as-is but ensure setupMonitoring’s signature (NetworkMonitor.setupMonitoring)
and all its callers are updated to accept and use the provided target URL for
first‑party detection.
- Around line 339-384: The execAsync call that interpolates traceDir and
config.name into a shell command is vulnerable to command injection and not
cross-platform; replace the execAsync(...) usage in the trace extraction block
with a safe extraction method: either call the unzip binary via a non-shell API
(e.g., child_process.execFile or spawn) passing arguments as an array (e.g.,
program "unzip" and args ["-o", traceZipPath, "-d", tempDir, "trace.trace"]) to
avoid shell interpolation, or better — eliminate the external dependency and use
a Node zip library (e.g., adm-zip, unzipper, or yauzl) to open traceZipPath,
extract the "trace.trace" entry and write it to traceJsonPath; keep the existing
context.tracing.stop, the subsequent read/write/unlink logic, and logging
(this.saveTrace, traceZipPath, traceJsonPath, traceDir, config.name) but ensure
all path values are never passed to a shell and handle Windows platform
differences if you opt for the unzip binary approach.
In `@packages/runner/src/performance-aggregator.ts`:
- Around line 405-416: The logged bannerRenderTime can become negative when
finalMetrics.timing.cookieBanner.renderEnd is 0 while renderStart > 0; update
the computation in the logger.debug call (where bannerRenderTime is computed
from finalMetrics.timing.cookieBanner.renderEnd and renderStart) to clamp the
duration to zero using a max(0, renderEnd - renderStart) pattern (e.g.,
Math.max(0, renderEnd - renderStart)) so negative durations are never emitted.
- Around line 170-187: Change the TTFB handling so missing values are treated
like other injectable metrics: update BenchmarkDetails.timing.timeToFirstByte to
be number | null, set timeToFirstByte in aggregateMetrics to use "?? null" when
perfumeMetrics is absent (instead of "?? 0"), and modify calculateAverages to
filter out null/<=0 TTFB entries (similar to
firstInputDelay/interactionToNextPaint) — compute the mean only when non-null
values exist and otherwise default the averaged TTFB to 0. Ensure you update
references to perfumeMetrics?.timeToFirstByte and any consumers of
BenchmarkDetails.timing.timeToFirstByte accordingly.
In `@packages/runner/src/server.ts`:
- Around line 41-70: The readiness polling needs a timeout and cleanup: wrap
each fetch(url) with an AbortController and a per-request timeout (Node 18+),
install exit/error listeners on the spawned serverProcess to detect early
crashes and break the loop, and ensure serverProcess.kill() is called when
polling fails or when throwing the final error; update the loop around
serverProcess (spawn) and the variables url/retries/maxRetries/ONE_SECOND to
abort pending fetches, remove listeners and kill the process on error paths, and
log the crash before rethrowing so failures are surfaced instead of leaking the
child process.
- Around line 15-28: The Promise waiting on buildProcess currently only listens
for the "close" event and can hang if spawn(pm.command, ...) emits an "error";
add an "error" event listener on the buildProcess (the variable buildProcess
returned by spawn) that immediately rejects the Promise with the received error
(include context in the rejection message) and ensure the listener is attached
before awaiting so failures like command-not-found or permission denied are
propagated; keep existing "close" handling in the same Promise and remove
potential double-resolution by ensuring the "error" handler rejects and the
"close" handler only resolves/rejects when appropriate.
In `@packages/runner/src/statistics.ts`:
- Around line 84-107: calculateTrimmedMean can receive out-of-range trimPercent
values which lead to negative slice bounds or fully trimmed arrays; inside
calculateTrimmedMean clamp trimPercent to a safe range and/or cap the computed
trimCount: e.g., ensure trimPercent is constrained to
0..(PERCENTAGE_CONVERSION/2) or compute trimCount then set trimCount =
Math.max(0, Math.min(trimCount, Math.floor(values.length / 2))). Update the
logic around trimCount (and references to DEFAULT_TRIM_PERCENT and
PERCENTAGE_CONVERSION) so slicing uses a valid trimmed range and never produces
negative indices or an empty trimmed array due to over-trimming.
In `@packages/runner/src/types.ts`:
- Line 1: The techStack.bundleType property is currently typed as string |
string[] which allows invalid values; change its type to the enum re-export
BundleStrategy so it becomes BundleStrategy | BundleStrategy[] (BundleStrategy
is re-exported at the top of this file), and apply the same tightening to the
other occurrence of bundleType in the file (the second definition around the
later block that currently uses string | string[]). Update any imports/exports
if necessary to ensure BundleStrategy is used everywhere instead of raw strings.
In `@packages/shared/package.json`:
- Around line 26-27: The package.json lists an invalid dependency version for
"@types/node" ("^24.9.2"); update the "@types/node" entry in package.json to a
valid published version (e.g., "^24.0.9" for 24.x or "^25.2.3" for the current
stable) while leaving "typescript" untouched; ensure you run npm/yarn install
afterward to update lockfiles and CI.
In `@packages/shared/README.md`:
- Around line 67-69: The README example uses await setTimeout(ONE_SECOND)
without clarifying which API is intended; update the example to state that this
async form requires importing setTimeout from "node:timers/promises" (Node.js
16+) — mention the import and that ONE_SECOND is a constant — or alternatively
show the callback-based global setTimeout usage for broader Node versions;
reference the symbols setTimeout and ONE_SECOND in the explanation so readers
know to either import the promise-based setTimeout or use the traditional
callback form.
In `@packages/shared/src/utils/config.ts`:
- Around line 21-24: The catch block that logs a failure currently hardcodes
"config.json" which is misleading when a custom configPath is supplied; update
the error log in the catch of the function that reads the config (use the
configPath variable or the computed path/default filename) to include the actual
path attempted (e.g., `console.error(\`Failed to read config at ${configPath ||
defaultPath}:\`, error)`), preserving the existing lint ignore comment and the
null return behavior.
- Around line 14-20: The readConfig<T> function currently uses a TypeScript
assertion (as T) without runtime validation so callers may receive incorrectly
typed objects; update the readConfig<T> declaration with a JSDoc comment that
clearly states the generic T is compile-time only, that the JSON is not
schema-validated at runtime, and instruct callers to perform their own
validation (or pass/implement a validator) if strict runtime type safety is
required; reference the function name readConfig and mention the configPath
parameter and returned value so readers know where to apply this note.
In `@packages/shared/src/utils/conversion.ts`:
- Around line 17-25: In formatBytes, guard against negative and extremely large
inputs: if bytes < 0, treat as 0 (or throw) before using Math.log to avoid NaN;
expand or cap unit selection so i = Math.floor(Math.log(bytes) /
Math.log(KILOBYTE)) is clamped to a valid range (e.g., i = Math.max(0,
Math.min(i, sizes.length - 1))) and extend sizes
(["bytes","KB","MB","GB","TB","PB"]) or at minimum clamp to the largest unit;
also ensure you handle non-finite results by falling back to 0 to avoid runtime
errors when computing the formatted value.
In `@packages/shared/src/utils/package-manager.ts`:
- Around line 5-36: The getPackageManager function currently probes executables
and prefers npm; instead first inspect process.env.npm_config_user_agent and the
repository's package.json "packageManager" field and the presence of
pnpm-lock.yaml to respect declared or invoked package manager, then only
fallback to execSync probes; update getPackageManager to (1) read
process.env.npm_config_user_agent for strings like "pnpm" or "yarn", (2) read
package.json's packageManager property (if present) and return the corresponding
command/args, (3) check for pnpm-lock.yaml or yarn.lock as a heuristic, and only
if none indicate a manager, run the existing execSync checks (preserving the
same return shapes for command and args) and finally default to npm.
In `@README.md`:
- Line 103: Update the README's documentation of the bundleType enum to match
the schema by replacing any occurrences of "iife" with the correct value "iffe";
locate the `bundleType` documentation and examples (the line listing
`bundleType: Output format (esm, iife, cjs)`) and update the enum list to "esm,
cjs, iffe, bundled" and adjust any sample config snippets or examples that
reference `bundleType` to use "iffe" instead of "iife".
In `@tsconfig.json`:
- Around line 17-21: The tsconfig currently sets "strict": true and also
redundantly specifies "strictNullChecks": true; remove the explicit
"strictNullChecks" entry so the compiler relies on the umbrella "strict" setting
instead, i.e., delete the "strictNullChecks" property while keeping "strict" and
the other options unchanged to avoid duplicate configuration.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (12)
benchmarks/c15t-nextjs/app/favicon.icois excluded by!**/*.icobenchmarks/c15t-react/app/favicon.icois excluded by!**/*.icobenchmarks/cookie-control/app/favicon.icois excluded by!**/*.icobenchmarks/cookie-yes/app/favicon.icois excluded by!**/*.icobenchmarks/didomi/app/favicon.icois excluded by!**/*.icobenchmarks/enzuzo/app/favicon.icois excluded by!**/*.icobenchmarks/iubenda/app/favicon.icois excluded by!**/*.icobenchmarks/ketch/app/favicon.icois excluded by!**/*.icobenchmarks/onetrust/app/favicon.icois excluded by!**/*.icobenchmarks/osano/app/favicon.icois excluded by!**/*.icobenchmarks/usercentrics/app/favicon.icois excluded by!**/*.icopnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (138)
.cursor/rules/ultracite.mdc.gitignoreMETHODOLOGY.mdREADME.mdbenchmarks/baseline/next-env.d.tsbenchmarks/baseline/package.jsonbenchmarks/c15t-nextjs/app/layout.tsxbenchmarks/c15t-nextjs/app/page.tsxbenchmarks/c15t-nextjs/config.jsonbenchmarks/c15t-nextjs/next-env.d.tsbenchmarks/c15t-nextjs/next.config.tsbenchmarks/c15t-nextjs/package.jsonbenchmarks/c15t-nextjs/tsconfig.jsonbenchmarks/c15t-react/app/layout.tsxbenchmarks/c15t-react/app/page.tsxbenchmarks/c15t-react/config.jsonbenchmarks/c15t-react/next-env.d.tsbenchmarks/c15t-react/next.config.tsbenchmarks/c15t-react/package.jsonbenchmarks/c15t-react/tsconfig.jsonbenchmarks/cookie-control/app/layout.tsxbenchmarks/cookie-control/app/page.tsxbenchmarks/cookie-control/config.jsonbenchmarks/cookie-control/next-env.d.tsbenchmarks/cookie-control/next.config.tsbenchmarks/cookie-control/package.jsonbenchmarks/cookie-control/tsconfig.jsonbenchmarks/cookie-yes/app/layout.tsxbenchmarks/cookie-yes/app/page.tsxbenchmarks/cookie-yes/config.jsonbenchmarks/cookie-yes/next-env.d.tsbenchmarks/cookie-yes/next.config.tsbenchmarks/cookie-yes/package.jsonbenchmarks/cookie-yes/tsconfig.jsonbenchmarks/didomi/app/layout.tsxbenchmarks/didomi/app/page.tsxbenchmarks/didomi/config.jsonbenchmarks/didomi/next-env.d.tsbenchmarks/didomi/next.config.tsbenchmarks/didomi/package.jsonbenchmarks/didomi/tsconfig.jsonbenchmarks/enzuzo/app/layout.tsxbenchmarks/enzuzo/app/page.tsxbenchmarks/enzuzo/config.jsonbenchmarks/enzuzo/next-env.d.tsbenchmarks/enzuzo/next.config.tsbenchmarks/enzuzo/package.jsonbenchmarks/enzuzo/tsconfig.jsonbenchmarks/iubenda/app/layout.tsxbenchmarks/iubenda/app/page.tsxbenchmarks/iubenda/config.jsonbenchmarks/iubenda/next-env.d.tsbenchmarks/iubenda/next.config.tsbenchmarks/iubenda/package.jsonbenchmarks/iubenda/tsconfig.jsonbenchmarks/ketch/app/layout.tsxbenchmarks/ketch/app/page.tsxbenchmarks/ketch/config.jsonbenchmarks/ketch/next-env.d.tsbenchmarks/ketch/next.config.tsbenchmarks/ketch/package.jsonbenchmarks/ketch/tsconfig.jsonbenchmarks/onetrust/app/layout.tsxbenchmarks/onetrust/app/page.tsxbenchmarks/onetrust/config.jsonbenchmarks/onetrust/next-env.d.tsbenchmarks/onetrust/next.config.tsbenchmarks/onetrust/package.jsonbenchmarks/onetrust/tsconfig.jsonbenchmarks/osano/app/layout.tsxbenchmarks/osano/app/page.tsxbenchmarks/osano/config.jsonbenchmarks/osano/next-env.d.tsbenchmarks/osano/next.config.tsbenchmarks/osano/package.jsonbenchmarks/osano/tsconfig.jsonbenchmarks/usercentrics/app/layout.tsxbenchmarks/usercentrics/app/page.tsxbenchmarks/usercentrics/config.jsonbenchmarks/usercentrics/next-env.d.tsbenchmarks/usercentrics/next.config.tsbenchmarks/usercentrics/package.jsonbenchmarks/usercentrics/tsconfig.jsonpackage.jsonpackages/benchmark-schema/schema.jsonpackages/benchmark/README.mdpackages/benchmark/package.jsonpackages/benchmark/rslib.config.tspackages/benchmark/src/bundle-strategy.tspackages/benchmark/src/constants.tspackages/benchmark/src/cookie-banner-collector.tspackages/benchmark/src/index.tspackages/benchmark/src/network-monitor.tspackages/benchmark/src/perfume-collector.tspackages/benchmark/src/resource-timing-collector.tspackages/benchmark/src/types.tspackages/benchmark/tsconfig.jsonpackages/cookiebench-cli/README.mdpackages/cookiebench-cli/base.jsonpackages/cookiebench-cli/package.jsonpackages/cookiebench-cli/rslib.config.tspackages/cookiebench-cli/src/commands/benchmark.tspackages/cookiebench-cli/src/commands/db.tspackages/cookiebench-cli/src/commands/results.tspackages/cookiebench-cli/src/commands/save.tspackages/cookiebench-cli/src/commands/scores.tspackages/cookiebench-cli/src/components/intro.tspackages/cookiebench-cli/src/index.tspackages/cookiebench-cli/src/types/index.tspackages/cookiebench-cli/src/utils/auth.tspackages/cookiebench-cli/src/utils/constants.tspackages/cookiebench-cli/src/utils/index.tspackages/cookiebench-cli/src/utils/logger.tspackages/cookiebench-cli/src/utils/scoring.tspackages/cookiebench-cli/tsconfig.jsonpackages/runner/README.mdpackages/runner/package.jsonpackages/runner/rslib.config.tspackages/runner/src/benchmark-runner.tspackages/runner/src/index.tspackages/runner/src/performance-aggregator.tspackages/runner/src/server.tspackages/runner/src/statistics.tspackages/runner/src/types.tspackages/runner/src/utils.tspackages/runner/tsconfig.jsonpackages/shared/README.mdpackages/shared/package.jsonpackages/shared/rslib.config.tspackages/shared/src/constants.tspackages/shared/src/index.tspackages/shared/src/utils/config.tspackages/shared/src/utils/conversion.tspackages/shared/src/utils/package-manager.tspackages/shared/src/utils/time.tspackages/shared/tsconfig.jsontsconfig.jsonturbo.json
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-07T16:17:07.589Z
Learnt from: danielsitek
Repo: consentdotio/benchmarks PR: 6
File: benchmarks/with-dgp-cookie-consent/config.json:15-20
Timestamp: 2025-11-07T16:17:07.589Z
Learning: In the cookiebench/benchmark-schema package (packages/benchmark-schema/schema.json), the valid enum values for bundleType are: "esm", "cjs", "iffe", and "bundled". Note that it's "iffe" (not "iife").
Applied to files:
benchmarks/c15t-nextjs/config.jsonbenchmarks/c15t-react/config.jsonbenchmarks/osano/config.jsonbenchmarks/onetrust/config.jsonbenchmarks/cookie-yes/config.jsonbenchmarks/enzuzo/config.jsonpackages/benchmark/src/constants.tsbenchmarks/ketch/config.jsonpackages/benchmark/src/bundle-strategy.tsbenchmarks/baseline/package.jsonbenchmarks/osano/package.jsonpackages/benchmark/package.jsonbenchmarks/cookie-yes/package.jsonbenchmarks/didomi/config.jsonpackages/cookiebench-cli/package.jsonpackages/cookiebench-cli/tsconfig.jsonbenchmarks/iubenda/config.jsonpackages/benchmark/README.mdbenchmarks/usercentrics/config.jsonpackages/runner/src/index.tsbenchmarks/onetrust/package.jsonbenchmarks/cookie-control/config.jsonpackages/benchmark/src/index.tsbenchmarks/cookie-control/package.jsonpackages/runner/src/types.tspackages/benchmark/src/types.tspackages/cookiebench-cli/base.jsonbenchmarks/iubenda/package.jsonpackage.jsonbenchmarks/enzuzo/package.jsonpackages/benchmark-schema/schema.jsonpackages/cookiebench-cli/src/types/index.tspackages/benchmark/tsconfig.jsontsconfig.json
📚 Learning: 2025-11-07T16:19:37.939Z
Learnt from: danielsitek
Repo: consentdotio/benchmarks PR: 6
File: benchmarks/with-dgp-cookie-consent/config.json:37-39
Timestamp: 2025-11-07T16:19:37.939Z
Learning: In the cookiebench/benchmark-schema package (packages/benchmark-schema/schema.json), the includes.backend property accepts either a string or an array of strings (oneOf), not a boolean.
Applied to files:
benchmarks/c15t-nextjs/config.jsonbenchmarks/c15t-react/config.jsonbenchmarks/osano/config.jsonbenchmarks/onetrust/config.jsonbenchmarks/cookie-yes/config.jsonbenchmarks/enzuzo/config.jsonbenchmarks/ketch/config.jsonbenchmarks/didomi/config.jsonbenchmarks/iubenda/config.jsonbenchmarks/usercentrics/config.jsonbenchmarks/cookie-control/config.jsonpackages/benchmark-schema/schema.json
🧬 Code graph analysis (10)
packages/cookiebench-cli/src/components/intro.ts (1)
packages/cookiebench-cli/src/utils/logger.ts (2)
logger(162-162)CliLogger(9-9)
packages/benchmark/src/resource-timing-collector.ts (1)
packages/cookiebench-cli/src/utils/logger.ts (1)
logger(162-162)
packages/runner/src/server.ts (1)
packages/cookiebench-cli/src/utils/logger.ts (1)
logger(162-162)
packages/cookiebench-cli/src/commands/scores.ts (2)
packages/cookiebench-cli/src/utils/logger.ts (2)
logger(162-162)CliLogger(9-9)packages/cookiebench-cli/src/utils/constants.ts (1)
DEFAULT_DOM_SIZE(3-3)
packages/benchmark/src/cookie-banner-collector.ts (2)
packages/cookiebench-cli/src/utils/logger.ts (1)
logger(162-162)packages/cli/src/lib/performance-enhanced.ts (1)
detectCookieBanner(117-209)
packages/cookiebench-cli/src/commands/save.ts (2)
packages/cookiebench-cli/src/utils/logger.ts (2)
logger(162-162)CliLogger(9-9)packages/cookiebench-cli/src/utils/auth.ts (1)
isAdminUser(5-11)
packages/runner/src/performance-aggregator.ts (1)
packages/cookiebench-cli/src/utils/logger.ts (1)
logger(162-162)
packages/cookiebench-cli/src/index.ts (5)
packages/cookiebench-cli/src/utils/logger.ts (3)
logger(162-162)CliLogger(9-9)createCliLogger(112-159)packages/cookiebench-cli/src/utils/auth.ts (1)
isAdminUser(5-11)packages/cookiebench-cli/src/components/intro.ts (1)
displayIntro(11-93)packages/cookiebench-cli/src/commands/scores.ts (1)
scoresCommand(94-184)packages/cookiebench-cli/src/commands/save.ts (1)
saveCommand(228-391)
packages/cookiebench-cli/src/commands/db.ts (2)
packages/cookiebench-cli/src/utils/logger.ts (2)
logger(162-162)CliLogger(9-9)packages/cookiebench-cli/src/utils/auth.ts (1)
isAdminUser(5-11)
packages/runner/src/benchmark-runner.ts (1)
packages/cookiebench-cli/src/utils/logger.ts (1)
logger(162-162)
🪛 ast-grep (0.40.5)
benchmarks/onetrust/app/layout.tsx
[warning] 23-23: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
benchmarks/iubenda/app/layout.tsx
[warning] 18-18: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
benchmarks/cookie-control/app/layout.tsx
[warning] 22-22: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Gitleaks (8.30.0)
benchmarks/cookie-control/app/layout.tsx
[high] 26-26: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
benchmarks/enzuzo/app/layout.tsx
[high] 26-26: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
METHODOLOGY.md
[grammar] ~79-~79: Ensure spelling is correct
Context: ...aintains speed - Fast 3G (1.6 Mbps, 562ms latency): More realistic, significantly...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
packages/cookiebench-cli/README.md
[uncategorized] ~373-~373: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ice overhead 4. Transparency (15%): Open source status, documentation, licensing 5. **U...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.20.0)
packages/cookiebench-cli/README.md
[warning] 99-99: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 99-99: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 170-170: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 170-170: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 336-336: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
benchmarks/c15t-nextjs/package.json
Outdated
| "dependencies": { | ||
| "@c15t/nextjs": "1.7.1", | ||
| "next": "16.0.1", | ||
| "react": "^19.2.0", | ||
| "react-dom": "^19.2.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@cookiebench/benchmark-schema": "workspace:*", | ||
| "@cookiebench/ts-config": "workspace:*", | ||
| "@types/node": "^24.9.2", | ||
| "@types/react": "^19.2.2", | ||
| "@types/react-dom": "^19.2.2", | ||
| "typescript": "^5.9.3" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
next.js 16.0.1 npm package published version
💡 Result:
The Next.js npm package (next) version 16.0.1 was published on October 28, 2025. [1], [2]
🌐 Web query:
react 19.2.0 react-dom 19.2.0 npm published
💡 Result:
Yes—both react@19.2.0 and react-dom@19.2.0 were published to the npm registry on October 1, 2025 (about 21:38 UTC). The React team’s React 19.2 announcement also states it’s available on npm. [1][2][3]
Sources: React 19.2 release post [1]; npm publish metadata mirrors for react@19.2.0 [2] and react-dom@19.2.0 [3].
🌐 Web query:
typescript 5.9.3 npm published version
💡 Result:
TypeScript v5.9.3 (the typescript package on npm) was published on 2025‑09‑30 at 21:19 UTC (by typescript-bot). [1][2]
🌐 Web query:
@types/node 24.9.2 npm published
💡 Result:
@types/node@24.9.2 was published to npm on October 28, 2025. [1]
🌐 Web query:
Next.js 16 React 19 compatibility breaking changes
💡 Result:
Compatibility status (Next.js 16 ↔ React 19)
- Next.js 16 is built to run on React 19 (including React 19.2 features), so upgrading Next to 16 typically implies upgrading
react/react-domto 19.x. [1][2]
Next.js 16 breaking changes that commonly surface during a React 19 upgrade
These are “it broke when I moved to Next 16 + React 19” changes, even though they’re Next-driven:
- Async Request APIs are now truly async (sync access removed):
- Must
awaitcookies(),headers(),draftMode(), plusparams/searchParamsin the listed App Router entrypoints (e.g.page.js,layout.js, route handlers, metadata images). [2]
- Must
- Metadata image routes now receive async params/id:
- In
opengraph-image,twitter-image,icon,apple-icon,params(and sometimesid) are Promises in v16. [2]
- In
next/imagedefault/behavior changes (several are breaking):- Local image URLs with query strings now require
images.localPatterns.search. - Default
images.minimumCacheTTLchanged 60s → 4 hours. - Default
images.imageSizesremoved16. - Default
images.qualitieschanged to[75]. - Local-IP image optimization blocked by default (
dangerouslyAllowLocalIP). - Default
images.maximumRedirectsnow3. next/legacy/imagedeprecated;images.domainsdeprecated in favor ofremotePatterns. [2]
- Local image URLs with query strings now require
- Removals:
- AMP support removed
next lintcommand removed (Next no longer runs linting as part ofnext build). [1][2]
- Runtime/tooling minimums changed:
- Node.js 20.9+, TypeScript 5.1+, and newer browser baselines are required. [1][2]
React 19 breaking changes / upgrade gotchas that show up in Next.js apps
- Modern JSX transform is required (older JSX runtimes will warn / may break). [3]
refis treated as a normal prop;element.refis deprecated (accessing it warns and will be removed from JSX Element typing later). [3]- UMD builds are removed (script-tag usage must switch to ESM-based CDNs). [3]
- Libraries that rely on React internals may break under React 19 (common in some rendering/animation/3D libs). [3]
- TypeScript type breaking changes + codemods:
- React 19 cleans up/changes types; React recommends running
types-react-codemod. - Notably,
useReducertype arguments usage changes (often fix is “don’t pass generic args”). [3]
- React 19 cleans up/changes types; React recommends running
React 19.2-specific note (often paired with Next.js 16)
- React 19.2 adds features like
<Activity />anduseEffectEvent, which Next.js 16 highlights as supported. [1][4]
🌐 Web query:
Next.js 16 vs 15 breaking changes
💡 Result:
Breaking changes in Next.js 15 (vs 14)
- React 19 is required (minimum
react/react-domis 19). [2] - Async Request APIs introduced:
cookies(),headers(),draftMode(), plusparams/searchParamsin App Router became async but still had temporary synchronous compatibility to ease migration. [2] runtime: 'experimental-edge'no longer allowed (must useedge). [2]fetchis no longer cached by default (you must opt into caching viacache: 'force-cache'orexport const fetchCache = ...). [2]
Breaking changes in Next.js 16 (vs 15)
- Async Request APIs: synchronous access removed (must be fully async everywhere for
cookies,headers,draftMode,params,searchParams). [1] - Node / platform minimums bumped: Node.js 20.9+ (Node 18 dropped), TypeScript 5.1+, and newer minimum browser versions. [1]
middleware→proxy:middlewarefilename (and named export) is deprecated in favor ofproxy, with config flag renames; also noteproxyis Node.js runtime only (edge not supported inproxy). [1]- Metadata route params become async in more places:
opengraph-image/twitter-image/icon/apple-icon:params(and sometimesid) are now Promises. [1]sitemap:idparameter is now a Promise. [1]
next/imagebreaking/security/default changes (not exhaustive):- Local images with query strings require
images.localPatterns.search. [1] - Defaults changed:
images.minimumCacheTTL(60s → 4h),images.imageSizes(removes 16),images.qualities(now defaults to[75]), defaultimages.maximumRedirects(unlimited → 3), and local IP optimization is blocked by default unless explicitly allowed. [1]
- Local images with query strings require
- Parallel Routes now require explicit
default.jsfor every slot (build fails otherwise). [1] - Removals (previously deprecated):
- AMP support removed
next lintcommand removed (andnext buildno longer runs linting)serverRuntimeConfig/publicRuntimeConfigremovedexperimental.dynamicIOremoved/renamed tocacheComponentsunstable_rootParamsremoved [1]
Sources: Next.js upgrade guides for v16 and v15. [1] [2]
Next.js 16 introduces breaking changes requiring code updates—verify the app handles them.
All specified versions (next@16.0.1, react@19.2.0, typescript@5.9.3, @types/node@24.9.2) are published and compatible. However, Next.js 16 has significant breaking changes from v15 that may cause build/runtime failures if not addressed:
- Async Request APIs (
cookies(),headers(),draftMode(),params,searchParams) now requireawaitthroughout App Router components and route handlers - Metadata routes (
opengraph-image,twitter-image,icon, etc.) receive asyncparamsas Promises - Minimum requirements: Node.js 20.9+ and TypeScript 5.1+ (ensure CI/deployments support this)
next/imagechanges: local images with query strings requireimages.localPatterns.search; cache TTL defaults changed (60s → 4h); local IP optimization blocked by default- Parallel Routes now require explicit
default.jsin every slot next lintcommand removed (no longer runs during build)
Ensure the benchmark app's code is updated to handle these changes before merging.
🤖 Prompt for AI Agents
In `@benchmarks/c15t-nextjs/package.json` around lines 12 - 24, Update the
benchmark app to be compatible with Next.js 16 by (1) auditing server and App
Router code to await async request APIs (e.g., calls to cookies(), headers(),
draftMode(), access to params/searchParams) and update route handlers/components
that use them, (2) updating metadata routes to handle async params (treat params
as Promises in opengraph-image/twitter-image/icon handlers), (3) ensuring
CI/deploy uses Node >=20.9 and TypeScript >=5.1 (package.json dev/proc configs),
(4) adjusting next.config.js to account for next/image changes (add
images.localPatterns.search or equivalent and review cache TTL/IP optimization
settings), (5) adding explicit default.js files for any Parallel Routes slots,
and (6) removing or replacing any usage of the removed next lint command in
build scripts; search for symbols like cookies(), headers(), draftMode(),
next.config.js, metadata route handlers, and any build scripts invoking "next
lint" to locate and fix the code.
…ight, Turbo, and Drizzle Kit. Refactor package.json and pnpm-lock.yaml to use catalog references for type definitions and Next.js, enhancing consistency and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
benchmarks/with-iubenda/package.json (1)
7-7:⚠️ Potential issue | 🟡 MinorPort conflict with
benchmarks/iubenda.Both
with-iubendaandiubendause port 3001 for development. This will cause conflicts if running both simultaneously. Consider assigning unique ports.
🤖 Fix all issues with AI agents
In `@benchmarks/c15t-nextjs/package.json`:
- Line 7: The dev script in package.json currently uses a conflicting port
("dev": "next dev --port 3001"); update the "dev" npm script to use a unique,
non-conflicting port (e.g., change the --port value to 3008 or higher) so it
doesn't clash with other benchmarks; edit the "dev" entry in package.json to the
new port and ensure any documentation or CI that expects port 3001 is updated
accordingly.
- Around line 12-16: Update the dependency entry for "@c15t/nextjs" in
package.json from "1.7.1" to "1.8.1" to obtain Next.js 16 compatibility; modify
the "dependencies" block where "@c15t/nextjs" is declared and then
reinstall/update the lockfile (e.g., run your package manager install) so the
new version is reflected in the lockfile.
In `@benchmarks/iubenda/package.json`:
- Around line 4-9: Add a "benchmark" npm script to the package.json scripts
object and add the `@cookiebench/cli` devDependency so the project can be run via
the Cookiebench CLI; specifically, add a "benchmark": "cookiebench" entry under
"scripts" and add "@cookiebench/cli" to "devDependencies" (with an appropriate
version used by sibling projects), then run install to update lockfiles.
Reference: package.json -> scripts (add "benchmark") and package.json ->
devDependencies (add "@cookiebench/cli").
In `@package.json`:
- Around line 27-29: The root package.json's engines.node currently reads ">=18"
which conflicts with the stricter requirement of the workspace packages under
/benchmarks/* that require ">=20.9.0"; update the root "engines.node" entry (the
engines field in package.json) to ">=20.9.0" so the root matches the workspace
packages and avoids CI/install failures, keeping the existing "packageManager":
"pnpm@10.29.3" unchanged.
In `@packages/runner/package.json`:
- Around line 1-33: Add a Node engines constraint to package.json to match other
benchmark packages requiring Node >=20.9.0: update the package root (package
name "@consentio/runner") to include an "engines" field with "node": ">=20.9.0".
Ensure the constraint is placed alongside existing top-level fields (name,
version, type, exports, etc.) so package managers and consumers enforce the same
Node version as the rest of the workspace.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (38)
benchmarks/baseline/package.jsonbenchmarks/c15t-nextjs/package.jsonbenchmarks/c15t-react/package.jsonbenchmarks/cookie-control/package.jsonbenchmarks/cookie-yes/package.jsonbenchmarks/didomi/package.jsonbenchmarks/enzuzo/package.jsonbenchmarks/iubenda/package.jsonbenchmarks/ketch/package.jsonbenchmarks/onetrust/package.jsonbenchmarks/osano/package.jsonbenchmarks/usercentrics/package.jsonbenchmarks/with-c15t-nextjs/package.jsonbenchmarks/with-c15t-react/package.jsonbenchmarks/with-cookie-control/app/layout.tsxbenchmarks/with-cookie-control/config.jsonbenchmarks/with-cookie-control/next.config.tsbenchmarks/with-cookie-control/package.jsonbenchmarks/with-cookie-control/tsconfig.jsonbenchmarks/with-cookie-yes/package.jsonbenchmarks/with-didomi/package.jsonbenchmarks/with-enzuzo/app/layout.tsxbenchmarks/with-enzuzo/config.jsonbenchmarks/with-enzuzo/next.config.tsbenchmarks/with-enzuzo/package.jsonbenchmarks/with-enzuzo/tsconfig.jsonbenchmarks/with-iubenda/package.jsonbenchmarks/with-ketch/package.jsonbenchmarks/with-onetrust/package.jsonbenchmarks/with-osano/package.jsonbenchmarks/with-usercentrics/package.jsonpackage.jsonpackages/benchmark/package.jsonpackages/cli/package.jsonpackages/cookiebench-cli/package.jsonpackages/runner/package.jsonpackages/shared/package.jsonpnpm-workspace.yaml
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: danielsitek
Repo: consentdotio/benchmarks PR: 6
File: benchmarks/with-dgp-cookie-consent/config.json:37-39
Timestamp: 2025-11-07T16:19:37.939Z
Learning: In the cookiebench/benchmark-schema package (packages/benchmark-schema/schema.json), the includes.backend property accepts either a string or an array of strings (oneOf), not a boolean.
📚 Learning: 2025-11-07T16:17:07.589Z
Learnt from: danielsitek
Repo: consentdotio/benchmarks PR: 6
File: benchmarks/with-dgp-cookie-consent/config.json:15-20
Timestamp: 2025-11-07T16:17:07.589Z
Learning: In the cookiebench/benchmark-schema package (packages/benchmark-schema/schema.json), the valid enum values for bundleType are: "esm", "cjs", "iffe", and "bundled". Note that it's "iffe" (not "iife").
Applied to files:
benchmarks/with-cookie-control/config.jsonbenchmarks/with-enzuzo/config.jsonpackages/benchmark/package.jsonbenchmarks/baseline/package.jsonbenchmarks/cookie-yes/package.jsonbenchmarks/cookie-control/package.jsonpackages/cookiebench-cli/package.jsonbenchmarks/iubenda/package.jsonbenchmarks/with-c15t-nextjs/package.jsonpackage.json
📚 Learning: 2025-11-07T16:19:37.939Z
Learnt from: danielsitek
Repo: consentdotio/benchmarks PR: 6
File: benchmarks/with-dgp-cookie-consent/config.json:37-39
Timestamp: 2025-11-07T16:19:37.939Z
Learning: In the cookiebench/benchmark-schema package (packages/benchmark-schema/schema.json), the includes.backend property accepts either a string or an array of strings (oneOf), not a boolean.
Applied to files:
benchmarks/with-cookie-control/config.jsonbenchmarks/with-enzuzo/config.json
🧬 Code graph analysis (2)
benchmarks/with-cookie-control/app/layout.tsx (1)
benchmarks/with-enzuzo/app/layout.tsx (1)
metadata(4-6)
benchmarks/with-enzuzo/app/layout.tsx (1)
benchmarks/with-cookie-control/app/layout.tsx (1)
metadata(4-6)
🔇 Additional comments (37)
benchmarks/with-cookie-control/config.json (1)
2-38: Looks good — schema-aligned values and consistent config.No issues found; values match the expected schema constraints for
bundleTypeandincludes.backend.benchmarks/with-cookie-control/tsconfig.json (1)
2-10: Formatting-only change; config remains intact.No functional changes detected in this tsconfig; looks good.
benchmarks/with-enzuzo/app/layout.tsx (1)
1-18: LGTM! Style-only changes.The modifications are purely cosmetic—converting single quotes to double quotes and adjusting indentation. No functional, logical, or type-related changes have been made. The Next.js 15 layout structure and TypeScript types are correct.
benchmarks/with-cookie-control/app/layout.tsx (1)
1-5: No functional impact; consistent metadata and import style.
Looks good. The changes are purely stylistic and keep behavior intact.benchmarks/with-cookie-control/next.config.ts (1)
1-7: LGTM!The Next.js configuration is correctly structured. The import and export are valid, and the empty configuration object is appropriate for a benchmark project scaffold.
benchmarks/with-enzuzo/next.config.ts (1)
1-7: LGTM!Standard Next.js configuration boilerplate with proper TypeScript typing. The empty config is appropriate for a benchmark project that doesn't require custom configuration.
benchmarks/with-enzuzo/config.json (1)
1-40: LGTM!The configuration correctly follows the benchmark-schema. Based on learnings,
bundleType: "iffe"is the valid enum value (not "iife"), andincludes.backendcorrectly uses an array of strings as permitted by the schema.benchmarks/with-ketch/package.json (1)
12-24: LGTM!The migration to
catalog:placeholders for dependency versions is correctly implemented. This centralizes version management through the workspace catalog, improving maintainability across benchmark packages.benchmarks/with-enzuzo/package.json (1)
12-25: LGTM!Consistent application of the
catalog:placeholder pattern for centralized dependency management. The package structure is correct and aligns with other benchmark packages in this PR.benchmarks/with-enzuzo/tsconfig.json (1)
1-11: LGTM!Standard Next.js TypeScript configuration with appropriate includes, excludes, and path aliases. The extension from the shared ts-config promotes consistency across benchmark packages.
benchmarks/with-c15t-nextjs/package.json (1)
13-27: LGTM!The
catalog:migration is correctly applied. Keeping@c15t/nextjsat a pinned version (1.8.1) is appropriate since it's the package under benchmark—you want reproducible measurements against a specific version.benchmarks/enzuzo/package.json (1)
4-9: This is an intentional architectural pattern, not an oversight.The
"benchmark"script and@cookiebench/clidependency are only present inwith-*packages (e.g.,with-enzuzo,with-didomi, etc.), not in base packages (e.g.,enzuzo,didomi). This is a deliberate design where base packages contain the benchmark implementation without CLI integration, andwith-*variants provide the CLI integration for benchmarking. Theenzuzopackage follows this established pattern correctly.Likely an incorrect or invalid review comment.
pnpm-workspace.yaml (1)
6-13: All catalog package versions are valid and published on npm. No issues found.benchmarks/with-osano/package.json (1)
12-24: LGTM!The migration to
catalog:placeholders for centralized version management is clean and consistent with the broader PR objective.benchmarks/with-iubenda/package.json (1)
12-24: LGTM!The catalog: migration is consistent with the PR-wide standardization effort.
packages/cli/package.json (1)
20-34: LGTM!Dependency updates are reasonable minor version bumps. The hybrid approach of using
catalog:for shared dependencies (@types/node,typescript) while keeping explicit versions for CLI-specific packages is a sensible pattern.benchmarks/baseline/package.json (1)
1-28: LGTM!The baseline project appropriately omits the benchmark CLI tooling since it serves as a reference without consent management integration. The
catalog:migration andenginesconstraint are consistent with the PR's standardization goals.benchmarks/osano/package.json (1)
1-27: LGTM otherwise.The manifest structure is clean with appropriate
enginesconstraint andcatalog:usage for dependencies.benchmarks/c15t-react/package.json (2)
1-28: LGTM otherwise.The manifest follows the established pattern with appropriate
enginesconstraint and catalog-based dependency resolution.
11-16: No action required.@c15t/react@1.7.1is fully compatible with the catalog-resolved React 19.2.4 (package documentation confirms support for React 19, 18, 17, and 16.8, and v1.7.1 is a patch release with no React version support changes).benchmarks/usercentrics/package.json (1)
1-26: Looks good — manifest is consistent with the catalog-based setup.benchmarks/with-c15t-react/package.json (1)
15-17: Catalog-based dependency switch looks correct.Also applies to: 23-26
benchmarks/cookie-control/package.json (1)
1-26: LGTM — benchmark manifest is well-formed.benchmarks/didomi/package.json (1)
1-27: Looks good — dependencies are clear and consistent.benchmarks/ketch/package.json (1)
1-26: LGTM — benchmark package manifest is straightforward.packages/cookiebench-cli/package.json (1)
1-38: Looks good — CLI manifest is coherent (exports/bin/scripts align).benchmarks/with-usercentrics/package.json (1)
13-15: Catalog placeholder update looks good.Also applies to: 21-24
benchmarks/cookie-yes/package.json (1)
1-26: LGTM — benchmark manifest is consistent with the suite.benchmarks/with-didomi/package.json (1)
14-16: LGTM — catalog placeholders are consistent with workspace resolution.Also applies to: 21-24
benchmarks/with-cookie-yes/package.json (1)
13-16: LGTM — catalog placeholders match the monorepo dependency strategy.Also applies to: 21-24
benchmarks/onetrust/package.json (1)
1-26: LGTM — manifest is consistent, and the Node engine constraint is defined.benchmarks/with-onetrust/package.json (1)
13-16: LGTM — catalog placeholders match the monorepo resolution approach.Also applies to: 21-24
benchmarks/with-cookie-control/package.json (1)
13-16: LGTM — catalog placeholders are consistent with workspace dependency management.Also applies to: 21-24
packages/benchmark/package.json (1)
1-33: LGTM — manifest structure and exports are consistent with the new packages.packages/shared/package.json (1)
1-27: LGTM — manifest looks consistent and ready for workspace resolution.package.json (2)
15-25: LGTM: workspace/tooling dependency additions look coherent.The workspace references align with the updated cookiebench scripts, and explicit version pins keep tooling deterministic.
5-12: No action needed. ThecookiebenchCLI binary is properly exported by thepackages/cookiebench-cliworkspace package (bin entry:{'cookiebench': 'dist/index.mjs'}) and is correctly included as a devDependency ("cookiebench": "workspace:*"). The scripts on lines 5–12 will execute correctly.Likely an incorrect or invalid review comment.
| "private": true, | ||
| "scripts": { | ||
| "build": "next build", | ||
| "dev": "next dev --port 3001", |
There was a problem hiding this comment.
Port 3001 conflicts with other benchmarks.
Port 3001 is already used by iubenda and with-iubenda. Consider using a unique port (e.g., 3008 or higher based on existing allocations).
🤖 Prompt for AI Agents
In `@benchmarks/c15t-nextjs/package.json` at line 7, The dev script in
package.json currently uses a conflicting port ("dev": "next dev --port 3001");
update the "dev" npm script to use a unique, non-conflicting port (e.g., change
the --port value to 3008 or higher) so it doesn't clash with other benchmarks;
edit the "dev" entry in package.json to the new port and ensure any
documentation or CI that expects port 3001 is updated accordingly.
| "dependencies": { | ||
| "@c15t/nextjs": "1.7.1", | ||
| "next": "catalog:", | ||
| "react": "catalog:", | ||
| "react-dom": "catalog:" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and read pnpm-workspace.yaml to check catalog
fd -t f 'pnpm-workspace.yaml' -o 'pnpm-workspace.yml' --exec cat {}Repository: consentdotio/benchmarks
Length of output: 222
🏁 Script executed:
# Check root package.json for catalog definitions
cat package.json | head -100Repository: consentdotio/benchmarks
Length of output: 894
🏁 Script executed:
# Search for catalog sections in any workspace config files
rg "catalog" -A 10 --max-count=20 -t yaml -t jsonRepository: consentdotio/benchmarks
Length of output: 27898
🌐 Web query:
@c15t/nextjs version 1.7.1 npm package peer dependencies
💡 Result:
I couldn’t fetch the exact peerDependencies field for @c15t/nextjs@1.7.1 from the npm registry in this environment (npm pages/registry endpoints were blocked), but c15t’s published prerequisites indicate it expects:
next>= 13.5.4react>= 18- (and Node.js >= 18.17.0) — not a peer dep, but a runtime requirement
[1][2]
To get the authoritative peer dependency list for exactly v1.7.1 on your machine, run:
npm view `@c15t/nextjs`@1.7.1 peerDependencies --jsonor with pnpm:
pnpm view `@c15t/nextjs`@1.7.1 peerDependencies --jsonSources: [1] [2]
🏁 Script executed:
# Check the specific resolved version in lock file for `@c15t/nextjs`
rg '@c15t/nextjs' pnpm-lock.yaml -A 3 | head -20Repository: consentdotio/benchmarks
Length of output: 1203
Upgrade @c15t/nextjs to 1.8.1 for Next.js 16.1.6 compatibility.
@c15t/nextjs@1.7.1 has peer dependency next: ^15.0.0 || ^14.0.0 || ^13.0.0 and does not officially support the catalog-resolved next@16.1.6. Version 1.8.1 adds support for ^16.0.0 and should be used instead.
🤖 Prompt for AI Agents
In `@benchmarks/c15t-nextjs/package.json` around lines 12 - 16, Update the
dependency entry for "@c15t/nextjs" in package.json from "1.7.1" to "1.8.1" to
obtain Next.js 16 compatibility; modify the "dependencies" block where
"@c15t/nextjs" is declared and then reinstall/update the lockfile (e.g., run
your package manager install) so the new version is reflected in the lockfile.
| "scripts": { | ||
| "build": "next build", | ||
| "dev": "next dev --port 3001", | ||
| "fmt": "biome format . --write", | ||
| "lint": "biome lint .", | ||
| "start": "next start" |
There was a problem hiding this comment.
Missing benchmark script and @cookiebench/cli dependency.
This benchmark project is missing the benchmark script and @cookiebench/cli devDependency that are present in similar projects like with-iubenda and with-osano. If this project should be runnable via the CLI, add them:
Proposed fix
"scripts": {
+ "benchmark": "pnpm exec benchmark-cli benchmark",
"build": "next build",
"dev": "next dev --port 3001",And in devDependencies:
"devDependencies": {
"@cookiebench/benchmark-schema": "workspace:*",
+ "@cookiebench/cli": "workspace:*",
"@cookiebench/ts-config": "workspace:*",🤖 Prompt for AI Agents
In `@benchmarks/iubenda/package.json` around lines 4 - 9, Add a "benchmark" npm
script to the package.json scripts object and add the `@cookiebench/cli`
devDependency so the project can be run via the Cookiebench CLI; specifically,
add a "benchmark": "cookiebench" entry under "scripts" and add
"@cookiebench/cli" to "devDependencies" (with an appropriate version used by
sibling projects), then run install to update lockfiles. Reference: package.json
-> scripts (add "benchmark") and package.json -> devDependencies (add
"@cookiebench/cli").
| "packageManager": "pnpm@10.29.3", | ||
| "engines": { | ||
| "node": ">=18" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# List node engine constraints across the repo.
python - <<'PY'
import json, pathlib
for path in pathlib.Path(".").rglob("package.json"):
try:
data = json.loads(path.read_text())
except Exception:
continue
node = (data.get("engines") or {}).get("node")
if node:
print(f"{path}: {node}")
PYRepository: consentdotio/benchmarks
Length of output: 607
🌐 Web query:
What is the minimum supported Node.js version for pnpm 10.29.3?
💡 Result:
pnpm 10.29.3 requires Node.js >= 18.12.0 (minimum supported version). [1][2]
🌐 Web query:
What is the minimum supported Node.js version for Playwright 1.58.2?
💡 Result:
Playwright 1.58.2 requires Node.js 18 or newer (package.json engines.node is >=18). [1]
Sources:
[1] (app.unpkg.com)
Citations:
Update engines.node to ">=20.9.0" to match workspace package requirements.
The root engines.node is set to ">=18" (Line 29), but all 13 workspace packages under /benchmarks/* require >=20.9.0. This mismatch will cause install/CI failures when Node 18–20.8 is used. While pnpm@10.29.3 and Playwright meet the ">=18" requirement, the workspace packages' stricter constraints take precedence.
🤖 Prompt for AI Agents
In `@package.json` around lines 27 - 29, The root package.json's engines.node
currently reads ">=18" which conflicts with the stricter requirement of the
workspace packages under /benchmarks/* that require ">=20.9.0"; update the root
"engines.node" entry (the engines field in package.json) to ">=20.9.0" so the
root matches the workspace packages and avoids CI/install failures, keeping the
existing "packageManager": "pnpm@10.29.3" unchanged.
| { | ||
| "name": "@consentio/runner", | ||
| "version": "0.0.1", | ||
| "type": "module", | ||
| "exports": { | ||
| ".": { | ||
| "import": "./dist/index.js", | ||
| "types": "./dist/index.d.ts" | ||
| } | ||
| }, | ||
| "main": "./dist/index.js", | ||
| "module": "./dist/index.js", | ||
| "types": "./dist/index.d.ts", | ||
| "scripts": { | ||
| "build": "rslib build", | ||
| "check-types": "tsc --noEmit", | ||
| "dev": "rslib build --watch", | ||
| "fmt": "biome format . --write", | ||
| "lint": "biome lint ." | ||
| }, | ||
| "dependencies": { | ||
| "@c15t/logger": "1.0.0", | ||
| "@consentio/benchmark": "workspace:*", | ||
| "@consentio/shared": "workspace:*", | ||
| "@playwright/test": "^1.58.2", | ||
| "playwright-performance-metrics": "^1.2.5" | ||
| }, | ||
| "devDependencies": { | ||
| "@rsdoctor/rspack-plugin": "^1.5.2", | ||
| "@rslib/core": "^0.16.1", | ||
| "@types/node": "catalog:", | ||
| "typescript": "catalog:" | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding a Node engines constraint for consistency.
This would align @consentio/runner with other benchmark packages in the PR that require Node >=20.9.0.
♻️ Suggested change
"devDependencies": {
"@rsdoctor/rspack-plugin": "^1.5.2",
"@rslib/core": "^0.16.1",
"@types/node": "catalog:",
"typescript": "catalog:"
- }
+ },
+ "engines": {
+ "node": ">=20.9.0"
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| "name": "@consentio/runner", | |
| "version": "0.0.1", | |
| "type": "module", | |
| "exports": { | |
| ".": { | |
| "import": "./dist/index.js", | |
| "types": "./dist/index.d.ts" | |
| } | |
| }, | |
| "main": "./dist/index.js", | |
| "module": "./dist/index.js", | |
| "types": "./dist/index.d.ts", | |
| "scripts": { | |
| "build": "rslib build", | |
| "check-types": "tsc --noEmit", | |
| "dev": "rslib build --watch", | |
| "fmt": "biome format . --write", | |
| "lint": "biome lint ." | |
| }, | |
| "dependencies": { | |
| "@c15t/logger": "1.0.0", | |
| "@consentio/benchmark": "workspace:*", | |
| "@consentio/shared": "workspace:*", | |
| "@playwright/test": "^1.58.2", | |
| "playwright-performance-metrics": "^1.2.5" | |
| }, | |
| "devDependencies": { | |
| "@rsdoctor/rspack-plugin": "^1.5.2", | |
| "@rslib/core": "^0.16.1", | |
| "@types/node": "catalog:", | |
| "typescript": "catalog:" | |
| } | |
| { | |
| "name": "@consentio/runner", | |
| "version": "0.0.1", | |
| "type": "module", | |
| "exports": { | |
| ".": { | |
| "import": "./dist/index.js", | |
| "types": "./dist/index.d.ts" | |
| } | |
| }, | |
| "main": "./dist/index.js", | |
| "module": "./dist/index.js", | |
| "types": "./dist/index.d.ts", | |
| "scripts": { | |
| "build": "rslib build", | |
| "check-types": "tsc --noEmit", | |
| "dev": "rslib build --watch", | |
| "fmt": "biome format . --write", | |
| "lint": "biome lint ." | |
| }, | |
| "dependencies": { | |
| "@c15t/logger": "1.0.0", | |
| "@consentio/benchmark": "workspace:*", | |
| "@consentio/shared": "workspace:*", | |
| "@playwright/test": "^1.58.2", | |
| "playwright-performance-metrics": "^1.2.5" | |
| }, | |
| "devDependencies": { | |
| "@rsdoctor/rspack-plugin": "^1.5.2", | |
| "@rslib/core": "^0.16.1", | |
| "@types/node": "catalog:", | |
| "typescript": "catalog:" | |
| }, | |
| "engines": { | |
| "node": ">=20.9.0" | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@packages/runner/package.json` around lines 1 - 33, Add a Node engines
constraint to package.json to match other benchmark packages requiring Node
>=20.9.0: update the package root (package name "@consentio/runner") to include
an "engines" field with "node": ">=20.9.0". Ensure the constraint is placed
alongside existing top-level fields (name, version, type, exports, etc.) so
package managers and consumers enforce the same Node version as the rest of the
workspace.
…ne benchmark configurations. Includes changes to bundle types, TypeScript settings, and enhancements to cookie banner detection across various projects.
There was a problem hiding this comment.
Actionable comments posted: 18
🤖 Fix all issues with AI agents
In `@benchmarks/didomi/app/layout.tsx`:
- Around line 7-9: The DEBUG_DIDOMI constant currently reads
process.env.DEBUG_DIDOMI which is not exposed to the browser in this client
component; update the definition of DEBUG_DIDOMI (the constant that currently
checks process.env.NEXT_PUBLIC_DEBUG_DIDOMI and process.env.DEBUG_DIDOMI) to
only use NEXT_PUBLIC_DEBUG_DIDOMI for client-side checks (i.e., remove the
process.env.DEBUG_DIDOMI branch), or alternatively compute the debug flag on the
server and pass it into the client component via props if you need a non-public
value.
In `@benchmarks/enzuzo/app/layout.tsx`:
- Around line 17-20: Replace the raw HTML <script> tag in the layout component
with Next.js's Script component to match other benchmarks: import Script from
'next/script' (or use the existing import) and render <Script
src="https://app.enzuzo.com/scripts/cookiebar/ede431e2-968b-11eb-9cc0-2358d2c9e564"
strategy="afterInteractive" /> in place of the current script element so loading
behavior is controlled consistently; if the raw tag was intentional for a
specific benchmark, leave as-is and add a comment explaining why.
In `@packages/benchmark/src/bundle-strategy.ts`:
- Around line 23-30: The isArrayWithModules check includes unreachable
IIFE/IFFE/legacyIifeType branches; update the condition in the
isArrayWithModules boolean (which inspects rawBundleType and BUNDLE_TYPES) to
remove BUNDLE_TYPES.IIFE, BUNDLE_TYPES.IFFE, and legacyIifeType checks so it
only tests for Array.isArray(rawBundleType) &&
(rawBundleType.includes(BUNDLE_TYPES.ESM) ||
rawBundleType.includes(BUNDLE_TYPES.CJS) ||
rawBundleType.includes(BUNDLE_TYPES.BUNDLED)); keep the variable name
isArrayWithModules and other surrounding logic unchanged.
In `@packages/cookiebench-cli/README.md`:
- Around line 284-286: The README example sets includes.backend to a boolean
(false), but the benchmark-schema's includes.backend accepts a string or array
of strings; update the example in the README so includes.backend is a string or
string array (for example change "backend": false to "backend": "" or "backend":
[]), ensuring the example matches the oneOf schema for includes.backend.
In `@packages/cookiebench-cli/src/commands/benchmark.ts`:
- Around line 583-590: The dynamic import in benchmark.ts is using the wrong
extension and should match the build output; update the import that loads
resultsCommand (the await import(...) call that destructures resultsCommand) to
reference the .mjs output file instead of ".js" so ESM can resolve the module at
runtime (i.e., change the import path to the .mjs filename).
In `@packages/cookiebench-cli/src/commands/results.ts`:
- Around line 483-485: The isBundled calculation currently inspects only
results[0] and can misclassify mixed runs; change it to derive across the whole
results array by using results.every(...) to ensure every iteration has
size.thirdParty === 0 (and guard for empty results, e.g. treat empty as false).
Update the isBundled variable (alongside related symbols like _bannerDetected
and results) to use this aggregated check so mixed samples are classified
correctly.
- Around line 348-365: The code currently accepts files where data.results may
be an empty array, causing later divide-by-zero/NaN averages; inside the loop
that processes files (the block that checks if (!(data.app && data.results)) and
assigns results[data.app] = data.results), add a guard to skip when data.results
is an empty array (e.g., if (!Array.isArray(data.results) || data.results.length
=== 0) { logger.warn(`Skipping empty or invalid results for ${file} (no
iterations)`); continue; }) so you only load non-empty result arrays and
preserve the existing duplicate-name warning and assignment logic.
- Around line 259-266: Replace the invalid string "unknown" used as the default
for bundleType with a schema-valid placeholder (e.g., an empty array [])
wherever the techStack default is constructed; specifically update the techStack
object literal (the code that sets techStack: config.techStack || { ... }) to
use bundleType: [] instead of "unknown", and make the same change for the other
occurrences that set a default bundleType in the scores-related code paths (the
analogous defaults in the scores.ts and utils/scoring.ts locations) so the value
matches the schema (either a valid enum string or an array).
- Around line 1033-1045: Change timeToFirstByte from optional number to number |
null in the scoring types (update the type where timeToFirstByte is declared in
utils/scoring.ts), then in results aggregation (the average computation that
currently uses
appResults.reduce((a,b)=>a+(b.timing.timeToFirstByte||0),0)/appResults.length)
and in scores.ts (the similar aggregation) replace the current default-0 logic
with the same filtering pattern used for interactionToNextPaint: map to
b.timing.timeToFirstByte, filter to keep only finite numbers (inp !== null &&
inp !== undefined && Number.isFinite(inp)), and compute the average or return
null if no valid values; finally update the scoring function in utils/scoring.ts
where timeToFirstByte is used (the scoring logic block around where INP is
handled) to accept and correctly handle null timeToFirstByte values using the
same null-check/branching used for interactionToNextPaint.
In `@packages/cookiebench-cli/src/commands/save.ts`:
- Line 537: Replace the hardcoded domSize: 1500 with the shared constant
DEFAULT_DOM_SIZE and add an import for DEFAULT_DOM_SIZE from the constants
module; specifically update the object/property setting where domSize is defined
(look for "domSize: 1500" in save.ts) to use DEFAULT_DOM_SIZE and add the import
statement (import { DEFAULT_DOM_SIZE } from "../utils/constants";) at the top of
the file so the value stays consistent with scores.ts.
In `@packages/cookiebench-cli/src/commands/scores.ts`:
- Around line 60-67: The fallback techStack object currently uses bundleType:
"unknown" which violates the benchmark schema; update the default in the
config.techStack fallback (the techStack object in scores.ts) to use a
schema-valid value (e.g., "bundled" or one of "esm","cjs","iffe") so validation
won't fail; change only the bundleType fallback value in the techStack default
to a valid enum and keep the rest of the fallback fields unchanged.
In `@packages/cookiebench-cli/src/utils/logger.ts`:
- Around line 136-140: The note helper currently does an unnecessary runtime
string-check and conversion for the message param; update extendedLogger.note
(the function that calls note(messageStr, title)) to directly pass the message
parameter to note without the typeof check or String(...) conversion since
message is already typed as string.
- Around line 131-134: The implemented extendedLogger.message currently accepts
only (message: string) but the CliExtensions.message type expects (...args:
unknown[]); update the implementation of extendedLogger.message to match the
signature and forward all args to the underlying logger (e.g., change to
extendedLogger.message = (...args: unknown[]) => { log.message(...args); }) or
alternatively change the CliExtensions.message type to accept a single string if
you intend only one param; ensure the symbol names extendedLogger.message,
CliExtensions.message and log.message are the ones updated so types and
implementation stay consistent.
In `@packages/cookiebench-cli/src/utils/project-root.ts`:
- Around line 22-35: The fallback in resolveBenchmarkPath can produce duplicate
"benchmarks/benchmarks/..." when appPath already starts with "benchmarks/";
update resolveBenchmarkPath to detect and normalize appPath (e.g., strip a
leading "benchmarks/" or use a conditional join) before returning
join(projectRoot, "benchmarks", appPath) so that when config.json isn't found on
directPath the final path does not duplicate the prefix; refer to the
resolveBenchmarkPath function and parameters appPath, projectRoot, and
directPath when making the change.
In `@packages/runner/src/benchmark-runner.ts`:
- Around line 354-369: Warmup reuses warmupPage across iterations causing
state/collector accumulation; instead create and close a fresh page each
iteration. Inside the WARMUP_ITERATIONS loop, replace use of warmupPage with a
per-iteration page created before calling runSingleBenchmark (e.g. via your page
factory or this.browser.newPage()), pass that page to runSingleBenchmark, then
close the page and call cleanupBetweenIterations() (and reset any
collectors/handlers if you have a reset method) to ensure each warmup iteration
runs in isolation.
In `@packages/runner/src/performance-aggregator.ts`:
- Around line 304-370: The return object in performance-aggregator.ts is
populating many third-party fields (thirdPartyRequests, thirdPartySize,
thirdPartyDomains, thirdPartyCookies, etc.) with hardcoded 0s; either compute
these from existing trace/har values (e.g., derive thirdPartyRequests from
filtering request records by origin, sum sizes into thirdPartySize, count unique
domains into thirdPartyDomains) using existing helpers like calculateTrimmedMean
and TRIM_PERCENT, or if not implemented yet add a clear TODO and remove them
from the public return type (and update the associated AggregatedMetrics/return
type) to avoid misleading consumers; locate the fields in the return block and
either replace the zero literals with computed values based on available arrays
(totalRequestsValues/totalSizeValues or request-level arrays) or annotate each
field with a TODO comment and remove from exported type/interface.
- Around line 21-23: STABILITY_THRESHOLD is declared but never used; update the
callers of isStable in logStatisticalSummary to use STABILITY_THRESHOLD instead
of VARIABILITY_WARNING_THRESHOLD (so the file-level stability constant is
applied), or remove the STABILITY_THRESHOLD constant if you prefer the existing
warning threshold to be the single source of truth; locate the references by the
symbols STABILITY_THRESHOLD, VARIABILITY_WARNING_THRESHOLD,
logStatisticalSummary, and isStable (and note statistics.ts supplies a default)
and either swap the argument to STABILITY_THRESHOLD or delete the unused
constant accordingly.
In `@packages/shared/README.md`:
- Around line 37-38: The README incorrectly documents getPackageManager as
returning a Promise; update the docs to match the actual synchronous function
signature by changing the documented return type from Promise<{ command: string;
args: string[] }> to { command: string; args: string[] } and ensure any
descriptive text referencing async behavior for getPackageManager is removed or
adjusted to reflect that getPackageManager() is synchronous.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (53)
METHODOLOGY.mdREADME.mdbenchmarks/c15t-nextjs/next.config.tsbenchmarks/c15t-react/config.jsonbenchmarks/cookie-control/config.jsonbenchmarks/cookie-yes/config.jsonbenchmarks/didomi/app/layout.tsxbenchmarks/enzuzo/app/layout.tsxbenchmarks/enzuzo/config.jsonbenchmarks/iubenda/config.jsonbenchmarks/ketch/config.jsonbenchmarks/ketch/next-env.d.tsbenchmarks/onetrust/config.jsonbenchmarks/osano/app/layout.tsxbenchmarks/osano/config.jsonbenchmarks/usercentrics/config.jsonbenchmarks/usercentrics/next-env.d.tspackage.jsonpackages/benchmark/package.jsonpackages/benchmark/src/bundle-strategy.tspackages/benchmark/src/constants.tspackages/benchmark/src/cookie-banner-collector.tspackages/benchmark/src/index.tspackages/benchmark/src/perfume-collector.tspackages/benchmark/src/types.tspackages/cookiebench-cli/README.mdpackages/cookiebench-cli/package.jsonpackages/cookiebench-cli/rslib.config.tspackages/cookiebench-cli/src/commands/benchmark.tspackages/cookiebench-cli/src/commands/db.tspackages/cookiebench-cli/src/commands/results.tspackages/cookiebench-cli/src/commands/save.tspackages/cookiebench-cli/src/commands/scores.tspackages/cookiebench-cli/src/components/intro.tspackages/cookiebench-cli/src/index.tspackages/cookiebench-cli/src/types/index.tspackages/cookiebench-cli/src/utils/auth.tspackages/cookiebench-cli/src/utils/index.tspackages/cookiebench-cli/src/utils/logger.tspackages/cookiebench-cli/src/utils/project-root.tspackages/cookiebench-cli/src/utils/scoring.tspackages/cookiebench-cli/tsconfig.base.jsonpackages/runner/src/benchmark-runner.tspackages/runner/src/performance-aggregator.tspackages/runner/src/server.tspackages/runner/src/statistics.tspackages/runner/src/types.tspackages/shared/README.mdpackages/shared/package.jsonpackages/shared/src/utils/config.tspackages/shared/src/utils/conversion.tspackages/shared/src/utils/package-manager.tstsconfig.json
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-07T16:17:07.589Z
Learnt from: danielsitek
Repo: consentdotio/benchmarks PR: 6
File: benchmarks/with-dgp-cookie-consent/config.json:15-20
Timestamp: 2025-11-07T16:17:07.589Z
Learning: In the cookiebench/benchmark-schema package (packages/benchmark-schema/schema.json), the valid enum values for bundleType are: "esm", "cjs", "iffe", and "bundled". Note that it's "iffe" (not "iife").
Applied to files:
benchmarks/osano/config.jsonpackages/cookiebench-cli/src/types/index.tspackages/benchmark/src/bundle-strategy.tsbenchmarks/c15t-react/config.jsonpackages/benchmark/src/constants.tsbenchmarks/usercentrics/config.jsonpackages/benchmark/package.jsonpackages/cookiebench-cli/package.jsonbenchmarks/cookie-control/config.jsonbenchmarks/ketch/config.jsonpackages/benchmark/src/types.tsREADME.mdpackages/cookiebench-cli/tsconfig.base.jsonbenchmarks/cookie-yes/config.jsonbenchmarks/enzuzo/config.jsonpackages/cookiebench-cli/src/commands/save.tstsconfig.jsonbenchmarks/onetrust/config.jsonpackage.jsonpackages/runner/src/types.tspackages/benchmark/src/index.tsbenchmarks/iubenda/config.json
📚 Learning: 2025-11-07T16:19:37.939Z
Learnt from: danielsitek
Repo: consentdotio/benchmarks PR: 6
File: benchmarks/with-dgp-cookie-consent/config.json:37-39
Timestamp: 2025-11-07T16:19:37.939Z
Learning: In the cookiebench/benchmark-schema package (packages/benchmark-schema/schema.json), the includes.backend property accepts either a string or an array of strings (oneOf), not a boolean.
Applied to files:
benchmarks/osano/config.jsonpackages/cookiebench-cli/src/types/index.tsbenchmarks/c15t-react/config.jsonbenchmarks/usercentrics/config.jsonbenchmarks/cookie-control/config.jsonbenchmarks/ketch/config.jsonbenchmarks/cookie-yes/config.jsonbenchmarks/enzuzo/config.jsonpackages/cookiebench-cli/src/commands/save.tsbenchmarks/onetrust/config.jsonbenchmarks/iubenda/config.json
🧬 Code graph analysis (12)
packages/cookiebench-cli/src/components/intro.ts (1)
packages/cookiebench-cli/src/utils/logger.ts (2)
logger(162-162)CliLogger(9-9)
packages/shared/src/utils/config.ts (1)
packages/cookiebench-cli/src/utils/index.ts (1)
readConfig(22-24)
benchmarks/osano/app/layout.tsx (2)
benchmarks/enzuzo/app/layout.tsx (2)
metadata(4-6)RootLayout(8-25)benchmarks/didomi/app/layout.tsx (1)
RootLayout(20-92)
packages/benchmark/src/bundle-strategy.ts (2)
packages/benchmark/src/types.ts (2)
Config(15-58)BundleStrategy(124-128)packages/benchmark/src/constants.ts (1)
BUNDLE_TYPES(76-82)
packages/cookiebench-cli/src/commands/scores.ts (7)
packages/cookiebench-cli/src/commands/results.ts (2)
BenchmarkOutput(180-214)RawBenchmarkDetail(38-178)packages/cookiebench-cli/src/types/index.ts (2)
BenchmarkScores(10-39)Config(5-5)packages/benchmark/src/types.ts (1)
Config(15-58)packages/cookiebench-cli/src/utils/index.ts (2)
HALF_SECOND(11-11)findProjectRoot(20-20)packages/cookiebench-cli/src/utils/project-root.ts (1)
findProjectRoot(4-20)packages/cookiebench-cli/src/utils/scoring.ts (2)
printScores(1383-1442)calculateScores(1010-1379)packages/cookiebench-cli/src/utils/constants.ts (1)
DEFAULT_DOM_SIZE(3-3)
packages/cookiebench-cli/src/commands/benchmark.ts (8)
packages/cookiebench-cli/src/types/index.ts (2)
BenchmarkResult(4-4)ServerInfo(6-6)packages/runner/src/types.ts (2)
BenchmarkResult(163-276)ServerInfo(20-23)packages/cookiebench-cli/src/utils/logger.ts (2)
logger(162-162)CliLogger(9-9)packages/cookiebench-cli/src/utils/index.ts (3)
PERCENTAGE_DIVISOR(14-14)findProjectRoot(20-20)resolveBenchmarkPath(20-20)packages/cookiebench-cli/src/utils/constants.ts (3)
DEFAULT_DOM_SIZE(3-3)DEFAULT_ITERATIONS(2-2)SEPARATOR_WIDTH(5-5)packages/runner/src/server.ts (2)
buildAndServeNextApp(6-119)cleanupServer(121-125)packages/cookiebench-cli/src/utils/scoring.ts (2)
calculateScores(1010-1379)printScores(1383-1442)packages/cookiebench-cli/src/utils/project-root.ts (2)
findProjectRoot(4-20)resolveBenchmarkPath(22-36)
benchmarks/enzuzo/app/layout.tsx (1)
benchmarks/osano/app/layout.tsx (2)
metadata(5-7)RootLayout(9-25)
packages/cookiebench-cli/src/commands/results.ts (7)
packages/benchmark/src/index.ts (1)
Config(15-15)packages/cookiebench-cli/src/types/index.ts (2)
Config(5-5)BenchmarkScores(10-39)packages/cookiebench-cli/src/utils/index.ts (6)
formatTime(9-9)formatBytes(8-8)KILOBYTE(12-12)PERCENTAGE_DIVISOR(14-14)ONE_SECOND(13-13)findProjectRoot(20-20)packages/cookiebench-cli/src/utils/constants.ts (15)
SCORE_THRESHOLD_POOR(10-10)SCORE_THRESHOLD_FAIR(11-11)CLS_DECIMAL_PLACES(7-7)CLS_THRESHOLD_GOOD(14-14)CLS_THRESHOLD_FAIR(15-15)COL_WIDTH_NAME(18-18)MAX_FILENAME_LENGTH(27-27)TRUNCATED_FILENAME_LENGTH(28-28)MIN_DURATION_THRESHOLD(31-31)COL_WIDTH_TYPE(19-19)COL_WIDTH_SOURCE(20-20)COL_WIDTH_SIZE(21-21)COL_WIDTH_DURATION(22-22)COL_WIDTH_TAGS(23-23)DEFAULT_DOM_SIZE(3-3)packages/cookiebench-cli/src/utils/project-root.ts (1)
findProjectRoot(4-20)packages/cookiebench-cli/src/utils/scoring.ts (1)
calculateScores(1010-1379)packages/cookiebench-cli/src/utils/auth.ts (1)
isAdminUser(5-11)
benchmarks/didomi/app/layout.tsx (1)
benchmarks/with-didomi/app/layout.tsx (1)
RootLayout(6-66)
packages/cookiebench-cli/src/utils/scoring.ts (3)
packages/cookiebench-cli/src/utils/index.ts (2)
formatTime(9-9)formatBytes(8-8)packages/shared/src/utils/conversion.ts (1)
formatBytes(17-28)packages/cookiebench-cli/src/types/index.ts (1)
BenchmarkScores(10-39)
packages/runner/src/performance-aggregator.ts (4)
packages/benchmark/src/types.ts (8)
CoreWebVitals(219-230)CookieBannerData(97-106)CookieBannerMetrics(82-95)NetworkRequest(109-116)NetworkMetrics(118-121)ResourceTimingData(131-216)Config(15-58)PerfumeMetrics(233-264)packages/runner/src/types.ts (10)
CoreWebVitals(12-12)CookieBannerData(10-10)CookieBannerMetrics(11-11)NetworkRequest(14-14)NetworkMetrics(13-13)ResourceTimingData(16-16)Config(8-8)PerfumeMetrics(15-15)BenchmarkDetails(26-161)BenchmarkResult(163-276)packages/cookiebench-cli/src/utils/index.ts (1)
PERCENTAGE_MULTIPLIER(15-15)packages/runner/src/statistics.ts (3)
isStable(140-146)calculateCoefficientOfVariation(120-135)calculateTrimmedMean(84-115)
packages/runner/src/types.ts (3)
packages/cookiebench-cli/src/types/index.ts (3)
ServerInfo(6-6)BenchmarkDetails(3-3)BenchmarkResult(4-4)packages/benchmark/src/index.ts (1)
BundleType(13-13)packages/benchmark/src/types.ts (1)
BundleType(13-13)
🪛 Gitleaks (8.30.0)
benchmarks/didomi/app/layout.tsx
[high] 71-71: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 LanguageTool
METHODOLOGY.md
[grammar] ~79-~79: Ensure spelling is correct
Context: ...aintains speed - Fast 3G (1.6 Mbps, 562ms latency): More realistic, significantly...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
packages/cookiebench-cli/README.md
[uncategorized] ~377-~377: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ice overhead 4. Transparency (15%): Open source status, documentation, licensing 5. **U...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.20.0)
packages/cookiebench-cli/README.md
[warning] 22-22: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 27-27: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 31-31: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 37-37: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 40-40: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (79)
benchmarks/enzuzo/app/layout.tsx (2)
1-6: LGTM!Type imports and metadata export follow the standard Next.js App Router pattern and are consistent with other benchmark files in this PR.
8-24: The previously flagged API key issue has been addressed.The commented-out code block containing the hardcoded API key (noted in the past review) has been removed. The current implementation correctly uses only the public script URL.
[duplicate_comment, approve_code_changes]benchmarks/didomi/app/layout.tsx (2)
12-18: LGTM!The
redactTokenhelper correctly handles edge cases and properly masks sensitive token data.
70-87: **API key is a public client-side key (static analysis false positive).**The Gitleaks alert is a false positive. The DidomiapiKeyis a public API key used for integrating Didomi's SDKs into applications and websites. The SDK documentation recommends instantiating it "as early as possible" in client-side code, confirming the key is designed to be embedded in browser JavaScript. This is expected for consent management SDKs and not a secret.packages/cookiebench-cli/rslib.config.ts (3)
10-15: Confirmdts: falsematches the intended consumers.If
cookiebench-cliexposes any programmatic API, disabling DTS will drop TypeScript typings. If it’s strictly a CLI, ensure notypesfield or library exports are advertised in its package.json.
17-22: No action needed. The bin/exports in package.json are already correctly aligned with the.mjsoutput configuration. The entry point "index" will output asdist/index.mjs, which matches all references in package.json (bin,exports, andmainall point to./dist/index.mjs). The configuration is correct.
4-9: Removesource.exclude: ["figlet"]—it doesn't prevent bundling.
source.excludeonly skips SWC transformation for matched files; it does not externalize or prevent bundling. Sincefigletis a runtime dependency, rslib's ESM format auto-externalizes it by default vialib.autoExternal: true. The exclude entry serves no purpose and can be removed.tsconfig.json (1)
4-28: Formatting-only change looks good.No behavioral impact; compiler options remain intact.
benchmarks/usercentrics/next-env.d.ts (1)
1-5: Standard Next.js ambient typings.Matches expected Next.js template.
packages/cookiebench-cli/tsconfig.base.json (1)
1-19: Solid base TS config for the CLI.Looks consistent and appropriate for NodeNext builds.
benchmarks/ketch/next-env.d.ts (1)
1-5: Standard Next.js ambient typings.No concerns.
benchmarks/cookie-control/config.json (1)
1-39: Config looks consistent with the benchmark schema.No issues found.
benchmarks/usercentrics/config.json (1)
1-39: Config looks consistent with the benchmark schema.No issues found.
benchmarks/c15t-nextjs/next.config.ts (1)
1-15: Minimal Next.js config looks good.No concerns.
benchmarks/iubenda/config.json (1)
1-45: Config structure looks consistent and complete.The metadata, tech stack, and includes sections are well-formed for the benchmark schema.
packages/shared/src/utils/config.ts (1)
4-34: Clear docs and safe failure behavior.The compile-time-only typing note and null-on-error behavior are well communicated and implemented.
packages/benchmark/package.json (1)
1-37: Package manifest looks solid.Exports, scripts, and dependencies are aligned for the new benchmark package.
benchmarks/onetrust/config.json (1)
1-46: Well-structured benchmark config.Selectors and metadata are comprehensive and consistent with the schema.
packages/shared/package.json (1)
1-28: Shared package manifest is coherent.Scripts and devDependencies align with the rest of the workspace tooling.
benchmarks/cookie-yes/config.json (1)
1-44: Config looks good, including remote settings.The benchmark metadata and detection rules are consistent with the schema pattern.
packages/cookiebench-cli/src/components/intro.ts (1)
1-94: Nice guard on the gradient calculation.Using
Math.max(1, lines.length - 1)prevents division-by-zero artifacts for single-line output.benchmarks/enzuzo/config.json (1)
1-40: Config is consistent with the benchmark schema.Sections and metadata are complete and aligned with other benchmark configs.
benchmarks/ketch/config.json (1)
1-39: Config looks consistent.
No issues spotted in this benchmark configuration.packages/shared/src/utils/conversion.ts (4)
8-10: bytesToKB is clear and correct.
17-28: formatBytes handles edge cases well.
35-37: decimalToPercentage is straightforward.
44-46: percentageToDecimal is straightforward.packages/cookiebench-cli/src/utils/project-root.ts (1)
4-20: findProjectRoot logic is sound.METHODOLOGY.md (1)
1-289: Methodology doc is thorough and clear.README.md (1)
9-52: README updates look good.Also applies to: 103-104
packages/shared/src/utils/package-manager.ts (1)
10-21: fromName helper is clean.packages/shared/README.md (1)
1-35: Docs are clear and helpful.benchmarks/c15t-react/config.json (1)
23-29:bundleTypearray is valid per schema.
The schema supports arrays forbundleTypevia aoneOfpattern. The array["esm", "cjs"]is valid according to the defined schema and will pass validation.Likely an incorrect or invalid review comment.
packages/cookiebench-cli/src/utils/auth.ts (1)
5-10: LGTM!The admin flag check now correctly normalizes the environment variable by trimming whitespace and converting to lowercase before comparison, addressing the previous review feedback.
packages/runner/src/server.ts (2)
20-46: LGTM!The build process now correctly handles spawn errors via the
buildProcess.once("error", onError)listener, preventing the promise from hanging indefinitely when the command fails to start.
68-118: LGTM!The readiness polling now properly implements:
- Early crash detection via
exit/errorlisteners on the server process- Per-request timeout using
AbortController(5 second timeout)- Proper cleanup (
serverProcess.kill()) on both crash detection and retry exhaustion- Listener removal to prevent memory leaks
packages/benchmark/src/constants.ts (1)
76-82: LGTM!The
BUNDLE_TYPES.IIFEnow correctly maps to"iffe"to match the schema enum values. The additionalIFFEalias provides flexibility for both common spellings while maintaining consistency with the schema.packages/cookiebench-cli/src/types/index.ts (1)
1-39: LGTM!The type definitions are well-structured with proper union types for
gradeandstatusfields. The re-exports from@consentio/runnerprovide a clean public API surface for CLI consumers.packages/cookiebench-cli/src/utils/index.ts (1)
1-24: LGTM!The barrel file provides a clean typed wrapper around the shared
readConfigutility, ensuring CLI consumers receive properly typedConfig | nullresults. The re-exports are logically organized.packages/cookiebench-cli/src/commands/db.ts (3)
32-52: LGTM!The
runDrizzleCommandhelper correctly usesexecFileSyncwith thecwdoption instead of shell string interpolation, avoiding path injection issues and handling paths with spaces correctly.
228-246: LGTM!The
migrationsPathis now correctly resolved relative toprojectRootusing theDB_PACKAGE_RELATIVE_PATHconstant, ensuring consistent path resolution regardless of the current working directory.
210-226: Do not remove the localfindProjectRootimplementation.The function in
utils/project-root.tshas different logic and requirements than the local implementation indb.ts. The utils version requirespnpm-workspace.yamlANDbenchmarksANDpackagesdirectories, while the local version requires(pnpm-workspace.yamlORpackage.json)ANDpackages. Additionally, the utils version accepts astartDirparameter and has different fallback behavior. These implementations are not interchangeable without changing the command's behavior.Likely an incorrect or invalid review comment.
packages/runner/src/types.ts (2)
163-173: LGTM!The
techStack.bundleTypeproperty is now correctly typed asBundleType | BundleType[]instead of the looserstring | string[], ensuring type safety and preventing invalid bundle type values.
1-17: LGTM!The type re-exports from
@consentio/benchmarkprovide a clean public API surface, and theServerInfotype properly encapsulates the server process and URL.packages/cookiebench-cli/src/utils/scoring.ts (3)
1193-1202: LGTM!The resource data array now uses
Array.from()with a factory function to create distinct objects for each element, avoiding the shared reference issue from usingArray.fill().
1381-1442: LGTM!The
printScoresfunction now properly outputs both tables viaconsole.log()and also prints insights and recommendations sections when available.
1-108: LGTM!The scoring module is well-structured with clearly defined types, documented weight constants, and appropriate biome-ignore comments for the necessarily complex scoring logic.
packages/runner/src/statistics.ts (3)
1-52: LGTM!The
calculateStatisticsfunction handles edge cases well with the early return for empty arrays and correctly computes statistical metrics. The sorted array indexing is safe given the empty check.
84-115: Trimming logic now properly guarded.The implementation correctly clamps
trimPercentto [0, 50] and further constrainstrimCountto avoid over-trimming. The fallback to median when trimming results in an empty array is a sensible safeguard.
117-146: LGTM!The coefficient of variation and stability check functions are straightforward and handle edge cases (empty array, zero mean) appropriately.
packages/cookiebench-cli/src/utils/logger.ts (1)
68-74: Redundant type cast addressed.The previous review comment about the unnecessary type cast has been resolved by using
String(logLevel)which is a safer approach for the default case.packages/runner/src/performance-aggregator.ts (2)
175-177: TTFB null handling now correct.The
timeToFirstByteis now correctly set tonullwhenperfumeMetricsis absent, and the averaging logic filters out null/non-positive values. This addresses the previous review feedback.
417-421: Banner render time clamping implemented.The
Math.max(0, ...)guard prevents negative durations from being logged when timing data is inconsistent. This addresses the previous review feedback.packages/runner/src/benchmark-runner.ts (2)
125-125: Network monitoring URL parameter added.The target URL is now correctly passed to
setupMonitoring, ensuring accurate first-party detection. This addresses the previous review feedback.
291-297: Command injection vulnerability correctly addressed.Using
execFileAsyncwith an argument array eliminates command injection risk by preventing shell interpretation. However,unzipis not available on Windows by default, which will cause failures on that platform. Either document this Unix/Linux-only requirement, or consider using a cross-platform Node.js library likeextract-zipornode-stream-zipfor broader compatibility.benchmarks/osano/config.json (1)
1-40: LGTM!The configuration is well-structured. The
bundleTypecorrectly uses"iffe"per the schema, and the website URLs are now consistent. Previous review feedback has been addressed.packages/benchmark/src/bundle-strategy.ts (1)
9-16: IIFE detection now handles schema and legacy values.The implementation correctly detects both the schema-compliant
"iffe"and the legacy"iife"values for backward compatibility. This addresses the previous review feedback.Note: Since
BUNDLE_TYPES.IIFEandBUNDLE_TYPES.IFFEare both"iffe"(per constants.ts), lines 10-11 and 14-15 are redundant but harmless.packages/cookiebench-cli/package.json (2)
1-40: LGTM on overall structure.The package manifest is well-structured with proper ESM configuration, workspace dependencies, and build tooling. The shebang issue from past reviews should be addressed in the source file (
src/index.ts), and@clack/promptsis now at stable version^1.0.1.
28-28: No action needed. Version17.3.1is the current latest stable release ofdotenvon npm, making^17.3.1a valid and appropriate dependency specification.Likely an incorrect or invalid review comment.
packages/cookiebench-cli/src/index.ts (1)
1-145: LGTM!The CLI entry point is well-structured:
- Shebang is correctly present at line 1 (addressing previous review).
- Interactive mode now includes the
scorescommand (lines 91-95), resolving the prior omission.- Admin-only commands (
save,db) are properly gated both in direct CLI and interactive modes.- Clean cancellation handling and error propagation.
packages/cookiebench-cli/src/commands/benchmark.ts (2)
34-42: LGTM on helper functions.The
calculateAveragefunction correctly handles empty arrays by returning 0, preventing division-by-zero errors. The metric calculation helpers (calculateTimingMetrics,calculateSizeMetrics, etc.) are well-structured and follow consistent patterns.
252-395: LGTM onrunSingleBenchmarkimplementation.Good practices observed:
- Server cleanup in
finallyblock ensures resources are released even on errors.- Graceful handling of
EEXISTwhen creating trace directory.- Proper distinction between remote and local benchmarking modes.
- Comprehensive error handling with informative messages.
packages/benchmark/src/cookie-banner-collector.ts (2)
239-263: LGTM - Detection timing issue resolved.The implementation now correctly starts detection immediately (line 257) and only adds a supplementary
DOMContentLoadedlistener when the document is still loading (lines 258-262). This ensures early-rendering banners are caught while still providing a second detection pass after DOM is fully parsed.
287-386: LGTM oncollectMetricsimplementation.Good practices:
- PerformanceObserver is properly disconnected to prevent memory leaks (lines 302-305).
- Viewport coverage calculation correctly handles intersection with viewport boundaries.
- Fallback to render time when visibility time isn't recorded (lines 329-330).
packages/benchmark/src/index.ts (1)
1-27: LGTM!The barrel file is well-organized with clear separation between utility exports and type exports. The biome ignore directive appropriately acknowledges this is an intentional barrel file.
packages/benchmark/src/perfume-collector.ts (2)
241-253: LGTM - FP metric casing fixed.The
firstPaintmetric now correctly reads fromrawMetrics.fp(lowercase), matching Perfume.js's actual emission format. Other metrics correctly use uppercase (FCP,LCP, etc.) as per Perfume.js conventions.
88-160: LGTM onloadPerfumeScriptimplementation.The fallback chain for locating
perfume.jsis robust:
- Tries multiple monorepo path patterns
- Falls back to
createRequirefor ESM module resolution- Returns empty string with warning if all attempts fail
This handles various monorepo layouts gracefully.
packages/cookiebench-cli/src/commands/scores.ts (1)
213-218: LGTM - Empty results guard added.The guard at lines 213-218 correctly handles the case where
appResultsis empty, preventing division-by-zero errors in the averaging calculations below. This addresses the previous review concern.packages/cookiebench-cli/src/commands/save.ts (3)
172-172: LGTM -bundleTypenow uses schema-valid default.The fallback
bundleTypeis now"bundled", which is a valid schema enum value, addressing the previous review concern.
405-408: LGTM - Empty results guard added.The guard correctly handles empty
appResultsby logging a warning and returning early, preventing division-by-zero errors in subsequent calculations.
446-456: LGTM - INP and third-party aggregates fixed.The code now correctly:
- Averages
interactionToNextPaintacross all valid iterations (lines 446-456)- Counts third-party requests from all resource types (scripts, styles, images, fonts, other) at lines 485-495
- Computes
thirdPartyDomainsby extracting unique hostnames from actual resources (lines 499-517)This addresses the previous review concerns about skewed scores.
Also applies to: 485-517
packages/benchmark/src/types.ts (5)
3-58: Clean, well-scoped configuration types.
The config and cookie banner shapes are clear and consistently structured.
60-128: Performance, cookie-banner, and network types look solid.
No issues spotted with these interface/alias definitions.
130-216: Resource timing schema is comprehensive and consistent.
The nested timing/size/resource structures are coherent.
218-264: Core Web Vitals and Perfume metrics types are clear.
Good coverage of required and optional fields.
266-282: Window augmentation is clean and safe.
Optional metrics map is appropriately typed.packages/cookiebench-cli/src/commands/results.ts (3)
37-214: Raw result and output shapes are well-defined.
Strong, explicit typing for benchmark payloads.
216-243: Directory traversal logic looks good.
The recursion and filtering are straightforward.
387-434: Formatting helpers are clear and consistent.
No issues spotted in the helper logic.
| const DEBUG_DIDOMI = | ||
| process.env.NEXT_PUBLIC_DEBUG_DIDOMI === "true" || | ||
| process.env.DEBUG_DIDOMI === "true"; |
There was a problem hiding this comment.
DEBUG_DIDOMI without NEXT_PUBLIC_ prefix won't work client-side.
In Next.js, only environment variables prefixed with NEXT_PUBLIC_ are exposed to the browser. Since this is a client component ("use client"), the check on line 9 will always be undefined client-side.
🔧 Suggested fix
const DEBUG_DIDOMI =
- process.env.NEXT_PUBLIC_DEBUG_DIDOMI === "true" ||
- process.env.DEBUG_DIDOMI === "true";
+ process.env.NEXT_PUBLIC_DEBUG_DIDOMI === "true";🤖 Prompt for AI Agents
In `@benchmarks/didomi/app/layout.tsx` around lines 7 - 9, The DEBUG_DIDOMI
constant currently reads process.env.DEBUG_DIDOMI which is not exposed to the
browser in this client component; update the definition of DEBUG_DIDOMI (the
constant that currently checks process.env.NEXT_PUBLIC_DEBUG_DIDOMI and
process.env.DEBUG_DIDOMI) to only use NEXT_PUBLIC_DEBUG_DIDOMI for client-side
checks (i.e., remove the process.env.DEBUG_DIDOMI branch), or alternatively
compute the debug flag on the server and pass it into the client component via
props if you need a non-public value.
| <script | ||
| src="https://app.enzuzo.com/scripts/cookiebar/ede431e2-968b-11eb-9cc0-2358d2c9e564" | ||
| type="text/javascript" | ||
| /> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using next/script for consistency with other benchmarks.
The osano benchmark uses the next/script component with strategy="afterInteractive", while this file uses a raw <script> tag. Using next/script provides better control over loading behavior and is the recommended approach in Next.js.
If the raw <script> approach is intentional for benchmarking purposes (e.g., comparing loading strategies), please disregard this suggestion.
♻️ Proposed refactor to use next/script
import type { Metadata } from "next";
import type { ReactNode } from "react";
+import Script from "next/script";
export const metadata: Metadata = {
title: "benchmark",
};
export default function RootLayout({
children,
}: Readonly<{
children: ReactNode;
}>) {
return (
<html lang="en" suppressHydrationWarning>
- <head>
- {/* Cookie Control Script */}
- <script
- src="https://app.enzuzo.com/scripts/cookiebar/ede431e2-968b-11eb-9cc0-2358d2c9e564"
- type="text/javascript"
- />
- </head>
- <body>{children}</body>
+ <body>
+ {/* Cookie Control Script */}
+ <Script
+ src="https://app.enzuzo.com/scripts/cookiebar/ede431e2-968b-11eb-9cc0-2358d2c9e564"
+ strategy="afterInteractive"
+ />
+ {children}
+ </body>
</html>
);
}🤖 Prompt for AI Agents
In `@benchmarks/enzuzo/app/layout.tsx` around lines 17 - 20, Replace the raw HTML
<script> tag in the layout component with Next.js's Script component to match
other benchmarks: import Script from 'next/script' (or use the existing import)
and render <Script
src="https://app.enzuzo.com/scripts/cookiebar/ede431e2-968b-11eb-9cc0-2358d2c9e564"
strategy="afterInteractive" /> in place of the current script element so loading
behavior is controlled consistently; if the raw tag was intentional for a
specific benchmark, leave as-is and add a comment explaining why.
| const isArrayWithModules = | ||
| Array.isArray(rawBundleType) && | ||
| (rawBundleType.includes(BUNDLE_TYPES.ESM) || | ||
| rawBundleType.includes(BUNDLE_TYPES.CJS) || | ||
| rawBundleType.includes(BUNDLE_TYPES.BUNDLED) || | ||
| rawBundleType.includes(BUNDLE_TYPES.IIFE) || | ||
| rawBundleType.includes(BUNDLE_TYPES.IFFE) || | ||
| rawBundleType.includes(legacyIifeType)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the bundle-strategy.ts file and examine it
find . -name "bundle-strategy.ts" -type fRepository: consentdotio/benchmarks
Length of output: 167
🏁 Script executed:
# Search for BUNDLE_TYPES constant definitions
rg -n "BUNDLE_TYPES\s*=" --type tsRepository: consentdotio/benchmarks
Length of output: 217
🏁 Script executed:
# Check the benchmark schema for valid bundleType enum values
find . -name "schema.json" -type f | head -5Repository: consentdotio/benchmarks
Length of output: 106
🏁 Script executed:
# Search for bundleType usage in config files
rg -n '"bundleType"' benchmarks/ --type jsonRepository: consentdotio/benchmarks
Length of output: 1573
🏁 Script executed:
# Look for IIFE vs IFFE references in the codebase
rg -n "IIFE|IFFE" --type tsRepository: consentdotio/benchmarks
Length of output: 5281
🏁 Script executed:
# Read the complete bundle-strategy.ts file to understand the full logic
cat -n packages/benchmark/src/bundle-strategy.tsRepository: consentdotio/benchmarks
Length of output: 1571
🏁 Script executed:
# Also check the schema.json to confirm bundleType enum values
cat packages/benchmark-schema/schema.json | grep -A 10 -B 2 "bundleType"Repository: consentdotio/benchmarks
Length of output: 590
Remove IIFE/IFFE checks from isArrayWithModules — they're forbidden by the schema.
The schema explicitly limits array bundleType values to ["esm", "cjs"] only (no "iife" or "bundled"). However, the isArrayWithModules check on lines 28-30 includes BUNDLE_TYPES.IIFE, BUNDLE_TYPES.IFFE, and legacyIifeType. Since the schema forbids these values in arrays, these checks are unreachable and should be removed for clarity.
Current code (lines 23-30)
const isArrayWithModules =
Array.isArray(rawBundleType) &&
(rawBundleType.includes(BUNDLE_TYPES.ESM) ||
rawBundleType.includes(BUNDLE_TYPES.CJS) ||
rawBundleType.includes(BUNDLE_TYPES.BUNDLED) ||
rawBundleType.includes(BUNDLE_TYPES.IIFE) ||
rawBundleType.includes(BUNDLE_TYPES.IFFE) ||
rawBundleType.includes(legacyIifeType));
Simplify to only check for valid array values: ESM, CJS, and BUNDLED.
🤖 Prompt for AI Agents
In `@packages/benchmark/src/bundle-strategy.ts` around lines 23 - 30, The
isArrayWithModules check includes unreachable IIFE/IFFE/legacyIifeType branches;
update the condition in the isArrayWithModules boolean (which inspects
rawBundleType and BUNDLE_TYPES) to remove BUNDLE_TYPES.IIFE, BUNDLE_TYPES.IFFE,
and legacyIifeType checks so it only tests for Array.isArray(rawBundleType) &&
(rawBundleType.includes(BUNDLE_TYPES.ESM) ||
rawBundleType.includes(BUNDLE_TYPES.CJS) ||
rawBundleType.includes(BUNDLE_TYPES.BUNDLED)); keep the variable name
isArrayWithModules and other surrounding logic unchanged.
| "includes": { | ||
| "backend": false, | ||
| "components": ["button", "banner"] |
There was a problem hiding this comment.
Fix invalid includes.backend type in the config example.
The schema doesn’t accept a boolean for includes.backend. Use a string or string array instead.
✅ Suggested fix
- "includes": {
- "backend": false,
- "components": ["button", "banner"]
- },
+ "includes": {
+ "backend": ["proprietary"],
+ "components": ["button", "banner"]
+ },Based on learnings: In the cookiebench/benchmark-schema package (packages/benchmark-schema/schema.json), the includes.backend property accepts either a string or an array of strings (oneOf), not a boolean.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "includes": { | |
| "backend": false, | |
| "components": ["button", "banner"] | |
| "includes": { | |
| "backend": ["proprietary"], | |
| "components": ["button", "banner"] | |
| }, |
🤖 Prompt for AI Agents
In `@packages/cookiebench-cli/README.md` around lines 284 - 286, The README
example sets includes.backend to a boolean (false), but the benchmark-schema's
includes.backend accepts a string or array of strings; update the example in the
README so includes.backend is a string or string array (for example change
"backend": false to "backend": "" or "backend": []), ensuring the example
matches the oneOf schema for includes.backend.
| // Dynamically import and run the results command with specific benchmarks | ||
| try { | ||
| const { resultsCommand } = await import("./results.js"); | ||
| await resultsCommand(logger, successfulBenchmarks); | ||
| } catch (error) { | ||
| logger.error("Failed to load results panel"); | ||
| logger.debug(error instanceof Error ? error.message : String(error)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check rslib configuration for output format
echo "=== Checking rslib.config.ts ==="
if [ -f packages/cookiebench-cli/rslib.config.ts ]; then
cat packages/cookiebench-cli/rslib.config.ts
else
echo "rslib.config.ts not found"
fi
echo ""
echo "=== Checking package.json ==="
if [ -f packages/cookiebench-cli/package.json ]; then
cat packages/cookiebench-cli/package.json | grep -A 5 '"type"'
fi
echo ""
echo "=== Checking for dist directory structure ==="
if [ -d packages/cookiebench-cli/dist ]; then
find packages/cookiebench-cli/dist -type f -name "*.js" -o -name "*.mjs" | head -20
else
echo "dist directory not found (build may not have been run)"
fi
echo ""
echo "=== Checking source file structure ==="
ls -la packages/cookiebench-cli/src/commands/ | grep -E "results\.(ts|js|mjs)"Repository: consentdotio/benchmarks
Length of output: 821
Fix dynamic import path to match build output format.
The rslib configuration explicitly outputs all JavaScript files as .mjs extension (filename: { js: "[name].mjs" }), but the import statement uses "./results.js". This causes a runtime module not found error. Change line 585 to:
const { resultsCommand } = await import("./results.mjs");
ESM requires exact file extension matching. The current code will silently fail (caught by the try-catch), preventing the results panel from loading.
🤖 Prompt for AI Agents
In `@packages/cookiebench-cli/src/commands/benchmark.ts` around lines 583 - 590,
The dynamic import in benchmark.ts is using the wrong extension and should match
the build output; update the import that loads resultsCommand (the await
import(...) call that destructures resultsCommand) to reference the .mjs output
file instead of ".js" so ESM can resolve the module at runtime (i.e., change the
import path to the .mjs filename).
| export function resolveBenchmarkPath( | ||
| projectRoot: string, | ||
| appPath: string | ||
| ): string { | ||
| if (isAbsolute(appPath)) { | ||
| return appPath; | ||
| } | ||
|
|
||
| const directPath = join(projectRoot, appPath); | ||
| if (existsSync(join(directPath, "config.json"))) { | ||
| return directPath; | ||
| } | ||
|
|
||
| return join(projectRoot, "benchmarks", appPath); |
There was a problem hiding this comment.
Avoid double benchmarks/ prefix for prequalified paths.
If appPath already starts with benchmarks/ and config.json isn’t found, the current fallback builds benchmarks/benchmarks/.... Normalize the fallback to prevent path duplication.
🛠️ Suggested fix
const directPath = join(projectRoot, appPath);
if (existsSync(join(directPath, "config.json"))) {
return directPath;
}
- return join(projectRoot, "benchmarks", appPath);
+ const normalizedAppPath = appPath.replace(/^benchmarks(?:[\\/]|$)/, "");
+ return join(projectRoot, "benchmarks", normalizedAppPath);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function resolveBenchmarkPath( | |
| projectRoot: string, | |
| appPath: string | |
| ): string { | |
| if (isAbsolute(appPath)) { | |
| return appPath; | |
| } | |
| const directPath = join(projectRoot, appPath); | |
| if (existsSync(join(directPath, "config.json"))) { | |
| return directPath; | |
| } | |
| return join(projectRoot, "benchmarks", appPath); | |
| export function resolveBenchmarkPath( | |
| projectRoot: string, | |
| appPath: string | |
| ): string { | |
| if (isAbsolute(appPath)) { | |
| return appPath; | |
| } | |
| const directPath = join(projectRoot, appPath); | |
| if (existsSync(join(directPath, "config.json"))) { | |
| return directPath; | |
| } | |
| const normalizedAppPath = appPath.replace(/^benchmarks(?:[\\/]|$)/, ""); | |
| return join(projectRoot, "benchmarks", normalizedAppPath); |
🤖 Prompt for AI Agents
In `@packages/cookiebench-cli/src/utils/project-root.ts` around lines 22 - 35, The
fallback in resolveBenchmarkPath can produce duplicate
"benchmarks/benchmarks/..." when appPath already starts with "benchmarks/";
update resolveBenchmarkPath to detect and normalize appPath (e.g., strip a
leading "benchmarks/" or use a conditional join) before returning
join(projectRoot, "benchmarks", appPath) so that when config.json isn't found on
directPath the final path does not duplicate the prefix; refer to the
resolveBenchmarkPath function and parameters appPath, projectRoot, and
directPath when making the change.
| for (let i = 0; i < WARMUP_ITERATIONS; i += 1) { | ||
| try { | ||
| await this.runSingleBenchmark( | ||
| warmupPage, | ||
| `${serverUrl}?t=${Date.now()}&warmup=true`, | ||
| true | ||
| ); | ||
| this.logger.debug(`Warmup iteration ${i + 1} completed`); | ||
| } catch (error) { | ||
| this.logger.debug( | ||
| `Warmup iteration ${i + 1} failed (non-critical):`, | ||
| error instanceof Error ? error.message : String(error) | ||
| ); | ||
| } | ||
| await this.cleanupBetweenIterations(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Warmup iterations use a single page without cleanup.
Warmup runs reuse the same page (warmupPage) across all iterations without resetting collectors or page state between runs. This may cause metric accumulation or stale handlers that don't reflect actual warmup isolation.
♻️ Suggested improvement
Consider creating a fresh page for each warmup iteration, similar to actual benchmark iterations:
for (let i = 0; i < WARMUP_ITERATIONS; i += 1) {
+ const warmupPage = await warmupContext.newPage();
try {
await this.runSingleBenchmark(
warmupPage,
`${serverUrl}?t=${Date.now()}&warmup=true`,
true
);
this.logger.debug(`Warmup iteration ${i + 1} completed`);
} catch (error) {
// ...
+ } finally {
+ await warmupPage.close();
}
await this.cleanupBetweenIterations();
}🤖 Prompt for AI Agents
In `@packages/runner/src/benchmark-runner.ts` around lines 354 - 369, Warmup
reuses warmupPage across iterations causing state/collector accumulation;
instead create and close a fresh page each iteration. Inside the
WARMUP_ITERATIONS loop, replace use of warmupPage with a per-iteration page
created before calling runSingleBenchmark (e.g. via your page factory or
this.browser.newPage()), pass that page to runSingleBenchmark, then close the
page and call cleanupBetweenIterations() (and reset any collectors/handlers if
you have a reset method) to ensure each warmup iteration runs in isolation.
| const VARIABILITY_WARNING_THRESHOLD = 20; // Coefficient of variation threshold for warnings | ||
| const STABILITY_THRESHOLD = 15; // Coefficient of variation threshold for stability checks | ||
| const TRIM_PERCENT = 10; // Percentage to trim from each end for trimmed mean |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused constant.
STABILITY_THRESHOLD is defined but never used in this file. The isStable calls use VARIABILITY_WARNING_THRESHOLD instead.
♻️ Suggested fix
const VARIABILITY_WARNING_THRESHOLD = 20; // Coefficient of variation threshold for warnings
-const STABILITY_THRESHOLD = 15; // Coefficient of variation threshold for stability checks
const TRIM_PERCENT = 10; // Percentage to trim from each end for trimmed meanOr use it in the logStatisticalSummary method where the imported STABILITY_THRESHOLD from statistics.ts is being used via the isStable function's default.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const VARIABILITY_WARNING_THRESHOLD = 20; // Coefficient of variation threshold for warnings | |
| const STABILITY_THRESHOLD = 15; // Coefficient of variation threshold for stability checks | |
| const TRIM_PERCENT = 10; // Percentage to trim from each end for trimmed mean | |
| const VARIABILITY_WARNING_THRESHOLD = 20; // Coefficient of variation threshold for warnings | |
| const TRIM_PERCENT = 10; // Percentage to trim from each end for trimmed mean |
🤖 Prompt for AI Agents
In `@packages/runner/src/performance-aggregator.ts` around lines 21 - 23,
STABILITY_THRESHOLD is declared but never used; update the callers of isStable
in logStatisticalSummary to use STABILITY_THRESHOLD instead of
VARIABILITY_WARNING_THRESHOLD (so the file-level stability constant is applied),
or remove the STABILITY_THRESHOLD constant if you prefer the existing warning
threshold to be the single source of truth; locate the references by the symbols
STABILITY_THRESHOLD, VARIABILITY_WARNING_THRESHOLD, logStatisticalSummary, and
isStable (and note statistics.ts supplies a default) and either swap the
argument to STABILITY_THRESHOLD or delete the unused constant accordingly.
| return { | ||
| firstContentfulPaint: calculateTrimmedMean(fcpValues, TRIM_PERCENT), | ||
| largestContentfulPaint: calculateTrimmedMean(lcpValues, TRIM_PERCENT), | ||
| timeToInteractive: calculateTrimmedMean(ttiValues, TRIM_PERCENT), | ||
| totalBlockingTime: calculateTrimmedMean(tbtValues, TRIM_PERCENT), | ||
| speedIndex: 0, // Default value | ||
| timeToFirstByte: | ||
| ttfbValues.length > 0 | ||
| ? calculateTrimmedMean(ttfbValues, TRIM_PERCENT) | ||
| : 0, | ||
| firstInputDelay: | ||
| fidValues.length > 0 | ||
| ? calculateTrimmedMean(fidValues, TRIM_PERCENT) | ||
| : 0, | ||
| interactionToNextPaint: | ||
| inpValues.length > 0 | ||
| ? calculateTrimmedMean(inpValues, TRIM_PERCENT) | ||
| : 0, | ||
| cumulativeLayoutShift: calculateTrimmedMean(clsValues, TRIM_PERCENT), | ||
| domSize: 0, // Default value | ||
| totalRequests: calculateTrimmedMean(totalRequestsValues, TRIM_PERCENT), | ||
| totalSize: calculateTrimmedMean(totalSizeValues, TRIM_PERCENT), | ||
| jsSize: calculateTrimmedMean(jsSizeValues, TRIM_PERCENT), | ||
| cssSize: calculateTrimmedMean(cssSizeValues, TRIM_PERCENT), | ||
| imageSize: calculateTrimmedMean(imageSizeValues, TRIM_PERCENT), | ||
| fontSize: calculateTrimmedMean(fontSizeValues, TRIM_PERCENT), | ||
| otherSize: calculateTrimmedMean(otherSizeValues, TRIM_PERCENT), | ||
| thirdPartyRequests: 0, // Default value | ||
| thirdPartySize: 0, // Default value | ||
| thirdPartyDomains: 0, // Default value | ||
| thirdPartyCookies: 0, // Default value | ||
| thirdPartyLocalStorage: 0, // Default value | ||
| thirdPartySessionStorage: 0, // Default value | ||
| thirdPartyIndexedDB: 0, // Default value | ||
| thirdPartyCache: 0, // Default value | ||
| thirdPartyServiceWorkers: 0, // Default value | ||
| thirdPartyWebWorkers: 0, // Default value | ||
| thirdPartyWebSockets: 0, // Default value | ||
| thirdPartyBeacons: 0, // Default value | ||
| thirdPartyFetch: 0, // Default value | ||
| thirdPartyXHR: 0, // Default value | ||
| thirdPartyScripts: 0, // Default value | ||
| thirdPartyStyles: 0, // Default value | ||
| thirdPartyImages: 0, // Default value | ||
| thirdPartyFonts: 0, // Default value | ||
| thirdPartyMedia: 0, // Default value | ||
| thirdPartyOther: 0, // Default value | ||
| thirdPartyTiming: { | ||
| total: 0, | ||
| blocking: 0, | ||
| dns: 0, | ||
| connect: 0, | ||
| ssl: 0, | ||
| send: 0, | ||
| wait: 0, | ||
| receive: 0, | ||
| }, | ||
| cookieBannerTiming: { | ||
| firstPaint: 0, | ||
| firstContentfulPaint: calculateTrimmedMean(fcpValues, TRIM_PERCENT), | ||
| domContentLoaded: calculateTrimmedMean( | ||
| domContentLoadedValues, | ||
| TRIM_PERCENT | ||
| ), | ||
| load: calculateTrimmedMean(loadValues, TRIM_PERCENT), | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider computing third-party metrics from available data.
Multiple third-party metric fields are hardcoded to 0 (lines 331-360). If these metrics are planned for future implementation, consider adding a TODO comment. If not needed, consider removing them from the return type to avoid confusion.
🤖 Prompt for AI Agents
In `@packages/runner/src/performance-aggregator.ts` around lines 304 - 370, The
return object in performance-aggregator.ts is populating many third-party fields
(thirdPartyRequests, thirdPartySize, thirdPartyDomains, thirdPartyCookies, etc.)
with hardcoded 0s; either compute these from existing trace/har values (e.g.,
derive thirdPartyRequests from filtering request records by origin, sum sizes
into thirdPartySize, count unique domains into thirdPartyDomains) using existing
helpers like calculateTrimmedMean and TRIM_PERCENT, or if not implemented yet
add a clear TODO and remove them from the public return type (and update the
associated AggregatedMetrics/return type) to avoid misleading consumers; locate
the fields in the return block and either replace the zero literals with
computed values based on available arrays (totalRequestsValues/totalSizeValues
or request-level arrays) or annotate each field with a TODO comment and remove
from exported type/interface.
| #### Package Manager Detection | ||
| - **`getPackageManager(): Promise<{ command: string; args: string[] }>`** - Detect and return available package manager (npm/yarn/pnpm) |
There was a problem hiding this comment.
Fix getPackageManager signature in docs.
The function is synchronous in code, but the README lists a Promise return type.
🛠️ Suggested fix
-- **`getPackageManager(): Promise<{ command: string; args: string[] }>`** - Detect and return available package manager (npm/yarn/pnpm)
+- **`getPackageManager(): { command: string; args: string[] }`** - Detect and return available package manager (npm/yarn/pnpm)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #### Package Manager Detection | |
| - **`getPackageManager(): Promise<{ command: string; args: string[] }>`** - Detect and return available package manager (npm/yarn/pnpm) | |
| #### Package Manager Detection | |
| - **`getPackageManager(): { command: string; args: string[] }`** - Detect and return available package manager (npm/yarn/pnpm) |
🤖 Prompt for AI Agents
In `@packages/shared/README.md` around lines 37 - 38, The README incorrectly
documents getPackageManager as returning a Promise; update the docs to match the
actual synchronous function signature by changing the documented return type
from Promise<{ command: string; args: string[] }> to { command: string; args:
string[] } and ensure any descriptive text referencing async behavior for
getPackageManager is removed or adjusted to reflect that getPackageManager() is
synchronous.
… renaming terms for consistency. Update README.md to reflect changes in metric definitions. Remove outdated benchmark configurations and files for 'with-c15t-nextjs', 'with-c15t-react', 'with-cookie-control', 'with-cookie-yes', 'with-didomi', 'with-enzuzo', and 'with-ketch'.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@packages/benchmark/src/cookie-banner-collector.ts`:
- Around line 287-376: The layoutShiftImpact calculation in collectMetrics
currently always computes metrics.layoutShiftsAfter -
metrics.layoutShiftsBefore, which can misattribute unrelated CLS when no banner
is detected; change the layoutShiftImpact assignment in collectMetrics to be
conditional on metrics.detected (e.g., set layoutShiftImpact = metrics.detected
? metrics.layoutShiftsAfter - metrics.layoutShiftsBefore : 0) so CLS is only
counted when a banner was actually detected.
- Around line 148-254: The detector currently stops polling as soon as
detectCookieBanner() sees a rendered banner, which can leave
bannerMetrics.bannerInteractive at 0 if interactivity appears later; change the
logic so polling only stops when interactivity has been observed
(bannerMetrics.bannerInteractive > 0) or the timeout elapses. Concretely, update
detectCookieBanner to still return a boolean that indicates whether
interactivity was observed (or expose banner metrics via (window as
WindowWithCookieMetrics).__cookieBannerMetrics), and change startDetection so it
does not clearInterval on the first rendered detection — instead keep polling
and only clear the interval when the shared bannerMetrics.bannerInteractive has
been set (or when detectionTimeout occurs). Ensure references to
detectCookieBanner, startDetection, and bannerMetrics.bannerInteractive (and
bannerVisibleTime/bannerFirstSeen if used) are updated accordingly.
In `@packages/cookiebench-cli/src/utils/scoring.ts`:
- Around line 49-53: The UX scoring currently trusts bannerVisibleTimeMs (which
becomes 0 when the banner is not detected) and thus gives an incorrect top score
for missing banners; update the scoring logic (where bannerVisibleTimeMs and
bannerDetected are used—e.g., in the functions that compute UX score /
scoreBenchmark in scoring.ts) to check a bannerDetected flag and apply a penalty
or treat undetected banners as a very high visibility time (or otherwise fail
the UX metric) instead of 0. Locate usages of bannerVisibleTimeMs and
bannerDetected (and the BenchmarkData type) and change the algorithm so missing
bannerDetected === false causes the UX subscore to be downgraded (e.g., add a
fixed penalty or map to worst bucket) rather than counting as instant
visibility.
- Around line 1174-1188: The MetricsData currently sets scriptLoadTime to 0
which incorrectly awards maximum points for missing timing; change the
assignment of scriptLoadTime in the MetricsData object to use the real timing
when present (e.g. transparencyMetrics.scriptLoadTimeMs or the appropriate
timing field) and otherwise set it to null/undefined (not 0), and then update
the scoring logic that reads MetricsData.scriptLoadTime (the scorer that
consumes MetricsData) to treat null/undefined as "unknown" and avoid giving full
points; also apply the same change to the other copy of this MetricsData block
around the 615-638 area so missing script timing is not treated as very fast.
- Around line 185-195: The parseTechStack function returns techStack.typescript
as-is which treats the string "false" as truthy; update parseTechStack to coerce
techStack.typescript into a real boolean (e.g., treat true or "true" as true,
everything else as false) before returning the TechStackData so TypeScript
points are only awarded when typescript is actually true; modify the typescript
field in parseTechStack accordingly to normalize string/boolean inputs.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (31)
METHODOLOGY.mdREADME.mdbenchmarks/ketch/next-env.d.tsbenchmarks/usercentrics/next-env.d.tsbenchmarks/with-c15t-nextjs/package.jsonbenchmarks/with-c15t-react/package.jsonbenchmarks/with-cookie-control/config.jsonbenchmarks/with-cookie-control/next.config.tsbenchmarks/with-cookie-control/tsconfig.jsonbenchmarks/with-didomi/app/layout.tsxbenchmarks/with-didomi/next-env.d.tsbenchmarks/with-enzuzo/app/layout.tsxbenchmarks/with-enzuzo/config.jsonbenchmarks/with-enzuzo/next-env.d.tsbenchmarks/with-enzuzo/tsconfig.jsonbenchmarks/with-iubenda/next-env.d.tsbenchmarks/with-ketch/next-env.d.tsbenchmarks/with-onetrust/next-env.d.tsbenchmarks/with-onetrust/package.jsonbenchmarks/with-osano/next-env.d.tsbenchmarks/with-usercentrics/next-env.d.tspackages/benchmark/src/cookie-banner-collector.tspackages/benchmark/src/types.tspackages/cookiebench-cli/src/commands/benchmark.tspackages/cookiebench-cli/src/commands/results.tspackages/cookiebench-cli/src/commands/save.tspackages/cookiebench-cli/src/commands/scores.tspackages/cookiebench-cli/src/index.tspackages/cookiebench-cli/src/utils/scoring.tspackages/runner/src/performance-aggregator.tspackages/runner/src/types.ts
💤 Files with no reviewable changes (17)
- benchmarks/with-c15t-nextjs/package.json
- benchmarks/with-didomi/app/layout.tsx
- benchmarks/with-enzuzo/config.json
- benchmarks/with-iubenda/next-env.d.ts
- benchmarks/with-usercentrics/next-env.d.ts
- benchmarks/with-cookie-control/tsconfig.json
- benchmarks/with-onetrust/next-env.d.ts
- benchmarks/with-cookie-control/next.config.ts
- benchmarks/with-ketch/next-env.d.ts
- benchmarks/with-c15t-react/package.json
- benchmarks/with-enzuzo/tsconfig.json
- benchmarks/with-cookie-control/config.json
- benchmarks/with-onetrust/package.json
- benchmarks/with-didomi/next-env.d.ts
- benchmarks/with-osano/next-env.d.ts
- benchmarks/with-enzuzo/app/layout.tsx
- benchmarks/with-enzuzo/next-env.d.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-07T16:17:07.589Z
Learnt from: danielsitek
Repo: consentdotio/benchmarks PR: 6
File: benchmarks/with-dgp-cookie-consent/config.json:15-20
Timestamp: 2025-11-07T16:17:07.589Z
Learning: In the cookiebench/benchmark-schema package (packages/benchmark-schema/schema.json), the valid enum values for bundleType are: "esm", "cjs", "iffe", and "bundled". Note that it's "iffe" (not "iife").
Applied to files:
README.mdpackages/cookiebench-cli/src/commands/scores.tspackages/cookiebench-cli/src/commands/results.tspackages/cookiebench-cli/src/commands/save.tspackages/runner/src/types.tspackages/benchmark/src/types.ts
📚 Learning: 2025-11-07T16:19:37.939Z
Learnt from: danielsitek
Repo: consentdotio/benchmarks PR: 6
File: benchmarks/with-dgp-cookie-consent/config.json:37-39
Timestamp: 2025-11-07T16:19:37.939Z
Learning: In the cookiebench/benchmark-schema package (packages/benchmark-schema/schema.json), the includes.backend property accepts either a string or an array of strings (oneOf), not a boolean.
Applied to files:
packages/cookiebench-cli/src/commands/scores.tspackages/cookiebench-cli/src/commands/results.tspackages/cookiebench-cli/src/commands/save.ts
🧬 Code graph analysis (5)
packages/cookiebench-cli/src/commands/benchmark.ts (7)
packages/runner/src/types.ts (2)
BenchmarkResult(167-280)ServerInfo(20-23)packages/cookiebench-cli/src/utils/logger.ts (2)
logger(162-162)CliLogger(9-9)packages/cookiebench-cli/src/utils/index.ts (3)
PERCENTAGE_DIVISOR(14-14)findProjectRoot(20-20)resolveBenchmarkPath(20-20)packages/cookiebench-cli/src/utils/constants.ts (3)
DEFAULT_DOM_SIZE(3-3)DEFAULT_ITERATIONS(2-2)SEPARATOR_WIDTH(5-5)packages/runner/src/server.ts (2)
buildAndServeNextApp(6-119)cleanupServer(121-125)packages/cookiebench-cli/src/utils/scoring.ts (2)
calculateScores(1011-1382)printScores(1386-1445)packages/cookiebench-cli/src/utils/project-root.ts (2)
findProjectRoot(4-20)resolveBenchmarkPath(22-36)
packages/runner/src/performance-aggregator.ts (2)
packages/benchmark/src/types.ts (8)
CoreWebVitals(225-236)CookieBannerData(97-112)CookieBannerMetrics(82-95)NetworkRequest(115-122)NetworkMetrics(124-127)ResourceTimingData(137-222)Config(15-58)PerfumeMetrics(239-270)packages/runner/src/types.ts (10)
CoreWebVitals(12-12)CookieBannerData(10-10)CookieBannerMetrics(11-11)NetworkRequest(14-14)NetworkMetrics(13-13)ResourceTimingData(16-16)Config(8-8)PerfumeMetrics(15-15)BenchmarkDetails(26-165)BenchmarkResult(167-280)
packages/cookiebench-cli/src/commands/scores.ts (7)
packages/cookiebench-cli/src/commands/results.ts (2)
BenchmarkOutput(184-218)RawBenchmarkDetail(38-182)packages/cookiebench-cli/src/types/index.ts (2)
BenchmarkScores(10-39)Config(5-5)packages/benchmark/src/types.ts (1)
Config(15-58)packages/runner/src/types.ts (1)
Config(8-8)packages/cookiebench-cli/src/utils/index.ts (2)
HALF_SECOND(11-11)findProjectRoot(20-20)packages/cookiebench-cli/src/utils/scoring.ts (1)
printScores(1386-1445)packages/cookiebench-cli/src/utils/constants.ts (1)
DEFAULT_DOM_SIZE(3-3)
packages/cookiebench-cli/src/utils/scoring.ts (2)
packages/shared/src/utils/conversion.ts (1)
formatBytes(17-28)packages/cookiebench-cli/src/types/index.ts (1)
BenchmarkScores(10-39)
packages/cookiebench-cli/src/commands/save.ts (6)
packages/cookiebench-cli/src/commands/results.ts (2)
BenchmarkOutput(184-218)RawBenchmarkDetail(38-182)packages/runner/src/types.ts (1)
BenchmarkResult(167-280)packages/cookiebench-cli/src/utils/logger.ts (2)
logger(162-162)CliLogger(9-9)packages/cookiebench-cli/src/utils/project-root.ts (1)
findProjectRoot(4-20)packages/cookiebench-cli/src/utils/auth.ts (1)
isAdminUser(5-11)packages/cookiebench-cli/src/utils/scoring.ts (1)
calculateScores(1011-1382)
🪛 LanguageTool
METHODOLOGY.md
[grammar] ~79-~79: Ensure spelling is correct
Context: ...aintains speed - Fast 3G (1.6 Mbps, 562ms latency): More realistic, significantly...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (39)
METHODOLOGY.md (1)
11-185: Clear, consistent methodology definitions and scoring linkage.The dual timing modes, opacity threshold, and scoring usage are clearly specified and align with the stated UX focus.
README.md (3)
9-26: Good documentation alignment with the new timing metrics.Clear explanation of DOM presence vs user-visible time and the scoring choice improves clarity.
50-51: Limitation note and methodology link are helpful.This keeps the README concise while pointing to the authoritative details.
103-103: bundleType enum list matches the schema.Nice to see “iffe” and “bundled” reflected correctly in the docs.
packages/cookiebench-cli/src/commands/results.ts (3)
37-182: LGTM!The
RawBenchmarkDetailtype definition is comprehensive and well-structured, capturing all timing metrics, resources, and cookie banner information needed for benchmarking.
220-247: LGTM!The
findResultsFilesfunction now properly handles the missing directory case with graceful error handling, checking forENOENTand re-throwing other errors.
920-1004: LGTM!The
resultsCommandfunction has well-structured flow handling for direct app execution,__all__mode, and interactive multi-select with proper cancel handling.packages/cookiebench-cli/src/index.ts (1)
1-146: LGTM!The CLI entry point is well-structured with:
- Proper admin gating for restricted commands (
save,db)- Both direct command execution and interactive mode support
- The
scorescommand is now properly included in both direct and interactive flows (addressing the previous review feedback)- Clean error handling with global catch
packages/cookiebench-cli/src/commands/save.ts (2)
94-132: LGTM!The
saveBenchmarkResultfunction has proper error handling with specific messaging for connection failures and HTTP errors.
397-554: LGTM!The
saveAppToDatabasefunction now includes:
- Empty results guard (lines 405-408)
- Proper INP averaging with null filtering (lines 446-456)
- Correct third-party domain calculation across all resource types (lines 499-517)
These address the previously flagged issues.
packages/cookiebench-cli/src/commands/benchmark.ts (3)
133-211: Well-documented cookie banner metrics calculation.The
calculateCookieBannerMetricsfunction has excellent documentation explaining:
- The distinction between DOM presence time vs user-visible time
- Why user-visible time is used for scoring
- The penalty logic for inconsistent detection
This aligns with the METHODOLOGY.md referenced in the docstring.
253-396: LGTM!The
runSingleBenchmarkfunction properly handles:
- Remote vs local benchmarking
- Trace directory creation with EEXIST handling
- Server lifecycle with cleanup in finally block
- Result file generation with comprehensive metadata
401-593: LGTM!The
benchmarkCommandfunction has well-structured flow:
- Multi-select benchmark discovery and selection
- Per-benchmark iteration override support
- Sequential execution with progress indicators
- Optional results panel with dynamic import
packages/cookiebench-cli/src/commands/scores.ts (4)
212-218: LGTM!The empty results guard was properly added, preventing divide-by-zero errors during score calculation.
253-266: LGTM!The
interactionToNextPaintaveraging correctly filters for valid finite numbers before computing the average, handling null/undefined values appropriately.
309-331: LGTM!The
thirdPartyDomainscalculation now correctly aggregates unique third-party domains across all resource types (scripts, styles, images, fonts, other), with proper URL parsing error handling.
96-187: LGTM!The
scoresCommandfunction has well-structured flow with proper handling of specific app requests, interactive selection, and the "all" option for batch display.packages/runner/src/types.ts (5)
1-17: Re-exported type surface looks consistent.
Centralizing benchmark types here makes the runner API easier to consume.
19-23: ServerInfo type is straightforward.
Clear structure for server process + URL.
25-165: BenchmarkDetails captures the needed metric structure.
The timing/size/resources layout is cohesive and matches downstream needs.
167-280: BenchmarkResult schema is well-scoped for downstream use.
Includes the right summary + scoring structure for reporting.
282-302: Internal cookie/third‑party timing aliases are clear.
These private types keep the public surface clean.packages/runner/src/performance-aggregator.ts (9)
1-56: Imports/constants + TTI computation are clear.
The buffer-based TTI calculation is easy to follow.
58-94: Cookie banner timing builder is consistent with visibility-first scoring.
Good alignment with the methodology notes.
96-118: Third‑party timing scaffold is fine for now.
Structure is ready for later enrichment.
120-141: Main-thread blocking derivation is correct.
Percentage calculation is guarded against divide-by-zero.
143-227: Metric aggregation wiring looks solid.
Good mapping from collectors to final timing/resource fields.
229-243: Network impact aggregation is straightforward.
Summed size/duration is a clean baseline.
376-396: Statistical summary helper is fine.
The computed stats align with the summary log format.
398-432: Result logging looks good.
The debug payload is complete and consistent.
434-472: Stability logging uses the intended threshold.
Nice separation of variability warnings vs stability checks.packages/benchmark/src/types.ts (6)
1-58: Config and bundle type definitions are consistent.
The config surface is explicit and easy to consume.
60-112: Cookie banner metric types are well defined.
The render/visibility/interactive split is clear and documented.
114-134: Network and bundle strategy types are clear.
Names and structure are aligned with the collector logic.
136-222: ResourceTimingData structure is thorough.
Good granularity for scripts/styles/images/fonts/other.
224-270: Core Web Vitals and Perfume metrics types look good.
Optionality and grouping are sensible.
272-287: Perfume window typing is fine.
Covers attribution and navigator info cleanly.packages/benchmark/src/cookie-banner-collector.ts (2)
1-23: Visibility threshold setup is clear and documented.
Good rationale for the 0.5 opacity threshold.
34-70: Metric initialization is clean.
Returns a well-structured baseline for collection.
| // Cookie banner detection logic | ||
| const detectCookieBanner = () => { | ||
| const metrics = (window as unknown as WindowWithCookieMetrics) | ||
| .__cookieBannerMetrics; | ||
| // performance.now() returns time since navigation start (timeOrigin) | ||
| metrics.bannerDetectionStart = performance.now(); | ||
|
|
||
| for (const selector of bannerSelectors) { | ||
| try { | ||
| const element = document.querySelector(selector); | ||
| if (element) { | ||
| const rect = element.getBoundingClientRect(); | ||
| const computedStyle = window.getComputedStyle(element); | ||
| const opacity = Number.parseFloat(computedStyle.opacity); | ||
|
|
||
| /** | ||
| * Technical render check: Element exists in DOM with dimensions. | ||
| * This determines when the banner element first appears, regardless | ||
| * of visibility. Used for bannerRenderTime metric. | ||
| */ | ||
| const isRendered = | ||
| rect.width > 0 && | ||
| rect.height > 0 && | ||
| computedStyle.visibility !== "hidden" && | ||
| computedStyle.display !== "none"; | ||
|
|
||
| /** | ||
| * User-perceived visibility check: Element is rendered AND visible. | ||
| * Uses opacity threshold to account for CSS animations. A banner that | ||
| * renders instantly but fades in slowly will only be considered "visible" | ||
| * once opacity > 0.5. This ensures scores reflect actual user experience. | ||
| * Used for bannerVisibilityTime metric (primary scoring metric). | ||
| */ | ||
| const isVisible = isRendered && opacity > opacityThreshold; | ||
|
|
||
| if (isRendered) { | ||
| const bannerMetrics = ( | ||
| window as unknown as WindowWithCookieMetrics | ||
| ).__cookieBannerMetrics; | ||
| // performance.now() returns time since navigation start (timeOrigin) | ||
| const now = performance.now(); | ||
|
|
||
| /** | ||
| * Track technical render time: When banner element first appears in DOM. | ||
| * This is measured from navigationStart and represents when the element | ||
| * has dimensions and is not hidden. Used for bannerRenderTime metric. | ||
| */ | ||
| if (!bannerMetrics.detected) { | ||
| bannerMetrics.detected = true; | ||
| bannerMetrics.selector = selector; | ||
| bannerMetrics.bannerFirstSeen = now; | ||
| bannerMetrics.layoutShiftsBefore = cumulativeLayoutShift; | ||
| } | ||
|
|
||
| /** | ||
| * Track user-perceived visibility time: When banner becomes visible to users. | ||
| * Only recorded when opacity > threshold (0.5), accounting for CSS animations. | ||
| * This is the primary metric used for scoring (bannerVisibilityTime). | ||
| * Only update if not set yet or if this measurement is earlier. | ||
| */ | ||
| if ( | ||
| isVisible && | ||
| (bannerMetrics.bannerVisibleTime === 0 || | ||
| now < bannerMetrics.bannerVisibleTime) | ||
| ) { | ||
| bannerMetrics.bannerVisibleTime = now; | ||
| } | ||
|
|
||
| // Check if banner is interactive | ||
| const buttons = element.querySelectorAll( | ||
| 'button, a, [role="button"], [onclick]' | ||
| ); | ||
| if (buttons.length > 0) { | ||
| // Test if buttons are actually clickable | ||
| const firstButton = buttons[0] as HTMLElement; | ||
| if (firstButton.offsetParent !== null) { | ||
| // Element is visible and clickable | ||
| bannerMetrics.bannerInteractive = now; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
| } | ||
| } catch (_error) { | ||
| // Ignore selector errors and continue checking other selectors | ||
| } | ||
| } | ||
| return false; | ||
| }; | ||
|
|
||
| // Start detection immediately - try right away, then poll if needed | ||
| // This ensures we catch banners that appear instantly (like in offline/bundled mode) | ||
| const startDetection = () => { | ||
| // Try immediate detection first (catches instant renders) | ||
| if (!detectCookieBanner()) { | ||
| // If not found immediately, start polling | ||
| const interval = setInterval(() => { | ||
| if (detectCookieBanner()) { | ||
| clearInterval(interval); | ||
| } | ||
| }, pollInterval); | ||
|
|
||
| // Stop checking after timeout | ||
| setTimeout(() => clearInterval(interval), detectionTimeout); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Keep polling until interactivity is observed.
detectCookieBanner() returns true as soon as the banner is rendered, which stops polling and can leave bannerInteractive at 0 for banners that become clickable later. This underreports interactivity time.
🔧 Suggested fix
- // Check if banner is interactive
- const buttons = element.querySelectorAll(
- 'button, a, [role="button"], [onclick]'
- );
- if (buttons.length > 0) {
- // Test if buttons are actually clickable
- const firstButton = buttons[0] as HTMLElement;
- if (firstButton.offsetParent !== null) {
- // Element is visible and clickable
- bannerMetrics.bannerInteractive = now;
- }
- }
-
- return true;
+ // Check if banner is interactive
+ const buttons = element.querySelectorAll(
+ 'button, a, [role="button"], [onclick]'
+ );
+ let interactiveCaptured = false;
+ if (buttons.length > 0) {
+ // Test if buttons are actually clickable
+ const firstButton = buttons[0] as HTMLElement;
+ if (firstButton.offsetParent !== null) {
+ // Element is visible and clickable
+ bannerMetrics.bannerInteractive = now;
+ interactiveCaptured = true;
+ }
+ }
+
+ // Keep polling until interactivity is observed or timeout expires
+ return interactiveCaptured;🤖 Prompt for AI Agents
In `@packages/benchmark/src/cookie-banner-collector.ts` around lines 148 - 254,
The detector currently stops polling as soon as detectCookieBanner() sees a
rendered banner, which can leave bannerMetrics.bannerInteractive at 0 if
interactivity appears later; change the logic so polling only stops when
interactivity has been observed (bannerMetrics.bannerInteractive > 0) or the
timeout elapses. Concretely, update detectCookieBanner to still return a boolean
that indicates whether interactivity was observed (or expose banner metrics via
(window as WindowWithCookieMetrics).__cookieBannerMetrics), and change
startDetection so it does not clearInterval on the first rendered detection —
instead keep polling and only clear the interval when the shared
bannerMetrics.bannerInteractive has been set (or when detectionTimeout occurs).
Ensure references to detectCookieBanner, startDetection, and
bannerMetrics.bannerInteractive (and bannerVisibleTime/bannerFirstSeen if used)
are updated accordingly.
| async collectMetrics(page: Page): Promise<CookieBannerData | null> { | ||
| return await page.evaluate( | ||
| (config: { percentageMultiplier: number }) => { | ||
| const { percentageMultiplier } = config; | ||
| const metrics = (window as unknown as WindowWithCookieMetrics) | ||
| .__cookieBannerMetrics; | ||
| if (!metrics) { | ||
| return null; | ||
| } | ||
|
|
||
| // Read final layout shift value before disconnecting observer | ||
| const layoutShiftImpact = | ||
| metrics.layoutShiftsAfter - metrics.layoutShiftsBefore; | ||
|
|
||
| // Clean up PerformanceObserver to prevent memory leaks | ||
| if (metrics.clsObserver) { | ||
| metrics.clsObserver.disconnect(); | ||
| metrics.clsObserver = undefined; | ||
| } | ||
|
|
||
| const domPresenceTime = | ||
| metrics.detected && metrics.bannerFirstSeen > 0 | ||
| ? metrics.bannerFirstSeen - metrics.pageLoadStart | ||
| : 0; | ||
| const userVisibleTime = (() => { | ||
| if (metrics.detected && metrics.bannerVisibleTime > 0) { | ||
| return metrics.bannerVisibleTime - metrics.pageLoadStart; | ||
| } | ||
| if (metrics.detected && metrics.bannerFirstSeen > 0) { | ||
| return metrics.bannerFirstSeen - metrics.pageLoadStart; | ||
| } | ||
| return 0; | ||
| })(); | ||
|
|
||
| return { | ||
| detected: metrics.detected, | ||
| selector: metrics.selector, | ||
| bannerRenderTime: domPresenceTime, | ||
| bannerVisibilityTime: userVisibleTime, | ||
| domPresenceTime, | ||
| userVisibleTime, | ||
| bannerInteractiveTime: | ||
| metrics.detected && metrics.bannerInteractive > 0 | ||
| ? metrics.bannerInteractive - metrics.pageLoadStart | ||
| : 0, | ||
| bannerHydrationTime: | ||
| metrics.detected && metrics.bannerInteractive > 0 | ||
| ? metrics.bannerInteractive - metrics.bannerFirstSeen | ||
| : 0, | ||
| layoutShiftImpact, | ||
| viewportCoverage: metrics.detected | ||
| ? (() => { | ||
| if (!metrics.selector) { | ||
| return 0; | ||
| } | ||
| const element = document.querySelector(metrics.selector); | ||
| if (element) { | ||
| const rect = element.getBoundingClientRect(); | ||
| const viewportWidth = window.innerWidth; | ||
| const viewportHeight = window.innerHeight; | ||
|
|
||
| // Calculate the intersection of element rect with viewport | ||
| // Only count the visible portion of the banner | ||
| const visibleLeft = Math.max(0, rect.left); | ||
| const visibleTop = Math.max(0, rect.top); | ||
| const visibleRight = Math.min( | ||
| viewportWidth, | ||
| rect.left + rect.width | ||
| ); | ||
| const visibleBottom = Math.min( | ||
| viewportHeight, | ||
| rect.top + rect.height | ||
| ); | ||
|
|
||
| // Calculate visible area (intersection with viewport) | ||
| const visibleWidth = Math.max(0, visibleRight - visibleLeft); | ||
| const visibleHeight = Math.max(0, visibleBottom - visibleTop); | ||
| const visibleArea = visibleWidth * visibleHeight; | ||
| const viewportArea = viewportWidth * viewportHeight; | ||
|
|
||
| if (viewportArea === 0) { | ||
| return 0; | ||
| } | ||
|
|
||
| return (visibleArea / viewportArea) * percentageMultiplier; | ||
| } | ||
| return 0; | ||
| })() | ||
| : 0, | ||
| }; |
There was a problem hiding this comment.
Zero out layout shift when no banner is detected.
When detected is false, layoutShiftsAfter - layoutShiftsBefore can reflect unrelated page CLS and get misattributed to the banner.
🧯 Suggested fix
- const layoutShiftImpact =
- metrics.layoutShiftsAfter - metrics.layoutShiftsBefore;
+ const layoutShiftImpact = metrics.detected
+ ? metrics.layoutShiftsAfter - metrics.layoutShiftsBefore
+ : 0;🤖 Prompt for AI Agents
In `@packages/benchmark/src/cookie-banner-collector.ts` around lines 287 - 376,
The layoutShiftImpact calculation in collectMetrics currently always computes
metrics.layoutShiftsAfter - metrics.layoutShiftsBefore, which can misattribute
unrelated CLS when no banner is detected; change the layoutShiftImpact
assignment in collectMetrics to be conditional on metrics.detected (e.g., set
layoutShiftImpact = metrics.detected ? metrics.layoutShiftsAfter -
metrics.layoutShiftsBefore : 0) so CLS is only counted when a banner was
actually detected.
| type BenchmarkData = { | ||
| bannerRenderTime?: number; | ||
| bannerInteractionTime?: number; | ||
| layoutShift?: number; | ||
| }; |
There was a problem hiding this comment.
Banner detection is ignored, causing false “instant visibility.”
If the banner isn’t detected, bannerVisibleTimeMs becomes 0 and earns top UX points. Wire the detection flag into UX scoring and penalize missing banners.
🛠️ Suggested fix
type BenchmarkData = {
+ bannerDetected?: boolean;
bannerRenderTime?: number;
bannerInteractionTime?: number;
layoutShift?: number;
}; const benchmarkData: BenchmarkData = {
+ bannerDetected: transparencyMetrics.cookieBannerDetected,
bannerRenderTime: transparencyMetrics.cookieBannerVisibleTimeMs || 0,
bannerInteractionTime: userExperienceMetrics.mainThreadBlocking,
layoutShift: userExperienceMetrics.layoutShifts,
};- const bannerVisibleTimeMs =
- Number.isFinite(benchmarkData.bannerRenderTime)
- ? benchmarkData.bannerRenderTime || 0
- : metrics.bannerVisibilityTime || 0;
- const renderScore =
- bannerVisibleTimeMs <= 25
- ? 35
- : bannerVisibleTimeMs <= 50
- ? 25
- : bannerVisibleTimeMs <= 100
- ? 15
- : bannerVisibleTimeMs <= 200
- ? 10
- : 5;
+ const bannerDetected = benchmarkData.bannerDetected ?? true;
+ const bannerVisibleTimeMs = bannerDetected
+ ? Number.isFinite(benchmarkData.bannerRenderTime)
+ ? benchmarkData.bannerRenderTime || 0
+ : metrics.bannerVisibilityTime || 0
+ : null;
+ const renderScore =
+ bannerVisibleTimeMs === null
+ ? 0
+ : bannerVisibleTimeMs <= 25
+ ? 35
+ : bannerVisibleTimeMs <= 50
+ ? 25
+ : bannerVisibleTimeMs <= 100
+ ? 15
+ : bannerVisibleTimeMs <= 200
+ ? 10
+ : 5;
totalScore += renderScore;
details.push({
metric: "Banner visible time",
- value: formatTime(bannerVisibleTimeMs),
+ value:
+ bannerVisibleTimeMs === null
+ ? "Not detected"
+ : formatTime(bannerVisibleTimeMs),
score: renderScore,
maxScore: 35,
- reason:
- bannerVisibleTimeMs <= 25
- ? "Instant visibility"
- : bannerVisibleTimeMs <= 50
- ? "Very fast visibility"
- : bannerVisibleTimeMs <= 100
- ? "Fast visibility"
- : bannerVisibleTimeMs <= 200
- ? "Moderate visibility"
- : "Slow visibility",
+ reason:
+ bannerVisibleTimeMs === null
+ ? "Banner not detected"
+ : bannerVisibleTimeMs <= 25
+ ? "Instant visibility"
+ : bannerVisibleTimeMs <= 50
+ ? "Very fast visibility"
+ : bannerVisibleTimeMs <= 100
+ ? "Fast visibility"
+ : bannerVisibleTimeMs <= 200
+ ? "Moderate visibility"
+ : "Slow visibility",
});Also applies to: 739-770, 1207-1211
🤖 Prompt for AI Agents
In `@packages/cookiebench-cli/src/utils/scoring.ts` around lines 49 - 53, The UX
scoring currently trusts bannerVisibleTimeMs (which becomes 0 when the banner is
not detected) and thus gives an incorrect top score for missing banners; update
the scoring logic (where bannerVisibleTimeMs and bannerDetected are used—e.g.,
in the functions that compute UX score / scoreBenchmark in scoring.ts) to check
a bannerDetected flag and apply a penalty or treat undetected banners as a very
high visibility time (or otherwise fail the UX metric) instead of 0. Locate
usages of bannerVisibleTimeMs and bannerDetected (and the BenchmarkData type)
and change the algorithm so missing bannerDetected === false causes the UX
subscore to be downgraded (e.g., add a fixed penalty or map to worst bucket)
rather than counting as instant visibility.
| function parseTechStack(techStackJson: string): TechStackData { | ||
| try { | ||
| const techStack = JSON.parse(techStackJson); | ||
| return { | ||
| languages: techStack.languages || [], | ||
| frameworks: techStack.frameworks || [], | ||
| bundler: techStack.bundler || "unknown", | ||
| bundleType: techStack.bundleType || "unknown", | ||
| packageManager: techStack.packageManager || "unknown", | ||
| typescript: techStack.typescript, | ||
| }; |
There was a problem hiding this comment.
Normalize techStack.typescript to a real boolean.
If techStack.typescript comes in as "false", it becomes truthy and incorrectly awards TypeScript points. Coerce the value during parsing.
🛠️ Suggested fix
function parseTechStack(techStackJson: string): TechStackData {
try {
const techStack = JSON.parse(techStackJson);
+ const rawTypescript = techStack.typescript;
return {
languages: techStack.languages || [],
frameworks: techStack.frameworks || [],
bundler: techStack.bundler || "unknown",
bundleType: techStack.bundleType || "unknown",
packageManager: techStack.packageManager || "unknown",
- typescript: techStack.typescript,
+ typescript:
+ rawTypescript === true ||
+ (typeof rawTypescript === "string" &&
+ rawTypescript.toLowerCase() === "true"),
};
} catch {
return {🤖 Prompt for AI Agents
In `@packages/cookiebench-cli/src/utils/scoring.ts` around lines 185 - 195, The
parseTechStack function returns techStack.typescript as-is which treats the
string "false" as truthy; update parseTechStack to coerce techStack.typescript
into a real boolean (e.g., treat true or "true" as true, everything else as
false) before returning the TechStackData so TypeScript points are only awarded
when typescript is actually true; modify the typescript field in parseTechStack
accordingly to normalize string/boolean inputs.
| // Create metrics data (scoring uses user-visible time only; cookieBannerVisibleTimeMs must be user-visible) | ||
| const metricsData: MetricsData = { | ||
| fcp: metrics.fcp, | ||
| lcp: metrics.lcp, | ||
| cls: metrics.cls, | ||
| tti: metrics.tti, | ||
| tbt: metrics.tbt, | ||
| totalSize: bundleMetrics.totalSize, | ||
| thirdPartySize: networkMetrics.thirdPartySize, | ||
| bannerVisibilityTime: transparencyMetrics.cookieBannerVisibleTimeMs || 0, | ||
| viewportCoverage: transparencyMetrics.cookieBannerCoverage * 100, | ||
| resourceCount: networkMetrics.totalRequests, | ||
| scriptLoadTime: 0, // TODO: Calculate from timing data | ||
| isBundled: networkMetrics.thirdPartyRequests === 0, | ||
| isIIFE: networkMetrics.thirdPartyRequests > 0, |
There was a problem hiding this comment.
Missing script timing data is scored as “very fast.”
scriptLoadTime is hard-coded to 0, which guarantees the max score and misrepresents network impact. Treat missing timing as unknown and avoid awarding full points.
🐛 Proposed fix
const metricsData: MetricsData = {
fcp: metrics.fcp,
lcp: metrics.lcp,
cls: metrics.cls,
tti: metrics.tti,
tbt: metrics.tbt,
totalSize: bundleMetrics.totalSize,
thirdPartySize: networkMetrics.thirdPartySize,
bannerVisibilityTime: transparencyMetrics.cookieBannerVisibleTimeMs || 0,
viewportCoverage: transparencyMetrics.cookieBannerCoverage * 100,
resourceCount: networkMetrics.totalRequests,
- scriptLoadTime: 0, // TODO: Calculate from timing data
+ scriptLoadTime: undefined, // TODO: Calculate from timing data
isBundled: networkMetrics.thirdPartyRequests === 0,
isIIFE: networkMetrics.thirdPartyRequests > 0,
// NEW: Add Perfume.js metrics
timeToFirstByte: metrics.timeToFirstByte || 0,
interactionToNextPaint: metrics.interactionToNextPaint,
networkInformation,
};- const scriptLoadTime = Number.isFinite(metrics.scriptLoadTime)
- ? metrics.scriptLoadTime || 0
- : 0;
+ const scriptLoadTime = Number.isFinite(metrics.scriptLoadTime)
+ ? metrics.scriptLoadTime
+ : null;
// Script load time (15 points)
- const scriptScore =
- scriptLoadTime <= 50
- ? 15
- : scriptLoadTime <= 100
- ? 10
- : scriptLoadTime <= 200
- ? 5
- : 0;
+ const scriptScore =
+ scriptLoadTime === null
+ ? 0
+ : scriptLoadTime <= 50
+ ? 15
+ : scriptLoadTime <= 100
+ ? 10
+ : scriptLoadTime <= 200
+ ? 5
+ : 0;
totalScore += scriptScore;
details.push({
metric: "Script Load Time",
- value: formatTime(scriptLoadTime),
+ value: scriptLoadTime === null ? "N/A" : formatTime(scriptLoadTime),
score: scriptScore,
maxScore: 15,
- reason:
- scriptLoadTime <= 50
- ? "Very fast loading"
- : scriptLoadTime <= 100
- ? "Fast loading"
- : scriptLoadTime <= 200
- ? "Moderate loading"
- : "Slow loading",
+ reason:
+ scriptLoadTime === null
+ ? "No timing data"
+ : scriptLoadTime <= 50
+ ? "Very fast loading"
+ : scriptLoadTime <= 100
+ ? "Fast loading"
+ : scriptLoadTime <= 200
+ ? "Moderate loading"
+ : "Slow loading",
});Also applies to: 615-638
🤖 Prompt for AI Agents
In `@packages/cookiebench-cli/src/utils/scoring.ts` around lines 1174 - 1188, The
MetricsData currently sets scriptLoadTime to 0 which incorrectly awards maximum
points for missing timing; change the assignment of scriptLoadTime in the
MetricsData object to use the real timing when present (e.g.
transparencyMetrics.scriptLoadTimeMs or the appropriate timing field) and
otherwise set it to null/undefined (not 0), and then update the scoring logic
that reads MetricsData.scriptLoadTime (the scorer that consumes MetricsData) to
treat null/undefined as "unknown" and avoid giving full points; also apply the
same change to the other copy of this MetricsData block around the 615-638 area
so missing script timing is not treated as very fast.
…reamlining output for benchmark display.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/cookiebench-cli/src/commands/results.ts`:
- Line 14: Remove the unused import isAdminUser from the top-level imports in
the results command file; locate the import statement that reads "import {
isAdminUser } ..." and delete that specifier (or the whole import line if it
only contained isAdminUser) so no dead code remains and run the
linter/TypeScript build to confirm the unused import is gone.
In `@packages/runner/src/benchmark-runner.ts`:
- Around line 120-203: The method currently initializes
PerformanceMetricsCollector (collector) and calls operations like page.goto,
this.waitForElement, collector.collectMetrics, etc., but calls
collector.cleanup() and this.networkMonitor.reset() only on success; wrap the
main work (from after collector initialization through aggregation/logging) in a
try/finally so cleanup always executes: declare/initialize collector before the
try, run navigation, waitForElement, collects and aggregation inside try, and in
finally call await collector.cleanup() and this.networkMonitor.reset(); ensure
any caught errors are rethrown after finally so failures still propagate.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/cookiebench-cli/src/commands/results.tspackages/runner/src/benchmark-runner.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-07T16:17:07.589Z
Learnt from: danielsitek
Repo: consentdotio/benchmarks PR: 6
File: benchmarks/with-dgp-cookie-consent/config.json:15-20
Timestamp: 2025-11-07T16:17:07.589Z
Learning: In the cookiebench/benchmark-schema package (packages/benchmark-schema/schema.json), the valid enum values for bundleType are: "esm", "cjs", "iffe", and "bundled". Note that it's "iffe" (not "iife").
Applied to files:
packages/cookiebench-cli/src/commands/results.ts
📚 Learning: 2025-11-07T16:19:37.939Z
Learnt from: danielsitek
Repo: consentdotio/benchmarks PR: 6
File: benchmarks/with-dgp-cookie-consent/config.json:37-39
Timestamp: 2025-11-07T16:19:37.939Z
Learning: In the cookiebench/benchmark-schema package (packages/benchmark-schema/schema.json), the includes.backend property accepts either a string or an array of strings (oneOf), not a boolean.
Applied to files:
packages/cookiebench-cli/src/commands/results.ts
🧬 Code graph analysis (1)
packages/runner/src/benchmark-runner.ts (4)
packages/benchmark/src/types.ts (1)
Config(15-58)packages/runner/src/types.ts (3)
Config(8-8)BenchmarkDetails(26-165)BenchmarkResult(167-280)packages/runner/src/performance-aggregator.ts (1)
PerformanceAggregator(36-473)packages/benchmark/src/constants.ts (1)
BENCHMARK_CONSTANTS(17-74)
🔇 Additional comments (15)
packages/cookiebench-cli/src/commands/results.ts (8)
38-182: LGTM!The
RawBenchmarkDetailtype is well-structured with appropriate optional fields and union types for nullable metrics likeinteractionToNextPaint.
184-218: LGTM!The
BenchmarkOutputtype accurately represents the benchmark output structure with appropriate optional scores and metadata.
220-247: LGTM!The
findResultsFilesfunction properly handles missing directories with ENOENT error handling, returning an empty array gracefully.
391-438: LGTM!The formatting utilities (
formatTime,formatBytes,getPerformanceRating) are well-implemented with appropriate thresholds aligned to Core Web Vitals standards.
496-918: LGTM!The table rendering and resource aggregation logic is well-implemented with proper averaging across iterations and clear visual formatting.
920-1005: LGTM!The
resultsCommandfunction handles multiple selection modes cleanly with proper validation and user-friendly interactive defaults.
1104-1123: LGTM!The
thirdPartyDomainscalculation now properly aggregates unique hostnames across all resource types with appropriate URL parsing error handling.
1161-1176: LGTM!The final printing logic correctly prioritizes baseline display and passes results to the detailed printer.
packages/runner/src/benchmark-runner.ts (7)
35-61: Solid initialization and early validation wiring.
Constructor wiring is clear and validation happens immediately, which keeps failures explicit.
66-95: Config validation and warnings look good.
The warnings for missing selectors or wait conditions are practical and non-blocking.
209-268: Per-attempt isolation and retry flow look solid.
New context/page per attempt keeps retries clean and avoids cross-attempt contamination.
270-319: Trace persistence flow is clear and failure-tolerant.
The optional trace path is well-contained and degrades gracefully.
324-332: Cleanup pacing is sensible.
The delay and optional GC hook provide a clear buffer between iterations.
337-456: Iteration orchestration and result aggregation look good.
The control flow is straightforward and reporting is clear.
463-483: Wait condition fallback logic is clear.
Explicit waits with a selector fallback are straightforward to reason about.
| import color from "picocolors"; | ||
| import prettyMilliseconds from "pretty-ms"; | ||
| import type { BenchmarkScores } from "../types"; | ||
| import { isAdminUser } from "../utils/auth"; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unused import: isAdminUser.
The isAdminUser import appears to be unused in this file, consistent with the commit message about removing admin user logging from the results command. This should be removed to avoid dead code.
-import { isAdminUser } from "../utils/auth";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { isAdminUser } from "../utils/auth"; | |
| import { isAdminUser } from "../utils/auth"; |
🤖 Prompt for AI Agents
In `@packages/cookiebench-cli/src/commands/results.ts` at line 14, Remove the
unused import isAdminUser from the top-level imports in the results command
file; locate the import statement that reads "import { isAdminUser } ..." and
delete that specifier (or the whole import line if it only contained
isAdminUser) so no dead code remains and run the linter/TypeScript build to
confirm the unused import is gone.
| // Initialize collectors | ||
| const collector = new PerformanceMetricsCollector(); | ||
| const cookieBannerMetrics = this.cookieBannerCollector.initializeMetrics(); | ||
|
|
||
| // Setup monitoring and detection | ||
| await this.networkMonitor.setupMonitoring(page, url); | ||
| await this.cookieBannerCollector.setupDetection(page); | ||
| await this.perfumeCollector.setupPerfume(page); | ||
|
|
||
| // Navigate to the page with timeout | ||
| this.logger.debug(`Navigating to: ${url}`); | ||
| try { | ||
| await page.goto(url, { | ||
| waitUntil: "networkidle", | ||
| timeout: NAVIGATION_TIMEOUT_MS, | ||
| }); | ||
| } catch (error) { | ||
| throw new Error( | ||
| `Navigation timeout or failed: ${error instanceof Error ? error.message : String(error)}` | ||
| ); | ||
| } | ||
|
|
||
| // Wait for the specified element | ||
| await this.waitForElement(page); | ||
|
|
||
| // Wait for network to be idle | ||
| this.logger.debug("Waiting for network idle..."); | ||
| await page.waitForLoadState("networkidle"); | ||
|
|
||
| // Collect core web vitals from playwright-performance-metrics (primary source) | ||
| this.logger.debug("Collecting core web vitals..."); | ||
| const coreWebVitals = await collector.collectMetrics(page, { | ||
| timeout: BENCHMARK_CONSTANTS.METRICS_TIMEOUT, | ||
| retryTimeout: BENCHMARK_CONSTANTS.METRICS_RETRY_TIMEOUT, | ||
| }); | ||
|
|
||
| this.logger.debug("Core web vitals collected:", { | ||
| fcp: coreWebVitals.paint?.firstContentfulPaint, | ||
| lcp: coreWebVitals.largestContentfulPaint, | ||
| cls: coreWebVitals.cumulativeLayoutShift, | ||
| tbt: coreWebVitals.totalBlockingTime, | ||
| }); | ||
|
|
||
| // Collect Perfume.js metrics (supplementary - TTFB, navigation timing, network info) | ||
| this.logger.debug("Collecting Perfume.js supplementary metrics..."); | ||
| const perfumeMetrics = await this.perfumeCollector.collectMetrics(page); | ||
| this.logger.debug("Perfume.js metrics:", perfumeMetrics); | ||
|
|
||
| // Collect cookie banner specific metrics | ||
| const cookieBannerData = | ||
| await this.cookieBannerCollector.collectMetrics(page); | ||
| this.logger.debug("Cookie banner metrics:", cookieBannerData); | ||
|
|
||
| // Collect detailed resource timing data | ||
| const resourceMetrics = await this.resourceTimingCollector.collect(page); | ||
|
|
||
| // Get network metrics | ||
| const networkRequests = this.networkMonitor.getNetworkRequests(); | ||
| const networkMetrics = this.networkMonitor.getMetrics(); | ||
|
|
||
| // Aggregate all metrics | ||
| const finalMetrics = this.performanceAggregator.aggregateMetrics({ | ||
| coreWebVitals, | ||
| cookieBannerData, | ||
| cookieBannerMetrics, | ||
| networkRequests, | ||
| networkMetrics, | ||
| resourceMetrics, | ||
| config: this.config, | ||
| perfumeMetrics, | ||
| }); | ||
|
|
||
| // Log results | ||
| this.performanceAggregator.logResults( | ||
| finalMetrics, | ||
| cookieBannerMetrics, | ||
| this.config | ||
| ); | ||
|
|
||
| // Cleanup | ||
| await collector.cleanup(); | ||
| this.networkMonitor.reset(); | ||
|
|
||
| return finalMetrics; |
There was a problem hiding this comment.
Ensure cleanup/reset runs on failures to avoid stale metrics.
If an exception occurs before Line 199 (e.g., navigation or wait failures), collector.cleanup() and this.networkMonitor.reset() are skipped, which can bleed state into retries. Wrap the body in a try/finally to guarantee cleanup.
🔧 Proposed fix
- // Initialize collectors
- const collector = new PerformanceMetricsCollector();
- const cookieBannerMetrics = this.cookieBannerCollector.initializeMetrics();
-
- // Setup monitoring and detection
- await this.networkMonitor.setupMonitoring(page, url);
- await this.cookieBannerCollector.setupDetection(page);
- await this.perfumeCollector.setupPerfume(page);
-
- // Navigate to the page with timeout
- this.logger.debug(`Navigating to: ${url}`);
- try {
- await page.goto(url, {
- waitUntil: "networkidle",
- timeout: NAVIGATION_TIMEOUT_MS,
- });
- } catch (error) {
- throw new Error(
- `Navigation timeout or failed: ${error instanceof Error ? error.message : String(error)}`
- );
- }
-
- // Wait for the specified element
- await this.waitForElement(page);
-
- // Wait for network to be idle
- this.logger.debug("Waiting for network idle...");
- await page.waitForLoadState("networkidle");
-
- // Collect core web vitals from playwright-performance-metrics (primary source)
- this.logger.debug("Collecting core web vitals...");
- const coreWebVitals = await collector.collectMetrics(page, {
- timeout: BENCHMARK_CONSTANTS.METRICS_TIMEOUT,
- retryTimeout: BENCHMARK_CONSTANTS.METRICS_RETRY_TIMEOUT,
- });
-
- this.logger.debug("Core web vitals collected:", {
- fcp: coreWebVitals.paint?.firstContentfulPaint,
- lcp: coreWebVitals.largestContentfulPaint,
- cls: coreWebVitals.cumulativeLayoutShift,
- tbt: coreWebVitals.totalBlockingTime,
- });
-
- // Collect Perfume.js metrics (supplementary - TTFB, navigation timing, network info)
- this.logger.debug("Collecting Perfume.js supplementary metrics...");
- const perfumeMetrics = await this.perfumeCollector.collectMetrics(page);
- this.logger.debug("Perfume.js metrics:", perfumeMetrics);
-
- // Collect cookie banner specific metrics
- const cookieBannerData =
- await this.cookieBannerCollector.collectMetrics(page);
- this.logger.debug("Cookie banner metrics:", cookieBannerData);
-
- // Collect detailed resource timing data
- const resourceMetrics = await this.resourceTimingCollector.collect(page);
-
- // Get network metrics
- const networkRequests = this.networkMonitor.getNetworkRequests();
- const networkMetrics = this.networkMonitor.getMetrics();
-
- // Aggregate all metrics
- const finalMetrics = this.performanceAggregator.aggregateMetrics({
- coreWebVitals,
- cookieBannerData,
- cookieBannerMetrics,
- networkRequests,
- networkMetrics,
- resourceMetrics,
- config: this.config,
- perfumeMetrics,
- });
-
- // Log results
- this.performanceAggregator.logResults(
- finalMetrics,
- cookieBannerMetrics,
- this.config
- );
-
- // Cleanup
- await collector.cleanup();
- this.networkMonitor.reset();
-
- return finalMetrics;
+ const collector = new PerformanceMetricsCollector();
+ const cookieBannerMetrics = this.cookieBannerCollector.initializeMetrics();
+ try {
+ // Setup monitoring and detection
+ await this.networkMonitor.setupMonitoring(page, url);
+ await this.cookieBannerCollector.setupDetection(page);
+ await this.perfumeCollector.setupPerfume(page);
+
+ // Navigate to the page with timeout
+ this.logger.debug(`Navigating to: ${url}`);
+ try {
+ await page.goto(url, {
+ waitUntil: "networkidle",
+ timeout: NAVIGATION_TIMEOUT_MS,
+ });
+ } catch (error) {
+ throw new Error(
+ `Navigation timeout or failed: ${error instanceof Error ? error.message : String(error)}`
+ );
+ }
+
+ // Wait for the specified element
+ await this.waitForElement(page);
+
+ // Wait for network to be idle
+ this.logger.debug("Waiting for network idle...");
+ await page.waitForLoadState("networkidle");
+
+ // Collect core web vitals from playwright-performance-metrics (primary source)
+ this.logger.debug("Collecting core web vitals...");
+ const coreWebVitals = await collector.collectMetrics(page, {
+ timeout: BENCHMARK_CONSTANTS.METRICS_TIMEOUT,
+ retryTimeout: BENCHMARK_CONSTANTS.METRICS_RETRY_TIMEOUT,
+ });
+
+ this.logger.debug("Core web vitals collected:", {
+ fcp: coreWebVitals.paint?.firstContentfulPaint,
+ lcp: coreWebVitals.largestContentfulPaint,
+ cls: coreWebVitals.cumulativeLayoutShift,
+ tbt: coreWebVitals.totalBlockingTime,
+ });
+
+ // Collect Perfume.js metrics (supplementary - TTFB, navigation timing, network info)
+ this.logger.debug("Collecting Perfume.js supplementary metrics...");
+ const perfumeMetrics = await this.perfumeCollector.collectMetrics(page);
+ this.logger.debug("Perfume.js metrics:", perfumeMetrics);
+
+ // Collect cookie banner specific metrics
+ const cookieBannerData =
+ await this.cookieBannerCollector.collectMetrics(page);
+ this.logger.debug("Cookie banner metrics:", cookieBannerData);
+
+ // Collect detailed resource timing data
+ const resourceMetrics = await this.resourceTimingCollector.collect(page);
+
+ // Get network metrics
+ const networkRequests = this.networkMonitor.getNetworkRequests();
+ const networkMetrics = this.networkMonitor.getMetrics();
+
+ // Aggregate all metrics
+ const finalMetrics = this.performanceAggregator.aggregateMetrics({
+ coreWebVitals,
+ cookieBannerData,
+ cookieBannerMetrics,
+ networkRequests,
+ networkMetrics,
+ resourceMetrics,
+ config: this.config,
+ perfumeMetrics,
+ });
+
+ // Log results
+ this.performanceAggregator.logResults(
+ finalMetrics,
+ cookieBannerMetrics,
+ this.config
+ );
+
+ return finalMetrics;
+ } finally {
+ await collector.cleanup();
+ this.networkMonitor.reset();
+ }🤖 Prompt for AI Agents
In `@packages/runner/src/benchmark-runner.ts` around lines 120 - 203, The method
currently initializes PerformanceMetricsCollector (collector) and calls
operations like page.goto, this.waitForElement, collector.collectMetrics, etc.,
but calls collector.cleanup() and this.networkMonitor.reset() only on success;
wrap the main work (from after collector initialization through
aggregation/logging) in a try/finally so cleanup always executes:
declare/initialize collector before the try, run navigation, waitForElement,
collects and aggregation inside try, and in finally call await
collector.cleanup() and this.networkMonitor.reset(); ensure any caught errors
are rethrown after finally so failures still propagate.
1 participant
…ookie-control'; update configurations and dependencies. Includes new METHODOLOGY.md for benchmarking guidelines and adjustments to .gitignore and package.json scripts.