fix: sub-habits no longer inherit the parent's frequency when the user picks one-time - #426
Conversation
CreateSubHabitCommandHandler always fell back to the parent's FrequencyUnit/FrequencyQuantity whenever the request omitted them (request.FrequencyUnit ?? parent.FrequencyUnit). The REST API (used by both web and mobile) omits frequency fields specifically to mean "one-time task" - so picking "Tarefa unica" for a sub-habit under a recurring parent silently created it as recurring with the parent's cadence and no days selected. Add an explicit InheritParentFrequency flag on CreateSubHabitCommand, defaulted false. The REST controller never sets it (an omitted frequency is always the user's explicit "one-time" choice). Only the AI chat tool opts in, since its "Override parent frequency" contract genuinely means "unspecified = match the parent." Winning hypothesis: confirmed via a fresh local repro (recurring parent -> add sub-habit -> switch to one-time -> submit) that the child was created with FrequencyUnit=Day, FrequencyQuantity=1, no days - traced to this handler's inheritance fallback firing on every omitted field regardless of caller intent. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VsTmCprkkcg9o3meMrET8W
|
There was a problem hiding this comment.
Code Review: fix/sub-habit-frequency-inheritance
Scope: PR #426 in thomasluizon/orbit-api
Recommendation: APPROVE
Summary
Small, well-scoped fix for a real bug: CreateSubHabitCommandHandler used to unconditionally fall back to the parent's frequency whenever the request omitted one, so a user picking "one-time" for a sub-habit was silently coerced back to the parent's recurring cadence. The fix introduces an explicit InheritParentFrequency flag (default false), so REST clients (whose omitted frequency always means an explicit "one-time" choice) get their real intent, while the AI chat tool opts in explicitly since its contract is "unspecified = match the parent." Two focused regression tests cover both branches.
Findings
Critical
None.
High
None.
Medium
None.
Low / Info
None — nothing rises to the concretely-actionable bar; the diff is small, targeted, and doesn't introduce new patterns worth flagging.
Subagents
| Agent | Verdict |
|---|---|
| security-reviewer | PASS — InheritParentFrequency is hardcoded true in CreateSubHabitTool.cs, not read from AI-tool JSON args, so it isn't attacker-controllable via prompt injection; REST DTO has no matching field so REST always defaults to false; parent-habit ownership check (h.UserId == request.UserId) still gates the read of parent.FrequencyUnit/FrequencyQuantity; no new logging/authz/injection surface. |
| contract-aligner | N/A — no DTO, Controller route, or packages/shared type changed in this diff (the new InheritParentFrequency param is Application-internal only; CreateSubHabitRequest/HabitsController.CreateSubHabit are untouched, confirmed by reading HabitsController.cs:537-558). |
Validation
| Check | Result |
|---|---|
| Build (dotnet) | N/A — covered by separate required CI check per this run's adaptation |
| Tests (dotnet) | N/A — covered by separate required CI check per this run's adaptation |
Deferred — N/A dimensions & files not verdicted
- Dimension 8 (DESIGN.md/AI-slop): N/A — no
apps/*files in this diff. - Dimension 9 (Parity) / 10 (i18n): N/A — backend-only diff, and both are frontend-only dimensions owned by the
orbit-ui-mobile-side review; not verifiable in this checkout. - Dimension 14 (FEATURES.md parity): N/A — this is a bugfix restoring documented behavior, not a new/changed/removed feature surface (rubric explicitly excludes pure bugfixes).
- All three changed files (
CreateSubHabitTool.cs,CreateSubHabitCommand.cs,CreateSubHabitCommandHandlerTests.cs) were read in full and given a verdict — nothing deferred there.
What's good
- Correctly root-causes the ambiguity (a shared
nullmeaning two different things for two callers) instead of patching around it — no workaround, no special-casing by caller type. - The new XML-doc on
InheritParentFrequencyclearly states the contract and why each caller sets it the way it does — exactly the kind of WHY note the comment policy wants. - Two new regression tests pin both branches (one-time child overrides parent cadence; explicit inherit opt-in still matches parent), and both assert on the actual persisted
Habitfields viaAddAsyncinterception rather than theResultalone. - Verified locally against a running instance + DB inspection per the PR description, in addition to the new unit tests.
Recommendation
Safe to merge as-is.



Summary
InheritParentFrequencyflag toCreateSubHabitCommand, defaultedfalse. The REST endpoint (used by bothorbit-ui-mobileweb and mobile) never sets it, so an omittedFrequencyUnit/FrequencyQuantityalways means the user's explicit "one-time" choice — it no longer silently falls back to the parent's cadence.CreateSubHabitTool) opts in viaInheritParentFrequency: true, preserving its documented "unspecified frequency = match the parent" contract.Root cause
CreateSubHabitCommandHandlerunconditionally didrequest.FrequencyUnit ?? parent.FrequencyUnit. Both the REST API (meaning "one-time" by omission) and the AI tool (meaning "inherit" by omission) shared the same ambiguousnull, so the handler could not distinguish the two intents.Repro (before the fix)
Confirmed via a local repro against a running instance + direct DB inspection before/after.
Test plan
dotnet test tests/Orbit.Application.Tests— 40/40 passing, including 2 new regression tests (Handle_OneTimeChildUnderRecurringParent_DoesNotInheritParentFrequency,Handle_InheritParentFrequencyWithNoOverride_UsesParentCadence)dotnet build— cleanFrequencyUnit=NULL, Days={}for the one-time sub-habit)Paired with orbit-ui-mobile PR (frontend timing bug for the same sub-habit-creation flow, bug #1 in that PR's description).
https://claude.ai/code/session_01VsTmCprkkcg9o3meMrET8W