Skip to content

Patch channel round-6 hardening + patch-mode default flip - #3091

Open
ryansolid wants to merge 7 commits into
nextfrom
patch-hardening-r6
Open

Patch channel round-6 hardening + patch-mode default flip#3091
ryansolid wants to merge 7 commits into
nextfrom
patch-hardening-r6

Conversation

@ryansolid

Copy link
Copy Markdown
Member

Audit brief — round 6 + patch-mode default flip

Scope: next..patch-hardening-r6. Two bodies of work: (A) fixes for the
six round-6 findings against adf10e9b, (B) the patch-mode DEFAULT-ON flip
(both compilers). Everything below states what changed, the soundness claim,
and — most useful to attack — the reasoning each claim depends on.

A. Round-6 findings

A1. Prod-sound getter demotion (was: dev-only — reverted)

The dev-only trade is gone. Design: accessed-key recording + bounded
probes
.

  • patchDriver (web) runs the registration-time initial force-apply through
    a recording Proxy and hands the read set to registerPatch(record, fn, keys). Hydration registrations (no initial apply) record at their FIRST
    drain apply instead (applyEntries, entry.k).
  • The channel unions keys into pc.ak (deduped array). Both adoption
    emission seams (reconcile walk, fold commit) gate on targetKeysPlain:
    probe ONLY ak's keys for own getters on the adopted backing; ak === null
    (registered-but-never-applied) falls back to the full scan.
  • Claim to attack JSXFragment / jsx-dom-expressions  #1: the recorded set is COMPLETE because patch bodies
    are grammar-guaranteed sequences of if (force || n.k !== p.k) { write reading n.k } — under force the COMPARES short-circuit but every WRITE
    executes and reads its keys; under non-force first applies the compares
    read both sides. Is there any compiled body shape whose key read is
    conditional on something other than force/compare? (Eligibility grammar:
    pure member chains of one subject — check wrapPatchMode emission shapes.)
  • Claim to attack Why ES6 Proxies? #2: ak is a UNION across registrations and never
    shrinks; adoption probes are O(|ak|) per patched-record adoption.
    Measured on dbmon: tick 1.8 ms vs 1.7 no-check vs 1.9 full-scan (midday
    machine; re-measure welcome).

A2. Transition-merge collisions coalesce (scheduler.ts)

Same-channel entries in BOTH stashes now merge to ONE entry that resolves
next LIVE at drain (entry.t = pc.t, drain reads t.pb ?? t.v), keeping
the destination's prev. Attack: the prev choice — both captures are
committed pre-write values of the same record; are there merge orders where
they differ and the kept one is wrong? Also the opaque backref contract
(core mutating entry.pc.qa/qe/t) — is any other holder of these fields
surprised?

A3. Row/slot queued work respects unbinds (patch.ts)

Emitters no longer clone wrapper entries; queue items carry the LIVE
registration list plus payload (ops / si), dispatched by
applyStructural with the same unbound-mark (entry.u) + disposed-owner
checks and error routing as value patches. Attack: ordering — value
entries and structural entries interleave in emission order; the live-list
change means late registrations see earlier-queued structural work. Driver
double-applies? (registerRowOps consumers are driver-internal only.)

A4. Dispatch windows (applyEntries)

Snapshot for multi-consumer lists; FIXED length window + undefined guard for
the single-consumer alias (a callback registering another patch mid-dispatch
must not run it in the same drain — it just received its initial apply).
Attack: entry removed mid-dispatch shifts the aliased single-entry list —
covered by the undefined guard?

A5. Initial list construction severs on throw (patch-driver.ts)

Client + hydration first-build loops now sever completed rows' registrations
AND the throwing row's partials, remove inserted/claimed DOM (including the
claimed server row under hydration), dispose the list owner, rethrow.
Attack: patchCount accounting across sever-then-rethrow; boundary
remount re-engagement.

A6. Failed-apply recovery is ACTIVE (patch-driver.ts)

resyncNeeded + slot ticks now trigger an immediate identity resync (deep
value-only recovery still waits for the next list event — documented).
Identity swaps register the new subject's channels BEFORE the apply.
Attack: resync loops when the poison row keeps throwing (flag stays set,
retried per event — bounded?).

B. Default flip (patch mode ON)

  • Babel config.patchDriver: "patchDriver"; Rust patch_driver resolves
    Wrapper::Default like every other wrapper (opt out: false). The JS
    loader already normalizes true/absent.
  • All Babel dom fixture outputs regenerated; parity tier dom-patch
    replaced by dom-nopatch (fences the explicit opt-out — plain dom now
    covers patch grammar). Byte parity previously held on the whole corpus
    with patch on (108/108, zero ratchet files).
  • Attack: anything still assuming dormancy — treeshake/metafile tests,
    size-scenario notes, docs, the driveList "compiler is default-on"
    comments (now true), octane fixture flags (now redundant), the loader's
    patchDriver: true normalization interacting with default-on.
  • Known accepted costs (ruled by Ryan at flip-preview time): ~+1.5 kB brotli
    typical apps (value tier), ~+3.6 kB store-list apps, portal-swarm ~5%
    effect-fallback tax on signal-only mount churn.

Standing accepted trades (pre-existing, documented)

  • Keyless rows: adoption pairs positionally, ops rebuild — content-correct,
    retention churn (design §21a).
  • Demoted LIST-ROW bodies re-drive under the list owner (per-row severing
    lost for demoted rows) — §20.
  • Deep value-only recovery after a failed apply waits for the next list
    event (A6).

Test map

  • packages/signals/tests/store/patch-channel.test.ts — channel semantics,
    all rounds' regressions (31+ tests).
  • packages/web/test/for.patchlist.spec.tsx — driver incl. exception
    atomicity, severing, recovery (15+ tests).
  • packages/web/test/for.equivalence.spec.tsx — driver ≡ classic matrix.
  • packages/compiler/__tests__/parity* — Babel↔Oxc byte parity (dom =
    patch-on, dom-nopatch = opt-out).

ryansolid and others added 2 commits August 28, 2026 12:48
Prod-sound getter demotion (accessed-key recording + bounded adoption
probes), transition-merge same-channel coalescing, unbind-safe structural
queue dispatch, fixed dispatch windows, initial-construction severing on
throw, and active failed-apply resync.

Co-authored-by: Cursor <cursoragent@cursor.com>
Both compilers now emit patchDriver/rowProof templates by default
(opt out: patchDriver: false). Regenerated fixture outputs and Oxc
expectations; parity tier dom-nopatch fences the opt-out. Size scenarios
renamed from flip-preview to default-on and ratcheted with round-6 notes.
Includes AUDIT-BRIEF-R6.md for the pre-merge audit.

Co-authored-by: Cursor <cursoragent@cursor.com>
@changeset-bot

changeset-bot Bot commented Aug 28, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 36d1d38

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 Major
@solidjs/web Major
@solidjs/babel-plugin Major
@solidjs/compiler Major
@solidjs/universal Major
test-integration Patch
@solidjs/element Major
@solidjs/h Major
@solidjs/html Major
solid-js Major
@solidjs/diagnostics Major

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

@github-actions

Copy link
Copy Markdown

size-limit report 📦

Path Size
signals: core floor (createSignal/Memo/Effect/Root/flush) 7.72 KB (+0.69% 🔺)
signals: + createStore 14.12 KB (+1.02% 🔺)
signals: + isPending/latest 9.7 KB (+0.96% 🔺)
app: render + one signal (the simple-app floor) 10.44 KB (+0.84% 🔺)
app: hydrating (no stores) with Show/For/Loading/Errored/lazy 17.15 KB (+0.46% 🔺)
app: hydrating + every store primitive family 25.52 KB (+0.54% 🔺)
app: CSR with Show/For/Loading/Errored/lazy 12.55 KB (+0.18% 🔺)
app: CSR flip preview — + patchDriver (non-list patch templates) 0 B (-100% 🔽)
app: CSR flip preview — + rowProof (patch-mode list driver) 0 B (-100% 🔽)
frames: eager client consumer (frames client + transport, lazy codec) 10.64 KB (0%)
app: CSR default-on — + patchDriver (non-list patch templates) 14.56 KB (+100% 🔺)
app: CSR default-on — + rowProof (patch-mode list driver) 16.79 KB (+100% 🔺)

@coveralls

coveralls commented Aug 28, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 33239878602

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.652%

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: 1003
Covered Lines: 767
Line Coverage: 76.47%
Relevant Branches: 789
Covered Branches: 517
Branch Coverage: 65.53%
Branches in Coverage %: Yes
Coverage Strength: 14.54 hits per line

💛 - Coveralls

@codspeed-hq

codspeed-hq Bot commented Aug 28, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 44.18%

⚡ 2 improved benchmarks
✅ 134 untouched benchmarks
⏩ 132 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
merge 138.1 µs 71.6 µs +93%
reconcile: deep tree, 10 of ~12k paths subscribed 3.4 ms 3.2 ms +7.71%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing patch-hardening-r6 (36d1d38) with next (5be07a8)

Open in CodSpeed

Footnotes

  1. 132 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@ryansolid

Copy link
Copy Markdown
Member Author

Post-round-6 + default-flip dbmon (octane harness, alternating 30-iter rounds, midday machine — medians in ms):

op octane solid (this branch) ratio (octane/solid score)
mount 4.00 6.40 0.63–0.67x
tick 1.60 1.80 0.83–0.85x
tick_partial 0.70 0.50 0.93x
remount 4.10 4.80 0.77–0.79x
sort 2.00 2.30 0.74–0.84x
unmount 1.60 1.70 high variance

Tick at 1.8 matches the recording+probe expectation measured when A1 landed (1.8 vs 1.7 with no demotion check vs 1.9 full-scan) — the round-6 hardening added no regression beyond the already-priced probe cost. Partial-tick diff-skip ratio 0.28–0.30 (better than octane's 0.32). Semantic gate green both rounds.

@ryansolid

Copy link
Copy Markdown
Member Author

Patch-mode worth-proof: same branch, same runtime, same fixture source — the ONLY difference is the compiler flag (three-way alternating, 30 iters, medians ms):

op octane solid patch (default) solid classic (patchDriver: false) patch speedup
mount 4.10 6.40 14.10 2.2x
tick 1.60 1.90 6.70 3.5x
tick_partial 0.70 0.50 1.10 2.2x
remount 4.30 4.80 9.10 1.9x
sort 2.10 2.20 2.60 1.2x
unmount 1.70 1.70 2.20 1.3x

Patch mode is what puts this next to octane: classic sits 4.2x behind on the full tick; patch is within ~19%. The ~1.5–3.6 kB bundle cost in the size scenarios buys this column.

(Incidentally the flip surfaced in the harness itself: the fixture's opt-out env passed empty compiler options, which now mean patch-ON — fixed to explicit patchDriver: false in the octane repo before measuring.)

ryansolid and others added 3 commits August 28, 2026 14:00
Contract-derived tests, not instance reproductions: accessor-safety across
admission/adoption axes, one-apply-per-batch under lane interleaving,
consumer-list lifecycle at emission/drain/merge boundaries, throw-atomicity
at every row-build entry point, hydration region surrender, Tier-2 read-set
completeness, and compiled-imports ⊆ documented-runtime-surface contracts.
10 red / 8 green against the round-6 branch; each red is a confirmed
audit-7 finding, each green documents an invariant that already holds.

Co-authored-by: Cursor <cursoragent@cursor.com>
Each fix flips a red invariant-harness test green: stateless adoption
probes against the incoming backing, prototype-plainness admission,
split normal/optimistic stamps, emission-snapshot structural queues +
drain-live value lists (with cross-release coalescing), build-before-
destroy slot rebuilds, and full server-region surrender on hydration
throw. Store+core suites 465 green; driver suites green.

Co-authored-by: Cursor <cursoragent@cursor.com>
…1; renderer surface documents patchDriver (P1-4)

Both compilers emit the body's full static read envelope (hoisted _mf$
arrays, deduped per module); the runtime interns manifests by identity,
probes deep paths as a prefix tree at adoption gates and forced applies,
bubbles ancestors on targeted reconciles, and resolves forced next through
the proxy for deep-path channels. Eligibility: bare-subject reads and
dotted string keys compile classic. Renderer type/README/contract tests
pin patchDriver into the documented surface. PINV per-flush ledger checks
wired into the __TEST__ invariant infra. dbmon: tick 2.1ms quiet-machine
(was 1.9 unsound / 2.5 first sound cut), mount intern misses eliminated.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ryansolid

Copy link
Copy Markdown
Member Author

Round 7 pushed — all nine audit findings fixed, harness-first. Three commits:

  1. Red invariant harness (committed failing, before any fix): channel contracts, driver throw-atomicity matrix over every build entry point, lane-interleaving with an effect-parity oracle, consumer-list lifecycle at emission/drain/merge boundaries, and imports⊆documented-surface contract tests per generate mode. 10 red / 8 green against the audited tree — each red reproduced a finding; the greens document invariants that already held (initial-construction atomicity, staged-update atomicity, deep resync atomicity, universal import discipline).
  2. Runtime fixes for P1-2/3/5/6 and P2-2/3/9 (see AUDIT-BRIEF-R6.md §Round 7 for the per-finding approach).
  3. P1-1 + P1-4: static read manifests from BOTH compilers (hoisted _mf$ arrays, byte-parity, full fixture regen — 4321 oxc + 153 babel tests green) replacing runtime recording, which provably cannot see untaken ternary branches; Renderer/README/type now document patchDriver with contract tests pinning compiled imports to documented surfaces.

Also new: PINV-1..3 per-flush channel-ledger assertions in the __TEST__ invariant infra — registration accounting, stamp retention, and queue drainage are now checked at every quiescence point in every test, not per-scenario.

Perf, quiet machine: dbmon tick 2.1 ms vs round-6's 1.9 — the +0.2 is the honest price of sound accessor demotion across branches and nested chains (first sound cut measured 2.5; manifest interning, prefix-tree probing, and leaf inlining recovered the rest — two profiler passes attached to the work, not guesses). Partial tick still beats octane (0.5 vs 0.7). Mount's intern misses were profiled to zero after array hoisting; final mount number needs one quiet-machine confirmation (a parallel build was loading the box during the last sweep).

Gates: 32/32 monorepo tasks, size scenarios ratcheted with dated notes (net round-7 cost: ~+0.3 kB store apps, ~+0.6 kB patch-list apps), changesets included.

@ryansolid

Copy link
Copy Markdown
Member Author

Quiet-machine confirmation sweep (evening, 80% idle, both orders, 30 iters):

op octane solid (round 7 final) round-6 reference
mount 4.10–4.20 6.20–6.40 6.30–6.40
tick 1.60 2.00 1.90
tick_partial 0.70 0.50–0.60 0.50
remount 4.00–4.30 4.60–4.70 4.80

Mount fully recovered — the _mf$ hoisting eliminated the intern misses exactly as profiled. The final price of prod-sound accessor demotion across ternary branches and nested chains is +0.1 ms on the full tick (~5%), zero on mount, and partial ticks still beat octane. Classic (patch off) remains 6.7 on the same tick for scale.

That closes round 7 end to end: 9/9 findings fixed harness-first, gates green, perf within noise of the pre-audit numbers with the soundness holes closed.

Admission deep-probes manifests + committed-view reads, captured-record
structural binds (patchProxyFor via createTarget-installed wrap hook —
keeps the trap engine shakeable), lane-timed tentative ancestor bubbles
with settle twins, generation-stamped drains, forced-bubble coalescing,
decimal-key ineligibility (both compilers), rowProof on the createRenderer
surface, untracked universal commit phase. dbmon unchanged (6.4/2.0/0.5).

Co-authored-by: Cursor <cursoragent@cursor.com>
@ryansolid

Copy link
Copy Markdown
Member Author

Round 8 pushed — 8/8 findings fixed, harness-first again (the new invariant tests committed red where reproducible). Highlights beyond the straight fixes:

  • The P2 duplicate-apply finding had a deeper root cause: patchableRaw served pb ?? v, leaking uncommitted transition drafts to mid-transition mounts. Initial applies now read the COMMITTED view, and generation-stamped drains skip consumers that initialized from an entry's state (transition releases exempt themselves — their late consumers saw the pre-commit view). The fold path was verified unreachable for the reported interleaving — the walk queues value entries before structural ops by design, and a test now pins that — but the cross-queue optimistic window was real.
  • The structural-capture fix nearly shipped a 3.7 kB regression: binding captured records needs raw→proxy wrapping, and importing wrapNext into the channel would retain the entire trap/write engine in store-less bundles that merely compile a rowProof list. The size gate caught it; the wrap now rides a hook createTarget installs (if no store ever created a target, no raw can need resolving).
  • Tentative ancestor bubbles are two-sided: lane-timed forced entries for in-flight visibility plus settle-held twins, so reverts restore ancestor expressions and landings show landed truth — the harness asserts both directions.

Everything else as reported: manifest deep-probe at admission, decimal-key ineligibility in both compilers (byte-parity fixtures regenerated), rowProof on the createRenderer surface with the contract test extended to the whole patch tier, and the universal patchDriver commit phase untracked.

Gates: 32/32 tasks, 4,323 compiler + 153 Babel + 468 signals + 71 driver tests, size ratcheted (+~0.3 kB per tier with dated notes). dbmon identical to round-7 finals: mount 6.4 / tick 2.0 / partial 0.5, quiet machine, both orders.

Committed-visible skip markers + held-view admission, optimistic-view
mounts, manifest-read fallback compute (write-free), tentative self
emission + immediate lane demotion, per-queue forced stamps + merge
repair, isWrappable binds, server patch-tier exports, function-carrier
probes, safe-int keys, unchanged-reconcile bubble gate. Attribution test
warn-count flake fixed two-sided. dbmon 6.3/2.1/0.6 (parity with r7/r8).

Co-authored-by: Cursor <cursoragent@cursor.com>
@ryansolid

Copy link
Copy Markdown
Member Author

Round 9 pushed — 11/11 findings addressed, harness-first, with three findings materially refined during verification:

  • The gen-stale finding's ambient case self-corrects (eager adoption swaps the committed backing at walk time, so pre-flush mounts read the new state — pinned by a test). The REAL stale window was mid-transaction, and the fix is semantic: the skip rule now applies only to entries emitted from committed-VISIBLE state, patchableRaw serves the held view for masked targets, and the mount invariant test anchors to what an untracked proxy reader sees rather than to any implementation detail.
  • The optimistic-getter finding uncovered a missing seam, not just a missing probe: tentative reconciles never emitted on the record's own channel at all — effects saw the tentative view, patches didn't. They now emit the view at lane timing, the optimistic drain probes payloads for accessors, and demotion there runs its re-drives IMMEDIATELY (the global render queue is stashed in flight; deferral would postpone visibility to settle).
  • The NaN/unstable-getter fallback fix is structural: the manifest IS the read set, so the effect fallback's compute pass reads the declared envelope directly and never executes the body — write-free by construction, applied to both web and universal drivers.

The rest as reported: per-queue forced-stamp clearing + merge repair, isWrappable bind guards, server-entry patchDriver/rowProof (notSup/identity, matching the template stub class), function-intermediate probes (accessor carriers demote conservatively), safe-integer-only manifest keys in both compilers, and unchanged reconciles no longer bubble. The attribution CI flake is fixed two-sided: the harness mutes its expected demotion warnings, and the attribution test now counts its own diagnostic's warns instead of the process-global total.

Gates: 32/32 tasks, 4,325 compiler tests, size gates ratcheted with dated notes. dbmon 6.3 / 2.1 / 0.6 — parity with rounds 7–8.

Standing note for the next design pass (also in the brief): rounds 7–9's P1s are dominated by hand-replicated visibility rules at channel seams. I'm prototyping node-driven delivery (one hidden node per patched record, compiled bodies unchanged) as the by-construction answer before the next audit round — if per-record nodes hold dbmon, that eliminates the timing-divergence class entirely.

@ryansolid

Copy link
Copy Markdown
Member Author

Review note from #3161 (For + siblings hydration desync, fixed in 91e300a): the regression came from the list seam's lazy mapArray creation minting the list's hydration id scope at insert time instead of source position. The fix makes a hydrating <For> create eagerly, which covers the classic path this PR inherits — but once this PR arms the driver by default, driver-ENGAGED hydration takes a different road: driveList claims rows positionally by _hk and mints its own listOwner child id at insert time. Before flipping, the parity matrix should pin the for-then-siblings fragment shape (now in test/harness/scenarios.tsx) with a store-backed, stamped-row list that actually engages — verifying siblings after a DRIVEN list keep their keys on both the engage and decline roads.

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.

2 participants