fix(routines): keep reps when repRange is set

[charliecreates]

Fixes #269 by keeping reps when repRange is present, and by backfilling reps for fixed ranges (e.g. 8–8) so the Hevy apps show reps again.

Changes

• Update create-routine and update-routine set mapping so reps is no longer forced to null when repRange is provided.
• If repRange.start === repRange.end and reps is not a number, infer reps from the fixed range.
• Only show the rep-range display warning for non-fixed rep ranges (still linked to Public API does not support input_modifier field — rep ranges created via MCP appear blank in app #261).
• Extend src/tools/routines.test.ts coverage for fixed and non-fixed rep ranges.

Verification

# Build: success
pnpm run build

# Vitest (unit): 143 tests passed
pnpm vitest run --exclude 'tests/integration/**'

# Biome: 43 files checked
pnpm run check

# TypeScript: no errors
pnpm run check:types

reviewChanges notes skipped:

• Recomputing getFixedRepsFromRepRange() in the rep-range warning check — small refactor/perf cleanup, out of scope for this fix.
• More defensive semantics around reps: null + non-fixed repRange on updates — would change tool contract, out of scope.
• Dedicated unit tests for getFixedRepsFromRepRange edge cases — current behavior is covered indirectly via routine tool tests; keeping helper private.

Closes #269.

✨ PR Description

Purpose: Fix routine creation and update logic to preserve reps field when repRange is set, preventing data loss during routine operations.

Main changes:

• Added getFixedRepsFromRepRange() helper to extract fixed rep count when repRange start equals end
• Modified create-routine and update-routine handlers to preserve explicit reps values or copy from fixed repRange
• Updated usesRepRanges flag logic to only trigger warning for non-fixed rep ranges

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}

[gitstream-cm]

✨ 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]

The core logic change looks correct and aligned with the PR goal (preserve reps and infer fixed-range reps). Main improvements are around maintainability: avoid duplicated normalization between buildRepRange() and getFixedRepsFromRepRange(), and avoid recomputing fixed-range checks in a second pass for warning detection. Tests add valuable coverage but are currently quite repetitive; refactoring into helpers/table-driven tests would reduce maintenance overhead.

Additional notes (3)

• Maintainability | src/tools/routines.ts:93-120
getFixedRepsFromRepRange() duplicates the normalization logic already present in buildRepRange() (coercing undefined to null, checking for nulls, etc.). This creates two sources of truth for rep-range normalization and increases the chance these helpers drift (e.g., if buildRepRange ever changes its semantics).

Since you already call buildRepRange() before getFixedRepsFromRepRange(), consider tightening the contract and accepting only the normalized output type of buildRepRange (i.e., {start: number|null; end: number|null} | null). That eliminates redundant checks and makes the call sites clearer.

• Performance | src/tools/routines.ts:253-253
  The usesRepRanges detection now recomputes getFixedRepsFromRepRange(set.rep_range) for each set after you already computed fixedReps while building sets. This is minor but it’s still avoidable extra work and a bit harder to read.

More importantly, the logic is now subtly split across two passes: one pass computes reps, another pass decides whether to warn. That separation makes it easier to accidentally change one without updating the other.

A single-pass approach can compute both sets and usesRepRanges together, or at least reuse fixedReps computed earlier.

• Maintainability | src/tools/routines.test.ts:241-382
  The new tests introduce substantial duplication (same routine scaffold, same hevyClient mock setup, same registration/handler fetch). This makes the suite harder to maintain and increases the cost of future changes to the tool contract.

These are good scenarios to cover, but they can be expressed with a shared helper (e.g., setupCreateRoutine() / setupUpdateRoutine()) and/or table-driven tests for the cases (reps omitted), (reps null), (reps provided).

Also, both new create-routine tests cast the handler input to Record<string, unknown>, which weakens the test’s ability to catch shape regressions. Even if you keep the cast, consider centralizing it in a helper so you’re not repeating it everywhere.

Summary of changes

What changed

✅ Routine set mapping now preserves/infers reps

• In both create-routine and update-routine, set mapping no longer forces reps to null when repRange is present.
• Added a helper getFixedRepsFromRepRange() to infer reps when repRange is fixed (start === end).
• Mapping logic now computes reps as:
  • set.reps when it’s a number, otherwise
  • inferred from a fixed repRange, otherwise
  • null

⚠️ Rep-range warning logic refined

• The “rep ranges may not display” warning is now only emitted for non-fixed rep ranges.

🧪 Tests expanded

• Updated existing expectations to reflect the new behavior (e.g., reps: 10 is retained).
• Added coverage for:
  • inferring reps from fixed repRange when reps is omitted or null (create + update)
  • ensuring non-fixed repRange keeps reps: null and triggers warning

[greptile-apps]

Greptile Summary

This PR fixes a bug where reps was incorrectly forced to null whenever repRange was provided in routine creation/update requests, causing Hevy apps to not display rep counts. The fix preserves user-supplied reps alongside repRange, and for fixed ranges (e.g. 8–8), automatically infers reps from the range when not explicitly provided. The rep-range display warning is now only shown for non-fixed ranges.

• reps is no longer overwritten to null when repRange is set — user-provided reps values are preserved
• New getFixedRepsFromRepRange() helper backfills reps from fixed rep ranges (where start equals end) when reps is omitted or null
• Rep-range display warning is suppressed for fixed ranges since they behave identically to plain reps in the Hevy app
• Logic changes are applied consistently in both create-routine and update-routine handlers
• 4 new test cases cover the fixed/non-fixed repRange scenarios for both create and update paths

Confidence Score: 5/5

• This PR is safe to merge — it fixes a clear bug with well-tested, consistent logic changes across both routine handlers.
• The changes are focused and well-scoped: a single helper function and consistent updates to two parallel code paths. All edge cases (reps omitted, reps null, reps provided, fixed range, non-fixed range, no range) produce correct results. The logic aligns with the generated API types. Test coverage is thorough, and the PR description explicitly acknowledges known out-of-scope cleanup items. No security or correctness concerns.
• No files require special attention

Important Files Changed

Filename	Overview
src/tools/routines.ts	Adds getFixedRepsFromRepRange helper and updates both create/update set mapping to preserve reps when repRange is set, backfilling from fixed ranges. Warning condition refined to skip fixed rep ranges. Logic is correct and consistent across both handlers.
src/tools/routines.test.ts	Adds 4 new test cases covering fixed repRange reps inference (omitted and null) for both create and update handlers, plus assertions for non-fixed repRange reps/warning behavior. Existing tests updated to match new behavior. Good coverage of the changed logic.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Input: set.reps, set.repRange] --> B{buildRepRange}
    B --> C{repRange result}
    C -->|null| D[fixedReps = null]
    C -->|non-null| E{getFixedRepsFromRepRange}
    E -->|start == end| F[fixedReps = start]
    E -->|start != end or partial nulls| D
    D --> G{typeof set.reps === number?}
    F --> G
    G -->|yes| H[reps = set.reps]
    G -->|no| I[reps = fixedReps ?? null]
    H --> J[Build API request set]
    I --> J
    J --> K{Check warning condition}
    K -->|rep_range != null AND not fixed| L[usesRepRanges = true]
    K -->|rep_range is null or fixed| M[No warning]
    L --> N[Append rep-range display warning]
    M --> O[Return response]
    N --> O

Loading

_{Last reviewed commit: aa2dd87}

[mieubrisse]

would love a merge here @chrisdoc !