# AD-10 retirement-by-deletion — architecture review **Reviewer scope:** completeness, blast radius, test quality. Retail fidelity is a separate reviewer's report (`2026-08-06-ad10-review-retail.md`). **Range:** `ef976c6d..2223ed17` on `claude/acdream-physics-divergence-5aa784` (`fe6ee877`, `886333a2`, `fb454b74`, `2223ed17`). **Date:** 2026-08-06. ## Verdict: **PASS**, with one process defect and one under-scoped issue The Stage 0 measurement is **sound and I believe it** — I re-derived both reported numbers independently and added four cases the implementer did not report. The deletion is complete, the blast radius claim is correct, all three reported sabotages reproduce with matching numerals, and the two deleted tests were genuinely obsolete. Nothing here is a workaround, a suppression flag, a grace period, a symptom guard, a new skip, or a weakened test. No known flake was conflated. The defects are: one **mandatory contract test silently dropped** (D1), a **stale roadmap entry** (D2), and a **malformed doc comment** (D3). None justifies blocking the change; D1 should be settled before the visual gate is called done. --- ## Part 1 — Is the Stage 0 measurement sound? (priority 1) ### Method I did not take the implementer's word for it. I deleted all 44 `bin`/`obj` directories, built Release clean (0 errors), byte-scanned `AcDream.Core.dll` to confirm the string `SampleTerrainNormal` is absent from the compiled metadata (0 occurrences; `SampleTerrainWalkable` still present at 2, proving the scan works), then drove `RemoteRampHarness` through the production `RuntimeRemotePhysicsUpdater.Tick` for 30 ticks and dumped every position as **raw IEEE-754 bits**. I then restored the pre-deletion production files verbatim (`git checkout ef976c6d -- ` the three `src/` files — those files changed only in `886333a2`, so this is an exact revert) and re-ran the identical dump. ### Result — reproduced, and extended | Case | gradient | root motion / tick | pre-deletion vs HEAD | |---|---|---|---| | downhill 31° | 0.6 | (0, +0.10, 0) | **bit-identical, all 30 ticks** | | downhill 8.4° | 0.1477 | (0, +0.10, 0) | X, Y bit-identical; Z differs by **2.9e-5 m** at tick 30 | | **cross-slope 31°** | 0.6 | (+0.10, 0, 0) | **bit-identical** | | **diagonal 31°** | 0.6 | (+0.0707, +0.0707, 0) | **bit-identical** | | **flat** | 0.0 | (0, +0.10, 0) | **bit-identical** | | uphill 31° | 0.6 | (0, −0.10, 0) | bit-identical — **but vacuous, see L1** | Both reported numbers land exactly. The last three rows are mine; they close the obvious "only one direction was measured" objection. ### Is "bit-identical" vacuous — did neither path do anything? **No, for the downhill case.** Over 30 ticks the body advances Y by 2.206 m and descends 1.324 m. Those are not noise: they are exactly the analytically predicted projected step. For `N = (0, 0.5145, 0.8575)` and a requested `(0, 0.1, 0)`, the projection `v − N·(v·N)` gives `(0, 0.07353, −0.04412)`; the measured per-tick advance is `0.07353 / −0.04412`. The sweep alone reproduces the deleted projection to the last digit. The idempotency argument the commit message gives also holds in the code, not just on paper: `Transition.AdjustOffset` (`src/AcDream.Core/Physics/TransitionTypes.cs:5246-5258`) subtracts the full normal component in **both** its `collisionAngle <= 0` and `> 0` arms, so a vector already on the plane is returned unchanged. That is why the composition collapses. **Yes, for the uphill case** — and the implementer said so and refused to ship the test. That refusal was correct (see L1). ### Does the fixture make it vacuous? The concern is real and worth naming: the ramp is one constant-gradient plane over the whole landblock, so `SampleTerrainNormal(x, y)` **equals** `Body.ContactPlane.Normal` everywhere. That is precisely the case where idempotency guarantees no change — the friendliest possible geometry for the "redundant" claim. But it is also the **right** geometry for the question actually being asked. Deletion can only regress where the two planes agree (there the old layer was doing correct work that must now come from somewhere else); where they disagree the old layer was applying a *wrong* plane, so removing it cannot be a regression. The measurement covers the regression-capable case exactly, and my cross-slope/diagonal/flat additions cover it in three more directions. **Verdict on Stage 0: I believe it.** The redundancy claim is established for terrain. What is *not* established is the claimed improvement — see D1. --- ## Part 2 — Findings ### Defects #### D1 — a mandatory contract test was dropped with no record (process) `docs/research/2026-08-06-ad10-contract.md:546-577` specifies **T2**, "the discriminating test", on a fixture where the committed contact plane differs from the terrain sample at the same XY. Construction (a), a two-gradient terrain crest, is marked **"Mandatory."** Construction (b), an off-terrain walkable surface, is **"Required, with a documented fallback: if the fixture cannot be built in reasonable time, say so explicitly in the closeout, ship on (a), and record (b) as an untested axis rather than silently dropping it."** Line 690 of the same contract puts T2–T5 in the deletion commit. Neither exists. `grep -rn -i "crest|ridge|two-gradient" tests/AcDream.Runtime.Tests/` returns nothing, and no commit message, register row, or doc in the range records the disposition of T2. T3, T4 and T5 are defensibly moot under deletion (T3 and T4 test guards that no longer exist; T5's intent is superseded by the stronger compile-error guarantee). **T2 is not moot.** Under deletion it becomes the test for the one behavioural *benefit* the change claims — and that claim is stated as fact in two places without evidence: - `docs/architecture/retail-divergence-register.md:122` (the retired AD-10 row); - `886333a2` commit message: *"What deletion does improve is the case #32 never covered: a remote on a WALKABLE non-terrain surface … That surface now gets the body's own committed contact plane."* **Concrete failure scenario.** A remote runs along a sloped wooden bridge or a dock over sloped terrain. After deletion the *only* projection is the sweep's, which runs `only when ci.ContactPlaneValid` (`TransitionTypes.cs:5202-5224` takes the no-contact-plane branch otherwise). If `check_contact` ever fails to seed a contact plane on a tick where the body is `OnWalkable`, the offset is not projected at all, where before the terrain sample supplied one. The body holds Z between server updates — the exact ~5 Hz staircase the projection existed to remove — and **nothing in CI goes red**, because every automated assertion about this behaviour lives on a pure-terrain fixture. I judge the residual risk *low* (on flat terrain under a bridge the deleted projection was a near-no-op anyway), but "low risk" is a different statement from "measured", and the contract explicitly forbade making it silently. **Ask:** either build T2(a) — the harness already supports it, it is a two-gradient heightmap in `RemoteRampHarness.Ramp` — or add one paragraph to the register row recording T2 as a deliberately untested axis, per the contract's own fallback clause. #### D2 — stale roadmap entry (docs) `docs/plans/2026-04-11-roadmap.md:108` still lists > `AP-22 authored object shapes, and AD-10 remote contact-plane projection.` as open campaign work. AD-10 is retired. CLAUDE.md's roadmap discipline rule 3 requires the roadmap update in the same commit as, or immediately after, the work. The register row, both downstream `docs/ISSUES.md` cross-references (`:10513-10515`, `:10672-10676`) and the historical record were all swept correctly — the roadmap was the one miss. #### D3 — malformed XML doc comment (style) `src/AcDream.Core/Physics/RemoteMotionCombiner.cs:38-53`. The AD-10 retirement rationale is written as a `` block placed *after* the closing `` on line 38, and terminated by a second `` on line 52: ``` 38 /// 39 /// 40 /// AD-10, retired 2026-08-06. … 52 /// …unchanged. 53 /// <- unmatched close tag ``` Two closes, one open, one orphaned ``. The build is green only because this project does not generate documentation files; the rationale will not render in IntelliSense, and the file would emit CS1570 the moment `GenerateDocumentationFile` is turned on. Same paragraph reads fine — just move it inside the first ``. ### Latent risks #### L1 — #331 is real, is under-scoped, and one cheap probe moves it a long way The issue as filed (`docs/ISSUES.md:75-150`) is honest and unusually thorough: it rules out gradient, step size, cell boundaries, Z seating, and axis, and it names the vacuous test that found it. I reproduced every one of those exclusions. But the "fixture-versus-production" question it leaves open is answerable more cheaply than it says, and the answer points away from the lead the issue names. **What I measured**, calling `PhysicsEngine.ResolveWithTransition` *directly* on the harness's engine (no remote tick involved): | call | result | |---|---| | uphill, `body: rm.Body`, `isOnGround: true` | `ok=False`, moved `(0,0,0)` | | uphill, `body: rm.Body`, `isOnGround: false` | `ok=False`, moved `(0,0,0)` | | **uphill, `body: null`** | **`ok=True`, moved `(0, −0.0999, +0.060)`** | | uphill, `body: null`, lifted 0.5 m | `ok=True`, moved `(0, −0.0999, 0)` | | uphill, with `IsPlayer\|EdgeSlide` + the human two-sphere Setup list | `ok=False` | | uphill diagonal `(0.1, −0.1, 0)`, `body:` supplied | `ok=True`, moved **`(0.1, 0, 0)`** | | downhill / cross-slope / straight-up / straight-down, `body:` supplied | `ok=True`, all correct | Three things follow that the issue does not yet carry: 1. **The discriminator is the `body:` parameter**, i.e. the seeded contact-plane / retained-walkable-polygon path (retail `check_contact`). Without a body the same uphill sweep climbs; with one it refuses. Production *always* passes a body — the local player at `src/AcDream.Runtime/Gameplay/PlayerMovementController.cs:2646`, the remote at `src/AcDream.Runtime/Physics/RuntimeRemotePhysicsUpdater.cs:424`. 2. **It is not remote-specific and not sphere-list-specific.** It reproduces under a call profile identical to the local player's, including `IsPlayer | EdgeSlide` and the human Setup's two-sphere list. The issue's stated "first thing to check" — the terrain publication path (`AddLandblock` vs `LandblockPhysicsContentBuilder.PublishStaticCollision`) — is now the *less* likely lead of the two. 3. **The up-slope component is clipped, not the whole step.** The diagonal case returns `ok=True` and keeps its cross-slope X while zeroing its up-slope Y. A walkable 31° ramp is behaving like a wall for up-slope motion, and a 1.1° ramp (gradient 0.02) does the same. **Nothing in the repository proves walkable uphill progress works anywhere.** `RetailEdgeResponseOrderingTests.cs:277-312` runs a downhill / uphill / tangential theory and asserts horizontal progress for *downhill and tangential only* — the `uphill` arm is routed to a no-bounce assertion instead. That is correct for its unwalkable steep-roof fixture, but it means the suite has no uphill-progress assertion to lean on. **This is not a regression from AD-10** — I confirmed the uphill trajectory is bit-identical before and after the deletion, so nothing here blocks this change. But #331's severity should not stay UNKNOWN: the trigger is now known to be a production-shaped call, and the remaining unknown is narrow enough to settle in one probe. #### L2 — the surviving tests cannot see the projection, only the Z seating Verified, and quantified. With `Transition.AdjustOffset` short-circuited to `return offset;`: - `RuntimeRemoteSlopeProjectionTests` — **all green**, confirming the implementer's rejection of the contract's proposed T1 sabotage; - the downhill Y advance goes from **0.0735 m/tick to 0.1000 m/tick**, i.e. the remote runs **36% faster downhill**, and the tests do not notice; - Z still tracks the surface exactly, because `ValidateWalkable`'s push-out (`TransitionTypes.cs:3445-3452`, reached from `FindEnvCollisions`) re-seats the sphere at its natural resting distance every sub-step. The implementer's stated third mechanism is real, both structurally and empirically. This is disclosed, at length and unprompted, in the test's own doc comment (`RuntimeRemoteSlopeProjectionTests.cs:97-113`), including the instruction that it "must not be cited as" a unit test of `adjust_offset`. That is the correct handling of a non-discriminating test. It is also not a coverage hole overall: running the full suite under that same sabotage produces **15 failures** — 14 in `AcDream.Core.Tests` plus `RuntimeRemoteSteepContactSlideTests.SteepContactKeepsTheBodySlidingDownhill` ("expected a slide, body moved 0.0000 m"), which is the Runtime remote path's own guard. No action required; recorded so nobody later mistakes the new file for `adjust_offset` coverage. #### L3 — "bit-identical" is a property of this fixture, not of the change The 8.4° ramp differs by 2.9e-5 m in Z because `dot(v', N)` is not exactly zero in float after the first projection. The 31° case lands bit-identical; a third gradient might not. The commit message already reports both numbers and calls the difference float ordering noise, which is the honest framing. Nobody should generalise "bit-identical" into a guarantee. --- ## Part 3 — What I checked and found clean (so the PASS is auditable) **Deletion completeness (priority 2).** Three independent sweeps: `SampleTerrainNormal` (zero code references — only comments and docs), `terrainNormal` case-insensitive (same), and the formula shape `* Vector3.Dot(` / `-= N *` across all of `src/` (only an unrelated `Vfx/ParticleSystem.cs:1245` axis projection). No fourth copy, no dead diagnostic, no test reference. Both fork branches carried the block verbatim pre-deletion (`:272-282` and `:317-325`) and both are gone — the AP-22 shape was handled. `SampleTerrainWalkable`, the internal the deleted wrapper called, is **not** orphaned: it retains live callers at `TransitionTypes.cs:2909` and `:3432`. `ComputeOffset` confirmed production-dead — the only callers are in `tests/AcDream.Core.Tests/`. Removing the parameter rather than passing null does make a one-site reintroduction a compile error, as claimed. **Sabotage reproduction (priority 3).** All three reproduce, from clean builds: | sabotage | reported | reproduced | |---|---|---| | `rm.Body.Position = postIntegratePos` (`:457`) | RED at tick 1, 0.05999 m off surface | **RED at tick 1, "body root sits 0.05999 m above the terrain under it"** | | flatten ramp to gradient 0 | RED on the anti-vacuity guard, `dz = 0.0000` | **RED, "fixture is not exercising slope descent: dz = 0.0000 m"** | | short-circuit `Transition.AdjustOffset` | GREEN | **GREEN** (see L2) | The anti-vacuity guard is real and load-bearing: note that the *other* fixture test, `TheFixtureRampIsWalkableAndItsPlaneIsTheGeometricOne`, passes happily on a flattened ramp (`RampNormal(0)` is `(0,0,1)` and so is the flat contact plane), so the guard is the only thing standing between this file and a silently-flattened fixture. **Deleted tests (priority 4).** Both were genuinely obsolete. `..._SlopedTerrainNormal_ProjectsZOntoSlope` asserted only the deleted formula, against expected values re-derived from that same formula in a comment — it could catch a wrong multiply but never a wrong plane, exactly as the commit message says. `..._FlatTerrainNormal_NoZChange` is a *literal duplicate* of the surviving `ComputeOffset_AnimationOnly_Forward_BodyAdvances` (`RemoteMotionCombinerTests.cs:51-68`): same `dt: 0.1`, same `rootMotionLocalDelta: (0, 0.4, 0)`, same identity orientation, same asserted `(0, 0.4, 0)`. Nothing was asserted by either test that is not still asserted. The tombstone comment left in their place names both and points at the replacement. **Harness extraction (`fe6ee877`).** Filtering the diff of `RuntimeRemoteSteepContactSlideTests.cs` to `Assert` / `[Fact]` / `[Theory]` lines yields **zero** added or removed assertions — the extraction moved the private nested `Harness` out verbatim and nothing else. Its start position constants are unchanged (96, 96). All ten Bug B tests still pass. **Blast radius (priority 6).** Confirmed exactly as stated. `new RuntimeRemotePhysicsUpdater(` has one production site, `src/AcDream.App/Physics/RemotePhysicsUpdater.cs:46`; every other match is a test fixture. `src/AcDream.Headless/` contains no reference to `RuntimeRemotePhysicsUpdater`, `RemotePhysicsUpdater`, `RemoteMotion` or `RemoteMotionCombiner`. The class is `internal` to `AcDream.Runtime` and reaches production only via `InternalsVisibleTo` into `AcDream.App` (`AcDream.Runtime.csproj:12`). `RemoteMotionCombiner` lives in `AcDream.Core` and is instantiated in `RemoteMotion.cs:242`, but the only things that *drive* it are the three `ComposeOffset` sites in `RuntimeRemotePhysicsUpdater`. The inverse of C5b's lesson does apply here and the implementer got it right: assembly placement is not reachability, and a green headless gate would have been vacuous evidence. #332 records that reasoning correctly. **`TickHidden` (priority 7).** Correct to leave untouched. I byte-compared the call at `RuntimeRemotePhysicsUpdater.cs:940-948` against `ef976c6d`: it is character-for-character identical, and it used the named argument `inContact:` while skipping the optional `terrainNormal`, so the hidden-remote path always received `null`. Deleting the parameter is a no-op there by construction. **Gates.** From a state with all 44 `bin`/`obj` directories deleted: Release build **0 errors**; full solution suite **11,196 passed / 4 skipped / 0 failed**, matching the claim exactly and reconciling with the stated `11,195 / 4 / 0` baseline as +3 new Runtime `[Fact]`s and −2 deleted Core tests. Run twice — once before my sabotage experiments and once after restoring the tree — with identical totals. Skip count unchanged at 4 (3 App, 1 Core); no skip was added. **Tree state.** `git status --porcelain` and `git diff HEAD` are clean. The one untracked file, `docs/research/2026-08-06-ad10-review-retail.md`, belongs to the concurrent retail-fidelity reviewer and was not touched. Every temporary sabotage and probe file I introduced was reverted or deleted and verified gone. **Rule compliance.** No workaround, no suppression flag, no grace period, no `if (badState) return`, no swallowed exception, no widened tolerance, no new skip. The 5 mm tolerance in the new tracking test is justified in its own doc comment against both the failure it must catch (~1.8 m) and the noise it must tolerate (<1e-4 m), and I confirmed the ratio by measurement. Known flakes #302 / #308 / #321 were not touched or conflated. AD-65 and AD-66 were filed as separate register rows rather than folded into this change, which is the right call — both change local-player feel and need their own gate.