Skip to content

fix: preserve weights when updating routines & workouts - #190

Merged
chrisdoc merged 1 commit into
mainfrom
ai-188-updating-routine-endpoint-resets-weights-to
Dec 10, 2025
Merged

fix: preserve weights when updating routines & workouts#190
chrisdoc merged 1 commit into
mainfrom
ai-188-updating-routine-endpoint-resets-weights-to

Conversation

@charliecreates

@charliecreates charliecreates Bot commented Dec 10, 2025

Copy link
Copy Markdown
Contributor

Align routine and workout MCP tools so that updating routines/workouts no longer clears weight fields, and make the tool inputs match the formatted outputs.

Changes

  • Routines tools (create-routine, update-routine)

    • Extend set schemas to accept both the old field names (weightKg, distanceMeters, durationSeconds) and the new, more human-friendly names (weight, distance, duration).
    • Map to the Hevy API using a null-safe preference order:
      • weight_kg now uses weight if present, otherwise weightKg.
      • distance_meters now uses distance if present, otherwise distanceMeters.
      • duration_seconds now uses duration if present, otherwise durationSeconds.
    • This keeps existing clients using weightKg working, while allowing tools and LLMs to work naturally with the weight/distance/duration fields they see in formatted routine outputs.
  • Workouts tools (create-workout, update-workout)

    • Mirror the same dual-field schema for workout sets, accepting both weight/weightKg, distance/distanceMeters, and duration/durationSeconds.
    • Update the mapping to the Hevy API so weight_kg, distance_meters, and duration_seconds prefer the new short names but fall back to the legacy ones.
    • Switch superset_id and notes mapping to use ?? null instead of || null so falsy-but-valid values (like 0 or empty strings) are preserved.
    • Remove unused SetType/ExerciseSetInput/ExerciseInput helper interfaces and rely on InferToolParams-derived types for the tool contracts.
  • General

    • Keep the existing integer semantics (.int()) for distance and duration fields, matching the underlying Hevy API, while fixing the weight naming mismatch that caused values like 62.5 kg to be dropped.

Verification

# Build
pnpm run build

# Unit tests (excluding integration tests that require HEVY_API_KEY)
pnpm vitest run --exclude 'tests/integration/**'

# Biome format/lint (auto-fix on)
pnpm run check

# TypeScript typecheck
pnpm run check:types  # fails with existing error in src/index.test.ts (unchanged in this branch)
  • pnpm run build: ✅
  • pnpm vitest run --exclude 'tests/integration/**': ✅ (8 files, 36 tests)
  • pnpm run check: ✅ (only pre-existing Biome warnings about config $schema and any in webhooks.ts)
  • pnpm run check:types: ❌ already failing on src/index.test.ts due to a process.exit spy signature mismatch; this file was not modified in this branch.

Notes on self-review feedback

  • Weight vs. weightKg precedence (routines & workouts): when both fields are present, weight now wins. This is intentional to softly deprecate weightKg while keeping backward compatibility. In practice, MCP clients should send one field or the other, so I did not add extra validation logic for conflicting values.
  • Distance/duration naming and units: kept the short names (distance, duration) to match the formatted tool outputs and avoid reintroducing the earlier mismatch. Units remain identical to the Hevy API: meters and seconds, respectively.
  • Integer duration semantics: left duration/durationSeconds as z.coerce.number().int() because the Hevy API expects whole seconds. Changing this would be a larger behavior change than the current bug fix.

Closes #188

✨ PR Description

Purpose: Fix weight and metric data loss when updating workout routines and exercise sets by adding backward-compatible field aliases.

Main changes:

  • Added shorthand field aliases (weight, distance, duration) to set schema for backward compatibility
  • Updated set mapping to prioritize shorthand fields over legacy camelCase fields using nullish coalescing
  • Replaced logical OR operators with nullish coalescing for proper null/0 value handling

Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Description using Guidelines Learn how

@codecov

codecov Bot commented Dec 10, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (943c712) to head (9216911).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #190   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            2         2           
=========================================
  Hits             2         2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gitstream-cm gitstream-cm Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨ PR Review

LGTM

Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using.
💡 Tip: You can customize your AI Review using Guidelines Learn how

@charliecreates charliecreates Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, the changes correctly introduce dual-field support (weight/weightKg, distance/distanceMeters, duration/durationSeconds) and fix the || null bug by switching to ?? null, which is a clear correctness improvement. The main risk introduced is subtle: the silent precedence of new fields over legacy ones when both are present could be confusing without documentation, and the altered semantics for notes (empty string vs null) might have downstream effects if any consumers distinguish them. It would be beneficial to add small inline comments to document precedence decisions and, if necessary, normalize notes values consistently. No obvious performance or architectural regressions are visible in the modified code.

Additional notes (5)
  • Maintainability | src/tools/routines.ts:149-152
    Using weight ?? weightKg ?? null preserves existing behavior and adds the desired precedence for the new weight field. However, when both weight and weightKg are provided and differ, the current logic silently prefers weight. Given your intentional soft deprecation, that’s acceptable, but it would be safer to at least document that precedence in a comment here so future maintainers don't accidentally reverse it or assume they must be equal.

If conflicting values are likely due to tool misuse, a follow-up improvement could be to surface a validation error or log a warning in that specific case.

  • Maintainability | src/tools/workouts.ts:18-43
    The removal of these custom types in favor of InferToolParams-derived types is a good simplification. One potential follow-up concern is that any external code (tests, helper functions) that might have been relying on these interfaces as a stable contract will now need to depend directly on the tool param types, which are more tightly coupled to the Zod schemas.

If those interfaces were ever exported (they weren't here), this would be a breaking API surface change; as-is, it's internal-only, but worth keeping in mind when evolving the schemas in the future.

  • Maintainability | src/tools/workouts.ts:170-170
    The schema now allows both weight and weightKg (and likewise for distance/duration) as number | null | undefined. Downstream, the mapping uses ?? to choose a value, which will treat 0 as a valid value for both fields. This is correct for preserving zero values but increases the number of possible shapes that a set can have.

Given that both fields are accepted for backward compatibility, this complexity is acceptable, but be aware that any future validation or business logic working on these tool params must always prefer the new short names to avoid accidentally reintroducing the bug you're fixing here.

  • Maintainability | src/tools/workouts.ts:213-213
    Switching from || null to ?? null is a functional improvement, preserving falsy-but-valid values like 0 or empty strings. However, for notes, this means an empty string will now be sent to the API instead of null. If the Hevy API (or your formatting utilities) have any semantic difference between "no notes" and "empty notes", this may subtly change behavior or output.

If that distinction doesn't matter, this is fine; if it does, you might want to normalize empty strings to null specifically for notes while still using ?? for numeric fields.

  • Maintainability | src/tools/workouts.ts:200-207
    Same precedence / conflict resolution concern as in the routines mapping: weight/distance/duration now silently override the legacy fields when both are present. That matches the stated intent, but the behavior is implicit and duplicated in multiple places (create & update, routines & workouts).

If you expect more such dual-field transitions in the future, this logic might be a candidate for a small shared helper to centralize precedence and keep the mapping code from diverging or getting copy-pasted inconsistently.

Summary of changes

Summary of Changes

  • Routines tools (src/tools/routines.ts)
    • Extended the set schemas for both create-routine and update-routine to accept new human-friendly fields: weight, distance, and duration alongside existing weightKg, distanceMeters, and durationSeconds.
    • Updated the mapping to the Hevy API to prefer the new short field names (weight, distance, duration) and fall back to the legacy names when the new ones are absent for weight_kg, distance_meters, and duration_seconds.
  • Workouts tools (src/tools/workouts.ts)
    • Removed unused helper types/interfaces (SetType, ExerciseSetInput, ExerciseInput).
    • Updated the create-workout and update-workout schemas to mirror the dual-field pattern used in routines (adding weight, distance, duration fields while keeping the legacy ones).
    • Changed mappings to the Hevy API to prefer the new short field names while falling back to the legacy names, and switched several || null usages to ?? null to preserve falsy-but-valid values.
    • Slight cleanup of comments and minor simplifications around type assertions and mapping logic.

@charliecreates
charliecreates Bot removed the request for review from CharlieHelps December 10, 2025 20:22
@chrisdoc
chrisdoc merged commit 14d10d1 into main Dec 10, 2025
17 checks passed
github-actions Bot pushed a commit that referenced this pull request Dec 10, 2025
## [1.13.2](v1.13.1...v1.13.2) (2025-12-10)

### Bug Fixes

* align routine and workout weight fields ([#190](#190)) ([14d10d1](14d10d1))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Updating routine endpoint resets weights to null - possible decimal and 'number' issue

2 participants