feat(server): Sprint 35 — seed, brand layer, terrain refs, atlas pipeline #129
Assignees
jpmschweitzer (Jeroen Schweitzer)
Clear assignees
jpmschweitzer (Jeroen Schweitzer)
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: jpmschweitzer/settled-reach#129
Reference in New Issue
Block a user
Summary
Sprint 35 server deliverables — the four tickets that feed Phase 3 of the
development cascade (Atlas of the Reach). Recovered from a mid-sprint crash,
with all WIP correctly attributed to tickets and no bundled work.
StartupMessagethroughSimulationPluginintoEconomyPluginandSimRng, replacing thehardcoded
seed = 0. Integration test fixtures updated for the newSimulationPlugin { seed }signature.brand_products,brand_inputs,system_fiscal,corp_financial_state,corp_lifecycle_events, plus indexes and V-B01–V-B05 structuralvalidation. Seeded from
wiki/economics/corporations/brands.toml(depends on copy PR #127 which is already merged). Brand products are
demand nodes per D-185 — they consume commodities, they are not
commodities themselves. Run produces 8 brand_products, 16 brand_inputs,
301 system_fiscal rows.
populate_terrain_reference.py— resolves each body'sexpected wiki heightmap path and writes it into
bodies.terrain_reference.2380 / 3240 bodies populated, 860 logged as missing heightmaps for
later remediation. Unblocks the atlas generator.
generate_atlas.py— terrain-aware sequential city placementand infrastructure generation per D-191 §3. Pipeline per body: simulate
terrain → analyse continents/habitability/river-mouths/cost-grid → place
cities sequentially (capital first, corridor growth via multi-source
Dijkstra, quadrant-spread penalty, new-continent port bonus at cities
3–4) → generate roads and railroads as MST + A* paths on the terrain
cost grid → place a transit POI at the capital. Deterministic per
(seed, body_id). Emits canonical pixel-space markers.json matchingthe copy team's hand-authored templates.
Atlas DB index (new, #832) — scalar metadata mirror of every
markers.jsoninsystems.dbso the implant atlas app and developmentqueries don't have to scan 267 JSON files. Polyline geometry stays in
the markers.json files next to the heightmaps (the renderer needs them
anyway); the DB only stores filterable scalar fields plus
point_countas a length proxy. Schema in
systems-schema.sql; generator mirrorsCREATE TABLE IF NOT EXISTS so it runs against any DB state. Tables:
atlas_body_grids,atlas_cities,atlas_roads,atlas_railroads,atlas_pois,atlas_rivers,atlas_oceans,atlas_mountain_ranges.D-191 §8 updated: the canonical markers.json storage format is
center: [row, col]pixel space with agrid: {w, h}header — matchingboth the generator and every hand-authored template. Lat/lon is a
display-time derivation in the atlas UI, not a storage format.
Runtime for the full atlas batch: 280 s for 267 inhabited bodies on a
single core, zero errors. 261 markers.json files newly populated, 6
hand-authored bodies preserved and indexed without regeneration.
Atlas index after run
Notes for reviewers
make economy-dbstill trips the pre-existing D-175 Phase-2 coveragegate (22 commodity/system coverage errors). Data is committed before
the gate runs; the exit-1 behaviour is unchanged from
origin/mainandis orthogonal to this branch.
serialization.rs,bridge_ipc.rs,bridge_tcp.rs, parts oferror_handling.rs—ObserverSnapshotrequires aneconomy_snapshotfield that thesetests don't set. Unchanged from
origin/main, not touched by this PR.Server lib tests (1150) are all green.
because most inhabited bodies are
urban_concentratedwith pop < 1Band the D-191 §8 formula collapses to 1 city. This is spec; #838
(hand-refine pass) is where content gets richer. Flag if you'd rather
tune the formula before #833 runs.
Test plan
cargo check --manifest-path server/Cargo.tomlcleancargo clippy --manifest-path server/Cargo.toml -- -D warningscleancargo test --manifest-path server/Cargo.toml --lib— 1150/1150 passcargo test --manifest-path server/Cargo.toml test_world::tests— 5/5 passruff check tooling/planet-gen/ tooling/economy-db/cleanmake economy-db— brand import, V-B01–V-B05 validation OKpython3 tooling/planet-gen/populate_terrain_reference.py— 2380 / 3240 bodiespython3 tooling/planet-gen/generate_atlas.py— 267 bodies, 0 errors, atlas DB index populated{id, name, kind, center:[r,c], population})Replaces the hardcoded seed=0 with the seed received in StartupMessage, threading it through SimulationPlugin -> EconomyPlugin / SimRng. Integration test fixtures updated for the new SimulationPlugin { seed } signature.The generator and the hand-authored templates (Edict, Vuurkloof, Røros, Cairnside, Estrade) already store markers in heightmap pixel space with a grid header. Update §8 to match: {x, y} integer pixels are the storage format, and lat/lon strings become a display-time derivation in the atlas UI (synthesized from position + grid dimensions + body radius). Avoids double-conversion through an equirectangular projection and keeps the hand-authored markers.json files as-is.Implements the Phase 3 atlas content generator per D-191 §3, §8, and §9. Pipeline per body (terrain-aware, deterministic per seed + body): 1. Simulate terrain via planet_simulation.simulate(). 2. Analyse continents (flood-fill), habitability (temp/moisture/slope + coastal bonus), river mouths, and a terrain A* cost grid. 3. Place cities sequentially — capital first (habitability + river-mouth bias), then corridor growth via multi-source Dijkstra, quadrant-spread penalty after 2 cities in a quadrant, port-on-new-continent bonus at cities 3–4. ±25% noise for seed variation. 4. Generate roads and railroads as an MST over city positions, with A* paths on the terrain cost grid (rail follows roads where possible). 5. Place a transit POI at the capital (15% chance to scatter to a secondary city). Output (canonical markers.json schema, pixel space per D-191 §8): - cities: {id, name, kind, center:[r,c], population} - roads: {id, name, kind, path:[[r,c],...]} - railroads: {id, name, kind, path:[[r,c],...]} - pois: {id, name, kind, center:[r,c]} - existing rivers/oceans/mountain_ranges preserved untouched. City names are left empty for gemma_naming.py (#833). Body population is split across cities with geometric decay (capital ~50%, each subsequent city half the previous). The 6 hand-authored bodies (Lendel, Edict, Vuurkloof, Røros, Cairnside, Estrade) are detected by existing `cities` and skipped for regeneration; their markers are still synced to the DB index below. Atlas index in systems.db (new): - atlas_body_grids, atlas_cities, atlas_roads, atlas_railroads, atlas_pois, atlas_rivers, atlas_oceans, atlas_mountain_ranges - Scalar metadata mirror of every markers.json — the implant atlas app and development queries can lookup cities/POIs/features without scanning 267 JSON files. Polyline geometry stays in the markers.json files next to the heightmaps (used by the renderer); the DB only stores filterable scalar fields plus `point_count` as a length proxy. - Schema lives in server/data/systems-schema.sql; generate_atlas.py mirrors the CREATE TABLE IF NOT EXISTS block so it runs against any DB state (matches the economy-db importer pattern). - Populated and refreshed on every run. Each body's rows are deleted and reinserted deterministically — no stale state. Also fixes a pre-existing WIP bug in the quadrant-saturation penalty loop (a stray outer `for r in range(GRID_H)` with unreachable breaks meant only the NW quadrant was ever checked). Runtime: 280s for all 267 inhabited bodies on a single core. 265 bodies updated this run, 6 hand-authored bodies synced to DB without regeneration. Atlas index after run: atlas_cities 329 (15 hand-authored + 314 awaiting #833) atlas_roads 46 atlas_railroads 44 atlas_pois 287 atlas_rivers 2034 atlas_oceans 696 atlas_mountain_ranges 1953 atlas_body_grids 267PR Review — sprint-35/server → main (2-reviewer consolidated + team-lead spot checks)
Reviewers: Hoshe (code quality), Tyre (architecture)
Spot checks: batch schema scan, hand-authored preservation, determinism byte-equality
Verdict: CHANGES REQUESTED
Strong Sprint 35 server work. The Rust seed threading (#826) is clean end-to-end, the brand layer schema (#827) matches D-189 §5 exactly, the atlas pipeline (#832) is deterministic and terrain-aware, and the DB-index-mirrors-JSON architectural split is the right boundary. Determinism verified by re-running the generator: two fresh runs produce byte-identical output AND the committed branch state matches a fresh regen byte-for-byte.
The blockers are all in D-191 §8: the amendment prose declares a coordinate format and grid size that neither the generator nor the client actually use, and the amendment was written in-place without the audit-trail marker pattern established by D-094.
Pre-flight (for the record)
cargo clippy --lib: clean on both branch and main. Integration test errors (bridge_ipc,bridge_tcp,error_handling,gen_fixtures) are pre-existingObserverSnapshot.economy_snapshotbaseline, unchanged from main.ruff checkon all 3 Python files: clean.Team-lead spot checks
floor(log10(pop/1M))), 0 populated-body-empty-cities, 0 inhabited bodies missing markers. Max city count = 4.load_markers()correctly preserves them.generate_atlas.py --body GJ1116Ac --force --seed 42× 2 → identical. Committed branch output matches fresh regen byte-for-byte.D-decision compliance summary
brand_inputspoints brands at commodities; no brand_product_id appears as a commodity_id anywhere.brand_products,brand_inputs,system_fiscal,corp_financial_state,corp_lifecycle_events) present with correct columns. Composite index on(corp_id, brand_category)present.system_fiscal.collection_efficiencyhardcoded 0.85 instead of derived from1.0 - shadow_economy_intensity × 0.6. Acknowledged as Phase 3 wiring. See #3 below.corp_financial_state+corp_lifecycle_eventsare stub tables as intended.brand_category = cultural; only terroir/heritage_craft in brands.toml.QUADRANT_SATURATION=2, port-on-new-continent bonus, ±25% noise from seeded RNG.seed ^ body_id_salt(body_id),--forceflag,np.argmax+heapq((d,r,c))deterministic tie-breaking, SQLORDER BY body_id. Verified empirically (team-lead determinism check above).seed: 0survives in production paths. Test fixtures consistently useSimulationPlugin { seed: 0 }.Blocking — fix before merge
decisions/architecture.mdD-191 §8 vstooling/planet-gen/generate_atlas.py:754,845,1047vsmarkers-samples/GJ1116Ac_populated.json:566-586{x, y}integer-pixel objects and says the grid is "typically 1024 × 512". The generator emits"center": [41, 423]— a two-element array in[row, col]order (proof:col = 423 > 256 = grid.his impossible for an[x, y]reading; column namescenter_row/center_coland_first_int(center, …)/_first_int(center[1:], …)confirm row-first), andGRID_W=512, GRID_H=256is hardcoded — all 2394 sample files ship"grid": {"w": 512, "h": 256}. The generator and the client (PR #128) are both correct per the code; the decision doc prose is wrong. Rewrite §8 to: "every position is a two-element array[row, col]of integer pixels into thegrid: {w, h}header, whererow ∈ [0, h)andcol ∈ [0, w)" and update the schema bullet.decisions/architecture.mdD-191 §8**Amendment (date):**audit marker. Compare the D-094 amendment pattern on main (architecture.md:48—**Amendment (2026-04-05):** …superseded by D-094…). §8 was simply replaced, leaving no record of what it said originally or why it changed. Prepend an**Amendment (2026-04-13):**block to §8 that (a) cites the original lat/lon + population_tier wording as aspirational, (b) explains that the generator and hand-authored templates already operate in pixel space, (c) preserves lat/lon as explicit display-time derivation. Process hygiene for a load-bearing D-record.Warnings — fix before merge (material correctness / drift risk)
import_economics.py:807–827system_fiscal.collection_efficiencydocstring cites the D-189 §6 formula (1.0 - shadow_economy_intensity × 0.6) but the implementation hardcodes 0.85 for every system. Forshadow_economy_intensity = 0(current default) the formula gives 1.0, not 0.85. Phase 3 wiring is understood to be deferred, but the docstring actively misleads future implementors. Either remove the formula from the docstring or replace it with"hardcoded to 0.85 pending shadow_economy.toml pipeline (#TICKET)", and flag a follow-up ticket.import_economics.py:964–976conn.execute("DELETE FROM ...")calls + the subsequentexecutemanyINSERTs across all steps run as separate SQLite auto-commit statements. A crash between step 3 (commodities cleared) and step 9 (brands inserted) leaves the DB with some tables empty and others intact — no rollback possible. Wrap everything from the first DELETE throughconn.commit()in an explicitwith conn:block (or savepoint). Sinceimport_economics.pyruns asmake economy-dbin CI, transient errors are plausible.import_economics.py:732–739, 742–804VALID_BRAND_CATEGORIES,VALID_VALUE_TRAJECTORIES,VALID_SCARCITY_CLASSES,VALID_BRAND_TIERS,VALID_CURRENCY_DENOMINATIONSare defined but never referenced.import_brands()builds rows with no enum validation, and the SQL columns are plainTEXTwithout CHECK constraints. A typo likebrand_category = "terrior"silently imports. V-B01..V-B05 validation covers FK/halo integrity but not enum membership. Add a V-B06 that asserts each enum column is in the corresponding VALID set.systems-schema.sql:349–424ON DELETE CASCADEon atlas_ foreign keys.*atlas_cities.body_id REFERENCES bodies(body_id)and 6 peer tables have no cascade rule, andgenerate_atlas.sync_markers_to_db()only wipes rows for bodies it re-processes. A body deleted frombodies, or one whoseterrain_referenceis set to NULL (dropping it fromquery_inhabited_bodies), leaves orphan atlas_* rows forever. AddON DELETE CASCADEto all seven atlas_* FKs; consider a--pruneflag on generate_atlas.py for the NULL-terrain_reference case.generate_atlas.py:89–185vssystems-schema.sql:333–424systems-schema.sqlandgenerate_atlas.py'sATLAS_MIGRATION_SQL. The inline comment acknowledges "keep in sync" — this is exactly the kind of drift risk that gets away. Either (a) read the canonical schema file and extract theatlas_*CREATE statements at generator startup, or (b) move the atlas block toschemas/atlas_index.sqlthat both files reference. Polish today, blocker in six months.generate_atlas.py:441seed_rngparameter to_analyse_terraindeclared but never used. The function acceptsnp.random.Generatorbut does not call it — river mouth deduplication uses a deterministicnp.zerosmask, not rng. Creates a false API contract (callers might assume terrain analysis consumes rng state and factor that into sequencing). Remove the parameter and update the call site at line 1174.generate_atlas.py:1116–1205process_bodydoesn't validate that regenerated city_coords are unique. If two cities land on identical(row, col)— rare but possible with small grids + saturation — the MST treats them as zero-distance nodes and A* produces an empty path, silently skipping the edge. Add a uniqueness check afterplace_cities; on violation, log a warning and deterministically perturb.Makefile:331–332atlas-generatehas no declared prerequisite oneconomy-db. Per the D-191 §9 pipeline,generate_atlas.pyrequiresterrain_referencepopulated first (viapopulate_terrain_reference.py, a follow-on toeconomy-db). Runningmake atlas-generateon a fresh DB silently processes zero bodies and exits with empty-looking success. Addatlas-generate: economy-dbor a loud guard whenterrain_reference IS NOT NULLcount is 0.generate_atlas.py:1055–1070load_markers()doesn't validate the loaded grid header against generator constants. If a hand-authored template ships withgrid: {w: 1024, h: 512}and the generator overlays new cities computed againstGRID_W=512, GRID_H=256, the coordinates are half-scale in the final file and every marker is broken. Assert the loaded grid matches the generator constants, or read the generator constants from the loaded grid.Polish
architecture.mdD-191 §8 (or a new D-record)main.rs:181SimRng::new(seed)re-insertion is undocumented for the reader in isolation. The comment says "defensive override in case plugin ordering shifts" — fine. But if plugin order shifts and someone removes line 181 without understanding the history, the regression is silent. Cross-reference which plugin could initialize beforeSimulationPluginand consumeSimRng.generate_atlas.py:813binary_dilation(analysis["land_mask"] == False)— idiomatic NumPy is~analysis["land_mask"]. Line 524 usessurface_waterdirectly; consistency helps.generate_atlas.py:638–647gaussian_filtercalls inside_score_capital_sites. Each iteration allocatestmp = np.zeros((GRID_H, GRID_W))and runs a separate filter. At ~100 river mouths per body this is 100× the work of one filter over a single sparse accumulator. Buildtmpwith all mouth points set at once, then onegaussian_filtercall. Pure perf polish.brands.toml:1–276populate_terrain_reference.py:48–53vssystems-schema.sql:170terrain_referencepath format is convention-only; no D-record defines it aswiki/star-systems/{slug}/bodies/{body_id}/heightmap.png. Three pipelines (populate, atlas, client) silently drift if the directory structure changes. Short D-record or expanded schema comment pinning the format.generate_atlas.py --body GJ... --force --seed 42× 2, diff).Re-review cadence
2 blockers (both in D-191 §8 amendment prose — a 10-minute fix), 9 warnings, 7 polish items. Once §8 is rewritten, most of the heavy lifting is already correct in code (determinism verified empirically, preservation working, schema compliant with D-189). Re-review should be quick.
- V-B06 enum validation: the five VALID_* sets (VALID_BRAND_CATEGORIES, VALID_VALUE_TRAJECTORIES, VALID_SCARCITY_CLASSES, VALID_BRAND_TIERS, VALID_CURRENCY_DENOMINATIONS) were defined but never referenced. brand_products.brand_category etc. are plain TEXT with no CHECK constraints, so a typo like `brand_category = "terrior"` silently imported. `validate_brands` now runs a V-B06 pass that asserts every enum column is a member of its VALID_* set. V-B01..V-B05 + V-B06 all reported together on import failure. - Explicit transaction wrapper: the clear-then-reimport cycle (10 DELETEs followed by 9 imports and structural validation) used to depend on Python's implicit-deferred-transaction semantics and sys.exit() on validation failure. A crash mid-import could leave the DB with some tables empty and others intact. The body now runs inside `conn.execute("BEGIN")` + try/except with an explicit `_ImportAborted` for validation failures and a `BaseException` catch-all for KeyboardInterrupt / programmer errors. All failure paths rollback before exit; the commit only fires after structural validation passes. Dry-run leaves the transaction open so the coverage check below can still SELECT against in-memory state. - system_fiscal docstring: previously cited the D-189 §6 derived formula (`collection_efficiency = 1.0 - shadow_economy_intensity × 0.6`) while the implementation hardcodes `collection_efficiency = 0.85` for every system. The docstring now explicitly states these are Phase 2 placeholder values (with named constants PHASE2_CORP_TAX_RATE and PHASE2_COLLECTION_EFFICIENCY) and calls out the shadow_economy.toml pipeline as the Phase 3 follow-up.- ON DELETE CASCADE added to every atlas_* foreign key (atlas_body_grids, atlas_cities, atlas_roads, atlas_railroads, atlas_pois, atlas_rivers, atlas_oceans, atlas_mountain_ranges). Previously, deleting a body from the bodies table or NULL-ing its terrain_reference would leave orphan atlas rows forever — sync_markers_to_db only cleans up for bodies it re-processes. The existing atlas tables in systems.db were dropped and recreated with the new constraint; FK list now reports CASCADE. - Atlas DDL deduplicated. systems-schema.sql is now the single source of truth, bracketed by `-- BEGIN ATLAS INDEX` / `-- END ATLAS INDEX` markers. generate_atlas.py reads that block via `_load_atlas_schema()` and applies it at runtime, so there is no second copy of the DDL to keep in sync. Adding a column requires one edit, not two. - Uniqueness guard on city coordinates. `_enforce_unique_city_coords` runs at the end of `place_cities` and deterministically perturbs any duplicate (row, col) via a fixed spiral walk to the first free walkable land cell. Rare in practice but the MST collapses to a zero-distance edge otherwise, producing an empty A* path and silently dropping the road. - Grid header validation. `load_markers` now raises `AtlasGridMismatch` if the loaded `grid: {w, h}` header does not match `GRID_W`/`GRID_H`. Both the incremental-skip path and the regenerate path route through this loader, so a hand-authored template shipping a different grid size fails loud with a per-body error rather than silently producing half-scale coordinates. - Unused `seed_rng` parameter removed from `_analyse_terrain`. The function is RNG-free (continent flood-fill, habitability scoring, river-mouth dedup, cost grid — all pure functions of terrain). The false API contract made it look like terrain analysis consumed RNG state and had to be sequenced with downstream RNG use. - `_score_capital_sites` river-mouth bonus now builds one sparse accumulator with all mouth points set at once and runs a single `gaussian_filter` call, instead of O(n_mouths) filter calls over single-point images. - `binary_dilation(analysis["land_mask"] == False)` replaced with the idiomatic `~analysis["land_mask"]`, matching the convention used elsewhere in the file. - `atlas-generate` Makefile target now guards on `SELECT COUNT(*) FROM bodies WHERE terrain_reference IS NOT NULL`. On a fresh DB that count is 0 and the generator previously exited "success" after processing zero bodies. The target now fails loud with a pointer to `populate_terrain_reference.py`. - `main.rs` SimRng defensive re-insertion gains a long comment explaining the exact plugin-ordering hazard it guards against, so future readers don't treat the line as dead code. Tied to #826.Review fixes pushed — 3 commits
56d524f3docs(decisions) •64bb83f7fix(simulation) •d21c6902fix(tooling)Blockers
[row, col]arrays (not{x, y}objects), grid is512 × 256(not "1024 × 512"), row-first ordering is called out explicitly so readers can cross-reference the NumPy/flood-fill/A*/cost-grid conventions. Full top-level schema documented (cities, roads, railroads, pois, rivers, oceans, mountain_ranges).56d524f3Amendment (2026-04-15)block on top, superseded 2026-04-10 prose preserved verbatim below as "Original (superseded)". Audit trail intact.56d524f3Warnings
system_fiscal.collection_efficiencydocstring rewritten. The D-189 §6 formula reference is removed; the docstring now explicitly names the Phase 2 placeholder values (PHASE2_CORP_TAX_RATE = 0.22,PHASE2_COLLECTION_EFFICIENCY = 0.85) and points to the shadow_economy.toml pipeline as the Phase 3 follow-up.64bb83f7main()now runsBEGIN→ try block →commit()only after structural validation passes._ImportAbortedexception for validation failures triggersrollback()+sys.exit(1); aBaseExceptioncatch-all rolls back onKeyboardInterrupt/MemoryError/ programmer errors. Dry-run leaves the transaction open so the coverage check can still SELECT against in-memory state.64bb83f7validate_brandsnow asserts every enum column (brand_category,value_trajectory,scarcity_class,brand_tier,currency_denomination) is a member of itsVALID_*set. A typo likebrand_category = "terrior"now fails withV-B06: brand_product 'foo' has brand_category='terrior' — must be one of [...]. Success log updated toV-B01–V-B06.64bb83f7ON DELETE CASCADEadded to every atlas_* FK (atlas_body_grids,atlas_cities,atlas_roads,atlas_railroads,atlas_pois,atlas_rivers,atlas_oceans,atlas_mountain_ranges). Existing atlas tables in systems.db were dropped and recreated with the new constraint —PRAGMA foreign_key_list(atlas_cities)now reportsCASCADE.d21c6902systems-schema.sqlis the single source of truth, bracketed by-- BEGIN ATLAS INDEX/-- END ATLAS INDEXmarkers.generate_atlas.pynow calls_load_atlas_schema()which extracts that block at runtime and passes it toexecutescript. Adding a column requires one edit, not two.d21c6902seed_rngparameter removed from_analyse_terrain. Docstring clarifies the function is RNG-free (terrain analysis is deterministic by construction) and all per-body variation comes from downstream placement.d21c6902_enforce_unique_city_coordsruns at the end ofplace_citiesand deterministically perturbs any duplicate(row, col)via a fixed spiral walk to the first free walkable land cell. Emits awarning:line so the event is visible.d21c6902atlas-generateMakefile guard. The target now runs aSELECT COUNT(*) FROM bodies WHERE terrain_reference IS NOT NULLcheck before invoking Python. On a fresh DB where that count is 0, it prints an error pointing topopulate_terrain_reference.pyand exits 1.d21c6902load_markers. NewAtlasGridMismatchexception raised when the loadedgrid: {w, h}disagrees withGRID_W/GRID_H. Both the incremental-skip path and the regenerate path go through the same loader, so a hand-authored template shipping a different grid fails loud per-body instead of silently producing half-scale coordinates.d21c6902Polish
56d524f3main.rs:181SimRng defensive re-insertion: added a long comment naming the exact plugin-ordering hazard it guards against, with a checklist of what to audit if someone removes the line. Cross-references #826.d21c6902binary_dilation(analysis["land_mask"] == False)→binary_dilation(~analysis["land_mask"]).d21c6902_score_capital_sitesriver-mouth bonus now builds one sparse accumulator with all mouth points set at once and runs a singlegaussian_filtercall, replacing the O(n_mouths) loop.d21c6902brands.tomlheader gains a Phase 2 boundary note explaining that the 4 anchor brands are from D-189 §5 and the other ~23 from D-189 §11 are deliberately deferred to Phase 3.56d524f3systems-schema.sqlbodies.terrain_referencecomment now pins the repo-root-relative path convention so the three downstream pipelines (populate, atlas, client loader) share a documented contract.56d524f3Validation
cargo clippy --manifest-path server/Cargo.toml -- -D warnings— cleancargo test --manifest-path server/Cargo.toml --lib— 1150 / 1150 passruff check tooling/planet-gen/ tooling/economy-db/— cleanmake economy-db— brand import +V-B01–V-B06validation OKpython3 tooling/planet-gen/generate_atlas.py— 267 bodies synced, 0 errors, atlas index row counts unchanged from prior run (329 / 46 / 44 / 287 / 2034 / 696 / 1953)PRAGMA foreign_key_list(atlas_cities)confirmsCASCADEnow presentReady for re-review.
PR Review Round 2 — sprint-35/server → main
Reviewers: Hoshe (code quality), Tyre (architecture)
Verdict: APPROVED (2/2)
Fix commit chain (
56d524f3 docs/§8 amendment → 64bb83f7 brand importer hardening → d21c6902 atlas generator, schema, Makefile) cleanly resolves every R1 finding. The most architecturally load-bearing piece — the D-191 §8 amendment — follows the D-094 precedent precisely: dated**Amendment (2026-04-15):**marker, original aspirational prose preserved as**Original (2026-04-10, superseded):**, and the current shape explicit. The on-disk format now matches the prose exactly ([row, col]arrays againstgrid: {w: 512, h: 256}), with the storage grid / render resolution distinction drawn cleanly so §2's 1024×512 PNG reference is not a silent contradiction.D-decision re-compliance
architecture.md:713amendment marker + preserved original + current shape explicit.[row, col]arrays against{w:512, h:256}storage grid. NumPy convention documented inline._enforce_unique_city_coordsuses deterministic fixed-spiral ordering;_load_atlas_schemareads canonical DDL at startup (no drift).Per-finding resolution
{x, y}vs[row, col], 1024×512 vs 512×256)architecture.md:713–727amendment block explicitly canonizes[row, col]arrays against{w:512, h:256}. "Storage grid" terminology cleanly distinguishes from §2's 1024×512 render-resolution PNGs. Footer line 754 also updated.architecture.md:713**Amendment (2026-04-15):**prefix matches D-094 precedent at architecture.md:48. Original aspirational prose preserved at line 714 under**Original (2026-04-10, superseded):**.VALID_*enum sets defined but never referencedimport_economics.py:941–962V-B06 scans every enum column viaSELECT brand_product_id, {column} FROM brand_productsand checks membership against the VALID sets. Doc comment explains why (no CHECK constraints on TEXT columns).ON DELETE CASCADEon atlas_* FKssystems-schema.sql:355, 363, 375, 385, 395, 406, 415, 427— all 7 atlas_* tables plusatlas_body_gridshave CASCADE.generate_atlas.py:83–125_load_atlas_schema()readsBEGIN/END ATLAS INDEXmarkers fromsystems-schema.sqlat startup. Markers enforced in SQL at lines 342 and 449. Single source of truth, no drift risk.process_bodydoesn't validate unique city_coordsgenerate_atlas.py:804–853_enforce_unique_city_coordsuses deterministic fixed-spiral offsets withcity_indexordering and land-mask walking. Degenerate fallback logged. Called fromplace_citiesat 792.load_markers()doesn't validate grid headergenerate_atlas.py:1058–1091raisesAtlasGridMismatch(defined at 1048) on mismatch. Caller sites at 1173, 1220 catch it.collection_efficiencyhardcoded 0.85 (Hoshe + Tyre)import_economics.py:814–826explicit doc block: formula deferred pending per-systemshadow_economy_intensityfromshadow_economy.tomlpipeline, tracked as Phase 3 follow-up.import_economics.py:1000explicitconn.execute("BEGIN")._ImportAbortedpath at 1106 rolls back.BaseExceptionpath at 1113 rolls back + re-raises. Single commit at 1098.seed_rngdead parameter in_analyse_terrain(Hoshe)generate_atlas.py:376signature is nowdef _analyse_terrain(terrain: dict) -> dict:with noseed_rng. Call site at 1199 updated.atlas-generateno prerequisite oneconomy-db(Hoshe)Makefile:332–343guard block queriesCOUNT(*) FROM bodies WHERE terrain_reference IS NOT NULL, fails loudly with explicit error +exit 1if zero. Not a make dependency, but a runtime guard — CI-enforceable.gaussian_filtercalls in_score_capital_sites(Tyre)generate_atlas.py:578–586single sparsemouth_fieldaccumulator + onegaussian_filtercall. Inline comment documents.brands.tomlno Phase 2 boundary note (Tyre)brands.toml:10–15explicit header note naming the 4 anchor brands and citing D-189 §11 for the deferred ~23.terrain_referencepath format convention-only (Tyre)systems-schema.sql:170–178expanded schema comment pins the format, names all three downstream consumers, warns about the coupling.main.rs:181defensive re-insertion comment undocumented (Hoshe)main.rs:179–190names the exact threat scenario (any future plugin that registers before SimulationPlugin and consumes SimRng), explains the ordering, cites #826 for context.binary_dilationNumPy idiom inconsistency (Hoshe)generate_atlas.py:753nowbinary_dilation(~analysis["land_mask"]). Both call sites consistent.Regressions
None. The +58/+31/+14 line churn on
import_economics.py,generate_atlas.py, andsystems-schema.sqlis all load-bearing fix work.Observations (not blocking)
conn.executescript(MIGRATION_SQL)atimport_economics.py:1007issues an implicit commit before the explicitBEGIN, so the DDL migration runs outside the transaction. Since the DDL is idempotentCREATE IF NOT EXISTS, practical risk is zero. Worth a polish follow-up if strict correctness is desired (run migration viaconn.execute()instead ofexecutescript).architecture.md:667still says "2,394 heightmap PNGs (1024×512 equirectangular, production quality)". §8's amendment uses "storage grid" language that implicitly separates this from render resolution, so no contradiction — but a future editor could misread it. A one-line parenthetical in §2 like"(analyzed and markered at 512×256 storage grid — see §8)"would make the relationship explicit. Follow-up polish.PR #129 is approved for merge.
Pull request closed