acdream/docs/research/2026-07-24-audit-review-remediation-handoff.md
2026-07-24 12:19:24 +02:00

173 lines
11 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# 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,2421,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-13 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=<path>` 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 <agentBase> <target> -- <touched files>` 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 were unregistered from git and
their branches deleted after cherry-picking. Three orphaned DIRECTORIES
(`.claude/worktrees/agent-a5cccad7a993e2db7`, `agent-a4124a11d80d1805b`,
`agent-a5670375d0717edf5`) remained file-locked at handoff time plain
disk litter, delete whenever the locks clear. `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 (AG: 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.