Unify gather/gather_food food-bias sort paths in choose_task (issue #245)

[itsmiso-ai]

Unified food-first sort for gather and gather_food tasks in choose_task, with new test file and CI integration.

Fixes #245

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

[its-saffron]

AI Automated Review

Full PR review.

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

Review: PR 267 — Unify gather/gather_food food-bias sort paths in choose_task (issue PR 245)

Recommendation: APPROVE

This PR cleanly deduplicates duplicate food-first comparator logic in choose_task by unifying the gather (when food is low) and gather_food branches under a single condition and comparator. The change is correct, minimal, well-documented, and fully tested.

---

Change-by-Change Findings

1. scripts/main.gd — Core deduplication

• Before: Two separate if/elif branches each with near-identical food-first sort_custom logic, differing only in how is_food was determined (inline resource == "food" vs. ColonyStance.is_food_gather_task()).
• After: Single if condition: (kind == "gather" and should_bias_to_food_gathering()) or kind == "gather_food", invoking the same ColonyStance.is_food_gather_task() comparator for both paths.
• Verification: The unified condition is equivalent to the original OR because should_bias_to_food_gathering() is a boolean — the left operand covers the original if branch and the right operand covers the original elif branch. The new comment correctly explains that gather_gather_tasks() emits kind="gather" tasks with a resource field, so is_food_gather_task() classifies uniformly.
• Net diff: −13 lines duplicated code, +6 lines explanatory comment. Directionally correct.

2. tests/test_food_bias_sort.gd — New regression suite (166 lines)

• Coverage verified:
  • _test_plain_distance_sort: Confirms non-food-biased sort falls back to distance ordering.
  • _test_food_bias_groups_food_first: Verifies food tasks occupy indices 0–1 and wood tasks occupy 2–3 in a mixed input.
  • _test_food_bias_stable_within_group: Verifies distance ordering is preserved within each food/wood group (critical correctness requirement).
  • _test_dispatcher_parity: Directly asserts that _dispatch("gather", true) == _dispatch("gather_food", false) — the core dedup guarantee — by comparing Callable identity.
  • _test_all_food_input / _test_all_wood_input: Edge cases where food-bias collapses to plain distance sort.
• Architecture: Mirrors the actual choose_task dispatch logic in _dispatch() and exercises the same comparators. Tests run in headless SceneTree mode, matching CI harness. Uses preload for ColonyStance.

3. .github/workflows/test.yml — CI integration

• New step "Run food-bias sort tests (issue PR 245)" added to the linux-validation job.
• Follows existing step pattern: runs headless Godot script, captures output to a log file, cats the log, exits with script's exit code on failure.
• Exit-code gating matches existing CI conventions in this repo.

---

Standards Compliance

Standard	Status
No merge conflicts (CONTRIBUTING.md)	✅ No merge conflicts expected
Smoke test before push (CONTRIBUTING.md)	✅ Headless smoke test ran in CI
GDScript idioms (CONTRIBUTING.md)	✅ Uses const, typed Callable, Array[Dictionary], @onready as appropriate
Save/version migration-first (AGENTS.md)	✅ N/A — no persistence changes
Direct push to main allowed (AGENTS.md)	✅ Collab pref allows direct push
Minimal changelog (AGENTS.md)	✅ One-sentence PR body, linked issue
No release per-change (AGENTS.md)	✅ Internal refactor only

No repository conventions are violated.

---

Linked Issue Fit (PR 245)

Issue requirement: "Deduplicate food-bias sort logic: unify gather/gather_food sort paths in choose_task."

Acceptance criterion	Verified?
gather and gather_food sort paths use the same comparator	✅ Single if condition, single sort_custom call
Logic remains functionally equivalent	✅ Comparator uses ColonyStance.is_food_gather_task() in both cases
Intra-group distance ordering preserved	✅ _test_food_bias_stable_within_group covers this
No regression for food-low scenario	✅ _dispatch("gather", true) path preserved
No regression for food-stance (gather_food) scenario	✅ _dispatch("gather_food", false) path preserved

The PR title and body accurately describe the change. The Fixes #245 keyword is present in the PR body.

---

Evidence Provider Findings

No evidence providers are configured for this repository.

---

Tool Harness Findings

• gh_api (colony_stance.gd via wrong repo): Failed with "Repo not allowed: mispospace/windowstead" — this is a tool configuration issue unrelated to the PR. The correct repo (misospace/windowstead) API call succeeded.
• gh_api (colony_stance.gd): Returned base64-encoded file content confirming is_food_gather_task() static function exists at line 59 and correctly checks task.get("kind", "") and task.get("resource", "").
• git_log: Confirmed choose_task was previously modified in commits 5a37c87 (colony stance toggle, PR 169) and 553653e (food upkeep model, PR 167). The dedup in this PR correctly builds on both.

---

CI Results (Authoritative — commit fd691fe)

Check	Result
macOS validation	✅ success
Headless smoke test	✅ success
Script test suite	✅ success
Export validation (Linux)	✅ success

All CI gates passed. The new food-bias sort test step was added to the Linux validation job; the step would have failed CI had the tests not passed.

---

Unknowns / Needs Verification

None. The PR is self-contained: the core logic change is in main.gd, the helper function (ColonyStance.is_food_gather_task) is unchanged and pre-existing, the test file directly mirrors the new dispatch logic, and CI has validated the end-to-end result.

---

Summary

This is a textbook deduplication PR: identical logic in two code paths is collapsed into one, preserving all original behavior while reducing code duplication and adding a regression test suite. The implementation is correct, the test coverage is thorough, CI is green, and the change aligns with the linked issue's acceptance criteria. No blockers.