fix(workouts): omit routine_id from create/update - #229
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #229 +/- ##
==========================================
+ Coverage 64.25% 64.40% +0.15%
==========================================
Files 13 13
Lines 414 413 -1
Branches 130 128 -2
==========================================
Hits 266 266
Misses 99 99
+ Partials 49 48 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
✨ 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
There was a problem hiding this comment.
Main risk is contract ambiguity: tests still pass routineId even though the tool schema removed it, so it’s unclear whether the tool should reject unknown keys or silently ignore/strip them. The Kubb wrapper fix is correct but suggests a maintainability gap—wrapper signatures can drift from generated APIs unless you mechanically align them via Parameters<>/typeof. Minor correctness/behavior concern: description || null converts empty strings to null; use description ?? null if you want to preserve intentional empty-string input.
Additional notes (2)
- Maintainability |
src/tools/workouts.test.ts:203-203
The tests now includeroutineIdin args, butcreateWorkoutSchemano longer accepts it. If the handler is wired to validate using this Zod schema (likely), this test may now be unintentionally exercising the validation/unknown-key stripping behavior rather than explicitly asserting thatroutineIdis rejected/ignored at the tool boundary.
Right now, the test asserts routine_id is omitted from the outbound payload, which is good—but it doesn’t clearly assert whether routineId is rejected (error) or silently ignored (strip/passthrough). If the intended contract is “routineId is not part of the tool API anymore”, it’s better to assert that providing it fails validation (or at least that it is stripped before handler logic).
- Readability |
src/tools/workouts.ts:188-197
workoutPayloadis now typed asNonNullable<PostWorkoutsRequestBody["workout"]>, which is a good guardrail. However, the code still acceptsdescriptionasoptional().nullable()and then usesdescription || null, which will convert an empty string ("") intonull.
That might be fine, but it’s a behavior change from “send empty string if provided” to “drop to null”, and it’s not obviously intended. If the API distinguishes between empty-string vs null (or you want to preserve user input), this should be description ?? null.
Summary of changes
What changed
-
Workouts tool payloads
- Removed
routineIdfrom thecreate-workoutandupdate-workoutZod schemas. - Stopped sending
workout.routine_idin the/v1/workoutsrequest body. - Added a stricter payload type:
NonNullable<PostWorkoutsRequestBody["workout"]>to reduce the chance of reintroducing unsupported fields.
- Removed
-
Tests
- Updated
create-workouttests to assertroutine_idis not present in the outbound payload. - Refactored stdout pollution test to use
vi.spyOn(process.stdout, "write")instead of manual reassignment. - Tweaked JSON env var parsing test to avoid non-null assertions.
- Updated
-
Kubb client wrapper
- Fixed
createExerciseTemplatewrapper argument ordering to matchapi.postV1ExerciseTemplates(data, headers, { client }).
- Fixed
| createExerciseTemplate: ( | ||
| data: PostV1ExerciseTemplatesMutationRequest, | ||
| ): ReturnType<typeof api.postV1ExerciseTemplates> => | ||
| api.postV1ExerciseTemplates(headers, data, { client }), | ||
| api.postV1ExerciseTemplates(data, headers, { client }), |
There was a problem hiding this comment.
This change correctly fixes the wrapper call signature, but it also highlights a bigger risk: these hand-written wrapper methods can silently drift from the generated Kubb client signatures.
Given this caused a real bug, consider adding a small compile-time guard around these wrappers—e.g., by typing the wrapper function as typeof api.postV1ExerciseTemplates (or using Parameters<>/ReturnType<>) so argument order mismatches are caught immediately during implementation.
Suggestion
Make wrapper signatures mechanically follow the generated client to prevent arg-order regressions. For example:
createExerciseTemplate: (
...args: Parameters<typeof api.postV1ExerciseTemplates>
): ReturnType<typeof api.postV1ExerciseTemplates> =>
api.postV1ExerciseTemplates(...args, { client }),If you need to inject headers consistently, you can still use Parameters<> for the other args (e.g., data/params) and keep the wrapper aligned.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit implementing a safer typing pattern for this and similar wrapper methods.
Hevy's /v1/workouts API rejects routine_id in the request body. Also fixes a mismatched arg order in the Kubb client wrapper and cleans up a couple of Biome violations in unit tests.
38d1477 to
a76bdcc
Compare
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| const workoutPayload: NonNullable<PostWorkoutsRequestBody["workout"]> = { | ||
| title, | ||
| description: description || null, | ||
| description: description ?? null, |
There was a problem hiding this comment.
Empty string description handling behavior change
Low Severity
The change from description || null to description ?? null alters behavior for empty string inputs. Previously, an empty string "" would be converted to null (since || treats empty strings as falsy). Now, empty strings are passed through unchanged (since ?? only checks for null/undefined). If the Hevy API expects null for "no description" but receives "" instead, this could cause unexpected behavior. This change affects both create-workout and update-workout operations.
Additional Locations (1)
…sions Addresses PR review comment about wrapper signatures drifting from generated Kubb client signatures. The wrapApi helper uses Parameters<> to enforce compile-time type checking on API call argument order.
3bd68b3 to
ee3a094
Compare
All wrapper methods now use wrapApi() to enforce compile-time type checking, preventing arg-order regressions across the entire client.
## [1.18.11](v1.18.10...v1.18.11) (2026-01-08) ### Bug Fixes * **workouts:** omit routine_id from create/update ([#229](#229)) ([bca641a](bca641a))
Fixes the Hevy API 400 error where
routine_idwas being sent (often asnull) in the/v1/workoutsrequest body. The Hevy API schema rejects this field (\"workout.routine_id\" is not allowed).Changes
routineIdfrom thecreate-workoutandupdate-workouttool schemas and stopped sendingworkout.routine_idin payloads.workoutpayload asPostWorkoutsRequestBody[\"workout\"]so invalid request fields don’t creep back in.createExerciseTemplateinsrc/utils/hevyClientKubb.tsto match the generated Kubb client signature.Verification
HEVY_API_KEY, which isn’t available in the devbox.reviewChanges skipped:
routineIdfrom tool schemas — intentional; the upstream API rejectsroutine_id, so the tool contract now matches reality.hevyClientKubbwrapper argument ordering — out of scope for this fix.Closes #228.
✨ PR Description
Purpose: Fix API schema validation errors by removing unsupported routine_id field from workout create and update operations to align with Hevy API specifications.
Main changes:
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
Note
Fixes Hevy workout create/update payloads and hardens the generated client call signatures.
routineIdfromcreate-workout/update-workoutschemas and stops sendingworkout.routine_id; types outgoingworkoutasPostWorkoutsRequestBody["workout"]and normalizes nullable fieldswrapApihelper and refactorssrc/utils/hevyClientKubb.tscalls to match generated Kubb signatures; correctspostV1ExerciseTemplatesarg orderroutine_idis omitted in create payloads and improves stdout mocking intests/unit/index.test.tsWritten by Cursor Bugbot for commit a78951f. This will update automatically on new commits. Configure here.