diff --git a/scripts/sync_cost_map.py b/scripts/sync_cost_map.py index 50f85eee64f..956e7136a88 100644 --- a/scripts/sync_cost_map.py +++ b/scripts/sync_cost_map.py @@ -385,35 +385,35 @@ def _ordered_result(cost_map: CostMap, result: CostMap, outcomes: Sequence[Provi ) -def _section_block(title: str, lines: Sequence[str], backtick: bool) -> str: - shown: Final = lines[:PR_BODY_SECTION_LIMIT] +def _section_block(title: str, lines: Sequence[str], backtick: bool, limit: int | None, rest: str) -> str: + shown: Final = lines[:limit] bullets: Final = "\n".join(f"- `{line}`" if backtick else f"- {line}" for line in shown) or "- none" overflow: Final = len(lines) - len(shown) - trailer: Final = f"\n- and {overflow} more, see the diff" if overflow else "" + trailer: Final = f"\n- and {overflow} more, see the {rest}" if overflow else "" return f"### {title} ({len(lines)})\n{bullets}{trailer}\n" -def _provider_body(outcome: ProviderOutcome) -> str: +def _provider_body(outcome: ProviderOutcome, limit: int | None) -> str: skipped: Final = ", ".join(f"{reason} ({count})" for reason, count in sorted(outcome.skipped.items())) or "none" return ( f"## {outcome.provider}\n" "\n" - f"{_section_block('Added', outcome.added, backtick=True)}" + f"{_section_block('Added', outcome.added, True, limit, 'diff')}" "\n" - f"{_section_block('Updated', outcome.updated, backtick=True)}" + f"{_section_block('Updated', outcome.updated, True, limit, 'diff')}" "\n" - f"{_section_block('Warnings needing a human call', outcome.warnings, backtick=False)}" + f"{_section_block('Warnings needing a human call', outcome.warnings, False, limit, 'workflow log')}" "\n" f"Catalog rows skipped: {skipped}\n" ) -def render_pr_body(outcome: SyncOutcome) -> str: +def render_pr_body(outcome: SyncOutcome, section_limit: int | None = PR_BODY_SECTION_LIMIT) -> str: return ( "Automated sync of the openrouter and vercel_ai_gateway entries in model_prices_and_context_window.json " f"against `GET {OPENROUTER_MODELS_URL}` and `GET {VERCEL_MODELS_URL}` by scripts/sync_cost_map.py. " "The cost-map-guard check enforces that this PR only adds or reprices models.\n" - "\n" + "\n".join(_provider_body(provider) for provider in outcome.providers) + "\n" + "\n".join(_provider_body(provider, section_limit) for provider in outcome.providers) ) @@ -454,16 +454,15 @@ def main(argv: Sequence[str]) -> int: cost_map_path: Final = args.repo_root / COST_MAP_RELPATHS[0] cost_map: Final = json.loads(cost_map_path.read_text()) outcome: Final = compute_sync(cost_map, catalogs) - body: Final = render_pr_body(outcome) if args.pr_body_file is not None: - args.pr_body_file.write_text(body) + args.pr_body_file.write_text(render_pr_body(outcome)) if args.write and outcome.has_changes: for relpath in COST_MAP_RELPATHS: (args.repo_root / relpath).write_text(_serialize(outcome.cost_map)) print(render_summary(outcome)) print() - print(body) + print(render_pr_body(outcome, section_limit=None)) if not args.write: print("dry run: no files were touched") elif not outcome.has_changes: diff --git a/tests/test_litellm/test_sync_cost_map.py b/tests/test_litellm/test_sync_cost_map.py index 54946ac86dc..b693a7211d6 100644 --- a/tests/test_litellm/test_sync_cost_map.py +++ b/tests/test_litellm/test_sync_cost_map.py @@ -277,10 +277,47 @@ def test_pr_body_caps_every_section_so_a_large_first_sync_fits_github_limit(sync body: Final = sync.render_pr_body(outcome) assert len(body) < 65_536 - assert body.count("### Added (400)") == 2 and body.count("- and 370 more, see the diff") == 6 + assert body.count("### Added (400)") == 2 and body.count("- and 370 more, see the diff") == 4 + assert body.count("- and 370 more, see the workflow log") == 2 assert body.count("- `provider/model-29: ") == 4 and "provider/model-30: " not in body +def test_workflow_log_lists_every_warning_the_capped_pr_body_drops(sync: ModuleType, tmp_path: Path, capsys) -> None: + keys: Final = tuple(f"openrouter/acme/model-{index:02d}" for index in range(40)) + catalog: Final = { + "data": [ + {"id": key.removeprefix("openrouter/"), "pricing": {"prompt": "0.000001", "completion": "0.000002"}} + for key in keys + ] + } + cost_map: Final = {**_base_map(), **{key: {"litellm_provider": "openrouter", "mode": "embedding"} for key in keys}} + for relpath in ("model_prices_and_context_window.json", "litellm/model_prices_and_context_window_backup.json"): + (tmp_path / relpath).parent.mkdir(parents=True, exist_ok=True) + (tmp_path / relpath).write_text(json.dumps(cost_map, indent=4) + "\n") + (tmp_path / "openrouter.json").write_text(json.dumps(catalog)) + body_file: Final = tmp_path / "body.md" + + code: Final = sync.main( + [ + "--openrouter-json", + str(tmp_path / "openrouter.json"), + "--vercel-json", + str(FIXTURES / "vercel_models.json"), + "--pr-body-file", + str(body_file), + "--repo-root", + str(tmp_path), + ] + ) + + body: Final = body_file.read_text() + log: Final = capsys.readouterr().out + assert code == 0 + assert "### Warnings needing a human call (40)" in body and "- and 10 more, see the workflow log" in body + assert keys[29] in body and keys[30] not in body + assert all(key in log for key in keys) and "more, see the" not in log + + @pytest.mark.parametrize( ("loader", "raw"), [