Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .pr_agent.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
[pr_reviewer]
enable_review_labels_effort = true
enable_auto_approval = true
cache_tickets = true

[github_app]
pr_commands = [
Expand Down
12 changes: 12 additions & 0 deletions pr_agent/mosaico/env_bridge.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ def apply_mosaico_env() -> None:
if model_name:
model = model_name if "/" in model_name else f"openai/{model_name}"
settings.set("CONFIG.MODEL", model)
# Dynaconf merge_enabled=True appends list values instead of replacing;
# pop the leaf first so the subsequent .set() produces a fresh list.
# Use case-insensitive matching because Dynaconf can normalize key casing.
for _k in list(settings.CONFIG.keys()):
if _k.lower() == "fallback_models":
settings.CONFIG.pop(_k, None)
settings.set("CONFIG.FALLBACK_MODELS", [])
# MOSAICO models are not in pr-agent's built-in MAX_TOKENS table; declare a budget
# so reviews don't fail with "not defined in MAX_TOKENS". Overridable via env.
Expand Down Expand Up @@ -68,6 +74,12 @@ def _register_langfuse_callback(settings) -> None:
current = [c for c in (settings.get(key, []) or []) if c != LEGACY_LANGFUSE_CALLBACK]
if LANGFUSE_CALLBACK_NAME not in current:
current.append(LANGFUSE_CALLBACK_NAME)
# Dynaconf merge_enabled=True appends list values; pop the leaf first.
# Use case-insensitive matching because Dynaconf can normalize key casing.
leaf = key.split(".", 1)[-1]
for _k in list(settings.LITELLM.keys()):
if _k.lower() == leaf.lower():
settings.LITELLM.pop(_k, None)
settings.set(key, current)
settings.set("LITELLM.ENABLE_CALLBACKS", True)
get_logger().info("MOSAICO: registered Langfuse litellm callback.")
1 change: 1 addition & 0 deletions pr_agent/settings/configuration.toml
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ require_security_review=true
require_estimate_contribution_time_cost=false
require_todo_scan=false
require_ticket_analysis_review=true
cache_tickets=true
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
# general options
publish_output_no_suggestions=true # Set to "false" if you only need the reviewer's remarks (not labels, not "security audit", etc.) and want to avoid noisy "No major issues detected" comments.
persistent_comment=true
Expand Down
19 changes: 18 additions & 1 deletion pr_agent/tools/pr_reviewer.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import copy
import datetime
import hashlib
import traceback
from collections import OrderedDict
from functools import partial
Expand Down Expand Up @@ -96,7 +97,7 @@ def __init__(self, pr_url: str, is_answer: bool = False, is_auto: bool = False,
"custom_labels": "",
"enable_custom_labels": get_settings().config.enable_custom_labels,
"is_ai_metadata": get_settings().get("config.enable_ai_metadata", False),
"related_tickets": get_settings().get('related_tickets', []),
"related_tickets": self._load_related_tickets(),
'duplicate_prompt_examples': get_settings().config.get('duplicate_prompt_examples', False),
"date": datetime.datetime.now().strftime('%Y-%m-%d'),
}
Expand All @@ -108,6 +109,22 @@ def __init__(self, pr_url: str, is_answer: bool = False, is_auto: bool = False,
get_settings().pr_review_prompt.user
)

def _load_related_tickets(self):
if not get_settings().get('pr_reviewer.cache_tickets', True):
return []
pr_url = getattr(self.git_provider, 'pr_url', None)
if pr_url:
cache_key = f'related_tickets_{hashlib.md5(pr_url.encode()).hexdigest()}'
from pr_agent.tools.ticket_pr_compliance_check import _tickets_cache, _TICKETS_CACHE_TTL
import time
cached = _tickets_cache.get(cache_key)
if cached is not None:
ts, tickets = cached
if time.time() - ts < _TICKETS_CACHE_TTL:
return tickets
del _tickets_cache[cache_key]
return get_settings().get('related_tickets', [])

def parse_incremental(self, args: List[str]):
is_incremental = False
if args and len(args) >= 1:
Expand Down
36 changes: 33 additions & 3 deletions pr_agent/tools/ticket_pr_compliance_check.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
import hashlib
import re
import time
import traceback

from pr_agent.config_loader import get_settings
from pr_agent.git_providers import GithubProvider
from pr_agent.git_providers import AzureDevopsProvider
from pr_agent.log import get_logger

# In-memory ticket cache with TTL to avoid unbounded Dynaconf key accumulation.
# Keys: md5(pr_url), values: (timestamp, ticket_list)
_tickets_cache: dict = {}
_TICKETS_CACHE_TTL = 3600 # 1 hour

# Compile the regex pattern once, outside the function
GITHUB_TICKET_PATTERN = re.compile(
r'(https://github[^/]+/[^/]+/[^/]+/issues/\d+)|(\b(\w+)/(\w+)#(\d+)\b)|(#\d+)'
Expand Down Expand Up @@ -244,7 +251,29 @@ async def extract_and_cache_pr_tickets(git_provider, vars):
if not get_settings().get('pr_reviewer.require_ticket_analysis_review', False):
return

related_tickets = get_settings().get('related_tickets', [])
pr_url = getattr(git_provider, 'pr_url', None)
if not pr_url:
get_logger().info("No PR URL available — cannot cache tickets per-PR")
related_tickets = []
use_cache = False
else:
use_cache = get_settings().get('pr_reviewer.cache_tickets', True)
cache_key = (
f'related_tickets_{hashlib.md5(pr_url.encode()).hexdigest()}'
)
if use_cache:
cached = _tickets_cache.get(cache_key)
if cached is not None:
ts, tickets = cached
if time.time() - ts < _TICKETS_CACHE_TTL:
related_tickets = tickets
else:
del _tickets_cache[cache_key]
related_tickets = []
else:
related_tickets = []
else:
related_tickets = []

if not related_tickets:
tickets_content = await extract_tickets(git_provider)
Expand All @@ -261,8 +290,9 @@ async def extract_and_cache_pr_tickets(git_provider, vars):
get_logger().info("Extracted tickets and sub-issues from PR description",
artifact={"tickets": related_tickets})

vars['related_tickets'] = related_tickets
get_settings().set('related_tickets', related_tickets)
vars['related_tickets'] = related_tickets
if use_cache:
_tickets_cache[cache_key] = (time.time(), related_tickets)
else:
get_logger().info("Using cached tickets", artifact={"tickets": related_tickets})
vars['related_tickets'] = related_tickets
Expand Down
12 changes: 8 additions & 4 deletions tests/unittest/_settings_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,14 @@ def _remove_key(settings, key):


def restore_settings(snapshot):
"""Restore ``snapshot``; truly remove entries whose snapshot is SENTINEL."""
"""Restore ``snapshot``; truly remove entries whose snapshot is SENTINEL.

Because ``merge_enabled=True`` causes ``settings.set()`` to *append* list
values rather than replace them, the key is always removed first via
``_remove_key`` before setting the restored value.
"""
settings = get_settings()
for key, value in snapshot.items():
if value is SENTINEL:
_remove_key(settings, key)
else:
_remove_key(settings, key)
if value is not SENTINEL:
settings.set(key, value)
13 changes: 10 additions & 3 deletions tests/unittest/test_ignore_repositories.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from pr_agent.servers.bitbucket_app import should_process_pr_logic as bitbucket_should_process_pr_logic
from pr_agent.servers.github_app import should_process_pr_logic as github_should_process_pr_logic
from pr_agent.servers.gitlab_webhook import should_process_pr_logic as gitlab_should_process_pr_logic
from tests.unittest._settings_helpers import _remove_key


def make_bitbucket_payload(full_name):
Expand Down Expand Up @@ -42,11 +43,15 @@ def make_gitlab_body(full_name):

class TestIgnoreRepositories:
def setup_method(self):
get_settings().set("CONFIG.IGNORE_REPOSITORIES", [])
settings = get_settings()
_remove_key(settings, "CONFIG.IGNORE_REPOSITORIES")
settings.set("CONFIG.IGNORE_REPOSITORIES", [])

@pytest.mark.parametrize("provider_name, provider_func, body_func", PROVIDERS)
def test_should_ignore_matching_repository(self, provider_name, provider_func, body_func):
get_settings().set("CONFIG.IGNORE_REPOSITORIES", ["org/repo-to-ignore"])
settings = get_settings()
_remove_key(settings, "CONFIG.IGNORE_REPOSITORIES")
settings.set("CONFIG.IGNORE_REPOSITORIES", ["org/repo-to-ignore"])
body = {
"pull_request": {},
"repository": {"full_name": "org/repo-to-ignore"},
Expand All @@ -58,7 +63,9 @@ def test_should_ignore_matching_repository(self, provider_name, provider_func, b

@pytest.mark.parametrize("provider_name, provider_func, body_func", PROVIDERS)
def test_should_not_ignore_non_matching_repository(self, provider_name, provider_func, body_func):
get_settings().set("CONFIG.IGNORE_REPOSITORIES", ["org/repo-to-ignore"])
settings = get_settings()
_remove_key(settings, "CONFIG.IGNORE_REPOSITORIES")
settings.set("CONFIG.IGNORE_REPOSITORIES", ["org/repo-to-ignore"])
body = {
"pull_request": {},
"repository": {"full_name": "org/other-repo"},
Expand Down
7 changes: 3 additions & 4 deletions tests/unittest/test_mosaico_env_bridge.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
langfuse_env_present)
from pr_agent.mosaico.observability import (mosaico_log_context,
parse_observability_metadata)
from tests.unittest._settings_helpers import _remove_key

_SNAPSHOT_KEYS = [
"OPENAI.API_BASE",
Expand Down Expand Up @@ -41,10 +42,8 @@ def restore_settings():
litellm_failure = litellm.failure_callback
yield
for k, v in snapshot.items():
if v is _SENTINEL:
# Key was absent before; best-effort reset to empty so the global is not polluted.
global_settings.set(k, [] if k.endswith("CALLBACK") or k == "CONFIG.FALLBACK_MODELS" else None)
else:
_remove_key(global_settings, k)
if v is not _SENTINEL:
global_settings.set(k, v)
litellm.success_callback = litellm_success
litellm.failure_callback = litellm_failure
Expand Down
72 changes: 45 additions & 27 deletions tests/unittest/test_retry_with_fallback_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from pr_agent.algo.pr_processing import retry_with_fallback_models
from pr_agent.algo.utils import ModelType
from pr_agent.config_loader import get_settings
from tests.unittest._settings_helpers import SENTINEL, restore_settings, snapshot_settings
from tests.unittest._settings_helpers import SENTINEL, restore_settings, snapshot_settings, _remove_key

_TRACKED_KEYS = (
"config.model",
Expand All @@ -28,10 +28,13 @@ def _restore_settings(snapshot):
def test_primary_model_success_invoked_once_and_returns_value():
snapshot = _snapshot_settings()
try:
get_settings().set("config.model", "primary-model")
get_settings().set("config.fallback_models", ["fallback-1", "fallback-2"])
get_settings().set("openai.deployment_id", None)
get_settings().set("openai.fallback_deployments", [])
settings = get_settings()
_remove_key(settings, "config.fallback_models")
settings.set("config.model", "primary-model")
settings.set("config.fallback_models", ["fallback-1", "fallback-2"])
_remove_key(settings, "openai.fallback_deployments")
settings.set("openai.deployment_id", None)
settings.set("openai.fallback_deployments", [])

calls = []

Expand All @@ -50,10 +53,13 @@ async def fake_f(model):
def test_primary_fails_fallback_succeeds():
snapshot = _snapshot_settings()
try:
get_settings().set("config.model", "primary-model")
get_settings().set("config.fallback_models", ["fallback-1", "fallback-2"])
get_settings().set("openai.deployment_id", None)
get_settings().set("openai.fallback_deployments", [])
settings = get_settings()
_remove_key(settings, "config.fallback_models")
settings.set("config.model", "primary-model")
settings.set("config.fallback_models", ["fallback-1", "fallback-2"])
_remove_key(settings, "openai.fallback_deployments")
settings.set("openai.deployment_id", None)
settings.set("openai.fallback_deployments", [])

calls = []

Expand All @@ -74,10 +80,13 @@ async def fake_f(model):
def test_all_models_fail_raises_with_aggregate_message_and_cause():
snapshot = _snapshot_settings()
try:
get_settings().set("config.model", "primary-model")
get_settings().set("config.fallback_models", ["fallback-1"])
get_settings().set("openai.deployment_id", None)
get_settings().set("openai.fallback_deployments", [])
settings = get_settings()
_remove_key(settings, "config.fallback_models")
settings.set("config.model", "primary-model")
settings.set("config.fallback_models", ["fallback-1"])
_remove_key(settings, "openai.fallback_deployments")
settings.set("openai.deployment_id", None)
settings.set("openai.fallback_deployments", [])

last_error = ValueError("last failure")
attempted = []
Expand All @@ -102,10 +111,13 @@ async def fake_f(model):
def test_deployment_id_updated_per_attempt():
snapshot = _snapshot_settings()
try:
get_settings().set("config.model", "primary-model")
get_settings().set("config.fallback_models", ["fallback-1", "fallback-2"])
get_settings().set("openai.deployment_id", "deployment-primary")
get_settings().set(
settings = get_settings()
_remove_key(settings, "config.fallback_models")
_remove_key(settings, "openai.fallback_deployments")
settings.set("config.model", "primary-model")
settings.set("config.fallback_models", ["fallback-1", "fallback-2"])
settings.set("openai.deployment_id", "deployment-primary")
settings.set(
"openai.fallback_deployments",
["deployment-fb1", "deployment-fb2"],
)
Expand Down Expand Up @@ -134,11 +146,14 @@ async def fake_f(model):
def test_weak_model_type_uses_weak_setting_and_forwards_identifier():
snapshot = _snapshot_settings()
try:
get_settings().set("config.model", "regular-model")
get_settings().set("config.model_weak", "weak-model-id")
get_settings().set("config.fallback_models", [])
get_settings().set("openai.deployment_id", None)
get_settings().set("openai.fallback_deployments", [])
settings = get_settings()
_remove_key(settings, "config.fallback_models")
_remove_key(settings, "openai.fallback_deployments")
settings.set("config.model", "regular-model")
settings.set("config.model_weak", "weak-model-id")
settings.set("config.fallback_models", [])
settings.set("openai.deployment_id", None)
settings.set("openai.fallback_deployments", [])

calls = []

Expand All @@ -159,11 +174,14 @@ async def fake_f(model):
def test_reasoning_model_type_uses_reasoning_setting():
snapshot = _snapshot_settings()
try:
get_settings().set("config.model", "regular-model")
get_settings().set("config.model_reasoning", "reasoning-model-id")
get_settings().set("config.fallback_models", [])
get_settings().set("openai.deployment_id", None)
get_settings().set("openai.fallback_deployments", [])
settings = get_settings()
_remove_key(settings, "config.fallback_models")
_remove_key(settings, "openai.fallback_deployments")
settings.set("config.model", "regular-model")
settings.set("config.model_reasoning", "reasoning-model-id")
settings.set("config.fallback_models", [])
settings.set("openai.deployment_id", None)
settings.set("openai.fallback_deployments", [])

calls = []

Expand Down
Loading
Loading