From d24b6c59e274d02613040d733f7ffb403c878c93 Mon Sep 17 00:00:00 2001 From: Eric J Date: Sat, 1 Aug 2026 19:18:17 -0700 Subject: [PATCH] docs(cliffs): correct the notes that #83 falsified An audit after the multisample grid fix. Several documents still asserted the opposite of what is now measured, and three of them were TITLES - the lines a reader trusts fastest. - `cliffs-NOTES.md` gains a STATUS banner at the top. The root cause sat at line 636 under 635 lines of superseded investigation, so a cold reader met the wrong conclusions first. The banner carries the current recall and orientation numbers, points at the ROOT CAUSE section, and names the specific stale figures below it (the ~90%, the 12.5%/29.8%/8.1%/11.7% tables, the 0.806/0.938/0.853 recalls) as pre-fix. - Two headings there were plainly wrong: "validated ~90% tile-for-tile", and "Validation result: EXACT since 2026-07-30 (the residual is resolved)" - which was only ever true of Nauvis. - `vulcanus-cliffs-NOTES.md` gains the same banner, and its "## The residual is in the RULE, not the fields" section is marked FALSIFIED. Its experiment and numbers are sound; the verdict is not. The fixture it substitutes came through calculate_tile_properties (1-tile grid) while the cliff generator reads a 4-tile grid, so the fixture and the port shared the mistake and could never disagree. - `client-preview-ROADMAP.md` said the residual "is now believed to be a RULE error rather than a field error (PR #57)". It was a field error. Corrected with why PR #57's substitution missed it. - `vulcanusCliffCornerFields.spec.ts`'s doc header and `vulcanusOreCliffSeparation.spec.ts`'s describe name both still claimed the fields are exact and the residual is in the rule. Both now say the fields are exact IN THE TILE CHANNEL, which is the accurate and more useful statement. - CLAUDE.md's spec-file count was 143; it is 152. No behaviour change. `pnpm run verify`: 310 files clean, 1306 passed / 3 skipped, preview 12 passed - the suite is green on an unloaded machine, which also confirms the earlier local timeouts were load and not a regression. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GRx1CU29RKLsRuUAErEDRB --- CLAUDE.md | 5 +-- docs/noise/client-preview-ROADMAP.md | 10 ++++-- docs/noise/cliffs-NOTES.md | 27 +++++++++++++-- docs/noise/vulcanus-cliffs-NOTES.md | 44 +++++++++++++++++++++---- test/vulcanusCliffCornerFields.spec.ts | 15 +++++++-- test/vulcanusOreCliffSeparation.spec.ts | 2 +- 6 files changed, 88 insertions(+), 15 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 66e9967..864f42a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -175,8 +175,9 @@ matters, set it explicitly. one gate. **~65-90s on a dev machine, ~4 minutes on a CI runner**, of which the app test suite is ~57s and `check:vue` ~2.5s. The `~9.5s` this line claimed for a long time was simply wrong - already wrong by a factor of six - before `check:vue` existed, because the suite grew to 143 files through the - Vulcanus and cliff work - and the gap mattered: ~63s is exactly the duration + before `check:vue` existed, because the suite grew through the Vulcanus and + cliff work (143 files then; **152 as of 2026-08-01**, which is what the + `test/**/*.spec.ts` include actually matches) - and the gap mattered: ~63s is exactly the duration at which people start skipping a manual gate, which is half the argument for the CI workflow below. Don't budget 10 seconds for this. - `pnpm refs:sync` - pin `factorioLuaAPI/` + `~/GitHub/factorio-data` to the diff --git a/docs/noise/client-preview-ROADMAP.md b/docs/noise/client-preview-ROADMAP.md index 5a64d77..cf7371c 100644 --- a/docs/noise/client-preview-ROADMAP.md +++ b/docs/noise/client-preview-ROADMAP.md @@ -329,8 +329,14 @@ Done = ore patches overlaid on land, responding to the frequency/size/richness s **zero** predictions on Nauvis; the exact `wouldCollide` rejection is moot because **no Nauvis cliff touches water at all**; and the ore-on-cliff exclusion was withdrawn outright (see the M3a follow-up above - there is no - such rule in the game). The residual is still open and is now believed to be - a **rule** error rather than a field error (PR #57). `VoronoiNoise` (layer-1 primitive table below) is + such rule in the game). **The residual is CLOSED as of 2026-08-01 (PR #83).** + It was a FIELD error after all, not the rule error PR #57 concluded: + `multisample`'s offsets are in the calling noise program's GRID UNITS, so + the cliff generator (4-tile lattice) and the tile generator (1 tile) read + different `vulcanus_elevation`. PR #57's substitution missed it because its + fixture came through the same 1-tile channel the port used. Vulcanus recall + is now 1.000/0.973/0.965 with 2.4% wrong orientations; see the ROOT CAUSE + section of `cliffs-NOTES.md` and issue #84 for the remainder. `VoronoiNoise` (layer-1 primitive table below) is confirmed **unneeded for Nauvis** - it appears nowhere in the cliff tree or any other Nauvis expression traced so far, only on Space-Age planets - so it remains un-ported with no open TODO against it. diff --git a/docs/noise/cliffs-NOTES.md b/docs/noise/cliffs-NOTES.md index 81345aa..411e0bd 100644 --- a/docs/noise/cliffs-NOTES.md +++ b/docs/noise/cliffs-NOTES.md @@ -1,5 +1,28 @@ # Cliffs (M4) - reverse-engineering notes +> ## STATUS, 2026-08-01: both planets validated; issue #18 is CLOSED +> +> | | recall | precision | wrong orientation | +> | --- | --- | --- | --- | +> | **Nauvis** | 1.0000 | 1.0000 | **0 / 334** | +> | **Vulcanus** | 1.000 / 0.973 / 0.965 | 0.872 | **37 / 1531 = 2.4%** | +> +> Vulcanus's precision is measured **without** the lava-collision rejection, which +> the shipping renderer does apply; the remainder is tracked in **issue #84**. +> +> **Read `## ROOT CAUSE, 2026-08-01` (further down) before anything else in this +> file.** It is the resolution of issue #18: `multisample`'s offsets are in the +> calling noise program's GRID UNITS, not tiles, so the cliff generator and the +> tile generator read genuinely different elevation fields. +> +> **Everything between here and that section is the investigation record.** Its +> measurements stand and are worth reading for method; its *conclusions* are +> superseded, several of them within hours of being written. Do not act on a +> number from those sections without checking it against this banner - in +> particular the `~90%` in the next heading, the 12.5% / 29.8% / 8.1% / 11.7% +> orientation-error tables, and the 0.806 / 0.938 / 0.853 recalls all describe the +> port BEFORE the fix. + Factorio 2.1.11 (build 86962, mac-arm64). Measured 2026-07-20 against the headless oracle (`test/oracle/oracle.ts` `sampleExpression` for the noise fields; a `find_entities_filtered{type="cliff"}` chunk-forced dump for placement) and the @@ -89,7 +112,7 @@ refactor), the offset/ringbreak chain (string seeds `'nauvis_offset_x/y'`), the 86883 `basis_noise`, and `slider_to_linear`. **No `VoronoiNoise`** anywhere in the Nauvis cliff tree (it appears only on Space-Age planets). -## Placement rule (disasm-confirmed + validated ~90% tile-for-tile) +## Placement rule (disasm-confirmed; see the STATUS banner for current accuracy) Reverse-engineered from the non-stripped binary and validated against a real `find_entities_filtered{type="cliff"}` dump (chunk-forced generation, default preset). @@ -963,7 +986,7 @@ presets, but all 9 strings in `builtin-presets.json` decode to `u8 = 0` and either way; what is unknown is whether the wire carries smoothing at all. Worth an issue if the Nauvis render is ever driven by a Lakes/Island preset. -## Validation result: EXACT since 2026-07-30 (the residual is resolved) +## Validation result: NAUVIS exact since 2026-07-30 (historical - Nauvis only) > **The ~6% Nauvis residual documented throughout this section is GONE, and the > cause was none of the six things named for it.** The port sampled the two diff --git a/docs/noise/vulcanus-cliffs-NOTES.md b/docs/noise/vulcanus-cliffs-NOTES.md index d8c9a39..681e3c2 100644 --- a/docs/noise/vulcanus-cliffs-NOTES.md +++ b/docs/noise/vulcanus-cliffs-NOTES.md @@ -1,5 +1,21 @@ # Vulcanus cliffs - port notes +> ## STATUS, 2026-08-01: issue #18 is CLOSED; remainder tracked in #84 +> +> Recall **1.000 / 0.973 / 0.965** across the three regions, **37 / 1531 = 2.4%** +> wrong orientations (was 0.806 / 0.938 / 0.853 and 12.5%). Precision 0.872, +> measured **without** the lava-collision rejection the shipping renderer applies. +> +> **Root cause: `multisample`'s offsets are in the calling noise program's GRID +> UNITS, not tiles**, so `vulcanus_basalt_lakes_multisample`'s `min` is a 4-tile +> min-filter for the cliff generator and a 1-tile one for every per-tile consumer. +> The port used the 1-tile field for both, making cliff elevation too rough. Full +> write-up at `## ROOT CAUSE, 2026-08-01` in `cliffs-NOTES.md`; the measurement is +> `test/multisampleGrid.spec.ts`. +> +> **Every accuracy table below this banner is the PRE-FIX state**, kept because the +> reasoning is the useful part. Do not quote one as current. + Factorio 2.1.12 (build 87038, mac-arm64). Ported 2026-07-26. Companion to `cliffs-NOTES.md`, which holds the reverse-engineering of the placement rule itself - none of that had to be redone, because the placement geometry is engine @@ -523,10 +539,14 @@ the `all` path the Vulcanus DAG is evaluated four times per pixel region. > because the reasoning that got from one to the other is the useful part. The > current figures are in "ROOT CAUSE: `cliff_smoothing`". > -> **And read "The residual is in the RULE, not the fields" at the end of this -> file before spending any time on field accuracy.** Substituting the game's own -> `elevation` and `cliffiness` for ours does not move a single placed cell, so -> the residual this section chases is not in the fields. +> ~~**And read "The residual is in the RULE, not the fields"...**~~ **FALSIFIED +> 2026-08-01.** The residual WAS in the field. The substitution test that +> "does not move a single placed cell" was comparing against a fixture sampled +> through `calculate_tile_properties` - a 1-tile-grid noise program - while the +> cliff generator reads the same expression on a 4-tile grid, where `multisample` +> returns different values. The fixture and the port shared the same mistake and +> so agreed with each other. That substitution now correctly MOVES cells; see +> `test/vulcanusCliffCornerFields.spec.ts`. **Superseded the "Not validated" section below.** `test/vulcanusCliffEntities.spec.ts` now compares the port against every real `cliff-vulcanus` the game places, captured @@ -710,12 +730,24 @@ That capture is the obvious next step and is the same shape as `captureCliffEntities` in `test/oracle/capture.ts`, pointed at a Vulcanus surface. -## The residual is in the RULE, not the fields - measured 2026-07-29 +## ~~The residual is in the RULE, not the fields~~ - FALSIFIED 2026-08-01 + +> **This section's conclusion is wrong, and the way it was wrong is the most +> useful thing in this file.** The residual WAS in the field. The substitution +> below is sound as an experiment and its numbers are real - but the fixture it +> substitutes was captured through `LuaSurface.calculate_tile_properties`, whose +> noise program has a **1-tile grid**, while the cliff generator reads the same +> expression on a **4-tile** grid. `multisample`'s offsets are in grid units, so +> the two channels return different values for `vulcanus_elevation`. The fixture +> and the port were making the same mistake, so substituting one into the other +> could never move a cell. See `## ROOT CAUSE, 2026-08-01` in `cliffs-NOTES.md`. +> +> Read on for the method, not the verdict. Everything above, and all of #18, treats the Vulcanus cliff gap as a **field accuracy** problem: the noise field matches to 5e-6, the constants are read from the game, so the ~1.15-1.19x over-placement must be residual field error -somewhere. **That is wrong, and it is now measured rather than argued.** +somewhere. **That was believed to be wrong; it was in fact right.** `test/oracle/capture.ts vulcanus-cliff-corner-fields` dumps the game's own `vulcanus_elevation` and `cliffiness_basic` at every corner of the placement diff --git a/test/vulcanusCliffCornerFields.spec.ts b/test/vulcanusCliffCornerFields.spec.ts index 0115400..ee00905 100644 --- a/test/vulcanusCliffCornerFields.spec.ts +++ b/test/vulcanusCliffCornerFields.spec.ts @@ -17,8 +17,19 @@ import { withCtxDefaults } from "../src/noise/eval/ctx"; const key = (x: number, y: number): string => `${String(x)},${String(y)}`; /** - * **The Vulcanus cliff FIELDS are exact at the regions the port is scored on - - * including `[0,0]`, where it is worst** (issue #18). + * **The Vulcanus cliff fields are exact IN THE TILE CHANNEL - which turned out + * not to be the channel the cliff generator reads** (issue #18). + * + * This fixture samples `vulcanus_elevation` and `cliffiness_basic` through + * `LuaSurface.calculate_tile_properties`, whose noise program has a 1-tile grid. + * The cliff generator walks the 4-tile corner lattice, and `multisample`'s + * offsets are in GRID UNITS, so `vulcanus_basalt_lakes_multisample` returns + * different values in the two channels. The port now reads the cliff-channel + * field, so these values no longer reproduce our placement - and must not. + * See `test/multisampleGrid.spec.ts`. + * + * The history below is kept because it is how the wrong channel stayed hidden: + * every check agreed, because the fixture and the port shared the mistake. * * PR #57 established this by substituting the game's own `vulcanus_elevation` * and `cliffiness_basic` into our placement and finding it moved not one cell. diff --git a/test/vulcanusOreCliffSeparation.spec.ts b/test/vulcanusOreCliffSeparation.spec.ts index 4dc28e9..23d310e 100644 --- a/test/vulcanusOreCliffSeparation.spec.ts +++ b/test/vulcanusOreCliffSeparation.spec.ts @@ -399,7 +399,7 @@ function gameCornerFields(): { elevation: Map; cliffiness: Map key(x / cornerFix.grid, Math.round((y - cornerFix.cornerOffsetY) / cornerFix.grid)); -describe("Vulcanus cliffs: the port's FIELDS are exact; the residual is in the RULE", () => { +describe("Vulcanus cliffs: the game's TILE-CHANNEL fields are not the cliff channel", () => { /** * The measurement `#18` never had, and the one that resolves `#24`. *