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

fix(ui): Atlas header gets its implant panel scrim — legible on any terrain (T-1197) #217

Closed
jpmschweitzer wants to merge 0 commits from header-ghost-fix into main
pull from: header-ghost-fix
merge into: :main
Conversation 2 Commits - Files Changed -
+208 -7
jpmschweitzer commented 2026-07-26 14:54:47 +02:00
Owner
Copy Link
Copy Source

Summary

Batch-9 client track. The ticket suspected a fade-animation race; the investigation disproved it — no animation exists anywhere under client/ui/implant (grep-proven, and settle+2 vs settle+90 captures byte-identical). The real bug: _build_screen_header() was the ONLY ImplantHeader call site in the codebase adding the header bare instead of through ImplantPanel.add_component() — its fixed light-gray text washed out against pale terrain (Quarter upland scatter, District olive) while reading fine on dark ocean. One committed golden (atlas_GJ820Bc_District) already carried the ghosting baked in, proving the bug static.

Fix: wrap the header in an ImplantPanel like every sibling — the theme's panel_bg scrim makes it legible on any terrain. One-script change + all 13 goldens regenerated (header renders differently everywhere).

Test plan

  • Live verification at the original evidence center (GJ1002b Quarter, world 385024,-598016) via the AtlasAgentInterface path: header crisp at settle+2/+30/+90, captures archived
  • Golden regeneration: two independent live-server runs, diff -q identical (T-1157 two-run stability discipline)
  • Pre-push gate green (full gdUnit; one unrelated fog perf-test flake re-verified 46/46 solo and ticketed as T-1210 — 0.5ms budget has no headroom on a shared box)
## Summary Batch-9 client track. The ticket suspected a fade-animation race; the investigation disproved it — no animation exists anywhere under client/ui/implant (grep-proven, and settle+2 vs settle+90 captures byte-identical). The real bug: `_build_screen_header()` was the ONLY ImplantHeader call site in the codebase adding the header bare instead of through ImplantPanel.add_component() — its fixed light-gray text washed out against pale terrain (Quarter upland scatter, District olive) while reading fine on dark ocean. One committed golden (atlas_GJ820Bc_District) already carried the ghosting baked in, proving the bug static. **Fix:** wrap the header in an ImplantPanel like every sibling — the theme's panel_bg scrim makes it legible on any terrain. One-script change + all 13 goldens regenerated (header renders differently everywhere). ## Test plan - Live verification at the original evidence center (GJ1002b Quarter, world 385024,-598016) via the AtlasAgentInterface path: header crisp at settle+2/+30/+90, captures archived - Golden regeneration: two independent live-server runs, diff -q identical (T-1157 two-run stability discipline) - Pre-push gate green (full gdUnit; one unrelated fog perf-test flake re-verified 46/46 solo and ticketed as T-1210 — 0.5ms budget has no headroom on a shared box)
jpmschweitzer added 2 commits 2026-07-26 14:54:47 +02:00
fix(ui): Atlas header gets its implant panel scrim — legible on any terrain (T-1197) 41144f317f 
Root cause was not the suspected fade race — no animation exists
anywhere under client/ui/implant (grep-proven, and settle+2 vs
settle+90 captures were byte-identical). _build_screen_header() was
the single ImplantHeader call site in the codebase that added the
header bare instead of through ImplantPanel.add_component(), so its
fixed light-gray text washed out against pale terrain (Quarter upland
scatter, District olive) while reading fine on dark ocean. Wrapped in
an ImplantPanel like every sibling — the theme's panel_bg scrim makes
it legible everywhere. One committed golden (atlas_GJ820Bc_District)
already carried the ghosting baked in, confirming the bug was static;
all 13 goldens regenerated per the header change, two independent
live-server runs byte-identical (T-1157 stability discipline).
Verified live at the original evidence center: header crisp at
settle+2/+30/+90.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merge remote-tracking branch 'origin/main' into header-ghost-fix 34ecdad46f
jpmschweitzer commented 2026-07-26 14:59:14 +02:00
Author
Owner
Copy Link
Copy Source

Review: header-ghost-fix -> main (type: visual/client)

Reviewers: Hoshe (QA) + Araminta (art direction — acceptance on her own T-1196 observation). Both REQUEST_CHANGES, converging on the same core regression.

Converged finding (both reviewers, pixel-proven independently)

Issue Detail
Header and legend panels now fuse into one malformed double-height box The ImplantPanel wrapper grew the header's footprint (border + 12px content margins), but step_canvas_legend.gd:56 still hardcodes reposition() at Vector2(PANEL_MARGIN, 60.0) — tuned for the old bare header. Both reviewers pixel-sampled it: panel background runs unbroken from y≈11 into the legend with zero terrain gap, in the verification capture AND all 13 regenerated goldens. The original (ghosted) frame had a clear gap. A new visual defect introduced while fixing the old one. Fix: derive the legend's y from the header panel's measured height + a fixed gap, recomputed on NOTIFICATION_RESIZED (as reposition() already is) — not a second magic constant.

Hoshe (QA): REQUEST_CHANGES — additional findings

# File Issue
2 golden/visual/*.png (all 13) Silent resolution change 960x540 → 1280x720, verified against the parent commit for all 13 — contradicting visual_capture.gd:6's own documented canonical invocation (unchanged on the branch). An unexplained rider in a fix-scoped commit. Revert to the documented 960x540 and regenerate, or supply explicit rationale + doc update if 1280x720 is now intended.
3 test_step_canvas_viewer.gd Gap: no structural test pins header-panel-vs-legend-panel non-overlap — the exact regression this round caught. The T-1192 precedent (canvas-vs-legend-column non-overlap, line 674) exists one relationship over; add the header/legend equivalent.

Hoshe's confirmations: the fade hypothesis correctly disproven (no Tween anywhere under client/ui/implant); the sole-outlier claim verified across all 13 other ImplantHeader sites; theme-before-add_child ordering exact vs _build_legend_panel; no other _screen_header reference breaks; terrain unchanged outside the chrome region.

Araminta (art direction): REQUEST_CHANGES — acceptance withheld pending the fusion fix

Her confirmations: the contrast fix itself is correct and textbook D-169 library discipline (not a hand-rolled scrim); legibility resolved in every capture; the top-left placement occludes nothing new; T-1198 (elevation-white vs ice-white) confirmed pixel-untouched by this branch. Acceptance of her T-1196 observation follows once the two panels separate cleanly.

Verdict: CHANGES REQUESTED

All findings to the authoring agent: legend offset derived from measured header height (+resize recompute), third golden regeneration at the documented 960x540, the non-overlap structural test. Fix round → gate → araminta re-acceptance → merge.

# Review: header-ghost-fix -> main (type: visual/client) Reviewers: Hoshe (QA) + Araminta (art direction — acceptance on her own T-1196 observation). Both REQUEST_CHANGES, converging on the same core regression. ## Converged finding (both reviewers, pixel-proven independently) | Issue | Detail | |-------|--------| | **Header and legend panels now fuse into one malformed double-height box** | The ImplantPanel wrapper grew the header's footprint (border + 12px content margins), but step_canvas_legend.gd:56 still hardcodes `reposition()` at `Vector2(PANEL_MARGIN, 60.0)` — tuned for the old bare header. Both reviewers pixel-sampled it: panel background runs unbroken from y≈11 into the legend with zero terrain gap, in the verification capture AND all 13 regenerated goldens. The original (ghosted) frame had a clear gap. A new visual defect introduced while fixing the old one. Fix: derive the legend's y from the header panel's measured height + a fixed gap, recomputed on NOTIFICATION_RESIZED (as reposition() already is) — not a second magic constant. | ## Hoshe (QA): REQUEST_CHANGES — additional findings | # | File | Issue | |---|------|-------| | 2 | golden/visual/*.png (all 13) | Silent resolution change 960x540 → 1280x720, verified against the parent commit for all 13 — contradicting visual_capture.gd:6's own documented canonical invocation (unchanged on the branch). An unexplained rider in a fix-scoped commit. Revert to the documented 960x540 and regenerate, or supply explicit rationale + doc update if 1280x720 is now intended. | | 3 | test_step_canvas_viewer.gd | Gap: no structural test pins header-panel-vs-legend-panel non-overlap — the exact regression this round caught. The T-1192 precedent (canvas-vs-legend-column non-overlap, line 674) exists one relationship over; add the header/legend equivalent. | Hoshe's confirmations: the fade hypothesis correctly disproven (no Tween anywhere under client/ui/implant); the sole-outlier claim verified across all 13 other ImplantHeader sites; theme-before-add_child ordering exact vs _build_legend_panel; no other _screen_header reference breaks; terrain unchanged outside the chrome region. ## Araminta (art direction): REQUEST_CHANGES — acceptance withheld pending the fusion fix Her confirmations: the contrast fix itself is correct and textbook D-169 library discipline (not a hand-rolled scrim); legibility resolved in every capture; the top-left placement occludes nothing new; T-1198 (elevation-white vs ice-white) confirmed pixel-untouched by this branch. Acceptance of her T-1196 observation follows once the two panels separate cleanly. ## Verdict: CHANGES REQUESTED All findings to the authoring agent: legend offset derived from measured header height (+resize recompute), third golden regeneration at the documented 960x540, the non-overlap structural test. Fix round → gate → araminta re-acceptance → merge.
jpmschweitzer added 2 commits 2026-07-26 15:23:32 +02:00
fix(ui): PR #217 review fixes — derived legend offset, 960x540 goldens, non-overlap tests a4d9a4fa09 
The legend's reposition() now derives its Y from the header panel's
MEASURED bottom plus a named 12px gap (new accessors on the viewer;
deferred recompute so it reads settled layout, refreshed on header
content change) — the old hardcoded 60.0 sat 17px inside the wrapped
header's real 77px bottom, fusing the panels. Pixel-proven at the
evidence center: bottom=77.0, legend top=89.0, gap=12.0 exact. All 13
goldens regenerated at the DOCUMENTED 960x540 (the 1280x720 rider was
my own CLI flag, not a config; PIL-verified) with two-run byte
stability. Two structural tests pin the header/legend relationship
(T-1192 precedent), proven failing-first against the old constant
(-17.0px reported), and the legacy test that pinned the literal 60.0
now asserts the derived relationship so it cannot re-enforce drift.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merge remote-tracking branch 'origin/main' into header-ghost-fix 3821e6718f
jpmschweitzer commented 2026-07-26 15:23:46 +02:00
Author
Owner
Copy Link
Copy Source

Fix round — all findings addressed in a4d9a4fa0; araminta re-ACCEPTS

Finding Resolution
Converged (fusion regression) Fixed per the prescribed shape — the legend's reposition() derives its Y from the header panel's MEASURED bottom (new viewer accessors) plus a named 12px gap, recomputed deferred on settled layout and on header content change. Pixel-proven at the evidence center: header bottom=77.0, legend top=89.0, gap exactly 12.0. Fresh capture shows two distinct chrome panels with clean terrain between them
Hoshe 2 (resolution rider) Fixed — the 1280x720 was a stray capture-driver CLI flag (visual_capture.gd reads no config resolution); all 13 goldens regenerated at the documented 960x540, PIL-verified, two-run byte-stable
Hoshe 3 (missing structural test) Fixed — two tests pin header/legend non-overlap + genuine minimum gap against real production Controls' measured positions (T-1192 precedent), proven FAILING-FIRST against the old constant (reported -17.0px); the legacy legend test that pinned the literal 60.0 now asserts the derived relationship so it cannot re-enforce drift

Araminta re-acceptance on record: clean separation at every sampled rung (olive terrain, heat overlay, navy water), no new occlusion, tests assert the real relationship not a copied formula, T-1198 confirmed untouched. Her T-1196 header-ghosting observation closes.

No retractions — all findings accurate. Suites green (viewer 92/92, legend 12/12); gate green on push. Merging.

# Fix round — all findings addressed in `a4d9a4fa0`; araminta re-ACCEPTS | Finding | Resolution | |---------|------------| | Converged (fusion regression) | **Fixed per the prescribed shape** — the legend's reposition() derives its Y from the header panel's MEASURED bottom (new viewer accessors) plus a named 12px gap, recomputed deferred on settled layout and on header content change. Pixel-proven at the evidence center: header bottom=77.0, legend top=89.0, gap exactly 12.0. Fresh capture shows two distinct chrome panels with clean terrain between them | | Hoshe 2 (resolution rider) | **Fixed** — the 1280x720 was a stray capture-driver CLI flag (visual_capture.gd reads no config resolution); all 13 goldens regenerated at the documented 960x540, PIL-verified, two-run byte-stable | | Hoshe 3 (missing structural test) | **Fixed** — two tests pin header/legend non-overlap + genuine minimum gap against real production Controls' measured positions (T-1192 precedent), proven FAILING-FIRST against the old constant (reported -17.0px); the legacy legend test that pinned the literal 60.0 now asserts the derived relationship so it cannot re-enforce drift | **Araminta re-acceptance on record:** clean separation at every sampled rung (olive terrain, heat overlay, navy water), no new occlusion, tests assert the real relationship not a copied formula, T-1198 confirmed untouched. **Her T-1196 header-ghosting observation closes.** No retractions — all findings accurate. Suites green (viewer 92/92, legend 12/12); gate green on push. Merging.
jpmschweitzer closed this pull request 2026-07-26 15:24:45 +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#217