Phase 7: engine accuracy, abilities/items, BST-aware suggestions - #36
Merged
Conversation
Adds docs/plan/phase-7-accuracy-and-customization.md, the executable plan for the next phase, plus its pointers in CLAUDE.md and docs/plan/README.md. The plan opens with an audit measured against the pinned dataset revision, not recalled: - The suggestion ranking degenerates to "lowest Pokedex id" once a team's coverage is complete: every candidate ties on the composite score, so the entryId tie-break decides. That is exactly the reported Raticate/Persian/Kangaskhan output. - PokemonEntry.defaultAbility carries the raw PokeAPI slug, so the same ability renders as "sap-sipper" in the field and "Sap Sipper" in the picker one row below. - prettify() cannot produce correct English names (Double-Edge, U-turn, Well-Baked Body); only ability_names.csv / move_names.csv can. - Ten defensive abilities that change type effectiveness are unmodelled, carried by 26 catalogue forms, 16 of them as the slot-1 default; dry-skin is modelled only halfway; AbilityEffectSide.OFFENSIVE is declared but never constructed or consumed. - Held items are absent from the whole app, and Showdown import discards them. - findEntry() is a linear scan run once per candidate. The plan then specifies the dataset additions (four CSVs, 212,818 to 578,165 bytes, measured), the per-generation BST rule with worked examples and the Gen-I five-stat decision, the canonical-plus-custom ability picker, the defensive-item subset with an explicit application order, the BST tie-break (score formula and its shared weights untouched), the 5-10 suggestion-count setting, Room v3, and the test suite that has to prove it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011j5jieoE4nqU2ZBSM7USHc
Implements docs/plan/phase-7-accuracy-and-customization.md steps 1-4 of its task order (§9): the dataset layer, Room v3, and the ability picker/engine fixes. BST-aware suggestions, held items, the suggestion-count setting, docs and the exhaustive type-chart/dual-typing test sweep are follow-up commits on this same PR. Dataset (§2): - 4 new pinned CSVs (pokemon_stats, pokemon_stats_past, ability_names, move_names), parsed and joined into the assembled catalogue. - PokemonEntry.baseStatTotal (current BST) and ParsedDataset.pastBst (per-generation historical BST, five-stat Gen-I rule) per §2.2's worked examples. - ParsedDataset.pokemonAbilities: every canonical (pokemonId, slot) ability row, for the picker's canonical list. - Ability/move display names now come from ability_names.csv/move_names.csv instead of prettify(), which cannot produce a correct hyphenated name (Well-Baked Body, Double-Edge, U-turn, ...). defaultAbility now carries the display name, not the raw PokeAPI slug (§0.2's Sap Sipper/sap-sipper bug). Room v3 (§8): - poke_species.baseStatTotal, team_member.item, custom_pokemon.item (nullable, item wiring itself lands in the next commit). - New cache tables poke_pokemon_ability, poke_species_bst_past, wired into PokedexDao.replaceCache()/clearCache() by name, never clearAllTables(). - MIGRATION_2_3 + schemas/3.json + DATASET_SCHEMA_VERSION bump to 2, so every existing install re-syncs and actually gets the new columns. - Migration2To3Test mirrors Migration1To2Test's MigrationTestHelper coverage. Ability picker (§3): - abilityKey() (strip every symbol, not just spaces) replaces normalizeAbilityName(), so a slug and a display name with a mismatched hyphen still resolve to the same ABILITY_EFFECTS entry. - ui/common/AbilityPicker.kt: a species' canonical abilities (normal, then hidden) plus a "Custom ability..." row that falls back to the existing free-text-with-suggestions picker — a ROM hack ability with no PokeAPI entry stays typeable. Canonical options with a coverage effect are marked. Wired into SlotEditorScreen and RosterEditorScreen. Ability effect gaps (§7.1/§7.2), found by auditing every ability's PokeAPI short_effect text: - 10 previously-unmodelled defensive abilities (Heatproof, Water Bubble, Purifying Salt, Filter, Solid Rock, Prism Armor, Primordial Sea, Desolate Land, Delta Stream, Tera Shell) plus Dry Skin's missing Fire 1.25x half. - Wonder Guard promoted from a UI-only badge to a real effect: only a super-effective hit deals damage. - The offensive gap: Scrappy/Mind's Eye (Normal/Fighting bypass Ghost immunity, grid-display only) and the -ate/Normalize abilities (rewrite a Normal move's type, gated on real moves being entered). - CoverageEngineTest/AbilityEffectsTest extended accordingly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011j5jieoE4nqU2ZBSM7USHc
Implements docs/plan/phase-7-accuracy-and-customization.md §4 — step 5 of
the task order (§9). BST-aware suggestions, the suggestion-count setting,
docs and the exhaustive type-chart sweep are still follow-up commits.
- domain/item/ItemEffects.kt: Air Balloon, Iron Ball, Ring Target, and one
resist berry per type (18 items total: 17 types + Chilan for Normal,
which applies unconditionally since nothing is super-effective against
Normal). items.csv is deliberately not downloaded — this is a hardcoded
table, same shape as ABILITY_EFFECTS, with the same symbol-insensitive
itemKey() lookup.
- TeamMember.item (nullable, default null — a suggestion candidate never
sets one). Room columns landed in the previous commit; this one wires
them through TeamMappers, CustomPokemonDao's upsert (already fixed
earlier) and BackupRepositoryImpl's restore path, which had its own
direct CustomPokemonEntity(...) construction that bypassed the mapper
and would have silently dropped the item on every restore — caught by a
new round-trip test before it shipped.
- CoverageEngine.defensiveMultiplier gains an item parameter and applies
ability+item effects in the order §4.2 specifies: item
RemovesTypeImmunities/GroundsHolder cancel a type-chart 0, then ability
immunity (skipped for a Ground attack under Iron Ball, which also
neutralizes Levitate/Earth Eater), then item Immunity, then ability
multiplier and super-effective reducer, then the resist berry. Threaded
through defensiveProfile/sharedWeaknessCounts/mostVulnerableByType and
Scoring.kt's weaknesses()/teamScoringContext()/computeCompositeScore().
- Showdown export/import: "Species @ Item" round-trips instead of being
discarded on import. Backup format bumped to v2 (a v1 file has no `item`
key at all, so it still decodes with item defaulting to null).
- UI: ui/common/ItemPicker.kt (free text + suggestions from the modelled
set, same contract as the ability field), wired into SlotEditorScreen
and RosterEditorScreen; shown on SlotSummaryCard ("@ Item") and
PerPokemonCard (with its effect summary, mirroring the ability row).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011j5jieoE4nqU2ZBSM7USHc
Implements docs/plan/phase-7-accuracy-and-customization.md §5 and §6 —
steps 6-7 of the task order (§9). Docs, the exhaustive type-chart/dual-
typing test sweep, and a broader ability/item test pass are still
follow-up commits on this same PR.
Suggestion ranking (§5):
- Suggestion.baseStatTotal (null for a custom candidate); the ranking
comparator gains a single new tie-break step — bestScore, then isFinal,
then baseStatTotal descending, then catalogue id ascending. The
composite-score formula and its 0.5/1.0 weights are untouched, so
ScoringTest/TeamGeneratorTest keep passing unchanged.
- domain/model/PastBst.kt: bstResolverFor(pastBst, generation) — null
means the current, latest-generation value; a real generation resolves
to the smallest stored historical breakpoint at or after it (proven
correct in the plan's own §2.2, and exercised here against every case:
no history, before/at/after the only breakpoint, strictly between two
independent breakpoints, and no cross-contamination between forms).
AnalysisViewModel now fetches allPastBst() alongside the chart/pool and
passes a generation-aware resolver into computeSuggestions.
- SuggestionCard shows the base stat total and a plain-language score
hint ("coverage gained minus weaknesses introduced").
- SuggestionEngine.findEntry's per-candidate linear scan over the ~1351-
entry pool (up to ~1.8M string comparisons per recomputation) replaced
with two maps built once per computeSuggestions call, same displayName-
first precedence.
- The old "secondary sort ... ascending catalogue id" test replaced with
two regression tests against a hand-built pool: a higher baseStatTotal
now outranks a lower one on a composite-score tie (reproducing the
reported Raticate/Persian/Kangaskhan case), and id still decides when
baseStatTotal also ties.
Suggestion count (§6):
- SettingsPreferences.suggestionCount, clamped to 5-10 on both read and
write, default 5 (matching the value Phase 4 hardcoded).
- ui/common/StepperCounter.kt: the -/+ row promoted out of
ui/surprise/SurpriseMeScreen.kt's private ConstraintCounter (0-floor,
no shared ceiling) into a shared composable with explicit
canDecrement/canIncrement bounds; Surprise Me's five constraint rows and
the new Settings row both use it now.
- AnalysisUiState.suggestionCount flows from the setting through to
AnalysisScreen's take(state.suggestionCount), replacing the hardcoded
take(5); AnalysisUiState.suggestions itself stays unsliced.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011j5jieoE4nqU2ZBSM7USHc
Implements docs/plan/phase-7-accuracy-and-customization.md §7.3.1-3 — the
point-3 ask ("verify the weakness/resistance engine is correct across all
1000+ Pokemon"), answered at the level where it's actually decidable: the
complete 171-typing space (18 single types + all 153 unordered dual-type
pairs), not a re-test of the same space 1351 times over with worse
failure messages.
- Every one of the 324 type-chart cells is one of {0, 0.5, 1, 2}.
- The eight classic asymmetric matchups (Ghost/Normal, Fighting/Ghost,
Ground/Flying immunities; Fairy/Dragon, Steel/Fairy, Fire/Steel;
Poison/Steel, Electric/Ground) assert their exact value.
- defensiveMultiplier for all 171 typings against all 18 attacking types
equals the product of the two single-type lookups.
- defensiveProfile buckets every non-neutral type into exactly one of
weaknesses/resistances/immunities and omits every neutral one, checked
against the real multiplier for all 171 typings x 18 attacking types —
this is the test that would have caught a mis-bucketing or
double-counting bug anywhere in the whole type-effectiveness surface.
- A cheap structural guard in DatasetAssemblyTest that every assembled
form's typing is non-null and has no duplicate second type.
- One more ability case (Filter turning a genuine x4 weakness into x3,
the plan's own worked example) alongside the ones already covered when
the ability-effect gaps were closed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011j5jieoE4nqU2ZBSM7USHc
Update CHANGELOG, native spec, reference dataset contract, architecture docs, status snapshot, roadmap, implementation decisions, post-migration review, and README to reflect Phase 7's shipped scope: base stat totals, canonical ability names/picker, held item coverage effects, BST tie-break in suggestions, configurable suggestion count, and the exhaustive coverage-engine verification. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011j5jieoE4nqU2ZBSM7USHc
KNOWN_ABILITIES_WITH_EFFECTS was declared before ABILITY_EFFECTS, which
it reads at initialization — Kotlin does not allow a top-level property
initializer to forward-reference another one declared later in the same
file ("Variable 'ABILITY_EFFECTS' must be initialized"). This broke
compilation on every commit since it was introduced; move the derived
list below the map it derives from.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011j5jieoE4nqU2ZBSM7USHc
Marcogn
changed the title
Marcogn
marked this pull request as ready for review
September 6, 2026 18:15
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Full implementation of Phase 7 —
docs/plan/phase-7-accuracy-and-customization.md.Why
Four things were raised against the shipped app: the suggestion alternatives look irrelevant; abilities render as raw slugs and can't be picked freely for ROM hacks; the weakness/resistance and suggestion engines needed verifying; the number of suggestions should be configurable.
What changed
pokemon_stats,pokemon_stats_past,ability_names,move_names); dataset grows to ~12 requests / ~565 KB. Room bumped to schema v3 (baseStatTotal, per-form abilities, past-BST, item columns), with a real migration andMigrationTestHelpertest.ability_names.csv/move_names.csvinstead of aprettify()heuristic that mangled hyphenated proper names (Well-Baked Body,U-turn). New canonical-ability-or-custom-text picker (AbilityPicker) for ROM hack support, wired into both the team slot editor and the custom roster editor.-ate/Normalize type-override gap, a Dry Skin fix (was immunity-only, now also takes 1.25x Fire), and a Wonder Guard bug (was a UI-only badge, not applied to the actual multiplier).TeamMember/CustomPokemon, Room columns, the coverage engine's item+ability pipeline, Showdown import/export, and backup format v2.pokemon_stats_past.csv. The composite score formula itself is untouched, per an explicit scope decision. Also replaced a linearfindEntry()scan (run once per candidate, every team edit) with a lookup map.AnalysisViewModel/AnalysisScreen.AbilityEffects.kt(KNOWN_ABILITIES_WITH_EFFECTSreadingABILITY_EFFECTSbefore its declaration) broke every build from the first Phase 7 commit onward — this sandbox has no local Android SDK/JDK 17, so it went undetected until CI ran. Fixed by reordering the declarations; confirmed green after.Five findings were deliberately deferred rather than folded into this phase — see
docs/post-migration-review.md("Phase 7 audit") andROADMAP.md(generational type charts chief among them, since the pinned dataset already has the CSVs but no game/generation rule-set concept exists anywhere else in the app).Test plan
./gradlew testDebugUnitTest— green in CI (build-and-test) on the final commit../gradlew lintDebug— green in CI.docs/test-plan.md's new Phase 7 section (ability picker, item picker, BST-ordered suggestions, suggestion-count stepper) — not run in this sandbox, no device available.🤖 Generated with Claude Code
https://claude.ai/code/session_011j5jieoE4nqU2ZBSM7USHc
Generated by Claude Code