feat(cliffs): port fixImpossibleCells - and falsify the claim it explains Nauvis's residual - #32
Merged
Merged
Conversation
…ains Nauvis's residual `CellEdgeCliffCrossingArray::fixImpossibleCells` (`0x10160c550`), the pass at the tail of `crossingsForChunk`, ported. Since 2026-07-20 `cliffs-NOTES.md` has said Nauvis's ~6% cliff residual "is fixImpossibleCells". **Measurement says it is not**: with the pass ported, Nauvis's cell SETS are identical with it on and off, at both oracle seeds - 0.9433/0.9433/1.000 and 0.9423/0.9423/1.000 either way. It never fires there, because `cliffiness_nauvis` is a hard 0-or-10 gate and the configurations it produces are already legal. That was a guess recorded as a finding. The remaining named candidate from the original disasm is `tryToAddCliff`'s `wouldCollide` rejection, never tested. ## The rule A single forward sweep over one chunk's 8x8 cells (row-major, cy outer), NOT a fixpoint - clearing an edge changes the two cells sharing it, and visited cells are never revisited. Per cell it clears edges until the code is legal, taking the first clearable edge in order L, T, R, B, where clearable means not on the chunk's outer boundary. That boundary rule keeps the pass chunk-local, so the port needs no chunk ordering and worker tiling stays byte-identical (test/tiledEquality.spec.ts passes unchanged). Legality needs no new table. The disasm splits on `code <= 0x50` (0x51-byte jump tables at 0x102d00115 / 0x102d00166, two branches encoding the same accept/reject split) and `code >= 0xC0` (bitmask 0x0001000000001003, set bits 0/1/12/48 -> 0xC0, 0xC1, 0xCC, 0xF0). Extracted and compared against CLIFF_PLACED_TABLE: the accepted set is exactly `isCliffPlaced(code)` plus code 0. Two corrections to the old note while here: - It does NOT zero the whole chunk border. The bool parameter gates zeroing the outer edges of the four CORNER cells only, and `crossingsForChunk` passes `false` (`mov w1, #0x0` at 0x10160d0c8) - so that step never runs in this path. - The binary is a UNIVERSAL Mach-O. Raw reads of those jump tables need the arm64 slice offset added or they silently return x86_64 bytes; the first extraction did exactly that and produced 47 plausible-looking targets instead of 2. ## Effect | | recall | precision | ratio | | --- | --- | --- | --- | | Nauvis, both seeds | unchanged | unchanged | unchanged | | Vulcanus [0,0] | 0.788 -> 0.792 | 0.684 -> 0.685 | 1.152 -> 1.155 | | Vulcanus [1500,1500] | 0.855 -> 0.870 | 0.718 -> 0.719 | 1.192 -> 1.210 | | Vulcanus [-1200,800] | 0.801 -> 0.803 | 0.865 -> 0.866 | 0.925 -> 0.928 | Small and one-sided: recall up everywhere (+1.5 points at best), precision a shade up, count a shade worse. Costs ~10% on the cliff pass (6.15s -> 6.77s over placedCells(0,0,1024,1024) on Vulcanus, paired runs), because the chunk path evaluates every edge of every chunk overlapping the query box. Defaults to ON, because the game runs it unconditionally - deliberately unlike `smoothing`, which defaults to Nauvis's 0 rather than the prototype's 1. `test/cliffFixImpossibleCells.spec.ts` pins the Nauvis no-change result, proves the pass is not a global no-op (it does fire on Vulcanus), pins the default, and pins the code-table correspondence. Refs #18, #22. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016vZjgASLZyxUfXx1dsq2HX
wormeyman
added a commit
that referenced
this pull request
Jul 28, 2026
…uvis's residual to field precision (#33) Second and last of the two causes `cliffs-NOTES.md` named for Nauvis's ~6% cliff residual on 2026-07-20. `fixImpossibleCells` fell yesterday's way (PR #32, no change to Nauvis at all). `tryToAddCliff`'s `wouldCollide` rejection falls here. ## Why wouldCollide cannot be it - Existing entities are not there yet. `computeInternal` runs `generateCliffs()` BEFORE `generateEntities()`, so the per-tile mask grid the check consults holds only the tiles' masks. There is nothing else to hit. - The only tile layer the cliff mask intersects is `water_tile`, so the whole rejection reduces to "no cliffs on water" here. - And that can never fire. Over [512,1024)^2 at both oracle seeds, NOT ONE cliff cell touches water - not ours, not the game's, not matched, not mismatched. `cliff_elevation_nauvis` is `10 + 30 * (...)` and `crossesCliff` needs both corners non-negative with max >= elevation_0, so the geometry already excludes everywhere water can be. The regions are 21.1% and 71.9% water, so that is a real exclusion rather than a dry test window - asserted separately, because "no cliff touches water" would pass just as happily against a resolver that never returned water at all. ## What the residual actually is Distance from the nearest band boundary (10 + 40k), minimised over each cell's four corners: | seed | matched p10/p50/p90 | mismatched p10/p50/p90 | | --- | --- | --- | | 123456 | 0.04 / 0.24 / 0.60 (n=266) | 0.02 / 0.07 / 0.25 (n=16) | | 777771 | 0.06 / 0.26 / 0.53 (n=49) | 0.04 / 0.06 / 0.06 (n=3) | The cells we get wrong sit 3-4x closer to a band edge than the ones we get right. That is what a small field difference looks like: our cliff elevation and the game's disagree by enough to flip a corner across a boundary, but only where the corner was already sitting on one. A structural rule we had failed to port would not select for boundary proximity like that. So the follow-up is FIELD PRECISION (f32 vs f64, the fastapprox floor compounding through the hills chain), not a missing pass - a materially different piece of work from the two that were assumed for eight days. Vulcanus: `wouldCollide` cannot apply there either, since the planet has no water tile. Its residual is much larger than Nauvis's and has not been characterised the same way yet; the Nauvis result says to look at field accuracy first. Refs #18, #22. Claude-Session: https://claude.ai/code/session_016vZjgASLZyxUfXx1dsq2HX Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
wormeyman
added a commit
that referenced
this pull request
Jul 28, 2026
) (#36) `pnpm perf` reported a median of 3, taken with every iteration of one view run back-to-back before the next started. That could not resolve the size of change it was used to gate - worse than no benchmark, because it produced confident numbers that were noise. Two perf decisions are already parked on hand-rolled loops because of it. Rebuilt around four changes: minimum of N (timing noise is additive and positive, so the min is the least-biased estimator); interleaved round-robin so load drift hits every arm alike; spread (max/min) printed beside every figure; and derived within-process figures rather than absolutes. Adds the Vulcanus block the gate was always quoted against but the tool never measured - 512x512 at origin (0,0), terrain/resources/rocks/cliffs/ all, with marginals and the all/terrain ratio. Defaults satisfy the issue as written (min of 7, all blocks, ~19 min), but FMW_PERF_N / FMW_PERF_TILE_N / FMW_PERF_BLOCK let you run one arm without paying for the rest - the Vulcanus block alone is ~3.7 min. Cheap iteration is the point: the friction of a 19-minute answer is what got the timing loops hand-rolled three times. **One correction to the issue's own prescription, from measuring the result.** #19 proposed `all/terrain` as the stable statistic. Over two back-to-back runs the ratio moved 3.8% while the MARGINALS held to ~2.5% (rocks identical to the millisecond) - the ratio divides two absolutes that drift independently (terrain +4.8%, `all` +0.8%), so it amplifies their disagreement rather than cancelling it. The ratio is still printed because it is the form the "under 2x terrain" gate is written in, but the header, the output preamble and both notes files now say the marginals are what to compare. Reproduction against the hand-measured figures is partial and the notes now say why: terrain reproduces (3394 -> 3402/3566, inside the baseline's own drift) but every overlay marginal is well outside it (resources 2013 -> ~1730, rocks 1362 -> 1079, cliffs 2133 -> ~1875). Three PRs landed on those paths after that measurement - #25, #28, #32 - so the recorded figures are stale rather than the instrument wrong. The gate conclusion is unchanged: `all` measures 2.31-2.40x terrain, still past 2x. Verified all three blocks end-to-end (exit 0, correct emit order) and that the default `vp test` still skips them: 1184 passed, 3 skipped. Claude-Session: https://claude.ai/code/session_01Mcc72rnThnmTuBXZg63D4Y Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Jul 30, 2026
wormeyman
added a commit
that referenced
this pull request
Jul 30, 2026
#69) client-preview-ROADMAP.md still carried three items as open work that are not open, and one of them as a to-do for a rule the game does not have. The roadmap is what someone reads before picking up cliff work, so these read as the next thing to build. - "Ore excluded from cliffs" -> WITHDRAWN. generateCliffs() (0x1016229b4) touches no tile, entity or resource data, so there is no exclusion to port (#24, now closed). The observation behind it also fails its own null: region [0,0]'s 945 ore tiles are 2 blobs, and a torus-shift null leaves two of three regions not significant (P = 0.51, 0.29). - "Still deferred: fixImpossibleCells" -> ported in PR #32 (dea73ac), and it changes ZERO predictions, so it never explained the ~6% residual. - "Still deferred: wouldCollide" -> moot; no Nauvis cliff touches water. The residual stays open, reclassified by PR #57 as a rule error rather than a field error. Claude-Session: https://claude.ai/code/session_01BkKb3S2cGyRFFHSzuCQBgj Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CellEdgeCliffCrossingArray::fixImpossibleCells(0x10160c550) ported. The headline result is negative, and it's the reason this is worth landing even though the numbers barely move.It does not explain Nauvis's residual
Since 2026-07-20
cliffs-NOTES.mdhas stated Nauvis's ~6% cliff residual "isfixImpossibleCells". With the pass ported, Nauvis's cell sets are identical with it on and off, at both oracle seeds - 0.9433/0.9433/1.000 and 0.9423/0.9423/1.000 either way. It never fires there:cliffiness_nauvisis a hard 0-or-10 gate, so the configurations it produces are already legal.That attribution was a guess recorded as a finding, and it survived eight days of being the "known cause". The remaining named candidate from the original disasm is
tryToAddCliff'swouldColliderejection, which has never been tested on either planet.The 2026-07-20 decision to defer the pass was still right on the evidence available - deferring it cost Nauvis nothing.
The rule
A single forward sweep over one chunk's 8x8 cells (row-major,
cyouter), not a fixpoint - clearing an edge changes the two cells sharing it, and visited cells are never revisited. Per cell it clears edges until the code is legal, taking the first clearable edge in orderL, T, R, B, where clearable means not on the chunk's outer boundary. That boundary rule keeps the pass chunk-local, so the port needs no chunk ordering and worker tiling stays byte-identical (tiledEquality.spec.tspasses unchanged).Legality needs no new table. The disasm splits on
code <= 0x50(0x51-byte jump tables at0x102d00115/0x102d00166, two branches encoding the same accept/reject split) andcode >= 0xC0(bitmask0x0001000000001003, set bits 0/1/12/48 →0xC0,0xC1,0xCC,0xF0). Extracted and compared againstCLIFF_PLACED_TABLE: the accepted set is exactlyisCliffPlaced(code)plus code0.Two corrections to the old note while here:
boolgates zeroing the outer edges of the four corner cells only, andcrossingsForChunkpassesfalse(mov w1, #0x0at0x10160d0c8) - that step never runs in this path.Effect, measured
[0,0][1500,1500][-1200,800]Small and one-sided: recall up everywhere (+1.5 points at best), precision a shade up, count a shade worse. Costs ~10% on the cliff pass (6.15s → 6.77s over
placedCells(0,0,1024,1024)on Vulcanus, paired runs), because the chunk path evaluates every edge of every chunk overlapping the query box rather than only the cells asked for.This is a marginal change and I'd understand dropping it - the case for merging is faithfulness (the game runs this unconditionally) plus the falsified attribution, not the numbers. The case against is 10% for +1.5 points of recall at best. Happy either way.
Defaults to ON, because the game runs it unconditionally - deliberately unlike
smoothing, which defaults to Nauvis's0rather than the prototype's1.Tests
test/cliffFixImpossibleCells.spec.tspins the Nauvis no-change result (cell sets, not counts), proves the pass is not a global no-op by asserting it does fire on Vulcanus - without which "no change on Nauvis" would be equally consistent with the port doing nothing - pins the default, and pins the code-table correspondence.pnpm run verify: 1175 passed, 0 failed.🤖 Generated with Claude Code
https://claude.ai/code/session_016vZjgASLZyxUfXx1dsq2HX