Close out the 2026-07-15 tech debt audit#285
Conversation
- worker_texture pixel rendering and crowd offsets move to scripts/worker_renderer.gd; collision offsets are now ring-distributed so crowds above six workers no longer overlap (audit items 2, 3). - apply_theme shares one stylebox instance per button state instead of creating four per button (item 4). - Reservation-balance regression test covering the do_gather early-return race the audit flagged — the lifecycle is balanced; the test pins it (item 7). - tests/test_dock_layout.gd covers apply_anchor_layout reparenting across bottom/side/bottom transitions without loading the scene (item 11). - Stray 'test fix:' trailer removed from README.md (item 10). - labels.yaml mirrors the live repo labels (item 13).
There was a problem hiding this comment.
AI Automated Review
Full PR review.
Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — escalated (fast_low_confidence)
Recommendation: Approve
This PR cleanly closes out the 2026-07-15 tech debt audit (PR 279) with well-scoped changes. All modifications are consistent with repository conventions, address the linked issue's findings directly, and CI is green across all six validation gates.
Change-by-Change Findings
1. scripts/worker_renderer.gd (new file, 69 lines)
A new WorkerRenderer class extracted from main.gd containing two static methods:
create_texture()— pixel-art worker sprite generation (12×14 RGBA8 Image → ImageTexture). The logic is byte-for-byte identical to what was inworker_texture(); no behavioral change, just a location move.collision_offset()— ring-distributed offsets replacing the hardcoded 6-element table. UsesTAU * (slot % total) / totalto spread workers evenly around a circle, so any crowd size stays distinct (fixes item 3's overlap bug above 6 workers).
The class comment correctly states "No scene references; main.gd owns the texture cache and calls in" — this is a pure utility class with no lifecycle of its own, consistent with how TileRender, WorkerCapLogic, and LayoutMath are structured in the codebase.
2. scripts/main.gd (net −48 lines)
apply_button_theme() (was apply_theme() but split out): The stylebox caching change addresses item 4. Instead of .new() + .duplicate() per button per call, _button_styles is now a dictionary cached at class scope — built once on first use, then shared by reference. This is a straightforward optimization with no behavioral impact.
worker_texture(): Replaced ~50 lines of inline pixel pushing with a single delegation to WorkerRenderer.create_texture(). The worker_texture_cache in main.gd remains the caller's responsibility (the PR summary explicitly notes "main.gd owns the texture cache and calls in"), which is correct.
worker_collision_offset(): Replaced the inline hardcoded 6-offset table with a delegation to WorkerRenderer.collision_offset(slot, total, spacing). The single-element case (total <= 1) is now handled inside the static method rather than inline — functionally equivalent, just relocated.
3. tests/test_reservations.gd (+66 lines)
test_gather_reservation_balance_on_depleted_tile() addresses item 7. The test:
- Sets up a
ColonySimwith two workers, each assigned to a single-unit wood tile - Verifies both hold reservations (count = 2)
- Empties worker A's tile out from under it, then calls
do_gather()— verifies the early-return releases exactly one reservation (count = 1) - Worker B gathers normally — verifies successful gather releases the last reservation (count = 0)
- Calls
do_gather()on the same empty tile again — verifies the counter clamps at zero and never goes negative
This pins the invariant without requiring a code change to ColonySim, which matches the PR summary's claim.
4. tests/test_dock_layout.gd (new file, 107 lines)
Addresses item 11. The test:
- Instantiates
main.gddirectly (no scene tree, no autoloads) - Manually wires the minimal node graph via
_build_layout_harness() - Tests bottom mode parentage assertions (resource_label → bottom_header_row, hud_row → bottom_button_row, etc.)
- Switches to side mode, verifies the inverse reparenting
- Returns to bottom mode, verifies full restoration
This is the orchestration test for apply_anchor_layout() that was previously missing. The approach of calling .new() without entering the tree is consistent with how the test suite isolates unit-under-test from autoloads.
5. README.md (−1 line)
Removed the stray # test fix: _assert_empty accepts Variant trailer, closing item 10.
6. .github/labels.yaml (+92 lines)
Added 28 labels that mirror the live repo's label set (agent/, area/, p0/p1/p2, type/*, tech-debt, and GitHub defaults), closing item 13.
Standards Compliance
- Extraction pattern: The PR follows the established extraction convention (TileRender, WorkerCapLogic, ColonySim) by moving a cohesive unit (
worker_texture+collision_offset) into its own module. Each extracted module in this repo owns a single concern;WorkerRendererfits that pattern. - No
main.gdline-count regression: The net change is −48 lines inmain.gd, continuing the reduction trajectory toward the <1000-line target from item 1 (which the PR correctly defers to a scoped issue per the PR summary's rationale). - Stylebox caching:
apply_button_theme()now shares stylebox references rather than duplicating per button. This is the correct fix for item 4 — the issue explicitly called for caching styleboxes as class members, which is exactly what_button_stylesdoes. - No release process triggered: This is a non-release merge; no version bump needed per the "Do not cut a release after every small change" convention.
Linked Issue Fit
Issue PR 279 mapped 13 findings. This PR's disposition matches the issue's intent:
| Item | Issue Finding | PR Action | Fit |
|---|---|---|---|
| 2 | Worker texture per-frame pixel push | Extracted to worker_renderer.gd |
✅ |
| 3 | 6 hard-coded offsets overlap above 6 workers | Ring distribution in collision_offset() |
✅ |
| 4 | Stylebox .duplicate() churn per button |
Cached _button_styles dictionary |
✅ |
| 7 | do_gather reservation drift |
Regression test pins invariant | ✅ |
| 11 | No apply_anchor_layout test |
New test_dock_layout.gd |
✅ |
| 10 | Stray README trailer | Removed | ✅ |
| 13 | labels.yaml stale |
Updated with 28 live labels | ✅ |
| 6 | Per-frame overlay waste | Waste removed in PR 282; item dispositioned | ✅ |
| 9 | Status text overflow | UX design decision; scoped to separate issue | ✅ |
| 1 | main.gd <1000 lines | Deferred to scoped issue per PR rationale | ✅ |
| 5 | persist() debounce |
Fixed in PR 282 | ✅ |
| 8 | Event drawer GC pressure | Fixed in PR 282 | ✅ |
| 12 | Double load_or_boot path |
Deleted in PR 282 | ✅ |
Tool Harness Findings
No tool harness failures. The four tool calls (read_file for worker_renderer.gd, git_log twice, git_blame) all returned successfully and provided additional context confirming:
- The new
WorkerRendererconstant was added at line 11 ofmain.gd(commit 4160bc5, authored 2026-07-16) - The extraction follows the same pattern as prior extractions (TileRender, WorkerCapLogic, ColonySim)
CI Results
All six CI gates passed on commit 4160bc5b65f824b4dae46288b76a2173a9248af2:
- Export validation (Linux, macOS, Windows) — success
- macOS validation — success
- Script test suite — success
- Headless smoke test — success
Unknowns / Needs Verification
None. The PR body provides explicit mapping between each finding and the code change. The diff is self-consistent and traceable to the linked issue. CI is green. No blockers.
Closes #279.
Most of the audit predates #282–#284, which already resolved findings 5 (persist debounce), 8 (event drawer re-renders only on change), 12 (duplicate
load_or_bootpath deleted), and the #234/#236 carry-forwards; finding 1's simulation half landed asColonySim(main.gd 2,646→~1,950 lines). This PR handles the remainder.Summary
worker_texturepixel rendering + crowd offsets extracted toscripts/worker_renderer.gd; offsets are now ring-distributed by crowd size, so >6 workers on one tile no longer overlap (items 2, 3).apply_themeshares one stylebox instance per button state — styleboxes were never mutated per-button, so the per-button.new()/.duplicate()churn was pure waste (item 4).do_gatherearly-return race (item 7). Traced the lifecycle: exactly one reserve per chosen task, exactly one release per gather attempt — no code change needed, the test pins the invariant.tests/test_dock_layout.gdcoveringapply_anchor_layoutreparenting across bottom→side→bottom, without loading the scene or autoloads (item 11).# test fix:trailer removed (item 10);labels.yamlnow mirrors the live repo labels (item 13).Dispositioned without code
settlement_status_textoverflow): a UX design decision about what to drop from the status line — recommend a scoped issue.<1000-line target: the remaining main.gd is view-layer code; further extraction (render manager) deserves its own issue rather than a bundled refactor.Verification