mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-07 02:59:05 +00:00
fix(roi-calculator): correct estimator and dashboard behavior
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
This commit is contained in:
Devin AI
parent
99bc544735
commit
738a136150
17 changed files with 477 additions and 172 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
|
|
|
|||
|
|
@ -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],
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
(
|
||||
|
|
|
|||
|
|
@ -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": '<https://api.github.com/next>; rel="next"'}
|
||||
)
|
||||
_NEXT_PAGE_HEADERS: Final = MappingProxyType({"link": '<https://api.github.com/next>; 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:
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
<!-- BEGIN:nextjs-agent-rules -->
|
||||
|
||||
# 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.
|
||||
|
||||
<!-- END:nextjs-agent-rules -->
|
||||
|
|
|
|||
|
|
@ -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({
|
|||
<>
|
||||
<DialogHeader>
|
||||
<DialogTitle>{pull.title}</DialogTitle>
|
||||
<DialogDescription>{pull.repo} #{pull.number} · {pull.login}</DialogDescription>
|
||||
<DialogDescription>
|
||||
{pull.repo} #{pull.number} · {pull.login}
|
||||
</DialogDescription>
|
||||
</DialogHeader>
|
||||
<div>
|
||||
<p className="text-sm text-muted-foreground">Estimated engineering hours</p>
|
||||
|
|
@ -45,7 +54,9 @@ export function PullReasoningDialog({
|
|||
</div>
|
||||
<section>
|
||||
<h3 className="mb-2 font-medium">Reasoning</h3>
|
||||
<p className="whitespace-pre-wrap leading-relaxed">{pull.estimate.reasoning || "No estimate available."}</p>
|
||||
<p className="whitespace-pre-wrap leading-relaxed">
|
||||
{pull.estimate.reasoning || "No estimate available."}
|
||||
</p>
|
||||
</section>
|
||||
<dl className="grid grid-cols-[auto_1fr] gap-x-5 gap-y-2 text-xs">
|
||||
<dt className="text-muted-foreground">Model</dt>
|
||||
|
|
@ -132,14 +143,20 @@ export function IdentityMatchDialog({
|
|||
required
|
||||
/>
|
||||
</div>
|
||||
{error && <p role="alert" className="text-sm text-destructive">{error}</p>}
|
||||
{error && (
|
||||
<p role="alert" className="text-sm text-destructive">
|
||||
{error}
|
||||
</p>
|
||||
)}
|
||||
<DialogFooter>
|
||||
{existingEmail && (
|
||||
<Button disabled={busy} type="button" variant="outline" onClick={() => void save(null)}>
|
||||
Use automatic match
|
||||
</Button>
|
||||
)}
|
||||
<Button disabled={busy || !email.trim()} type="submit">{busy ? "Saving…" : "Save match"}</Button>
|
||||
<Button disabled={busy || !email.trim()} type="submit">
|
||||
{busy ? "Saving…" : "Save match"}
|
||||
</Button>
|
||||
</DialogFooter>
|
||||
</form>
|
||||
</DialogContent>
|
||||
|
|
|
|||
|
|
@ -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(<ROICalculatorView accessToken="token" userRole="Admin" isViewOnly />);
|
||||
|
||||
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(<ROICalculatorView accessToken="token" />);
|
||||
|
||||
|
|
@ -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(<ROICalculatorView accessToken="token" />);
|
||||
|
||||
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 () => {
|
||||
|
|
|
|||
|
|
@ -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<View>("overview");
|
||||
const [settings, setSettings] = React.useState<ROISettings | null>(null);
|
||||
const [summary, setSummary] = React.useState<ROISummary | null>(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<ROISyncStatus>("/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<ROISyncStatus>("/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 (
|
||||
<main className="w-full space-y-6 p-8">
|
||||
|
|
@ -171,11 +185,23 @@ export default function ROICalculatorView({ accessToken }: { accessToken: string
|
|||
icon={<BarChart3 />}
|
||||
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 && (
|
||||
<span className="mt-1 block text-xs text-muted-foreground" role="status">
|
||||
Up to date · Last synced {formatSyncedAt(syncedAt)}
|
||||
</span>
|
||||
)}
|
||||
</>
|
||||
}
|
||||
/>
|
||||
{readOnly && (
|
||||
<p className="text-sm text-muted-foreground" role="note">
|
||||
Read-only access. Settings, analysis runs, and email matches are unavailable.
|
||||
</p>
|
||||
)}
|
||||
|
||||
<div className="flex flex-wrap items-center justify-between gap-3">
|
||||
<Tabs value={view} onValueChange={(value) => setView(value as View)}>
|
||||
|
|
@ -185,7 +211,7 @@ export default function ROICalculatorView({ accessToken }: { accessToken: string
|
|||
<TabsTrigger value="settings">Settings</TabsTrigger>
|
||||
</TabsList>
|
||||
</Tabs>
|
||||
{view !== "settings" && (
|
||||
{view !== "settings" && !readOnly && (
|
||||
<Button onClick={() => void startSync()} disabled={status.running || !settings.ready}>
|
||||
<RefreshCw className={status.running ? "animate-spin" : ""} />
|
||||
{status.running ? "Syncing…" : "Run analysis"}
|
||||
|
|
@ -230,9 +256,11 @@ export default function ROICalculatorView({ accessToken }: { accessToken: string
|
|||
{status.done} of {status.total} pull requests processed · {status.reused} reused
|
||||
</p>
|
||||
</div>
|
||||
<Button variant="outline" onClick={() => void cancelSync()}>
|
||||
Cancel sync
|
||||
</Button>
|
||||
{!readOnly && (
|
||||
<Button variant="outline" onClick={() => void cancelSync()}>
|
||||
Cancel sync
|
||||
</Button>
|
||||
)}
|
||||
</CardContent>
|
||||
</Card>
|
||||
)}
|
||||
|
|
@ -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}
|
||||
/>
|
||||
)}
|
||||
<PullReasoningDialog pull={selectedPull} summary={summary} onClose={() => setSelectedPull(null)} />
|
||||
<IdentityMatchDialog
|
||||
key={matchingPerson?.login.toLowerCase() ?? "closed"}
|
||||
selection={matchingPerson}
|
||||
identityMap={settings.identity_map}
|
||||
onClose={() => setMatchingPerson(null)}
|
||||
onSave={updateIdentity}
|
||||
/>
|
||||
{!readOnly && (
|
||||
<IdentityMatchDialog
|
||||
key={matchingPerson?.login.toLowerCase() ?? "closed"}
|
||||
selection={matchingPerson}
|
||||
identityMap={settings.identity_map}
|
||||
onClose={() => setMatchingPerson(null)}
|
||||
onSave={updateIdentity}
|
||||
/>
|
||||
)}
|
||||
</main>
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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({
|
|||
<MetricCard title="PR email coverage" value={coverageLabel(summary)} />
|
||||
</section>
|
||||
<p className="text-sm text-muted-foreground">
|
||||
{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.
|
||||
</p>
|
||||
<details className="rounded-lg border p-4 text-sm">
|
||||
<summary className="cursor-pointer font-medium">Calculation details</summary>
|
||||
|
|
@ -68,14 +62,14 @@ export function ROIOverview({
|
|||
: "A rate is available when matched estimated hours are greater than zero."}
|
||||
</p>
|
||||
<p>
|
||||
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.
|
||||
</p>
|
||||
<p>
|
||||
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.
|
||||
</p>
|
||||
<Button variant="link" className="h-auto p-0" onClick={onViewPeople}>
|
||||
Review email matches
|
||||
|
|
@ -96,7 +90,7 @@ export function ROIOverview({
|
|||
<CartesianGrid vertical={false} />
|
||||
<XAxis dataKey="date" tickLine={false} axisLine={false} minTickGap={36} />
|
||||
<YAxis yAxisId="spend" tickFormatter={(value) => formatMoney(Number(value))} />
|
||||
<YAxis yAxisId="hours" orientation="right" />
|
||||
<YAxis yAxisId="hours" orientation="right" domain={[0, "auto"]} />
|
||||
<ChartTooltip content={<ChartTooltipContent />} />
|
||||
<ChartLegend content={<ChartLegendContent />} />
|
||||
<Bar yAxisId="spend" dataKey="spend" fill="var(--color-spend)" isAnimationActive={false} />
|
||||
|
|
@ -206,10 +200,12 @@ export function ROIPeopleView({
|
|||
summary,
|
||||
identityMap,
|
||||
onMatch,
|
||||
readOnly = false,
|
||||
}: {
|
||||
summary: ROISummary;
|
||||
identityMap: Record<string, string>;
|
||||
onMatch: (person: ROIPerson, login: string) => void;
|
||||
readOnly?: boolean;
|
||||
}) {
|
||||
return (
|
||||
<div className="space-y-6">
|
||||
|
|
@ -234,16 +230,20 @@ export function ROIPeopleView({
|
|||
<TableCell>
|
||||
<div className="flex flex-wrap items-center gap-2">
|
||||
{person.logins.length ? (
|
||||
person.logins.map((login) => (
|
||||
<Button
|
||||
key={login}
|
||||
variant="link"
|
||||
className="h-auto p-0"
|
||||
onClick={() => onMatch(person, login)}
|
||||
>
|
||||
{login}
|
||||
</Button>
|
||||
))
|
||||
person.logins.map((login) =>
|
||||
readOnly ? (
|
||||
<span key={login}>{login}</span>
|
||||
) : (
|
||||
<Button
|
||||
key={login}
|
||||
variant="link"
|
||||
className="h-auto p-0"
|
||||
onClick={() => onMatch(person, login)}
|
||||
>
|
||||
{login}
|
||||
</Button>
|
||||
),
|
||||
)
|
||||
) : (
|
||||
<span>Unassigned gateway spend</span>
|
||||
)}
|
||||
|
|
|
|||
|
|
@ -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<void>;
|
||||
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<HTMLFormElement>) => {
|
||||
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({
|
|||
</CardDescription>
|
||||
</CardHeader>
|
||||
<CardContent className="space-y-5">
|
||||
{error && <p className="text-sm text-destructive" role="alert">{error}</p>}
|
||||
{message && <p className="text-sm text-emerald-700" role="status">{message}</p>}
|
||||
{error && (
|
||||
<p className="text-sm text-destructive" role="alert">
|
||||
{error}
|
||||
</p>
|
||||
)}
|
||||
{message && (
|
||||
<p className="text-sm text-emerald-700" role="status">
|
||||
{message}
|
||||
</p>
|
||||
)}
|
||||
<form className="space-y-5" onSubmit={(event) => void saveSettings(event)}>
|
||||
<div className="grid gap-2">
|
||||
<Label htmlFor="roi-github-url">GitHub API URL</Label>
|
||||
<Input id="roi-github-url" value={apiUrl} onChange={(event) => setApiUrl(event.target.value)} />
|
||||
<Input
|
||||
disabled={readOnly}
|
||||
id="roi-github-url"
|
||||
value={apiUrl}
|
||||
onChange={(event) => setApiUrl(event.target.value)}
|
||||
/>
|
||||
</div>
|
||||
<div className="grid gap-2">
|
||||
<Label htmlFor="roi-github-token">GitHub token</Label>
|
||||
<Input
|
||||
autoComplete="new-password"
|
||||
disabled={readOnly}
|
||||
id="roi-github-token"
|
||||
type="password"
|
||||
value={token}
|
||||
|
|
@ -147,6 +159,7 @@ export default function ROISettingsPanel({
|
|||
<input
|
||||
aria-label="Clear saved GitHub token"
|
||||
checked={clearToken}
|
||||
disabled={readOnly}
|
||||
type="checkbox"
|
||||
onChange={(event) => setClearToken(event.target.checked)}
|
||||
/>
|
||||
|
|
@ -184,17 +197,21 @@ export default function ROISettingsPanel({
|
|||
<input
|
||||
aria-label={`Select ${repository.name}`}
|
||||
checked={repos.includes(repository.name)}
|
||||
disabled={readOnly}
|
||||
type="checkbox"
|
||||
onChange={() => toggleRepository(repository.name)}
|
||||
/>
|
||||
<span>{repository.name}</span>
|
||||
<span className="text-xs text-muted-foreground">
|
||||
{repository.visibility}{repository.archived ? " · archived" : ""}
|
||||
{repository.visibility}
|
||||
{repository.archived ? " · archived" : ""}
|
||||
</span>
|
||||
</label>
|
||||
))}
|
||||
{availableRepos.length === 0 && (
|
||||
<p className="text-sm text-muted-foreground">Load repositories to choose which pull requests to analyze.</p>
|
||||
<p className="text-sm text-muted-foreground">
|
||||
Load repositories to choose which pull requests to analyze.
|
||||
</p>
|
||||
)}
|
||||
</div>
|
||||
{hasMoreRepos && (
|
||||
|
|
@ -214,13 +231,16 @@ export default function ROISettingsPanel({
|
|||
<select
|
||||
id="roi-estimator-model"
|
||||
className="h-9 rounded-md border bg-background px-3 text-sm"
|
||||
disabled={readOnly}
|
||||
value={model}
|
||||
onChange={(event) => setModel(event.target.value)}
|
||||
>
|
||||
<option value="">Select a router model</option>
|
||||
{model && !initialSettings.available_models.includes(model) && <option value={model}>{model}</option>}
|
||||
{initialSettings.available_models.map((availableModel) => (
|
||||
<option key={availableModel} value={availableModel}>{availableModel}</option>
|
||||
<option key={availableModel} value={availableModel}>
|
||||
{availableModel}
|
||||
</option>
|
||||
))}
|
||||
</select>
|
||||
</div>
|
||||
|
|
@ -229,6 +249,7 @@ export default function ROISettingsPanel({
|
|||
<Textarea
|
||||
id="roi-estimator-prompt"
|
||||
rows={5}
|
||||
disabled={readOnly}
|
||||
value={prompt}
|
||||
onChange={(event) => setPrompt(event.target.value)}
|
||||
/>
|
||||
|
|
@ -240,21 +261,26 @@ export default function ROISettingsPanel({
|
|||
min={1}
|
||||
max={3650}
|
||||
type="number"
|
||||
disabled={readOnly}
|
||||
value={backfillDays}
|
||||
onChange={(event) => setBackfillDays(event.target.value)}
|
||||
/>
|
||||
</div>
|
||||
<div className="flex flex-wrap gap-2">
|
||||
<Button disabled={busy} type="submit">{busy ? "Saving…" : "Save settings"}</Button>
|
||||
<Button
|
||||
disabled={!initialSettings.ready || syncDisabled || busy}
|
||||
type="button"
|
||||
variant="outline"
|
||||
onClick={() => void onStartSync()}
|
||||
>
|
||||
Run analysis
|
||||
</Button>
|
||||
</div>
|
||||
{!readOnly && (
|
||||
<div className="flex flex-wrap gap-2">
|
||||
<Button disabled={busy} type="submit">
|
||||
{busy ? "Saving…" : "Save settings"}
|
||||
</Button>
|
||||
<Button
|
||||
disabled={!initialSettings.ready || syncDisabled || busy}
|
||||
type="button"
|
||||
variant="outline"
|
||||
onClick={() => void onStartSync()}
|
||||
>
|
||||
Run analysis
|
||||
</Button>
|
||||
</div>
|
||||
)}
|
||||
</form>
|
||||
</CardContent>
|
||||
</Card>
|
||||
|
|
|
|||
|
|
@ -1,30 +1,37 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { coverageLabel, effortNote, estimateLabel, filterPulls, formatMoney, formatNumber } from "./roiCalculatorData";
|
||||
import {
|
||||
coverageLabel,
|
||||
effortNote,
|
||||
estimateLabel,
|
||||
filterPulls,
|
||||
formatMoney,
|
||||
formatNumber,
|
||||
formatSyncedAt,
|
||||
} from "./roiCalculatorData";
|
||||
import type { ROIPull } from "./roiCalculatorData";
|
||||
|
||||
const pull = (overrides: Partial<ROIPull>): ROIPull =>
|
||||
({
|
||||
repo: "org/repo",
|
||||
number: 42,
|
||||
title: "Improve request routing",
|
||||
url: "https://github.com/org/repo/pull/42",
|
||||
login: "alice",
|
||||
emails: ["alice@example.com"],
|
||||
profile_email: "alice@example.com",
|
||||
merged_at: "2026-09-12T00:00:00Z",
|
||||
head_sha: "abc",
|
||||
additions: 10,
|
||||
deletions: 2,
|
||||
changed_files: 1,
|
||||
commit_count: 1,
|
||||
incomplete_metadata: false,
|
||||
estimate: { status: "estimated", hours: 4.5, reasoning: "Metadata-based estimate.", cached: false },
|
||||
email: "alice@example.com",
|
||||
match_method: "profile email",
|
||||
matched: true,
|
||||
...overrides,
|
||||
});
|
||||
const pull = (overrides: Partial<ROIPull>): ROIPull => ({
|
||||
repo: "org/repo",
|
||||
number: 42,
|
||||
title: "Improve request routing",
|
||||
url: "https://github.com/org/repo/pull/42",
|
||||
login: "alice",
|
||||
emails: ["alice@example.com"],
|
||||
profile_email: "alice@example.com",
|
||||
merged_at: "2026-09-12T00:00:00Z",
|
||||
head_sha: "abc",
|
||||
additions: 10,
|
||||
deletions: 2,
|
||||
changed_files: 1,
|
||||
commit_count: 1,
|
||||
incomplete_metadata: false,
|
||||
estimate: { status: "estimated", hours: 4.5, reasoning: "Metadata-based estimate.", cached: false },
|
||||
email: "alice@example.com",
|
||||
match_method: "profile email",
|
||||
matched: true,
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const summary = {
|
||||
metrics: { matched_prs: 1, merged_prs: 2 },
|
||||
|
|
@ -38,6 +45,11 @@ describe("ROI calculator display helpers", () => {
|
|||
expect(formatNumber(null)).toBe("—");
|
||||
});
|
||||
|
||||
it("formats report sync timestamps in UTC", () => {
|
||||
expect(formatSyncedAt("2026-09-30T12:00:00Z")).toBe("Sep 30, 2026, 12:00 PM UTC");
|
||||
expect(formatSyncedAt("invalid")).toBe("invalid");
|
||||
});
|
||||
|
||||
it("keeps caveat copy tied to the estimate basis and reports match coverage", () => {
|
||||
expect(effortNote("without_ai")).toContain("not actual hours worked or hours saved");
|
||||
expect(effortNote(null)).toContain("Earlier estimates");
|
||||
|
|
|
|||
|
|
@ -13,6 +13,16 @@ export type ROIReportResponse = components["schemas"]["ROIReportResponse"];
|
|||
export type ROIIdentityMapUpdate = components["schemas"]["ROIIdentityMapUpdate"];
|
||||
export type ROIIdentityMapResponse = components["schemas"]["ROIIdentityMapResponse"];
|
||||
|
||||
const SYNCED_AT_FORMAT_OPTIONS: Intl.DateTimeFormatOptions = {
|
||||
year: "numeric",
|
||||
month: "short",
|
||||
day: "numeric",
|
||||
hour: "numeric",
|
||||
minute: "2-digit",
|
||||
timeZone: "UTC",
|
||||
timeZoneName: "short",
|
||||
};
|
||||
|
||||
export const formatMoney = (value: number | null | undefined): string =>
|
||||
value == null
|
||||
? "—"
|
||||
|
|
@ -25,6 +35,12 @@ export const formatMoney = (value: number | null | undefined): string =>
|
|||
export const formatNumber = (value: number | null | undefined): string =>
|
||||
value == null ? "—" : new Intl.NumberFormat("en-US", { maximumFractionDigits: 1 }).format(value);
|
||||
|
||||
export const formatSyncedAt = (value: string): string => {
|
||||
const timestamp = Date.parse(value);
|
||||
if (!Number.isFinite(timestamp)) return value;
|
||||
return new Intl.DateTimeFormat("en-US", SYNCED_AT_FORMAT_OPTIONS).format(timestamp);
|
||||
};
|
||||
|
||||
export const effortNote = (basis: string | null | undefined): string =>
|
||||
basis === "without_ai"
|
||||
? "Estimated engineering hours without AI assistance, not actual hours worked or hours saved."
|
||||
|
|
@ -32,8 +48,7 @@ export const effortNote = (basis: string | null | undefined): string =>
|
|||
|
||||
export const coverageLabel = (summary: {
|
||||
metrics: Pick<ROISummary["metrics"], "matched_prs" | "merged_prs">;
|
||||
}): string =>
|
||||
`${summary.metrics.matched_prs} of ${summary.metrics.merged_prs} PRs have email matches`;
|
||||
}): string => `${summary.metrics.matched_prs} of ${summary.metrics.merged_prs} PRs have email matches`;
|
||||
|
||||
export const estimateLabel = (estimate: ROIEstimate): string => {
|
||||
if (estimate.status === "estimated") return `${formatNumber(estimate.hours)} hrs`;
|
||||
|
|
|
|||
|
|
@ -4,6 +4,6 @@ import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized";
|
|||
import ROICalculatorView from "./_components/ROICalculatorView";
|
||||
|
||||
export default function ROICalculatorPage() {
|
||||
const { accessToken } = useAuthorized();
|
||||
return <ROICalculatorView accessToken={accessToken} />;
|
||||
const { accessToken, userRole, isViewOnly } = useAuthorized();
|
||||
return <ROICalculatorView accessToken={accessToken} userRole={userRole} isViewOnly={isViewOnly} />;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue