From b888b86729adba4c86bf60630c8cbd2d7040fd6c Mon Sep 17 00:00:00 2001 From: Fr0zka Date: Tue, 28 Jul 2026 02:58:33 +0200 Subject: [PATCH] docs: stage A verified with real coverage; rewrite the handoff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The run that matters: bit-identical over 6000 samples with 23.9% of them in open cave, and a bake carrying 49 rooms / 56 pits / 28 chimneys / 0 columns — so the pit and chimney loops finally ran on real data, and 0 columns confirms STEP 4d stayed dormant as stage A requires. Same code and same colour as the previous green run, three different strengths of evidence. That is the argument for printing coverage numbers rather than pass/fail. Handoff rewritten for a fresh context: the three-stage TunnelNetwork plan and why stage A is verifiable while incomplete, the calls-not-transcribes rule for BuildChunkCache, FRAME ops recorded as retired (0 of 3 candidates needed one), the per-room override reclassified as load-bearing rather than polish, and the method lessons regrouped around the three coverage traps. Co-Authored-By: Claude Opus 5 --- OPSTACK-HANDOFF.md | 162 ++++++++++++++++++++++++++++++-------------- OPSTACK-PROGRESS.md | 33 +++++++++ 2 files changed, 143 insertions(+), 52 deletions(-) diff --git a/OPSTACK-HANDOFF.md b/OPSTACK-HANDOFF.md index 5b28666..e59005e 100644 --- a/OPSTACK-HANDOFF.md +++ b/OPSTACK-HANDOFF.md @@ -1,4 +1,4 @@ -# Handoff — VoxelForge operator stack, 2026-07-27 (end of day 2) +# Handoff — VoxelForge operator stack, 2026-07-28 (end of day 3) > Paste the block below into a fresh session. Everything it refers to is on disk and in git. @@ -16,60 +16,99 @@ don't re-derive them. 2. **`OPSTACK-PROGRESS.md` — THE LAST ENTRY FIRST.** Append-only log of what is built vs merely written. This is the resume point. 3. **`OPSTACK-PLAN.md`** — the plan. **§2.6.1 is the current acceptance bar** and supersedes §2.6. -4. **`OPSTACK-DECOMPOSITION.md`** — all 8 archetypes broken into ops. §5 SurfaceWorld, §6 shafts, - §7 islands, §2 TunnelNetwork. -5. **`AUDIT-2026-07.md`** — §C9 is the top open risk (half fixed). **§C10 is SOLVED — don't reopen.** +4. **`OPSTACK-DECOMPOSITION.md`** — all 8 archetypes broken into ops. §2 TunnelNetwork (the one in + progress), §0.2 the worm bound, §7 islands, §5 SurfaceWorld. +5. **`AUDIT-2026-07.md`** — §C9's library half is the top open risk. **§C10 is SOLVED — don't + reopen.** §C1 was reopened and re-closed on 2026-07-28; §C2 has a new *suspected* item worth + verifying before acting on. 6. **`CODEMAP.md`** — navigation. Trust symbol names over line numbers. -## Where things stand — 5 of 8 archetypes ported, 11 tests green +## Where things stand — 6 of 8 ported and wired, the 7th half-ported, 13 tests green | Archetype | State | |---|---| | `Maze` | ✅ ported, bit-identical, wired | | `FlatPlain` + `CrystalChamber` | ✅ **one op for both**, bit-identical, wired | | `SurfaceWorld` | ✅ ported incl. **biome blending**, bit-identical, wired | -| `VerticalShafts` | ✅ ported, bit-identical, wired — **3 ops reused from Maze unchanged** | -| `FloatingIslands` | ❌ next (§7) | -| `Underwater` | ❌ (§8 — TunnelNetwork + a water flag) | -| `TunnelNetwork` | ❌ **LAST**, deliberately — owns `BuildChunkCache`'s two-region window-invariance discipline (§8.4), the most delicate code in the plugin | +| `VerticalShafts` | ✅ ported, bit-identical, wired — 3 ops reused from Maze unchanged | +| `FloatingIslands` | ✅ ported, bit-identical, wired — the stack that runs **backwards** | +| `TunnelNetwork` | 🔶 **STAGE A of 3 done and verified.** Not wired — see below | +| `Underwater` | ❌ TunnelNetwork + a water flag; folds in at stage C | Everything is behind `UVoxelStrateDefinition::bUseOperatorStack`; the ported list lives **only** in `UVoxelStrateManager::UsesOperatorStackForChunk`. Un-ported archetypes ignore the flag, so ticking it anywhere is harmless. -## Two things Phase 2 invented that were not in the original design +## TunnelNetwork is staged — read this before touching it -1. **Height space** (`VoxelHeightOp.h`) — a *second operator family*. SurfaceWorld's terrain ops - (cliff/terrace/layer-lines/beach) read and write an **altitude**, not a density: no input Z, - XY-pure per column, neither density nor SDF touched. They do not fit `IVoxelDensityOp`. §0.1 found - density needed a second *channel*; this found terrain needs a second **space**. Bonus: a height - stack *cannot* hold Z-dependent data because there is no Z in the signature — `AUDIT §6.3`'s - hazard became a type error instead of a convention. +`GetDensityWithParams` is ~1080 lines: 13 detail modifiers, a two-region cache, worms, and a per-room +op override. Porting it whole before anything can be checked would be ~600 unverified lines on top of +~200 — the `AUDIT §P3` pattern this refactor has dodged seven times. So: + +- **Stage A — DONE, bit-identical over 6000 samples.** Vertical scale · base rock · cave warp · room + graph (+ pits + chimneys) · carve · worms · structural post. **7 ops.** +- **Stage B — NEXT.** The 13 detail modifiers of `STEP 4b–4h`, gated on `Sdf < SDFBlendRadius·3`. + They insert between the carve and the worms, so the op-count assertion in the test must move. +- **Stage C.** The per-room op override (`§2`'s option (a)), the `Underwater` water flag, and only + then does `UsesOperatorStackForChunk` return true for either. + +**Why stage A is verifiable while incomplete:** every detail modifier is amplitude-gated, and +`FStrateGenerationParams` already defaults all of them to zero. Zeroing `SurfaceRoughness` (the one +exception) sends the *original* down exactly the path stage A ported. Stage B's test will do the +reverse — turn them on one group at a time. + +**⚠️ The decision that must not be undone:** `FRoomGraphSource` **CALLS** `BuildChunkCache` / +`EvaluateSDFCached`; it does not transcribe them. That is where `ARCHITECTURE §8.4`'s two-region +window-invariance discipline lives, and a transcription would *fork* it — with the fork "validated" +by a test that compares it to the original. Only the ~60 lines of glue are transcribed. + +## Three things Phase 2 settled that were not in the original design + +1. **Height space** (`VoxelHeightOp.h`) — a *second operator family*. SurfaceWorld's terrain ops read + and write an **altitude**, not a density: no input Z, XY-pure per column. They do not fit + `IVoxelDensityOp`. §0.1 found density needed a second *channel*; this found terrain needs a second + **space**. Bonus: a height stack *cannot* hold Z-dependent data, because there is no Z in the + signature — `AUDIT §6.3`'s hazard became a type error instead of a convention. 2. **`IVoxelBiomeField`** — ops depend on a *capability*, never on `UVoxelGenerator`. The adapter (`FGeneratorBiomeField`) lives in `VoxelGenerator.cpp`. This is what lets ops become assets in Phase 3; an op holding a generator pointer never could. +3. **`FRAME` ops are RETIRED — porting all three candidates killed the idea.** `CaveWarp`'s scope is + exactly one operator (pits/chimneys explicitly read *unwarped* coords, the thing §2 called the + fiddliest in the decomposition — inside one op it evaporates). `VerticalScale` is `Z / Scale`, a + one-line pure function. The island warp was already local. **Zero frames from three candidates:** + not missing infrastructure, one idea seen three times from a distance. Marked retired in §1. ## What is left, in the order I'd do it -1. **`FloatingIslands`** (§7) — next port. -2. **`Underwater`** (§8) — TunnelNetwork + a flag; fold in once TunnelNetwork is done. -3. **`TunnelNetwork`** (§2) — last, and take §8.4's window-invariance discipline seriously. -4. **PERF — deliberately parked by Jahni until the transition is complete.** Generation is measurably - slower on the op path. One cause found and fixed (the column memo was discarding itself every - chunk). Remaining suspects in order: the hashed column lookup vs. `GSurfColCache`'s direct-indexed - box, then per-voxel virtual dispatch. **Measure before optimising** — that is the C10 lesson. -5. **`AUDIT §C9` library half** — `FPSemantics = Precise` fixed the compiler half; `sinf`/`cosf` are - not IEEE-754 specified, so MSVC's CRT and glibc's libm can still differ. Currently **0 samples - within 1e-6 of the isosurface**, i.e. no measured risk. If it ever must be zero *by construction*, - the fix is a deterministic in-house sin/cos, not a build flag. Also: run - `CrossPlatformDigest` on Linux and compare the SHAPE digest, then pin it. -6. **VerticalShafts proves 0 of 60 tiles** (Maze 23, slabs 36-40). Pessimistic, not wrong: its - `EffectOverBox` returns `CarveOnly` whenever any shaft is within a `Spacing*1.6` halo instead of - testing real connector capsules. Lost CPU, never a hole. -7. **`ClassifyTile` still uses hand-written guards.** `ClassifyBox` exists and is brute-force verified - per archetype, but **nothing consumes it in production yet**. That is where the measured - tile-skipping (23 / 36 / 40 of 60, against today's zero) actually turns into frames — arguably the - biggest single win still on the table. +1. **Stage B** — the 13 detail modifiers. The bulk of the remaining lines. +2. **Stage C** — per-room op override + `Underwater` + flip both on in `UsesOperatorStackForChunk`. + **The override is NOT optional polish:** pits, chimneys and columns exist *only* through a + per-room `UVoxelTerrainOpDefinition` (`BuildChunkCache` opens its bake with + `if (!CR.RoomOp) continue;` and then reads a *fresh* param struct). No override ⇒ no pits, ever. +3. **The worm amplitude cap (`DECOMPOSITION §0.2`)** — the largest single perf item in the plan. + TunnelNetwork proves **0 of 40** tiles today because a fielded-noise carve has no spatial bound. + But its amplitude is bounded and trivial (`t ∈ [0,1]`, `Mask ∈ [0,1]` ⇒ at most `WormStrength` + toward air), and that bound already has a home in `FWormFieldSource::MaxCarveAmplitude()`. It + needs a fold that carries **numbers**, not just directions. +4. **`ClassifyTile` still uses hand-written guards.** `ClassifyBox` is brute-force verified per + archetype but **nothing consumes it in production**. That is where measured tile-skipping turns + into frames — arguably the biggest single win still on the table. +5. **PERF — parked by Jahni until the transition is complete.** The op path is measurably slower. + One cause found and fixed (the column memo discarded itself every chunk). Remaining suspects in + order: the hashed column lookup vs `GSurfColCache`'s direct-indexed box, then per-voxel virtual + dispatch. **Measure before optimising** — that is the C10 lesson. +6. **`AUDIT §C9` library half** — `sinf`/`cosf` are not IEEE-754 specified, so MSVC's CRT and glibc's + libm can differ. Currently **0 samples within 1e-6 of the isosurface**, i.e. no measured risk. The + real fix, if ever needed, is a deterministic in-house sin/cos. Also: run `CrossPlatformDigest` on + Linux, compare the SHAPE digest, pin it. +7. **`AUDIT §C2`, suspected** — `GetGenerationParams` blends params across Gradient transitions, so + two chunk Zs inside one strate may hold different params with the same XY box, strate index and + seed ⇒ the SDF cache never rebuilds ⇒ the lower chunk gets the upper chunk's rooms. **Verify the + premise before acting** (does Gradient blending actually vary within a strate?). The op stack does + not inherit it — its key folds in a params CRC. +8. **VerticalShafts proves 0 of 60 tiles.** Pessimistic, not wrong: `EffectOverBox` returns + `CarveOnly` whenever any shaft is within a `Spacing*1.6` halo instead of testing real connector + capsules. Lost CPU, never a hole. ## Hard rules that prevent real bugs @@ -79,28 +118,47 @@ anywhere is harmless. - **The acceptance bar is `§2.6.1`:** *same seed ⇒ same world on every peer*. Resemblance to the pre-refactor world is **not** required. The equivalence tests are **port-correctness oracles**, not fidelity checks — keep them for that reason. -- **Every cache key includes `LayoutVersion` AND the params.** See the §C2 note and the overhang - regression of 2026-07-27, where omitting the params silently deleted the overhang and only 1 sample - in 20 000 crossed the isosurface. +- **Every cache key includes `LayoutVersion` AND the params.** See `§C2` and the overhang regression + of 2026-07-27, where omitting the params silently deleted the overhang and only 1 sample in 20 000 + crossed the isosurface. - `ProcessQueue` stays `EQueueMode::Mpsc`; `Epoch` carries through every async path; don't "optimize" the `ARCHITECTURE §8.10` invariants. - Commit per coherent unit with a real message. **Never push.** `main` is the known-good fallback. - Update `CODEMAP §3`, `ARCHITECTURE §8`, tick `OPSTACK-PLAN`, append to `OPSTACK-PROGRESS.md`. - -## Method lessons this refactor actually paid for - -- **Instrument before hypothesising.** §C10 cost six builds and five refuted hypotheses, then was - solved for free by a build setting changed for an unrelated reason. Park a question whose - consequences are measured and benign. -- **Verify the premise before reasoning from it.** Twice, a confident chain rested on an unchecked - assumption, and twice the check reversed the answer — C1's *documented* fix was wrong, and "C9's - risk is gone after FPSemantics" was wrong. -- **A test that prints nothing on success is indistinguishable from one that never ran.** Report - coverage counts, not just failures. This bit three times before it stuck. -- **A perf change can be a correctness change.** The column-memo optimisation silently deleted the - overhang; the test suite caught it the same day. This is the concrete answer to "why all these - tests" — the failure was invisible to inspection and produced plausible terrain. - **When inserting a class into `VoxelDensityOpStack.cpp` / `VoxelHeightOpStack.cpp`, put it ABOVE the labelled end of the anonymous namespace.** Anchoring on the FACTORIES banner puts it outside, and the brace added with it closes nothing. Made that mistake twice; both files now say so at the exact line. + +## Method lessons this refactor actually paid for + +Ordered by how much they cost. + +- **Instrument before hypothesising.** §C10 cost six builds and five refuted hypotheses, then was + solved for free by a build setting changed for an unrelated reason. Park a question whose + consequences are measured and benign. +- **Verify the premise before reasoning from it.** Four times now, a confident chain rested on an + unchecked assumption and the check reversed it: C1's *documented* fix was wrong; "C9's risk is gone + after FPSemantics" was wrong; "C1 is closed, 0 sites left behind" was wrong (the sweep matched a + *spelling*, `SeedF * K`, and the survivor spelled it `(float)S * K`); "PitDensity enables pits" was + wrong. **A grep over a spelling is evidence about the spelling.** +- **A perf change can be a correctness change.** The column-memo optimisation silently deleted the + overhang; the tests caught it the same day. The failure was invisible to inspection and produced + plausible terrain. +- **Coverage is a number, not a boolean.** Three related traps, each of which produced a green run + that proved almost nothing: + - *A test that prints nothing on success is indistinguishable from one that never ran.* Report + counts, not just failures. + - *A guard that only trips at zero does not measure coverage, it notices absence.* A run with 65 of + 6000 samples in open cave (1.1 %) passed a `== 0` guard silently. Use fractions. + - *A success message that **asserts** coverage instead of reporting it reads as evidence while + measuring nothing.* One said "pits and chimneys exercised" through a whole run in which zero pits + existed. +- **Enabling a feature is not evidence it fired — ask the structure, not the output.** Setting + `PitDensity` did nothing (wrong struct). Diffing two stacks with/without the op pool would have + *lied* (the pool is not in the SDF cache key, so both share the `thread_local` cache). What worked: + call `BuildChunkCache` and look at `Pits.Num()`. Prefer the check that can fail for exactly one + reason. +- **An oracle that shares the defect under test proves nothing.** The stale-cache check was going to + compare two interleaved param sets against the original — which keys its SDF cache without the + params and would have failed. Compare against the thing itself evaluated alone instead. diff --git a/OPSTACK-PROGRESS.md b/OPSTACK-PROGRESS.md index 563fe17..1490995 100644 --- a/OPSTACK-PROGRESS.md +++ b/OPSTACK-PROGRESS.md @@ -2106,3 +2106,36 @@ point at the bake-coverage number rather than claim the coverage. still bit-identical — *that* run is the one where stage A is genuinely covered. Then stage B. --- + +## 2026-07-28 — STAGE A GENUINELY COVERED. 13 tests green. Handoff rewritten. + +``` +TunnelNetworkSpineEquivalence ......................... Success + Equivalence : bit-identical, 6000 samples, 1431 in open cave + Cave coverage : 23.9 % (floor 10 %) + Bake coverage : 49 rooms, 56 pits, 28 chimneys, 0 columns + Fingerprint : 383/400 = 95.8 % differ, 0 served wrong + Box verdicts : 0 proved, 40 Mixed (asserted — §0.2's worm bound) +``` + +**This is the run where stage A means something.** The previous green one did not: it compared +mostly solid rock to solid rock, its stale-cache check asked its question three times out of 400, and +its pit/chimney loops ran on an empty list while the success message claimed otherwise. Same code, +same colour, three different strengths of evidence — which is the whole argument for printing +coverage numbers instead of pass/fail. + +56 pits and 28 chimneys now exist in the bake, so the two loops `§2` calls the fiddliest thing in the +decomposition (unwarped coords `SmoothMin`'d into the warped room SDF) were exercised by 6000 +bit-identical samples. 0 columns confirms `STEP 4d` stayed dormant, as stage A requires. + +`OPSTACK-HANDOFF.md` rewritten for a fresh context: the three-stage TunnelNetwork plan and why stage +A is verifiable while incomplete, the `FRoomGraphSource` calls-not-transcribes rule, `FRAME` ops +recorded as retired, and the method lessons regrouped — notably the three coverage traps and +"enabling a feature is not evidence it fired". + +**Next single action:** stage B — the 13 detail modifiers of `STEP 4b–4h`, inserted between the carve +and the worms. The test's op-count assertion moves, and its `EnableTunnelFeatures` starts turning the +amplitudes back ON one group at a time (a `Terrace` op in the room pool will be the first thing to +break stage A's equivalence, by design). + +---