Skip to content

Fix date scoped completion for general habits - #456

Open
thomasluizon wants to merge 7 commits into
mainfrom
fix/orb-234-general-habit-undo
Open

Fix date scoped completion for general habits#456
thomasluizon wants to merge 7 commits into
mainfrom
fix/orb-234-general-habit-undo

Conversation

@thomasluizon

@thomasluizon thomasluizon commented Aug 7, 2026

Copy link
Copy Markdown
Owner

DEGRADED: same-vendor review

Fixes ORB-234.

Summary

  • General habits now keep completion in dated logs and no longer write or honor the permanent IsCompleted flag.
  • General unlog deletes only an active log on the requested date and fails without changing other dates when that date has no active log.
  • One time tasks still set permanent completion and reject later logs.
  • Converting a completed one time task into a general habit clears the permanent flag.
  • A reversible data migration clears existing general habit completion flags while preserving all logs and all one time task state. Its rollback restores the old flag only for general habits that still have an active positive log.
  • Habit schedule, by-id, and full-detail responses derive general habit completion from an active log on the user's current date, including nested descendants. Non-general habits retain lifetime completion.
  • The query_habits assistant tool applies the same current-date rule to is_completed filtering and completion labels, including child labels.
  • The existing isCompleted response field and all API contracts remain unchanged.

Verification

  • dotnet build Orbit.slnx: succeeded with 0 errors.
  • dotnet test --no-restore --no-build: 5,614 passed, 0 failed.
  • The three current-head regressions failed against f08d10a before implementation.
  • Four focused current-head regressions pass for detail descendants and chat query filtering and labels.
  • The three touched consumer suites pass: 51 passed, 0 failed.
  • node tools/arch-map.mjs: regenerated the architecture map with no tracked drift.
  • dotnet format verification, the dash guard, the narration comment check, and git diff --check pass.
  • The combined pull request stays within the approved cap at 13 files and 613 inserted lines.

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.

@thomasluizon

Copy link
Copy Markdown
Owner Author

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.

@thomasluizon

Copy link
Copy Markdown
Owner Author

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.

@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 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".

Comment thread src/Orbit.Domain/Entities/Habit.cs
@thomasluizon

Copy link
Copy Markdown
Owner Author

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.

@thomasluizon

Copy link
Copy Markdown
Owner Author

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.

@thomasluizon

Copy link
Copy Markdown
Owner Author

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.

@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

1 similar comment
@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 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".

Comment thread src/Orbit.Domain/Entities/Habit.cs
@thomasluizon

Copy link
Copy Markdown
Owner Author

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.

@thomasluizon

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 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".

Comment thread src/Orbit.Application/Habits/Queries/GetHabitByIdQuery.cs
Comment thread src/Orbit.Domain/Entities/Habit.cs
@thomasluizon

Copy link
Copy Markdown
Owner Author

Current head fixer approach for the two frozen findings:

  1. In GetHabitByIdQuery.cs, extend the existing descendant log batch through the user current date and map each general descendant IsCompleted value from an active positive log on that date. Both by-id and full-detail already share this loader and mapper, so I will add focused regressions to both handler suites.
  2. In QueryHabitsTool.cs, load the current-date log graph even when metrics are not requested, use date-scoped active logs for general-habit completion filtering, and pass the same derived value into top-level and child completion labels. Non-general habits will continue to use the lifetime IsCompleted flag. I will cover the completed filter and label output in QueryHabitsToolTests.cs.
  3. Reuse HabitScheduleService.HasCompletedLogInRange and the existing batched query shapes. This avoids mutating the lifetime flag, duplicating the completion rule, or introducing per-habit log queries.

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.

@sonarqubecloud

sonarqubecloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

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