Skip to content

fix: include habit ids in retrospective rankings - #694

Merged
thomasluizon merged 2 commits into
redesign/mainfrom
fix/ticket-1154-retrospective-habit-id
Oct 3, 2026
Merged

thomasluizon merged 2 commits into
redesign/mainfrom
fix/ticket-1154-retrospective-habit-id

Conversation

@thomasluizon

Copy link
Copy Markdown
Owner

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.cs appends Guid? HabitId = null to RetrospectiveHabitStat.
  • src/Orbit.Application/Habits/Services/RetrospectiveMetricsCalculator.cs copies habit.Id in BuildHabitStat. The shared record also carries the id through PeriodInsightCard.
  • src/Orbit.Api/openapi.json contains 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.cs verifies distinct ids for duplicate titles in current and historical calculations and their insight cards. tests/Orbit.Application.Tests/Queries/Habits/GetRetrospectiveQueryHandlerTests.cs checks 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 IMemoryCache for 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_MapsEveryCountToItsMetric had 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, producing EarlyLogs = 1 against the existing zero assertion.

In separate commit a74c1aa5, tests/Orbit.Application.Tests/Gamification/AchievementProgressServiceTests.cs sets that log's CreatedAtUtc to 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 to main.

Test evidence

  • Before the timestamp fix: dotnet test tests/Orbit.Application.Tests --filter FullyQualifiedName~LoadAsync_WithHabits_MapsEveryCountToItsMetric --verbosity minimal exited 1. One test failed: expected EarlyLogs zero, observed one.
  • After the fix and build: dotnet test tests/Orbit.Application.Tests --no-build --filter FullyQualifiedName~LoadAsync_WithHabits_MapsEveryCountToItsMetric --verbosity minimal exited 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 minimal exited 0 with zero errors.
  • env -u LANG dotnet test --verbosity minimal exited 0: 8,637 passed, zero failed, zero skipped across all four projects.
  • env LC_ALL=en_US.UTF-8 dotnet build Orbit.slnx --verbosity minimal exited 0 with zero errors.
  • env LC_ALL=en_US.UTF-8 dotnet test --verbosity minimal exited 0: 8,637 passed, zero failed, zero skipped across all four projects.
  • The builds preserve the committed OpenAPI snapshot. Diff whitespace, dash, timeless-text checks and commit hooks passed. Builds retain existing dependency and obsolete-API warnings.

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

  • After merge, the orchestrator releases the staging API before the UI follow-up merges. In GitHub Actions, open Release API (.github/workflows/release.yml), dispatch from main, and set environment=staging and branch=redesign/main. A successful staging release and verification of habitId in both retrospective rankings on staging prove the change is live. Keep the ticket open until that release is complete.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 habitId and 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.

Pullfrog  | View workflow run | Using gpt-6.1-sol | 𝕏

@thomasluizon
thomasluizon merged commit d29aca2 into redesign/main Oct 3, 2026
22 checks passed
@thomasluizon
thomasluizon deleted the fix/ticket-1154-retrospective-habit-id branch October 3, 2026 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant