mirror of
https://github.com/cloudflare/vinext.git
synced 2026-09-14 19:04:59 +08:00
vinext@0.1.7
15 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Alexander Lichter
|
92e4df5e33 | fix(web): prevent benchmark chart label overlap (#2187) | ||
|
Alexander Lichter
|
5581c506b3 | feat(web): link benchmark chart points to commits (#2185) | ||
|
Alexander Lichter
|
175662f108 | feat(web): link benchmark references to GitHub (#2184) | ||
|
James Anderson
|
0fb4fe475a | feat(web): show one hundred benchmark runs (#2178) | ||
|
Alexander Lichter
|
44b0c9dcff |
chore(perf): pair pull request benchmark baselines (#2159)
* fix(perf): pair pull request benchmark baselines * fix(perf): bootstrap benchmark fingerprint rollout * fix(perf): harden paired benchmark publishing * fix(perf): allow isolated build output creation * fix(perf): transfer isolated build outputs * fix(perf): verify isolated output permissions * fix(perf): validate diagnostic trace categories * fix(perf): retain trusted benchmark manifests * fix(perf): close benchmark validation gaps --------- Co-authored-by: James <james@eli.cx> |
||
|
James Anderson
|
44802b7dd6 | fix(perf): stabilize stale pull request benchmarks (#2153) | ||
|
James Anderson
|
bb9a034021 |
feat(web): show fifty benchmark runs (#2143)
* feat(web): show fifty benchmark runs * fix(perf): run nextjs benchmarks first |
||
|
James Anderson
|
e46e2dbb05 |
feat(web): add performance benchmark dashboard (#2131)
* feat(web): add performance benchmark dashboard * fix(web): clean up benchmark dashboard exports * fix(web): support benchmark runs without baselines * fix(perf): support targeted main baseline runs * fix(web): revalidate benchmark commit baselines * feat(perf): improve vinext trace profiles * fix(web): neutralize insignificant benchmark diffs * fix(perf): replace existing benchmark runs * fix(perf): expose vinext trace frames * feat(perf): retain complete vinext trace detail * feat(perf): filter flame graph categories * fix(web): compact flame graph rows * fix(perf): replace runs by commit identity * fix(benchmarks): preserve complete performance traces * fix(benchmarks): use commit time for performance runs * fix(benchmarks): store performance profiles in R2 * fix(benchmarks): stream performance profiles to R2 * fix(benchmarks): decode raw performance profiles * fix(benchmarks): show inclusive filtered profile time * fix(benchmarks): resize filtered flame graphs * fix(benchmarks): render narrow flame frames * perf(benchmarks): bound flame graph detail * ci(perf): split benchmark publication privileges * fix(perf): harden benchmark publication * fix(perf): initialize runner paths in job * fix(perf): install through vite plus * fix(perf): disable pnpm hooks during install * fix(perf): select trusted harness ref * fix(perf): resolve vite plus for isolated user * fix(perf): grant isolated tool access * fix(perf): share benchmark cache cleanup * fix(perf): harden trace publication * chore(perf): format migration snapshots * feat(web): restore benchmarks navigation |
||
|
James Anderson
|
e9b0f68658 | feat(web): expose latest compatibility failures (#1941) | ||
|
James Anderson
|
8d00797ada |
feat(cache): extract Cloudflare cache adapters into @vinext/cloudflare (#1748)
* refactor(cache): extract Cloudflare cache adapters into @vinext/cloudflare Move the Cloudflare KV data cache and edge CDN cache adapters out of vinext into a new publishable @vinext/cloudflare package: - cache/kv-data-adapter(.runtime).ts (KVCacheHandler, kvDataAdapter) - cache/cdn-adapter(.runtime).ts (CloudflareCdnCacheAdapter, cdnAdapter) tpr.ts stays in vinext. vinext now depends on @vinext/cloudflare (workspace:*) and the package declares vinext as a peer dep; both build from source via tsconfig paths so there is no build-order cycle. The vinext/cloudflare barrel still re-exports KVCacheHandler for back-compat. Wires up tsconfig paths, a vitest source alias, root build/postinstall, and the preview/publish workflows for the new package. Updates internal consumers (apps/web, examples/workers-cache), docs, and tests. * ci(create-next-app): install @vinext/cloudflare from local tarball vinext now depends on @vinext/cloudflare, which isn't published to npm yet. The create-next-app smoke test packs vinext locally and resolves its deps from the registry, so the install (and dev server) failed with ERR_PNPM_FETCH_404 for @vinext/cloudflare. Pack @vinext/cloudflare alongside vinext and add a pnpm override in the scaffolded project pointing at the local tarball so the dependency resolves offline. * refactor(cloudflare): address review feedback - Remove the root barrel export from @vinext/cloudflare; expose only the ./cache/* subpaths via a wildcard export (no root main/types). - vinext/cloudflare re-exports KVCacheHandler from the full subpath. - Drop the redundant .npmignore (the package.json "files" allowlist already restricts the publish to dist). - Remove the unsupported imperative setCacheHandler/KVCacheHandler usage from both READMEs; the cache plugin config is the supported approach. - Simplify test wiring: drop the now-unused @vinext/cloudflare tsconfig path and dedupe the vitest source alias into a shared constant. * chore(cloudflare): drop unused vite devDependency The @vinext/cloudflare config uses vite-plus and nothing imports vite, so the vite devDependency was unused. build/check/knip stay green without it. * Apply suggestion from @james-elicx |
||
|
James Anderson
|
f0f6aa72e0 |
feat(cache): configure cache adapters from vite plugin config (#1733)
* feat(cache): configure cache adapters from vite plugin config
Add a `cache` option to the vinext() plugin so CDN and data cache
adapters can be declared in vite.config instead of calling
setDataCacheHandler() / setCdnCacheAdapter() from a worker entry:
vinext({
cache: {
cdn: { adapter: require.resolve('vinext/cloudflare/cache/cdn-adapter') },
data: { adapter: require.resolve('vinext/cloudflare/cache/kv-data-adapter') },
},
})
Each slot points at an adapter module whose default export is a factory
(DataCacheAdapterFactory / CdnCacheAdapterFactory). The plugin generates
a virtual:vinext-cache-adapters module that the App Router worker entry
calls per request (self-guarded, once per isolate), passing the host env
so binding-backed adapters (e.g. KV) can read their namespace.
Ships ready-made Cloudflare adapter entry points:
- vinext/cloudflare/cache/kv-data-adapter (KVCacheHandler)
- vinext/cloudflare/cache/cdn-adapter (CloudflareCdnCacheAdapter)
* feat(cache): add typed adapter builders (kvDataAdapter/cdnAdapter)
Instead of `{ adapter: require.resolve(...) }`, each adapter module now
also exports a config-time builder from the same path:
import { cdnAdapter } from 'vinext/cloudflare/cache/cdn-adapter';
import { kvDataAdapter } from 'vinext/cloudflare/cache/kv-data-adapter';
vinext({ cache: { cdn: cdnAdapter(), data: kvDataAdapter({ binding: 'MY_KV' }) } })
A builder returns a plain, serializable { adapter, options } descriptor —
it never touches the Workers runtime, so nothing throws at config / build
/ dev time when bindings aren't available. Descriptor `options` (e.g. the
KV binding name) are inlined into the generated registration module and
forwarded to the factory's { env, options } context, where the binding is
resolved lazily on the first request.
- shims/cache-adapter: descriptors + options-aware factory/context types
- kv-data-adapter: kvDataAdapter() builder + configurable binding/appPrefix/ttl
- cdn-adapter: cdnAdapter() builder
- raw { adapter, options } path form still supported
* test(cache): verify absolute (require.resolve) local adapter path bundles
Real Cloudflare build pointing cache.data at a local adapter file by
absolute path (what require.resolve('./adapter') yields). Proves the
generated registration module resolves the absolute import, bundles the
local adapter into the worker, and does not need any Workers context at
build time.
* refactor(cache): builder require.resolve + register across all routers/runtimes
Addresses review feedback:
* Move adapters into their own runtime modules instead of re-exporting.
Each adapter is now a builder module (kv-data-adapter.ts / cdn-adapter.ts)
plus a sibling *.runtime.ts holding the default-export factory. Type
definitions have a single home in shims/cache-adapter.ts (dropped the
re-export shim; index.ts imports the config type from there).
* The exposed builder utility resolves the relative runtime path internally
via import.meta.resolve (the ESM require.resolve), so the descriptor carries
an absolute path to the runtime factory rather than a bare specifier — the
example is just kvDataAdapter({ binding }), no require.resolve at the call site.
* Register configured cache handlers EVERYWHERE, not just the App Router worker:
- App Router: the generated RSC entry passes registerConfiguredCacheAdapters
into createAppRscHandler, which calls it per request — covering Workers,
the Node server, and dev through the one shared handler.
- Pages Router: the generated server entry registers in renderPage and
handleApiRoute (Node/dev), and the generated worker registers with env
(Workers, for KV bindings).
Registration self-guards (first call with real env wins) and is now resilient:
a factory that throws on an incompatible runtime is logged and skipped, so the
default handler stays in place instead of failing every request.
Tests: generator-level assertions that every router/runtime entry wires
registration, plus the existing builder/codegen/factory and full-build coverage.
vp check clean; app-router (339) and pages-router (272) suites pass.
* refactor(cache): keep all Cloudflare adapter code under cloudflare/
The adapter factory contract lived in shims/cache-adapter.ts (outside
cloudflare/), and the Cloudflare adapters reached out to it. Move the
contract into cloudflare/cache/adapter.ts so every Cloudflare-specific
cache adapter file is self-contained under cloudflare/ — importing only
cloudflare-local modules and the core CacheHandler/CdnCacheAdapter
interfaces it implements.
The plugin's config schema (CacheAdapterDescriptor / VinextCacheConfig)
is genuinely framework-level (it's the vinext() `cache` option), so it
moves into the codegen module the plugin already owns; index.ts imports
it from there. Builders return a structural { adapter, options } so they
don't import the descriptor type either. Deletes shims/cache-adapter.ts.
* refactor(cache): merge KV/CDN classes into the runtime adapter files
All Cloudflare cache code now lives in one directory, cloudflare/cache/,
and each runtime file holds both the implementation class and its
config-driven factory (no separate class module to reach for):
- kv-cache-handler.ts -> cache/kv-data-adapter.runtime.ts
(KVCacheHandler + ENTRY_PREFIX + createKvDataCacheAdapter default export)
- cloudflare-cdn-cache.ts -> cache/cdn-adapter.runtime.ts
(CloudflareCdnCacheAdapter + createCloudflareCdnCacheAdapter default export)
Updated importers: cloudflare/index.ts re-exports the classes from the
runtime files, tpr.ts pulls ENTRY_PREFIX from there, shims/cdn-cache.ts
imports the edge adapter from there, and the tests follow the moved paths.
git mv preserves history.
vp check clean; cache/kv/cdn/app-route/tpr/shims suites pass (1300+ tests).
* chore(cache): trim low-value comments added in this branch
Remove narrating/redundant comments that just restated the code; keep
the non-obvious why (registration ordering/resilience, import.meta.resolve
rationale, edge cache-control semantics). No code changes.
* review: address PR #1733 feedback
- Make registerCacheAdapters a required field on the RSC handler options
(the generated entry already passes it; test factory updated).
- Remove the separate cloudflare/cache/adapter.ts contract file; inline the
factory param types directly into the two runtime adapters.
- Drop the CloudflareCdnCacheAdapter re-export from cloudflare/index.ts.
- Fold the virtual:vinext-cache-adapters declaration into global.d.ts and
delete the standalone .d.ts.
- Remove the ./cloudflare/cache/* package.json export for now; README uses a
local-adapter require.resolve example with a note that the built-in adapter
export paths are pending.
- Rename the config-driven KV default binding to VINEXT_KV_CACHE (imperative
deploy/tpr path keeps VINEXT_CACHE — flagged on the thread).
* refactor(cache): align KV binding name to VINEXT_KV_CACHE everywhere
Rename the KV cache binding from VINEXT_CACHE to VINEXT_KV_CACHE across the
whole codebase so the config-driven adapter, the imperative deploy-generated
worker, TPR's wrangler detection, and the apps/web example all agree. The
unrelated X-Vinext-Cache response-header constant (VINEXT_CACHE_HEADER) is
untouched.
* tidy
* .
* .
* .
* .
* .
* Move apps/web cache to plugin config
Co-authored-by: james-elicx <james-elicx@users.noreply.github.com>
---------
Co-authored-by: ask-bonk[bot] <ask-bonk[bot]@users.noreply.github.com>
Co-authored-by: james-elicx <james-elicx@users.noreply.github.com>
|
||
|
James Anderson
|
61cf2e78f5 |
fix(image): emit /_next/image URLs (#1562)
* fix(image): emit /_next/image URLs to match Next.js Closes #1513 The default image loader and optimization endpoint switched from the vinext-specific /_vinext/image path to Next.js's canonical /_next/image. This unblocks the deploy suite tests that import Next.js's expected URL shape (/_next/image?url=...&w=...&q=...). * refactor(image): use IMAGE_OPTIMIZATION_PATH constant at remaining call sites Replace hardcoded "/_next/image" strings in index.ts, app-rsc-handler.ts, and the generated worker entry templates in deploy.ts with the IMAGE_OPTIMIZATION_PATH constant from server/image-optimization, matching the pattern already used in prod-server.ts. Prevents future drift if the path ever changes again. * feat(image): accept both /_next/image and /_vinext/image at the optimizer Add a VINEXT_IMAGE_OPTIMIZATION_PATH constant and an isImageOptimizationPath() helper, then route through every match site (prod-server, dev server passthrough, app RSC handler, generated worker templates, and shipped example workers). Apps that wire image URLs to either prefix now hit the same handler; new URLs are still emitted via IMAGE_OPTIMIZATION_PATH. |
||
|
James Anderson
|
f27c40a77f |
chore: split e2e compatibility by App Router vs Pages Router (#1321)
* feat(compat): split e2e compatibility by App Router vs Pages Router
The /compatibility page previously showed one undifferentiated grid of
~1000 Next.js test files. This change classifies each test by which
router(s) its fixture exercises (App, Pages, both, or unknown) and
surfaces the breakdown in the UI.
How:
- A new `router` column on `compat_file_results` (enum: app | pages |
both | unknown). Defaults to 'unknown' so pre-classifier rows still
render — they just show up under 'Other'.
- A new `scripts/classify-nextjs-suites.mjs` walks each test's fixture
directory and looks for app/page.tsx, app/route.ts, app/layout.tsx,
pages/*.tsx (excluding _app / _document / _error specials). A fixture
with real routes in both folders is classified as 'both' — these are
the genuine parity tests and counting them only once would hide a
router-specific failure.
- Edge cases handled in the classifier:
* pageExtensions naming (layout.page.tsx → layout)
* Suites whose .test.ts lives in a test/ subdir alongside the fixture
* Inline-fixture suites under test/e2e/app-dir/ (no on-disk routes,
but path convention says App Router)
* APP_ROUTER_NON_APP_DIR_SUITES curated override
* Skips node_modules and .next when scanning fixtures
- The nightly workflow now runs the classifier in the build job once
per run (the cheap part — happens with Next.js still on disk),
uploads the suite → router map as an artifact, and the report job
joins it into the ingest payload before POSTing.
- The compatibility page gains a 'By router' card row showing per-router
pass rates and file counts (parity tests are counted toward both
router buckets — adding them exceeds the total, see the explainer
card). The contribution grid grows filter chips above it for
interactive narrowing.
Verified:
- vp check (format + type + lint) clean
- 14 new classifier unit tests pass (vp test run tests/classify-nextjs-suites.test.ts)
- apps/web build:vinext succeeds end-to-end
- Local classifier run against .nextjs-ref reports
app=576 / pages=325 / both=113 / unknown=22 out of 1036 suites, with
spot-checked classifications matching expectations.
Backward compatibility: the ingest endpoint treats `router` as optional;
existing workflow runs and historical rows continue to function and
render as 'Other' until the next nightly classifies them.
* refactor(compat): store router classification in its own table
Replaces the inline `router` column on `compat_file_results` (introduced
in the previous commit) with a dedicated `compat_suite_meta` table,
classification keyed by `suite`. The /compatibility UI LEFT JOINs the
two tables at query time.
Rationale (per design discussion in PR #1321):
- Classifications conceptually describe test files, not test runs. Storing
them per-row coupled their cadence to results ingestion, which is wrong
when (a) the Next.js ref bumps and re-classifies everything, (b) an
override fix lands without re-running tests, or (c) a partial test run
still wants fresh classifications.
- One row per suite (PK on `suite`) means re-classifying is an upsert,
not a backfill loop. Provenance (`next_ref`, `classified_at`) is
stored on the row for debugging.
- Decoupling lets the workflow POST classifications from the build job
immediately after running the classifier, without round-tripping through
the report job. Results ingestion stays focused on results.
Endpoints:
- `POST /api/compatibility` no longer accepts `router` per file
(reverted to its pre-PR shape).
- `POST /api/compatibility/classify` (new): accepts
`{ nextRef, classifiedAt?, suites: [{ suite, router }] }`,
upserts in chunks (25 rows per INSERT to fit SQLite's variable cap),
shares auth with the results endpoint via a new `_auth.ts` helper.
Workflow:
- The build job now POSTs the classification map directly to the new
endpoint after running the classifier. Same guardrails as the results
POST (only full-suite, only against main).
- The report job no longer needs to download / merge the classification
artifact — it's already in D1 by the time results land.
- The classification JSON is still uploaded as a workflow artifact for
manual re-submission / debugging.
Schema:
- New table `compat_suite_meta(suite PK, router, next_ref, classified_at)`
with an index on `router` for the per-router count queries.
- `compat_file_results` reverts to its pre-PR columns. No data migration
needed because the column was only ever populated on this branch.
Trade-off: classification changes are now retroactive — re-classifying a
suite updates how it appears in every historical run. This is usually
what you want (corrections heal the whole history) but means the trend
chart isn't a strict point-in-time record. The provenance fields on the
meta row let you tie a reclassification back to a specific Next.js ref.
Verified:
- vp check clean
- 14 classifier unit tests still pass
- apps/web build:vinext succeeds; /api/compatibility/classify shows up
in the route list
* refactor(compat): drop redundant next_ref from compat_suite_meta
The Next.js ref a classification was produced against is already
recoverable from the run history — compat_runs records next_ref per
run, and the most recent classification's ref is implicit (it's the
ref of the most recent classify POST, which the workflow always pairs
with a run).
The future-proofing case for per-ref classifications would want a
composite (suite, next_ref) PK rather than a single global row anyway,
so this column doesn't help with that scenario either. classified_at
is enough for the debug case ('when was this last computed?').
Changes:
- Drop next_ref column from compat_suite_meta (3 columns now:
suite PK, router, classified_at)
- /api/compatibility/classify no longer requires nextRef in the body
- Workflow no longer passes NEXT_REF when building the classify payload
- Migration regenerated as 0001_romantic_skullbuster.sql (one fewer
column on the CREATE TABLE)
- Chunk size bumped to 33 rows/INSERT (100-var cap / 3 columns)
Verified: vp check clean, classifier tests pass, apps/web build succeeds.
* feat(compat-ui): share router filter between grid and trend chart
Lifts the router-filter state out of ContributionGrid into a new
CompatibilityViews client wrapper that owns the Kumo segmented Tabs
control. The grid and the line chart both consume the active filter
as a prop, so changing the tab updates both visualisations in lockstep.
The line chart now plots per-router series. The trend query was
rewritten to aggregate via JOIN against compat_suite_meta — one row
per run with app/pages/both/unknown rollups in a single round-trip,
~90 runs × ~1000 file rows over an indexed join. The TrendPoint
carries all five series; the chart picks one based on the filter
without re-fetching.
Other UI tweaks in this commit:
- Replaced the hand-rolled pill row with Kumo Tabs (variant='segmented',
size='sm'). Fixes a weird active state (bg-kumo-default + text-kumo-base
was using the text color for backgrounds, producing a saturated
inversion).
- 'Parity' → 'Mixed' in user-visible labels. Internal identifiers
(DB enum 'both', bucket variable 'parity') unchanged.
- Removed the standalone 'Other' stat card; the segmented Tabs still
expose the 'Other' filter so unclassified suites are reachable.
- Reorganised the page into one 'Test files and trend' section
containing both visualisations, gated by the shared Tabs.
- .gitignore: ignore .dev.vars (Wrangler's local-secrets convention)
so the COMPAT_INGEST_SECRET we use for local testing never gets
committed by accident.
Verified:
- vp check clean (format + type + lint)
- 14 classifier unit tests pass
- apps/web build:vinext succeeds; both API endpoints + page render
* fix(compat): address PR review comments
Addresses actionable items from the two /bigbonk review passes:
1. scanFixture: skip recursion into already-checked app/ and pages/
(with a wrinkle around fixture wrappers).
The previous code unconditionally pushed every child onto the walk
stack, which (a) wasted work re-walking subtrees the route checker
already covered and (b) risked a false positive on App Router route
groups literally named 'pages' (e.g. app/pages/index.tsx would have
tripped the Pages Router detector).
Naive fix (skip descent into any app/ or pages/) regressed ~10 real
Next.js fixtures that use a wrapping directory literally named 'app'
as the test app's project root, with the real app/ and pages/ nested
inside (test/e2e/og-api, test/e2e/middleware-static-files, etc.).
Final fix: a directory named app or pages is treated as a fixture
wrapper (and recursed into) if it contains a top-level next.config.*
OR an inner app/ alongside an inner pages/. Otherwise it's handed to
the route checker, which either finds real routes or returns nothing
and the walk skips it. The wrapper detector intentionally does NOT
treat middleware.{js,ts} as a wrapper signal, because Next.js tests
put noop middleware.js files inside real App Router app/ to assert
the file is ignored at that level (test/e2e/app-dir/app-middleware).
Verified against the full test/e2e/ tree of the local Next.js
checkout: 1036 suites classify identically to the pre-fix output
(576 app / 325 pages / 113 both / 22 unknown), with the
app/pages/index.tsx false-positive now correctly handled.
2. list-nextjs-e2e-suites.mjs: add the same import.meta.url guard the
classifier already has, so importing the module programmatically
doesn't immediately parse process.argv and write a file.
3. _auth.ts: early-exit when the X-Compat-Secret header is missing
or empty, before paying two SHA-256 digests. The constant-time
guarantee we care about (don't leak the contents of the expected
secret via length or prefix matching) is preserved — we only
short-circuit on values we already know can't match a non-empty
secret.
4. Migration 0001_romantic_skullbuster.sql: add trailing newline.
Tests:
- 3 new regression tests added (18 total, all pass):
* fixture wrapper with next.config.js + inner app/ + pages/
* fixture wrapper with next.config.js + inner pages/ only
* App Router app/ containing a route group named 'pages'
* App Router app/ containing a noop middleware.js
- vp check clean
- Classifier output diff against current Next.js HEAD: empty
* fix(compat-ui): unexport cellMatchesFilter (knip)
CI's knip step flagged `cellMatchesFilter` as an unused export. It's only
used inside contribution-grid.tsx itself — the shared wrapper has its
own bucketing logic in compatibility-views.tsx. Drop the `export`.
* fix(compat): address third-pass review comments
1. sqlExcluded: narrow parameter type to a closed union
("router" | "classified_at") so the no-user-input invariant for
sql.raw is compiler-enforced rather than relying on a code comment.
2. ContributionGrid: gate the SVG render on visibleCells.length > 0,
and also clamp svgWidth/svgHeight to >= 0. Previously, when a
filter emptied the grid, the SVG rendered with width=-3/height=-3
alongside the placeholder div (browsers clamped silently but it
was invalid SVG).
3. compatibility/page.tsx 'How this works' card: add a short note
that per-router trend lines use the latest classification, so
reclassifying a suite updates how it appears in historical runs.
The aggregate "All" line is unaffected. Avoids a confusing
support question if users notice old per-router numbers shift
after a classifier improvement.
4. scanFixture readability: simplify the early-return checks from
'if (hasApp && hasPages)' to 'if (hasPages)' / 'if (hasApp)',
since the other flag was just set on the line above. No
behavioural change; classifier output against the full Next.js
test/e2e tree is byte-identical (1036 suites, 576/325/113/22).
5. /api/compatibility/classify body validator: bounds-check
classifiedAt. Reject NaN/Infinity and any timestamp earlier
than 2023-01-01 UTC. Catches the common seconds-vs-milliseconds
mistake and accidental 0/-1 values from buggy callers. The
only legitimate caller (the GH workflow) sends Date.now() so
this is purely defensive.
Verified:
- vp check clean (format + type + lint + knip)
- 18 classifier unit tests pass
- apps/web build:vinext succeeds
- Classifier output against current Next.js HEAD is unchanged
* refactor(compat-ui): extract shared router bucketing module
Addresses items 1, 3, and 4 from the fourth review pass.
1. Trend GROUP BY note (item 1): added an inline comment explaining
that selecting r.created_at while grouping by r.id only is a SQLite
functional-dependency affordance. Standard SQL (postgres etc.)
would reject it; the comment heads off a future 'fix' that breaks
the query if it's ever ported.
2. Unified router bucketing (item 3): the three places that sliced
cells by router filter all used slightly different naming
conventions ('parity' vs 'both', 'other' vs 'unknown'). Extracted
the canonical logic to a new ./router-buckets module with one
shared vocabulary (the same as RouterKind), and updated all three
call sites:
- contribution-grid.tsx: cellMatchesFilter from shared module
- compatibility-views.tsx: countByFilter for tab labels
- page.tsx: bucketByRouter + bucketPassRate for stat cards,
byRouter.parity/other renamed to byRouter.both/unknown
Doc comment at the top of router-buckets.ts pins the
'Mixed counts in both app and pages' rule in one place. No
behavioural change.
3. passRate function rename (item 4): renamed the line chart's local
ratio helper from passRate -> computePassRateRatio to avoid
shadowing the passRate variable in page.tsx (and to distinguish
it from bucketPassRate in router-buckets which returns a
percentage, not a ratio).
Not addressed (intentionally):
- 'Mixed (both)' vs 'Parity'/'Interop' label (item 2): the user
explicitly chose 'Mixed' over 'Parity' in an earlier message.
- trendRowsDesc rename (item 5): reviewer self-tagged 'very minor';
renaming would muddle the diff.
- deriveSuiteGroup inconsistent levels (item 5b): pre-existing
tooltip-only behaviour, not introduced by this PR.
Verified:
- vp check clean (format + type + lint + knip)
- 18 classifier unit tests pass
- apps/web build:vinext succeeds
|
||
|
James Anderson
|
1c0bdacda6 |
feat(apps/web): /compatibility page backed by D1 + deploy-suite ingest (#1239)
* feat(apps/web): /compatibility page backed by D1 + deploy-suite ingest
Adds a /compatibility page to apps/web that visualises Next.js
compatibility over time, populated by the nightly deploy-suite workflow.
- D1 binding (`vinext-metrics`) with Drizzle schema and migrations.
Tables are namespaced (`compat_runs`, `compat_file_results`) so
future metric kinds can live alongside.
- POST /api/compatibility ingest endpoint. Authenticates with a
`COMPAT_INGEST_SECRET` worker secret and upserts by
(kind, runKey).
- GitHub-style contribution grid (one dot per test file) and a
pass-rate-over-time line chart. Both are SVG, no chart libs.
Pass rate excludes Next.js-skipped tests from the denominator.
- Shared header/footer extracted into app/_components and rendered
by the root layout. Nav uses next/link for soft client nav.
- Workflow change: on top of the existing per-test pass/fail/skip
aggregation, the report job now POSTs results to the ingest
endpoint. Guarded to only run when (a) filter=all and
(b) targeting main, so partial runs and branch spot-checks don't
pollute the historical record.
The first nightly run after merge will seed the production D1.
* address review + add ISR with KV cache
Review fixes (apps/web/app/api/compatibility/route.ts):
- constant-time secret comparison via SHA-256 + bytewise XOR
- single-statement atomic upsert with onConflictDoUpdate (no
SELECT->INSERT race for concurrent retried runs)
- top-level try/catch with structured error body so workflow logs
show why ingest failed instead of a generic 500
- MAX_FILES=2000 cap in validation as defensive bound
Page perf (apps/web/app/compatibility/page.tsx):
- latest-run and trend D1 queries run in parallel via Promise.all
ISR + KV:
- new vinext-web-cache KV namespace bound as VINEXT_CACHE
- worker installs KVCacheHandler once per isolate
- both pages opt into ISR with revalidate = 300; compat page drops
force-dynamic
* address bonk re-review
- Drop the ?kind= query param. Only one kind exists today; gating it
behind an unused param invited ISR cache pollution. Hardcoded to
"deploy" with a comment pointing future kinds at a dedicated route.
- Wrap the DELETE + chunked INSERTs in a single `db.batch()` so a
crash mid-write can no longer leave a run with zero file results.
D1 executes batch statements inside one transaction.
- Resolve workflow expressions once via step-level `env:` and read
them as `process.env.*` from the github-script body. Removes the
fragile splicing of `${{ inputs.next-ref || 'v16.2.6' }}` into JS
string literals (the nested single quotes were the worst offender).
|
||
|
James Anderson
|
d9906b94d6 |
chore: add basic vinext site (#1236)
* chore: scaffold new next app with vinext * basic app * add to deploy workflow * fmt |