Repository navigation
fix(web): render local-path media in both dashboard chains - #1562
Merged
Merged
Conversation
Drop the legacy Gradio convention from `messages_to_markdown` and make the Web dashboard resolve local media paths in Markdown as well. `messages_to_markdown` emitted ``, a convention from the Gradio-era dashboard (#1019). `gradio` is no longer a dependency, `evalscope app` is only a deprecated alias of `evalscope service`, and nothing in the codebase interprets the prefix, so local-path images were unusable in the compare view and printed as noise by `Sample.pretty_print`. Dropping the prefix alone is not enough: `MarkdownRenderer` passed the image `src` straight to `ImageLightbox`, while only the structured `Messages` chain proxied media through `/api/v1/reports/media/file?path=`. `resolveMediaSrc` is therefore extracted into `utils/media.ts` and shared by both chains. The structured chain had a related gap: `ContentImage` values may be relative paths (e.g. the custom VQA data used by `general_vqa`), which the client cannot tell apart from a base64 payload, producing an invalid `data:` URI. Local media paths are now absolutised when serialising prediction messages, matching what `messages_to_markdown` already does, so both chains expose media identically. Verified against a real `general_vqa` run (DashScope qwen-vl-plus / qwen-vl-max on the custom VQA sample data): images now render in both the compare view and the single-report chat view, requesting the media proxy endpoint.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes the legacy gradio_api/file= Markdown convention and makes the Web dashboard consistently proxy server-side media paths for both rendering chains (structured Messages blocks and Markdown-rendered content), including fixing relative-path handling by absolutizing media paths during message serialization.
Changes:
- Update
messages_to_markdownto emit local images as plain absolute paths (nogradio_apiprefix). - Share a single
resolveMediaSrchelper across structured and Markdown rendering so local paths are proxied uniformly. - Absolutize relative media paths when serializing prediction messages, with regression tests across Python and React.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
evalscope/api/messages/chat_message.py |
Drops gradio_api/file= and emits absolute local image paths in Markdown. |
evalscope/utils/data_utils.py |
Absolutizes local media paths inside serialized structured ContentBlocks for frontend compatibility. |
evalscope/web/src/utils/media.ts |
New shared resolveMediaSrc utility to proxy absolute paths and preserve http(s)/data URIs. |
evalscope/web/src/components/single/chat/MediaBlocks.tsx |
Uses shared resolveMediaSrc for structured media blocks. |
evalscope/web/src/components/common/MarkdownRenderer.tsx |
Proxies Markdown <img src> through resolveMediaSrc to match structured chain behavior. |
evalscope/web/src/components/common/MarkdownRenderer.test.tsx |
Adds coverage for Markdown image source proxying and passthrough behavior. |
tests/api/test_messages_to_markdown.py |
Adds regression tests for Markdown emission (no gradio_api, absolutization, passthrough, truncation). |
tests/report/test_prediction_media_paths.py |
Adds tests ensuring structured-message media paths are absolutized/preserved appropriately. |
Suppressed comments (1)
tests/api/test_messages_to_markdown.py:28
- Test expectation should match the safer local-path Markdown emission (wrapping the destination in
<...>), otherwise it will fail once local paths are escaped for Markdown parsing.
assert f'})' in markdown
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Local paths were emitted as bare markdown destinations, which breaks in two ways found while addressing review feedback: - A destination containing a space or an unbalanced parenthesis does not parse as an image at all, so the whole `` degrades to literal text. - A markdown parser percent-encodes the destination, so a path with non-ASCII characters (common in the OCR datasets) or spaces reached the frontend as `/tmp/%E5%9B%BE.jpg`. `encodeURIComponent` then escaped the `%` again and the media proxy received a literal `%E5%9B%BE` path and answered 404. The destination is now wrapped in `<...>`, and the markdown image renderer decodes the src before resolving it via `resolveMarkdownMediaSrc`. Remote URLs and data: URIs are still passed through untouched so an intentionally escaped remote URL is never rewritten. Also simplify `_MEDIA_BLOCK_TYPES` (the previous mapping was an identity dict) and tighten a test helper's return type. Verified end to end on a path containing both CJK characters and spaces (`.../图片 目录/狗 图片.jpg`): the compare view and the single-report chat view both render the image, the src carries single-encoded UTF-8 with no `%25`, and the media endpoint answers 200/304.
Follow-up from review: the resolver classified sources with a case-sensitive prefix check, so `HTTPS://host/a.png` and `blob:` URIs fell through to the base64 branch, and `//cdn/a.png` was proxied as if it were a local path. In the markdown chain these all rendered fine before this PR, so it was a regression. Scheme matching is now case-insensitive and covers `blob:`. Protocol-relative URLs are only treated as remote in the markdown resolver: markdown is authored text, whereas a structured `ContentBlock` carries a server-side media path where a `//server/share/...` value should still go through the proxy. Adds `utils/media.test.ts` covering both resolvers, including the `decodeURI` fallback for a malformed escape sequence.
A blob URL is created client-side by URL.createObjectURL and is scoped to a browser session, so it can never appear in a server-side prediction record. Matching it only served a hypothetical case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Removes the legacy Gradio-era image convention from
messages_to_markdownand makes the Web dashboard resolve server-side media paths in both rendering chains.Why
messages_to_markdownemitted, introduced by #1019 when the dashboard was Gradio-based. That convention is dead:gradioappears in norequirements/*.txtand not inpyproject.tomlevalscope appis only a[DEPRECATED] Use "evalscope service" insteadaliasevalscope/web/srcortests/interpretsgradio_api/api/v1/reports/media/file?path=Consequence: every benchmark that puts a local path in
ContentImage(miniwob, browsergym, general_vqa custom data, …) had broken images in the compare view, andSample.pretty_printprinted the unusable prefix as noise.Dropping the prefix alone is not enough —
MarkdownRendererpassed the markdownsrcstraight toImageLightbox, which renders<img src>verbatim. Only the structuredMessageschain proxied media, via the module-privateresolveMediaSrcinMediaBlocks.tsx.Changes
evalscope/api/messages/chat_message.py— emitinstead of thegradio_api/file=form.evalscope/web/src/utils/media.ts(new) —resolveMediaSrcextracted here and shared. It is not exported fromMediaBlocks.tsxbecauseMediaBlocksalready importsMarkdownRenderer, so re-importing would create a cycle.MarkdownRenderer.tsx— theimgrenderer now resolvessrcthroughresolveMediaSrc, so the markdown chain proxies local paths exactly like the structured chain.evalscope/utils/data_utils.py— absolutise local media paths when serialising prediction messages (see below).Extra fix: relative paths in the structured chain
While verifying, the single-report detail view turned out to be broken too — not by this change, but pre-existing on
main.ContentImagevalues may be relative paths (the custom VQA data behindgeneral_vqausescustom_eval/multimodal/images/dog.jpg). A client cannot tell a relative path from a base64 payload, soresolveMediaSrcfell into the base64 branch and produceddata:image/jpeg;base64,custom_eval/...→net::ERR_INVALID_URL.Same prediction, before this PR:
Local media paths are now absolutised in
_serialize_messages, mirroring whatmessages_to_markdownalready did, so both chains expose media identically. Note this resolution is relative to the service process CWD — that constraint already applied to the markdown chain (messages_markdownis computed at read time), so it is not newly introduced.Verification
Real run:
general_vqaon the custom VQA sample data, DashScopeqwen-vl-plus+qwen-vl-max, then a live dashboard.API payload after the fix — both fields now carry an absolute path:
In the browser:
src = /api/v1/reports/media/file?path=%2F...%2Fdog.jpg, 200 OKTest gate:
tests/api/test_messages_to_markdown.py(new, 4 cases) — nogradio_api, absolute path emitted, relative path absolutised, data URI passthrough, base64max_lengthtruncationtests/report/test_prediction_media_paths.py(new, 3 cases) — relative path absolutised, absolute path preserved, data URI / base64 untouchedMarkdownRenderer.test.tsx(+2 cases) — local absolute path proxied; http anddata:sources untouchedChatView.test.tsxunchanged and still green (structured-chain guard)make lintclean;pytest tests/cli/test_all.py::TestRun::test_ci_litepasses;npm run test243/243;npm run lintandnpm run build(tsc -b + vite) cleanNote for reviewers
evalscope/web/distis not tracked, so the frontend part needsmake web-buildto take effect in a running dashboard.