From 738a13615078af3846ee212da5dc672694e21429 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 29 Sep 2026 07:00:59 +0000 Subject: [PATCH] fix(roi-calculator): correct estimator and dashboard behavior Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com> --- .../roi_calculator_endpoints.py | 65 +++++++++++++++- litellm/proxy/roi_calculator/estimator.py | 37 ++++++--- litellm/proxy/roi_calculator/github.py | 74 +++++++++++++----- litellm/proxy/roi_calculator/sync.py | 29 ++++--- litellm/types/llms/custom_http.py | 1 + .../test_roi_calculator_endpoints.py | 33 ++++++-- .../proxy/roi_calculator/test_estimator.py | 34 +++++++- .../unit/proxy/roi_calculator/test_github.py | 15 ++-- ui/litellm-dashboard/AGENTS.md | 10 +++ .../_components/ROICalculatorDialogs.tsx | 27 +++++-- .../ROICalculatorView.integration.test.tsx | 35 ++++++++- .../_components/ROICalculatorView.tsx | 78 +++++++++++++------ .../_components/ROICalculatorViews.tsx | 56 ++++++------- .../_components/ROISettingsPanel.tsx | 74 ++++++++++++------ .../_components/roiCalculatorData.test.ts | 58 ++++++++------ .../_components/roiCalculatorData.ts | 19 ++++- .../app/(dashboard)/roi-calculator/page.tsx | 4 +- 17 files changed, 477 insertions(+), 172 deletions(-) diff --git a/litellm/proxy/management_endpoints/roi_calculator_endpoints.py b/litellm/proxy/management_endpoints/roi_calculator_endpoints.py index e04ae75c87d..5f4835f0cac 100644 --- a/litellm/proxy/management_endpoints/roi_calculator_endpoints.py +++ b/litellm/proxy/management_endpoints/roi_calculator_endpoints.py @@ -1,4 +1,4 @@ -from collections.abc import Mapping +from collections.abc import Mapping, Sequence from datetime import date from enum import Enum from types import MappingProxyType @@ -12,7 +12,7 @@ from litellm.proxy._types import CommonProxyErrors, LitellmUserRoles, UserAPIKey from litellm.proxy.auth.user_api_key_auth import user_api_key_auth from litellm.proxy.common_utils.encrypt_decrypt_utils import decrypt_value_helper, encrypt_value_helper from litellm.proxy.roi_calculator.analytics import normalize_email, summarize -from litellm.proxy.roi_calculator.estimator import CompletionCaller +from litellm.proxy.roi_calculator.estimator import CompletionCaller, EstimatorModel from litellm.proxy.roi_calculator.github import GitHub, SourceError from litellm.proxy.roi_calculator.sync import SpendReader, SyncManager, read_spend, spend_prisma_client from litellm.repositories.config_repository import ConfigRepository @@ -53,6 +53,27 @@ class _StoredSettings(BaseModel): identity_map: Mapping[str, str] = Field(default_factory=lambda: MappingProxyType({})) +class _RouterEstimatorParams(BaseModel): + model_config = ConfigDict(extra="ignore", from_attributes=True) + + model: str | None = None + base_model: str | None = None + custom_llm_provider: str | None = None + + +class _RouterEstimatorModelInfo(BaseModel): + model_config = ConfigDict(extra="ignore", from_attributes=True) + + base_model: str | None = None + + +class _RouterEstimatorDeployment(BaseModel): + model_config = ConfigDict(extra="ignore", from_attributes=True) + + litellm_params: _RouterEstimatorParams + model_info: _RouterEstimatorModelInfo | None = None + + async def _read_admin( user_api_key_dict: Annotated[UserAPIKeyAuth, Depends(user_api_key_auth)], ) -> UserAPIKeyAuth: @@ -93,10 +114,41 @@ def get_github_transport() -> httpx.AsyncBaseTransport | None: return None +_ROUTER_ESTIMATOR_DEPLOYMENTS: Final = TypeAdapter(tuple[_RouterEstimatorDeployment, ...]) _MODEL_NAMES: Final = TypeAdapter(tuple[str, ...]) _ROUTER_MESSAGES: Final = TypeAdapter(list[AllMessageValues]) +def _estimator_models_from_deployments(deployments: Sequence[object]) -> tuple[EstimatorModel, ...]: + parsed_deployments: Final = _ROUTER_ESTIMATOR_DEPLOYMENTS.validate_python(deployments) + return tuple( + estimator_model + for deployment in parsed_deployments + if (estimator_model := _estimator_model(deployment)) is not None + ) + + +def _estimator_model(deployment: _RouterEstimatorDeployment) -> EstimatorModel | None: + parameters: Final = deployment.litellm_params + model: Final = ( + (deployment.model_info.base_model if deployment.model_info is not None else None) + or parameters.base_model + or parameters.model + ) + if model is None: + return None + return model, parameters.custom_llm_provider + + +def _router_estimator_models(model_group: str) -> tuple[EstimatorModel, ...]: + from litellm.proxy.proxy_server import llm_router + + if llm_router is None: + return () + deployments: Final = llm_router.get_model_list(model_name=model_group) or () + return _estimator_models_from_deployments(deployments) + + def _router_models() -> tuple[str, ...]: from litellm.proxy.proxy_server import llm_router @@ -336,7 +388,14 @@ async def start_roi_calculator_sync( public: Final = _public_settings(settings) if not public.ready: raise HTTPException(status_code=409, detail="Connect GitHub, select repositories, and choose a router model.") - if not manager.start(settings, repository, _spend_reader(repository), _completion_caller(), transport): + if not manager.start( + settings, + repository, + _spend_reader(repository), + _completion_caller(), + transport, + _router_estimator_models(settings.estimator_model), + ): raise HTTPException(status_code=409, detail="A sync is already running.") return manager.status diff --git a/litellm/proxy/roi_calculator/estimator.py b/litellm/proxy/roi_calculator/estimator.py index 7c8182245bf..db0102b7c46 100644 --- a/litellm/proxy/roi_calculator/estimator.py +++ b/litellm/proxy/roi_calculator/estimator.py @@ -1,8 +1,7 @@ import hashlib import json -import re from collections.abc import Awaitable -from typing import Final, Literal, Protocol +from typing import Final, Literal, Protocol, TypeAlias from pydantic import ValidationError from typing_extensions import NotRequired, ReadOnly, TypedDict @@ -24,9 +23,11 @@ from litellm.types.roi_calculator import ( ROIResponseFormat, ROISettings, ) +from litellm.utils import supports_none_reasoning_effort MAX_EVIDENCE_CHARS: Final = 160000 ESTIMATE_VERSION: Final = "estimate-v3-without-ai" +EstimatorModel: TypeAlias = tuple[str, str | None] RESPONSE_CONTRACT: Final = ( 'Return only a JSON object with "hours" (a nonnegative number) and "reasoning" (a short string). ' "Hours mean estimated engineering effort to complete the work without AI assistance, not actual time worked or " @@ -62,33 +63,39 @@ def metadata_evidence(pull: ROIPullEvidence) -> ROIEstimatorEvidence: ) -def estimator_options(model: str) -> _EstimatorOptions: - if _requires_no_reasoning(model): +def estimator_options(models: tuple[EstimatorModel, ...]) -> _EstimatorOptions: + if models and all( + supports_none_reasoning_effort(model, custom_llm_provider=provider) for model, provider in models + ): options_without_reasoning: Final[_EstimatorOptions] = {"reasoning_effort": "none"} return options_without_reasoning default_options: Final[_EstimatorOptions] = {} return default_options -def _requires_no_reasoning(model: str) -> bool: - return re.search(r"(?:^|[/.])gpt-6-(?:luna|sol)$", model) is not None +def _configured_models(settings: ROISettings, models: tuple[EstimatorModel, ...] | None) -> tuple[EstimatorModel, ...]: + return models if models is not None else ((settings.estimator_model, None),) -def cache_context(settings: ROISettings) -> str: +def cache_context(settings: ROISettings, models: tuple[EstimatorModel, ...] | None = None) -> str: context: Final = json.dumps( ( ESTIMATE_VERSION, settings.estimator_model, settings.estimator_prompt, RESPONSE_CONTRACT, - estimator_options(settings.estimator_model), + estimator_options(_configured_models(settings, models)), ), ensure_ascii=False, ) return hashlib.sha256(context.encode()).hexdigest() -def pull_cache_key(settings: ROISettings, pull: ROIPullEvidence) -> str: +def pull_cache_key( + settings: ROISettings, + pull: ROIPullEvidence, + models: tuple[EstimatorModel, ...] | None = None, +) -> str: evidence: Final = json.dumps( metadata_evidence(pull).model_dump(exclude_unset=True), ensure_ascii=False, @@ -99,7 +106,7 @@ def pull_cache_key(settings: ROISettings, pull: ROIPullEvidence) -> str: settings.estimator_model, settings.estimator_prompt, RESPONSE_CONTRACT, - estimator_options(settings.estimator_model), + estimator_options(_configured_models(settings, models)), pull["repo"], pull["number"], pull["head_sha"], @@ -111,9 +118,15 @@ def pull_cache_key(settings: ROISettings, pull: ROIPullEvidence) -> str: class Estimator: - def __init__(self, settings: ROISettings, complete: CompletionCaller) -> None: + def __init__( + self, + settings: ROISettings, + complete: CompletionCaller, + models: tuple[EstimatorModel, ...] | None = None, + ) -> None: self.settings: Final = settings self.complete: Final = complete + self.models: Final = _configured_models(settings, models) async def estimate(self, pull: ROIPullEvidence) -> ROIEstimate: evidence: Final = json.dumps( @@ -152,7 +165,7 @@ class Estimator: response_format=response_format, max_tokens=1200, metadata=metadata, - reasoning_effort="none" if _requires_no_reasoning(self.settings.estimator_model) else None, + reasoning_effort="none" if estimator_options(self.models) else None, ) try: response: Final = await self.complete(request) diff --git a/litellm/proxy/roi_calculator/github.py b/litellm/proxy/roi_calculator/github.py index cfb5b4608d5..57dc5a20d69 100644 --- a/litellm/proxy/roi_calculator/github.py +++ b/litellm/proxy/roi_calculator/github.py @@ -9,7 +9,9 @@ import httpx from pydantic import BaseModel, ConfigDict, Field, TypeAdapter from typing_extensions import ReadOnly, TypedDict +from litellm.llms.custom_httpx.http_handler import get_async_httpx_client from litellm.proxy.roi_calculator.analytics import normalize_email +from litellm.types.llms.custom_http import httpxSpecialProvider from litellm.types.roi_calculator import ROIPullCommit, ROIPullEvidence, ROIPullFile, ROISettings _T: Final = TypeVar("_T") @@ -240,6 +242,7 @@ async def _fetch_page( adapter: TypeAdapter[tuple[_T, ...]], params: Mapping[str, str | int] | None, page: int, + headers: Mapping[str, str] | None = None, ) -> tuple[tuple[_T, ...], bool]: response: Final = await _request( client, @@ -252,6 +255,7 @@ async def _fetch_page( "page": page, } ), + headers=headers, ) try: parsed: Final[tuple[_T, ...]] = adapter.validate_python(response.json()) @@ -266,9 +270,10 @@ async def _pages( adapter: TypeAdapter[tuple[_T, ...]], params: Mapping[str, str | int] | None = None, limit: int = 10000, + headers: Mapping[str, str] | None = None, ) -> AsyncIterator[tuple[_T, ...]]: for page in range(1, limit + 1): - result = await _fetch_page(client, path, adapter, params, page) + result = await _fetch_page(client, path, adapter, params, page, headers) yield result[0] if not result[1]: return @@ -285,9 +290,16 @@ class _GitHubUserProfile(_GitHubModel): class GitHub: - def __init__(self, settings: ROISettings, transport: httpx.AsyncBaseTransport | None = None) -> None: + def __init__( + self, + settings: ROISettings, + transport: httpx.AsyncBaseTransport | None = None, + client: httpx.AsyncClient | None = None, + ) -> None: + if client is not None and transport is not None: + raise ValueError("Pass either an injected GitHub client or a transport.") token: Final = settings.github_token.get_secret_value() - headers: Final[Mapping[str, str]] = ( + self._headers: Final[Mapping[str, str]] = ( MappingProxyType( { "Accept": "application/vnd.github+json", @@ -297,16 +309,28 @@ class GitHub: if token else MappingProxyType({"Accept": "application/vnd.github+json"}) ) - self.client: Final = httpx.AsyncClient( - base_url=settings.github_api_url + "/", - headers=headers, - timeout=45, - transport=transport, - follow_redirects=False, + self._api_url: Final = settings.github_api_url.rstrip("/") + client_params: Final[dict[str, object]] = { + "timeout": 45, + "follow_redirects": False, + **({"transport": transport} if transport is not None else {}), + } + self.client: Final[httpx.AsyncClient] = ( + client + if client is not None + else get_async_httpx_client( + llm_provider=httpxSpecialProvider.ROICalculator, + params=client_params, + ).client ) + self._close_client: Final = client is not None or transport is not None async def close(self) -> None: - await self.client.aclose() + if self._close_client: + await self.client.aclose() + + def _url(self, path: str) -> str: + return f"{self._api_url}/{path.lstrip('/')}" async def repositories( self, @@ -316,7 +340,7 @@ class GitHub: response: Final = await _request( self.client, "GET", - "user/repos", + self._url("user/repos"), params=MappingProxyType( { "per_page": 100, @@ -326,6 +350,7 @@ class GitHub: "affiliation": "owner,collaborator,organization_member", } ), + headers=self._headers, ) try: repositories: Final[tuple[_RepositoryItem, ...]] = _REPOSITORIES.validate_python(response.json()) @@ -346,9 +371,10 @@ class GitHub: async def pull_pages() -> AsyncIterator[GitHubPullListItem]: async for page in _pages( self.client, - f"repos/{repo}/pulls", + self._url(f"repos/{repo}/pulls"), _PULLS, MappingProxyType({"state": "closed", "sort": "updated", "direction": "desc"}), + headers=self._headers, ): for pull in page: yield pull @@ -363,7 +389,12 @@ class GitHub: return await _collect(matching_pulls()) async def evidence(self, repo: str, pull: GitHubPullListItem) -> ROIPullEvidence: - detail_response: Final = await _request(self.client, "GET", f"repos/{repo}/pulls/{pull.number}") + detail_response: Final = await _request( + self.client, + "GET", + self._url(f"repos/{repo}/pulls/{pull.number}"), + headers=self._headers, + ) try: detail: Final = _PullDetail.model_validate(detail_response.json()) except Exception: @@ -373,9 +404,10 @@ class GitHub: async def file_pages() -> AsyncIterator[_PullFile]: async for page in _pages( self.client, - f"repos/{repo}/pulls/{pull.number}/files", + self._url(f"repos/{repo}/pulls/{pull.number}/files"), _PULL_FILES, limit=30, + headers=self._headers, ): for item in page: yield item @@ -415,7 +447,10 @@ class GitHub: async def _profile_email(self, login: str) -> str: try: - response: Final = await self.client.get(f"users/{quote(login, safe='')}") + response: Final = await self.client.get( + self._url(f"users/{quote(login, safe='')}"), + headers=self._headers, + ) if response.status_code != 200: return "" profile: Final = _GitHubUserProfile.model_validate(response.json()) @@ -426,14 +461,15 @@ class GitHub: async def _commit_metadata( self, repo: str, number: int, detail: _PullDetail ) -> tuple[tuple[ROIPullCommit, ...], tuple[tuple[str, str], ...], int]: - if not self.client.headers.get("Authorization"): + if not self._headers.get("Authorization"): async def commit_pages() -> AsyncIterator[_RestCommit]: async for page in _pages( self.client, - f"repos/{repo}/pulls/{number}/commits", + self._url(f"repos/{repo}/pulls/{number}/commits"), _REST_COMMITS, limit=3, + headers=self._headers, ): for item in page: yield item @@ -449,7 +485,7 @@ class GitHub: ) count: Final = detail.commits if detail.commits is not None else len(commits) return commits, authors, count - base: Final = str(self.client.base_url).rstrip("/") + base: Final = self._api_url endpoint: Final = ( base.removesuffix("/api/v3") + "/api/graphql" if base.endswith("/api/v3") else base + "/graphql" ) @@ -474,7 +510,7 @@ class GitHub: self.client, "POST", endpoint, - headers=MappingProxyType({"Authorization": self.client.headers["Authorization"]}), + headers=self._headers, json_body=_GraphQLPayload( query=_GRAPHQL_QUERY, variables=_GraphQLVariables(owner=owner, name=name, number=number, cursor=cursor), diff --git a/litellm/proxy/roi_calculator/sync.py b/litellm/proxy/roi_calculator/sync.py index d551fe1d0b5..e06174149c8 100644 --- a/litellm/proxy/roi_calculator/sync.py +++ b/litellm/proxy/roi_calculator/sync.py @@ -9,7 +9,7 @@ import httpx from pydantic import BaseModel, ConfigDict, Field, TypeAdapter from typing_extensions import ReadOnly, TypedDict, Unpack -from litellm.proxy.roi_calculator.estimator import CompletionCaller, Estimator, cache_context +from litellm.proxy.roi_calculator.estimator import CompletionCaller, Estimator, EstimatorModel, cache_context from litellm.proxy.roi_calculator.github import GitHub, GitHubPullListItem, SourceError from litellm.proxy.roi_calculator.pull_cache import cache_key, settings_fingerprint from litellm.types.roi_calculator import ( @@ -114,20 +114,15 @@ async def read_spend( group_by: Final[list[Literal["user_id", "date"]]] = [ "user_id", "date", - ] # mutable-ok: prisma client serializer only accepts builtin dict/list - sums: Final[dict[str, object]] = { - "spend": True, - "api_requests": True, - } # mutable-ok: prisma client serializer only accepts builtin dict/list + ] + sums: Final[dict[str, object]] = {"spend": True, "api_requests": True} date_filter: Final[dict[str, object]] = { "date": { "gte": start.isoformat(), "lte": end.isoformat(), } - } # mutable-ok: prisma client serializer only accepts builtin dict/list - order: Final[dict[str, object]] = { - "date": "asc" - } # mutable-ok: prisma client serializer only accepts builtin dict/list + } + order: Final[dict[str, object]] = {"date": "asc"} groups: Final = _DAILY_SPEND_GROUPS.validate_python( await daily_table.group_by( by=group_by, @@ -138,9 +133,7 @@ async def read_spend( ) user_ids: Final = tuple(sorted(frozenset(group.user_id for group in groups if group.user_id))) user_table: Final = database.litellm_usertable - user_filter: Final[dict[str, object]] = { - "user_id": {"in": list(user_ids)} - } # mutable-ok: prisma client serializer only accepts builtin dict/list + user_filter: Final[dict[str, object]] = {"user_id": {"in": list(user_ids)}} users: Final = _USER_EMAILS.validate_python( await user_table.find_many( where=user_filter, @@ -246,6 +239,7 @@ class SyncManager: spend_reader: SpendReader, complete: CompletionCaller, github_transport: httpx.AsyncBaseTransport | None = None, + estimator_models: tuple[EstimatorModel, ...] | None = None, ) -> bool: if self._status.running or not settings.repos or not settings.estimator_model: return False @@ -260,7 +254,9 @@ class SyncManager: needs_attention=0, error=None, ) - self._task = asyncio.create_task(self._run(settings, repository, spend_reader, complete, github_transport)) + self._task = asyncio.create_task( + self._run(settings, repository, spend_reader, complete, github_transport, estimator_models) + ) return True async def cancel(self) -> bool: @@ -282,6 +278,7 @@ class SyncManager: spend_reader: SpendReader, complete: CompletionCaller, github_transport: httpx.AsyncBaseTransport | None, + estimator_models: tuple[EstimatorModel, ...] | None, ) -> None: github: Final = self._github_factory(settings, github_transport) try: @@ -295,7 +292,7 @@ class SyncManager: ((repo, pull) for pull in pulls) for repo, pulls in zip(settings.repos, pull_groups, strict=True) ) ) - context: Final = cache_context(settings) + context: Final = cache_context(settings, estimator_models) previous: Final = await self._previous_report(repository) previous_pulls: Final[Mapping[str, ROIPullRecord]] = MappingProxyType( { @@ -329,7 +326,7 @@ class SyncManager: reused=reused_count, ) semaphore: Final = asyncio.Semaphore(PR_CONCURRENCY) - estimator: Final = Estimator(settings, complete) + estimator: Final = Estimator(settings, complete, estimator_models) async def process( item: tuple[int, str, GitHubPullListItem, str | None], diff --git a/litellm/types/llms/custom_http.py b/litellm/types/llms/custom_http.py index 6ab8fe9dfa8..858123b5232 100644 --- a/litellm/types/llms/custom_http.py +++ b/litellm/types/llms/custom_http.py @@ -31,6 +31,7 @@ class httpxSpecialProvider(str, Enum): A2A = "a2a" PromptManagement = "prompt_management" UI = "ui" + ROICalculator = "roi_calculator" Sandbox = "sandbox" ModelCostMap = "model_cost_map" PasswordBreachCheck = "password_breach_check" diff --git a/tests/unit/proxy/management_endpoints/test_roi_calculator_endpoints.py b/tests/unit/proxy/management_endpoints/test_roi_calculator_endpoints.py index 7e021b9da7f..ff131fff32e 100644 --- a/tests/unit/proxy/management_endpoints/test_roi_calculator_endpoints.py +++ b/tests/unit/proxy/management_endpoints/test_roi_calculator_endpoints.py @@ -11,9 +11,11 @@ from pydantic import TypeAdapter from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth from litellm.proxy.auth.user_api_key_auth import user_api_key_auth from litellm.proxy.management_endpoints.roi_calculator_endpoints import ( + _estimator_models_from_deployments, get_roi_config_repository, router, ) +from litellm.proxy.roi_calculator.estimator import estimator_options from litellm.types.roi_calculator import ROISettings _JSON_HEADERS: Final = MappingProxyType({"content-type": "application/json"}) @@ -52,6 +54,28 @@ def _client(role: LitellmUserRoles, repository: _ConfigRepository) -> TestClient return TestClient(app) +def test_router_group_uses_underlying_model_metadata_for_reasoning_option() -> None: + import litellm + + supported_model: Final = next( + model + for model, metadata in litellm.model_cost.items() + if metadata.get("supports_none_reasoning_effort") is True + ) + deployments: Final = ( + { + "model_name": "roi-estimator", + "litellm_params": {"model": "custom-deployment"}, + "model_info": {"base_model": supported_model}, + }, + ) + + estimator_models: Final = _estimator_models_from_deployments(deployments) + + assert estimator_models == ((supported_model, None),) + assert estimator_options(estimator_models) == {"reasoning_effort": "none"} + + def test_non_admin_cannot_read_roi_settings() -> None: client: Final = _client(LitellmUserRoles.INTERNAL_USER, _ConfigRepository()) @@ -79,19 +103,14 @@ def test_github_token_is_never_returned_and_url_change_clears_it(monkeypatch: py saved: Final = client.put( "/roi-calculator/settings", - content=( - '{"github_token":"private-test-token","repos":["org/repo"],' - '"estimator_model":"test-estimator"}' - ), + content=('{"github_token":"private-test-token","repos":["org/repo"],"estimator_model":"test-estimator"}'), headers=_JSON_HEADERS, ) assert saved.status_code == 200 assert saved.json()["has_github_token"] is True assert "private-test-token" not in saved.text - stored_settings: Final = TypeAdapter(ROISettings).validate_python( - repository.values["roi_calculator_settings"] - ) + stored_settings: Final = TypeAdapter(ROISettings).validate_python(repository.values["roi_calculator_settings"]) encrypted_token: Final = stored_settings.github_token.get_secret_value() assert encrypted_token != "private-test-token" assert "private-test-token" not in encrypted_token diff --git a/tests/unit/proxy/roi_calculator/test_estimator.py b/tests/unit/proxy/roi_calculator/test_estimator.py index e8699386c13..82ad397ee2e 100644 --- a/tests/unit/proxy/roi_calculator/test_estimator.py +++ b/tests/unit/proxy/roi_calculator/test_estimator.py @@ -5,7 +5,8 @@ from typing import Final import pytest from pydantic import TypeAdapter -from litellm.proxy.roi_calculator.estimator import Estimator +import litellm +from litellm.proxy.roi_calculator.estimator import Estimator, estimator_options from litellm.proxy.roi_calculator.github import SourceError from litellm.types.roi_calculator import ( ROICompletionRequest, @@ -15,6 +16,7 @@ from litellm.types.roi_calculator import ( ROIResponseFormat, ROISettings, ) +from litellm.utils import supports_none_reasoning_effort def _pull() -> ROIPullEvidence: @@ -44,6 +46,14 @@ def _settings() -> ROISettings: return ROISettings(estimator_model="test-estimator") +def _model_with_none_reasoning_effort() -> str: + return next( + model + for model, metadata in litellm.model_cost.items() + if metadata.get("supports_none_reasoning_effort") is True and supports_none_reasoning_effort(model) + ) + + def _completion(content: str) -> Mapping[str, object]: message: Final = MappingProxyType({"content": content}) choice: Final = MappingProxyType({"finish_reason": "stop", "message": message}) @@ -62,6 +72,7 @@ def _completion(content: str) -> Mapping[str, object]: @pytest.mark.asyncio async def test_estimator_sends_metadata_only_json_request_and_parses_valid_result(content: str) -> None: async def complete(request: ROICompletionRequest) -> object: + assert request.reasoning_effort is None evidence: Final = TypeAdapter(ROIEstimatorEvidence).validate_json(request.messages[1]["content"]) assert request.temperature == 0 expected_response_format: Final[ROIResponseFormat] = {"type": "json_object"} @@ -80,6 +91,27 @@ async def test_estimator_sends_metadata_only_json_request_and_parses_valid_resul assert result.get("effort_basis") == "without_ai" +def test_estimator_options_follow_underlying_model_metadata() -> None: + supported_model: Final = _model_with_none_reasoning_effort() + + assert estimator_options(((supported_model, None),)) == {"reasoning_effort": "none"} + assert estimator_options(((supported_model, None), ("unknown-model", None))) == {} + assert estimator_options((("unknown-model", None),)) == {} + + +@pytest.mark.asyncio +async def test_estimator_sets_none_reasoning_effort_for_supported_underlying_model() -> None: + supported_model: Final = _model_with_none_reasoning_effort() + + async def complete(request: ROICompletionRequest) -> object: + assert request.reasoning_effort == "none" + return _completion('{"hours": 1, "reasoning": "Metadata-backed capability."}') + + result: Final = await Estimator(_settings(), complete, ((supported_model, None),)).estimate(_pull()) + + assert result["hours"] == 1 + + @pytest.mark.parametrize( "content", ( diff --git a/tests/unit/proxy/roi_calculator/test_github.py b/tests/unit/proxy/roi_calculator/test_github.py index 0432dcf88c8..2e3629065d3 100644 --- a/tests/unit/proxy/roi_calculator/test_github.py +++ b/tests/unit/proxy/roi_calculator/test_github.py @@ -9,9 +9,7 @@ from pydantic import SecretStr from litellm.proxy.roi_calculator.github import GitHub, SourceError from litellm.types.roi_calculator import ROISettings -_NEXT_PAGE_HEADERS: Final = MappingProxyType( - {"link": '; rel="next"'} -) +_NEXT_PAGE_HEADERS: Final = MappingProxyType({"link": '; rel="next"'}) _PULLS_PAGE_ONE_JSON: Final = """[ { "number": 1, @@ -61,6 +59,11 @@ def _settings() -> ROISettings: ) +def _github(transport: httpx.MockTransport) -> GitHub: + client: Final = httpx.AsyncClient(transport=transport, timeout=45, follow_redirects=False) + return GitHub(_settings(), client=client) + + @pytest.mark.parametrize("repo", ("../user", "org/..")) def test_github_rejects_repository_path_segments(repo: str) -> None: with pytest.raises(ValueError, match="owner/repo format"): @@ -79,7 +82,7 @@ async def test_github_paginates_and_filters_merged_pull_requests_to_the_requeste ) return httpx.Response(200, content=_PULLS_PAGE_TWO_JSON) - github: Final = GitHub(_settings(), httpx.MockTransport(respond)) + github: Final = _github(httpx.MockTransport(respond)) try: pulls: Final = await github.pulls("org/repo", date(2026, 9, 1), date(2026, 9, 30)) finally: @@ -93,7 +96,7 @@ async def test_github_maps_upstream_errors_without_returning_response_secrets() def respond(_: httpx.Request) -> httpx.Response: return httpx.Response(401, text="private token response") - github: Final = GitHub(_settings(), httpx.MockTransport(respond)) + github: Final = _github(httpx.MockTransport(respond)) try: with pytest.raises(SourceError) as error: await github.repositories() @@ -117,7 +120,7 @@ async def test_github_repository_listing_applies_search_and_reports_next_page() content=_REPOSITORIES_JSON, ) - github: Final = GitHub(_settings(), httpx.MockTransport(respond)) + github: Final = _github(httpx.MockTransport(respond)) try: repositories, has_more = await github.repositories(query="BACK", page=2) finally: diff --git a/ui/litellm-dashboard/AGENTS.md b/ui/litellm-dashboard/AGENTS.md index 7b1234e1cf3..e5d876fad84 100644 --- a/ui/litellm-dashboard/AGENTS.md +++ b/ui/litellm-dashboard/AGENTS.md @@ -25,3 +25,13 @@ Rules beyond the enabled set were measured against the whole suite and left off Never run the full unit suite (`npx vitest run` with no path). It is 380 files and thousands of tests, it saturates the machine for many minutes, and CI runs it anyway. Run only the test files your change touches, plus any file whose failure your change could plausibly explain, by passing explicit paths Type tests are `*.test-d.ts` files run by the `types` vitest project (`npm run test:types`). Keep them out of the `src/app/(dashboard)/` route group. Vitest matches a tsc error back to the test file by path, the parentheses break that match, and `ignoreSourceErrors: true` then drops the error as if it came from a source file. The test still collects and still reports as passing, so a `.test-d.ts` under a parenthesized directory is green no matter what it asserts. Confirm any new one has teeth by breaking the type it guards and watching it fail + + + +# This is NOT the Next.js you know + +This version has breaking changes — APIs, conventions, and file structure may all differ from your training data. Read the relevant guide in `node_modules/next/dist/docs/` (resolved from this file's directory; in monorepos the `next` package may not be visible from the repo root) before writing any code. Heed deprecation notices. + +This block is written and re-added by `next dev` — verify at `node_modules/next/dist/server/lib/generate-agent-files.js`. Removing it from a diff only re-creates the uncommitted change; committing it with your work keeps the tree clean. + + diff --git a/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorDialogs.tsx b/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorDialogs.tsx index 2b78e934781..56bd60ea1a7 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorDialogs.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorDialogs.tsx @@ -4,7 +4,14 @@ import React from "react"; import { extractErrorMessage } from "@/utils/errorUtils"; import { Button } from "@/components/ui/button"; -import { Dialog, DialogContent, DialogDescription, DialogFooter, DialogHeader, DialogTitle } from "@/components/ui/dialog"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@/components/ui/dialog"; import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { effortNote, estimateLabel } from "./roiCalculatorData"; @@ -29,7 +36,9 @@ export function PullReasoningDialog({ <> {pull.title} - {pull.repo} #{pull.number} · {pull.login} + + {pull.repo} #{pull.number} · {pull.login} +

Estimated engineering hours

@@ -45,7 +54,9 @@ export function PullReasoningDialog({

Reasoning

-

{pull.estimate.reasoning || "No estimate available."}

+

+ {pull.estimate.reasoning || "No estimate available."} +

Model
@@ -132,14 +143,20 @@ export function IdentityMatchDialog({ required /> - {error &&

{error}

} + {error && ( +

+ {error} +

+ )} {existingEmail && ( )} - + diff --git a/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorView.integration.test.tsx b/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorView.integration.test.tsx index 421bb03c31a..b1ceaff38d0 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorView.integration.test.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorView.integration.test.tsx @@ -156,6 +156,37 @@ describe("ROICalculatorView", () => { ); }); + it("lets a view-only admin read the report without write controls", async () => { + const runningStatus = { + ...idleStatus, + running: true, + phase: "estimating", + stage: "Estimating pull requests", + total: 1, + }; + vi.mocked(apiClient.get).mockImplementation((path: string) => { + if (path === "/roi-calculator/settings") return Promise.resolve(settings); + if (path === "/roi-calculator/report") return Promise.resolve({ report: summary }); + return Promise.resolve(runningStatus); + }); + + render(); + + expect(await screen.findByText("Spend per estimated engineering hour")).toBeInTheDocument(); + expect(screen.getByRole("note")).toHaveTextContent("Read-only access"); + expect(screen.queryByRole("button", { name: "Run analysis" })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Cancel sync" })).not.toBeInTheDocument(); + + fireEvent.click(screen.getByRole("tab", { name: "People" })); + expect(screen.getByText("alice-work")).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "alice-work" })).not.toBeInTheDocument(); + + fireEvent.click(screen.getByRole("tab", { name: "Settings" })); + expect(screen.getByLabelText("GitHub token")).toBeDisabled(); + expect(screen.queryByRole("button", { name: "Save settings" })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Run analysis" })).not.toBeInTheDocument(); + }); + it("lets an admin open the people view and save a manual email match", async () => { render(); @@ -188,7 +219,7 @@ describe("ROICalculatorView", () => { expect(screen.getAllByText("Connect GitHub to get started")).toHaveLength(1); }); - it("shows the completed report after polling a running sync", async () => { + it("returns to Overview and shows the last sync time when completion is polled from Settings", async () => { const runningStatus = { ...idleStatus, running: true, @@ -212,8 +243,10 @@ describe("ROICalculatorView", () => { render(); expect(await screen.findByRole("progressbar", { name: "Sync progress" })).toBeInTheDocument(); + fireEvent.click(screen.getByRole("tab", { name: "Settings" })); expect(await screen.findByText("Spend per estimated engineering hour", {}, { timeout: 5000 })).toBeInTheDocument(); expect(screen.queryByRole("heading", { name: "Connect GitHub to get started" })).not.toBeInTheDocument(); + expect(screen.getByRole("status")).toHaveTextContent("Up to date · Last synced Sep 30, 2026, 12:00 PM UTC"); }); it("shows the sync error returned by the status endpoint", async () => { diff --git a/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorView.tsx b/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorView.tsx index 1431c6b7bb2..5ee490340c5 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorView.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorView.tsx @@ -11,10 +11,11 @@ import { Card, CardContent } from "@/components/ui/card"; import { Skeleton } from "@/components/ui/skeleton"; import { Tabs, TabsList, TabsTrigger } from "@/components/ui/tabs"; import { extractErrorMessage } from "@/utils/errorUtils"; +import { isProxyAdminTierRole } from "@/utils/roles"; import ROISettingsPanel from "./ROISettingsPanel"; import { IdentityMatchDialog, type PersonMatchSelection, PullReasoningDialog } from "./ROICalculatorDialogs"; import { ROIOverview, ROIPeopleView } from "./ROICalculatorViews"; -import { filterPulls } from "./roiCalculatorData"; +import { filterPulls, formatSyncedAt } from "./roiCalculatorData"; import type { ROIIdentityMapResponse, ROIIdentityMapUpdate, @@ -39,7 +40,16 @@ const IDLE_STATUS: ROISyncStatus = { error: null, }; -export default function ROICalculatorView({ accessToken }: { accessToken: string | null }) { +export default function ROICalculatorView({ + accessToken, + userRole = null, + isViewOnly = false, +}: { + accessToken: string | null; + userRole?: string | null; + isViewOnly?: boolean; +}) { + const readOnly = isViewOnly && isProxyAdminTierRole(userRole ?? ""); const [view, setView] = React.useState("overview"); const [settings, setSettings] = React.useState(null); const [summary, setSummary] = React.useState(null); @@ -93,6 +103,7 @@ export default function ROICalculatorView({ accessToken }: { accessToken: string const report = await loadReport(); if (cancelled) return; setSummary(report); + if (view === "settings") setView("overview"); } if (!cancelled) setStatus(nextStatus); }) @@ -107,30 +118,30 @@ export default function ROICalculatorView({ accessToken }: { accessToken: string cancelled = true; window.clearInterval(interval); }; - }, [accessToken, loadReport, status.running]); + }, [accessToken, loadReport, status.running, view]); const startSync = React.useCallback(async () => { - if (!accessToken) return; + if (!accessToken || readOnly) return; try { setError(null); setStatus(await apiClient.post("/roi-calculator/sync", { accessToken })); } catch (reason) { setError(extractErrorMessage(reason)); } - }, [accessToken]); + }, [accessToken, readOnly]); const cancelSync = React.useCallback(async () => { - if (!accessToken) return; + if (!accessToken || readOnly) return; try { setStatus(await apiClient.delete("/roi-calculator/sync", { accessToken })); } catch (reason) { setError(extractErrorMessage(reason)); } - }, [accessToken]); + }, [accessToken, readOnly]); const updateIdentity = React.useCallback( async (payload: ROIIdentityMapUpdate) => { - if (!accessToken) return; + if (!accessToken || readOnly) return; const response: ROIIdentityMapResponse = await apiClient.put("/roi-calculator/identity-map", { accessToken, body: payload, @@ -138,7 +149,7 @@ export default function ROICalculatorView({ accessToken }: { accessToken: string setSummary(response.report); setSettings((current) => (current ? { ...current, identity_map: response.identity_map } : current)); }, - [accessToken], + [accessToken, readOnly], ); const filteredPulls = React.useMemo(() => (summary ? filterPulls(summary.pulls, query) : []), [query, summary]); @@ -164,6 +175,9 @@ export default function ROICalculatorView({ accessToken }: { accessToken: string } const progress = status.total > 0 ? Math.min(100, (status.done / status.total) * 100) : 0; + const statusIsIdleOrComplete = status.phase === "idle" || status.phase === "complete"; + const syncIsUpToDate = !status.running && statusIsIdleOrComplete; + const syncedAt = syncIsUpToDate ? summary?.synced_at : null; return (
@@ -171,11 +185,23 @@ export default function ROICalculatorView({ accessToken }: { accessToken: string icon={} title="ROI Calculator" subtitle={ - summary - ? `${summary.start} through ${summary.end} · UTC` - : "Compare gateway spend with estimated engineering effort for merged pull requests" + <> + {summary + ? `${summary.start} through ${summary.end} · UTC` + : "Compare gateway spend with estimated engineering effort for merged pull requests"} + {syncedAt && ( + + Up to date · Last synced {formatSyncedAt(syncedAt)} + + )} + } /> + {readOnly && ( +

+ Read-only access. Settings, analysis runs, and email matches are unavailable. +

+ )}
setView(value as View)}> @@ -185,7 +211,7 @@ export default function ROICalculatorView({ accessToken }: { accessToken: string Settings - {view !== "settings" && ( + {view !== "settings" && !readOnly && (
- + {!readOnly && ( + + )} )} @@ -245,6 +273,7 @@ export default function ROICalculatorView({ accessToken }: { accessToken: string onboarding={!summary} onSaved={setSettings} onStartSync={startSync} + readOnly={readOnly} syncDisabled={status.running} /> ) : null} @@ -263,16 +292,19 @@ export default function ROICalculatorView({ accessToken }: { accessToken: string summary={summary} identityMap={settings.identity_map} onMatch={(person, login) => setMatchingPerson({ person, login })} + readOnly={readOnly} /> )} setSelectedPull(null)} /> - setMatchingPerson(null)} - onSave={updateIdentity} - /> + {!readOnly && ( + setMatchingPerson(null)} + onSave={updateIdentity} + /> + )}
); } diff --git a/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorViews.tsx b/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorViews.tsx index 93e7cc83ecd..dff2ef6ffca 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorViews.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROICalculatorViews.tsx @@ -15,18 +15,12 @@ import { import type { ChartConfig } from "@/components/ui/chart"; import { Input } from "@/components/ui/input"; import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "@/components/ui/table"; -import { - coverageLabel, - effortNote, - estimateLabel, - formatMoney, - formatNumber, -} from "./roiCalculatorData"; +import { coverageLabel, effortNote, estimateLabel, formatMoney, formatNumber } from "./roiCalculatorData"; import type { ROIPerson, ROIPull, ROISummary } from "./roiCalculatorData"; const CHART_CONFIG = { - spend: { label: "Matched spend", color: "hsl(var(--chart-1))" }, - hours: { label: "Estimated hours", color: "hsl(var(--chart-2))" }, + spend: { label: "Matched spend", color: "var(--chart-1)" }, + hours: { label: "Estimated hours", color: "var(--chart-2)" }, } satisfies ChartConfig; export function ROIOverview({ @@ -56,8 +50,8 @@ export function ROIOverview({

- {formatMoney(metrics.excluded_spend)} of {formatMoney(metrics.total_spend)} total gateway spend is excluded - from the matched cohort. + {formatMoney(metrics.excluded_spend)} of {formatMoney(metrics.total_spend)} total gateway spend is excluded from + the matched cohort.

Calculation details @@ -68,14 +62,14 @@ export function ROIOverview({ : "A rate is available when matched estimated hours are greater than zero."}

- The comparison includes {metrics.cohort_people} matched{" "} - {metrics.cohort_people === 1 ? "person" : "people"} with complete PR estimates, for the same period in - UTC. {metrics.matched_prs} of {metrics.merged_prs} PRs have email matches. {formatMoney(metrics.excluded_spend)}{" "} - of {formatMoney(metrics.total_spend)} total gateway spend is excluded. + The comparison includes {metrics.cohort_people} matched {metrics.cohort_people === 1 ? "person" : "people"}{" "} + with complete PR estimates, for the same period in UTC. {metrics.matched_prs} of {metrics.merged_prs} PRs + have email matches. {formatMoney(metrics.excluded_spend)} of {formatMoney(metrics.total_spend)} total + gateway spend is excluded.

- Gateway spend includes all of each person’s usage, across repositories. This does not measure hours saved - by AI or financial returns. + Gateway spend includes all of each person’s usage, across repositories. This does not measure hours saved by + AI or financial returns.

- )) + person.logins.map((login) => + readOnly ? ( + {login} + ) : ( + + ), + ) ) : ( Unassigned gateway spend )} diff --git a/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROISettingsPanel.tsx b/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROISettingsPanel.tsx index 7b4ff4f0a1c..c37d71d7d7a 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROISettingsPanel.tsx +++ b/ui/litellm-dashboard/src/app/(dashboard)/roi-calculator/_components/ROISettingsPanel.tsx @@ -17,6 +17,7 @@ export default function ROISettingsPanel({ onboarding, onSaved, onStartSync, + readOnly, syncDisabled, }: { accessToken: string | null; @@ -24,6 +25,7 @@ export default function ROISettingsPanel({ onboarding: boolean; onSaved: (settings: ROISettings) => void; onStartSync: () => Promise; + readOnly: boolean; syncDisabled: boolean; }) { const [apiUrl, setApiUrl] = React.useState(initialSettings.github_api_url); @@ -52,9 +54,7 @@ export default function ROISettingsPanel({ accessToken, query: { query: repositoryQuery, page }, }); - setAvailableRepos((current) => - page === 1 ? response.repositories : [...current, ...response.repositories], - ); + setAvailableRepos((current) => (page === 1 ? response.repositories : [...current, ...response.repositories])); setHasMoreRepos(response.has_more); setRepositoryPage(page); setError(null); @@ -67,7 +67,7 @@ export default function ROISettingsPanel({ const saveSettings = async (event: React.FormEvent) => { event.preventDefault(); - if (!accessToken) return; + if (!accessToken || readOnly) return; const body: ROISettingsUpdate = { github_api_url: apiUrl, repos, @@ -94,9 +94,7 @@ export default function ROISettingsPanel({ }; const toggleRepository = (name: string) => { - setRepos((current) => - current.includes(name) ? current.filter((repo) => repo !== name) : [...current, name], - ); + setRepos((current) => (current.includes(name) ? current.filter((repo) => repo !== name) : [...current, name])); }; return ( @@ -112,17 +110,31 @@ export default function ROISettingsPanel({ - {error &&

{error}

} - {message &&

{message}

} + {error && ( +

+ {error} +

+ )} + {message && ( +

+ {message} +

+ )}
void saveSettings(event)}>
- setApiUrl(event.target.value)} /> + setApiUrl(event.target.value)} + />
setClearToken(event.target.checked)} /> @@ -184,17 +197,21 @@ export default function ROISettingsPanel({ toggleRepository(repository.name)} /> {repository.name} - {repository.visibility}{repository.archived ? " · archived" : ""} + {repository.visibility} + {repository.archived ? " · archived" : ""} ))} {availableRepos.length === 0 && ( -

Load repositories to choose which pull requests to analyze.

+

+ Load repositories to choose which pull requests to analyze. +

)}
{hasMoreRepos && ( @@ -214,13 +231,16 @@ export default function ROISettingsPanel({ @@ -229,6 +249,7 @@ export default function ROISettingsPanel({