From 41887af6380e42a21bfff743275211daaa9569e9 Mon Sep 17 00:00:00 2001 From: Elias-Sun <293278397+Elias-Sun@users.noreply.github.com> Date: Tue, 20 Oct 2026 18:54:08 +1100 Subject: [PATCH] fix(collector-builder): skip invalid rows in load_model_costs The ValidationError handler logged that the model was being skipped but then fell through to `costs.append(cost)`. An invalid first row raised UnboundLocalError, and an invalid row after a valid one re-appended the previous model. Continue to the next row instead. Adds regression tests for an invalid row in the first and middle positions. Fixes #1460 Co-Authored-By: Claude Opus 5.5 --- .../builder/costs/__init__.py | 1 + .../builder/tests/test_model_cost.py | 36 ++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) 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"]