pokebedrock/pokebedrock-showdown
4
0
Fork 0
Code Issues Pull requests 1 Projects Releases Packages Activity Actions

Add AI telemetry, SPRT, and baseline checks #13

Merged
smell-of-curry merged 1 commit from improve-ai into master 2026-09-11 19:30:42 +00:00
Conversation 4 Commits 1 Files changed 27 +2679 -184
smell-of-curry commented 2026-07-26 16:23:11 +00:00 (Migrated from github.com)
Copy link

Expands the self-play harness with mirrored seeded teams, confidence intervals, SPRT early stopping, optional decision telemetry, JSON output, and baseline regression gating (plus a new ai:baseline script and committed tier baselines). Reworks strategic AI tuning and evaluation across heuristic/one-ply/MCTS paths to avoid immune/useless choices, improve status/setup valuation, and apply info-forgetting via foe-state views instead of score scaling. Also fixes team pack/unpack symmetry for PokeBedrock fields (uuid, currentHealth, status, movesInfo) and adds targeted tests for round-tripping, status-block handling, and decision-quality guarantees.

Summary by CodeRabbit

  • New Features
    • Added self-play analysis with engine selection, mirrored matches, confidence intervals, Elo estimates, telemetry, and sequential early stopping.
    • Improved AI difficulty tuning, information handling, move evaluation, setup planning, immunity detection, and switching decisions.
    • Added support for preserving Pokémon health, status, identifiers, and move PP in team data.
  • Bug Fixes
    • Prevented ineffective, blocked, or immune moves from being incorrectly prioritized.
  • Documentation
    • Added Strategic-AI roadmap and baseline measurement guidance.
  • Tests
    • Added coverage for team round-tripping, status moves, and decision quality.
Expands the self-play harness with mirrored seeded teams, confidence intervals, SPRT early stopping, optional decision telemetry, JSON output, and baseline regression gating (plus a new `ai:baseline` script and committed tier baselines). Reworks strategic AI tuning and evaluation across heuristic/one-ply/MCTS paths to avoid immune/useless choices, improve status/setup valuation, and apply info-forgetting via foe-state views instead of score scaling. Also fixes team pack/unpack symmetry for PokeBedrock fields (`uuid`, `currentHealth`, `status`, `movesInfo`) and adds targeted tests for round-tripping, status-block handling, and decision-quality guarantees. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added self-play analysis with engine selection, mirrored matches, confidence intervals, Elo estimates, telemetry, and sequential early stopping. * Improved AI difficulty tuning, information handling, move evaluation, setup planning, immunity detection, and switching decisions. * Added support for preserving Pokémon health, status, identifiers, and move PP in team data. * **Bug Fixes** * Prevented ineffective, blocked, or immune moves from being incorrectly prioritized. * **Documentation** * Added Strategic-AI roadmap and baseline measurement guidance. * **Tests** * Added coverage for team round-tripping, status moves, and decision quality. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
cursor[bot] commented 2026-07-26 16:23:15 +00:00 (Migrated from github.com)
Copy link

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

<!-- BUGBOT_FREE_TIER_DISABLED_UPSELL --> Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the [Cursor dashboard](https://www.cursor.com/dashboard/bugbot) to get automatic reviews on future PRs.
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2026-07-26 16:23:17 +00:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment
Copy link

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
coderabbitai[bot] commented 2026-07-26 16:23:38 +00:00 (Migrated from github.com)
Copy link

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds PokeBedrock team-state round trips, expands Strategic-AI move evaluation and difficulty tuning, introduces decision telemetry and statistical self-play analysis, and adds mirrored baseline checks, tests, CLI options, and roadmap documentation.

Changes

Strategic AI and evaluation

Layer / File(s) Summary
Difficulty policy and information handling
sim/tools/strategic-ai/policy/*, sim/tools/strategic-ai/engines/Engine.ts, sim/tools/strategic-ai/engines/HeuristicEngine.ts		 Difficulty tiers gain engine validation, ladder-ratio tuning, updated knobs, and probabilistic foe-information forgetting.
Move, damage, and search evaluation
sim/tools/strategic-ai/mechanics/*, sim/tools/strategic-ai/engines/LightHeuristicEngine.ts, MctsEngine.ts, OnePlySearchEngine.ts, test/sim/tools/strategic-ai/status-move-blocked.js		 Move evaluation now rejects ineffective attacks and blocked statuses, prices accuracy and setup value, centralizes ability immunities, and updates search, rollout, and switch scoring.
Decision telemetry
sim/tools/strategic-ai/telemetry/DecisionStats.ts, sim/tools/player-ai.ts, test/sim/tools/strategic-ai/decision-quality.js		 PlayerAI optionally records decisions and rejections; telemetry counters and rates cover move outcomes, switches, passes, defaults, and rejections.
Self-play statistics and stopping
sim/tools/ai-stats.ts, sim/tools/selfplay.ts		 Self-play supports deterministic teams, mirrored games, engine pinning, telemetry, score/Elo intervals, ordered aggregation, and SPRT early stopping.
Baseline verification and roadmap
sim/tools/strategic-ai/baselines/*, sim/tools/strategic-ai/ROADMAP.md, package.json		 Adds five recorded tier baselines, a shell verification gate, the ai:baseline script, and Strategic-AI measurement and roadmap documentation.

Team state

Layer / File(s) Summary
PokeBedrock team-state round trips
sim/teams.ts, test/sim/misc/teams-pack-roundtrip.js		 Team packing preserves UUID, health, status, and move PP metadata while unpacking validates optional numeric fields and remains stable across repeated round trips.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

Suggested reviewers: copilot

🚥 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 accurately summarizes the main additions: telemetry, SPRT early stopping, and baseline regression checks.
Docstring Coverage ✅ Passed Docstring coverage is 84.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch improve-ai

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

<!-- This is an auto-generated comment: summarize by coderabbit.ai --> <!-- review_stack_entry_start --> [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/smell-of-curry/pokebedrock-showdown/pull/13?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- walkthrough_start --> <details> <summary>📝 Walkthrough</summary> ## Walkthrough The PR adds PokeBedrock team-state round trips, expands Strategic-AI move evaluation and difficulty tuning, introduces decision telemetry and statistical self-play analysis, and adds mirrored baseline checks, tests, CLI options, and roadmap documentation. ### Changes **Strategic AI and evaluation** |Layer / File(s)|Summary| |---|---| |**Difficulty policy and information handling** <br> `sim/tools/strategic-ai/policy/*`, `sim/tools/strategic-ai/engines/Engine.ts`, `sim/tools/strategic-ai/engines/HeuristicEngine.ts`|Difficulty tiers gain engine validation, ladder-ratio tuning, updated knobs, and probabilistic foe-information forgetting.| |**Move, damage, and search evaluation** <br> `sim/tools/strategic-ai/mechanics/*`, `sim/tools/strategic-ai/engines/LightHeuristicEngine.ts`, `MctsEngine.ts`, `OnePlySearchEngine.ts`, `test/sim/tools/strategic-ai/status-move-blocked.js`|Move evaluation now rejects ineffective attacks and blocked statuses, prices accuracy and setup value, centralizes ability immunities, and updates search, rollout, and switch scoring.| |**Decision telemetry** <br> `sim/tools/strategic-ai/telemetry/DecisionStats.ts`, `sim/tools/player-ai.ts`, `test/sim/tools/strategic-ai/decision-quality.js`|PlayerAI optionally records decisions and rejections; telemetry counters and rates cover move outcomes, switches, passes, defaults, and rejections.| |**Self-play statistics and stopping** <br> `sim/tools/ai-stats.ts`, `sim/tools/selfplay.ts`|Self-play supports deterministic teams, mirrored games, engine pinning, telemetry, score/Elo intervals, ordered aggregation, and SPRT early stopping.| |**Baseline verification and roadmap** <br> `sim/tools/strategic-ai/baselines/*`, `sim/tools/strategic-ai/ROADMAP.md`, `package.json`|Adds five recorded tier baselines, a shell verification gate, the `ai:baseline` script, and Strategic-AI measurement and roadmap documentation.| **Team state** |Layer / File(s)|Summary| |---|---| |**PokeBedrock team-state round trips** <br> `sim/teams.ts`, `test/sim/misc/teams-pack-roundtrip.js`|Team packing preserves UUID, health, status, and move PP metadata while unpacking validates optional numeric fields and remains stable across repeated round trips.| **Estimated code review effort:** 5 (Critical) | ~120 minutes **Possibly related PRs** - [smell-of-curry/pokebedrock-showdown#10](https://github.com/smell-of-curry/pokebedrock-showdown/pull/10): Overlaps in the PlayerAI and Strategic-AI engine and policy paths. - [smell-of-curry/pokebedrock-showdown#11](https://github.com/smell-of-curry/pokebedrock-showdown/pull/11): Overlaps in move, switch, and heuristic evaluation logic. - [smell-of-curry/pokebedrock-showdown#12](https://github.com/smell-of-curry/pokebedrock-showdown/pull/12): Overlaps in self-play, PlayerAI, EngineContext, and Strategic-AI integration. **Suggested reviewers:** `copilot` </details> <!-- walkthrough_end --> <!-- pre_merge_checks_walkthrough_start --> <details> <summary>🚥 Pre-merge checks | ✅ 5</summary> <details> <summary>✅ Passed checks (5 passed)</summary> | Check name | Status | Explanation | | :------------------------: | :------- | :------------------------------------------------------------------------------------------------------------------ | | Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. | | Title check | ✅ Passed | The title accurately summarizes the main additions: telemetry, SPRT early stopping, and baseline regression checks. | | Docstring Coverage | ✅ Passed | Docstring coverage is 84.00% which is sufficient. The required threshold is 80.00%. | | Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. | | Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. | </details> </details> <!-- pre_merge_checks_walkthrough_end --> <!-- finishing_touch_checkbox_start --> <details> <summary>✨ Finishing Touches</summary> <details> <summary>📝 Generate docstrings</summary> - [ ] <!-- {"checkboxId": "7962f53c-55bc-4827-bfbf-6a18da830691"} --> Create stacked PR - [ ] <!-- {"checkboxId": "3e1879ae-f29b-4d0d-8e06-d12b7ba33d98"} --> Commit on current branch </details> <details> <summary>🧪 Generate unit tests (beta)</summary> - [ ] <!-- {"checkboxId": "f47ac10b-58cc-4372-a567-0e02b2c3d479", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} --> Create PR with unit tests - [ ] <!-- {"checkboxId": "6ba7b810-9dad-11d1-80b4-00c04fd430c8", "radioGroupId": "utg-output-choice-group-unknown_comment_id"} --> Commit unit tests in branch `improve-ai` </details> </details> <!-- finishing_touch_checkbox_end --> <!-- tips_start --> --- Thanks for using [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=smell-of-curry/pokebedrock-showdown&utm_content=13)! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. <details> <summary>❤️ Share</summary> - [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code) </details> <sub>Comment `@coderabbitai help` to get the list of available commands.</sub> <!-- tips_end -->
coderabbitai[bot] (Migrated from github.com) reviewed 2026-07-26 16:33:51 +00:00
coderabbitai[bot] (Migrated from github.com) left a comment
Copy link

Actionable comments posted: 8

🧹 Nitpick comments (2)
test/sim/tools/strategic-ai/status-move-blocked.js (1)

30-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

mkTracked omits inferredImmunities, which TrackedPokemon declares as a required Set.

Nothing on the current evaluateMove path touches it, but any evaluator that later reads mon.inferredImmunities.has(...) will throw a TypeError from this fixture rather than fail an assertion. Cheap to seed alongside revealedMoves/volatiles.

♻️ Proposed tweak 
 		revealedMoves: new Set(opts.revealedMoves || []),
+		inferredImmunities: new Set(opts.inferredImmunities || []),
 		sameMoveStreak: 0,
🤖 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 `@test/sim/tools/strategic-ai/status-move-blocked.js` around lines 30 - 57,
Update mkTracked to initialize the required inferredImmunities field as a Set,
seeded from an optional opts.inferredImmunities value alongside revealedMoves
and volatiles, so evaluators can safely call .has(...).
sim/tools/selfplay.ts (1)

493-530: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

SPRT verdict/LLR reported at the end can include overshoot games from worker-pool concurrency, not the exact stopping point.

shouldStop() (Line 415) is only checked before claiming a new task; tasks already claimed by other lanes keep running to completion after stopped flips true inside drain() (Line 527). The final sprt.verdict()/sprt.llr() call (Line 576) re-reads the Sprt instance after Promise.all resolves, so with the default multi-worker jobs, the returned/printed verdict can reflect a handful of extra post-stop games rather than the exact moment the bound was cleared. This doesn't affect jobs === 1 (Line 556-560 breaks immediately), only the default parallel path.

Consider snapshotting the verdict/LLR the instant stopped is set, and returning that snapshot instead of re-querying sprt at the end.

♻️ Sketch of capturing the verdict at the stopping instant 
+	let stopVerdict: { verdict: SprtVerdict, llr: number } | undefined;
 	const drain = () => {
 		while (pending.has(nextToScore)) {
 			...
 			if (sprt) {
 				sprt.observe(scores[scores.length - 1]);
-				if (sprt.verdict() !== "continue") stopped = true;
+				if (!stopped && sprt.verdict() !== "continue") {
+					stopped = true;
+					stopVerdict = { verdict: sprt.verdict(), llr: sprt.llr() };
+				}
 			}
 		}
 	};
...
-		...(sprt ? { sprt: { verdict: sprt.verdict(), llr: sprt.llr(), config: sprt.config } } : {}),
+		...(sprt ? { sprt: { verdict: (stopVerdict ?? sprt).verdict, llr: (stopVerdict ?? sprt).llr, config: sprt.config } } : {}),

Also applies to: 556-579

🤖 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 `@sim/tools/selfplay.ts` around lines 493 - 530, Snapshot the SPRT verdict and
LLR at the moment `sprt.verdict()` first changes `stopped` inside `drain`,
before later worker results can be processed. Update the final result/reporting
path to return and print that snapshot instead of re-reading `sprt` after
`Promise.all`; preserve the existing behavior when no stopping verdict is
reached.
🤖 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 `@package.json`:
- Line 49: Update the ai:baseline npm script to build the current sources before
invoking sim/tools/strategic-ai/baselines/check.sh, ensuring clean checkouts and
source changes use fresh output. Also revise the README baseline instructions to
remove any now-redundant separate build step.

In `@sim/teams.ts`:
- Around line 173-181: Sanitize the externally supplied set.uuid and set.status
values before appending them in the packing logic near packName, removing or
rejecting the delimiter characters | and ] so they cannot alter field
boundaries. Preserve empty-value handling and ensure unpack continues to assign
subsequent item, ability, and move fields correctly.

In `@sim/tools/player-ai.ts`:
- Around line 368-370: Update the telemetry path around the caller of
DecisionStats.record so multi-slot choices resolve the acting and opposing
entities using each command’s slot/target before recording. Ensure
DecisionStats.recordSlot uses the provided slot index when evaluating
tracker.myActive and tracker.foeActive, preserving correct singles behavior and
avoiding telemetry for the wrong attacker or target.

In `@sim/tools/strategic-ai/engines/HeuristicEngine.ts`:
- Around line 309-314: Update the decision flow around trackedFoe, foeMon, and
the search-model setup so evalCtx.tracker uses a per-decision tracker view
consistent with the hazy foeMon, rather than the complete tracker. Ensure
OnePlySearchEngine and MctsEngine derive foe-move models from this same
infoForgetting-aware view while preserving the existing single-resolution
consistency for the turn.

In `@sim/tools/strategic-ai/mechanics/MoveEvaluator.ts`:
- Around line 415-429: Update the status type-immunity check in the
MoveEvaluator status flow so Steel and Poison immunity is bypassed when the
attacker has the Corrosion ability. Preserve the existing STATUS_IMMUNE_TYPES
behavior for all other abilities and statuses, while allowing Corrosion users to
inflict poison or toxic on those targets.

In `@sim/tools/strategic-ai/policy/DifficultyPolicy.ts`:
- Around line 153-157: Update autoFor so difficulty 0 returns RandomEngine,
while preserving the existing heuristic mapping for difficulties 1–3 and the
remaining difficulty behavior.

In `@sim/tools/strategic-ai/policy/InfoForgetting.ts`:
- Around line 76-88: Update the item-forgetting logic in the foe forgetting
flow, near forgetItem and the returned item field, so items that cause status
moves to fail—such as Safety Goggles—are preserved. Ensure statusMoveBlockedBy
can still access these move-blocking items during evaluation, while retaining
forgetting behavior for other items.

In `@sim/tools/strategic-ai/telemetry/DecisionStats.ts`:
- Around line 160-186: Update the attacker lookup in both recordSlot and
hadConnectingAlternative to use BattleStateTracker.getActive with the current
slot and the tracked mySide, instead of tracker.myActive. Preserve the existing
per-slot iteration and calculations so doubles/triples telemetry attributes
damage, immunity, and alternative-choice counters to the Pokémon that made each
slot’s choice.

---

Nitpick comments:
In `@sim/tools/selfplay.ts`:
- Around line 493-530: Snapshot the SPRT verdict and LLR at the moment
`sprt.verdict()` first changes `stopped` inside `drain`, before later worker
results can be processed. Update the final result/reporting path to return and
print that snapshot instead of re-reading `sprt` after `Promise.all`; preserve
the existing behavior when no stopping verdict is reached.

In `@test/sim/tools/strategic-ai/status-move-blocked.js`:
- Around line 30-57: Update mkTracked to initialize the required
inferredImmunities field as a Set, seeded from an optional
opts.inferredImmunities value alongside revealedMoves and volatiles, so
evaluators can safely call .has(...).
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 30178f3d-b041-42e7-92bf-d1e99a848662

📥 Commits

Reviewing files that changed from the base of the PR and between 85d344dac7 and 5af8f6194f.

📒 Files selected for processing (27)
  • package.json
  • sim/teams.ts
  • sim/tools/ai-stats.ts
  • sim/tools/player-ai.ts
  • sim/tools/selfplay.ts
  • sim/tools/strategic-ai/ROADMAP.md
  • sim/tools/strategic-ai/baselines/README.md
  • sim/tools/strategic-ai/baselines/check.sh
  • sim/tools/strategic-ai/baselines/d1-vs-random.json
  • sim/tools/strategic-ai/baselines/d2-vs-random.json
  • sim/tools/strategic-ai/baselines/d3-vs-random.json
  • sim/tools/strategic-ai/baselines/d4-vs-random.json
  • sim/tools/strategic-ai/baselines/d5-vs-random.json
  • sim/tools/strategic-ai/engines/Engine.ts
  • sim/tools/strategic-ai/engines/HeuristicEngine.ts
  • sim/tools/strategic-ai/engines/LightHeuristicEngine.ts
  • sim/tools/strategic-ai/engines/MctsEngine.ts
  • sim/tools/strategic-ai/engines/OnePlySearchEngine.ts
  • sim/tools/strategic-ai/mechanics/DamageCalc.ts
  • sim/tools/strategic-ai/mechanics/MoveEvaluator.ts
  • sim/tools/strategic-ai/mechanics/SwitchEvaluator.ts
  • sim/tools/strategic-ai/policy/DifficultyPolicy.ts
  • sim/tools/strategic-ai/policy/InfoForgetting.ts
  • sim/tools/strategic-ai/telemetry/DecisionStats.ts
  • test/sim/misc/teams-pack-roundtrip.js
  • test/sim/tools/strategic-ai/decision-quality.js
  • test/sim/tools/strategic-ai/status-move-blocked.js
**Actionable comments posted: 8** <details> <summary>🧹 Nitpick comments (2)</summary><blockquote> <details> <summary>test/sim/tools/strategic-ai/status-move-blocked.js (1)</summary><blockquote> `30-57`: _📐 Maintainability & Code Quality_ | _🔵 Trivial_ | _⚡ Quick win_ **`mkTracked` omits `inferredImmunities`, which `TrackedPokemon` declares as a required `Set`.** Nothing on the current `evaluateMove` path touches it, but any evaluator that later reads `mon.inferredImmunities.has(...)` will throw a `TypeError` from this fixture rather than fail an assertion. Cheap to seed alongside `revealedMoves`/`volatiles`. <details> <summary>♻️ Proposed tweak</summary> ```diff revealedMoves: new Set(opts.revealedMoves || []), + inferredImmunities: new Set(opts.inferredImmunities || []), sameMoveStreak: 0, ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/sim/tools/strategic-ai/status-move-blocked.js` around lines 30 - 57, Update mkTracked to initialize the required inferredImmunities field as a Set, seeded from an optional opts.inferredImmunities value alongside revealedMoves and volatiles, so evaluators can safely call .has(...). ``` </details> <!-- cr-comment:v1:29fe0d66f051880e04f145ae --> </blockquote></details> <details> <summary>sim/tools/selfplay.ts (1)</summary><blockquote> `493-530`: _🎯 Functional Correctness_ | _🔵 Trivial_ | _⚡ Quick win_ **SPRT verdict/LLR reported at the end can include overshoot games from worker-pool concurrency, not the exact stopping point.** `shouldStop()` (Line 415) is only checked before *claiming* a new task; tasks already claimed by other lanes keep running to completion after `stopped` flips true inside `drain()` (Line 527). The final `sprt.verdict()`/`sprt.llr()` call (Line 576) re-reads the `Sprt` instance after `Promise.all` resolves, so with the default multi-worker `jobs`, the returned/printed verdict can reflect a handful of extra post-stop games rather than the exact moment the bound was cleared. This doesn't affect `jobs === 1` (Line 556-560 breaks immediately), only the default parallel path. Consider snapshotting the verdict/LLR the instant `stopped` is set, and returning that snapshot instead of re-querying `sprt` at the end. <details> <summary>♻️ Sketch of capturing the verdict at the stopping instant</summary> ```diff + let stopVerdict: { verdict: SprtVerdict, llr: number } | undefined; const drain = () => { while (pending.has(nextToScore)) { ... if (sprt) { sprt.observe(scores[scores.length - 1]); - if (sprt.verdict() !== "continue") stopped = true; + if (!stopped && sprt.verdict() !== "continue") { + stopped = true; + stopVerdict = { verdict: sprt.verdict(), llr: sprt.llr() }; + } } } }; ... - ...(sprt ? { sprt: { verdict: sprt.verdict(), llr: sprt.llr(), config: sprt.config } } : {}), + ...(sprt ? { sprt: { verdict: (stopVerdict ?? sprt).verdict, llr: (stopVerdict ?? sprt).llr, config: sprt.config } } : {}), ``` </details> Also applies to: 556-579 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sim/tools/selfplay.ts` around lines 493 - 530, Snapshot the SPRT verdict and LLR at the moment `sprt.verdict()` first changes `stopped` inside `drain`, before later worker results can be processed. Update the final result/reporting path to return and print that snapshot instead of re-reading `sprt` after `Promise.all`; preserve the existing behavior when no stopping verdict is reached. ``` </details> <!-- cr-comment:v1:7a6f56cb06b21f7a7721a3ad --> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary> ``` 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 `@package.json`: - Line 49: Update the ai:baseline npm script to build the current sources before invoking sim/tools/strategic-ai/baselines/check.sh, ensuring clean checkouts and source changes use fresh output. Also revise the README baseline instructions to remove any now-redundant separate build step. In `@sim/teams.ts`: - Around line 173-181: Sanitize the externally supplied set.uuid and set.status values before appending them in the packing logic near packName, removing or rejecting the delimiter characters | and ] so they cannot alter field boundaries. Preserve empty-value handling and ensure unpack continues to assign subsequent item, ability, and move fields correctly. In `@sim/tools/player-ai.ts`: - Around line 368-370: Update the telemetry path around the caller of DecisionStats.record so multi-slot choices resolve the acting and opposing entities using each command’s slot/target before recording. Ensure DecisionStats.recordSlot uses the provided slot index when evaluating tracker.myActive and tracker.foeActive, preserving correct singles behavior and avoiding telemetry for the wrong attacker or target. In `@sim/tools/strategic-ai/engines/HeuristicEngine.ts`: - Around line 309-314: Update the decision flow around trackedFoe, foeMon, and the search-model setup so evalCtx.tracker uses a per-decision tracker view consistent with the hazy foeMon, rather than the complete tracker. Ensure OnePlySearchEngine and MctsEngine derive foe-move models from this same infoForgetting-aware view while preserving the existing single-resolution consistency for the turn. In `@sim/tools/strategic-ai/mechanics/MoveEvaluator.ts`: - Around line 415-429: Update the status type-immunity check in the MoveEvaluator status flow so Steel and Poison immunity is bypassed when the attacker has the Corrosion ability. Preserve the existing STATUS_IMMUNE_TYPES behavior for all other abilities and statuses, while allowing Corrosion users to inflict poison or toxic on those targets. In `@sim/tools/strategic-ai/policy/DifficultyPolicy.ts`: - Around line 153-157: Update autoFor so difficulty 0 returns RandomEngine, while preserving the existing heuristic mapping for difficulties 1–3 and the remaining difficulty behavior. In `@sim/tools/strategic-ai/policy/InfoForgetting.ts`: - Around line 76-88: Update the item-forgetting logic in the foe forgetting flow, near forgetItem and the returned item field, so items that cause status moves to fail—such as Safety Goggles—are preserved. Ensure statusMoveBlockedBy can still access these move-blocking items during evaluation, while retaining forgetting behavior for other items. In `@sim/tools/strategic-ai/telemetry/DecisionStats.ts`: - Around line 160-186: Update the attacker lookup in both recordSlot and hadConnectingAlternative to use BattleStateTracker.getActive with the current slot and the tracked mySide, instead of tracker.myActive. Preserve the existing per-slot iteration and calculations so doubles/triples telemetry attributes damage, immunity, and alternative-choice counters to the Pokémon that made each slot’s choice. --- Nitpick comments: In `@sim/tools/selfplay.ts`: - Around line 493-530: Snapshot the SPRT verdict and LLR at the moment `sprt.verdict()` first changes `stopped` inside `drain`, before later worker results can be processed. Update the final result/reporting path to return and print that snapshot instead of re-reading `sprt` after `Promise.all`; preserve the existing behavior when no stopping verdict is reached. In `@test/sim/tools/strategic-ai/status-move-blocked.js`: - Around line 30-57: Update mkTracked to initialize the required inferredImmunities field as a Set, seeded from an optional opts.inferredImmunities value alongside revealedMoves and volatiles, so evaluators can safely call .has(...). ``` </details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro Plus **Run ID**: `30178f3d-b041-42e7-92bf-d1e99a848662` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 85d344dac7bac95bc8d3ce9dd0197b9b9831025f and 5af8f6194fd1b102aafae77d40348b2fe7643f10. </details> <details> <summary>📒 Files selected for processing (27)</summary> * `package.json` * `sim/teams.ts` * `sim/tools/ai-stats.ts` * `sim/tools/player-ai.ts` * `sim/tools/selfplay.ts` * `sim/tools/strategic-ai/ROADMAP.md` * `sim/tools/strategic-ai/baselines/README.md` * `sim/tools/strategic-ai/baselines/check.sh` * `sim/tools/strategic-ai/baselines/d1-vs-random.json` * `sim/tools/strategic-ai/baselines/d2-vs-random.json` * `sim/tools/strategic-ai/baselines/d3-vs-random.json` * `sim/tools/strategic-ai/baselines/d4-vs-random.json` * `sim/tools/strategic-ai/baselines/d5-vs-random.json` * `sim/tools/strategic-ai/engines/Engine.ts` * `sim/tools/strategic-ai/engines/HeuristicEngine.ts` * `sim/tools/strategic-ai/engines/LightHeuristicEngine.ts` * `sim/tools/strategic-ai/engines/MctsEngine.ts` * `sim/tools/strategic-ai/engines/OnePlySearchEngine.ts` * `sim/tools/strategic-ai/mechanics/DamageCalc.ts` * `sim/tools/strategic-ai/mechanics/MoveEvaluator.ts` * `sim/tools/strategic-ai/mechanics/SwitchEvaluator.ts` * `sim/tools/strategic-ai/policy/DifficultyPolicy.ts` * `sim/tools/strategic-ai/policy/InfoForgetting.ts` * `sim/tools/strategic-ai/telemetry/DecisionStats.ts` * `test/sim/misc/teams-pack-roundtrip.js` * `test/sim/tools/strategic-ai/decision-quality.js` * `test/sim/tools/strategic-ai/status-move-blocked.js` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
package.json
@ -46,6 +46,7 @@
"build-npm": "node build && npx tsc sim/global-types.ts sim/index.ts --declaration --emitDeclarationOnly --declarationDir dist/ --target es2020 --strict --moduleResolution node --types node --lib es2020",
"test-npm": "npx tsc sim/global-types.ts sim/index.ts --noEmit --target es2020 --strict --moduleResolution node --types node --lib es2020",
"tsc": "tsc",
"ai:baseline": "sh sim/tools/strategic-ai/baselines/check.sh",
coderabbitai[bot] (Migrated from github.com) commented 2026-07-26 16:33:49 +00:00
Copy link

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Build the current sources before running the baseline gate.

check.sh assumes node build already ran, but this npm entrypoint invokes it directly. npm run ai:baseline can therefore fail on a clean checkout or compare baselines against stale dist output after source changes.

Proposed fix 
-    "ai:baseline": "sh sim/tools/strategic-ai/baselines/check.sh",
+    "ai:baseline": "node build && sh sim/tools/strategic-ai/baselines/check.sh",

Update the README to avoid documenting a redundant separate build step.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    "ai:baseline": "node build && sh sim/tools/strategic-ai/baselines/check.sh",
🤖 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 `@package.json` at line 49, Update the ai:baseline npm script to build the
current sources before invoking sim/tools/strategic-ai/baselines/check.sh,
ensuring clean checkouts and source changes use fresh output. Also revise the
README baseline instructions to remove any now-redundant separate build step.
_🎯 Functional Correctness_ | _🟠 Major_ | _⚡ Quick win_ **Build the current sources before running the baseline gate.** `check.sh` assumes `node build` already ran, but this npm entrypoint invokes it directly. `npm run ai:baseline` can therefore fail on a clean checkout or compare baselines against stale `dist` output after source changes. <details> <summary>Proposed fix</summary> ```diff - "ai:baseline": "sh sim/tools/strategic-ai/baselines/check.sh", + "ai:baseline": "node build && sh sim/tools/strategic-ai/baselines/check.sh", ``` Update the README to avoid documenting a redundant separate build step. </details> <!-- suggestion_start --> <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. ```suggestion "ai:baseline": "node build && sh sim/tools/strategic-ai/baselines/check.sh", ``` </details> <!-- suggestion_end --> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 49, Update the ai:baseline npm script to build the current sources before invoking sim/tools/strategic-ai/baselines/check.sh, ensuring clean checkouts and source changes use fresh output. Also revise the README baseline instructions to remove any now-redundant separate build step. ``` </details> <!-- fingerprinting:phantom:poseidon:luna --> <!-- cr-indicator-types:potential_issue --> <!-- cr-comment:v1:cdb22e029dec1c706d54d867 --> <!-- This is an auto-generated comment by CodeRabbit -->
sim/teams.ts
@ -173,0 +178,4 @@
// status - @pokebedrock
buf += `|${set.status || ''}`;
coderabbitai[bot] (Migrated from github.com) commented 2026-07-26 16:33:49 +00:00
Copy link

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

uuid / status are written unescaped into a delimiter-separated stream.

packName strips non-alphanumerics for name/item/ability precisely because | and ] are field/set separators. set.uuid (externally supplied) and set.status are appended raw, so a value containing | or ] shifts every subsequent field of that set — and unpack will then mis-assign item/ability/moves silently. Consider stripping those two characters (or asserting the UUID shape) on pack.

🛡️ Proposed guard 
-			// uuid - `@pokebedrock`
-			buf += `|${set.uuid || ''}`;
+			// uuid - `@pokebedrock`
+			// Strip the packed-format delimiters: a `|` or `]` here would
+			// shift every following field of this set on unpack.
+			buf += `|${(set.uuid || '').replace(/[|\]]/g, '')}`;
 
 			// currentHealth - `@pokebedrock`
 			buf += `|${set.currentHealth ?? ''}`;
 
-			// status - `@pokebedrock`
-			buf += `|${set.status || ''}`;
+			// status - `@pokebedrock`
+			buf += `|${(set.status || '').replace(/[|\]]/g, '')}`;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

			// uuid - `@pokebedrock`
			// Strip the packed-format delimiters: a `|` or `]` here would
			// shift every following field of this set on unpack.
			buf += `|${(set.uuid || '').replace(/[|\]]/g, '')}`;

			// currentHealth - `@pokebedrock`
			buf += `|${set.currentHealth ?? ''}`;

			// status - `@pokebedrock`
			buf += `|${(set.status || '').replace(/[|\]]/g, '')}`;
🤖 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 `@sim/teams.ts` around lines 173 - 181, Sanitize the externally supplied
set.uuid and set.status values before appending them in the packing logic near
packName, removing or rejecting the delimiter characters | and ] so they cannot
alter field boundaries. Preserve empty-value handling and ensure unpack
continues to assign subsequent item, ability, and move fields correctly.
_🗄️ Data Integrity & Integration_ | _🟡 Minor_ | _⚡ Quick win_ **`uuid` / `status` are written unescaped into a delimiter-separated stream.** `packName` strips non-alphanumerics for name/item/ability precisely because `|` and `]` are field/set separators. `set.uuid` (externally supplied) and `set.status` are appended raw, so a value containing `|` or `]` shifts every subsequent field of that set — and `unpack` will then mis-assign item/ability/moves silently. Consider stripping those two characters (or asserting the UUID shape) on pack. <details> <summary>🛡️ Proposed guard</summary> ```diff - // uuid - `@pokebedrock` - buf += `|${set.uuid || ''}`; + // uuid - `@pokebedrock` + // Strip the packed-format delimiters: a `|` or `]` here would + // shift every following field of this set on unpack. + buf += `|${(set.uuid || '').replace(/[|\]]/g, '')}`; // currentHealth - `@pokebedrock` buf += `|${set.currentHealth ?? ''}`; - // status - `@pokebedrock` - buf += `|${set.status || ''}`; + // status - `@pokebedrock` + buf += `|${(set.status || '').replace(/[|\]]/g, '')}`; ``` </details> <!-- suggestion_start --> <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. ```suggestion // uuid - `@pokebedrock` // Strip the packed-format delimiters: a `|` or `]` here would // shift every following field of this set on unpack. buf += `|${(set.uuid || '').replace(/[|\]]/g, '')}`; // currentHealth - `@pokebedrock` buf += `|${set.currentHealth ?? ''}`; // status - `@pokebedrock` buf += `|${(set.status || '').replace(/[|\]]/g, '')}`; ``` </details> <!-- suggestion_end --> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sim/teams.ts` around lines 173 - 181, Sanitize the externally supplied set.uuid and set.status values before appending them in the packing logic near packName, removing or rejecting the delimiter characters | and ] so they cannot alter field boundaries. Preserve empty-value handling and ensure unpack continues to assign subsequent item, ability, and move fields correctly. ``` </details> <!-- fingerprinting:phantom:medusa:komodo --> <!-- cr-indicator-types:potential_issue --> <!-- cr-comment:v1:1900ef497820f95b9c71b5d6 --> <!-- This is an auto-generated comment by CodeRabbit -->
sim/tools/player-ai.ts
coderabbitai[bot] (Migrated from github.com) commented 2026-07-26 16:33:49 +00:00
Copy link

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Make telemetry slot-aware before recording multi-slot choices.

DecisionStats.recordSlot receives each slot index but evaluates tracker.myActive and tracker.foeActive without using it. In doubles/triples, later choices are measured against the wrong attacker and target, corrupting telemetry. Resolve both from the command’s slot/target, or limit collection to singles.

🤖 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 `@sim/tools/player-ai.ts` around lines 368 - 370, Update the telemetry path
around the caller of DecisionStats.record so multi-slot choices resolve the
acting and opposing entities using each command’s slot/target before recording.
Ensure DecisionStats.recordSlot uses the provided slot index when evaluating
tracker.myActive and tracker.foeActive, preserving correct singles behavior and
avoiding telemetry for the wrong attacker or target.
_🗄️ Data Integrity & Integration_ | _🟠 Major_ | _🏗️ Heavy lift_ **Make telemetry slot-aware before recording multi-slot choices.** `DecisionStats.recordSlot` receives each slot index but evaluates `tracker.myActive` and `tracker.foeActive` without using it. In doubles/triples, later choices are measured against the wrong attacker and target, corrupting telemetry. Resolve both from the command’s slot/target, or limit collection to singles. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sim/tools/player-ai.ts` around lines 368 - 370, Update the telemetry path around the caller of DecisionStats.record so multi-slot choices resolve the acting and opposing entities using each command’s slot/target before recording. Ensure DecisionStats.recordSlot uses the provided slot index when evaluating tracker.myActive and tracker.foeActive, preserving correct singles behavior and avoiding telemetry for the wrong attacker or target. ``` </details> <!-- fingerprinting:phantom:poseidon:terra --> <!-- cr-indicator-types:potential_issue --> <!-- cr-comment:v1:5a831bfd624d3b035496881b --> <!-- This is an auto-generated comment by CodeRabbit -->
sim/tools/strategic-ai/engines/HeuristicEngine.ts
coderabbitai[bot] (Migrated from github.com) commented 2026-07-26 16:33:49 +00:00
Copy link

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Propagate the hazy foe state into search models.

Line 314 only replaces evalCtx.defender; evalCtx.tracker remains complete. OnePlySearchEngine and MctsEngine derive foe-move models from that raw tracker, so their configured infoForgetting does not actually hide revealed moves. Pass a per-decision foe view into those models instead.

🤖 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 `@sim/tools/strategic-ai/engines/HeuristicEngine.ts` around lines 309 - 314,
Update the decision flow around trackedFoe, foeMon, and the search-model setup
so evalCtx.tracker uses a per-decision tracker view consistent with the hazy
foeMon, rather than the complete tracker. Ensure OnePlySearchEngine and
MctsEngine derive foe-move models from this same infoForgetting-aware view while
preserving the existing single-resolution consistency for the turn.
_🎯 Functional Correctness_ | _🟠 Major_ | _🏗️ Heavy lift_ **Propagate the hazy foe state into search models.** Line 314 only replaces `evalCtx.defender`; `evalCtx.tracker` remains complete. `OnePlySearchEngine` and `MctsEngine` derive foe-move models from that raw tracker, so their configured `infoForgetting` does not actually hide revealed moves. Pass a per-decision foe view into those models instead. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sim/tools/strategic-ai/engines/HeuristicEngine.ts` around lines 309 - 314, Update the decision flow around trackedFoe, foeMon, and the search-model setup so evalCtx.tracker uses a per-decision tracker view consistent with the hazy foeMon, rather than the complete tracker. Ensure OnePlySearchEngine and MctsEngine derive foe-move models from this same infoForgetting-aware view while preserving the existing single-resolution consistency for the turn. ``` </details> <!-- fingerprinting:phantom:poseidon:terra --> <!-- cr-indicator-types:potential_issue --> <!-- cr-comment:v1:b0fdbf763a96f66fdf2cdbc3 --> <!-- This is an auto-generated comment by CodeRabbit -->
sim/tools/strategic-ai/mechanics/MoveEvaluator.ts
coderabbitai[bot] (Migrated from github.com) commented 2026-07-26 16:33:49 +00:00
Copy link

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Corrosion is not accounted for in the poison-type gate.

STATUS_IMMUNE_TYPES.psn/tox blocks Steel and Poison targets unconditionally, but a Corrosion attacker (Salazzle) legally poisons both. As written, Salazzle's Toxic into a Steel wall scores IMMUNE_MOVE_SCORE and is never clicked.

🐛 Proposed fix 
-	if (STATUS_IMMUNE_TYPES[status]?.some(t => defender.types.includes(t))) return "typeStatus";
+	// Corrosion ignores the Steel/Poison poison immunity entirely.
+	const corrosion = toID(attacker.ability || "") === "corrosion" &&
+		(status === "psn" || status === "tox");
+	if (!corrosion && STATUS_IMMUNE_TYPES[status]?.some(t => defender.types.includes(t))) {
+		return "typeStatus";
+	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

	// Everything past here is specific to inflicting a status.
	const status = move.status;
	if (!status) return null;
	if (defender.status) return "alreadyStatused";
	if (ALL_STATUS_IMMUNE_ABILITIES.has(foeAbility)) return foeAbility;
	if (STATUS_IMMUNE_ABILITIES[status]?.has(foeAbility)) return foeAbility;
	// Leaf Guard is sun-conditional; Flower Veil covers the whole side
	// but we only model the active target.
	const sun = tracker.field.weather === "sunnyday" || tracker.field.weather === "desolateland";
	if (foeAbility === "leafguard" && sun) return "leafguard";
	if (foeAbility === "flowerveil" && defender.types.includes("Grass")) return "flowerveil";
	// Safeguard blocks status infliction (but not stat drops), which is
	// why this check sits below the general-purpose ones.
	if (tracker.sides[ctx.foeSide].safeguardTurns > 0 && !infiltrator) return "safeguard";
	// Corrosion ignores the Steel/Poison poison immunity entirely.
	const corrosion = toID(attacker.ability || "") === "corrosion" &&
		(status === "psn" || status === "tox");
	if (!corrosion && STATUS_IMMUNE_TYPES[status]?.some(t => defender.types.includes(t))) {
		return "typeStatus";
	}
🤖 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 `@sim/tools/strategic-ai/mechanics/MoveEvaluator.ts` around lines 415 - 429,
Update the status type-immunity check in the MoveEvaluator status flow so Steel
and Poison immunity is bypassed when the attacker has the Corrosion ability.
Preserve the existing STATUS_IMMUNE_TYPES behavior for all other abilities and
statuses, while allowing Corrosion users to inflict poison or toxic on those
targets.
_🎯 Functional Correctness_ | _🟡 Minor_ | _⚡ Quick win_ **Corrosion is not accounted for in the poison-type gate.** `STATUS_IMMUNE_TYPES.psn/tox` blocks Steel and Poison targets unconditionally, but a Corrosion attacker (Salazzle) legally poisons both. As written, Salazzle's Toxic into a Steel wall scores `IMMUNE_MOVE_SCORE` and is never clicked. <details> <summary>🐛 Proposed fix</summary> ```diff - if (STATUS_IMMUNE_TYPES[status]?.some(t => defender.types.includes(t))) return "typeStatus"; + // Corrosion ignores the Steel/Poison poison immunity entirely. + const corrosion = toID(attacker.ability || "") === "corrosion" && + (status === "psn" || status === "tox"); + if (!corrosion && STATUS_IMMUNE_TYPES[status]?.some(t => defender.types.includes(t))) { + return "typeStatus"; + } ``` </details> <!-- suggestion_start --> <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. ```suggestion // Everything past here is specific to inflicting a status. const status = move.status; if (!status) return null; if (defender.status) return "alreadyStatused"; if (ALL_STATUS_IMMUNE_ABILITIES.has(foeAbility)) return foeAbility; if (STATUS_IMMUNE_ABILITIES[status]?.has(foeAbility)) return foeAbility; // Leaf Guard is sun-conditional; Flower Veil covers the whole side // but we only model the active target. const sun = tracker.field.weather === "sunnyday" || tracker.field.weather === "desolateland"; if (foeAbility === "leafguard" && sun) return "leafguard"; if (foeAbility === "flowerveil" && defender.types.includes("Grass")) return "flowerveil"; // Safeguard blocks status infliction (but not stat drops), which is // why this check sits below the general-purpose ones. if (tracker.sides[ctx.foeSide].safeguardTurns > 0 && !infiltrator) return "safeguard"; // Corrosion ignores the Steel/Poison poison immunity entirely. const corrosion = toID(attacker.ability || "") === "corrosion" && (status === "psn" || status === "tox"); if (!corrosion && STATUS_IMMUNE_TYPES[status]?.some(t => defender.types.includes(t))) { return "typeStatus"; } ``` </details> <!-- suggestion_end --> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sim/tools/strategic-ai/mechanics/MoveEvaluator.ts` around lines 415 - 429, Update the status type-immunity check in the MoveEvaluator status flow so Steel and Poison immunity is bypassed when the attacker has the Corrosion ability. Preserve the existing STATUS_IMMUNE_TYPES behavior for all other abilities and statuses, while allowing Corrosion users to inflict poison or toxic on those targets. ``` </details> <!-- fingerprinting:phantom:medusa:komodo --> <!-- cr-indicator-types:potential_issue --> <!-- cr-comment:v1:77815e7548f8c8ca6d0ff7c8 --> <!-- This is an auto-generated comment by CodeRabbit -->
sim/tools/strategic-ai/policy/DifficultyPolicy.ts
coderabbitai[bot] (Migrated from github.com) commented 2026-07-26 16:33:49 +00:00
Copy link

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Restore the random engine for difficulty 0.

Line 157 maps 0 to heuristic, contradicting the public PlayerAIOptions contract that difficulty 0 maps to RandomEngine.

Proposed fix 
-	if (difficulty <= 3) return "heuristic";
+	if (difficulty === 0) return "random";
+	if (difficulty <= 3) return "heuristic";
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

function autoFor(difficulty: number): Exclude<EngineName, "auto"> {
	// Tiers 1-3 share the heuristic engine and differ only by knobs; see
	// the module docs for why the random and light engines are no longer
	// on the auto ladder.
	if (difficulty === 0) return "random";
	if (difficulty <= 3) return "heuristic";
🤖 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 `@sim/tools/strategic-ai/policy/DifficultyPolicy.ts` around lines 153 - 157,
Update autoFor so difficulty 0 returns RandomEngine, while preserving the
existing heuristic mapping for difficulties 1–3 and the remaining difficulty
behavior.
_🎯 Functional Correctness_ | _🟠 Major_ | _⚡ Quick win_ **Restore the random engine for difficulty 0.** Line 157 maps `0` to `heuristic`, contradicting the public `PlayerAIOptions` contract that difficulty 0 maps to `RandomEngine`. <details> <summary>Proposed fix</summary> ```diff - if (difficulty <= 3) return "heuristic"; + if (difficulty === 0) return "random"; + if (difficulty <= 3) return "heuristic"; ``` </details> <!-- suggestion_start --> <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. ```suggestion function autoFor(difficulty: number): Exclude<EngineName, "auto"> { // Tiers 1-3 share the heuristic engine and differ only by knobs; see // the module docs for why the random and light engines are no longer // on the auto ladder. if (difficulty === 0) return "random"; if (difficulty <= 3) return "heuristic"; ``` </details> <!-- suggestion_end --> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sim/tools/strategic-ai/policy/DifficultyPolicy.ts` around lines 153 - 157, Update autoFor so difficulty 0 returns RandomEngine, while preserving the existing heuristic mapping for difficulties 1–3 and the remaining difficulty behavior. ``` </details> <!-- fingerprinting:phantom:poseidon:terra --> <!-- cr-indicator-types:potential_issue --> <!-- cr-comment:v1:71020e84f33912889eaa4131 --> <!-- This is an auto-generated comment by CodeRabbit -->
sim/tools/strategic-ai/policy/InfoForgetting.ts
@ -0,0 +85,4 @@
revealedMoves: forgetMoves ?
new Set([...foe.revealedMoves].filter(() => prng.random() >= forgetting)) :
foe.revealedMoves,
item: forgetItem ? "" : foe.item,
coderabbitai[bot] (Migrated from github.com) commented 2026-07-26 16:33:49 +00:00
Copy link

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not forget items that make a move fail.

Line 88 can hide Safety Goggles, but statusMoveBlockedBy relies on the item to reject powder moves. This reintroduces a knowingly ineffective choice; preserve move-blocking items (or stop forgetting items during move evaluation).

🤖 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 `@sim/tools/strategic-ai/policy/InfoForgetting.ts` around lines 76 - 88, Update
the item-forgetting logic in the foe forgetting flow, near forgetItem and the
returned item field, so items that cause status moves to fail—such as Safety
Goggles—are preserved. Ensure statusMoveBlockedBy can still access these
move-blocking items during evaluation, while retaining forgetting behavior for
other items.
_🎯 Functional Correctness_ | _🟠 Major_ | _⚡ Quick win_ **Do not forget items that make a move fail.** Line 88 can hide Safety Goggles, but `statusMoveBlockedBy` relies on the item to reject powder moves. This reintroduces a knowingly ineffective choice; preserve move-blocking items (or stop forgetting items during move evaluation). <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sim/tools/strategic-ai/policy/InfoForgetting.ts` around lines 76 - 88, Update the item-forgetting logic in the foe forgetting flow, near forgetItem and the returned item field, so items that cause status moves to fail—such as Safety Goggles—are preserved. Ensure statusMoveBlockedBy can still access these move-blocking items during evaluation, while retaining forgetting behavior for other items. ``` </details> <!-- fingerprinting:phantom:poseidon:terra --> <!-- cr-indicator-types:potential_issue --> <!-- cr-comment:v1:95c7f930ea2c2d6b897eabe0 --> <!-- This is an auto-generated comment by CodeRabbit -->
sim/tools/strategic-ai/telemetry/DecisionStats.ts
@ -0,0 +183,4 @@
if (calc.avgDamage <= 0) this.counters.zeroDamageMoves++;
else if (Dex.getEffectiveness(move.type, defender.types) < 0) this.counters.resistedMoves++;
}
}
coderabbitai[bot] (Migrated from github.com) commented 2026-07-26 16:33:49 +00:00
Copy link

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Attacker/defender ignore the slot parameter — wrong Pokemon for doubles/triples telemetry.

record()/recordSlot() iterate per-slot (for (let slot = 0; slot < parts.length; slot++)) specifically to support doubles/triples, per the comment at Line 102. But both recordSlot (Lines 166-167) and hadConnectingAlternative (Lines 206-207) pull the attacker from tracker.myActive, which is hardcoded to slot 0:

get myActive(): TrackedPokemon | null {
    return this.getActive(this.mySide, 0);
}

For a slot-1+ command in a multi-slot battle, this evaluates damage/immunity against the wrong attacker (always the slot-0 Pokemon instead of the Pokemon that actually made the choice), silently corrupting immuneMoves/zeroDamageMoves/resistedMoves/avoidableImmuneMoves counts for that slot.

BattleStateTracker.getActive(side, position) is already slot-aware and available for this.

🐛 Proposed fix for the attacker lookup 
-		const attacker = tracker.myActive;
-		const defender = tracker.foeActive;
+		const attacker = tracker.getActive(tracker.mySide, slot);
+		const defender = tracker.foeActive;
 		if (!attacker || !defender) return;

-	const attacker = tracker.myActive;
-	const defender = tracker.foeActive;
+	const attacker = tracker.getActive(tracker.mySide, slot);
+	const defender = tracker.foeActive;
 	if (!active?.moves || !attacker || !defender) return false;

(The foe-side defender is harder to resolve precisely without target data in doubles, but the attacker fix alone removes the primary misattribution.)

Based on learnings, active-slot indexing (`myPokemon[i]`/slot-index lookups) must match the actual active-slot position rather than a fixed index, the same class of mismatch flagged previously in `RandomEngine.ts`.

Also applies to: 199-229

🤖 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 `@sim/tools/strategic-ai/telemetry/DecisionStats.ts` around lines 160 - 186,
Update the attacker lookup in both recordSlot and hadConnectingAlternative to
use BattleStateTracker.getActive with the current slot and the tracked mySide,
instead of tracker.myActive. Preserve the existing per-slot iteration and
calculations so doubles/triples telemetry attributes damage, immunity, and
alternative-choice counters to the Pokémon that made each slot’s choice.

Source: Learnings

_🎯 Functional Correctness_ | _🟠 Major_ | _⚡ Quick win_ **Attacker/defender ignore the `slot` parameter — wrong Pokemon for doubles/triples telemetry.** `record()`/`recordSlot()` iterate per-slot (`for (let slot = 0; slot < parts.length; slot++)`) specifically to support doubles/triples, per the comment at Line 102. But both `recordSlot` (Lines 166-167) and `hadConnectingAlternative` (Lines 206-207) pull the attacker from `tracker.myActive`, which is hardcoded to slot 0: ```ts get myActive(): TrackedPokemon | null { return this.getActive(this.mySide, 0); } ``` For a slot-1+ command in a multi-slot battle, this evaluates damage/immunity against the wrong attacker (always the slot-0 Pokemon instead of the Pokemon that actually made the choice), silently corrupting `immuneMoves`/`zeroDamageMoves`/`resistedMoves`/`avoidableImmuneMoves` counts for that slot. `BattleStateTracker.getActive(side, position)` is already slot-aware and available for this. <details> <summary>🐛 Proposed fix for the attacker lookup</summary> ```diff - const attacker = tracker.myActive; - const defender = tracker.foeActive; + const attacker = tracker.getActive(tracker.mySide, slot); + const defender = tracker.foeActive; if (!attacker || !defender) return; ``` ```diff - const attacker = tracker.myActive; - const defender = tracker.foeActive; + const attacker = tracker.getActive(tracker.mySide, slot); + const defender = tracker.foeActive; if (!active?.moves || !attacker || !defender) return false; ``` (The foe-side defender is harder to resolve precisely without target data in doubles, but the attacker fix alone removes the primary misattribution.) </details> Based on learnings, active-slot indexing (`myPokemon[i]`/slot-index lookups) must match the actual active-slot position rather than a fixed index, the same class of mismatch flagged previously in `RandomEngine.ts`. Also applies to: 199-229 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sim/tools/strategic-ai/telemetry/DecisionStats.ts` around lines 160 - 186, Update the attacker lookup in both recordSlot and hadConnectingAlternative to use BattleStateTracker.getActive with the current slot and the tracked mySide, instead of tracker.myActive. Preserve the existing per-slot iteration and calculations so doubles/triples telemetry attributes damage, immunity, and alternative-choice counters to the Pokémon that made each slot’s choice. ``` </details> <!-- fingerprinting:phantom:medusa:beignet --> <!-- cr-indicator-types:potential_issue --> <!-- cr-comment:v1:ff966a11987086372cb49683 --> _Source: Learnings_ <!-- This is an auto-generated comment by CodeRabbit -->
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug
Something isn't working

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
question
Further information is requested

  
wontfix
This will not be worked on

No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
pokebedrock/pokebedrock-showdown!13
No description provided.