feat(studio): polish artifact viewers and podcast persistence#5
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces persistence for generated podcast scripts as structured artifacts in the backend, updating schemas, routes, and domain payloads accordingly. On the frontend, it adds support for rendering collapsible FAQ artifacts, wraps data tables in accessible horizontally scrollable regions, and integrates restored podcast artifact downloads. Feedback from the reviewer focuses on improving internationalization (i18n) by dynamically localizing hardcoded strings using the useLanguage hook, applying defensive programming patterns (optional chaining) to external payload properties, and removing redundant ARIA attributes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| @@ -1,4 +1,4 @@ | |||
| import React, { useEffect, useMemo, useState } from 'react'; | |||
| import React, { useEffect, useId, useMemo, useState } from 'react'; | |||
There was a problem hiding this comment.
Internationalization (i18n) Import
Import useLanguage from ../App to enable internationalization (i18n) for user-facing strings in the viewer components.
| import React, { useEffect, useId, useMemo, useState } from 'react'; | |
| import React, { useEffect, useId, useMemo, useState } from 'react'; | |
| import { useLanguage } from '../App'; |
| {items.map((item: AnyRecord, index: number) => { | ||
| const expanded = openIndex === index; | ||
| const question = textValue(item.question); | ||
| const answerId = `${answerIdPrefix}-faq-answer-${index}`; | ||
| return ( | ||
| <div key={`${question}-${index}`} className={`faq-item ${expanded ? 'open' : ''}`}> | ||
| <button | ||
| className="faq-question" | ||
| aria-label={question} | ||
| aria-expanded={expanded} | ||
| aria-controls={answerId} | ||
| onClick={() => setOpenIndex(expanded ? null : index)} | ||
| > | ||
| <span>{question}</span> | ||
| <small aria-hidden="true">{index + 1} / {items.length}</small> | ||
| </button> | ||
| {expanded && <p id={answerId} className="faq-answer">{textValue(item.answer)}</p>} | ||
| </div> | ||
| ); | ||
| })} |
There was a problem hiding this comment.
Defensive Programming & Accessibility Improvement
- Defensive Programming: The
itemobject insideitems.mapis typed asAnyRecordand could potentially benullorundefinedif the payload structure from the API/LLM is malformed or unexpected. Using optional chaining (item?.questionanditem?.answer) prevents potential runtime crashes. - Redundant
aria-label: The<button>element visible text is already exactly the question (since the<small>element hasaria-hidden="true"). Specifyingaria-label={question}is redundant and can interfere with screen readers or translation tools. Removing it simplifies the markup.
| {items.map((item: AnyRecord, index: number) => { | |
| const expanded = openIndex === index; | |
| const question = textValue(item.question); | |
| const answerId = `${answerIdPrefix}-faq-answer-${index}`; | |
| return ( | |
| <div key={`${question}-${index}`} className={`faq-item ${expanded ? 'open' : ''}`}> | |
| <button | |
| className="faq-question" | |
| aria-label={question} | |
| aria-expanded={expanded} | |
| aria-controls={answerId} | |
| onClick={() => setOpenIndex(expanded ? null : index)} | |
| > | |
| <span>{question}</span> | |
| <small aria-hidden="true">{index + 1} / {items.length}</small> | |
| </button> | |
| {expanded && <p id={answerId} className="faq-answer">{textValue(item.answer)}</p>} | |
| </div> | |
| ); | |
| })} | |
| {items.map((item: AnyRecord, index: number) => { | |
| const expanded = openIndex === index; | |
| const question = textValue(item?.question); | |
| const answerId = `${answerIdPrefix}-faq-answer-${index}`; | |
| return ( | |
| <div key={`${question}-${index}`} className={`faq-item ${expanded ? 'open' : ''}`}> | |
| <button | |
| className="faq-question" | |
| aria-expanded={expanded} | |
| aria-controls={answerId} | |
| onClick={() => setOpenIndex(expanded ? null : index)} | |
| > | |
| <span>{question}</span> | |
| <small aria-hidden="true">{index + 1} / {items.length}</small> | |
| </button> | |
| {expanded && <p id={answerId} className="faq-answer">{textValue(item?.answer)}</p>} | |
| </div> | |
| ); | |
| })} |
References
- Always use defensive programming patterns (such as optional chaining) when accessing properties of objects originating from external APIs or LLM payloads to prevent runtime crashes.
| const title = textValue(payload.title || 'Data table'); | ||
| if (!columns.length) { | ||
| return <MarkdownView content={markdown || ''} className="artifact-preview" />; | ||
| } | ||
| return ( | ||
| <div className="data-table-viewer"> | ||
| <div className="data-table-viewer data-table-scroll" role="region" aria-label="横向滚动查看完整表格" tabIndex={0}> |
There was a problem hiding this comment.
Internationalization (i18n)
The aria-label for the scrollable region is hardcoded in Chinese ("横向滚动查看完整表格"). Since the application supports both Chinese and English, we should dynamically localize this label using the useLanguage hook.
const title = textValue(payload.title || 'Data table');
const { lang } = useLanguage();
if (!columns.length) {
return <MarkdownView content={markdown || ''} className="artifact-preview" />;
}
return (
<div className="data-table-viewer data-table-scroll" role="region" aria-label={lang === 'zh' ? '横向滚动查看完整表格' : 'Scroll horizontally to view full table'} tabIndex={0}>
References
- Avoid hardcoding user-facing strings in a single language when the application supports multiple locales. Use the established internationalization patterns to localize all UI labels.
| {content.downloadJsonUrl && <a className="secondary-btn" href={content.downloadJsonUrl}><Download size={14} /> JSON</a>} | ||
| {content.downloadSvgUrl && <a className="secondary-btn" href={content.downloadSvgUrl}><Download size={14} /> SVG</a>} | ||
| {content.audioUrl && <a className="secondary-btn" href={content.audioUrl}><Download size={14} /> MP3</a>} | ||
| {content.transcriptUrl && <a className="secondary-btn" href={content.transcriptUrl}><Download size={14} /> Transcript</a>} |
There was a problem hiding this comment.
Internationalization (i18n)
The label "Transcript" is hardcoded in English. Since the application supports both Chinese and English, we should localize this label using the existing lang variable.
| {content.transcriptUrl && <a className="secondary-btn" href={content.transcriptUrl}><Download size={14} /> Transcript</a>} | |
| {content.transcriptUrl && <a className="secondary-btn" href={content.transcriptUrl}><Download size={14} /> {lang === 'zh' ? '转录文本' : 'Transcript'}</a>} |
References
- Avoid hardcoding user-facing strings in a single language when the application supports multiple locales. Use the established internationalization patterns to localize all UI labels.
There was a problem hiding this comment.
Pull request overview
This PR improves Studio artifact UX (interactive FAQ + better data table horizontal scrolling) and makes podcast generation more durable by persisting podcast scripts as first-class podcast_script artifacts, including restored download metadata via /api/artifacts.
Changes:
- Add an interactive FAQ artifact viewer and enhance Data Table rendering for horizontal scrolling + accessibility affordances.
- Persist generated podcast scripts as
podcast_scriptartifacts and returnartifact_id+ structured fields from/api/podcast/generate. - Extend frontend state/tests/e2e flows to restore podcast audio/transcript download metadata after refresh.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_podcast_workflow.py | Adds an API-level test asserting podcast script persistence as an artifact. |
| frontend/src/index.css | Adds styling for scrollable data tables and FAQ cards. |
| frontend/src/components/StudioPanel.tsx | Uses persisted podcast_script artifacts and preserves download metadata/links. |
| frontend/src/components/markdown-rendering.test.tsx | Adds unit tests for FAQ rendering, restored podcast links, and scrollable tables. |
| frontend/src/components/ArtifactViewer.tsx | Implements FAQ viewer and makes data tables focusable scroll regions with captions. |
| frontend/src/App.tsx | Extends GeneratedContent to carry transcript/audio URLs/filenames and file refs from artifacts. |
| frontend/e2e/non-ppt-workflow.spec.ts | Ensures podcast artifacts survive refresh and download links remain available. |
| frontend/e2e/interactive-artifacts.spec.ts | Adds FAQ artifact flow + asserts data tables overflow in a scroll region. |
| backend/domain/artifact_schemas.py | Expands PodcastScriptArtifactPayload to include persisted audio/transcript metadata. |
| backend/api/schemas/podcast.py | Extends podcast generate response with artifact_id, title, speakers, and turns. |
| backend/api/routes/podcast.py | Persists generated podcast scripts as podcast_script artifacts and returns artifact metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| raise HTTPException( | ||
| status_code=400, | ||
| detail="请提供 source_text 或 document_ids" | ||
| ) |
Summary
podcast_scriptartifacts so they survive refresh and remain available through/api/artifacts.Context
This branch is based on
origin/mainafter the Slide Deck/AIPPT integration merge. The merge base is15641de ci: add project quality checks, which follows PR #3notebooklm-slide-deck-heavy-integration.Validation
frontend:npm test -- --runfrontend:npm run typecheckfrontend:npm run buildbackend:python -m pytest -qbackend:python -m ruff check backend testsbackend:python -m compileall -q backendfrontend e2e:playwright test e2e/interactive-artifacts.spec.ts e2e/non-ppt-workflow.spec.ts