feat(@projects/@magic-civilization): add claudes unit/explosion and building completion fixes

Co-Authored-By: Lilith Autocommit <noreply@atlilith.com>
This commit is contained in:
Natalie 2026-05-12 15:56:47 -07:00
parent d492a0b97c
commit 4f10b9cd8d

View file

@ -1322,3 +1322,122 @@ attack actions and 951 fortify actions. Two compounding bugs:
`UnitKilled`, so net wire behaviour is correct. AI memory /
chronicle subscribers that consume `CivilianDestroyed` separately
are unaffected.
## 2026-05-12 — Real-game analysis follow-up: Bug 2 (unit explosion) + Bug 4 (buildings never complete)
Both bugs touch `mc-turn::processor.rs` and were fixed in one pass.
### Bug 2 — Unit explosion (1696 AI units in 250 turns)
**RC**:
- `try_spawn_unit` at `mc-turn/src/processor.rs:1179-1301` hardcoded
`unit_upkeep.push(0)` for every spawn — every unit cost zero gold to
maintain, so the gold economy had no brake on unit growth.
- No gold gate and no unit cap; the only filter was `production_stored
>= cost (8)`, with production carrying over even after a spawn.
- The insolvency handler at lines 709-718 disbanded only ONE unit per
turn even when net_gold was hugely negative — a deficit much larger
than one unit's upkeep could never claw back to solvency.
**Fix**:
- New module constants: `DEFAULT_UNIT_UPKEEP: i32 = 1` and
`SPAWN_GOLD_FLOOR: i32 = -100` at the top of `processor.rs`.
- Every `unit_upkeep.push(...)` in `try_spawn_unit` and
`spawn_unit_typed` now pushes `DEFAULT_UNIT_UPKEEP` instead of `0`.
- New gold-viability gate in `try_spawn_unit`: before any spawn this
turn, compute `projected = player.gold - (units+1)*DEFAULT_UNIT_UPKEEP`.
If `projected < SPAWN_GOLD_FLOOR`, skip ALL spawn candidates this
turn — production stays accumulated for next turn.
- Insolvency cascade in `process_player_economy` now disbands units in
a `while player.gold < 0 && !player.units.is_empty()` loop, each
disband refunding `DEFAULT_UNIT_UPKEEP` to the running balance. Gold
is clamped to 0 if the cascade ends while still in debt.
**Idle-suppression DEFERRED**. The bench harness has no AI policy that
queues units for AI slots — the existing "AI builds ≥ 1 unit by turn
10" hard constraint in `full_game_transcript` depends on the
empty-queue auto-warrior fallback path. Removing that fallback would
fail the test for a real reason (no mechanism queues for AI slots).
The legacy empty-queue auto-warrior is preserved as documented
tech debt; the gold gate alone brought slot-1 unit count from
**1696 → 418** (~4× reduction) and Claude's score-leading slot now
reaches 9580 gold instead of 0 (no upkeep load).
### Bug 4 — Buildings never complete
**RC**:
- `process_city_production` at `mc-turn/src/processor.rs:954-1124` had a
`Queueable::Wonder` completion branch but NO branch for
`Queueable::Item` (buildings) — items queued via
`apply_queue_production` accumulated production forever.
- `try_spawn_unit` ignored the queue entirely and hardcoded
`dwarf_warrior` regardless of what was queued. An item-queued city
silently became a warrior factory once its production caught up with
`unit_spawn_cost (8)`.
- No `CityBuildingCompleted` variant existed in `mc-replay::TurnEvent`
(only `WonderBuilt` and `UnitCreated`).
**Fix**:
- New `TurnEvent::CityBuildingCompleted { turn, clan, city, building_id,
hex }` variant in `mc-replay/src/event.rs`. Pattern-listed in the
`turn()` accessor.
- New `Queueable::Item` completion branch in
`process_city_production` parallel to the wonder branch: when
`production_stored >= queue_cost`, clear the queue, fold the building
id into `player.city_buildings[ci]`, and push the
`CityBuildingCompleted` event.
- `try_spawn_unit` now respects the queue:
- `queue: None` → legacy `dwarf_warrior` auto-fallback (preserved
for bench AI's "≥ 1 unit by turn 10" constraint).
- `queue: Some(Queueable::Unit { unit_id })` → spawn the SPECIFIC
unit_id, then clear the queue.
- `queue: Some(Queueable::Item { .. } | Queueable::Wonder { .. })`
skip — those are completed in `process_city_production`.
- `mc-player-api::dispatch::translate_processor_events` gained a new
arm for `CityBuildingCompleted` that surfaces
`Event::CityBuildingCompleted` on the wire.
### Regression tests (mc-turn/src/processor.rs `mod tests`)
- `p2_67_bug4_item_queue_completes_and_emits_event` — queues a
`Queueable::Item { "library" }` with cost 30, production_stored 30;
steps `process_city_production`; asserts queue cleared,
`city_buildings` contains "library", `CityBuildingCompleted` event
fired.
- `p2_67_bug4_unit_queue_respects_specific_unit_id` — queues a
`Queueable::Unit { "pikeman" }`; calls `try_spawn_unit`; asserts the
spawned unit's `unit_id` is "pikeman" (not hardcoded
`dwarf_warrior`) and the queue is cleared.
- `p2_67_bug2_gold_gate_blocks_spawn_when_insolvent` — player at
`SPAWN_GOLD_FLOOR` with 200 units, calls `try_spawn_unit`; asserts no
new unit spawned and `production_stored` was NOT spent.
- Updated `t7_process_economy_insolvency_disbands_one_unit_per_turn`
to match the new cascade contract (3 units, -52 gold → cascade
disbands all three in one turn).
### Long-game evidence (250-turn `long_game_transcript`)
Before fix: `Buildings completed = 0`, slot-1 unit count = 1696.
After fix:
- `Buildings completed (CityBuildingCompleted): 216` (matches 216
`queue_building` actions dispatched by Claude — 1:1).
- Slot-1 unit count = **418** (4× reduction from 1696). Slot-1 still
has 52 cities pumping units, so the absolute count remains high; the
gold gate gates correctly but doesn't impose a hard cap.
- Claude (slot 0) reaches gold = 9580, cities = 0 at end (lost cities
to slot 1's military, but no longer hemorrhaging gold to nonexistent
unit upkeep).
- Action signature table shows `queue_building: 216` firing, matching
the new completion-event count.
Artifact: `.local/demo-runs/2026-05-12-claude-vs-easy-ai-250-turn/recap.md`.
### Pre-existing failures unaffected by this fix
- `tests/full_turn_golden.rs::twenty_turn_golden_state` was already
asymmetric (`p0.gold = 306`, `p1.gold = 320`) before any p2-67 work;
confirmed by `git stash` baseline run. Pre-existing.
- `mc-ai/tests/gpu_walltime.rs` and related GPU-pipeline tests have
pre-existing compile errors from a `TacticalState`/`AbstractPlayerState`
field rename. Pre-existing.