fix(roi): preserve reports and identity during upstream outages

litellm/proxy/roi_calculator/github.py

@@ -315,7 +315,7 @@ class GitHub:
     ) -> None:
         if client is not None and transport is not None:
             raise ValueError("Pass either an injected GitHub client or a transport.")
-        self._profiles: Mapping[str, str] = MappingProxyType({})
+        self._profiles: Mapping[str, str | None] = MappingProxyType({})
         token: Final = settings.github_token.get_secret_value()
         self._headers: Final[Mapping[str, str]] = (
             MappingProxyType(
@@ -494,25 +494,26 @@ class GitHub:
         }
         return evidence
 
-    async def profile_email(self, login: str) -> str:
+    async def profile_email(self, login: str, *, fallback: str = "") -> str:
         if login.casefold() in self._profiles:
-            return self._profiles[login.casefold()]
+            cached: Final = self._profiles[login.casefold()]
+            return cached if cached is not None else fallback
         address: Final = await self._load_profile_email(login)
         self._profiles = MappingProxyType({**self._profiles, login.casefold(): address})
-        return address
+        return address if address is not None else fallback
 
-    async def _load_profile_email(self, login: str) -> str:
+    async def _load_profile_email(self, login: str) -> str | None:
         try:
             response: Final = await self.client.get(
                 self._url(f"users/{quote(login, safe='')}"),
                 headers=self._headers,
             )
             if response.status_code != 200:
-                return ""
+                return None
             profile: Final = _GitHubUserProfile.model_validate(response.json())
             return normalize_email(profile.email)
         except (httpx.HTTPError, ValueError):
-            return ""
+            return None
 
     async def _commit_metadata(
         self, repo: str, number: int, detail: _PullDetail

litellm/proxy/roi_calculator/sync.py

@@ -274,6 +274,11 @@ async def _read_repositories(github: GitHub, repos: tuple[str, ...], start: date
             "check repository access or try analysis again later."
         )
     queue: Final = tuple(chain.from_iterable(((group.repo, pull) for pull in group.pulls) for group in groups))
+    if unavailable and not queue:
+        raise SourceError(
+            f"GitHub could not read {', '.join(unavailable)}, and the accessible repositories returned no pull requests. "
+            "No new report was published; check repository access or try analysis again later."
+        )
     warnings: Final = (
         (
             (
@@ -298,6 +303,13 @@ def _processed_records(processed: tuple[_ProcessedPull, ...]) -> Mapping[int, RO
         raise SourceError(
             "GitHub could not provide PR metadata. No new report was published; try analysis again later."
         )
+    if any(item.record["estimate"]["status"] == "error" for item in processed) and not any(
+        item.record["estimate"]["status"] == "estimated" for item in processed
+    ):
+        raise SourceError(
+            "The estimator could not score any pull requests. No new report was published; "
+            "check the estimator connection or try analysis again later."
+        )
     return MappingProxyType({item.position: item.record for item in processed})
 
 
@@ -469,7 +481,9 @@ class SyncManager:
                     and cached_pull["estimate"]["status"] == "estimated"
                     and "commit_emails" in cached_pull
                 ):
-                    profile: Final = await github.profile_email(cached_pull["login"])
+                    profile: Final = await github.profile_email(
+                        cached_pull["login"], fallback=cached_pull.get("profile_email", "")
+                    )
                     cached_record: Final = TypeAdapter(ROIPullRecord).validate_python(
                         MappingProxyType(
                             {

litellm/types/roi_calculator.py

@@ -11,6 +11,15 @@ DEFAULT_PROMPT: Final = (
 )
 
 
+def _normalize_login(value: str) -> str:
+    import re
+
+    login: Final = value.strip().casefold()
+    if re.fullmatch(r"[A-Za-z0-9_\[\]-]+", login) is None:
+        raise ValueError("Enter a valid GitHub username.")
+    return login
+
+
 class ROISettings(BaseModel):
     model_config = ConfigDict(frozen=True)
 
@@ -81,15 +90,13 @@ class ROISettings(BaseModel):
     @field_validator("identity_map")
     @classmethod
     def normalize_identity_map(cls, values: Mapping[str, str]) -> Mapping[str, str]:
-        import re
-
         from litellm.proxy.roi_calculator.analytics import normalize_email
 
         normalized: Final[Mapping[str, str]] = MappingProxyType(
             {
-                login.strip().casefold(): normalize_email(address)
+                _normalize_login(login): normalize_email(address)
                 for login, address in values.items()
-                if re.fullmatch(r"[A-Za-z0-9_\[\]-]+", login.strip()) is not None and normalize_email(address)
+                if normalize_email(address)
             }
         )
         if len(normalized) != len(values):
@@ -421,6 +428,11 @@ class ROIIdentityMapUpdate(BaseModel):
     github_login: str
     email: str | None
 
+    @field_validator("github_login")
+    @classmethod
+    def normalize_login(cls, value: str) -> str:
+        return _normalize_login(value)
+
 
 class ROIIdentityMapResponse(BaseModel):
     report: ROISummaryResponse | None

tests/unit/proxy/management_endpoints/test_roi_calculator_endpoints.py

@@ -177,6 +177,16 @@ def test_all_writes_require_full_admin(role: LitellmUserRoles, method: str, path
     assert client.request(method, path, json=body).status_code == 403
 
 
+@pytest.mark.parametrize("login", ("invalid.name", " ", "user/name"))
+@pytest.mark.parametrize("email", ("alice@example.com", None))
+def test_invalid_identity_login_returns_validation_error(login: str, email: str | None) -> None:
+    repository: Final = _ConfigRepository()
+    client: Final = _client(LitellmUserRoles.PROXY_ADMIN, repository)
+    response: Final = client.put("/roi-calculator/identity-map", json={"github_login": login, "email": email})
+    assert response.status_code == 422
+    assert not repository.values
+
+
 def test_schedule_and_estimator_key_persist_without_exposing_secrets(monkeypatch: pytest.MonkeyPatch) -> None:
     monkeypatch.setenv("LITELLM_SALT_KEY", "roi-calculator-test-salt-key-0123456789")
     repository: Final = _ConfigRepository()

tests/unit/proxy/roi_calculator/test_sync.py

@@ -460,7 +460,9 @@ async def test_one_unreadable_pr_preserves_other_estimates_in_report() -> None:
     assert manager.status.needs_attention == 1
 
 
-def _repository_outage_transport(status: int, *, all_unavailable: bool = False) -> httpx.MockTransport:
+def _repository_outage_transport(
+    status: int, *, all_unavailable: bool = False, healthy_empty: bool = False
+) -> httpx.MockTransport:
     baseline: Final = _transport()
 
     def respond(request: httpx.Request) -> httpx.Response:
@@ -468,6 +470,8 @@ def _repository_outage_transport(status: int, *, all_unavailable: bool = False)
             return httpx.Response(status, json=[] if status == 200 else {"message": "Repository unavailable"})
         if all_unavailable and request.url.path.endswith("/pulls"):
             return httpx.Response(status)
+        if healthy_empty and request.url.path == "/repos/org/repo/pulls":
+            return httpx.Response(200, json=[])
         return baseline.handle_request(request)
 
     return httpx.MockTransport(respond)
@@ -512,7 +516,8 @@ async def test_unavailable_repository_publishes_flagged_partial_report_and_recov
 
 
 @pytest.mark.asyncio
-async def test_all_repository_outage_preserves_previous_report() -> None:
+@pytest.mark.parametrize("all_unavailable", (True, False))
+async def test_repository_outage_without_usable_pulls_preserves_previous_report(all_unavailable: bool) -> None:
     repository: Final = _ReportRepository()
     manager: Final = SyncManager(clock=_fixed_now)
     settings: Final = _settings().model_copy(update=MappingProxyType({"repos": ("org/repo", "org/unavailable")}))
@@ -521,9 +526,73 @@ async def test_all_repository_outage_preserves_previous_report() -> None:
     previous: Final = repository.values["roi_calculator_report"]
 
     assert await manager.start(
-        settings, repository, _spend_reader(), _completion(), _repository_outage_transport(403, all_unavailable=True)
+        settings,
+        repository,
+        _spend_reader(),
+        _completion(),
+        _repository_outage_transport(403, all_unavailable=all_unavailable, healthy_empty=not all_unavailable),
     )
     await _wait_until_finished(manager)
     assert manager.status.phase == "error"
     assert manager.status.error is not None and "No new report was published" in manager.status.error
     assert repository.values["roi_calculator_report"] == previous
+
+
+@pytest.mark.asyncio
+@pytest.mark.parametrize("profile_status", (200, 403, 429, 503))
+async def test_reused_profile_preserves_email_only_when_lookup_fails(profile_status: int) -> None:
+    repository: Final = _ReportRepository()
+    manager: Final = SyncManager(clock=_fixed_now)
+    baseline: Final = _transport()
+
+    def respond(request: httpx.Request) -> httpx.Response:
+        if request.url.path.endswith("/commits"):
+            return httpx.Response(200, content=_COMMITS_JSON.replace("alice@example.com", ""))
+        return baseline.handle_request(request)
+
+    assert await manager.start(_settings(), repository, _spend_reader(), _completion(), httpx.MockTransport(respond))
+    await _wait_until_finished(manager)
+
+    def refreshed(request: httpx.Request) -> httpx.Response:
+        if request.url.path == "/users/alice":
+            return httpx.Response(profile_status, json={"email": None})
+        return baseline.handle_request(request)
+
+    async def unexpected_completion(request: ROICompletionRequest) -> object:
+        raise AssertionError("A reused estimate must not call the estimator")
+
+    assert await manager.start(
+        _settings(), repository, _spend_reader(), unexpected_completion, httpx.MockTransport(refreshed)
+    )
+    await _wait_until_finished(manager)
+    report: Final = TypeAdapter(ROIReport).validate_python(repository.values["roi_calculator_report"])
+    expected: Final = "" if profile_status == 200 else "alice@example.com"
+    assert manager.status.phase == "complete"
+    assert manager.status.reused == 1
+    assert report["pulls"][0]["profile_email"] == expected
+    assert report["pulls"][0]["emails"] == ((expected,) if expected else ())
+    assert summarize(report, MappingProxyType({}))["metrics"]["cost_per_hour"] == (None if profile_status == 200 else 3)
+
+
+@pytest.mark.asyncio
+async def test_complete_estimator_outage_preserves_report_and_recovers() -> None:
+    repository: Final = _ReportRepository()
+    manager: Final = SyncManager(clock=_fixed_now)
+    assert await manager.start(_settings(), repository, _spend_reader(), _completion(), _transport())
+    await _wait_until_finished(manager)
+    previous: Final = repository.values["roi_calculator_report"]
+    changed: Final = _settings(estimator_prompt="Updated estimation instructions")
+
+    async def failed_completion(request: ROICompletionRequest) -> object:
+        raise httpx.ConnectError("Estimator unavailable")
+
+    assert await manager.start(changed, repository, _spend_reader(), failed_completion, _transport())
+    await _wait_until_finished(manager)
+    assert manager.status.phase == "error"
+    assert manager.status.error is not None and "No new report was published" in manager.status.error
+    assert repository.values["roi_calculator_report"] == previous
+    assert await manager.start(changed, repository, _spend_reader(), _completion(), _transport())
+    await _wait_until_finished(manager)
+    assert manager.status.phase == "complete"
+    recovered: Final = TypeAdapter(ROIReport).validate_python(repository.values["roi_calculator_report"])
+    assert recovered["pulls"][0]["estimate"]["hours"] == 4

ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorView.integration.test.tsx

@@ -179,6 +179,10 @@ describe("ROICalculatorView", () => {
     expect(await screen.findByRole("alert")).toHaveTextContent(warning);
     expect(screen.getByRole("button", { name: "Open estimate for org/repo pull request 42" })).toBeInTheDocument();
     expect(screen.queryByText("$3.00")).not.toBeInTheDocument();
+    fireEvent.click(screen.getByText("Calculation details"));
+    expect(
+      screen.getByText("Spend per estimated hour is unavailable until all selected repositories can be read."),
+    ).toBeVisible();
   });
 
   it("lets a view-only admin read the report without write controls", async () => {

ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorViews.tsx

@@ -41,6 +41,10 @@ export function ROIOverview({
   const [pagination, setPagination] = React.useState({ query, visibleCount: 10 });
   const visibleCount = pagination.query === query ? pagination.visibleCount : 10;
   const metrics = summary.metrics;
+  const unavailableRate =
+    metrics.output_hours > 0
+      ? "Spend per estimated hour is unavailable until all selected repositories can be read."
+      : "A rate requires matched estimated hours greater than zero and access to all selected repositories.";
   return (
     <div className="space-y-6">
       <section aria-label="Spend and estimated engineering effort" className="grid gap-4 md:grid-cols-2 xl:grid-cols-4">
@@ -59,7 +63,7 @@ export function ROIOverview({
           <p>
             {metrics.cost_per_hour != null
               ? `${formatMoney(metrics.matched_spend)} gateway spend ÷ ${formatNumber(metrics.output_hours)} estimated engineering hours = ${formatMoney(metrics.cost_per_hour)} per estimated hour.`
-              : "A rate is available when matched estimated hours are greater than zero."}
+              : unavailableRate}
           </p>
           <p>
             The comparison includes {metrics.cohort_people} matched {metrics.cohort_people === 1 ? "person" : "people"}{" "}