Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# The array-mutation loop-detection requirement contradicts its own fill scenario

**Status:** needs-triage

In the `array-mutation` spec as the `wrap-fill-and-copywithin` delta writes it, the requirement "Mutating methods are subject to loop detection" says in its second sentence that when a synchronous top-level effect calls a mutating method and the call writes a property, the system raises the infinite-loop error. The third sentence and the scenario "Fill throws only when the effect read an index it writes" say that `fill` alone writes properties and raises nothing. The scenario "Same loop outcome on both kinds of state" under the first requirement says both kinds raise the same error for a mutating method call, which is false for `fill` alone, where neither raises. The scout of the apply run found both on 2026-10-25. Code is unaffected; the fix is one sentence in the requirement and one qualifier in the scenario, through `/opsx:update` while the change is open or in the main spec after archive. The `tests/AGENTS.md` table also lists the test count of `state-hooks.test.ts` as 19 and goes stale when ticket 03 adds hook assertions there.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Length-only readers wake on fill and copyWithin once they are batched

**Status:** needs-triage

An effect that reads only `length` of a reactive array never re-runs on `fill` today, because `fill` writes no `length` and the unwrapped call notifies per written index. Once `fill` and `copyWithin` run inside the batch wrapper, as the `wrap-fill-and-copywithin` change makes them, the batch marks the whole array dirty and wakes every subscriber of the array once per call, so a length-only reader re-runs once per `fill` call. Probed on `a7e06bc` with Node v24.21.0: two `fill` calls give a length-only effect 0 re-runs unbatched and 2 re-runs batched. This follows the rule the `array-mutation` spec already states for every wrapped method, "regardless of which properties the subscriber read", and `push(3)` wakes an effect that reads only index 1 today for the same reason. Whether a method call should wake only subscribers of the properties it changed is a question about that rule, not about this change. The inside-effect variant is in `.do/inbox/2026-10-23-per-property-wake-inside-effects.md`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Same-value writes to frozen targets do not throw

**Status:** needs-triage

A native array rejects every write to a frozen target with a TypeError, whether or not the value changes: `Object.freeze([9]).fill(9)`, `Object.freeze([1, 2]).sort()` and a strict-mode `frozen[0] = 9` all throw. Reactive state over the same frozen arrays throws only when the value differs, because the set trap returns early on an unchanged write before it reaches the frozen target. Probed on `a7e06bc` with Node v24.21.0 during the `wrap-fill-and-copywithin` run: `state(Object.freeze([9])).fill(9)`, `state(Object.freeze([9]))[0] = 9` and `state(Object.freeze([1, 2])).sort()` return without throwing, while `state(Object.freeze([1, 2, 3])).fill(9)` and `state(Object.freeze([9]))[0] = 5` throw the native TypeError. The root cause is the unchanged-write check shared by every write, not an array method, so the fix touches the write path the benchmark gate measures. The user ruled on Q5 of that run that it is fixed as its own change; ticket 02 of `wrap-fill-and-copywithin` tests the frozen case with `[1, 2, 3]` only.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Shared load sites in subscriber scheduling deoptimize on mixed array and object writes

**Status:** needs-triage

The ticket 04 investigation of change `wrap-fill-and-copywithin` (second implementer, 2026-10-25, report section 15) found that `scheduleSubscribersForTarget` and `findSubscribers` in `src/index.ts` serve array and plain-object targets from the same load sites, and that a benchmark sequence mixing array and object writes (`state write 1 sub`, push/pop, `fill`, `many dependencies`) makes V8 bail out of them 14 and 3 to 6 times per run on every commit measured, including the pre-change baseline `87c8823`. It is the same class of pollution that commit `b595300` removed from `handleBatchFastPath` by giving arrays their own set handler and batch write path, where it cost `many dependencies` about 10% on the median after `fill` moved onto the batch path. This one is not a regression from the change and was left alone. Candidate for the library-wide performance tournament (`2026-10-23-library-wide-performance-tournament.md`): measure a split of those two functions by target kind with the same interleaved protocol, or confirm the bailouts cost nothing measurable.
21 changes: 21 additions & 0 deletions .do/ledger/wrap-fill-and-copywithin.md
Original file line number Diff line number Diff line change
@@ -1,2 +1,23 @@
Ruling: Change branch is `fix/wrap-fill-and-copywithin`, created at trunk `18f5a71` on the user's answer to Q1 (a) - no earlier run of this change exists, the historian found `<type>/<slug>` in every recent human PR branch, and the sibling change PR 186 used the `fix` type - if wrong, the branch is renamed before the pull request and nothing else moves.
Ruling: The uncommitted OpenSpec and .do/ files are committed on the change branch as `213bbcf` before the first dispatch, on the user's answer to Q2 (a) - worktrees check out tracked files only, and ticket 03 edits the main spec `openspec/specs/array-mutation/spec.md`, which was untracked - if wrong, one docs commit carries planning text onto the fix branch, which is harmless.
Ruling: Ticket 01's benchmark effect reads every index of the array, not only `length` - user chose it (Q3 a) after a probe on `a7e06bc` showed a length-only reader gets 0 wakes per `fill` today and 1 after the change, while an every-index reader gets 3 today and 1 after, and the every-index effect is the developer use `fill` affects - if wrong, the new case measures a path nobody writes and ticket 04's "expected to be faster" is judged on it.
Ruling: Ticket 03 criterion 3 "the arrays end with the native contents" means the contents a plain array holds after the same sequence of calls, so the microtask `copyWithin(0, 1)` effect ends at `[3, 3, 3]` after two runs, the child effect at `[2, 3, 3]`, both `fill` effects at `[9, 9, 9]` - user chose it (Q4 a); the microtask effect re-runs once because it wrote index 1 after reading it, and a plain array after two calls reads `[3, 3, 3]` - if wrong, the test pins a convergence outcome the author did not intend.
Ruling: Same-value writes to a frozen array, which the native array rejects with a TypeError and Beacon swallows through the set trap's unchanged-write check, are fixed as their own change and not in ticket 02; ticket 02's frozen test uses `[1, 2, 3]`, which throws today - user chose it (Q5 a); the gap predates the change and the fix is on the shared write path the ticket 04 gate measures - if wrong, the divergence ships one release longer; it is recorded in `.do/inbox/2026-10-25-same-value-writes-to-frozen-targets-do-not-throw.md`.
Ruling: Tickets 02 and 03 govern where the main spec's purpose line and the agent docs change, which is ticket 03, over design.md's sentence that puts them in the same commit as the set - design.md's migration plan and both tickets agree on the ticket order - if wrong, `src/AGENTS.md` says "five" for the one commit between 02 and 03.
Ruling: Ticket 02 criterion 5 governs the property-based test, which stays hookless as the file is today; the "both kinds of state" sentence of the ticket applies to the array-method test file - the criterion names only the generation of `fill` and `copyWithin` calls beside the existing methods - if wrong, the property test misses a hooked-state divergence the example tests also cover.
Ruling: Biome's reflow of the method-name set over several lines, forced by the 120-character line width when two names join the one-line set, counts as "changed only in the set of method names" for ticket 02 criterion 7 - the reflow is formatting of the same declaration - if wrong, the reviewer reads a formatting hunk as a library change.
Verdict: 01 - approve - Nothing wrong found. Criteria 1 and 2 are met, and the standards checks pass. Landed by fast-forward at `87c8823`.
Ruling: The ticket 01 worktree `.claude/worktrees/agent-a98f18db12d80bac2` and its branch `worktree-agent-a98f18db12d80bac2` stay on disk - the merger's `git worktree remove` exited 255 with "Operation not permitted" from the sandbox, and `git branch -d` refuses while the worktree is registered - if wrong, nothing is lost, the branch is at `87c8823`, which the change branch contains, and the closing report carries the commands that remove it.
Ruling: The ticket 02 implementer runs on after six `env` markers between 11:55:20Z and 11:56:10Z, all PreToolUse hook refusals of a scripted-edit detour, and no further question is asked about environment markers in this attempt - user chose it (Q6 a) after the implementer moved on to passing test runs at 11:55:59Z, 11:56:24Z and 11:56:56Z; a stall or a contract violation still stops for the user - if wrong, hook refusals that signal real trouble pass without a question.
Ruling: The ticket 02 review of `87c8823..4f85493` returned one finding, low standards, on the commit message of `4f85493` (body unwrapped, past tense), so the attempt does not fail and the finding goes to the same implementer as a fix-up, which is not its retry - section 4 of the apply skill - if wrong, a fix-up round is spent on a message the merger could have rewritten.
Verdict: 02 - approve - No findings. Criteria 1-7 met by `5b42960`; the round 1 commit message finding was fixed by amending the message only. Landed by fast-forward at `5b42960`.
Ruling: The ticket 02 worktree `.claude/worktrees/agent-afca106c6cf68d909` and its branch `worktree-agent-afca106c6cf68d909` stay on disk - the merger's `git worktree remove` exited 255 with "Operation not permitted" from the sandbox, and `git branch -d` exited 1 because the worktree is registered - if wrong, nothing is lost, the branch is at `5b42960`, which the change branch contains, and the closing report carries the commands that remove it.
Verdict: 03 - approve - No findings. Criteria 1-8 met by `8464f3f` in two commits, `17a14c5` tests and `8464f3f` docs; no library source changed. Landed by fast-forward at `8464f3f`.
Ruling: The ticket 03 worktree `.claude/worktrees/agent-a8b1f67e276534874` and its branch `worktree-agent-a8b1f67e276534874` stay on disk - the merger's `git worktree remove` exited 128 with "cannot remove a locked working tree, lock reason: claude agent agent-a8b1f67e276534874 (pid 63381 ...)" because the implementer agent still waits on a background shell it already killed, and `git branch -d` exited 1 because the worktree is registered - if wrong, nothing is lost, the branch is at `8464f3f`, which the change branch contains, and the closing report carries the unlock and removal commands.
Ruling: Ticket 04's 10-cycle alternating re-measurement for a case over the 1% line runs as four full-suite runs with `node --expose-gc scripts/benchmark.ts -R 10` in the order baseline `87c8823`, final, final, baseline `87c8823`, because the script has no single-case filter and a full run at 10 cycles costs about 75 seconds, so the four cost about 5 minutes - if wrong, the implementer spends 5 minutes the user would rather have skipped, and the result is still the measurement the ticket asks for.
Ruling: Q7 - ticket 04's two cases over the line after the four-run re-measurement, `state write 1 sub` +2.43% and `many dependencies` +7.57% on means of totals, +2.3% to +4.1% and +8.9% to +11.4% on medians against the nearest baseline, are investigated and not re-rolled or waved through: the maintainer directed senior methodology on 2026-10-25, which the record defines as an A/A test to learn each case's noise, interleaved A/B runs with medians per case and a verdict of faster, slower or indistinguishable within the A/A spread (`.do/inbox/2026-10-23-library-wide-performance-tournament.md`), targeted micro-benchmarks and CPU and deopt profiles of the two cases on both commits (Q66 d: "profiling is a good tool"), a library fix if a mechanism shows, and no existing benchmark case altered; ticket 04's Area gains the benchmark script for additions only, as Q66 d did for ticket 07 of the previous change - if wrong, ticket 04 spends about half an hour of machine time on two cases that noise alone explains, and the evidence for that is then on file.
Ruling: The ticket 04 investigation runs in a second implementer's fresh worktree as build B at `8464f3f`, with the finished ticket 01 worktree `.claude/worktrees/agent-a98f18db12d80bac2` at `87c8823` used read-only as build A, and no checkout switching - the first implementer's worktree was removed by Claude Code when it finished without a commit, its resume landed in the main checkout and it stopped correctly rather than detach HEAD there, and this session's `git worktree add` failed with "Could not reset index file to revision 'HEAD'" under the sandbox; the first attempt counts as attempt 04a, stopped without approval on the gate, and its report stands in the ticket file - if wrong, build A's worktree carries another agent's name in the report, which the report states.
Ruling: The ticket 04 attempt 04b review of `8464f3f..b595300` returned three findings, all low standards (a `src/AGENTS.md` sentence overstating which closures are split by target kind, a comment's "7 to 10%" against measured +10.6% medians and +6.8% totals, and a six-line dirty-check-and-lookup tail that `recordDirectWrite` does in one line), so the attempt does not fail and the findings go to the same implementer as a fix-up, which is not its retry; the duplicated-tail swap is adopted only if an interleaved re-measure shows `array fill` and `many dependencies` indistinguishable or faster, per the maintainer's methodology directive - if wrong, a fix-up round and about ten minutes of measurement are spent on a comment, a sentence and six lines.
Verdict: 04 - approve - Attempt 04a measured only and failed the gate after the four-run re-measurement; attempt 04b found the mechanism (arrays reaching the object batch path deoptimized its subscriber load, costing `many dependencies` +10.6% on medians interleaved against `87c8823`), fixed it in `b595300`, and round 1 returned three low standards findings fixed or measured in the amend `2dbcee2`; round 2 found nothing. After the fix no existing case is slower than `87c8823` beyond its A/A spread, `array fill (hookless)` is 50% faster, manifests read 2002.0.0 and the handbook is untouched. Landed by fast-forward at `2dbcee2`.
Ruling: The ticket 04 worktree `.claude/worktrees/agent-a0d5861de6e8b46d8` and its branch `worktree-agent-a0d5861de6e8b46d8` stay on disk - the merger's `git worktree remove` exited 255 with "Operation not permitted" from the sandbox and `git branch -d` exited 1 because the worktree is registered - if wrong, nothing is lost, the branch is at `2dbcee2`, which the change branch contains, and the closing report carries the removal commands.
Ruling: Q8 a - the change branch `fix/wrap-fill-and-copywithin` at `2dbcee2` is pushed to `origin` with `git push -u origin fix/wrap-fill-and-copywithin`; `git ls-remote` shows `2dbcee2a0597cdd8e5717c01a0fb1a0afca521a8 refs/heads/fix/wrap-fill-and-copywithin` on the remote, while the sandbox refused the upstream config write ("error: unable to write upstream branch configuration"), so the tracking link is set by the user outside the sandbox - if wrong, a second push without `-u` goes to the same remote branch and nothing diverges.
105 changes: 105 additions & 0 deletions .do/review/01-a7e06bc..87c8823.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
# Review package: ticket 01, a7e06bc..87c8823

## Commits
### 87c8823
perf(state): add array fill benchmark case for the baseline

The wrap-fill-and-copywithin change alters how many wakes one fill call
causes. This case subscribes one effect that reads every index of a
hookless array and times fill calls that change every element, so the
unchanged library records the baseline. Typed perf because it measures
performance; no library behaviour changes.


## Files changed
scripts/benchmark.ts | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

## Diff
diff --git a/scripts/benchmark.ts b/scripts/benchmark.ts
index 9685a7b..30a3d81 100644
--- a/scripts/benchmark.ts
+++ b/scripts/benchmark.ts
@@ -5,20 +5,22 @@ import { batch, derive, effect, state } from '../src/index.ts'
import type { StateHooks } from '../src/types.ts'

const DEFAULT_RUN_COUNT = 10
const LOOP_LENGTH = 1_000_000
const ERROR_FREQUENCY = 1000
const WARMUP_RUNS = 3
const BENCH_RUNS = 7

const EFFECT_TRIGGER_WRITES = 50_000
const ARRAY_PUSH_POP_CALLS = 50_000
+const ARRAY_FILL_CALLS = 50_000
+const ARRAY_FILL_LENGTH = 3
const MULTI_SOURCE_COUNT = 100
const MULTI_SOURCE_ITERATIONS = 100
const DERIVE_CHAIN_DEPTH = 10
const DERIVE_CHAIN_ITERATIONS = 1_000
const STATE_CREATION_COUNT = 100_000
const STATE_READ_COUNT = 1_000_000
const SINGLE_SUB_WRITES = 100_000
const MANY_SUB_COUNT = 100
const MANY_SUB_WRITES = 10_000
const DISJOINT_PROP_COUNT = 100
@@ -310,20 +312,34 @@ function arrayPushPop(items: number[]): number {
}
const end = performance.now()
cleanup()
return end - start
}

const arrayPushPopHookless = (): number => arrayPushPop(state<number[]>([]))

const arrayPushPopHooks = (): number => arrayPushPop(state<number[]>([], noopStateHooks))

+// The effect reads every index because fill never changes length: a length-only reader would not wake at all.
+// Alternating the fill value changes every element on every call
+function arrayFillHookless(): number {
+ const items = state<number[]>(new Array(ARRAY_FILL_LENGTH).fill(0))
+ const cleanup: Unsubscribe = effect((): void => {
+ for (let i = 0; i < items.length; i++) void items[i]
+ })
+ const start = performance.now()
+ for (let i = 0; i < ARRAY_FILL_CALLS; i++) items.fill((i % 2) + 1)
+ const end = performance.now()
+ cleanup()
+ return end - start
+}
+
function manyDependencies(): number {
const sources = Array.from(
{
length: MULTI_SOURCE_COUNT,
},
(
_: unknown,
i: number
): {
v: number
@@ -600,20 +616,24 @@ const suite: SuiteEntry[] = [
name: 'effect triggers ',
},
{
fn: arrayPushPopHookless,
name: 'array push/pop (hookless) ',
},
{
fn: arrayPushPopHooks,
name: 'array push/pop (hooks) ',
},
+ {
+ fn: arrayFillHookless,
+ name: 'array fill (hookless) ',
+ },
{
fn: manyDependencies,
name: 'many dependencies ',
},
{
fn: deriveChainDepth10,
name: 'derive chain depth 10 ',
},
{
fn: update100StatesIndividual,
Loading
Loading