From 7d99f49632bbf1d7200400a439000f66b3ae6223 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Wed, 23 Sep 2026 00:25:14 -0700 Subject: [PATCH 1/4] =?UTF-8?q?feat(signals):=20OPTIMISTIC=5FREVERTED=20?= =?UTF-8?q?=E2=80=94=20the=20person=20saw=20the=20guess,=20then=20the=20co?= =?UTF-8?q?rrection?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An optimistic override the screen displayed is replaced by a different value on two roads: the guess lifts at settle back to the committed value it covered (the action failed, or never wrote what it promised), or an authoritative value lands that differs from it. Both are the runtime doing what optimistic UI promises, so the finding is `info` and structured-channel only — a count that grows for one source is what says the guess, or the failure rate, is wrong. Judged by the node's own equality. `AttributionHooks.optimisticReverted(el, shown, truth, how)` is the seam, fired from supersedeOverride (how derived from whether a landing is staged) and from the non-superseded drop in resolveOptimisticNodes. Not covered yet, and said so: optimistic stores, and the interaction that wrote the guess (optimistic writes bypass the write hook, so the node carries no origin). The plan marks item 2 satisfied by #3580 — `at` equals the Event Timing entry's `startTime`; `interactionId` is not readable at dispatch. Size: engine +266 B, tier +38 B; the engine cap moved with a note. Co-Authored-By: Claude via Cursor Co-authored-by: Cursor --- .changeset/optimistic-reverted.md | 5 + .../plans/responsiveness-findings-plan.md | 19 +- documentation/solid-2.0/08-dev-diagnostics.md | 7 + .../signals/src/core/attribution-hooks.ts | 15 ++ packages/signals/src/core/attribution.ts | 43 +++++ packages/signals/src/core/dev.ts | 1 + packages/signals/src/core/optimistic.ts | 18 +- .../attribution-optimistic-revert.test.ts | 169 ++++++++++++++++++ .../skills/reactivity-diagnostics/SKILL.md | 25 +++ scripts/size/.size-limit.js | 5 + 10 files changed, 304 insertions(+), 3 deletions(-) create mode 100644 .changeset/optimistic-reverted.md create mode 100644 packages/signals/tests/attribution-optimistic-revert.test.ts diff --git a/.changeset/optimistic-reverted.md b/.changeset/optimistic-reverted.md new file mode 100644 index 000000000..3242d1e41 --- /dev/null +++ b/.changeset/optimistic-reverted.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +`OPTIMISTIC_REVERTED` (info, responsiveness): an optimistic value the screen showed was replaced by a different one — reverted at settle, or superseded by the truth — with the source and both values. `AttributionHooks.optimisticReverted` is the seam. diff --git a/documentation/plans/responsiveness-findings-plan.md b/documentation/plans/responsiveness-findings-plan.md index c1e087c8e..8d51c8aa2 100644 --- a/documentation/plans/responsiveness-findings-plan.md +++ b/documentation/plans/responsiveness-findings-plan.md @@ -92,7 +92,14 @@ any pre-await write opens the hold path, which `SILENT_HOLD` judges. once and its interaction record's `settledMs` covers the await; the same handler with `isPending(source)` read in JSX fires nothing. -### 2. Stamp the browser's `interactionId` on the interaction +### 2. Stamp the browser's `interactionId` on the interaction — SATISFIED BY #3580 + +`interactionId` lives on the `PerformanceEventTiming` entry, delivered by a +`PerformanceObserver` after the fact; it is not readable during dispatch, so +a stamp was never available. #3580 made `InteractionEvent.at` the event's +`timeStamp`, which equals the entry's `startTime`: the join is by equality +on `at`. Nothing further to do in the runtime; the adapter recipe belongs +in the docs. - **Known:** the web runtime is inside the event dispatch when it opens the frame; Event Timing (`PerformanceEventTiming.interactionId`) assigns @@ -112,7 +119,15 @@ any pre-await write opens the hold path, which `SILENT_HOLD` judges. - **Proof:** a click whose Event Timing entry reports 480 ms produces an interaction record with the same id and a hold that names the blocker. -### 3. Optimistic reverts +### 3. Optimistic reverts — LANDED (signals; stores follow) + +Shipped as `AttributionHooks.optimisticReverted` (two sites in +`optimistic.ts`: supersession, and the drop at settle) and +`OPTIMISTIC_REVERTED` (see RFC 08). Left for a follow-up: optimistic +_stores_ (the overlay folds off per path in `_clearOptimisticStores`), and +naming the interaction that wrote the guess — optimistic writes bypass the +`write` hook today, so the node carries no origin stamp; stamping them +touches the interaction accounting and is its own change. - **Known:** the optimistic lane knows the shown value and the settled value; `asyncEnd`'s `prev`/`value` and the lane commit see both. diff --git a/documentation/solid-2.0/08-dev-diagnostics.md b/documentation/solid-2.0/08-dev-diagnostics.md index 56e580929..c9a634130 100644 --- a/documentation/solid-2.0/08-dev-diagnostics.md +++ b/documentation/solid-2.0/08-dev-diagnostics.md @@ -566,6 +566,12 @@ The other end of the `SILENT_HOLD` spectrum: too much feedback for too little wa When a hold commits, `stackedHolds.count` or more interactions (default 3) were waiting in it — the person clicked or typed again while the first answer was still in the air, and the runtime folded every repeat into the same wait. The pile is the symptom; the hold's own verdict is the cause, so the repair is the acknowledgement plus a control that does not accept the repeat. `warn`, once per hold. `data`: the hold's data (`holdMs`, `blockers`, `heldWrites`, `interaction`, `navigation`) plus `interactions`, the count. Subject: the first held write's node. `false` disables. +#### `OPTIMISTIC_REVERTED` + +**Message:** "the optimistic value of `status` showed "saved"; it reverted to "idle". The person saw the guess, then the correction. A revert on failure is the feature; one that recurs says the guess is wrong for this input or the action fails often — show the failure where the value renders (the action's catch, an `Errored` boundary) rather than letting the value snap back on its own." + +An optimistic override (`createOptimistic`) the screen displayed was replaced by a different value. Two roads, named in `data.how`: `reverted` — nothing new landed and the override lifted back to the committed value it covered (the action failed, or never wrote what it promised); `superseded` — an authoritative value landed that differs from the guess (the server counted differently), and tracked readers re-derived to it. Both are correct by construction — the override reverting _is_ the feature — so the finding is `info`, structured channel only, always on while the engine is enabled: a count that grows for one source is what says the guess or the failure rate is wrong. Judged by the node's own `equals`, so a structurally equal correction is silent. `data`: `source`, `shown`, `truth` (previews, 40 characters), `how`. Subject: the node, so `ownerPath` locates it. Not yet covered: optimistic _stores_ (`createOptimisticStore`), whose overlay folds off per path; and the interaction that wrote the guess (optimistic writes are not stamped with an origin today). + ### Server rendering (`ssr`, `head`) The server runtime reports on the same channel. Two groups, two tiers. **Findings** are facts about a render whichever tier is running — an error a boundary contained, work the stream threw away, an error the server-function wire replaced. They ride `OBSERVE.diagnostics` in observe and dev builds (a production observability consumer subscribes to them; in dev they also print) and fold out of prod entirely. **Checks** are guidance for a developer at a console — a write on the server, an invalid preload descriptor — and exist only in the dev build, where they print like any client warning. Every entry carries `ownerPath` when it fired inside a component: on the server the component wrapper labels its owner `` exactly as the client's does, so `in › ` reads the same on both sides. Codes that already exist on the client (`ASYNC_OUTSIDE_LOADING_BOUNDARY`, `UNRECOGNIZED_INSERT_VALUE`) are shared, not duplicated; `data.side` or the message tells the platforms apart where it matters. @@ -901,6 +907,7 @@ The runtime derives a request's trace itself in every tier — the W3C `tracepar | `ABANDONED_FLIGHTS` | warn | responsiveness | One async source abandoned 3+ flights in 1s, each superseded before landing — the request-per-keystroke signature (attribution enabled) | | `FALLBACK_FLASH` | info | responsiveness | A `Loading` fallback showed for under 150ms — feedback for a wait too short to need it (attribution enabled) | | `STACKED_HOLDS` | warn | responsiveness | 3+ interactions were waiting in one hold when it committed — repeats piled behind the same source (attribution enabled) | +| `OPTIMISTIC_REVERTED` | info | responsiveness | An optimistic value the screen showed was replaced by a different one — reverted at settle, or superseded by the truth (attribution enabled) | | `SSR_RENDER_ERROR_CONTAINED` | error | ssr | Server render error routed by a boundary: `data.handling` fallback / client / failed (observe + dev) | | `SSR_SUBTREE_ABANDONED` | warn | ssr | A failed fragment's pending descendants were discarded (observe + dev) | | `SSR_STREAM_ABANDONED` | warn | ssr | Response stream cancelled or sink failed with fragments pending (observe + dev) | diff --git a/packages/signals/src/core/attribution-hooks.ts b/packages/signals/src/core/attribution-hooks.ts index c921a7d02..79b7164b6 100644 --- a/packages/signals/src/core/attribution-hooks.ts +++ b/packages/signals/src/core/attribution-hooks.ts @@ -199,6 +199,21 @@ export interface AttributionHooks { shown: boolean, transition?: Transition | null ): void; + /** + * An optimistic override the screen displayed is being replaced by a + * different value. `"superseded"`: a new authoritative value landed that + * differs from the guess (tracked readers re-derive to it). `"reverted"`: + * nothing new landed and the guess lifts back to the committed value it + * covered (the action failed, or never wrote what it promised). `shown` + * is the override as displayed. Fired when the two differ by identity; + * the engine applies the node's own equality before judging. + */ + optimisticReverted( + el: Signal | Computed, + shown: unknown, + truth: unknown, + how: "superseded" | "reverted" + ): void; /** * The one query on the surface: the provenance a root write performed at * this moment would be stamped with — the innermost open frame (an effect diff --git a/packages/signals/src/core/attribution.ts b/packages/signals/src/core/attribution.ts index 77770d381..57f16790b 100644 --- a/packages/signals/src/core/attribution.ts +++ b/packages/signals/src/core/attribution.ts @@ -2908,6 +2908,46 @@ function checkLongHold(event: HoldEvent, subject: Signal): void { if (severity === "warn") reportDiagnostic(entry); } +/** + * The person saw the guess, then the correction. An optimistic value is a + * promise the UI makes about the outcome; when the outcome differs — the + * action failed and the override lifted back to the old value, or the + * source answered with something else — the screen changes twice for one + * intent. Expected on failure and correct by construction (the override + * reverts; that is the feature), so `info`: a count that grows for one + * source is what says the guess, or the failure rate, is wrong. Judged by + * the node's own equality, so a structurally equal replacement is not a + * revert. + */ +function checkOptimisticRevert( + el: Signal | Computed, + shown: unknown, + truth: unknown, + how: "superseded" | "reverted" +): void { + const equals = (el as { _equals?: false | ((a: unknown, b: unknown) => boolean) })._equals; + if (equals && equals(shown, truth)) return; + const source = nodeName(el); + const message = + `[OPTIMISTIC_REVERTED] the optimistic value of ${source} showed ${preview(shown)}; it ` + + `${how === "superseded" ? "settled to" : "reverted to"} ${preview(truth)}. The person saw ` + + `the guess, then the correction. A revert on failure is the feature; one that recurs says ` + + `the guess is wrong for this input or the action fails often — show the failure where the ` + + `value renders (the action's catch, an Errored boundary) rather than letting the value ` + + `snap back on its own.`; + emitDiagnostic( + { + code: "OPTIMISTIC_REVERTED", + kind: "responsiveness", + severity: "info", + message, + nodeName: source, + data: { source, shown: preview(shown), truth: preview(truth), how } + }, + el + ); +} + // --- Graph growth ----------------------------------------------------------------- /** Per route: the graph's size at its last `visits` settles, oldest first. */ @@ -4129,6 +4169,9 @@ const engineHooks: AttributionHooks = { // The folds hear the show at its display (see trackFallback), not here. trackFallback(boundary, tree, shown, transition ?? null); }, + optimisticReverted(el, shown, truth, how) { + checkOptimisticRevert(el, shown, truth, how); + }, currentOrigin() { return ambientOrigin(); } diff --git a/packages/signals/src/core/dev.ts b/packages/signals/src/core/dev.ts index 43029a481..8f553fc1a 100644 --- a/packages/signals/src/core/dev.ts +++ b/packages/signals/src/core/dev.ts @@ -79,6 +79,7 @@ export type DiagnosticCode = | "ABANDONED_FLIGHTS" | "FALLBACK_FLASH" | "STACKED_HOLDS" + | "OPTIMISTIC_REVERTED" | "EFFECT_WRITES_OWN_SOURCE" | "EFFECT_RELAY_TEAR" | "IMMUTABLE_UPDATE_IN_STORE" diff --git a/packages/signals/src/core/optimistic.ts b/packages/signals/src/core/optimistic.ts index c6fab1964..53c681ae9 100644 --- a/packages/signals/src/core/optimistic.ts +++ b/packages/signals/src/core/optimistic.ts @@ -256,7 +256,13 @@ function resolveOptimisticNodes(nodes: OptimisticNode[]): void { node._value !== unwrapOverride(prevOverride) ) { if (derived) node._value = unwrapOverride(prevOverride); - else insertSubs(node, true); + else { + // The guess lifts and what was beneath it differs: the screen + // changes from the override to the committed value. + if (__OBSERVE__ && attrHooks !== null) + attrHooks.optimisticReverted(node, unwrapOverride(prevOverride), node._value, "reverted"); + insertSubs(node, true); + } } node._transition = null; if (node._x !== null) node._x._overrideOwner = null; @@ -323,6 +329,16 @@ function supersedeOverride(el: OptimisticNode, value: unknown): void { // 0 is mainline (no action): always the current question. if (origin && origin < el._x!._overrideStamp) return; el._config |= CONFIG_OVERRIDE_SUPERSEDED; + // A fresh landing is staged in `_pendingValue` (landOnOverride) or is the + // truth endOptimism read from it; the committed value with nothing staged + // is the value the guess covered — the screen goes back, not forward. + if (__OBSERVE__ && attrHooks !== null) + attrHooks.optimisticReverted( + el, + unwrapOverride(el._x!._overrideValue), + value, + el._pendingValue === NOT_PENDING && value === el._value ? "reverted" : "superseded" + ); const lane = el._x?._optimisticLane; if (lane) { const root = findLane(lane); diff --git a/packages/signals/tests/attribution-optimistic-revert.test.ts b/packages/signals/tests/attribution-optimistic-revert.test.ts new file mode 100644 index 000000000..8b5c9c307 --- /dev/null +++ b/packages/signals/tests/attribution-optimistic-revert.test.ts @@ -0,0 +1,169 @@ +/** + * OPTIMISTIC_REVERTED — the person saw the guess, then the correction. + * + * Claim under test: when an optimistic override the screen displayed is + * replaced by a different value — the source answered with something else + * (`superseded`), or the override lifted at settle back to a differing + * committed value (`dropped`) — the engine emits one `info` finding naming + * the source, both values, and the interaction that wrote the guess. A guess + * the truth confirms, by identity or by the node's own equality, is silent. + */ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { attribution } from "../src/attribution.js"; +import { + action, + createOptimistic, + createRenderEffect, + createRoot, + createSignal, + flush, + OBSERVE +} from "../src/index.js"; +import type { DiagnosticEvent } from "../src/core/dev.js"; + +afterEach(() => { + attribution.disable(); + flush(); + vi.restoreAllMocks(); +}); + +function deferred() { + let resolve!: (v: T) => void; + const promise = new Promise(r => (resolve = r)); + return { promise, resolve }; +} + +function arm() { + vi.spyOn(console, "warn").mockImplementation(() => {}); + vi.spyOn(console, "info").mockImplementation(() => {}); + attribution.enable({ log: false, hotRuns: false, hotTime: false, waterfalls: false }); + const findings: DiagnosticEvent[] = []; + OBSERVE!.diagnostics.subscribe(e => { + if (e.code === "OPTIMISTIC_REVERTED") findings.push(e); + }); + return { findings }; +} + +describe("OPTIMISTIC_REVERTED", () => { + it("reverted: the action ends without the guess coming true, and the value snaps back", async () => { + const { findings } = arm(); + const gate = deferred(); + const [status, setStatus] = createOptimistic("idle", { name: "status" }); + createRoot(() => createRenderEffect(status, () => {}, { name: "badge" })); + flush(); + const save = action(function* save() { + setStatus("saved"); + yield gate.promise; + // The server said no; nothing writes the truth. The override lifts. + }); + const p = save(); + flush(); + expect(status()).toBe("saved"); + expect(findings).toHaveLength(0); + gate.resolve(); + await p; + flush(); + expect(status()).toBe("idle"); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + code: "OPTIMISTIC_REVERTED", + kind: "responsiveness", + severity: "info", + nodeName: "status", + data: { source: "status", shown: '"saved"', truth: '"idle"', how: "reverted" } + }); + expect(findings[0].message).toContain('showed "saved"; it reverted to "idle"'); + }); + + it("superseded: the source answers with a different value while the guess is showing", async () => { + const { findings } = arm(); + const gate = deferred(); + const [count, setCount] = createSignal(0, { name: "count" }); + const [shown, setShown] = createOptimistic(() => count(), { name: "shownCount" }); + createRoot(() => createRenderEffect(shown, () => {}, { name: "counter" })); + flush(); + const increment = action(function* increment() { + setShown(1); + yield gate.promise; + setCount(2); // the server counted someone else's click too + }); + const p = increment(); + flush(); + expect(shown()).toBe(1); + gate.resolve(); + await p; + flush(); + expect(shown()).toBe(2); + expect(findings).toHaveLength(1); + expect(findings[0].data).toMatchObject({ + source: "shownCount", + shown: "1", + truth: "2", + how: "superseded" + }); + expect(findings[0].message).toContain("settled to 2"); + }); + + it("a guess the truth confirms is silent", async () => { + const { findings } = arm(); + const gate = deferred(); + const [count, setCount] = createSignal(0, { name: "count" }); + const [shown, setShown] = createOptimistic(() => count(), { name: "shownCount" }); + createRoot(() => createRenderEffect(shown, () => {}, { name: "counter" })); + flush(); + const increment = action(function* increment() { + setShown(1); + yield gate.promise; + setCount(1); + }); + const p = increment(); + flush(); + gate.resolve(); + await p; + flush(); + expect(shown()).toBe(1); + expect(findings).toHaveLength(0); + }); + + it("the node's own equality decides: a structurally equal correction is not a revert", async () => { + const { findings } = arm(); + const gate = deferred(); + const [user, setUser] = createSignal({ name: "Ada" }, { name: "user" }); + const [shownUser, setShownUser] = createOptimistic(() => user(), { + name: "shownUser", + equals: (a, b) => a.name === b.name + }); + createRoot(() => createRenderEffect(shownUser, () => {}, { name: "profile" })); + flush(); + const rename = action(function* rename() { + setShownUser({ name: "Grace" }); + yield gate.promise; + setUser({ name: "Grace" }); // a fresh object, the same name + }); + const p = rename(); + flush(); + gate.resolve(); + await p; + flush(); + expect(shownUser().name).toBe("Grace"); + expect(findings).toHaveLength(0); + }); + + it("with no engine, the sites are inert", async () => { + const gate = deferred(); + const [status, setStatus] = createOptimistic("idle", { name: "status" }); + createRoot(() => createRenderEffect(status, () => {}, { name: "badge" })); + flush(); + const save = action(function* save() { + setStatus("saved"); + yield gate.promise; + }); + const p = save(); + flush(); + gate.resolve(); + await p; + flush(); + expect(status()).toBe("idle"); + expect(attribution.history()).toEqual([]); + }); +}); diff --git a/packages/solid/skills/reactivity-diagnostics/SKILL.md b/packages/solid/skills/reactivity-diagnostics/SKILL.md index 475cb77bf..a93ad9e36 100644 --- a/packages/solid/skills/reactivity-diagnostics/SKILL.md +++ b/packages/solid/skills/reactivity-diagnostics/SKILL.md @@ -558,6 +558,31 @@ Do NOT block input by removing the hold or making the write synchronous; the repeats are a reaction to silence, and the hold is what keeps the screen consistent while the answer arrives. +### OPTIMISTIC_REVERTED + +An optimistic value the screen showed was replaced by a different one: the +person saw the guess, then the correction. `data.how` says which road: +`reverted` — the action ended without the guess coming true and the value +snapped back to what it covered; `superseded` — the real value landed and +differed from the guess (the server counted differently, rejected part of +the input). Both are the runtime doing exactly what optimistic UI promises, +which is why this is `info` and never reaches the console; it is a count to +read, not a warning to silence. Read it two ways: + +- One source, many reverts: the failure is common and the UI is hiding it. + Show the failure where the value renders — catch in the action and write + an error the UI reads, or put the reader behind an `Errored` boundary — so + the snap-back is explained rather than silent. +- One source, many supersessions: the guess is systematically wrong for this + input (it ignores what the server adds or normalizes). Guess less: write + only the part of the value the client can know, or none, and read + `isPending()` for the rest. + +Do NOT "fix" a revert by writing the guess to the authoritative signal so it +cannot revert — that is not optimistic UI, it is lying to the user about what +happened. Optimistic stores are not covered yet; a store-path revert shows +up only as the reader's re-run. + ### Where to start: `feedback()` Before chasing individual `SILENT_HOLD` events, read the ranked tables — the diff --git a/scripts/size/.size-limit.js b/scripts/size/.size-limit.js index 6666fb8e8..7e99c72ff 100644 --- a/scripts/size/.size-limit.js +++ b/scripts/size/.size-limit.js @@ -2206,6 +2206,11 @@ module.exports = [ // 30,649 (+719 B; the PR's own +769 was against the pre-#3604 28,999 — // the repair texts share more back-references with four more findings // in the bundle). The tier did not move: 17,618 either way. + // OPTIMISTIC_REVERTED (2026-09-23, rebased): 29.05 -> 29.35 KB, measured at + // 29,262 B on a full build (+263 B over next's 28,999). The `optimisticReverted` + // hook, the node-equality gate, and the finding with its two-road text + // (reverted / superseded). The tier moved 38 B for the two hook sites in + // optimistic.ts; under its cap. limit: "31.45 KB", modifyEsbuildConfig: observeEsbuildConfig }, From 0b69a7f8f53e86dd138ff4f3797dfc2d8b4670b6 Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Wed, 23 Sep 2026 23:13:16 -0700 Subject: [PATCH 2/4] chore(size): re-measure OPTIMISTIC_REVERTED at landing; engine cap 31.45 -> 31.65 KB Co-authored-by: Claude via Cursor --- scripts/size/.size-limit.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/size/.size-limit.js b/scripts/size/.size-limit.js index 7e99c72ff..1dbfe461d 100644 --- a/scripts/size/.size-limit.js +++ b/scripts/size/.size-limit.js @@ -2211,7 +2211,13 @@ module.exports = [ // hook, the node-equality gate, and the finding with its two-road text // (reverted / superseded). The tier moved 38 B for the two hook sites in // optimistic.ts; under its cap. - limit: "31.45 KB", + // Re-measured at landing (#3607 rebased over #3604/#3623/#3613/#3619/#3608, + // 2026-09-23): 31.45 -> 31.65 KB, measured at 31,590 B against `next`'s + // 31,368 (+222 B; the PR's own +263 was against the pre-#3604 28,999). + // The tier did not move on this base: 17,618 either way — the 38 B the + // note above saw was nameCache drift, the hook sites fold to the same + // minified shape. + limit: "31.65 KB", modifyEsbuildConfig: observeEsbuildConfig }, { From 7095de61cfeda5355448a3241c123c3f11e1e40a Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Thu, 24 Sep 2026 00:21:12 -0700 Subject: [PATCH 3/4] fix(signals): OPTIMISTIC_REVERTED never judges the runtime's own optimistic nodes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The finding fired on every acknowledged hold: isPending()'s companion is an optimistic signal that goes true while pending and back to false at commit by design — the acknowledgement SILENT_HOLD asks for — so the repair for one finding produced another. The check now skips companions and derived overrides by the predicate the hold census already uses (`isCompanion`), and `optimisticReverts: false` disables it like the other verdicts. The regression test is the acknowledged hold itself (isPending and latest readers over a held write): silent with the guard, one false finding without it. Co-Authored-By: Claude via Cursor Co-authored-by: Cursor --- .changeset/optimistic-reverted.md | 2 +- documentation/solid-2.0/08-dev-diagnostics.md | 126 +++++++++--------- packages/signals/src/core/attribution.ts | 16 +++ .../attribution-optimistic-revert.test.ts | 70 +++++++++- .../skills/reactivity-diagnostics/SKILL.md | 4 +- 5 files changed, 151 insertions(+), 67 deletions(-) diff --git a/.changeset/optimistic-reverted.md b/.changeset/optimistic-reverted.md index 3242d1e41..2cec07b2e 100644 --- a/.changeset/optimistic-reverted.md +++ b/.changeset/optimistic-reverted.md @@ -2,4 +2,4 @@ "@solidjs/signals": patch --- -`OPTIMISTIC_REVERTED` (info, responsiveness): an optimistic value the screen showed was replaced by a different one — reverted at settle, or superseded by the truth — with the source and both values. `AttributionHooks.optimisticReverted` is the seam. +`OPTIMISTIC_REVERTED` (info, responsiveness): an optimistic value the screen showed was replaced by a different one — reverted at settle, or superseded by the truth — with the source and both values. `AttributionHooks.optimisticReverted` is the seam; `optimisticReverts: false` disables it. The runtime's own optimistic nodes (`isPending`/`latest` companions, derived overrides) are never judged. diff --git a/documentation/solid-2.0/08-dev-diagnostics.md b/documentation/solid-2.0/08-dev-diagnostics.md index c9a634130..be0a4dfad 100644 --- a/documentation/solid-2.0/08-dev-diagnostics.md +++ b/documentation/solid-2.0/08-dev-diagnostics.md @@ -570,7 +570,7 @@ When a hold commits, `stackedHolds.count` or more interactions (default 3) were **Message:** "the optimistic value of `status` showed "saved"; it reverted to "idle". The person saw the guess, then the correction. A revert on failure is the feature; one that recurs says the guess is wrong for this input or the action fails often — show the failure where the value renders (the action's catch, an `Errored` boundary) rather than letting the value snap back on its own." -An optimistic override (`createOptimistic`) the screen displayed was replaced by a different value. Two roads, named in `data.how`: `reverted` — nothing new landed and the override lifted back to the committed value it covered (the action failed, or never wrote what it promised); `superseded` — an authoritative value landed that differs from the guess (the server counted differently), and tracked readers re-derived to it. Both are correct by construction — the override reverting _is_ the feature — so the finding is `info`, structured channel only, always on while the engine is enabled: a count that grows for one source is what says the guess or the failure rate is wrong. Judged by the node's own `equals`, so a structurally equal correction is silent. `data`: `source`, `shown`, `truth` (previews, 40 characters), `how`. Subject: the node, so `ownerPath` locates it. Not yet covered: optimistic _stores_ (`createOptimisticStore`), whose overlay folds off per path; and the interaction that wrote the guess (optimistic writes are not stamped with an origin today). +An optimistic override (`createOptimistic`) the screen displayed was replaced by a different value. Two roads, named in `data.how`: `reverted` — nothing new landed and the override lifted back to the committed value it covered (the action failed, or never wrote what it promised); `superseded` — an authoritative value landed that differs from the guess (the server counted differently), and tracked readers re-derived to it. Both are correct by construction — the override reverting _is_ the feature — so the finding is `info`, structured channel only, always on while the engine is enabled: a count that grows for one source is what says the guess or the failure rate is wrong. Judged by the node's own `equals`, so a structurally equal correction is silent. The runtime's own optimistic nodes are never judged: an `isPending()`/`latest()` companion is an optimistic signal that goes `true` while pending and back to `false` at commit by design — the acknowledgement `SILENT_HOLD` asks for — and a derived override promotes rather than reverts; the check skips them by the same predicate the hold census uses. `optimisticReverts: false` disables. `data`: `source`, `shown`, `truth` (previews, 40 characters), `how`. Subject: the node, so `ownerPath` locates it. Not yet covered: optimistic _stores_ (`createOptimisticStore`), whose overlay folds off per path; and the interaction that wrote the guess (optimistic writes are not stamped with an origin today). ### Server rendering (`ssr`, `head`) @@ -860,69 +860,69 @@ The runtime derives a request's trace itself in every tier — the W3C `tracepar ## Diagnostic codes (quick reference) -| Code | Severity | Category | Trigger | -| ---------------------------------- | --------- | -------------- | -------------------------------------------------------------------------------------------------------------------------- | -| `REACTIVE_WRITE_IN_OWNED_SCOPE` | error | write | Reactive write/invalidation inside component/computation | -| `ASYNC_STORE_SETTER` | error | write | Store setter callback returned a Promise (setters are synchronous transactions) | -| `PENDING_ASYNC_UNTRACKED_READ` | error | async | Reading pending async outside tracking scope | -| `ASYNC_OUTSIDE_LOADING_BOUNDARY` | warn | async | Async computation outside Loading boundary (non-halting; root mount is deferred) | -| `CLEANUP_IN_FORBIDDEN_SCOPE` | error | lifecycle | `onCleanup` inside trackedEffect/onSettled | -| `SETTLED_CLEANUP_UNOWNED` | error | lifecycle | `onSettled` returned a cleanup in an unowned (out-of-band) scope | -| `PRIMITIVE_IN_FORBIDDEN_SCOPE` | error | lifecycle | Reactive primitive created inside trackedEffect/onSettled | -| `ACTION_CALLED_IN_OWNED_SCOPE` | error | write | `action()` invoked from a component body or computation | -| `MISSING_EFFECT_FN` | error | lifecycle | `createEffect` called without the effect function | -| `SYNC_NODE_RECEIVED_ASYNC` | error | lifecycle | `sync: true` computation returned a Promise / AsyncIterable | -| `INVALID_REFRESH_TARGET` | error | write | `refresh()` target is not a source accessor or refreshable store | -| `INVALID_AFFECTS_TARGET` | error | write | `affects()` given a key path, or a key on an accessor | -| `REACTIVITY_HALTED` | error | error | Uncaught error escaped every boundary; scheduling stopped (reported, cause to `reportError`) | -| `INVARIANT_VIOLATION` | error | error | Internal consistency check failed (throws under `__TEST__`, reported in dev) | -| `SETTLE_WALK_UNINITIALIZED_SOURCE` | error | lifecycle | Internal: settle walk reached a source that never produced a value (reported) | -| `STRICT_READ_UNTRACKED` | warn | strict-read | Untracked reactive read in component/effect body | -| `UNTRACKED_READ_AFTER_AWAIT` | warn | async | Async computation first read a signal/memo/store key after an `await`; never a dependency (dev; V8 engines only) | -| `PENDING_ASYNC_FORBIDDEN_SCOPE` | warn | async | Pending async read in trackedEffect/onSettled | -| `LOADING_ON_OUTSIDE_HOLD` | warn | async | `` changed but its data is also read outside the boundary and holds the frame: the fallback can never be seen | -| `NO_OWNER_EFFECT` | warn | lifecycle | Effect created without reactive owner | -| `NO_OWNER_CLEANUP` | warn | lifecycle | `onCleanup` called without owner | -| `NO_OWNER_BOUNDARY` | warn | lifecycle | Boundary created without owner | -| `RUN_WITH_DISPOSED_OWNER` | warn | owner | `runWithOwner` with disposed owner | -| `FLUSH_IN_EFFECT_CALLBACK` | warn | lifecycle | `flush()` from an effect callback (no-op; the drain is already running) | -| `HUGE_FAN_OUT` | warn | graph | One change reached 2000 live subscribers (always on) | -| `HUGE_FAN_IN` | warn | graph | One recompute tracked 2000 sources (always on) | -| `GRAPH_GROWTH` | warn | perf | The live owner count at the same route's settle climbed on 3 consecutive visits to 1.25× — something each visit leaves behind (attribution enabled) | -| `HOT_SCOPE_RERUNS` | warn | perf | 120+ re-runs of one scope in 1s (attribution enabled) | -| `HOT_SCOPE_FANOUT` | warn | perf | 5+/50+/500+ scopes hot from one root cause (attribution enabled) | -| `HOT_SCOPE_TIME` | warn | perf | 8ms+ self-time in one scope in 1s (attribution enabled) | -| `WIDE_SCOPE_DEPS` | warn | perf | Scope subscribed to 30+ sources (attribution enabled) | -| `WIDE_WRITE` | warn | perf | Committed write reached 250+ subscribers (attribution enabled) | -| `ASYNC_WATERFALL` | info/warn | perf | 2+/3+ sequential async flights: origin-proven (attribution enabled), or a `` boundary's passes (server, dev) | -| `UNSTABLE_MEMO_OUTPUT` | warn | perf | Memo returned a new-but-equivalent container 4+ runs running (attribution enabled) | -| `WASTED_RECOMPUTE` | warn | perf | 80%+ of a scope's 5+ runs in a second produced an unchanged value for 2ms+ of compute — inputs change, result does not (attribution enabled) | -| `EFFECT_WRITES_OWN_SOURCE` | info/warn | perf | Effect's write provably feeds back into its own inputs; `info` for multi-effect rings (attribution enabled) | -| `EFFECT_RELAY_TEAR` | info/warn | perf | Reader ran twice for one root change because an effect relayed it; `warn` when derivable or repeated (attribution enabled) | -| `IMMUTABLE_UPDATE_IN_STORE` | warn | perf | Store container replaced by a mostly-identical copy (attribution enabled) | -| `UNSTABLE_LIST_IDENTITY` | warn | perf | `mapArray`/`For` recreated rows for equivalent items (attribution enabled) | -| `SILENT_HOLD` | info/warn | responsiveness | Write held 100ms+/200ms+ by pending async with no on-screen acknowledgement (attribution enabled) | -| `LONG_HOLD` | info/warn | responsiveness | Acknowledged hold whose tail (last input → commit) ran 500ms+/1000ms+ (attribution enabled) | +| Code | Severity | Category | Trigger | +| ---------------------------------- | --------- | -------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `REACTIVE_WRITE_IN_OWNED_SCOPE` | error | write | Reactive write/invalidation inside component/computation | +| `ASYNC_STORE_SETTER` | error | write | Store setter callback returned a Promise (setters are synchronous transactions) | +| `PENDING_ASYNC_UNTRACKED_READ` | error | async | Reading pending async outside tracking scope | +| `ASYNC_OUTSIDE_LOADING_BOUNDARY` | warn | async | Async computation outside Loading boundary (non-halting; root mount is deferred) | +| `CLEANUP_IN_FORBIDDEN_SCOPE` | error | lifecycle | `onCleanup` inside trackedEffect/onSettled | +| `SETTLED_CLEANUP_UNOWNED` | error | lifecycle | `onSettled` returned a cleanup in an unowned (out-of-band) scope | +| `PRIMITIVE_IN_FORBIDDEN_SCOPE` | error | lifecycle | Reactive primitive created inside trackedEffect/onSettled | +| `ACTION_CALLED_IN_OWNED_SCOPE` | error | write | `action()` invoked from a component body or computation | +| `MISSING_EFFECT_FN` | error | lifecycle | `createEffect` called without the effect function | +| `SYNC_NODE_RECEIVED_ASYNC` | error | lifecycle | `sync: true` computation returned a Promise / AsyncIterable | +| `INVALID_REFRESH_TARGET` | error | write | `refresh()` target is not a source accessor or refreshable store | +| `INVALID_AFFECTS_TARGET` | error | write | `affects()` given a key path, or a key on an accessor | +| `REACTIVITY_HALTED` | error | error | Uncaught error escaped every boundary; scheduling stopped (reported, cause to `reportError`) | +| `INVARIANT_VIOLATION` | error | error | Internal consistency check failed (throws under `__TEST__`, reported in dev) | +| `SETTLE_WALK_UNINITIALIZED_SOURCE` | error | lifecycle | Internal: settle walk reached a source that never produced a value (reported) | +| `STRICT_READ_UNTRACKED` | warn | strict-read | Untracked reactive read in component/effect body | +| `UNTRACKED_READ_AFTER_AWAIT` | warn | async | Async computation first read a signal/memo/store key after an `await`; never a dependency (dev; V8 engines only) | +| `PENDING_ASYNC_FORBIDDEN_SCOPE` | warn | async | Pending async read in trackedEffect/onSettled | +| `LOADING_ON_OUTSIDE_HOLD` | warn | async | `` changed but its data is also read outside the boundary and holds the frame: the fallback can never be seen | +| `NO_OWNER_EFFECT` | warn | lifecycle | Effect created without reactive owner | +| `NO_OWNER_CLEANUP` | warn | lifecycle | `onCleanup` called without owner | +| `NO_OWNER_BOUNDARY` | warn | lifecycle | Boundary created without owner | +| `RUN_WITH_DISPOSED_OWNER` | warn | owner | `runWithOwner` with disposed owner | +| `FLUSH_IN_EFFECT_CALLBACK` | warn | lifecycle | `flush()` from an effect callback (no-op; the drain is already running) | +| `HUGE_FAN_OUT` | warn | graph | One change reached 2000 live subscribers (always on) | +| `HUGE_FAN_IN` | warn | graph | One recompute tracked 2000 sources (always on) | +| `GRAPH_GROWTH` | warn | perf | The live owner count at the same route's settle climbed on 3 consecutive visits to 1.25× — something each visit leaves behind (attribution enabled) | +| `HOT_SCOPE_RERUNS` | warn | perf | 120+ re-runs of one scope in 1s (attribution enabled) | +| `HOT_SCOPE_FANOUT` | warn | perf | 5+/50+/500+ scopes hot from one root cause (attribution enabled) | +| `HOT_SCOPE_TIME` | warn | perf | 8ms+ self-time in one scope in 1s (attribution enabled) | +| `WIDE_SCOPE_DEPS` | warn | perf | Scope subscribed to 30+ sources (attribution enabled) | +| `WIDE_WRITE` | warn | perf | Committed write reached 250+ subscribers (attribution enabled) | +| `ASYNC_WATERFALL` | info/warn | perf | 2+/3+ sequential async flights: origin-proven (attribution enabled), or a `` boundary's passes (server, dev) | +| `UNSTABLE_MEMO_OUTPUT` | warn | perf | Memo returned a new-but-equivalent container 4+ runs running (attribution enabled) | +| `WASTED_RECOMPUTE` | warn | perf | 80%+ of a scope's 5+ runs in a second produced an unchanged value for 2ms+ of compute — inputs change, result does not (attribution enabled) | +| `EFFECT_WRITES_OWN_SOURCE` | info/warn | perf | Effect's write provably feeds back into its own inputs; `info` for multi-effect rings (attribution enabled) | +| `EFFECT_RELAY_TEAR` | info/warn | perf | Reader ran twice for one root change because an effect relayed it; `warn` when derivable or repeated (attribution enabled) | +| `IMMUTABLE_UPDATE_IN_STORE` | warn | perf | Store container replaced by a mostly-identical copy (attribution enabled) | +| `UNSTABLE_LIST_IDENTITY` | warn | perf | `mapArray`/`For` recreated rows for equivalent items (attribution enabled) | +| `SILENT_HOLD` | info/warn | responsiveness | Write held 100ms+/200ms+ by pending async with no on-screen acknowledgement (attribution enabled) | +| `LONG_HOLD` | info/warn | responsiveness | Acknowledged hold whose tail (last input → commit) ran 500ms+/1000ms+ (attribution enabled) | | `UNTRACKED_ASYNC_HANDLER` | info/warn | responsiveness | Handler awaited 100ms+/200ms+ past its frame with no write before the `await` and no `action()`: a dead click no hold could judge (attribution enabled) | -| `ABANDONED_FLIGHTS` | warn | responsiveness | One async source abandoned 3+ flights in 1s, each superseded before landing — the request-per-keystroke signature (attribution enabled) | -| `FALLBACK_FLASH` | info | responsiveness | A `Loading` fallback showed for under 150ms — feedback for a wait too short to need it (attribution enabled) | -| `STACKED_HOLDS` | warn | responsiveness | 3+ interactions were waiting in one hold when it committed — repeats piled behind the same source (attribution enabled) | -| `OPTIMISTIC_REVERTED` | info | responsiveness | An optimistic value the screen showed was replaced by a different one — reverted at settle, or superseded by the truth (attribution enabled) | -| `SSR_RENDER_ERROR_CONTAINED` | error | ssr | Server render error routed by a boundary: `data.handling` fallback / client / failed (observe + dev) | -| `SSR_SUBTREE_ABANDONED` | warn | ssr | A failed fragment's pending descendants were discarded (observe + dev) | -| `SSR_STREAM_ABANDONED` | warn | ssr | Response stream cancelled or sink failed with fragments pending (observe + dev) | -| `LATE_HEADER_WRITE` | error | head | Response header written after the head was sent; dropped (observe + dev; dev throws) | -| `SERVER_FN_ERROR_SANITIZED` | error | ssr | Server-function error replaced with the generic Error on the wire; `data.error` is the original (observe + dev) | -| `FRAME_MARKER_CORRUPTED` | error | ssr | Frame slot range missing its end marker on the client — nesting or an HTML rewriter (observe + dev) | -| `SERVER_WRITE` | warn | write | Signal/store/optimistic setter ran during a server render; inert, will throw (dev; once per category) | -| `REVEAL_IN_RENDER_TO_STRING` | warn | ssr | Nested `` with collapsed/together under `renderToString` (dev) | -| `SSR_CLIENT_CONTENT_MASKED` | warn | ssr | Client-only content in a `` surfaced only after a server wait; the server's work was discarded (dev) | -| `LAZY_ASSET_UNMAPPED` | warn | ssr | `lazy()` component's client assets could not be resolved for the page (dev) | -| `PRELOAD_DESCRIPTOR_INVALID` | warn | head | `registerAsset("preload")` descriptor broke a field rule; link dropped or field ignored (dev) | -| `HEAD_TAG_INVALID` | warn | head | `useHead` registration the render could not honor; `data.reason` names the rule (dev) | -| `UNRECOGNIZED_INSERT_VALUE` | warn | render | Value at an insert position the renderer cannot render; skipped (dev; server and client) | -| `UNSCOPED_HOLE_ALLOCATED_IDS` | warn | render | Unscoped hole was handed a function whose content took ids at a position the other side does not share; keys permute (dev) | -| `BEHAVIOR_CLAIM_DROPPED` | warn | ssr | Behavior position on a server-rendered element got a spread prop or a server-local function (dev) | +| `ABANDONED_FLIGHTS` | warn | responsiveness | One async source abandoned 3+ flights in 1s, each superseded before landing — the request-per-keystroke signature (attribution enabled) | +| `FALLBACK_FLASH` | info | responsiveness | A `Loading` fallback showed for under 150ms — feedback for a wait too short to need it (attribution enabled) | +| `STACKED_HOLDS` | warn | responsiveness | 3+ interactions were waiting in one hold when it committed — repeats piled behind the same source (attribution enabled) | +| `OPTIMISTIC_REVERTED` | info | responsiveness | An optimistic value the screen showed was replaced by a different one — reverted at settle, or superseded by the truth (attribution enabled) | +| `SSR_RENDER_ERROR_CONTAINED` | error | ssr | Server render error routed by a boundary: `data.handling` fallback / client / failed (observe + dev) | +| `SSR_SUBTREE_ABANDONED` | warn | ssr | A failed fragment's pending descendants were discarded (observe + dev) | +| `SSR_STREAM_ABANDONED` | warn | ssr | Response stream cancelled or sink failed with fragments pending (observe + dev) | +| `LATE_HEADER_WRITE` | error | head | Response header written after the head was sent; dropped (observe + dev; dev throws) | +| `SERVER_FN_ERROR_SANITIZED` | error | ssr | Server-function error replaced with the generic Error on the wire; `data.error` is the original (observe + dev) | +| `FRAME_MARKER_CORRUPTED` | error | ssr | Frame slot range missing its end marker on the client — nesting or an HTML rewriter (observe + dev) | +| `SERVER_WRITE` | warn | write | Signal/store/optimistic setter ran during a server render; inert, will throw (dev; once per category) | +| `REVEAL_IN_RENDER_TO_STRING` | warn | ssr | Nested `` with collapsed/together under `renderToString` (dev) | +| `SSR_CLIENT_CONTENT_MASKED` | warn | ssr | Client-only content in a `` surfaced only after a server wait; the server's work was discarded (dev) | +| `LAZY_ASSET_UNMAPPED` | warn | ssr | `lazy()` component's client assets could not be resolved for the page (dev) | +| `PRELOAD_DESCRIPTOR_INVALID` | warn | head | `registerAsset("preload")` descriptor broke a field rule; link dropped or field ignored (dev) | +| `HEAD_TAG_INVALID` | warn | head | `useHead` registration the render could not honor; `data.reason` names the rule (dev) | +| `UNRECOGNIZED_INSERT_VALUE` | warn | render | Value at an insert position the renderer cannot render; skipped (dev; server and client) | +| `UNSCOPED_HOLE_ALLOCATED_IDS` | warn | render | Unscoped hole was handed a function whose content took ids at a position the other side does not share; keys permute (dev) | +| `BEHAVIOR_CLAIM_DROPPED` | warn | ssr | Behavior position on a server-rendered element got a spread prop or a server-local function (dev) | ## Run attribution — "why did this run" diff --git a/packages/signals/src/core/attribution.ts b/packages/signals/src/core/attribution.ts index 57f16790b..5338f29e9 100644 --- a/packages/signals/src/core/attribution.ts +++ b/packages/signals/src/core/attribution.ts @@ -496,6 +496,15 @@ export interface AttributionOptions { * and every one of them waited on the same source. `false` disables. */ stackedHolds?: { count: number } | false; + /** + * Optimistic-revert finding: emit OPTIMISTIC_REVERTED (`info`) when an + * optimistic value the screen showed is replaced by a different one — + * reverted at settle, or superseded by the truth (default true). The + * runtime's own optimistic nodes — `isPending`/`latest` companions and + * derived overrides — are never judged: they are the acknowledgement + * machinery, not a guess the person saw. `false` disables. + */ + optimisticReverts?: boolean; } /** A fallback shown for less than this is a flash: feedback for a wait too short to need it. */ @@ -572,6 +581,7 @@ const defaultOptions = { graphGrowth: { visits: 3, ratio: 1.25 } as { visits: number; ratio: number } | false, abandonedFlights: { count: 3, windowMs: 1000 } as { count: number; windowMs: number } | false, fallbackFlashes: true, + optimisticReverts: true, stackedHolds: { count: 3 } as { count: number } | false }; let options: typeof defaultOptions = { ...defaultOptions }; @@ -2925,6 +2935,12 @@ function checkOptimisticRevert( truth: unknown, how: "superseded" | "reverted" ): void { + // The runtime's own optimistic nodes are not guesses the person saw: an + // `isPending()` companion goes true while pending and back to false at + // commit by design — the acknowledgement SILENT_HOLD asks for — and a + // derived override promotes rather than reverts. Same predicate the hold + // census uses to skip them. + if (!options.optimisticReverts || isCompanion(el)) return; const equals = (el as { _equals?: false | ((a: unknown, b: unknown) => boolean) })._equals; if (equals && equals(shown, truth)) return; const source = nodeName(el); diff --git a/packages/signals/tests/attribution-optimistic-revert.test.ts b/packages/signals/tests/attribution-optimistic-revert.test.ts index 8b5c9c307..13a5318a4 100644 --- a/packages/signals/tests/attribution-optimistic-revert.test.ts +++ b/packages/signals/tests/attribution-optimistic-revert.test.ts @@ -12,11 +12,14 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { attribution } from "../src/attribution.js"; import { action, + createMemo, createOptimistic, createRenderEffect, createRoot, createSignal, flush, + isPending, + latest, OBSERVE } from "../src/index.js"; import type { DiagnosticEvent } from "../src/core/dev.js"; @@ -33,10 +36,10 @@ function deferred() { return { promise, resolve }; } -function arm() { +function arm(opts: { optimisticReverts?: boolean } = {}) { vi.spyOn(console, "warn").mockImplementation(() => {}); vi.spyOn(console, "info").mockImplementation(() => {}); - attribution.enable({ log: false, hotRuns: false, hotTime: false, waterfalls: false }); + attribution.enable({ log: false, hotRuns: false, hotTime: false, waterfalls: false, ...opts }); const findings: DiagnosticEvent[] = []; OBSERVE!.diagnostics.subscribe(e => { if (e.code === "OPTIMISTIC_REVERTED") findings.push(e); @@ -149,6 +152,69 @@ describe("OPTIMISTIC_REVERTED", () => { expect(findings).toHaveLength(0); }); + it("an acknowledged hold — isPending and latest doing their job — is not a revert", async () => { + // The regression: isPending()'s companion is an optimistic signal that + // goes true while pending and back to false at commit, by design. It is + // the acknowledgement SILENT_HOLD asks for, not a guess the person saw. + const { findings } = arm(); + const [page, setPage] = createSignal(1, { name: "page" }); + let resolve!: (v: string) => void; + const posts = createMemo( + () => { + const p = page(); + return new Promise(r => (resolve = v => r(`${v}-p${p}`))); + }, + { name: "posts" } + ); + const shown: string[] = []; + createRoot(() => { + createRenderEffect(posts, v => void shown.push(String(v)), { name: "feed" }); + createRenderEffect( + () => isPending(() => posts()), + () => {}, + { name: "spinner" } + ); + createRenderEffect( + () => latest(page), + () => {}, + { name: "pageLabel" } + ); + }); + flush(); + resolve("a"); + await new Promise(r => setTimeout(r, 10)); + flush(); + expect(shown).toContain("a-p1"); + // A held write: the companions flip on, then off at commit. + setPage(2); + flush(); + await new Promise(r => setTimeout(r, 10)); + resolve("b"); + await new Promise(r => setTimeout(r, 10)); + flush(); + expect(shown).toContain("b-p2"); + expect(findings).toHaveLength(0); + }); + + it("`optimisticReverts: false` disables the finding", async () => { + const { findings } = arm({ optimisticReverts: false }); + const gate = deferred(); + const [status, setStatus] = createOptimistic("idle", { name: "status" }); + createRoot(() => createRenderEffect(status, () => {}, { name: "badge" })); + flush(); + const save = action(function* save() { + setStatus("saved"); + yield gate.promise; + }); + const p = save(); + flush(); + gate.resolve(); + await p; + flush(); + expect(status()).toBe("idle"); + expect(findings).toHaveLength(0); + }); + it("with no engine, the sites are inert", async () => { const gate = deferred(); const [status, setStatus] = createOptimistic("idle", { name: "status" }); diff --git a/packages/solid/skills/reactivity-diagnostics/SKILL.md b/packages/solid/skills/reactivity-diagnostics/SKILL.md index a93ad9e36..a3f5d81d4 100644 --- a/packages/solid/skills/reactivity-diagnostics/SKILL.md +++ b/packages/solid/skills/reactivity-diagnostics/SKILL.md @@ -580,7 +580,9 @@ read, not a warning to silence. Read it two ways: Do NOT "fix" a revert by writing the guess to the authoritative signal so it cannot revert — that is not optimistic UI, it is lying to the user about what -happened. Optimistic stores are not covered yet; a store-path revert shows +happened. The runtime's own optimistic nodes — the `isPending()`/`latest()` +companions that acknowledge a hold — are never reported; only values you +wrote optimistically are. Optimistic stores are not covered yet; a store-path revert shows up only as the reader's re-run. ### Where to start: `feedback()` From db043d4a8a70e3221f0b946b271c48b8c942bdcd Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Thu, 24 Sep 2026 00:39:19 -0700 Subject: [PATCH 4/4] chore(size): re-measure the OPTIMISTIC_REVERTED companion guard; engine 31,625 B under the 31.65 KB cap The `optimisticReverts` option and the `isCompanion` skip in checkOptimisticRevert are +35 B on the engine scenario (31,590 -> 31,625 B). The cap holds; the note records the new measurement. Tier unchanged at 17,618. Co-authored-by: Claude via Cursor --- scripts/size/.size-limit.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/size/.size-limit.js b/scripts/size/.size-limit.js index 1dbfe461d..ec56013ba 100644 --- a/scripts/size/.size-limit.js +++ b/scripts/size/.size-limit.js @@ -2216,7 +2216,11 @@ module.exports = [ // 31,368 (+222 B; the PR's own +263 was against the pre-#3604 28,999). // The tier did not move on this base: 17,618 either way — the 38 B the // note above saw was nameCache drift, the hook sites fold to the same - // minified shape. + // minified shape. The companion guard (2026-09-24: `optimisticReverts` + // option + the `isCompanion` skip in checkOptimisticRevert, so isPending/ + // latest companions and derived overrides are never judged) is +35 B: + // 31,625 B, under the same cap (25 B of headroom; the next finding moves + // it). Tier unchanged at 17,618. limit: "31.65 KB", modifyEsbuildConfig: observeEsbuildConfig },