fix(conformance): mill ZoneTransfer annotations emit affectorId

[delebedev]

Closes #176

Summary

• Add sourceForgeCardId to CardMilled event (same pattern as CardSurveiled/CardDestroyed)
• Source resolved from stack's resolving spell at zone-change time via bridge.getGame().stack.peek()
• Wire through affectorSourceFromEvents — mill annotations now carry the source spell instanceId

Test plan

• CategoryFromEventsTest — 58/58 pass, including new affectorSourceReturnsMillSourceCard test
• Updated existing test that asserted null for mill → now asserts source card

🤖 Generated with Claude Code

[github-actions]

Test Results

  227 files  ±0  227 suites  ±0   2m 0s ⏱️ -23s
  996 tests +1  964 ✅ +1   32 💤 ±0  0 ❌ ±0 
1 778 runs  +2  978 ✅ +1  800 💤 +1  0 ❌ ±0 

Results for commit 87a7abd. ± Comparison against base commit c58f0b2.

This pull request removes 1 and adds 2 tests. Note that renamed tests count towards both.

leyline.game.CategoryFromEventsTest ‑ affectorSourceReturnsNullForNonSurveil

leyline.game.CategoryFromEventsTest ‑ affectorSourceReturnsMillSourceCard
leyline.game.CategoryFromEventsTest ‑ affectorSourceReturnsNullWhenMillHasNoSource

[github-actions]

CI Report — Gate

Tests: 816/816 passed (189 skipped)

Coverage: 10.5% (615/5852 lines)

Module	Coverage	Lines
tooling	🔴 11%	615/5852

Slow tests (>3s): 1

• InstanceIdReallocTest.Limbo grows across multiple plays (5.4s)

[delebedev]

@claude please review this ralph PR

[claude]

Claude finished @delebedev's task in 1m 16s —— View job

---

Code Review — fix(conformance): mill ZoneTransfer annotations emit affectorId

• Read changed files
• Review implementation
• Post findings

---

The change is small and focused. The mechanical intent is correct — mill annotations now carry affectorId the same way surveil and destroy do. A few things worth discussing:

---

Architectural deviation from the documented pattern

GameEventCollector.kt:24-27 has an explicit warning:

When upstream Forge events lack the granularity we need, add a new event to our fork rather than retroactively correlating events here. See GameEventCardSurveiled for the pattern.

CardSurveiled follows that pattern — it gets its cause from ev.causeCard() on a dedicated Forge event. This PR takes the opposite approach for mill: it peeks at the live stack at zone-change time (bridge.getGame()?.stack?.peek()?.spellAbility?.hostCard?.id). That's exactly what the comment says not to do.

The CardSurveiled approach is stronger because the source is captured as part of the event (atomic, reliable). The stack.peek() approach ties correctness to timing assumptions.

---

Correctness risk: triggered mill

If a replacement effect or triggered ability is on top of the stack when the Library→GY zone change fires (e.g. a "whenever a card is put into a graveyard from a library" trigger), stack.peek() returns the trigger, not the mill spell. The affectorId would be wrong.

In practice this probably doesn't happen at zone-change time (triggers are queued, not immediately pushed), but it's a latent hazard that the causeCard() pattern avoids entirely. Worth a comment at the call site at minimum.

---

Stale docstring

AnnotationBuilder.kt:118-121 still reads:

* Currently only CardSurveiled carries source info; extend for other mechanics as needed.

This should be updated to mention mill (and destroy, which was already wired).

---

Tests

The new tests are solid. Splitting the old affectorSourceReturnsNullForNonSurveil into two cases (has source / no source) improves coverage and intent. No concerns there.

The GameEventCollector side (the stack.peek() logic) has no unit test — but that's pre-existing for the surveil collector path too, so not a new gap introduced here.

---

Verdict: Approve with suggested follow-up. The fix works for the common case (a mill spell on the stack), tests pass 58/58, and it matches the CardSurveiled/CardDestroyed shape at the builder level. The architectural deviation and stale docstring are worth a quick follow-up.