clean up comments and remove unused import

Author: randy-concepcion

src/mlpa/core/auth/authorize.py

@@ -154,15 +154,14 @@ async def authorize_chat_request(
             use_play_integrity=use_play_integrity,
         )
     except HTTPException as exc:
-        # Record only the terminal auth dispositions the dependency intentionally
-        # emits. 401 is an expected or normalized auth rejection. 400 is a
-        # client-error request to the auth layer (invalid purpose, or malformed App
-        # Attest base64 decoded in app_attest_auth). Any other status, including App
-        # Attest's explicit 500, is re-raised unrecorded; auth-system-failure capture
-        # is left to a follow-on auth backend change. Non-HTTPException errors are not
-        # caught here and propagate unrecorded. Purpose is unresolved at these
-        # dispositions, so a stable "" placeholder is used, never the unvalidated
-        # header value.
+        # Only record terminal HTTP failures from the shared auth call:
+        # - 401: expected or normalized auth rejection (bad creds, expired token, etc.)
+        # - 400: client error to the auth layer (invalid purpose, or malformed App
+        #        Attest base64 decoded in app_attest_auth before its try block)
+        # - anything else (e.g. App Attest's explicit 500): re-raised unrecorded;
+        #   auth-system-failure capture is left to a follow-on auth backend change
+        # Non-HTTPException errors are not caught here and propagate unrecorded.
+        # Purpose is unknown at this point, so "" is always used as a placeholder.
         if exc.status_code == 401:
             reason = AvailabilityReason.AUTH_REJECTED
         elif exc.status_code == 400:

src/mlpa/core/completions.py

@@ -54,9 +54,12 @@ def _build_litellm_body(req: AuthorizedChatRequest, *, stream: bool) -> dict:
 async def get_or_create_user_for_completion(
     user_id: str, req: AuthorizedChatRequest | AuthorizedSearchRequest
 ):
-    """Wraps get_or_create_user, recording the signup-cap rejection metric and the
-    pre-completion availability disposition (signup cap, or a 5xx provisioning
-    failure) for chat requests."""
+    """
+    Wraps get_or_create_user and records availability for chat requests:
+    - signup cap (403 + MAX_USERS_REACHED): excluded, alongside the existing rejection metric
+    - user-resolution failure (status >= 500): failure
+    - search requests and non-signup-cap, non-5xx failures: not recorded
+    """
     try:
         return await get_or_create_user(user_id)
     except HTTPException as exc:
@@ -72,10 +75,9 @@ async def get_or_create_user_for_completion(
                 )
                 record_chat_availability(req, AvailabilityReason.SIGNUP_CAP_EXCEEDED)
             elif exc.status_code >= 500:
-                # User-resolution / provisioning system failure for an eligible
-                # request. Non-signup-cap, non-5xx dispositions are left unrecorded;
-                # a future client-side 4xx should get its own classification rather
-                # than counting as an availability failure.
+                # User-resolution system failure. Non-signup-cap 4xx errors are
+                # not recorded; a client-side 4xx should get its own classification
+                # rather than counting as an availability failure.
                 record_chat_availability(req, AvailabilityReason.PROVISIONING_FAILURE)
         raise
 

src/mlpa/core/errors.py

@@ -37,8 +37,8 @@ class RejectionMatch:
     log_message: str = ""
 
     def availability_reason(self) -> AvailabilityReason:
-        # classify_upstream_error only ever produces these completion-stage
-        # rejection reasons; SIGNUP_CAP_EXCEEDED is recorded pre-completion.
+        # SIGNUP_CAP_EXCEEDED is recorded pre-completion, not via classify_upstream_error,
+        # so it is not in the mapping below.
         if self.reason == PrometheusRejectionReason.RATE_LIMITED:
             if self.error_code == ERROR_CODE_UPSTREAM_RATE_LIMIT_EXCEEDED:
                 return AvailabilityReason.RATE_LIMITED_UPSTREAM

src/mlpa/core/prometheus_metrics.py

@@ -27,36 +27,33 @@ class AvailabilityOutcome(StrEnum):
 
 
 class AvailabilityReason(StrEnum):
-    # Values overlapping PrometheusRejectionReason are kept identical so the
-    # availability counter and the rejection counter reconcile on shared strings.
-    # Keep them in sync if a rejection reason is added.
-    # success
-    CLEAN_COMPLETION = "clean_completion"
-    # failure
-    UPSTREAM_ERROR = "upstream_error"
-    EMPTY_RESPONSE = "empty_response"
-    # excluded (policy rejections)
-    BUDGET_EXCEEDED = "budget_exceeded"
-    RATE_LIMITED_OWN = "rate_limited_own"
-    RATE_LIMITED_UPSTREAM = "rate_limited_upstream"
-    PAYLOAD_TOO_LARGE = "payload_too_large"
-    INVALID_MODEL_NAME = "invalid_model_name"
-    INVALID_REQUEST = "invalid_request"
-    # abort
-    CLIENT_DISCONNECT = "client_disconnect"
-    # pre-completion (auth dependency and route body, before the completion path)
-    # excluded
-    AUTH_REJECTED = "auth_rejected"
-    INVALID_AUTH_REQUEST = "invalid_auth_request"
-    INVALID_SERVICE_TYPE_FOR_MODEL = "invalid_service_type_for_model"
-    SIGNUP_CAP_EXCEEDED = "signup_cap_exceeded"
-    BLOCKED = "blocked"
-    # failure
-    PROVISIONING_FAILURE = "provisioning_failure"
-    # Defined but not emitted yet: auth backends normalize system failures to 401
-    # (indistinguishable from expected rejections), so capturing this is left to a
-    # follow-on auth backend change that surfaces a real disposition.
-    AUTH_SYSTEM_FAILURE = "auth_system_failure"
+    # Strings shared with PrometheusRejectionReason are kept identical so the
+    # two counters reconcile. Keep them in sync when a rejection reason is added.
+
+    # --- completion-stage reasons (recorded inside stream_completion / get_completion) ---
+    CLEAN_COMPLETION = "clean_completion"  # success
+    UPSTREAM_ERROR = "upstream_error"  # failure
+    EMPTY_RESPONSE = "empty_response"  # failure
+    BUDGET_EXCEEDED = "budget_exceeded"  # excluded
+    RATE_LIMITED_OWN = "rate_limited_own"  # excluded
+    RATE_LIMITED_UPSTREAM = "rate_limited_upstream"  # excluded
+    PAYLOAD_TOO_LARGE = "payload_too_large"  # excluded
+    INVALID_MODEL_NAME = "invalid_model_name"  # excluded
+    INVALID_REQUEST = "invalid_request"  # excluded
+    CLIENT_DISCONNECT = "client_disconnect"  # abort
+
+    # --- pre-completion reasons (recorded in the auth dependency and route body) ---
+    AUTH_REJECTED = "auth_rejected"  # excluded
+    INVALID_AUTH_REQUEST = "invalid_auth_request"  # excluded
+    INVALID_SERVICE_TYPE_FOR_MODEL = "invalid_service_type_for_model"  # excluded
+    SIGNUP_CAP_EXCEEDED = "signup_cap_exceeded"  # excluded
+    BLOCKED = "blocked"  # excluded
+    PROVISIONING_FAILURE = "provisioning_failure"  # failure
+
+    # Defined but not yet emitted: auth backends normalize system failures to 401,
+    # making them indistinguishable from expected rejections. Capturing this
+    # properly requires a follow-on change to the auth backends themselves.
+    AUTH_SYSTEM_FAILURE = "auth_system_failure"  # failure
 
 
 _AVAILABILITY_OUTCOME_BY_REASON: dict[AvailabilityReason, AvailabilityOutcome] = {

src/tests/unit/test_precompletion_availability.py

@@ -22,7 +22,6 @@
 from mlpa.core.prometheus_metrics import (
     AvailabilityOutcome,
     AvailabilityReason,
-    PrometheusRejectionReason,
 )
 from tests.consts import SAMPLE_REQUEST