Repository navigation
feat: expand habit schedule model (ORB-221, step 1 of 3) - #792
Conversation
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes Reviewed the schedule-model expansion, persistence rollout, response mappings, and compatibility with the current orbit-ui-mobile consumer; no Critical or High findings were identified.
- Domain model: Adds derived
HabitScheduleand immutable originalStartDate, preserving legacy scheduling behavior and the separate recurrence anchor. - Persistence: Adds the columns, guarded historical backfill, legacy-writer compatibility trigger, rollback operations, and updated EF model snapshots.
- API contracts: Appends optional fields to detail, schedule, calendar, nested-child, and sync responses, with response annotations and generated contracts for both API versions; existing requests and legacy fields remain intact.
- Verification: Reviewed the characterization and regression coverage and ran 204 relevant unit tests, all passing. PostgreSQL trigger execution remains unverified against PostgreSQL; the migration tests exercise adapted backfill queries and inspect generated trigger SQL.
gpt-6.1-sol | 𝕏
There was a problem hiding this comment.
✅ No new Critical or High issues found.
Reviewed changes Reviewed ce5eefd0 since the prior Pullfrog review at 22b704ab, with the full PR diff as context.
- Corrected enum schemas: Documented
HabitScheduleandFrequencyUnitas named strings in both OpenAPI versions, matching existing serialization and the current consumer. - Preserved tracked values: Removed
StartDateandScheduleassignments from read-field restoration and added a divergent-projection regression test. - Enabled backfill synchronization: Updated existing habits'
UpdatedAtUtcduring backfill so delta sync resends the expanded fields. - Expanded behavioral coverage: Added four-type mutation and metrics characterizations, earlier-date anchor tests, and schema/backfill assertions. All 73 focused tests passed; the working tree is clean.
gpt-6.1-sol | 𝕏
There was a problem hiding this comment.
✅ No new Critical or High issues found.
Reviewed changes Reviewed f7c503fe since the prior Pullfrog review at ce5eefd0, with the full PR diff and current consumer schemas as context.
- Restored schema compatibility: Restricted the string-enum transformer to
HabitScheduleand restoredFrequencyUnitto its exact base schema in both OpenAPI versions, without changing runtime serialization. - Strengthened contract coverage: Retained assertions for the new schedule enum and added complete
FrequencyUnitbaseline assertions; all eight focused contract tests passed, and the working tree remains clean.
gpt-6.1-sol | 𝕏
|
…uppression cleanup (#746) (#793) * feat: expand habit schedule model (ORB-221, step 1 of 3) (#792) * test: characterize habit schedules before expand step (ORB-221) * feat: expand habit schedule and immutable start date (ORB-221) * test: include habit expansion in schema preservation guard * fix: address habit schedule review (thomasluizon/orbit-tickets#227) * fix: preserve FrequencyUnit OpenAPI contract (ORB-221) (cherry picked from commit 7a708b5) * Clear handwritten suppressions and audit retained exemptions (ORB-224) (#791) * fix: recognize UTC timestamp sinks without date exemptions * fix: guard generated logging arguments and remove blanket disables * refactor: remove catalog suppression scaffolding without changing metadata * fix: forward cancellation and remove obsolete library suppressions * chore: record every suppression verdict and reject duplicate reasons * fix: address suppression audit review for ticket 230 * fix: resolve suppression cleanup CI findings Refs thomasluizon/orbit-tickets#230 * test: prove OAuth authorize query validation parity (cherry picked from commit 05ab55f)




Logging advances
DueDate, so it cannot retain the initial date of a habit. This adds a persisted, immutableStartDateand the four-valueHabitScheduleenum alongside the legacy fields.This is step 1 of 3 for thomasluizon/orbit-tickets#227; keep the ticket open after this pull request.
Approach
src/Orbit.Domain/Entities/Habit.csandEnums/HabitSchedule.csderiveScheduleon creation and every successful update. Existing guards, flags, scheduling behavior, andDueDateremain in use.StartDatecaptures the creation input and survives logging, unlogging, catch-up, postponement, skipping, and updates.HabitScheduleSnapshot, its projection, and candidate hydration carry both persisted values through the existing read paths.ScheduledStartDatekeeps its separate recurrence-anchor role.OrbitDbContextandExpandHabitScheduleAndStartDateadd the columns with defaults suitable for older API processes. The migration locks habit writes, counts all four invalid legacy combinations, and raises an exception with the count before changing rows. It derives General, Flexible, OneTime, then Recurring in that order. Existing start dates are approximations: earliest non-deleted log date, otherwise the UTC creation date, capped at the current due date. The backfill also setsUpdatedAtUtc, so delta sync resendsscheduleandstartDatefor every existing habit once. A compatibility trigger keeps old API writers consistent, initializes their start date, rejects invalid combinations, and preserves the stored start on updates. Down removes the trigger and added columns; the legacy flags already remain intact.scheduleandstartDate. ExistingisGeneral,isFlexible, anddueDatestay unchanged. Create/update requests stay unchanged. Response annotations expose the additions insrc/Orbit.Api/openapi.jsonandopenapi_v2.json, with the newscheduledocumented as the string enum the API sends (JsonStringEnumConverter).FrequencyUnitkeepsmain's document: requests still accept integers, so a string-only enum would narrow the request contract (the OpenAPI Breaking-Change Gate rejects it); its exact document is step 2; generated migration files stay with the model change.StartDateis the creation anchor only: the date the habit was created to start. It stays fixed whenDueDatemoves, including to an earlier date; theDueDateclamp applies to the backfill only. A user-edited start date is step 2. The schedule read projection no longer assignsStartDateorScheduleon tracked entities.Steps remaining on this ticket:
NextDueDateproperty/column while retaining JSONdueDate.The UI/shared-schema follow-up belongs to its separate ticket after this merges. This PR does not change
MinSupportedVersion.Test evidence
Characterizations were committed first in
49cbc0e6, before production changes, and remain unchanged in the implementation commit. They cover all four types in the schedule service, metrics, query, and user streak service, including a Sao Paulo timezone boundary, plus log/unlog, skip, catch-up, and postponement outcomes.Observed regression sequence:
dotnet test tests/Orbit.Domain.Tests --no-restore --filter FullyQualifiedName~Log_Recurring_AdvancesDueDate: the existing unchanged test passed with noStartDatefield.dotnet test tests/Orbit.Domain.Tests --no-restore --filter FullyQualifiedName~LogTenTimes_PreservesOriginalStartDate: the new test logged ten times, checked the advanced due date, and failed because the original-start property was absent. Reflection let this assertion run against the original model; the final test uses the typed property.The first full infrastructure run caught a stale schema-preservation inventory (10 differences versus the new 12).
FeatureFlagAccountsMigrationTestsnow explicitly verifies both added columns and the appended migration; its focused rerun passes.Focused checks cover all type transitions, retained flags when frequency changes, rejected updates, the unchanged Days restriction, existing validator/command inputs, snapshots, actual response producers and serialized legacy fields, optional v1/v2 schemas, and migration backfill/guard/rollback operations.
The migration backfill and invalid-row count query execute against SQLite with only PostgreSQL function syntax adapted. The PostgreSQL exception block and compatibility trigger ran against a real PostgreSQL 17 database in the review round below. No database harness was added to the repository.
Tool evidence:
dotnet ef migrations add ExpandHabitScheduleAndStartDate --project ../Orbit.Infrastructure --startup-project . --verbosereportedUsing DbContext factory 'OrbitDbContextFactory'.dotnet ef migrations script 20261019091925_AddOwedSubscriptionCancellations 20261019165331_ExpandHabitScheduleAndStartDate --project ../Orbit.Infrastructure --startup-project . --output <temporary SQL file>exited 0 and emitted the transaction, preflight guard, date default, backfill, and trigger. These commands ran fromsrc/Orbit.Apiwithout connecting to a live database.Final validation (all exit 0):
dotnet build Orbit.slnx: 0 errors, 19 warnings.dotnet test tests/Orbit.Domain.Tests --no-build: 749 passed.dotnet test tests/Orbit.Application.Tests --no-build: 5,199 passed.dotnet test tests/Orbit.Infrastructure.Tests --no-build: 3,741 passed after updating the schema inventory.dotnet test tests/Orbit.Analyzers.Tests --no-build: 32 passed.Review round 1
Commit
ce5eefd0.All commands below exited 0:
dotnet build Orbit.slnx --nologodotnet test tests/Orbit.Domain.Tests --nologodotnet test tests/Orbit.Application.Tests --nologodotnet test tests/Orbit.Infrastructure.Tests --nologoBefore implementation changes, the unchanged tests passed with the defects present: 19 passed, exit 0.
The strengthened tests then produced 6 intended failures, exit 1: four integer-schema mismatches, overwritten tracked
StartDate, and unchanged backfill timestamp.All six pass after the fixes, including in the full infrastructure run above.
The identical expanded characterisation files also passed in a scratch checkout of
origin/main(7c793090), both exit 0:Results: 21 domain cases, 8 application cases. Commit hooks passed.
CI fix round 1
Commit
f7c503fe. The string enum is limited toHabitSchedule; theFrequencyUnitschema diff againstorigin/mainis empty in both OpenAPI files.dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~OpenApi_ScheduleEnumsMatchSerializedNames' --no-restore --verbosity quietexited 0, four passed.dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~OpenApi_HabitScheduleMatchesSerializedNamesAndFrequencyUnitPreservesMainSchema' --no-restore --verbosity quietexited 1, both cases failed on theFrequencyUnitbaseline assertion.dotnet test tests/Orbit.Infrastructure.Tests --filter 'FullyQualifiedName~HabitScheduleContractTests' --no-build --verbosity normalexited 0, eight passed.dotnet build Orbit.slnx --verbosity quiet: 0, zero errors.dotnet test tests/Orbit.Domain.Tests --no-restore --verbosity quiet: 0, 767 passed.dotnet test tests/Orbit.Application.Tests --no-restore --verbosity quiet: 0, 5,203 passed.dotnet test tests/Orbit.Infrastructure.Tests --no-restore --verbosity quiet: 0, 3,744 passed.PostgreSQL 17 proof
Run against a throwaway local
postgres:17container, removed afterwards. The database was migrated to20261019091925_AddOwedSubscriptionCancellationswithdotnet ef database update, seeded with one habit of each type (and, for the recurring habit, one active log on 2026-09-05 and one deleted log on 2026-09-02), then migratedUpto20261019165331_ExpandHabitScheduleAndStartDate(exit 0).StartDate2026-10-01,Schedule0StartDate2026-09-05,Schedule1StartDate2026-10-19,Schedule2StartDate2026-10-10 (capped atDueDate),Schedule3UpdatedAtUtcafter the backfillStartDate=DueDate(2026-11-01),Schedule1StartDate2026-10-05 and a wrongSchedule3StartDate2026-10-05 kept,Schedulederived as 0StartDateto 2020-01-01 withDueDate2026-10-21StartDatestays 2026-09-05,DueDate2026-10-21Scheduleto 3 on the recurring habitSchedulestays 1IsGeneralto true on the one-time habitSchedulebecomes 3FrequencyUnit0; INSERT a flexible habit with noFrequencyUnitRAISEinsync_habit_schedule_expand()); row count unchangedDownto the previous migrationScheduleandStartDatecolumns, the trigger and the function removed; all rows keptUpon a second database holding one general habit with aFrequencyUnitHabit schedule expansion aborted: 1 invalid legacy rows; no column added, migration not recordedAssumptions
StartDatecaptures the validated date passed toHabit.Create, retaining existing creation-date normalization rather than changing request semantics in the expand step.CannotSkipOneTimeTaskthrough a legacy snapshot instead of weakening factory validation.Manual steps
maindeploys nothing. Before a production release, obtain the owner's explicit yes and take a backup of the production Render Postgres database. Record its reference before running the migration.Release API(.github/workflows/release.yml), withenvironment=productionandbranch=main. Retain the migration's backfilled-row count and verify the released commit through that workflow's health check.StartDate > DueDateterm holds for the backfill only;DueDatemay later move before the creation anchor):StartDate; a later reapply recomputes its approximation.