fix(sync): preserve personal record tombstone snapshots

[9thLevelSoftware]

Follow-up correction for Issue #655

Fixes #655.

The merged implementation correctly introduced Personal Record tombstone sync, but post-merge review identified two correctness defects and one privacy defect:

• A portal tombstone payload may contain placeholder performance values. Applying the deletion now updates only deletion/LWW state and preserves the last active record snapshot; a newer active restore still replaces the full snapshot.
• A deleted Personal Record no longer acts as the active PR comparison baseline. A later valid PR can replace a deleted outlier while retaining that record's stable sync UUID.
• Delete failures now always show the localized generic failure message rather than raw backend exception text.

Regression coverage

• New lower valid PR after deleted outlier retains UUID and becomes active.
• Placeholder tombstone metrics preserve the active snapshot and an older active replay cannot resurrect the tombstone.
• Structural UI test rejects raw exception text in the delete failure message.

Validation

• ./gradlew :shared:testAndroidHostTest --no-daemon -Pskip.supabase.check=true
• ./gradlew :androidApp:assembleDebug --no-daemon -Pskip.supabase.check=true
• Regression tests fail on the pre-correction origin/main baseline and pass with this patch.

Visual evidence

This correction does not alter the successful Personal Records interaction or layout; the existing built-app evidence from the merged implementation remains current:

• Delete confirmation
• Post-delete list

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 56d1c17b91

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Recompute stored 1RM when reviving a deleted PR

When the deleted row was an outlier that previously raised Exercise.one_rep_max_kg, this new filtering makes a later lower lift count as a new active PR, but the same method still only updates the exercise 1RM when the new estimate is greater than the stored value. In that scenario the active PR is replaced with the lower valid lift while ESTIMATED_1RM / percentage-based weight paths keep using the deleted outlier’s stale 1RM, so deleting an erroneous max still leaves generated weights too high.

Useful? React with 👍 / 👎.

[kilo-code-bot]

Code Review Roast 🔥

Verdict: No Issues Found | Recommendation: Merge

Oh wait, this PR is actually clean. I had my flamethrower warmed up for the Privacy SNAFU and the tombstone-overwrite smell, and you came back with the smallest, tightest fix I've seen in a while. I'm suspicious.

The tombstone-aware CASE WHEN :deletedAt IS NULL in applyPulledPersonalRecordTombstoneState is the right call — portal tombstones carrying placeholder metrics used to silently nuke the active snapshot. Now they leave it alone, and an equal-time tombstone still wins via the LWW guard. The storedWeightPR vs currentWeightPR split in SqlDelightPersonalRecordRepository is the laziest way to express "tombstone's UUID, active baseline for comparison" — I was about to suggest extracting a helper, but two repetitions of selectPRIncludingDeleted().executeAsOneOrNull() plus a takeIf { it.deletedAt == null } doesn't deserve its own function. Leave it.

The privacy fix in AnalyticsScreen.kt is the part where I'd normally reach for the flamethrower — showing raw error.message from a backend exception to end users is a classic "we'll fix it later" debt — and you fixed it. Localized string, parameter explicitly ignored with _, structural test asserts the absence. Good.

Correctness: No findings. The LWW guard, UUID retention via storedWeightPR?.uuid, and the active-restore path in mergePersonalRecords lets a newer active update restore a tombstoned row all hold up.

Ponytail: Lean already. Ship. The diff is +64/-22 across 6 files, but almost all of it is required SQL CASE logic, tests, and one well-justified comment per site. Nothing to delete.

🏆 Best part: The test mergePersonalRecords keeps a newer tombstone and rejects stale active replay switched from a magic 99.17f to weight * 1.1667f and added assertions on the preserved weight/volume. The test now documents the invariant instead of just the constants.

💀 Worst part: Honestly nothing. The two-line comment at SqlDelightPersonalRecordRepository.kt:370-371 arguably restates :299-300, but it's load-bearing — it stops a future maintainer from "fixing" storedWeightPR?.uuid back to currentWeightPR?.uuid and breaking sync identity. Keep it.

📊 Overall: Like a re-run of a production incident postmortem — short, focused, and the tests prove the regression won't return. Merging.

---

Fix these issues in Kilo Cloud

Files Reviewed (6 files)

• shared/src/commonMain/kotlin/com/devil/phoenixproject/data/repository/SqlDelightPersonalRecordRepository.kt — stored vs current PR split, UUID retention
• shared/src/commonMain/sqldelight/com/devil/phoenixproject/database/VitruvianDatabase.sq — tombstone CASE WHEN preservation
• shared/src/commonMain/kotlin/com/devil/phoenixproject/presentation/screen/AnalyticsScreen.kt — privacy fix on delete failure
• shared/src/androidHostTest/kotlin/com/devil/phoenixproject/data/repository/SqlDelightPersonalRecordRepositoryTest.kt — replacement-after-deletion regression
• shared/src/androidHostTest/kotlin/com/devil/phoenixproject/data/repository/SqlDelightSyncRepositoryTest.kt — tombstone preservation + stale-replay rejection
• shared/src/commonTest/kotlin/com/devil/phoenixproject/presentation/screen/AnalyticsPersonalRecordDeleteReviewTest.kt — structural UI test for privacy

---

_{Reviewed by minimax-m3 · Input: 36.6K · Output: 20.7K · Cached: 852.9K}