phase: 180_markdown_inline_links_images
All verification complete. Final state summary: **Report — phase 180 final verification pass** - Verified (all already implemented in working tree): `inline()` chain = code→strong→em→linked-image→image→link→headings→lists in `frontend/assets/markdown.js`; `allowLinkUrl`/`allowImageUrl` allowlists (A5), shared `mdAnchorOpen` (target/rel only for absolute http(s)); `####`→`<h5>`, `#####`/`######`→`<h6>`, `#`/`##`→`<h3>`/`###`→`<h4>` unchanged; `.md-link`/`.md-img`/h5-h6 scale in `styles.css` (palette vars only). All surfaces (chat, thinking, restore, viewer, modal) route through the one renderer (pinned in `app.js`/`document.js`). - Completion criteria: - `[text](url)`→`a.md-link`, `javascript:`/`data:` inert — ✅ (node probe + unit + E2E) - ``→`img.md-img loading="lazy"` with `alt`; bad src inert — ✅ - `####`/`#####`/`######` ladder; shallow rungs unchanged — ✅ - Fenced link/image/`####` stay code; cell-level link renders in `.md-table` — ✅ (node probe) - Byte-identity fixture + phase-44/138 suites pass unedited (git-verified untouched; 6 + 3 E2E pass in isolation) — ✅ - `uv run pytest --cov=app --cov-report=term-missing`: **4198 passed, TOTAL 99%**; `uv run pytest tests/e2e/test_markdown_inline_links_images.py -v --no-cov`: **4 passed**; `uv run ruff check .`: clean; `uv run pyright`: **0 errors** — ✅ - Docs: full README/AGENTS.md review vs the whole project — env-example↔README table complete, page routes match `app/main.py`, AGENTS quick-ref artifacts exist. Fixed 1 inconsistency: README understated the image allowlist (impl per task 02 accepts first segments ≤2 chars, `^.{0,2}/`, not only `./`/`../`); phase-180 paragraphs otherwise accurate. - Notable: image allowlist `.{0,2}/` is broader than the overview's `^\.{1,2}/` but is what task 02 explicitly specifies and the docstring documents — not a deviation from the tasks; SVG sizing deliberately deferred to phase 181. - Next pending phase: `181_svg_document_preview`.
This commit is contained in:
@@ -32,6 +32,76 @@
|
||||
* a > inside a code fence never reaches the pass (step 1 placeholder),
|
||||
* and a > in a table cell is restored in step 4. Everything without a
|
||||
* quote line renders byte-identically to the pre-phase renderer.
|
||||
*
|
||||
* Phase 180 (2026-10-14, TODO.md L4/L5 + the in-answer half of L3) adds
|
||||
* the inline link/image transforms and the rest of the heading ladder
|
||||
* to the ONE `inline()` chain (the single source of every inline
|
||||
* transform, also applied per table cell — so a link inside a cell
|
||||
* works for free). The chain order is code → strong → em →
|
||||
* **linked-image → image → link** — both image rules sit AHEAD of the
|
||||
* link rule: the linked-image rule first of all so `[](url)`
|
||||
* cannot leave a stray `[` in the link text (locked A6), the plain image
|
||||
* rule next so the `!` of an image is never orphaned into `!<a>`
|
||||
* → headings → lists. The heading ladder (locked A4, TODO.md L5) is
|
||||
* `#`/`##` → `<h3>`, `###` → `<h4>`, `####` → `<h5>`, `#####` and
|
||||
* `######` → `<h6>`: the flattening of `#`/`##` to one `<h3>` level is
|
||||
* pre-existing and DELIBERATE (the phase adds the levels below `###`, it
|
||||
* does not re-rank the existing ones), `#####`/`######` share `<h6>`
|
||||
* because HTML stops at h6, and the rules are written deeper-first. Every
|
||||
* rule requires a SPACE after its hash run, so no rule can partially eat
|
||||
* a deeper level (`#### x` never matches `/^### /`) and a heading must
|
||||
* start at column 0 of its line (the `m` flag) — an indented `#### x`
|
||||
* stays plain text, exactly as an indented `### x` does today. Links:
|
||||
* `[text](url)` → `<a class="md-link" href="…">text</a>`, but ONLY for
|
||||
* URLs the allowlist function (`allowLinkUrl`, locked A5) accepts —
|
||||
* `http:`/`https:`, `mailto:`, `/`-rooted and `./`/`../` relative paths,
|
||||
* and `#fragment`; anything else (`javascript:`, `data:`, `vbscript:`,
|
||||
* `file:`, `blob:`) returns the ORIGINAL matched text, so it stays
|
||||
* visible as the inert escaped text it is today and never becomes a
|
||||
* live element. Absolute `http(s)://` links carry
|
||||
* `target="_blank" rel="noopener noreferrer"`; `mailto:` / relative /
|
||||
* fragment links carry neither (a same-origin target in the same tab is
|
||||
* correct). The captured URL passes through VERBATIM — the chain's
|
||||
* input is already escapeHtml-ed, so `&`/`'`/`"` in a query
|
||||
* string are already attribute-safe and decoding them would re-open the
|
||||
* injection surface this renderer exists to close. Images (task 02,
|
||||
* TODO.md L3): `` →
|
||||
* `<img class="md-img" src="…" alt="…" loading="lazy">` and
|
||||
* `[](url)` → that same image wrapped in the link, again ONLY
|
||||
* through an allowlist function (`allowImageUrl`, locked A5) — `/`-rooted
|
||||
* served paths (`/api/documents/<id>/image`, `/assets/…`), `./`/`../`
|
||||
* relative paths, `http(s)://` and `data:image/…` (the `data:` case is
|
||||
* the app's own CSP speaking: `img-src 'self' data:` in
|
||||
* app/core/security_headers.py::CSP already permits `data:` images, so
|
||||
* no CSP change is involved); `javascript:`, `file:`, `blob:` and a
|
||||
* non-image `data:` return null and the reference stays visible as inert
|
||||
* text. `alt` is ALWAYS emitted (empty allowed — `alt=""` for a
|
||||
* decorative image) because an `<img>` with no alt attribute fails WCAG
|
||||
* 1.1, and there is deliberately NO `onerror` handler: this renderer
|
||||
* returns an HTML string into DOM-sink surfaces, so an inline handler
|
||||
* would break the no-CDN/CSP posture — a broken src degrades to
|
||||
* the browser's alt text (the JS fallbacks live in document.js/app.js,
|
||||
* which do not route through this function). Documented limits
|
||||
* (known, not fixed): a URL containing a literal `)` or whitespace is a
|
||||
* miss (so title syntax `` is a miss too), as is a
|
||||
* relative image path whose first segment is longer than two characters
|
||||
* (`images/x.png` — the allowlist accepts `./`, `../` and `/`-rooted);
|
||||
* a linked image whose HREF alone is rejected falls through to the plain
|
||||
* image rule, so the image renders inside the inert `[…](url)` text, and
|
||||
* an image whose src is rejected by `allowImageUrl` but ACCEPTED by
|
||||
* `allowLinkUrl` (a link-only shape used as an image src — `#fragment`
|
||||
* or `mailto:`) falls through to the link rule, which turns `[alt](url)`
|
||||
* into a link behind the leftover `!`; neither ever yields a live image
|
||||
* or a live script;
|
||||
* inline code spans are NOT placeholder-protected (pre-existing —
|
||||
* `**bold**` inside backticks already transforms), so markdown syntax
|
||||
* inside `` `…` `` transforms too; 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;
|
||||
* autolinks (`<https://x>`) and reference-style links (`[text][ref]`) are
|
||||
* out of scope. Everything without a link/image/`####`+ construct renders
|
||||
* byte-identically to
|
||||
* the pre-phase renderer (pinned by the phase-180 unit suite).
|
||||
*/
|
||||
|
||||
function escapeHtml(s) {
|
||||
@@ -40,6 +110,52 @@ function escapeHtml(s) {
|
||||
}[c]));
|
||||
}
|
||||
|
||||
// Phase 180 (task 01, TODO.md L4): the link URL allowlist (LOCKED A5).
|
||||
// The inline chain always receives HTML-ESCAPED text, so a captured URL
|
||||
// may legitimately contain `&` / `'` / `"` — valid inside an
|
||||
// href — and is returned VERBATIM (never decoded, never re-escaped:
|
||||
// design D2, the escape-first boundary). Accepted schemes/shapes:
|
||||
// `http(s)://`, `mailto:`, `/`-rooted, `./` and `../` relative, and
|
||||
// `#fragment`. Everything else — `javascript:`, `data:`, `vbscript:`,
|
||||
// `file:`, `blob:` — returns null so the caller keeps the ORIGINAL
|
||||
// matched text as inert escaped prose.
|
||||
function allowLinkUrl(url) {
|
||||
return /^(?:https?:\/\/|mailto:|\/|\.{1,2}\/|#)/i.test(url) ? url : null;
|
||||
}
|
||||
|
||||
// Phase 180 (task 02, TODO.md L3): the image URL allowlist (LOCKED A5).
|
||||
// Same escape-first, verbatim contract as allowLinkUrl — the inline chain
|
||||
// always receives HTML-ESCAPED text, so a captured src may legitimately
|
||||
// contain `&` / `'` / `"` (valid inside a src) and is
|
||||
// returned untouched (never decoded, never re-escaped: design D2).
|
||||
// Accepted: `/`-rooted served paths (`/api/documents/<id>/image`,
|
||||
// `/assets/…`), `./` and `../` relative paths, `http(s)://`, and
|
||||
// `data:image/…` — the `data:` case exists because the app's OWN CSP
|
||||
// already permits it (`img-src 'self' data:`,
|
||||
// app/core/security_headers.py::CSP), so no CSP change is involved.
|
||||
// Everything else — `javascript:`, `file:`, `blob:`, a NON-image `data:`
|
||||
// (e.g. `data:text/html,…`) — returns null so the caller keeps the
|
||||
// ORIGINAL matched text as inert escaped prose.
|
||||
function allowImageUrl(url) {
|
||||
return /^(?:https?:\/\/|data:image\/|\/|.{0,2}\/)/i.test(url) ? url : null;
|
||||
}
|
||||
|
||||
// Phase 180 (tasks 01 + 02): the ONE anchor-opener builder, shared by the
|
||||
// link rule and the linked-image rule (locked A6) so the exact
|
||||
// `<a class="md-link"` marker and the `target`/`rel` rule have a single
|
||||
// source in the file. `safe` MUST already be an allowLinkUrl-accepted
|
||||
// URL (callers check first and keep the original text on null); the
|
||||
// pair is emitted ONLY for an absolute `http(s)://` target — `mailto:`,
|
||||
// `/`-rooted, `./`/`../` and `#fragment` targets stay in the same tab.
|
||||
function mdAnchorOpen(safe) {
|
||||
const external = /^https?:\/\//i.test(safe);
|
||||
return (
|
||||
`<a class="md-link" href="${safe}"` +
|
||||
(external ? ' target="_blank" rel="noopener noreferrer"' : "") +
|
||||
`>`
|
||||
);
|
||||
}
|
||||
|
||||
function renderMarkdown(md) {
|
||||
// 1. Protect fenced code blocks.
|
||||
const codeBlocks = [];
|
||||
@@ -56,6 +172,74 @@ function renderMarkdown(md) {
|
||||
s.replace(/`([^`\n]+)`/g, "<code>$1</code>")
|
||||
.replace(/\*\*([^*]+)\*\*/g, "<strong>$1</strong>")
|
||||
.replace(/(^|[\s(])\*([^*\n]+)\*/g, "$1<em>$2</em>")
|
||||
// Phase 180 (task 02, TODO.md L3, locked A6): a LINKED image —
|
||||
// `` inside `[…](url)`, i.e. `[](url)` — gets
|
||||
// its OWN rule, AHEAD of both the image and the link rules —
|
||||
// without it the image rule consumes the inner `` and
|
||||
// the link rule then leaves a
|
||||
// stray `[` in the link text. BOTH URLs are allowlist-checked (the
|
||||
// href through allowLinkUrl, the src through allowImageUrl) and
|
||||
// either rejection returns the whole original match, which stays
|
||||
// inert escaped text (locked A5). `alt` is ALWAYS emitted — empty
|
||||
// allowed, `alt=""` for a decorative image — because an `<img>`
|
||||
// with no alt attribute fails WCAG 1.1; like the URLs it is the
|
||||
// already-escaped text passed through VERBATIM (design D2).
|
||||
.replace(/\[!\[([^\]]*)\]\(([^)\s]+)\)\]\(([^)\s]+)\)/g, (_m, alt, src, url) => {
|
||||
const safeSrc = allowImageUrl(src);
|
||||
if (safeSrc === null) return _m;
|
||||
const safeHref = allowLinkUrl(url);
|
||||
if (safeHref === null) return _m;
|
||||
return (
|
||||
mdAnchorOpen(safeHref) +
|
||||
`<img class="md-img" src="${safeSrc}" alt="${alt}" loading="lazy">` +
|
||||
"</a>"
|
||||
);
|
||||
})
|
||||
// Phase 180 (task 02, TODO.md L3): a plain inline image
|
||||
// `` → a lazy `<img class="md-img">`. Runs AFTER the
|
||||
// linked-image rule (so the `!` is never orphaned into `!<a>`) and
|
||||
// BEFORE the link rule. Same allowlist discipline as links (locked
|
||||
// A5): a rejected src returns the whole original match, so the
|
||||
// reference stays visible as the inert escaped text it is today.
|
||||
// No `onerror` fallback on purpose — this renderer returns an HTML
|
||||
// STRING into DOM-sink surfaces and inline handlers break the
|
||||
// no-CDN/CSP posture, so a broken src degrades to the
|
||||
// browser's alt text (the JS fallbacks live in document.js/app.js,
|
||||
// which do not route through this function).
|
||||
.replace(/!\[([^\]]*)\]\(([^)\s]+)\)/g, (_m, alt, src) => {
|
||||
const safe = allowImageUrl(src);
|
||||
if (safe === null) return _m;
|
||||
return `<img class="md-img" src="${safe}" alt="${alt}" loading="lazy">`;
|
||||
})
|
||||
// Phase 180 (task 01): inline links, after the emphasis rules (so
|
||||
// link text carrying **strong**/*em*/`code` — or an image the two
|
||||
// rules above already transformed — is in place when it gets
|
||||
// wrapped) and before the heading ladder. Empty link text is
|
||||
// allowed; whitespace inside the URL is a miss (documented limit),
|
||||
// as is a URL containing a literal `)`. The replacement is a
|
||||
// FUNCTION so the URL goes through the allowLinkUrl allowlist
|
||||
// (locked A5) — a rejected URL returns the whole original match,
|
||||
// which stays inert escaped text. The anchor itself (marker +
|
||||
// `target`/`rel` for absolute http(s) targets) is built by the one
|
||||
// shared mdAnchorOpen helper, also used by the linked-image rule.
|
||||
.replace(/\[([^\]]*)\]\(([^)\s]+)\)/g, (_m, text, url) => {
|
||||
const safe = allowLinkUrl(url);
|
||||
if (safe === null) return _m;
|
||||
return `${mdAnchorOpen(safe)}${text}</a>`;
|
||||
})
|
||||
// Phase 180 (task 03, TODO.md L5, locked A4): the rest of the heading
|
||||
// ladder, written DEEPER-FIRST immediately before the existing
|
||||
// `^### ` rule. Rules cannot cross-match because every heading rule
|
||||
// requires a SPACE after its hash run — `#### x` never matches
|
||||
// `/^### /`, `##### x`/`###### x` never match `/^#### /` — so a deep
|
||||
// heading can never be partially eaten by a shallower rule. `#####`
|
||||
// and `######` BOTH map to `<h6>` (locked A4: HTML has no `<h7>` and
|
||||
// the app's type scale stops two levels below `###`). The existing
|
||||
// `#`/`##` → `<h3>` flattening stays UNTOUCHED (locked A4). Like the
|
||||
// rules it sits next to, each rule anchors at column 0 of its line
|
||||
// (the `m` flag), so an indented `#### x` remains plain text.
|
||||
.replace(/^#### (.*)$/gm, "<h5>$1</h5>")
|
||||
.replace(/^#{5,6} (.*)$/gm, "<h6>$1</h6>")
|
||||
.replace(/^### (.*)$/gm, "<h4>$1</h4>")
|
||||
.replace(/^## (.*)$/gm, "<h3>$1</h3>")
|
||||
.replace(/^# (.*)$/gm, "<h3>$1</h3>")
|
||||
|
||||
@@ -587,6 +587,120 @@ body::before {
|
||||
quote stays distinct. */
|
||||
.msg.user .bubble .md-quote { color: var(--bg); }
|
||||
|
||||
/* Markdown inline links (phase 180, task 04, TODO.md L4 — "Markdown
|
||||
[links](some-address) aren't being render correctly and show up as
|
||||
plain text"): the shared renderer (markdown.js inline chain) turns
|
||||
[text](url) — and a linked image, locked A6 — into
|
||||
<a class="md-link">, and an element with no rule anywhere in this file
|
||||
would paint as unmarked body text. ONE rule set serves every rendering
|
||||
context the renderer feeds (the chat .bubble brain + user, the
|
||||
details.thinking / details.reasoning scratchpad, the shared page's
|
||||
.msg/.bubble copy, the viewer's .doc-md and the document modal, which
|
||||
shares the renderDocument core) — all of them sit on --surface, and the
|
||||
deflected bubble's --accent-bg holds the pairing too (8.3:1), so a
|
||||
single AA pairing covers them all. Palette vars only, no hex/rgb/hsl
|
||||
literal (the :root-drift test parses the built-ins out of this file — a
|
||||
literal here would be a second source of truth, and a new hue would
|
||||
break the monochrome invariant):
|
||||
color --brand-ink — 9.9:1 on --surface, the :root-documented
|
||||
AA-safe BRAND TEXT token (the same rose the
|
||||
details.thinking summary and .msg.brain .avatar use);
|
||||
underline unconditional — a link in running text must not rely on
|
||||
color alone (WCAG 1.4.1, PLAN §7.2 "text + color, never
|
||||
color alone"), and it is what keeps the link marked inside
|
||||
the user bubble, where the color inverts to the bubble's
|
||||
own ink below;
|
||||
offset 2px — lifts the rule off the descenders (g/y/p stay open).
|
||||
The focus state is the GLOBAL `:focus-visible` 3px --brand ring above
|
||||
(do NOT restate or reset it here — one ring for the whole app).
|
||||
Static content — no animation (nothing for prefers-reduced-motion to
|
||||
still). */
|
||||
.md-link {
|
||||
color: var(--brand-ink); /* 9.9:1 on --surface (AA) — :root's brand text token */
|
||||
text-decoration: underline;
|
||||
text-underline-offset: 2px;
|
||||
}
|
||||
/* :hover brightens one notch on the same rose — --brand-hover is --brand
|
||||
mixed with 14% white in :root: 5.9:1 on --surface and 5.0:1 on the
|
||||
deflected bubble's --accent-bg, so the state change never drops under
|
||||
AA on any canvas the renderer writes into (brightening is the safe
|
||||
direction on a dark palette — every darker step fails there). */
|
||||
.md-link:hover { color: var(--brand-hover); }
|
||||
/* The USER bubble runs the question through the same renderer on the
|
||||
--brand fill (the .md-quote inversion above is the precedent):
|
||||
--brand-ink on --brand is 1.9:1 and --brand-hover on --brand is 1.2:1 —
|
||||
both AA failures — so the link keeps the bubble's own ink, --bg on
|
||||
--brand at 5.2:1 (the :root pairing). The unconditional underline is
|
||||
what marks it a link here, never the color. */
|
||||
.msg.user .bubble .md-link { color: var(--bg); }
|
||||
/* …and the SAME inversion on :hover: the default --brand-hover would
|
||||
erase the word (1.2:1 on the fill). A color-mix toward --bg cannot
|
||||
help — the text is ALREADY --bg — so the hover state moves into the
|
||||
DECORATION (a 2px rule at a deeper offset), which is a visible state
|
||||
change that needs no new pair, keeps 5.2:1 at every theme, and stays
|
||||
inside the palette (no new hue — the monochrome invariant). */
|
||||
.msg.user .bubble .md-link:hover {
|
||||
color: var(--bg);
|
||||
text-decoration-thickness: 2px;
|
||||
text-underline-offset: 3px;
|
||||
}
|
||||
|
||||
/* Markdown inline images (phase 180, task 04, TODO.md L3 — the in-answer
|
||||
half of "Need SVG render support on the front end"):  and
|
||||
[](url) render <img class="md-img"> through the shared
|
||||
renderer, so the painted diagram has to stay INSIDE the reading column
|
||||
instead of pushing a horizontal scrollbar (the TODO's complaint is a
|
||||
reference that prints; the fix is only useful when what replaces it
|
||||
fits). No fixed width/height — max-width:100% + height:auto keep any
|
||||
aspect ratio at 375px and at 1440px (the .doc-image-img / .source-image-img
|
||||
contract), object-fit:contain never crops, and the card language
|
||||
(--surface plate, --line hairline, --radius-sm) is the treatment every
|
||||
other image in the app already carries, so a diagram inside an answer
|
||||
reads as an image rather than a stray artifact. inline-block so the
|
||||
block-ish margin behaves and the image sits as its own box inside a
|
||||
wrapping <a> (locked A6) instead of on a text baseline. Palette vars
|
||||
only (no literal). Static content — no animation (nothing for
|
||||
prefers-reduced-motion to still). Known limit (phase 181): an SVG that
|
||||
declares a viewBox but no width/height paints at the replaced-element
|
||||
default box, constrained by max-width — vector sizing is phase 181's
|
||||
contract, so NO .md-img--vector modifier here. */
|
||||
.md-img {
|
||||
display: inline-block;
|
||||
max-width: 100%;
|
||||
height: auto;
|
||||
object-fit: contain;
|
||||
background: var(--surface);
|
||||
border: 1px solid var(--line);
|
||||
border-radius: var(--radius-sm);
|
||||
margin: 0.4rem 0;
|
||||
}
|
||||
|
||||
/* The deep rungs of the heading ladder (phase 180, task 04, TODO.md L5 —
|
||||
"Quad-level headers (####) aren't being rendered"): the renderer now
|
||||
emits <h5> for #### and <h6> for #####/###### (locked A4) and neither
|
||||
tag had a rule ANYWHERE in this file, so a deep heading in an answer
|
||||
inherited the UA defaults (h5 0.83em / h6 0.67em with 1.67em / 2.33em
|
||||
block margins — ~1.4rem / 1.6rem on this scale) — gaps wider than the
|
||||
paragraphs around it while the type itself went SMALLER than the answer
|
||||
text it sits in. ONE rule set per level for every renderer context (the
|
||||
viewer's .doc-md, the chat AND shared-page .bubble, the thinking
|
||||
scratchpad) — no chat-specific duplication — extending the existing
|
||||
.doc-md h3 1.15rem / .doc-md h4 1rem scale MONOTONICALLY
|
||||
(1.15 → 1.0 → 0.95 → 0.88): every level stays a visible step from the
|
||||
1rem body text and from its neighbor, and the margins
|
||||
tighten with the depth on the same h3 1.1/0.4 → h4 0.9/0.35 rhythm.
|
||||
Sizes are rem (never px), so the ladder survives a browser
|
||||
font-size/zoom change; no color is set, so the scale adds no new
|
||||
contrast pair (it inherits each context's own AA ink — ink on --surface
|
||||
15.3:1, ink-soft on --surface 8.2:1 in the scratchpad). Known, accepted:
|
||||
the 0.875rem scratchpad owns no h3/h4 rules of its own (UA sizes there),
|
||||
so the fixed 0.95rem h5 rides a hair above its body text — a heading
|
||||
stays a heading, and re-sizing the shallower rungs is out of this
|
||||
phase's scope. Static content — no animation (nothing for
|
||||
prefers-reduced-motion to still). */
|
||||
.doc-md h5, .msg .bubble h5, .thinking-text h5 { margin: 0.8rem 0 0.3rem; font-size: 0.95rem; }
|
||||
.doc-md h6, .msg .bubble h6, .thinking-text h6 { margin: 0.7rem 0 0.3rem; font-size: 0.88rem; }
|
||||
|
||||
.msg.user { justify-content: flex-end; }
|
||||
.msg.user .msg-body { align-items: flex-end; }
|
||||
.msg.user .bubble {
|
||||
|
||||
Reference in New Issue
Block a user