- ` grouping → `^\d+\. `→`
- `. Three verified gaps: **no link transform exists at all** (`renderMarkdown("[a](https://x)")` returns the text verbatim inside a `
`), **no image transform exists at all** (`` prints literally), and the heading ladder stops at `###` — `####`/`#####`/`######` match no rule and print as plain text (each rule requires a space after its hashes, so no rule partially eats a deeper level). `frontend/assets/styles.css` — the `:root` palette (`--ink`, `--ink-soft`, `--line`, `--surface`, `--bg`, the `--brand-*` family, `--radius*`) plus the precedent rule families: `.md-table`/`.md-table-wrap` (L539-557), `.md-quote` + the `.msg.user .bubble .md-quote` inversion (L561-588), `.source-image-img` / `.doc-image-img` (L981-1027, L5611-5640) for image treatment. `app/core/security_headers.py::CSP` (L67-68) — `default-src 'self'; base-uri 'none'; frame-ancestors 'none'; img-src 'self' data:`: served-path and `data:` images are ALREADY permitted, no CSP change is needed or allowed here. Test patterns: `tests/unit/test_markdown_blockquotes.py` (source pins + a `HAVE_NODE`-skipped node behaviour harness + a byte-identity fixture string) and `tests/e2e/test_markdown_blockquotes.py` (dedicated fixture tree `tests/fixtures/quote_docs/`, mock LLM trigger, chat + viewer + localStorage-restore assertions). + +## Objective +`[text](url)` renders as a real, safe ``, `` renders as a lazy `
`, and `####`–`######` render as `
`/`
` — in chat answers, the thinking block, saved/shared chats, the document viewer and the document modal, from the ONE shared renderer. Every input that contains none of the three constructs renders byte-identical to today, so the phase-44 table suite, the phase-138 blockquote suite and every renderer-dependent unit suite stay green unedited. + +## Design (shared by all tasks — the executor reads this, not the chat) +- **D1 — one chain, one place (tasks 01–03).** All three fixes are edits to the same `inline(s)` chain in `frontend/assets/markdown.js`. Nothing else in the pipeline moves: fences (step 1) and tables (step 1b) keep their precedence, and because a table cell goes through `inline(escapeHtml(cell))`, links/images/deep headings inside a cell work for free (pinned in task 05). +- **D2 — escape-first is the security boundary, so NEVER unescape.** `inline()` always receives HTML-escaped text. The captured URL may therefore contain `&`/`'`/`"` — that is *valid* in an `href`/`src` and must be passed through verbatim (never decoded, never re-escaped). Any attempt to decode would re-open the injection surface the renderer exists to close. +- **D3 — links (task 01).** `/\[([^\]]*)\]\(([^)\s]+)\)/g` (empty link text allowed, no whitespace inside the URL), applied AFTER the `*em*` rule and BEFORE the heading ladder. The URL is validated by an allowlist *function*, not a regex-in-replacement: allow `^https?://`, `^mailto:`, `^/`, `^#`, `^\.{1,2}/`; anything else (`javascript:`, `data:`, `vbscript:`, `file:`, bare `javascript:` variants) returns the ORIGINAL matched text unchanged — it stays visible as inert escaped text, never a live element. Absolute `http(s)://` links get `target="_blank" rel="noopener noreferrer"`; `mailto:`, root-relative and `#fragment` links get neither (a same-origin target in the same tab is the correct behaviour). Output marker: ``. +- **D4 — images (task 02), matched BEFORE the link rule.** `/!\[([^\]]*)\]\(([^)\s]+)\)/g` → `
`. The image rule runs FIRST so the `!` is never orphaned into `!`. Same allowlist discipline: `/`-rooted served paths, `http(s)://` and `data:image/*` render; anything else stays inert text. `alt` is ALWAYS emitted (empty allowed) — an `
` without `alt` fails WCAG. Linked images (`[](url)`) get their OWN rule ahead of both (locked A6): `
` — otherwise the image rule leaves a stray `[` inside the link text. +- **D5 — the heading ladder (task 03, locked A4).** Insert `/^#### (.*)$/gm →
$1
` and `/^#{5,6} (.*)$/gm →$1
` immediately BEFORE the existing `/^### /` rule (deeper-first is the readable order; the rules cannot cross-match because every rule requires a space after its hashes). `#` and `##` keep mapping to `` — existing behaviour, deliberately unchanged. The step-3 guard already matches `h\d`, so no paragraph-guard change is needed (pinned, not assumed). +- **D6 — CSS (task 04).** `.md-link` (underlined; `color: var(--brand-ink)` — 9.0:1 on `--surface`, the file's documented AA-safe brand text token — with `--brand-hover` on `:hover`, the shared 3px `--brand` `:focus-visible` ring already used at L163, and the `.msg.user .bubble .md-link { color: var(--bg) }` inversion following the `.md-quote` precedent at L588, 5.2:1 on the brand bubble), `.md-img` (`max-width:100%; height:auto; object-fit:contain` + the `.source-image-img` border/radius/surface language), and the `.doc-md` / `.bubble` / `.thinking-text` `h5`/`h6` type scale. Palette variables ONLY, no new hue (the monochrome invariant), every non-obvious color carrying its `/* n.n:1 on --x (AA) */` comment. +- **D7 — byte-identity is the regression contract (task 05).** A checked-in fixture string with no links, no image refs and no `####`+ heading, plus its byte-exact pre-phase renderer output (the phase-138 `NO_QUOTE_INPUT`/`NO_QUOTE_PRE_PHASE_OUTPUT` shape), pinned in the node behaviour harness. +- **Known, documented limits (recorded in the module docstring, not fixed):** inline code spans are NOT placeholder-protected (pre-existing: `**bold**` inside backticks already transforms), so markdown syntax inside an inline code span transforms too; a URL containing a literal `)` or whitespace is a miss; a size-less SVG (`viewBox` only, no `width`/`height`) in `.md-img` paints at the replaced-element default box — the vector sizing contract belongs to phase 181. +- **NOT touched:** `app/**` (no server change in this phase), the CSP string, `document.js`'s `renderDocument` structure, `app.js`'s chip/figure builders, `styles.css` families outside `.md-*` + the `h5`/`h6` scale. + +## Dependencies +- `179_git_clone_server_freeze` (todo) — numbering/pipeline order only. The code dependency is the shared renderer at the head of `complete/` (phases 44 tables, 138 blockquotes). + +## Tasks +1. `01_inline_links.md` — the `[text](url)` → `` transform with the protocol allowlist in `markdown.js`. +2. `02_inline_images.md` — the `` → `
` transform (ahead of the link rule) + the linked-image rule. +3. `03_heading_ladder.md` — `####` → `
`, `#####`/`######` → `
` in the ladder. +4. `04_styles_markdown_inline.md` — `.md-link`, `.md-img` and the `h5`/`h6` type scale in `styles.css`. +5. `05_unit_and_e2e.md` — `tests/unit/test_markdown_inline_links_images.py` + the phase's dedicated Playwright suite `tests/e2e/test_markdown_inline_links_images.py`, the full gates, the atomic commit. + +## Testing & Quality +- **Unit:** `tests/unit/test_markdown_inline_links_images.py` (new, the phase-138 pattern): source pins (the chain order code → strong → em → **image → linked-image → link** → headings → lists; the allowlist function; the exact `md-link`/`md-img` markers; no `unescape`/`decodeURIComponent` anywhere in the file; the `.md-link`/`.md-img` CSS rules reference palette vars only) + node behaviour (link renders with `target`/`rel`; `javascript:`/`data:` inert; relative + `#fragment` + `mailto:` link without `target`; `&` in a query string survives verbatim; image renders with `alt` (empty alt allowed), disallowed src inert; linked image wraps without a stray `[`; `####`→`
`, `#####`/`######`→`
`, `###`→`
` unchanged; a link inside a fence stays code; a link/image inside a table cell renders; byte-identity fixture). +- **E2E (AGENTS rule 4):** `tests/e2e/test_markdown_inline_links_images.py`, run **in isolation** (`uv run pytest tests/e2e/test_markdown_inline_links_images.py -v --no-cov`, DB up). +- **Coverage:** **>90%** on `app/` (`uv run pytest --cov=app --cov-report=term-missing`) — this phase adds no `app/` code, so the full suite staying green IS the gate. +- **Lint/types:** `uv run ruff check . && uv run pyright` clean. + +## Completion Criteria +- [ ] `[text](url)` renders as `` in a chat answer, the thinking block, a restored shared chat, the viewer and the document modal; `javascript:`/`data:` URLs stay visible as inert text +- [ ] `` renders as `
` with an `alt` present; a non-allowlisted src stays inert text +- [ ] `####` renders `
`, `#####`/`######` render `
`; `#`/`##`/`###` are unchanged +- [ ] a link/image/`####` inside a fenced code block is not transformed; a cell-level link renders in a `.md-table` +- [ ] an input with none of the three constructs renders byte-identical (phase-44 + phase-138 suites pass unedited) +- [ ] `uv run pytest` green; coverage TOTAL >90%; the phase's E2E green in isolation; `uv run ruff check . && uv run pyright` clean +- [ ] one `--no-gpg-sign` Conventional Commit; phase dir moved to `.agents/phases/complete/` by the pipeline gate + +## Locked decisions +- **A4 — `####` → `
`; `#####` and `######` both → `
` and `##### x` / `###### x` as ``; `#`/`##` keep rendering as `
` (owner-confirmed 2026-10-14, roadmap confirmation).** +- **A5 — link/image URL allowlist = `http:`, `https:`, `mailto:` (links), `/`-rooted + `./`/`../` relative paths, `#fragment`, and `data:image/*` (images). Everything else renders as inert text, never a live element (owner-confirmed 2026-10-14, roadmap confirmation).** +- **A6 — `[](url)` renders as a link wrapping the image (an explicit rule ahead of the two others), not as text with a stray bracket (assistant decision, surfaced here so the executor does not re-decide it).** + +## Commit +```bash +git add frontend/ tests/ .agents/phases/ && git commit --no-gpg-sign -m "feat(ui): render markdown links, inline images, and h5/h6 headings in the shared renderer" +``` diff --git a/.agents/phases/todo/180_markdown_inline_links_images/01_inline_links.md b/.agents/phases/todo/180_markdown_inline_links_images/01_inline_links.md new file mode 100644 index 0000000..edb1329 --- /dev/null +++ b/.agents/phases/todo/180_markdown_inline_links_images/01_inline_links.md @@ -0,0 +1,27 @@ +# Task 01 — The link transform in the shared renderer + +**Phase:** `180_markdown_inline_links_images` · **Source:** `TODO.md:4` — "Markdown [links](some-address) aren't being render correctly and show up as plain text" +**Story:** n/a + +## Objective +`renderMarkdown` turns `[text](url)` into `text` in the shared `inline()` chain — through an explicit URL allowlist, escape-first, with every existing transform's output byte-identical. + +## Work +1. `frontend/assets/markdown.js` — in `const inline = (s) => …`, insert one link `.replace()` **after** the `*em*` rule and **before** the `^### ` heading rule: `/(\[)([^\]]*)(\]\()([^)\s]+)(\))/g` written plainly as `/\[([^\]]*)\]\(([^)\s]+)\)/g` (empty link text allowed, no whitespace in the URL). The replacement is a FUNCTION, not a string, so the URL passes the allowlist check: + - `allowLinkUrl(url)` — returns the url for `^https?://`, `^mailto:`, `/`-rooted (`^/`), `^\.{1,2}/` relative, and `^#` fragment URLs; returns `null` for anything else (`javascript:`, `data:`, `vbscript:`, `file:`, `blob:`) — LOCKED A5; + - allowed → `text`; the `target`/`rel` pair is added ONLY for `^https?://` (a `mailto:`/relative/fragment link stays in the same tab); + - not allowed → return the ORIGINAL matched substring untouched, so it renders as the inert escaped text it is today (never a live element, never dropped). +2. `frontend/assets/markdown.js` — pass the captured URL through VERBATIM: the chain's input is already `escapeHtml`-ed, so `&`/`'`/`"` in a query string are already attribute-safe. No `decodeURIComponent`, no manual unescape, no re-escape (design D2 — this is the security boundary). +3. `frontend/assets/markdown.js` — module docstring: add the phase-180 paragraph (the chain order is now code → strong → em → **linked-image → image → link** (tasks 02 adds the first two ahead of this one) → headings → lists), the allowlist (LOCKED A5), the `target`/`rel` rule, and the documented limits (a URL containing a literal `)` or a space is a miss; inline code spans are not placeholder-protected, so markdown syntax inside `` `…` `` transforms — the pre-existing `**strong**` behaviour, unchanged here). +- ASSUMPTION: link text may itself contain inline markup (`**bold**`, `*em*`, `` `code` ``) — those rules run FIRST in the chain, so the text is already transformed when the link rule wraps it; no extra handling. +- ASSUMPTION: autolinks (`
` (LOCKED A6 — without this rule the image rule consumes the inner `` and the link rule then leaves a stray `[` in the link text). Both URLs are allowlist-checked (`allowLinkUrl` for the href, `allowImageUrl` for the src); either failing → the original text is returned untouched. + 2. **image second:** `/!\[([^\]]*)\]\(([^)\s]+)\)/g` → `
/image`, `/assets/…`), `^.{0,2}/` relative paths, `^https?://` and `^data:image/` — the `data:` case exists because the app's own CSP (`app/core/security_headers.py::CSP`, `img-src 'self' data:`) already permits `data:` images; everything else (`javascript:`, `file:`, non-image `data:`) returns `null` → the original escaped text stays visible and inert (LOCKED A5). +3. `frontend/assets/markdown.js` — `alt` is ALWAYS emitted, escaped-passthrough like the URL, empty allowed (`alt=""` for a decorative image) — an ` ` with no `alt` attribute fails WCAG 1.1. +4. `frontend/assets/markdown.js` — docstring: record the final chain order (code → strong → em → **linked-image → image → link** → headings → lists), the image allowlist, and the documented limit that a size-less SVG (`viewBox` only) in `.md-img` paints at the replaced-element default box (vector sizing is phase 181's `04_svg_preview_render.md` contract). +- ASSUMPTION: no `onerror` handler is added — the renderer emits an HTML STRING into `innerHTML`-style surfaces and the no-CDN/CSP posture forbids inline handlers; a broken src degrades to the browser's alt text (the house pattern for the shared renderer). The JS `onerror` fallbacks live in `document.js`/`app.js`, which do not route through this function. +- ASSUMPTION: title syntax (``) is a miss (the URL capture forbids whitespace) — out of scope. + +## Testing & Quality +- Unit: pinned by task 05 (chain order pins the two image rules before the link rule; markers `
` wrapping `
` with no stray bracket, empty alt allowed, an image inside a table cell renders). +- Coverage: n/a (frontend). + +## Completion Criteria +- [ ] node repro: `renderMarkdown('')` → `
` +- [ ] node repro: `renderMarkdown('[](https://example.com)')` → one `xx
`; `#`, `##` and `###` keep their existing output exactly. + +## Work +1. `frontend/assets/markdown.js` — in the `inline()` chain, insert TWO rules immediately BEFORE the existing `/^### (.*)$/gm → "$1
"` rule (deeper-first, so the ladder reads top-down by depth): + - `/^#### (.*)$/gm` → `"$1
"` + - `/^#{5,6} (.*)$/gm` → `"$1
"` (LOCKED A4: both `#####` and `######` map to `` — the app's type scale stops at two levels below `###`, no `
` exists in HTML) + The rules cannot cross-match each other or the existing ones because every heading rule requires a space after its hash run (`####` never matches `/^### /`). +2. `frontend/assets/markdown.js` — leave `/^## /` and `/^# /` mapping to ` ` UNTOUCHED (LOCKED A4 — the existing flattening of `#`/`##` to `
` is deliberate; the phase adds levels below `###`, it does not re-rank the existing ones). +3. `frontend/assets/markdown.js` — the step-3 paragraph guard `/^<(h\d|ul|ol|pre|li|blockquote)/` already matches `h5`/`h6`, so NO change is needed there: pin it with a unit assertion rather than editing it. +4. `frontend/assets/markdown.js` — docstring: record the ladder (`#`/`##` → `h3`, `###` → `h4`, `####` → `h5`, `#####`/`######` → `h6`) so the deliberate flattening is documented, not re-discovered. +- ASSUMPTION: a heading line must start at column 0 of its line (the `m` flag, the existing rule shape) — an indented `#### x` stays plain text, matching `^### ` today. + +## Testing & Quality +- Unit: pinned by task 05 (source pins: the two new rules sit before `/^### /`; the guard regex unchanged; node behaviour: `#### A` → `
A
`, `##### A`/`###### A` → `A
`, `### A` → `A
` unchanged, `## A`/`# A` → `` unchanged, a `####` line inside a fence stays code, a `####` heading immediately followed by a paragraph is not wrapped in `
`). +- Coverage: n/a (frontend). + +## Completion Criteria +- [ ] node repro: `renderMarkdown("#### Deep")` → `
Deep
` (no `` wrapper, no literal `####`) +- [ ] node repro: `renderMarkdown("##### Deeper\n###### Deepest")` → two `
` elements +- [ ] node repro: a `####` line inside a ``` fence renders inside `
` unchanged +- [ ] existing heading output byte-identical; `uv run pytest` green (phase-44/138 suites unedited) diff --git a/.agents/phases/todo/180_markdown_inline_links_images/04_styles_markdown_inline.md b/.agents/phases/todo/180_markdown_inline_links_images/04_styles_markdown_inline.md new file mode 100644 index 0000000..48aaae7 --- /dev/null +++ b/.agents/phases/todo/180_markdown_inline_links_images/04_styles_markdown_inline.md @@ -0,0 +1,26 @@ +# Task 04 — Styling for `.md-link`, `.md-img` and the `h5`/`h6` type scale + +**Phase:** `180_markdown_inline_links_images` · **Source:** `TODO.md:4–5` (link + heading rendering), `TODO.md:3` (inline image rendering) +**Story:** n/a + +## Objective +The three new elements are legible and theme-correct in every renderer context (chat bubble — brain and user, the thinking block, the shared page, the viewer, the document modal), built from the existing `:root` palette only. + +## Work +1. `frontend/assets/styles.css` — `.md-link` next to the `.md-quote` family (L561-588): `color: var(--brand-ink)` (the file's documented AA-safe brand text token, 9.0:1 on `--surface`), `text-decoration: underline` (a link must not rely on colour alone — WCAG 1.4.1), `text-underline-offset: 2px`, `:hover { color: var(--brand-hover) }`. The global `:focus-visible { outline: 3px solid var(--brand) }` (L162-166) already covers the link — do NOT add a competing focus rule, and do not reset the outline. +2. `frontend/assets/styles.css` — `.msg.user .bubble .md-link { color: var(--bg); }` — the exact inversion the `.md-quote` precedent applies at L588 (the user bubble is a `--brand` fill: `--bg` on `--brand` is 5.2:1, AA). Add the same inversion for `:hover` if the default `--brand-hover` fails contrast on the brand fill — a colour-mix toward `--bg` is acceptable, a new hue is not. +3. `frontend/assets/styles.css` — `.md-img`: `display: inline-block; max-width: 100%; height: auto; object-fit: contain;` plus the `.source-image-img` card language (`background: var(--surface)`, `border: 1px solid var(--line)`, `border-radius: var(--radius-sm)`) and `margin: 0.4rem 0`. No fixed width/height — an oversized screenshot must never widen the page. +4. `frontend/assets/styles.css` — the deep-heading type scale, ONE rule set for all renderer contexts (no chat-specific duplication): `.doc-md h5, .msg .bubble h5, .thinking-text h5 { margin: 0.8rem 0 0.3rem; font-size: 0.95rem; }` and the `h6` sibling at `0.88rem`, following the existing `.doc-md h3` / `.doc-md h4` rhythm (L5282-5283) — the scale stays monotonic (`h3` 1.15 → `h4` 1.0 → `h5` 0.95 → `h6` 0.88) and every level keeps a visible size difference from body text. +5. `frontend/assets/styles.css` — comment block above each rule in the house style: the phase number, the TODO source line, the contrast math for every non-obvious colour, and the "static content, no animation" note (the background-motion invariant). +- ASSUMPTION: `h5`/`h6` in the chat bubble get explicit sizing (they inherit UA defaults today, which are larger than `0.95rem`) so a deep heading in an answer cannot out-shout the answer text. +- ASSUMPTION: no `.md-img--vector` modifier here — vector (viewBox-only SVG) sizing belongs to `181_svg_document_preview/04_svg_preview_render.md`, which knows the document's format. + +## Testing & Quality +- Unit: pinned by task 05 — the `.md-link`, `.md-img`, `h5`/`h6` rules exist and reference ONLY `var(--…)` palette tokens (no hex literals, no `rgb(`) — the established `:root`-drift pin pattern used by the blockquote/table CSS tests; the `.msg.user .bubble .md-link` inversion is present. +- Coverage: n/a (frontend). + +## Completion Criteria +- [ ] a link in a brain bubble, a user bubble, the thinking block, the shared page, the viewer and the document modal is underlined and passes ≥4.5:1 against its surface +- [ ] Tab focus on a link shows the app's 3px `--brand` ring +- [ ] a wide image in an answer does not produce a horizontal scrollbar on the chat page at 375px and at 1440px +- [ ] `uv run pytest` green (the CSS-pinning unit suites unedited) diff --git a/.agents/phases/todo/180_markdown_inline_links_images/05_unit_and_e2e.md b/.agents/phases/todo/180_markdown_inline_links_images/05_unit_and_e2e.md new file mode 100644 index 0000000..e16d5ca --- /dev/null +++ b/.agents/phases/todo/180_markdown_inline_links_images/05_unit_and_e2e.md @@ -0,0 +1,36 @@ +# Task 05 — Unit pins, the dedicated Playwright suite, the gates, the commit + +**Phase:** `180_markdown_inline_links_images` · **Source:** `TODO.md:3–5` (the acceptance evidence for tasks 01–04) +**Story:** n/a + +## Objective +Lock the three renderer fixes with a source-pin + node-behaviour unit file and the phase's own Playwright E2E suite (AGENTS rules 4/9), run every gate, and land the single atomic commit. + +## Work +1. `tests/unit/test_markdown_inline_links_images.py` (new — the `tests/unit/test_markdown_blockquotes.py` shape, copy its `HAVE_NODE = shutil.which("node") is not None` skip and its `subprocess` node harness): + - **source pins on `frontend/assets/markdown.js`:** the chain order is code → strong → em → linked-image → image → link → `^#### ` → `^#{5,6} ` → `^### ` → `^## ` → `^# ` → lists (assert the index of each rule's literal in the file is strictly increasing); the exact markers ``, ``; `allowLinkUrl` / `allowImageUrl` named functions exist; the file contains NO `decodeURIComponent` / `unescape` / `innerHTML`; the step-3 guard regex is still exactly `/^<(h\d|ul|ol|pre|li|blockquote)/`. + - **CSS pins on `frontend/assets/styles.css`:** `.md-link`, `.md-img`, the `h5` and `h6` rules exist and their colour values reference `var(--…)` only (no hex, no `rgb(`); `.msg.user .bubble .md-link` exists (the inversion). + - **node behaviour (skipped without node):** every repro listed in tasks 01–04's Completion Criteria, PLUS the byte-identity contract — a checked-in `NO_INLINE_INPUT` / `NO_INLINE_PRE_PHASE_OUTPUT` fixture pair (a paragraph + a list + a pipe table + a fence + a blockquote, i.e. everything the renderer already supported) asserted byte-equal. +2. `tests/fixtures/links_docs/` (new dedicated fixture tree — the phase-138 `quote_docs` precedent: a NEW tree, never the shared 13-doc fixture set, whose import count many suites pin). One document `homelab/inline-markdown.md` whose body carries: a `[absolute link](https://example.com/a?x=1&y=2)`, a `[relative link](/sources)`, a `mailto:` link, a `#anchor`, an inert `[oops](javascript:alert(1))` probe, `` (a tiny inline SVG — CSP `img-src 'self' data:` already allows it, no bytes fixture needed), an inert `)`, a `[](https://example.com)`, and an `#### Deep header` + `##### Deeper` line, plus a pipe table with a `[cell link](https://example.com)` inside a cell. +3. `tests/e2e/test_markdown_inline_links_images.py` (new — the phase-138 E2E skeleton verbatim in structure: module-level fixture constants, `_import_fixtures` + `_run_in_thread` + `_reset_db` (truncate `chunks, documents, query_log, steering_notes, kb_overview` then re-import, `summary.added == 1`), `login(page, app_url, next="/")`, the mock LLM trigger answering with the byte-stable inline-markdown answer): + - `test_chat_inline_markdown_renders` — the brain bubble contains `a.md-link` elements for the absolute (with `target="_blank"`, `rel` containing `noopener noreferrer`, `href` preserving `&`), relative and `mailto:` links; NO `a.md-link` for the `javascript:` probe (its text stays visible); `img.md-img` for the data-URL image with a non-empty `alt` and `loading="lazy"`; the linked image is an `a.md-link > img.md-img` with no stray bracket in its text; `h5`/`h6` elements exist; `table.md-table a.md-link` exists (cell-level transform); the turn is NOT deflected. + - `test_viewer_inline_markdown_renders` — the fixture document opens from the Sources page in the document modal (admin, `openDocumentModal`) and renders the same `a.md-link` / `img.md-img` / `h5` / `h6` shapes (the shared renderer contract across surfaces). + - `test_restore_keeps_inline_markdown` — reload the chat page: the localStorage restore path re-runs `renderMarkdown` and the restored brain bubble keeps its links, image and headings. + - `test_no_horizontal_overflow_at_mobile` — at `viewport={"width": 375, "height": 720}`, `document.documentElement.scrollWidth <= clientWidth` with the wide image rendered (the `.md-img` `max-width: 100%` contract). +4. Run the gates, in order, and fix what fails (do not baseline, do not skip): + - `uv run pytest` + - `uv run pytest --cov=app --cov-report=term-missing` → TOTAL >90% + - `uv run pytest tests/e2e/test_markdown_inline_links_images.py -v --no-cov` (in isolation, DB up: `podman compose up -d db`) + - `uv run ruff check . && uv run pyright` +5. Move the phase directory to `.agents/phases/complete/` and commit — see the overview's Commit block. + +## Testing & Quality +- This task IS the phase's test gate. The unit file must fail if any of tasks 01–03's rules are reordered or removed, and must fail if task 04's CSS introduces a hard-coded colour. +- Coverage: **>90%** on `app/` (no `app/` code changes in this phase — the number must not move). + +## Completion Criteria +- [ ] `uv run pytest` green, including the new unit file (and green with node absent — the skip path) +- [ ] `uv run pytest --cov=app --cov-report=term-missing` TOTAL >90% +- [ ] `uv run pytest tests/e2e/test_markdown_inline_links_images.py -v --no-cov` green in isolation +- [ ] `uv run ruff check . && uv run pyright` clean +- [ ] one atomic commit, `--no-gpg-sign`: `feat(ui): render markdown links, inline images, and h5/h6 headings in the shared renderer`, containing `frontend/`, `tests/` and `.agents/` diff --git a/.agents/phases/todo/181_svg_document_preview/00_phase.md b/.agents/phases/todo/181_svg_document_preview/00_phase.md new file mode 100644 index 0000000..03c8e1b --- /dev/null +++ b/.agents/phases/todo/181_svg_document_preview/00_phase.md @@ -0,0 +1,56 @@ +# Phase 181 — SVG documents render as pictures, not as XML + +**Source:** `TODO.md` L3 — "Need SVG render support on the front end when citing and svg source. Also would be nice to click an svg and see a preview in the RAG tab rather than the xml" +**Story:** n/a (TODO-derived phase, Protocol B append — owner roadmap confirmation 2026-10-14) +**Context:** **The root cause is ingestion, not the viewer.** `app/config.py::image_extensions` (L436) defaults to `png,jpg,jpeg,webp,gif,bmp` and `.env` mirrors it (`BOR_IMAGE_EXTENSIONS`), so an `.svg` can only enter the KB through `BOR_IMPORT_EXTENSIONS` — i.e. as a **text** document whose `content` is the raw XML, which `frontend/assets/document.js::renderDocument` then paints in `
`. That is exactly the "xml instead of a preview" the owner sees. The rest of the image machinery is already there and needs no change: `app/rag/retriever.py::source_ref_with_image` (L1010-1022) puts `image_url = /api/documents/
/image` on a cited image doc, `app/schemas.py::SourceRef.image_url` (L154-181) carries it (omitted when `None`), `app/api/docs.py::get_document_content` (L219-318) returns `is_image` + `image_url` + `format`, `app/api/docs.py::get_document_image` (L319-378) serves the stored bytes with `IMAGE_MIMES.get(suffix, IMAGE_FALLBACK_MIME)`, `frontend/assets/app.js::appendSources` (L2386+) renders a `.source-image` figure per `image_url` ref, `frontend/assets/document.js` renders the `.doc-image` ` ` block for `doc.is_image` (L453-470, L560+), `frontend/assets/sources.js` (L2328) renders the KB-tree thumbnail. `app/rag/importer.py::_index_file` already prefers the image branch over the text path (L1417-1425) and `_index_image_file` (L3532+) owns bytes-digest → `_markdown_image_context` (phase 125) → `_describe_or_skip` → copy to `image_dir/
` → `is_image`/`image_path`/`content`/`summary`. `app/rag/summarizer.py::IMAGE_MIMES` (L152-159, dotted keys) is the one map for both the vision data-URL mime and the route's `Content-Type`; `describe_image` is a CHAT-model vision call (phase 122 LOCKED A3). `app/core/security_headers.py::CSP` (L67-68) + its middleware (which PRESERVES a CSP an inner layer set — L109-110) is the injection boundary. `app/rag/md_images.py::extract_image_refs` (the phase-125 `` scanner) already treats an `.svg` path like any other image. Test patterns: `tests/unit/test_image_documents.py` (toggle / ingest / describe-seam), `tests/integration/test_docs_api.py` (the image route), `tests/e2e/test_image_documents.py` + `tests/e2e/test_answer_images.py` (image E2E), `tests/unit/test_markdown_blockquotes.py` (source-pin + node harness). + +## Objective +An `.svg` in a source tree becomes an **image document**: its own text (` `/`<desc>`/`<text>`) is what gets indexed and retrieved, its bytes are served as `image/svg+xml` under a sandboxed CSP, and every citation surface — the chat figure, the document modal, the `/document.html` viewer, the Sources thumbnail — paints the rendered vector graphic instead of XML. Plus the phase-180 renderer, an SVG referenced as `` inside an answer or a document renders as a real image. + +## Design (shared by all tasks — the executor reads this, not the chat) +- **D1 — SVG is a *vector* image document; the vision call is bypassed (tasks 01–02, locked A1).** OpenAI-compatible vision endpoints do not accept `image/svg+xml`, and adding a rasteriser dependency is out of scope. So an `.svg` takes the phase-122 image branch but its indexed text comes from the file itself: a new pure module `app/rag/svg_text.py` (the `app/rag/md_images.py` precedent — stdlib only, no I/O, fully unit-testable) returns the SVG's own words. `content`/`summary` follow the phase-122 image convention (`summary = content`) and the normal chunk pipeline embeds them; the phase-125 `image_context` (the `` paragraphs around a reference to the file) still prepends, unchanged. +- **D2 — the text of a diagram is its `<title>`, `<desc>` and `<text>` (task 01).** Matched by LOCAL name (namespace-agnostic: plain `<title>`, `svg:title`, and namespaced variants all count), in DOCUMENT order, whitespace-normalised, de-duplicated case-insensitively, joined with `\n`. Parsing is `xml.etree.ElementTree` — never a regex over untrusted markup — guarded against the billion-laughs/XE class by (i) refusing any input containing a `<!DOCTYPE` or `<!ENTITY` declaration (→ extraction failure, no parse attempt) and (ii) a `MAX_SVG_BYTES = 2 MiB` input cap (over-cap → extraction failure). XML entity refs inside text (`&`) are decoded by the parser, which is the correct direction (the value is re-escaped on the way out by the renderer). +- **D3 — a textless SVG still gets honest, retrievable text (task 01, locked A2).** Many real diagrams carry no words at all (pure `<path>`s). Skipping them (the phase-122 "undescribed image is unsearchable noise" posture) would make them invisible and never citable — the opposite of what the owner asked. So `extract_svg_text` falls back to a structural line built ONLY from facts in the file: `Vector diagram "<file stem>" — <N> shapes (<the three most common element local names with counts>), viewBox <w>×<h>` (each clause omitted when the file does not state it, the stem always present). No invented prose, no model call. +- **D4 — ingestion mechanics (task 02).** `image_extensions` default gains `svg` (`.env.example` documents it; a live `.env` keeps its own value — the phase does not edit `.env`). `app/rag/importer.py::_index_image_file` branches on `full_path.suffix.lower() == ".svg"`: vector files take `extract_svg_text` INSTEAD of `_describe_or_skip`; everything else (copy to `image_dir`, digest rules, `is_image`/`image_path`, title = file stem, counters, the `on_chunk` hook) is unchanged and shared. Fail-soft stays LOCKED A3-shaped: extraction returning `None` (over-cap, DOCTYPE-bearing, or unparseable XML) → `summary.images_failed += 1`, a warning, no row, no orphan copy. **One extra guard is required for existing libraries:** `_index_file`'s fast path (L1403, `doc.file_hash == file_digest`) returns BEFORE the image branch, so a row that was indexed as TEXT when `svg` lived only in `BOR_IMPORT_EXTENSIONS` would stay a text doc forever even after the switch. Add a narrow mismatch guard — take the fast path only when the file's kind matches the row's (`settings.images` ∧ the extension is in `image_extension_set` ∧ `doc.is_image is False` ⇒ fall through to the image branch). Everything else keeps the phase-147 fast path byte-for-byte. +- **D5 — serving the bytes safely (task 03).** `IMAGE_MIMES[".svg"] = "image/svg+xml"` in `app/rag/summarizer.py` (one map, one truth — the vector branch never reaches `image_data_url`, so the vision path is unaffected). `get_document_image` keeps its public, id-as-credential posture (phase 124 LOCKED A4 — a shared chat shows the figure to a guest) and its `Cache-Control: private, max-age=3600`, and adds ONE response header for `image/svg+xml` only: `Content-Security-Policy: default-src 'none'; sandbox` (the `SecurityHeadersMiddleware` preserves an inner CSP, so the route's value wins) — an `<img>` can never execute SVG script, but a user who opens the URL directly must not get a script-capable same-origin document either. `X-Content-Type-Options: nosniff` is already global. +- **D6 — the frontend (task 04).** The modal, the viewer, the citation figure and the Sources thumbnail already key off `is_image`/`image_url`, so they get the SVG for free — the task is the sizing + honesty work: `.doc-image-img--vector` (applied in `document.js` where `doc.format === "svg"`) with `width: 100%` so a `viewBox`-only SVG (no intrinsic size, the CSS default replaced-element box is 300×150) fills the `.doc-image` card instead of sitting small in it; the onerror `.doc-image-unavailable` fallback stays as-is; a legacy SVG row still stored as TEXT keeps rendering its XML in `.doc-raw` (honest — there are no bytes behind it) until the next sync converts it. +- **D7 — the chat figure is a 96px-capped thumbnail** (`.source-image-img`, `max-height: 96px; max-width: 100%; height: auto`), so a size-less SVG inside it stays inside the cap; `SourceRef` carries no format field and NONE is added (the wire surface stays byte-identical for every text doc — phase-83/122 contract). Clicking the figure/chip opens the modal, which is the full-size preview (`doc.format` known there). Documented in the module comments. +- **NOT touched:** the CSP constant in `security_headers.py`, `app/api/chat_images.py::_CHAT_IMAGE_FILENAME_RE` (question attachments are raster-only by design — a vector question image is out of scope), any rasteriser dependency, `describe_image`'s prompt, the phase-124 answer-image path, the `SourceRef` schema, `.env` (user-owned), the PDF embedded-image pass (`pdf.py` — PDFs never embed SVG for this app's extractor). + +## Dependencies +- `180_markdown_inline_links_images` (todo) — its `` transform is what makes `` inside an answer or a document render; this phase supplies the route that answers that URL. No code-level dependency beyond that (the ingestion, the route and the viewer work without it). + +## Tasks +1. `01_svg_text_extraction.md` — the new pure `app/rag/svg_text.py` module: `<title>`/`<desc>`/`<text>` extraction with the XE/size guards and the structural fallback. +2. `02_svg_index_as_image.md` — `svg` in `image_extensions`, the vector branch in `_index_image_file`, and the fast-path kind-mismatch guard that converts existing text-SVG rows. +3. `03_svg_bytes_route_csp.md` — `IMAGE_MIMES[".svg"]` and the sandboxed `image/svg+xml` response from `GET /api/documents/{id}/image`. +4. `04_svg_preview_render.md` — the vector sizing class + CSS in the viewer, and the figure/modal/thumbnail verification (the "preview instead of XML" contract). +5. `05_unit_integration_e2e.md` — `tests/unit/test_svg_text_extraction.py`, `tests/integration/test_svg_document_image.py`, the phase's dedicated Playwright suite `tests/e2e/test_svg_document_preview.py`, the gates, the atomic commit. + +## Testing & Quality +- **Unit:** `tests/unit/test_svg_text_extraction.py` (new, pure-function, no DB): title+desc+text in document order, namespace-prefixed variants, whitespace normalisation, de-duplication, `&` decoded, the `<!DOCTYPE`/`<!ENTITY` refusal, the 2 MiB cap, malformed XML → `None`, the structural fallback for a textless SVG (stem + shape counts + viewBox, and each clause absent when unknown). Importer-level: the vector branch indexes an SVG with NO vision call (the `_describe_or_skip` seam patched to FAIL the test if called), `content == extracted text`, `summary == content`, the copy lands at `image_dir/<id>.svg`, `is_image=True`, the `images_failed` fail-soft path, and the fast-path conversion of a pre-existing text row (same bytes, `file_hash` unchanged). +- **Integration:** `tests/integration/test_svg_document_image.py` (new): `GET /api/documents/{id}/image` for an SVG row → `200`, `Content-Type: image/svg+xml`, the sandbox CSP header present, `Cache-Control: private, max-age=3600`; a raster row's response headers UNCHANGED (no CSP key added); the four 404 shapes (unknown id, unparseable id, text doc, missing file on disk) unchanged. +- **E2E (AGENTS rule 4):** `tests/e2e/test_svg_document_preview.py`, run **in isolation** (`uv run pytest tests/e2e/test_svg_document_preview.py -v --no-cov`, DB up). +- **Coverage:** **>90%** on `app/` (`uv run pytest --cov=app --cov-report=term-missing`) — `app/rag/svg_text.py` is new pure code and must be covered near-completely; the importer/route deltas are covered by the tests above. +- **Lint/types:** `uv run ruff check . && uv run pyright` clean. + +## Completion Criteria +- [ ] an `.svg` in a source tree imports as `is_image` with `image_path` set, no vision call made, and its indexed text drawn from its own `<title>`/`<desc>`/`<text>` +- [ ] `GET /api/documents/<id>/image` serves `image/svg+xml` with `Content-Security-Policy: default-src 'none'; sandbox` +- [ ] clicking an SVG citation chip/figure in the chat opens the modal showing a rendered `<img>` — no XML text anywhere in the modal body +- [ ] the `/document.html` viewer renders the SVG full-width in its `.doc-image` card (not the 300×150 default box) with the extracted text below it +- [ ] a `` reference inside a markdown document paints the graphic (the phase-180 renderer + this route — the E2E's two-stage seeding, `test_markdown_image_ref_renders`) +- [ ] every raster/PDF image behaviour is unchanged (the phase-122/123/124/133/140 suites pass unedited) +- [ ] `uv run pytest` green; coverage TOTAL >90%; the phase's E2E green in isolation; `uv run ruff check . && uv run pyright` clean +- [ ] one `--no-gpg-sign` Conventional Commit; phase dir moved to `.agents/phases/complete/` by the pipeline gate + +## Locked decisions +- **A1 — an SVG's indexed text is extracted from the file itself (`<title>`/`<desc>`/`<text>`), the vision describe call is bypassed for `.svg`; no rasteriser dependency is added (owner-confirmed 2026-10-14: "SVG as image document").** +- **A2 — a textless SVG is indexed with a structural metadata line built only from facts in the file (stem, shape counts, viewBox) rather than skipped, so it stays retrievable and previewable (assistant decision made explicit here; it relaxes the phase-122 fail-soft skip for the vector case only — a genuinely unparseable or over-cap file still skips).** +- **A3 — SVG bytes render ONLY through `<img>`, never inlined into the DOM, and the bytes route carries `default-src 'none'; sandbox` so a direct navigation cannot execute script (owner-confirmed 2026-10-14, roadmap confirmation).** +- **A7 — `svg` joins the `BOR_IMAGE_EXTENSIONS` DEFAULT; the `BOR_IMAGES` toggle remains the single knob (phase 122 LOCKED A3). With `BOR_IMAGES=0` an `.svg` stays a text document (raw XML in the viewer) and the walk never admits it as an image — no special-casing of the off state (owner-confirmed 2026-10-14, roadmap confirmation).** + +## Commit +```bash +git add app/ frontend/ tests/ .env.example .agents/phases/ && git commit --no-gpg-sign -m "feat(kb): index SVGs as image documents and preview them instead of raw XML" +``` diff --git a/.agents/phases/todo/181_svg_document_preview/01_svg_text_extraction.md b/.agents/phases/todo/181_svg_document_preview/01_svg_text_extraction.md new file mode 100644 index 0000000..3e9a73e --- /dev/null +++ b/.agents/phases/todo/181_svg_document_preview/01_svg_text_extraction.md @@ -0,0 +1,33 @@ +# Task 01 — `app/rag/svg_text.py`: the words inside a vector drawing + +**Phase:** `181_svg_document_preview` · **Source:** `TODO.md:3` — "Need SVG render support on the front end when citing … source" (the indexed-text half: an SVG document must be retrievable without a vision call) +**Story:** n/a + +## Objective +A new pure module turns SVG bytes into the text that makes the document searchable — its own `<title>`/`<desc>`/`<text>` words, or an honest structural line when it has none — with no third-party dependency, no I/O, and no network. + +## Work +1. `app/rag/svg_text.py` (new — the `app/rag/md_images.py` module shape: module docstring citing this phase + the LOCKED decisions, `from __future__ import annotations`, stdlib only, full type hints, pure functions): + - `MAX_SVG_BYTES: int = 2 * 1024 * 1024` and `MAX_SVG_TEXT_CHARS: int = 4000` (module constants, documented — the cap binds on the EXTRACTED text, never silently mid-word: truncate at the last newline/period before the cap). + - `@dataclass(frozen=True) class SvgText:` — `text: str` (the searchable text) and `structural: bool` (`True` when the fallback line was used, so the importer can log/count it). + - `def extract_svg_text(data: bytes, *, stem: str = "") -> SvgText | None`. +2. `app/rag/svg_text.py` — the guards, BEFORE any parse (LOCKED A3's sibling: untrusted XML): + - empty/whitespace-only input → `None`; + - `len(data) > MAX_SVG_BYTES` → `None` (the billion-laughs/memory guard, alongside the next one); + - the lower-cased bytes containing `<!doctype` or `<!entity` → `None` (a document type declaration is refused outright, which is what makes stdlib `ElementTree` safe here — no external entities, no entity expansion); + - `ElementTree.fromstring` raising `ParseError` → `None` (never re-raise: the caller's fail-soft is a skip, not a crash). +3. `app/rag/svg_text.py` — extraction: a `local_name(tag)` helper that strips the `{namespace}` prefix, then walk `root.iter()` (document order) and collect, for local name `title` / `desc` → the element's `.text` block (`"".join(el.itertext())` so nested markup is carried), and for `text` → `" ".join(el.itertext())` (this is what covers `<tspan>` children). Normalise each item with `" ".join(item.split())`, drop empties, de-duplicate case-insensitively keeping the FIRST occurrence, cap at `MAX_SVG_TEXT_CHARS`, and return `SvgText("\n".join(items), structural=False)` when at least one item survived (parser-decoded entities like `&` come out decoded — the renderer re-escapes on display). +4. `app/rag/svg_text.py` — the structural fallback (LOCKED A2), used only when the walk produced nothing: `Vector diagram "<stem>" — <N> shapes (<top-3 local names by count>), viewBox <w>×<h>` built ONLY from the file — `N` counts descendant elements excluding `svg`/`title`/`desc`; the `shapes` clause is omitted when `N == 0` and the noun pluralises normally (`1 shape`, `2 shapes`); the `viewBox` clause is omitted when the root has no parseable `viewBox` (four whitespace/comma-separated numbers; the last two are `w`/`h`) and when present reports `w×h` with integral values formatted without a trailing `.0`; the stem is `""` → the clause `Vector diagram` alone (no empty quotes). The em-dash form matches the app's own doc-comment typography and is the string the tests pin. +5. `app/rag/svg_text.py` — module docstring: why the vision call is bypassed (LOCKED A1: vision endpoints reject `image/svg+xml`; no rasteriser dependency), the guard list, the document-order/dedupe rules, and the fallback contract (LOCKED A2: facts from the file, never invented prose). +- ASSUMPTION: `<image>` elements embedding a raster inside the SVG are NOT followed (their `href` is not text; the byte-embedded raster belongs to the phase-151 PDF path, not here). +- ASSUMPTION: `aria-label`/`role` attributes are not extracted — `<title>`/`<desc>` are the SVG-spec carriers and the fallback covers the rest. + +## Testing & Quality +- Unit: `tests/unit/test_svg_text_extraction.py` starts here (completed in task 05) — every branch above is a pure-function test with a literal SVG string, no DB, no LLM, no filesystem: document order across `title`/`desc`/`text`, `tspan` joining, namespace-prefixed tags (`<svg:title>`, `xlink:` attributes ignored), whitespace normalisation, case-insensitive dedupe, `&` decoded, the `<!DOCTYPE`/`<!ENTITY` refusals (both a `<!DOCTYPE svg [<!ENTITY x "boom">]>` wrapper and a bare `<!ENTITY`), the over-cap refusal, malformed XML → `None`, and the fallback (shape counts + viewBox present/absent/malformed, empty stem). +- Coverage: **>90%** on this module (target: near-100% — it is pure). + +## Completion Criteria +- [ ] `uv run python -c "from app.rag.svg_text import extract_svg_text; print(extract_svg_text(b'<svg viewBox=\"0 0 8 6\"><title>Top a ', stem=\"topo\"))"` reports the two extracted items in order with `structural=False` +- [ ] a `