Skip to content

Fix Today completed habit filtering (#547) - #972

Merged
thomasluizon merged 8 commits into
mainfrom
fix/ticket-547-completed-today
Sep 15, 2026
Merged

thomasluizon merged 8 commits into
mainfrom
fix/ticket-547-completed-today

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Sep 15, 2026 •

Copy link
Copy Markdown
Owner

Fixes thomasluizon/orbit-tickets#547

Summary

  • Keep the Today view filtered when completed items are shown.
  • Use isLoggedInRange as the selected-date completion signal for top-level habits and descendants.
  • Preserve All and general behavior, parent recursion, and recentlyCompletedIds.
  • buildCommandHabitList is correct. It always uses showCompleted: false, then excludes completed one-time tasks from its All fallback.

Test evidence

Before implementation, the existing focused tests passed with the defect present:

  • npm run test --workspace=@orbit/shared -- --run src/__tests__/habit-visibility.test.ts: 19 passed.
  • npm run test --workspace=@orbit/web -- --run __tests__/components/habits/habit-list.test.tsx: 32 passed.
  • npm run test --workspace=@orbit/mobile -- --run __tests__/components/habits/habit-list.test.tsx: 37 passed.

With final regression tests added and the defect still present:

  • Shared: 2 failed and 19 passed. Historical completion was visible, and its stale child was returned.
  • Web: 2 failed and 32 passed. Both Today tests rendered completed-earlier.
  • Mobile: 2 failed and 37 passed. Both Today lists included completed-earlier.

After the fix:

  • Shared focused tests: 21 passed.
  • Web focused tests: 34 passed.
  • Mobile focused tests: 39 passed.
  • npm run type-check: 3 tasks passed.
  • npm test: 5,607 tests passed across four workspaces.
  • Pre-commit lint and repository gates passed.

External interface evidence

Both Today models send the selected date as identical dateFrom and dateTo values. The API maps IsLoggedInRange using positive, nondeleted logs inside that inclusive range. Completed habits return no instances, so selected-date completion must use isLoggedInRange.

Complete HabitScheduleItem key set and types:

Id: Guid
Title: string
Description: string?
FrequencyUnit: FrequencyUnit?
FrequencyQuantity: int?
IsBadHabit: bool
IsCompleted: bool
IsGeneral: bool
IsFlexible: bool
Days: IReadOnlyList<DayOfWeek>
Position: int?
CreatedAtUtc: DateTime
DueDate: DateOnly
DueTime: TimeOnly?
DueEndTime: TimeOnly?
EndDate: DateOnly?
ScheduledDates: IReadOnlyList<DateOnly>
IsOverdue: bool
ReminderEnabled: bool
ReminderTimes: IReadOnlyList<int>
ScheduledReminders: IReadOnlyList<ScheduledReminderTime>
SlipAlertEnabled: bool
ChecklistItems: IReadOnlyList<ChecklistItem>
Tags: IReadOnlyList<HabitTagItem>
LinkedGoals: IReadOnlyList<LinkedGoalDto>
Children: IReadOnlyList<HabitScheduleChildItem>
HasSubHabits: bool
FlexibleTarget: int?
FlexibleCompleted: int?
IsLoggedInRange: bool
Instances: IReadOnlyList<HabitInstanceItem>
SearchMatches: IReadOnlyList<SearchMatchField>?
Emoji: string?
IntervalWeeks: int?

Complete HabitScheduleChildItem key set and types:

Id: Guid
Title: string
Description: string?
FrequencyUnit: FrequencyUnit?
FrequencyQuantity: int?
IsBadHabit: bool
IsCompleted: bool
IsGeneral: bool
IsFlexible: bool
Days: IReadOnlyList<DayOfWeek>
DueDate: DateOnly
DueTime: TimeOnly?
DueEndTime: TimeOnly?
EndDate: DateOnly?
ScheduledDates: IReadOnlyList<DateOnly>
IsOverdue: bool
Position: int?
ChecklistItems: IReadOnlyList<ChecklistItem>
Tags: IReadOnlyList<HabitTagItem>
Children: IReadOnlyList<HabitScheduleChildItem>
HasSubHabits: bool
FlexibleTarget: int?
FlexibleCompleted: int?
IsLoggedInRange: bool
Instances: IReadOnlyList<HabitInstanceItem>
SearchMatches: IReadOnlyList<SearchMatchField>?
Emoji: string?
IntervalWeeks: int?

Re-derive the shapes at orbit-api/src/Orbit.Application/Habits/Queries/GetHabitScheduleQuery.cs:17 and :53. Re-derive the field mapping at HabitScheduleFilters.cs:284 and :447, the log predicate at HabitScheduleService.cs:398, and empty completed instances at HabitScheduleService.cs:535.

Implementation notes

The first mobile showCompleted branch served Today and caused the defect. The second served the implicit general fallback. It now remains only in an explicit general return, preserving general behavior.

Review round: completed general habits

Finding 1 was accepted. AppendGeneralHabits sets UserToday without DateFrom or DateTo, so IsLoggedInRange stays false. GetResponseCompletion instead maps a general habit's selected-date completion into IsCompleted. The shared Today predicate now preserves that signal for top-level and nested general habits.

API evidence: orbit-api/src/Orbit.Application/Habits/Queries/GetHabitScheduleQuery.cs:408-439 and HabitScheduleFilters.cs:275-286,447-492.

Red test evidence

Captured before the implementation change in %TEMP%\pr-972-finding-1-red.txt:

npm test --workspace=@orbit/shared -- src/__tests__/habit-visibility.test.ts
 ❯ src/__tests__/habit-visibility.test.ts (23 tests | 2 failed)
     × shows a completed top-level general habit only when completed items are enabled
     × shows a completed general descendant only when completed items are enabled
 Test Files  1 failed (1)
      Tests  2 failed | 21 passed (23)

After the fix, the shared test file passes 23 tests. The web component file passes 34 tests. The mobile component file passes 39 tests. Root npm run type-check, full npm run lint, and the Suppressions Ratchet pass.

Assumptions

  • No findings were rejected.
  • For general habits returned through includeGeneral, isCompleted is selected-date state, not sticky lifetime state.
  • Non-general lifetime isCompleted remains excluded from Today unless isLoggedInRange or a completed selected-date instance proves completion.

Coverage round after main sync

Merged origin/main as ec126a0d. Its 39 generated contract lines remain unchanged.

Added focused coverage for the shared general-child branch and mobile general top-level branch.

Red test evidence

With the shared general-view branch replaced by return children:

npm test --workspace=@orbit/shared -- src/__tests__/habit-visibility.test.ts -t "applies completion visibility to children in the general view"
AssertionError: expected [ 'active', 'completed', …(1) ] to deeply equal [ 'active', 'recently-completed' ]
Test Files  1 failed (1)
     Tests  1 failed | 23 skipped (24)

With the mobile general-view branch replaced by return topLevelHabits:

npm test --workspace=@orbit/mobile -- __tests__/components/habits/habit-list.test.tsx -t "hides completed general habits unless they were just completed"
AssertionError: expected [ 'active', 'completed', …(1) ] to deeply equal [ 'active', 'recently-completed' ]
Test Files  1 failed (1)
     Tests  1 failed | 39 skipped (40)

After restoring production code, both focused tests passed.

Root verification on 85ceaeec:

  • npm test: 5,611 tests passed across four workspaces.
  • npm run type-check: 3 tasks passed.
  • npm run lint: 3 tasks passed with existing warnings and no errors.
  • node tools/check-suppressions-ratchet.mjs: web stayed 623 and mobile stayed 625.

Review round: rendered bulk selection

Finding accepted. The mobile screen treated a valid empty HabitList result as missing, then rebuilt visibility with a stale show-completed shortcut. HabitList now remains the single visibility source. The screen represents no report as null and preserves a reported empty Set.

Web parity does not carry the defect. apps/web/app/(app)/use-today-selection.ts:66 reads HabitList allLoadedIds directly. An empty Set is truthy and remains authoritative. Only a missing ref falls back to habitsById.

Red test evidence

Against 3b2389e, with show completed enabled and only historical-completion returned:

npm test --workspace=@orbit/mobile -- __tests__/screens/today-screen.test.tsx -t "keeps hidden historical completions out of bulk selection"
AssertionError: expected vi.fn() to be called with arguments: [ [] ]
Received:
  1st vi.fn() call:
  [
    [
      "historical-completion",
    ],
  ]
Test Files  1 failed (1)
     Tests  1 failed | 27 skipped (28)

After the fix, the focused regression passed. The full Today screen file passed 28 tests, and the selection hook file passed 11 tests.

Root verification on 1427745:

  • npm test: 4 workspace tasks passed.
  • npm run type-check: all TypeScript workspaces passed.
  • npm run lint: 3 lint tasks passed with existing warnings and no errors.
  • node tools/check-suppressions-ratchet.mjs: web stayed 623 and mobile stayed 625.

Final main sync

Merged origin/main at e5d4044 as cc6e484. The incoming habit row changes remain unchanged.

Verification after the merge:

  • Focused mobile screen and selection tests: 39 passed.
  • npm run type-check: 3 tasks passed.
  • npm run lint: 3 tasks passed with existing warnings and no errors.
  • node tools/check-suppressions-ratchet.mjs: web stayed 623 and mobile stayed 625.

@vercel

vercel Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
orbit-ui-mobile-web Ignored Ignored Sep 15, 2026 7:28pm UTC

Request Review

@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.

Important

Completed general habits are filtered out of Today even when “Show completed” is enabled.

Reviewed changes I reviewed the complete six-file patch and its interaction with the API’s selected-date completion mapping.

  • Today filtering — Web and mobile now route Today through the shared recursive visibility predicate instead of returning every top-level habit when completed items are shown.
  • Completion semantics — Shared visibility now uses selected-range logs rather than sticky lifetime completion while preserving the explicit All and General view branches.
  • Regression coverage — Shared and mirrored component tests distinguish selected-day completions from historical completions, with the mobile harness delegating translations to the real locale bundle.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using GPT Sol | 𝕏

Comment thread packages/shared/src/utils/habit-visibility.ts Outdated
pullfrog[bot]
pullfrog Bot previously approved these changes Sep 15, 2026

@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 I reviewed the general-habit completion fix added since the prior Pullfrog review and rechecked it in the context of the complete six-file patch.

  • Preserved general-habit completion — Limited the isCompleted fallback to general habits, matching the API's date-scoped mapping without restoring lifetime completion for scheduled habits.
  • Added regression coverage — Covered completed top-level and descendant general habits with completed items both hidden and shown.

Pullfrog  | View workflow run | Using GPT Sol | 𝕏

@thomasluizon

Copy link
Copy Markdown
Owner Author

@pullfrog review

@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 I reviewed the General-view regression coverage and base synchronization added since the prior Pullfrog review, then rechecked them against the complete six-file patch.

  • Covered mobile top-level filtering — Added an exact rendered-ID assertion proving General view hides stale completions while retaining active and recently completed habits.
  • Covered shared descendant filtering — Added exact child-list assertions for both showCompleted states, including the recentlyCompletedIds exception.
  • Synchronized main — Merged the latest base without introducing an additional net behavioral change in the incremental diff.

Pullfrog  | View workflow run | Using GPT Sol | 𝕏

@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 I reviewed the complete six-file patch, its resolved general-habit follow-up, and the selected-date completion contract in orbit-api.

  • Selected-day visibility — Today now routes top-level habits and descendants through the shared recursive predicate, keeping historical completions hidden while retaining selected-day completions.
  • General and list behavior — General habits preserve the API's date-scoped isCompleted signal, while All and General filtering plus recentlyCompletedIds remain intact across web and mobile.
  • Regression coverage — Shared, web, and mobile tests use exact outcomes for historical one-time completions, selected-day completions, general habits, and descendants; all three focused suites pass.

Pullfrog  | View workflow run | Using GPT Sol | 𝕏

@thomasluizon

Copy link
Copy Markdown
Owner Author

@pullfrog review

@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.

Caution

The mobile bulk-selection path can target hidden historical habits when the corrected Today list is empty, allowing destructive actions against rows the user cannot see.

Reviewed changes I reviewed the complete six-file patch, the resolved general-habit follow-up, both platform integrations, and the selected-date completion contract in orbit-api.

  • Selected-day visibility — Today now routes top-level habits and descendants through the shared recursive predicate, retaining selected-day completions while excluding historical completions.
  • General and list behavior — General habits preserve the API's date-scoped isCompleted signal, while All and General filtering plus recentlyCompletedIds remain intact.
  • Regression coverage — Shared, web, and mobile tests assert exact outcomes for historical one-time completions, selected-day completions, general habits, and descendants.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using GPT Sol | 𝕏

Comment thread apps/mobile/components/habit-list.tsx

@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 I reviewed the mobile bulk-selection correction and typed test-harness follow-up added since the prior Pullfrog review, then rechecked them in the context of the complete ten-file patch.

  • Made rendered IDs authoritative — Removed the duplicated screen-level visibility fallback and distinguished an unreported list (null) from a valid empty rendered-ID set.
  • Covered the empty rendered list — Added exact hook and screen regressions proving Select all excludes hidden historical completions and leaves bulk actions inactive.
  • Restored typed verification — Gave the synchronously assigned test renderer a definite-assignment type so the mobile type-check passes without changing runtime behavior.

Pullfrog  | View workflow run | Using GPT Sol | 𝕏

@sonarqubecloud

Copy link
Copy Markdown

@thomasluizon

Copy link
Copy Markdown
Owner Author

@pullfrog review

1 similar comment
@thomasluizon

Copy link
Copy Markdown
Owner Author

@pullfrog review

@pullfrog

pullfrog Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
  • Check out PR 972 and read every authoritative diff range
  • Trace changed behavior, tests, callers, and cross-repo contracts
  • Validate actionable findings and submit exactly one PR review

Pullfrog  | View workflow run | via Pullfrog | Using GPT Sol | 𝕏

@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 I reviewed the complete ten-file patch, its resolved follow-up fixes, the Android bulk-selection seam, and the selected-date completion contract in orbit-api.

  • Selected-date visibility — The shared predicate distinguishes range-scoped completion from sticky lifetime completion while preserving the API's date-scoped isCompleted signal for included general habits.
  • Mirrored list behavior — Web and mobile both route Today top-level habits and descendants through the shared recursive predicate without changing All, General, search, overdue, or recentlyCompletedIds behavior.
  • Rendered bulk selection — Mobile now treats HabitList's visible ID set as authoritative, including a valid empty set, so hidden historical completions cannot enter bulk actions.
  • Regression coverage — Exact shared, web, and mobile assertions cover historical completions, selected-day completions, general habits, descendants, and empty rendered selection; the focused suites pass.

Pullfrog  | View workflow run | Using GPT Sol | 𝕏

@thomasluizon
thomasluizon merged commit 81f5d61 into main Sep 15, 2026
38 checks passed
@thomasluizon
thomasluizon deleted the fix/ticket-547-completed-today branch September 15, 2026 19:43
thomasluizon added a commit that referenced this pull request Sep 16, 2026
* fix(mobile): clear Play large screen warnings (#977)

* fix(mobile): support edge-to-edge large screens

* test(mobile): assert Today content column exists

* fix(mobile): isolate habit menu press target (#971)

* fix(mobile): isolate habit menu press target

* fix(mobile): preserve habit row press padding

* test(mobile): cover habit row press behavior

* test(mobile): assert habit row pressed state

* fix(mobile): preserve select row press area

* Fix Today completed habit filtering (#547) (#972)

* fix: scope completed habits to selected day

* fix: preserve completed general habits on today

* test: cover completed general habit visibility

* fix(mobile): restrict bulk selection to rendered habits

* test(mobile): keep selection harness typed

* fix(mobile): preserve toast edge gap

* fix: stop mobile offline sync spin (#552) (#973)

* fix: bound mobile offline sync retries (#552)

* fix: hydrate connectivity before offline replay (#552)

* fix: back off rejected offline flushes (#552)

* fix: settle offline retry timer failures (#552)

* fix: unify offline replay state (#552)

* fix: retain offline retry timer ownership (#552)

* chore: triage post-redesign backlog fixes (#554) (#978)

* chore: port reachable backlog fixes

* test: align backlog port coverage

* test: remove obsolete gradient assertion

* fix: resolve backlog port review findings

* feat: suggest habit emoji without changing form (#976)

* feat: suggest habit emoji without changing form

* fix: isolate emoji suggestion pending state

* fix: bind habit suggestions to form sessions

* fix: invalidate habit suggestions on unmount

* test: cover habit suggestion lifecycle

* fix: use Astra marker for habit suggestions

* fix(mobile): keep auth-stopped replay closed

* fix(web): refresh sessions silently (#974)

* fix(web): refresh sessions silently

* test(web): align refresh failure expectation

* fix(web): preserve retryable refreshed sessions

* fix(web): expose dedicated refresh failures

* fix(web): propagate terminal refresh failures

* test(web): exercise cross-instance refresh race

* test(shared): lock session copy split

* test(web): target wrapped account actions

* fix(web): confirm terminal session state

* fix(web): await session revalidation

* fix(web): recover inverse refresh races

* fix(web): recover successful server actions

* test(web): cover session-aware action boundary

* fix(web): route streak repair through the session boundary

repairStreakGap came from the raw Server Action module and was unwrapped by
hand, so a confirmed refresh rejection never reached the auth store.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(web): stop inner catches swallowing session refresh failures

confirmApiKeyCreationChallenge and confirmDeletion caught every error
before wrapServerAction could read sessionRefreshFailed. Both rethrow it
now. The habit emoji selector also honours isDisabled on both platforms.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test(mobile): await retry persistence report

* test(mobile): await retry error without fake timers

* test(mobile): stabilize retry error capture

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
thomasluizon added a commit that referenced this pull request Oct 25, 2026
…1740 plan posted and #972 and #1734 closed

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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