auto-fix batch claude/friendly-maxwell-0olke (2026-04-29)#488
Merged
Conversation
intendednull
commented
Apr 29, 2026
Owner
Author
|
resolve conflicts |
Empty session-open commit per resolving-issues skill. https://claude.ai/code/session_01FHLLfYeh9P9FP7Y47wu9We
tests.rs grew to 4714 LOC, 117 #[test] fns, single file. Adding new test = scroll monolith. Split by concern matching #[path] pattern from crates/client/src/lib.rs:49-55: tests/dag.rs — 17 tests (insert, equivocation, topo sort, ManagedDag, pending buffer) tests/materialize.rs — 54 tests (channels, messages, profiles, mute, pin, ephemeral, idempotency) tests/permissions.rs — 31 tests (grant/revoke, check_permission, kick, member edits, set_permission round-trip) tests/stress.rs — 7 tests (1000 events, 100 authors, sort perf, governance scale) tests/sync.rs — 2 tests (joining peer + grant batches) tests/voting.rs — 6 tests (proposal/vote ordering, multi-admin kick majority) All 117 tests preserved verbatim — only relocated. Test count before/after: 227 -> 227 (cargo test -p willow-state). Sixth bucket (stress.rs) added since the issue's five suggested buckets don't cleanly absorb the scale tests; CLAUDE.md guidance says add a sixth file rather than force-fit. Common helpers (genesis_kind, test_dag, do_emit) duplicated per-file with #![allow(unused_imports, dead_code)] so each bucket compiles standalone — every bucket only references a subset, but factoring them into a shared helper module would have required a load-bearing #[path]-glued helpers.rs that the issue's mechanical-split scope didn't ask for. Refs #263 https://claude.ai/code/session_01FHLLfYeh9P9FP7Y47wu9We Co-authored-by: Claude <noreply@anthropic.com>
#487) Refs #271. Replace 10 `tokio::time::sleep(50ms)` "wait for actor" stalls in `crates/client/src/lib.rs` test module with deterministic synchronization. Per-call-site decision: - 6 sites where the read goes to the same StateActor that was just mutated: drop the sleep entirely. The actor mailbox is FIFO and every hop (`mutate`, `get`, `select`) is `ask`-based, so the mutation is visible to the next `ask` without any wall-clock wait. Sites: send_message_and_read_back, switch_server_updates_event_state (2x), generate_invite_grants_send_permission_to_recipient, mutate_channel_mute_emits_event_and_flips_stats, mutate_channel_mute_toggle_off_clears_set. - 4 sites where the read goes through a `DerivedActor` whose recompute is genuinely async (Notify -> spawn snapshot -> do_send(UpdateCache)): replace with a polling helper `await_view(probe)` that yields between `get().await` attempts under a 5s deadline. Faster on quick machines, reliable on slow ones. Sites: presence_self_override_round_trip, presence_reachable_peer_defaults_to_here, presence_queued_then_gone_after_threshold (2x). Why polling rather than subscribing to `Notify`: subscribing after the mutation is racy (Notify may have already fired) and a fresh `get()` forces the cache; tight polling with `yield_now` is simpler and bounded. Same strategy as the existing `wait_for_message`/`wait_until` helpers in `crates/client/src/tests/multi_peer_sync.rs`. Test count: 303 client tests pass before and after; presence tests run 3x in a row clean. Co-authored-by: Claude <noreply@anthropic.com>
Two lessons folded into resolving-issues from auto-fix batch claude/friendly-maxwell-0olke (2026-04-29): 1. Implementer step 2 (research) — re-grep cited line numbers / LOC counts at HEAD before working from issue-body literal positions. #263 cited 3,726 LOC (actual 4,714); #271 cited "outside #[cfg(test)]" but the sleeps had since shifted INSIDE a test module after a refactor. Drift is the norm. 2. New Implementer step 12 (structural-deps follow-up family path). Three structural-deps follow-ups now in a row (rand #246, getrandom #481, convert_case #485 this run). On the 3rd in this family, file/update an upstream-domino meta-tracker issue rather than another standalone TD-NN follow-up. Future runs check the meta-tracker; we stop refiling the same shape. Mechanical-moves promotion (in earlier rev of this branch) dropped per reviewer's pushback — risk it would skew the agent toward cosmetic moves over real fixes. https://claude.ai/code/session_01FHLLfYeh9P9FP7Y47wu9We
8b885bd to
a8581be
Compare
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
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.
2 small-scope fixes from open issue queue + 1 structural follow-up. Branch rebased onto current
main(which already has #484's skill rewrite). Force-pushed clean —mergeable_stateshould now be clean, full CI re-runs.Fixes
crates/state/src/tests.rsis 3,726 LOC — single-file monolith, 44% of the crate #263 — splitcrates/state/src/tests.rs(4,714 LOC monolith — issue cited 3,726, drift) into 6 per-concern files undercrates/state/src/tests/. Wired via#[path]matchingcrates/client/src/lib.rs:49-55. 117#[test]fns preserved verbatim; full state-crate suite 227 → 227 green. Largest bucket (materialize.rstests, 1,916 LOC) now smaller thanmaterialize.rssource (1,625 LOC). Sixth buckettests/stress.rsadded beyond the issue's 5 because the perf cases didn't fit the per-concern split cleanly. Tradeoff: each bucket carries duplicatedgenesis_kind/test_dag/do_emithelpers (~30 LOC each) under#![allow(unused_imports, dead_code)]; runner-up of factoring intotests/common.rsrejected as new abstraction beyond the mechanical-split scope.tokio::time::sleep(50ms)used to "wait for actor propagation" in ~10ClientHandlemethods #271 — droptokio::time::sleep(Duration::from_millis(50))"wait for actor propagation" patterns incrates/client/src/lib.rs. 10/10 sites converted: 6 × bucket-DROP (StateActor's FIFO mailbox already orders the next call —send_message_and_read_back,switch_server_updates_event_state×2,generate_invite_grants_send_permission_to_recipient,mutate_channel_mute_emits_event_and_flips_stats,mutate_channel_mute_toggle_off_clears_set); 4 × bucket-POLL (derived-actor lag —Notify→ spawn →UpdateCache—presence_self_override_round_trip,presence_reachable_peer_defaults_to_here,presence_queued_then_gone_after_threshold×2; newawait_view(probe)helper, 5s timeout, mirroringwait_for_message/wait_untilinmulti_peer_sync.rs); 0 × bucket-ASK (every site already usedstate::mutate(...).await, which isaskunder the hood — sleeps were cargo cult); 0 × bucket-WATCH. Stale-issue note: issue body claimed sleeps were "outside#[cfg(test)]"; they were all inside the#[cfg(test)] mod testsblock (lines 1255-2082) after a recent test-file refactor. Fix pattern still applies. Diff: 1 file, +63/-19.cargo test -p willow-client --lib303 passed before + after; presence tests ran 3× clean.Already-Fixed / Parked
[TD-02]
convert_caselinked at three incompatible versions (0.6, 0.7, 0.10) #251 [TD-02]convert_case3 versions — same shape as [TD-03]getrandomlinked in four versions (0.2 / 0.3 / 0.4 x2) — feature-unification risk for WASM #252/[TD-03 follow-up]getrandom3-way split is structural — driven by aes-gcm 0.10, ahash, and iroh 0.98 #481. Three majors (0.6.0, 0.7.1, 0.10.0) pinned by transitive crates we don't own:0.6.0←server_fn_macro 0.7.8+config 0.15.22(Leptos 0.7 internals)0.7.1←leptos_macro 0.7.9+reactive_stores_macro 0.1.8(Leptos 0.7 disagrees with itself)0.10.0←derive_more-impl 2.1.1(entire iroh stack: iroh, iroh-base, iroh-blobs, iroh-gossip, iroh-relay, iroh-tickets, n0-future, n0-watcher, netwatch, portmapper, noq, irpc)cargo update -p convert_caseis a no-op — all already at latest in their semver range. Workspace[patch]can't bridge^0.6↔^0.7↔^0.10without lying about semver, which would breakderive_more's actual API usage. Real fix requires three separate upstream events (Leptos 0.8 internal alignment, derive_more 3.x ecosystem catch-up). Filed follow-up [TD-02 follow-up]convert_case3-way split is structural — pinned by Leptos internals + derive_more #485 documenting the holdout chain. [TD-02]convert_caselinked at three incompatible versions (0.6, 0.7, 0.10) #251 closednot_planned(audit's "obvious fix" premise was structurally moot; not just a missed PR).This is the 3rd structural-deps follow-up in a row (rand [DEP-02] Three concurrent
randmajor versions + RUSTSEC-2026-0097 across all of them #246, getrandom [TD-03 follow-up]getrandom3-way split is structural — driven by aes-gcm 0.10, ahash, and iroh 0.98 #481, convert_case [TD-02 follow-up]convert_case3-way split is structural — pinned by Leptos internals + derive_more #485). Skill update below codifies an "upstream-domino meta-tracker" path so the next run files a single tracker instead of accumulating individual TD-NN follow-ups.Skill Evolution
Single skill commit on this branch:
a8581be— two lessons folded into the new SKILL.md structure landed via #484:crates/state/src/tests.rsis 3,726 LOC — single-file monolith, 44% of the crate #263 cited 3,726 LOC (actual 4,714); [GEN-12]tokio::time::sleep(50ms)used to "wait for actor propagation" in ~10ClientHandlemethods #271 cited "outside#[cfg(test)]" but the sleeps had since shifted INSIDE a test module. Drift is the norm.Earlier rev of this branch also tried to promote "mechanical splits / file moves" to a top pick in Core Loop §2; reviewer pushed back on the prioritization framing (would skew the agent toward cosmetic moves over real fixes), so it was dropped on rebase.
Lessons Learned
git fetch + reset --hard origin/) held clean across 3 dispatches.convert_caselinked at three incompatible versions (0.6, 0.7, 0.10) #251's "obvious fix — will be auto-PR'd" framing was structurally moot, just like [TD-03]getrandomlinked in four versions (0.2 / 0.3 / 0.4 x2) — feature-unification risk for WASM #252. The implementer correctly investigatedcargo treefirst instead of pushing a half-fix.Test plan
just check(fmt + clippy + test + wasm + browser + Playwright).just dev+ agent-browser navigation, focused on:crates/statematerialization still passes through the split test buckets in production code paths (no test-file regression risk for production, but the#[path]wiring inlib.rsis the only behavioural surface that could break).bucket-DROPandbucket-POLLconversions inlib.rstests didn't drift in client-handle semantics. Tests pass; smoke confirms the same flows still work end-to-end.Generated by Claude Code