Track supplement doses separately from plans - #2225
Merged
Merged
Conversation
Keep scheduled supplements out of intake totals until a current append-only dose event explicitly records them as taken. Preserve stable definition schedules, source history, and installed V1 API shapes across web and mobile.
# Conflicts: # docs/production-incident-baseline.md
Local database validation remains unavailable after the volume cleanup incident. Host logs contain no fatal line or crash report, so the daemon termination cause remains unresolved.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Sorry @Asherlc, your pull request is larger than the review limit of 150000 diff characters
📝 WalkthroughWalkthroughThis PR replaces automatic supplement food entries with versioned supplement definitions and append-only dose events. Only current ChangesSupplement dose event redesign
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant supplementsRouter
participant SupplementsRepository
participant supplement_dose_event
participant v_nutrition_canonical_nutrient
Client->>supplementsRouter: recordDose(expectedCurrentEventId, status)
supplementsRouter->>SupplementsRepository: recordDose()
SupplementsRepository->>supplement_dose_event: lock current leaf and insert successor
supplement_dose_event-->>SupplementsRepository: recorded event
SupplementsRepository-->>supplementsRouter: dose result
supplementsRouter-->>Client: invalidate nutrition caches and return result
v_nutrition_canonical_nutrient->>supplement_dose_event: read current taken leaves
Assessment against linked issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Reviewer's GuideImplements per-user immutable supplement definitions with schedule identities and append-only dose events, rewrites the auto-supplements provider to materialize planned/unknown occurrences instead of food entries, integrates current taken doses into canonical nutrition, and exposes dose status/history APIs and UI on web/mobile while preserving existing V1 supplement list/save shapes. Sequence diagram for recording a supplement dose event and updating nutritionsequenceDiagram
actor Client
participant SupplementsRouter
participant SupplementsRepository
participant Database
participant NutritionCache
Client->>SupplementsRouter: recordDose(input)
SupplementsRouter->>SupplementsRepository: recordDose(expectedCurrentEventId, status)
SupplementsRepository->>Database: executeWithSchema(SELECT current event FOR UPDATE)
Database-->>SupplementsRepository: currentDoseEvent
SupplementsRepository->>Database: ensureProvider(DOFEK_PROVIDER_ID, DOFEK_PROVIDER_NAME, userId)
SupplementsRepository->>Database: executeWithSchema(INSERT INTO fitness.supplement_dose_event)
Database-->>SupplementsRepository: insertedId
SupplementsRepository-->>SupplementsRouter: { id, scheduledDate, status }
SupplementsRouter->>NutritionCache: invalidateNutritionCaches(userId)
NutritionCache-->>SupplementsRouter: void
SupplementsRouter-->>Client: recorded dose response
Entity relationship diagram for supplement definitions and dose eventserDiagram
supplement {
uuid id
uuid userId
uuid scheduleId
uuid supersedesSupplementId
date effectiveFrom
date effectiveTo
}
supplementDoseEvent {
uuid id
uuid userId
uuid scheduleId
uuid supplementId
date scheduledDate
text status
uuid supersedesEventId
}
supplement ||--o{ supplementDoseEvent : has_occurrence
supplementDoseEvent }o--|| supplementDoseEvent : supersedes
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
Mobile PreviewScan to open on device:
To test on device:
|
Contributor
|
Storybook previews for This comment updates automatically on each PR push. |
PR Summary by QodoTrack supplement doses separately from plans via append-only dose events
AI Description
Diagram
High-Level Assessment
Files changed (45)
|
Code Review by Qodo
Context used✅ Compliance rules (platform):
233 rules✅ Skills:
fix-provider, write-tests, cloudflare 1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Replace supplement rows atomically with stable schedules and immutable definitions. Keep dose events linked to both identities and enforce transaction-bound normalization in migration policy.
|
LGTM! Summary of Review
🤖 Reviewed by codereviewbot.ai - Catch bugs before your team does. |
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Reload router modules in schema tests so Stryker exercises static Zod initialization, and assert provider date/status/count boundaries through the public sync contract.
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
…sue-2064 # Conflicts: # docs/production-incident-baseline.md
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
Assert reconciliation, occurrence, row-boundary, and dose-conflict behavior so the repository mutation shard verifies its observable contracts.
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
drizzle/.sqlfluff (1)
1-4: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winPin sqlfluff to a version that accepts
large_file_skip_byte_limit = 0.
large_file_skip_byte_limit = 0disables the size skip, but sqlfluff has a known regression where this can raiseTypeError: '>' not supported between instances of 'int' and 'str'in affected versions. Pin sqlfluff to a fixed release to keep CI linting consistent indrizzle/.sqlfluff.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@drizzle/.sqlfluff` around lines 1 - 4, Pin the sqlfluff dependency used by CI to a fixed release that supports large_file_skip_byte_limit = 0 without the TypeError regression. Keep the existing drizzle/.sqlfluff setting unchanged and ensure all linting environments resolve the same compatible version.packages/server/src/repositories/settings-repository.test.ts (1)
232-250: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winThe test name promises ordering ("dose events before supplements") but only asserts a call count.
supplement_dose_eventhas FKs intosupplement/supplement_definition, so deletion order is the thing that can break. A count of 5 passes even if the order is inverted. Assert the relative index of the two statements, e.g. by stringifyingtransactionExecute.mock.callsand comparing positions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/server/src/repositories/settings-repository.test.ts` around lines 232 - 250, Update the test around SettingsRepository.deleteAllUserData to verify deletion ordering, not just the five-call count: inspect transactionExecute.mock.calls, identify the supplement_dose_event and supplement deletion statements, and assert the dose-event statement occurs at a lower index than the supplement statement. Preserve the existing call-count assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/production-incident-baseline.md`:
- Around line 19419-19435: Update the incident record’s “Root cause” section to
cite authoritative PostgreSQL documentation for the restriction on concurrent
CREATE INDEX and DROP INDEX operations inside transactions. Add links to the
relevant PostgreSQL CREATE INDEX and DROP INDEX references while preserving the
existing explanation and mitigation details.
In `@drizzle/0061_supplement_dose_events.sql`:
- Around line 240-242: Before the DELETE in the migration, add a preflight count
for rows matching provider_id = 'auto-supplements' and use it to assess the
migration’s expected workload; preserve the existing DDL transaction and
all-or-nothing rollback behavior. If execution is slow, record the observed
timing in docs/production-incident-baseline.md, without splitting the DELETE
into a separate transaction.
In `@packages/mobile/components/SupplementDoseEventsPanel.test.tsx`:
- Around line 81-98: Add a top-level invalidate function backed by
mocks.invalidate to the mocked useUtils() return value in
SupplementDoseEventsPanel.test.tsx, while preserving the existing food,
nutritionAnalytics, and supplements utilities. This ensures the assertion around
the no-op dose flow can detect unexpected invalidation calls.
In `@packages/server/src/repositories/settings-repository.ts`:
- Line 35: Reorder the table deletion entries so fitness.supplement_dose_event
appears before fitness.supplement_definition and fitness.supplement. Preserve
all existing entries and deletion behavior; only adjust the ordering in the
relevant settings repository list.
In `@packages/server/src/repositories/supplements-repository.test.ts`:
- Around line 627-716: Replace the catch-all test around freshSupplementSchema
and FreshSupplementsRepository with focused tests using the existing static
imports, removing vi.resetModules() and the dynamic import. Keep one test for
schema boundary rejection and another for invalid view-row rejection; remove the
duplicated list, occurrences, and recordDose happy-path assertions, while
preserving the existing invalid inputs and repository rejection behavior.
In `@packages/server/src/repositories/supplements-repository.ts`:
- Around line 207-219: Resolve the effective-date boundary semantics across the
update flow around the transaction that closes entries and the successor
creation logic: prevent the archived definition and successor from both being
active on effectiveDate. Either set the prior version’s effective_to to the
preceding day and update the related schema constraint/consumers accordingly, or
establish effective_to as exclusive and update every effective-window predicate
in v_supplement_with_nutrition and the dose materializer to use that convention
consistently.
- Around line 118-122: Update definitionsEqual to compare the parsed Supplement
fields explicitly rather than comparing JSON.stringify results. Parse both
values with supplementSchema, then compare each relevant normalized field by
value so equivalent definitions remain equal regardless of object key ordering.
In `@packages/server/src/routers/admin.ts`:
- Around line 200-201: Add `supplement_dose_event` to the `target_tables` list
used by the admin overview counts, alongside the existing supplement-related
table entries. Preserve the current counting and diagnostic behavior for all
other tables.
In `@packages/web/src/components/SupplementDoseEventsPanel.test.tsx`:
- Around line 81-98: Wire mocks.invalidate into the top-level object returned by
the mocked trpc.useUtils() in SupplementDoseEventsPanel.test.tsx, alongside the
existing food, nutritionAnalytics, and supplements entries. Keep the existing
assertion on mocks.invalidate so it can detect unintended broad
utils.invalidate() calls, and apply the same mock wiring to the corresponding
mobile test.
- Line 1: Update the useUtils() mock return objects in both
SupplementDoseEventsPanel test files to expose the declared mocks.invalidate spy
as the top-level invalidate property. Keep the existing not.toHaveBeenCalled
assertions unchanged so they validate the component behavior rather than an
unwired mock field.
In `@src/db/schema/nutrition.ts`:
- Around line 68-86: Add a self-supersedence CHECK constraint to the
supplementDefinition schema, named supplement_definition_not_self_superseding,
rejecting rows where supersedesDefinitionId equals id while allowing NULL
references. Add the matching constraint to the fitness.supplement_definition
table definition in drizzle/0061_supplement_dose_events.sql, alongside the
existing constraints.
In `@src/db/supplement-dose-migration.integration.test.ts`:
- Around line 187-214: Add assertions in the migration integration test around
definitionRowsSchema to verify each migrated definition has effective_to set to
null, updating the selected fields/schema and expected rows as needed. Then
query fitness.v_supplement_with_nutrition, parse the result with a suitable
schema, and assert the expected user-visible rows and nutrition values in sort
order.
In `@src/processing/dataset-contracts.ts`:
- Around line 281-286: Update the nutrition dataset’s sources declaration in
dataset-contracts to include supplement_dose_event alongside the existing
supplement tables. Preserve the current source entries and ordering, and ensure
the declared sources cover the table used to gate whether supplement nutrient
amounts count toward nutrition totals.
In `@src/providers/auto-supplements.integration.test.ts`:
- Around line 291-346: Update the migration test around “migration cleanup
removes only fictional auto-supplement foods and cascades nutrients” so it
exercises drizzle/0061_supplement_dose_events.sql rather than issuing its own
equivalent DELETE. Seed the fictional and real food rows before applying the
migration, then assert only provider_id='auto-supplements' rows and their
nutrients are removed while the real provider rows remain; preserve the cascade
assertions.
- Around line 123-130: Update the dose-event query in the test around
supplementDoseEvent to filter by both PRIMARY_USER_ID and the relevant
definition primary.id, then add an explicit ascending order by scheduledDate
before executing it. Keep the existing order-sensitive assertion unchanged.
- Around line 252-260: Replace the generic db.execute call assigned to takenRows
with executeWithSchema(), defining and applying a Zod schema for amount,
food_entry_id, and supplement_dose_event_id so the query result is validated at
runtime before the subsequent assertions.
In `@src/providers/auto-supplements.test.ts`:
- Around line 87-90: The serialized SQL assertions in
src/providers/auto-supplements.test.ts lines 87-90 and 162-164 need updating:
remove the not.toContain("fitness.food_entry") assertion at lines 87-90, and at
lines 162-164 capture each insert’s bound status parameter and positively assert
it is either "planned" or "unknown" instead of searching serialized SQL for
'"taken"'.
In `@src/providers/auto-supplements.ts`:
- Around line 111-145: Replace the nested loops in the supplement
synchronization flow with one set-based INSERT ... SELECT using generate_series
for the date window and the existing definition applicability predicate,
eliminating per-(definition, date) awaits and repeated datesInRange calls.
Preserve the current status, external_id, conflict handling, and inserted-row
counting semantics; add a concise SQL comment documenting that the partial
root-key uniqueness absorbs planned/unknown duplicates.
- Around line 34-43: Validate the three dates passed to datesInRange in sync()
before generating the range, rejecting any " -- " sentinel with a specific named
error. Keep datesInRange focused on valid YYYY-MM-DD values and ensure the
validation occurs once before the definitions query or event generation.
---
Outside diff comments:
In `@drizzle/.sqlfluff`:
- Around line 1-4: Pin the sqlfluff dependency used by CI to a fixed release
that supports large_file_skip_byte_limit = 0 without the TypeError regression.
Keep the existing drizzle/.sqlfluff setting unchanged and ensure all linting
environments resolve the same compatible version.
In `@packages/server/src/repositories/settings-repository.test.ts`:
- Around line 232-250: Update the test around
SettingsRepository.deleteAllUserData to verify deletion ordering, not just the
five-call count: inspect transactionExecute.mock.calls, identify the
supplement_dose_event and supplement deletion statements, and assert the
dose-event statement occurs at a lower index than the supplement statement.
Preserve the existing call-count assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b8901579-4a26-4ba8-935d-d064740347b3
📒 Files selected for processing (63)
AGENTS.mdREADME.mddocs/production-incident-baseline.mddocs/roadmap.mddocs/schema.dbmldocs/schema.mddocs/schema.pumldocs/superpowers/plans/2026-04-25-review-app-seed-database.mddocs/superpowers/plans/2026-06-04-slow-query-optimization-sequence.mddocs/superpowers/plans/2026-07-27-supplement-dose-events.mddrizzle/.sqlfluffdrizzle/0061_supplement_dose_events.sqldrizzle/meta/_journal.jsonpackage.jsonpackages/format/package.jsonpackages/format/src/supplement-dose-events.test.tspackages/format/src/supplement-dose-events.tspackages/mobile/app/supplements.test.tsxpackages/mobile/app/supplements.tsxpackages/mobile/components/SupplementDoseEventsPanel.stories.tsxpackages/mobile/components/SupplementDoseEventsPanel.test.tsxpackages/mobile/components/SupplementDoseEventsPanel.tsxpackages/server/src/lib/nutrition-cache.tspackages/server/src/lib/typed-sql.test.tspackages/server/src/lib/typed-sql.tspackages/server/src/repositories/food-repository.tspackages/server/src/repositories/provider-detail-repository.test.tspackages/server/src/repositories/provider-detail-repository.tspackages/server/src/repositories/settings-repository.test.tspackages/server/src/repositories/settings-repository.tspackages/server/src/repositories/supplement-dose-events.integration.test.tspackages/server/src/repositories/supplements-repository.test.tspackages/server/src/repositories/supplements-repository.tspackages/server/src/routers/admin.tspackages/server/src/routers/food.test.tspackages/server/src/routers/food.tspackages/server/src/routers/provider-detail.test.tspackages/server/src/routers/recovery-settings-sleep-need-sport-settings.test.tspackages/server/src/routers/router-data.integration.test.tspackages/server/src/routers/settings.integration.test.tspackages/server/src/routers/supplements-sync.test.tspackages/server/src/routers/supplements.test.tspackages/server/src/routers/supplements.tspackages/web/src/components/SupplementDoseEventsPanel.stories.tsxpackages/web/src/components/SupplementDoseEventsPanel.test.tsxpackages/web/src/components/SupplementDoseEventsPanel.tsxpackages/web/src/components/SupplementStackPanel.tsxpackages/web/src/routes/nutrition/supplements.tsxscripts/migration-policy.test.tsscripts/migration-policy.tsscripts/seed/core.tsscripts/seed/nutrition.tssrc/db/README.mdsrc/db/execute-with-schema.test.tssrc/db/execute-with-schema.tssrc/db/schema/enums.tssrc/db/schema/nutrition.tssrc/db/supplement-dose-migration.integration.test.tssrc/processing/dataset-contracts.tssrc/providers/README.mdsrc/providers/auto-supplements.integration.test.tssrc/providers/auto-supplements.test.tssrc/providers/auto-supplements.ts
Use canonical nutrient identities, assert wrapped PostgreSQL constraints, and reset cloned test data between retry attempts.
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
|
🤖 Review skipped: Repository rate limit exceeded. Free accounts are limited to 2 reviews per 4 hours per repository. Upgrade to a paid plan for unlimited reviews. |
This was referenced Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
supplements.listandsupplements.saveV1 projectionsauto-supplementsfood rows through the forward migration and document the durable schema contractRoot Cause
The internal auto-supplements provider ignored the requested user and materialized every scheduled definition as a confirmed food entry, so a plan was indistinguishable from consumed nutrition and one user's sync could recreate another user's fictional intake.
Validation
pnpm tsc --noEmitpnpm --dir packages/server exec tsc --noEmitpnpm --dir packages/web exec tsc --noEmitpnpm --dir packages/mobile exec tsc --noEmitpnpm test: 889 files / 14,102 tests passed; 2 files / 21 tests skippedCompatibility
Fixes #2064
Summary by cubic
Separate supplement plans from real doses using immutable schedules and append‑only dose events. Adds dose history on web/mobile, a 7‑day occurrences API, and only counts current taken doses toward nutrition. Fixes #2064.
New Features
supplements.occurrences(7‑day per‑user with counts) andsupplements.recordDose(conflict‑safe taken/skipped); uses shared nutrition cache invalidation.fitness.supplement_definitionversions and row-basedfitness.supplement_definition_nutrient; timezone‑bounded materialization with provider provenance; V1supplements.list/saveshapes unchanged.SupplementDoseEventsPanelto review history and record doses; recording refreshes occurrences and daily nutrition; auto‑supplements materializes occurrences only and never creates food entries.Migration
drizzle/0061_supplement_dose_events.sqlto replace legacy supplements withfitness.supplement,fitness.supplement_definition,fitness.supplement_definition_nutrient,fitness.supplement_dose_event, andv_supplement_dose_current; backfills atomically.fitness.supplement_dose_event.Written for commit bdf4ca6. Summary will update on new commits.
Summary by CodeRabbit