fix(ai): answer every prompt from the engine-issued candidate domain - #6973
Conversation
`choose_action` answered a prompt from one of three sources, and only two of them selected from `AiDecisionContract`'s domain. The seven specialist heuristics CONSTRUCTED a `GameAction` off the `waiting_for` payload and hoped the contract accepted it. PR #6829 added that guard without a source of truth, which converted "AI submits an illegal action" into "AI submits nothing" — and `aiController.ts` cannot distinguish a refusal from "this seat owes nothing", so it halts after three with `ai-controller-stuck:<prompt>`. Reported shape: an AI cast Praetor's Grasp against an 88-card opponent library and hung. `SearchChoice` enumeration was capped at 12 candidates while the tutor scorer ranked all 88, so the AI's own argmax was unsubmittable. - `candidates.rs`: apply the beam cap only to genuinely combinatorial enumerations. C(n,0)+C(n,1) is linear, so a single-card search now issues the whole pool. Gifts Ungiven (count=4) is unaffected. - `search.rs`: `deterministic_choice`'s `SearchChoice` arm ranks the engine's issued `SelectCards` candidates instead of re-deriving a pool; the local `BEAM_K` enumerator and `score_search_choice_cards` are deleted. All seven specialist sites bind to the contract and FALL THROUGH on a miss rather than hard-returning `None`. The `OpponentGuess` and card-predicate samplers draw from the issued actions, preserving CR 608.2d uniformity. - `ai_support/mod.rs` + `filter.rs`: `structurally_valid_search_selection` lets `SimulationFilter` skip its clone-and-apply probe for search selections. All three conditions the submission guard checks (cardinality, pool membership, CR 608.2c selection constraint) are decidable without mutating state; scoped searches defer to the simulation. Uncapping alone cost 217ms/88 candidates; with this the same list validates in 180us with zero legality clones. Gates: `choose_action_never_answers_outside_the_engine_issued_domain` mounts the invariant at `choose_action` altitude — #6964's gate sits on `fallback_action`, which the seven specialists return before, and stayed green on a tree carrying this P0. Verified RED on a reverted tree.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe engine preserves full pools for linear search actions and validates non-scoped selections structurally. The phase AI pipeline binds specialist, fallback, and scored decisions to engine-issued actions and scores complete issued selections. ChangesAI action contract alignment
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Sequence Diagram(s)sequenceDiagram
participant choose_action
participant AiDecisionContract
participant SpecialistPolicy
participant SearchSelectionScorer
choose_action->>AiDecisionContract: materialize issued actions
choose_action->>SpecialistPolicy: request specialist action
SpecialistPolicy->>AiDecisionContract: validate proposed action
AiDecisionContract-->>choose_action: accept or reject action
choose_action->>SearchSelectionScorer: score issued SelectCards selections
SearchSelectionScorer-->>choose_action: return selected issued action
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/phase-ai/src/policies/tutor.rs (1)
86-115: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winApply positional decay by card value, not selection order.
search.rssorts complete selections, not the cards inside each selection. The engine preserves input order when generating combinations, so0.88^indexdepends on enumerator order. Compute per-card scores, sort them by descending value, then apply decay by rank to discount the weakest cards.🤖 Prompt for 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. In `@crates/phase-ai/src/policies/tutor.rs` around lines 86 - 115, Update score_search_choice_selection to compute each selected card’s base score first, sort those scores in descending order, then apply the 0.88 positional decay by sorted rank so the weakest cards receive the largest discount. Preserve the existing combo bonus and duplicate-name multiplier before sorting, and sum the decayed scores without relying on chosen iteration order.
🤖 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.
Nitpick comments:
In `@crates/phase-ai/src/policies/tutor.rs`:
- Around line 86-115: Update score_search_choice_selection to compute each
selected card’s base score first, sort those scores in descending order, then
apply the 0.88 positional decay by sorted rank so the weakest cards receive the
largest discount. Preserve the existing combo bonus and duplicate-name
multiplier before sorting, and sum the decayed scores without relying on chosen
iteration order.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 8dd8c0e6-5a9d-44d9-b466-4c66f231a79d
📒 Files selected for processing (5)
crates/engine/src/ai_support/candidates.rscrates/engine/src/ai_support/filter.rscrates/engine/src/ai_support/mod.rscrates/phase-ai/src/policies/tutor.rscrates/phase-ai/src/search.rs
|
Generated for head Parse changes introduced by this PR✓ No card-parse changes detected. |
`score_search_choice_selection` applied both the redundancy discount and the `0.88^n` decay by position in the `chosen` slice. That was only ever correct because the deleted AI-local beam fed it combinations built from a score-sorted `beam_ids`, so slice position WAS value rank. Ranking the engine's issued selections removed that precondition without replacing it: the enumerator emits combinations in pool order and the candidate list is then sorted by `GameAction::cmp_stable`, i.e. by `ObjectId`. The decay therefore discounted by object id, and the same two cards scored 0.9472 or 0.9328 depending purely on which id sorted first. Sort by score before applying either discount, so the score is a function of the selected SET — which is what "an opponent takes the worst card of the set" (Gifts Ungiven, CR 608.2c) actually models. The redundancy discount now falls on the less valuable copy, and the decay on whatever is weakest after it. Caught by CodeRabbit on #6973.
|
CodeRabbit's The positional decay was correct before this PR only by accident of the caller: the deleted AI-local beam built its combinations from a score-sorted Measured on the queued commit: the same two cards scored 0.9472 or 0.9328 depending purely on which id sorted first, and the strong card took the discount whenever it sorted second. Fix sorts by score before applying either discount, making the score a function of the selected set. |
1 participant
choose_actionanswered a prompt from one of three sources, and only two ofthem selected from
AiDecisionContract's domain. The seven specialistheuristics CONSTRUCTED a
GameActionoff thewaiting_forpayload and hopedthe contract accepted it. PR #6829 added that guard without a source of truth,
which converted "AI submits an illegal action" into "AI submits nothing" — and
aiController.tscannot distinguish a refusal from "this seat owes nothing",so it halts after three with
ai-controller-stuck:<prompt>.Reported shape: an AI cast Praetor's Grasp against an 88-card opponent library
and hung.
SearchChoiceenumeration was capped at 12 candidates while thetutor scorer ranked all 88, so the AI's own argmax was unsubmittable.
candidates.rs: apply the beam cap only to genuinely combinatorialenumerations. C(n,0)+C(n,1) is linear, so a single-card search now issues
the whole pool. Gifts Ungiven (count=4) is unaffected.
search.rs:deterministic_choice'sSearchChoicearm ranks the engine'sissued
SelectCardscandidates instead of re-deriving a pool; the localBEAM_Kenumerator andscore_search_choice_cardsare deleted. All sevenspecialist sites bind to the contract and FALL THROUGH on a miss rather than
hard-returning
None. TheOpponentGuessand card-predicate samplers drawfrom the issued actions, preserving CR 608.2d uniformity.
ai_support/mod.rs+filter.rs:structurally_valid_search_selectionlets
SimulationFilterskip its clone-and-apply probe for searchselections. All three conditions the submission guard checks (cardinality,
pool membership, CR 608.2c selection constraint) are decidable without
mutating state; scoped searches defer to the simulation. Uncapping alone
cost 217ms/88 candidates; with this the same list validates in 180us with
zero legality clones.
Gates:
choose_action_never_answers_outside_the_engine_issued_domainmountsthe invariant at
choose_actionaltitude — #6964's gate sits onfallback_action, which the seven specialists return before, and stayed greenon a tree carrying this P0. Verified RED on a reverted tree.
Summary by CodeRabbit
Bug Fixes
Performance
Tests