Skip to content

fix: strip only the trailing extension in LlamaCloud local file name - #723

Open
JSap0914 wants to merge 1 commit into
run-llama:mainfrom
JSap0914:fix/llamacloud-local-file-name-extension
Open

fix: strip only the trailing extension in LlamaCloud local file name#723
JSap0914 wants to merge 1 commit into
run-llama:mainfrom
JSap0914:fix/llamacloud-local-file-name-extension

Conversation

@JSap0914

@JSap0914 JSap0914 commented Jun 16, 2026

Copy link
Copy Markdown

Bug

get_local_file_name (in python/llama-index-server/.../utils/llamacloud.py) builds the local filename for a LlamaCloud-sourced file by stripping the extension like this:

file_ext = os.path.splitext(llamacloud_file_name)[1]
file_name = llamacloud_file_name.replace(file_ext, "")

str.replace removes every occurrence of the extension substring, not just the trailing one. So when a filename's stem also ends with the extension, the stem gets corrupted:

  • report.pdf.pdf → stem becomes report instead of report.pdf
  • backup.tar.tar → stem becomes backup instead of backup.tar

This produces the wrong local file name (and therefore the wrong source-node file URL via get_file_url_from_metadata).

Fix

Use the root returned by os.path.splitext, which strips only the trailing extension:

file_root, file_ext = os.path.splitext(llamacloud_file_name)
file_name = file_root

Single-extension filenames are unaffected (report.pdfreport_<pipeline_id>.pdf).

Verification

Added tests/utils/test_llamacloud.py covering single-extension, repeated-extension, and metadata-based inputs.

$ cd python/llama-index-server
$ uv run pytest tests/utils/test_llamacloud.py -q
5 passed
$ uv run pytest tests -q
33 passed

Before the fix, the repeated-extension cases failed (e.g. assert 'report_123.pdf' == 'report_pdf_123.pdf').

A changeset is included.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed local LlamaCloud filename generation to correctly handle filenames with repeated extensions (e.g., report.pdf.pdf), preventing filename corruption.
  • Tests

    • Added comprehensive unit tests for LlamaCloud filename generation and validation across multiple edge cases.

get_local_file_name used str.replace(file_ext, "") to drop the extension,
which removes every occurrence of the extension substring. For files whose
stem also ends with the extension (e.g. report.pdf.pdf) the stem was
corrupted (report instead of report.pdf). Use os.path.splitext root instead.
Copilot AI review requested due to automatic review settings June 16, 2026 05:02
@changeset-bot

changeset-bot Bot commented Jun 16, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: e97e0a1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@create-llama/llama-index-server Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b225b7b4-2b47-4cdd-a5bd-3f36f0c39607

📥 Commits

Reviewing files that changed from the base of the PR and between 97a7d9b and e97e0a1.

📒 Files selected for processing (3)
  • .changeset/llamacloud-local-file-name-extension.md
  • python/llama-index-server/llama_index/server/utils/llamacloud.py
  • python/llama-index-server/tests/utils/test_llamacloud.py

📝 Walkthrough

Walkthrough

get_local_file_name in llamacloud.py is patched to use os.path.splitext for deriving the file root and extension instead of str.replace, preventing stem corruption for filenames with repeated extensions. A new test module adds coverage for this function and is_llamacloud_file, and a changeset entry documents the fix.

Changes

get_local_file_name Extension Fix

Layer / File(s) Summary
Bug fix: os.path.splitext in get_local_file_name
python/llama-index-server/llama_index/server/utils/llamacloud.py
file_root and file_ext are now derived via os.path.splitext; file_name is set to file_root directly, replacing str.replace-based extension removal that could corrupt stems containing repeated extensions.
Tests and changeset
python/llama-index-server/tests/utils/test_llamacloud.py, .changeset/llamacloud-local-file-name-extension.md
New test module covers get_local_file_name for single extension, repeated extension, extension substring in stem, and metadata-derived input; adds a test for is_llamacloud_file. Changeset entry documents the fix.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐇 A filename with .pdf.pdf once caused such a mess,
The old str.replace would strip more, not less!
Now splitext cuts cleanly, just one little snip,
No stem goes corrupted on this bunny's trip.
Tests confirmed — the carrots are safe! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main change: fixing the trailing extension stripping logic in LlamaCloud's local file name generation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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