fix: resolve voice ID detection and flag handling bugs #7
Conversation
Voice IDs were not being recognized as IDs when they contained only letters because the detection logic required at least one digit. This caused such IDs to be treated as voice names, triggering an unnecessary API call and falling back to the first available voice. Changed the detection logic to check for: - Length >= 15 characters (typical voice ID length) - No spaces (voice names typically contain spaces) This correctly identifies voice IDs regardless of their character composition. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
The --voice flag was using a variable reference (opts.voiceID) as its default value instead of an empty string literal. This could cause issues with cobra's flag parsing when both --voice-id and --voice flags point to the same variable. Changed to use "" for consistency with the --voice-id flag. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
Previously, --speed validation happened before --rate processing, which caused false validation errors when both flags were provided. For example, `--speed 0.3 --rate 200` would fail even though --rate 200 (speed 1.14) is valid and would override the invalid --speed value. Changed to: - Process and validate --rate first if provided - Only validate --speed if --rate was not provided This matches the intended behavior where --rate overrides --speed. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 152505be79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // If input looks like an ID (alphanumeric >= 15 chars without spaces), use directly. | ||
| if len(voiceInput) >= 15 && !strings.ContainsRune(voiceInput, ' ') { | ||
| return voiceInput, nil |
There was a problem hiding this comment.
The new heuristic treats any input with length ≥15 and no spaces as a voice ID, skipping the name lookup entirely. This regresses cases where a valid voice name is a single long token (e.g., a custom name like “LongSingleWordName”), which previously would be resolved via ListVoices but now is sent directly as an ID and will fail or pick the wrong voice. Consider tightening the ID check to a specific ID pattern (e.g., UUID/known charset) or keeping name lookup for non-matching IDs.
Useful? React with 👍 / 👎.
1 participant
Summary
Fix an issue preventing voice IDs without integers from being recognized, plus two minor flag validation fixes.
Bug Fixes
1. Voice ID detection without digits
Problem: Voice IDs containing only letters were not recognized as IDs because the detection logic required at least one digit. This caused such IDs to be treated as voice names, triggering an unnecessary API call and falling back to the first available voice instead of using the specified ID.
Solution: Changed detection logic to identify voice IDs by:
This correctly identifies voice IDs regardless of their character composition.
2. Flag default value inconsistency
Problem: The
--voiceflag used a variable reference (opts.voiceID) as its default value instead of an empty string literal. This could cause issues with cobra's flag parsing when both--voice-idand--voiceflags point to the same variable.Solution: Changed to use
""for consistency with the--voice-idflag.3. Speed vs rate validation order
Problem:
--speedvalidation happened before--rateprocessing, causing false validation errors when both flags were provided. For example,--speed 0.3 --rate 200would fail validation even though--rate 200(which maps to speed 1.14) is valid and would override the invalid--speedvalue.Solution: Process and validate
--ratefirst if provided, and only validate--speedif--ratewas not provided. This matches the intended behavior where--rateoverrides--speed.