review-changes

name

review-changes

description

Domain-specific review of uncommitted changes to collection classes, database migrations, or flags — checks migration guards (!fromImport, oldVersion <=), collection-upgrade completeness, checkbox flag round-trip and masks, COIN_IMG_IDS/COLLECTION_TYPES array immutability, and test coverage. Use before committing any change that touches DatabaseHelper migrations, collection classes, or CollectionListInfo flags. NOT a general code review — use /code-review for that.

Review Changes

Audit the current uncommitted changes (or a specified set of files) against the project's known pitfall areas. Use git diff to identify what changed, then verify each checklist section below. Skip sections that don't apply to the current changes.

1. Database migration guards (upgradeDbStructure)

For each version-gated block in DatabaseHelper.upgradeDbStructure():

• Classify it as schema change (ALTER TABLE, ADD COLUMN) or data-value migration (UPDATE existing column values, ORing flag bits)
• Schema changes must have && !fromImport — imported databases already have the latest columns
• Data-value migrations must omit the !fromImport guard — imported databases have the latest schema but carry old data values from the export that also need updating
• All blocks use oldVersion <= N (incremental), never oldVersion == N
• Use Xxx.COLLECTION_TYPE qualified references for collection type names in migration blocks (e.g., LincolnCents.COLLECTION_TYPE not "Pennies"). If a COLLECTION_TYPE value was renamed in this diff, verify that every old migration block that used Xxx.COLLECTION_TYPE has been updated to use the old literal string value instead of the constant — only the new migration performing the rename should reference the constant.
• Verify that no old migration block references a constant whose value changed in this diff. COLLECTION_TYPE constants, column name constants (COL_*), table name constants (TBL_*), display constants (e.g., SIMPLE_DISPLAY), flag constants — any symbol whose literal value is meaningful to migration SQL or logic. If a constant's value was modified, every old migration block that used it must be updated to use the old literal value instead of the constant, or it will silently pick up the new value and break.

2. Collection upgrade completeness

For each collection class that changed:

• Every collection that adds coins in populateCollectionLists() has a matching onCollectionDatabaseUpgrade() block for the current DATABASE_VERSION
• When populateCollectionLists() sets imageId via getImgId(), the corresponding onCollectionDatabaseUpgrade() passes the same image ID
• addFromYear / addFromArrayList calls respect the user's mint mark configuration

3. Checkbox/flag round-trip integrity

For any new checkbox or flag added to a collection:

• Bit constant defined in CollectionListInfo (appended, not inserted)
• has*() getter method in CollectionListInfo
• Encoder block in CoinPageCreator.getCheckboxFlagsFromParameters()
• Decoder block in CoinPageCreator.getParametersFromCollectionListInfo()
• ALL_CHECKBOXES_MASK in CollectionListInfo updated to cover new bits
• ALL_MINT_MASK in CollectionListInfo updated if mint mark flags changed
• Masks use the formula (1L << (highestBit + 1)) - 1 — no padding bits
• Flag bit positions are contiguous — no gaps between existing and new flags
• testFixedCheckBoxIds / testFixedMintMarkIds in MainApplicationTests updated with the new flag assertion and updated mask assertion
• Migration in upgradeDbStructure() to set the flag for existing collections (so coins already added by upgrade are visible)

4. Test coverage

• CollectionCreationTests updated for affected collections — new checkbox columns added to test arrays, expected counts adjusted
• CollectionUpgradeTests covers new upgrade paths
• SharedTest.COLLECTION_LIST_INFO_SCENARIOS updated if checkbox or mint mark flags changed
• Round-trip test (CoinPageCreatorTests.test_createFromParameters) passes

5. Resource completeness

• New string resources in res/values/strings.xml for any new UI labels
• New drawable resources for any new coin images
• COIN_IMG_IDS array ordering is immutable — the array index is the imageId persisted in user databases. Verify that:
  • New entries are only appended to the end of the array
  • No existing entries were reordered, removed, or inserted in the middle
  • Index comments (e.g., // 19) are accurate after any additions
  • Reordering or inserting would silently remap images for every existing user collection — this is a data-corruption-level bug
• COLLECTION_TYPES array ordering is immutable — removing an entry causes getIndexFromCollectionNameStr() to return -1, crashing the app for any user who has that collection. Verify that:
  • New entries are only appended to the end of the array
  • No existing entries were reordered, removed, or inserted in the middle
  • The display-order arrays (BASIC_COLLECTIONS, ADVANCED_COLLECTIONS, MORE_COLLECTIONS) can be freely reordered, but COLLECTION_TYPES cannot

6. Scope discipline

• Changes are limited to what was requested — no unrelated refactors
• No modifications to collections excluded from scope
• DATABASE_VERSION bumped only if shipping a new release (not for unreleased changes already covered by the current version)

Output

Report a summary table with one row per section. Skip sections that don't apply to the current diff.

Area	Status	Findings
DB migration guards	OK / ISSUE	details
Collection upgrades	OK / ISSUE	details
Checkbox round-trip	OK / ISSUE	details
Test coverage	OK / ISSUE	details
Resources	OK / ISSUE	details
Scope	OK / ISSUE	details

If all applicable areas are OK, confirm the changes look good. If any issues are found, list the specific problems and recommended fixes.