jpmschweitzer/settled-reach
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(simulation): dispatch RegionProfile layer in production (T-1032) #159

Closed
jpmschweitzer wants to merge 0 commits from region-profile-dispatch into main
pull from: region-profile-dispatch
merge into: :main
Conversation 1 Commits - Files Changed -
jpmschweitzer commented 2026-06-08 12:38:14 +02:00
Owner
Copy Link
Copy Source

Summary

T-1032 — make the D-239 carrier layer actually run in production. The carrier foundation (T-1023/1024/1026, merged) wired the cascade to run the RegionProfile layer when AnalyzeBody carries body_params, but every production enqueue passed None, so it only ran in unit tests. This wires the real DB read.

Changes

  • server/src/atlas/body_params_reader.rs (new) — BodyParamsReader: read-only systems.db connection, one query per body joining bodies → star_systems (LEFT JOIN) for the climate/tectonic inputs. SQL verified against systems-schema.sql (bodies.system_id REFERENCES star_systems(system_id); all queried columns exist). Every field Option, NULLs handled; tectonic_activity is absent from the schema → left None (derivation falls back to planet_class). 5 unit tests.
  • layer_proxy.rs — on a cache miss, read the body's params and pass Some(Box::new(..)). On read error → tracing::warn! + fall back to None (cascade stops at Settlement, no panic) — graceful degradation for unknown bodies / DB errors.
  • plugin.rs / main.rs — register BodyParamsReaderResource (same pattern as CityContextReaderResource) and thread it through serve_atlas_requests.

Result: BodyWorldState.regions now populates for real bodies in the D-206 background pass.

Tests

  • body_params_reader: all-columns, all-NULL, unknown-body error, body-without-system-row (LEFT JOIN), determinism.
  • layer_proxy: body_params_reader_wired_produces_populated_regions (end-to-end) + no_body_params_reader_leaves_regions_empty (fallback).

Verification

cargo test 1504 pass, clippy --all-targets -D warnings clean, fmt clean (pre-push gate, incl. the cargo test step, ran on push).

No schema/generator change — reads existing columns (axial_tilt_deg landed with the merged T-1024). Closes the production-dispatch gap flagged in PR #158.

Ticket: T-1032 (epic T-974, Phase 4).

🤖 Generated with Claude Code

## Summary **T-1032** — make the D-239 carrier layer actually run in production. The carrier foundation (T-1023/1024/1026, merged) wired the cascade to run the RegionProfile layer *when `AnalyzeBody` carries `body_params`*, but every production enqueue passed `None`, so it only ran in unit tests. This wires the real DB read. ### Changes - **`server/src/atlas/body_params_reader.rs`** (new) — `BodyParamsReader`: read-only `systems.db` connection, one query per body joining `bodies → star_systems` (LEFT JOIN) for the climate/tectonic inputs. SQL verified against `systems-schema.sql` (`bodies.system_id REFERENCES star_systems(system_id)`; all queried columns exist). Every field `Option`, NULLs handled; `tectonic_activity` is absent from the schema → left `None` (derivation falls back to `planet_class`). 5 unit tests. - **`layer_proxy.rs`** — on a cache miss, read the body's params and pass `Some(Box::new(..))`. **On read error → `tracing::warn!` + fall back to `None`** (cascade stops at Settlement, no panic) — graceful degradation for unknown bodies / DB errors. - **`plugin.rs` / `main.rs`** — register `BodyParamsReaderResource` (same pattern as `CityContextReaderResource`) and thread it through `serve_atlas_requests`. Result: `BodyWorldState.regions` now populates for real bodies in the D-206 background pass. ### Tests - `body_params_reader`: all-columns, all-NULL, unknown-body error, body-without-system-row (LEFT JOIN), determinism. - `layer_proxy`: `body_params_reader_wired_produces_populated_regions` (end-to-end) + `no_body_params_reader_leaves_regions_empty` (fallback). ### Verification `cargo test` 1504 pass, `clippy --all-targets -D warnings` clean, `fmt` clean (pre-push gate, incl. the `cargo test` step, ran on push). No schema/generator change — reads existing columns (`axial_tilt_deg` landed with the merged T-1024). Closes the production-dispatch gap flagged in PR #158. Ticket: T-1032 (epic T-974, Phase 4). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jpmschweitzer added 1 commit 2026-06-08 12:38:14 +02:00
feat(simulation): dispatch RegionProfile layer in production (T-1032) 1887c886ef 
The D-239 carrier layer (T-1023/1024/1026) only ran in tests because every
production AnalyzeBody enqueue passed body_params: None. Wire the real path:

- New BodyParamsReader (server/src/atlas/body_params_reader.rs): read-only
  systems.db reader, joins bodies -> star_systems (LEFT JOIN) for the climate/
  tectonic inputs. SQL verified against systems-schema.sql. All fields Option,
  NULLs handled; tectonic_activity absent from schema -> None (derives from
  planet_class). 5 unit tests.
- layer_proxy.rs: on cache miss, read the body's params and pass
  Some(Box::new(..)). On read error, warn + fall back to None (cascade stops at
  Settlement, no panic) — graceful degradation.
- plugin.rs / main.rs: register BodyParamsReaderResource (CityContextReader
  pattern) and thread it through serve_atlas_requests.

BodyWorldState.regions now populates for real bodies in the D-206 background
pass. End-to-end tests cover wired (regions populated) + unwired (empty) paths.

cargo test 1504 pass, clippy -D warnings clean, fmt clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jpmschweitzer added 1 commit 2026-06-08 12:51:11 +02:00
test+docs(simulation): address PR #159 review (T-1032) 0a8933fbcc 
- Hoshe: body_without_system_row test inserted a NULL system_id, impossible
  under production schema (system_id NOT NULL REFERENCES star_systems). Rewrite
  as body_with_orphan_system_id_* — non-NULL system_id with no matching
  star_systems row (the real case the LEFT JOIN guards) + NOT NULL in the test
  schema + corrected comment.
- Tyre: cascade.rs PERF/TODO comment said the region path 'defers to T-1032';
  T-1032 IS this PR, so production dispatch is now live. Update to track T-1028
  only and note the cost is live in production.

No production logic change. cargo test body_params_reader 7 pass, clippy clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jpmschweitzer commented 2026-06-08 12:51:34 +02:00
Author
Owner
Copy Link
Copy Source

Review: region-profile-dispatch → main (type: code)

Reviewers: Hoshe (code quality) + Tyre (architecture). Source read from the branch worktree. No merge-path surface. Gate green (cargo test 1504, clippy -D warnings, fmt).

Hoshe (Code Quality): REQUEST_CHANGES → resolved

SQL, column names, and JOIN logic all verified correct against the real schema; production wiring sound end-to-end. One test-fixture issue.

Tyre (Architecture): APPROVE

Clean, correctly-scoped application of the D-225 pre-dispatch reader pattern — mirrors CityContextReader faithfully, cascade stays DB-free in the Rayon work item, body_params.is_some() gate is the right switch, graceful error→None→Settlement fallback, end-to-end tests at the right fidelity. One stale comment noted.

Findings & resolution (0a8933fb)

# Reviewer Finding Resolution
1 Hoshe body_without_system_row test inserted a NULL system_id — impossible in production (bodies.system_id TEXT NOT NULL REFERENCES star_systems). The LEFT JOIN actually guards an orphan FK (non-NULL system_id with no matching star_systems row), and the test/comment mislabeled it. Rewrote as body_with_orphan_system_id_returns_null_stellar_fields — non-NULL orphan system_id, NOT NULL in the test schema, corrected comment.
2 Tyre cascade.rs PERF/TODO said the region path "defers to T-1032" — but this PR is T-1032, so production dispatch is now live. Updated: tracks T-1028 only (the TerrainAnalysis-caching optimization), and notes the drainage-re-run cost is now a live production cost.

Tyre's other notes (connection model, error fallback, registration, gate, tests) were all positive confirmations, not change requests.

Verdict: APPROVED (after resolution)

Both findings were test/comment-only — no production logic changed. Production code was approved by Tyre and SQL-verified by Hoshe. Merging.

🤖 Generated with Claude Code

## Review: region-profile-dispatch → main (type: code) Reviewers: **Hoshe** (code quality) + **Tyre** (architecture). Source read from the branch worktree. No merge-path surface. Gate green (cargo test 1504, clippy `-D warnings`, fmt). ### Hoshe (Code Quality): REQUEST_CHANGES → resolved SQL, column names, and JOIN logic all verified correct against the real schema; production wiring sound end-to-end. One test-fixture issue. ### Tyre (Architecture): APPROVE Clean, correctly-scoped application of the D-225 pre-dispatch reader pattern — mirrors `CityContextReader` faithfully, cascade stays DB-free in the Rayon work item, `body_params.is_some()` gate is the right switch, graceful error→`None`→Settlement fallback, end-to-end tests at the right fidelity. One stale comment noted. ### Findings & resolution (`0a8933fb`) | # | Reviewer | Finding | Resolution | |---|----------|---------|------------| | 1 | Hoshe | `body_without_system_row` test inserted a **NULL `system_id`** — impossible in production (`bodies.system_id TEXT NOT NULL REFERENCES star_systems`). The LEFT JOIN actually guards an **orphan FK** (non-NULL `system_id` with no matching `star_systems` row), and the test/comment mislabeled it. | Rewrote as `body_with_orphan_system_id_returns_null_stellar_fields` — non-NULL orphan `system_id`, `NOT NULL` in the test schema, corrected comment. | | 2 | Tyre | `cascade.rs` PERF/TODO said the region path "defers to T-1032" — but this PR **is** T-1032, so production dispatch is now live. | Updated: tracks **T-1028** only (the TerrainAnalysis-caching optimization), and notes the drainage-re-run cost is now a live production cost. | Tyre's other notes (connection model, error fallback, registration, gate, tests) were all positive confirmations, not change requests. ### Verdict: APPROVED (after resolution) Both findings were test/comment-only — no production logic changed. Production code was approved by Tyre and SQL-verified by Hoshe. Merging. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
jpmschweitzer closed this pull request 2026-06-08 12:53:48 +02:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
jpmschweitzer (Jeroen Schweitzer)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: jpmschweitzer/settled-reach#159