Repository navigation
fix: include habit ids in retrospective rankings - #694
Merged
thomasluizon merged 2 commits intoOct 3, 2026
Merged
Conversation
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes Reviewed all six changed files, the shared ranking consumers, and the orbit-ui-mobile contract for compatibility.
- Stable habit identity: Adds optional nullable
habitIdand populates it from the source habit for current and historical rankings, including chat insight cards and newly generated recaps. - Append-only contract: Preserves existing fields and callers, updates OpenAPI, and allows payloads without an id to remain readable; the client update is a subsequent API-first rollout step.
- Regression coverage: Checks exact ids for duplicate titles, serialized ids in both ranking lists, and legacy payload handling; also makes the achievement test fixture independent of the wall clock.
- Verification: All 87 focused retrospective, achievement, recap, and insight-card tests passed; the full solution build and suite were not rerun during this review.
gpt-6.1-sol | 𝕏
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
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.

Retrospective rankings now carry each habit's id, so the mobile client can open the correct habit even when titles match.
Refs thomasluizon/orbit-tickets#1154
Change and compatibility
src/Orbit.Application/Habits/Queries/GetRetrospectiveQuery.csappendsGuid? HabitId = nulltoRetrospectiveHabitStat.src/Orbit.Application/Habits/Services/RetrospectiveMetricsCalculator.cscopieshabit.IdinBuildHabitStat. The shared record also carries the id throughPeriodInsightCard.src/Orbit.Api/openapi.jsoncontains the optional nullable UUID property, generated through the normal API build path. Existing fields retain their names and types.tests/Orbit.Application.Tests/Services/RetrospectiveHabitStatTests.csverifies distinct ids for duplicate titles in current and historical calculations and their insight cards.tests/Orbit.Application.Tests/Queries/Habits/GetRetrospectiveQueryHandlerTests.cschecks serialized ids in both rankings and compatibility with a cached stat lacking the id.The optional final parameter preserves existing callers and readers. Repository references show the retrospective response is held in
IMemoryCachefor one hour; the chat card is built from the transient tool payload and returned in the response. No persisted retrospective-stat payload reader was found. Missing ids deserialize as null and remain readable by the insight card.Achievement test finding
AchievementProgressServiceTests.LoadAsync_WithHabits_MapsEveryCountToItsMetrichad one historical log whose creation timestamp came from the wall clock. Its five streak logs already had fixed noon timestamps. The unchanged test failed during this run because the extra log counted as early, producingEarlyLogs = 1against the existing zero assertion.In separate commit
a74c1aa5,tests/Orbit.Application.Tests/Gamification/AchievementProgressServiceTests.cssets that log'sCreatedAtUtcto noon UTC on its fixed log date, following the existing fixture pattern. Product code and every assertion are unchanged. This implements the ticket's continuation decision; the orchestrator carries the same test fix tomain.Test evidence
dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~LoadAsync_WithHabits_MapsEveryCountToItsMetric --verbosity minimalexited 1. One test failed: expectedEarlyLogszero, observed one.dotnet test tests/Orbit.Application.Tests --no-build --filter FullyQualifiedName~LoadAsync_WithHabits_MapsEveryCountToItsMetric --verbosity minimalexited 0; one test passed. The unchanged assertions already exposed the fixture defect, so no strengthened assertion was needed.env -u LANG dotnet build Orbit.slnx --verbosity minimalexited 0 with zero errors.env -u LANG dotnet test --verbosity minimalexited 0: 8,637 passed, zero failed, zero skipped across all four projects.env LC_ALL=en_US.UTF-8 dotnet build Orbit.slnx --verbosity minimalexited 0 with zero errors.env LC_ALL=en_US.UTF-8 dotnet test --verbosity minimalexited 0: 8,637 passed, zero failed, zero skipped across all four projects.This continuation preserves the already committed habit-id implementation and regression tests in
f4f37e3e. Pre-fix red/green observations for those retrospective tests were not available to this worker: the continuation began after that implementation was committed, and the order requires preserving it. Both full-suite runs include those tests; no unobserved pre-fix outcome is claimed.Manual steps
Release API(.github/workflows/release.yml), dispatch frommain, and setenvironment=stagingandbranch=redesign/main. A successful staging release and verification ofhabitIdin both retrospective rankings on staging prove the change is live. Keep the ticket open until that release is complete.