fix(batches): encode public model group on background-created output file ids - #35406
Conversation
…file ids
CheckBatchCost built unified output file ids with the provider model name, so key model-access checks resolved the file to e.g. gpt-5.5 and every GET /v1/files/{output_file_id}/content failed. Resolve the model group from the batch's managed input file, falling back to the deployment's model_name.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
|
Greptile SummaryFixes background-created batch output and error file IDs to preserve the public model group used by access checks.
Confidence Score: 5/5The PR appears safe to merge with no actionable regressions identified. The managed input-file parser safely falls back for raw or malformed IDs, supports multiple comma-separated model groups, and valid deployments provide the public model name required by the fallback path.
|
| Filename | Overview |
|---|---|
| enterprise/litellm_enterprise/proxy/common_utils/check_batch_cost.py | Background batch tracking now encodes managed output IDs with the public model group while retaining the provider model for cost calculation. |
| tests/proxy_unit_tests/test_check_batch_cost.py | Updates the previous expectation and adds end-to-end regression coverage for model extraction, authorization, and unmanaged-input fallback. |
Reviews (1): Last reviewed commit: "fix(batches): encode public model group ..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
QA'd end to end against a real proxy on localhost:4000 (local Postgres, mock OpenAI-compatible batch provider as the upstream so no provider keys are needed). The deployment maps public group Before (only After (PR code, nothing else changed): Regressions checked: plain managed input-file content retrieval -> 200; client-poll path ( Unit suites: Unrelated pre-existing issue noticed while checking (same on the parent commit, not touched by this PR): on the retrieve path the minted output id decodes to |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6062ed7. Configure here.
TLDR
Problem this solves:
How it solves it:
model_name, never the provider modelRelevant issues
Linear ticket
Resolves LIT-4964
Pre-Submission checklist
Screenshots / Proof of Fix
Run end to end against a live proxy on
localhost:4000backed by a real Postgres, with a virtual key scoped to the model group only. The upstream is a local OpenAI-compatible batch server (/v1/files,/v1/batches,/v1/batches/{id},/v1/files/{id}/content) because this sandbox has no batch-capable provider credentials; everything inside litellm, the managed-files hook, the backgroundCheckBatchCostjob, and theauth_utils->can_key_call_modelcheck, is the real code pathProxy config; the point is that the public model group and the provider model are different strings, and the poll interval is short so the background job runs during the test:
Before, with only
check_batch_cost.pychecked out at the parent commitfa56283806:After, with this branch and nothing else changed:
The id really was minted by the poller and not by a client retrieve: the proxy log carries
CheckBatchCost: converted output_file_id 'file-mockoutput-batch_mock_9f8dd7f41002' -> managed ID, andgrep -c 'GET /v1/batches/bGl' litellm_after.logreturns0Regressions checked on the same live proxy: reading the managed input file (
GET /v1/files/{input_file_id}/content) returns 200, and the client-poll path (GET /v1/batches/{id}right after create, then reading its output file) still returns 200One unrelated finding from the regression pass, same on the parent commit and not touched here: the id minted on the retrieve path decodes to
target_model_names,with nothing after it, becauselitellm/proxy/batches_endpoints/endpoints.pysets theunified_file_idhidden param on create but not on retrieve, so the hook's fallback that recovers the group from the input file never fires. That read only succeeds because an emptytarget_model_namesyields no model candidates and skips the key model-access check altogetherType
🐛 Bug Fix
Changes
CheckBatchCost._track_completed_batch_costconverts the provider's rawoutput_file_id/error_file_idinto managed unified ids when it beats the client to a completed batch. It passedmodel_namefromget_llm_provider(deployment.litellm_params.model), so the id carriedtarget_model_names,gpt-5.5. The HTTP retrieve path (managed_files.async_post_call_success_hook) instead resolves that name from the batch's managed input file, which holds the model group the caller uploaded with.That difference is not cosmetic:
auth_utils._extract_models_from_managed_resource_idturnstarget_model_namesback into the model a key must be allowed to call, so an id minted by the poller was checked against a provider model no key can call and everyGET /v1/files/{output_file_id}/contentfailed; whichever writer won the race decided whether the customer could read their results._get_managed_file_model_namenow derives the name the same way the HTTP path does, from the job's managed input file, falling back todeployment_info.model_name(still the model group) when the batch has no managed input file.get_llm_provider's model name stays where it belongs, in the cost calculation.test_raw_output_file_id_converted_to_managed_idasserted the old provider-model behavior, so its expectation moves to the deployment's model group.Final Attestation
Note
Medium Risk
Touches batch auth metadata and managed file ID encoding on a background path customers rely on for reading results; scope is narrow with strong regression tests but incorrect fallback could still break key-scoped file access.
Overview
When the CheckBatchCost background poller finishes a batch before the client polls, it mints managed unified IDs for raw
output_file_id/error_file_id. Those IDs previously used the provider model fromget_llm_providerastarget_model_names, so API keys scoped to a public model group failedGET /v1/files/{id}/contentwith model-access denials.This change adds
_get_managed_file_model_name, which derives the name from the batch job’s managed input file’starget_model_names(same idea as the HTTP managed-files path), and falls back todeployment_info.model_namewhen there is no managed input file.get_unified_output_file_idnow receives that value instead of the provider model.Tests are updated for the conversion hook expectation and new LIT-4964 coverage asserts encoded model groups and
can_key_call_modelbehavior.Reviewed by Cursor Bugbot for commit 6062ed7. Bugbot is set up for automated code reviews on this repo. Configure here.