feat: add body measurement history and explicit remeasurement - #204
krzysiek-kurek wants to merge 8 commits into
Conversation
github-actions
Bot
added
javascript
Reviewer's GuideIntroduces time-aware body measurement history alongside the existing profile snapshot, with validated and concurrency-safe APIs for explicit remeasurement and additive profile updates. The settings UI now distinguishes saved values from active drafts, supports clearing and explicit confirmation, and preserves canonical values across unit changes and pending saves. Sequence diagram for explicit body measurement remeasurementsequenceDiagram
actor User
participant Settings as SettingsUI
participant API as UsersAPI
participant Service as BodyMeasurementService
participant DB as Database
User->>Settings: handleConfirmMeasurement()
Settings->>API: POST /users/me/body-measurements
API->>API: normalize_measurement_value()
API->>Service: record_measurements()
Service->>DB: lock_user_for_measurement_update()
Service->>DB: insert BodyMeasurementObservation
Service->>DB: update user.body_measurements snapshot
API->>Service: current_state()
Service-->>API: current measurement state
API-->>Settings: BodyMeasurementStateResponse
Settings-->>User: show saved measurement
Entity relationship diagram for body measurement historyerDiagram
USER ||--o{ BODY_MEASUREMENT_OBSERVATION : has
USER {
uuid id PK
jsonb body_measurements "compatibility snapshot"
}
BODY_MEASUREMENT_OBSERVATION {
uuid id PK
uuid user_id FK
string metric
numeric value
string unit
datetime measured_at
string source
datetime created_at
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
@sourcery-ai review |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="backend/app/api/users.py" line_range="119-125" />
<code_context>
)
+ if "body_measurements" in update_data:
+ measurement_patch = update_data["body_measurements"]
+ await BodyMeasurementService(db).record_profile_changes(current_user, measurement_patch)
+ if measurement_patch is not None:
+ update_data["body_measurements"] = {
+ **(current_user.body_measurements or {}),
+ **measurement_patch,
+ }
+
for field, value in update_data.items():
</code_context>
<issue_to_address>
**issue (bug_risk):** Concurrent partial profile updates read the same old snapshot, then each writes a complete merged snapshot based on that stale value. The last transaction overwrites measurements written by the other transaction, even though both observations remain in history.
**Triggers:** When two profile measurement patches for different metrics are submitted concurrently.
**Suggested fix:** Apply the JSONB patch atomically in the database, or lock and refresh the user row before merging the snapshot.
</issue_to_address>
### Comment 2
<location path="backend/app/api/users.py" line_range="167-177" />
<code_context>
+ detail="at least one measurement is required",
+ )
+
+ for metric, value in data.measurements.items():
+ if metric not in MEASUREMENT_UNITS:
+ raise HTTPException(
+ status_code=status.HTTP_422_UNPROCESSABLE_ENTITY,
+ detail=f"unsupported body measurement: {metric}",
+ )
+ if value <= 0:
+ raise HTTPException(
</code_context>
<issue_to_address>
**issue (bug_risk):** A non-finite numeric value such as NaN passes the endpoint's `value <= 0` validation, then `_number` evaluates `result > 0` and raises `decimal.InvalidOperation` instead of returning a validation error. The request therefore produces an internal server error rather than a 422 response.
**Triggers:** When a client submits a JSON NaN value to the explicit body-measurement endpoint.
**Suggested fix:** Reject non-finite values with `math.isfinite(value)` before calling the service, and handle non-finite Decimal values in `_number`.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 2 findings to address first, and this adds persisted measurement observations and backfills existing profile data, so a faulty value or timestamp can remain in history after the code is reverted. The current profile snapshot and observation table are bounded and can be corrected or rebuilt, but the migration and writes are not purely ephemeral.
Blocking findings: backend/app/api/users.py:125, backend/app/api/users.py:177
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="frontend/lib/hooks/use-user.ts" line_range="20" />
<code_context>
role: string;
onboarding_completed: boolean;
- body_measurements?: Record<string, number | string> | null;
+ body_measurements?: Record<string, number | string | null> | null;
+}
+
</code_context>
<issue_to_address>
**issue (bug_risk):** When the profile contains a cleared size field represented as `null`, the settings initialization converts it with `String(value)`, so the input displays the literal text `"null"` instead of remaining empty.
**Triggers:** When a body-measurement size has been saved as null, such as after clearing it.
**Suggested fix:** Treat null values as empty strings during settings initialization, e.g. use `value == null ? '' : String(value)`.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if the measurement projection or recording logic is wrong, incorrect values and historical observations can remain persisted after the code is reverted. The affected user measurement data is bounded and can be corrected or removed, but reverting alone does not undo it.
Blocking findings: frontend/lib/hooks/use-user.ts:20
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="frontend/app/dashboard/settings/page.tsx" line_range="472-477" />
<code_context>
- body_measurements: Object.keys(parsed).length > 0 ? parsed : null,
- });
- setMeasurementsDirty(false);
+ if (measurementsDirty) {
+ const sizeKeys = ['shirt_size', 'pants_size', 'dress_size', 'shoe_size'];
+ const sizeMeasurements: Record<string, string | null> = {};
+ for (const key of sizeKeys) {
+ const value = measurements[key]?.trim();
+ sizeMeasurements[key] = value || null;
+ }
+ await updateUserProfile.mutateAsync({ body_measurements: sizeMeasurements });
</code_context>
<issue_to_address>
**issue (broader_impact):** Editing one clothing-size field sends all four size keys, assigning `null` to untouched fields; the backend merges those nulls into the profile snapshot, so existing unrelated size values are erased on save.
**Triggers:** When a user has an existing pants, dress, or shoe size and edits only another size field.
**Suggested fix:** Send only size fields that were actually edited, or make the backend distinguish omitted fields from explicit clears before merging the patch.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if the new recording or backfill logic is wrong, incorrect body-measurement observations and profile values remain after a revert; the history cannot be restored by rerunning the old code. The affected records are user-scoped and bounded, so they can be corrected or rebuilt with a repair migration.
Blocking findings: frontend/app/dashboard/settings/page.tsx:477
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="frontend/tests/body-measurement-session.test.ts" line_range="15-19" />
<code_context>
+)
+
+describe('body measurement session UX', () => {
+ it('writes confirmed measurements through the explicit endpoint', () => {
+ expect(userHooksSource).toContain('useRecordBodyMeasurements')
+ expect(userHooksSource).toContain("'/users/me/body-measurements'")
+ expect(userHooksSource).toContain('api.post')
+ })
+
+ it('keeps measurement inputs empty and shows the last value as placeholder', () => {
</code_context>
<issue_to_address>
**issue (testing):** This test only searches source-code text and never renders the settings page or invokes the save flow, so it passes even when the hook is not wired correctly or confirmed measurements are sent through the wrong runtime path.
**Triggers:** When the endpoint wiring is changed without updating the literal source strings.
**Suggested fix:** Render the component with mocked hooks and assert that saving an activated measurement calls the mutation with the expected canonical value.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if the history or current-state projection is wrong, this change can persist incorrect measurement observations and profile values beyond the request that created them. Reverting removes the new code, but already-written observations would need a bounded cleanup or recomputation rather than being automatically undone.
Blocking findings: frontend/tests/body-measurement-session.test.ts:19
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="backend/app/models/body_measurement.py" line_range="22" />
<code_context>
+ UUID(as_uuid=True), ForeignKey("users.id", ondelete="CASCADE"), nullable=False, index=True
+ )
+ metric: Mapped[str] = mapped_column(String(32), nullable=False, index=True)
+ value: Mapped[Decimal] = mapped_column(Numeric(12, 4), nullable=False)
+ unit: Mapped[str] = mapped_column(String(16), nullable=False)
+ measured_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True)
</code_context>
<issue_to_address>
**issue (bug_risk):** The endpoint accepts any positive finite float, but observations use `Numeric(12, 4)` without range or precision validation. A value such as `0.00001` is rounded to zero in persistence while the profile snapshot retains the positive float, so the current-state response no longer matches the newly recorded observation and history reports a different value; sufficiently large values raise a database numeric overflow instead of returning the documented validation error.
**Triggers:** When a client submits a positive measurement outside the database column's representable precision or scale.
**Suggested fix:** Validate and quantize values against the `Numeric(12, 4)` constraints before creating the observation, rejecting values that would round to zero or overflow.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and this persists body-measurement observations and backfills existing profile data into a new history table, so an incorrect value or duplicate observation would outlive the request and require correction or history cleanup. The affected records are bounded and repairable, although reverting the code alone would not correct data already written.
Blocking findings: backend/app/models/body_measurement.py:22
| UUID(as_uuid=True), ForeignKey("users.id", ondelete="CASCADE"), nullable=False, index=True | ||
| ) | ||
| metric: Mapped[str] = mapped_column(String(32), nullable=False, index=True) | ||
| value: Mapped[Decimal] = mapped_column(Numeric(12, 4), nullable=False) |
There was a problem hiding this comment.
issue (bug_risk): The endpoint accepts any positive finite float, but observations use Numeric(12, 4) without range or precision validation. A value such as 0.00001 is rounded to zero in persistence while the profile snapshot retains the positive float, so the current-state response no longer matches the newly recorded observation and history reports a different value; sufficiently large values raise a database numeric overflow instead of returning the documented validation error.
Triggers: When a client submits a positive measurement outside the database column's representable precision or scale.
Suggested fix: Validate and quantize values against the Numeric(12, 4) constraints before creating the observation, rejecting values that would round to zero or overflow.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. A bad validation, unit conversion, or history projection could write incorrect body measurements and durable observation rows for users. Reverting the code would stop the new behavior but would not remove those persisted observations, although the impact is bounded to affected user records and can be corrected with data repair or later measurements.
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="frontend/tests/body-measurement-session.test.ts" line_range="1-19" />
<code_context>
+import { readFileSync } from 'node:fs'
+import { resolve } from 'node:path'
+import { describe, expect, it } from 'vitest'
+
+const settingsSource = readFileSync(
+ resolve(process.cwd(), 'app/dashboard/settings/page.tsx'),
+ 'utf8',
+)
+describe('body measurement session UX', () => {
+ it('keeps measurement inputs empty and shows the last value as placeholder', () => {
+ expect(settingsSource).toContain("value={measurementSession[field.key] ?? ''}")
+ expect(settingsSource).toContain('placeholder={measurements[field.key] ?? placeholder}')
+ })
+
+ it('marks focused measurements active and lets an accidental activation be cleared', () => {
+ expect(settingsSource).toContain('handleActivateMeasurement')
+ expect(settingsSource).toContain('onFocus={() => handleActivateMeasurement(field.key)}')
+ expect(settingsSource).toContain('handleDeactivateMeasurement')
+ expect(settingsSource).toContain('isActive &&')
+ })
+})
</code_context>
<issue_to_address>
**issue (testing):** The session UX test only searches the source text for implementation strings, so it passes even if the component does not render correctly, activation does not update state, or clearing does not work at runtime. It provides no behavioral coverage for the stated input/session invariant.
**Suggested fix:** Render the settings component with mocked hooks and assert the actual input values, placeholder, focus activation, and clear-button behavior.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and this adds persisted measurement observations and changes the profile snapshot, so an incorrect or concurrent update can leave users with a wrong current value or history after a revert. The damage is bounded and can be repaired from the observation history or corrected records, but reverting alone does not undo it.
Blocking findings: frontend/tests/body-measurement-session.test.ts:19
| import { readFileSync } from 'node:fs' | ||
| import { resolve } from 'node:path' | ||
| import { describe, expect, it } from 'vitest' | ||
|
|
||
| const settingsSource = readFileSync( | ||
| resolve(process.cwd(), 'app/dashboard/settings/page.tsx'), | ||
| 'utf8', | ||
| ) | ||
| describe('body measurement session UX', () => { | ||
| it('keeps measurement inputs empty and shows the last value as placeholder', () => { | ||
| expect(settingsSource).toContain("value={measurementSession[field.key] ?? ''}") | ||
| expect(settingsSource).toContain('placeholder={measurements[field.key] ?? placeholder}') | ||
| }) | ||
|
|
||
| it('marks focused measurements active and lets an accidental activation be cleared', () => { | ||
| expect(settingsSource).toContain('handleActivateMeasurement') | ||
| expect(settingsSource).toContain('onFocus={() => handleActivateMeasurement(field.key)}') | ||
| expect(settingsSource).toContain('handleDeactivateMeasurement') | ||
| expect(settingsSource).toContain('isActive &&') |
There was a problem hiding this comment.
issue (testing): The session UX test only searches the source text for implementation strings, so it passes even if the component does not render correctly, activation does not update state, or clearing does not work at runtime. It provides no behavioral coverage for the stated input/session invariant.
Suggested fix: Render the settings component with mocked hooks and assert the actual input values, placeholder, focus activation, and clear-button behavior.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="frontend/app/dashboard/settings/page.tsx" line_range="475-479" />
<code_context>
+
+ try {
+ await recordBodyMeasurements.mutateAsync({ [key]: canonicalValue });
+ const reconciledValue = formatMeasurementDisplay(
+ canonicalValue,
+ key,
+ 'metric',
+ unitSystemRef.current,
+ );
+ setMeasurements((prev) => ({ ...prev, [key]: reconciledValue }));
</code_context>
<issue_to_address>
**issue (bug_risk):** Both reconciliation call sites pass five arguments to `formatMeasurementDisplay`, but the function accepts only four, so TypeScript compilation fails with an argument-count error.
**Suggested fix:** Add the intended fifth parameter to the function signature or remove the extra argument and perform the unit conversion separately.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and this adds a persistent observation table, backfills legacy measurements, and writes new measurement values and history that would remain if the application code were reverted. Incorrect values or duplicate observations would be bounded and can be corrected or removed, so the impact is repairable rather than irreversible.
Blocking findings: frontend/app/dashboard/settings/page.tsx:479
| const reconciledValue = formatMeasurementDisplay( | ||
| canonicalValue, | ||
| key, | ||
| 'metric', | ||
| unitSystemRef.current, |
There was a problem hiding this comment.
issue (bug_risk): Both reconciliation call sites pass five arguments to formatMeasurementDisplay, but the function accepts only four, so TypeScript compilation fails with an argument-count error.
Suggested fix: Add the intended fifth parameter to the function signature or remove the extra argument and perform the unit conversion separately.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. This writes durable measurement observations and backfills legacy profile data into a new table, so reverting the application does not remove potentially incorrect history. The affected values are bounded and users can correct current measurements, but historical rows require explicit cleanup or a migration rollback.
|
Closing this draft for now while the measurement-history UX and semantics are being refined. The problem and proposed direction are tracked in #205. The implementation branch is being kept so the work can be resumed without losing the tested backend and migration work. |
2 participants
Description
Adds time-aware body measurement history and an explicit remeasurement flow while preserving the existing profile snapshot for compatibility.
Backend
POST /api/v1/users/me/body-measurementsendpoint so remeasuring the same value still creates a new observationFrontend
Related Issue
N/A — no matching open issue or pull request was found.
Type of Change
Checklist
Testing
Scoped branch after rebasing onto current
upstream/main:Aggregate/self-hosted validation:
Screenshots
Not included; this changes the behavior of the existing Settings → Body Measurements UI rather than introducing a new layout.
Additional Notes
body_measurementsprofile data remains the compatibility snapshot.source = legacy_profileand nomeasured_attimestamp.This change was developed with AI assistance (ChatGPT, OpenAI). The implementation and tests were reviewed and verified locally before submission.
Summary by Sourcery
Add time-aware body measurement history and explicit remeasurement while preserving compatibility with the existing profile snapshot.
New Features:
Bug Fixes:
Enhancements:
Tests: