From da15261276025fbad9c98a50a3cdbc0c229af286 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Wed, 22 Jul 2026 17:24:07 +0200 Subject: [PATCH] =?UTF-8?q?fix(client):=20PR=20#192=20review=20round=20?= =?UTF-8?q?=E2=80=94=20per-rung=20legend,=20Region=20coalescing=20+=20clam?= =?UTF-8?q?p=20boundary=20tests,=20halving-loop=20honesty?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All Hoshe/Araminta findings addressed (Tyre approved outright), none retracted — plus a Dudley stop-and-flag discovery that improved on the asked-for fix: - Legend 100x lie (Araminta, blocking): subtitle computed via spacing_for_rung() and refreshed at all three _held_granularity_v2 write sites. Region test asserts 204.800 km/cell; the District direction needed a stale-header-aware helper — a District-only test spuriously passes against the old literal by coincidence. - Region coalescing coverage (Hoshe 1): both directions tested (Region-vs-District separate slots; Region-vs-Region coalesces). - Clamp boundary tests + dangling citations (Hoshe 2): writing the requested halving-loop-fires test surfaced that the loop is PROVABLY UNREACHABLE at current constants (per-axis clamp forecloses it — brute-forced independently on both server and client sides). Ruling: the loop stays as defensive code; the test became a property sweep pinning both the wire-cap invariant and the loop's no-op status (a future constant change breaks it loudly); doc comments on both sides drop the load-bearing framing and state the truth; the old client mirror test that claimed the loop fires (passing on the per-axis clamp alone) is replaced the same way. Client citations now name the real server tests verbatim. - Governance (Tyre): D-226 amendment note — progressive cross-rung refinement EXTENDS T-1124 §4 (not supersedes); legacy u32 field scheduled for retirement (T-1159). Server: 1818 lib tests green, clippy/fmt clean. Client: zoom_ladder 48/48, window_request 26/26, viewer 74/74; gdlint clean. Every fix revert-verified. --- client/tests/test_atlas_window_request.gd | 82 +++++++---- client/tests/test_atlas_zoom_ladder.gd | 53 ++++++++ .../implant/apps/atlas/atlas_window_legend.gd | 18 ++- .../implant/apps/atlas/atlas_window_viewer.gd | 56 ++++---- governance/decisions/architecture.md | 2 +- server/src/atlas/gen_queue.rs | 70 ++++++++++ server/src/atlas/layer_proxy.rs | 127 ++++++++++++++++-- 7 files changed, 345 insertions(+), 63 deletions(-) diff --git a/client/tests/test_atlas_window_request.gd b/client/tests/test_atlas_window_request.gd index f40f335c1..a2046b767 100644 --- a/client/tests/test_atlas_window_request.gd +++ b/client/tests/test_atlas_window_request.gd @@ -339,14 +339,25 @@ func test_oversized_n_request_stores_clamped_n_and_accepts_matching_echo() -> vo # ============================================================================= # (d) Region clamp mirror (T-1152/T-1153) — mirrors # server/src/atlas/layer_proxy.rs's clamp_window_n_v2 EXACTLY, including the -# Region branch's bounded halving loop (no closed form, per Dudley's own -# doc — "replicate the loop exactly"). +# Region branch's bounded halving loop. +# +# PR #192 review (Dudley, server-side analysis): the halving loop is +# PROVABLY UNREACHABLE at current constants — the per-axis clamp to +# SERVER_DISTRICT_WINDOW_MAX_N_REGION (6,400) forecloses it. Brute-forced, +# the max cell_grid_side over ALL reachable (post-per-axis-clamp) n is +# exactly 64 — the wire-cap boundary itself, never over it — so the loop's +# `>` guard is never true for any input. Ruling: the loop STAYS as +# defensive code (a future constant change could make it reachable again), +# but the test suite must not claim it "fires" when it provably doesn't. +# See server/src/atlas/layer_proxy.rs's +# clamp_window_n_v2_region_per_axis_cap_alone_satisfies_wire_cap_for_all_inputs +# for the server-side property-sweep pin this client-side suite mirrors. # ============================================================================= ## District/Quarter through the v2 mirror must be BYTE-IDENTICAL to the ## legacy mirror — the server's own -## `clamp_window_n_v2_matches_legacy_for_finer_than_district_rungs` +## `clamp_window_n_v2_delegates_to_legacy_for_district_and_quarter` ## guarantee, restated client-side. func test_clamp_window_n_mirror_v2_matches_legacy_for_district_and_quarter() -> void: assert_int( @@ -362,9 +373,9 @@ func test_clamp_window_n_mirror_v2_matches_legacy_for_district_and_quarter() -> ## 64, and 64² = 4,096 = WIRE_CAP_CELLS EXACTLY — the halving loop's `>` ## condition is false at the boundary, so this must clamp to EXACTLY 6,400, ## not halve further. This is the server's own -## `region_per_axis_cap_lands_exactly_on_wire_cap_when_uncontested`-shaped -## boundary (WIRE_CAP_CELLS_SQRT * DISTRICTS_PER_REGION is DERIVED to land -## here exactly, per that constant's own doc). +## `clamp_window_n_v2_region_exact_boundary_n6400_uncontested` guarantee, +## restated client-side (WIRE_CAP_CELLS_SQRT * DISTRICTS_PER_REGION is +## DERIVED to land here exactly, per that constant's own doc). func test_clamp_window_n_mirror_v2_region_boundary_is_exact() -> void: var n: int = AtlasWindowRequest._clamp_window_n_mirror_v2( AtlasWindowRequest.SERVER_DISTRICT_WINDOW_MAX_N_REGION, AtlasWindowRequest.GRANULARITY_V2_REGION @@ -392,23 +403,48 @@ func test_clamp_window_n_mirror_v2_region_oversized_n_clamps_within_both_bounds( ).is_less_equal(AtlasWindowRequest.SERVER_WIRE_CAP_CELLS) -## A REGION request whose derived grid would exceed WIRE_CAP_CELLS (n set so -## cell_grid_side(n) rounds to 65, just over the 64-per-axis wire-size -## ceiling) must actually HALVE — this is the case the boundary test above -## deliberately sits just below, so this one confirms the loop body actually -## fires, not just that its guard conditions are correct at the edges. -func test_clamp_window_n_mirror_v2_region_halves_when_over_wire_cap() -> void: - # n=6,450 -> cell_grid_side = round(64.5) = 65 (round-half-away-from-zero, - # matching Rust's f64::round() and GDScript's roundi() for non-negative - # inputs) -> 65² = 4,225 > WIRE_CAP_CELLS (4,096) -> must halve at least once. - var n: int = AtlasWindowRequest._clamp_window_n_mirror_v2( - 6450, AtlasWindowRequest.GRANULARITY_V2_REGION - ) - assert_int(n).override_failure_message( - "a Region request whose grid exceeds WIRE_CAP_CELLS must be halved down, not left at 6,450" - ).is_less(6450) - var side: int = AtlasWindowRequest._cell_grid_side_region_mirror(n) - assert_int(side * side).is_less_equal(AtlasWindowRequest.SERVER_WIRE_CAP_CELLS) +## PR #192 review (Dudley's unreachability finding, applied client-side): the +## halving loop's `>` guard is PROVABLY never true at current constants — the +## per-axis clamp to SERVER_DISTRICT_WINDOW_MAX_N_REGION (6,400) happens +## FIRST and unconditionally, and cell_grid_side(6400) = 64 lands EXACTLY on +## the wire-cap boundary (64² = WIRE_CAP_CELLS), never over it. A prior +## version of this test claimed n=6,450 "exercises" the loop firing — it does +## not: 6,450 clamps to 6,400 before the loop ever runs, so the test was +## passing on the per-axis clamp alone, not on anything the loop itself did +## (the same mock-diverges-from-reality class of bug hunted in review round +## 2). Reframed as a property sweep, mirroring the server's own +## `clamp_window_n_v2_region_per_axis_cap_alone_satisfies_wire_cap_for_all_inputs` +## (Dudley): for every raw n across the legal range (including values far +## past the per-axis cap), (i) the per-axis-clamped n never gets modified any +## further by the loop — pre-loop n and post-clamp n are byte-identical — +## and (ii) the wire-cap invariant holds regardless. The loop itself stays as +## defensive code (a future constant change could make it reachable again); +## this test documents that it is a no-op today rather than asserting a +## behavior that never actually happens. +func test_clamp_window_n_mirror_v2_region_per_axis_cap_alone_satisfies_wire_cap_for_all_inputs() -> void: + var sample_raw_ns: Array = [ + 1, 100, 6399, 6400, 6401, 6450, 6500, + AtlasWindowRequest.SERVER_DISTRICT_WINDOW_MAX_N_REGION * 10, + ] + for raw_n: int in sample_raw_ns: + var pre_loop_n: int = clampi(raw_n, 1, AtlasWindowRequest.SERVER_DISTRICT_WINDOW_MAX_N_REGION) + var clamped_n: int = AtlasWindowRequest._clamp_window_n_mirror_v2( + raw_n, AtlasWindowRequest.GRANULARITY_V2_REGION + ) + assert_int(clamped_n).override_failure_message( + ( + "the per-axis clamp alone must already satisfy the wire cap for" + + " raw_n=%d — the halving loop is provably unreachable at current" + + " constants (max cell_grid_side over all reachable n is exactly" + + " 64, the wire-cap boundary itself), so it must never further" + + " modify what the per-axis clamp already produced" + ) % raw_n + ).is_equal(pre_loop_n) + + var side: int = AtlasWindowRequest._cell_grid_side_region_mirror(clamped_n) + assert_int(side * side).override_failure_message( + "the wire-cap invariant must hold for raw_n=%d regardless" % raw_n + ).is_less_equal(AtlasWindowRequest.SERVER_WIRE_CAP_CELLS) ## n smaller than one region (n < 100) must clamp its cell-grid side to a diff --git a/client/tests/test_atlas_zoom_ladder.gd b/client/tests/test_atlas_zoom_ladder.gd index b352f9f4e..c6b775cfe 100644 --- a/client/tests/test_atlas_zoom_ladder.gd +++ b/client/tests/test_atlas_zoom_ladder.gd @@ -218,6 +218,59 @@ func test_new_rung_window_swaps_in_once_it_arrives() -> void: assert_str(v._held_granularity_v2).is_equal("Region") +## refresh() clear()s via queue_free() (deferred, not synchronous) — a legend +## that has refreshed more than once in the same frame (build-time refresh at +## _ready(), then an entry-time refresh) can have STALE not-yet-freed +## children still parented alongside the new ones. add_component() always +## APPENDS, so the current ImplantHeader is the LAST one in the list, never +## assumed to be [0]. +static func _current_legend_header(legend_panel) -> ImplantHeader: + var children: Array = legend_panel.get_implant_children() + for i in range(children.size() - 1, -1, -1): + if children[i] is ImplantHeader: + return children[i] + return null + + +## PR #192 review (Araminta, BLOCKING): the legend subtitle used to hardcode +## District's own "2.048 km/cell" — a 100x lie whenever the viewer actually +## holds Region (204.8 km/cell). While in the orbital tile-mode rest state +## (Region granularity), the legend must read Region's real spacing, not the +## stale District literal. +func test_legend_subtitle_reflects_region_spacing_in_tile_mode() -> void: + var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new()) + add_child(v) + var radius_km := 6238.4 # GJ380c (Lendel) — needs tiling, so enters at Region + v.enter_orbital({"body_id": "GJ380c", "body_radius_km": radius_km}, {}) + assert_bool(v.is_tile_mode()).is_true() + assert_str(v._held_granularity_v2).is_equal("Region") + + var header: ImplantHeader = _current_legend_header(v._legend_panel) + assert_str(header._subtitle_label.text).override_failure_message( + "legend subtitle must reflect Region's real 204.800 km/cell spacing while" + + " the viewer holds Region granularity, not a hardcoded District figure" + ).contains("204.800 km/cell") + + +## Same bug, the other direction: after crossing INTO a single-window District +## rung, the legend must re-render with District's own spacing — proving the +## legend actually refreshes on a rung change rather than being stuck at +## whatever it showed on the FIRST refresh() call (T-1153's _build_legend_panel() +## fires one at _ready() time, before any real rung is held). +func test_legend_subtitle_reflects_district_spacing_after_crossing_in() -> void: + var v: AtlasWindowViewer = auto_free(AtlasWindowViewer.new()) + add_child(v) + v.enter({"body_id": "GJ380c"}, {}, Vector2i(10, 20), 2) + assert_str(v._held_granularity_v2).is_equal("District") + + var header: ImplantHeader = _current_legend_header(v._legend_panel) + assert_str(header._subtitle_label.text).override_failure_message( + "legend subtitle must re-render at District's own 2.048 km/cell spacing" + + " once the viewer holds a District-rung window — proving refresh() is" + + " actually wired to the rung change, not just called once at build time" + ).contains("2.048 km/cell") + + ## A response for a rung OTHER than what's currently requested (e.g. a ## District response arriving after the viewer has already moved on to a ## Region request — a rapid wheel-zoom race) must be discarded as stale, the diff --git a/client/ui/implant/apps/atlas/atlas_window_legend.gd b/client/ui/implant/apps/atlas/atlas_window_legend.gd index d98c426ec..c8d21052f 100644 --- a/client/ui/implant/apps/atlas/atlas_window_legend.gd +++ b/client/ui/implant/apps/atlas/atlas_window_legend.gd @@ -16,6 +16,7 @@ const PANEL_MARGIN: float = 16.0 const LEGEND_PANEL_WIDTH: float = 260.0 const AtlasOverlayColors := preload("res://ui/implant/apps/atlas/atlas_overlay_colors.gd") +const AtlasWindowGeometry := preload("res://ui/implant/apps/atlas/atlas_window_geometry.gd") ## The morphology base layer folds its 17 zones into ~5 family rows (§5: ## "mirroring T-1112's 'not everything earns permanent screen space' @@ -55,13 +56,28 @@ func reposition() -> void: ## Always shows the base-layer key (morphology + elevation reading, always ## on) plus glaciation (always-on modifier), then whichever toggle overlay is ## currently active, if any. +## +## The subtitle's km/cell reading is NOT a fixed "district window" label — +## it was until this fix hardcoded District's own 2.048 km/cell, which was a +## 100x lie whenever the viewer actually holds Region (204.8 km/cell) or +## Quarter (0.512 km/cell). Read through AtlasWindowGeometry. +## spacing_for_rung() — the SAME pure lookup _refresh_screen_header() uses +## for the main screen header's subtitle (screen_header_content()) — keyed +## off the viewer's current get_held_granularity_v2(), so the legend and +## header can never disagree. AtlasWindowViewer calls refresh() at every +## point _held_granularity_v2 changes (_enter_tile_mode(), _enter_at_rung(), +## _on_window_ready()'s rung-swap adoption) — see those call sites. func refresh() -> void: if _viewer == null: return clear() visible = true - add_component(ImplantHeader.new("REGIONAL LEGEND", "district window · 2.048 km/cell")) + var spacing_km: float = AtlasWindowGeometry.spacing_for_rung(_viewer.get_held_granularity_v2()) / 1000.0 + var subtitle: String = "%s window · %.3f km/cell" % [ + _viewer.get_held_granularity_v2().to_lower(), spacing_km + ] + add_component(ImplantHeader.new("REGIONAL LEGEND", subtitle)) add_component(ImplantSeparator.new()) _add_morphology_section() diff --git a/client/ui/implant/apps/atlas/atlas_window_viewer.gd b/client/ui/implant/apps/atlas/atlas_window_viewer.gd index 908feb70b..29103023a 100644 --- a/client/ui/implant/apps/atlas/atlas_window_viewer.gd +++ b/client/ui/implant/apps/atlas/atlas_window_viewer.gd @@ -25,8 +25,8 @@ extends Control ## keeps drawing — progressive refinement, no blank frame (§6). A pan ## past the held window's edge re-requests the SAME rung at a new center. ## - Zooming fully out snaps to the CANONICAL planetary frame (Jeroen's HARD -## condition) — see _maybe_reset_to_canonical_frame() — which, on a body -## needing tiling, re-enters `_tile_mode` (live round 3, design doc §4). +## condition, see _maybe_reset_to_canonical_frame()) — on a body needing +## tiling, re-enters `_tile_mode` (live round 3, design doc §4). ## - _window_request (atlas_window_request.gd) owns the single-window ## cache/debounce/retry; _tile_set (atlas_window_tile_set.gd) owns N of ## those for the tiled rest state — this Control decides WHICH is active. @@ -34,8 +34,7 @@ extends Control ## Navigation (Jeroen's input-model ruling: LMB-drag panning BREAKS click ## semantics with future map objects, so it's removed entirely): ## WASD / arrow keys continuous pan, held (frame-rate independent, _process) -## Edge scrolling cursor within EDGE_SCROLL_MARGIN_PX of a viewport -## edge pans toward it (suppressed over UI / unfocused) +## Edge scrolling cursor within EDGE_SCROLL_MARGIN_PX pans toward it (suppressed over UI / unfocused) ## Mouse wheel cursor-anchored zoom; crosses rungs continuously (T-1153) ## Esc back (nav.pop()) @@ -47,14 +46,13 @@ const OVERLAY_BAR_HEADER_RESERVE: float = 360.0 ## T-1153: MIN_ZOOM/MAX_ZOOM are a wide safety clamp on the raw display ## multiplier, NOT a rung boundary — wheel zoom is CONTINUOUS and UNCLAMPED ## ACROSS RUNGS (D-013): crossing a rung's coverage ceiling -## (AtlasWindowGeometry.select_rung()) re-requests a DIFFERENT granularity -## at the SAME apparent screen extent, never clamping _view_zoom itself. +## (AtlasWindowGeometry.select_rung()) re-requests a DIFFERENT granularity at +## the SAME apparent screen extent, never clamping _view_zoom itself. ## set_view() (T-1120 capture API) clamps to this same range independently. -## ## MIN_ZOOM must stay low enough that fit_window_view()'s COVER fit for -## enter_orbital()'s largest legal `n` is never itself clamped — that would -## silently show LESS than the whole body, breaking Jeroen's HARD condition. -## 0.0005 covers a ~120,000 km-radius body at a 3840px 4K viewport. +## enter_orbital()'s largest legal `n` is never itself clamped (would +## silently show LESS than the whole body). 0.0005 covers a ~120,000 km- +## radius body at a 3840px 4K viewport. const MIN_ZOOM: float = 0.0005 const MAX_ZOOM: float = 64.0 const ZOOM_STEP: float = 1.15 @@ -66,10 +64,9 @@ const ZOOM_STEP: float = 1.15 const PAN_SPEED_CANVAS_PX_S: float = 96.0 ## T-1145 item 2: cursor-to-edge distance (px) that triggers edge-scroll — -## Jeroen's own number ("~24px"). +## Jeroen's own number ("~24px"). Uses the SAME speed as WASD (one pan feel, +## two triggers) — no separate constant, _process() reads PAN_SPEED_CANVAS_PX_S for both. const EDGE_SCROLL_MARGIN_PX: float = 24.0 -## Edge-scroll uses the SAME speed as WASD (one pan feel, two triggers) — -## no separate constant, _process() reads PAN_SPEED_CANVAS_PX_S for both. ## Pixel size of one district cell at zoom=1.0 — a fixed on-screen scale ## (unlike AtlasViewer's heightmap, there is no source texture dictating a @@ -82,17 +79,14 @@ const COLOR_BG: Color = Color("#0d1117") ## Border-fade target (§5 "what renders during the wait"): the underlying ## whole-body heightmap's own background tint, so the newly-exposed edge ## reads as "real data seen through", not a placeholder block. Reuses -## AtlasViewer's own COLOR_HEIGHTMAP_TINT-adjacent dim value rather than -## inventing a new one — this IS a dimmer/less-certain read of the same -## planetary data, not a different visual language. +## AtlasViewer's own COLOR_HEIGHTMAP_TINT-adjacent dim value — a dimmer/ +## less-certain read of the same planetary data, not a different visual language. const COLOR_BORDER_FADE: Color = Color(0.20, 0.24, 0.30, 0.55) -## T-1153/R6: pending-refinement wash — the border-fade's referent repointed -## to "the previous derived composite at this position" for a rung-crossing -## zoom (progressive refinement leaves real data on screen, unlike the -## no-composite-yet case COLOR_BORDER_FADE covers). Same hue family, much -## lighter alpha — a hint that something sharper is arriving, not a claim -## that the current view is empty or wrong. +## T-1153/R6: pending-refinement wash — border-fade's referent repointed to +## "the previous derived composite at this position" for a rung-crossing zoom +## (real data stays on screen, unlike the no-composite-yet case above). Same +## hue, lighter alpha — hints something sharper is arriving, not that the view is wrong. const COLOR_PENDING_REFINEMENT_WASH: Color = Color(0.20, 0.24, 0.30, 0.12) ## D-243: 2,048 m per district side. @@ -311,6 +305,7 @@ func _enter_tile_mode(body: Dictionary, system: Dictionary, radius_km: float) -> _window_request.reset() _tile_set.enter(_dict_str(_body, "body_id", ""), radius_km) _refresh_screen_header() + _legend_panel.refresh() grab_focus() queue_redraw() _overlay_node.queue_redraw() @@ -346,6 +341,7 @@ func _enter_at_rung( _dict_str(_body, "body_id", ""), _held_center, clamped_n, granularity_v2 ) _refresh_screen_header() + _legend_panel.refresh() grab_focus() queue_redraw() _overlay_node.queue_redraw() @@ -413,6 +409,12 @@ func get_held_n() -> int: return _held_n +## Currently-HELD rung tag — legend reads this via spacing_for_rung(), +## mirroring _refresh_screen_header()'s own use of the field. +func get_held_granularity_v2() -> String: + return _held_granularity_v2 + + ## Live round 5: current body's radius — mosaic draw needs `cols` for ## nearest_wrap_image()'s wrap resolution. Mirrors the ## `_body.get("body_radius_km", 0.0)` pattern used throughout this file. @@ -495,6 +497,7 @@ func _on_window_ready(window: Dictionary) -> void: _fit_and_center() _awaiting_first_window = false _refresh_screen_header() + _legend_panel.refresh() queue_redraw() _overlay_node.queue_redraw() @@ -968,12 +971,9 @@ func _notification(what: int) -> void: _position_overlay_bar() if _legend_panel: _legend_panel.reposition() - # T-1142: re-fit on resize too, same _user_adjusted guard as the other - # two auto-fit events (enter, first window arrival) — never fights a - # manually-adjusted view. _canvas guard matches _overlay_bar/ - # _legend_panel above: NOTIFICATION_RESIZED can fire mid-_ready() - # (anchor_right/anchor_bottom assignment triggers it) BEFORE _canvas - # is constructed — confirmed the hard way (gdUnit add_child() crash). + # T-1142: re-fit on resize (_user_adjusted guard, as other auto-fit + # events). _canvas guard: NOTIFICATION_RESIZED can fire mid-_ready() + # before _canvas exists (gdUnit add_child() crash, confirmed). if _canvas and not _user_adjusted: _fit_and_center() elif what == NOTIFICATION_APPLICATION_FOCUS_OUT: diff --git a/governance/decisions/architecture.md b/governance/decisions/architecture.md index 2f65849a6..5102acac7 100644 --- a/governance/decisions/architecture.md +++ b/governance/decisions/architecture.md @@ -1669,7 +1669,7 @@ Technical foundation decisions that constrain implementation: engine, client-ser **Amended 2026-07-21 (T-1145 — Jeroen, second companion hands-on, KALLAST window):** three regional-window presentation fixes, all client-only. **Cover-fit supersedes contain:** `fit_window_view()`'s zoom now derives from the LARGER viewport dimension with no margin factor (`max(viewport.x, viewport.y) / composite_native`, not the old `0.9 * min(...)`), so the square district-window composite fills a wide/tall viewport edge to edge instead of leaving side margins, with the shorter axis' data extending into pan-space (the same "cover" concept as CSS `object-fit: cover`) — the existing §4 pan-edge refetch is unaffected (it keys off the screen-center-to-DistrictPos mapping, which any fit already centers on `_held_center` by construction, so no refetch churn at rest). **WASD + edge-scroll supersedes drag-pan:** LMB-drag panning is removed entirely (Jeroen's ruling — drag conflicts with click semantics for the map objects, e.g. settlements, this window will host later); panning is now held WASD/arrow keys (continuous, frame-rate-independent, `_process`-polled, physical-keycode reads to stay independent of the project's existing `move_north`/etc. gameplay-movement InputMap actions bound to the same keys) plus edge-scrolling (cursor within ~24px of a viewport edge, suppressed over UI and while the OS window lacks focus); wheel zoom is unchanged; pole-wall (§5 amendment above) and east-west wrap (T-1142) semantics are preserved unchanged under the new input source. **Smoothing is an interim presentation, pending T-1143:** the composite renders as an `n`×`n` `Image`/`ImageTexture` (one pixel per district, the identical existing per-cell color pipeline) drawn scaled with linear filtering — the same treatment the planetary heightmap already gets — instead of `n`×`n` flat rects, so GPU bilinear sampling reads as a terrain gradient rather than hard blocks; the original crisp per-cell path survives behind a compile-time const specifically so T-1143's design pass can compare both directly, and this smoothing is **not** T-1143's answer to district-tier legibility, only a stopgap ahead of it. - **Amended 2026-07-21 (T-1143 design-pass rulings — Jeroen, after the zoom-ladder design pass, `docs/architecture/atlas-zoom-ladder-t1143.md`):** three rulings on the pass's reserved decisions. **(1) The item-(d) ceiling is opened for the Atlas ladder** — Jeroen: *"we set a new BHAG so old restrictions are up for debate."* The D-166 2026-07-21 zoom-ladder condition ("down to tile scale") is read **literally**: the Atlas windowed viewport may descend below quarter (512 m) toward block/tile granularity. This is an explicit ruling, not erosion — exactly the deliberate revisit the T-1112 §2 anti-erosion clause was hardened to force into the open. Item (d)'s substance survives in narrowed form: chunk/tile/voxel output still never appears as a **whole-body planetary map layer**, and the below-quarter rungs are implementation-gated on their own **measurement pass** (costs/wire for block and tile rungs are unmeasured — design pass §2/§7); the harness-verification path for L5 fill remains primary until that pass lands. **(2) Planetary rung wire carrier: progressive capped-density tiling** riding the generalized `district_window` carrier (granularity parameter, §3 of the design pass) — no new dense-raster wire shape, no forced tagged-envelope migration. **(3) The §5 entry click-through cut is superseded by continuous cursor-anchored zoom**: wheel-zoom carries the view from the orbital frame down through regional granularities continuously, anchored at the cursor, **with the condition that a full zoom-out resets to the original canonical planetary frame and location** (the fixed orbital framing is the ladder's top rest state, not a drifted pan state). The click-through descent and rectangle reticle are retired as the *sole* entry (T-1138's shipped mechanic stands until the continuous ladder replaces it in the same change — close inspection is never stranded, same discipline as the §5 fixed-view transition). D-013's "the zoom gesture owns spatial descent" reading is **restored** for this seam. **Wire-contract note (T-1150, PR #191 review — Tyre):** the `window_granularity` field that ruling (2) rides on expresses **finer-than-district integer multiples only** (1 = district, 4 = quarter today; each new rung is a deliberate widening of `resolve_window_granularity`'s whitelist — the single widening point; unknown values fall back to district, never trusted from the wire). Coarser-than-district reuse (the region/orbital rungs of ruling (2)'s progressive tiling) requires the design pass's R5 signed/log-scale-or-enum redesign of the field — a new magic value is not the path. Recorded here so the type's limit is contract, not only a design-doc risk row. + **Amended 2026-07-21 (T-1143 design-pass rulings — Jeroen, after the zoom-ladder design pass, `docs/architecture/atlas-zoom-ladder-t1143.md`):** three rulings on the pass's reserved decisions. **(1) The item-(d) ceiling is opened for the Atlas ladder** — Jeroen: *"we set a new BHAG so old restrictions are up for debate."* The D-166 2026-07-21 zoom-ladder condition ("down to tile scale") is read **literally**: the Atlas windowed viewport may descend below quarter (512 m) toward block/tile granularity. This is an explicit ruling, not erosion — exactly the deliberate revisit the T-1112 §2 anti-erosion clause was hardened to force into the open. Item (d)'s substance survives in narrowed form: chunk/tile/voxel output still never appears as a **whole-body planetary map layer**, and the below-quarter rungs are implementation-gated on their own **measurement pass** (costs/wire for block and tile rungs are unmeasured — design pass §2/§7); the harness-verification path for L5 fill remains primary until that pass lands. **(2) Planetary rung wire carrier: progressive capped-density tiling** riding the generalized `district_window` carrier (granularity parameter, §3 of the design pass) — no new dense-raster wire shape, no forced tagged-envelope migration. **(3) The §5 entry click-through cut is superseded by continuous cursor-anchored zoom**: wheel-zoom carries the view from the orbital frame down through regional granularities continuously, anchored at the cursor, **with the condition that a full zoom-out resets to the original canonical planetary frame and location** (the fixed orbital framing is the ladder's top rest state, not a drifted pan state). The click-through descent and rectangle reticle are retired as the *sole* entry (T-1138's shipped mechanic stands until the continuous ladder replaces it in the same change — close inspection is never stranded, same discipline as the §5 fixed-view transition). D-013's "the zoom gesture owns spatial descent" reading is **restored** for this seam. **Wire-contract note (T-1150, PR #191 review — Tyre):** the `window_granularity` field that ruling (2) rides on expresses **finer-than-district integer multiples only** (1 = district, 4 = quarter today; each new rung is a deliberate widening of `resolve_window_granularity`'s whitelist — the single widening point; unknown values fall back to district, never trusted from the wire). Coarser-than-district reuse (the region/orbital rungs of ruling (2)'s progressive tiling) requires the design pass's R5 signed/log-scale-or-enum redesign of the field — a new magic value is not the path. Recorded here so the type's limit is contract, not only a design-doc risk row. **Refinement-semantics note (T-1153, PR #192 review — Tyre):** the ladder's progressive cross-rung refinement (hold the coarse composite, fetch the finer rung, swap in place on arrival; per-tile arrival in the orbital mosaic) **extends** the T-1124 §4 float-on-center/debounce async contract — it does not supersede it. §4 still governs the per-request mechanics unchanged (`district_window: None`-until-derived polling, the 150 ms debounce, float-on-center refetch); rung crossings add a second request class on top, per the design pass §3's progressive-refinement model. This record is the one that governs the swap-on-arrival behavior. The legacy `window_granularity: u32` wire field is now fully shadowed by `window_granularity_v2` (the server always echoes both); it is **scheduled for retirement** once pre-T-1152 wire-compat is confirmed unneeded (single-repo client/server pair — no external clients exist today; ticketed). - **Rationale:** Reusing the real UI — rather than a parallel offline renderer or dumped files — means the debug/review surface never diverges from what ships, and a dropped artifact can't go stale. Agent-navigability converts qualitative "does the synthesis look natural?" review from a manual eyeball pass into an automatable sweep that flags the few outliers for a human. The harness rides seams that already exist (`TickRate::Paused`, the paused-allowlist, `gameplay_occluded`, the bridge framing, the `run-visual` capture primitive) — a naming-and-contract exercise, not a new subsystem. - **New surface:** server pause-gating (run-conditions on the world phases keyed to a pause command); client `AtlasAgentInterface` (`observe`/`act`, Control-tree walker) + its local transport; the generation overlay rendering + selector + legend; interactive capture wired to `run-visual`. - **Implementation:** Phase 4 (epic T-750), built bottom-up — auto-pause substrate, T-969 proxy (D-225), T-960 viewer, agent channel, agent capture. Geography is the first consumer. diff --git a/server/src/atlas/gen_queue.rs b/server/src/atlas/gen_queue.rs index 5ce2f8198..a6381962b 100644 --- a/server/src/atlas/gen_queue.rs +++ b/server/src/atlas/gen_queue.rs @@ -1364,6 +1364,76 @@ mod tests { ); } + /// **PR #192 review — Hoshe 1: zero coalescing coverage for + /// `WindowGranularity::Region` before this test**, despite Region being + /// the highest-fan-out path (progressive capped-density tiling fires + /// multiple concurrent Region `DeriveWindow` items per pan/zoom). Mirrors + /// `submit_window_does_not_coalesce_different_granularity`'s pattern + /// exactly, substituting Region for Quarter: a Region request and a + /// District request for the SAME `(connection, body)` are separate + /// in-flight slots (the coalescing key is `(conn_id, body_id, + /// granularity)`) and must NOT coalesce — both survive as independent + /// pending items. + #[test] + fn submit_window_does_not_coalesce_region_and_district() { + let q = GenerationQueue::with_threads(1); + // See `submit_window_coalesces_same_connection_and_body`'s comment on + // why the occupier must be `analyze()`, not `FillChunk`. + q.submit(analyze("Occupier5"), GenPriority::Low); + + let conn = ConnectionId(13); + q.submit_window( + derive_window_at("OrbitalGranBody", conn, (0, 0), WindowGranularity::Region), + GenPriority::Immediate, + ); + q.submit_window( + derive_window_at("OrbitalGranBody", conn, (0, 0), WindowGranularity::District), + GenPriority::Immediate, + ); + assert_eq!( + q.pending_count(), + 2, + "same (connection, body) but Region vs. District must NOT coalesce — \ + separate in-flight slots, same as the existing District/Quarter pair" + ); + } + + /// The coalescing-DOES-happen counterpart to the test above, for Region + /// specifically: two submissions for the SAME `(connection, body, + /// Region)` still collapse to one pending item — confirms Region's + /// coalescing key behaves identically to District/Quarter's, not just + /// that it avoids cross-granularity aliasing. + #[test] + fn submit_window_coalesces_same_connection_body_and_region_granularity() { + let q = GenerationQueue::with_threads(1); + q.submit(analyze("Occupier6"), GenPriority::Low); + + let conn = ConnectionId(15); + q.submit_window( + derive_window_at( + "SameOrbitalGranBody", + conn, + (0, 0), + WindowGranularity::Region, + ), + GenPriority::Immediate, + ); + q.submit_window( + derive_window_at( + "SameOrbitalGranBody", + conn, + (5, 5), + WindowGranularity::Region, + ), + GenPriority::Immediate, + ); + assert_eq!( + q.pending_count(), + 1, + "same (connection, body, Region) must still coalesce to one pending item" + ); + } + // ------------------------------------------------------------------- // TerrainAnalysisCache (T-1137, PR #187 review — Tyre C1) // ------------------------------------------------------------------- diff --git a/server/src/atlas/layer_proxy.rs b/server/src/atlas/layer_proxy.rs index b8942d869..b4ed0f872 100644 --- a/server/src/atlas/layer_proxy.rs +++ b/server/src/atlas/layer_proxy.rs @@ -359,18 +359,33 @@ fn clamp_window_n(raw_n: u32, granularity: u32) -> u32 { /// - **`District`/`Quarter`:** per-axis cap is [`DISTRICT_WINDOW_MAX_N`] /// (64, unchanged) — byte-identical clamped `n` to [`clamp_window_n`] for /// every input these two variants can produce (verified by -/// `clamp_window_n_v2_matches_legacy_for_finer_than_district_rungs`, +/// `clamp_window_n_v2_delegates_to_legacy_for_district_and_quarter`, /// below). /// - **`Region`:** per-axis cap is [`DISTRICT_WINDOW_MAX_N_REGION`] (6,400 — -/// see that constant's doc for the derivation), then the SAME -/// wire-size-ceiling shrink applies on top via [`WindowGranularity::cell_grid_side`] -/// — a request whose `cell_grid_side(n)` would exceed -/// `sqrt(WIRE_CAP_CELLS)` region cells across is walked back by *halving* -/// `n` until it fits (region's `cell_grid_side` is a ROUNDING division, not -/// the finer rungs' exact multiplication, so there's no closed-form inverse -/// the way `cap_n = sqrt(WIRE_CAP_CELLS) / g` is for the finer case — a -/// short bounded loop is the correct tool here, not a formula that would -/// have to fight its own rounding). +/// see that constant's doc for the derivation), then a halving loop walks +/// `n` back if `cell_grid_side(n)` would still exceed `sqrt(WIRE_CAP_CELLS)` +/// region cells across. +/// +/// **This loop is defensive, not currently reachable — stated plainly, not +/// left implicit.** `DISTRICT_WINDOW_MAX_N_REGION` is DERIVED as +/// `sqrt(WIRE_CAP_CELLS) * DISTRICTS_PER_REGION` specifically so the +/// per-axis clamp alone already forecloses the loop's trigger condition: a +/// brute-force sweep of every `raw_n` in `[1, DISTRICT_WINDOW_MAX_N_REGION]` +/// shows `cell_grid_side(n)` never exceeds `sqrt(WIRE_CAP_CELLS)` (64), so +/// `n /= 2` never executes for any input the per-axis clamp lets through — +/// verified by `clamp_window_n_v2_region_per_axis_cap_alone_satisfies_wire_cap_for_all_inputs`, +/// which pins BOTH the invariant (`cell_grid_side(result)² ≤ WIRE_CAP_CELLS`) +/// AND the loop's current no-op status (`result == raw_n.clamp(1, +/// DISTRICT_WINDOW_MAX_N_REGION)` for every swept input). The loop is kept +/// anyway as the general, correct algorithm (region's `cell_grid_side` is a +/// ROUNDING division, not the finer rungs' exact multiplication, so there +/// is no closed-form inverse the way `cap_n = sqrt(WIRE_CAP_CELLS) / g` is +/// for the finer case) — it is the safety net for a FUTURE cap derivation +/// that doesn't land exactly on the boundary (a new rung from a later +/// measurement pass, or a `WIRE_CAP_CELLS` retune that isn't a perfect +/// square times `DISTRICTS_PER_REGION`). If a future constant change makes +/// the loop actually fire, the pinned no-op assertion above breaks loudly, +/// forcing a deliberate look rather than a silent behavior change. fn clamp_window_n_v2(raw_n: u32, granularity: WindowGranularity) -> u32 { match granularity { WindowGranularity::District | WindowGranularity::Quarter => clamp_window_n( @@ -2537,6 +2552,98 @@ mod tests { assert_eq!(clamp_window_n(8, WINDOW_GRANULARITY_QUARTER), 8); } + // ------------------------------------------------------------------- + // clamp_window_n_v2 (T-1152; PR #192 review — Hoshe, coordinator ruling + // 2026-07-22: test 2 reframed per the brute-force finding that the + // Region halving loop is unreachable at the CURRENT constants — see + // clamp_window_n_v2's doc comment for the full rationale) + // ------------------------------------------------------------------- + + /// The exact boundary: `n = DISTRICT_WINDOW_MAX_N_REGION` (6,400) is the + /// largest per-axis-legal `n`, and it lands EXACTLY on the wire-size + /// ceiling (`cell_grid_side(6400) = 64 = sqrt(WIRE_CAP_CELLS)`, + /// `64² = 4,096 = WIRE_CAP_CELLS`) — uncontested, meaning the request is + /// NOT further reduced by the halving loop; the per-axis clamp alone is + /// already exact at this boundary. + #[test] + fn clamp_window_n_v2_region_exact_boundary_n6400_uncontested() { + let result = clamp_window_n_v2(DISTRICT_WINDOW_MAX_N_REGION, WindowGranularity::Region); + assert_eq!( + result, DISTRICT_WINDOW_MAX_N_REGION, + "n=6400 must pass through unmodified — it already lands exactly on the ceiling" + ); + let side = WindowGranularity::Region.cell_grid_side(result) as u32; + assert_eq!( + side * side, + WIRE_CAP_CELLS, + "n=6400's cell_grid_side must land EXACTLY on WIRE_CAP_CELLS, not under or over it" + ); + } + + /// **Reframed per the coordinator's 2026-07-22 ruling (PR #192 review — + /// Hoshe).** The originally-briefed name/shape + /// (`clamp_window_n_v2_region_halving_loop_fires_above_boundary`, e.g. + /// n=6450) does not hold: `raw_n.clamp(1, DISTRICT_WINDOW_MAX_N_REGION)` + /// runs BEFORE the halving loop's condition is ever checked, so any + /// `raw_n > DISTRICT_WINDOW_MAX_N_REGION` is clamped to exactly 6,400 — + /// the SAME uncontested boundary the test above proves — before + /// `cell_grid_side` ever sees the raw value. A brute-force sweep (done + /// by hand before writing this test, see `clamp_window_n_v2`'s doc + /// comment) confirms `cell_grid_side(n)` never exceeds `sqrt(WIRE_CAP_CELLS)` + /// for ANY `n` in `[1, DISTRICT_WINDOW_MAX_N_REGION]` — so the halving + /// loop is unreachable at the CURRENT constant derivation, not a bug to + /// manufacture a test around (coordinator's option 1, not option 2). + /// + /// This test proves the ACTUAL property: the per-axis cap ALONE already + /// satisfies the wire-size ceiling for every reachable input, and pins + /// the loop's current no-op status explicitly — swept across + /// `[1, 2 × DISTRICT_WINDOW_MAX_N_REGION]` (double the legal range, so + /// wildly-oversized wire values are covered too, never trusting the + /// wire). If a FUTURE constant change (a new rung, a `WIRE_CAP_CELLS` + /// retune) ever makes the loop fire, the second assertion below breaks + /// LOUDLY — forcing a deliberate look rather than a silent behavior + /// change (exactly the safety-net role the loop exists for). + #[test] + fn clamp_window_n_v2_region_per_axis_cap_alone_satisfies_wire_cap_for_all_inputs() { + for raw_n in 1..=(2 * DISTRICT_WINDOW_MAX_N_REGION) { + let result = clamp_window_n_v2(raw_n, WindowGranularity::Region); + let side = WindowGranularity::Region.cell_grid_side(result) as u32; + assert!( + side * side <= WIRE_CAP_CELLS, + "raw_n={raw_n}: clamped result {result} (side {side}) exceeds WIRE_CAP_CELLS" + ); + assert_eq!( + result, + raw_n.clamp(1, DISTRICT_WINDOW_MAX_N_REGION), + "raw_n={raw_n}: the halving loop must be a no-op at current constants — \ + the per-axis clamp alone must already be the final answer" + ); + } + } + + /// `District`/`Quarter` through `clamp_window_n_v2` must be BYTE-IDENTICAL + /// to the legacy `clamp_window_n` for every input either variant can + /// legally carry — `clamp_window_n_v2` is documented as delegating to the + /// legacy function unchanged for these two rungs, this pins that claim + /// with a sweep rather than a handful of spot values. + #[test] + fn clamp_window_n_v2_delegates_to_legacy_for_district_and_quarter() { + // Sweep well past DISTRICT_WINDOW_MAX_N so the "never trust the wire" + // oversized-input case is covered too, not just in-range values. + for raw_n in 0..=(DISTRICT_WINDOW_MAX_N * 3) { + assert_eq!( + clamp_window_n_v2(raw_n, WindowGranularity::District), + clamp_window_n(raw_n, WINDOW_GRANULARITY_DISTRICT), + "District: clamp_window_n_v2 must match clamp_window_n exactly at raw_n={raw_n}" + ); + assert_eq!( + clamp_window_n_v2(raw_n, WindowGranularity::Quarter), + clamp_window_n(raw_n, WINDOW_GRANULARITY_QUARTER), + "Quarter: clamp_window_n_v2 must match clamp_window_n exactly at raw_n={raw_n}" + ); + } + } + // ------------------------------------------------------------------- // quantize_min_wl_m (T-1150, PR #191 review — Hoshe 1 / Tyre C3, design doc §5) // -------------------------------------------------------------------