Skip to content

refactor(litellm_ai_handler): deduplicate config coercion, API key capture, and AWS region resolution - #3320

Merged
IsmaelMartinez merged 6 commits into
The-PR-Agent:mainfrom
high-tail:fix/refactoring-litellm_ai_handler
Sep 13, 2026
Merged

IsmaelMartinez merged 6 commits into
The-PR-Agent:mainfrom
high-tail:fix/refactoring-litellm_ai_handler

Conversation

@high-tail

@high-tail high-tail commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

What

Refactoring for pr_agent/algo/ai_handlers/litellm_ai_handler.py.
Duplication removal and extraction.

Changes

  • Deleted duplicated function (_as_bool) and move (_as_int and _as_list) as class-wise helper
  • Extract api key fetcher as helper
  • Extract aws region setting as helper
  • Extract watsonx envs as constant

Unit Test and Lint

PYTHONPATH=. uv run pytest tests/unittest
7745 passed, 33 skipped, 1 xfailed, 829 warnings in 56.98s
uv run pre-commit run --files pr_agent/algo/ai_handlers/litellm_ai_handler.py
check for added large files..............................................Passed
check python ast.........................................................Passed
check for case conflicts.................................................Passed
check json...........................................(no files to check)Skipped
check for merge conflicts................................................Passed
check toml...........................................(no files to check)Skipped
check yaml...........................................(no files to check)Skipped
check illegal windows names..........................(no files to check)Skipped
check for broken symlinks............................(no files to check)Skipped
debug statements (python)................................................Passed
detect destroyed symlinks................................................Passed
fix end of files.........................................................Passed
mixed line ending........................................................Passed
trim trailing whitespace.................................................Passed
Lint GitHub Actions workflow files...................(no files to check)Skipped
ruff check...............................................................Passed
uv run ruff check --fix pr_agent/algo/ai_handlers/litellm_ai_handler.py      
All checks passed!

… scope

Move the _as_int and _as_list helpers out of the OpenRouter extra-body builder and next to the existing module-level _as_bool, dropping the duplicated nested _as_bool with passing default=True
@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Deduplicate LiteLLM configuration and credential resolution

✨ Enhancement 🕐 10-20 Minutes

Grey Divider

AI Description

• Centralizes OpenRouter config coercion to remove nested duplicate parsers.
• Reuses API-key and AWS-region resolution across provider request paths.
• Shares WatsonX routing environment aliases between both WatsonX providers.
Diagram

graph TD
  CFG["Operator Config"] --> COERCE["Coercion Helpers"] --> REQUEST["Request Assembly"]
  ENV["Environment"] --> WATSON["WatsonX Routes"] --> SNAP["Provider Snapshots"] --> REQUEST
  CFG --> AWS["AWS Region"] --> SNAP
  ENV --> AWS
  CFG --> KEYS["API Key Capture"] --> REQUEST
  ENV --> KEYS
Loading
High-Level Assessment

The helper-extraction approach is appropriate because it removes duplication while preserving existing precedence and request behavior. A broader provider-configuration abstraction would add unnecessary scope and risk for these localized repetitions.

Files changed (1) +51 / -61

Refactor (1) +51 / -61
litellm_ai_handler.pyCentralize LiteLLM configuration and credential helpers +51/-61

Centralize LiteLLM configuration and credential helpers

• Hoists integer and list coercion beside the existing boolean parser and reuses them for OpenRouter controls. Extracts shared provider API-key capture and AWS-region resolution, while consolidating duplicate WatsonX routing aliases into one constant.

pr_agent/algo/ai_handlers/litellm_ai_handler.py

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Unused integer parser adds dead code ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
_as_int is introduced at module scope but has no call sites in the handler or elsewhere in the
repository. Because the refactor only uses the hoisted _as_list and _as_bool helpers, later
readers may incorrectly assume integer coercion remains part of the request-control path.
Code

pr_agent/algo/ai_handlers/litellm_ai_handler.py[R1896-1899]

+def _as_int(value) -> int:
+    """Parse a config value that may arrive as an int (toml) or a string (env override)."""
+    try:
+        return int(value)
Evidence
Compliance rule 2 requires every submitted code section to be active and necessary. The added
_as_int definition is present on the PR branch, while repository-wide inspection shows no
invocation of this helper.

Rule 2: No Dead or Commented-Out Code
pr_agent/algo/ai_handlers/litellm_ai_handler.py[1896-1901]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The newly hoisted `_as_int` helper has no call sites and leaves unnecessary dead code in the handler module.
## Fix Focus Areas
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[1896-1901]
## Recommended Fix
Remove `_as_int` from the module unless integer coercion is still required; if it is required, connect the helper to the intended configuration-processing call site and add coverage for that behavior.

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


  • Author self-review: I have reviewed the code review findings, and addressed the relevant ones.

Grey Divider

Tip of the day
💡 Did you know, you can start a comment with 'qodo' or '@qodo' to chat about any finding

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

…lm_ai_handler

Fix conflict by accepting our changes
Conflicts: pr_agent/algo/ai_handlers/litellm_ai_handler.py
Comment thread pr_agent/algo/ai_handlers/litellm_ai_handler.py Outdated
@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit f4fe817

the latest main branch no more used this function
@qodo-free-for-open-source-projects

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 0d3fe3a

@high-tail

Copy link
Copy Markdown
Contributor Author

Even after merging the latest main branch all test still passible

PYTHONPATH=. uv run pytest tests/unittest
7829 passed, 33 skipped, 1 xfailed, 829 warnings in 51.22s
uv run ruff check --fix pr_agent/algo/ai_handlers/litellm_ai_handler.py 
All checks passed!
uv run pre-commit run --files pr_agent/algo/ai_handlers/litellm_ai_handler.py
check for added large files..............................................Passed
check python ast.........................................................Passed
check for case conflicts.................................................Passed
check json...........................................(no files to check)Skipped
check for merge conflicts................................................Passed
check toml...........................................(no files to check)Skipped
check yaml...........................................(no files to check)Skipped
check illegal windows names..........................(no files to check)Skipped
check for broken symlinks............................(no files to check)Skipped
debug statements (python)................................................Passed
detect destroyed symlinks................................................Passed
fix end of files.........................................................Passed
mixed line ending........................................................Passed
trim trailing whitespace.................................................Passed
Lint GitHub Actions workflow files...................(no files to check)Skipped
ruff check...............................................................Passed

@IsmaelMartinez IsmaelMartinez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the tidy-up, and for folding Qodo's _as_int point into the same commit.

Checked behaviour rather than shape: the OpenRouter controls, the AWS credential precedence and the per-provider key capture give identical results on main and on this branch across every input combination I could construct, and the full suite is exactly baseline.

Approving now that CI has run.

@IsmaelMartinez
IsmaelMartinez merged commit bd70782 into The-PR-Agent:main Sep 13, 2026
5 checks passed
@high-tail

Copy link
Copy Markdown
Contributor Author

Thank you so much for your review and approve!

@high-tail
high-tail deleted the fix/refactoring-litellm_ai_handler branch September 21, 2026 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants