From ea385d1af1e20506f9b4502f1f36d767355087e6 Mon Sep 17 00:00:00 2001 From: Phil Merrell Date: Tue, 15 Sep 2026 14:17:17 -0600 Subject: [PATCH 01/11] fix(excel-tool): document freeze panes, number formats and formula caching Audited 17 sheets across four real agent-generated workbooks pulled from the dev files store. Three gaps, all traceable to the docstring: freeze_panes: set on 0 of 17 sheets. Every generated workbook scrolls its header row off the top. The docstring never mentions it. number_format: 16 cells across all 17 sheets carry anything but 'General', and two of the four workbooks have none at all. Currency renders as 1234567.891 and rates as 0.1834. Also never mentioned. Formulas are written with no cached result -- openpyxl does not evaluate them. Excel fills them in on open, so a human never sees the problem, but until then the cell is empty to every other reader: all four formulas in the corpus read back as None under `data_only=True`. That blinds read_excel_spreadsheet to totals the agent itself just wrote, and would show a blank cell in the in-app preview pane. Deliberately NOT added: column widths, fills, alignment and borders. The audit shows the model already emits all four heavily (461 fill / 1150 alignment / 1057 border cells) with no example to copy, so documenting them would be prefix cost for no behavior change. The three above are the ones it never does unprompted. Guidance is folded into the example the model copies rather than added as separate notes, following the word-tool fix in 6498ad77. This docstring is part of the cacheable `toolConfig` prefix, so it costs a one-time cache re-write per session on the next turn after deploy. Co-Authored-By: Claude Opus 5 --- .../builtin_tools/excel_spreadsheet_tool.py | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/backend/src/agents/builtin_tools/excel_spreadsheet_tool.py b/backend/src/agents/builtin_tools/excel_spreadsheet_tool.py index a7c5cd5e..bfc13e8d 100644 --- a/backend/src/agents/builtin_tools/excel_spreadsheet_tool.py +++ b/backend/src/agents/builtin_tools/excel_spreadsheet_tool.py @@ -277,6 +277,28 @@ async def create_excel_spreadsheet( ws.append(['Q1', 100]) ws.append(['Q2', 120]) ws['B4'] = '=SUM(B2:B3)' + # Freeze the header so it stays visible while + # scrolling -- do this on EVERY sheet that has a + # header row ('A2' freezes row 1, 'B2' also freezes + # column A): + ws.freeze_panes = 'A2' + # Give every number a format. Without one, Excel + # shows raw values -- 1234567.891 instead of + # $1,234,568, and 0.1834 instead of 18.3%: + for cell in ws['B'][1:]: + cell.number_format = '#,##0' + # Common formats: '#,##0.00' (2dp), '$#,##0' + # (currency), '0.0%' (percent -- store 0.183, not + # 18.3), 'yyyy-mm-dd' (date). + + Formulas are written WITHOUT a cached result, because + openpyxl does not evaluate them. Excel fills them in on + open, but until then the cell reads as empty to + everything else -- including read_excel_spreadsheet and + the in-app preview. So when a total is meant to be read + back or shown, compute it in Python and write the value + (optionally alongside the formula on another cell): + ws['B4'] = sum(r[1] for r in rows) Example (add a second sheet + a bar chart): ws2 = wb.create_sheet('Chart') From e818f4413765a46360182e02ac3e049a827ec7f6 Mon Sep 17 00:00:00 2001 From: Phil Merrell Date: Tue, 15 Sep 2026 14:17:17 -0600 Subject: [PATCH 02/11] fix(pptx-tool): document speaker notes on the create tool read_powerpoint_presentation extracts speaker notes, but create_powerpoint_presentation never says how to write them -- the toolset could read a capability it could not produce. Across four real decks from the dev files store (27 slides), not one slide has notes. The design guidance already pushes for sparse slides; without notes that detail is simply lost rather than moved to where a presenter would say it. Same cacheable-prefix cost note as the Excel and Word docstring fixes. Co-Authored-By: Claude Opus 5 --- .../agents/builtin_tools/powerpoint_presentation_tool.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/backend/src/agents/builtin_tools/powerpoint_presentation_tool.py b/backend/src/agents/builtin_tools/powerpoint_presentation_tool.py index 3dedddad..d2dbf041 100644 --- a/backend/src/agents/builtin_tools/powerpoint_presentation_tool.py +++ b/backend/src/agents/builtin_tools/powerpoint_presentation_tool.py @@ -368,6 +368,14 @@ async def create_powerpoint_presentation( Inches(8), Inches(2)).table tbl.cell(0, 0).text = 'Quarter'; tbl.cell(0, 1).text = 'Revenue' + Speaker notes -- add them to every content slide unless + the user says otherwise. They are what the presenter + actually says, so keep the detail there and the slide + itself sparse: + slide.notes_slide.notes_text_frame.text = ( + 'Revenue grew 15% on enterprise renewals; ' + 'call out the churn improvement before moving on.') + A matplotlib chart image: import matplotlib.pyplot as plt plt.figure(figsize=(8, 4.5)) From 285d621feaef18b51b5fcf548293b1235cb1406c Mon Sep 17 00:00:00 2001 From: Phil Merrell Date: Tue, 15 Sep 2026 15:47:13 -0600 Subject: [PATCH 03/11] chore(deps): add pptx-preview 1.0.7 with echarts stubbed out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pptx-preview reads OOXML and reproduces each slide's own geometry, including the theme inherited from slide masters and layouts — which is what preserves branding on decks built from an uploaded template. Tested against every .pptx in the dev files store: three real agent-generated decks render correctly, including a 4.3 MB branded template deck with 15 images in 122 ms. It reaches for ECharts in exactly one place, rendering a *native* OOXML chart part, through a static namespace import that cannot be tree-shaken. Our decks never contain one — create_powerpoint_presentation embeds charts as matplotlib PNGs via add_picture — and the survey found zero chart parts across all four decks. Shipping ECharts anyway would be 337 kB gzipped, five sixths of the viewer's weight, and every release below 6.1.0 carries GHSA-fgmj-fm8m-jvvx. So `echarts` resolves to a local stub instead: the lazy chunk is 48 kB gzipped rather than 357 kB, and `npm audit` returns to its exact pre-existing baseline of 11 findings — pptx-preview contributes none. The cost is that an uploaded deck containing a native chart throws, and the viewer reports it as unreadable. shims/echarts-stub/README.md records the trade and why a top-level dependency is the mechanism that works (tsconfig `paths` does not reach a dependency's own imports, and a `file:` spec nested under `overrides` resolves relative to wherever npm places the package). Co-Authored-By: Claude Opus 5 --- frontend/ai.client/package-lock.json | 30 ++++++++ frontend/ai.client/package.json | 5 ++ .../ai.client/shims/echarts-stub/README.md | 72 +++++++++++++++++++ .../ai.client/shims/echarts-stub/index.d.ts | 4 ++ .../ai.client/shims/echarts-stub/index.js | 24 +++++++ .../ai.client/shims/echarts-stub/package.json | 17 +++++ 6 files changed, 152 insertions(+) create mode 100644 frontend/ai.client/shims/echarts-stub/README.md create mode 100644 frontend/ai.client/shims/echarts-stub/index.d.ts create mode 100644 frontend/ai.client/shims/echarts-stub/index.js create mode 100644 frontend/ai.client/shims/echarts-stub/package.json diff --git a/frontend/ai.client/package-lock.json b/frontend/ai.client/package-lock.json index e826acc4..43e950f6 100644 --- a/frontend/ai.client/package-lock.json +++ b/frontend/ai.client/package-lock.json @@ -22,11 +22,13 @@ "chart.js": "4.5.1", "clipboard": "2.0.11", "docx-preview": "0.4.0", + "echarts": "file:./shims/echarts-stub", "katex": "0.16.45", "marked": "17.0.6", "mermaid": "11.16.1", "ng2-charts": "10.0.0", "ngx-markdown": "21.2.0", + "pptx-preview": "1.0.7", "prismjs": "1.30.0", "rxjs": "7.8.2", "tslib": "2.8.1", @@ -7963,6 +7965,10 @@ "node": ">= 0.4" } }, + "node_modules/echarts": { + "resolved": "shims/echarts-stub", + "link": true + }, "node_modules/ee-first": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/ee-first/-/ee-first-1.1.1.tgz", @@ -9523,6 +9529,12 @@ "@lmdb/lmdb-win32-x64": "3.5.1" } }, + "node_modules/lodash": { + "version": "4.18.1", + "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.18.1.tgz", + "integrity": "sha512-dMInicTPVE8d1e5otfwmmjlxkZoUpiVLwyeTdUsi/Caj/gfzzblBcCE5sRHV/AsjuCmxWrte2TNGSYuCeCq+0Q==", + "license": "MIT" + }, "node_modules/lodash-es": { "version": "4.18.1", "resolved": "https://registry.npmjs.org/lodash-es/-/lodash-es-4.18.1.tgz", @@ -10889,6 +10901,19 @@ "postcss": "^8.4.31" } }, + "node_modules/pptx-preview": { + "version": "1.0.7", + "resolved": "https://registry.npmjs.org/pptx-preview/-/pptx-preview-1.0.7.tgz", + "integrity": "sha512-YByocJuyxAR4YB4Q3+VAxdLfEvA5LojG1gAJsx2Mw0QU5FJPps/2fkJOupJ6oBbA+KdWRpuAk6G6T34rKCHVxw==", + "license": "ISC", + "dependencies": { + "echarts": "^5.5.1", + "jszip": "^3.10.1", + "lodash": "^4.17.21", + "tslib": "^2.7.0", + "uuid": "^10.0.0" + } + }, "node_modules/prismjs": { "version": "1.30.0", "resolved": "https://registry.npmjs.org/prismjs/-/prismjs-1.30.0.tgz", @@ -12946,6 +12971,11 @@ "integrity": "sha512-Eky7p2Z1Ig3NnbfodSPoARCjKBSTFMnE/ACsP1L/XJEfY4SdOFce19BsUCWVwL6K5ABZFy5J3bjcMWffX+YM3Q==", "license": "MIT", "peer": true + }, + "shims/echarts-stub": { + "name": "echarts-stub", + "version": "5.6.0", + "license": "MIT" } } } diff --git a/frontend/ai.client/package.json b/frontend/ai.client/package.json index 0895e4fb..a16611ca 100644 --- a/frontend/ai.client/package.json +++ b/frontend/ai.client/package.json @@ -43,11 +43,13 @@ "chart.js": "4.5.1", "clipboard": "2.0.11", "docx-preview": "0.4.0", + "echarts": "file:./shims/echarts-stub", "katex": "0.16.45", "marked": "17.0.6", "mermaid": "11.16.1", "ng2-charts": "10.0.0", "ngx-markdown": "21.2.0", + "pptx-preview": "1.0.7", "prismjs": "1.30.0", "rxjs": "7.8.2", "tslib": "2.8.1", @@ -88,6 +90,9 @@ "@babel/core": ">=7.29.6 <8.0.0", "mermaid": { "uuid": "14.0.0" + }, + "pptx-preview": { + "uuid": "14.0.0" } } } diff --git a/frontend/ai.client/shims/echarts-stub/README.md b/frontend/ai.client/shims/echarts-stub/README.md new file mode 100644 index 00000000..2841192f --- /dev/null +++ b/frontend/ai.client/shims/echarts-stub/README.md @@ -0,0 +1,72 @@ +# echarts-stub + +A deliberately empty stand-in for [`echarts`](https://echarts.apache.org/), +substituted into `pptx-preview` by declaring it as the project's own +`echarts` dependency: + +```json +"dependencies": { "echarts": "file:./shims/echarts-stub" } +``` + +Its version (`5.6.0`) satisfies the `^5.5.1` that `pptx-preview` asks +for, so npm dedupes both onto this one directory instead of fetching the +real library. + +## Why + +`pptx-preview` reaches for ECharts in exactly one place: rendering a +*native* OOXML chart part (`ppt/charts/*.xml`) — a chart PowerPoint draws +itself from embedded data, rather than a picture. It does so via +`import * as echarts from 'echarts'`, a static namespace import, so no +amount of tree-shaking will drop it. Depending on `pptx-preview` means +shipping the whole charting library. + +Measured against this repo's build, that is the difference between a +**357 kB** and a **~60 kB** gzipped lazy chunk — ECharts is roughly five +sixths of the viewer's weight. + +We do not need it. `create_powerpoint_presentation` builds charts by +rendering a matplotlib PNG and calling `add_picture`, which lands in the +deck as an ordinary image. A survey of every `.pptx` in the dev files +store found **zero** native chart parts across four real decks, including +a 4.3 MB branded template deck carrying 15 images. + +## What it costs + +An *uploaded* deck that does contain a native chart throws instead of +drawing it, and `PptxViewerComponent` reports the file as unreadable +rather than silently dropping a slide's centrepiece. That is the whole +trade. + +## Why a dependency, and not `tsconfig` `paths` + +The import lives inside `node_modules/pptx-preview/dist/pptx-preview.es.js`, +a pre-built JavaScript file. `paths` governs TypeScript's resolution of +*our own sources*; the bundler resolves a dependency's own imports with +the node resolver and walks straight past it. This was tried first and +the built chunk still contained the whole of ECharts. + +Substituting the package is the only mechanism that applies at the point +the import is resolved. It has to be a **top-level dependency** rather +than a `file:` spec nested under `overrides`: npm resolves the latter +relative to wherever it happens to place the package, so the symlink +lands in a different spot depending on whether the dependency is hoisted, +and it pointed at a non-existent path. A top-level `file:` spec is +defined to resolve against the package root. + +Verified by grepping the built chunks for `zrender`, ECharts' renderer, +which is absent. + +## Removing it + +Replace the `echarts` dependency with a real version and delete this +directory. Pin **6.1.0 or later**: every release below it carries +GHSA-fgmj-fm8m-jvvx, and `pptx-preview`'s own `^5.5.1` range resolves to +a vulnerable one. + +## Why the version says 5.6.0 + +It has to satisfy the `^5.5.1` range `pptx-preview` declares, or `npm ls` +reports the tree as invalid and exits non-zero. The number tracks that +range and says nothing about the contents — there is no ECharts code +here at any version. diff --git a/frontend/ai.client/shims/echarts-stub/index.d.ts b/frontend/ai.client/shims/echarts-stub/index.d.ts new file mode 100644 index 00000000..4ab70f16 --- /dev/null +++ b/frontend/ai.client/shims/echarts-stub/index.d.ts @@ -0,0 +1,4 @@ +export declare function init(): never; +export declare function use(): void; +declare const _default: { init: typeof init; use: typeof use }; +export default _default; diff --git a/frontend/ai.client/shims/echarts-stub/index.js b/frontend/ai.client/shims/echarts-stub/index.js new file mode 100644 index 00000000..647d87b8 --- /dev/null +++ b/frontend/ai.client/shims/echarts-stub/index.js @@ -0,0 +1,24 @@ +/** + * Build-time stand-in for `echarts`. See README.md for why this exists. + * + * Only the two entry points `pptx-preview` actually calls are provided. + */ + +/** Thrown when a deck really does contain a native OOXML chart. */ +function unsupported() { + throw new Error( + 'pptx-preview: native OOXML charts are not supported in this build', + ); +} + +export function init() { + return unsupported(); +} + +export function use() { + // No-op. Registration is meaningless without a charting runtime, and + // it may be called before the deck is known to contain a chart — + // throwing here would fail decks that have none. +} + +export default { init, use }; diff --git a/frontend/ai.client/shims/echarts-stub/package.json b/frontend/ai.client/shims/echarts-stub/package.json new file mode 100644 index 00000000..c713013b --- /dev/null +++ b/frontend/ai.client/shims/echarts-stub/package.json @@ -0,0 +1,17 @@ +{ + "name": "echarts-stub", + "version": "5.6.0", + "description": "Build-time stand-in for echarts, substituted into pptx-preview. Version tracks the range pptx-preview asks for (^5.5.1) so npm ls does not report the tree as invalid; it carries no echarts code. See README.md.", + "license": "MIT", + "private": true, + "type": "module", + "main": "index.js", + "module": "index.js", + "types": "index.d.ts", + "exports": { + ".": { + "types": "./index.d.ts", + "default": "./index.js" + } + } +} From a2d41106ca98e9a3b6a31162c753a557cf9bc5e2 Mon Sep 17 00:00:00 2001 From: Phil Merrell Date: Tue, 15 Sep 2026 15:47:26 -0600 Subject: [PATCH 04/11] feat(files): preview .pptx in the docked pane MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pane was already format-agnostic apart from the renderer, so this is a viewer component plus a widened gate: previewKindFor() replaces the docx-only predicate and drives both the viewer switch and the header's subtitle, which was hardcoded to "Word document". The MIME check now cross-references the extension rather than comparing against a single constant. The extension picks the viewer before any request is made, so a file named .pptx that the server records as a .docx has to fail in the service rather than reach a renderer that cannot read it. Two things differ from the docx viewer, both forced by the library: pptx-preview takes a fixed pixel size at init() and has no responsive mode, so the deck is rendered once at 960px and CSS-zoomed to fit. A rail drag re-fits without re-parsing. Same zoom-not-transform reasoning as the docx viewer. It also writes background, height and overflow as *inline* styles on its wrapper — a black backdrop and a nested scroller — so those three overrides carry !important. Without them the deck sits in a black letterbox inside a second scrollbar, which is what it did until this was caught in the browser. The zero-slide guard is the one piece of behaviour with no counterpart in the docx viewer. pptx-preview resolves *successfully* when it cannot make sense of a presentation's theme or layout parts, returning no slides — reproduced against PR873-Verification-Deck.pptx in the dev files store, whose theme is stripped to 1.8 KB. Unchecked that paints an empty pane with no error and no retry, which reads as the app being broken rather than the file being unreadable. .xlsx is deliberately still excluded, and file-preview.model.ts records why: the npm build of SheetJS is frozen at a 2022 release with unfixed advisories, and the only maintained grid renderer is built on ExcelJS, which throws outright on a workbook containing a native chart — which is what create_excel_spreadsheet's own documented example produces. Verified in the browser against all four real decks: fidelity, fit at two rail widths with no horizontal overflow, resize re-fit, the failure path, and dark mode. Co-Authored-By: Claude Opus 5 --- .../file-preview-panel.component.spec.ts | 4 +- .../file-preview-panel.component.ts | 51 +++- .../pptx-viewer.component.spec.ts | 145 ++++++++++ .../file-preview/pptx-viewer.component.ts | 247 ++++++++++++++++++ .../file-download-renderer.component.spec.ts | 17 +- .../file-preview-http.service.spec.ts | 42 ++- .../file-preview/file-preview-http.service.ts | 21 +- .../file-preview/file-preview.model.spec.ts | 50 ++++ .../file-preview/file-preview.model.ts | 46 +++- 9 files changed, 598 insertions(+), 25 deletions(-) create mode 100644 frontend/ai.client/src/app/session/components/message-list/components/file-preview/pptx-viewer.component.spec.ts create mode 100644 frontend/ai.client/src/app/session/components/message-list/components/file-preview/pptx-viewer.component.ts create mode 100644 frontend/ai.client/src/app/session/services/file-preview/file-preview.model.spec.ts diff --git a/frontend/ai.client/src/app/session/components/message-list/components/file-preview/file-preview-panel.component.spec.ts b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/file-preview-panel.component.spec.ts index 74a01cbc..8adcdb4f 100644 --- a/frontend/ai.client/src/app/session/components/message-list/components/file-preview/file-preview-panel.component.spec.ts +++ b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/file-preview-panel.component.spec.ts @@ -74,7 +74,7 @@ describe('FilePreviewPanelComponent', () => { const aside = fixture.nativeElement.querySelector('aside'); expect(aside).not.toBeNull(); expect(aside.getAttribute('aria-label')).toBe( - 'Document preview: plan.docx', + 'File preview: plan.docx', ); expect(fixture.nativeElement.textContent).toContain('plan.docx'); expect(fetchDocument).toHaveBeenCalledExactlyOnceWith('up1'); @@ -91,7 +91,7 @@ describe('FilePreviewPanelComponent', () => { it('clears the loading state once the document paints', async () => { await openPreview(); - expect(fixture.nativeElement.textContent).not.toContain('Loading document'); + expect(fixture.nativeElement.textContent).not.toContain('Loading preview'); }); it('shows a retry affordance for a retryable failure', async () => { diff --git a/frontend/ai.client/src/app/session/components/message-list/components/file-preview/file-preview-panel.component.ts b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/file-preview-panel.component.ts index 442b9d41..6d304025 100644 --- a/frontend/ai.client/src/app/session/components/message-list/components/file-preview/file-preview-panel.component.ts +++ b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/file-preview-panel.component.ts @@ -23,9 +23,16 @@ import { ConfigService } from '../../../../../services/config.service'; import { downloadUrlFor } from '../../../../../shared/utils/file-download-url'; import { TooltipDirective } from '../../../../../components/tooltip/tooltip.directive'; import { DocxViewerComponent } from './docx-viewer.component'; +import { PptxViewerComponent } from './pptx-viewer.component'; +import { + PREVIEW_KIND_LABELS, + PreviewKind, + previewKindFor, +} from '../../../../services/file-preview/file-preview.model'; /** - * Right-docked pane that previews one uploaded `.docx` in the browser. + * Right-docked pane that previews one uploaded Office file in the + * browser — `.docx` and `.pptx` today. * * Shares the rail with `ArtifactPanelComponent` through * `DockedPaneService` — same width, same resize affordance, same @@ -45,7 +52,7 @@ import { DocxViewerComponent } from './docx-viewer.component'; @Component({ selector: 'app-file-preview-panel', changeDetection: ChangeDetectionStrategy.OnPush, - imports: [NgIcon, TooltipDirective, DocxViewerComponent], + imports: [NgIcon, TooltipDirective, DocxViewerComponent, PptxViewerComponent], providers: [ provideIcons({ heroArrowDownTray, @@ -63,7 +70,7 @@ import { DocxViewerComponent } from './docx-viewer.component'; class="fixed inset-y-0 right-0 z-40 flex w-full flex-col border-l border-gray-200 bg-white dark:border-gray-700 dark:bg-gray-900" [style.maxWidth]="paneWidthCss()" [class.select-none]="dragging()" - [attr.aria-label]="'Document preview: ' + ref.filename" + [attr.aria-label]="'File preview: ' + ref.filename" >

- Word document + {{ kindLabel() }}

@@ -151,11 +158,22 @@ import { DocxViewerComponent } from './docx-viewer.component'; } } @else { - + @switch (kind()) { + @case ('pptx') { + + } + @default { + + } + } @if (!ready()) { } @@ -191,6 +209,17 @@ export class FilePreviewPanelComponent { protected readonly open = this.previewState.openFile; protected readonly bytes = signal(null); + /** Which viewer renders the current file. Derived from the filename so + * the header reads correctly while the fetch is still in flight, then + * confirmed against the server's MIME type in + * `FilePreviewHttpService.fetchDocument` before any bytes are shown. */ + protected readonly kind = computed(() => { + const ref = this.open(); + return (ref && previewKindFor(ref.filename)) || 'docx'; + }); + protected readonly kindLabel = computed( + () => PREVIEW_KIND_LABELS[this.kind()], + ); protected readonly error = signal(null); protected readonly retryable = signal(false); /** Cleared only once the renderer reports a painted document, so the @@ -251,7 +280,7 @@ export class FilePreviewPanelComponent { const failure = e instanceof FilePreviewError ? e - : new FilePreviewError('Something went wrong loading this document.', true); + : new FilePreviewError('Something went wrong loading this file.', true); this.error.set(failure.message); this.retryable.set(failure.retryable); } diff --git a/frontend/ai.client/src/app/session/components/message-list/components/file-preview/pptx-viewer.component.spec.ts b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/pptx-viewer.component.spec.ts new file mode 100644 index 00000000..4f523fba --- /dev/null +++ b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/pptx-viewer.component.spec.ts @@ -0,0 +1,145 @@ +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { PptxViewerComponent } from './pptx-viewer.component'; + +/** Slides the fake previewer will paint on the next `preview()`. */ +let slidesToRender = 1; +/** Rejection for the next `preview()`, if the test wants one. */ +let previewRejection: Error | null = null; + +const init = vi.fn(); + +// Intercepts the component's dynamic `import('pptx-preview')`. The real +// library unzips the OOXML and lays out every shape, which is neither +// fast nor the thing under test: what matters here is that the component +// swaps the deck in only on success, discards a stale render, and treats +// a zero-slide result as a failure. +vi.mock('pptx-preview', () => ({ + init: (dom: HTMLElement, options: unknown) => { + init(dom, options); + return { + get slideCount() { + return slidesToRender; + }, + preview: async () => { + if (previewRejection) throw previewRejection; + for (let i = 0; i < slidesToRender; i++) { + const slide = document.createElement('div'); + slide.className = 'pptx-preview-slide-wrapper'; + dom.appendChild(slide); + } + }, + destroy: () => undefined, + }; + }, +})); + +describe('PptxViewerComponent', () => { + let fixture: ComponentFixture; + + beforeEach(async () => { + init.mockReset(); + slidesToRender = 1; + previewRejection = null; + + TestBed.resetTestingModule(); + await TestBed.configureTestingModule({ + imports: [PptxViewerComponent], + }).compileComponents(); + + fixture = TestBed.createComponent(PptxViewerComponent); + }); + + /** + * Set bytes and let the async render settle. + * + * The component caches its dynamic `import('pptx-preview')` in a + * static field, so whichever test runs first pays for resolving the + * module graph — more turns of the event loop than a single + * `whenStable()` covers. Draining a fixed number of macrotasks keeps + * that cost off the assertions without depending on which test the + * runner happens to schedule first. + */ + async function render(bytes: ArrayBuffer | null): Promise { + fixture.componentRef.setInput('bytes', bytes); + fixture.detectChanges(); + + for (let i = 0; i < 10; i++) { + await new Promise((resolve) => setTimeout(resolve, 0)); + await fixture.whenStable(); + fixture.detectChanges(); + } + } + + function slides(): NodeListOf { + return fixture.nativeElement.querySelectorAll('.pptx-preview-slide-wrapper'); + } + + it('paints the deck and reports it rendered', async () => { + slidesToRender = 3; + const rendered = vi.fn(); + fixture.componentInstance.rendered.subscribe(rendered); + + await render(new ArrayBuffer(16)); + + expect(slides().length).toBe(3); + expect(rendered).toHaveBeenCalledTimes(1); + }); + + it('renders at a fixed width and scales to fit rather than re-parsing', async () => { + await render(new ArrayBuffer(16)); + + // The library has no responsive mode — it lays the deck out against + // the size given at init. Re-initialising on a rail drag would + // re-parse the file, so the width must not come from the pane. + expect(init).toHaveBeenCalledTimes(1); + expect(init.mock.calls[0][1]).toMatchObject({ mode: 'list' }); + const { width, height } = init.mock.calls[0][1] as { + width: number; + height: number; + }; + expect(width).toBeGreaterThan(0); + // 16:9, the ratio the PowerPoint tool always emits. + expect(Math.round((width * 9) / 16)).toBe(height); + }); + + it('treats a deck that parsed into zero slides as a failure', async () => { + // `pptx-preview` resolves successfully when it cannot make sense of + // a presentation's theme or layout parts, returning no slides. + // Without this guard the pane paints empty, with no error and no + // retry, which reads as the app being broken rather than the file + // being unreadable. + slidesToRender = 0; + const failed = vi.fn(); + const rendered = vi.fn(); + fixture.componentInstance.renderFailed.subscribe(failed); + fixture.componentInstance.rendered.subscribe(rendered); + + await render(new ArrayBuffer(16)); + + expect(failed).toHaveBeenCalledTimes(1); + expect(failed.mock.calls[0][0]).toContain('PowerPoint'); + expect(rendered).not.toHaveBeenCalled(); + expect(slides().length).toBe(0); + }); + + it('reports a parse failure without painting a partial deck', async () => { + previewRejection = new Error('corrupt zip'); + const failed = vi.fn(); + fixture.componentInstance.renderFailed.subscribe(failed); + + await render(new ArrayBuffer(16)); + + expect(failed).toHaveBeenCalledTimes(1); + expect(slides().length).toBe(0); + }); + + it('clears the deck when the bytes go away', async () => { + await render(new ArrayBuffer(16)); + expect(slides().length).toBe(1); + + await render(null); + + expect(slides().length).toBe(0); + }); +}); diff --git a/frontend/ai.client/src/app/session/components/message-list/components/file-preview/pptx-viewer.component.ts b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/pptx-viewer.component.ts new file mode 100644 index 00000000..ae43c236 --- /dev/null +++ b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/pptx-viewer.component.ts @@ -0,0 +1,247 @@ +import { + ChangeDetectionStrategy, + Component, + ElementRef, + effect, + input, + output, + signal, + viewChild, +} from '@angular/core'; + +/** + * Width, in CSS px, the deck is rendered at before it is scaled to fit. + * + * `pptx-preview` takes a fixed pixel size at `init()` and lays the deck + * out against it — it has no responsive mode. Re-initialising on every + * frame of a rail drag would re-parse the whole file, so instead the + * deck is rendered once at this width and CSS-zoomed to fit, exactly as + * the docx viewer scales its pages. + * + * 960 is a deliberate over-render: comfortably wider than the rail gets, + * so fitting always scales *down* and text stays sharp rather than being + * enlarged from an undersized layout. + */ +const RENDER_WIDTH_PX = 960; + +/** 16:9, the aspect ratio `create_powerpoint_presentation` always emits. */ +const RENDER_HEIGHT_PX = Math.round((RENDER_WIDTH_PX * 9) / 16); + +/** + * Renders a `.pptx` in the browser, from its raw bytes. + * + * Uses `pptx-preview`, which reads the OOXML and reproduces each slide's + * own geometry — shape positions, fills, text frames, tables, images and + * the theme inherited from slide masters and layouts — as absolutely + * positioned DOM. That last part is what makes it worth a dependency: + * decks built on an uploaded corporate template get their branding from + * the master, and a renderer that only walked the slides would drop it. + * + * Nothing leaves the browser, for the same reason as the docx viewer — + * the alternative is a server-side render at Microsoft, which needs the + * file reachable by an unauthenticated URL. + * + * The library is loaded with a dynamic `import()`: it is dead weight in + * the initial bundle for the large majority of sessions that never open + * a deck, and it touches `document` at module scope, so a static import + * would run during SSR. + * + * Native OOXML charts are deliberately not supported — see + * `shims/echarts-stub/README.md` for why, and what a deck containing one + * does. + */ +@Component({ + selector: 'app-pptx-viewer', + changeDetection: ChangeDetectionStrategy.OnPush, + host: { '[style.--pptx-zoom]': 'zoom()' }, + template: ` +
+ +
+
+ `, + styles: ` + :host { + display: block; + height: 100%; + } + + /* The library builds its subtree imperatively, so it never receives + Angular's emulated-encapsulation attribute and cannot be reached + by an ordinary rule here. ::ng-deep under :host is the supported + escape hatch and keeps the selector anchored to this component. + + The !important declarations below are load-bearing, not + defensive: the library writes those three properties as *inline* + styles on the wrapper — a black backdrop, a fixed pixel height + sized for its own internal scroller, and an overflow that drives + it — and an inline style beats any selector we can write here. + Without them the deck sits in a black letterbox inside a second, + nested scrollbar. */ + :host ::ng-deep .pptx-preview-wrapper { + /* Transparent so the scroller's themed gutter shows through, + rather than the library's black. */ + background: transparent !important; + display: flex; + flex-direction: column; + align-items: center; + gap: 1.5rem; + /* CSS zoom rather than transform: scale() — zoom participates in + layout, so the flow collapses to the scaled height on its own. A + transform would paint smaller while still reserving the + unscaled height, leaving dead space under every slide. Same + reasoning as the docx viewer. */ + zoom: var(--pptx-zoom, 1); + /* The pane is the scroller, so let the content decide the height + and scroll the pane rather than a box inside it. */ + height: auto !important; + overflow: visible !important; + } + + /* Each slide. Kept on a white card in both themes: this previews + something the user will present or export, and recolouring a slide + would misrepresent the deck. The gutter carries the theme. */ + :host ::ng-deep .pptx-preview-slide-wrapper { + background: var(--color-white); + box-shadow: + 0 1px 3px rgb(0 0 0 / 0.12), + 0 1px 2px rgb(0 0 0 / 0.08); + border-radius: 0.125rem; + flex: none; + max-width: 100%; + } + `, +}) +export class PptxViewerComponent { + /** Raw `.pptx` bytes. Null clears the view (pane closed / still loading). */ + readonly bytes = input(null); + + /** The deck painted. */ + readonly rendered = output(); + + /** Rendering failed — the bytes were not a readable `.pptx`. */ + readonly renderFailed = output(); + + private readonly deckHost = + viewChild.required>('deckHost'); + private readonly scroller = + viewChild.required>('scroller'); + + /** Scale applied to the deck so a full slide fits the pane width. + * Capped at 1 — see `applyFit`. */ + protected readonly zoom = signal(1); + + /** Bumped per render so a slow parse that resolves after the input + * changed (or the pane closed) cannot paint over the current deck. */ + private renderSeq = 0; + + /** Cached module so reopening the pane doesn't re-request the chunk. */ + private static libraryPromise: Promise< + typeof import('pptx-preview') + > | null = null; + + constructor() { + // Re-fit when the rail is resized. The deck's own layout is fixed at + // render time, so this only adjusts the zoom — no re-parse. Guarded + // for SSR, where there is no ResizeObserver and no layout to fit to. + effect((onCleanup) => { + if (typeof ResizeObserver === 'undefined') return; + const ro = new ResizeObserver(() => this.applyFit()); + ro.observe(this.scroller().nativeElement); + onCleanup(() => ro.disconnect()); + }); + + effect(() => { + const data = this.bytes(); + const host = this.deckHost().nativeElement; + + const seq = ++this.renderSeq; + + if (!data) { + host.replaceChildren(); + this.zoom.set(1); + return; + } + + void this.render(data, host, seq); + }); + } + + private async render( + data: ArrayBuffer, + host: HTMLElement, + seq: number, + ): Promise { + // Render into a detached container and swap it in only on success, + // so two overlapping renders cannot interleave their output into the + // live host — the sequence guard below cannot undo DOM the library + // wrote on its own. + const target = document.createElement('div'); + + try { + PptxViewerComponent.libraryPromise ??= import('pptx-preview'); + const { init } = await PptxViewerComponent.libraryPromise; + if (seq !== this.renderSeq) return; + + const previewer = init(target, { + width: RENDER_WIDTH_PX, + height: RENDER_HEIGHT_PX, + // Every slide stacked in one scrollable column, matching how the + // docx viewer presents pages. 'slide' would paginate with the + // library's own prev/next chrome, which duplicates the pane's + // scrollbar and reads as a second, competing set of controls. + mode: 'list', + }); + await previewer.preview(data); + + if (seq !== this.renderSeq) return; + + // A deck that parsed but produced no slides is a failure, not an + // empty document. `pptx-preview` resolves successfully when it + // cannot make sense of a presentation's theme or layout parts — + // observed on a deck with a stripped-down theme, which returned + // zero slides without throwing. Left unchecked that paints an + // empty pane with no error and no retry, which reads as the app + // being broken rather than the file being unreadable. + if (previewer.slideCount === 0) { + throw new Error('pptx-preview produced no slides'); + } + + host.replaceChildren(target); + this.applyFit(); + this.rendered.emit(); + } catch { + if (seq !== this.renderSeq) return; + host.replaceChildren(); + this.renderFailed.emit( + "This file couldn't be read as a PowerPoint presentation. It may be corrupted or saved in an older format.", + ); + } + } + + /** + * Scale the deck so one full slide fits the pane's content box. + * + * Never scales above 1: the deck is laid out at `RENDER_WIDTH_PX`, + * wider than the rail goes, so enlarging would only magnify a layout + * that is already the reference size. + */ + private applyFit(): void { + const el = this.scroller().nativeElement; + const styles = getComputedStyle(el); + const available = + el.clientWidth - + parseFloat(styles.paddingLeft || '0') - + parseFloat(styles.paddingRight || '0'); + + if (available <= 0) { + this.zoom.set(1); + return; + } + this.zoom.set(Math.min(1, available / RENDER_WIDTH_PX)); + } +} diff --git a/frontend/ai.client/src/app/session/components/message-list/components/inline-visual/renderers/file-download-renderer.component.spec.ts b/frontend/ai.client/src/app/session/components/message-list/components/inline-visual/renderers/file-download-renderer.component.spec.ts index fcb34fc3..c3b04b25 100644 --- a/frontend/ai.client/src/app/session/components/message-list/components/inline-visual/renderers/file-download-renderer.component.spec.ts +++ b/frontend/ai.client/src/app/session/components/message-list/components/inline-visual/renderers/file-download-renderer.component.spec.ts @@ -90,10 +90,21 @@ describe('FileDownloadRendererComponent', () => { }); }); + it('offers a preview for a .pptx as well as a .docx', () => { + expect(render({ filename: 'deck.pptx', upload_id: 'up2' })).not.toBeNull(); + expect(previewButton()).not.toBeNull(); + }); + it('offers no preview for formats the pane cannot render', () => { - // .xlsx and legacy binary .doc both still get their download link; - // only the button is withheld. - for (const filename of ['budget.xlsx', 'deck.pptx', 'old.doc', 'notes.txt']) { + // .xlsx has no renderer we are willing to ship, and the legacy + // binary .doc/.ppt formats are not OOXML at all. All of them still + // get their download link; only the button is withheld. + for (const filename of [ + 'budget.xlsx', + 'old.doc', + 'old.ppt', + 'notes.txt', + ]) { expect(render({ filename, upload_id: 'up1' })).not.toBeNull(); expect(previewButton()).toBeNull(); } diff --git a/frontend/ai.client/src/app/session/services/file-preview/file-preview-http.service.spec.ts b/frontend/ai.client/src/app/session/services/file-preview/file-preview-http.service.spec.ts index 7981fb47..69aaf795 100644 --- a/frontend/ai.client/src/app/session/services/file-preview/file-preview-http.service.spec.ts +++ b/frontend/ai.client/src/app/session/services/file-preview/file-preview-http.service.spec.ts @@ -12,7 +12,7 @@ import { FilePreviewError, FilePreviewHttpService, } from './file-preview-http.service'; -import { DOCX_MIME } from './file-preview.model'; +import { DOCX_MIME, PPTX_MIME } from './file-preview.model'; import { ConfigService } from '../../../services/config.service'; describe('FilePreviewHttpService', () => { @@ -69,6 +69,46 @@ describe('FilePreviewHttpService', () => { expect(doc.bytes).toBe(bytes); expect(doc.filename).toBe('plan.docx'); expect(doc.mimeType).toBe(DOCX_MIME); + expect(doc.kind).toBe('docx'); + }); + + it('resolves a .pptx to the pptx viewer', async () => { + fetchMock.mockResolvedValue({ + ok: true, + status: 200, + arrayBuffer: () => Promise.resolve(new ArrayBuffer(8)), + }); + + const pending = service.fetchDocument('up1'); + flushPreviewUrl({ mimeType: PPTX_MIME, filename: 'deck.pptx' }); + const doc = await pending; + + expect(doc.kind).toBe('pptx'); + expect(doc.mimeType).toBe(PPTX_MIME); + }); + + it('refuses a file whose MIME type contradicts its extension', async () => { + // The extension picked the viewer before any request was made, so a + // file named .pptx that the server knows to be a .docx has to fail + // here rather than reach a renderer that cannot read it. + const pending = service.fetchDocument('up1'); + flushPreviewUrl({ mimeType: DOCX_MIME, filename: 'deck.pptx' }); + + await expect(pending).rejects.toThrow(FilePreviewError); + await expect(pending).rejects.toMatchObject({ retryable: false }); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('refuses a format the pane has no renderer for', async () => { + const pending = service.fetchDocument('up1'); + flushPreviewUrl({ + mimeType: + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + filename: 'budget.xlsx', + }); + + await expect(pending).rejects.toThrow(FilePreviewError); + expect(fetchMock).not.toHaveBeenCalled(); }); it('fetches S3 without credentials', async () => { diff --git a/frontend/ai.client/src/app/session/services/file-preview/file-preview-http.service.ts b/frontend/ai.client/src/app/session/services/file-preview/file-preview-http.service.ts index a857bf00..ba95895e 100644 --- a/frontend/ai.client/src/app/session/services/file-preview/file-preview-http.service.ts +++ b/frontend/ai.client/src/app/session/services/file-preview/file-preview-http.service.ts @@ -2,7 +2,11 @@ import { Injectable, inject } from '@angular/core'; import { HttpClient } from '@angular/common/http'; import { firstValueFrom } from 'rxjs'; import { ConfigService } from '../../../services/config.service'; -import { DOCX_MIME } from './file-preview.model'; +import { + PREVIEW_KIND_MIMES, + PreviewKind, + previewKindFor, +} from './file-preview.model'; /** `GET /files/{uploadId}/preview-url` — camelCase aliases on the wire. */ interface PreviewUrlResponseDto { @@ -18,6 +22,8 @@ export interface PreviewDocument { bytes: ArrayBuffer; mimeType: string; filename: string; + /** Which viewer should render these bytes. */ + kind: PreviewKind; } /** The preview failed in a way the pane should explain, not swallow. */ @@ -66,16 +72,23 @@ export class FilePreviewHttpService { private readonly config = inject(ConfigService); /** - * Resolve `uploadId` to a Word document's bytes. + * Resolve `uploadId` to a previewable document's bytes. * * Rejects with a `FilePreviewError` on every failure path so the pane * has one thing to catch and a `retryable` flag to decide whether to * offer the button. + * + * The MIME type the server recorded must agree with the extension the + * card was rendered from. Checking both directions is the point: the + * extension chose the viewer before any request was made, so a file + * named `.pptx` that the server knows to be a `.docx` has to fail here + * rather than reach a renderer that cannot read it. */ async fetchDocument(uploadId: string): Promise { const meta = await this.requestPreviewUrl(uploadId); - if (meta.mimeType !== DOCX_MIME) { + const kind = previewKindFor(meta.filename); + if (kind === null || meta.mimeType !== PREVIEW_KIND_MIMES[kind]) { throw new FilePreviewError( `This file is a ${meta.mimeType || 'unknown type'}, which can't be previewed here.`, false, @@ -83,7 +96,7 @@ export class FilePreviewHttpService { } const bytes = await this.fetchBytes(meta.url); - return { bytes, mimeType: meta.mimeType, filename: meta.filename }; + return { bytes, mimeType: meta.mimeType, filename: meta.filename, kind }; } private async requestPreviewUrl( diff --git a/frontend/ai.client/src/app/session/services/file-preview/file-preview.model.spec.ts b/frontend/ai.client/src/app/session/services/file-preview/file-preview.model.spec.ts new file mode 100644 index 00000000..999bf627 --- /dev/null +++ b/frontend/ai.client/src/app/session/services/file-preview/file-preview.model.spec.ts @@ -0,0 +1,50 @@ +import { describe, it, expect } from 'vitest'; +import { + isPreviewableFilename, + PREVIEW_KIND_LABELS, + previewKindFor, +} from './file-preview.model'; + +describe('previewKindFor', () => { + it('maps the OOXML formats the pane can render', () => { + expect(previewKindFor('plan.docx')).toBe('docx'); + expect(previewKindFor('deck.pptx')).toBe('pptx'); + }); + + it('ignores case and surrounding whitespace', () => { + expect(previewKindFor(' REPORT.DOCX ')).toBe('docx'); + expect(previewKindFor('Quarterly Review.PPTX')).toBe('pptx'); + }); + + it('declines the pre-2007 binary formats', () => { + // Not OOXML at all — the renderers cannot read them, so offering a + // preview would only produce an error the user cannot act on. + expect(previewKindFor('old.doc')).toBeNull(); + expect(previewKindFor('old.ppt')).toBeNull(); + }); + + it('declines .xlsx', () => { + // Deliberate: there is no spreadsheet renderer we are willing to + // ship. See the note on previewKindFor. + expect(previewKindFor('budget.xlsx')).toBeNull(); + }); + + it('declines an extension that merely contains a known one', () => { + expect(previewKindFor('plan.docx.pdf')).toBeNull(); + expect(previewKindFor('notdocx')).toBeNull(); + }); + + it('agrees with isPreviewableFilename', () => { + for (const name of ['a.docx', 'b.pptx', 'c.xlsx', 'd.txt', 'e.doc']) { + expect(isPreviewableFilename(name)).toBe(previewKindFor(name) !== null); + } + }); + + it('labels every kind it can return', () => { + for (const name of ['a.docx', 'b.pptx']) { + const kind = previewKindFor(name); + expect(kind).not.toBeNull(); + expect(PREVIEW_KIND_LABELS[kind!]).toBeTruthy(); + } + }); +}); diff --git a/frontend/ai.client/src/app/session/services/file-preview/file-preview.model.ts b/frontend/ai.client/src/app/session/services/file-preview/file-preview.model.ts index 0e5ca0ed..02f80005 100644 --- a/frontend/ai.client/src/app/session/services/file-preview/file-preview.model.ts +++ b/frontend/ai.client/src/app/session/services/file-preview/file-preview.model.ts @@ -15,7 +15,30 @@ export const DOCX_MIME = 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'; /** - * Whether a filename is one the preview pane can render. + * MIME type of a PowerPoint presentation (OOXML). Matches + * `apis.shared.files.ALLOWED_MIME_TYPES` and the `_PPTX_MIME` constant in + * `agents/builtin_tools/powerpoint_presentation_tool.py`. + */ +export const PPTX_MIME = + 'application/vnd.openxmlformats-officedocument.presentationml.presentation'; + +/** What the pane knows how to render, and which viewer does it. */ +export type PreviewKind = 'docx' | 'pptx'; + +/** Human label for the pane header's subtitle. */ +export const PREVIEW_KIND_LABELS: Readonly> = { + docx: 'Word document', + pptx: 'PowerPoint presentation', +}; + +/** The MIME type each viewer requires, checked against `/preview-url`. */ +export const PREVIEW_KIND_MIMES: Readonly> = { + docx: DOCX_MIME, + pptx: PPTX_MIME, +}; + +/** + * Which viewer a filename maps to, or null if the pane can't render it. * * Extension-based rather than MIME-based on purpose: the inline download * card is rendered from a persisted tool payload that carries only @@ -26,9 +49,24 @@ export const DOCX_MIME = * `/preview-url` reports, so a mislabelled `.docx` fails there rather * than feeding garbage to the renderer. * - * Legacy `.doc` is deliberately excluded: it is the pre-2007 binary - * format, which the OOXML renderer cannot read at all. + * Legacy `.doc` and `.ppt` are deliberately excluded: they are the + * pre-2007 binary formats, which the OOXML renderers cannot read at all. + * + * `.xlsx` is deliberately absent. There is no renderer for it we are + * willing to ship: the npm build of SheetJS is frozen at a 2022 release + * carrying unfixed advisories, and the only maintained grid renderer is + * built on ExcelJS, which throws outright on the workbooks + * `create_excel_spreadsheet` produces whenever one contains a native + * chart. Download-and-open remains the path for spreadsheets. */ +export function previewKindFor(filename: string): PreviewKind | null { + const name = filename.trim(); + if (/\.docx$/i.test(name)) return 'docx'; + if (/\.pptx$/i.test(name)) return 'pptx'; + return null; +} + +/** Whether a filename is one the preview pane can render. */ export function isPreviewableFilename(filename: string): boolean { - return /\.docx$/i.test(filename.trim()); + return previewKindFor(filename) !== null; } From 9ab491e287bd147c859b803e9922129ba9ea8f91 Mon Sep 17 00:00:00 2001 From: Phil Merrell Date: Tue, 15 Sep 2026 16:14:56 -0600 Subject: [PATCH 05/11] feat(files): preview an uploaded .docx or .pptx, not just a generated one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docked pane's only entry point was the Preview button on the generated-file download card, so a deck the platform created previewed and an identical deck the user uploaded did not. The uploaded one fell through to the presigned-URL branch, which hands the browser an OOXML file it cannot render — "open" silently became "download", and the agent's own fallback was to read the file and describe it in prose. Attachment cards now route the formats the pane can render to the pane, keyed off `isPreviewableFilename` — the same gate the download card uses, so the two surfaces cannot disagree about what is previewable. Markdown keeps its modal and everything else still opens in a tab. The button's accessible name follows the action rather than stating "Open" for something that previews. Verified against a real uploaded deck in dev: 10 slides, branding intact. Co-Authored-By: Claude Opus 5 --- .../file-attachment-badge.component.spec.ts | 98 ++++++++++++++++++- .../file-attachment-badge.component.ts | 29 +++++- 2 files changed, 123 insertions(+), 4 deletions(-) diff --git a/frontend/ai.client/src/app/session/components/message-list/components/file-attachment/file-attachment-badge.component.spec.ts b/frontend/ai.client/src/app/session/components/message-list/components/file-attachment/file-attachment-badge.component.spec.ts index ecc01429..055eb904 100644 --- a/frontend/ai.client/src/app/session/components/message-list/components/file-attachment/file-attachment-badge.component.spec.ts +++ b/frontend/ai.client/src/app/session/components/message-list/components/file-attachment/file-attachment-badge.component.spec.ts @@ -1,9 +1,15 @@ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, vi } from 'vitest'; +import { TestBed } from '@angular/core/testing'; import { FILE_TYPE_STYLES, DEFAULT_STYLE, + FileAttachmentBadgeComponent, } from './file-attachment-badge.component'; -import { ALLOWED_MIME_TYPES } from '../../../../../services/file-upload'; +import { + ALLOWED_MIME_TYPES, + FileUploadService, +} from '../../../../../services/file-upload'; +import { FilePreviewStateService } from '../../../../services/file-preview/file-preview-state.service'; /** * Every uploadable type needs its own card style. @@ -39,3 +45,91 @@ describe('file attachment card styles', () => { expect(unstyled).toEqual([]); }); }); + +/** + * Where a click on an attachment card goes. + * + * An uploaded `.docx`/`.pptx` used to fall through to the presigned-URL + * branch, which hands the browser an OOXML file it cannot render — so + * "open" silently became "download". The docked pane already renders both, + * and it only ever appeared on the *generated*-file download card, so an + * uploaded deck and a generated one behaved differently for no reason the + * user could see. + */ +describe('FileAttachmentBadgeComponent click routing', () => { + const PPTX_MIME = + 'application/vnd.openxmlformats-officedocument.presentationml.presentation'; + const PDF_MIME = 'application/pdf'; + + async function mount(filename: string, mimeType: string) { + const opened: unknown[] = []; + const windowOpen = vi.fn(); + vi.stubGlobal('open', windowOpen); + + const getPreviewUrl = vi + .fn() + .mockResolvedValue({ url: 'https://s3.example/x?X-Amz-Signature=abc' }); + + TestBed.resetTestingModule(); + await TestBed.configureTestingModule({ + imports: [FileAttachmentBadgeComponent], + providers: [ + { + provide: FilePreviewStateService, + useValue: { open: (ref: unknown) => opened.push(ref) }, + }, + { + provide: FileUploadService, + useValue: { + getPreviewUrl, + getTextSnippet: vi.fn().mockResolvedValue({ snippet: '' }), + getThumbnail: vi.fn().mockResolvedValue({ status: 'error' }), + }, + }, + ], + }).compileComponents(); + + const fixture = TestBed.createComponent(FileAttachmentBadgeComponent); + fixture.componentRef.setInput('attachment', { + uploadId: 'up1', + filename, + mimeType, + sizeBytes: 1024, + }); + fixture.detectChanges(); + await fixture.whenStable(); + return { fixture, opened, windowOpen, getPreviewUrl }; + } + + it('opens an uploaded .pptx in the docked pane, not a new tab', async () => { + const { fixture, opened, windowOpen } = await mount('deck.pptx', PPTX_MIME); + + fixture.nativeElement.querySelector('button').click(); + await fixture.whenStable(); + + expect(opened).toEqual([{ uploadId: 'up1', filename: 'deck.pptx' }]); + expect(windowOpen).not.toHaveBeenCalled(); + }); + + it('still opens a format the pane cannot render in a new tab', async () => { + const { fixture, opened, getPreviewUrl } = await mount('paper.pdf', PDF_MIME); + + fixture.nativeElement.querySelector('button').click(); + await fixture.whenStable(); + + expect(opened).toEqual([]); + expect(getPreviewUrl).toHaveBeenCalledWith('up1'); + }); + + it('names the action Preview only when it previews', async () => { + const deck = await mount('deck.pptx', PPTX_MIME); + expect( + deck.fixture.nativeElement.querySelector('button').getAttribute('aria-label'), + ).toBe('Preview deck.pptx'); + + const pdf = await mount('paper.pdf', PDF_MIME); + expect( + pdf.fixture.nativeElement.querySelector('button').getAttribute('aria-label'), + ).toBe('Open paper.pdf'); + }); +}); diff --git a/frontend/ai.client/src/app/session/components/message-list/components/file-attachment/file-attachment-badge.component.ts b/frontend/ai.client/src/app/session/components/message-list/components/file-attachment/file-attachment-badge.component.ts index f95ae6b6..94a5c7db 100644 --- a/frontend/ai.client/src/app/session/components/message-list/components/file-attachment/file-attachment-badge.component.ts +++ b/frontend/ai.client/src/app/session/components/message-list/components/file-attachment/file-attachment-badge.component.ts @@ -12,6 +12,8 @@ import { import { MarkdownComponent } from 'ngx-markdown'; import { formatBytes, FileUploadService } from '../../../../../services/file-upload'; import { FileAttachmentData } from '../../../../services/models/message.model'; +import { FilePreviewStateService } from '../../../../services/file-preview/file-preview-state.service'; +import { isPreviewableFilename } from '../../../../services/file-preview/file-preview.model'; import { MarkdownPreviewModalComponent } from './markdown-preview-modal.component'; interface FileTypeStyle { @@ -137,7 +139,9 @@ const SLIDE_BULLET_WIDTHS = [78, 92, 60]; * excerpt (for txt/md/csv/html) or skeleton lines (for binary docs), a * folded top-right corner detail, and a footer with filename + size. * - * Clicking opens the file in a new tab via a short-lived presigned URL. + * Clicking previews the file where we can render one — Markdown in a modal, + * `.docx` / `.pptx` in the docked pane — and otherwise opens it in a new tab + * via a short-lived presigned URL. */ @Component({ selector: 'app-file-attachment-badge', @@ -235,7 +239,7 @@ const SLIDE_BULLET_WIDTHS = [78, 92, 60]; type="button" (click)="openFile()" class="group flex w-60 shrink-0 flex-col overflow-hidden rounded-xl border border-gray-200 bg-white text-left shadow-sm transition-all hover:-translate-y-0.5 hover:shadow-md focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary-500 dark:border-gray-700 dark:bg-gray-800" - [attr.aria-label]="'Open ' + attachment().filename" + [attr.aria-label]="actionLabel() + ' ' + attachment().filename" >
(); private readonly fileUploadService = inject(FileUploadService); + private readonly filePreview = inject(FilePreviewStateService); protected readonly skeletonWidths = SKELETON_LINE_WIDTHS; protected readonly slideBulletWidths = SLIDE_BULLET_WIDTHS; @@ -402,6 +407,18 @@ export class FileAttachmentBadgeComponent { () => this.attachment().mimeType === PRESENTATION_MIME, ); + /** Whether clicking opens the docked preview pane rather than a new tab. + * Keyed off the filename, the same gate the generated-file download card + * uses, so the two surfaces can never disagree about what is previewable. */ + protected readonly isPanePreviewable = computed(() => + isPreviewableFilename(this.attachment().filename), + ); + + /** What the click will do, for the button's accessible name. */ + protected readonly actionLabel = computed(() => + this.isPanePreviewable() || this.isMarkdown() ? 'Preview' : 'Open', + ); + /** Cap chars so very long unbroken lines don't blow out the card. */ protected readonly truncatedSnippet = computed(() => { const raw = this.snippet(); @@ -462,6 +479,14 @@ export class FileAttachmentBadgeComponent { this.markdownModalOpen.set(true); return; } + // An uploaded .docx/.pptx gets the same docked pane as a generated one. + // Falling through to the presigned URL would just hand the browser an + // OOXML file it cannot render, which downloads it instead of showing it. + if (this.isPanePreviewable()) { + const att = this.attachment(); + this.filePreview.open({ uploadId: att.uploadId, filename: att.filename }); + return; + } try { const response = await this.fileUploadService.getPreviewUrl(this.attachment().uploadId); window.open(response.url, '_blank', 'noopener,noreferrer'); From 2036cc1d89ca09e9d208c4fe4de679f83f29e1d6 Mon Sep 17 00:00:00 2001 From: Phil Merrell Date: Tue, 15 Sep 2026 16:31:17 -0600 Subject: [PATCH 06/11] fix(prompts): point "show me this file" at the viewer, and stop escaping $ into files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two clauses, both prompted by watching a real session. Asked to preview an uploaded deck, the agent read it, and when pushed for a *visual* preview it called create_powerpoint_presentation and rebuilt the deck from scratch — a lossy copy of a file already sitting in the session, purely to obtain a Preview button, which only generated files had. It was not being obtuse: the docked pane has exactly two callers, both user clicks, and no SSE event can open it, so the model had no way to show anything. Its only verbs were read and create. The pane now also opens on uploaded files, so the prompt just has to say the viewer exists and belongs to the user. It draws the line at "show me" (point at the button) versus "tell me about" (read the file), because reading is right for summarize/check/edit and wrong for looking — a text dump is not what was asked for, and it puts the whole document in the cacheable history where it is paid for on every later turn. That session had spent 9,053 characters of tool results on it. The second clause fixes a systematic defect this work surfaced. The KaTeX guidance said "other uses of $ should be use the HTML entity $" with no scope, so the model applied it inside generated files: a deck came out holding the literal string "$100K", which is wrong in PowerPoint too, not just in our preview. I first wrote this off as noise because none of the eight real files in dev contained the entity — but none of them contain a dollar sign at all, so the corpus never had the chance to show it. Every file that did carry currency reproduced it. The rule is now scoped to chat markdown and explicitly excluded from files, code and tool arguments. Verified against the running stack with inference-api pointed at this branch: "Show me the preview-pane-test deck" now answers "Click the Preview button" with zero tool calls, and a deck asked for with dollar amounts comes out holding $100K, not $100K. Caveat worth knowing: this steers new conversations. In the session that had already read-then-created four times, Haiku followed its own precedent and rebuilt the deck anyway. Base prompt grows 6,244 -> 7,695 chars (~362 tokens), so it costs a one-time cache re-write per session on the next turn after deploy. Co-Authored-By: Claude Opus 5 --- .../main_agent/core/system_prompt_builder.py | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/backend/src/agents/main_agent/core/system_prompt_builder.py b/backend/src/agents/main_agent/core/system_prompt_builder.py index 035b28a8..78218f75 100644 --- a/backend/src/agents/main_agent/core/system_prompt_builder.py +++ b/backend/src/agents/main_agent/core/system_prompt_builder.py @@ -81,7 +81,12 @@ - Respond using markdown. - You can ONLY use tools that are explicitly provided to you in each conversation - When approriate, you may use KaTeX to render mathematical equations. -- Since the $ character is used to denote a variable in KaTeX, other uses of $ should be use the HTML entity $ +- KaTeX treats $ as a math delimiter, so in your own chat replies write other + uses of $ as the HTML entity $. This applies ONLY to the markdown you + send to the user. Never use the entity inside a file you generate, inside + code, or inside a tool argument -- a spreadsheet cell or slide holding + "$100K" is simply wrong, and it stays wrong when the user opens the file. + There, write a plain $. - When the user asks for a diagram or chart, you may use Mermaid to render it. - Available tools may change throughout the conversation based on user preferences - When multiple tools are available, select and use the most appropriate combination in the optimal order to fulfill the user's request @@ -89,6 +94,28 @@ - Always explain your reasoning when using tools - If you don't have the right tool for a task, clearly inform the user about the limitation +PREVIEWING FILES — THE USER ALREADY HAS A VIEWER: +Word documents (.docx) and PowerPoint decks (.pptx) in the conversation carry +a "Preview" button that opens them in a viewer beside the chat, laid out as +they really look. This is true whether the user uploaded the file or you +generated it, and you cannot open that viewer yourself -- it is theirs to +click. + +So when the user wants to LOOK at such a file ("preview this", "show me this +deck", "can I see it"), point them at the button. Do not read the file to +answer that. Reading it returns a flat text dump, which is not what they +asked for, and it pushes the whole document into the conversation where it is +paid for on every later turn. + +Never create or re-create a file to produce a preview. Regenerating a deck +the user already gave you yields a lossy copy of something they can already +see, at real cost. + +Still read the file whenever the request is about its CONTENT -- summarize it, +check it, answer questions from it, use it as a template, edit it. The +distinction is "show me" (point at the button) versus "tell me about" (read +it). Spreadsheets (.xlsx) have no viewer, so read those as before. + HANDLING MISSING TOOLS: Users can toggle individual tools on and off from Customize → Tools in the sidebar. When a user asks for something you would normally handle with a tool From 48b97bcb6f73951543f948351078b8002cbf2c49 Mon Sep 17 00:00:00 2001 From: Phil Merrell Date: Tue, 15 Sep 2026 16:43:31 -0600 Subject: [PATCH 07/11] fix(prompts): trim the preview clause to the part that prevents waste MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gap that mattered — previewing an UPLOADED deck — is fixed in the UI (9ab491e2); it needs no prompt at all. What the prompt still has to prevent is the model reading a file, or re-creating it, just to show it to someone. Cut the explanation of what the viewer is, the uploaded-vs-generated aside and the .xlsx exception, and kept the rule. 362 -> 169 tokens of cacheable prefix. Co-Authored-By: Claude Opus 5 --- .../main_agent/core/system_prompt_builder.py | 27 +++++-------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/backend/src/agents/main_agent/core/system_prompt_builder.py b/backend/src/agents/main_agent/core/system_prompt_builder.py index 78218f75..b784264f 100644 --- a/backend/src/agents/main_agent/core/system_prompt_builder.py +++ b/backend/src/agents/main_agent/core/system_prompt_builder.py @@ -94,27 +94,12 @@ - Always explain your reasoning when using tools - If you don't have the right tool for a task, clearly inform the user about the limitation -PREVIEWING FILES — THE USER ALREADY HAS A VIEWER: -Word documents (.docx) and PowerPoint decks (.pptx) in the conversation carry -a "Preview" button that opens them in a viewer beside the chat, laid out as -they really look. This is true whether the user uploaded the file or you -generated it, and you cannot open that viewer yourself -- it is theirs to -click. - -So when the user wants to LOOK at such a file ("preview this", "show me this -deck", "can I see it"), point them at the button. Do not read the file to -answer that. Reading it returns a flat text dump, which is not what they -asked for, and it pushes the whole document into the conversation where it is -paid for on every later turn. - -Never create or re-create a file to produce a preview. Regenerating a deck -the user already gave you yields a lossy copy of something they can already -see, at real cost. - -Still read the file whenever the request is about its CONTENT -- summarize it, -check it, answer questions from it, use it as a template, edit it. The -distinction is "show me" (point at the button) versus "tell me about" (read -it). Spreadsheets (.xlsx) have no viewer, so read those as before. +PREVIEWING FILES: +Every .docx and .pptx in the conversation has a "Preview" button the user +clicks to see it laid out; you cannot open it for them. When they ask to LOOK +at one ("show me this deck"), say to use that button -- never read the file +or re-create it just to show it. Reading is still right when the request is +about its CONTENT: summarize, check, answer from it, edit it. HANDLING MISSING TOOLS: Users can toggle individual tools on and off from Customize → Tools in the From 7a21fe8961db14693ed948d69a9afbd452e096fb Mon Sep 17 00:00:00 2001 From: Phil Merrell Date: Tue, 15 Sep 2026 16:43:31 -0600 Subject: [PATCH 08/11] fix(files): make the attachment card say it previews MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The card is the whole click target — there is no separate button — and the only hint that clicking did anything was an "open in new tab" glyph that faded in on hover. So at rest it advertised nothing, on a touch screen it advertised nothing ever, and the glyph it did show described the wrong action once .docx/.pptx started opening a side pane instead of a tab. Previewable types now carry a persistent eye + "PREVIEW" in the header strip, the same pairing the generated file's download card uses so the two read as one feature. Other types keep the quieter hover hint. Two contrast fixes found by measuring rather than eyeballing: the label started at gray-500, which is 4.55:1 on the pptx header tint and scrapes past AA by a hair, now gray-600 at 7.12:1; and the hover state used primary-accessible, which is BSU navy and lands at 1.41:1 in dark mode -- effectively invisible. Hover is now a neutral emphasis that measures 14.92:1 there. Co-Authored-By: Claude Opus 5 --- .../file-attachment-badge.component.spec.ts | 20 +++++++++++++ .../file-attachment-badge.component.ts | 29 +++++++++++++++---- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/frontend/ai.client/src/app/session/components/message-list/components/file-attachment/file-attachment-badge.component.spec.ts b/frontend/ai.client/src/app/session/components/message-list/components/file-attachment/file-attachment-badge.component.spec.ts index 055eb904..9a0c2086 100644 --- a/frontend/ai.client/src/app/session/components/message-list/components/file-attachment/file-attachment-badge.component.spec.ts +++ b/frontend/ai.client/src/app/session/components/message-list/components/file-attachment/file-attachment-badge.component.spec.ts @@ -121,6 +121,26 @@ describe('FileAttachmentBadgeComponent click routing', () => { expect(getPreviewUrl).toHaveBeenCalledWith('up1'); }); + it('advertises Preview on the card at rest, not only on hover', async () => { + // The card is the whole affordance — there is no separate button — so a + // hover-only hint tells a reader of the thread nothing, and a touch user + // nothing at all. + const { fixture } = await mount('deck.pptx', PPTX_MIME); + + const label = fixture.nativeElement.textContent; + expect(label).toContain('PREVIEW'); + + const badge = [...fixture.nativeElement.querySelectorAll('span')].find( + (el: HTMLElement) => el.textContent?.includes('PREVIEW'), + ) as HTMLElement; + expect(badge.className).not.toContain('opacity-0'); + }); + + it('does not advertise Preview for a format the pane cannot render', async () => { + const { fixture } = await mount('paper.pdf', PDF_MIME); + expect(fixture.nativeElement.textContent).not.toContain('PREVIEW'); + }); + it('names the action Preview only when it previews', async () => { const deck = await mount('deck.pptx', PPTX_MIME); expect( diff --git a/frontend/ai.client/src/app/session/components/message-list/components/file-attachment/file-attachment-badge.component.ts b/frontend/ai.client/src/app/session/components/message-list/components/file-attachment/file-attachment-badge.component.ts index 94a5c7db..d038da8a 100644 --- a/frontend/ai.client/src/app/session/components/message-list/components/file-attachment/file-attachment-badge.component.ts +++ b/frontend/ai.client/src/app/session/components/message-list/components/file-attachment/file-attachment-badge.component.ts @@ -8,6 +8,7 @@ import { heroPhoto, heroPresentationChartBar, heroArrowTopRightOnSquare, + heroEye, } from '@ng-icons/heroicons/outline'; import { MarkdownComponent } from 'ngx-markdown'; import { formatBytes, FileUploadService } from '../../../../../services/file-upload'; @@ -156,6 +157,7 @@ const SLIDE_BULLET_WIDTHS = [78, 92, 60]; heroPhoto, heroPresentationChartBar, heroArrowTopRightOnSquare, + heroEye, }), ], host: { class: 'contents' }, @@ -260,11 +262,28 @@ const SLIDE_BULLET_WIDTHS = [78, 92, 60]; {{ style().label }}
-