test(core): create/load distinction test suites (FEP-2548)#726
Draft
ENvironmentSet wants to merge 46 commits into
Draft
test(core): create/load distinction test suites (FEP-2548)#726ENvironmentSet wants to merge 46 commits into
ENvironmentSet wants to merge 46 commits into
Conversation
- CONTEXT.md: add Navigation History term; narrow load fidelity scope to navigation history (R12); snapshot codec is outside core contract (R13) - run-plan-fep-2548-mechanism.md: diverge-converge-refine run plan (7 seeded generators -> curation -> league tournament -> review-loop) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Confirmed mechanism design for distinguishing Stack init (creation) from load (snapshot restoration) in stackflow core. - Snapshot = navigation event log; load = replay through existing aggregate (reachability + fidelity hold by construction — no new validator) - Public contract: StackSnapshot, actions.captureSnapshot(), provideSnapshot / onLoadError / onBeforeInitialPush hooks, onInit(createdBy) one-shot signal - Non-breaking: unchanged creation path when no snapshot provided; overrideInitialEvents preserved and treated as init - Load validation rejects any snapshot event (Pushed/Replaced/future activity-introducing) whose activity is unregistered in load-time config - Resolves the four deferred decisions; proves persister / guard / history-sync consumers close over the core contract alone Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ok, rename init→create path
Maintainer-confirmed revision of the init/load distinction design.
- Remove onBeforeInitialPush; public surface 5 → 4. Create-path entry
interception is done via the existing overrideInitialEvents chain
(inspect / strip = block / replace = redirect), since initial events are
pre-aggregate array data — stripping is the pre-aggregate equivalent of
preventDefault, so no dedicated hook is warranted. Guard now requires zero
new core surface; ordering discipline + an onInit validation belt cover it.
- Terminology: createdBy → initializedBy ("create" | "load"); the "init"
creation path is renamed "create" (built from scratch). "initialize" is the
bootstrap umbrella (onInit / store.init, fires on both paths), so
onInit({ initializedBy: "load" }) is no longer self-contradictory. The
initial* inputs (initialEvents, initialActivity, …) and the Initialized
event keep their names.
- CONTEXT.md ubiquitous language updated: 초기화(Init) → 생성(Create).
- The anti-unification rejection (post-effect pushState duplication, loader
plugin pause counterexample) still stands; only "realize it via a dedicated
hook" is reversed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the FEP-2548 create/load verification suite (core/src/*.spec.ts), 1:1 with the confirmed test plan, plus type-signature stubs for the new public surface so the suite compiles. Implementation is deferred: the new harness is red at the unimplemented points (stub throw / assertion), while the existing core suite stays green (no behavioral change). New public surface (stubs, per design §3): - StackSnapshot / NavigationEvent types (navigation-event subset union) - SnapshotLoadError class with a three-kind cause (incompatible-schema / invalid-events / empty-navigation) - StackflowActions.captureSnapshot() — throws "not implemented yet" - StackflowPlugin.provideSnapshot? / onLoadError? optional hooks - StackflowPluginHook gains initializedBy: "create" | "load", with a "create" placeholder at the onInit call site Suites (core/src): - captureSnapshot: snapshot format + capture edges (transition/pause) - provideSnapshot: single snapshot slot, conflict, null/undefined - createPath: create sequence, override interception, N1/N2, no-hook - loadPath: structure/registration/replay/postcondition/routing, L1/L3/L6, sync - rebase: order (RB1), settle (RB2), clock reversal (RB4), id preserve (RB5) - initializedBy: create signal, no persisted trace - snapshotRoundtrip: capture∘load∘capture stability (L5) - persisterRoundtrip: capture→JSON→load round trip + corrupt-snapshot recovery The empty-navigation "all-popped" case uses a pops-only snapshot (events:[Popped]) because core cannot pop the root activity, so a Pushed→Popped history never reaches zero enter-state activities; the pops-only snapshot reaches that state and preserves the item's intent (non-empty events, zero enter activities). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fill the create/load stubs so a stack knows how it was born — freshly
(`create`) or restored from a snapshot (`load`) — and close the snapshot
round-trip (capture → persist → load) through additive plugin contracts.
A store with no snapshot provider takes the same code path as before.
- captureSnapshot() normalizes the raw event log (sort by eventDate,
dedupe by id, keep only navigation events), so it reflects pause-queued
events and the array order equals the replay order.
- makeCoreStore polls provideSnapshot across plugins once: zero non-null
-> create; exactly one -> load; more than one -> a plain creation error
naming the conflicting keys (not a SnapshotLoadError, not routed).
- loadSnapshot reconstructs the stack by replaying the snapshot's
navigation events through the existing aggregate machinery: structure
check (incompatible-schema), registration check over Pushed and Replaced
(invalid-events), rebase to a settled past window, replay, then an
enter-state postcondition (empty-navigation). Static info
(transitionDuration, registered set) is re-derived from the current
config; id/activityId/stepId are byte-preserved, only eventDate rebased.
- A failed load is routed only to the providing plugin's onLoadError;
{ recover: "create" } resumes the create path without re-polling, and
void/no-handler throws out of makeCoreStore.
- onInit now receives the real initializedBy signal.
No new domain events, stack state properties, makeCoreStore options, or
changes to aggregate/validateEvents/reducers/overrideInitialEvents.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The JSDoc, changeset, and runtime (`?? null`) all promise that returning `undefined` means "nothing to provide", but the declared return type `StackSnapshot | null` rejected a bare `return;`. Widen it to `StackSnapshot | null | void`, following the sibling hook `onLoadError`'s existing `| void` precedent (including the biome-ignore rationale), and drop the forced cast the old type required in provideSnapshot.spec.ts. Runtime behavior is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…often additive claim (C1#9, C2#5)
- `provideSnapshot?(({ initialContext }))` / `onLoadError?(({ error, ... }))`
double-parenthesis typos — these render verbatim in the CHANGELOG.
- `SnapshotLoadError`'s cause is a discriminated object, not a string enum:
describe it as `cause.kind` so release-note readers don't write
`error.cause === "invalid-events"` (always false).
- "entirely additive" overstated the change: `StackflowActions.captureSnapshot`
and `onInit`'s `initializedBy` are required members, which is compile-breaking
for mock constructors and wrap-and-forward hook callers. State the claim as
runtime-additive with a type-level caution.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…w; make persister mimic self-handle decode failure (C1#4) A throwing hook is undefined behavior today: the error propagates raw out of makeCoreStore and onLoadError is never involved. Two characterization tests pin that propagation so any future change to it is a conscious contract decision. The persister mimic previously demonstrated the fragile pattern (raw JSON.parse inside provideSnapshot — a truncated storage write would crash boot). As the reference implementation persister authors will copy, it now self-handles decode failure: discard the stored value and return null so creation falls back to the create path. A third round-trip test exercises that branch end-to-end. Existing assertions are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…1#10)
The recover contract is that a { recover: "create" } return resumes the
create path from its start — not merely that some stack comes out. Existing
recover tests only asserted the resulting activity list, so a regression
that skipped the overrideInitialEvents chain or the initial-activity
handlers on the recovery path would have passed unnoticed.
Pin the coexistence case of a failing provider and an
overrideInitialEvents plugin: the chain runs over the option's initial
events, its substitution is what the stack is built from, the
initial-activity handler judges that substitution, and onInit reports
initializedBy "create".
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…uplicate-id last-wins, unknown-property tolerance (C1-nit-3, C2#7)
Three edges of the load contract existed only as unstated behavior:
- onLoadError returning a truthy value other than the exact
{ recover: "create" } decision rethrows the SnapshotLoadError — the
recovery check must never loosen into truthiness.
- A consumer-transformed snapshot with duplicate event ids is accepted,
and dedup keeps the last occurrence (the earlier event is dropped).
Note the direction: last-wins, the opposite of what C2#7's prose
described.
- Unknown properties — both on the snapshot object and on individual
event items — are tolerated (forward compatibility), so the structure
check isn't tightened by accident.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Post-load navigation was only tested for push, leaving half of the homogeneity postcondition unpinned: a rebase regression that broke pop/step targeting would have passed the suite. Both interact directly with rebased dates — pop exits the latest activity and StepPushed resolves its target as the latest active activity by eventDate. Pin both on a restored two-activity stack: pop starts the exit transition on the restored top and re-exposes the restored activity below as active; stepPush lands its step on the restored top, not the activity underneath. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…C2#4) The capture contract promises that replaying a pause-time snapshot yields the state as if the pause never happened — queued navigation fully applied. Only the capture half was tested (the queued event is included in the snapshot); nothing verified that loading such a snapshot actually materializes the queued activity. Close the round trip: pause, push (queued, not yet visible), capture, then load into a fresh store — the queued activity comes out settled (enter-done, on top) and the restored stack is idle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three comments restated what the code (or a type) already says and would drift as the code changes: - loadSnapshot.ts header enumerated the four checks and their error mapping — that truth lives in the SnapshotLoadError cause type and the function body. The WHY that stays: static info is re-derived from the current config, never the snapshot. - assertSnapshotStructure's JSDoc walked through its own checks; keep only why the check exists (fail loudly before replay, not fold into a corrupt stack). - createStack's JSDoc narrated chain/handlers/aggregate steps; keep only the guarantee it encodes (no provider → observably identical to before). Design-doc marker references (WHY pointers) are intentionally kept. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… (C1#5, C2#1) The rebase used a fixed 1ms integer spacing that never looked at static event dates. The react integration backdates static events by only 2×transitionDuration, so a snapshot with more events than transitionDuration ms (350 by default) had its earliest events folding before Initialized/ActivityRegistered — the design's general placement rule (inside the window after the static events, fractional dates fitting any count) existed only on paper, while the code comment claimed RB1–RB5. Give the rebase the latest static event date as the window's exclusive lower bound and space events fractionally: min(1, window/(N+1)) ms keeps any history length inside the window, and a roomy window degrades to the previous 1ms spacing. A degenerate window (static events dated at or past the settled bound) falls back to settledness alone, as the design's exception path argues is safe. Pin the placement with a 400-event snapshot against react-style static events: every rebased date lands after every static event, at or before creation − transitionDuration, strictly increasing, and the replay settles. The pin fails on the previous fixed-spacing implementation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…napshot→events→stack pipeline (review: error-kind rename) The three kinds misnamed what they detect. `incompatible-schema` suggested a version incompatibility, but the check is a catch-all for any unrecognizable snapshot shape ($schema mismatch, non-array events, an item that is not a navigation event) — now `unrecognized-snapshot`. `invalid-events` suggested a defect intrinsic to the events, but the failure is relational — the events are fine, they just don't fit the current config (e.g. they materialize an unregistered activity) — now `incompatible-events`. `empty-navigation` describes a replay that settles with zero enter-state activities, i.e. an empty stack — now `empty-stack`. Alongside the rename: - `unrecognized-snapshot` gains a required `detail: string` naming which structural check failed (its sibling `incompatible-events` already carried one); the item check includes the offending index. It is a `string` (not `unknown`) because core authors the message itself, whereas `incompatible-events` wraps arbitrary thrown values. - The cause JSDoc states `empty-stack`'s precise condition: zero enter-state activities, not an empty `activities` array — exit-done activities may remain. All three kinds are public exports but unreleased, so the rename is free now and breaking later. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…record (review: initInfo record)
The one-shot create/load signal was a bare string
(initializedBy: "create" | "load"). Promote it to a discriminated
record, initInfo: { kind: "create" | "load" }, because the two shapes
age differently: promoting a string to a record later is a breaking
change to the hook signature, while adding fields to an existing record
is additive. Per-path payloads are already latent — "create" is set
both on plain creation and on recover-after-load-failure, which a
consumer cannot currently tell apart but a record could expose
additively (e.g. { kind: "create", recoveredFrom }), and the load side
has room for provenance (e.g. { kind: "load", providedBy }). The
discriminant is named kind, matching SnapshotLoadErrorCause's
discriminated union.
The signal is unreleased, so the rename is free now and breaking later.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… an initInfo signal (review: load interception)
Activities restored from a snapshot never met an interception point:
the replay bypassed both overrideInitialEvents (skipped on load) and
onBeforePush (replay is not the action pipeline), so a consumer like
plugin-loader could not attach loaderData to restored activities —
"an entered activity runs its loader" broke exactly on the load path.
Run the plugins' overrideInitialEvents chain on load too:
- The hook signature widens to NavigationEvent[] in and out and gains
initInfo: { kind: "create" | "load" } — the same one-shot record
onInit receives (StackInitInfo, now a shared exported type). On load
the chain receives the snapshot's full replay sequence and its
return is adopted as the replay sequence.
- The chain runs after the structure check (hooks never see an
unrecognizable value) and before everything else, so the
registration check, rebase, replay, and enter-state postcondition
all apply to the sequence that actually replays. A reshaped return
that fails validation surfaces as a SnapshotLoadError routed to the
snapshot provider; a throwing hook is a plugin bug, not a snapshot
defect, and propagates raw.
- Load non-interception is no longer structural — it is each
consumer's responsibility. A plugin with no load policy must return
initialEvents unchanged, and plugins that fabricate initial events
(history-sync's URL interpretation) or write activityContext from
creation-scoped input (plugin-loader's initialLoaderData branch)
must guard on initInfo.kind === "load" in their own packages before
any snapshot provider ships alongside them — required follow-ups
within the same unreleased window as this mechanism.
- MakeCoreStoreOptions' onInitialActivityIgnored handler parameter
widens with the chain's return type, and the history-sync spec's
hand-built hook-argument call sites add the now-required initInfo.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…adError (review: dead prototype workaround) Object.setPrototypeOf(this, SnapshotLoadError.prototype) restores the prototype chain that breaks only when `class extends Error` is down-level transpiled to ES5. This package never targets that low: the JS build (esbuild) is es2015 and the type build (tsc) is ESNext, both of which emit a native `class` whose native `extends Error` keeps the prototype chain — so `instanceof SnapshotLoadError` (makeCoreStore's load recovery branches) already holds without it. The workaround guards against an emission mode this build never produces. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…(review: unify registration check) loadSnapshot ran its own Pushed/Replaced registration check before replay. The Pushed half was already redundant — aggregate calls validateEvents, whose throw loadSnapshot wraps into incompatible-events — and validateEvents only omitted Replaced, though Replaced materializes an activity by name exactly as Pushed does. Add the Replaced arm to validateEvents (correcting that asymmetry at the source) and delete loadSnapshot's dedicated block. rebase touches only eventDate, and validateEvents is order-independent, so moving the check from pre-rebase to the post-rebase aggregate leaves the result identical; both routes surface as incompatible-events, so the observable failure is unchanged. The Replaced check is a missing-check correction, not a contract change: config-first replace only targets registered names, so it does not fire on the live path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rop the window math (review: rebase static together) The load rebase carried a latestStaticEventDate reduce, a static→settled window, fractional spacing, and a degenerate-window fallback for one reason: the self-imposed rule that static events (Initialized, ActivityRegistered) keep their original eventDate. But static events are navigation-inert — aggregate reads their eventDate only as a sort key, transitionDuration is a payload field rather than that event's date, and captureSnapshot never persists static events — so they need only stay ordered before the navigation events. Re-date static and navigation events in a single backward walk from now − transitionDuration, one step per event, and the window machinery falls away. Static-before-navigation becomes structural at any history length. It also fixes a latent td=0 defect: the react integration backdates static by 2·td, so at td=0 static and freshly re-dated navigation collided and the degenerate fallback sorted navigation ahead of static — surviving only because the reducer tolerates event order. The shared re-dating removes that reliance. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ew: per-event settled capture) captureSnapshot took every navigation event from the raw log, including events whose own transition had not settled. Load is effect-silent — it assigns the reconstructed stack directly, replaying no PUSHED/onChanged effect — and the rebase drives every restored event to done, so a mid-transition or pause-queued event, once captured, reappeared on reload as a settled state the live session never committed and never fired its done-effect for. A push queued behind a pause that never resumed came back as a done activity that was never shown. Re-aggregate the log at capture time (flooring now at the latest event date, as dispatchEvent does, so a just-committed event reads as settled) and drop any event whose transition is still in flight: the entering event of an enter-active activity (its activity has not committed) and the exiting event of an exit-active one (its last committed state is preserved), plus everything queued in an unresumed pause. The predicate is per-event, not per-activity — a settled Pushed and an unsettled Popped can share a target, and only the Popped drops. This reverses two capture behaviors: a pause-queued push is no longer carried into the snapshot (previously it re-materialized on load), and a mid-transition event is no longer captured until it commits. The snapshot now equals the last committed navigation history. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… not just its entering event (review B1: capture dependency closure) The committed-navigation capture predicate collected uncommitted events per element — the entering event of an enter-active activity, the exiting event of an exit-active one, and pause-queued events. But a step event settles the instant it applies (Step* reducers have no transition gate), so a step dispatched onto an activity still mid-enter stayed "committed" and survived into the snapshot even as its parent's Pushed was dropped. On reload that step is an orphan: core's stepPush leaves targetActivityId unset, so findTargetActivityIndices falls back to the latest active activity and grafts the step onto a different, committed activity below — silent corruption, no error. A persister capturing on every onChanged reaches this through any step taken inside an enter transition. Bind the drop to the activity, not the element. Fold the log once (as aggregate does) and attribute every event to the activity it enters (Pushed/Replaced) or targets (Popped/Step*), resolving step and pop targets against the running stack the same way the reducer does — so a step's parent is known even after the step is superseded. An event is kept only if its activity committed (entry settled), which drops an uncommitted activity's entire span — its steps, surviving or superseded, included — alongside the exit-in-flight Popped and pause-queued events already handled. No step outlives its parent, so no orphan can graft. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…review A1: validateEvents Replaced note) Record in the changeset that folding the snapshot load's registration check into validateEvents also makes aggregate reject an unregistered Replaced on every path, matching its existing Pushed check — dormant in config-first usage where a replace only targets registered activities. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…resumed-paused (review: capture records, not settlement) An earlier revision restricted captureSnapshot to committed (settled) navigation, excluding mid-transition events and adding activity-level attribution so a dropped activity's steps did not orphan. Revisit the model: a transition is only a visual effect, and a navigation event's effect hook (onPushed/onPopped) already fires in the active state — so reconstructing an unsettled event as state on load loses no effect and produces no state/effect mismatch. What makes an event part of the history is that it entered the aggregate, not that its transition settled. Capture every navigation event the log records, excluding only events queued behind a pause that never resumed — those are quarantined out of the aggregate (never applied), so they were never part of the recorded history. This drops the settlement predicate and the attribution machinery that existed only to keep steps from outliving an excluded parent: with mid-transition parents now included, no step is orphaned. Snapshot tests move to the new contract — mid-transition push and mid-transition pop both round-trip, and only an unresumed-paused event is excluded. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…atics (review: as-is capture) captureSnapshot previously held one semantic opinion: events queued behind a pause that never resumed were excluded as "not recorded". Drop that opinion — core exports the stack it is asked about, as-is. The only remaining filter is vocabulary: static events (Initialized, ActivityRegistered) are config-grade information that may legitimately change across reloads, so the current config re-derives them at load time instead of trusting the snapshot. Everything else the stack recorded now rides along, Paused/Resumed included. A snapshot captured during a pause restores a paused stack — the queued navigation stays quarantined, the restored visible state matches what was visible at capture, and resuming in the new session applies the pending navigation. Whether to capture at such a moment is the caller's timing choice; deciding it is no longer core's business. The snapshot event union widens from the six navigation events to SnapshotEvent (navigation + Paused/Resumed): StackSnapshot.events, the overrideInitialEvents chain, the onInitialActivityIgnored handler param, and the load-side structure check all speak the widened vocabulary. Capture no longer re-aggregates the log — it is a pure log transform (sort by eventDate, dedupe by id, drop statics). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…g only statics (review: as-is load) loadSnapshot previously re-dated the whole replay sequence into the settled past so every restored activity folded to enter-done. Drop that normalization: the snapshot's recorded eventDates are the replay truth. A stack captured mid-transition restores mid-transition, a paused stack restores paused, and capture∘load is an identity on the snapshot events (byte-for-byte, eventDate included). Replay order follows the dates, not the array (core-captured snapshots normalize the array to date order, so the two only disagree in hand-crafted snapshots). Only the static events are re-dated, pinned just before the earliest replayed event: Initialized seeds transitionDuration for every later reducer step, and a snapshot whose tail is an unresumed Paused would quarantine statics sorted after it — while their natural "now" dates would sort after a past-dated snapshot. Snapshot events are never touched. Guarantees core no longer provides — a fully-settled restore, and protection from a capture clock that ran ahead (future dates restore unsettled until the local clock catches up, and later navigation sorts after them only once it does) — belong to plugins now: re-date the sequence in overrideInitialEvents. Tests pin both the as-is behavior and that escape hatch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mes, set as ReadonlySet<string> (review: isSnapshotEvent name set) The event-name set is the set of snapshot event names, so constrain its members to `SnapshotEvent["name"]` at construction. Type the set itself as `ReadonlySet<string>` so `isSnapshotEvent` and `isSnapshotEventName` can check membership with a plain event name, dropping both `as` casts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…unflattened (review: incompatible-events detail unknown) Pass the error `aggregate`/`validateEvents` threw straight into `detail` instead of flattening it to `error.message`, so the replay call stack stays inspectable on the failure. `detail` stays `unknown` because it is that raw thrown error, not the offending events — document why, answering the "why not NonEmptyArray<SnapshotEvent>" note. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…review: assertSnapshotStructure optional chaining) `snapshot` is a non-null `StackSnapshot` and makeCoreStore filters out null supplies before calling loadSnapshot, so `snapshot?.$schema` guarded against a value that cannot reach here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…k (review: init-only hook name) The hook type now carries `initInfo` and is only ever the shape of `onInit`, which fires solely at initialization. Rename it so the name states that. The module file keeps its name (it still houses the pre/post effect hook types). BREAKING CHANGE: the exported type `StackflowPluginHook` is renamed to `StackflowPluginInitHook`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…l (review: provideSnapshot explicit null) Drop the `void` alternative so the hook takes an explicit `null` for "nothing to provide" rather than mixing null/undefined/void. Runtime still coerces a stray undefined via `?? null` (the no-handler defense), so the undefined-return test is kept as characterization, not contract. BREAKING CHANGE: `provideSnapshot` must return `StackSnapshot | null`; `return;`/undefined is no longer type-allowed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…licit policy discriminant (review: onLoadError explicit propagate)
Replace the `void` propagate signal with an explicit return so a handler
picks its outcome rather than propagating by falling off the end. Unify
both outcomes under a single `policy` discriminant —
`{ policy: "recover" } | { policy: "propagate" }` — rather than mixing
`{ recover: "create" }` with `{ policy: "propagate" }`, which would force
an `in`-narrowing at the consumer. "recover" resumes the sole create
path; a future recovery target extends the branch, not the discriminant.
BREAKING CHANGE: `onLoadError` now returns
`{ policy: "recover" } | { policy: "propagate" }`; the old
`{ recover: "create" }` / `void` return is replaced.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tatic), matching the snapshot vocabulary (review: initial events non-static/static split) The split fed by these buckets is the same non-static/static division a snapshot draws, and `onInitialActivityIgnored` now receives `SnapshotEvent[]`. Split with `isSnapshotEvent` instead of the narrower Pushed/StepPushed predicate and rename the buckets `initialSnapshotEvents`/`initialStaticEvents`, so the static bucket handed to loadSnapshot is honestly named. Behavior is unchanged: on the create path the only non-static seed is the initial Pushed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dEvents param and bump to major (review: onInitialActivityIgnored param name + major) The handler's parameter is now `SnapshotEvent[]`, so `initialPushedEvents` no longer describes it — rename it (and the local it is fed from) to `overriddenInitialEvents`, the override chain's output that supersedes the ignored initialActivity. The `@stackflow/core` bump goes minor → major, since the widened `onInitialActivityIgnored`/`overrideInitialEvents` signatures, the `StackflowPluginInitHook` rename, and the `provideSnapshot`/`onLoadError` return-shape changes are type-breaking; the changeset now enumerates them. BREAKING CHANGE: `@stackflow/core` is a major release — see the changeset for the full set of type-level breaking changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s recorded (review: captureSnapshot normalize redundant) Load replays through `aggregate`, which already sorts by eventDate and dedupes by id, so the capture-side sort + uniqBy only duplicated that work. Export `events.value.filter(isSnapshotEvent)` in recorded order. The two tests that pinned the old sort/dedupe now pin the as-recorded contract (order preserved; a pathological duplicate id survives capture and is collapsed at load). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…n (review round follow-up) Reflow the SNAPSHOT_EVENT_NAMES set generic and the now-single-line provideSnapshot signature to biome's format. Pure formatting of code added earlier in this review round; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…es (review: captureSnapshot normalize redundant follow-up) Dropping capture-side sort/dedupe (9653237) made two sibling spec comments false: loadPath.spec.ts said duplicate ids "only come from a consumer-transformed snapshot (capture dedupes)", and rebase.spec.ts said "core-captured snapshots normalize array order to date order". Both now contradict this PR's own captureSnapshot tests — correct them to the as-recorded behavior. Comment-only; assertions unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s, not changed types (review: onInitialActivityIgnored param name + major follow-up)
The major bump prose framed two brand-new hooks as shape changes ("now
returns ... in place of the former ..."), but neither existed on the last
published version — the "former" shapes were intra-PR iterations no
consumer saw. Reframe them as new additive surface landing in their final
shape; keep the genuine break vs. published (the StackflowPluginHook ->
StackflowPluginInitHook rename) in the major justification.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the create/load test suites onto a PR stacked on top of this branch so #723 carries the implementation and changeset only: delete the eight new core spec files and return validateEvents.spec.ts to its main-branch state (its added cases were purely additive). historySyncPlugin.spec.ts stays here: its diff is API-conformance only (overrideInitialEvents' initialEvents widened to SnapshotEvent[] and the now-required initInfo field), with no new test cases, so it must ship with the implementation to keep typecheck green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Stacked on top of #723 (feature/fep-2548). Adds the create/load test coverage split out of that PR: - captureSnapshot, provideSnapshot, snapshotRoundtrip, persisterRoundtrip - createPath, loadPath, rebase, initInfo - validateEvents: the registration-check cases added for the load path historySyncPlugin.spec.ts is not here — its change is API conformance, not a new test, so it ships with the implementation in #723. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@stackflow/core
@stackflow/link
@stackflow/plugin-basic-ui
@stackflow/plugin-blocker
@stackflow/plugin-devtools
@stackflow/plugin-google-analytics-4
@stackflow/plugin-history-sync
@stackflow/plugin-lifecycle
@stackflow/plugin-renderer-basic
@stackflow/plugin-renderer-web
@stackflow/plugin-sentry
@stackflow/plugin-stack-depth-change
@stackflow/react-ui-core
@stackflow/react
commit: |
Deploying stackflow-demo with
|
| Latest commit: |
9e80959
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://04a5a336.stackflow-demo.pages.dev |
| Branch Preview URL: | https://feature-fep-2548-tests.stackflow-demo.pages.dev |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
stackflow-docs | 9e80959 | Commit Preview URL | Jul 11 2026, 02:29 PM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test coverage for the create/load distinction in
@stackflow/core, split out of #723 so that PR carries the implementation + changeset only. Stacked on top offeature/fep-2548(#723) — review/merge #723 first.Together with #723, this is byte-for-byte the tree #723 originally carried; nothing was rewritten, only relocated.
What's here
New
@stackflow/corespec suites:captureSnapshot.spec.ts,provideSnapshot.spec.ts— the public capture / provide contractsnapshotRoundtrip.spec.ts,persisterRoundtrip.spec.ts— round-trip + persister-style integrationcreatePath.spec.ts,loadPath.spec.ts— the create vs. load initialization sequencesrebase.spec.ts— event-timestamp rebasing on loadinitInfo.spec.ts—onInitreceivinginitializedBy: "create" | "load"validateEvents.spec.ts— the registration-check cases added for the load pathhistorySyncPlugin.spec.tsis intentionally not here: its change in #723 is API conformance (overrideInitialEvents'initialEventswidened toSnapshotEvent[], plus the now-requiredinitInfo), not a new test — so it ships with the implementation.Notes
changeset statusis satisfied by the@stackflow/corechangeset inherited from feat(core): create/load distinction mechanism for snapshot restoration (FEP-2548) #723.feature/fep-2548; retarget tomainonce feat(core): create/load distinction mechanism for snapshot restoration (FEP-2548) #723 merges.🤖 Generated with Claude Code