Skip to content

fix(signals): draft reads compose overrides over a truth-staged backing - #3266

Merged
ryansolid merged 1 commit into
solidjs:nextfrom
brenelz:fix/optimistic-draft-reads-compose
Sep 4, 2026
Merged

fix(signals): draft reads compose overrides over a truth-staged backing#3266
ryansolid merged 1 commit into
solidjs:nextfrom
brenelz:fix/optimistic-draft-reads-compose

Conversation

@brenelz

@brenelz brenelz commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

With several optimistic-store actions in flight (optimistic votes++, server confirm, refresh(store)), rapid clicks rendered 1, 2, 3, 4, then fell back to 2, then 5, while the server counted every vote. Reproduces identically at the core with a plain promise source and with an async-generator (live stream) source, so the transport is not involved.

Root cause

When the first vote's truth lands while the second is still retained, the landing is staged into the retaining transaction (#3164 fold) and the second vote's +1 replays over it. A third click's draft then read the staged truth without the replayed override: the four draft-read paths (data key, hot path, has, ownKeys) composed live overrides only while target.pb === null, on the assumption that an existing pending backing was the draft's own view-seeded clone. A truth-staged backing is authoritative truth without overrides. ensurePB's hand-off parks such a backing and reseeds from the view on the draft's first write, but votes++ reads first — it read base, wrote base + 1, and the override it emitted landed on the value already displayed. The click was invisible and the count stuck (or fell back) until truth caught up.

Minimal sequence:

step expected before
click A 1 1
click B 2 2
A's truth (1) lands, B retained 2 2
click C 3 2
B's truth (2) lands 3 2
C's truth (3) lands 3 3

Fix

draftSeesOverrides(target): draft reads compose overrides whenever the pending backing is not the draft's own tentative clone (tentativePBs), applied to all four draft paths. Nothing changes for a draft that has opened its own backing (reads must see its writes) or for authoritative drafts.

Tests

tests/optimistic-store-refetch-hold.test.ts gains "#2951 compose half": three votes in flight, the third made after the first landing, every visible value monotonic. Fails without the fix at click C (expected 2 to be 3). A ten-click replay on the app's exact schedule renders 1 through 10 with each click instant. Full signals suite, solid suite, and the web Loading/optimistic/store specs pass on the rebuilt dist.

🤖 Generated with Claude Code

With several optimistic-store actions in flight (optimistic `votes++`,
server confirm, `refresh(store)`), the first vote's truth landing is staged
into the transaction that still retains the second vote (solidjs#3164 fold) and
that vote's increment replays over it. A third click's draft then read the
staged truth WITHOUT the replayed override: draft reads composed live
overrides only while `pb === null`, on the assumption that an existing
pending backing was the draft's own view-seeded clone. ensurePB's hand-off
parks a truth-staged backing and reseeds from the view on the draft's
first WRITE, but `votes++` reads first — base 1, wrote 2, and the override
it emitted landed on the value already displayed. The click was invisible
and the count stuck (or fell back) until truth caught up; ten rapid votes
showed 1, 2, 3, 4, then 2, then 5.

Draft reads now compose overrides whenever the pending backing is not the
draft's own tentative clone (tentativePBs), in all four draft paths (data
key, hot path, has, ownKeys). Regression in the solidjs#2951 refetch-hold suite:
three votes in flight, the third made after the first landing, every
visible value monotonic.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Sep 4, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 0212cba

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
@solidjs/signals Patch
test-integration Patch
@solidjs/web Patch
@solidjs/babel-plugin Patch
@solidjs/compiler Patch
@solidjs/diagnostics Patch
@solidjs/element Patch
@solidjs/h Patch
@solidjs/html Patch
solid-js Patch
@solidjs/universal Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codspeed-hq

codspeed-hq Bot commented Sep 4, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 136 untouched benchmarks


Comparing brenelz:fix/optimistic-draft-reads-compose (0212cba) with next (439a2fe)

Open in CodSpeed

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 33825405910

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage remained the same at 71.814%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 998
Covered Lines: 764
Line Coverage: 76.55%
Relevant Branches: 783
Covered Branches: 515
Branch Coverage: 65.77%
Branches in Coverage %: Yes
Coverage Strength: 14.94 hits per line

💛 - Coveralls

@ryansolid
ryansolid merged commit 067e3bc into solidjs:next Sep 4, 2026
7 of 8 checks passed
ryansolid added a commit that referenced this pull request Sep 4, 2026
…atch

#3264/#3265/#3266 (+ dev-only #3263) land ~35 B on always-retained store
paths, and #3262's handleAsync try/catch adds ~5 B to the core scenarios.
A golf pass was attempted and measured strictly worse in every direction
(helper extraction +29 B, full inline +70 B, has-trap arm merge -1/+7 B) —
the graph is at its brotli optimum post-#3270. Ratchet per convention with
audit notes: createStore 14.16 -> 14.20 KB, isPending/latest 10.04 -> 10.05,
store-family app 26.25 -> 26.27. No package source changes, no changeset.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants