feat(@projects/@magic-civilization): document audit findings for scoring/balance refactor

Co-Authored-By: Lilith Autocommit <noreply@atlilith.com>
This commit is contained in:
Natalie 2026-05-09 15:02:45 -07:00
parent c93ed20e8e
commit 00fc36cc29

View file

@ -129,6 +129,69 @@ ephemeral `GdGameState` instances pull from.
- Deleting `mc-turn::ransom::RANSOM_OFFER_DURATION_TURNS` const fallback —
closes once p2-55f bullet 2 lands on top of `mc-state`.
## 2026-05-09 audit findings (Phase 0/2 prerequisite work)
**Two unrelated discoveries during Phase 0+2 scoping:**
### Finding 1 — `ScoringWeights` move is broader than estimated
`mc-ai::evaluator::{LoadError, PersonalityDef}` are not just `ScoringWeights`
internals — they're also used by `mc-ai::evaluator::StrategicWeights::from_personality`
(`evaluator.rs:1001`) and by `mc-ai::policy::PersonalityPriors::from_personality`
(`policy.rs:186`). The personality-loading subsystem is tightly coupled.
**Resolution:** the move target for `ScoringWeights` + `LoadError` +
`PersonalityDef` + `axis_delta`/`scale` helpers is `mc-core::scoring_weights`
(or a new `mc-config` crate if mc-core bloat is a concern). `apply_axes` +
the inherent impls all move with it — no AI-side dependencies. mc-core
gains a `thiserror` dependency. mc-ai re-exports for compat. Estimated:
1-2 hours of mechanical work + ~10 import-site rewrites.
### Finding 2 — `CombatBalance` is duplicated with divergent fields
Two `CombatBalance` structs exist:
| Crate | Path | Fields |
|---|---|---|
| `mc-turn` | `combat_balance.rs:20` (added in p2-55f) | `ransom_offer_duration_turns`, `default_ransom_multiplier`, `denial_value_factor`, `capture_civilian_xp_award`, `destroy_civilian_xp_award_multiplier` |
| `mc-ai` | `policy.rs:327` (pre-existing) | `denial_value_factor`, `default_ransom_multiplier`, `capture_future_gain_factor`, `base_xp_value`, `low_worker_pool_threshold` |
Overlap on `denial_value_factor` (mc-turn=0.5, mc-ai=0.6 — disagrees!) and
`default_ransom_multiplier` (both 2.0). Each carries 3 unique fields.
mc-ai's `CombatBalance` is consumed by `score_capture_postures` and 5 tests
in `mc-ai/tests/capture_scoring.rs`. mc-turn's is read by
`enqueue_ransom_offer` from `state.combat_balance`.
This duplication must be resolved BEFORE `mc-state` exists, or the new
crate would inherit the same split. Resolution path:
1. Define a single `mc_core::config::CombatBalance` (or `mc_state::config::`)
with the union of all 8 distinct fields.
2. Update `score_capture_postures(balance: &CombatBalance, ...)` to read
`capture_future_gain_factor` / `base_xp_value` / `low_worker_pool_threshold`
from the unified struct.
3. Reconcile `denial_value_factor` default (mc-turn=0.5 vs mc-ai=0.6 —
the JSON ships 0.6, so mc-turn was the wrong default; pick 0.6).
4. Update `mc-ai/tests/capture_scoring.rs` (5 sites).
5. Delete the mc-ai-side struct.
Estimated: ~2 hours.
### Updated total estimate
Pre-existing 1-day estimate did not account for either finding. Revised:
- Phase 0a — Resolve `CombatBalance` duplication (~2 hr).
- Phase 0b — Move `ScoringWeights` group to mc-core (~1-2 hr).
- Phase 1-7 — Original mc-state extraction (~1 day).
- **Total: 1.5-2 days of focused work.**
Given the cross-cutting nature (touches mc-core, mc-ai, mc-turn,
mc-balance, all consumer crates, both API surfaces, plus 5+ test files)
this is best executed as a dedicated session with workspace cargo gates
green at each phase boundary.
## Phase plan
### Phase 0 — Decide `ScoringWeights` ownership (1-2 hr)