Remove orphan assertion referencing out-of-scope variables by itsmiso-ai · Pull Request #266 · misospace/windowstead

itsmiso-ai · 2026-07-08T08:43:59Z

Integrates MilestoneManager into main.gd's tick loop, save/load, and bootstrap (Option A).

Fixes #237

Opened by foreman on review GO (workload wl-misospace-windowstead-237).

The merge-base changed after approval.

Superseded by a newer automated review for this pull request.

itsmiso-ai · 2026-07-09T08:30:05Z

⚠️ This PR fix item has been marked as BLOCKED and needs human attention.

Reason: Failing check: Headless smoke test (failure)

Latest note: foreman fix exhausted 3/3 attempts on ESCALATED (all coder tiers exhausted) (prfix-misospace-windowstead-266)

Posted automatically by Dispatch on 2026-07-09T08:30:05.126Z

itsmiso-ai · 2026-07-09T09:30:05Z

⚠️ This PR fix item has been marked as BLOCKED and needs human attention.

Reason: Failing check: Headless smoke test (failure)

Latest note: foreman fix exhausted 3/3 attempts on ESCALATED (all coder tiers exhausted) (prfix-misospace-windowstead-266)

Posted automatically by Dispatch on 2026-07-09T09:30:04.874Z

itsmiso-ai · 2026-07-09T10:30:05Z

⚠️ This PR fix item has been marked as BLOCKED and needs human attention.

Reason: Failing check: Headless smoke test (failure)

Latest note: foreman fix exhausted 3/3 attempts on ESCALATED (all coder tiers exhausted) (prfix-misospace-windowstead-266)

Posted automatically by Dispatch on 2026-07-09T10:30:04.754Z

itsmiso-ai · 2026-07-09T11:30:05Z

⚠️ This PR fix item has been marked as BLOCKED and needs human attention.

Reason: Failing check: Headless smoke test (failure)

Latest note: foreman fix exhausted 3/3 attempts on ESCALATED (all coder tiers exhausted) (prfix-misospace-windowstead-266)

Posted automatically by Dispatch on 2026-07-09T11:30:05.061Z

itsmiso-ai · 2026-07-09T12:30:05Z

⚠️ This PR fix item has been marked as BLOCKED and needs human attention.

Reason: Failing check: Headless smoke test (failure)

Latest note: foreman fix exhausted 3/3 attempts on ESCALATED (all coder tiers exhausted) (prfix-misospace-windowstead-266)

Posted automatically by Dispatch on 2026-07-09T12:30:05.564Z

itsmiso-ai · 2026-07-09T13:30:05Z

⚠️ This PR fix item has been marked as BLOCKED and needs human attention.

Reason: Failing check: Headless smoke test (failure)

Latest note: foreman fix exhausted 3/3 attempts on ESCALATED (all coder tiers exhausted) (prfix-misospace-windowstead-266)

Posted automatically by Dispatch on 2026-07-09T13:30:05.483Z

itsmiso-ai · 2026-07-09T14:30:05Z

⚠️ This PR fix item has been marked as BLOCKED and needs human attention.

Reason: Failing check: Headless smoke test (failure)

Latest note: foreman fix exhausted 3/3 attempts on ESCALATED (all coder tiers exhausted) (prfix-misospace-windowstead-266)

Posted automatically by Dispatch on 2026-07-09T14:30:05.080Z

Superseded by a newer automated review for this pull request.

Wire scripts/milestone_manager.gd into scripts/main.gd (Option A from the issue). The previous integration attempt was removed in a revert; re-apply it with the same correctness standards, and add explicit type annotations to milestone_manager.gd so the GDScript "inferred from Variant" warnings stop being treated as errors by the headless smoke test (CI failure root cause). Changes in scripts/main.gd: - Preload MilestoneManager alongside GoalReward at the class scope. - Add current_milestone_id (String) and completed_milestone_ids (Array) state variables so milestone progress is tracked between ticks and across save/load. - bootstrap_state() seeds both fields from MilestoneManager.make_goal_state() on a fresh colony, so the player starts on the first catalog entry. - load_or_boot() restores the two fields from the save dict, falling back to make_goal_state() defaults for legacy saves that predate milestone tracking. - _on_tick() evaluates the active milestone once per tick (at function scope, not inside the reward loop — the previous attempt's bad indentation is fixed), pushes a "Milestone reached: …" event on completion, and advances to the next milestone via MilestoneManager.advance_to_next(). - persist() writes both fields into the save dict (using .duplicate() for the array, matching the pattern used for completed_goal_ids and active_rewards). Changes in scripts/milestone_manager.gd: - Add explicit type annotations to every `:=` declaration in evaluate_milestone(), is_milestone_complete(), and advance_to_next() whose right-hand side comes from a Dictionary.get() Variant (mtype, target, build_kind, builds, resource, amount, harvested, current, count, workers, active, eval, current_index, next_index). This silences the "variable type is being inferred from a Variant value" parse warnings, which the project's CI runs as errors and which were the root cause of the headless smoke test failure. Acceptance criteria for Option A (all met): - MilestoneManager preloaded and initialized in main.gd - Milestones evaluated each tick - Completion fires a push_event with the milestone description - Milestone state persists in save data - tests/test_milestone_goals.gd continues to pass: existing tests already cover the make_goal_state / get_current_milestone / is_milestone_complete / advance_to_next / milestone_description surface and a full JSON round-trip via the same field names (milestone_id, completed_ids) that main.gd now persists. Fixes #237 Signed-off-by: Saffron <263493777+itsmiso-ai@users.noreply.github.com>

Superseded by a newer automated review for this pull request.

its-saffron

AI Automated Review (incremental)

Incremental review: reviewed the changes since the last managed review; unresolved findings from that review are carried forward.

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — escalated (fast_low_confidence)

PR Review: Wire MilestoneManager into main.gd (Fixes PR 237)

Recommendation: Approve

This PR correctly implements Option A from issue PR 237: wiring MilestoneManager into main.gd rather than removing it as dead code. All CI checks pass, the implementation follows existing repository patterns, and there are no blockers.

Change-by-Change Analysis

`scripts/main.gd`

Preload addition (line 18): Added MilestoneManager preload alongside the existing GoalProgression and RotatingGoal preloads. This follows the established pattern for game logic modules.
State variables (lines 103-104): Introduced current_milestone_id: String and completed_milestone_ids: Array to track milestone progression, paralleling how active_goal and completed_goal_ids track goal progression.
bootstrap_state() initialization (lines 769-772): Seeds milestone state using MilestoneManager.make_goal_state() when starting a new colony. This is correct — new games get the first milestone in the catalog.
load_saved_game() restoration (lines 924-927): Restores milestone state with safe defaults for legacy saves that lack milestone keys. Uses state.get() with fallback to MilestoneManager.make_goal_state() defaults. This handles backward compatibility correctly.
_on_tick() evaluation (lines 1412-1418): Added milestone completion check after reward processing. When complete:
- Pushes "Milestone reached" event to the event log
- Adds completed milestone ID to tracking array
- Advances to next milestone via advance_to_next()
  This follows the same flow as goal rewards.
persist() save (lines 2461-2462): Saves both current_milestone_id and completed_milestone_ids to the save state.

`scripts/milestone_manager.gd`

The diff shows only type annotation changes (from := inference to explicit typed declarations). These are style-only changes that improve code clarity and IDE support with no runtime behavior change.

Standards Compliance

Requirement	Status
Save/version migration handled (migration-first principle)	✅ Correctly handled — `bootstrap_state()` seeds defaults; `load_saved_game()` falls back gracefully for legacy saves
Follows existing code patterns	✅ Uses the same initialization/load/persist/evaluate pattern as `GoalProgression`
No blocking patterns or regressions	✅ All CI checks pass

Linked Issue Fit

Issue PR 237 asked to either wire MilestoneManager into main.gd or remove as dead code. This PR implements the wiring option (Option A) by integrating milestone state into:

Bootstrap: New colonies get the initial milestone
Save/Load: Milestone progress is persisted and restored with legacy save compatibility
Tick loop: Milestones are evaluated each tick and advance when complete

The event log integration ("Milestone reached: %s") provides player feedback consistent with the existing reward notification pattern.

Evidence Provider Findings

No evidence providers were configured for this review.

Tool Harness Findings

All planned verification calls completed successfully. Key evidence confirmed:

milestone_manager.gd exists at the expected path with the expected class_name MilestoneManager
test_milestone_goals.gd exists and tests milestone functionality
Issue PR 237 is open and correctly labeled as enhancement/tech-debt

CI Results

All CI checks passed for commit 7422627:

✅ macOS validation
✅ Script test suite
✅ Export validation (Linux, Windows, macOS)
✅ Headless smoke test

Unknowns / Needs Verification

None. The implementation is complete and verifiable:

Logic follows existing repository patterns
Save/load handling is correct with backward compatibility
Tests exist for milestone functionality
CI validates the change comprehensively