Skip to content

fix: use pr_url variable instead of literal "pr_url" for provider cache lookup#2257

Open
karesansui-u wants to merge 1 commit intoqodo-ai:mainfrom
karesansui-u:fix/provider-cache-key-literal
Open

fix: use pr_url variable instead of literal "pr_url" for provider cache lookup#2257
karesansui-u wants to merge 1 commit intoqodo-ai:mainfrom
karesansui-u:fix/provider-cache-key-literal

Conversation

@karesansui-u
Copy link
Copy Markdown

Bug description

get_git_provider_with_context() stores the provider with the actual URL as key but retrieves it using the literal string "pr_url":

# Store (L64) — uses variable value as key
context["git_provider"] = {pr_url: git_provider}

# Retrieve (L52-53) — uses literal string "pr_url" as key
if ... context.get("git_provider", {}).get("pr_url", {}):
    git_provider = context["git_provider"]["pr_url"]

The cache never hits because "pr_url" (the string) never matches the actual PR URL stored as the key.

Impact

In server mode (GitHub App, etc.), a new GitProvider is created for every tool invocation on the same PR. This increases API calls to GitHub/GitLab and makes it easier to hit rate limits.

Fix

Use the pr_url variable instead of the "pr_url" literal on lines 52-53.

Affected files

  • pr_agent/git_providers/__init__.py (L51-53) — 2 line change

The provider cache stores entries with the actual PR URL as key
({pr_url: git_provider}) but retrieves them using the literal string
"pr_url". This means the cache never hits, and a new GitProvider is
created for every tool invocation on the same PR.
@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Fix provider cache lookup to use pr_url variable

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Fix provider cache lookup using actual PR URL variable instead of literal string
• Prevents unnecessary GitProvider recreation on every tool invocation
• Reduces redundant API calls to GitHub/GitLab in server mode
Diagram
flowchart LR
  A["Cache Storage<br/>pr_url variable as key"] -- "Before: literal 'pr_url' key" --> B["Cache Miss<br/>New provider created"]
  A -- "After: pr_url variable key" --> C["Cache Hit<br/>Provider reused"]
Loading

Grey Divider

File Changes

1. pr_agent/git_providers/__init__.py 🐞 Bug fix +3/-3

Fix cache key lookup to use variable instead of literal

• Changed cache lookup from literal string "pr_url" to variable pr_url on line 52
• Updated cache access from context["git_provider"]["pr_url"] to context["git_provider"][pr_url]
 on line 53
• Removed unnecessary default empty dict parameter in .get() call
• Updated comment to reflect correct cache key format

pr_agent/git_providers/init.py


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects bot commented Mar 16, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant