refactor(router): one segment walker; fresh.ts deleted (S9) - #1244

devcorner: rango-t138 · session rango-s312

Problem

The router resolved a matched chain's segments with two walkers: segment-resolution/fresh.ts (945 lines) for a full match (a document, the background re-render of a stale route record, a build-time prerender) and segment-resolution/revalidation.ts for a partial match. They held parallel copies of the same orderings (entry loaders, handler first, parallels, orphan layouts, nested routeless layouts, inherited loaders, the cache() and ppr header latches, dedupe), so every fix to one had to be found and repeated in the other. This is step S9 of the internals reorganisation (plan rango-d195 section 3 S9, changed by section 7; target rango-d194 section 3.2). No public API or rendered output changes. Three types that the @rangojs/router/__internal subpath type-exports (each marked @internal) change shape: RouterContext loses resolveAllSegments and resolveLoadersOnly; ResolutionPassInput.request becomes Request | undefined, and the type gains skipLoaders and throwOnError; MatchContext loses loaderPromises. No app in the repo imports them, and they are not a Breaking changelog entry.

Before / after (internal shape)

Before, a document:

resolveAllSegments(entries, routeKey, params, handlerContext, loaderPromises, {
  skipLoaders,
  throwOnError,
  chain, // ResolveSegmentOptions
});

After, every match goes through the one walker; a full match passes a FRESH pass:

resolveAllSegmentsWithRevalidation(
  freshPassInput(handlerContext, routeKey, params, {
    skipLoaders,
    throwOnError,
  }),
  entries,
  { pathname, chain },
);

FRESH (resolution-pass.ts) is an empty ClientHeld compared by identity: a partial request whose client holds nothing (NOTHING_HELD, an empty _rsc_segments) still asks the revalidate() rules. On a FRESH pass every revalidation decision short-circuits to "new" with no trace entry and no revalidation.decision event.

The differential's named exceptions, as branches

walker-differential.test.ts compares a FRESH pass with a partial pass whose client holds an empty set. Its exception list now describes these branches; the names and expected lines are unchanged, and the FRESH-output pins (fresh: lines) are unchanged.

Divergences the differential does not see (it compares segments only) are kept as explicit FRESH branches too, so the full match stays as it was. They are recorded on the ticket as finding rango-t138-c2:

• resolveOwnerLoaders: synchronous kickoffs with no decision; loader(Def, { ssr: false }) registration before kickoff, awaitBeforeFlush stamp, default-boundary argument to wrapLoaderPromise, and the await of flagged loaders; loading(false) settles every loader outside a shell capture; the loader error context has no isPartial.
• Slot handlers: FRESH meters them and runs them in the rango.handler span (observeSegmentHandler), with no streamed-Response warning; an evicted slot handler with no static copy emits no slot segment.
• Orphan layouts: FRESH meters the handler; a FRESH orphan's slot loaders come after the slot segment.
• Route handlers: FRESH meters only a handler that runs, not a static hit.
• A void layout handler keeps component: undefined on FRESH (cache-scope.ts refuses a document record whose layout component is null).
• resolveAllSegmentsWithRevalidation reports an entry error without isPartial on FRESH.

Code change

• Deleted src/router/segment-resolution/fresh.ts. No wrapper exports remain: RouterContext.resolveAllSegments, RouterContext.resolveLoadersOnly, ResolveSegmentOptions and their segment-wrappers.ts bindings are gone. MatchContext.loaderPromises, whose last reader was the fresh walker's unused argument, is gone too (match-api.ts still hands its local map to setupLoaderAccess).
• resolution-pass.ts: FRESH, freshPassInput, and the pass fields skipLoaders and throwOnError.
• revalidation.ts: the FRESH branches above. The partial lane's decision code moved unchanged into loadersToRerun, entryRevalidates and orphanRevalidates, so a FRESH pass runs the handler step without asking them.
• Callers: match-middleware/segment-resolution.ts (one call for full and partial), cache-lookup.ts (the live pass above a cache() HIT and the loaders-only serve, with the resolveLoadersOnly parameter threading removed), background-revalidation.ts, prerender-match.ts (PrerenderMatchDeps.resolveAllSegmentsWithRevalidation), router.ts.
• Not touched: loader resolution (loader-cache.ts, loader-resolution.ts; comments only, naming the new functions), intercept selection, tree-shape files.
• Tests: the 13 unit files that called the fresh walker now call the walker with a FRESH pass, through a new kit helper src/__tests__/kit/resolution-pass.ts (makeFreshPass). RouterContext mocks drop resolveAllSegments / resolveLoadersOnly. prefetch-false-unflagged.rsc-test.tsx's snapshot was not re-recorded.
• New bench src/router/segment-resolution/__tests__/resolve-all.bench.ts: one FRESH pass per route over 13 routes with the shapes the test-app's documents walk.
• Docs: docs/internal/feature-map.md, docs/internal/feature-file-map.md, and every doc, skill and comment that named fresh.ts or its functions.

Size: non-test source 24 files, +787 / -1549; fresh.ts, revalidation.ts and resolution-pass.ts together +639 / -1301. Most other source lines are comment renames.

Test plan

• walker-differential.test.ts: 42 passed on the base (7b38ae4) and 42 passed on this branch, same exception list, same FRESH pins.
• The five revalidation-trace*.test.ts files produce identical traces: each pushRevalidationTraceEntry call recorded to a file per test file on the base and on this branch; all five pairs are byte-identical (29 entries; 13 + 5 + 6 + 4 + 5 tests pass on both).
• Temporary golden comparison (not committed): the base's fresh.ts against the FRESH pass on one router, 16 routes x 2 manifest partitions x 3 modes (request, background skipLoaders, build time), plus an evicted-slot case and the live pass above a cache() marker: 99/99 equal on segments (key order included, _handlerRan aside), delivered segments after buildMatchResult, perf-metric labels, rango.handler spans, awaitBeforeFlush registrations, response status, and telemetry events as a multiset. Event order differed on one route (/orphan): the route loader's loader.end now precedes the inherited loader's loader.start (see Notes).
• E2e, packages/rangojs-router, --project=dev --no-deps and --project=production --no-deps on semantic-matrix, handler-first, loader-behavior, revalidation, parallel-loader-inherit, parallel-new-slot-reval, prerender, prerender-intercept, intercept-whole-request (file filters, confirmed with --list): dev 105 passed, production 95 passed. revalidation.test.ts has no production describe (allowlisted in tools/e2e-parity-allowlist.json before this change); loader-behavior runs 11 dev and 4 production tests.
• E2e, tests/cloudflare-basic: production prerender, prerender-ctx, cache-orphan-nested 35 passed; dev prerender-dev (the dev counterpart: prerender.test.ts is production-only), prerender-ctx, cache-orphan-nested 14 passed.
• Hard rule 1 gate, at the head: pnpm run typecheck (16 tasks); pnpm run test:unit:all: router 564 files, 9347 passed (5 skipped, 6 todo); router RSC 83 files, 1065 passed (3 expected fail); mini, vite-rsc-demo, cloudflare-basic and shared-e2e suites all passed; oxlint --deny-warnings 0 findings; oxfmt --check clean; CI lint scripts (check-e2e-bucketing, check-e2e-parity --strict, check-e2e-console-baseline, check-suspense-contract, check-no-external-store, node --test tools/*.test.mjs 51 passed, check-doc-links, check-docs-api, check-skills, generate-skills-catalog --check, generate-route-types --check) and node tools/check-internals.mjs all OK. No *.gen.ts changes.
• Timing: see "After the timing gate".

Practices

• Lint: oxlint --deny-warnings and oxfmt --check on the whole repo, plus the CI lint scripts above: no findings. tsc --noUnusedLocals, diffed against the base, found one import this change left unused (HandlerContext in prerender-match.ts); removed.
• Dead code: removed RouterContext.resolveAllSegments / resolveLoadersOnly, ResolveSegmentOptions, their segment-wrappers.ts bindings, the loaderPromises argument the fresh walker never read, the resolveLoadersOnly parameter threaded through seven cache-lookup.ts sites, and the resolveLoadersOnly / resolveAllSegments fields of test mocks, and MatchContext.loaderPromises, left unread by the fresh walker's removal (the field, its two match-api.ts assignments and four test fixtures). Found and left, as it predates this change: RouterContext.resolveAllSegmentsGenerator and collectSegmentsFromGenerator are declared (optional) and never set or read.
• Tests: the touched unit files, the full test:unit:all, the walker-differential before and after, a trace comparison and a temporary golden comparison, and the targeted e2e above. No consumer-visible behaviour is added; two new unit tests pin the base's static slot behaviour (R3), and the 13 unit files of the deleted functions now run the same assertions against the FRESH pass.
• Code review: the merged walker reviewed by hand against the base's fresh.ts, branch by branch; that review found the divergences listed above (finding rango-t138-c2), each kept as a FRESH branch. A second review of the diff (comment rango-t138-c3) compared every function of the deleted fresh.ts with its FRESH branch, the revalidation traces on the base and the head, and the telemetry order; it requested four changes, all made (see "After review rango-t138-c3"). Left as follow-ups: the ...WithRevalidation renames and items 1-3 of finding rango-t138-c2.
• Docs: docs/internal/feature-map.md, docs/internal/feature-file-map.md (pass description, kit helper, and the stale resolveOrphanLayout and ResolveSegmentOptions.* references), docs/internal/execution-model.md, internal and design docs, the handler-use, ppr and cache-guide skills, and source comments naming fresh.ts or its functions. docs/archive/ left unchanged: it quotes historical code.

After review rango-t138-c3

Two commits on top of the reviewed head; the history before them is unchanged.

• R3, refactor(router): hand a static slot value to render unawaited: the merged slot loop awaited a non-function slot value when the slot has no loading() (component = await run()), so a Promise-valued static slot (a React 19 ReactNode) was unwrapped during the walk and a static JSX slot paid one extra microtask. Both walkers before the merge assigned the value as is; the slot loop does again (typeof handler === "function" ? await run() : handler). New tests in segment-resolution-parallel-loading.test.ts, one per pass (FRESH and partial), pin that the slot segment's component is the declared Promise itself. Without the fix both fail (expected 'slot-content' to be Promise{…}); with it, the file passes 10/10.
• R1, refactor(router): drop the unread MatchContext.loaderPromises: the field and its doc line, the two match-api.ts assignments and the fixtures in background-revalidation.test.ts, revalidation-trace-match-handlers.test.ts, revalidation-trace-cache-hit.test.ts and match-result.test.ts. The background regression test still asserts that setupLoaderAccess gets a new, empty Map, now on the background handler context; without the field the foreground map cannot reach the re-render.
• R2 (same commit): stale names fixed in feature-map.md (resolveOrphanLayout, ResolveSegmentOptions.chain, .boundaryEntry, .throwOnError, "the fresh path"), feature-file-map.md (ResolveSegmentOptions.throwOnError) and execution-model.md ("fresh and revalidation route handlers"). A repo-wide grep for resolveAllSegments, resolveLoadersOnly, ResolveSegmentOptions, fresh.ts, resolveOrphanLayout and resolveParallelEntry outside docs/archive/ finds none.
• R4: the Problem section now names the @rangojs/router/__internal type changes.

Checks at the head: walker-differential.test.ts 42 passed; the five revalidation-trace*.test.ts files 13 + 5 + 6 + 4 + 5 passed; background-revalidation.test.ts 9 and match-result.test.ts 41 passed; the Hard rule 1 gate and CI lint scripts as listed in the test plan. No e2e: R3 restores the base's behaviour and a unit test pins it.

After the timing gate

The committed bench (resolve-all.bench.ts, resolveAll over 13 routes) ran 9.8% slower than the base (7b38ae4) on a quiet machine, slower in every round. Three commits on top of 8f7d350 close it; the history before them is unchanged. After them, two driver runs under load average 4 measure head/base 1.000 and 0.991.

Profile finding (heap sampling profile that keeps collected objects, 1000 iterations of the bench workload; CPU profiles of 4000 iterations; V8 --trace-opt --trace-deopt of 3000 iterations from a cold start):

• Allocation per iteration: base 837 KB, 8f7d350 984 KB (+17.6%); the walker's own frames 237 KB against 393 KB. The largest new sites: resolveParallelSegmentsWithRevalidation 56 KB (the base's slot code allocated 8 KB), resolveEntryHandlerWithRevalidation 48 KB plus its run closure 36 KB (the base ran the handler inline in resolveSegment), resolveOwnerLoaders 41 KB (the base's resolveLoaders 34 KB).
• Cause, allocation: for every chain entry the walker made an async call to resolveParallelSegmentsWithRevalidation (a promise, a Set, two result arrays and the getParallelSlotEntries arrays, for an entry without a slot) and to resolveOwnerLoaders (an ownerOf object and two empty arrays, for an entry without a loader), and resolveEntryHandlerWithRevalidation built its decide and run closures and one more async layer on every call, FRESH included, which never asks decide. The entry segment also spread three conditional object literals: about 80 ns per segment, against 12 ns with the keys assigned (measured standalone).
• Cause, warm-up: with the allocation fixed, the default bench (100 ms warmup, 500 ms measured) still read 1.041 on a quiet machine while a 3 s measurement read 1.002. The trace shows why: resolveOwnerLoaders deoptimized twice (wrong map at its per-loader object array's map calls) and reached stable optimized code after about 2200 iterations, against about 1100 for the base's resolveLoaders; the bench measures roughly the first 2000.
• Ruled out: pass shapes (the bench builds one FRESH pass shape, copied once per walk by createSegmentWrappers); revalidation inputs on FRESH (revalidationInputOf runs only on a partial pass's held-segment branches); GC survival (--trace-gc-nvp: 80 scavenges and 8 mark-compacts on both sides, promoted bytes within 1%); prefetch-deferral reads on FRESH (skipping them, tried and not kept, moved the instruction count by under 0.5% and left the deopts as they were).

What changed (revalidation.ts only):

• perf(router): drop the walker's empty async calls and per-entry closures: ownsLoaders and declaresSlot guard the calls to resolveOwnerLoaders and resolveParallelSegmentsWithRevalidation (chain entries, orphan layouts, slot loaders, the loaders-only walk), each equivalent to the callee's own early return. The partial lane's decisions move unchanged to entryRevalidates and orphanRevalidates, and the route handler run to resolveRouteComponent, so a FRESH pass allocates no closure for them. A loader segment is built in one literal, the awaitBeforeFlush stamp is shared, and no settled-loader array is built when nothing is awaited.
• perf(router): assign a chain entry segment's optional keys: clientGroup, layoutName, mountPath, _handlerRan and deferred are assigned after the fixed keys, in the same key order.
• perf(router): build loader segments by index in resolveOwnerLoaders: the segment ids are built once, loadersToRerun returns the indices of the loaders that run, and the segments come from one loop over the loaders (FRESH) or those indices. In the same trace, resolveOwnerLoaders deoptimizes once and is stable by about iteration 800.

Results at the head:

• Allocation per iteration: 842 KB (base 837 KB, +0.5%); the walker's frames 261 KB.
• Instructions retired by the test worker per iteration (proc_pid_rusage, 2000 iterations after a 2000-iteration warmup, median of 3 runs): base 6.66 M, 8f7d350 6.99 M (+5.0%), after the first two commits 6.72 M (+1.0%).
• Timing driver (the committed bench, base and head in alternating rounds, 10 rounds each), at the head: base 0.3096 ms, head 0.3096 ms, head/base 1.000 (1-minute load average at most 3.39 during the run); rerun: base 0.3180 ms, head 0.3151 ms, 0.991 (at most 3.97). Before the third commit: 1.041 (at most 3.31).

Segments (key order included), matched ids, revalidation traces and telemetry are unchanged; only the number of microtask hops changes, fewer than at 8f7d350. Checks at the head: walker-differential.test.ts 42 passed; the five revalidation-trace*.test.ts files 13 + 6 + 5 + 5 + 4 passed, and every pushRevalidationTraceEntry call recorded per test file at 8f7d350 and at the head is byte-identical (30 entries in four files; revalidation-trace-match-handlers.test.ts mocks the pipeline and records none); segment-resolution-parallel-loading.test.ts 10 passed; prefetch-false-unflagged.rsc-test.tsx 3 passed, snapshot untouched; the Hard rule 1 gate (pnpm run typecheck 16 tasks; pnpm run test:unit:all: router 9347 and RSC 1065 passed, mini, vite-rsc-demo, cloudflare-basic and shared-e2e passed; oxlint --deny-warnings 0 findings; oxfmt --check clean) and the CI lint scripts with check-internals, all OK. No e2e: no behaviour changes. The temporary golden comparison was not re-run.

Practices for these three commits:

• Lint: oxlint --deny-warnings and oxfmt --check on the repo: no findings.
• Dead code: the removed closures and the OwnedLoader type had no other users; revalidate (loader-resolution.ts) is still used by loadersToRerun. One read left unused by the change (routeKey in resolveOrphanLayoutWithRevalidation) was removed.
• Tests: the gates above, and the trace recording compared at 8f7d350 and the head. No new test: no behaviour changes, and the differential, trace and snapshot tests pin the output.
• Code review: the diff reviewed by hand against 8f7d350, function by function: each guard matches the callee's early return, the slot-loader dedupe check is taken where nothing between it and the call can change it, the entry segment keeps its key order, the loader loop runs defersLoader, kickoff and deferredLoaderRuns in the same order per loader, and the partial lane's decision code is unchanged. No findings left.
• Docs: no doc or skill names the changed internals (repo-wide grep); nothing to update.

Updated onto reorg/stack

reorg/stack at 2fa88da is merged in (a merge commit; the history before it is unchanged). It brings S16 (WIRE), S5 (deriveRenderPass), S15 (discovery phases), S14 and S14b (request stages, dispatch), S10b (Vercel store), S6 (ShellCaptureState), #1246 and #1247.

Conflicts and their resolution:

• src/server/request-context.ts: S6 removed _shellCaptureRun, whose doc comment this PR edited. S6's _shellCaptureState field and doc are kept. This PR's fact, that the capture switch is read by resolveOwnerLoaders rather than fresh.ts emitStreaming, now sits on ShellCaptureState.run in rsc/shell-capture.ts, which S6 had copied the old text to.
• docs/design/ppr-shell-resume.md: S6's _shellCaptureState.run, with this PR's resolveOwnerLoaders settle guard in place of fresh.ts emitStreaming.
• docs/internal/feature-file-map.md: one table hunk, because both sides realigned the columns. Each row comes from the side that changed it. The two rows both sides changed are combined: "PPR shell caching (axis 2)" keeps S6's field names and this PR's revalidation.ts link in place of fresh.ts, and "Internal unit-test kit" lists S6's makeShellCaptureState and this PR's makeFreshPass.
• docs/internal/feature-map.md: the transition() entry keeps S15's vite/discovery/dev-server-phase.ts path and this PR's revalidation.ts without fresh.ts.

Semantic conflicts:

• src/server/__tests__/render-pass.bench.tsx (new in S5): the background re-render's RouterContext stub provided resolveAllSegments, which this PR removes. rerenderAndCacheRoute calls resolveAllSegmentsWithRevalidation, so the stub now provides that. Without the fix, the "background re-render (stub resolution)" bench reports NaN. With it, the bench measures 89,933 hz.
• Checked, and no change was needed: the background FRESH pass in rerenderAndCacheRoute runs inside runWithRequestContext(renderCtx), where renderCtx is S5's deriveRenderPass(requestCtx, "background-revalidation", …), exactly as the base pass does. The walker reads the capture switch only through loader-mask.ts isShellCaptureActive(), which S6 rewrote to _shellCaptureState?.run. No source or test reads _shellCaptureRun, _shellCaptureForVisitor, _shellCaptureLoaderRecords, _shellCaptureLoaderLanes or _routeRecordWindow. Outside docs/archive/, no file names fresh.ts, emitStreaming, resolveLoadersOnly or ResolveSegmentOptions. No added line on reorg/stack uses a name this PR removes.

Checks at the merge:

• pnpm run typecheck: 16 tasks passed. pnpm run test:unit:all: router 567 files, 9363 passed (5 skipped, 6 todo); router RSC 84 files, 1067 passed (3 expected fail); mini 25 + 3, vite-rsc-demo 27 + 1, cloudflare-basic 90 + 39 and shared-e2e 20 all passed.
• Unchanged by the merge and passing: walker-differential.test.ts, the five revalidation-trace*.test.ts files, segment-resolution-parallel-loading.test.ts, and prefetch-false-unflagged.rsc-test.tsx, whose snapshot was not re-recorded. Also passing: S3's derived-context-shape.rsc-test.tsx, and every test file S6 touched (render-pass-capture-state, context-fast-properties, cookie-store, request-headers, shell-capture, loader-mask-capture, await-before-flush, loader-pins, loader-seed-overlay, and the rest). In total, 26 unit files with 718 tests and 4 RSC files with 88 tests.
• oxlint --deny-warnings: 0 findings. oxfmt --check: clean. The CI lint scripts (check-e2e-bucketing, check-e2e-parity --strict, check-e2e-console-baseline, check-suspense-contract, check-no-external-store, node --test tools/*.test.mjs with 53 passed, check-doc-links, check-docs-api, check-skills, generate-skills-catalog --check and generate-route-types --check) all pass. node tools/check-internals.mjs passes at the baseline, with no raise. No *.gen.ts changes.
• E2e, packages/rangojs-router, --project=dev --project=production --no-deps --grep "semantic-matrix|handler-first|loader-behavior|revalidation|shell-cache|prerender" (confirmed with --list: 600 tests in 49 files, 303 dev and 297 production): 599 passed and 1 flaky. The flaky test was dev shell-cache "stale SWR navigation replays the captured handler promise, top-level handles, and Meta". On its first attempt its wall-clock budget (toBeLessThan(1_200)) read 1320 ms after the content assertions had passed. It passed on retry, and its production counterpart passed. Rerun alone in dev with --repeat-each=3 --retries=0, it passed 3/3. It has not been run on the base.

Practices for the merge:

• Lint: oxlint --deny-warnings and oxfmt --check on the repo: no findings.
• Dead code: neither side's removals leave a caller on the other side. Typecheck covers the removed RouterContext methods, and the repo-wide grep above covers the old names. Nothing new was left unused.
• Tests: the gates and the targeted e2e above. No new test: the merge adds no behaviour, and the bench stub fix is shown by the bench result.
• Code review: the combined diff of the merge commit was reviewed by hand against both parents: each conflict hunk and the clean-merged files both sides touched (background-revalidation.ts, loader-mask.ts, loader-cache.ts, context.ts, match-api.ts and the shared tests). No findings left.
• Docs: the three conflicted docs above, and ShellCaptureState.run's comment. No other doc names fresh.ts or the old shell-capture fields.

Notes

• Microtask timing: the merged full match resolves a chain entry's handler in its own async function (resolveEntryHandlerWithRevalidation, and resolveRouteComponent for a route), where fresh.ts ran it inline; it skips the loader and slot calls for an entry that owns no loader or slot, where fresh.ts made an async loader call for every entry. Kickoff order is unchanged (loaders before the handler, handler first, the route loop's kickoffs synchronous), but independent async work interleaves differently at microtask granularity; the golden comparison shows it as loader telemetry event order on one route. A streamed handler that pushes a handle after an await reads the live _currentSegmentId, a race the partial lane already has.
• On a document, buildMatchResult (match-result.ts) now strips _handlerRan from each FRESH segment that carries it (the non-loader segments), a shallow copy each. The bench does not cover it; one object spread per segment is not expected to show.
• Names: the walker functions keep their ...WithRevalidation names to keep this diff reviewable; a rename to the target names (resolveAll, resolveLoaders) is a follow-up.
• Items 1-3 of finding rango-t138-c2 (slot and orphan metering and spans, static-hit metering) look like gaps of the partial lane; each is a behaviour change for its own PR.