fix(ai): bind AI actions to engine-issued proposals - #6829
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR replaces direct AI actions with authority-bound proposals across the engine, WASM bridge, adapters, worker pool, dispatch pipeline, and AI controller. It also updates semantic-owner validation, Resolve All handling, card-pool lifecycle behavior, multiplayer retries, and sacrifice-cost synergy rules. ChangesAI Action Proposal Migration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant AiController
participant WasmAdapter
participant AiWorkerPool
participant EngineWorker
participant EngineWasm
AiController->>WasmAdapter: request AiActionProposal
WasmAdapter->>AiWorkerPool: score candidates
AiWorkerPool-->>WasmAdapter: score-only results
WasmAdapter->>EngineWorker: create proposal from scores
EngineWorker->>EngineWasm: issue authority-bound proposal
EngineWasm-->>EngineWorker: token, actor, and action
EngineWorker-->>WasmAdapter: AiActionProposal
WasmAdapter-->>AiController: proposal
AiController->>WasmAdapter: submit proposal
WasmAdapter->>EngineWorker: submit token, actor, and action
EngineWorker->>EngineWasm: validate and apply
EngineWasm-->>EngineWorker: applied, stale, or rejected
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Parse changes introduced by this PR✓ No card-parse changes detected. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/phase-ai/src/search.rs (1)
232-249: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftContract filtering can turn the deadlock-escape fallback into
None.
fallback_actionexists so "the game never deadlocks waiting for the AI", but.and_then(exact_contract_action)now discards it whenever the escape action is not in the issued domain. The issued domain is the simulated-legal candidate set (see the comment inai_support/context.rsLines 61-63), which is deliberately narrower than engine legality for beam/clone-capped families such asSearchChoiceand attacker/blocker declarations. Where that gap exists, the previous failure mode was one engine rejection; the new one isNone→ChooseActionNoneand a prompt nobody can answer.Same exposure on Line 249 for the softmax pick, though that source is contract-derived and safe by construction. Consider making a nonmember fallback loud (log + pick any contract candidate for the current prompt) rather than silently returning
None.🛠️ Degrade to an issued candidate instead of no action
let mut scored = score_candidates_with_session(state, ai_player, config, session); if scored.is_empty() { // No valid candidates from search — fall back to a safe escape action // so the game never deadlocks waiting for the AI. - return fallback_action(state, config).and_then(exact_contract_action); + if let Some(action) = fallback_action(state, config) { + if let Some(action) = exact_contract_action(action.clone()) { + return Some(action); + } + tracing::warn!( + ?action, + "fallback escape action is outside the issued contract; using an issued candidate" + ); + } + return contract.candidates.first().map(|c| c.action.clone()); }🤖 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/search.rs` around lines 232 - 249, Update the result handling around fallback_action and the final chosen.and_then(exact_contract_action) so contract filtering never returns None when issued candidates exist. Preserve exact_contract_action for contract-derived picks, but if fallback_action is outside the issued domain, log the mismatch and select any available contract candidate for the current prompt instead; only return None when no contract candidate exists.crates/engine/src/game/engine.rs (1)
239-244: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winValidate
semantic_ownerat the public boundary.
check_actor_authorizationauthenticates onlyauthenticated_actor, butapply_actionand interaction rebinding use the caller-suppliedsemantic_owner. A caller can therefore submit another pending player’s decision (for example, during simultaneous mulligans). Derive the owner from an engine-issued interaction contract or reject mismatched actor/owner pairs before dispatch.🤖 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/engine/src/game/engine.rs` around lines 239 - 244, Update the public apply_interaction boundary to validate semantic_owner before dispatching actions: derive it from an engine-issued interaction contract or reject any actor/owner mismatch, ensuring callers cannot submit another pending player’s decision while preserving valid interaction handling.
🧹 Nitpick comments (6)
client/src/adapter/card-db-subset.ts (1)
6-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
autoandsubsetare indistinguishable.
resolveAiPoolCardDbPlanonly branches on"full", so the three-variant mode carries two behaviors. Either collapse to"bounded" | "full"or giveautoa real decision (e.g. subset only when the engine reports a bounded universe under a size threshold).🤖 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 `@client/src/adapter/card-db-subset.ts` around lines 6 - 24, Update resolveAiPoolCardDbPlan so AiCardDataMode values have distinct behavior: either remove the redundant auto/subset distinction by collapsing the mode type, or implement an explicit auto decision based on the engine’s bounded-universe and size threshold result. Ensure subset mode remains explicitly bounded and full mode remains full.client/src/game/controllers/aiController.ts (1)
264-272: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe fallback escalation is gone, so
MAX_TOTAL_FAILURESis now dead and the surrounding comments are stale.
consecutiveFailuresandtotalFailuresare incremented together (Lines 410-411) and reset together (Lines 244-245), andMAX_CONSECUTIVE_FAILURESis 3. With this branch now doing exactly what thetotalFailures >= MAX_TOTAL_FAILURESbranch does (notifyEngineLost+stop()), the total-failure guard at Line 254 can never fire first — two counters, one behavior. The comments at Lines 90-93 and 249-253 still describe a "normal→fallback transition" and "fallback failures" that no longer exist.Collapse to a single failure budget and update the comments to describe the proposal-only flow.
🤖 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 `@client/src/game/controllers/aiController.ts` around lines 264 - 272, Collapse the failure tracking in the AI controller around consecutiveFailures and totalFailures into the single MAX_CONSECUTIVE_FAILURES budget, removing the dead MAX_TOTAL_FAILURES guard and any related counter updates or declarations. Update the stale comments near the failure-limit definitions and stop logic to describe the proposal-only flow, while preserving the existing notifyEngineLost and stop behavior when the limit is reached.crates/phase-ai/src/search.rs (1)
155-162: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftEvery AI decision now enumerates the validated candidate domain twice.
choose_action_with_sessionissues its ownAiDecisionContractand already guarantees the returned action is an exact member, yet both callers issue a second contract for the same state and owner and re-check membership.AiDecisionContract::issuerunsvalidated_candidate_actions_for_semantic_owner, which clones and re-applies state per candidate (this file's ownSearchChoicecomment calls that "hundreds of state clones"), so the interactive path pays it twice.
crates/phase-ai/src/search.rs#L155-L162: return the issued contract alongside the action (or accept one from the caller) so the domain is enumerated once per decision.crates/phase-ai/src/auto_play.rs#L205-L206: consume the contract produced by the chooser instead of issuing a separate one before the call.crates/engine-wasm/src/lib.rs#L1961-L1981: reuse that same contract for both the membership check and the registry insert rather than issuing a second one.🤖 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/search.rs` around lines 155 - 162, The AI decision flow redundantly issues and validates the same candidate domain twice. In crates/phase-ai/src/search.rs:155-162, update choose_action_with_session and its callers to return or accept the issued AiDecisionContract alongside the selected action; in crates/phase-ai/src/auto_play.rs:205-206, consume that returned contract instead of issuing another; and in crates/engine-wasm/src/lib.rs:1961-1981, reuse the same contract for membership validation and registry insertion.crates/phase-ai/src/auto_play.rs (1)
215-230: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winContract violations are reported as
ApplyFailed, contradicting that variant's documented meaning.
ApplyFailed's doc comment states "apply()rejectedplayer's chosenaction", but here nothing was applied — the AI selected outside its issued domain, which is a policy bug in a different category (and callers that surfaceApplyFailedas an engine rejection will misattribute it). A dedicated variant keeps the break reasons a faithful typed record of why the loop stopped.♻️ Add a distinct break reason
+ /// `player`'s chosen `action` was not a member of the engine-issued + /// `AiDecisionContract` for `semantic_owner`. Nothing was applied. + ContractViolation { + semantic_owner: PlayerId, + player: PlayerId, + action: Box<GameAction>, + },- if !contract.permits(state, actor, &action) { - let error = EngineError::InvalidAction( - "AI chose an action outside its issued decision contract".to_string(), - ); - tracing::error!( + if !contract.permits(state, actor, &action) { + tracing::error!( ?semantic_owner, ?actor, "AI action violated decision contract" ); - break_reason = Some(AiActionsBreakReason::ApplyFailed { - player: actor, - action: Box::new(action), - error, - }); + break_reason = Some(AiActionsBreakReason::ContractViolation { + semantic_owner, + player: actor, + action: Box::new(action), + }); break; }🤖 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/auto_play.rs` around lines 215 - 230, Add a dedicated AiActionsBreakReason variant for decision-contract violations, documenting that the AI selected an action outside its issued contract, and update the contract check in the auto-play loop to use it instead of ApplyFailed. Preserve the existing player/action/error context and logging while keeping ApplyFailed exclusively for actions rejected by apply().crates/engine-wasm/src/lib.rs (1)
1944-1961: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSemantic-owner derivation is copy-pasted three times.
The same
acting_players().contains(requested)→acting_player()→acting_players().first()cascade appears here, inget_ai_action_proposal_from_scores(Lines 2038-2047), and inresolve_all_inner(Lines 2178-2184, minus the requested-AI preference). Divergence between them silently changes which seat a proposal is bound to. Extract one engine-side helper (e.g. alongsideAiDecisionContract::issue) and call it from all three.🤖 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/engine-wasm/src/lib.rs` around lines 1944 - 1961, Extract the repeated semantic-owner selection into one engine-side helper near AiDecisionContract::issue, preserving the requested-player preference followed by acting_player, first acting player, and requested-player fallback. Replace the inline logic in the shown contract creation, get_ai_action_proposal_from_scores, and resolve_all_inner with this helper, while preserving resolve_all_inner’s existing behavior when no requested-AI preference applies.crates/engine/src/game/engine_resolve_batch.rs (1)
117-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test covers the nonmember/stale proposal rejection arm.
raw_non_pass_callback_action_is_rejected_without_applyingexercisesAction(_), andcurrent_contract_proposal_is_applied_for_non_requester_priorityexercises the permittedProposal. TheProposal { .. }fall-through — a contract that no longer permits its action — is the arm that actually protects Resolve All's action boundary, and it is untested. A case returning a proposal whoseactionis not incontract.candidates(or whosestate_revisionis stale) should assertitems_resolved == 0and an unmutated stack.🤖 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/engine/src/game/engine_resolve_batch.rs` around lines 117 - 131, Add a regression test covering the `ResolveAllCallbackDecision::Proposal { .. }` fall-through in the Resolve All callback handling. Return a proposal with an action not permitted by `contract.candidates` or with a stale `state_revision`, then assert `items_resolved == 0` and that the stack remains unchanged; keep the existing permitted-proposal and raw-action tests intact.
🤖 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 `@client/src/adapter/__tests__/p2p-adapter-multiplayer.test.ts`:
- Line 268: Update the test reset block near mockGetAiActionProposal.mockClear()
to also clear the submitAiActionProposal mock before each test, ensuring
submission call history cannot carry over into later assertions.
In `@client/src/adapter/p2p-adapter.ts`:
- Around line 1897-1913: Update submitAiActionProposal to enforce the same
gameRunState gate as submitAction and submitInteraction: reject with the
established P2P_PAUSED result whenever the state is not "running", before
invoking wasm.submitAiActionProposal. Preserve the existing authority checks and
applied-outcome broadcast, AI-loop, and persistence behavior for running games.
- Around line 1163-1174: Update runAiLoop around submitAiActionProposal so stale
outcomes cannot trigger an unbounded continue loop. Track consecutive stale
retries, cap them using the existing AI retry-limit convention, and surface an
AdapterError once the cap is reached; reset the counter after a non-stale
successful submission while preserving existing rejected and result handling.
In `@client/src/game/dispatch.ts`:
- Around line 746-755: Update dispatchAiActionProposal and the
dispatchActionInternal/processAction flow to derive status from positive
submission evidence: have processAction invoke the callback with "applied"
immediately after submit() returns a non-null result, while preserving the
existing stale notification. Track that applied signal in
dispatchAiActionProposal and return "applied" only when it is received;
otherwise return "stale" for every silent-drop or stale path.
In `@crates/engine-wasm/src/lib.rs`:
- Around line 179-201: Update the proposal registry around insert and proposal
so issuance remains bounded: retain only the newest token for the current
generation, removing any previously stored proposal when insert creates a
replacement, and discard entries from older generations as needed. Preserve
lookup validation so only the current-generation proposal can be submitted.
In `@crates/phase-ai/src/policies/self_cost.rs`:
- Around line 785-792: Update contains_sacrifice_cost to replace the wildcard
fallback with explicit matches for every current non-sacrifice AbilityCost leaf
variant. Preserve recursive traversal for Composite and OneOf and return false
for each listed leaf, so newly added variants cause a compilation failure until
handled.
---
Outside diff comments:
In `@crates/engine/src/game/engine.rs`:
- Around line 239-244: Update the public apply_interaction boundary to validate
semantic_owner before dispatching actions: derive it from an engine-issued
interaction contract or reject any actor/owner mismatch, ensuring callers cannot
submit another pending player’s decision while preserving valid interaction
handling.
In `@crates/phase-ai/src/search.rs`:
- Around line 232-249: Update the result handling around fallback_action and the
final chosen.and_then(exact_contract_action) so contract filtering never returns
None when issued candidates exist. Preserve exact_contract_action for
contract-derived picks, but if fallback_action is outside the issued domain, log
the mismatch and select any available contract candidate for the current prompt
instead; only return None when no contract candidate exists.
---
Nitpick comments:
In `@client/src/adapter/card-db-subset.ts`:
- Around line 6-24: Update resolveAiPoolCardDbPlan so AiCardDataMode values have
distinct behavior: either remove the redundant auto/subset distinction by
collapsing the mode type, or implement an explicit auto decision based on the
engine’s bounded-universe and size threshold result. Ensure subset mode remains
explicitly bounded and full mode remains full.
In `@client/src/game/controllers/aiController.ts`:
- Around line 264-272: Collapse the failure tracking in the AI controller around
consecutiveFailures and totalFailures into the single MAX_CONSECUTIVE_FAILURES
budget, removing the dead MAX_TOTAL_FAILURES guard and any related counter
updates or declarations. Update the stale comments near the failure-limit
definitions and stop logic to describe the proposal-only flow, while preserving
the existing notifyEngineLost and stop behavior when the limit is reached.
In `@crates/engine-wasm/src/lib.rs`:
- Around line 1944-1961: Extract the repeated semantic-owner selection into one
engine-side helper near AiDecisionContract::issue, preserving the
requested-player preference followed by acting_player, first acting player, and
requested-player fallback. Replace the inline logic in the shown contract
creation, get_ai_action_proposal_from_scores, and resolve_all_inner with this
helper, while preserving resolve_all_inner’s existing behavior when no
requested-AI preference applies.
In `@crates/engine/src/game/engine_resolve_batch.rs`:
- Around line 117-131: Add a regression test covering the
`ResolveAllCallbackDecision::Proposal { .. }` fall-through in the Resolve All
callback handling. Return a proposal with an action not permitted by
`contract.candidates` or with a stale `state_revision`, then assert
`items_resolved == 0` and that the stack remains unchanged; keep the existing
permitted-proposal and raw-action tests intact.
In `@crates/phase-ai/src/auto_play.rs`:
- Around line 215-230: Add a dedicated AiActionsBreakReason variant for
decision-contract violations, documenting that the AI selected an action outside
its issued contract, and update the contract check in the auto-play loop to use
it instead of ApplyFailed. Preserve the existing player/action/error context and
logging while keeping ApplyFailed exclusively for actions rejected by apply().
In `@crates/phase-ai/src/search.rs`:
- Around line 155-162: The AI decision flow redundantly issues and validates the
same candidate domain twice. In crates/phase-ai/src/search.rs:155-162, update
choose_action_with_session and its callers to return or accept the issued
AiDecisionContract alongside the selected action; in
crates/phase-ai/src/auto_play.rs:205-206, consume that returned contract instead
of issuing another; and in crates/engine-wasm/src/lib.rs:1961-1981, reuse the
same contract for membership validation and registry insertion.
🪄 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: e7d68b54-5fbe-4340-b2ab-6501106769d0
⛔ Files ignored due to path filters (1)
client/src/wasm/engine_wasm.d.tsis excluded by!client/src/wasm/**,!**/*.d.ts
📒 Files selected for processing (32)
client/src/adapter/__tests__/ai-card-subset.test.tsclient/src/adapter/__tests__/ai-worker-pool.test.tsclient/src/adapter/__tests__/p2p-adapter-multiplayer.test.tsclient/src/adapter/__tests__/wasm-adapter.test.tsclient/src/adapter/__tests__/ws-adapter.test.tsclient/src/adapter/ai-worker-pool.tsclient/src/adapter/card-db-subset.tsclient/src/adapter/engine-worker-client.tsclient/src/adapter/engine-worker.tsclient/src/adapter/p2p-adapter.tsclient/src/adapter/replay-adapter.tsclient/src/adapter/server-draft-adapter.tsclient/src/adapter/types.tsclient/src/adapter/wasm-adapter.tsclient/src/adapter/ws-adapter.tsclient/src/game/__tests__/dispatchSplitEpochSoftlock.test.tsclient/src/game/__tests__/dispatchTurnControlPayCostQueue.test.tsclient/src/game/controllers/__tests__/aiController.test.tsclient/src/game/controllers/aiController.tsclient/src/game/dispatch.tsclient/src/pages/__tests__/greenwardenDoubledTrigger.test.tsclient/src/pages/__tests__/optionalEffectChoiceTransition.test.tsxclient/src/test/factories/engineAdapterFactory.tscrates/engine-wasm/src/lib.rscrates/engine/src/ai_support/context.rscrates/engine/src/ai_support/mod.rscrates/engine/src/game/engine.rscrates/engine/src/game/engine_resolve_batch.rscrates/phase-ai/src/auto_play.rscrates/phase-ai/src/policies/self_cost.rscrates/phase-ai/src/policies/self_cost_value.rscrates/phase-ai/src/search.rs
💤 Files with no reviewable changes (7)
- client/src/adapter/tests/ws-adapter.test.ts
- client/src/adapter/ws-adapter.ts
- client/src/adapter/replay-adapter.ts
- client/src/pages/tests/optionalEffectChoiceTransition.test.tsx
- client/src/adapter/server-draft-adapter.ts
- client/src/adapter/tests/wasm-adapter.test.ts
- client/src/pages/tests/greenwardenDoubledTrigger.test.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/src/adapter/p2p-adapter.ts (1)
1168-1172: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRe-check host authority and run state before direct WASM submission.
A disconnect or host-lease handoff can occur during either
await. This path then bypassessubmitAiActionProposal’s fences and may apply an AI action after the game pauses or this host is superseded. Revalidate immediately before submitting.Proposed fix
if (!proposal) { return; } + if (!this.ownsAuthority()) return; + if (this.gameRunState !== "running") return; const outcome = await this.wasm.submitAiActionProposal(proposal);As per path instructions, check relevant async races.
🤖 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 `@client/src/adapter/p2p-adapter.ts` around lines 1168 - 1172, Before calling submitAiActionProposal in the AI action flow, revalidate the current host authority and active game/run state after await this.wasm.getAiActionProposal returns. Abort without submitting when either fence is no longer satisfied, then preserve the existing proposal check and direct WASM submission for valid state.Source: Path instructions
🤖 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.
Outside diff comments:
In `@client/src/adapter/p2p-adapter.ts`:
- Around line 1168-1172: Before calling submitAiActionProposal in the AI action
flow, revalidate the current host authority and active game/run state after
await this.wasm.getAiActionProposal returns. Abort without submitting when
either fence is no longer satisfied, then preserve the existing proposal check
and direct WASM submission for valid state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: edcec272-1a07-4b26-bd7f-5c51224e1569
📒 Files selected for processing (8)
client/src/adapter/__tests__/p2p-adapter-multiplayer.test.tsclient/src/adapter/p2p-adapter.tsclient/src/game/dispatch.tscrates/engine-wasm/src/lib.rscrates/engine/src/ai_support/context.rscrates/engine/src/game/engine_resolve_batch.rscrates/phase-ai/src/policies/self_cost.rscrates/phase-ai/src/policies/self_cost_value.rs
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/engine-wasm/src/lib.rs (1)
179-199: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winScope proposal invalidation by semantic owner.
self.proposals.clear()invalidates P0’s token when a P1 proposal is issued during a simultaneous mulligan, despite each contract being owner-scoped. Retain only prior entries for the incomingsemantic_ownerand add a regression test that submits P0 after issuing P1.🤖 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/engine-wasm/src/lib.rs` around lines 179 - 199, The insert method currently clears proposals across all semantic owners, invalidating P0 when issuing P1. In AiDecisionContract::insert, retain existing entries whose semantic owner matches the incoming contract and remove proposals belonging to other owners; then add a regression test that issues P1 followed by submitting P0 and verifies P0 remains valid.
🤖 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/src/ai_support/context.rs`:
- Around line 79-93: Update candidate_action_matches to normalize and compare
the Vec payloads of ChooseKeptCreatures and ChooseKeptPermanents as unordered
sets, matching the existing SelectCards behavior. Preserve positional comparison
for all other action variants, including target slots, distributions, and
trigger ordering.
---
Outside diff comments:
In `@crates/engine-wasm/src/lib.rs`:
- Around line 179-199: The insert method currently clears proposals across all
semantic owners, invalidating P0 when issuing P1. In AiDecisionContract::insert,
retain existing entries whose semantic owner matches the incoming contract and
remove proposals belonging to other owners; then add a regression test that
issues P1 followed by submitting P0 and verifies P0 remains valid.
🪄 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: fc9d2801-aae9-4542-827a-0beb293ef9df
📒 Files selected for processing (3)
crates/engine-wasm/src/lib.rscrates/engine/src/ai_support/context.rscrates/phase-ai/src/search.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/phase-ai/src/search.rs
1 participant
Summary by CodeRabbit
New Features
Bug Fixes