Repository navigation
Conversation
There was a problem hiding this comment.
The message and answer states are easy to inspect in Storybook, and the components can be developed independently of the server. Thanks for putting these together.
Could you remove the TemporaryThemeCheck story now that the actual Assistant stories are available?
The PR description still says this was opened against master, but the current base is the temporary Storybook branch. Please update that explanation so the diff and dependencies are clear to reviewers.
Since different contributors are building the UI pieces, we may want one integration pass to align the design and state presentation afterward. I can open a follow-up issue if that becomes necessary.
|
Thanks for the review!
|
|
@kimyenac #5561 is on If it helps, c55b0cf on my branch (voidmatcha@c55b0cf7b) makes that change. It cherry-picks cleanly onto the rebased branch, and the typecheck and tests pass. |
What is this PR for?
Add the AI Assistant message and answer state UI: user and Assistant messages are shown distinctly, and an answer displays its loading, streaming, tool-use, completed and error states. Everything is driven by fixture props. There is no server, transport or panel state. The components live in
projects/zeppelin-react/src/entities/assistant/ui.Base:
branch-voidmatcha-assistant-storybook-temp(branched at4599ad2d9). This PR adds only the four commits below: new files underentities/assistant/ui, plus the removal of the temporaryAssistant/Setupstory. The base already contains the prerequisite PRs it depends on: ZEPPELIN-6740 (#5562, Storybook), ZEPPELIN-6741 (#5561, the shared contract reused here) and ZEPPELIN-6743 (#5565). Once those merge, the base goes back tomaster.Commits:
AssistantMessageItemrenders a question or an answer:aria-busyand shows a caret.AssistantAnswerViewis the display union (loading/streaming/done/error). It reusesAssistantMessageandAssistantToolCallReffrom the ZEPPELIN-6741 contract.AssistantAnswerrenders one view at a time. Each state fully replaces the last.role="status") reads "Thinking…", "Writing answer…" or "Running <tool>…".AssistantToolLogis open while a tool runs and collapses once the answer completes. A tool whosetool_call.donecarried no name is labelled "Tool".AssistantAnswerErroris an antdAlert(role="alert"):failed(run.failed) andstream(AssistantStreamError) are errors with Retry.connection(AssistantConnectionError) is a warning with Reload conversation, because the answer may have finished on the server.onRetry, no button is shown.Assistant/Message: User, CompletedReply, StreamingReply, LongContent, Conversation.Assistant/Status: Loading, Streaming, ToolRunning, Completed, CompletedWithTools, Failed, StreamError, ConnectionLost, ErrorWithoutRetry.Assistant/Status/StateSwitcherswitches the fixture from the Controls panel, so you can check that the states stay consistent in both themes.Assistant/Setup › TemporaryThemeCheck(AssistantScope.stories.tsx) was a placeholder until the Assistant UI stories landed. Nothing else referenced it.Styling uses
@vanilla-extract/cssthrough the Assistant theme contract only, so light and dark need no per-component rules. The HTML/CSS screens invoidmatcha/assistant-storybook-sharewere used as a design reference. The code is written from scratch.Markdown (ZEPPELIN-6744): that PR is not open yet, so answers render through a
renderContentslot that defaults to plain text (white-space: pre-wrap). There is no second Markdown renderer here. When ZEPPELIN-6744 merges, a follow-up commit makes its component the default.Out of scope (per the issue / ZEPPELIN-6748): run event → view derivation, panel mounting in
AssistantWorkspace, real tool calls, approval, persistence and WebSocket handling.What type of PR is it?
Feature
Todos
renderContentonce it mergesWhat is the Jira issue?
How should this be tested?
In
zeppelin-web-angular/projects/zeppelin-react:npm ci,npm test(142 passed, 12 new),npm run typecheck,npm run lintandnpm run build:storybookall pass.typecheckneedsnpm ciinzeppelin-web-angularfirst, because the SDK resolvesrxjsfrom there.prettier --check.npm run storybook: both themes checked manually for every story above, including StateSwitcher.renderContent, or keeping the tool log open each fails exactly the intended test.npm run test:storybook:--workers=1, all 30 tests pass.python3 -m http.serverweb server failing (status -1) and a blank page.4599ad2d9) reproduces this: with--repeat-each=8, 1–2 of 16 time out.Screenshots (if appropriate)
See the
Assistant/MessageandAssistant/Statusstories.Questions: