Fix date scoped completion for general habits - #456
Conversation
|
Approach for ORB-234: I will change the Habit domain behavior so general habits use dated logs as their completion state and never write or honor the permanent IsCompleted flag. One time tasks will retain the permanent flag and its log guard. Unlog will remove the requested dated log when present; for a general habit with a stale permanent flag and no log on the requested date, it will clear that stale flag so the checked state the client received can be undone. This keeps request date semantics inside the domain and avoids changing command signatures or API contracts. The implementation will land in src/Orbit.Domain/Entities/Habit.cs, the existing habit domain test file under tests/Orbit.Domain.Tests/Entities, and an EF migration plus model snapshot under src/Orbit.Infrastructure/Migrations. The migration will clear IsCompleted only where IsGeneral is true, preserving one time task completion and all habit log history. I chose this shape instead of adding a new response field or changing handlers because the defect is shared domain state and current clients already send a date to log and unlog. I also rejected deleting history or resetting every null frequency habit because those approaches could alter correct one time task state. |
|
Implementation refinement after the initial approach comment: The final domain path prioritizes an exact dated log during unlog, then uses the newest active general log only when there is no exact match. That compatibility fallback is required by the supplied cross date unlog regression. The update path also clears permanent completion when a completed one time task is converted into a general habit, preventing the same invalid state from being recreated after the cleanup migration. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8b7acd9c4
ℹ️ 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".
|
DEGRADED: same-vendor review Review verdict: 1 Blocking finding after round 1. F1 Critical at src/Orbit.Domain/Entities/Habit.cs:293: when the requested date has no log, Unlog falls back to a different active general-habit log and deletes another day's history. Frozen for one fixer round. Same-family bias is not eliminated and its magnitude is unmeasured. |
|
Fixer approach: remove the general-habit fallback in src/Orbit.Domain/Entities/Habit.cs so Unlog resolves only an active log whose Date matches the requested date. Extend tests/Orbit.Domain.Tests/Entities/HabitTests.cs with a two-date regression that proves a missing-date unlog fails without mutating either active log, then proves an existing-date unlog deletes only its matching log. This keeps the correction inside the aggregate's date-scoped invariant and avoids handler or contract changes, since widening command behavior would preserve the corruption path instead of fixing it. |
|
Fixer approach for the unresolved dated completion finding: I will make schedule response completion date aware only for general habits. The general only handler will obtain the user's current date, and both that path and the general habits appended to scheduled results will load and map active completion logs for that date. The shared mapper will use the dated result for top level and nested general habits while continuing to map Habit.IsCompleted for every non-general habit. The changes will land in src/Orbit.Application/Habits/Queries/GetHabitScheduleQuery.cs, src/Orbit.Application/Habits/Queries/HabitScheduleFilters.cs, and focused handler tests in tests/Orbit.Application.Tests/Queries/Habits/GetHabitScheduleQueryHandlerTests.cs. The regressions will cover a log today, a log only yesterday, a soft deleted log today, nested general children, appended general habits, and lifetime completion for a non-general one time task. This keeps the existing contract and domain semantics intact while correcting the response projection at its source. |
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0bf010a91
ℹ️ 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".
|
Current head fix approach: add one handler regression for GetHabitById and one for GetHabitFullDetail at their existing MediatR query seams. Each test will pin a general habit with an active log on the user current date as completed while retaining a non-general lifetime-completion control. Then update only the two query mappings to obtain today through IUserDateService and select completion by habit type. This keeps the response contract unchanged and avoids moving date-scoped behavior into the entity flag. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f08d10af8f
ℹ️ 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".
|
Current head fixer approach for the two frozen findings:
After the focused tests, I will regenerate and check architecture drift, run the solution build and complete test solution, run git diff --check, then stage named files only, commit, and push this branch. |
|




DEGRADED: same-vendor review
Fixes ORB-234.
Summary
Verification
ORB-128 evidence
A repository search for ORB-128, three dot menu references, and completed habit menu references returned no reproduction for ORB-128. This pull request therefore does not claim that ORB-128 turned green as a side effect. Its separate reproduction remains unverified.
External interfaces
The implementation reads no new external field, response shape, flag, or exit code.