Repository navigation
[ZEPPELIN-6744] Add AI Assistant Markdown answer component - #5570
Conversation
voidmatcha
left a comment
There was a problem hiding this comment.
Thanks. I ran the Storybook and checked the specs.
CI: the frontend job fails in npm run lint: prettier --check flags MarkdownAnswer.tsx (the new RegExp(...) call in textReferences fits on one line). prettier --write fixes it.
Fixes for the accessibility and footnote issues below are on my branch: voidmatcha/zeppelin@40506d0...ZEPPELIN-6744-pr5570-review-fixes
Please take whatever you find useful.
|
Markdown images are rendered by react-markdown's default, so once real LLM output is connected, an injected Captured request: Not a blocker here since the component has no caller yet. Please track it as a follow-up, for example: resolve image references in the app, load http(s) images only after a click, and/or restrict Related: this is a well-documented pattern in LLM assistants. The closest analog is Google Colab AI (a notebook assistant): https://embracethered.com/blog/posts/2024/google-colab-image-render-exfil/. Microsoft's Bing Chat fixed it with a Content-Security-Policy that restricts images to a few first-party domains (https://embracethered.com/blog/posts/2023/bing-chat-data-exfiltration-poc-and-fix/), and Claude stopped auto-rendering images and asks the reader to click "Show Image" (https://embracethered.com/blog/posts/2023/anthropic-fixes-claude-data-exfiltration-via-images/). |
|
All non-blocking.
|
|
Thanks for providing the fixes. I cherry-picked your six commits covering:
|
Agreed, thanks for flagging this. I’ll open a follow-up issue to track the image-loading policy so it can be addressed before real LLM output is connected. |
|
I found one follow-up issue in the fragment focus handling: setting |
voidmatcha
left a comment
There was a problem hiding this comment.
Thanks for applying the changes. I confirmed all six suggested commits are in, and thanks for catching and fixing the lost tab stop after following a footnote back-reference. The condition is right and the regression test covers it.
I agree to track the image-loading policy as a follow-up issue.
LGTM 👍 Once the prerequisite PRs merge, I will retarget this PR to master and check the final diff and CI before merging.
|
@miinhho #5562 is on It rebases without conflicts and the tests pass. Once this merges, #5572 will need the same rebase onto |
(cherry picked from commit b1a3640)
…es of references and code blocks (cherry picked from commit cdf499c)
…ent renders (cherry picked from commit fa56ba7)
…onents, prop docs, no memo (cherry picked from commit b4b8ed4)
…er Assistant files (cherry picked from commit 50321b6)
…cus to a footnote and drop useDeferredValue (cherry picked from commit bf28065)
f4b3f16 to
b54814c
Compare
|
Merged into master (baeff2f). |
### What is this PR for? Addresses the image-loading follow-up raised in [#5570](#5570 (comment)). Model-generated Markdown can include image URLs containing notebook data. Loading these images automatically sends a request to the destination without user interaction. This PR replaces automatic image loading with a **Load image** button. Before loading, the reader can review the image description, destination host, full URL, and a warning that the request may transmit information from the answer. Only HTTP(S) image URLs are supported. Approval is specific to each image in an answer. Unchanged images retain approval during streaming, including when a reference definition gains a title. Changing the destination or removing and restoring an image requires fresh approval. ### What type of PR is it? Bug Fix ### What is the Jira issue? * https://issues.apache.org/jira/browse/ZEPPELIN-6751 ### How should this be tested? From `zeppelin-web-angular/projects/zeppelin-react`, run: ```bash npm test -- src/entities/assistant/ui/MarkdownAnswer.spec.tsx npm run typecheck npm run build:storybook npm run test:storybook -- markdown-answer-images.spec.mjs ``` For manual verification, run `npm run storybook` and open **Assistant / MarkdownAnswer**: 1. In **Image Loading**, inspect the browser’s Network panel. Confirm that no image request is sent before activation. Use Enter or Space to activate **Load image** and confirm that only the selected image loads. 2. In **Streaming Images**, load the image and append the next chunk. Confirm that the image remains loaded while its destination is unchanged. When a later chunk changes the destination, confirm that the new URL is displayed and requires another activation before loading. 3. Using the image story’s `content` control, remove an approved image and restore the same Markdown. Confirm that it requires approval again. ### Questions - **License changes?** No. - **Breaking changes?** No. - **Documentation?** The React README explains image loading and the destination warning. Closes #5572 from miinhho/assistant-image-loading-policy. Signed-off-by: YONGJAE LEE <dev.yongjaelee@gmail.com>

What is this PR for?
Add a React Markdown answer component for the AI Assistant.
The component renders GFM tables and language-labelled code blocks, supports unclosed code fences during streaming, and reports paragraph reference clicks through a callback. HTML is displayed as text and dangerous link URLs are blocked.
Includes Storybook examples and unit tests for rendering, input updates, paragraph references, and unsafe content.
What type of PR is it?
Feature
What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-6744
How should this be tested?
From
zeppelin-web-angular/projects/zeppelin-react:npm ci npm test -- --maxWorkers=1 npm run typecheck npm run lint npm run build:storybook npm run storybookOpen Assistant / MarkdownAnswer and check both light and dark themes:
Questions:
Does the license files need to update?
Yes. MIT license entries and texts for react-markdown and remark-gfm are included.
Is there breaking changes for older versions?
No existing public API is changed.
Does this needs documentation?
The React README includes a short component description and a pointer to its Storybook examples.