fix(phase-ai): clamp scaled policy deltas to the critical band (Closes #5473)

[minion1227]

Summary

Fixes the unclamped-policy-delta crash in phase-ai (Closes #5473). Driving a Commander game with get_ai_action on a debug/wasm build panicked at registry.rs — policy LandAnimation scaled delta -69.9 exceeds critical band ceiling 15 (and the same for ControlChangeAwareness), followed by RuntimeError: unreachable.

Two compounding defects, fixed at the seam and at the source:

1. Seam (load-bearing). PolicyRegistry::verdicts() computed scaled_delta = delta * activation and only debug_assert!d / tracing::warn!d on overflow — it never clamped, so in release builds an out-of-band delta leaked straight into the softmax priors, while debug/wasm hard-panicked. The assert was also mis-specified: activation legitimately exceeds 1.0 (arch_times_turn reaches ~2.6 = archetype_scale 2.0 × late_game_mult 1.3), so even a perfectly band-compliant critical delta of 15 scales to 39 and trips it. The fix re-points the debug_assert at the invariant the band helpers actually guarantee — the pre-scale |delta| ≤ CRITICAL_MAX — and re-bands the amplified product through PolicyVerdict::score, the single existing clamp authority. This is identity for in-band contributions and makes every policy safe-by-construction, present and future.

2. Source. Five policies hand-built PolicyVerdict::Score { delta } literals that bypassed the band helpers and could emit out-of-band magnitudes: LandAnimation / ControlChangeAwareness (-100 sentinels — issue-named), CopyValue (a +100/±125 analog anchor — not issue-named; found by a full-class audit), and RedundancyAvoidance / DownsideAwareness (unclamped additive totals). Each now routes through a band helper (critical(-CRITICAL_MAX) for the two sentinels, score(..) for the computed deltas). The two -100 constants become -CRITICAL_MAX — the strongest finite discouragement the score contract allows, not a hard Reject (giving away a permanent / self-tapping a manland is contextually not always wrong — Donate combos, liability permanents — so it stays overridable).

3. Recurrence (build for the class). The score_contract_lint meant to catch this had a blind spot: split("#[cfg(test)]").next() truncated the production scan at the first #[cfg(test)] attribute, which for ~24 policy files is a test-only use import in the top import block — hiding their entire impl (exactly why CopyValue/RedundancyAvoidance/DownsideAwareness went undetected). Replaced with a production_source() helper that truncates only at the real #[cfg(test)] mod boundary, and grandfathered the now-visible legacy counts so no future policy can hide an out-of-band emit behind a test-only import.

Files changed

• crates/phase-ai/src/policies/registry.rs — seam clamp + pre-scale assert + regression test
• crates/phase-ai/src/policies/control_change_awareness.rs — route -100 sentinel through critical(-CRITICAL_MAX)
• crates/phase-ai/src/policies/land_animation.rs — route -100 sentinel through critical(-CRITICAL_MAX)
• crates/phase-ai/src/policies/copy_value.rs — re-band the verdict wrapper through score()
• crates/phase-ai/src/policies/redundancy_avoidance.rs — re-band the additive total through score()
• crates/phase-ai/src/policies/downside_awareness.rs — re-band the verdict through score()
• crates/phase-ai/src/policies/tests/score_contract_lint.rs — fix #[cfg(test)] blind spot + grandfather legacy counts

CR references

None — this is the AI tactical-scoring layer, not MTG game logic. The PolicyVerdict band ceiling is a CR-equivalent scoring invariant (documented as such in registry.rs), not a Comprehensive Rules rule; no CR-annotated game logic is added or changed.

Track

Developer

LLM

Model: claude-opus-4-8[1m]
Thinking: high

Verification

• cargo fmt --all — clean
• cargo clippy -p phase-ai --all-targets — 0 warnings
• cargo test -p phase-ai --lib — 1262 pass / 0 fail (incl. new scaled_delta_is_clamped_to_critical_band_when_activation_exceeds_one, production_source_stops_only_at_the_test_module, and the tightened new_policy_files_use_score_contract_helpers)
• scripts/ai-gate.sh — 0 FAIL, 2 WARN, 1 PASS; all matchups within absolute mirror tolerance. The WARN drift is the expected, benign consequence of clamping deltas that previously leaked out-of-band into release priors (the AI now scores these policies at contract-correct magnitude), and no baseline refresh is required since nothing FAILs:

matchup	exercises	baseline p0%	current p0%	flips W→L	flips L→W	sign p	status
affinity-mirror	Artifacts, PlusOneCounters, AggroPressure	20%	30%	0	0	—	WARN (avg-turn drift +3.4)
enchantress-mirror	Enchantments	50%	50%	0	0	—	PASS
red-mirror	AggroPressure	70%	80%	1	2	0.3125	WARN (not significant)

Behavior note: re-banding CopyValue's +100 "preferred-X" anchor to the band compresses its ranking when a copy target evaluates ≥ 15 (e.g. a 6/6). Argmax-first X-selection is unaffected (it still lands on the smallest preferred X); softmax sampling among only-large targets loses some separation. This is within ai-gate tolerance and is the correct contract-respecting behavior; flagged here for transparency.

Scope Expansion

None.

Validation Failures

None.

CI Failures

None.

[gemini-code-assist]

Code Review

This pull request addresses issue #5473 by ensuring that policy verdict scores are properly clamped to the critical band (CRITICAL_MAX) before and after scaling by the activation factor. It replaces raw score literals with band-dispatch helpers across several policies (control_change_awareness, copy_value, downside_awareness, land_animation, and redundancy_avoidance) and updates the registry to clamp post-scale values. Additionally, the score contract lint was corrected to only truncate at the test module boundary rather than on any test-only attribute. Feedback on the PR highlights a potential bypass in the lint parser if trailing comments are added to the #[cfg(test)] line, and suggests a robust fix to find the line end first.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Potential Lint Bypass with Trailing Comments

In production_source, the helper scans lines starting from idx + "#[cfg(test)]".len(). If a developer adds a trailing comment or other characters on the same line as #[cfg(test)] (e.g., #[cfg(test)] // tests), after.lines().find(...) will evaluate that trailing comment line first. Since it does not start with mod , the lint will fail to identify the test module boundary and skip truncation.

Suggested Fix

Find the end of the line containing #[cfg(test)] first, and then search for the next non-blank line starting from the following line.

Suggested change

	fn production_source(contents: &str) -> &str {
	for (idx, _) in contents.match_indices("#[cfg(test)]") {
	// Consider only a `#[cfg(test)]` that begins its line (column 0), i.e. a
	// top-level attribute — the test module. Attributes on indented items or
	// occurrences inside strings are not module boundaries.
	let at_line_start = idx == 0 || contents[..idx].ends_with('\n');
	if !at_line_start {
	continue;
	}
	let after = &contents[idx + "#[cfg(test)]".len()..];
	let next_non_blank = after.lines().find(|l| !l.trim().is_empty()).unwrap_or("");
	if next_non_blank.trim_start().starts_with("mod ") {
	return &contents[..idx];
	}
	}
	contents
	}
	fn production_source(contents: &str) -> &str {
	for (idx, _) in contents.match_indices("#[cfg(test)]") {
	// Consider only a `#[cfg(test)]` that begins its line (column 0), i.e. a
	// top-level attribute — the test module. Attributes on indented items or
	// occurrences inside strings are not module boundaries.
	let at_line_start = idx == 0 || contents[..idx].ends_with('\n');
	if !at_line_start {
	continue;
	}
	let line_end = contents[idx..].find('\n').map(|i| idx + i).unwrap_or(contents.len());
	let after_line = &contents[line_end..];
	let next_non_blank = after_line.lines().find(|l| !l.trim().is_empty()).unwrap_or("");
	if next_non_blank.trim_start().starts_with("mod ") {
	return &contents[..idx];
	}
	}
	contents
	}

References

1. Ensure the score contract lint is robust and cannot be bypassed by trailing comments on the #[cfg(test)] line. ^(link)

[matthewevans]

Reviewed at head f4153707. Paired-seed AI gate, Decision-cost perf gate, and all Rust gates green. Premise verified against issue #5473 — the panic (policy LandAnimation scaled delta -69.99 exceeds critical band ceiling 15, then RuntimeError: unreachable, dead wasm instance) is real and correctly localized.

Most of this is right, and the diagnosis is better than the issue's. The old debug_assert!(scaled_delta.abs() <= 15) asserted an invariant the system never held: arch_times_turn activation legitimately reaches ~2.6, so even a perfectly band-compliant critical -15 produces -39 and panics. Moving the assert onto the pre-scale delta (registry.rs:420) turns it into a true "did you route through the band helpers?" tripwire, and re-banding the product through PolicyVerdict::score (registry.rs:442) reuses the one existing clamp authority rather than introducing a second constant that could drift. scaled_delta_is_clamped_to_critical_band_when_activation_exceeds_one (registry.rs:748) drives the real PolicyRegistry::verdicts and flips on revert. The score_contract_lint repair is a genuine false-green fix — the old split("#[cfg(test)]").next() truncated at the first test-only use, which is precisely how the out-of-band literals in redundancy_avoidance / downside_awareness shipped undetected. Sibling coverage is complete: I checked the other policies and none of them emit a raw out-of-band delta.

One blocking item.

---

MED — source-side clamping collapses intra-policy ordering; rescale instead of saturate

crates/phase-ai/src/policies/copy_value.rs:215

PolicyVerdict::score(self.score(ctx), PolicyReason::new("copy_value_score"))

copy_value::score() is an analog signal with a range around ±125. PolicyVerdict::score saturates rather than rescales — anything above STRONG_MAX becomes signum · min(|x|, CRITICAL_MAX) (registry.rs:199). So:

copy candidate	raw score()	emitted delta
A	−110	−15
B	−30	−15
C	−8	−8 (identity, in band)

A and B are very different board states and copy_value can no longer distinguish them. Below the band, ordering is preserved exactly; above it, the top of the distribution flattens to a single value. This degrades the softmax move-ordering prior for SelectTarget / ChooseX copy and legend-adjacent decisions. Alpha-beta still evaluates positions, so it isn't a correctness bug — but it is a silent behavioral regression in release builds, where the full range previously reached the prior, and it is not what issue #5473 asked for.

The repo already has the right idiom for this, one file over. anti_self_harm.rs:53:

const ANTI_SELF_HARM_RAW_CRITICAL_CEILING: f64 = CRITICAL_MAX / 1.3;

which maps the raw signal into the band before dispatch, so ordering survives.

Please either:

1. Rescale copy_value::score()'s analog range into (STRONG_MAX, CRITICAL_MAX] so ordering is preserved within the band (matching the anti_self_harm self-ceiling pattern) — the preferred fix; the same question applies to the downside_awareness doubling and the redundancy_avoidance chain sum, so please state whether their ranges can exceed 15 in practice; or
2. Keep the saturation and justify it with a paired-seed report showing move-ordering is unaffected on the affected decision classes.

Either is a small change. I'm not asking for a redesign — just don't let a crash fix quietly flatten a policy's signal.

---

LOW — activation is now inert at the critical ceiling

A verdict pinned at ±CRITICAL_MAX re-bands to ±CRITICAL_MAX for any activation (-15 × 2.6 → -39 → -15). That follows from a saturating ceiling and is intended, but it means activation() tuning has no effect on the critical branch of control_change_awareness and land_animation. Worth one line of doc so the next person tuning activation doesn't spend an afternoon on it.

---

Refuting the bot finding

Gemini flags a lint bypass via a trailing comment on the #[cfg(test)] line. Inverted. If a #[cfg(test)] // comment form existed, production_source's after.lines().find(non-blank) would see the comment, skip truncation, and treat the whole file as production — a superset. That can only over-count and fail the lint loudly; it cannot hide a production literal. It fails closed, and no such form exists in the tree. The suggested tweak is a harmless robustness nit, not a correctness issue.

[minion1227]

Thanks for the thorough read — addressed all three at head 595e3892.

MED — copy_value: rescale instead of saturate ✅

Added a reusable rescale_into_critical_band(raw, raw_ceiling) in registry.rs — the anti_self_harm self-ceiling idea generalized for a signal too wide for a bare clamp. Magnitudes within STRONG_MAX pass through unchanged (below-band ordering stays exact); (STRONG_MAX, raw_ceiling] maps linearly into (STRONG_MAX, CRITICAL_MAX], so the tail keeps its order instead of colliding at the ceiling. copy_value routes score() through it with COPY_VALUE_RAW_CEILING = 130, then band-dispatches as before. Your case is now order-preserving:

copy candidate	raw score()	emitted delta
A	−110	−13.4
B	−30	−7.0
C	−8	−5.2

rescale_into_critical_band_preserves_order_where_saturation_collapses locks it (A < B < C, |A−B| > 1, band bounds) and flips on revert.

The other two ranges (you asked) — both stay in-band, no rescale

• downside_awareness: largest gift penalty is gift_card −3.0, doubled to −6.0 for pure-downside removal → score() never leaves [−6.0, 0.0]. PolicyVerdict::score is identity here; it only enforces the contract uniformly. Noted in a code comment.
• redundancy_avoidance: per-effect penalties are −3.0 / −2.0 / −1.5 / −0.5; real cards carry 1–3 redundant effects so total stays in-band. Only a pathological 5+ redundant-effect chain (no printed card) could approach 15, and there every candidate is already "strongly disprefer", so ceiling saturation is harmless. Noted in a comment; kept plain score().

LOW — activation inert at the ceiling ✅

Added the one-line doc note to both control_change_awareness and land_animation: a verdict pinned at −CRITICAL_MAX re-bands to −CRITICAL_MAX for any activation, so activation() tuning is inert on that branch.

Gemini nit ✅ (you're right it fails closed)

Agreed it can only over-count → fail loudly, never hide a literal. Applied the two-line robustness tweak anyway: production_source skips to the end of the #[cfg(test)] line before scanning for the mod boundary, so a trailing comment can't fool it. production_source_ignores_trailing_comment_on_cfg_test_line covers it.

---

Verification at 595e3892: cargo test -p phase-ai --lib 1264 pass / 0 fail; cargo clippy -p phase-ai --all-targets 0 warnings; scripts/ai-gate.sh **0 FAIL · 2 WARN · 1 PASS (identical drift to the reviewed head — the rescale doesn't move the suite, those mirrors cast no copies)**.

[github-actions]

Parse changes introduced by this PR

Baseline pending for 845c36db88c3cdef212933a7190ee2f61284aa70 — this populates once main publishes its coverage snapshot (a few minutes after that commit landed).

[matthewevans]

Approving — the blocking MED from my prior CHANGES_REQUESTED (review 4668626037) is resolved, and I re-verified the fix against head 595e3892.

MED resolved (copy_value saturation collapsing intra-policy move-ordering). copy_value::verdict now routes its wide analog signal through the new shared helper before band-dispatch:

PolicyVerdict::score(
    rescale_into_critical_band(self.score(ctx), COPY_VALUE_RAW_CEILING),
    PolicyReason::new("copy_value_score"),
)

rescale_into_critical_band (registry.rs) is identity below STRONG_MAX, maps (STRONG_MAX, raw_ceiling] linearly into (STRONG_MAX, CRITICAL_MAX], saturates only beyond the ceiling, and is sign-correct via raw.signum(). I independently verified it is monotonic and order-preserving on the overflow interval.

Hand-trace of my exact case (ceiling 130): raw −110 → over = 105/125 = 0.84 → −13.4; raw −30 → over = 25/125 = 0.20 → −7.0. Both re-band through PolicyVerdict::score as identity (magnitudes in (5,15]), emitting −13.4 and −7.0 — distinct and correctly ordered. Under the old bare-clamp path both collapsed to −15. Regression fixed.

Test discriminates. rescale_into_critical_band_preserves_order_where_saturation_collapses asserts a < b && b < c and (a−b).abs() > 1.0 for rescale(−110/−30/−8). Reverting the rescale to a bare clamp yields −15/−15/−8, so a == b and both assertions fail — the test would break on revert, not pass vacuously.

CI: all required checks green on 595e3892, including Paired-seed AI gate (pass) and Decision-cost perf gate (pass) — the designated authorities for AI behavior change.

The pre-scale debug_assert!(delta.abs() <= CRITICAL_MAX) on the raw verdict, plus the release-safe re-band of delta * activation through the single PolicyVerdict::score clamp authority, are the right shape.

One LOW observation, not blocking and not worth a CI re-trigger: activation is now inert for any verdict pinned at ±CRITICAL_MAX (−15 × any activation → −15 after re-banding), so activation() tuning has no effect on the critical branch of control_change_awareness / land_animation. Noting it here rather than requesting a comment-only change that would re-run the full AI gate cycle.