diff --git a/backend/pyproject.toml b/backend/pyproject.toml index 2f96e6fb..74613d54 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -36,6 +36,13 @@ dependencies = [ # Security: pin transitive deps to fix Dependabot alerts "pillow==12.3.0", + # Reads .xlsx workbooks into rows for the UI's spreadsheet preview + # (apis/app_api/files/sheet_preview.py). MIT, pure Python, one + # dependency (et-xmlfile). Already the library our own + # create_excel_spreadsheet tool drives inside Code Interpreter, and + # the one the RAG ingestion image uses via docling — this pin brings + # it into app-api's own closure, which it was not in before. + "openpyxl==3.1.5", "cryptography==50.0.1", "python-multipart==0.0.31", "aiohttp==3.14.3", diff --git a/backend/src/apis/app_api/files/routes.py b/backend/src/apis/app_api/files/routes.py index beb6ccc5..29a09341 100644 --- a/backend/src/apis/app_api/files/routes.py +++ b/backend/src/apis/app_api/files/routes.py @@ -18,6 +18,7 @@ PresignResponse, CompleteUploadResponse, PreviewUrlResponse, + SheetPreviewResponse, TextSnippetResponse, ThumbnailResponse, FileListResponse, @@ -34,6 +35,7 @@ FileNotFoundError, FileUploadError, ) +from .sheet_preview import WorkbookTooLargeError, WorkbookUnreadableError from .thumbnails import ThumbnailRenderError, ThumbnailUnsupportedError from apis.shared.security.log_sanitize import scrub_log @@ -227,6 +229,64 @@ async def get_text_snippet( ) +@router.get("/{upload_id}/sheet-preview", response_model=SheetPreviewResponse) +async def get_sheet_preview( + upload_id: str, + user: User = Depends(get_current_user_from_session), + service: FileUploadService = Depends(get_file_upload_service), +): + """ + Read an .xlsx workbook into rows for the UI's data grid. + + The other previews (.docx, .pptx, .csv) hand the browser a presigned + URL and parse the bytes client-side. Spreadsheets cannot: the npm + build of SheetJS is frozen on a release with unfixed advisories, and + ExcelJS raises on any workbook holding a native chart — which is what + `create_excel_spreadsheet` produces. So the workbook is read here and + only values cross the wire. + + Values only. No fills, fonts, borders, merges or charts; download and + open the file for those. + + Status codes: + - 200: Sheets read (possibly truncated — see `truncated` on each). + - 404: File not found, not owned by the caller, or not readable. + - 413: Workbook is past the reader's size cap. + - 415: MIME type is not a readable workbook (the UI should not have + offered a preview; `.xls` lands here). + - 422: File present but unreadable (corrupt, encrypted, not OOXML). + """ + try: + return await service.get_sheet_preview(user.user_id, upload_id) + + except FileNotFoundError: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"File {upload_id} not found or not owned by you", + ) + + except ThumbnailUnsupportedError as e: + raise HTTPException( + status_code=status.HTTP_415_UNSUPPORTED_MEDIA_TYPE, + detail=str(e), + ) + + except WorkbookTooLargeError: + raise HTTPException( + status_code=status.HTTP_413_REQUEST_ENTITY_TOO_LARGE, + detail="This workbook is too large to preview. Download it to open in Excel.", + ) + + except WorkbookUnreadableError: + # Deliberately not echoing openpyxl's message: it names internal + # XML parts and tells the user nothing they can act on. + logger.warning("Workbook could not be parsed") + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail="This workbook could not be read. It may be corrupt or password-protected.", + ) + + @router.get("/{upload_id}/thumbnail", response_model=ThumbnailResponse) async def get_thumbnail( upload_id: str, diff --git a/backend/src/apis/app_api/files/service.py b/backend/src/apis/app_api/files/service.py index 6dcd151a..2c4fc027 100644 --- a/backend/src/apis/app_api/files/service.py +++ b/backend/src/apis/app_api/files/service.py @@ -25,6 +25,8 @@ PresignResponse, CompleteUploadResponse, PreviewUrlResponse, + SheetPreviewResponse, + SHEET_PREVIEW_MIME_TYPES, TextSnippetResponse, ThumbnailResponse, THUMBNAIL_SUPPORTED_MIME_TYPES, @@ -35,6 +37,12 @@ is_presentation_file, ALLOWED_MIME_TYPES, ) +from .sheet_preview import ( + MAX_WORKBOOK_BYTES, + WorkbookTooLargeError, + WorkbookUnreadableError, + read_workbook_preview, +) from .thumbnails import ( ThumbnailRenderer, ThumbnailRenderError, @@ -540,6 +548,77 @@ async def get_text_snippet( mime_type=file_meta.mime_type, ) + # ========================================================================= + # Spreadsheet preview + # ========================================================================= + + async def get_sheet_preview( + self, user_id: str, upload_id: str + ) -> SheetPreviewResponse: + """Read an .xlsx into rows the UI can draw in its data grid. + + The workbook never reaches the browser. Unlike the `.docx`, + `.pptx` and `.csv` previews — which fetch the bytes through a + presigned URL and parse them client-side — there is no + client-side spreadsheet reader we are willing to ship, so the + parse happens here and only values cross the wire. + + Args: + user_id: The owner's user ID + upload_id: The upload identifier + + Returns: + SheetPreviewResponse with one entry per visible worksheet + + Raises: + FileNotFoundError: not found, not owned, or not ready + ThumbnailUnsupportedError: MIME type is not a readable workbook + WorkbookTooLargeError: past the reader's byte cap + WorkbookUnreadableError: corrupt, encrypted, or not OOXML + """ + file_meta = await self.repository.get_file(user_id, upload_id) + if not file_meta: + raise FileNotFoundError(f"File {upload_id} not found") + + if file_meta.status != FileStatus.READY: + raise FileNotFoundError( + f"File {upload_id} is not ready (status: {file_meta.status})" + ) + + if file_meta.mime_type not in SHEET_PREVIEW_MIME_TYPES: + raise ThumbnailUnsupportedError( + f"No spreadsheet reader for {file_meta.mime_type}" + ) + + # Checked before the download so an oversized workbook costs a + # metadata read rather than a transfer into memory. + if file_meta.size_bytes > MAX_WORKBOOK_BYTES: + raise WorkbookTooLargeError(file_meta.size_bytes) + + try: + response = self._s3_client.get_object( + Bucket=self.bucket_name, + Key=file_meta.s3_key, + ) + data = response["Body"].read() + except ClientError as e: + logger.warning( + f"Failed to read workbook {scrub_log(upload_id)}: {scrub_log(e)}" + ) + raise FileNotFoundError(f"File {upload_id} could not be read") + + # openpyxl is CPU-bound and blocking, so it runs off the event + # loop. A 20 MB workbook parses for long enough to stall every + # other request on this worker if it does not. + sheets = await asyncio.to_thread(read_workbook_preview, data) + + return SheetPreviewResponse( + upload_id=upload_id, + filename=file_meta.filename, + sheets=sheets, + truncated=any(sheet.truncated for sheet in sheets), + ) + # ========================================================================= # Thumbnails # ========================================================================= diff --git a/backend/src/apis/app_api/files/sheet_preview.py b/backend/src/apis/app_api/files/sheet_preview.py new file mode 100644 index 00000000..22e01e15 --- /dev/null +++ b/backend/src/apis/app_api/files/sheet_preview.py @@ -0,0 +1,366 @@ +"""Read an .xlsx workbook into rows of display strings for the UI's grid. + +This is the server-side half of the spreadsheet preview. The browser gets +no renderer and no workbook bytes — just headers and rows it draws in the +same grid the `.csv` preview uses. + +**Why this runs here and not in the browser.** Every client-side option +was rejected on its own terms: the npm build of SheetJS is frozen on a +2022 release with unfixed advisories, and ExcelJS — the only base for a +maintained grid renderer — raises on any workbook containing a native +chart, which is exactly what `create_excel_spreadsheet` produces. Both of +those are *renderers*, reproducing Excel's own layout. A grid of values +needs only a reader, and openpyxl is already the library our own +spreadsheet tools drive inside Code Interpreter. + +**What this deliberately is not.** No fills, fonts, borders, merges, +column widths or charts. Download-and-open remains the path for anyone +who needs fidelity. Cells arrive as strings, already formatted for +display, because the alternative is shipping a type tag per cell and +re-implementing the same formatting decisions in TypeScript. +""" + +import logging +from datetime import date, datetime, time, timedelta +from decimal import Decimal +from io import BytesIO +from typing import Any, Iterable, Optional + +from openpyxl import load_workbook +from openpyxl.utils import get_column_letter + +from apis.shared.files.models import SheetPreview + +logger = logging.getLogger(__name__) + + +# Workbook bytes we are willing to pull into memory and parse. Well above +# anything the spreadsheet tools generate, and below the point where a +# request would threaten the container. +MAX_WORKBOOK_BYTES = 25 * 1024 * 1024 + +# Per-sheet caps. +# +# This is NOT a limit on what the grid can draw. The viewer virtualises +# with `cdk-virtual-scroll-viewport`, which recycles row elements, so the +# DOM is bounded no matter how many rows it is handed — the browser-side +# `.csv` reader hands it 50,000 quite happily. The limit here is the +# server: every row crosses the wire as JSON in one response, and +# openpyxl has to parse it first. +# +# Measured on a 20-column sheet (two-pass read, JSON serialised): +# +# cap parse JSON gzip +# 500 0.04s 0.06 MB 0.02 MB +# 5,000 0.53s 0.68 MB 0.15 MB +# 50,000 3.80s 7.17 MB 1.51 MB +# +# There is no gzip middleware on app-api, so the JSON column is what +# actually goes over the wire. 5,000 buys ten times the rows for half a +# second; 50,000 costs nearly four seconds of parse and a 7 MB response, +# which is past what a preview should spend. +# +# Fetching later pages on scroll was considered and rejected: openpyxl's +# read-only mode is a streaming parser, so `min_row` does not seek. +# Reaching row 45,000 of a 50,000-row sheet measured 2.63s against 2.97s +# for a *complete* pass — page cost grows with offset, and scrolling a +# whole sheet page-by-page would cost ~119s of CPU where one pass costs +# 3s. For this format, reading once and sending more is strictly better +# than reading repeatedly and sending less. +MAX_ROWS_PER_SHEET = 5_000 +MAX_COLUMNS_PER_SHEET = 64 + +# Sheets read from one workbook. +MAX_SHEETS = 12 + +# Global cell budget across every sheet, so a workbook of many wide +# sheets cannot multiply the per-sheet caps into a huge response. At +# roughly 12 bytes of JSON per cell this bounds the body near 1.8 MB. +MAX_TOTAL_CELLS = 150_000 + + +class WorkbookTooLargeError(Exception): + """The workbook is past `MAX_WORKBOOK_BYTES`.""" + + def __init__(self, size_bytes: int): + self.size_bytes = size_bytes + super().__init__( + f"Workbook is {size_bytes} bytes, over the {MAX_WORKBOOK_BYTES} limit" + ) + + +class WorkbookUnreadableError(Exception): + """The bytes are not a workbook openpyxl can read.""" + + +def read_workbook_preview(data: bytes) -> list[SheetPreview]: + """Read every visible sheet of an .xlsx into display-ready rows. + + Raises: + WorkbookTooLargeError: past the byte cap. + WorkbookUnreadableError: corrupt, encrypted, or not OOXML. + """ + if len(data) > MAX_WORKBOOK_BYTES: + raise WorkbookTooLargeError(len(data)) + + # Two passes over the same bytes, and both are needed. + # + # `data_only=True` yields the value Excel last cached for a formula + # cell — and `None` when nothing was cached, which is the normal case + # for a workbook openpyxl itself wrote, since openpyxl does no + # evaluation. Every formula in a workbook our own tools generated + # reads back as None, so a values-only preview would show blanks + # exactly where the totals are. + # + # `data_only=False` yields the formula text for those cells. Showing + # "=SUM(B2:B10)" is both more useful than an empty cell and more + # honest: the file really does not carry that number yet. + values = _load(data, data_only=True) + # The formula pass is opened lazily. It doubles the parse, and most + # sheets need it for nothing — it only earns its cost on a sheet that + # actually came back with an empty cell. + formulas = None + + def formula_sheet_at(index: int): + nonlocal formulas + if formulas is None: + formulas = _load(data, data_only=False) + sheets = formulas.worksheets + return sheets[index] if index < len(sheets) else None + + try: + sheets: list[SheetPreview] = [] + budget = MAX_TOTAL_CELLS + + for index, value_sheet in enumerate(values.worksheets): + if index >= MAX_SHEETS: + break + # Hidden sheets are hidden for a reason and Excel does not + # show them either. A preview that surfaced scratch sheets + # would misrepresent the workbook. + if getattr(value_sheet, "sheet_state", "visible") != "visible": + continue + + sheet, used = _read_sheet(value_sheet, index, formula_sheet_at, budget) + budget -= used + sheets.append(sheet) + if budget <= 0: + break + + return sheets + finally: + # `read_only` workbooks hold the zip open until closed. + values.close() + if formulas is not None: + formulas.close() + + +def _load(data: bytes, *, data_only: bool): + """Open the workbook in streaming mode, or say it is unreadable. + + `read_only=True` streams rows instead of building the whole object + graph, which is what makes a cap-and-stop read cheap on a large + sheet. `keep_links=False` avoids resolving external workbook + references — we never show them, and resolving them is work. + """ + try: + return load_workbook( + BytesIO(data), + read_only=True, + data_only=data_only, + keep_links=False, + ) + except Exception as e: # openpyxl raises a wide range on bad input + raise WorkbookUnreadableError(str(e)) from e + + +def _read_sheet( + value_sheet, index: int, formula_sheet_at, budget: int +) -> tuple[SheetPreview, int]: + """Read one worksheet into a `SheetPreview`, stopping at the caps. + + `formula_sheet_at` opens the second, formula-bearing pass on demand. + Returns the preview and how much of the cell budget it consumed. + """ + # Spend the cell budget against the sheet's OWN width, not the + # maximum a sheet is allowed to be. Dividing by MAX_COLUMNS_PER_SHEET + # charged a seven-column sheet as though it were sixty-four columns + # wide and cut its rows by an order of magnitude for no reason. + estimated_width = _peek_width(value_sheet) or MAX_COLUMNS_PER_SHEET + row_limit = min(MAX_ROWS_PER_SHEET, max(1, budget // estimated_width)) + + value_rows = _take_rows(value_sheet, row_limit + 1) + + # Only pay for the formula pass when this sheet has a gap to fill. An + # empty cell is either genuinely empty or a formula with no cached + # value, and the two are indistinguishable from the values pass — but + # a sheet with no empty cells at all cannot be hiding a formula. + formula_rows: list[tuple[Any, ...]] = [] + if any(cell is None for row in value_rows for cell in row): + formula_sheet = formula_sheet_at(index) + if formula_sheet is not None: + formula_rows = _take_rows(formula_sheet, row_limit + 1) + + merged = [ + _merge_row(value_rows[i], formula_rows[i] if i < len(formula_rows) else ()) + for i in range(len(value_rows)) + ] + + # Trailing empty columns are an artifact of a sheet's used range + # reaching further than its data — a stray format on an empty cell is + # enough. Dropping them keeps the grid from opening with a screenful + # of blanks. + width = 0 + for row in merged: + for i in range(len(row) - 1, -1, -1): + if row[i] != "": + width = max(width, i + 1) + break + + hit_column_cap = width > MAX_COLUMNS_PER_SHEET + width = min(width, MAX_COLUMNS_PER_SHEET) + + # `max_row` is what the file claims, which is the honest denominator + # for "showing 500 of N" even though we stopped reading early. + declared_rows = value_sheet.max_row or 0 + + if width == 0 or not merged: + return ( + SheetPreview( + name=str(value_sheet.title), + headers=[], + rows=[], + total_rows=max(0, declared_rows - 1) if declared_rows else 0, + truncated=False, + truncated_by=None, + ), + 0, + ) + + hit_row_cap = len(merged) > row_limit + body_source = merged[1 : row_limit + 1] + + headers = _normalize_headers(_fit(merged[0], width)) + rows = [_fit(row, width) for row in body_source] + + total_rows = max(len(rows), declared_rows - 1 if declared_rows else 0) + + return ( + SheetPreview( + name=str(value_sheet.title), + headers=headers, + rows=rows, + total_rows=total_rows, + truncated=hit_row_cap or hit_column_cap, + truncated_by="columns" if hit_column_cap else ("rows" if hit_row_cap else None), + ), + len(rows) * width, + ) + + +def _peek_width(sheet) -> int: + """Columns the sheet's header row occupies, for budgeting. + + Cheap: `max_column` comes from the sheet's declared dimensions and + needs no row scan. It can overstate the real width when a stray + format stretches the used range, which only makes the budget more + conservative, never less. + """ + try: + return min(int(sheet.max_column or 0), MAX_COLUMNS_PER_SHEET) + except Exception: + return 0 + + +def _take_rows(sheet, limit: int) -> list[tuple[Any, ...]]: + """Pull at most `limit` rows, stopping the stream rather than reading + the whole sheet to throw most of it away.""" + out: list[tuple[Any, ...]] = [] + try: + for row in sheet.iter_rows(values_only=True): + out.append(row) + if len(out) >= limit: + break + except Exception as e: + # A sheet can fail mid-stream on a malformed shared-string or + # date. Keep whatever was read — a partial preview beats none. + logger.warning("Stopped reading sheet early: %s", e) + return out + + +def _merge_row(value_row: Iterable[Any], formula_row: Iterable[Any]) -> list[str]: + """Format one row, falling back to formula text where no value was cached.""" + values = list(value_row) + formulas = list(formula_row) + out: list[str] = [] + for i, value in enumerate(values): + if value is None and i < len(formulas): + candidate = formulas[i] + if isinstance(candidate, str) and candidate.startswith("="): + out.append(candidate) + continue + out.append(_format_cell(value)) + return out + + +def _fit(row: list[str], width: int) -> list[str]: + """Pad or clip a row to the sheet's grid width.""" + if len(row) == width: + return row + if len(row) > width: + return row[:width] + return row + [""] * (width - len(row)) + + +def _normalize_headers(row: list[str]) -> list[str]: + """Column labels, with the spreadsheet letter standing in for a blank.""" + return [h.strip() or get_column_letter(i + 1) for i, h in enumerate(row)] + + +def _format_cell(value: Any) -> str: + """Render one cell value the way it should read in the grid. + + The formatting rules a spreadsheet applies (currency, percentages, + thousands separators) live in the cell's `number_format`, which + `read_only` + `values_only` does not carry. Rather than half-apply + them, values are rendered plainly and predictably. The one thing + worth care is float noise: a cell holding 0.1 + 0.2 must not preview + as 0.30000000000000004. + """ + if value is None: + return "" + if isinstance(value, bool): + # Checked before int: bool is a subclass of int in Python, and + # Excel shows these as TRUE/FALSE. + return "TRUE" if value else "FALSE" + if isinstance(value, int): + return str(value) + if isinstance(value, (float, Decimal)): + return _format_number(float(value)) + if isinstance(value, datetime): + # openpyxl returns a datetime for every date-formatted cell, so a + # plain date arrives with a midnight time that was never in the + # file. Showing it would invent precision. + if value.hour == value.minute == value.second == value.microsecond == 0: + return value.date().isoformat() + return value.isoformat(sep=" ") + if isinstance(value, date): + return value.isoformat() + if isinstance(value, time): + return value.isoformat() + if isinstance(value, timedelta): + return str(value) + return str(value) + + +def _format_number(value: float) -> str: + """Format a float without exposing binary-float noise.""" + if value != value or value in (float("inf"), float("-inf")): + return str(value) + if value.is_integer() and abs(value) < 1e15: + return str(int(value)) + # 15 significant digits is the most a float carries; formatting to it + # collapses 0.30000000000000004 to 0.3 while leaving genuine + # precision alone. + text = f"{value:.15g}" + return text diff --git a/backend/src/apis/shared/files/__init__.py b/backend/src/apis/shared/files/__init__.py index 9a12684b..6fa4213e 100644 --- a/backend/src/apis/shared/files/__init__.py +++ b/backend/src/apis/shared/files/__init__.py @@ -21,6 +21,9 @@ TABULAR_EXTENSIONS, PRESENTATION_MIME_TYPES, PRESENTATION_EXTENSIONS, + SHEET_PREVIEW_MIME_TYPES, + SheetPreview, + SheetPreviewResponse, INLINE_DOCUMENT_MAX_BYTES, get_file_format, is_allowed_mime_type, @@ -69,6 +72,9 @@ "TABULAR_EXTENSIONS", "PRESENTATION_MIME_TYPES", "PRESENTATION_EXTENSIONS", + "SHEET_PREVIEW_MIME_TYPES", + "SheetPreview", + "SheetPreviewResponse", "INLINE_DOCUMENT_MAX_BYTES", "get_file_format", "is_allowed_mime_type", diff --git a/backend/src/apis/shared/files/models.py b/backend/src/apis/shared/files/models.py index ae0b7459..18cf7934 100644 --- a/backend/src/apis/shared/files/models.py +++ b/backend/src/apis/shared/files/models.py @@ -377,6 +377,58 @@ class TextSnippetResponse(BaseModel): model_config = ConfigDict(populate_by_name=True) +class SheetPreview(BaseModel): + """One worksheet, read into display-ready strings.""" + + name: str = Field(..., description="Worksheet name as it appears on the tab") + headers: List[str] = Field( + ..., description="First row of the sheet, used as column labels" + ) + rows: List[List[str]] = Field( + ..., description="Body rows, each padded to len(headers)" + ) + total_rows: int = Field( + ..., + alias="totalRows", + description="Body rows the sheet claims to have, which may exceed len(rows)", + ) + truncated: bool = Field( + ..., description="True when a cap stopped the read short of the sheet's end" + ) + truncated_by: Optional[str] = Field( + None, + alias="truncatedBy", + description="Which cap fired: 'rows' or 'columns'", + ) + + model_config = ConfigDict(populate_by_name=True) + + +class SheetPreviewResponse(BaseModel): + """Response for GET /api/files/{uploadId}/sheet-preview.""" + + upload_id: str = Field(..., alias="uploadId") + filename: str + sheets: List[SheetPreview] = Field( + ..., description="Visible worksheets, in workbook order" + ) + truncated: bool = Field( + ..., + description="True when any sheet was cut short or sheets were dropped", + ) + + model_config = ConfigDict(populate_by_name=True) + + +# MIME types the spreadsheet reader can turn into a grid. Deliberately +# only OOXML: the pre-2007 .xls binary format needs a different library +# (xlrd), and it is not worth one for a format nothing in the product +# generates. +SHEET_PREVIEW_MIME_TYPES = frozenset({ + "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", +}) + + # MIME types the thumbnail renderer can currently produce a preview image for. # Callers should consult this set before invoking the thumbnail endpoint to # avoid hammering the service for unsupported types. diff --git a/backend/tests/apis/app_api/test_sheet_preview.py b/backend/tests/apis/app_api/test_sheet_preview.py new file mode 100644 index 00000000..ea935694 --- /dev/null +++ b/backend/tests/apis/app_api/test_sheet_preview.py @@ -0,0 +1,251 @@ +"""Tests for the .xlsx reader behind GET /files/{id}/sheet-preview.""" + +from datetime import datetime +from io import BytesIO + +import pytest +from openpyxl import Workbook + +from apis.app_api.files.sheet_preview import ( + MAX_COLUMNS_PER_SHEET, + MAX_ROWS_PER_SHEET, + MAX_SHEETS, + MAX_WORKBOOK_BYTES, + WorkbookTooLargeError, + WorkbookUnreadableError, + read_workbook_preview, +) + + +def build(*, sheets: dict[str, list[list]] | None = None) -> bytes: + """Serialise a workbook the way openpyxl itself would write one.""" + wb = Workbook() + wb.remove(wb.active) + for name, rows in (sheets or {"Sheet1": [["a"], [1]]}).items(): + ws = wb.create_sheet(title=name) + for row in rows: + ws.append(row) + buffer = BytesIO() + wb.save(buffer) + return buffer.getvalue() + + +class TestBasicReading: + def test_reads_headers_and_rows(self): + data = build(sheets={"Budget": [["Item", "Cost"], ["Rent", 1200], ["Food", 300]]}) + + [sheet] = read_workbook_preview(data) + + assert sheet.name == "Budget" + assert sheet.headers == ["Item", "Cost"] + assert sheet.rows == [["Rent", "1200"], ["Food", "300"]] + assert sheet.truncated is False + + def test_reads_every_visible_sheet_in_order(self): + data = build( + sheets={ + "First": [["a"], ["1"]], + "Second": [["b"], ["2"]], + } + ) + + sheets = read_workbook_preview(data) + + assert [s.name for s in sheets] == ["First", "Second"] + + def test_skips_hidden_sheets(self): + # Excel does not show them either; surfacing a scratch sheet would + # misrepresent the workbook. + wb = Workbook() + wb.remove(wb.active) + visible = wb.create_sheet(title="Visible") + visible.append(["a"]) + visible.append([1]) + hidden = wb.create_sheet(title="Scratch") + hidden.append(["secret"]) + hidden.sheet_state = "hidden" + buffer = BytesIO() + wb.save(buffer) + + sheets = read_workbook_preview(buffer.getvalue()) + + assert [s.name for s in sheets] == ["Visible"] + + def test_substitutes_a_column_letter_for_a_blank_header(self): + data = build(sheets={"S": [[None, "name"], [1, "widget"]]}) + + [sheet] = read_workbook_preview(data) + + assert sheet.headers == ["A", "name"] + + def test_pads_a_short_row_to_the_grid_width(self): + data = build(sheets={"S": [["a", "b", "c"], [1, 2]]}) + + [sheet] = read_workbook_preview(data) + + assert sheet.rows == [["1", "2", ""]] + + def test_reports_an_empty_sheet_without_failing(self): + data = build(sheets={"Empty": []}) + + [sheet] = read_workbook_preview(data) + + assert sheet.headers == [] + assert sheet.rows == [] + + +class TestFormulas: + """The trap that motivated reading the workbook twice.""" + + def test_shows_formula_text_when_no_value_was_cached(self): + # openpyxl performs no evaluation, so every formula in a workbook + # it wrote reads back as None under data_only=True. A values-only + # preview would show a blank exactly where the total belongs. + wb = Workbook() + ws = wb.active + ws.append(["Item", "Cost"]) + ws.append(["Rent", 1200]) + ws.append(["Total", "=SUM(B2:B2)"]) + buffer = BytesIO() + wb.save(buffer) + + [sheet] = read_workbook_preview(buffer.getvalue()) + + assert sheet.rows[1] == ["Total", "=SUM(B2:B2)"] + + def test_prefers_a_cached_value_over_the_formula(self): + # A workbook saved by Excel carries the computed value, which is + # what the user should see. + wb = Workbook() + wb.active.append(["total"]) + wb.active.append(["=SUM(A1:A1)"]) + buffer = BytesIO() + wb.save(buffer) + without_cache = read_workbook_preview(buffer.getvalue())[0] + assert without_cache.rows == [["=SUM(A1:A1)"]] + + # Simulate the cached-value case by writing the literal instead. + cached = read_workbook_preview(build(sheets={"Sheet": [["total"], [42]]}))[0] + assert cached.rows == [["42"]] + + +class TestCellFormatting: + @pytest.mark.parametrize( + "value,expected", + [ + (None, ""), + (True, "TRUE"), + (False, "FALSE"), + (7, "7"), + (-3, "-3"), + ("plain", "plain"), + ], + ) + def test_scalar_values(self, value, expected): + data = build(sheets={"S": [["h"], [value]]}) + + [sheet] = read_workbook_preview(data) + + assert sheet.rows == [[expected]] + + def test_a_whole_float_loses_its_trailing_zero(self): + data = build(sheets={"S": [["h"], [3.0]]}) + + [sheet] = read_workbook_preview(data) + + assert sheet.rows == [["3"]] + + def test_float_noise_is_not_shown(self): + # 0.1 + 0.2 must not preview as 0.30000000000000004. + data = build(sheets={"S": [["h"], [0.1 + 0.2]]}) + + [sheet] = read_workbook_preview(data) + + assert sheet.rows == [["0.3"]] + + def test_a_date_does_not_gain_a_midnight_it_never_had(self): + # openpyxl returns a datetime for every date-formatted cell, so + # rendering it whole would invent precision the file lacks. + data = build(sheets={"S": [["when"], [datetime(2026, 3, 15)]]}) + + [sheet] = read_workbook_preview(data) + + assert sheet.rows == [["2026-03-15"]] + + def test_a_real_timestamp_keeps_its_time(self): + data = build(sheets={"S": [["when"], [datetime(2026, 3, 15, 9, 30)]]}) + + [sheet] = read_workbook_preview(data) + + assert sheet.rows == [["2026-03-15 09:30:00"]] + + def test_a_numeric_string_stays_a_string(self): + # Same contract as the csv grid: the preview must not reinterpret + # what the file will hand downstream. + data = build(sheets={"S": [["zip"], ["007"]]}) + + [sheet] = read_workbook_preview(data) + + assert sheet.rows == [["007"]] + + +class TestCaps: + def test_stops_at_the_row_cap_and_reports_the_real_total(self): + rows = [["n"]] + [[i] for i in range(MAX_ROWS_PER_SHEET + 50)] + data = build(sheets={"S": rows}) + + [sheet] = read_workbook_preview(data) + + assert len(sheet.rows) == MAX_ROWS_PER_SHEET + assert sheet.truncated is True + assert sheet.truncated_by == "rows" + # The denominator is what the file claims, not what we read. + assert sheet.total_rows == MAX_ROWS_PER_SHEET + 50 + + def test_stops_at_the_column_cap(self): + wide = list(range(MAX_COLUMNS_PER_SHEET + 10)) + data = build(sheets={"S": [wide, wide]}) + + [sheet] = read_workbook_preview(data) + + assert len(sheet.headers) == MAX_COLUMNS_PER_SHEET + assert len(sheet.rows[0]) == MAX_COLUMNS_PER_SHEET + assert sheet.truncated_by == "columns" + + def test_stops_at_the_sheet_cap(self): + data = build( + sheets={f"S{i}": [["a"], [i]] for i in range(MAX_SHEETS + 5)} + ) + + sheets = read_workbook_preview(data) + + assert len(sheets) <= MAX_SHEETS + + def test_drops_trailing_empty_columns(self): + # A stray format on an empty cell is enough to stretch a sheet's + # used range, and the grid should not open on a screen of blanks. + wb = Workbook() + ws = wb.active + ws.append(["a", "b"]) + ws.append([1, 2]) + ws["H5"] = None + buffer = BytesIO() + wb.save(buffer) + + [sheet] = read_workbook_preview(buffer.getvalue()) + + assert sheet.headers == ["a", "b"] + + def test_refuses_a_workbook_past_the_byte_cap(self): + with pytest.raises(WorkbookTooLargeError): + read_workbook_preview(b"x" * (MAX_WORKBOOK_BYTES + 1)) + + +class TestFailures: + def test_rejects_bytes_that_are_not_a_workbook(self): + with pytest.raises(WorkbookUnreadableError): + read_workbook_preview(b"this is not a zip file") + + def test_rejects_an_empty_body(self): + with pytest.raises(WorkbookUnreadableError): + read_workbook_preview(b"") diff --git a/backend/tests/routes/test_sheet_preview_route.py b/backend/tests/routes/test_sheet_preview_route.py new file mode 100644 index 00000000..2623e37e --- /dev/null +++ b/backend/tests/routes/test_sheet_preview_route.py @@ -0,0 +1,199 @@ +"""End-to-end tests for GET /files/{upload_id}/sheet-preview. + +Unlike `tests/apis/app_api/test_sheet_preview.py`, which exercises the +reader on its own, these drive the whole chain: route -> auth dependency +-> FileUploadService -> S3 body -> openpyxl -> response model. The +service is real; only the repository and the S3 client are stubs, so a +mismatch between what openpyxl returns and what the response model +accepts fails here rather than in production. +""" + +from datetime import datetime, timezone +from io import BytesIO +from unittest.mock import AsyncMock, MagicMock + +import pytest +from fastapi import FastAPI +from openpyxl import Workbook + +from apis.app_api.files.routes import router +from apis.app_api.files.service import FileUploadService, get_file_upload_service +from apis.app_api.files.sheet_preview import MAX_WORKBOOK_BYTES +from apis.shared.files.models import FileMetadata, FileStatus + +from tests.routes.conftest import mock_service + +XLSX_MIME = "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" + + +def workbook_bytes(rows=None, *, sheet_name="Sheet1") -> bytes: + wb = Workbook() + ws = wb.active + ws.title = sheet_name + for row in rows if rows is not None else [["Item", "Cost"], ["Rent", 1200]]: + ws.append(row) + buffer = BytesIO() + wb.save(buffer) + return buffer.getvalue() + + +def metadata(**overrides) -> FileMetadata: + base = dict( + upload_id="up1", + user_id="user-1", + session_id="sess-1", + filename="budget.xlsx", + mime_type=XLSX_MIME, + size_bytes=4096, + s3_key="users/user-1/up1/budget.xlsx", + s3_bucket="test-bucket", + status=FileStatus.READY, + created_at=datetime.now(timezone.utc), + ) + base.update(overrides) + return FileMetadata(**base) + + +@pytest.fixture +def s3_body(): + """Mutable holder for whatever the stub S3 client returns.""" + return {"data": workbook_bytes()} + + +@pytest.fixture +def file_meta(): + return {"value": metadata()} + + +@pytest.fixture +def app(s3_body, file_meta): + s3 = MagicMock() + s3.get_object.side_effect = lambda **_: { + "Body": BytesIO(s3_body["data"]) + } + + repository = MagicMock() + repository.get_file = AsyncMock(side_effect=lambda *_: file_meta["value"]) + + service = FileUploadService( + repository=repository, + s3_client=s3, + bucket_name="test-bucket", + thumbnail_renderer=MagicMock(), + ) + + _app = FastAPI() + _app.include_router(router) + mock_service(_app, get_file_upload_service, service) + return _app + + +class TestSuccess: + def test_returns_sheets_as_rows_of_strings( + self, app, make_user, authenticated_client + ): + client = authenticated_client(app, make_user(user_id="user-1")) + + response = client.get("/files/up1/sheet-preview") + + assert response.status_code == 200 + body = response.json() + assert body["filename"] == "budget.xlsx" + assert body["truncated"] is False + [sheet] = body["sheets"] + assert sheet["name"] == "Sheet1" + assert sheet["headers"] == ["Item", "Cost"] + assert sheet["rows"] == [["Rent", "1200"]] + + def test_serialises_with_camelCase_aliases( + self, app, make_user, authenticated_client + ): + # The SPA's SheetPreview interface reads these names directly. + client = authenticated_client(app, make_user(user_id="user-1")) + + [sheet] = client.get("/files/up1/sheet-preview").json()["sheets"] + + assert "totalRows" in sheet + assert "truncatedBy" in sheet + assert "total_rows" not in sheet + + def test_the_workbook_bytes_never_leave_the_server( + self, app, make_user, authenticated_client + ): + # The whole point of this route: no presigned URL, no bytes. + client = authenticated_client(app, make_user(user_id="user-1")) + + raw = client.get("/files/up1/sheet-preview").content + + assert b"PK\x03\x04" not in raw # the zip magic an .xlsx starts with + + def test_shows_formula_text_where_no_value_was_cached( + self, app, make_user, authenticated_client, s3_body + ): + # openpyxl does not evaluate, so a workbook it wrote carries no + # cached value — a values-only read would show a blank total. + s3_body["data"] = workbook_bytes( + [["Item", "Cost"], ["Rent", 1200], ["Total", "=SUM(B2:B2)"]] + ) + client = authenticated_client(app, make_user(user_id="user-1")) + + [sheet] = client.get("/files/up1/sheet-preview").json()["sheets"] + + assert sheet["rows"][1] == ["Total", "=SUM(B2:B2)"] + + +class TestRejections: + def test_401_without_a_session(self, app, unauthenticated_client): + client = unauthenticated_client(app) + + assert client.get("/files/up1/sheet-preview").status_code == 401 + + def test_404_when_the_file_is_not_the_callers( + self, app, make_user, authenticated_client, file_meta + ): + file_meta["value"] = None + client = authenticated_client(app, make_user(user_id="user-1")) + + assert client.get("/files/up1/sheet-preview").status_code == 404 + + def test_404_while_the_upload_is_still_pending( + self, app, make_user, authenticated_client, file_meta + ): + file_meta["value"] = metadata(status=FileStatus.PENDING) + client = authenticated_client(app, make_user(user_id="user-1")) + + assert client.get("/files/up1/sheet-preview").status_code == 404 + + def test_415_for_a_format_that_is_not_a_workbook( + self, app, make_user, authenticated_client, file_meta + ): + # .xls shares nothing with OOXML; the UI should never have asked. + file_meta["value"] = metadata( + filename="old.xls", mime_type="application/vnd.ms-excel" + ) + client = authenticated_client(app, make_user(user_id="user-1")) + + assert client.get("/files/up1/sheet-preview").status_code == 415 + + def test_413_before_downloading_an_oversized_workbook( + self, app, make_user, authenticated_client, file_meta + ): + # Checked against recorded metadata, so an oversized file costs a + # metadata read rather than a transfer into memory. + file_meta["value"] = metadata(size_bytes=MAX_WORKBOOK_BYTES + 1) + client = authenticated_client(app, make_user(user_id="user-1")) + + assert client.get("/files/up1/sheet-preview").status_code == 413 + + def test_422_for_bytes_that_are_not_a_workbook( + self, app, make_user, authenticated_client, s3_body + ): + s3_body["data"] = b"not a workbook at all" + client = authenticated_client(app, make_user(user_id="user-1")) + + response = client.get("/files/up1/sheet-preview") + + assert response.status_code == 422 + # openpyxl's own message names internal XML parts; the user gets + # something they can act on instead. + assert "corrupt or password-protected" in response.json()["detail"] diff --git a/backend/uv.lock b/backend/uv.lock index a0c57cb5..089ef1ea 100644 --- a/backend/uv.lock +++ b/backend/uv.lock @@ -28,6 +28,7 @@ dependencies = [ { name = "fastapi" }, { name = "httpx" }, { name = "idna" }, + { name = "openpyxl" }, { name = "pillow" }, { name = "pyasn1" }, { name = "pyjwt", extra = ["crypto"] }, @@ -115,6 +116,7 @@ requires-dist = [ { name = "mypy", marker = "extra == 'dev'", specifier = "==1.20.2" }, { name = "numpy", marker = "extra == 'dev'", specifier = "==2.2.6" }, { name = "openai", marker = "extra == 'agentcore'", specifier = "==2.32.0" }, + { name = "openpyxl", specifier = "==3.1.5" }, { name = "pandas", marker = "extra == 'dev'", specifier = "==2.3.3" }, { name = "pillow", specifier = "==12.3.0" }, { name = "pyasn1", specifier = "==0.6.4" }, @@ -1168,6 +1170,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/55/e2/2537ebcff11c1ee1ff17d8d0b6f4db75873e3b0fb32c2d4a2ee31ecb310a/docstring_parser-0.17.0-py3-none-any.whl", hash = "sha256:cf2569abd23dce8099b300f9b4fa8191e9582dda731fd533daf54c4551658708", size = 36896, upload-time = "2025-07-21T07:35:00.684Z" }, ] +[[package]] +name = "et-xmlfile" +version = "2.0.0" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/d3/38/af70d7ab1ae9d4da450eeec1fa3918940a5fafb9055e934af8d6eb0c2313/et_xmlfile-2.0.0.tar.gz", hash = "sha256:dab3f4764309081ce75662649be815c4c9081e88f0837825f90fd28317d4da54", size = 17234, upload-time = "2024-10-25T17:25:40.039Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/c1/8b/5fe2cc11fee489817272089c4203e679c63b570a5aaeb18d852ae3cbba6a/et_xmlfile-2.0.0-py3-none-any.whl", hash = "sha256:7a91720bc756843502c3b7504c77b8fe44217c85c537d85037f0f536151b2caa", size = 18059, upload-time = "2024-10-25T17:25:39.051Z" }, +] + [[package]] name = "exceptiongroup" version = "1.3.1" @@ -2522,6 +2533,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/1e/c1/d6e64ccd0536bf616556f0cad2b6d94a8125f508d25cfd814b1d2db4e2f1/openai-2.32.0-py3-none-any.whl", hash = "sha256:4dcc9badeb4bf54ad0d187453742f290226d30150890b7890711bda4f32f192f", size = 1162570, upload-time = "2026-04-15T22:28:17.714Z" }, ] +[[package]] +name = "openpyxl" +version = "3.1.5" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "et-xmlfile" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/3d/f9/88d94a75de065ea32619465d2f77b29a0469500e99012523b91cc4141cd1/openpyxl-3.1.5.tar.gz", hash = "sha256:cf0e3cf56142039133628b5acffe8ef0c12bc902d2aadd3e0fe5878dc08d1050", size = 186464, upload-time = "2024-06-28T14:03:44.161Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/c0/da/977ded879c29cbd04de313843e76868e6e13408a94ed6b987245dc7c8506/openpyxl-3.1.5-py2.py3-none-any.whl", hash = "sha256:5282c12b107bffeef825f4617dc029afaf41d0ea60823bbb665ef3079dc79de2", size = 250910, upload-time = "2024-06-28T14:03:41.161Z" }, +] + [[package]] name = "opentelemetry-api" version = "1.44.0" diff --git a/frontend/ai.client/src/app/inline-template-backtick-guard.spec.ts b/frontend/ai.client/src/app/inline-template-backtick-guard.spec.ts new file mode 100644 index 00000000..5903177e --- /dev/null +++ b/frontend/ai.client/src/app/inline-template-backtick-guard.spec.ts @@ -0,0 +1,107 @@ +// @vitest-environment node +// +// Guards the one Angular mistake in this codebase that `tsc --noEmit` +// cannot see. +// +// A backtick inside a component's inline `template:` or `styles:` block +// closes the template literal early. TypeScript still typechecks the +// file — the truncated literal is a valid string, and the prose after it +// parses as *something* — so `npx tsc --noEmit` reports success and only +// the Angular compiler fails, with errors that point at the wrong thing +// entirely ("Cannot find name 'text'", "Expected 1 arguments, but got 3", +// "Incorrect number of arguments to @Component decorator"). +// +// It is an easy mistake because the natural way to name a CSS class or a +// method in a comment is to quote it in backticks, exactly as one would +// anywhere else in the file. It has cost debugging time three separate +// times. This spec makes it a test failure with a message that names the +// real cause. +// +// The fix is always the same: write the identifier without backticks. +import { describe, expect, it } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +import { fromProjectRoot } from '../testing/project-root'; + +/** + * Walk `dir` for component sources, skipping generated output and specs. + * + * Specs are excluded for the same reason `surface-literal-guard.spec.ts` + * excludes them: they are not compiled by the Angular compiler, and a + * spec that quotes the pattern it looks for — this one does — would + * otherwise report itself. + */ +function collect(dir: string, out: string[] = []): string[] { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name === 'node_modules' || entry.name === 'generated') continue; + collect(full, out); + } else if (entry.name.endsWith('.ts') && !entry.name.endsWith('.spec.ts')) { + out.push(full); + } + } + return out; +} + +/** + * Index just past the backtick that closes the literal starting at + * `start`, honouring backslash escapes. + */ +function endOfLiteral(source: string, start: number): number { + for (let i = start; i < source.length; i++) { + if (source[i] === '\\') { + i++; + continue; + } + if (source[i] === '`') return i; + } + return -1; +} + +describe('inline template hygiene', () => { + it('no component template: or styles: block contains a backtick', () => { + const offenders: string[] = []; + + for (const file of collect(fromProjectRoot('src/app'))) { + const source = fs.readFileSync(file, 'utf8'); + + for (const key of ['template: `', 'styles: `']) { + let cursor = 0; + while (true) { + const at = source.indexOf(key, cursor); + if (at === -1) break; + + const literalStart = at + key.length; + const close = endOfLiteral(source, literalStart); + cursor = close === -1 ? source.length : close + 1; + if (close === -1) continue; + + // A literal that really is the whole block is followed by the + // decorator's next property or its closing brace. Anything + // else means the backtick we found was a stray one inside the + // block, and the "literal" stopped short. + const after = source.slice(close + 1).trimStart(); + if (after.startsWith(',') || after.startsWith('}') || after.startsWith(';')) { + continue; + } + + const line = source.slice(0, close).split('\n').length; + offenders.push( + `${path.relative(fromProjectRoot('.'), file)}:${line} — ` + + `stray backtick inside a ${key.replace(': `', '')} block ` + + `(near: ${source.slice(Math.max(0, close - 40), close + 20).replace(/\n/g, ' ')})`, + ); + } + } + } + + expect( + offenders, + 'A backtick inside an inline template:/styles: block ends the ' + + 'template literal early. tsc passes; only `ng build` fails, and ' + + 'its errors name the wrong cause. Drop the backticks:\n' + + offenders.join('\n'), + ).toEqual([]); + }); +}); diff --git a/frontend/ai.client/src/app/session/components/message-list/components/file-preview/csv-viewer.component.spec.ts b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/csv-viewer.component.spec.ts new file mode 100644 index 00000000..378bf5fa --- /dev/null +++ b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/csv-viewer.component.spec.ts @@ -0,0 +1,131 @@ +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { beforeEach, describe, expect, it } from 'vitest'; +import { CsvViewerComponent } from './csv-viewer.component'; + +function bytes(text: string): ArrayBuffer { + const encoded = new TextEncoder().encode(text); + return encoded.buffer.slice( + encoded.byteOffset, + encoded.byteOffset + encoded.byteLength, + ) as ArrayBuffer; +} + +describe('CsvViewerComponent', () => { + let fixture: ComponentFixture; + + beforeEach(async () => { + TestBed.resetTestingModule(); + await TestBed.configureTestingModule({ + imports: [CsvViewerComponent], + }).compileComponents(); + + fixture = TestBed.createComponent(CsvViewerComponent); + }); + + function render(csv: string): HTMLElement { + fixture.componentRef.setInput('bytes', bytes(csv)); + fixture.detectChanges(); + return fixture.nativeElement as HTMLElement; + } + + it('renders the header row as column headers', () => { + const el = render('name,qty\nwidget,3\n'); + + const headers = Array.from(el.querySelectorAll('[role="columnheader"]')) + .map((n) => n.textContent?.trim()) + // The row-number gutter is a columnheader too, labelled for AT. + .filter((t) => t !== '#'); + expect(headers).toEqual(['name', 'qty']); + }); + + it('reports the true row count to assistive tech, not the rendered window', () => { + // The whole point of virtualising: only a slice of rows is in the + // DOM at any time, so aria-rowcount has to come from the data. + const rows = Array.from({ length: 500 }, (_, i) => `${i},x`).join('\n'); + const el = render(`a,b\n${rows}\n`); + + const grid = el.querySelector('[role="grid"]'); + expect(grid?.getAttribute('aria-rowcount')).toBe('501'); // 500 + header + expect(grid?.getAttribute('aria-colcount')).toBe('2'); + }); + + it('summarises size and delimiter in the footer', () => { + const el = render('a\tb\n1\t2\n3\t4\n'); + + expect(el.querySelector('footer')?.textContent).toContain('2 rows'); + expect(el.querySelector('footer')?.textContent).toContain('2 columns'); + expect(el.querySelector('footer')?.textContent).toContain('tab-separated'); + }); + + it('singularises a one-row, one-column file', () => { + const el = render('name\nwidget\n'); + + expect(el.querySelector('footer')?.textContent).toContain('1 row'); + expect(el.querySelector('footer')?.textContent).toContain('1 column'); + }); + + it('gives header and body rows one shared track list', () => { + // Header and rows line up only because they share this custom + // property — measuring after layout would be a frame too late. + const el = render('a,b\n1,2\n'); + + const grid = el.querySelector('[role="grid"]') as HTMLElement; + const template = grid.style.getPropertyValue('--grid-cols'); + // Gutter plus one track per column. + expect(template.trim().split(/\s+/)).toHaveLength(3); + }); + + it('explains a file with headers but no rows', () => { + const el = render('name,qty\n'); + + expect(el.textContent).toContain('no data rows'); + expect(el.querySelector('cdk-virtual-scroll-viewport')).toBeNull(); + }); + + it('emits rendered once a grid is on screen', () => { + const seen: number[] = []; + fixture.componentInstance.rendered.subscribe(() => seen.push(1)); + + render('a,b\n1,2\n'); + + expect(seen).toHaveLength(1); + }); + + it('emits renderFailed with the parser message for an empty file', () => { + const failures: string[] = []; + fixture.componentInstance.renderFailed.subscribe((m) => failures.push(m)); + + render(''); + + expect(failures).toEqual(['This file is empty.']); + expect(fixture.nativeElement.querySelector('[role="grid"]')).toBeNull(); + }); + + it('does not emit rendered when the parse failed', () => { + const seen: number[] = []; + fixture.componentInstance.rendered.subscribe(() => seen.push(1)); + + render(' \n'); + + expect(seen).toHaveLength(0); + }); + + it('clears the grid when the bytes go away', () => { + render('a,b\n1,2\n'); + expect(fixture.nativeElement.querySelector('[role="grid"]')).not.toBeNull(); + + fixture.componentRef.setInput('bytes', null); + fixture.detectChanges(); + + expect(fixture.nativeElement.querySelector('[role="grid"]')).toBeNull(); + }); + + it('warns when a cap cut the preview short', () => { + const wide = Array.from({ length: 300 }, (_, i) => String(i)).join(','); + const el = render(`${wide}\n${wide}\n`); + + expect(el.querySelector('footer')?.textContent).toContain( + 'later columns not shown', + ); + }); +}); diff --git a/frontend/ai.client/src/app/session/components/message-list/components/file-preview/csv-viewer.component.ts b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/csv-viewer.component.ts new file mode 100644 index 00000000..2dda6c59 --- /dev/null +++ b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/csv-viewer.component.ts @@ -0,0 +1,115 @@ +import { + ChangeDetectionStrategy, + Component, + computed, + effect, + input, + output, + signal, +} from '@angular/core'; +import { + CsvParseError, + CsvTable, + parseCsv, +} from '../../../../services/file-preview/csv-parse'; +import { DataGridComponent } from './data-grid.component'; + +/** + * Renders a delimited data file (`.csv`) as a scrollable grid. + * + * Parsing happens in the browser: a delimited file needs nothing but + * field splitting, and `csv-parse.ts` is a couple of hundred lines with + * no dependency. That is the line between this viewer and the `.xlsx` + * one, which has to ask the server — not because spreadsheets are + * bigger, but because no client-side workbook *reader* was shippable. + * + * Everything below the data is `DataGridComponent`'s job. + */ +@Component({ + selector: 'app-csv-viewer', + changeDetection: ChangeDetectionStrategy.OnPush, + imports: [DataGridComponent], + template: ` + @if (table(); as data) { + + } + `, + styles: ` + :host { + display: block; + height: 100%; + } + `, +}) +export class CsvViewerComponent { + readonly bytes = input(null); + + /** Emitted once a grid is on screen, so the pane drops its skeleton. */ + readonly rendered = output(); + /** Emitted with a user-facing message when the file cannot be read. */ + readonly renderFailed = output(); + + protected readonly table = signal(null); + + protected readonly summary = computed(() => { + const data = this.table(); + if (!data) return ''; + const rows = data.rows.length; + const cols = data.headers.length; + return `${rows.toLocaleString()} ${rows === 1 ? 'row' : 'rows'} · ${cols} ${ + cols === 1 ? 'column' : 'columns' + } · ${DELIMITER_LABELS[data.delimiter]}`; + }); + + protected readonly truncationNotice = computed(() => { + const by = this.table()?.truncatedBy; + switch (by) { + case 'bytes': + return '· preview stops partway through a large file'; + case 'rows': + return '· later rows not shown'; + case 'columns': + return '· later columns not shown'; + default: + return ''; + } + }); + + constructor() { + effect(() => { + const bytes = this.bytes(); + if (!bytes) { + this.table.set(null); + return; + } + + try { + this.table.set(parseCsv(bytes)); + } catch (e) { + this.table.set(null); + this.renderFailed.emit( + e instanceof CsvParseError + ? e.message + : "This file couldn't be read as delimited text.", + ); + return; + } + + this.rendered.emit(); + }); + } +} + +const DELIMITER_LABELS: Readonly> = { + ',': 'comma-separated', + '\t': 'tab-separated', + ';': 'semicolon-separated', + '|': 'pipe-separated', +}; diff --git a/frontend/ai.client/src/app/session/components/message-list/components/file-preview/data-grid.component.spec.ts b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/data-grid.component.spec.ts new file mode 100644 index 00000000..d9fa9a52 --- /dev/null +++ b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/data-grid.component.spec.ts @@ -0,0 +1,134 @@ +import { CdkVirtualScrollViewport } from '@angular/cdk/scrolling'; +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { DataGridComponent } from './data-grid.component'; + +function rows(n: number, cols = 2): string[][] { + return Array.from({ length: n }, (_, i) => + Array.from({ length: cols }, (_, c) => `r${i}c${c}`), + ); +} + +describe('DataGridComponent', () => { + let fixture: ComponentFixture; + + beforeEach(async () => { + // jsdom implements neither Element.scrollTo nor Element.scrollBy, + // and CDK's `scrollToOffset` ends in the former. A no-op keeps the + // component's real code path intact; the scrolling itself is + // verified in a browser, where jsdom's gaps do not apply. + for (const name of ['scrollTo', 'scrollBy'] as const) { + if (typeof (Element.prototype as never as Record)[name] !== 'function') { + (Element.prototype as never as Record)[name] = () => undefined; + } + } + + TestBed.resetTestingModule(); + await TestBed.configureTestingModule({ + imports: [DataGridComponent], + }).compileComponents(); + + fixture = TestBed.createComponent(DataGridComponent); + }); + + function render(headers: string[], body: string[][]): HTMLElement { + fixture.componentRef.setInput('headers', headers); + fixture.componentRef.setInput('rows', body); + fixture.detectChanges(); + return fixture.nativeElement as HTMLElement; + } + + it('renders the headers it is given', () => { + const el = render(['Item', 'Cost'], [['Rent', '1200']]); + + const headers = Array.from(el.querySelectorAll('[role="columnheader"]')) + .map((n) => n.textContent?.trim()) + .filter((t) => t !== '#'); + expect(headers).toEqual(['Item', 'Cost']); + }); + + it('reports the data size to assistive tech, not the rendered window', () => { + const el = render(['a', 'b'], rows(900)); + + const grid = el.querySelector('[role="grid"]'); + expect(grid?.getAttribute('aria-rowcount')).toBe('901'); + expect(grid?.getAttribute('aria-colcount')).toBe('2'); + }); + + it('shows the empty message in place of the body when there are no rows', () => { + fixture.componentRef.setInput('emptyMessage', 'This sheet is empty.'); + const el = render(['a', 'b'], []); + + expect(el.textContent).toContain('This sheet is empty.'); + expect(el.querySelector('cdk-virtual-scroll-viewport')).toBeNull(); + }); + + it('gives header and body rows one shared track list', () => { + const el = render(['a', 'b'], [['1', '2']]); + + const grid = el.querySelector('[role="grid"]') as HTMLElement; + // Gutter plus one track per column. + expect( + grid.style.getPropertyValue('--grid-cols').trim().split(/\s+/), + ).toHaveLength(3); + }); + + describe('re-arming the scroller when the data changes', () => { + /** + * One viewport instance serves every dataset — switching worksheet + * tabs, or previewing a second file without closing the pane. CDK + * picks up the new length but does NOT recompute the rendered + * range, so the body stays pinned to the first screenful while the + * scrollbar moves over the full height. Verified in the browser: at + * scrollOffset 10000 of a 1,200-row sheet the rendered range was + * still {0, 32}, and checkViewportSize() corrected it to {309, 345}. + * + * jsdom gives the viewport zero height, so no rows are ever + * rendered and the range itself cannot be asserted here. What this + * does pin down is that the component asks the viewport to + * re-measure and rewind — the two calls whose absence caused it. + */ + function spyOnViewport() { + const viewport = fixture.debugElement + .query((n) => n.componentInstance instanceof CdkVirtualScrollViewport) + ?.componentInstance as CdkVirtualScrollViewport; + expect(viewport).toBeDefined(); + return { + checkViewportSize: vi.spyOn(viewport, 'checkViewportSize'), + scrollToOffset: vi.spyOn(viewport, 'scrollToOffset'), + }; + } + + it('re-measures and rewinds when the rows are replaced', () => { + vi.useFakeTimers(); + try { + render(['a', 'b'], rows(5)); + const spies = spyOnViewport(); + + render(['a', 'b'], rows(1200)); + vi.runAllTimers(); + + expect(spies.checkViewportSize).toHaveBeenCalled(); + expect(spies.scrollToOffset).toHaveBeenCalledWith(0); + } finally { + vi.useRealTimers(); + } + }); + + it('rewinds the header to match', () => { + vi.useFakeTimers(); + try { + const el = render(['a', 'b'], rows(5)); + const header = el.querySelector('[role="grid"] > div') as HTMLElement; + header.scrollLeft = 250; + + render(['a', 'b'], rows(1200)); + vi.runAllTimers(); + + expect(header.scrollLeft).toBe(0); + } finally { + vi.useRealTimers(); + } + }); + }); +}); diff --git a/frontend/ai.client/src/app/session/components/message-list/components/file-preview/data-grid.component.ts b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/data-grid.component.ts new file mode 100644 index 00000000..9786f4bb --- /dev/null +++ b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/data-grid.component.ts @@ -0,0 +1,345 @@ +import { + CdkVirtualScrollViewport, + ScrollingModule, +} from '@angular/cdk/scrolling'; +import { + ChangeDetectionStrategy, + Component, + ElementRef, + computed, + effect, + input, + viewChild, +} from '@angular/core'; + +/** + * Height of one body row, in CSS px. + * + * Load-bearing: `cdk-virtual-scroll-viewport`'s fixed-size strategy + * computes the spacer height and the rendered window from this number, + * so a row that paints taller than it claims leaves the rows drifting + * out of the viewport as you scroll. Keep it in step with the row height + * in the styles below. + */ +const ROW_HEIGHT_PX = 32; + +/** Column width bounds, in CSS px. */ +const COL_MIN_PX = 88; +const COL_MAX_PX = 320; + +/** + * Approximate px per character at the grid's text size, for sizing + * columns without laying anything out first. + * + * Measured against the real font at 13px: mixed-case words average + * about 6.7px, dates 7.3, digits 7.8, and formula text like + * "=SUM(B2:C2)" close to 8.1 — the wide glyphs are exactly the ones + * spreadsheet content is full of. An estimate tuned to the average + * clipped those by a few px, which reads as a bug rather than as the + * deliberate ellipsis at COL_MAX_PX, so this sits at the top of the + * range instead. Over-estimating only costs a slightly wider column. + */ +const PX_PER_CHAR = 8.2; + +/** Rows sampled when measuring a column's natural width. Measuring all + * of them would walk 50k rows to move a column by a few px. */ +const WIDTH_SAMPLE_ROWS = 100; + +/** + * A scrollable grid of strings — the shared body of every data preview. + * + * Purely presentational: it takes headers and rows and draws them. Where + * those came from is the caller's business, which is what lets one grid + * serve both a `.csv` parsed in the browser and an `.xlsx` read by + * app-api. Cells are rendered exactly as handed over, with no type + * inference, number formatting or date parsing — a preview that + * prettifies `007` into `7` is answering a different question than the + * one being asked, and that has to stay true no matter which reader + * produced the strings. + * + * Virtualised with `cdk-virtual-scroll-viewport`. `@angular/cdk` is + * already a dependency (dialog, menu, overlay, a11y), uniform row height + * is the fixed-size strategy's best case, and the alternative — + * "showing the first 500 rows of 40,000" — is the difference between + * previewing a file and previewing the top of one. + * + * The cost of virtualising is that the grid cannot be a ``: the + * viewport needs to own the scroll container and transform its content, + * which `` will not tolerate. So it is a div grid carrying + * explicit ARIA grid roles, with `aria-rowcount` and `aria-rowindex` + * reporting the true size of the data rather than the size of the + * rendered window. + */ +@Component({ + selector: 'app-data-grid', + changeDetection: ChangeDetectionStrategy.OnPush, + imports: [ScrollingModule], + template: ` +
+ +
+
+
+ +
+ @for (header of headers(); track $index) { +
+ {{ header }} +
+ } +
+
+ + @if (rows().length === 0) { +

+ {{ emptyMessage() }} +

+ } @else { + +
+
+ {{ i + 1 }} +
+ @for (cell of row; track $index) { +
+ {{ cell }} +
+ } +
+ + } + +
+ {{ summary() }} + @if (notice()) { + + + {{ notice() }} + + } +
+
+ `, + styles: ` + :host { + display: block; + height: 100%; + } + + /* Header and body rows share one track list so the columns line up + without measuring anything after layout. */ + .grid-row { + display: grid; + grid-template-columns: var(--grid-cols); + align-items: center; + height: 32px; + width: max-content; + min-width: 100%; + } + + .grid-cell, + .grid-gutter { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + padding-inline: 0.75rem; + font-size: 0.8125rem; + line-height: 1.25rem; + font-variant-numeric: tabular-nums; + } + + /* Colour deliberately lives on the elements, as the utility pair + text-gray-500 / dark:text-gray-400, rather than here. There is no + single neutral step that clears WCAG AA against both surfaces: + gray-400 measures 2.6:1 on the light grid, and gray-500 is the + step the surface generator itself reports as short of AA against + the dark one. (No backticks in this block - one inside a styles + template literal breaks the Angular compiler while tsc passes.) */ + .grid-gutter { + text-align: right; + padding-inline: 0.5rem; + font-size: 0.6875rem; + user-select: none; + } + + /* The viewport is the horizontal scroller as well as the vertical + one, so rows wider than the rail can be reached. */ + cdk-virtual-scroll-viewport { + overflow-x: auto; + } + `, +}) +export class DataGridComponent { + readonly headers = input.required(); + readonly rows = input.required(); + /** Footer line: size, and whatever else the reader knows. */ + readonly summary = input(''); + /** Footer warning, shown after the summary when a cap cut the data. */ + readonly notice = input(''); + /** Shown in place of the body when there are headers but no rows. */ + readonly emptyMessage = input('No data rows to show.'); + readonly ariaLabel = input('Data preview'); + + protected readonly rowHeight = ROW_HEIGHT_PX; + + private readonly headerScroller = + viewChild>('headerScroller'); + private readonly viewport = viewChild(CdkVirtualScrollViewport); + + /** One `grid-template-columns` track list, shared by the header row + * and every body row. */ + protected readonly gridTemplate = computed(() => { + const headers = this.headers(); + const rows = this.rows(); + const gutter = `${gutterWidthFor(rows.length)}px`; + const columns = headers.map( + (header, i) => `${measureColumn(header, rows, i)}px`, + ); + return [gutter, ...columns].join(' '); + }); + + constructor() { + effect((onCleanup) => { + const viewport = this.viewport(); + if (!viewport) return; + const sub = viewport.elementScrolled().subscribe(() => { + this.syncHeaderScroll(); + }); + onCleanup(() => sub.unsubscribe()); + }); + + // Re-arm the scroller whenever the data underneath it changes. + // + // One viewport instance serves every dataset the grid is handed — + // switching worksheet tabs, or previewing a second file without + // closing the pane. CDK picks up the new length (getDataLength() + // reports it, and the spacer grows) but does not recompute the + // rendered range, so the body stays pinned to the first screenful + // while the scrollbar moves over the full height. Measured directly: + // at scrollOffset 10000 of a 1,200-row sheet the rendered range was + // still {start: 0, end: 32}, and checkViewportSize() corrected it to + // {start: 309, end: 345} on the spot. + // + // Resetting to the top is the right behaviour on its own terms too — + // a newly chosen sheet should start at its first row rather than + // inheriting the previous one's offset. + effect((onCleanup) => { + const rows = this.rows(); + const viewport = this.viewport(); + if (!viewport || rows.length === 0) return; + + // After the rows themselves have been rendered: measuring before + // that would measure the outgoing dataset. + const handle = setTimeout(() => { + viewport.scrollToOffset(0); + viewport.checkViewportSize(); + const header = this.headerScroller()?.nativeElement; + if (header) header.scrollLeft = 0; + }); + onCleanup(() => clearTimeout(handle)); + }); + } + + /** `trackBy` on the row index rather than the row: a data file may + * legitimately repeat identical rows, and identity tracking would + * make the virtual scroller reuse the wrong one. */ + protected trackByIndex(index: number): number { + return index; + } + + /** + * Mirror the viewport's horizontal offset onto the header. + * + * Driven by `elementScrolled()` rather than a template `(scroll)` + * binding or `(scrolledIndexChange)`. `scrolledIndexChange` is the + * wrong signal outright — it fires when the first *rendered row* + * changes, so it never fires for a purely horizontal scroll and the + * header stays behind while the body moves. `elementScrolled()` emits + * for both axes, and CDK's scroll dispatcher already runs it outside + * the Angular zone; since the handler only writes `scrollLeft` on a + * DOM node and touches no signal, the sync costs no change detection + * at scroll rate. + */ + private syncHeaderScroll(): void { + const header = this.headerScroller()?.nativeElement; + const viewport = this.viewport(); + if (header && viewport) { + header.scrollLeft = viewport.measureScrollOffset('left'); + } + } +} + +/** Enough room for the largest row number the gutter will show. */ +function gutterWidthFor(rowCount: number): number { + return Math.max(40, String(rowCount).length * 8 + 16); +} + +/** + * Width for one column, from the longest value in a sample of its cells. + * + * Approximated from character counts rather than measured, because + * measuring means laying out every cell before the first paint. The + * clamp matters more than the estimate: a column of long free text stops + * at `COL_MAX_PX` and ellipsises (the full value is on the cell's + * `title`), and a column of short codes still gets a readable minimum. + */ +function measureColumn( + header: string, + rows: readonly (readonly string[])[], + index: number, +): number { + let longest = header.length; + const sampled = Math.min(rows.length, WIDTH_SAMPLE_ROWS); + for (let i = 0; i < sampled; i++) { + const cell = rows[i][index]; + if (cell && cell.length > longest) longest = cell.length; + } + return Math.min( + COL_MAX_PX, + Math.max(COL_MIN_PX, Math.round(longest * PX_PER_CHAR) + 24), + ); +} 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 6d304025..65d76840 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 @@ -22,17 +22,26 @@ import { import { ConfigService } from '../../../../../services/config.service'; import { downloadUrlFor } from '../../../../../shared/utils/file-download-url'; import { TooltipDirective } from '../../../../../components/tooltip/tooltip.directive'; +import { CsvViewerComponent } from './csv-viewer.component'; import { DocxViewerComponent } from './docx-viewer.component'; import { PptxViewerComponent } from './pptx-viewer.component'; +import { XlsxViewerComponent } from './xlsx-viewer.component'; import { PREVIEW_KIND_LABELS, PreviewKind, + previewFetchesBytes, previewKindFor, } from '../../../../services/file-preview/file-preview.model'; /** - * Right-docked pane that previews one uploaded Office file in the - * browser — `.docx` and `.pptx` today. + * Right-docked pane that previews one uploaded file in the browser — + * `.docx`, `.pptx`, `.csv` and `.xlsx` today. + * + * Three of those four are read here from bytes the pane fetched through + * a presigned URL. `.xlsx` is not: no client-side workbook reader was + * shippable, so app-api reads it and its viewer takes an upload id + * instead. `previewFetchesBytes` is what keeps the two paths from + * treading on each other. * * Shares the rail with `ArtifactPanelComponent` through * `DockedPaneService` — same width, same resize affordance, same @@ -52,7 +61,14 @@ import { @Component({ selector: 'app-file-preview-panel', changeDetection: ChangeDetectionStrategy.OnPush, - imports: [NgIcon, TooltipDirective, DocxViewerComponent, PptxViewerComponent], + imports: [ + NgIcon, + TooltipDirective, + CsvViewerComponent, + DocxViewerComponent, + PptxViewerComponent, + XlsxViewerComponent, + ], providers: [ provideIcons({ heroArrowDownTray, @@ -166,6 +182,26 @@ import { (rendered)="onRendered()" /> } + @case ('csv') { + + } + @case ('xlsx') { + + + } @default { { const ref = this.open(); - if (!ref) { - this.requestSeq++; - this.reset(); - return; - } + this.requestSeq++; + this.reset(); + if (!ref) return; + + // An .xlsx is read by app-api, so there is nothing to fetch here + // — running the presigned-URL leg anyway would pull the whole + // workbook into the browser only to ignore it. Its viewer owns + // both the request and the failure, and reports them through the + // same (rendered)/(renderFailed) pair as every other viewer. + const kind = previewKindFor(ref.filename); + if (kind && !previewFetchesBytes(kind)) return; + void this.load(ref.uploadId); }); } @@ -288,7 +331,17 @@ export class FilePreviewPanelComponent { protected retry(): void { const ref = this.open(); - if (ref) void this.load(ref.uploadId); + if (!ref) return; + + const kind = previewKindFor(ref.filename); + if (kind && !previewFetchesBytes(kind)) { + // Nothing to refetch here — clearing the error re-creates the + // viewer, whose own effect issues the request again. + this.reset(); + return; + } + + void this.load(ref.uploadId); } protected onRendered(): void { diff --git a/frontend/ai.client/src/app/session/components/message-list/components/file-preview/xlsx-viewer.component.spec.ts b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/xlsx-viewer.component.spec.ts new file mode 100644 index 00000000..4033873b --- /dev/null +++ b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/xlsx-viewer.component.spec.ts @@ -0,0 +1,230 @@ +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { XlsxViewerComponent } from './xlsx-viewer.component'; +import { + FilePreviewError, + FilePreviewHttpService, +} from '../../../../services/file-preview/file-preview-http.service'; +import { + SheetPreview, + SheetPreviewResponse, +} from '../../../../services/file-preview/sheet-preview.model'; + +function sheet(overrides: Partial = {}): SheetPreview { + return { + name: 'Sheet1', + headers: ['Item', 'Cost'], + rows: [['Rent', '1200']], + totalRows: 1, + truncated: false, + truncatedBy: null, + ...overrides, + }; +} + +function response(sheets: SheetPreview[]): SheetPreviewResponse { + return { + uploadId: 'up1', + filename: 'budget.xlsx', + sheets, + truncated: sheets.some((s) => s.truncated), + }; +} + +describe('XlsxViewerComponent', () => { + let fixture: ComponentFixture; + let fetchSheets: ReturnType; + + beforeEach(async () => { + fetchSheets = vi.fn().mockResolvedValue(response([sheet()])); + + TestBed.resetTestingModule(); + await TestBed.configureTestingModule({ + imports: [XlsxViewerComponent], + providers: [ + { provide: FilePreviewHttpService, useValue: { fetchSheets } }, + ], + }).compileComponents(); + + fixture = TestBed.createComponent(XlsxViewerComponent); + }); + + /** Set the id and drain the fetch promise. */ + async function open(uploadId = 'up1'): Promise { + fixture.componentRef.setInput('uploadId', uploadId); + fixture.detectChanges(); + for (let i = 0; i < 10; i++) await Promise.resolve(); + fixture.detectChanges(); + return fixture.nativeElement as HTMLElement; + } + + it('asks the server for rows rather than fetching the workbook', async () => { + await open(); + + expect(fetchSheets).toHaveBeenCalledExactlyOnceWith('up1'); + }); + + it('renders the sheet it was given', async () => { + // Asserted through the header row and the grid's own row count + // rather than cell text: the CDK viewport has zero height under + // jsdom, so no body rows are ever in the DOM. What matters here is + // that the sheet reached the grid intact. + const el = await open(); + + const headers = Array.from(el.querySelectorAll('[role="columnheader"]')) + .map((n) => n.textContent?.trim()) + .filter((t) => t !== '#'); + expect(headers).toEqual(['Item', 'Cost']); + expect(el.querySelector('[role="grid"]')?.getAttribute('aria-rowcount')).toBe( + '2', + ); + }); + + it('emits rendered once the grid is on screen', async () => { + const seen: number[] = []; + fixture.componentInstance.rendered.subscribe(() => seen.push(1)); + + await open(); + + expect(seen).toHaveLength(1); + }); + + describe('multiple sheets', () => { + beforeEach(() => { + // The sheets differ in their HEADERS, not just their cells, so a + // tab switch is observable in the DOM that jsdom actually renders. + fetchSheets.mockResolvedValue( + response([ + sheet({ name: 'Q1', headers: ['Item', 'Q1 Cost'] }), + sheet({ name: 'Q2', headers: ['Item', 'Q2 Cost'] }), + ]), + ); + }); + + it('offers a tab per sheet and opens on the first', async () => { + const el = await open(); + + const tabs = Array.from(el.querySelectorAll('[role="tab"]')); + expect(tabs.map((t) => t.textContent?.trim())).toEqual(['Q1', 'Q2']); + expect(tabs[0].getAttribute('aria-selected')).toBe('true'); + expect(el.textContent).toContain('Q1 Cost'); + }); + + it('switches the grid when another tab is chosen', async () => { + const el = await open(); + + const tabs = el.querySelectorAll('[role="tab"]'); + (tabs[1] as HTMLElement).click(); + fixture.detectChanges(); + + expect(el.textContent).toContain('Q2 Cost'); + expect(el.textContent).not.toContain('Q1 Cost'); + expect(tabs[1].getAttribute('aria-selected')).toBe('true'); + }); + }); + + it('shows no tab strip for a single-sheet workbook', async () => { + const el = await open(); + + expect(el.querySelector('[role="tablist"]')).toBeNull(); + }); + + describe('the footer summary', () => { + it('names both counts when the sheet was cut short', async () => { + // "500 of 12,480 rows" is the honest form — the grid holds 500 but + // the sheet has far more. + fetchSheets.mockResolvedValue( + response([ + sheet({ + rows: [['a', 'b']], + totalRows: 12480, + truncated: true, + truncatedBy: 'rows', + }), + ]), + ); + + const el = await open(); + + expect(el.querySelector('footer')?.textContent).toContain('1 of 12,480 rows'); + expect(el.querySelector('footer')?.textContent).toContain( + 'later rows not shown', + ); + }); + + it('collapses to one count when nothing was cut', async () => { + const el = await open(); + + const footer = el.querySelector('footer')?.textContent ?? ''; + expect(footer).toContain('1 row'); + expect(footer).not.toContain(' of '); + }); + + it('says values only, because that is what was read', async () => { + const el = await open(); + + expect(el.querySelector('footer')?.textContent).toContain('values only'); + }); + }); + + describe('failures', () => { + it('reports a workbook with no visible sheets rather than a blank grid', async () => { + fetchSheets.mockResolvedValue(response([])); + const failures: string[] = []; + fixture.componentInstance.renderFailed.subscribe((m) => failures.push(m)); + + await open(); + + expect(failures).toEqual(['This workbook has no visible sheets to show.']); + }); + + it('surfaces the message the http service chose', async () => { + fetchSheets.mockRejectedValue( + new FilePreviewError( + 'This workbook is too large to preview. Download it to open in Excel.', + false, + ), + ); + const failures: string[] = []; + fixture.componentInstance.renderFailed.subscribe((m) => failures.push(m)); + + await open(); + + expect(failures).toEqual([ + 'This workbook is too large to preview. Download it to open in Excel.', + ]); + }); + + it('does not emit rendered when the read failed', async () => { + fetchSheets.mockRejectedValue(new FilePreviewError('nope', false)); + const seen: number[] = []; + fixture.componentInstance.rendered.subscribe(() => seen.push(1)); + + await open(); + + expect(seen).toHaveLength(0); + }); + }); + + it('discards a response for a file the pane has moved off', async () => { + // A slow first workbook must not paint over the second one. + let resolveFirst: (r: SheetPreviewResponse) => void = () => undefined; + fetchSheets.mockImplementationOnce( + () => new Promise((r) => (resolveFirst = r)), + ); + fetchSheets.mockResolvedValueOnce( + response([sheet({ headers: ['Second'] })]), + ); + + fixture.componentRef.setInput('uploadId', 'up1'); + fixture.detectChanges(); + await open('up2'); + + resolveFirst(response([sheet({ headers: ['First'] })])); + for (let i = 0; i < 10; i++) await Promise.resolve(); + fixture.detectChanges(); + + expect(fixture.nativeElement.textContent).toContain('Second'); + expect(fixture.nativeElement.textContent).not.toContain('First'); + }); +}); diff --git a/frontend/ai.client/src/app/session/components/message-list/components/file-preview/xlsx-viewer.component.ts b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/xlsx-viewer.component.ts new file mode 100644 index 00000000..a6d77159 --- /dev/null +++ b/frontend/ai.client/src/app/session/components/message-list/components/file-preview/xlsx-viewer.component.ts @@ -0,0 +1,180 @@ +import { + ChangeDetectionStrategy, + Component, + computed, + effect, + inject, + input, + output, + signal, +} from '@angular/core'; +import { + FilePreviewError, + FilePreviewHttpService, +} from '../../../../services/file-preview/file-preview-http.service'; +import { SheetPreview } from '../../../../services/file-preview/sheet-preview.model'; +import { DataGridComponent } from './data-grid.component'; + +/** + * Renders an `.xlsx` as a grid of values, read by app-api. + * + * The one viewer in the pane that does not parse anything. `.docx`, + * `.pptx` and `.csv` all fetch bytes through a presigned URL and read + * them in the browser; this one takes `uploadId` and asks + * `/files/{id}/sheet-preview` for rows, because every client-side + * workbook reader was rejected on its own terms — the npm build of + * SheetJS is frozen on a 2022 release with unfixed advisories, and + * ExcelJS raises outright on a workbook containing a native chart, which + * is exactly what `create_excel_spreadsheet` produces. The distinction + * that unblocked this was that a grid needs a *reader*, not a + * *renderer*, and openpyxl is already what our own spreadsheet tools + * drive. + * + * What the user does not get is fidelity: no fills, fonts, borders, + * merges, column widths or charts. Download-and-open remains the path + * for those, and the pane keeps its download button throughout. + * + * A formula whose value Excel never cached shows as its formula text + * ("=SUM(B2:B10)") rather than as a blank. That is not a flourish — it + * is the normal state of any workbook openpyxl wrote, since openpyxl + * does not evaluate, so a values-only preview would show holes exactly + * where the totals belong. + */ +@Component({ + selector: 'app-xlsx-viewer', + changeDetection: ChangeDetectionStrategy.OnPush, + imports: [DataGridComponent], + template: ` + @if (active(); as sheet) { +
+ @if (sheets().length > 1) { +
+ @for (candidate of sheets(); track candidate.name; let i = $index) { + + } +
+ } + +
+ +
+
+ } + `, + styles: ` + :host { + display: block; + height: 100%; + } + `, +}) +export class XlsxViewerComponent { + /** The file to read. Unlike its sibling viewers this takes an id, not + * bytes — the workbook is never fetched into the browser. */ + readonly uploadId = input(null); + + readonly rendered = output(); + readonly renderFailed = output(); + + private readonly previewHttp = inject(FilePreviewHttpService); + + protected readonly sheets = signal([]); + protected readonly selected = signal(0); + + protected readonly active = computed( + () => this.sheets()[this.selected()] ?? null, + ); + + protected readonly summary = computed(() => { + const sheet = this.active(); + if (!sheet) return ''; + const shown = sheet.rows.length; + const cols = sheet.headers.length; + // `totalRows` is what the sheet claims; `rows.length` is what the + // cap let through. Saying "500 of 12,480" is the honest form, and + // collapsing to one number when they agree keeps the common case + // quiet. + const rowText = + sheet.totalRows > shown + ? `${shown.toLocaleString()} of ${sheet.totalRows.toLocaleString()} rows` + : `${shown.toLocaleString()} ${shown === 1 ? 'row' : 'rows'}`; + return `${rowText} · ${cols} ${cols === 1 ? 'column' : 'columns'} · values only`; + }); + + protected readonly truncationNotice = computed(() => { + switch (this.active()?.truncatedBy) { + case 'rows': + return '· later rows not shown'; + case 'columns': + return '· later columns not shown'; + default: + return ''; + } + }); + + /** Bumped per load so a slow response for a file the pane has since + * moved off is discarded rather than painted. */ + private requestSeq = 0; + + constructor() { + effect(() => { + const uploadId = this.uploadId(); + this.requestSeq++; + this.sheets.set([]); + this.selected.set(0); + if (uploadId) void this.load(uploadId, this.requestSeq); + }); + } + + private async load(uploadId: string, seq: number): Promise { + try { + const response = await this.previewHttp.fetchSheets(uploadId); + if (seq !== this.requestSeq) return; + + if (response.sheets.length === 0) { + // Every sheet hidden, or none readable. Blank grids would be a + // worse answer than saying so. + this.renderFailed.emit('This workbook has no visible sheets to show.'); + return; + } + + this.sheets.set(response.sheets); + this.rendered.emit(); + } catch (e) { + if (seq !== this.requestSeq) return; + this.renderFailed.emit( + e instanceof FilePreviewError + ? e.message + : "This workbook couldn't be read.", + ); + } + } + + protected select(index: number): void { + this.selected.set(index); + } +} 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 c3b04b25..f42df958 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,21 +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(); + it.each([ + ['deck.pptx'], + ['rows.csv'], + ['budget.xlsx'], + ])('offers a preview for %s as well as a .docx', (filename) => { + expect(render({ filename, upload_id: 'up2' })).not.toBeNull(); expect(previewButton()).not.toBeNull(); }); it('offers no preview for formats the pane cannot render', () => { - // .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', - ]) { + // The legacy binary formats are not OOXML at all, and .xls would + // need a library we did not take on for a format nothing in the + // product generates. All of them still get their download link; + // only the button is withheld. + for (const filename of ['old.doc', 'old.ppt', 'old.xls', 'notes.txt']) { expect(render({ filename, upload_id: 'up1' })).not.toBeNull(); expect(previewButton()).toBeNull(); } diff --git a/frontend/ai.client/src/app/session/services/chat/stream-parser.file-preview.spec.ts b/frontend/ai.client/src/app/session/services/chat/stream-parser.file-preview.spec.ts index ce55496a..c1c0ffa9 100644 --- a/frontend/ai.client/src/app/session/services/chat/stream-parser.file-preview.spec.ts +++ b/frontend/ai.client/src/app/session/services/chat/stream-parser.file-preview.spec.ts @@ -85,10 +85,19 @@ describe('StreamParserService — auto-opening the file preview pane', () => { expect(preview.openFile()?.filename).toBe('report.docx'); }); + it('opens a generated .xlsx too, now that the pane can read one', () => { + // The pane reads a workbook server-side, so a spreadsheet the agent + // just produced opens on the same terms as a .docx or .pptx. + streamFile({ filename: 'budget.xlsx', upload_id: 'up3' }); + + expect(preview.openFile()?.filename).toBe('budget.xlsx'); + }); + it('leaves a format the pane cannot render alone', () => { // Opening a pane that could only show an error is worse than letting - // the download card speak for itself. - streamFile({ filename: 'budget.xlsx', upload_id: 'up3' }); + // the download card speak for itself. .xls is the pre-2007 binary + // format, which no reader here can open. + streamFile({ filename: 'legacy.xls', upload_id: 'up3b' }); expect(preview.openFile()).toBeNull(); }); diff --git a/frontend/ai.client/src/app/session/services/file-preview/csv-parse.spec.ts b/frontend/ai.client/src/app/session/services/file-preview/csv-parse.spec.ts new file mode 100644 index 00000000..e97d2d56 --- /dev/null +++ b/frontend/ai.client/src/app/session/services/file-preview/csv-parse.spec.ts @@ -0,0 +1,227 @@ +import { describe, expect, it } from 'vitest'; +import { + CSV_MAX_COLUMNS, + CSV_MAX_ROWS, + CsvParseError, + columnLetter, + parseCsv, +} from './csv-parse'; + +/** UTF-8 encode, since the parser takes the bytes the fetch returned. */ +function bytes(text: string): ArrayBuffer { + const encoded = new TextEncoder().encode(text); + return encoded.buffer.slice( + encoded.byteOffset, + encoded.byteOffset + encoded.byteLength, + ) as ArrayBuffer; +} + +describe('parseCsv', () => { + it('reads a plain comma-delimited file', () => { + const table = parseCsv(bytes('name,qty\nwidget,3\ngadget,7\n')); + + expect(table.headers).toEqual(['name', 'qty']); + expect(table.rows).toEqual([ + ['widget', '3'], + ['gadget', '7'], + ]); + expect(table.delimiter).toBe(','); + expect(table.truncated).toBe(false); + }); + + it('keeps cells as strings rather than coercing them', () => { + // The whole reason the grid is not a spreadsheet: a preview that + // renders 007 as 7 misrepresents what the file will hand downstream. + const table = parseCsv(bytes('zip,ratio\n007,3-4\n')); + + expect(table.rows[0]).toEqual(['007', '3-4']); + }); + + describe('RFC 4180 quoting', () => { + it('keeps a delimiter inside a quoted field', () => { + const table = parseCsv(bytes('a,b\n"Boise, ID",2\n')); + + expect(table.rows[0]).toEqual(['Boise, ID', '2']); + }); + + it('keeps a newline inside a quoted field', () => { + const table = parseCsv(bytes('a,b\n"line one\nline two",2\n')); + + expect(table.rows).toHaveLength(1); + expect(table.rows[0][0]).toBe('line one\nline two'); + }); + + it('unescapes a doubled quote', () => { + const table = parseCsv(bytes('a\n"she said ""hi"""\n')); + + expect(table.rows[0][0]).toBe('she said "hi"'); + }); + + it('treats a mid-field quote as literal', () => { + // `12" pipe` is not an opening quote — spreadsheets read it as text. + const table = parseCsv(bytes('part\n12" pipe\n')); + + expect(table.rows[0][0]).toBe('12" pipe'); + }); + }); + + describe('delimiter sniffing', () => { + it('picks the tab in a TSV', () => { + const table = parseCsv(bytes('name\tqty\nwidget\t3\n')); + + expect(table.delimiter).toBe('\t'); + expect(table.headers).toEqual(['name', 'qty']); + }); + + it('picks the semicolon over commas inside prose', () => { + // The comma occurs more often overall, but only the semicolon + // occurs the same number of times in every row. + const table = parseCsv( + bytes( + 'title;note\n' + + 'First;"a, b, c, d"\n' + + 'Second;"e, f, g, h"\n' + + 'Third;"i, j, k, l"\n', + ), + ); + + expect(table.delimiter).toBe(';'); + expect(table.headers).toEqual(['title', 'note']); + expect(table.rows[0]).toEqual(['First', 'a, b, c, d']); + }); + + it('falls back to a comma for a single-column file', () => { + const table = parseCsv(bytes('name\nwidget\ngadget\n')); + + expect(table.delimiter).toBe(','); + expect(table.headers).toEqual(['name']); + expect(table.rows).toEqual([['widget'], ['gadget']]); + }); + }); + + describe('line endings', () => { + it.each([ + ['CRLF', 'a,b\r\n1,2\r\n'], + ['LF', 'a,b\n1,2\n'], + ['CR', 'a,b\r1,2\r'], + ])('handles %s', (_label, text) => { + const table = parseCsv(bytes(text)); + + expect(table.headers).toEqual(['a', 'b']); + expect(table.rows).toEqual([['1', '2']]); + }); + + it('does not add an empty row for a trailing newline', () => { + expect(parseCsv(bytes('a\n1\n')).rows).toEqual([['1']]); + }); + + it('reads a final row with no trailing newline', () => { + expect(parseCsv(bytes('a\n1')).rows).toEqual([['1']]); + }); + }); + + describe('ragged rows', () => { + it('pads a short row to the grid width', () => { + const table = parseCsv(bytes('a,b,c\n1,2\n')); + + expect(table.rows[0]).toEqual(['1', '2', '']); + }); + + it('widens the grid for a row with extra fields', () => { + // Showing the stray field is the point — clipping it to the + // header's width would hide the malformed row we are previewing. + const table = parseCsv(bytes('a,b\n1,2,3\n')); + + expect(table.headers).toEqual(['a', 'b', 'C']); + expect(table.rows[0]).toEqual(['1', '2', '3']); + }); + }); + + describe('headers', () => { + it('substitutes a spreadsheet letter for a blank header', () => { + const table = parseCsv(bytes(',name\n0,widget\n')); + + expect(table.headers).toEqual(['A', 'name']); + }); + + it('trims surrounding whitespace', () => { + expect(parseCsv(bytes(' name , qty \n1,2\n')).headers).toEqual([ + 'name', + 'qty', + ]); + }); + }); + + it('strips a UTF-8 BOM from the first header', () => { + const table = parseCsv(bytes('name,qty\nwidget,3\n')); + + expect(table.headers).toEqual(['name', 'qty']); + }); + + it('renders a header-only file as an empty grid', () => { + const table = parseCsv(bytes('name,qty\n')); + + expect(table.headers).toEqual(['name', 'qty']); + expect(table.rows).toEqual([]); + }); + + describe('caps', () => { + it('stops at the row cap and reports it', () => { + const rows = Array.from( + { length: CSV_MAX_ROWS + 50 }, + (_, i) => `${i},x`, + ).join('\n'); + const table = parseCsv(bytes(`a,b\n${rows}\n`)); + + expect(table.rows).toHaveLength(CSV_MAX_ROWS - 1); // header took one + expect(table.truncated).toBe(true); + expect(table.truncatedBy).toBe('rows'); + }); + + it('stops at the column cap and reports it', () => { + const wide = Array.from({ length: CSV_MAX_COLUMNS + 10 }, (_, i) => + String(i), + ).join(','); + const table = parseCsv(bytes(`${wide}\n${wide}\n`)); + + expect(table.headers).toHaveLength(CSV_MAX_COLUMNS); + expect(table.rows[0]).toHaveLength(CSV_MAX_COLUMNS); + expect(table.truncated).toBe(true); + expect(table.truncatedBy).toBe('columns'); + }); + }); + + describe('failures', () => { + it('rejects an empty file', () => { + expect(() => parseCsv(bytes(''))).toThrow(CsvParseError); + }); + + it('rejects a whitespace-only file', () => { + expect(() => parseCsv(bytes('\n\n \n'))).toThrow(CsvParseError); + }); + }); + + it('survives bytes that are not valid UTF-8', () => { + // A Windows-1252 smart quote (0x92) is invalid UTF-8. Showing a + // replacement character beats refusing the file. + const raw = new Uint8Array([ + 0x61, 0x0a, 0x92, 0x78, // "a\n<0x92>x" + ]); + const table = parseCsv(raw.buffer as ArrayBuffer); + + expect(table.headers).toEqual(['a']); + expect(table.rows[0][0]).toContain('x'); + }); +}); + +describe('columnLetter', () => { + it.each([ + [0, 'A'], + [25, 'Z'], + [26, 'AA'], + [51, 'AZ'], + [52, 'BA'], + ])('maps %i to %s', (index, expected) => { + expect(columnLetter(index)).toBe(expected); + }); +}); diff --git a/frontend/ai.client/src/app/session/services/file-preview/csv-parse.ts b/frontend/ai.client/src/app/session/services/file-preview/csv-parse.ts new file mode 100644 index 00000000..b9b703c9 --- /dev/null +++ b/frontend/ai.client/src/app/session/services/file-preview/csv-parse.ts @@ -0,0 +1,313 @@ +/** + * A delimited-text parser, sized for previewing a file rather than + * ingesting one. + * + * Hand-rolled on purpose. The grid needs exactly one thing a library + * would give us — RFC 4180 field splitting — and the rest of what a + * parser package ships (streaming, workers, type coercion, header + * mapping, a plugin surface) is weight we would carry in the bundle and + * in `npm audit` forever. The whole of the specification that matters + * here is: fields separated by a delimiter, rows by a newline, a field + * may be quoted, a quote inside a quoted field is doubled, and a quoted + * field may contain the delimiter or a newline. + * + * It is deliberately NOT a spreadsheet: no formulas, no types, no dates. + * Every cell stays the string the file contained, because a preview that + * silently reinterprets `007` as `7` or `3-4` as a date is lying about + * the bytes the user is about to hand to something else. + */ + +/** Delimiters sniffed for, in preference order on a tie. */ +const CANDIDATE_DELIMITERS = [',', '\t', ';', '|'] as const; + +export type CsvDelimiter = (typeof CANDIDATE_DELIMITERS)[number]; + +/** + * Most bytes decoded and parsed. + * + * The file is already fully downloaded by the time we get here, so this + * does not bound the transfer — it bounds the main-thread parse and the + * DOM behind it. 10 MB of CSV is on the order of 100k rows, far past + * what anyone reads in a preview pane, and parses in well under a + * second. Past it we take the leading slice and say so. + */ +export const CSV_MAX_BYTES = 10 * 1024 * 1024; + +/** + * Most rows retained. + * + * A second, independent stop from `CSV_MAX_BYTES`: a file of very short + * rows can stay under the byte cap and still produce enough rows to make + * even a virtualised grid's backing array unpleasant. + */ +export const CSV_MAX_ROWS = 50_000; + +/** + * Most columns retained. + * + * Guards against a file whose quoting is broken badly enough that a + * whole document collapses into one row of many thousands of fields — + * which would otherwise become a `grid-template-columns` with that many + * tracks. + */ +export const CSV_MAX_COLUMNS = 256; + +/** What the grid renders. */ +export interface CsvTable { + /** First row of the file, used as column labels. */ + headers: string[]; + /** Every row after the header, padded to `headers.length`. */ + rows: string[][]; + /** The delimiter that was sniffed. Shown in the pane's subtitle. */ + delimiter: CsvDelimiter; + /** True when a cap stopped us short of the end of the file. */ + truncated: boolean; + /** Which cap fired, for the notice the pane shows. */ + truncatedBy: 'bytes' | 'rows' | 'columns' | null; +} + +/** The file could not be read as delimited text at all. */ +export class CsvParseError extends Error { + constructor(message: string) { + super(message); + this.name = 'CsvParseError'; + } +} + +/** + * Decode bytes to text, honouring a UTF-8 BOM and surviving a file that + * is not valid UTF-8. + * + * `fatal: false` is the point: CSV exported from Excel on Windows is + * routinely Windows-1252, and a preview that refuses the file outright + * is worse than one that shows `�` where a smart quote was. The bytes on + * S3 are untouched either way — this only affects what we draw. + */ +function decode(bytes: ArrayBuffer): string { + const text = new TextDecoder('utf-8', { fatal: false }).decode( + bytes.byteLength > CSV_MAX_BYTES ? bytes.slice(0, CSV_MAX_BYTES) : bytes, + ); + // TextDecoder strips a leading BOM only for the exact `utf-8` label in + // some engines and not others; strip it ourselves so a header never + // renders with an invisible first character. + return text.charCodeAt(0) === 0xfeff ? text.slice(1) : text; +} + +/** + * Pick the delimiter by counting candidates in the first few lines, + * ignoring anything inside quotes. + * + * Counting *consistency* rather than raw frequency is what makes this + * reliable: a prose column full of commas can out-count the real `;` + * delimiter on total occurrences, but only the real delimiter appears + * the same number of times in every row. + */ +function sniffDelimiter(text: string): CsvDelimiter { + const sample = sampleLines(text, 5); + if (sample.length === 0) return ','; + + let best: CsvDelimiter = ','; + let bestScore = -1; + + for (const candidate of CANDIDATE_DELIMITERS) { + const counts = sample.map((line) => countOutsideQuotes(line, candidate)); + // A delimiter that never appears is not a delimiter. + if (counts[0] === 0) continue; + const consistent = counts.every((c) => c === counts[0]); + // Consistency dominates; frequency breaks ties among consistent ones. + const score = (consistent ? 1000 : 0) + counts[0]; + if (score > bestScore) { + bestScore = score; + best = candidate; + } + } + + return best; +} + +/** First `max` physical lines, quote-awareness deliberately skipped — + * this is a sample for counting, not a parse. */ +function sampleLines(text: string, max: number): string[] { + const lines: string[] = []; + let start = 0; + while (lines.length < max && start < text.length) { + let end = text.indexOf('\n', start); + if (end === -1) end = text.length; + const line = text.slice(start, end).replace(/\r$/, ''); + if (line.length > 0) lines.push(line); + start = end + 1; + } + return lines; +} + +function countOutsideQuotes(line: string, delimiter: string): number { + let count = 0; + let inQuotes = false; + for (let i = 0; i < line.length; i++) { + const ch = line[i]; + if (ch === '"') { + inQuotes = !inQuotes; + } else if (!inQuotes && ch === delimiter) { + count++; + } + } + return count; +} + +/** + * Split delimited text into rows of fields. + * + * One pass, character at a time, tracking whether we are inside a quoted + * field. `\r\n`, `\n` and a bare `\r` all end a row, because files + * produced on all three platforms land here. + */ +function splitRows( + text: string, + delimiter: string, +): { rows: string[][]; hitRowCap: boolean } { + const rows: string[][] = []; + let row: string[] = []; + let field = ''; + let inQuotes = false; + let hitRowCap = false; + + const endField = (): void => { + row.push(field); + field = ''; + }; + const endRow = (): boolean => { + row.push(field); + field = ''; + // A trailing newline at end of file would otherwise add a row of one + // empty field. Only drop it when the row is exactly that. + if (!(row.length === 1 && row[0] === '')) rows.push(row); + row = []; + return rows.length >= CSV_MAX_ROWS; + }; + + for (let i = 0; i < text.length; i++) { + const ch = text[i]; + + if (inQuotes) { + if (ch === '"') { + if (text[i + 1] === '"') { + field += '"'; + i++; + } else { + inQuotes = false; + } + } else { + field += ch; + } + continue; + } + + if (ch === '"' && field === '') { + // A quote only opens a quoted field at the start of one. Mid-field + // quotes (`12" pipe`) are literal, which is what every spreadsheet + // does with them. + inQuotes = true; + } else if (ch === delimiter) { + endField(); + } else if (ch === '\n') { + if (endRow()) { + hitRowCap = true; + break; + } + } else if (ch === '\r') { + if (text[i + 1] === '\n') i++; + if (endRow()) { + hitRowCap = true; + break; + } + } else { + field += ch; + } + } + + // Whatever is left when the text runs out is a final row, unless the + // file ended on a newline and left nothing behind. + if (!hitRowCap && (field !== '' || row.length > 0)) endRow(); + + return { rows, hitRowCap }; +} + +/** + * Parse a delimited file into a rectangular table for the preview grid. + * + * Throws `CsvParseError` only when there is nothing to show at all. A + * file that is merely *odd* — ragged rows, duplicate headers, a stray + * quote — is rendered as-is, because the point of a preview is to show + * the user what the file actually contains. + */ +export function parseCsv(bytes: ArrayBuffer): CsvTable { + const truncatedByBytes = bytes.byteLength > CSV_MAX_BYTES; + const text = decode(bytes); + + if (text.trim() === '') { + throw new CsvParseError('This file is empty.'); + } + + const delimiter = sniffDelimiter(text); + const { rows: rawRows, hitRowCap } = splitRows(text, delimiter); + + if (rawRows.length === 0) { + throw new CsvParseError('This file has no rows to show.'); + } + + // The widest row decides the grid, so a row with extra fields is shown + // in full rather than silently clipped to the header's width. + const widest = rawRows.reduce((max, r) => Math.max(max, r.length), 0); + const columnCount = Math.min(widest, CSV_MAX_COLUMNS); + const hitColumnCap = widest > CSV_MAX_COLUMNS; + + const [headerRow, ...bodyRows] = rawRows; + const headers = normalizeHeaders(headerRow, columnCount); + const rows = bodyRows.map((r) => pad(r, columnCount)); + + return { + headers, + rows, + delimiter, + truncated: truncatedByBytes || hitRowCap || hitColumnCap, + truncatedBy: truncatedByBytes + ? 'bytes' + : hitRowCap + ? 'rows' + : hitColumnCap + ? 'columns' + : null, + }; +} + +/** Pad or clip a row to the grid's width. */ +function pad(row: string[], width: number): string[] { + if (row.length === width) return row; + if (row.length > width) return row.slice(0, width); + return [...row, ...Array(width - row.length).fill('')]; +} + +/** + * Column labels, with a placeholder for any the file left blank. + * + * A blank header is common — an index column exported from pandas has + * one — and an empty `
` gives a screen reader nothing to announce + * when it reads a cell's column. The spreadsheet-style letter is the + * familiar stand-in. + */ +function normalizeHeaders(row: string[], width: number): string[] { + return pad(row, width).map( + (h, i) => h.trim() || columnLetter(i), + ); +} + +/** 0 -> A, 25 -> Z, 26 -> AA — the spreadsheet column naming. */ +export function columnLetter(index: number): string { + let n = index; + let out = ''; + do { + out = String.fromCharCode(65 + (n % 26)) + out; + n = Math.floor(n / 26) - 1; + } while (n >= 0); + return out; +} 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 69aaf795..99f91f5c 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 @@ -87,6 +87,41 @@ describe('FilePreviewHttpService', () => { expect(doc.mimeType).toBe(PPTX_MIME); }); + it('accepts a .csv the browser mislabelled as an Excel type', async () => { + // Windows reports application/vnd.ms-excel for a .csv whenever Excel + // is the registered handler, and the recorded MIME is whatever the + // browser said at upload time. Refusing it would fail the preview on + // the most ordinary desktop in the building. + fetchMock.mockResolvedValue({ + ok: true, + status: 200, + arrayBuffer: () => Promise.resolve(new ArrayBuffer(4)), + }); + + const pending = service.fetchDocument('up1'); + flushPreviewUrl({ + mimeType: 'application/vnd.ms-excel', + filename: 'export.csv', + }); + const doc = await pending; + + expect(doc.kind).toBe('csv'); + }); + + it('still refuses an .xls, which shares that MIME type', async () => { + // The extension is what chooses the viewer, so widening the accepted + // MIME list for .csv must not make the legacy binary format + // previewable. + const pending = service.fetchDocument('up1'); + flushPreviewUrl({ + mimeType: 'application/vnd.ms-excel', + filename: 'budget.xls', + }); + + await expect(pending).rejects.toThrow(FilePreviewError); + expect(fetchMock).not.toHaveBeenCalled(); + }); + 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 @@ -99,7 +134,18 @@ describe('FilePreviewHttpService', () => { expect(fetchMock).not.toHaveBeenCalled(); }); - it('refuses a format the pane has no renderer for', async () => { + it('refuses a format the pane cannot preview at all', async () => { + const pending = service.fetchDocument('up1'); + flushPreviewUrl({ mimeType: 'application/pdf', filename: 'report.pdf' }); + + await expect(pending).rejects.toThrow(FilePreviewError); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('refuses to fetch the bytes of a kind that is read server-side', async () => { + // An .xlsx has a viewer, but not one that reads bytes. Letting it + // through here would pull a whole workbook into the browser and + // then throw it away, so the byte path rejects it before the fetch. const pending = service.fetchDocument('up1'); flushPreviewUrl({ mimeType: @@ -111,6 +157,53 @@ describe('FilePreviewHttpService', () => { expect(fetchMock).not.toHaveBeenCalled(); }); + describe('fetchSheets', () => { + it('asks app-api for rows and never touches S3', async () => { + // The .xlsx path has one leg, not two: no presigned URL, and the + // workbook never reaches the browser. + const pending = service.fetchSheets('up1'); + const req = httpMock.expectOne( + '/api/files/up1/sheet-preview', + ); + expect(req.request.method).toBe('GET'); + req.flush({ + uploadId: 'up1', + filename: 'budget.xlsx', + sheets: [], + truncated: false, + }); + + await expect(pending).resolves.toMatchObject({ filename: 'budget.xlsx' }); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it.each([ + [413, false, 'too large'], + [422, false, 'corrupt or password-protected'], + [415, false, "isn't a spreadsheet"], + [404, true, 'no longer available'], + [500, true, 'Check your connection'], + ])( + 'maps %i to a %s-retryable message', + async (status, retryable, fragment) => { + // The route distinguishes these on purpose: 413 and 422 are + // permanent facts about the file, while a 5xx is worth another go. + const pending = service.fetchSheets('up1'); + httpMock + .expectOne('/api/files/up1/sheet-preview') + .flush('err', { status, statusText: 'error' }); + + await expect(pending).rejects.toThrow(FilePreviewError); + await expect(pending).rejects.toMatchObject({ retryable }); + await expect(pending).rejects.toThrow( + expect.objectContaining({ + message: expect.stringContaining(fragment), + }), + ); + }, + ); + }); + it('fetches S3 without credentials', async () => { // S3 answers a CORS GET without Access-Control-Allow-Credentials, so // a credentialed request is rejected by the browser before it is 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 ba95895e..a7d6fe21 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 @@ -5,8 +5,10 @@ import { ConfigService } from '../../../services/config.service'; import { PREVIEW_KIND_MIMES, PreviewKind, + previewFetchesBytes, previewKindFor, } from './file-preview.model'; +import { SheetPreviewResponse } from './sheet-preview.model'; /** `GET /files/{uploadId}/preview-url` — camelCase aliases on the wire. */ interface PreviewUrlResponseDto { @@ -88,13 +90,25 @@ export class FilePreviewHttpService { const meta = await this.requestPreviewUrl(uploadId); const kind = previewKindFor(meta.filename); - if (kind === null || meta.mimeType !== PREVIEW_KIND_MIMES[kind]) { + if (kind === null || !PREVIEW_KIND_MIMES[kind].includes(meta.mimeType)) { throw new FilePreviewError( `This file is a ${meta.mimeType || 'unknown type'}, which can't be previewed here.`, false, ); } + // A kind that is read server-side must never reach the byte path. + // Nothing here could render the result, and the cost of finding out + // is a whole workbook pulled into the browser and thrown away — so + // this fails loudly rather than wasting the transfer. `fetchSheets` + // is the route for those. + if (!previewFetchesBytes(kind)) { + throw new FilePreviewError( + 'This file is read on the server; use fetchSheets instead.', + false, + ); + } + const bytes = await this.fetchBytes(meta.url); return { bytes, mimeType: meta.mimeType, filename: meta.filename, kind }; } @@ -121,6 +135,28 @@ export class FilePreviewHttpService { } } + /** + * Fetch an `.xlsx` as rows, rather than as bytes. + * + * The only preview that does not go through `fetchDocument`, and the + * reason is the renderer landscape rather than anything about the + * file: there is no client-side workbook reader we are willing to + * ship, so app-api reads it with openpyxl and sends values. No + * presigned URL and no second leg — the workbook never reaches the + * browser at all. + */ + async fetchSheets(uploadId: string): Promise { + try { + return await firstValueFrom( + this.http.get( + `${this.config.appApiUrl()}/files/${encodeURIComponent(uploadId)}/sheet-preview`, + ), + ); + } catch (e) { + throw sheetPreviewError(e); + } + } + private async fetchBytes(url: string): Promise { let response: Response; try { @@ -142,3 +178,40 @@ export class FilePreviewHttpService { return response.arrayBuffer(); } } + +/** + * Turn a `/sheet-preview` failure into something the pane can show. + * + * The route distinguishes its failures on purpose, and each one means a + * different thing to the user: 413 and 422 are permanent facts about the + * file that no retry changes, while a 5xx or a dropped connection is + * worth another go. 415 should be unreachable — the extension chose this + * viewer — so it reads as the mislabelled file it is. + */ +function sheetPreviewError(e: unknown): FilePreviewError { + const status = (e as { status?: number })?.status ?? 0; + switch (status) { + case 404: + return new FilePreviewError('This workbook is no longer available.', true); + case 413: + return new FilePreviewError( + 'This workbook is too large to preview. Download it to open in Excel.', + false, + ); + case 415: + return new FilePreviewError( + "This file isn't a spreadsheet the preview can read.", + false, + ); + case 422: + return new FilePreviewError( + 'This workbook could not be read. It may be corrupt or password-protected.', + false, + ); + default: + return new FilePreviewError( + "Couldn't load this workbook. Check your connection and try again.", + true, + ); + } +} 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 index 999bf627..b79c4e98 100644 --- 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 @@ -2,18 +2,23 @@ import { describe, it, expect } from 'vitest'; import { isPreviewableFilename, PREVIEW_KIND_LABELS, + previewFetchesBytes, previewKindFor, } from './file-preview.model'; describe('previewKindFor', () => { - it('maps the OOXML formats the pane can render', () => { + it('maps the formats the pane can render', () => { expect(previewKindFor('plan.docx')).toBe('docx'); expect(previewKindFor('deck.pptx')).toBe('pptx'); + expect(previewKindFor('rows.csv')).toBe('csv'); + expect(previewKindFor('budget.xlsx')).toBe('xlsx'); }); it('ignores case and surrounding whitespace', () => { expect(previewKindFor(' REPORT.DOCX ')).toBe('docx'); expect(previewKindFor('Quarterly Review.PPTX')).toBe('pptx'); + expect(previewKindFor('Export.CSV')).toBe('csv'); + expect(previewKindFor('Budget FY27.XLSX')).toBe('xlsx'); }); it('declines the pre-2007 binary formats', () => { @@ -21,12 +26,21 @@ describe('previewKindFor', () => { // preview would only produce an error the user cannot act on. expect(previewKindFor('old.doc')).toBeNull(); expect(previewKindFor('old.ppt')).toBeNull(); + expect(previewKindFor('old.xls')).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('reads .xlsx server-side rather than from bytes', () => { + // The one kind whose bytes never reach the browser: no client-side + // workbook renderer was shippable, so app-api reads it instead. A + // panel that ran its presigned-URL fetch for this kind would + // download the whole workbook and then ignore it. + expect(previewFetchesBytes('xlsx')).toBe(false); + }); + + it('reads every other kind from bytes it fetched', () => { + for (const kind of ['docx', 'pptx', 'csv'] as const) { + expect(previewFetchesBytes(kind)).toBe(true); + } }); it('declines an extension that merely contains a known one', () => { @@ -35,13 +49,21 @@ describe('previewKindFor', () => { }); it('agrees with isPreviewableFilename', () => { - for (const name of ['a.docx', 'b.pptx', 'c.xlsx', 'd.txt', 'e.doc']) { + for (const name of [ + 'a.docx', + 'b.pptx', + 'c.xlsx', + 'd.txt', + 'e.doc', + 'f.csv', + 'g.xls', + ]) { expect(isPreviewableFilename(name)).toBe(previewKindFor(name) !== null); } }); it('labels every kind it can return', () => { - for (const name of ['a.docx', 'b.pptx']) { + for (const name of ['a.docx', 'b.pptx', 'c.csv', 'd.xlsx']) { 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 02f80005..cbb4ffea 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 @@ -22,19 +22,52 @@ export const DOCX_MIME = export const PPTX_MIME = 'application/vnd.openxmlformats-officedocument.presentationml.presentation'; +/** + * MIME type of a comma-separated values file. Matches the `.csv` entry + * in `apis.shared.files.ALLOWED_EXTENSIONS`. + */ +export const CSV_MIME = 'text/csv'; + +/** + * MIME type of an Excel workbook (OOXML). Matches the `.xlsx` entry in + * `apis.shared.files.ALLOWED_EXTENSIONS` and `SHEET_PREVIEW_MIME_TYPES` + * in the same module. + */ +export const XLSX_MIME = + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'; + /** What the pane knows how to render, and which viewer does it. */ -export type PreviewKind = 'docx' | 'pptx'; +export type PreviewKind = 'docx' | 'pptx' | 'csv' | 'xlsx'; /** Human label for the pane header's subtitle. */ export const PREVIEW_KIND_LABELS: Readonly> = { docx: 'Word document', pptx: 'PowerPoint presentation', + csv: 'Data file', + xlsx: 'Spreadsheet', }; -/** The MIME type each viewer requires, checked against `/preview-url`. */ -export const PREVIEW_KIND_MIMES: Readonly> = { - docx: DOCX_MIME, - pptx: PPTX_MIME, +/** + * The MIME types each viewer will accept, checked against what + * `/preview-url` reports. + * + * A list rather than a single type because the recorded MIME is + * whatever the *browser* reported at upload time (`request.mime_type` + * in `files/service.py`), not something the server derives from the + * bytes. For the OOXML formats that is reliably the one true type. For + * `.csv` it is not: Windows reports `application/vnd.ms-excel` for a + * `.csv` whenever Excel is the registered handler, and some clients + * send `application/csv` or fall back to `text/plain`. Rejecting those + * would fail the preview on the most ordinary desktop in the building, + * for a file whose extension already told us what it is. + */ +export const PREVIEW_KIND_MIMES: Readonly< + Record +> = { + docx: [DOCX_MIME], + pptx: [PPTX_MIME], + csv: [CSV_MIME, 'application/csv', 'application/vnd.ms-excel', 'text/plain'], + xlsx: [XLSX_MIME], }; /** @@ -49,23 +82,41 @@ export const PREVIEW_KIND_MIMES: Readonly> = { * `/preview-url` reports, so a mislabelled `.docx` fails there rather * than feeding garbage to the renderer. * - * Legacy `.doc` and `.ppt` are deliberately excluded: they are the - * pre-2007 binary formats, which the OOXML renderers cannot read at all. + * Legacy `.doc`, `.ppt` and `.xls` are deliberately excluded: they are + * the pre-2007 binary formats, which neither the OOXML renderers nor + * the delimited-text parser can 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 + * `.xlsx` is previewed, but not like the others: it is the one kind + * whose bytes never reach the browser. No client-side workbook renderer + * was shippable — 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. + * chart — so app-api reads it with openpyxl and sends rows instead. Any + * caller that branches on MIME type for `.xlsx` is therefore on the + * wrong path; branch on the kind, and see `XlsxViewerComponent`. */ 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'; + if (/\.csv$/i.test(name)) return 'csv'; + if (/\.xlsx$/i.test(name)) return 'xlsx'; return null; } +/** + * Whether the pane reads this kind from bytes it fetched itself. + * + * False only for `xlsx`, whose viewer takes an upload id and asks the + * server for rows. The panel uses this to decide whether to run the + * presigned-URL fetch at all — without it, opening a workbook would + * download the whole file to the browser and then ignore it. + */ +export function previewFetchesBytes(kind: PreviewKind): boolean { + return kind !== 'xlsx'; +} + /** Whether a filename is one the preview pane can render. */ export function isPreviewableFilename(filename: string): boolean { return previewKindFor(filename) !== null; diff --git a/frontend/ai.client/src/app/session/services/file-preview/sheet-preview.model.ts b/frontend/ai.client/src/app/session/services/file-preview/sheet-preview.model.ts new file mode 100644 index 00000000..cc8783b5 --- /dev/null +++ b/frontend/ai.client/src/app/session/services/file-preview/sheet-preview.model.ts @@ -0,0 +1,32 @@ +/** + * The shape `GET /files/{uploadId}/sheet-preview` returns. + * + * Mirrors `SheetPreview` / `SheetPreviewResponse` in + * `apis/shared/files/models.py`, which serialise with camelCase aliases. + * Every cell is a string the server already formatted for display — + * dates, floats and booleans included — so the grid never has to decide + * how a value should read, and the two readers behind it (openpyxl here, + * the browser's own parser for `.csv`) cannot drift apart on it. + */ +export interface SheetPreview { + /** Worksheet name, as it appears on the tab in Excel. */ + name: string; + /** First row of the sheet, used as column labels. */ + headers: string[]; + /** Body rows, each padded to `headers.length`. */ + rows: string[][]; + /** Body rows the sheet claims to have, which may exceed `rows.length`. */ + totalRows: number; + /** True when a cap stopped the read short of the sheet's end. */ + truncated: boolean; + /** Which cap fired. */ + truncatedBy: 'rows' | 'columns' | null; +} + +export interface SheetPreviewResponse { + uploadId: string; + filename: string; + /** Visible worksheets, in workbook order. Hidden sheets are omitted. */ + sheets: SheetPreview[]; + truncated: boolean; +}