Fix count tokens #254
Merged
Fix count tokens #254
Conversation
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
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Status |
|---|---|---|
 |
Over-broad Exception Handling ▹ view |
Files scanned
| File Path | Reviewed |
|---|---|
| src/agentlab/llm/llm_utils.py | ✅ |
Explore our documentation to understand the languages and file types we support and the files we ignore.
Check out our docs on how you can make Korbit work best for you and your team.
Comment on lines
189
to
+192
| try: | ||
| return AutoTokenizer.from_pretrained(model_name) | ||
| except OSError: | ||
| logging.info(f"Could not find a tokenizer for model {model_name}. Defaulting to gpt-4.") | ||
| except Exception as e: | ||
| logging.info(f"Could not find a tokenizer for model {model_name}: {e} Defaulting to gpt-4.") |
There was a problem hiding this comment.
Over-broad Exception Handling 
Tell me more
What is the issue?
Using a bare Exception catch is too broad and could mask critical errors that should be handled differently.
Why this matters
This could catch and ignore serious issues like memory errors or import errors that require different handling, potentially making debugging more difficult.
Suggested change ∙ Feature Preview
Catch specific exceptions that are expected in tokenizer loading:
try:
return AutoTokenizer.from_pretrained(model_name)
except (OSError, ValueError) as e:
logging.info(f"Could not find a tokenizer for model {model_name}: {e} Defaulting to gpt-4.")Provide feedback to improve future suggestions
💬 Looking for more details? Reply to this comment to chat with Korbit.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Changes:
How to reproduce the error
Error that this PR addresses
Description by Korbit AI
What change is being made?
Update the exception handling in
get_tokenizerto catch all exceptions and log the specific error message, then default to "gpt-4".Why are these changes being made?
The change aims to improve the robustness and clarity of the error handling by logging the specific exception encountered when a tokenizer cannot be found, ensuring that unexpected issues are detected and reported more precisely. This approach guarantees fallback to a known default model while maintaining detailed logging information for potential troubleshooting.