Skip to content

fix(managed_files): skip unparseable rows when listing managed files - #36021

Merged
mateo-berri merged 4 commits into
litellm_internal_stagingfrom
claude/open-source-pr-merge-ven7h6
Aug 7, 2026
Merged

fix(managed_files): skip unparseable rows when listing managed files#36021
mateo-berri merged 4 commits into
litellm_internal_stagingfrom
claude/open-source-pr-merge-ven7h6

Conversation

@mateo-berri

@mateo-berri mateo-berri commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

TLDR

Problem this solves:

How it solves it:

  • Parse each row through a helper returning None on failure
  • Skip and warn, matching how list_user_batches already behaves
  • Warning carries field locations only, never rejected values

Relevant issues

Follow-up to #35365, which fixed the null-file_object half of #35361

Linear ticket

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have added meaningful tests
  • My PR passes all CI/CD checks (e.g., lint, format, unit tests)
  • My PR's scope is as isolated as possible; it only solves 1 specific problem
  • I have received a Greptile Confidence Score of at least 4/5 before requesting a maintainer review (Greptile reviews automatically once the PR is opened; only comment @greptileai to re-request a review after pushing changes)

Screenshots / Proof of Fix

Live proxies against one shared Postgres, before leg on litellm_internal_staging at 388943ac, after leg on this PR's merge head 3deadd76, same database state for both. The listing path never reaches an inference endpoint, so the upstream provider here is a stub returning a two-file OpenAI listing; no LLM call is involved in reproducing this and nothing about the fix depends on which provider serves the list

Setup, shared by both legs: one owner key, one valid managed-file row, and one row whose file_object is missing required fields while carrying a filename, matching a partially written or schema-drifted row

curl -s http://localhost:4000/key/generate -H "Authorization: Bearer sk-1234" \
  -H "Content-Type: application/json" \
  -d '{"user_id":"qa-files-owner","models":["qa-files-model"]}'
INSERT INTO "LiteLLM_ManagedFileTable"
  (id, unified_file_id, file_object, model_mappings, flat_model_file_ids, created_by, updated_by, updated_at)
VALUES
  (gen_random_uuid()::text, 'unified-qa-valid',
   '{"id":"file-qa-valid","object":"file","bytes":120,"created_at":1700000000,"filename":"valid.jsonl","purpose":"batch","status":"processed"}'::jsonb,
   '{}'::jsonb, ARRAY['file-qa-valid'], 'qa-files-owner', 'qa-files-owner', now()),
  (gen_random_uuid()::text, 'unified-qa-corrupt',
   '{"id":"file-qa-corrupt","object":"file","filename":"confidential.jsonl"}'::jsonb,
   '{}'::jsonb, ARRAY['file-qa-corrupt'], 'qa-files-owner', 'qa-files-owner', now());

Before, litellm_internal_staging at 388943ac: the single bad row takes the whole response down, so the caller cannot see file-qa-valid either

$ curl -s -w "\nHTTP %{http_code}" "http://localhost:4000/v1/files?target_model_names=qa-files-model" \
  -H "Authorization: Bearer $OWNER_KEY"
{"error":{"message":"4 validation errors for OpenAIFileObject\nbytes\n  Field required [type=missing, input_value={'id': 'file-qa-corrupt', 'object': 'file'}, input_type=dict]\n...","type":"None","param":"None","code":"500"}}
HTTP 500

After, merge head 3deadd76, same database state: the valid file comes back and the bad row is dropped. The returned id is the row's unified_file_id because #35362's remap landed on staging while this was open, and the two compose: bad rows are skipped, surviving rows are remapped

$ curl -s -w "\nHTTP %{http_code}" "http://localhost:4003/v1/files?target_model_names=qa-files-model" \
  -H "Authorization: Bearer $OWNER_KEY"
{"data":[{"id":"unified-qa-valid","bytes":120,"created_at":1700000000,"filename":"valid.jsonl","object":"file","purpose":"batch","status":"processed","expires_at":null,"status_details":null}],"has_more":null,"object":"list"}
HTTP 200

The skip is recorded rather than silent, naming the row and the fields that failed, and the rejected values stay out of the log

$ grep -o "Failed to parse managed file object unified-qa-corrupt: .*" proxy.log
Failed to parse managed file object unified-qa-corrupt: [{'type': 'missing', 'loc': ('bytes',), 'msg': 'Field required'}, {'type': 'missing', 'loc': ('created_at',), 'msg': 'Field required'}, {'type': 'missing', 'loc': ('purpose',), 'msg': 'Field required'}]

$ grep -c "confidential.jsonl" proxy.log
0
Checkpoint Commit Route Result
Owner list, one bad row 388943ac GET /v1/files 500, valid file lost, FAIL
Owner list, one bad row 3deadd76 GET /v1/files 200, valid file returned as unified id, PASS
Skipped row logged 3deadd76 proxy log warning names row and failed fields, PASS
Rejected values in log 3deadd76 proxy log filename absent, PASS

Type

🐛 Bug Fix
✅ Test

Changes

get_user_created_file_ids validated every row's file_object inline, guarding only against None. Any row that fails OpenAIFileObject validation raised ValidationError out of the list comprehension and turned the whole listing into a 500, so one unparseable row cost the caller every file they could otherwise see

Rows now go through _parse_managed_file_object, which returns None on a null or unparseable column and logs a warning, and the comprehension keeps only what parsed. This matches list_user_batches in the same class, which already tolerates rows it cannot parse instead of failing the page. The null case returns early without warning, since the batch cost poller registers output and error ids with a null file_object as normal operation and those rows carry no provider metadata worth showing

The warning reports e.errors(include_input=False, include_url=False, include_context=False) rather than the stringified ValidationError, whose message embeds input_value with the rejected row's contents. Managed-file rows carry a caller-supplied filename, so the plain form would copy it into operational logs. Field locations, types, and messages are kept, so the diagnostics survive without the values. Non-validation failures fall back to the exception type

#35362's unified_file_id remap landed on staging while this was open and touched the same expression. The merge composes them: each row is parsed defensively, dropped if it fails, and remapped if it survives

Worth noting for anyone reading the sibling PRs on #35361: the file_object column comes back from Prisma already decoded, which the before-leg error above shows directly (input_type=dict). The JSON-string handling proposed in #35368 is not needed on this path

Final Attestation

  • The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR

get_user_created_file_ids validated every row's file_object without a
guard, so a single row failing OpenAIFileObject validation raised
ValidationError and turned the whole GET /v1/files response into a 500.
#35365 covered the null case only, leaving malformed or partial rows
able to take the entire listing down.

Rows now parse through a helper that returns None on failure and logs a
warning, matching how list_user_batches already tolerates rows it cannot
parse, so one bad row costs its own entry instead of the caller's whole
listing. Null rows stay silent since the batch cost poller registers
those legitimately.

Refs #35361
@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes managed-file listing resilient to malformed persisted file objects while sanitizing validation warnings.

  • Adds a parsing helper that skips null or unparseable managed-file rows.
  • Logs validation field diagnostics without rejected input values.
  • Adds regression coverage for mixed valid/malformed rows and sanitized warnings.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains, and the previously reported logging issue is addressed by excluding Pydantic input, URL, and context data while retaining schema-defined diagnostics.

Important Files Changed

Filename Overview
enterprise/litellm_enterprise/proxy/hooks/managed_files.py Adds isolated parsing and sanitized warning behavior so malformed rows no longer fail the complete managed-file listing.
tests/test_litellm/enterprise/proxy/test_managed_files_hook.py Adds focused regression tests confirming malformed rows are skipped and rejected filename values are omitted from warnings.

Reviews (3): Last reviewed commit: "fix(managed_files): log sanitized valida..." | Re-trigger Greptile

Comment thread enterprise/litellm_enterprise/proxy/hooks/managed_files.py
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

The skip warning interpolated the full pydantic ValidationError, whose
string embeds input_value with the rejected row's contents. Managed-file
rows carry a caller-supplied filename, so a malformed row copied that
into operational logs.

Log the error locations, types, and messages via errors() with input,
url, and context excluded, keeping the field-level diagnostics without
the values. Non-validation failures fall back to the exception type.

mateo-berri commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

@greptileai

…laude/open-source-pr-merge-ven7h6

# Conflicts:
#	enterprise/litellm_enterprise/proxy/hooks/managed_files.py

mateo-berri commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

osv-scan flags h2 4.3.0 and js-yaml, both pinned on the base branch. This PR touches no lockfiles, so the failure predates it and needs a bump on staging

@mateo-berri
mateo-berri merged commit 795fa43 into litellm_internal_staging Aug 7, 2026
75 of 76 checks passed
@mateo-berri
mateo-berri deleted the claude/open-source-pr-merge-ven7h6 branch August 7, 2026 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants