fix: use int type for RoutineFolder ID fields#1
Open
Conversation
The Hevy API returns routine folder IDs as JSON numbers, but the Go structs declared them as strings, causing json.Unmarshal to fail with "cannot unmarshal number into Go struct field RoutineFolder.id of type string". Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis change converts folder ID handling from strings to integers across the codebase, updating the API type definitions, client method signatures, and CLI command parsing and output formatting to enforce numeric ID representation consistently. Changes
Poem
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/folder/get.go (1)
23-24:⚠️ Potential issue | 🟡 MinorUpdate
folder getexamples to numeric IDsThese examples still show string IDs, which conflicts with the new integer-only parsing and will mislead users.
Suggested patch
- hevycli folder get abc123-def456 # Get folder by ID - hevycli folder get abc123 -o json # Output as JSON`, + hevycli folder get 123456 # Get folder by ID + hevycli folder get 123456 -o json # Output as JSON`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/folder/get.go` around lines 23 - 24, Update the command examples in the folder get help text so they use numeric IDs instead of string UUIDs (e.g., change "hevycli folder get abc123-def456" to "hevycli folder get 123" and "hevycli folder get abc123 -o json" to "hevycli folder get 123 -o json"); locate and edit the example/usage string in cmd/folder/get.go (the folder get command help/usage variable or function that builds the examples) so all example invocations reflect integer-only IDs.
🧹 Nitpick comments (2)
cmd/folder/delete.go (1)
90-90: Don't drop the interactive conversion error before delete.If
selected.IDis ever malformed, Line 90 falls back to0and carries that value into the fetch/delete path. It's safer to fail fast here.Suggested fix
- folderID, _ = strconv.Atoi(selected.ID) + folderID, err = strconv.Atoi(selected.ID) + if err != nil { + return fmt.Errorf("invalid folder ID: %s", selected.ID) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/folder/delete.go` at line 90, The code currently ignores strconv.Atoi errors by doing folderID, _ = strconv.Atoi(selected.ID); instead, check the error returned from strconv.Atoi when converting selected.ID to folderID and fail fast (return the error or call the command's error/exit path) rather than defaulting to 0; update the conversion in the delete command so it validates selected.ID, logs/returns a clear error mentioning selected.ID, and stops before calling the fetch/delete logic that uses folderID.cmd/folder/update.go (1)
84-84: Don't drop the interactive conversion error.
prompt.SelectOption.IDis still a string. If it ever stops being numeric, Line 84 silently turns that intofolderID == 0before the update call. Bubble the parse failure up here instead.Suggested fix
- folderID, _ = strconv.Atoi(selected.ID) + folderID, err = strconv.Atoi(selected.ID) + if err != nil { + return fmt.Errorf("invalid folder ID: %s", selected.ID) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/folder/update.go` at line 84, The current code silently ignores strconv.Atoi errors when converting selected.ID to folderID (folderID, _ = strconv.Atoi(selected.ID)); change this to capture and handle the error instead: use folderID, err := strconv.Atoi(selected.ID) and if err != nil return or propagate the error (or wrap it with context) before proceeding to the Update call so non-numeric IDs don't become 0; update references in cmd/folder/update.go around the selected.ID conversion and the surrounding function (where folderID is used) to properly bubble the parse failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/folder/get.go`:
- Line 74: The code currently ignores the error from strconv.Atoi(selected.ID)
causing a silent fallback to 0; update the conversion to capture the error
(e.g., folderID, err := strconv.Atoi(selected.ID)) and handle it by returning or
logging a clear error (for example return fmt.Errorf("invalid folder ID %q: %w",
selected.ID, err) or printing and exiting), ensuring the calling function (where
selected and folderID are used) treats the conversion failure as an actual error
rather than proceeding with 0.
---
Outside diff comments:
In `@cmd/folder/get.go`:
- Around line 23-24: Update the command examples in the folder get help text so
they use numeric IDs instead of string UUIDs (e.g., change "hevycli folder get
abc123-def456" to "hevycli folder get 123" and "hevycli folder get abc123 -o
json" to "hevycli folder get 123 -o json"); locate and edit the example/usage
string in cmd/folder/get.go (the folder get command help/usage variable or
function that builds the examples) so all example invocations reflect
integer-only IDs.
---
Nitpick comments:
In `@cmd/folder/delete.go`:
- Line 90: The code currently ignores strconv.Atoi errors by doing folderID, _ =
strconv.Atoi(selected.ID); instead, check the error returned from strconv.Atoi
when converting selected.ID to folderID and fail fast (return the error or call
the command's error/exit path) rather than defaulting to 0; update the
conversion in the delete command so it validates selected.ID, logs/returns a
clear error mentioning selected.ID, and stops before calling the fetch/delete
logic that uses folderID.
In `@cmd/folder/update.go`:
- Line 84: The current code silently ignores strconv.Atoi errors when converting
selected.ID to folderID (folderID, _ = strconv.Atoi(selected.ID)); change this
to capture and handle the error instead: use folderID, err :=
strconv.Atoi(selected.ID) and if err != nil return or propagate the error (or
wrap it with context) before proceeding to the Update call so non-numeric IDs
don't become 0; update references in cmd/folder/update.go around the selected.ID
conversion and the surrounding function (where folderID is used) to properly
bubble the parse failure.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4070201e-2551-4779-9c2c-47eadb69c920
📒 Files selected for processing (9)
cmd/folder/create.gocmd/folder/delete.gocmd/folder/get.gocmd/folder/list.gocmd/folder/update.gocmd/routine/get.gocmd/routine/list.gointernal/api/client.gointernal/api/types.go
| return err | ||
| } | ||
| folderID = selected.ID | ||
| folderID, _ = strconv.Atoi(selected.ID) |
There was a problem hiding this comment.
Handle selected.ID conversion errors instead of discarding them
Ignoring Atoi errors can silently coerce to 0 and produce misleading “not found” behavior if ID formatting changes later.
Suggested patch
- folderID, _ = strconv.Atoi(selected.ID)
+ var err error
+ folderID, err = strconv.Atoi(selected.ID)
+ if err != nil {
+ return fmt.Errorf("invalid selected folder ID: %q", selected.ID)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| folderID, _ = strconv.Atoi(selected.ID) | |
| var err error | |
| folderID, err = strconv.Atoi(selected.ID) | |
| if err != nil { | |
| return fmt.Errorf("invalid selected folder ID: %q", selected.ID) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@cmd/folder/get.go` at line 74, The code currently ignores the error from
strconv.Atoi(selected.ID) causing a silent fallback to 0; update the conversion
to capture the error (e.g., folderID, err := strconv.Atoi(selected.ID)) and
handle it by returning or logging a clear error (for example return
fmt.Errorf("invalid folder ID %q: %w", selected.ID, err) or printing and
exiting), ensuring the calling function (where selected and folderID are used)
treats the conversion failure as an actual error rather than proceeding with 0.
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
1 participant
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
RoutineFolder.IDfromstringtointandRoutine.FolderIDfrom*stringto*intto match the Hevy API response formatjson: cannot unmarshal number into Go struct field RoutineFolder.routine_folders.id of type stringTest plan
go build ./...passesgo test ./...passeshevycli folder listno longer errorshevycli folder get <id>,update,deletework with integer IDs🤖 Generated with Claude Code
Summary by CodeRabbit