fix(engine): Consuming Vapors gains life from sacrificed toughness (#5925)

[claytonlin1110]

Summary

• Fixes Consuming Vapors — Opponent sacrificed a creature, I did not gain the life from that creature's toughness. #5925: after target-player sacrifice-of-their-choice, life gain equal to that creature's toughness reads sacrificed LKI via Demonstrative on the real cast/EffectZoneChoice path.
• Single authority: stamp effect_context_object only at the sacrifice-completion seam (perform_player_scope_sacrifices when propagate_parent_context); removed the duplicate EffectZoneChoice re-scan.
• Cache-miss path: snapshot_for_sacrificed_object falls back to the same-span battlefield→public ZoneChanged record when LKI cache is absent.

Test plan

• Unit: choice and auto-sacrifice Demonstrative toughness life gain (effect_zone_choice_sacrifice_then_gain_life_reads_sacrificed_toughness, auto_sacrifice_then_gain_life_reads_sacrificed_toughness)
• Unit: record-only/cache-miss discriminator (sacrificed_referent_binds_zone_changed_record_on_lki_cache_miss) — empty LKI + extra ZoneChanged so moved-object multi-guard cannot rescue
• Integration: consuming_vapors_life_gain_5925 cast-pipeline tests (choice + auto-sacrifice)
• CI test-engine green on head e7ce34fc8 (local MSVC linker unavailable; evidence from this push)

Closes #5925

Summary by CodeRabbit

• Bug Fixes
  • Fixed Consuming Vapors to use the sacrificed creature’s effective battlefield toughness for life gain, even when cached card details are missing.
  • Improved sacrifice target binding so demonstrative quantities consistently resolve to the correct sacrificed creature across both interactive choice and automatic sacrifice.
  • Added a resolution fallback so characteristics can be derived from the sacrificed object identity when prior data isn’t available.
• Tests
  • Added integration tests for Consuming Vapors covering interactive selection and single-target auto-sacrifice, asserting exact life gained and correct battlefield/graveyard results.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: d12e6f92-1214-4df0-85fc-2cb7f3b7146c

📥 Commits

Reviewing files that changed from the base of the PR and between 26bd169 and aba2b42.

📒 Files selected for processing (1)

• crates/engine/src/game/effects/mod.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• crates/engine/src/game/effects/mod.rs

---

📝 Walkthrough

Walkthrough

This change preserves sacrificed-creature referents when LKI lookup is unavailable, propagates them through sacrifice continuations, adds a characteristic lookup fallback, and tests toughness-based life gain for selected and automatic sacrifices.

Changes

Sacrifice referent flow

Layer / File(s)	Summary
Sacrificed object snapshot construction crates/engine/src/game/effects/mod.rs, crates/engine/src/game/effects/sacrifice.rs	Sacrificed-object snapshots now use LKI data or matching battlefield-exit records, while automatic sacrifices propagate continuation context flags.
Continuation context propagation crates/engine/src/game/engine_resolution_choices.rs, crates/engine/src/game/effects/mod.rs, crates/engine/src/game/quantity.rs	Sacrifice completion and chained abilities propagate the resolved referent; demonstrative power/toughness lookup falls back by object ID when snapshot LKI is incomplete.
Sacrifice life-gain regression coverage crates/engine/tests/integration/consuming_vapors_life_gain_5925.rs, crates/engine/src/game/effects/mod.rs, crates/engine/tests/integration/main.rs	Tests verify cache-miss snapshot binding, toughness-based life gain for manual and automatic sacrifice paths, resulting zones, and continuation draining.

Estimated code review effort: 4 (Complex) | ~45 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly states the main Consuming Vapors life-gain fix from sacrificed toughness.
Linked Issues check	✅ Passed	The changes address #5925 by preserving the sacrificed creature's snapshot so Consuming Vapors gains life from its toughness.
Out of Scope Changes check	✅ Passed	The snapshot fallback, quantity handling, and tests all support the reported sacrifice-life gain bug fix.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[matthewevans]

Blocked — the cache-miss sacrifice path lacks discriminating coverage, and its continuation stamp is duplicated.

🔴 Blocker

crates/engine/src/game/effects/mod.rs:1826 adds the ZoneChanged-record fallback specifically for an absent lki_cache entry, but crates/engine/tests/integration/consuming_vapors_life_gain_5925.rs:25 only exercises an ordinary sacrifice. It neither removes the cache entry nor creates the record-only event shape, so it cannot demonstrate that the new fallback works. This leaves the stated redirected/cache-gap behavior unproved. Add a focused record-only/cache-miss test (with the production-pipeline coverage retained) that fails if snapshot_for_sacrificed_object falls back to None.

crates/engine/src/game/effects/mod.rs:6769 already resolves and stamps parent_referent_context_from_events when propagate_parent_context is enabled; crates/engine/src/game/engine_resolution_choices.rs:4655 repeats the same scan and recursive continuation mutation for the same EffectZoneChoice path. This creates two authorities that can drift when the snapshot ladder changes. Keep the stamp at the sacrifice-completion seam and remove the duplicate outer mutation.

🟡 Required proof

The PR body leaves all three verification items unchecked, including both proposed tests. With the required proof gate active for this head, provide current-head evidence for the fixed cache-miss branch and the real cast pipeline before reconsideration.

Recommendation: request changes for one authoritative stamp, a cache-miss discriminator, and completed current-head verification.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/engine/tests/integration/consuming_vapors_life_gain_5925.rs`:
- Around line 21-44: Extend the test function
consuming_vapors_gains_life_equal_to_chosen_sacrificed_toughness with a creature
whose battlefield toughness is modified, then cast Consuming Vapors, choose that
creature through the effect-zone selection, and assert life gain equals its
modified toughness rather than printed toughness. Ensure the scenario exercises
the cache-gap/LKI fallback and verifies the creature is sacrificed.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a2c1fa03-d1a6-4df8-be5a-5f3b8afa577a

📥 Commits

Reviewing files that changed from the base of the PR and between 7195a8e and e9b9304.

📒 Files selected for processing (6)

• crates/engine/src/game/effects/mod.rs
• crates/engine/src/game/effects/sacrifice.rs
• crates/engine/src/game/engine_resolution_choices.rs
• crates/engine/src/game/quantity.rs
• crates/engine/tests/integration/consuming_vapors_life_gain_5925.rs
• crates/engine/tests/integration/main.rs

[github-actions]

Parse changes introduced by this PR

✓ No card-parse changes detected.

[matthewevans]

Maintainer review: the current head keeps sacrificed-object provenance at the sacrifice-completion seam, covers the cache-miss record fallback and the cast pipeline, and resolves the prior blocking feedback. Enqueued to let required checks complete in the merge queue.