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

feat(simulation): FillChunk shell derivation (T-987) #171

Closed
jpmschweitzer wants to merge 0 commits from fillchunk-shell-derive into main
pull from: fillchunk-shell-derive
merge into: :main
Conversation 1 Commits - Files Changed -
jpmschweitzer commented 2026-06-28 08:28:29 +02:00
Owner
Copy Link
Copy Source

Summary

Implements the D-230 on-demand FillChunk derive phase (T-987) — the building-shell half of the cascade chunk fill. New atlas/shell.rs derives a sparse {Void|Wall|FloorSlab|Roof} shell for one 64 m chunk from the plan-phase BuildingPropertyTags: rectangle-containment (footprint within chunk) times z-range (per-floor bands from FloorExtent), in a quarter-ground z frame (D-110). Walls on the footprint perimeter, floor slabs on interior floor bases, a roof above the top floor; interior air is Void and never stored.

Changes

  • atlas/shell.rs (new): ShellVoxel, sparse FilledChunk (D-010), pure fill_chunk().
  • gen_queue.rs: FillChunk work item carries pre-resolved block_tags (run_work_item stays cache-free, mirroring GenerateSkeleton); ChunkFilled carries the FilledChunk; pure build_fill_chunk_item().
  • plugin.rs: ChunkFilled handler accepts the shell (trace-only).

Scope

Data-production half only. The on-demand streaming dispatch trigger (the stale T-959 note about wiring chunk_streaming.rs) is deferred to Phase 5 — chunk_streaming.rs is legacy Phase-5 rendering code (CLAUDE.md forbids building on it pre-Phase-5; gated by T-962). Surface-material vocabulary (D-235) = T-988; condition overlay = T-999.

Tests

determinism, sparsity-vs-dense-volume, wall/floor/roof correctness, basement z-origin, sub-chunk clipping, full submit-drain-ChunkFilled round-trip. clippy -D warnings clean, 1571 lib tests pass.

Ticket: T-987

## Summary Implements the D-230 on-demand FillChunk derive phase (T-987) — the building-shell half of the cascade chunk fill. New atlas/shell.rs derives a sparse {Void|Wall|FloorSlab|Roof} shell for one 64 m chunk from the plan-phase BuildingPropertyTags: rectangle-containment (footprint within chunk) times z-range (per-floor bands from FloorExtent), in a quarter-ground z frame (D-110). Walls on the footprint perimeter, floor slabs on interior floor bases, a roof above the top floor; interior air is Void and never stored. ## Changes - atlas/shell.rs (new): ShellVoxel, sparse FilledChunk (D-010), pure fill_chunk(). - gen_queue.rs: FillChunk work item carries pre-resolved block_tags (run_work_item stays cache-free, mirroring GenerateSkeleton); ChunkFilled carries the FilledChunk; pure build_fill_chunk_item(). - plugin.rs: ChunkFilled handler accepts the shell (trace-only). ## Scope Data-production half only. The on-demand streaming dispatch trigger (the stale T-959 note about wiring chunk_streaming.rs) is deferred to Phase 5 — chunk_streaming.rs is legacy Phase-5 rendering code (CLAUDE.md forbids building on it pre-Phase-5; gated by T-962). Surface-material vocabulary (D-235) = T-988; condition overlay = T-999. ## Tests determinism, sparsity-vs-dense-volume, wall/floor/roof correctness, basement z-origin, sub-chunk clipping, full submit-drain-ChunkFilled round-trip. clippy -D warnings clean, 1571 lib tests pass. Ticket: T-987
jpmschweitzer added 1 commit 2026-06-28 08:28:29 +02:00
feat(simulation): FillChunk shell derivation — Void/Wall/FloorSlab/Roof from cached tags (T-987) 5f85dc55d5 
Implement the D-230 on-demand derive phase (data-production half). New
atlas/shell.rs derives a sparse building shell for one 64 m chunk from the
plan-phase BuildingPropertyTags: rectangle-containment (footprint ∩ chunk) ×
z-range (per-floor voxel bands from FloorExtent), in a quarter-ground z frame
(D-110). Walls on the footprint perimeter, FloorSlab on interior floor bases,
Roof above the top floor; interior air is Void and never stored.

- gen_queue.rs: GenWorkItem::FillChunk carries pre-resolved block_tags +
  block_pos + sub_chunk (run_work_item stays cache-free, mirroring
  GenerateSkeleton); GenCompletion::ChunkFilled carries Box<FilledChunk>; the
  arm calls shell::fill_chunk; pure build_fill_chunk_item(&QuarterWorldState,..)
  added.
- plugin.rs: ChunkFilled handler accepts the shell (trace-only — the Phase-5
  rendering consumer and the on-demand streaming dispatch trigger are deferred;
  do not build on legacy chunk_streaming.rs before Phase 5, gated by T-962).

Tests: determinism, sparsity-vs-dense-volume, wall/floor/roof correctness,
basement z-origin, sub-chunk clipping, full submit→drain→ChunkFilled round-trip.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jpmschweitzer added 1 commit 2026-06-28 08:29:19 +02:00
chore(meta): T-987 -> review (PR #171) e3dcf57054 
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
jpmschweitzer added 1 commit 2026-06-28 10:25:31 +02:00
fix(simulation): address PR #171 review — all findings processed (T-987) 7597e3165e 
Every review finding fixed (no non-blocking parking lot):
- shell.rs: debug_assert the D-110 floor-range i8 invariant (H1); clarify the
  ground_offset fallback comment (T1) and the roof_z None-case (T3); clarify the
  sub-chunk clip test comment (T4); add a test for the elevated/no-ground-floor
  fallback (H2).
- scale.rs: add CHUNKS_PER_BLOCK (= BLOCK_M/CHUNK_M) + compile-time asserts; used
  by shell.rs fill_chunk's sub_chunk bounds assert (T5).
- gen_queue.rs: document why FillChunk's Vec<BuildingPropertyTag> needs no Box
  (large_enum_variant non-issue) (T2).

clippy --all-targets -D warnings clean; 1572 lib tests pass (+1).

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

Review: fillchunk-shell-derive → main (type: code) — APPROVED, all findings processed

Reviewers: Hoshe (code quality) + Tyre (architecture), both APPROVE. Branch read via git show origin/fillchunk-shell-derive:…; .pql/changelog/* excluded.

Both reviewers found the core implementation correct and faithful to D-230: the z-origin math (D-110), the footprint∩chunk clip, perimeter/slab/roof classification, the cache-free run_work_item with tags pre-resolved into the work item (mirroring GenerateSkeleton), D-010 determinism (integer + BTreeMap), D-227 re-derivability, and D-243 scale math. No security/panic concerns.

Per the zero-suggestion-tier rule, every finding was processed — fixed in commit 7597e3165, none parked:

# Reviewer Finding Resolution
H1 Hoshe i8 floor-range truncation unguarded debug_assert! pins the invariant in shell_derive_into
H2 Hoshe base_floor>0 (no floor-0) fallback untested added elevated_building_with_no_ground_floor_anchors_at_its_own_bottom test
T1 Tyre ground_offset comment conflated two zeroes comment rewritten to explain the no-shift fallback
T2 Tyre FillChunk Vec payload sizing unexplained doc added on why no Box is needed (vs AnalyzeBody)
T3 Tyre roof_z None-case undocumented comment added on the well-formed-extent guarantee
T4 Tyre clip test comment misleading clarified that chunk-local == block-local only at sub_chunk (0,0)
T5 Tyre no CHUNKS_PER_BLOCK constant added to scale.rs (+ compile-time asserts); used by fill_chunk's sub_chunk bounds assert

Post-fix gate: clippy --all-targets -D warnings clean; 1572 lib tests pass (+1).

Verdict: APPROVED — ready to merge.

## Review: fillchunk-shell-derive → main (type: code) — APPROVED, all findings processed Reviewers: **Hoshe** (code quality) + **Tyre** (architecture), both **APPROVE**. Branch read via `git show origin/fillchunk-shell-derive:…`; `.pql/changelog/*` excluded. Both reviewers found the core implementation correct and faithful to D-230: the z-origin math (D-110), the footprint∩chunk clip, perimeter/slab/roof classification, the cache-free `run_work_item` with tags pre-resolved into the work item (mirroring `GenerateSkeleton`), D-010 determinism (integer + `BTreeMap`), D-227 re-derivability, and D-243 scale math. No security/panic concerns. Per the zero-suggestion-tier rule, **every finding was processed** — fixed in commit `7597e3165`, none parked: | # | Reviewer | Finding | Resolution | |---|----------|---------|------------| | H1 | Hoshe | `i8` floor-range truncation unguarded | `debug_assert!` pins the invariant in `shell_derive_into` | | H2 | Hoshe | `base_floor>0` (no floor-0) fallback untested | added `elevated_building_with_no_ground_floor_anchors_at_its_own_bottom` test | | T1 | Tyre | `ground_offset` comment conflated two zeroes | comment rewritten to explain the no-shift fallback | | T2 | Tyre | FillChunk `Vec` payload sizing unexplained | doc added on why no `Box` is needed (vs `AnalyzeBody`) | | T3 | Tyre | `roof_z` None-case undocumented | comment added on the well-formed-extent guarantee | | T4 | Tyre | clip test comment misleading | clarified that chunk-local == block-local only at sub_chunk (0,0) | | T5 | Tyre | no `CHUNKS_PER_BLOCK` constant | added to `scale.rs` (+ compile-time asserts); used by `fill_chunk`'s sub_chunk bounds assert | Post-fix gate: `clippy --all-targets -D warnings` clean; **1572 lib tests pass** (+1). ### Verdict: APPROVED — ready to merge.
jpmschweitzer closed this pull request 2026-06-28 10:59:55 +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#171