diff --git a/docs/research/2026-07-24-audit-review-remediation-handoff.md b/docs/research/2026-07-24-audit-review-remediation-handoff.md new file mode 100644 index 00000000..553e512f --- /dev/null +++ b/docs/research/2026-07-24-audit-review-remediation-handoff.md @@ -0,0 +1,170 @@ +# 2026-07-24 — Runtime-audit adversarial review + remediation: HANDOFF + +**For:** the next agent/session picking up this work. +**State when written:** all remediation commits landed on local `main` +(`7b456b49` tip), full solution test suite green (Debug + Release build, +`dotnet test AcDream.slnx` exit 0), **user visual pass PASSED 2026-07-24** +for both render-affecting changes. Remotes deliberately not pushed. + +--- + +## 1. What happened today + +An independent adversarial review (43 verification agents against pinned +commit `e627d9a4`) audited both `docs/research/2026-07-24-runtime-performance-audit.md` +and `docs/plans/2026-07-24-modern-runtime-architecture.md`. Verdict: +**APPROVE WITH CHANGES for both** — the audit's code-level diagnosis was +almost entirely correct (12/12 spot-checked file:line citations accurate), +but its measurement provenance was not, and the plan inherited gates its +tooling could not measure plus one governance hole. The user then approved +executing the five headline recommendations, which are now done. + +## 2. Review findings that must not be re-litigated (all verified at file:line) + +**Corrections to the audit (now recorded as audit §13 errata — read it +before citing the audit's §1/§2/§4 numbers):** + +1. The capture ran **forced-uncapped** (`ACDREAM_UNCAPPED_RENDER=1` set + unconditionally by the soak script) with undisclosed + `ACDREAM_DUMP_MOVE_TRUTH=1`; "normal capped presentation" was false — + the 32 FPS ceiling was purely the RDP display path. Unresolved wrinkle: + the CPU trace's 6.44% `FramePacingController.CompleteFrame` should be a + no-op under those flags (launcher provenance is muddy). +2. The headline **203.6 MiB "single-frame allocation" is frame-thread-only** + (`GC.GetAllocatedBytesForCurrentThread`); worker-side `MeshExtractor` + decode structurally cannot be in it. The frame spike belongs to + main-thread publication/retirement (**Slice E owns it**); the pak + (**Slices B/C**) owns the process-wide 40.8 MB/s rate, Gen-2 pressure, + and 256 ms GC pauses. Win attribution must respect that split. +3. 65% LOH fragmentation (524 MB of 807 MB) sat unanalyzed in the audit's + own JSON — the best committed explanation of the committed-vs-live gap. +4. Exception figures: only the focused-trace 1,587 (1,335 AOOR + 252 + socket) are verifiable; the plan's old "4,520 + 1,168" had no committed + derivation (plan §1 row corrected). +5. gen0=99 < gen1=105 is legal (.NET 10 meter reports per-generation + EXCLUSIVE counts; ≈242 total GCs); "38 Gen-2" stands. +6. GiB conversions were MiB/1000 (true peaks: 1.62 GiB WS / 2.22 GiB + private); committed-heap peak was 1,242–1,298 MiB, not 830.6. + +**Pivotal confirmations (the audit was right):** the streaming +budget-bypass is real (`MaxCompletionsPerFrame=4` count budget; unloads + +teleport priority-ring apply uncapped; `BeginFullWindowRetirement` retires +~600 landblocks in one frame); `LandblockRetirementCoordinator.RunForEachEntity` +is unmetered; `LandblockStaticPresentationPublisher` rebuilds dicts/sets/sorts +per publication (plus a third dictionary + per-attempt re-sort the audit +missed); `ResolveActivation` probes every static id as Setup with a bare +catch and no negative cache anywhere; `InteriorEntityPartition.Partition` +walks all resident entities per frame with its AABB parameter unused +(nuance: far-tier blocks are entity-stripped, so the walk is bounded by +near-tier + live); `GpuFrameTimer` gpu_ms is boundary-to-boundary and +invalid under a cap; the `viewport-before-ready` barrier is real but +observational-only today (never gates reveal). + +**Cache reality (correction to the audit's framing):** four bounded LRU +tiers already front the mesh/texture path (GPU render-data 1 GiB/50 obj; +CPU mesh 128 MiB/100; decoded-texture 64 MiB/128; DAT typed-object +64 MiB/256 ×4) — revisits rebuild because they are UNDERSIZED vs the +route's working set (~599 textures), not because caching is absent. Real +fence-gated eviction exists for atlases/composites/arena — the route never +pushed the ceilings. **Slice D = unify accounting + retune, not rebuild.** + +**Lock contention (4,614) is unattributed** — no stacks were captured. +Leading candidates: `_datLock` (whole-landblock build vs render-thread +CreateObject hydration — a known 2026-06-23 campaign; portal windows are +exactly its worst case) and `ObjectMeshManager._pendingRequests`, not only +the DAT typed-object lock the audit named. + +## 3. What was executed (all on local `main`, in order) + +| Commit | Content | +|---|---| +| `38e14c6a` | **docs:** audit errata §13 + plan amendments — §0 governance (F/G user gate, side-track session discipline, freeze-exception scope for H-c/J, evidence corrections, baseline prerequisite blocking Slice C), Slice A/C/D/E/F/G/H/I/J/K contracts, §11 gate corrections, §12 adjudication protocol, §14 order incl. Slice I | +| `c2cb83d1` | **EnvCellRenderer prepare gate** — the dead `NeedsPrepare` wired into a real dirty gate (landblock changes, filter content, trim, new `ObjectMeshManager.RenderDataAvailabilityVersion`, camera = 1 mm ABSOLUTE eye epsilon + rows-1–3 relative 1e-5; a matrix-relative epsilon would mask sub-meter motion at AC's ~5e4 coords). Pool-cursor discipline verified safe. **USER-ACCEPTED (visual pass 2026-07-24).** | +| `cf253304` | **net:** receive-thread no longer dies on stray SocketException — `SIO_UDP_CONNRESET` suppression, per-iteration catch/continue+log, setsockopt hoist; deterministic loop-resilience tests | +| `a4b1214e` | **dead code:** ParticleBatcher/ParticleEmitterRenderer/ActiveParticleEmitter chain deleted (+ orphaned EmbeddedResourceReader + wb_particle shaders); production particle path untouched. **USER-ACCEPTED (visual pass).** | +| `60bc3139` | **MeshExtractor:** CellStruct clip-map clone guard (was unconditional), solid-color texture cache (clone-protect flag wired), redundant double `.ToList()` dropped (Portal-wins ordering untouched) | +| `1e9031e7` | **audio:** `DatSoundCache` 32 MiB LRU + negative set; OpenAL buffer 48 MiB budget with attachment-safe eviction; 19 tests | +| `bfc5e473` | **ISSUES #236–#244** — deferred P2 findings (see §5) | +| `7b456b49` | **measurement tooling:** `ACDREAM_FRAME_HISTORY=` per-frame CSV export (zero frame-thread I/O, written at Dispose); checkpoint JSON gains LOH size/fragmentation, process-total-allocated, and hit/miss/evict counters for the CPU-mesh/decoded-texture/DAT caches; soak script gains `-Uncapped` switch (**default is now CAPPED**), 26 s dwell fix, per-run `env-disclosure.json` | + +Memory updated: `feedback_agent_worktree_base` (new — see §6), +`project_mp_track_findings` (2026-07-24 update block; EnvCell rebuild gate +was its parked MP-Alloc item, now landed + accepted). + +## 4. Next actions, in order (the incoming agent's decision space) + +1. **Mint the baselines** (measurement, no code; plan §0.5 blocks Slice C on + these): one capped nine-stop soak (new script default) + one `-Uncapped` + soak; the AUTHORITATIVE uncapped-local run needs the user at the + physical display (not RDP). ACE was up on 127.0.0.1:9000. Command: + `.\tools\run-connected-r6-soak.ps1 -Repository (Get-Location).Path -SkipBuild [-Uncapped]` + with `ACDREAM_TEST_USER/PASS` set; artifacts land in the automation dir + + commit the JSON/CSV like the 2026-07-24 set. Set `ACDREAM_FRAME_HISTORY` + for the per-frame export. +2. **Slice A remainder** (small coding): pinned dense-town scene baseline + (pick Arwic or Fort Tethana, pin camera in a route file), + screenshot-diff tool + match rule, contention-stack capture + (`dotnet-trace` Contention keyword) on the route, a percentile script + over the per-frame CSV, the committed reference-config block. +3. **Then Slice B** (EnvCell geomId dedup + full pak bake) in dedicated MP + side-track sessions — first real plan slice. +4. **Main line always wins:** M4's six-slice world-interaction program is + the active milestone critical path; MP work is side-track-only (plan §0.2). +5. **Fillers (one commit each):** #236 UiText cache and #241 partition + frustum cull are the best value; #237 Transition pooling stays gated on + Slice I lifetime tests. +6. **Do NOT start Slices F/G** without the user's recorded go-ahead + (plan §0.1 — they resume the deferred 2026-07-05 MP3/ECS decision; the + prior "GPU-bound ~200fps" evidence is invalidated by the GpuFrameTimer + artifact, so the question stays open until the corrected uncapped-local + profile exists post-Slice-E). + +## 5. Deferred, filed, and known-not-done + +- **ISSUES #236–#244:** UiText per-frame re-wrap; Transition per-resolve + allocation graph; EquippedChildRenderController double-tick + ToArray + snapshots; inbound per-packet allocation chain; RetailAnimationLoader + unbounded cache; partition landblock frustum cull; publisher third + dictionary/per-attempt re-sort; `_boundsCache` bound; SequencerFactory + unguarded Setup probe + `[dat-miss]` under-lock logging. +- **Remotes:** local `main` is 30+ commits ahead of `origin` and `github` + (both at `20e2998a`). Deliberately not pushed — prior convention says + local main is the truth; push with `git push origin main github main` + (or per-remote) when the user wants the stack published. +- **No baseline soak was run yet** (held to avoid colliding with the + user's visual-pass session). The tooling is validated by contract tests + + a clean PS1 parse only. +- The audit's raw artifacts remain uncommitted (by design); several §4 + numbers stay reproducible-not-reviewed until the Slice A recapture. + +## 6. Gotchas for the incoming agent + +- **Worktree-agent base skew:** Agent-tool worktrees in this repo were + created from the REMOTE tip (`20e2998a`), ~90 commits behind local main. + Always have agents report their worktree HEAD, run + `git diff --name-only -- ` before + cherry-picking, and re-run the full suite on the target tree. (Memory: + `feedback_agent_worktree_base`.) +- The four agent worktrees from this session + (`.claude/worktrees/agent-*`) were removed after their commits were + cherry-picked; `vigorous-joliot-f0c3ad` was detached from `main` so the + primary tree could check it out — it still exists, detached at `e627d9a4`. +- The soak script's route gating GREPS `move-truth OUT` lines — + `ACDREAM_DUMP_MOVE_TRUTH` must stay enabled for soak runs (now disclosed + via `env-disclosure.json`, not hidden). +- Plan §14's "landed post-review" notes mark work done today — do not + redo it when executing slices. + +## 7. Pointers + +- Audit + errata: `docs/research/2026-07-24-runtime-performance-audit.md` (§13) +- Amended plan: `docs/plans/2026-07-24-modern-runtime-architecture.md` (§0 first) +- Issues: `docs/ISSUES.md` #236–#244 (+ current queue block) +- Memory: `claude-memory/project_mp_track_findings.md` (2026-07-24 update), + `claude-memory/feedback_agent_worktree_base.md`, + `claude-memory/project_render_pipeline_digest.md` (unchanged, still the + render SSOT) +- Full review structure (A–G: verdict, findings, benchmark/architecture + assessments, revised execution order, acceptance matrix) was delivered + in-session 2026-07-24; its actionable content is fully folded into the + errata, the plan amendments, the issues above, and this handoff.