Skip to content

fix: initialize max_tokens before try block in _calc_claude_tokens #2269

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wishhyt wants to merge 1 commit into
base: main
Choose a base branch
from
+2 −1
Open

fix: initialize max_tokens before try block in _calc_claude_tokens #2269

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pr_agent/algo/token_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,11 @@ def _get_system_user_tokens(self, pr, encoder, vars: dict, system, user):
return 0

def _calc_claude_tokens(self, patch: str) -> int:
max_tokens = 0
try:
Comment on lines 99 to 101
Copy link
Copy Markdown
Contributor

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. No tests for _calc_claude_tokens 📘 Rule violation ⛯ Reliability

This PR changes runtime error-handling behavior in _calc_claude_tokens (preventing a NameError
and altering fallback behavior), but it does not add/update pytest coverage for these scenarios.
Without tests, regressions in the Anthropic-token-counting fallback path may go undetected.
Agent Prompt 
## Issue description
This PR changes `_calc_claude_tokens` error-path behavior, but there are no pytest tests added/updated to ensure the fallback behavior remains correct.

## Issue Context
The function dynamically imports `anthropic` and reads `MAX_TOKENS` and the configured model; exceptions in these steps are expected to return a safe fallback (`max_tokens`) without masking the original failure.

## Fix Focus Areas
- pr_agent/algo/token_handler.py[99-127]
- tests/unittest/test_token_handler.py[1-200]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

import anthropic
from pr_agent.algo import MAX_TOKENS

client = anthropic.Anthropic(api_key=get_settings(use_context=False).get('anthropic.key'))
max_tokens = MAX_TOKENS[get_settings().config.model]
Comment on lines 99 to 106
Copy link
Copy Markdown
Contributor

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Zero tokens on failure 🐞 Bug ✓ Correctness

_calc_claude_tokens now returns 0 tokens when an exception occurs before max_tokens is
assigned (e.g., client initialization failure or MAX_TOKENS[...] KeyError). Returning 0 for
non-empty input can cause downstream code that trims/clips based on token counts to skip trimming
and attempt oversized requests.
Agent Prompt 
## Issue description
`_calc_claude_tokens` initializes `max_tokens` to `0` and returns it on exceptions that occur before `max_tokens` is assigned from `MAX_TOKENS[...]`. This can produce a 0 token count for non-empty text, which can bypass downstream clipping decisions.

## Issue Context
- Exceptions can occur before `max_tokens = MAX_TOKENS[...]` (e.g., Claude-like model name not in `MAX_TOKENS`, or client init failures).
- Downstream code (e.g., help docs prompt trimming) uses the returned token count to decide whether to clip input.

## Fix Focus Areas
- pr_agent/algo/token_handler.py[99-127]

## Implementation notes
- Prefer computing `max_tokens` *before* constructing the Anthropic client.
- Replace `MAX_TOKENS[model]` with a safe fallback:
  - `MAX_TOKENS.get(model)` and/or `get_settings().config.custom_model_max_tokens`.
  - If neither is available, either raise (consistent with `get_max_tokens`) or fallback to a local estimate like `len(self.encoder.encode(patch, disallowed_special=()))`.
- In the `except`, return the conservative `max_tokens`/estimate (not 0) so downstream trimming remains safe.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Expand Down