pokebedrock/pokebedrock-showdown

Fork 0

Feature/evolutions #2

Closed

evc24004 wants to merge 22 commits from feature/evolutions into master

evc24004 commented

2025-08-06 23:39:59 +00:00

(Migrated from github.com)

Summary by CodeRabbit

New Features
- Updated battle stat calculation logic for certain formats, improving accuracy and consistency.
- Enhanced evolution methods for several Pokémon, including new requirements such as party slot, nickname, and weather conditions.
Bug Fixes
- Corrected evolution triggers and conditions for multiple Pokémon species.
- Updated tier assignment for Archaludon.
Refactor
- Replaced manual SQL queries with a structured query builder for improved database safety and maintainability.
- Streamlined and unified stat calculation methods across the battle engine.
Chores
- Updated and simplified documentation, including replacing the README with the official Pokémon Showdown version.
- Removed obsolete or deprecated Pokémon entries and properties.
- Improved privacy handling and command structure for team management.
Documentation
- Revised help ticket messaging for password resets to reflect current procedures.

## Summary by CodeRabbit * **New Features** * Updated battle stat calculation logic for certain formats, improving accuracy and consistency. * Enhanced evolution methods for several Pokémon, including new requirements such as party slot, nickname, and weather conditions. * **Bug Fixes** * Corrected evolution triggers and conditions for multiple Pokémon species. * Updated tier assignment for Archaludon. * **Refactor** * Replaced manual SQL queries with a structured query builder for improved database safety and maintainability. * Streamlined and unified stat calculation methods across the battle engine. * **Chores** * Updated and simplified documentation, including replacing the README with the official Pokémon Showdown version. * Removed obsolete or deprecated Pokémon entries and properties. * Improved privacy handling and command structure for team management. * **Documentation** * Revised help ticket messaging for password resets to reflect current procedures.

coderabbitai[bot] commented

2025-08-06 23:40:05 +00:00

(Migrated from github.com)

Walkthrough

This set of changes refactors stat calculation logic across multiple modules, updates Pokémon evolution data, restructures and renames rulesets for LGPE formats, and overhauls database handling to improve SQL safety and abstraction. The Postgres database module is removed and replaced with a new abstraction, requiring updates to all dependent modules and tests. Documentation, help pages, and error handling are also updated, alongside numerous adjustments to public interfaces and method signatures.

Changes

Cohort / File(s)	Change Summary
Documentation and Help Content `README.md`, `server/chat-plugins/helptickets.ts`	README replaced with the official Pokémon Showdown documentation, removing Minecraft-specific details. Help ticket password reset instructions updated to reflect process closure.
LGPE Ruleset Refactor `data/mods/gen7letsgo/rulesets.ts`, `data/rulesets.ts`, `sim/dex-formats.ts`, `sim/team-validator.ts`	LGPE ruleset renamed from "Allow AVs" to "LGPE Normal Rules", with corresponding updates to ruleset keys, descriptions, and references in format logic. Stat validation logic for AVs updated to use new rule.
Stat Calculation Refactor `sim/battle.ts`, `sim/global-types.ts`, `sim/pokemon.ts`, `data/mods/gen7letsgo/scripts.ts`, `config/formats.ts`	Stat calculation unified to per-stat `statModify` methods, replacing batch `spreadModify` and `natureModify`. Interfaces and implementations updated accordingly in core simulation and Gen 7 LGPE logic. Format definitions updated to use new stat calculation hooks.
Pokédex Evolution Data Updates `data/pokedex.ts`, `sim/dex-species.ts`	Evolution methods, triggers, and requirements clarified and expanded for several Pokémon. Evolution-related properties in the Species class refined, with obsolete properties removed and new ones added. Entries for certain Pokémon removed.
Format and Tier Adjustments `data/formats-data.ts`, `config/formats.ts`	Tier for Archaludon updated. Specific format rules and spotlight messages adjusted for Gen 9 and Gen 7 Let's Go.
Database Abstraction Overhaul `lib/database.ts`, `lib/postgres.ts`, `lib/index.ts`, `server/chat-plugins/teams.ts`, `test/lib/postgres.js`	The custom Postgres library is removed and replaced with a new database abstraction (`PGDatabase`). All SQL queries in teams handling and tests refactored to use tagged template literals and structured methods. Exports updated. Integration tests for Postgres are replaced with unit tests for the new SQL template system.
Error Handling and Logging `server/chat-plugins/seasons.ts`	Error logging in ladder top retrieval simplified to silently return null on error.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant TeamsHandler
    participant PGDatabase
    participant teamsTable

    User->>TeamsHandler: save({name, packedTeam, format, privacy})
    TeamsHandler->>teamsTable: selectOne(SQL`WHERE name = ...`)
    alt Team exists
        TeamsHandler->>teamsTable: updateOne(SQL`SET ... WHERE teamid = ...`)
    else Team does not exist
        TeamsHandler->>teamsTable: insert(SQL`...`)
    end
    TeamsHandler-->>User: {teamid, teamName, privacy}

sequenceDiagram
    participant Battle
    participant Pokemon

    Battle->>Pokemon: updateMaxHp()
    Pokemon->>Battle: statModify(baseStats, set, 'hp')
    Battle-->>Pokemon: new HP value
    Pokemon-->>Battle: HP updated

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

In burrows deep, the rabbits code,
Stat math refactored, evolution's road.
Postgres hops out, new DB hops in,
LGPE rules get a shiny new skin.
With queries safe and logic neat,
This patch is quite the coder's treat!
🐇✨

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.

✨ Finishing Touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feature/evolutions

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

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai generate unit tests to generate unit tests for this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

## Walkthrough This set of changes refactors stat calculation logic across multiple modules, updates Pokémon evolution data, restructures and renames rulesets for LGPE formats, and overhauls database handling to improve SQL safety and abstraction. The Postgres database module is removed and replaced with a new abstraction, requiring updates to all dependent modules and tests. Documentation, help pages, and error handling are also updated, alongside numerous adjustments to public interfaces and method signatures. ## Changes | Cohort / File(s) | Change Summary | |----------------------------------------------------|---------------| | **Documentation and Help Content** `README.md`, `server/chat-plugins/helptickets.ts` | README replaced with the official Pokémon Showdown documentation, removing Minecraft-specific details. Help ticket password reset instructions updated to reflect process closure. | | **LGPE Ruleset Refactor** `data/mods/gen7letsgo/rulesets.ts`, `data/rulesets.ts`, `sim/dex-formats.ts`, `sim/team-validator.ts` | LGPE ruleset renamed from "Allow AVs" to "LGPE Normal Rules", with corresponding updates to ruleset keys, descriptions, and references in format logic. Stat validation logic for AVs updated to use new rule. | | **Stat Calculation Refactor** `sim/battle.ts`, `sim/global-types.ts`, `sim/pokemon.ts`, `data/mods/gen7letsgo/scripts.ts`, `config/formats.ts` | Stat calculation unified to per-stat `statModify` methods, replacing batch `spreadModify` and `natureModify`. Interfaces and implementations updated accordingly in core simulation and Gen 7 LGPE logic. Format definitions updated to use new stat calculation hooks. | | **Pokédex Evolution Data Updates** `data/pokedex.ts`, `sim/dex-species.ts` | Evolution methods, triggers, and requirements clarified and expanded for several Pokémon. Evolution-related properties in the Species class refined, with obsolete properties removed and new ones added. Entries for certain Pokémon removed. | | **Format and Tier Adjustments** `data/formats-data.ts`, `config/formats.ts` | Tier for Archaludon updated. Specific format rules and spotlight messages adjusted for Gen 9 and Gen 7 Let's Go. | | **Database Abstraction Overhaul** `lib/database.ts`, `lib/postgres.ts`, `lib/index.ts`, `server/chat-plugins/teams.ts`, `test/lib/postgres.js` | The custom Postgres library is removed and replaced with a new database abstraction (`PGDatabase`). All SQL queries in teams handling and tests refactored to use tagged template literals and structured methods. Exports updated. Integration tests for Postgres are replaced with unit tests for the new SQL template system. | | **Error Handling and Logging** `server/chat-plugins/seasons.ts` | Error logging in ladder top retrieval simplified to silently return null on error. | ## Sequence Diagram(s) ```mermaid sequenceDiagram participant User participant TeamsHandler participant PGDatabase participant teamsTable User->>TeamsHandler: save({name, packedTeam, format, privacy}) TeamsHandler->>teamsTable: selectOne(SQL`WHERE name = ...`) alt Team exists TeamsHandler->>teamsTable: updateOne(SQL`SET ... WHERE teamid = ...`) else Team does not exist TeamsHandler->>teamsTable: insert(SQL`...`) end TeamsHandler-->>User: {teamid, teamName, privacy} ``` ```mermaid sequenceDiagram participant Battle participant Pokemon Battle->>Pokemon: updateMaxHp() Pokemon->>Battle: statModify(baseStats, set, 'hp') Battle-->>Pokemon: new HP value Pokemon-->>Battle: HP updated ``` ## Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes ## Poem > In burrows deep, the rabbits code, > Stat math refactored, evolution's road. > Postgres hops out, new DB hops in, > LGPE rules get a shiny new skin. > With queries safe and logic neat, > This patch is quite the coder's treat! > 🐇✨   > [!NOTE] > <details open="true"> > <summary>⚡️ Unit Test Generation is now available in beta!</summary> > > Learn more [here](https://docs.coderabbit.ai/finishing-touches/unit-test-generation), or try it out under "Finishing Touches" below. > > </details>      <details> <summary>✨ Finishing Touches</summary> - [ ]  📝 Generate Docstrings <details> <summary>🧪 Generate unit tests</summary> - [ ]  Create PR with unit tests - [ ]  Post copyable unit tests in a comment - [ ]  Commit unit tests in branch `feature/evolutions` </details> </details>   --- 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. <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> <details> <summary>🪧 Tips</summary> ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai?utm_source=oss&utm_medium=github&utm_campaign=smell-of-curry/pokebedrock-showdown&utm_content=2): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit <commit_id>, please review it.` - `Explain this complex logic.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai explain this code block.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.` - `@coderabbitai read src/utils.ts and explain its main purpose.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` ### Support Need help? Create a ticket on our [support page](https://www.coderabbit.ai/contact-us/support) for assistance with any issues or questions. ### CodeRabbit Commands (Invoked using PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai generate docstrings` to [generate docstrings](https://docs.coderabbit.ai/finishing-touches/docstrings) for this PR. - `@coderabbitai generate sequence diagram` to generate a sequence diagram of the changes in this PR. - `@coderabbitai generate unit tests` to generate unit tests for this PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. ### Other keywords and placeholders - Add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. - Add `@coderabbitai summary` to generate the high-level summary at a specific location in the PR description. - Add `@coderabbitai` anywhere in the PR title to generate the title automatically. ### CodeRabbit Configuration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://docs.coderabbit.ai) for detailed information on how to use CodeRabbit. - Join our [Discord Community](http://discord.gg/coderabbit) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements. </details>

coderabbitai[bot] (Migrated from github.com) reviewed

2025-08-06 23:45:16 +00:00

coderabbitai[bot] (Migrated from github.com) left a comment

Actionable comments posted: 6

🔭 Outside diff range comments (2)

server/chat-plugins/seasons.ts (1)
145-152: Swallowing all exceptions removes observability – keep lightweight logging.

The new catch block now ignores every network/parse error and silently returns null. This makes production issues (API outage, malformed JSON, etc.) invisible and very hard to debug.

Restore minimal reporting (behind a debug gate if noise is a concern):
-	} catch {
-		return null;
+	} catch (e: any) {
+		if (Config.debug) Monitor.warn(`getLadderTop(${format}) failed: ${e?.stack || e}`);
+		return null;
 	}
And extend the import list accordingly:
-import { FS, Net, Utils } from '../../lib';
+import { FS, Net, Utils, Monitor } from '../../lib';
This preserves silent fallback behaviour for callers while still surfacing actionable signals for operators.
README.md (1)
14-37: Unresolved Git conflict markers break the README rendering

The section still contains <<<<<<<, =======, and >>>>>>> markers. These will render literally on GitHub and signal an unfinished merge, confusing readers and breaking the Markdown outline.
-<<<<<<< Updated upstream
-Introduction
-------------------------------------------------------------------------
-=======
-### Evolution System Enhancements:
-...
- - `changeEvent`: Event that triggers forme changes
->>>>>>> Stashed changes
+
+## Introduction
+
+

🧹 Nitpick comments (6)

server/chat-plugins/helptickets.ts (1)
1688-1688: Wrap user-facing string in this.tr for i18n consistency

Most help-request UI strings in this file are wrapped with this.tr\`` to enable downstream localisation.
The newly added password-reset notice is plain text, so it will always appear in English even when the rest of the page is translated.

Consider:
-buf += `The password reset process is no longer open to the public.`;
+buf += `${this.tr`The password reset process is no longer open to the public.`}`;
Minor, but keeps the i18n story uniform.
README.md (1)

18-32: Markdown style nitpicks (optional)

Heading on Line 18 is ATX style while the file primarily uses Setext for top-level headings. Also, the trailing : violates MD026. Consider re-styling or removing the colon to silence markdown-lint noise.

test/lib/postgres.js (1)

33-60: Integration tests need updating for the new database API.

The skipped integration tests should be updated to:

Use the new SQL template tag instead of raw SQL strings

Properly configure the PGDatabase instance with connection details

Verify the new API works correctly with an actual database

Would you like me to help update these integration tests to work with the new PGDatabase and SQL API?

sim/battle.ts (1)

3084-3103: Review the useBagItem function implementation.

The function handles item usage correctly but has a few considerations:

The commented-out fainted check might be intentional for revival items

The health comparison logic properly detects healing

Proper cleanup with the ability End event

Consider adding parameter validation and documenting the intended behavior for fainted Pokémon.
sim/dex-species.ts (1)
248-258: Well-designed evolution property additions with minor typo.

The new evolution properties provide comprehensive coverage for complex evolution conditions. The types are appropriately restrictive and the documentation is clear.

Fix the typo in the minDamage comment:
-	/** The minimum amount of damage required for this evolution(in a battle, wihtout fainting) */
+	/** The minimum amount of damage required for this evolution (in a battle, without fainting) */
server/chat-plugins/teams.ts (1)
252-254: Consider avoiding mutation of input parameter.

Mutating the team parameter directly could lead to unexpected side effects for the caller. Consider working with a copy or returning the modified values.
-// eslint-disable-next-line require-atomic-updates
-team.packedTeam = Teams.pack(sets);
+const packedTeam = Teams.pack(sets);
Then use packedTeam in subsequent operations and return it as part of the result.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d3ecadea5 and 0bba0d69a6.

📒 Files selected for processing (20)

README.md (2 hunks)
config/formats.ts (3 hunks)
data/formats-data.ts (1 hunks)
data/mods/gen7letsgo/rulesets.ts (1 hunks)
data/mods/gen7letsgo/scripts.ts (1 hunks)
data/pokedex.ts (8 hunks)
data/rulesets.ts (1 hunks)
lib/database.ts (6 hunks)
lib/index.ts (1 hunks)
lib/postgres.ts (0 hunks)
server/chat-plugins/helptickets.ts (1 hunks)
server/chat-plugins/seasons.ts (1 hunks)
server/chat-plugins/teams.ts (14 hunks)
sim/battle.ts (1 hunks)
sim/dex-formats.ts (1 hunks)
sim/dex-species.ts (6 hunks)
sim/global-types.ts (1 hunks)
sim/pokemon.ts (2 hunks)
sim/team-validator.ts (1 hunks)
test/lib/postgres.js (2 hunks)

💤 Files with no reviewable changes (1)

lib/postgres.ts

🧰 Additional context used

🧬 Code Graph Analysis (3)

test/lib/postgres.js (1)

lib/database.ts (2)

PGDatabase (384-415)

SQL (144-147)

sim/global-types.ts (3)

sim/battle.ts (1)

Battle (110-3442)

sim/dex.ts (2)

StatsTable (948-948)

StatID (946-946)

sim/teams.ts (1)

PokemonSet (33-151)

sim/battle.ts (2)

sim/dex.ts (2)

StatsTable (948-948)

StatID (946-946)

sim/teams.ts (1)

PokemonSet (33-151)

🪛 markdownlint-cli2 (0.17.2)

README.md

18-18: Heading style
Expected: setext; Actual: atx

(MD003, heading-style)

18-18: Trailing punctuation in heading
Punctuation: ':'

(MD026, no-trailing-punctuation)

43-43: Bare URL used

(MD034, no-bare-urls)

67-67: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)

68-68: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)

69-69: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)

70-70: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)

71-71: Unordered list style
Expected: dash; Actual: asterisk

(MD004, ul-style)

121-121: Bare URL used

(MD034, no-bare-urls)

🪛 LanguageTool

README.md

[uncategorized] ~71-~71: Do not mix variants of the same word (‘Pokemon’ and ‘Pokémon’) within a single text.
Context: ...FAQ compiled by Kaiepi regarding making Pokemon Showdown bots - mainly chatbots and bat...

(EN_WORD_COHERENCY)

🔇 Additional comments (27)

data/formats-data.ts (1)

5614-5618: Tier mismatch: archaludon is Uber but natDexTier is RUBL

tier: "Uber" (Line 5615) implies the mon is banned from standard OU, yet natDexTier: "RUBL" (Line 5617) says it’s only banned from RU in NatDex. That’s an unusually large gap and may be a copy-paste error.

Would you double-check Smogon’s current NatDex tiering for Archaludon and update accordingly?

data/mods/gen7letsgo/rulesets.ts (1)

4-9: Confirm new ruleset identifier exists

'LGPE Normal Rules' replaces the old 'Adjust Level = 50'. Ensure:

A format entry named exactly lgpenormalrules is defined in data/rulesets.ts.

Down-stream look-ups (TeamValidator, Battle scripts) use the same canonical id.

A quick grep after rebasing will catch case/spacing mismatches.

sim/team-validator.ts (1)

1108-1108: LGTM! Correct implementation of rule logic inversion.

The change correctly updates the allowAVs logic to reflect the renaming and semantic inversion of the ruleset from 'allowavs' to 'lgpenormalrules'. The new logic properly disables AVs when LGPE normal rules are active (which zero out AVs) and allows them otherwise.

sim/dex-formats.ts (1)

280-280: LGTM! Clean rule renaming from 'allowavs' to 'lgpenormalrules'.

The rule identifier has been updated consistently with the broader refactoring mentioned in the AI summary. The logic remains identical - checking for the rule's presence to set evLimit appropriately for gen7letsgo formats.

lib/index.ts (1)

3-5: Verified re-enabled exports—no issues found

All three exports (crashlogger, ProcessManager, SQL) are present under lib/ and are imported throughout the codebase.
The implementation in lib/sql.ts provides the full SQLDatabaseManager, Statement, DatabaseTable, and SQL template abstractions as expected, and all dependent imports have been updated to use these exports correctly.

data/rulesets.ts (1)

1725-1731: LGTM! Excellent refactoring of the LGPE ruleset.

The rename from allowavs to lgpenormalrules significantly improves clarity:

Better naming: The new ID clearly indicates this is for Let's Go Pokémon Eevee/Pikachu normal rules

Accurate description: The updated description correctly reflects that this rule sets level to 50 and all AVs to 0, rather than "allowing" them

Proper implementation: The addition of ruleset: ['Adjust Level = 50'] provides the actual mechanism for enforcing level 50

Consistent behavior: The rule now clearly communicates that it standardizes LGPE format conditions

This change makes the codebase more maintainable and the rule's purpose much clearer to developers and users.

test/lib/postgres.js (1)

10-31: Comprehensive test coverage for SQL template strings.

The test suite provides excellent coverage of the SQL template functionality, including edge cases like undefined values and empty arrays. Good job testing both the declarative SQL template literals and the imperative appendRaw/append methods.

sim/global-types.ts (1)

370-370: Interface updated for per-stat calculation approach.

The change from batch stat processing (spreadModify/natureModify) to per-stat calculation (statModify) provides more granular control and cleaner separation of concerns. This aligns well with the broader refactoring across the codebase.

sim/pokemon.ts (2)

345-345: Happiness override for Let's Go mechanics.

Setting happiness to 70 for the gen7letsgo mod is consistent with the stat calculation in data/mods/gen7letsgo/scripts.ts. This ensures uniform happiness-based stat modifiers in Let's Go formats.

1514-1520: Improved HP calculation with proper edge case handling.

The refactored updateMaxHp method correctly:

Uses the centralized statModify method for consistency

Properly handles the edge case where HP could be 0 or negative by ensuring it's at least 1

Triggers a silent heal event when HP changes

data/mods/gen7letsgo/scripts.ts (1)

65-82: Well-implemented per-stat calculation for Let's Go mechanics.

The refactored statModify method correctly:

Applies appropriate stat formulas for HP vs other stats

Integrates nature modifiers with proper 16-bit truncation

Applies friendship-based stat modifiers specific to Let's Go

Uses consistent truncation throughout the calculations

The consolidation of stat, nature, and friendship calculations into a single method improves maintainability.

config/formats.ts (2)

851-853: Promotional message addition looks good

The onBegin() method correctly adds a promotional message for the Ladder Achievements Project. The HTML structure is proper and the message provides clear call-to-action with appropriate links.

3423-3423: Removal of “Allow AVs” verified against “lgpenormalrules”
No references to the legacy Allow AVs or allowavs remain, and the code correctly gates EVs and AVs off via the new lgpenormalrules entry:

sim/team-validator.ts uses
const allowAVs = !ruleTable.has('lgpenormalrules');

sim/dex-formats.ts applies
this.evLimit = this.has('lgpenormalrules') ? 0 : null;

data/rulesets.ts defines the lgpenormalrules ruleset

This change is intentional and preserves existing functionality.

sim/battle.ts (3)

2331-2337: LGTM! Clean refactoring that improves modularity.

The delegation to statModify for each stat calculation follows the single responsibility principle and makes the code more maintainable. The logic remains equivalent while improving code organization.

2339-2357: LGTM! Well-implemented stat calculation consolidation.

The new statModify method correctly consolidates stat calculation logic with proper handling of:

Different formulas for HP vs other stats

Nature modifications with 16-bit truncation

Overflow stat mod rule edge cases for specific Pokémon

The mathematical formulas are accurate and the edge case handling is appropriate. The inline comments provide helpful context for the specialized logic.

2535-2535: @pokebedrock addition looks good.

The killed message addition provides useful logging information without affecting battle mechanics. The implementation correctly passes the source and effect parameters.

sim/dex-species.ts (4)

168-168: Evolution type consolidation looks good.

The simplification of the evoType union type to seven core evolution methods aligns with the broader refactoring mentioned in the summary. This should improve type safety and consistency.

181-187: Excellent improvement to party species evolution requirements.

The rename from evoPartySpecies to partySpecies removes redundancy, and changing from string to string array provides better flexibility for complex evolution requirements. The JSDoc examples are particularly helpful and reflect actual Pokémon game mechanics.

657-657: Good migration to native deep equality checking.

Replacing the custom Utils.isDeepStrictEqual with Node.js native isDeepStrictEqual reduces dependencies on custom utilities and leverages well-tested standard library functionality. This should provide better performance and reliability for the species caching optimization.

3-4: Approve both Node.js and MoonPhase imports

All import changes are valid:

The isDeepStrictEqual import replaces custom deep‐equality logic appropriately.

The @minecraft/server dependency is declared in package.json ("^2.1.0-beta.1.21.90-stable") and the MoonPhase type is used for evoMoonPhase in:

sim/dex-species.ts

data/pokedex.ts

No further action required.

data/pokedex.ts (7)

2381-2385: LGTM! New party slot evolution requirement implemented correctly.

The addition of partySlotRequirement with slot and species properties provides a clear structure for the complex evolution requirement. The evoCondition text also clearly describes the requirement for users.

5639-5639: LGTM! Evolution item requirement added correctly.

Adding Thunder Stone as the evolution item for Magneton provides a clear evolution path.

8501-8503: Evolution method change from level-based to item-based.

The change from level-based evolution to Thunder Stone usage represents a significant mechanic change for Probopass. Ensure this aligns with the intended game mechanics and that any dependent code handles the new evolution type correctly.

12333-12333: LGTM! Nickname requirement added for Malamar evolution.

The addition of the "Dinnerbone" nickname requirement is a clever reference to Minecraft's upside-down easter egg, fitting well with the existing upside-down console evolution condition.

12606-12606: LGTM! Weather property added for consistency.

Adding evoWeather: "rain" provides a machine-readable complement to the existing rain condition text, improving data structure consistency.

12625-12625: LGTM! Consistent weather property added.

Adding evoWeather: "rain" maintains consistency with the regular Goodra form and provides machine-readable weather conditions.

15965-15965: LGTM! Evolution condition clarified with specific requirements.

The updated condition text provides clearer, more specific requirements (49+ damage threshold and desert biome), improving user understanding and implementation clarity.

**Actionable comments posted: 6** <details> <summary>🔭 Outside diff range comments (2)</summary><blockquote> <details> <summary>server/chat-plugins/seasons.ts (1)</summary><blockquote> `145-152`: **Swallowing all exceptions removes observability – keep lightweight logging.** The new `catch` block now ignores every network/parse error and silently returns `null`. This makes production issues (API outage, malformed JSON, etc.) invisible and very hard to debug. Restore minimal reporting (behind a debug gate if noise is a concern): ```diff - } catch { - return null; + } catch (e: any) { + if (Config.debug) Monitor.warn(`getLadderTop(${format}) failed: ${e?.stack || e}`); + return null; } ``` And extend the import list accordingly: ```diff -import { FS, Net, Utils } from '../../lib'; +import { FS, Net, Utils, Monitor } from '../../lib'; ``` This preserves silent fallback behaviour for callers while still surfacing actionable signals for operators. </blockquote></details> <details> <summary>README.md (1)</summary><blockquote> `14-37`: **Unresolved Git conflict markers break the README rendering** The section still contains `<<<<<<<`, `=======`, and `>>>>>>>` markers. These will render literally on GitHub and signal an unfinished merge, confusing readers and breaking the Markdown outline. ```diff -<<<<<<< Updated upstream -Introduction ------------------------------------------------------------------------- -======= -### Evolution System Enhancements: -... - - `changeEvent`: Event that triggers forme changes ->>>>>>> Stashed changes + +## Introduction + + ``` </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (6)</summary><blockquote> <details> <summary>server/chat-plugins/helptickets.ts (1)</summary><blockquote> `1688-1688`: **Wrap user-facing string in `this.tr` for i18n consistency** Most help-request UI strings in this file are wrapped with `this.tr\`\`` to enable downstream localisation. The newly added password-reset notice is plain text, so it will always appear in English even when the rest of the page is translated. Consider: ```diff -buf += `The password reset process is no longer open to the public.`; +buf += `${this.tr`The password reset process is no longer open to the public.`}`; ``` Minor, but keeps the i18n story uniform. </blockquote></details> <details> <summary>README.md (1)</summary><blockquote> `18-32`: **Markdown style nitpicks (optional)** Heading on Line 18 is ATX style while the file primarily uses Setext for top-level headings. Also, the trailing `:` violates MD026. Consider re-styling or removing the colon to silence markdown-lint noise. </blockquote></details> <details> <summary>test/lib/postgres.js (1)</summary><blockquote> `33-60`: **Integration tests need updating for the new database API.** The skipped integration tests should be updated to: 1. Use the new `SQL` template tag instead of raw SQL strings 2. Properly configure the `PGDatabase` instance with connection details 3. Verify the new API works correctly with an actual database Would you like me to help update these integration tests to work with the new `PGDatabase` and `SQL` API? </blockquote></details> <details> <summary>sim/battle.ts (1)</summary><blockquote> `3084-3103`: **Review the useBagItem function implementation.** The function handles item usage correctly but has a few considerations: - The commented-out fainted check might be intentional for revival items - The health comparison logic properly detects healing - Proper cleanup with the ability End event Consider adding parameter validation and documenting the intended behavior for fainted Pokémon. </blockquote></details> <details> <summary>sim/dex-species.ts (1)</summary><blockquote> `248-258`: **Well-designed evolution property additions with minor typo.** The new evolution properties provide comprehensive coverage for complex evolution conditions. The types are appropriately restrictive and the documentation is clear. Fix the typo in the `minDamage` comment: ```diff - /** The minimum amount of damage required for this evolution(in a battle, wihtout fainting) */ + /** The minimum amount of damage required for this evolution (in a battle, without fainting) */ ``` </blockquote></details> <details> <summary>server/chat-plugins/teams.ts (1)</summary><blockquote> `252-254`: **Consider avoiding mutation of input parameter.** Mutating the `team` parameter directly could lead to unexpected side effects for the caller. Consider working with a copy or returning the modified values. ```diff -// eslint-disable-next-line require-atomic-updates -team.packedTeam = Teams.pack(sets); +const packedTeam = Teams.pack(sets); ``` Then use `packedTeam` in subsequent operations and return it as part of the result. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 4d3ecadea59a8e6054be050a3b266ba12ebeaf71 and 0bba0d69a60b55aff05be7759c63f69a4031e25f. </details> <details> <summary>📒 Files selected for processing (20)</summary> * `README.md` (2 hunks) * `config/formats.ts` (3 hunks) * `data/formats-data.ts` (1 hunks) * `data/mods/gen7letsgo/rulesets.ts` (1 hunks) * `data/mods/gen7letsgo/scripts.ts` (1 hunks) * `data/pokedex.ts` (8 hunks) * `data/rulesets.ts` (1 hunks) * `lib/database.ts` (6 hunks) * `lib/index.ts` (1 hunks) * `lib/postgres.ts` (0 hunks) * `server/chat-plugins/helptickets.ts` (1 hunks) * `server/chat-plugins/seasons.ts` (1 hunks) * `server/chat-plugins/teams.ts` (14 hunks) * `sim/battle.ts` (1 hunks) * `sim/dex-formats.ts` (1 hunks) * `sim/dex-species.ts` (6 hunks) * `sim/global-types.ts` (1 hunks) * `sim/pokemon.ts` (2 hunks) * `sim/team-validator.ts` (1 hunks) * `test/lib/postgres.js` (2 hunks) </details> <details> <summary>💤 Files with no reviewable changes (1)</summary> * lib/postgres.ts </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧬 Code Graph Analysis (3)</summary> <details> <summary>test/lib/postgres.js (1)</summary><blockquote> <details> <summary>lib/database.ts (2)</summary> * `PGDatabase` (384-415) * `SQL` (144-147) </details> </blockquote></details> <details> <summary>sim/global-types.ts (3)</summary><blockquote> <details> <summary>sim/battle.ts (1)</summary> * `Battle` (110-3442) </details> <details> <summary>sim/dex.ts (2)</summary> * `StatsTable` (948-948) * `StatID` (946-946) </details> <details> <summary>sim/teams.ts (1)</summary> * `PokemonSet` (33-151) </details> </blockquote></details> <details> <summary>sim/battle.ts (2)</summary><blockquote> <details> <summary>sim/dex.ts (2)</summary> * `StatsTable` (948-948) * `StatID` (946-946) </details> <details> <summary>sim/teams.ts (1)</summary> * `PokemonSet` (33-151) </details> </blockquote></details> </details><details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>README.md</summary> 18-18: Heading style Expected: setext; Actual: atx (MD003, heading-style) --- 18-18: Trailing punctuation in heading Punctuation: ':' (MD026, no-trailing-punctuation) --- 43-43: Bare URL used (MD034, no-bare-urls) --- 67-67: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 68-68: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 69-69: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 70-70: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 71-71: Unordered list style Expected: dash; Actual: asterisk (MD004, ul-style) --- 121-121: Bare URL used (MD034, no-bare-urls) </details> </details> <details> <summary>🪛 LanguageTool</summary> <details> <summary>README.md</summary> [uncategorized] ~71-~71: Do not mix variants of the same word (‘Pokemon’ and ‘Pokémon’) within a single text. Context: ...FAQ compiled by Kaiepi regarding making Pokemon Showdown bots - mainly chatbots and bat... (EN_WORD_COHERENCY) </details> </details> </details> <details> <summary>🔇 Additional comments (27)</summary><blockquote> <details> <summary>data/formats-data.ts (1)</summary> `5614-5618`: **Tier mismatch: `archaludon` is `Uber` but `natDexTier` is `RUBL`** `tier: "Uber"` (Line 5615) implies the mon is banned from standard OU, yet `natDexTier: "RUBL"` (Line 5617) says it’s only banned from RU in NatDex. That’s an unusually large gap and may be a copy-paste error. Would you double-check Smogon’s current NatDex tiering for Archaludon and update accordingly? </details> <details> <summary>data/mods/gen7letsgo/rulesets.ts (1)</summary> `4-9`: **Confirm new ruleset identifier exists** `'LGPE Normal Rules'` replaces the old `'Adjust Level = 50'`. Ensure: 1. A format entry named exactly `lgpenormalrules` is defined in `data/rulesets.ts`. 2. Down-stream look-ups (TeamValidator, Battle scripts) use the same canonical id. A quick grep after rebasing will catch case/spacing mismatches. </details> <details> <summary>sim/team-validator.ts (1)</summary> `1108-1108`: **LGTM! Correct implementation of rule logic inversion.** The change correctly updates the `allowAVs` logic to reflect the renaming and semantic inversion of the ruleset from `'allowavs'` to `'lgpenormalrules'`. The new logic properly disables AVs when LGPE normal rules are active (which zero out AVs) and allows them otherwise. </details> <details> <summary>sim/dex-formats.ts (1)</summary> `280-280`: **LGTM! Clean rule renaming from 'allowavs' to 'lgpenormalrules'.** The rule identifier has been updated consistently with the broader refactoring mentioned in the AI summary. The logic remains identical - checking for the rule's presence to set `evLimit` appropriately for gen7letsgo formats. </details> <details> <summary>lib/index.ts (1)</summary> `3-5`: **Verified re-enabled exports—no issues found** All three exports (`crashlogger`, `ProcessManager`, `SQL`) are present under `lib/` and are imported throughout the codebase. The implementation in `lib/sql.ts` provides the full `SQLDatabaseManager`, `Statement`, `DatabaseTable`, and `SQL` template abstractions as expected, and all dependent imports have been updated to use these exports correctly. </details> <details> <summary>data/rulesets.ts (1)</summary> `1725-1731`: **LGTM! Excellent refactoring of the LGPE ruleset.** The rename from `allowavs` to `lgpenormalrules` significantly improves clarity: 1. **Better naming**: The new ID clearly indicates this is for Let's Go Pokémon Eevee/Pikachu normal rules 2. **Accurate description**: The updated description correctly reflects that this rule sets level to 50 and all AVs to 0, rather than "allowing" them 3. **Proper implementation**: The addition of `ruleset: ['Adjust Level = 50']` provides the actual mechanism for enforcing level 50 4. **Consistent behavior**: The rule now clearly communicates that it standardizes LGPE format conditions This change makes the codebase more maintainable and the rule's purpose much clearer to developers and users. </details> <details> <summary>test/lib/postgres.js (1)</summary> `10-31`: **Comprehensive test coverage for SQL template strings.** The test suite provides excellent coverage of the SQL template functionality, including edge cases like undefined values and empty arrays. Good job testing both the declarative SQL template literals and the imperative `appendRaw`/`append` methods. </details> <details> <summary>sim/global-types.ts (1)</summary> `370-370`: **Interface updated for per-stat calculation approach.** The change from batch stat processing (`spreadModify`/`natureModify`) to per-stat calculation (`statModify`) provides more granular control and cleaner separation of concerns. This aligns well with the broader refactoring across the codebase. </details> <details> <summary>sim/pokemon.ts (2)</summary> `345-345`: **Happiness override for Let's Go mechanics.** Setting happiness to 70 for the gen7letsgo mod is consistent with the stat calculation in `data/mods/gen7letsgo/scripts.ts`. This ensures uniform happiness-based stat modifiers in Let's Go formats. --- `1514-1520`: **Improved HP calculation with proper edge case handling.** The refactored `updateMaxHp` method correctly: 1. Uses the centralized `statModify` method for consistency 2. Properly handles the edge case where HP could be 0 or negative by ensuring it's at least 1 3. Triggers a silent heal event when HP changes </details> <details> <summary>data/mods/gen7letsgo/scripts.ts (1)</summary> `65-82`: **Well-implemented per-stat calculation for Let's Go mechanics.** The refactored `statModify` method correctly: 1. Applies appropriate stat formulas for HP vs other stats 2. Integrates nature modifiers with proper 16-bit truncation 3. Applies friendship-based stat modifiers specific to Let's Go 4. Uses consistent truncation throughout the calculations The consolidation of stat, nature, and friendship calculations into a single method improves maintainability. </details> <details> <summary>config/formats.ts (2)</summary> `851-853`: **Promotional message addition looks good** The `onBegin()` method correctly adds a promotional message for the Ladder Achievements Project. The HTML structure is proper and the message provides clear call-to-action with appropriate links. --- `3423-3423`: **Removal of “Allow AVs” verified against “lgpenormalrules”** No references to the legacy `Allow AVs` or `allowavs` remain, and the code correctly gates EVs and AVs off via the new `lgpenormalrules` entry: - sim/team-validator.ts uses `const allowAVs = !ruleTable.has('lgpenormalrules');` - sim/dex-formats.ts applies `this.evLimit = this.has('lgpenormalrules') ? 0 : null;` - data/rulesets.ts defines the `lgpenormalrules` ruleset This change is intentional and preserves existing functionality. </details> <details> <summary>sim/battle.ts (3)</summary> `2331-2337`: **LGTM! Clean refactoring that improves modularity.** The delegation to `statModify` for each stat calculation follows the single responsibility principle and makes the code more maintainable. The logic remains equivalent while improving code organization. --- `2339-2357`: **LGTM! Well-implemented stat calculation consolidation.** The new `statModify` method correctly consolidates stat calculation logic with proper handling of: - Different formulas for HP vs other stats - Nature modifications with 16-bit truncation - Overflow stat mod rule edge cases for specific Pokémon The mathematical formulas are accurate and the edge case handling is appropriate. The inline comments provide helpful context for the specialized logic. --- `2535-2535`: **@pokebedrock addition looks good.** The killed message addition provides useful logging information without affecting battle mechanics. The implementation correctly passes the source and effect parameters. </details> <details> <summary>sim/dex-species.ts (4)</summary> `168-168`: **Evolution type consolidation looks good.** The simplification of the `evoType` union type to seven core evolution methods aligns with the broader refactoring mentioned in the summary. This should improve type safety and consistency. --- `181-187`: **Excellent improvement to party species evolution requirements.** The rename from `evoPartySpecies` to `partySpecies` removes redundancy, and changing from string to string array provides better flexibility for complex evolution requirements. The JSDoc examples are particularly helpful and reflect actual Pokémon game mechanics. --- `657-657`: **Good migration to native deep equality checking.** Replacing the custom `Utils.isDeepStrictEqual` with Node.js native `isDeepStrictEqual` reduces dependencies on custom utilities and leverages well-tested standard library functionality. This should provide better performance and reliability for the species caching optimization. --- `3-4`: **Approve both Node.js and MoonPhase imports** All import changes are valid: - The `isDeepStrictEqual` import replaces custom deep‐equality logic appropriately. - The `@minecraft/server` dependency is declared in package.json (`"^2.1.0-beta.1.21.90-stable"`) and the `MoonPhase` type is used for `evoMoonPhase` in: - `sim/dex-species.ts` - `data/pokedex.ts` No further action required. </details> <details> <summary>data/pokedex.ts (7)</summary> `2381-2385`: **LGTM! New party slot evolution requirement implemented correctly.** The addition of `partySlotRequirement` with slot and species properties provides a clear structure for the complex evolution requirement. The `evoCondition` text also clearly describes the requirement for users. --- `5639-5639`: **LGTM! Evolution item requirement added correctly.** Adding Thunder Stone as the evolution item for Magneton provides a clear evolution path. --- `8501-8503`: **Evolution method change from level-based to item-based.** The change from level-based evolution to Thunder Stone usage represents a significant mechanic change for Probopass. Ensure this aligns with the intended game mechanics and that any dependent code handles the new evolution type correctly. --- `12333-12333`: **LGTM! Nickname requirement added for Malamar evolution.** The addition of the "Dinnerbone" nickname requirement is a clever reference to Minecraft's upside-down easter egg, fitting well with the existing upside-down console evolution condition. --- `12606-12606`: **LGTM! Weather property added for consistency.** Adding `evoWeather: "rain"` provides a machine-readable complement to the existing rain condition text, improving data structure consistency. --- `12625-12625`: **LGTM! Consistent weather property added.** Adding `evoWeather: "rain"` maintains consistency with the regular Goodra form and provides machine-readable weather conditions. --- `15965-15965`: **LGTM! Evolution condition clarified with specific requirements.** The updated condition text provides clearer, more specific requirements (49+ damage threshold and desert biome), improving user understanding and implementation clarity. </details> </blockquote></details> </details>

config/formats.ts

⚠️ Potential issue

Critical issue with stat calculation logic - potential division by zero and incorrect formula

The new statModify method has several serious problems:

Division by zero risk: Line 1614 uses tr(tr(stat * 110, 16) / 100) but if stat is 0, this could cause issues
Incorrect base stat swapping: Lines 1600-1606 swap the base stat names based on nature, but this logic seems flawed - it's swapping the stat being calculated rather than the values themselves
Missing validation: No bounds checking on the calculated stats
Complex nested truncation: The double truncation pattern tr(tr(...), 16) on line 1614 is unusual and may not be intended

The stat swapping logic appears fundamentally incorrect. Consider this fix:

-		let baseStatName = statName;
-		if (nature.plus) {
-			if (statName === nature.minus) {
-				baseStatName = nature.plus;
-			} else if (statName === nature.plus) {
-				baseStatName = nature.minus!;
-			}
-		}
-		let stat = baseStats[baseStatName];
+		// Get the actual base stat value for this stat
+		let stat = baseStats[statName];
+		
+		// For nature swap, we need to swap the FINAL calculated values, not the base stats

Additionally, add bounds checking and fix the truncation:

-		stat = tr(tr(stat * 110, 16) / 100);
+		stat = tr(stat * 110 / 100);
+		stat = Math.min(stat, 999); // Add reasonable bounds

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In config/formats.ts between lines 1596 and 1617, fix the statModify method by
correcting the nature-based stat modification logic to adjust the stat value
itself rather than swapping stat names. Remove the nested truncation calls and
replace them with a single truncation using the correct precision. Add
validation to ensure no division by zero occurs and clamp the final stat values
within valid bounds to prevent invalid results.

_⚠️ Potential issue_ **Critical issue with stat calculation logic - potential division by zero and incorrect formula** The new `statModify` method has several serious problems: 1. **Division by zero risk**: Line 1614 uses `tr(tr(stat * 110, 16) / 100)` but if `stat` is 0, this could cause issues 2. **Incorrect base stat swapping**: Lines 1600-1606 swap the base stat names based on nature, but this logic seems flawed - it's swapping the stat being calculated rather than the values themselves 3. **Missing validation**: No bounds checking on the calculated stats 4. **Complex nested truncation**: The double truncation pattern `tr(tr(...), 16)` on line 1614 is unusual and may not be intended The stat swapping logic appears fundamentally incorrect. Consider this fix: ```diff - let baseStatName = statName; - if (nature.plus) { - if (statName === nature.minus) { - baseStatName = nature.plus; - } else if (statName === nature.plus) { - baseStatName = nature.minus!; - } - } - let stat = baseStats[baseStatName]; + // Get the actual base stat value for this stat + let stat = baseStats[statName]; + + // For nature swap, we need to swap the FINAL calculated values, not the base stats ``` Additionally, add bounds checking and fix the truncation: ```diff - stat = tr(tr(stat * 110, 16) / 100); + stat = tr(stat * 110 / 100); + stat = Math.min(stat, 999); // Add reasonable bounds ``` > Committable suggestion skipped: line range outside the PR's diff. <details> <summary>🤖 Prompt for AI Agents</summary> ``` In config/formats.ts between lines 1596 and 1617, fix the statModify method by correcting the nature-based stat modification logic to adjust the stat value itself rather than swapping stat names. Remove the nested truncation calls and replace them with a single truncation using the correct precision. Add validation to ensure no division by zero occurs and clamp the final stat values within valid bounds to prevent invalid results. ``` </details>

data/pokedex.ts

💡 Verification agent

🧩 Analysis chain

Evolution method changed to item-based with Leaders Crest.

The change from level-based to item-based evolution using "Leaders Crest" aligns with the pattern of evolution updates seen throughout this file (similar to Probopass). The item name fits the regal theme of Kingambit.

Verify that the evolution system properly handles the new item-based evolutions and that "Leaders Crest" is properly defined in the items database.

🏁 Script executed:

#!/bin/bash
# Search for Leaders Crest definition and other evolution item usage
rg -A 3 -B 3 "Leaders Crest" --type ts
rg -A 3 "evoType.*useItem" --type ts | head -20

Length of output: 1245

🏁 Script executed:

#!/bin/bash
# Locate the items definition file(s)
fd -e ts items

# Search for "Leaders Crest" in the items database
rg -C3 'name:\s*"Leaders Crest"' data/items.ts
rg -C3 'leaderscrest' data/items.ts

Length of output: 813

🏁 Script executed:

#!/bin/bash
# Comprehensive search for any "Crest" item definitions across data and sim directories
rg --type ts --ignore-case -C3 crest data sim

Length of output: 358

Missing “Leaders Crest” item definition

I wasn’t able to locate a Leaders Crest entry in the items database (e.g. any of data/items.ts, sim/dex-items.ts, or their mod variants). Without that definition, the new evoItem reference will be invalid at runtime.

• data/pokedex.ts (lines ~17929–17931):
evoType: "useItem",
evoItem: "Leaders Crest",
evoCondition: "Give Bisharp a Leaders Crest"

Please add a corresponding Leaders Crest item in your items files (name, id, effects, etc.) so that this evolution method resolves correctly.

🤖 Prompt for AI Agents

In data/pokedex.ts around lines 17929 to 17931, the evoItem "Leaders Crest" is
referenced but there is no corresponding item definition in the items database
files such as data/items.ts or sim/dex-items.ts. To fix this, add a new item
entry for "Leaders Crest" in the appropriate items file, including its name, id,
and any relevant effects or properties, so that the evoItem reference resolves
correctly at runtime.

_💡 Verification agent_ <details> <summary>🧩 Analysis chain</summary> **Evolution method changed to item-based with Leaders Crest.** The change from level-based to item-based evolution using "Leaders Crest" aligns with the pattern of evolution updates seen throughout this file (similar to Probopass). The item name fits the regal theme of Kingambit. Verify that the evolution system properly handles the new item-based evolutions and that "Leaders Crest" is properly defined in the items database. --- 🏁 Script executed: ```shell #!/bin/bash # Search for Leaders Crest definition and other evolution item usage rg -A 3 -B 3 "Leaders Crest" --type ts rg -A 3 "evoType.*useItem" --type ts | head -20 ``` Length of output: 1245 --- 🏁 Script executed: ```shell #!/bin/bash # Locate the items definition file(s) fd -e ts items # Search for "Leaders Crest" in the items database rg -C3 'name:\s*"Leaders Crest"' data/items.ts rg -C3 'leaderscrest' data/items.ts ``` Length of output: 813 --- 🏁 Script executed: ```shell #!/bin/bash # Comprehensive search for any "Crest" item definitions across data and sim directories rg --type ts --ignore-case -C3 crest data sim ``` Length of output: 358 --- </details> **Missing “Leaders Crest” item definition** I wasn’t able to locate a `Leaders Crest` entry in the items database (e.g. any of `data/items.ts`, `sim/dex-items.ts`, or their mod variants). Without that definition, the new evoItem reference will be invalid at runtime. • data/pokedex.ts (lines ~17929–17931): evoType: "useItem", evoItem: "Leaders Crest", evoCondition: "Give Bisharp a Leaders Crest" Please add a corresponding `Leaders Crest` item in your items files (name, id, effects, etc.) so that this evolution method resolves correctly. <details> <summary>🤖 Prompt for AI Agents</summary> ``` In data/pokedex.ts around lines 17929 to 17931, the evoItem "Leaders Crest" is referenced but there is no corresponding item definition in the items database files such as data/items.ts or sim/dex-items.ts. To fix this, add a new item entry for "Leaders Crest" in the appropriate items file, including its name, id, and any relevant effects or properties, so that the evoItem reference resolves correctly at runtime. ``` </details>

lib/database.ts

⚠️ Potential issue

Missing LIMIT 1 clause in updateOne method.

The updateOne method no longer includes LIMIT 1, which means it could update multiple rows despite its name suggesting a single row update. This is a breaking change and could lead to unintended data modifications.

Consider adding back the LIMIT clause for MySQL or using a primary key constraint:

 updateOne(partialRow: PartialOrSQL<Row> | SQLStatement):
 (strings: TemplateStringsArray, ...rest: SQLValue[]) => Promise<OkPacketOf<DB>> {
   return (s, ...r) =>
-    this.queryExec()`UPDATE "${this.name}" SET ${partialRow as any} ${new SQLStatement(s, r)}`;
+    this.queryExec()`UPDATE "${this.name}" SET ${partialRow as any} ${new SQLStatement(s, r)} ${this.db.type === 'mysql' ? SQL`LIMIT 1` : SQL``}`;
 }

🤖 Prompt for AI Agents

In lib/database.ts around lines 260 to 264, the updateOne method is missing a
LIMIT 1 clause, which can cause multiple rows to be updated instead of just one.
To fix this, modify the SQL query string to append "LIMIT 1" at the end of the
update statement to ensure only one row is updated, preserving the intended
behavior of the method.

_⚠️ Potential issue_ **Missing LIMIT 1 clause in updateOne method.** The `updateOne` method no longer includes `LIMIT 1`, which means it could update multiple rows despite its name suggesting a single row update. This is a breaking change and could lead to unintended data modifications. Consider adding back the LIMIT clause for MySQL or using a primary key constraint: ```diff updateOne(partialRow: PartialOrSQL<Row> | SQLStatement): (strings: TemplateStringsArray, ...rest: SQLValue[]) => Promise<OkPacketOf<DB>> { return (s, ...r) => - this.queryExec()`UPDATE "${this.name}" SET ${partialRow as any} ${new SQLStatement(s, r)}`; + this.queryExec()`UPDATE "${this.name}" SET ${partialRow as any} ${new SQLStatement(s, r)} ${this.db.type === 'mysql' ? SQL`LIMIT 1` : SQL``}`; } ``` <details> <summary>🤖 Prompt for AI Agents</summary> ``` In lib/database.ts around lines 260 to 264, the updateOne method is missing a LIMIT 1 clause, which can cause multiple rows to be updated instead of just one. To fix this, modify the SQL query string to append "LIMIT 1" at the end of the update statement to ensure only one row is updated, preserving the intended behavior of the method. ``` </details>

lib/database.ts

⚠️ Potential issue

Remove unsafe non-null assertion in PGDatabase constructor.

Using null! when config is falsy will cause runtime errors. The constructor should properly handle missing configuration.

-super(config ? new pg.Pool(config) : null!);
+if (!config) {
+  throw new Error('PostgreSQL configuration is required');
+}
+super(new pg.Pool(config));

📝 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.

	// Ensure we have a valid PostgreSQL configuration before calling super
	if (!config) {
		throw new Error('PostgreSQL configuration is required');
	}
	super(new pg.Pool(config));

🤖 Prompt for AI Agents

In lib/database.ts at line 387, the constructor uses a non-null assertion with
null! which can cause runtime errors if config is falsy. Modify the constructor
to properly handle the case when config is missing by either providing a default
configuration, throwing an error, or handling the null case safely without using
the non-null assertion.

_⚠️ Potential issue_ **Remove unsafe non-null assertion in PGDatabase constructor.** Using `null!` when config is falsy will cause runtime errors. The constructor should properly handle missing configuration. ```diff -super(config ? new pg.Pool(config) : null!); +if (!config) { + throw new Error('PostgreSQL configuration is required'); +} +super(new pg.Pool(config)); ```  <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 // Ensure we have a valid PostgreSQL configuration before calling super if (!config) { throw new Error('PostgreSQL configuration is required'); } super(new pg.Pool(config)); ``` </details>  <details> <summary>🤖 Prompt for AI Agents</summary> ``` In lib/database.ts at line 387, the constructor uses a non-null assertion with null! which can cause runtime errors if config is falsy. Modify the constructor to properly handle the case when config is missing by either providing a default configuration, throwing an error, or handling the null case safely without using the non-null assertion. ``` </details>

server/chat-plugins/teams.ts

⚠️ Potential issue

Remove unsafe non-null assertion that can cause runtime errors.

The non-null assertion null! is dangerous. If Config.usepostgres is falsy, teamsDB will be null but TypeScript will treat it as non-null, leading to runtime errors when any methods are called on it.

Consider using a more defensive approach:

-export const teamsDB = Config.usepostgres ? new PGDatabase(Config.usepostgres) : null!;
-export const teamsTable = teamsDB?.getTable<
-	StoredTeam
->('teams', 'teamid');
+export const teamsDB = Config.usepostgres ? new PGDatabase(Config.usepostgres) : null;
+export const teamsTable = teamsDB ? teamsDB.getTable<StoredTeam>('teams', 'teamid') : null;

Then add null checks where these are used, or throw an error immediately if the database is required but not configured.

📝 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.

export const teamsDB = Config.usepostgres ? new PGDatabase(Config.usepostgres) : null;
export const teamsTable = teamsDB ? teamsDB.getTable<StoredTeam>('teams', 'teamid') : null;

🤖 Prompt for AI Agents

In server/chat-plugins/teams.ts around lines 17 to 20, remove the unsafe
non-null assertion `null!` when assigning `teamsDB` because it can cause runtime
errors if `Config.usepostgres` is falsy. Instead, assign `teamsDB` to either a
new PGDatabase instance or null without the assertion. Then, add proper null
checks before using `teamsDB` or throw an error early if the database connection
is mandatory to prevent runtime failures.

_⚠️ Potential issue_ **Remove unsafe non-null assertion that can cause runtime errors.** The non-null assertion `null!` is dangerous. If `Config.usepostgres` is falsy, `teamsDB` will be null but TypeScript will treat it as non-null, leading to runtime errors when any methods are called on it. Consider using a more defensive approach: ```diff -export const teamsDB = Config.usepostgres ? new PGDatabase(Config.usepostgres) : null!; -export const teamsTable = teamsDB?.getTable< - StoredTeam ->('teams', 'teamid'); +export const teamsDB = Config.usepostgres ? new PGDatabase(Config.usepostgres) : null; +export const teamsTable = teamsDB ? teamsDB.getTable<StoredTeam>('teams', 'teamid') : null; ``` Then add null checks where these are used, or throw an error immediately if the database is required but not configured.  <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 export const teamsDB = Config.usepostgres ? new PGDatabase(Config.usepostgres) : null; export const teamsTable = teamsDB ? teamsDB.getTable<StoredTeam>('teams', 'teamid') : null; ``` </details>  <details> <summary>🤖 Prompt for AI Agents</summary> ``` In server/chat-plugins/teams.ts around lines 17 to 20, remove the unsafe non-null assertion `null!` when assigning `teamsDB` because it can cause runtime errors if `Config.usepostgres` is falsy. Instead, assign `teamsDB` to either a new PGDatabase instance or null without the assertion. Then, add proper null checks before using `teamsDB` or throw an error early if the database connection is mandatory to prevent runtime failures. ``` </details>

test/lib/postgres.js

⚠️ Potential issue

Potential issue with uninitialized PGDatabase instance.

Creating a PGDatabase instance without configuration may cause issues. Based on the implementation in lib/database.ts, when no config is provided, the connection is set to null!, which could lead to runtime errors if any methods that require a database connection are called.

Consider either:

Passing a mock configuration to avoid potential null reference issues
Adding a comment explaining this is only for testing the _resolveSQL method which doesn't require a connection

🤖 Prompt for AI Agents

In test/lib/postgres.js at line 5, the PGDatabase instance is created without
any configuration, which sets its connection to null and may cause runtime
errors if methods requiring a connection are called. To fix this, either provide
a mock configuration object when instantiating PGDatabase to ensure a valid
connection or add a clear comment explaining that this instance is only used for
testing the _resolveSQL method, which does not require a database connection.

_⚠️ Potential issue_ **Potential issue with uninitialized PGDatabase instance.** Creating a `PGDatabase` instance without configuration may cause issues. Based on the implementation in `lib/database.ts`, when no config is provided, the connection is set to `null!`, which could lead to runtime errors if any methods that require a database connection are called. Consider either: 1. Passing a mock configuration to avoid potential null reference issues 2. Adding a comment explaining this is only for testing the `_resolveSQL` method which doesn't require a connection <details> <summary>🤖 Prompt for AI Agents</summary> ``` In test/lib/postgres.js at line 5, the PGDatabase instance is created without any configuration, which sets its connection to null and may cause runtime errors if methods requiring a connection are called. To fix this, either provide a mock configuration object when instantiating PGDatabase to ensure a valid connection or add a clear comment explaining that this instance is only used for testing the _resolveSQL method, which does not require a database connection. ``` </details>