From b533294f5de8a8f572aaff11ad72ae51cef274ba Mon Sep 17 00:00:00 2001 From: Fr0zka Date: Tue, 28 Jul 2026 14:13:59 +0200 Subject: [PATCH] docs(opstack): record the green build and rewrite the handoff for a fresh context OPSTACK-PROGRESS gets the measured entry: every number the run produced, what each one settles, and the single open warning. OPSTACK-PLAN's status header goes from UNVERIFIED to BUILT AND GREEN. OPSTACK-HANDOFF is rewritten end to end. It no longer describes a transition in progress but a completed one, and it leads with the two things a cold context needs: the Underwater 0%-coverage warning (with the truncation-vs-floor finding behind it, and the reminder that a green bit-identity over solid rock is not evidence), and the one task everything else now waits on -- making FRoomGraphSource::EffectOverBox answer spatially. It also carries forward the two debts that must be paid BEFORE that lands rather than after: box bounds computed from strate params can be too optimistic once a per-room op raises them, and AUDIT C2 is confirmed but unfixed on the switch path. Co-Authored-By: Claude Opus 5 --- OPSTACK-HANDOFF.md | 376 +++++++++++++++----------------------------- OPSTACK-PLAN.md | 8 +- OPSTACK-PROGRESS.md | 72 +++++++++ 3 files changed, 208 insertions(+), 248 deletions(-) diff --git a/OPSTACK-HANDOFF.md b/OPSTACK-HANDOFF.md index 1fcffa3..50e6a8d 100644 --- a/OPSTACK-HANDOFF.md +++ b/OPSTACK-HANDOFF.md @@ -1,266 +1,147 @@ -# Handoff — VoxelForge operator stack, 2026-07-28 (end of day 3) +# Handoff — VoxelForge operator stack, 2026-07-28 (Phase 2 complete and green) > Paste the block below into a fresh session. Everything it refers to is on disk and in git. +> +> **State:** Phase 2 is **DONE — 8 of 8 archetypes ported, built, and green** (14 tests). +> `ClassifyTile` consumes `ClassifyBox`. One open warning, one clear next task. --- -You're picking up an agreed refactor of the VoxelForge UE5 voxel plugin, on branch `experimental` -(already checked out — do not create another). I'm Jahni. Design and progress are written down so you -don't re-derive them. +You're picking up the VoxelForge UE5 voxel plugin on branch `experimental` (already checked out — +do not create another). I'm Jahni. The design and the history are written down so you don't +re-derive them. ## Read first, in this order 1. **`CLAUDE.md`** — project rules. **Rule #1 is absolute: never build, compile, or run the editor.** I build everything myself. When code is done, stop, say "ready to build", list the likely compile-error spots, and wait. -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. §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. +2. **`OPSTACK-PROGRESS.md` — THE LAST ENTRY FIRST.** Append-only log; the resume point. The last + entry is the green build with every measured number in it. +3. **`OPSTACK-PLAN.md`** — the plan. **§2.6.1 is the acceptance bar** and supersedes §2.6. +4. **`OPSTACK-DECOMPOSITION.md`** — per-archetype breakdown. **§0.2** (the amplitude bound) is the + live one; §2 TunnelNetwork and §8 Underwater are now history, not instructions. +5. **`AUDIT-2026-07.md`** — **§C2 has a CONFIRMED sub-item as of 2026-07-28, read it**; §C10 is + SOLVED, don't reopen; §C9's library half is the top open theoretical risk with 0 measured + exposure. 6. **`CODEMAP.md`** — navigation. Trust symbol names over line numbers. -## Where things stand — 6 of 8 ported and wired, the 7th half-ported, 13 tests green +## Where things stand — the transition is COMPLETE and VERIFIED + +All 8 archetypes have an operator-stack twin, per-strate opt-in, each equivalence-tested **bit for +bit** against its original density function. The `switch` and the stack are now two complete, +interchangeable implementations. | 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 | +| `SurfaceWorld` | ✅ ported incl. biome blending, bit-identical, wired | +| `VerticalShafts` | ✅ ported, bit-identical, wired | | `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 | +| `TunnelNetwork` | ✅ **19 ops**, bit-identical incl. all 12 detail modifiers + per-room override | +| `Underwater` | ✅ same builder, second `case` — ⚠️ its check has a coverage gap, see below | -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. +Everything sits behind `UVoxelStrateDefinition::bUseOperatorStack`; the ported list lives **only** in +`UVoxelStrateManager::UsesOperatorStackForChunk` (now all 8). **No strate asset has the box ticked** +— that is my call and I haven't made it. But the flag is no longer a no-op anywhere: ticking it now +really switches that strate onto the stack, for density *and* for tile classification. -## TunnelNetwork is staged — read this before touching it +`ClassifyTile` **consumes `ClassifyBox`** for cave archetypes (SurfaceWorld and bedrock gaps keep +their hand-written exact-lattice proofs). `GetDensityAt` and `ClassifyTile` build the stack through +the **same** factory, `VF_BuildOpStackForChunk` — a second copy would be a hole, not a bug. -`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: +## The one open warning -- **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. +``` +Underwater (stage C2): bit-identical across 2000 samples — 0 of them in open cave (0.0%) +``` -**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. +**A green bit-identity over 2000 samples of solid rock is not evidence** — it is exactly what two +agreeing voids look like. Same failure as stage A's 1.1 % run, in a different slot, caught by a +counter written for it. -**⚠️ 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. +A real bug surfaced while diagnosing: the sampled chunk-Z range used `Z / CHUNK_SIZE`, and C++ +integer division **truncates toward zero**. TunnelNetwork is at the top of the layout in positive Z +where truncation == floor, so it could not show there; Underwater is at the **bottom, in negative +Z**, where it shifts the upper chunk bound a notch high and the `Clamp` piles samples into the top +seal band. Fixed (`FloorDivChunk`), sampling widened 8 → 24 clusters, **and not trusted**: new +**check 5b** gives each of the three possible causes its own number and prints how to read them. -## Three things Phase 2 settled that were not in the original design +**⚠️ That commit (`ce409e7`) is written but NOT BUILT.** First action: build, run the `VoxelForge` +filter, read the `Underwater diagnosis` line. It names the cause; the fix follows from it. -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. +## Then the one task everything is waiting on -## What is left, in the order I'd do it +**Make `FRoomGraphSource::EffectOverBox` answer spatially.** -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 +TunnelNetwork proves **0 of 40** tiles today, and the test asserts that. The chain dies at the room +source, which returns `Both` with unknown amplitude before anything downstream is reached. Its room +and tunnel bounds (`FCachedRoom::CullRadiusSq`, `FCachedTunnel::BoundRadiusSq`) are **already in the +SDF cache**; what it costs is building that cache for the *queried box*, on the querying thread. + +That cost is now clearly worth paying, and every other piece is already built to receive it: + +- `ClassifyTile` consumes `ClassifyBox` in production, so a proved tile skips `GenerateMesh` — + 30 000+ density evaluations saved against one `BuildChunkCache`; +- the fold carries **numbers** (`MaxCarveOverBox` / `MaxFillOverBox` / `ForcedMarginOverBox`), so a + bounded worm no longer kills `AllSolid` on rock that is solid by more than it can carve; +- the twelve detail modifiers already **inherit** the room source's verdict via `VF_NoCaveOverBox` — + the day the source says `Identity` for a box, all twelve follow, in one place rather than thirteen. + +**Keep the brute-force check.** `VoxelForge.OpStack.ClassifyTileSoundness` verifies verdicts against +`GetDensityAt` on a world where every strate opted in. A false verdict is an invisible hole: no +geometry, **no collision**, until a player falls through it. + +## ⚠️ Debts that must be paid BEFORE that lands, not after + +Both were introduced knowingly and are written at the exact site a reader would land on. + +1. **Box bounds read STRATE params, but a per-room op can raise them.** `EffectOverBox` and the new + amplitude bounds are computed from strate params, because a box spans many rooms. But `ApplyTo` + writes the op's value **even where the strate's was 0**, so a room op can enable a modifier the + strate had switched off, or give it a bigger amplitude. A box verdict on a strate with a + terrain-op pool can therefore be **too optimistic** — the dangerous direction. Harmless while the + room source answers `Both` (nothing is provable anyway); **not harmless the moment it doesn't.** + Noted at `FLayerLineMod::EffectOverBox` and `FRoomGraphSource::LocalParams()`. +2. **`AUDIT §C2` is confirmed and unfixed on the `switch` path.** `GetGenerationParams` blends params + *within* a strate (`Alpha` depends on chunk Z for `Gradient`, and on chunk XY too for + `Interleaved`), and `Gradient` + `TransitionBlendChunks = 2` are the **defaults**. The original's + SDF cache key has neither params nor chunk Z, so a worker evaluates the second chunk it builds + against the first chunk's rooms — and *which* chunk came first depends on worker order, so two + peers can diverge from the same seed. The op stack does **not** inherit it (params CRC in the + key), and `ClassifyTile`'s new path guards against it explicitly (params must be bit-identical + across every chunk coord the box touches). The fix on the `switch` path is a params CRC in its + key — a live-generation change that wants a build in front of it. + +## After that, in order + +1. **PERF — unparked.** 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. Also measured and stated: + the gate is now tested twelve times per voxel instead of once (stage B5's deliberate trade). + **Measure before optimising** — that is the §C10 lesson. +2. **`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. - -## ⚠️ IF YOU ARE RUNNING UNATTENDED — read this section before starting - -Jahni may start this session and leave, then have you shut the PC down when you're done. - -**You cannot build, and nobody will build for you during this run.** Rule #1 still holds absolutely. -Everything you write stays unverified until he returns. - -**That is his explicit, repeated decision, not an oversight — do not re-litigate it and do not -quietly scale the work down.** Verbatim: *"I'm fine with unverified work, if committed, I can always -manually go back and try — I want him to work towards the full op stack completeness."* So the goal -of an unattended run is **to finish the transition**, not to stop somewhere safe. Take the queue -below as far as it goes. - -Two things make that reasonable rather than reckless, and they are worth knowing: - -1. **Committing per group IS the safety net.** He reverts by sha. That is why the one-commit-per-group - rule is the one thing not to bend. -2. **Reaching 8 of 8 does not change his world by itself.** `UsesOperatorStackForChunk` returns false - unless a strate has `bUseOperatorStack` ticked, so adding `TunnelNetwork` and `Underwater` to the - ported list is inert until *he* ticks a box. The flag flip is safe to include; pointing a live - world at it stays his call. - -### The rules that replace "build after each step" - -1. **One commit per group below. Never bundle two.** Jahni reverts by group, and a failed build must - bisect to a group rather than to 700 lines. Name the group explicitly in the subject line. -2. **Every commit leaves the tree coherent.** No half-written op, no dangling brace, no factory - declared without a definition. Assume the next thing that happens is a compile. -3. **Transcribe literally. Do not improve anything.** With no test feedback, a "cleanup" is an - unfalsifiable guess. Keep the original's operation order, its constants, its `FVector` - round-trips, its odd `FMath::Max` floors. If something looks wrong, **write it down in the - progress log and port it as-is** — that is how `§C1` and the `MinDivisor` split were both found - without breaking anything. -4. **Update the test in the same commit as the code it covers**, including its coverage counters. Do - not leave "I'll add the test after" — after is a build away. - -### The work queue, in order. Run it to the end — **8 of 8 is the finish line.** - -- **B1 — Surface roughness** (`STEP 4b`, ~130 lines). Density-space variant: a *different op* from - `FSdfRoughnessMod` (quadratic fade, anti-fill clamp, 4 noise types). See `DECOMPOSITION §1`. -- **B2 — Terrace · LayerLines · Ribbing** (~150 lines). The three "sedimentary" remaps. -- **B3 — Overhang · Cliff · Scallop · Arch** (~210 lines). -- **B4 — Columns · Domes · Pinch · FloorBias** (~190 lines). Columns read `SDFCache.Columns`, so this - group needs a Column op in the test's room pool — and the stage-A guard that *errors* on - `TotalColumns > 0` must move to this commit, not before it. -- **B5 — the gate itself.** All of B1–B4 live inside `if (bNearCaveSurface)`, i.e. - `Sdf < SDFBlendRadius·3 && Sdf < FLT_MAX`. Decide deliberately whether that is one scoping wrapper - or a repeated early-out in each op, write down which and why, and make the test prove a voxel - *outside* the gate is untouched. - -After each group, extend the test: turn that group's amplitudes ON in `EnableTunnelFeatures`, add a -coverage counter proving the group actually fired (**not** just that its param is non-zero — see the -pit/chimney lesson below), and move the op-count assertion. - -**⚠️ The subtlety that lets B precede C.** Inside `if (bNearCaveSurface)` the original *shadows* -`Params` with `LocalTerrainParams` — a copy with the nearest room's terrain op applied — and **all 13 -modifiers read the shadowed copy**. Porting them against strate-level params is therefore equivalent -*only while no room carries a detail-type op*. The test's pool is Pit/Chimney only and `ApplyTo(Pit)` -writes just the four pit fields, so the two are identical today. **Keep the pool that way through -B1–B5**; C1 is what makes a `Terrace` op in the pool legal. (B4's Column op is safe: `STEP 4d` reads -`SDFCache.Columns` from the bake, never the shadowed params.) - -- **C1 — the per-room op override.** `§2` calls this the piece with "no clean home" and offers two - options. **Take option (a), and note that the mechanism is already established in this codebase — - do not invent a scoping predicate.** Three ops already read state owned by an upstream op via a - non-owning pointer handed over at build time: `FOverhangShelfMod` ← `FSurfaceColumnSource`, - `FShaftLedgeMod` ← `FShaftFieldSource`. Do the same here: - - `FRoomGraphSource` gains `const FStrateGenerationParams& LocalParamsAt(x, y, z) const`, which - reproduces the original exactly — take the voxel's `NearestRoomIdx` (it already computes it), apply - that room's `RoomOp` onto a `thread_local` copy of the params, return it. Every detail op holds a - `const FRoomGraphSource*` and reads its fields from that copy instead of from its own. - - This is the pit/chimney resolution a second time: the difficulty in `§2` came from assuming each - modifier must own its params. Let one op own the shared state and the rest read it, and the problem - disappears. **Transcribe the per-voxel copy as-is** — yes, the original copies a ~74-field struct - per voxel inside the gate; note it in the log as a perf item and change nothing. - - Acceptance: put a `Terrace` op in the test's room pool alongside Pit/Chimney and require - bit-identity. That is the check B could not make. -- **C2 — `Underwater`.** Nearly free: `§8` establishes there is **no density difference at all** — - `GetDensityAt` routes it to `GetDensityWithParams` and `WaterLevelRelative` is consumed by the - render-side water system, not by density. So it is the TunnelNetwork stack, reached from a second - `case`. Add the wiring case and an equivalence test that samples the `Underwater` slot. If you find - a real density difference, **stop and write it down** — that would contradict `§8` and is worth - more as a finding than as code. -- **C3 — 8 of 8.** Add `TunnelNetwork` and `Underwater` to `UsesOperatorStackForChunk`'s ported list. - Update `CODEMAP §3.2d` and its `UsesOperatorStackForChunk` row, tick `OPSTACK-PLAN` Phase 2, and - write the closing `OPSTACK-PROGRESS` entry: the archetype `switch` now has a complete operator-stack - twin, per-strate opt-in, every one equivalence-tested. - -### After 8 of 8 — keep going, in this order - -1. **Settle `AUDIT §C2`'s suspected item.** Does `GetGenerationParams` actually vary within one strate - across a Gradient transition? Answerable by *reading* `GetGenerationParams` and - `FStrateGenerationParams::Lerp` — no build. Turn the suspicion into a yes or a no with the - evidence, and correct the audit either way. Cheap, and it closes an open question honestly. -2. **The worm amplitude cap (`DECOMPOSITION §0.2`).** The largest single perf item in the plan, and - the reason TunnelNetwork proves 0 of 40 tiles. It needs the fold to carry a **number**, not just a - direction: add a numeric carve/fill amplitude alongside `EVoxelOpEffect`, have - `FWormFieldSource::MaxCarveAmplitude()` (already written, already unused) feed it, and let - `AllSolid` survive when the rock is solid by more than the sum of every remaining carve. - **⚠️ This changes the fold contract that all 13 tests rest on — its own commit, nothing else in - it,** and `VoxelForge.OpStack.BoxVerdictFold` must be extended in the same commit. -3. **Make `ClassifyTile` consume `ClassifyBox`.** Verified per archetype, still unconsumed in - production — this is where measured tile-skipping (Maze 23/60, slabs 36–40/60) becomes frames. - A false verdict is an invisible hole, so keep the brute-force check in every archetype's test. -4. **Re-read your own diffs against the original**, group by group, as a reviewer rather than an - author. On a run with no builds, a second reading is the only oracle you have. Do this rather than - starting anything new when the queue runs out. - -### Still not yours to do, even unattended - -- **Do NOT build, do not launch the editor, do not push.** Unchanged, not negotiable. -- **Do NOT tick `bUseOperatorStack` on any strate asset.** C3 makes the archetypes *available*; - pointing a live world at an unverified path stays Jahni's decision. -- **Do NOT start Phase 3** (ops as data assets). It is a design conversation, not a transcription. - -### The commit messages ARE the ledger - -**Jahni's words: "just commit with explicit name what changed, I'll revert if needed."** So there is -no separate bookkeeping — the commit history is the record, and it has one job: let him revert a -single group without touching the others. - -That makes the subject line load-bearing. Name the group and the scope, not the intent: - -``` -feat(opstack B2): port Terrace, LayerLines, Ribbing detail modifiers -feat(opstack B4): port Columns, Domes, Pinch, FloorBias + column op in the test pool -feat(opstack C1): per-room op override — detail ops read LocalParamsAt from the room source -feat(opstack C3): TunnelNetwork + Underwater in the ported list — 8 of 8 -``` - -Body: what was transcribed, anything that looked wrong and was ported as-is anyway, and what breaks -first if the group is wrong. One group per commit — that rule exists *because* of the revert -workflow, so it is the one thing not to bend. - -**Close the run with one `OPSTACK-PROGRESS` entry** listing every commit made, in order, with a -one-line "what this touches" each, and the test filter to run (`VoxelForge`). Jahni's first action on -return is a single build; that entry is what makes it efficient. Then shut down if he asked you to. +3. **`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. Run + `CrossPlatformDigest` on Linux, compare the SHAPE digest, pin it. The real fix if ever needed is a + deterministic in-house sin/cos. +4. **Phase 3 — ops as data assets.** A design conversation, not a transcription. Don't start it + unprompted. What makes it possible is already in place: ops depend on capabilities + (`IVoxelBiomeField`), never on `UVoxelGenerator`. ## Hard rules that prevent real bugs - **Density sign:** negative = solid at the mesher. Inside the op stack the convention is INTERNAL (**positive = solid**), negated once by the caller. The SDF channel uses standard SDF convention. -- **Never run both density paths in one world.** -- **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 `§C2` and the overhang regression - of 2026-07-27, where omitting the params silently deleted the overhang and only 1 sample in 20 000 +- **Never run both density paths in one world.** **Comparing them is legitimate** — §C10 is closed + since `FPSemantics = Precise`, and all eight equivalence tests compare bit for bit. They are + **port-correctness oracles**, not fidelity checks: §2.6.1 requires *same seed ⇒ same world on every + peer*, not resemblance to the pre-refactor world. +- **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. @@ -268,7 +149,7 @@ return is a single build; that entry is what makes it efficient. Then shut down - Update `CODEMAP §3`, `ARCHITECTURE §8`, tick `OPSTACK-PLAN`, append to `OPSTACK-PROGRESS.md`. - **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 + and the brace added with it closes nothing. Made that mistake twice; both files say so at the exact line. ## Method lessons this refactor actually paid for @@ -278,28 +159,33 @@ 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 +- **Verify the premise before reasoning from it.** Five 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.** + *spelling*); "PitDensity enables pits" was wrong; "there are 13 detail modifiers" was wrong (twelve, + and only eleven read the per-room copy). **A grep over a spelling is evidence about the spelling.** +- **Read the code, not the comment.** The cliff modifier's comment promises a sampled Z±1 gradient; + the code samples nothing and uses a Z-stretched Perlin it *calls* `VertGrad`. Ported as written — + and written down, so nobody "fixes" it from the comment. - **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 + overhang; the tests caught it the same day. Invisible to inspection, and it produced plausible + terrain. +- **Coverage is a number, not a boolean.** Four 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 test that prints nothing on success is indistinguishable from one that never ran.* + - *A guard that only trips at zero notices absence, it does not measure coverage.* 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. + measuring nothing.* + - *A check can be vacuous as well as a counter.* "Nothing leaked" is worthless unless something + happened — so the gate check also reports how many samples move when the modifiers are zeroed. - **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. + call `BuildChunkCache` and look at `Pits.Num()`. **Prefer the check that can fail for exactly one + reason** — and when a zero has three possible causes, give each one its own number. +- **An oracle that shares the defect under test proves nothing.** The stale-cache check compares each + stack against *itself evaluated alone*, never against the original — which keys its SDF cache + without the params and would fail it. +- **One definition, not two kept in sync.** `VF_BuildOpStackForChunk` exists because a tile skipped on + the verdict of a stack that is not the one producing its density is a hole. A "keep these in sync" + comment would not have been enough. diff --git a/OPSTACK-PLAN.md b/OPSTACK-PLAN.md index 6422f03..a242b5f 100644 --- a/OPSTACK-PLAN.md +++ b/OPSTACK-PLAN.md @@ -10,9 +10,11 @@ > FloatingIslands · TunnelNetwork · Underwater.** The archetype `switch` now has a complete > operator-stack twin, opt-in per strate. > -> ⚠️ **The last three groups (stage B, C1, C2, C3) are UNVERIFIED — written, committed, never -> compiled.** Run the `VoxelForge` filter; `OPSTACK-PROGRESS.md`'s last entry lists every commit and -> what breaks first if one is wrong. +> ✅ **BUILT AND GREEN, 2026-07-28 — 14 tests.** TunnelNetwork A+B bit-identical over 6000 samples +> with all twelve group-coverage probes non-zero, all four noise branches covered, 0 gate leaks, and +> C1 proved by 10 Terrace-op rooms containing 1119 samples. One open warning: the `Underwater` check +> landed 0 samples in open cave, so its bit-identity proves little — diagnosed, not guessed, in +> `OPSTACK-PROGRESS.md`'s last entry. > > **Not done, and it is the next real prize:** `ClassifyTile` still uses hand-written guards and does > not consume `ClassifyBox`. That is where measured tile-skipping becomes frames. diff --git a/OPSTACK-PROGRESS.md b/OPSTACK-PROGRESS.md index b2d272d..368f9d9 100644 --- a/OPSTACK-PROGRESS.md +++ b/OPSTACK-PROGRESS.md @@ -2395,3 +2395,75 @@ difference *is* the T1.d gain. After that: make `FRoomGraphSource::EffectOverBox` answer spatially. Everything else is now waiting on it, and the two commits above were built to receive it. + +--- + +## 2026-07-28 — **IT ALL COMPILES AND IT IS ALL GREEN.** 14 tests. 8 of 8, verified. + +Jahni built it. Everything written across the unattended run is now verified except one number. + +``` +TunnelNetwork STAGE A+B ... bit-identical, 6000 samples (1002 open cave, 4699 rock) +Cave coverage ............ 16.7 % (floor 10 %) +Group coverage ........... roughness 64.8 % · terrace 60.2 % · layer lines 25.7 % + ribbing 13.4 % · overhang 39.4 % · cliff 26.2 % + scallop 1.6 % · arches 3.4 % · domes 1.6 % · pinch 1.0 % + floor bias 18.2 % ← ALL TWELVE NON-ZERO +Roughness noise sweep .... 8/8 variants bit-identical over 1500 samples +Gate check (B5) .......... 979 outside the gate, 0 leaked; 4880 inside move +Params fingerprint ....... 395/400 = 98.8 % differ, 0 served wrong +Bake coverage ............ 49 rooms, 26 pits, 30 chimneys, 39 columns +Per-room override (C1) ... 51/51 rooms carry an op, 10 Terrace, 1119 samples inside them +Box verdicts ............. 0 proved, 40 Mixed (asserted) +Underwater (C2) .......... bit-identical, 2000 samples — 0 in open cave ⚠️ +``` + +**Every claim this run made about correctness now has a measurement behind it.** The twelve +modifiers are transcribed correctly *and* each demonstrably fired; the four noise branches are +covered, not just the one; the gate suppresses everything outside it with zero leaks; and C1 is +proved by the only check that could prove it — 10 rooms drawing a `Terrace` op whose params +overwrite the strate's, with 1119 samples inside them, still bit-identical. + +### The one number that is a warning, and it is the test working + +`Underwater: 0 of 2000 samples in open cave.` **A green bit-identity over 2000 samples of solid rock +is not evidence** — it is exactly what two agreeing voids look like. Same failure as stage A's 1.1 % +run, in a different slot, caught by a counter written for precisely that. + +**A real bug surfaced while diagnosing it.** The sampled chunk-Z range used `Z / CHUNK_SIZE`, and +C++ integer division **truncates toward zero**. TunnelNetwork sits at the top of the layout in +positive Z, where truncation and floor agree — so the bug could not show there. Underwater sits at +the **bottom, in negative Z**: `-1 / 32` is `0` truncated and `-1` floored, so the upper chunk bound +starts a notch too high and the `Clamp` piles the excess onto the strate's last voxel, inside the +top seal band, i.e. solid rock. Fixed in both point builders (`FloorDivChunk`). Same family as the +`DivideAndRoundDown` lesson already in the project notes. + +That is a **candidate**, not a conclusion, so the fix does not stand alone: new **check 5b** gives +each of the three possible causes its own number — rooms baked for the Underwater strate index (the +bake), samples landing inside the seal-free interior (the Z range), and both non-zero (the XY +spread) — and prints how to read them. Sampling also widens 8 → 24 clusters. **Unverified: this +commit has not been built.** + +### What the green run settles, beyond the tests + +- `AUDIT §C10` stays closed: eight archetypes now compare **bit for bit** against their originals + across every detail path, including four noise types and a domain warp. +- The `FRoomGraphSource::FState` hoist, the lazy `LocalParams()` memo and the whole C1 mechanism are + compiled and exercised — the pattern "one op owns the state, the rest read it" now has three + users, all green. +- The extraction of `VF_BuildOpStackForChunk` did not disturb `GetDensityAt`: the equivalence tests + that route through it are unchanged. +- `0 proved, 40 Mixed` remains **asserted** for TunnelNetwork. That is not a defect and not stale: + the chain still dies at `FRoomGraphSource::EffectOverBox`, which is the next piece of work. + +### Next single action + +Re-run the filter and read **one line**: `Underwater diagnosis`. It names which of the three causes +produced the 0 %, and the fix follows from that rather than from a guess. + +Then the one thing everything else is now waiting on: **make `FRoomGraphSource::EffectOverBox` +answer spatially.** Its room and tunnel bounds are already in the SDF cache; what it costs is +building that cache for the queried box, and that is now clearly worth it — a skipped tile saves +30 000+ density evaluations, `ClassifyTile` consumes `ClassifyBox` in production, the numeric +amplitude fold is in place, and the twelve modifiers already inherit the source's verdict. Every +piece is built to receive it and nothing else moves until it lands.