Skip to content

Commit 8f84bee

Browse files
Aurelioloclaude
Aureliolo
and
claude
committed
refactor: address PR review findings (25 items from 10 agents + CodeRabbit)
SSRF: add discovery policy allowlist guard in ProviderHealthProber before sending auth headers to provider base_url endpoints. Accept an optional discovery_policy_loader callback to validate probe URLs. Security: truncate error messages to 200 chars, improve connect error fidelity, add MemoryError/RecursionError guard in _run_loop. Conventions: fix vendor names in tests/stories (anthropic, ollama, local-llama), split _probe_one into _execute_probe helper (50-line limit), extract _enrich_with_usage from get_provider_health, remove dto.py re-export block (800-line limit), replace type:ignore with explicit DTO construction, validate interval_seconds >= 1, use NotBlankStr for provider param in CostTracker, hoist _server_error_threshold to module constant. Docs: update ProviderHealthSummary docstring (new fields + UNKNOWN), fix get_provider_health docstring, add formatTokenCount/formatCost to brand-and-ux.md utility table, remove incorrect USD references. Tests: add usage enrichment failure graceful degradation test, add _build_auth_headers parametrized tests, add HTTP 5xx and timeout prober tests, add interval validation tests, verify token/cost rendering in ProviderDetailPage test. Stories: set NoUsage health_status to 'unknown', fix vendor fixtures. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 3596208 commit 8f84bee

12 files changed

Lines changed: 360 additions & 128 deletions

File tree

‎docs/design/brand-and-ux.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,10 @@ The following shared components live in `web/src/components/ui/` and form the bu
248248
| `getPriorityLabel()` | `utils/tasks.ts` | Maps `Priority` to display label. |
249249
| `getTaskTypeLabel()` | `utils/tasks.ts` | Maps `TaskType` to display label. |
250250
| `getProviderHealthColor()` | `utils/providers.ts` | Maps `ProviderHealthStatus` to `SemanticColor \| "muted"`. |
251+
| `formatLatency()` | `utils/providers.ts` | Formats milliseconds to human-readable string (e.g. "123ms", "1.5s"). |
252+
| `formatErrorRate()` | `utils/providers.ts` | Formats error rate percentage with <0.1% handling. |
253+
| `formatTokenCount()` | `utils/providers.ts` | Formats token count with K/M suffixes. |
254+
| `formatCost()` | `utils/providers.ts` | Formats cost value using project currency (defaults to EUR). |
251255
| `toRuntimeStatus()` | `utils/agents.ts` | Maps API-layer `AgentStatus` (HR lifecycle) to `AgentRuntimeStatus` for UI components. |
252256
| `getRiskLevelColor()` | `utils/approvals.ts` | Maps `ApprovalRiskLevel` to `SemanticColor \| "accent-dim"`. |
253257
| `getRiskLevelLabel()` | `utils/approvals.ts` | Maps `ApprovalRiskLevel` to display label. |

‎src/synthorg/api/controllers/providers.py‎

Lines changed: 46 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,48 @@
5757
logger = get_logger(__name__)
5858

5959

60+
async def _enrich_with_usage(
61+
summary: ProviderHealthSummary,
62+
app_state: AppState,
63+
name: str,
64+
) -> ProviderHealthSummary:
65+
"""Enrich a health summary with token/cost data from CostTracker.
66+
67+
Args:
68+
summary: Base health summary from the health tracker.
69+
app_state: Application state.
70+
name: Provider name.
71+
72+
Returns:
73+
Enriched summary (or unchanged if enrichment is unavailable).
74+
"""
75+
if not app_state.has_cost_tracker:
76+
return summary
77+
try:
78+
now = datetime.now(UTC)
79+
usage = await app_state.cost_tracker.get_provider_usage(
80+
name,
81+
start=now - timedelta(hours=24),
82+
end=now,
83+
)
84+
return summary.model_copy(
85+
update={
86+
"total_tokens_24h": usage.total_tokens,
87+
"total_cost_24h": usage.total_cost,
88+
},
89+
)
90+
except MemoryError, RecursionError:
91+
raise
92+
except Exception as exc:
93+
logger.warning(
94+
API_PROVIDER_USAGE_ENRICHMENT_FAILED,
95+
provider=name,
96+
error=str(exc),
97+
error_type=type(exc).__qualname__,
98+
)
99+
return summary
100+
101+
60102
class ProviderController(Controller):
61103
"""LLM provider management: CRUD, test, and presets."""
62104

@@ -245,7 +287,9 @@ async def get_provider_health(
245287
"""Get provider health summary.
246288
247289
Returns health status, error rate, average response time,
248-
and call count for the last 24 hours.
290+
call count, total tokens, and total cost for the last 24
291+
hours. Token and cost totals are enriched from the cost
292+
tracker when available.
249293
250294
Args:
251295
state: Application state.
@@ -264,29 +308,7 @@ async def get_provider_health(
264308
logger.warning(API_RESOURCE_NOT_FOUND, resource="provider", name=name)
265309
raise NotFoundError(msg)
266310
summary = await app_state.provider_health_tracker.get_summary(name)
267-
if app_state.has_cost_tracker:
268-
try:
269-
now = datetime.now(UTC)
270-
usage = await app_state.cost_tracker.get_provider_usage(
271-
name,
272-
start=now - timedelta(hours=24),
273-
end=now,
274-
)
275-
summary = summary.model_copy(
276-
update={
277-
"total_tokens_24h": usage.total_tokens,
278-
"total_cost_24h": usage.total_cost,
279-
},
280-
)
281-
except MemoryError, RecursionError:
282-
raise
283-
except Exception as exc:
284-
logger.warning(
285-
API_PROVIDER_USAGE_ENRICHMENT_FAILED,
286-
provider=name,
287-
error=str(exc),
288-
error_type=type(exc).__qualname__,
289-
)
311+
summary = await _enrich_with_usage(summary, app_state, name)
290312
logger.debug(
291313
API_PROVIDER_HEALTH_QUERIED,
292314
provider=name,

‎src/synthorg/api/dto.py‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -827,10 +827,3 @@ def to_provider_response(config: ProviderConfig) -> ProviderResponse:
827827
oauth_scope=config.oauth_scope,
828828
custom_header_name=config.custom_header_name,
829829
)
830-
831-
832-
# Re-export from dto_providers to keep imports stable.
833-
from synthorg.api.dto_providers import ( # noqa: E402, F401
834-
ProviderModelResponse,
835-
to_provider_model_response,
836-
)

‎src/synthorg/api/dto_providers.py‎

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ class ProviderModelResponse(BaseModel):
1616
Attributes:
1717
id: Model identifier.
1818
alias: Short alias for routing rules.
19-
cost_per_1k_input: Cost per 1k input tokens in USD (base currency).
20-
cost_per_1k_output: Cost per 1k output tokens in USD (base currency).
19+
cost_per_1k_input: Cost per 1k input tokens.
20+
cost_per_1k_output: Cost per 1k output tokens.
2121
max_context: Maximum context window size in tokens.
2222
estimated_latency_ms: Estimated median latency in milliseconds.
2323
supports_tools: Whether the model supports tool/function calling.
@@ -83,16 +83,20 @@ def to_provider_model_response(
8383
Returns:
8484
Enriched model response DTO.
8585
"""
86-
kwargs: dict[str, object] = {
87-
"id": config.id,
88-
"alias": config.alias,
89-
"cost_per_1k_input": config.cost_per_1k_input,
90-
"cost_per_1k_output": config.cost_per_1k_output,
91-
"max_context": config.max_context,
92-
"estimated_latency_ms": config.estimated_latency_ms,
93-
}
94-
if capabilities is not None:
95-
kwargs["supports_tools"] = capabilities.supports_tools
96-
kwargs["supports_vision"] = capabilities.supports_vision
97-
kwargs["supports_streaming"] = capabilities.supports_streaming
98-
return ProviderModelResponse(**kwargs) # type: ignore[arg-type]
86+
return ProviderModelResponse(
87+
id=config.id,
88+
alias=config.alias,
89+
cost_per_1k_input=config.cost_per_1k_input,
90+
cost_per_1k_output=config.cost_per_1k_output,
91+
max_context=config.max_context,
92+
estimated_latency_ms=config.estimated_latency_ms,
93+
supports_tools=(
94+
capabilities.supports_tools if capabilities is not None else False
95+
),
96+
supports_vision=(
97+
capabilities.supports_vision if capabilities is not None else False
98+
),
99+
supports_streaming=(
100+
capabilities.supports_streaming if capabilities is not None else True
101+
),
102+
)

‎src/synthorg/budget/tracker.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@
5858
)
5959
from synthorg.budget.cost_record import CostRecord
6060

61+
from synthorg.core.types import NotBlankStr # noqa: TC001
62+
6163
logger = get_logger(__name__)
6264

6365
_COST_WINDOW_HOURS = 168 # 7 days
@@ -248,7 +250,7 @@ async def get_records(
248250
*,
249251
agent_id: str | None = None,
250252
task_id: str | None = None,
251-
provider: str | None = None,
253+
provider: NotBlankStr | None = None,
252254
start: datetime | None = None,
253255
end: datetime | None = None,
254256
) -> tuple[CostRecord, ...]:
@@ -291,7 +293,7 @@ async def get_records(
291293

292294
async def get_provider_usage(
293295
self,
294-
provider_name: str,
296+
provider_name: NotBlankStr,
295297
*,
296298
start: datetime | None = None,
297299
end: datetime | None = None,

‎src/synthorg/providers/health.py‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,11 @@ class ProviderHealthSummary(BaseModel):
8787
avg_response_time_ms: Average response time over the last 24h.
8888
error_rate_percent_24h: Error rate percentage over the last 24h.
8989
calls_last_24h: Total calls in the last 24h.
90-
health_status: Derived (computed_field) from error rate
91-
(up/degraded/down). Not a constructor parameter.
90+
total_tokens_24h: Total tokens (input + output) in the last 24h.
91+
total_cost_24h: Total cost in the last 24h.
92+
health_status: Derived (computed_field) from call count and
93+
error rate (unknown/up/degraded/down). Not a constructor
94+
parameter.
9295
"""
9396

9497
model_config = ConfigDict(frozen=True, allow_inf_nan=False)
@@ -121,7 +124,7 @@ class ProviderHealthSummary(BaseModel):
121124
total_cost_24h: float = Field(
122125
default=0.0,
123126
ge=0.0,
124-
description="Total cost in USD (base currency) in the last 24h",
127+
description="Total cost in the last 24h",
125128
)
126129

127130
@computed_field # type: ignore[prop-decorator]

0 commit comments

Comments
 (0)