diff --git a/app/opentelemetry-collector/builder/costs/__init__.py b/app/opentelemetry-collector/builder/costs/__init__.py index b8c638e90..5bcce211f 100644 --- a/app/opentelemetry-collector/builder/costs/__init__.py +++ b/app/opentelemetry-collector/builder/costs/__init__.py @@ -116,6 +116,7 @@ def load_model_costs() -> list[ModelCost]: cost = ModelCost.from_tokencost_json(model_name, model_data) except pydantic.ValidationError: print(f"Model {model_name} has invalid data. Skipping.") + continue costs.append(cost) print(f"Loaded {len(costs)} model costs from {json_path}") diff --git a/app/opentelemetry-collector/builder/tests/test_model_cost.py b/app/opentelemetry-collector/builder/tests/test_model_cost.py index 678ab2c62..c75a7b470 100644 --- a/app/opentelemetry-collector/builder/tests/test_model_cost.py +++ b/app/opentelemetry-collector/builder/tests/test_model_cost.py @@ -1,3 +1,4 @@ +import json import pytest from decimal import Decimal from pathlib import Path @@ -218,4 +219,37 @@ def test_load_model_costs_integration(self): # At least one of these common models should be present assert any(model in model_names for model in common_models), \ - f"None of the common models {common_models} found in {model_names[:5]}..." \ No newline at end of file + f"None of the common models {common_models} found in {model_names[:5]}..." + +class TestLoadModelCostsInvalidRows: + """Regression tests for skipping invalid rows in load_model_costs (issue #1460).""" + + VALID_A = {"litellm_provider": "openai", "input_cost_per_token": 0.00001, "output_cost_per_token": 0.00003} + VALID_B = {"litellm_provider": "anthropic", "input_cost_per_token": 0.000015, "output_cost_per_token": 0.000075} + # zero input cost is converted to None by the validator, which fails the required Decimal field + INVALID = {"litellm_provider": "openai", "input_cost_per_token": 0, "output_cost_per_token": 0.00003} + + def _load(self, tmp_path, monkeypatch, data): + (tmp_path / "model_prices.json").write_text(json.dumps(data)) + monkeypatch.setattr("builder.costs.get_tokencost_path", lambda: tmp_path) + return load_model_costs() + + def test_invalid_first_row_is_skipped(self, tmp_path, monkeypatch): + """An invalid first row must not raise UnboundLocalError and must be omitted.""" + costs = self._load(tmp_path, monkeypatch, { + "bad-model": self.INVALID, + "gpt-4": self.VALID_A, + "claude-3-opus": self.VALID_B, + }) + + assert [cost.model for cost in costs] == ["gpt-4", "claude-3-opus"] + + def test_invalid_middle_row_does_not_duplicate_previous(self, tmp_path, monkeypatch): + """An invalid row after a valid one must not re-append the previous model.""" + costs = self._load(tmp_path, monkeypatch, { + "gpt-4": self.VALID_A, + "bad-model": self.INVALID, + "claude-3-opus": self.VALID_B, + }) + + assert [cost.model for cost in costs] == ["gpt-4", "claude-3-opus"]