fix(api): idempotent habit logging via partial unique index - #196
Merged
Conversation
Concurrent or replayed POST /habits/{id}/log for the same habit+date could
create duplicate completion rows. Add a partial unique index on
HabitLogs(HabitId, Date) WHERE "Value" > 0 (one completion per habit+date)
plus a Value>0-scoped dedupe migration step, and treat a unique-violation on
insert as an idempotent "already logged" success instead of an error.
The partial filter is deliberate: flexible-habit skips share the table at
Value=0 and must coexist with a completion, and log -> unlog (row delete) ->
log must keep working. The existing non-unique index is retained for lookups;
the sequential re-tap toggle (unlog) is unchanged.
Refs thomasluizon/orbit-ui-mobile#150
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
Paired frontend PR: thomasluizon/orbit-ui-mobile#152 (Closes #149/#150/#151). Squash-merge this API PR first so the idempotency contract is live before the client consumes it. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Two issues to fix before merge:
-
Correctness bug — infinite recursion in
IsUniqueViolation(LogHabitCommand.cs:200): the?? dbUpdateExceptionnull-coalescing fallback recurses back into the sameDbUpdateExceptionarm instead of returningfalse, causing aStackOverflowExceptionon anyDbUpdateExceptionwithout an inner exception. The fix is a one-liner: guard withdbUpdateException.InnerException is not null &&. -
Hard-rule violation — empty
catch {}(HabitLogIdempotencyTests.cs:46): CLAUDE.md prohibits empty catch blocks everywhere. In a sequential-collection fixture, a silent teardown failure can corrupt the shared DB state for subsequent tests.
…rrors Addresses review on PR #196: - IsUniqueViolation: a DbUpdateException with no InnerException recursed on itself via the `?? dbUpdateException` fallback (StackOverflow on any future DbUpdateException without an inner). Guard on `InnerException is not null`. - HabitLogIdempotencyTests.DisposeAsync: replace the empty catch with try/finally so cleanup errors surface while the client is still disposed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Both issues from the prior review are correctly addressed: the IsUniqueViolation recursion guard (dbUpdateException.InnerException is not null &&) prevents the StackOverflowException path, and the test teardown now uses try/finally so cleanup errors surface.
The rest of the implementation is solid: the partial unique index is correctly modelled in EF Core (HasFilter("\"Value\" > 0").IsUnique()) and matches the migration SQL; the dedupe step in Up() keeps the newest row (a.Id < b.Id); BuildAlreadyLoggedResultAsync correctly skips streak recalculation and gamification on the idempotent path; the LogHabitResponse shape is unchanged so there is no API contract break; and unit + integration tests cover the concurrent-log, toggle-preserved, and migration-on-dirty-data paths. Clean merge.
|
thomasluizon
added a commit
that referenced
this pull request
Jun 30, 2026
#196) (#268) * feat(api): achievement-system expansion + free-tier XP earning (#196) - Append Social/Sharing/Together achievement categories - Define 10 achievements: FirstFriend, SquadGoals, Cheerleader, ShowOff, YearInReview, TeamPlayer, MissionAccomplished, BattleBuddy (defined for siblings #197-#201) + StreakImmortal/Unstoppable (wired on streak/volume) - Add idempotent IGamificationService.TryGrantAchievementsAsync grant funnel - Add POST /api/achievements/report-event (auth, whitelisted keys: card_shared->ShowOff, wrapped_viewed->YearInReview); no Pro gate - A1: free users earn habit-log + goal XP and level up (achievements stay Pro-gated) behind the gamification_free_tier flag predicate - Data-only migration enabling gamification_free_tier - Portuguese notification strings for the 10 new achievements - Inventory report-event in the agent capability catalog Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(api): pin Microsoft.OpenApi to patched 2.7.5 (GHSA-v5pm-xwqc-g5wc) The Dependency Scan flags a high-severity advisory: Microsoft.AspNetCore.OpenApi 10.0.9 transitively resolves the vulnerable Microsoft.OpenApi 2.0.0 ("circular schema references may terminate OpenAPI parsing"). Pin a direct reference to the patched 2.7.5. This affects main and every open branch, not just this PR; it rides on the foundation PR so it lands on main first. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 30, 2026
thomasluizon
added a commit
that referenced
this pull request
Jun 30, 2026
…t BattleBuddy now #196 ships Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon
added a commit
that referenced
this pull request
Jun 30, 2026
* feat(api): 1:1 accountability buddies (#201) Adds a 1:1 accountability-buddy system on top of the #193 social foundation: invite -> accept between two accepted friends, per-user linked-habit joins, moderated check-ins that push the buddy, a Daily/Weekly cadence, multiple pairs per user up to a cap, and either-party revoke. - Domain: AccountabilityPair / AccountabilityPairHabit / AccountabilityCheckIn entities + AccountabilityPairStatus / AccountabilityCadence enums + guards. - Application: AccountabilityPairService, 6 commands + 2 queries, FluentValidation, no-N+1 pairs query. - API: AccountabilityController (7 routes) + accountability-invites / accountability-checkins rate-limit policies. - Persistence: AddAccountabilityBuddies migration (3 tables, unique (PairId,UserId,HabitId) and (PairId,UserId,Date) indexes). - Battle Buddy award wired on pair-accept via the "battle_buddy" string id; no-ops until #196 ships the achievement definition. - Registered the new controller actions in the agent catalog. - Tests: domain entity guards + handler/validator/query unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(api): integrate main, rescaffold AddAccountabilityBuddies, grant BattleBuddy now #196 ships Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon
added a commit
that referenced
this pull request
Jun 30, 2026
…r/MissionAccomplished now #196 ships Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon
added a commit
that referenced
this pull request
Jun 30, 2026
* feat(api): cooperative challenges domain (#200) Add a cooperative Challenges domain: Challenge + ChallengeParticipant + ChallengeParticipantHabit entities (no per-user ranking), CoopGoal (summed target over a window) and StreakTogether (strict all-must-log shared streak) types, create/join/leave/detail endpoints with FluentValidation, friend-invite auto-join + Crockford-base32 join code, and read-side shared-progress aggregation mirroring HabitMetricsCalculator. Completion + Mission Accomplished fire from a post-log seam (IChallengeProgressService); Team Player on join. Team Player / Mission Accomplished award via the conventional string ids passed to AchievementChecks.TryGrant, which no-op until #196 ships the definitions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(api): integrate main, rescaffold AddChallenges, grant TeamPlayer/MissionAccomplished now #196 ships Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
thomasluizon
added a commit
that referenced
this pull request
Jun 30, 2026
…) (#273) * feat(api): general cheer + social award triggers + profile handle (#199) Relax Cheer.HabitId to optional so cheers are general encouragement, not habit-specific: nullable entity property + factory, optional command/body/DTO, validator drops the NotEmpty rule, EF config switches the habit FK to IsRequired(false) + OnDelete SetNull, and a RelaxCheerHabitId migration alters the column and FK. SendCheer skips the habit-ownership check when no habitId is supplied. Surface Handle + SocialOptIn on ProfileResponse so the client can render a first-run opt-in gate instead of a raw 403. Wire the three social-achievement award triggers via the existing TryGrant funnel using conventional achievement ids ("first_friend", "squad_goals", "cheerleader"): First Friend + Squad Goals (at 5) award BOTH participants on accept; Cheerleader on the 10th sent cheer. These stay dormant until the definitions ship in #196, then activate automatically. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(api): integrate main, rescaffold RelaxCheerHabitId, merge profile response fields Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
What
Makes
POST /habits/{id}/logidempotent under concurrent/duplicate same-habit+date requests.IX_HabitLogs_HabitId_Date_CompletedonHabitLogs(HabitId, Date)WHERE "Value" > 0(one completion per habit+date), plus aValue>0-scoped dedupe step in the migration (keep newest).IsFirstCompletionToday: false), never flipped into an unlog.Why partial (not a plain unique index)
Flexible-habit skips share the
HabitLogstable atValue=0and must coexist with aValue=1completion on the same date; andlog → unlog (row delete) → logmust keep working. A blanketUNIQUE(HabitId, Date)would break both. The existing non-unique index is retained for lookups; the sequential re-tap toggle (unlog) is untouched.Migration
20260608175140_HabitLogCompletionUniqueIndex— not yet applied to any environment. The dedupe step only collapses duplicateValue>0rows; skip rows are preserved.Tests
POST /log→ one completion row + both "completed"; migration applies on duplicate-seeded data with a coexisting skip.dotnet build+ 17 unit + 3 integration green.Refs thomasluizon/orbit-ui-mobile#150 · paired frontend PR linked below.
🤖 Generated with Claude Code