☂️ Python Coverage

current status: ✅

Overall Coverage

New Files

No new covered files...

Modified Files

updated for commit: 7146081 by action🐍

Test Results (mariadb)

1 002 tests  +18   1 001 ✅ +18   2m 36s ⏱️ +4s
    1 suites ± 0       1 💤 ± 0 
    1 files   ± 0       0 ❌ ± 0 

Results for commit 7146081. ± Comparison against base commit 6f02a4b.

♻️ This comment has been updated with latest results.

Test Results (postgresql)

1 002 tests  +18   1 001 ✅ +18   2m 35s ⏱️ -11s
    1 suites ± 0       1 💤 ± 0 
    1 files   ± 0       0 ❌ ± 0 

Results for commit 7146081. ± Comparison against base commit 6f02a4b.

♻️ This comment has been updated with latest results.

Greptile Summary

This PR auto-derives and persists rom_user.status from a user's RetroAchievements award kind during the scheduled progress sync, mapping mastered/completed → COMPLETED_100, beaten-* → FINISHED, and no award → INCOMPLETE. It also adds highest_award_kind to the RAUserGameProgression cache structure so the field stays fresh even on cache-hit paths without extra API calls.

Key changes:

• ra_handler.py: new highest_award_kind: NotRequired[str | None] field on RAUserGameProgression; populated from HighestAwardKind in the RA completion-progress response; cache-hit branch always refreshes this field from the live API value.
• sync_retroachievements_progress.py: adds _get_rom_user_status_from_ra_award_kind() mapping helper and _sync_rom_user_statuses() which writes the derived status after each successful progression update, skipping updates when the status already matches or is RETIRED/NEVER_PLAYING.
• test_sync_retroachievements_progress.py: covers the mapping helper and five status-sync scenarios, though the RETIRED/NEVER_PLAYING guard itself is not directly tested.

Issues found:

• _sync_rom_user_statuses is invoked in the else block of the try/except, which means any exception it raises (DB error, etc.) is unhandled and will abort the sync for all remaining users in the current run — the existing error-and-continue pattern used for get_user_progression failures should be applied here too.
• No test covers the RETIRED/NEVER_PLAYING preservation logic, which is the primary safety mechanism requested in the original issue.

Confidence Score: 3/5

• PR is mostly correct but has an unhandled exception path that can abort the entire sync run for all remaining users.
• The mapping logic and cache-refresh behaviour are sound, and the RETIRED/NEVER_PLAYING guard addresses the core concern from the original issue. However, placing _sync_rom_user_statuses in the unprotected else block is a real bug: a single DB exception aborts the loop for all subsequent users, which undermines the resilience pattern already present in the task. The missing test for the preservation guard also reduces confidence. These are fixable but need to be addressed before merging.
• Pay close attention to backend/tasks/scheduled/sync_retroachievements_progress.py around the _sync_rom_user_statuses call and its error handling.

Important Files Changed

Sequence Diagram

sequenceDiagram
    participant Task as SyncRetroAchievementsProgressTask
    participant RA as RAHandler
    participant UserDB as DBUsersHandler
    participant RomDB as DBRomsHandler

    Task->>UserDB: get_users(has_ra_username=True)
    loop For each user
        Task->>RA: get_user_progression(username, current_progression)
        RA-->>Task: RAUserProgression (with highest_award_kind)
        Task->>UserDB: update_user(id, {ra_progression})
        Note over Task: else block (unprotected)
        Task->>Task: _sync_rom_user_statuses(user, progression)
        loop For each game in progression
            Task->>Task: _get_rom_user_status_from_ra_award_kind(highest_award_kind)
            Task->>RomDB: get_rom_by_metadata_id(ra_id=rom_ra_id)
            RomDB-->>Task: Rom or None
            Task->>RomDB: get_rom_user(rom.id, user.id)
            RomDB-->>Task: RomUser or None
            alt RomUser is None
                Task->>RomDB: add_rom_user(rom.id, user.id)
                RomDB-->>Task: new RomUser
            end
            alt status != new_status AND status not RETIRED/NEVER_PLAYING
                Task->>RomDB: update_rom_user(id, {status: new_status})
            end
        end
    end

_{Last reviewed commit: 7146081}