fix(signals): L2 lane takeover of held effects (#3766) and optimistic writes over a held row (#3796) - #3812
Open
ryansolid wants to merge 2 commits into
Open
fix(signals): L2 lane takeover of held effects (#3766) and optimistic writes over a held row (#3796)#3812ryansolid wants to merge 2 commits into
ryansolid wants to merge 2 commits into
Conversation
A render effect born held in a transaction whose hold a lane took over (#3698: lane work never makes its node transaction work) lost the lane on its next pass while keeping the held flag: - A leaf has no lane value to seat it, so an effect left waiting on the lane's flight re-ran outside the lane when the landing woke it and published past the lane's hold. It is now marked as the lane's (REACTIVE_LANE_DIRTY) when its pass was interrupted, so the landing that wakes it — the lane's re-staging — runs it as the lane's (A31), and the lane holds it until its derivations land (A17). - A pass interrupted before it reached the lane's world (pending, errored) no longer counts as leaving the lane (A30: it never got there). - A pass that does leave the lane drops the hold the lane took over along with the lane's transaction, instead of keeping CONFIG_HELD with no transaction behind it (the `resolveTx` null crash the semantic fuzzer found against the mark alone: seed 3289, optimistic-readiness case 112). The mark is brenelz's from #3794; the leave and errored arms make it safe. Tests: #3794's ispending-memo-gate-3766 (signals + web), the F5 pin flips to passing, and a new pin for a held reader that leaves the lane. Fixes #3766 Co-authored-by: Brenley Dueck <brenleydueck@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…eys, not its slot (#3796) An optimistic store chained over a derived store composes its draft from the inner store's raws (the descriptor read of the inner proxy): a row's slot holds the row's committed backing. When an earlier write in the same action staged that row (a local `setLocal` held with the action), the view the setter is judged against resolves the slot to the row's staging instead. The two differ by identity only, so the setter's exit guessed the container's slot itself — with the committed row — and that slot guess served a fresh target over the old raw, shadowing the row's own `title` and `saving` guesses: `Old / saving: false` while the action was pending. A raw in a chained draft is now resolved to the inner slot it stands for — as a read of it is served (`resolveChainedRaw`) — before it is compared, so an unwritten slot is the same row in both frames and no guess. The edit shows while the action is pending (A17: an active override is the displayed value until its transaction commits). Fixes #3796 Co-authored-by: Cursor <cursoragent@cursor.com>
🦋 Changeset detectedLatest commit: 4e40669 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
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 |
Size (brotli, eager entry chunk)
Bundled with Rolldown (what Vite ships), brotli q11, decimal KB. Caps in |
Coverage Report for CI Build 37386808735Warning No base build found for commit Coverage: 75.991%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
Merging this PR will improve performance by 7.02%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | memo + sync render effect only (reference) |
29.3 ms | 27 ms | +8.63% |
| ⚡ | dbmon shallow full tick |
53.2 ms | 50.4 ms | +5.42% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing fix/l2-remaining-bugs (4e40669) with next (a8c98bd)
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
2 participants
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.
Two L2 hold-model fixes from the remaining-bugs list, one commit each, each with a fail-first test and the rule it implements.
Fixes #3766
Fixes #3796
Commits
1.
fix(signals): a lane keeps the held render effect it took over (F5)— #3766A render effect born held in a transaction whose hold a lane took over (#3698: lane work never makes its node transaction work) dropped the lane on its next pass but kept
CONFIG_HELD.REACTIVE_LANE_DIRTY. The landing that wakes it is the lane's re-staging, so it runs as the lane's (A31), and the lane holds it until its derivations land (A17).The mark alone crashed the semantic fuzzer:
resolveTxreturned null, because the leave arm nulled the transaction of an effect that was stillCONFIG_HELD(seed 3289, optimistic-readiness case 112, plus 11 more). The second and third changes make the mark safe.Tests:
ispending-memo-gate-3766(signals and web), 4 of 6 signals tests failing at base.fuzz-findings-l2.test.tsflips fromit.failsto passing.it.failspin, F5b, records a separatedisplay()issue found along the way (see below).2.
fix(signals): an optimistic write over a held row guesses the row's keys, not its slot— #3796createOptimisticStore(local)over a derived store composes its draft from the inner store's raws, so a row's slot holds the row's committed backing. When an earlier write in the same action (setLocal(d => { d[0].failed = false })) had staged that row, the view the setter is judged against resolved the slot to the row's staging. The two differed only by identity, so the setter's exit guessed the container slotcards[0]itself, with the committed row. That slot guess served a fresh target over the old raw object and shadowed the row's owntitleandsavingguesses: the screen showedOld / saving: falsewhile the action was pending.The fix resolves a raw in a chained draft to the inner slot it stands for before comparing, the same way a read of it is served (
resolveChainedRaw). An unwritten slot is now the same row in both frames, not a guess (A17: an active override is the displayed value until its transaction commits).Test:
optimistic-over-held-row-3796.test.tscovers the report's shape with and without the local write. The local-write case fails at base.Not in this PR
latest(s)and then a lane memo whose answer hasn't shown yet waits as verdict-lane work, so the mount control publishes alone. Reading the memo first holds the whole mount. The outcome depends on read order. Under--shrink-mode focused, cases 416 and 936 shrink to the same family; the earlier "disposal order" reading came from a discovery shrink that drifted to another signature. I've written this up for the maintainer separately, with options and a recommendation.display()returns the latest lane value once the lane has shown). It's pinned asit.fails. Fixing it needs the revealed lane value tracked apart from the latest guess.Size
Rolldown + brotli via
scripts/size,next(a8c98bd) vs this branch. Minified bytes are the attributable unit; brotli moves by a few bytes even where minified is unchanged.observeartifacts)The hello-world scenario (app: render + one signal) is unchanged. In the store scenario, the F5 commit's +27 is +25 in signals plus 2 B of solid/web mangling noise.
Size decision needed
signals: + isPending/latestis 15 B brotli over its cap (9505 vs 9490). That scenario had 4 B of headroom onnext, and F5's +20 B minified (inlaneStage) uses it up. No cap was raised in this PR. The options are a 20 B cap bump for this scenario (it isn't one of the frozen floor caps), or a trim elsewhere in the lanes module.Fuzzer gate
fuzz/semantic-fuzzer-l2(oracle rev 19), 1000 cases,--shrink. "Before" isnext's sources and "after" is this branch's.Suites
pnpm types: passesrules-index --check: currentOverlap with in-flight branches
fix/create-time-holdsandfix/f6-boundary-frame-reader: the only textual conflict is the generatedpackages/signals/docs/RULES-INDEX.md(regenerate). F6's edits tofuzz-findings-l2.test.tsauto-merge.dist-artifacts, because the scratch tree had no build.Public API changes
None.
resolveChainedRawis now exported fromsrc/store/store.tstosrc/store/optimistic.ts. That is an internal module edge, and the export is not re-exported from any package entry.Co-authored-by: Brenley Dueck brenleydueck@gmail.com