diff --git a/lib/export/narration.ts b/lib/export/narration.ts new file mode 100644 index 0000000000..fd4306aa3b --- /dev/null +++ b/lib/export/narration.ts @@ -0,0 +1,64 @@ +/** + * Shared narration walk for the export family. + * + * The script (.md/.docx) exporter and the PPTX speaker-notes exporter each + * used to hand-roll "iterate `scene.actions`, keep `type === 'speech'`, + * join the text" — two copies of the same domain traversal that could drift on + * what counts as narration (issue #1142). Both now delegate here. + * + * The two callers genuinely differ in two knobs, both preserved explicitly: + * + * - `keepWhitespaceOnly` — the script exporter drops whitespace-only speech + * (no empty paragraphs in the document); the PPTX exporter historically kept + * it. Defaults to `true` so the PPTX path is unchanged. + * - `trim` — the script exporter trims each kept part before joining; the PPTX + * exporter does not. Defaults to `false` so the PPTX path is unchanged. + * + * Defaults therefore reproduce the PPTX behaviour, and the script exporter opts + * into its stricter behaviour at its call site. + * + * Unlike the script exporter's previous inline walk, a `speech` action with a + * missing `text` field is tolerated instead of throwing (the old code called + * `.trim()` on `undefined`). Well-typed scenes are unaffected. + */ +export interface SpeechTextOptions { + /** Keep speech actions whose `text` is empty or whitespace-only. */ + readonly keepWhitespaceOnly?: boolean; + /** Trim each kept speech text before joining. */ + readonly trim?: boolean; +} + +/** Minimal structural shape of a speech action for the walk. */ +interface SpeechLike { + readonly type: string; + readonly text?: string; +} + +/** Minimal structural shape of a scene for the walk. */ +interface SceneLike { + readonly actions?: readonly SpeechLike[] | undefined; +} + +/** + * Concatenate a scene's speech text in playback order, one part per speech + * action, joined with `\n`. + * + * Single source of truth for "what counts as narration" — see #1142. + */ +export function collectSpeechText( + scene: SceneLike | null | undefined, + options: SpeechTextOptions = {}, +): string { + const { keepWhitespaceOnly = true, trim = false } = options; + const actions = scene?.actions; + if (!actions || actions.length === 0) return ''; + + const parts: string[] = []; + for (const action of actions) { + if (action.type !== 'speech') continue; + const text = action.text ?? ''; + if (!keepWhitespaceOnly && !text.trim()) continue; + parts.push(trim ? text.trim() : text); + } + return parts.join('\n'); +} diff --git a/lib/export/use-export-pptx.ts b/lib/export/use-export-pptx.ts index 96f0e48cc0..b2120f92f9 100644 --- a/lib/export/use-export-pptx.ts +++ b/lib/export/use-export-pptx.ts @@ -19,13 +19,13 @@ import { type PPTElementLink, } from '@openmaic/dsl'; import type { Scene, SlideContent } from '@/lib/types/stage'; -import type { SpeechAction } from '@/lib/types/action'; import { getElementRange, getLineElementPath, getTableSubThemeColor } from '@/lib/utils/element'; import { type AST, toAST } from '@/lib/export/html-parser'; import { type SvgPoints, toPoints, getSvgPathRange } from '@/lib/export/svg-path-parser'; import { svg2Base64 } from '@/lib/export/svg2base64'; import { latexToOmml } from '@/lib/export/latex-to-omml'; import { createLogger } from '@/lib/logger'; +import { collectSpeechText } from './narration'; import { inlineHtmlAssets, createAssetFetcher } from './inline-assets'; import type { FetchAsset } from './inline-assets'; import { createProxiedFetch } from './proxied-fetch'; @@ -370,16 +370,14 @@ function isSVGImage(url: string) { * Extract speaker notes text from a scene's actions. * Concatenates speech text and action labels into plain text. */ +/** + * Speaker notes for one slide: the scene's speech text in action order. + * Delegates to the shared narration walk in `./narration` (#1142) with its + * historical options — whitespace-only speech is kept and parts are not + * trimmed, matching the pre-refactor notes output. + */ function buildSpeakerNotes(scene: Scene): string { - if (!scene.actions || scene.actions.length === 0) return ''; - - const parts: string[] = []; - for (const action of scene.actions) { - if (action.type === 'speech') { - parts.push((action as SpeechAction).text); - } - } - return parts.join('\n'); + return collectSpeechText(scene); } async function blobToDataUrl(blob: Blob): Promise { diff --git a/lib/export/use-export-script.ts b/lib/export/use-export-script.ts index beb1af719e..6c53e02344 100644 --- a/lib/export/use-export-script.ts +++ b/lib/export/use-export-script.ts @@ -18,6 +18,7 @@ import { useStageStore } from '@/lib/store'; import { useMediaGenerationStore } from '@/lib/store/media-generation'; import { useI18n } from '@/lib/hooks/use-i18n'; import { createLogger } from '@/lib/logger'; +import { collectSpeechText } from './narration'; import type { Scene } from '@/lib/types/stage'; const log = createLogger('ExportScript'); @@ -64,6 +65,11 @@ export function isScriptExportReady( * Collect each scene's narration: concatenate its `SpeechAction.text` values in * action order. Scenes with no speech text are omitted entirely. `slideFallback` * supplies the locale-appropriate label for scenes with an empty title. + * + * The speech walk itself lives in `./narration` so this exporter and the PPTX + * speaker-notes exporter can't drift on what counts as narration (#1142). + * Whitespace-only speech is dropped and kept text is trimmed — see the + * `keepWhitespaceOnly` / `trim` options there. */ export function collectSceneScripts( scenes: Scene[], @@ -71,13 +77,7 @@ export function collectSceneScripts( ): SceneScript[] { const scripts: SceneScript[] = []; for (const scene of scenes) { - const parts: string[] = []; - for (const action of scene.actions ?? []) { - if (action.type === 'speech' && action.text.trim()) { - parts.push(action.text.trim()); - } - } - const text = parts.join('\n'); + const text = collectSpeechText(scene, { keepWhitespaceOnly: false, trim: true }); if (!text) continue; scripts.push({ sceneId: scene.id, diff --git a/tests/export/narration.test.ts b/tests/export/narration.test.ts new file mode 100644 index 0000000000..a3c1a1686f --- /dev/null +++ b/tests/export/narration.test.ts @@ -0,0 +1,99 @@ +import { describe, it, expect } from 'vitest'; +import { collectSpeechText } from '@/lib/export/narration'; +import type { Scene } from '@/lib/types/stage'; +import type { SpeechAction } from '@/lib/types/action'; + +function speechAction(id: string, text: string): SpeechAction { + return { id, type: 'speech', text }; +} + +function scene(overrides: Partial = {}): Scene { + return { + id: 's1', + stageId: 'stg1', + title: 'Scene One', + order: 1, + type: 'slide', + content: { + type: 'slide', + canvas: { width: 960, height: 540, elements: [] }, + animations: [], + }, + actions: [], + ...overrides, + } as unknown as Scene; +} + +describe('collectSpeechText', () => { + it('returns an empty string for a scene with no actions', () => { + expect(collectSpeechText(scene({ actions: [] }))).toBe(''); + }); + + it('returns an empty string for a scene with undefined actions', () => { + expect(collectSpeechText(scene({ actions: undefined }))).toBe(''); + }); + + it('tolerates a missing scene', () => { + expect(collectSpeechText(null)).toBe(''); + expect(collectSpeechText(undefined)).toBe(''); + }); + + it('collects only speech text, in action order', () => { + const s = scene({ + actions: [ + speechAction('a1', 'First.'), + { id: 'a2', type: 'spotlight', elementId: 'e1' }, + speechAction('a3', 'Second.'), + { id: 'a4', type: 'wb_draw_text', content: 'board', x: 0, y: 0 }, + ], + }); + expect(collectSpeechText(s)).toBe('First.\nSecond.'); + }); + + it('keeps whitespace-only speech by default (PPTX behaviour)', () => { + const s = scene({ actions: [speechAction('a1', ' ')] }); + expect(collectSpeechText(s)).toBe(' '); + }); + + it('drops whitespace-only speech with keepWhitespaceOnly: false (script behaviour)', () => { + const s = scene({ actions: [speechAction('a1', ' '), speechAction('a2', 'Kept')] }); + expect(collectSpeechText(s, { keepWhitespaceOnly: false })).toBe('Kept'); + }); + + it('drops empty-string speech with keepWhitespaceOnly: false', () => { + const s = scene({ actions: [speechAction('a1', ''), speechAction('a2', 'Kept')] }); + expect(collectSpeechText(s, { keepWhitespaceOnly: false })).toBe('Kept'); + }); + + it('does not trim parts by default (PPTX behaviour)', () => { + const s = scene({ actions: [speechAction('a1', ' Hello ')] }); + expect(collectSpeechText(s)).toBe(' Hello '); + }); + + it('trims each part with trim: true (script behaviour)', () => { + const s = scene({ actions: [speechAction('a1', ' Hello ')] }); + expect(collectSpeechText(s, { trim: true })).toBe('Hello'); + }); + + it('trims each part independently, preserving the join', () => { + const s = scene({ actions: [speechAction('a1', ' one '), speechAction('a2', ' two ')] }); + expect(collectSpeechText(s, { trim: true })).toBe('one\ntwo'); + }); + + it('applies the script options together', () => { + const s = scene({ + actions: [speechAction('a1', ' '), speechAction('a2', ' Hello ')], + }); + expect(collectSpeechText(s, { keepWhitespaceOnly: false, trim: true })).toBe('Hello'); + }); + + it('keeps internal newlines and blank lines inside a single speech part', () => { + const s = scene({ actions: [speechAction('a1', 'Line one.\n\nLine two.')] }); + expect(collectSpeechText(s)).toBe('Line one.\n\nLine two.'); + }); + + it('joins multiple speech parts with a single newline, not a blank line', () => { + const s = scene({ actions: [speechAction('a1', 'A'), speechAction('a2', 'B')] }); + expect(collectSpeechText(s)).toBe('A\nB'); + }); +});