Skip to content

fix(messages): absolutise local audio and video paths in markdown output - #1833

Merged
Yunnglin merged 1 commit into
modelscope:mainfrom
Winoren:fix/markdown-media-local-paths
Oct 10, 2026
Merged

Yunnglin merged 1 commit into
modelscope:mainfrom
Winoren:fix/markdown-media-local-paths

Conversation

@Winoren

@Winoren Winoren commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

What this fixes

messages_to_markdown renders local media differently depending on the content type. The ContentImage branch resolves a local file to an absolute path and skips max_length truncation; the ContentAudio and ContentVideo branches do neither, so:

  1. a relative path is emitted verbatim, which downstream renderers cannot tell apart from a base64 payload, and
  2. max_length truncates a real path into a prefix that no longer exists.

Evidence that this is an oversight, not a dialect difference

  • fix(web): render local-path media in both dashboard chains #1562 (970e9518, "fix(web): render local-path media in both dashboard chains") introduced the os.path.isfile / os.path.abspath handling, but only in the ContentImage branch of this same function.
  • The same commit added _absolutize_media_path (evalscope/report/data_frames.py:262), whose docstring says it "mirrors what messages_to_markdown does for the markdown chain, so both chains expose local media the same way". That mirror claim only holds for images: _MEDIA_BLOCK_TYPES covers image/audio/video, and the structured chain absolutises all three.
  • web/src/utils/media.ts documents resolveMediaSrc as "shared by both rendering chains so that a local media path shows up the same way in structured ContentBlock messages and in Markdown text".
  • ContentAudio.audio / ContentVideo.video are documented as "Audio file path or base64 encoded data URL" / "Video file path, URL, or base64 encoded data URL" — a path is a supported value for both.

Repro (real code path, before this change)

messages_to_markdown([ChatMessageUser(content=[ContentVideo(video='b.mp4', format='mp4')])])
# "<video controls src='b.mp4'></video>"        # image emits an absolute path instead

messages_to_markdown([...], max_length=10)
# "<video controls src='/private/t'></video>"   # path truncated to 10 characters

Sample.pretty_print (evalscope/api/dataset/dataset.py:77) defaults to max_length=50, which is shorter than most paths, and is called from evaluator.py:515.

Change

Guard the audio and video branches with os.path.isfile, the same way the image branch already does. Non-file values (base64 payloads, data: URIs, http(s) URLs, missing files) keep their previous behaviour.

Verification

  • New tests in the existing tests/api/test_messages_to_markdown.py: 5 added, 10 pass.
  • Test effectiveness: with the source fix reverted and the new tests kept, 4 of them fail ("<video controls src='/private/t'></video>"); with the fix restored, 10 pass. The fifth (test_base64_audio_and_video_are_still_truncated_by_max_length) passes both ways and pins the unchanged base64 behaviour.
  • Differential fuzz, 2430 cases (3 content types x local / non-local values x 5 max_length values x 3 roles x with / without a reasoning block). Snapshots were taken on the fixed tree and on the reverted tree using a probe that only calls the real function and contains no logic of its own: 1950 identical, 480 changed, 0 unintended. Every change is a local-file value gaining its absolute path; no non-file value changed at all.
  • tests/api/test_messages_to_markdown.py, tests/report/test_prediction_media_paths.py, tests/api/test_io_utils.py, tests/api/test_dataset_loader.py, tests/api/test_default_data_adapter.py: 87 passed.
  • ruff check and ruff format --check clean.

Stored-data impact

messages_to_markdown output is written into prediction caches and report text, so this does change what gets persisted. Analysed separately: that markdown is display-only — nothing in the repository parses the emitted path back out (a grep for controls src finds only the two producers). Existing entries stay readable as they are; the change only makes future entries point at the same file through an absolute path instead of a relative or already-truncated one. No migration is needed.

@Yunnglin Yunnglin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM — minimal, well-tested consistency fix that restores markdown/structured-chain parity for local audio/video paths; verified the new tests fail without it and the related suite (87) passes. Thanks!

@Yunnglin
Yunnglin merged commit 2496464 into modelscope:main Oct 10, 2026
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