From c0d3ceb17e02d779ee4956adcb792d7a568a855f Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Wed, 1 Jul 2026 16:49:55 +0530 Subject: [PATCH] fix(auxiliary_client): dedup resolve_provider_client fall-through warnings The two fall-through branches in resolve_provider_client (unknown provider, unhandled auth_type) logged at WARNING on every retry of a misconfigured provider, spamming logs during retry loops. Demote both to logger.debug with per-process dedup: the first occurrence still surfaces (a provider-name typo or PROVIDER_REGISTRY/auth_type-drift bug is worth seeing once), while identical repeats are suppressed for the process lifetime. Salvaged from #56283 (extracting only the stated auxiliary_client fix; the original PR also bundled ~2800 lines of unrelated changes across 10 other files, which are dropped). --- agent/auxiliary_client.py | 29 +++++++++-- .../test_auxiliary_client_resolve_dedup.py | 52 +++++++++++++++++++ 2 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 tests/agent/test_auxiliary_client_resolve_dedup.py diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 39b88ea95..7192dfd5d 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -110,6 +110,19 @@ from utils import base_url_host_matches, base_url_hostname, env_float, model_for logger = logging.getLogger(__name__) +# ── resolve_provider_client fall-through dedup ─────────────────────────── +# Both fall-through warning sites in resolve_provider_client (the "unknown +# provider" and "unhandled auth_type" branches) fire on every retry of a +# misconfigured provider, spamming the logs. Demote them to logger.debug with +# per-process dedup: the FIRST occurrence still surfaces (it carries real +# diagnostic value — a provider-name typo or PROVIDER_REGISTRY/auth_type +# drift), and identical repeats are suppressed for the lifetime of the +# process. Two independent sets keep each branch linear and let tests clear +# them independently. +_LOGGED_UNKNOWN_PROVIDER_KEYS: set = set() +_LOGGED_UNHANDLED_AUTHTYPE_KEYS: set = set() + + def _openai_http_client_kwargs( base_url: Optional[str], *, @@ -4336,7 +4349,11 @@ def resolve_provider_client( pconfig = PROVIDER_REGISTRY.get(provider) if pconfig is None: - logger.warning("resolve_provider_client: unknown provider %r", provider) + # Demoted from logger.warning to debug; dedup keyed by provider name + # so the first occurrence surfaces but repeated retries stay silent. + if provider not in _LOGGED_UNKNOWN_PROVIDER_KEYS: + _LOGGED_UNKNOWN_PROVIDER_KEYS.add(provider) + logger.debug("resolve_provider_client: unknown provider %r", provider) return None, None if pconfig.auth_type == "api_key": @@ -4528,8 +4545,14 @@ def resolve_provider_client( "directly supported, try 'auto'", provider) return None, None - logger.warning("resolve_provider_client: unhandled auth_type %s for %s", - pconfig.auth_type, provider) + # Demoted from logger.warning to debug; dedup keyed on (auth_type, + # provider) so the first occurrence surfaces (real schema-drift bug) but + # per-call retries stay silent. + _auth_dedup_key = (pconfig.auth_type, provider) + if _auth_dedup_key not in _LOGGED_UNHANDLED_AUTHTYPE_KEYS: + _LOGGED_UNHANDLED_AUTHTYPE_KEYS.add(_auth_dedup_key) + logger.debug("resolve_provider_client: unhandled auth_type %s for %s", + pconfig.auth_type, provider) return None, None diff --git a/tests/agent/test_auxiliary_client_resolve_dedup.py b/tests/agent/test_auxiliary_client_resolve_dedup.py new file mode 100644 index 000000000..56f3115db --- /dev/null +++ b/tests/agent/test_auxiliary_client_resolve_dedup.py @@ -0,0 +1,52 @@ +"""Tests for resolve_provider_client fall-through log dedup (salvage #56283). + +Both fall-through branches (unknown provider, unhandled auth_type) were demoted +from ``logger.warning`` to ``logger.debug`` with per-process dedup: the first +occurrence surfaces for diagnostics; identical repeats are suppressed for the +lifetime of the process so a retry loop can't spam the logs. +""" + +import logging + +import agent.auxiliary_client as ac +from agent.auxiliary_client import resolve_provider_client + + +class TestUnknownProviderDedup: + def setup_method(self): + ac._LOGGED_UNKNOWN_PROVIDER_KEYS.clear() + + def test_unknown_provider_logs_debug_once_not_warning(self, caplog): + with caplog.at_level(logging.DEBUG, logger="agent.auxiliary_client"): + client, model = resolve_provider_client("no_such_provider_xyz", "") + assert (client, model) == (None, None) + recs = [ + r for r in caplog.records + if "unknown provider" in r.getMessage() + ] + # Exactly one record, and it is DEBUG (never WARNING). + assert len(recs) == 1 + assert recs[0].levelno == logging.DEBUG + assert not any(r.levelno >= logging.WARNING for r in recs) + + def test_unknown_provider_repeat_is_suppressed(self, caplog): + with caplog.at_level(logging.DEBUG, logger="agent.auxiliary_client"): + resolve_provider_client("no_such_provider_xyz", "") + resolve_provider_client("no_such_provider_xyz", "") + resolve_provider_client("no_such_provider_xyz", "") + recs = [ + r for r in caplog.records + if "unknown provider" in r.getMessage() + ] + # Three calls, one log line — dedup suppressed the repeats. + assert len(recs) == 1 + + def test_distinct_unknown_providers_each_log_once(self, caplog): + with caplog.at_level(logging.DEBUG, logger="agent.auxiliary_client"): + resolve_provider_client("bogus_a", "") + resolve_provider_client("bogus_b", "") + recs = [ + r for r in caplog.records + if "unknown provider" in r.getMessage() + ] + assert len(recs) == 2