From 77936b4bf9208c35c16d7b5274d11aad00b3f24e Mon Sep 17 00:00:00 2001 From: coryslater <25396141+fivestarspicy@users.noreply.github.com> Date: Thu, 27 Aug 2026 09:58:12 -0700 Subject: [PATCH 1/7] feat(replay-vision): render scanner reasoning as markdown Scanner reasoning that carries a bold lead-in or a bullet list now renders as structure on the observation card, instead of showing the raw syntax. The same string feeds surfaces that read it as one line of prose: Slack alert messages, the Max and Signals search snippets, the group-summary feed, the search embeddings, the seekbar marks. Each gets a flattened copy, so a markdown bullet cannot arrive there as a literal row marker. Generated-By: PostHog Desktop Task-Id: 072e2d37-89a7-4ab7-90e1-a71f90862523 --- .../lemon-ui/LemonMarkdown/LemonMarkdown.tsx | 50 ++++++++++-- .../src/lib/lemon-ui/LemonMarkdown/index.ts | 8 +- .../backend/observation_formatting.py | 65 +++++++++++++++- .../temporal/activities/embed_observation.py | 8 +- .../tests/test_observation_formatting.py | 60 ++++++++++++++- .../components/CitedMarkdown.stories.tsx | 36 +++++++++ .../frontend/components/CitedMarkdown.tsx | 38 +++++++++ .../frontend/components/ObservationCard.tsx | 34 +++----- .../frontend/components/TimestampCitation.tsx | 24 ++++++ .../observations/ReplayObservation.tsx | 6 +- .../frontend/utils/citations.test.ts | 56 +++++++++++++- .../replay_vision/frontend/utils/citations.ts | 33 ++++++++ .../frontend/utils/markdown.test.ts | 61 +++++++++++++++ .../replay_vision/frontend/utils/markdown.ts | 77 +++++++++++++++++++ .../frontend/utils/observation.ts | 11 ++- 15 files changed, 521 insertions(+), 46 deletions(-) create mode 100644 products/replay_vision/frontend/components/CitedMarkdown.stories.tsx create mode 100644 products/replay_vision/frontend/components/CitedMarkdown.tsx create mode 100644 products/replay_vision/frontend/components/TimestampCitation.tsx create mode 100644 products/replay_vision/frontend/utils/markdown.test.ts create mode 100644 products/replay_vision/frontend/utils/markdown.ts diff --git a/frontend/src/lib/lemon-ui/LemonMarkdown/LemonMarkdown.tsx b/frontend/src/lib/lemon-ui/LemonMarkdown/LemonMarkdown.tsx index 5d1ef9f9c9ae..38fcbcce88e0 100644 --- a/frontend/src/lib/lemon-ui/LemonMarkdown/LemonMarkdown.tsx +++ b/frontend/src/lib/lemon-ui/LemonMarkdown/LemonMarkdown.tsx @@ -31,6 +31,23 @@ function chartRefId(href: unknown): string | null { return typeof href === 'string' ? (CHART_REF_TARGET.exec(href)?.[1] ?? null) : null } +/** + * Link target that marks a moment in a recording, e.g. `[01:32](t:92000)` — the offset in milliseconds. + * Same reasoning as `chart:`: a link rather than a bespoke token, so the same markdown degrades to a + * readable timestamp in the renderers that have no player to seek. + */ +export const TIMESTAMP_REF_PREFIX = 't:' + +// Bounded digits so an overlong target is an ordinary link rather than a reference, keeping it subject +// to the usual URL sanitizing. 12 digits is ~31 years of recording, well past any real session. +const TIMESTAMP_REF_TARGET = new RegExp(`^${TIMESTAMP_REF_PREFIX}(\\d{1,12})$`) + +/** The offset in milliseconds behind a `t:` link target, or null when `href` is an ordinary link. */ +function timestampRefMs(href: unknown): number | null { + const digits = typeof href === 'string' ? TIMESTAMP_REF_TARGET.exec(href)?.[1] : undefined + return digits === undefined ? null : parseInt(digits, 10) +} + /** Whether a paragraph's child node is a chart reference. */ function isChartRefNode(node: any): boolean { return node?.tagName === 'a' && chartRefId(node.properties?.href) !== null @@ -97,6 +114,13 @@ export interface LemonMarkdownProps { * is a scheme no browser can follow. */ renderChartRef?: (chartId: string, sourceOffset?: number) => React.ReactNode + /** + * Optional renderer for `t:` link targets (see `TIMESTAMP_REF_PREFIX`). Returning null or + * undefined renders the link's own label as plain text, which is the fallback for a reference no + * player is on screen to seek. Omitting this prop renders every timestamp target as its label — + * never as a link, since `t:` is a scheme no browser can follow. + */ + renderTimestampRef?: (timestampMs: number) => React.ReactNode } const HEADING_TAGS = ['h1', 'h2', 'h3', 'h4', 'h5', 'h6'] as const @@ -137,10 +161,17 @@ const LemonMarkdownRenderer = memo(function LemonMarkdownRenderer({ generateHeadingIds = false, renderMermaid, renderChartRef, + renderTimestampRef, }: LemonMarkdownProps): JSX.Element { const components = useMemo( () => ({ a: ({ href, children, node }: any): JSX.Element => { + const timestampMs = timestampRefMs(href) + if (timestampMs !== null) { + // Falls back to its own label — the timestamp itself — which is what a reader with no + // player on screen needs anyway. + return <>{renderTimestampRef?.(timestampMs) ?? children} + } const chartId = chartRefId(href) if (chartId !== null) { // Falls back to its own label as plain text: for a consumer that can't draw @@ -288,19 +319,22 @@ const LemonMarkdownRenderer = memo(function LemonMarkdownRenderer({ generateHeadingIds, renderMermaid, renderChartRef, + renderTimestampRef, ] ) - // `chart:` is not a protocol the default URL sanitizer knows, so it strips the href before the - // `a` override ever sees it — and an emptied href is what `Link` turns into a focusable - // target-blank stub that goes nowhere. Let the scheme through whether or not this caller can - // draw charts, so the `a` override can recognize the reference and render its label as text. - // Only on `href`: the same transform runs for an image `src`, where nothing downstream reads a - // chart reference, so `![x](chart:y)` would carry an unsanitized scheme into a broken image. + // Neither `chart:` nor `t:` is a protocol the default URL sanitizer knows, so it strips the href + // before the `a` override ever sees it — and an emptied href is what `Link` turns into a focusable + // target-blank stub that goes nowhere. Let both schemes through whether or not this caller can draw + // charts or seek a player, so the `a` override can recognize the reference and render its label as + // text. Only on `href`: the same transform runs for an image `src`, where nothing downstream reads a + // reference, so `![x](chart:y)` would carry an unsanitized scheme into a broken image. const urlTransform = useMemo( () => (url: string, key: string): string => - key === 'href' && chartRefId(url) !== null ? url : defaultUrlTransform(url), + key === 'href' && (chartRefId(url) !== null || timestampRefMs(url) !== null) + ? url + : defaultUrlTransform(url), [] ) @@ -331,6 +365,7 @@ function LemonMarkdownComponent({ generateHeadingIds = false, renderMermaid, renderChartRef, + renderTimestampRef, className, }: LemonMarkdownProps): JSX.Element { return ( @@ -344,6 +379,7 @@ function LemonMarkdownComponent({ generateHeadingIds={generateHeadingIds} renderMermaid={renderMermaid} renderChartRef={renderChartRef} + renderTimestampRef={renderTimestampRef} > {children} diff --git a/frontend/src/lib/lemon-ui/LemonMarkdown/index.ts b/frontend/src/lib/lemon-ui/LemonMarkdown/index.ts index 04f49fa6afbd..68705c4e8d5d 100644 --- a/frontend/src/lib/lemon-ui/LemonMarkdown/index.ts +++ b/frontend/src/lib/lemon-ui/LemonMarkdown/index.ts @@ -1,4 +1,10 @@ -export { CHART_REF_PREFIX, LemonMarkdown, type LemonMarkdownProps, slugifyHeading } from './LemonMarkdown' +export { + CHART_REF_PREFIX, + LemonMarkdown, + type LemonMarkdownProps, + TIMESTAMP_REF_PREFIX, + slugifyHeading, +} from './LemonMarkdown' // LemonMarkdownWithMermaid is deliberately not re-exported: this barrel is imported by lib // components shipped in the toolbar bundle, and the mermaid variant must stay out of that graph. // Import it from 'lib/lemon-ui/LemonMarkdown/LemonMarkdownWithMermaid' directly. diff --git a/products/replay_vision/backend/observation_formatting.py b/products/replay_vision/backend/observation_formatting.py index 9e0a23eb67d2..eebccebca670 100644 --- a/products/replay_vision/backend/observation_formatting.py +++ b/products/replay_vision/backend/observation_formatting.py @@ -10,6 +10,66 @@ SEARCH_SNIPPET_LIMIT = 600 +# Markdown a scanner can emit in its free-text fields. Block markers are matched per line, the way a +# markdown parser reads them; inline markers are matched anywhere on the line. +_FENCE_RE = re.compile(r"^\s{0,3}(?:```|~~~)") +_RULE_RE = re.compile(r"^\s{0,3}(?:[-*_]\s*){3,}$") +_HEADING_RE = re.compile(r"^\s{0,3}#{1,6}\s+") +_QUOTE_RE = re.compile(r"^\s{0,3}>\s?") +_BULLET_RE = re.compile(r"^\s*(?:[-*+]|\d{1,9}[.)])\s+") +_IMAGE_RE = re.compile(r"!\[([^\]]*)\]\([^)]*\)") +_LINK_RE = re.compile(r"\[([^\]]*)\]\([^)]*\)") +_CODE_SPAN_RE = re.compile(r"`+([^`]*)`+") +_STRONG_RE = re.compile(r"(\*\*|__)(?=\S)(.+?)(?<=\S)\1") +_STRIKE_RE = re.compile(r"~~(?=\S)(.+?)(?<=\S)~~") +# The lookarounds keep `snake_case` and `2 * 3 * 4` intact: an underscore or star touching a word +# character on the outside is punctuation the model meant literally, not an emphasis delimiter. +_EMPHASIS_RE = re.compile(r"(?~|])") +# Terminal punctuation, after which one flattened block runs into the next without a sentence break. +_SENTENCE_ENDINGS = ".!?:;," + + +def flatten_markdown(text: str) -> str: + """Markdown-bearing model text as plain prose, keeping the line structure. + + Scanner reasoning can carry a bold lead-in, a heading, or bullets. Every consumer outside the UI reads + it as prose, where the syntax is noise at best (`**` and `##` inside a search snippet) and a forged list + row at worst. Flattening here rather than at each consumer keeps the stored field the one canonical + copy. `plain_snippet` is what folds the result onto a single line. + """ + lines: list[str] = [] + for raw in text.split("\n"): + if _FENCE_RE.match(raw) or _RULE_RE.match(raw): + continue + line = _BULLET_RE.sub("", _QUOTE_RE.sub("", _HEADING_RE.sub("", raw))) + line = _IMAGE_RE.sub(r"\1", line) + line = _LINK_RE.sub(r"\1", line) + line = _CODE_SPAN_RE.sub(r"\1", line) + line = _STRONG_RE.sub(r"\2", line) + line = _STRIKE_RE.sub(r"\1", line) + line = _EMPHASIS_RE.sub(r"\2", line) + lines.append(_ESCAPE_RE.sub(r"\1", line).rstrip()) + return re.sub(r"\n{3,}", "\n\n", "\n".join(lines)).strip() + + +def plain_snippet(text: str, *, limit: int | None = SEARCH_SNIPPET_LIMIT) -> str: + """One line of readable plain text from a model's free-text field. + + The single line is a safety property, not tidiness. These snippets are embedded inside markdown the + reader trusts — a Slack alert's bullet list, the untrusted-data fence in the synthesis prompt — where a + newline lets recording-derived text forge a row or a header. Flattening runs first so a markdown bullet + cannot survive as a literal `-` at the start of the folded line either. Blocks are joined with a + sentence break so a flattened list still reads as prose rather than one run-on clause. + """ + flat = flatten_markdown(EVENT_ID_CITATION_RE.sub("", text)) + out = "" + for block in (b for line in flat.split("\n") if (b := " ".join(line.split()))): + if out: + out += " " if out[-1] in _SENTENCE_ENDINGS else ". " + out += block + return out if limit is None else out[:limit] + def read_output(obs: "ReplayObservation") -> dict[str, Any] | None: scanner_result = obs.scanner_result if isinstance(obs.scanner_result, dict) else None @@ -36,11 +96,12 @@ def describe_output(output: dict[str, Any]) -> str | None: def explanation_text(output: dict[str, Any]) -> str: """The free text a scanner wrote about the session: monitors, scorers and classifiers reason, summarizers - summarize. Event-id citations are for the UI, so they are stripped here.""" + summarize. Event-id citations are for the UI, so they are stripped here, and any markdown the model wrote + is flattened: every caller reads this as one line of prose.""" explanation = output.get("reasoning") or output.get("summary") if not isinstance(explanation, str): return "" - return re.sub(r"\s+", " ", EVENT_ID_CITATION_RE.sub("", explanation)).strip() + return plain_snippet(explanation, limit=None) def format_line(obs: "ReplayObservation", output: dict[str, Any], *, show_scanner: bool) -> str: diff --git a/products/replay_vision/backend/temporal/activities/embed_observation.py b/products/replay_vision/backend/temporal/activities/embed_observation.py index 24879a32002c..5ecad96cffb4 100644 --- a/products/replay_vision/backend/temporal/activities/embed_observation.py +++ b/products/replay_vision/backend/temporal/activities/embed_observation.py @@ -27,6 +27,7 @@ EMBEDDING_PRODUCT, OBSERVATION_EMBEDDING_MODEL, ) +from products.replay_vision.backend.observation_formatting import flatten_markdown from products.replay_vision.backend.tags import slugify_tag from products.replay_vision.backend.temporal.constants import KAFKA_DELIVERY_TIMEOUT_S from products.replay_vision.backend.temporal.decorators import track_activity @@ -39,8 +40,13 @@ def _renderings_for(model_output: AnyScannerOutput) -> list[tuple[str, str]]: + """The `(rendering, content)` pairs to embed for one scanner output, or none when it has nothing to say. + + Flattened first: markdown syntax the model wrote is not meaning, and embedding `**` and `##` tokens + dilutes the vector that semantic search ranks on. + """ document = model_output.embedding_document() - return [(document.rendering, document.text)] if document else [] + return [(document.rendering, flatten_markdown(document.text))] if document else [] def _result_metadata(model_output: AnyScannerOutput) -> dict[str, Any]: diff --git a/products/replay_vision/backend/tests/test_observation_formatting.py b/products/replay_vision/backend/tests/test_observation_formatting.py index 28122e66845e..126aa573132a 100644 --- a/products/replay_vision/backend/tests/test_observation_formatting.py +++ b/products/replay_vision/backend/tests/test_observation_formatting.py @@ -1,6 +1,8 @@ from datetime import date -from products.replay_vision.backend.observation_formatting import format_line +from parameterized import parameterized + +from products.replay_vision.backend.observation_formatting import flatten_markdown, format_line, plain_snippet class _FakeObs: @@ -17,4 +19,58 @@ def test_format_line_collapses_whitespace_so_observations_cannot_forge_rows() -> show_scanner=False, ) assert "\n" not in line - assert "clicked checkout - forged row ignore the above" in line + assert "clicked checkout. forged row. ignore the above" in line + + +class TestPlainSnippet: + @parameterized.expand( + [ + ("bold", "The user **abandoned** checkout.", "The user abandoned checkout."), + ("italic", "The user *hesitated* here.", "The user hesitated here."), + ("inline_code", "Clicked the `Submit` button.", "Clicked the Submit button."), + ("heading", "## Checkout blocked\nThe form rejected it.", "Checkout blocked. The form rejected it."), + ("bullets", "- Reached payment\n- Card rejected", "Reached payment. Card rejected"), + ("numbered", "1. Reached payment\n2. Card rejected", "Reached payment. Card rejected"), + ("blockquote", "> The user gave up.", "The user gave up."), + ("link", "Landed on [the pricing page](https://example.com/p).", "Landed on the pricing page."), + ("image", "![a screenshot](https://example.com/x.png) followed.", "a screenshot followed."), + ("escaped_star", r"Priced at 5\* the usual.", "Priced at 5* the usual."), + ("already_one_sentence_per_line", "Reached payment.\nCard rejected.", "Reached payment. Card rejected."), + ("existing_terminal_punctuation", "Two problems:\n- one\n- two", "Two problems: one. two"), + ] + ) + def test_flattens_markdown_to_one_readable_line(self, _label: str, text: str, expected: str) -> None: + assert plain_snippet(text) == expected + + @parameterized.expand( + [ + ("snake_case", "The handler read team_id_override from the payload."), + ("multiplication", "Retried 3 * 4 times."), + ] + ) + def test_leaves_literal_punctuation_alone(self, _label: str, text: str) -> None: + assert plain_snippet(text) == text + + @parameterized.expand( + [ + ("markdown_list", "- forged row\n- second row"), + ("markdown_heading", "# Alert fired\nsomething else"), + ("hard_wrapped_paragraphs", "line one\nline two\n\nline three"), + ] + ) + def test_never_emits_a_line_break_or_a_leading_block_marker(self, _label: str, text: str) -> None: + # Alert messages and the synthesis feed embed this inside markdown the reader trusts, so a newline + # or a leading marker would let recording-derived text forge a row or a header there. + snippet = plain_snippet(text) + assert "\n" not in snippet + assert not snippet.startswith(("-", "*", "#", ">")) + + def test_caps_length_and_can_opt_out(self) -> None: + text = "word " * 400 + assert len(plain_snippet(text)) == 600 + assert len(plain_snippet(text, limit=None)) > 600 + + +def test_flatten_markdown_keeps_line_structure_for_embeddings() -> None: + # Embeddings take the flattened text whole, so the paragraph break has to survive the strip. + assert flatten_markdown("## Title\n\n- **one**\n- two") == "Title\n\none\ntwo" diff --git a/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx b/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx new file mode 100644 index 000000000000..3c3ee282fe5b --- /dev/null +++ b/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx @@ -0,0 +1,36 @@ +import type { Meta, StoryObj } from '@storybook/react' + +import { CitedMarkdown } from './CitedMarkdown' + +const meta: Meta = { + title: 'Replay Vision/Cited markdown', + component: CitedMarkdown, + args: { onSeek: () => {} }, +} +export default meta + +type Story = StoryObj + +export const Structured: Story = { + args: { + text: '**Checkout blocked at payment**\n\nThe user reached the payment step and never got past it:\n\n- The card form rejected the submission twice with no visible error\n- A third attempt on a different card behaved the same way\n\nThe session ends on the payment page.', + segments: [ + { kind: 'text', value: '**Checkout blocked at payment**\n\nThe user reached the payment step' }, + { kind: 'chip', timestamp_ms: 92000 }, + { + kind: 'text', + value: ' and never got past it:\n\n- The card form rejected the submission twice with no visible error\n- A third attempt on a different card behaved the same way', + }, + { kind: 'chip', timestamp_ms: 241000 }, + { kind: 'text', value: '\n\nThe session ends on the payment page.' }, + ], + }, +} + +/** Reasoning written before the prompt asked for structure still has to render as the paragraph it is. */ +export const PlainProse: Story = { + args: { + text: 'The user opened the pricing page, scrolled to the comparison table, and left without starting a trial.', + segments: [], + }, +} diff --git a/products/replay_vision/frontend/components/CitedMarkdown.tsx b/products/replay_vision/frontend/components/CitedMarkdown.tsx new file mode 100644 index 000000000000..b38eac11a962 --- /dev/null +++ b/products/replay_vision/frontend/components/CitedMarkdown.tsx @@ -0,0 +1,38 @@ +import { useMemo } from 'react' + +import { LemonMarkdown } from 'lib/lemon-ui/LemonMarkdown' + +import { citedMarkdown } from '../utils/citations' +import { TimestampCitation } from './TimestampCitation' + +/** + * A scanner's cited free-text output, rendered as markdown with its citations as seek controls. + * + * Scanners write reasoning that carries structure — a lead-in naming what they found, then the evidence — + * and that structure only reads as structure once it is rendered. Images are treated as untrusted: the + * text is model output derived from a customer recording, so a page that talked the model into writing an + * image tag must not get to fire a request from the reader's browser. + */ +export function CitedMarkdown({ + text, + segments, + onSeek, +}: { + text: string + segments: unknown + /** Called with timestamp_ms when a citation is clicked. If omitted, citations render as plain timestamps. */ + onSeek?: (timestampMs: number) => void +}): JSX.Element { + const markdown = useMemo(() => citedMarkdown(text, segments), [text, segments]) + return ( + } + > + {markdown} + + ) +} diff --git a/products/replay_vision/frontend/components/ObservationCard.tsx b/products/replay_vision/frontend/components/ObservationCard.tsx index 0602838399bf..c72782431da3 100644 --- a/products/replay_vision/frontend/components/ObservationCard.tsx +++ b/products/replay_vision/frontend/components/ObservationCard.tsx @@ -1,8 +1,7 @@ -import { IconCopy, IconRewindPlay, IconSparkles } from '@posthog/icons' +import { IconCopy, IconSparkles } from '@posthog/icons' import { LemonButton, LemonTag, Link, Spinner, Tooltip } from '@posthog/lemon-ui' import { copyToClipboard } from 'lib/utils/copyToClipboard' -import { colonDelimitedDuration } from 'lib/utils/durations' import { urls } from 'scenes/urls' import type { ReplayObservationApi } from '../generated/api.schemas' @@ -18,10 +17,12 @@ import { } from '../replay_scanners/types' import { citedTextToPlainText, parseCitedSegments } from '../utils/citations' import { readReasoning, scannerLabel } from '../utils/observation' +import { CitedMarkdown } from './CitedMarkdown' import { LabeledRow } from './LabeledRow' import { ObservationProgressBar } from './ObservationProgressBar' import { ObservationRetryButton } from './ObservationRetryButton' import { ScannerTypeBadge } from './ScannerTypeBadge' +import { TimestampCitation } from './TimestampCitation' export function ObservationStatusTag({ status, @@ -99,26 +100,13 @@ export function CitedText({ } return ( <> - {list.map((segment, i) => { - if (segment.kind === 'text') { - return {segment.value} - } - const seconds = Math.max(0, Math.floor(segment.timestamp_ms / 1000)) - const label = colonDelimitedDuration(seconds, null) - if (onSeek) { - return ( - onSeek(segment.timestamp_ms)} className="ml-0.5"> - - {label} - - ) - } - return ( - - {label} - + {list.map((segment, i) => + segment.kind === 'text' ? ( + {segment.value} + ) : ( + ) - })} + )} ) } @@ -496,9 +484,7 @@ export function ObservationDockCard({ {reasoning && ( -

- -

+ )} diff --git a/products/replay_vision/frontend/components/TimestampCitation.tsx b/products/replay_vision/frontend/components/TimestampCitation.tsx new file mode 100644 index 000000000000..7254e0bfffc0 --- /dev/null +++ b/products/replay_vision/frontend/components/TimestampCitation.tsx @@ -0,0 +1,24 @@ +import { IconRewindPlay } from '@posthog/icons' +import { Link } from '@posthog/lemon-ui' + +import { colonDelimitedDuration } from 'lib/utils/durations' + +/** One cited moment: a seek control into the recording, or a plain timestamp where no player is on screen. */ +export function TimestampCitation({ + timestampMs, + onSeek, +}: { + timestampMs: number + onSeek?: (timestampMs: number) => void +}): JSX.Element { + const label = colonDelimitedDuration(Math.max(0, Math.floor(timestampMs / 1000)), null) + if (!onSeek) { + return {label} + } + return ( + onSeek(timestampMs)} className="ml-0.5" data-attr="vision-observation-citation"> + + {label} + + ) +} diff --git a/products/replay_vision/frontend/observations/ReplayObservation.tsx b/products/replay_vision/frontend/observations/ReplayObservation.tsx index 935fae29ef98..1c1f54abe6c8 100644 --- a/products/replay_vision/frontend/observations/ReplayObservation.tsx +++ b/products/replay_vision/frontend/observations/ReplayObservation.tsx @@ -40,9 +40,9 @@ import { ProductKey } from '~/queries/schema/schema-general' import { BooleanTag } from '../components/BooleanTag' import { CardHeader } from '../components/CardHeader' +import { CitedMarkdown } from '../components/CitedMarkdown' import { LabeledRow } from '../components/LabeledRow' import { - CitedText, ObservationConfidence, ObservationPrimaryOutput, ObservationStatusTag, @@ -493,9 +493,7 @@ export function ReplayObservationSceneComponent(): JSX.Element { > } title="Model reasoning" /> {reasoning ? ( -

- -

+ ) : (

{observation.status === 'ineligible' diff --git a/products/replay_vision/frontend/utils/citations.test.ts b/products/replay_vision/frontend/utils/citations.test.ts index bc80bbb1e391..5eb2ddbbcdfc 100644 --- a/products/replay_vision/frontend/utils/citations.test.ts +++ b/products/replay_vision/frontend/utils/citations.test.ts @@ -1,4 +1,4 @@ -import { type Segment, citedTextToPlainText, parseCitedSegments } from './citations' +import { type Segment, citedMarkdown, citedTextToPlainText, parseCitedSegments } from './citations' describe('parseCitedSegments', () => { const text = (value: string): Segment => ({ kind: 'text', value }) @@ -74,3 +74,57 @@ describe('citedTextToPlainText', () => { expect(citedTextToPlainText(text, segments)).toBe(expected) }) }) + +describe('citedMarkdown', () => { + it.each<{ name: string; text: string; segments: unknown; expected: string }>([ + { + name: 'writes a chip back as a t: link target', + text: 'ignored when segments exist', + segments: [ + { kind: 'text', value: 'Abandoned the cart' }, + { kind: 'chip', timestamp_ms: 161_000 }, + { kind: 'text', value: ' and left.' }, + ], + expected: 'Abandoned the cart[02:41](t:161000) and left.', + }, + { + name: 'keeps a block that spans a citation in one document', + text: '', + segments: [ + { kind: 'text', value: '- **Reached payment**' }, + { kind: 'chip', timestamp_ms: 92_000 }, + { kind: 'text', value: '\n- Card rejected' }, + ], + expected: '- **Reached payment**[01:32](t:92000)\n- Card rejected', + }, + { + // `![label](target)` is an image, so a citation glued to a `!` would stop being a citation. + name: 'does not let a trailing exclamation mark turn the citation into an image', + text: '', + segments: [ + { kind: 'text', value: 'Something broke!' }, + { kind: 'chip', timestamp_ms: 5_000 }, + ], + expected: 'Something broke! [00:05](t:5000)', + }, + { + // Recording-derived text: a page that talked the model into writing a URL must not get a + // clickable link out of it. The citation's own `t:` target is the one link that survives. + name: 'leaves the model no way to write a link the reader can click', + text: '', + segments: [ + { kind: 'text', value: 'Saw [an offer](https://evil.example) and https://evil.example/x' }, + { kind: 'chip', timestamp_ms: 5_000 }, + ], + expected: 'Saw an offer and `https://evil.example/x`[00:05](t:5000)', + }, + { + name: 'keeps text without citations untouched', + text: 'No citations here.', + segments: [], + expected: 'No citations here.', + }, + ])('$name', ({ text, segments, expected }) => { + expect(citedMarkdown(text, segments)).toBe(expected) + }) +}) diff --git a/products/replay_vision/frontend/utils/citations.ts b/products/replay_vision/frontend/utils/citations.ts index cf330d850fe0..a4274c86c2e6 100644 --- a/products/replay_vision/frontend/utils/citations.ts +++ b/products/replay_vision/frontend/utils/citations.ts @@ -1,5 +1,8 @@ +import { TIMESTAMP_REF_PREFIX } from 'lib/lemon-ui/LemonMarkdown' import { colonDelimitedDuration } from 'lib/utils/durations' +import { defangMarkdownLinks } from './markdown' + // `uuid` is legacy (only old event-uuid citations carry it). Timestamp citations use `timestamp_ms` alone. export type Segment = { kind: 'text'; value: string } | { kind: 'chip'; timestamp_ms: number; uuid?: string } @@ -56,6 +59,36 @@ export function parseCitedSegments(text: string, segments: unknown): Segment[] { return persisted.flatMap((segment) => (segment.kind === 'text' ? splitLeakedCitations(segment.value) : [segment])) } +/** + * Markdown source for a cited field, with each citation chip written back as a `t:` link target + * LemonMarkdown hands to `renderTimestampRef`. + * + * The chips were split out of the prose at scan time, so a renderer that parses the text as markdown has + * to see one string, not a segment list — a bullet or a bold run that spans a citation would otherwise be + * cut in half and parsed as two documents. + * + * Defanging runs on the prose only, before the citations go in, so the `t:` targets this writes are the + * one kind of link that survives. + */ +export function citedMarkdown(text: string, segments: unknown): string { + const list = parseCitedSegments(text, segments) + if (list.length === 0) { + return defangMarkdownLinks(text) + } + let out = '' + for (const segment of list) { + if (segment.kind === 'text') { + out += defangMarkdownLinks(segment.value) + continue + } + // Glued to the preceding word, matching how the chips render outside markdown — except after a + // `!`, where gluing a link turns the pair into an image. One space there reads as normal prose. + const label = colonDelimitedDuration(Math.max(0, Math.floor(segment.timestamp_ms / 1000)), null) + out += `${out.endsWith('!') ? ' ' : ''}[${label}](${TIMESTAMP_REF_PREFIX}${segment.timestamp_ms})` + } + return out +} + /** Plain-text rendering of a cited field for the clipboard: citation chips become readable `(mm:ss)` timestamps. */ export function citedTextToPlainText(text: string, segments: unknown): string { const list = parseCitedSegments(text, segments) diff --git a/products/replay_vision/frontend/utils/markdown.test.ts b/products/replay_vision/frontend/utils/markdown.test.ts new file mode 100644 index 000000000000..75c89a57670e --- /dev/null +++ b/products/replay_vision/frontend/utils/markdown.test.ts @@ -0,0 +1,61 @@ +import { defangMarkdownLinks, flattenMarkdownToLine } from './markdown' + +describe('flattenMarkdownToLine', () => { + it.each<{ name: string; text: string; expected: string }>([ + { name: 'strips bold', text: 'The user **abandoned** checkout.', expected: 'The user abandoned checkout.' }, + { name: 'strips italics', text: 'The user *hesitated* here.', expected: 'The user hesitated here.' }, + { name: 'strips inline code', text: 'Clicked `Submit`.', expected: 'Clicked Submit.' }, + { + name: 'turns a heading and its body into one sentence pair', + text: '## Checkout blocked\nThe form rejected it.', + expected: 'Checkout blocked. The form rejected it.', + }, + { + name: 'reads a bullet list as prose', + text: '- Reached payment\n- Card rejected', + expected: 'Reached payment. Card rejected', + }, + { + name: 'does not double up punctuation the model already wrote', + text: 'Two problems:\n- one\n- two', + expected: 'Two problems: one. two', + }, + { + name: 'strips a link to its label', + text: 'Landed on [pricing](https://e.com).', + expected: 'Landed on pricing.', + }, + { name: 'leaves snake_case alone', text: 'Read team_id_override.', expected: 'Read team_id_override.' }, + { name: 'leaves multiplication alone', text: 'Retried 3 * 4 times.', expected: 'Retried 3 * 4 times.' }, + { name: 'renders plain prose unchanged', text: 'The user gave up.', expected: 'The user gave up.' }, + ])('$name', ({ text, expected }) => { + expect(flattenMarkdownToLine(text)).toBe(expected) + }) + + it('never emits a line break, so a one-line surface stays one line', () => { + expect(flattenMarkdownToLine('# Title\n\n- one\n- two\n\nEnd.')).not.toContain('\n') + }) +}) + +describe('defangMarkdownLinks', () => { + it.each<{ name: string; text: string; expected: string }>([ + { name: 'keeps an inline link label', text: 'Saw [an offer](https://e.example).', expected: 'Saw an offer.' }, + { name: 'keeps an image alt', text: '![a banner](https://e.example/b.png)', expected: 'a banner' }, + { name: 'inerts an autolink', text: 'Went to .', expected: 'Went to `https://e.example`.' }, + // remark-gfm turns a bare URL into a link on its own, so leaving one bare is the same exposure. + { name: 'inerts a bare url', text: 'Went to https://e.example/x.', expected: 'Went to `https://e.example/x`.' }, + { name: 'inerts a www url', text: 'Went to www.e.example.', expected: 'Went to `www.e.example`.' }, + { + name: 'leaves a url already in a code span alone', + text: 'At `https://e.example`.', + expected: 'At `https://e.example`.', + }, + { + name: 'leaves ordinary prose alone', + text: 'The user gave up on /checkout.', + expected: 'The user gave up on /checkout.', + }, + ])('$name', ({ text, expected }) => { + expect(defangMarkdownLinks(text)).toBe(expected) + }) +}) diff --git a/products/replay_vision/frontend/utils/markdown.ts b/products/replay_vision/frontend/utils/markdown.ts new file mode 100644 index 000000000000..1554043ce0e6 --- /dev/null +++ b/products/replay_vision/frontend/utils/markdown.ts @@ -0,0 +1,77 @@ +// Markdown a scanner can emit in its free-text fields. Block markers are matched per line, the way a +// markdown parser reads them; inline markers are matched anywhere on the line. Mirrors the backend's +// `flatten_markdown` (products/replay_vision/backend/observation_formatting.py), which does the same job +// for the surfaces that never reach a browser. +const FENCE_RE = /^ {0,3}(?:```|~~~)/ +const RULE_RE = /^ {0,3}(?:[-*_]\s*){3,}$/ +const HEADING_RE = /^ {0,3}#{1,6}\s+/ +const QUOTE_RE = /^ {0,3}>\s?/ +const BULLET_RE = /^\s*(?:[-*+]|\d{1,9}[.)])\s+/ +const IMAGE_RE = /!\[([^\]]*)\]\([^)]*\)/g +const LINK_RE = /\[([^\]]*)\]\([^)]*\)/g +const CODE_SPAN_RE = /`+([^`]*)`+/g +const STRONG_RE = /(\*\*|__)(\S(?:.*?\S)?)\1/g +const STRIKE_RE = /~~(\S(?:.*?\S)?)~~/g +// The captured leading character and the lookahead keep `snake_case` and `2 * 3 * 4` intact: an +// underscore or star touching a word character on the outside is punctuation the model meant literally, +// not an emphasis delimiter. Captured rather than a lookbehind, which breaks chunk parsing. +const EMPHASIS_RE = /(^|[^\w*_])([*_])(\S(?:.*?\S)?)\2(?![\w*_])/g +const ESCAPE_RE = /\\([\\`*_{}[\]()#+\-.!>~|])/g +// Terminal punctuation, after which one flattened block runs into the next without a sentence break. +const SENTENCE_ENDINGS = '.!?:;,' + +const AUTOLINK_RE = /<(https?:\/\/[^\s>]+)>/gi +// The final character class holds the sentence's own punctuation out of the URL, so `at https://x.` does +// not read the period as part of the address. +const BARE_URL_RE = /(^|[^`\w])((?:https?:\/\/|www\.)[^\s<>)\]`]*[^\s<>)\]`.,;:!?])/gi + +/** + * Model text with every link target removed, keeping the words around it. + * + * A scanner describes a recording; it has no reason to link anywhere, and what it writes is derived from + * pages a stranger controls. Rendering that as markdown is what would turn a URL the model picked up off + * a page — or was talked into writing — into something a reader can click. Labels survive and bare URLs + * become code spans, so the reader still sees what was written; it just isn't a link. + */ +export function defangMarkdownLinks(text: string): string { + return text + .replace(IMAGE_RE, '$1') + .replace(LINK_RE, '$1') + .replace(AUTOLINK_RE, '`$1`') + .replace(BARE_URL_RE, '$1`$2`') +} + +/** + * Markdown-bearing model text folded onto a single line of readable plain prose. + * + * Reasoning renders as markdown wherever there is room for it, but the places that only have room for a + * fragment — a seekbar hover snippet, the dock's collapsed preview — would show the raw `**` and `-` + * instead. Blocks join with a sentence break so a flattened list still reads as prose. + */ +export function flattenMarkdownToLine(text: string): string { + let out = '' + for (const raw of text.split('\n')) { + if (FENCE_RE.test(raw) || RULE_RE.test(raw)) { + continue + } + const block = raw + .replace(HEADING_RE, '') + .replace(QUOTE_RE, '') + .replace(BULLET_RE, '') + .replace(IMAGE_RE, '$1') + .replace(LINK_RE, '$1') + .replace(CODE_SPAN_RE, '$1') + .replace(STRONG_RE, '$2') + .replace(STRIKE_RE, '$1') + .replace(EMPHASIS_RE, '$1$3') + .replace(ESCAPE_RE, '$1') + .split(/\s+/) + .filter(Boolean) + .join(' ') + if (!block) { + continue + } + out += out ? `${SENTENCE_ENDINGS.includes(out.slice(-1)) ? ' ' : '. '}${block}` : block + } + return out +} diff --git a/products/replay_vision/frontend/utils/observation.ts b/products/replay_vision/frontend/utils/observation.ts index c2c667c66683..6da70952526c 100644 --- a/products/replay_vision/frontend/utils/observation.ts +++ b/products/replay_vision/frontend/utils/observation.ts @@ -2,6 +2,7 @@ import { dayjs } from 'lib/dayjs' import { type ReplayObservationApi, ScannerOriginEnumApi, ScannerTypeEnumApi } from '../generated/api.schemas' import { citedTextToPlainText, parseCitedSegments } from './citations' +import { flattenMarkdownToLine } from './markdown' /** The dock's built-in prompt and the observations it produces have to answer to one name. */ export const BUILT_IN_SUMMARY_LABEL = 'Quick summary' @@ -131,16 +132,18 @@ export interface ObservationSeekbarMark { const SNIPPET_MAX_LENGTH = 160 function lastSentence(text: string): string | null { - const trimmed = text.trim() - if (!trimmed) { + // Flattened first: a seekbar mark has room for one line, so markdown syntax would reach the reader as + // literal `**` and `-`, and a bullet's marker would read as the start of the sentence. + const flat = flattenMarkdownToLine(text) + if (!flat) { return null } // No lookbehind regex, it breaks chunk parsing on older browsers. let start = 0 - for (const match of trimmed.matchAll(/[.!?]+\s+/g)) { + for (const match of flat.matchAll(/[.!?]+\s+/g)) { start = match.index + match[0].length } - const last = trimmed + const last = flat .slice(start) .replace(/[.!?]+$/, '') .trim() From 65e96ab3fda3170b0e87a020ece9944414fcfa4e Mon Sep 17 00:00:00 2001 From: Kim Svatos Dugan <147102038+ksvat@users.noreply.github.com> Date: Thu, 27 Aug 2026 13:03:50 -0700 Subject: [PATCH 2/7] fix(replay-vision): hold the no-link line at the renderer, not with regex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stripping link syntax from the markdown source only covered inline `[x](url)`. Reference links, `[x][ref]` with the definition elsewhere in the document, went through untouched and rendered as a live external anchor, as did an image by reference via the untrusted-image click-to-open fallback. LemonMarkdown gains `disableLinks`, which renders every link as its label instead. No markdown form can get past it, because it simply never emits an anchor. The source-level defanging goes away with it. The flatteners keep their own reference-link handling — there the syntax is a readability problem in a one-line snippet, not a live link. Generated-By: PostHog Desktop Task-Id: 072e2d37-89a7-4ab7-90e1-a71f90862523 --- .../lemon-ui/LemonMarkdown/LemonMarkdown.tsx | 34 +++++++++++++-- .../backend/observation_formatting.py | 11 ++++- .../tests/test_observation_formatting.py | 5 +++ .../components/CitedMarkdown.stories.tsx | 29 +++++++++++++ .../frontend/components/CitedMarkdown.tsx | 9 ++-- .../frontend/utils/citations.test.ts | 10 ++--- .../replay_vision/frontend/utils/citations.ts | 11 +++-- .../frontend/utils/markdown.test.ts | 42 ++++++++----------- .../replay_vision/frontend/utils/markdown.ts | 32 +++++--------- 9 files changed, 118 insertions(+), 65 deletions(-) diff --git a/frontend/src/lib/lemon-ui/LemonMarkdown/LemonMarkdown.tsx b/frontend/src/lib/lemon-ui/LemonMarkdown/LemonMarkdown.tsx index 38fcbcce88e0..d30cace66876 100644 --- a/frontend/src/lib/lemon-ui/LemonMarkdown/LemonMarkdown.tsx +++ b/frontend/src/lib/lemon-ui/LemonMarkdown/LemonMarkdown.tsx @@ -89,6 +89,19 @@ export interface LemonMarkdownProps { * as ; every other image is rendered as a plain click-to-open link instead. */ disableImages?: boolean + /** + * Whether to render every link as plain text rather than an anchor, keeping its label. Use for + * content we don't control (e.g. LLM/agent output derived from a page a stranger wrote), where a + * link the reader can click is a phishing vector and the content has no reason to carry one. + * + * Set this rather than stripping link syntax from the source: markdown reaches a link through + * inline `[x](url)`, reference `[x][ref]` with a definition elsewhere in the document, a bare URL + * autolinked by remark-gfm, or an image that falls back to a link — so a sanitizer upstream of the + * parser has to out-guess every one of those forms, while this simply never emits an anchor. + * `renderChartRef` / `renderTimestampRef` targets are unaffected: those are rendered by the + * caller, not linked. + */ + disableLinks?: boolean className?: string wrapCode?: boolean /** @@ -156,6 +169,7 @@ const LemonMarkdownRenderer = memo(function LemonMarkdownRenderer({ lowKeyHeadings = false, disableDocsRedirect = false, disableImages = false, + disableLinks = false, wrapCode = false, codeMaxLines, generateHeadingIds = false, @@ -180,6 +194,9 @@ const LemonMarkdownRenderer = memo(function LemonMarkdownRenderer({ // instead would point the reader at a scheme no browser can follow. return <>{renderChartRef?.(chartId, node?.position?.start?.offset) ?? children} } + if (disableLinks) { + return <>{children} + } return ( {children} @@ -257,14 +274,20 @@ const LemonMarkdownRenderer = memo(function LemonMarkdownRenderer({ }, ...(disableImages ? { - img: ({ src, alt }: any): JSX.Element => - isTrustedPostHogUrl(src) ? ( - {alt} + img: ({ src, alt }: any): JSX.Element => { + if (isTrustedPostHogUrl(src)) { + return {alt} + } + // The click-to-open fallback is itself an anchor, so it has to answer to + // `disableLinks` too — otherwise an image is a way to reach a live link. + return disableLinks ? ( + <>{alt || src} ) : ( {alt || src} - ), + ) + }, } : {}), li: ({ children, node }: any): JSX.Element => { @@ -313,6 +336,7 @@ const LemonMarkdownRenderer = memo(function LemonMarkdownRenderer({ [ disableDocsRedirect, disableImages, + disableLinks, lowKeyHeadings, wrapCode, codeMaxLines, @@ -360,6 +384,7 @@ function LemonMarkdownComponent({ lowKeyHeadings = false, disableDocsRedirect = false, disableImages = false, + disableLinks = false, wrapCode = false, codeMaxLines, generateHeadingIds = false, @@ -374,6 +399,7 @@ function LemonMarkdownComponent({ lowKeyHeadings={lowKeyHeadings} disableDocsRedirect={disableDocsRedirect} disableImages={disableImages} + disableLinks={disableLinks} wrapCode={wrapCode} codeMaxLines={codeMaxLines} generateHeadingIds={generateHeadingIds} diff --git a/products/replay_vision/backend/observation_formatting.py b/products/replay_vision/backend/observation_formatting.py index eebccebca670..86e7ef3e04a4 100644 --- a/products/replay_vision/backend/observation_formatting.py +++ b/products/replay_vision/backend/observation_formatting.py @@ -19,6 +19,14 @@ _BULLET_RE = re.compile(r"^\s*(?:[-*+]|\d{1,9}[.)])\s+") _IMAGE_RE = re.compile(r"!\[([^\]]*)\]\([^)]*\)") _LINK_RE = re.compile(r"\[([^\]]*)\]\([^)]*\)") +# A link reference definition is a whole line of pure syntax, so the line goes. Its `[text][ref]` and +# `[text][]` usages keep their label. Bare `[text]` is left alone: in prose it is far more often a real +# bracket ("clicked [Save]") than a shortcut reference. +# The definition has to be the entire line — a space-free destination and an optional title, nothing +# after. Matching just the prefix would eat a sentence that merely opens the same way, such as +# "[Save]: clicked twice before it took". +_LINK_DEFINITION_RE = re.compile(r"^ {0,3}\[[^\]]+\]:\s*\S+(?:\s+(?:\"[^\"]*\"|'[^']*'|\([^)]*\)))?\s*$") +_REFERENCE_RE = re.compile(r"!?\[([^\]]*)\]\[[^\]]*\]") _CODE_SPAN_RE = re.compile(r"`+([^`]*)`+") _STRONG_RE = re.compile(r"(\*\*|__)(?=\S)(.+?)(?<=\S)\1") _STRIKE_RE = re.compile(r"~~(?=\S)(.+?)(?<=\S)~~") @@ -40,11 +48,12 @@ def flatten_markdown(text: str) -> str: """ lines: list[str] = [] for raw in text.split("\n"): - if _FENCE_RE.match(raw) or _RULE_RE.match(raw): + if _FENCE_RE.match(raw) or _RULE_RE.match(raw) or _LINK_DEFINITION_RE.match(raw): continue line = _BULLET_RE.sub("", _QUOTE_RE.sub("", _HEADING_RE.sub("", raw))) line = _IMAGE_RE.sub(r"\1", line) line = _LINK_RE.sub(r"\1", line) + line = _REFERENCE_RE.sub(r"\1", line) line = _CODE_SPAN_RE.sub(r"\1", line) line = _STRONG_RE.sub(r"\2", line) line = _STRIKE_RE.sub(r"\1", line) diff --git a/products/replay_vision/backend/tests/test_observation_formatting.py b/products/replay_vision/backend/tests/test_observation_formatting.py index 126aa573132a..a0db9bcca3cb 100644 --- a/products/replay_vision/backend/tests/test_observation_formatting.py +++ b/products/replay_vision/backend/tests/test_observation_formatting.py @@ -33,6 +33,11 @@ class TestPlainSnippet: ("numbered", "1. Reached payment\n2. Card rejected", "Reached payment. Card rejected"), ("blockquote", "> The user gave up.", "The user gave up."), ("link", "Landed on [the pricing page](https://example.com/p).", "Landed on the pricing page."), + ("reference_link", "Landed on [pricing][p].\n\n[p]: https://example.com/p", "Landed on pricing."), + # Brackets in prose are far more often literal than a shortcut reference, so they stay. + ("bracketed_prose", "The user clicked [Save].", "The user clicked [Save]."), + # Opens like a reference definition but is a sentence, so the line survives. + ("prose_after_bracket", "[Save]: clicked twice before it took", "[Save]: clicked twice before it took"), ("image", "![a screenshot](https://example.com/x.png) followed.", "a screenshot followed."), ("escaped_star", r"Priced at 5\* the usual.", "Priced at 5* the usual."), ("already_one_sentence_per_line", "Reached payment.\nCard rejected.", "Reached payment. Card rejected."), diff --git a/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx b/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx index 3c3ee282fe5b..e7383418065a 100644 --- a/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx +++ b/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx @@ -1,5 +1,7 @@ import type { Meta, StoryObj } from '@storybook/react' +import { expect } from 'storybook/test' + import { CitedMarkdown } from './CitedMarkdown' const meta: Meta = { @@ -34,3 +36,30 @@ export const PlainProse: Story = { segments: [], }, } + +/** + * Reasoning is model output derived from a page a stranger wrote, so a link in it is a phishing vector. + * Every markdown form that reaches a link is here, including the reference forms that a regex over the + * source misses. The assertion runs in a real browser under `test-storybook`, which is the only place + * this can be checked: `react-markdown` is ESM-only and mocked out under Jest. + */ +export const HostileLinks: Story = { + args: { + text: [ + 'Inline [click here](https://evil.example/phish).', + 'Reference [click here][ref] and collapsed [click here][].', + 'Image by reference ![a banner][img].', + 'Autolink and bare https://evil.example/bare.', + 'Scheme [click here](javascript:alert(1)).', + '', + '[ref]: https://evil.example/phish', + '[img]: https://evil.example/banner.png', + ].join('\n'), + segments: [], + }, + play: async ({ canvasElement }) => { + await expect(canvasElement.querySelectorAll('a')).toHaveLength(0) + // The labels survive as plain text, so the reader still sees what the model wrote. + await expect(canvasElement.textContent).toContain('click here') + }, +} diff --git a/products/replay_vision/frontend/components/CitedMarkdown.tsx b/products/replay_vision/frontend/components/CitedMarkdown.tsx index b38eac11a962..45a27ce4a127 100644 --- a/products/replay_vision/frontend/components/CitedMarkdown.tsx +++ b/products/replay_vision/frontend/components/CitedMarkdown.tsx @@ -9,9 +9,11 @@ import { TimestampCitation } from './TimestampCitation' * A scanner's cited free-text output, rendered as markdown with its citations as seek controls. * * Scanners write reasoning that carries structure — a lead-in naming what they found, then the evidence — - * and that structure only reads as structure once it is rendered. Images are treated as untrusted: the - * text is model output derived from a customer recording, so a page that talked the model into writing an - * image tag must not get to fire a request from the reader's browser. + * and that structure only reads as structure once it is rendered. + * + * The text is model output derived from a customer recording, so it is treated as hostile: an image must + * not get to fire a request from the reader's browser, and no link may be clickable. A citation is the + * one interactive thing here, and it seeks a player rather than navigating anywhere. */ export function CitedMarkdown({ text, @@ -30,6 +32,7 @@ export function CitedMarkdown({ // A heading inside one card's body is a lead-in, not a section of the page. lowKeyHeadings disableImages + disableLinks renderTimestampRef={(timestampMs) => } > {markdown} diff --git a/products/replay_vision/frontend/utils/citations.test.ts b/products/replay_vision/frontend/utils/citations.test.ts index 5eb2ddbbcdfc..2bf8d2a322ec 100644 --- a/products/replay_vision/frontend/utils/citations.test.ts +++ b/products/replay_vision/frontend/utils/citations.test.ts @@ -108,15 +108,15 @@ describe('citedMarkdown', () => { expected: 'Something broke! [00:05](t:5000)', }, { - // Recording-derived text: a page that talked the model into writing a URL must not get a - // clickable link out of it. The citation's own `t:` target is the one link that survives. - name: 'leaves the model no way to write a link the reader can click', + // Sanitizing is the renderer's job (`CitedMarkdown` sets `disableLinks`), so the prose is + // passed through whole — a link here must arrive intact rather than half-stripped. + name: 'passes the prose through untouched, including anything link-shaped', text: '', segments: [ - { kind: 'text', value: 'Saw [an offer](https://evil.example) and https://evil.example/x' }, + { kind: 'text', value: 'Saw [an offer](https://evil.example)' }, { kind: 'chip', timestamp_ms: 5_000 }, ], - expected: 'Saw an offer and `https://evil.example/x`[00:05](t:5000)', + expected: 'Saw [an offer](https://evil.example)[00:05](t:5000)', }, { name: 'keeps text without citations untouched', diff --git a/products/replay_vision/frontend/utils/citations.ts b/products/replay_vision/frontend/utils/citations.ts index a4274c86c2e6..42ca39706764 100644 --- a/products/replay_vision/frontend/utils/citations.ts +++ b/products/replay_vision/frontend/utils/citations.ts @@ -1,8 +1,6 @@ import { TIMESTAMP_REF_PREFIX } from 'lib/lemon-ui/LemonMarkdown' import { colonDelimitedDuration } from 'lib/utils/durations' -import { defangMarkdownLinks } from './markdown' - // `uuid` is legacy (only old event-uuid citations carry it). Timestamp citations use `timestamp_ms` alone. export type Segment = { kind: 'text'; value: string } | { kind: 'chip'; timestamp_ms: number; uuid?: string } @@ -67,18 +65,19 @@ export function parseCitedSegments(text: string, segments: unknown): Segment[] { * to see one string, not a segment list — a bullet or a bold run that spans a citation would otherwise be * cut in half and parsed as two documents. * - * Defanging runs on the prose only, before the citations go in, so the `t:` targets this writes are the - * one kind of link that survives. + * Nothing here sanitizes the prose. The renderer is what holds the line on links and images (see + * `CitedMarkdown`): stripping syntax at this layer would have to out-guess every markdown form that + * reaches a link, and reference-style `[x][ref]` alone already defeats that. */ export function citedMarkdown(text: string, segments: unknown): string { const list = parseCitedSegments(text, segments) if (list.length === 0) { - return defangMarkdownLinks(text) + return text } let out = '' for (const segment of list) { if (segment.kind === 'text') { - out += defangMarkdownLinks(segment.value) + out += segment.value continue } // Glued to the preceding word, matching how the chips render outside markdown — except after a diff --git a/products/replay_vision/frontend/utils/markdown.test.ts b/products/replay_vision/frontend/utils/markdown.test.ts index 75c89a57670e..b4c69e8da04f 100644 --- a/products/replay_vision/frontend/utils/markdown.test.ts +++ b/products/replay_vision/frontend/utils/markdown.test.ts @@ -1,4 +1,4 @@ -import { defangMarkdownLinks, flattenMarkdownToLine } from './markdown' +import { flattenMarkdownToLine } from './markdown' describe('flattenMarkdownToLine', () => { it.each<{ name: string; text: string; expected: string }>([ @@ -25,6 +25,23 @@ describe('flattenMarkdownToLine', () => { text: 'Landed on [pricing](https://e.com).', expected: 'Landed on pricing.', }, + { + name: 'strips a reference link to its label and drops the definition line', + text: 'Landed on [pricing][p].\n\n[p]: https://e.example/pricing', + expected: 'Landed on pricing.', + }, + { + // Brackets in prose are far more often literal than a shortcut reference, so they stay. + name: 'leaves bracketed prose alone', + text: 'The user clicked [Save].', + expected: 'The user clicked [Save].', + }, + { + // Opens like a reference definition but is a sentence, so the line survives. + name: 'keeps a sentence that merely opens like a reference definition', + text: '[Save]: clicked twice before it took', + expected: '[Save]: clicked twice before it took', + }, { name: 'leaves snake_case alone', text: 'Read team_id_override.', expected: 'Read team_id_override.' }, { name: 'leaves multiplication alone', text: 'Retried 3 * 4 times.', expected: 'Retried 3 * 4 times.' }, { name: 'renders plain prose unchanged', text: 'The user gave up.', expected: 'The user gave up.' }, @@ -36,26 +53,3 @@ describe('flattenMarkdownToLine', () => { expect(flattenMarkdownToLine('# Title\n\n- one\n- two\n\nEnd.')).not.toContain('\n') }) }) - -describe('defangMarkdownLinks', () => { - it.each<{ name: string; text: string; expected: string }>([ - { name: 'keeps an inline link label', text: 'Saw [an offer](https://e.example).', expected: 'Saw an offer.' }, - { name: 'keeps an image alt', text: '![a banner](https://e.example/b.png)', expected: 'a banner' }, - { name: 'inerts an autolink', text: 'Went to .', expected: 'Went to `https://e.example`.' }, - // remark-gfm turns a bare URL into a link on its own, so leaving one bare is the same exposure. - { name: 'inerts a bare url', text: 'Went to https://e.example/x.', expected: 'Went to `https://e.example/x`.' }, - { name: 'inerts a www url', text: 'Went to www.e.example.', expected: 'Went to `www.e.example`.' }, - { - name: 'leaves a url already in a code span alone', - text: 'At `https://e.example`.', - expected: 'At `https://e.example`.', - }, - { - name: 'leaves ordinary prose alone', - text: 'The user gave up on /checkout.', - expected: 'The user gave up on /checkout.', - }, - ])('$name', ({ text, expected }) => { - expect(defangMarkdownLinks(text)).toBe(expected) - }) -}) diff --git a/products/replay_vision/frontend/utils/markdown.ts b/products/replay_vision/frontend/utils/markdown.ts index 1554043ce0e6..5fda1b9ba2ce 100644 --- a/products/replay_vision/frontend/utils/markdown.ts +++ b/products/replay_vision/frontend/utils/markdown.ts @@ -9,6 +9,14 @@ const QUOTE_RE = /^ {0,3}>\s?/ const BULLET_RE = /^\s*(?:[-*+]|\d{1,9}[.)])\s+/ const IMAGE_RE = /!\[([^\]]*)\]\([^)]*\)/g const LINK_RE = /\[([^\]]*)\]\([^)]*\)/g +// A link reference definition is a whole line of pure syntax, so the line goes. Its `[text][ref]` and +// `[text][]` usages keep their label. Bare `[text]` is left alone: in prose it is far more often a real +// bracket ("clicked [Save]") than a shortcut reference. +// The definition has to be the entire line — a space-free destination and an optional title, nothing +// after. Matching just the prefix would eat a sentence that merely opens the same way, such as +// "[Save]: clicked twice before it took". +const LINK_DEFINITION_RE = /^ {0,3}\[[^\]]+\]:\s*\S+(?:\s+(?:"[^"]*"|'[^']*'|\([^)]*\)))?\s*$/ +const REFERENCE_RE = /!?\[([^\]]*)\]\[[^\]]*\]/g const CODE_SPAN_RE = /`+([^`]*)`+/g const STRONG_RE = /(\*\*|__)(\S(?:.*?\S)?)\1/g const STRIKE_RE = /~~(\S(?:.*?\S)?)~~/g @@ -20,27 +28,6 @@ const ESCAPE_RE = /\\([\\`*_{}[\]()#+\-.!>~|])/g // Terminal punctuation, after which one flattened block runs into the next without a sentence break. const SENTENCE_ENDINGS = '.!?:;,' -const AUTOLINK_RE = /<(https?:\/\/[^\s>]+)>/gi -// The final character class holds the sentence's own punctuation out of the URL, so `at https://x.` does -// not read the period as part of the address. -const BARE_URL_RE = /(^|[^`\w])((?:https?:\/\/|www\.)[^\s<>)\]`]*[^\s<>)\]`.,;:!?])/gi - -/** - * Model text with every link target removed, keeping the words around it. - * - * A scanner describes a recording; it has no reason to link anywhere, and what it writes is derived from - * pages a stranger controls. Rendering that as markdown is what would turn a URL the model picked up off - * a page — or was talked into writing — into something a reader can click. Labels survive and bare URLs - * become code spans, so the reader still sees what was written; it just isn't a link. - */ -export function defangMarkdownLinks(text: string): string { - return text - .replace(IMAGE_RE, '$1') - .replace(LINK_RE, '$1') - .replace(AUTOLINK_RE, '`$1`') - .replace(BARE_URL_RE, '$1`$2`') -} - /** * Markdown-bearing model text folded onto a single line of readable plain prose. * @@ -51,7 +38,7 @@ export function defangMarkdownLinks(text: string): string { export function flattenMarkdownToLine(text: string): string { let out = '' for (const raw of text.split('\n')) { - if (FENCE_RE.test(raw) || RULE_RE.test(raw)) { + if (FENCE_RE.test(raw) || RULE_RE.test(raw) || LINK_DEFINITION_RE.test(raw)) { continue } const block = raw @@ -60,6 +47,7 @@ export function flattenMarkdownToLine(text: string): string { .replace(BULLET_RE, '') .replace(IMAGE_RE, '$1') .replace(LINK_RE, '$1') + .replace(REFERENCE_RE, '$1') .replace(CODE_SPAN_RE, '$1') .replace(STRONG_RE, '$2') .replace(STRIKE_RE, '$1') From 9dfc9de678f3dc220ff57c02a47cfd168dde78ea Mon Sep 17 00:00:00 2001 From: Hayne Ryu Date: Mon, 7 Sep 2026 18:50:39 +0100 Subject: [PATCH 3/7] keep mentions and same-origin images out of scanner reasoning --- .../lemon-ui/LemonMarkdown/LemonMarkdown.tsx | 31 ++++++++++++++----- .../tool/widgets/ReplayVisionScanWidget.tsx | 7 +++-- .../backend/observation_formatting.py | 12 ++++--- .../tests/test_observation_formatting.py | 2 ++ .../components/CitedMarkdown.stories.tsx | 17 +++++++--- .../frontend/components/CitedMarkdown.tsx | 8 +++-- .../frontend/utils/citations.test.ts | 10 ++++++ .../replay_vision/frontend/utils/citations.ts | 7 +++-- .../frontend/utils/markdown.test.ts | 6 ++++ 9 files changed, 75 insertions(+), 25 deletions(-) diff --git a/frontend/src/lib/lemon-ui/LemonMarkdown/LemonMarkdown.tsx b/frontend/src/lib/lemon-ui/LemonMarkdown/LemonMarkdown.tsx index d30cace66876..29e177822867 100644 --- a/frontend/src/lib/lemon-ui/LemonMarkdown/LemonMarkdown.tsx +++ b/frontend/src/lib/lemon-ui/LemonMarkdown/LemonMarkdown.tsx @@ -87,8 +87,18 @@ export interface LemonMarkdownProps { * URL — leaking the viewer's IP, acting as a tracking pixel, or probing internal addresses. * When set, only images served from PostHog (same-origin or a `posthog.com` host) render inline * as ; every other image is rendered as a plain click-to-open link instead. + * + * `'all'` drops that exception too, so no is emitted at all. Use it for content that has no + * business carrying an image: a same-origin `src` is still a credentialed GET the content gets to + * aim at any path on this host, which is not something to hand model output. + */ + disableImages?: boolean | 'all' + /** + * Whether to leave `@member:` / `@role:` as plain text rather than resolving them into + * mention chips. Use for content we don't control: a chip names a real colleague, tooltip with + * their email included, so without this the content gets to fake a mention of anyone in the org. */ - disableImages?: boolean + disableMentions?: boolean /** * Whether to render every link as plain text rather than an anchor, keeping its label. Use for * content we don't control (e.g. LLM/agent output derived from a page a stranger wrote), where a @@ -170,6 +180,7 @@ const LemonMarkdownRenderer = memo(function LemonMarkdownRenderer({ disableDocsRedirect = false, disableImages = false, disableLinks = false, + disableMentions = false, wrapCode = false, codeMaxLines, generateHeadingIds = false, @@ -275,7 +286,7 @@ const LemonMarkdownRenderer = memo(function LemonMarkdownRenderer({ ...(disableImages ? { img: ({ src, alt }: any): JSX.Element => { - if (isTrustedPostHogUrl(src)) { + if (disableImages !== 'all' && isTrustedPostHogUrl(src)) { return {alt} } // The click-to-open fallback is itself an anchor, so it has to answer to @@ -365,14 +376,16 @@ const LemonMarkdownRenderer = memo(function LemonMarkdownRenderer({ // remark-breaks: a single newline becomes a line break, so prose authored without the arcane // two-trailing-spaces hard-break rule (e.g. agent-written report summaries) renders with the // line breaks the author intended. + // Mentions are gated at the plugin: with it out of the pipeline no `ph-mention` node is ever + // produced, and `skipHtml` already keeps a hand-written one out. + const remarkPlugins = useMemo( + () => (disableMentions ? [remarkGfm, remarkBreaks] : [remarkGfm, remarkBreaks, remarkMentions]), + [disableMentions] + ) + return ( /* eslint-disable-next-line react/forbid-elements */ - + {children} ) @@ -385,6 +398,7 @@ function LemonMarkdownComponent({ disableDocsRedirect = false, disableImages = false, disableLinks = false, + disableMentions = false, wrapCode = false, codeMaxLines, generateHeadingIds = false, @@ -400,6 +414,7 @@ function LemonMarkdownComponent({ disableDocsRedirect={disableDocsRedirect} disableImages={disableImages} disableLinks={disableLinks} + disableMentions={disableMentions} wrapCode={wrapCode} codeMaxLines={codeMaxLines} generateHeadingIds={generateHeadingIds} diff --git a/products/posthog_ai/frontend/components/tool/widgets/ReplayVisionScanWidget.tsx b/products/posthog_ai/frontend/components/tool/widgets/ReplayVisionScanWidget.tsx index eaa97fa98d23..77d5616838ce 100644 --- a/products/posthog_ai/frontend/components/tool/widgets/ReplayVisionScanWidget.tsx +++ b/products/posthog_ai/frontend/components/tool/widgets/ReplayVisionScanWidget.tsx @@ -5,6 +5,7 @@ import { LemonBanner, Link, Spinner } from '@posthog/lemon-ui' import { urls } from 'scenes/urls' import type { ReplayObservationApi } from 'products/replay_vision/frontend/generated/api.schemas' +import { flattenMarkdownToLine } from 'products/replay_vision/frontend/utils/markdown' import { readErrorMessage, readReasoning, @@ -98,9 +99,11 @@ function ObservationRow({ observation }: { observation: ReplayObservationApi }): ) } - // Summarizers write a title and a body; a monitor scan on the same widget has reasoning instead. + // Summarizers write a title and a body; a monitor scan on the same widget has reasoning instead. The + // reasoning can carry markdown, and this widget has one paragraph of plain text to show it in. const title = readTitle(observation) - const body = readSummary(observation) ?? readReasoning(observation) + const reasoning = readReasoning(observation) + const body = readSummary(observation) ?? (reasoning ? flattenMarkdownToLine(reasoning) : null) return (

diff --git a/products/replay_vision/backend/observation_formatting.py b/products/replay_vision/backend/observation_formatting.py index 86e7ef3e04a4..8a588ff8961f 100644 --- a/products/replay_vision/backend/observation_formatting.py +++ b/products/replay_vision/backend/observation_formatting.py @@ -11,11 +11,13 @@ SEARCH_SNIPPET_LIMIT = 600 # Markdown a scanner can emit in its free-text fields. Block markers are matched per line, the way a -# markdown parser reads them; inline markers are matched anywhere on the line. -_FENCE_RE = re.compile(r"^\s{0,3}(?:```|~~~)") -_RULE_RE = re.compile(r"^\s{0,3}(?:[-*_]\s*){3,}$") -_HEADING_RE = re.compile(r"^\s{0,3}#{1,6}\s+") -_QUOTE_RE = re.compile(r"^\s{0,3}>\s?") +# markdown parser reads them; inline markers are matched anywhere on the line. Kept identical to the +# frontend's `flattenMarkdownToLine` (products/replay_vision/frontend/utils/markdown.ts): the two have to +# agree on what is syntax, or a snippet flattens differently in a Slack message and in the seekbar. +_FENCE_RE = re.compile(r"^ {0,3}(?:```|~~~)") +_RULE_RE = re.compile(r"^ {0,3}(?:[-*_]\s*){3,}$") +_HEADING_RE = re.compile(r"^ {0,3}#{1,6}\s+") +_QUOTE_RE = re.compile(r"^ {0,3}>\s?") _BULLET_RE = re.compile(r"^\s*(?:[-*+]|\d{1,9}[.)])\s+") _IMAGE_RE = re.compile(r"!\[([^\]]*)\]\([^)]*\)") _LINK_RE = re.compile(r"\[([^\]]*)\]\([^)]*\)") diff --git a/products/replay_vision/backend/tests/test_observation_formatting.py b/products/replay_vision/backend/tests/test_observation_formatting.py index a0db9bcca3cb..d4db6f2b8121 100644 --- a/products/replay_vision/backend/tests/test_observation_formatting.py +++ b/products/replay_vision/backend/tests/test_observation_formatting.py @@ -40,6 +40,8 @@ class TestPlainSnippet: ("prose_after_bracket", "[Save]: clicked twice before it took", "[Save]: clicked twice before it took"), ("image", "![a screenshot](https://example.com/x.png) followed.", "a screenshot followed."), ("escaped_star", r"Priced at 5\* the usual.", "Priced at 5* the usual."), + # Only spaces indent a block marker: a tab makes it literal text, in the browser flattener too. + ("tab_indented_heading", "\t# literal hash", "# literal hash"), ("already_one_sentence_per_line", "Reached payment.\nCard rejected.", "Reached payment. Card rejected."), ("existing_terminal_punctuation", "Two problems:\n- one\n- two", "Two problems: one. two"), ] diff --git a/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx b/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx index e7383418065a..fd02af40eee3 100644 --- a/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx +++ b/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx @@ -38,10 +38,12 @@ export const PlainProse: Story = { } /** - * Reasoning is model output derived from a page a stranger wrote, so a link in it is a phishing vector. - * Every markdown form that reaches a link is here, including the reference forms that a regex over the - * source misses. The assertion runs in a real browser under `test-storybook`, which is the only place - * this can be checked: `react-markdown` is ESM-only and mocked out under Jest. + * Reasoning is model output derived from a page a stranger wrote, so a link in it is a phishing vector, an + * image is a request it gets to aim from the reader's browser, and a mention is a colleague it gets to + * name. Every markdown form that reaches one of those is here, including the reference forms that a regex + * over the source misses and the same-origin image that the ordinary `disableImages` lets through. The + * assertion runs in a real browser under `test-storybook`, which is the only place this can be checked: + * `react-markdown` is ESM-only and mocked out under Jest. */ export const HostileLinks: Story = { args: { @@ -49,8 +51,10 @@ export const HostileLinks: Story = { 'Inline [click here](https://evil.example/phish).', 'Reference [click here][ref] and collapsed [click here][].', 'Image by reference ![a banner][img].', + 'Same-origin image ![a probe](/api/projects/@current/session_recordings) fires a credentialed GET.', 'Autolink and bare https://evil.example/bare.', 'Scheme [click here](javascript:alert(1)).', + 'Mentions @member:1 and @role:1 name whoever holds those ids.', '', '[ref]: https://evil.example/phish', '[img]: https://evil.example/banner.png', @@ -59,7 +63,10 @@ export const HostileLinks: Story = { }, play: async ({ canvasElement }) => { await expect(canvasElement.querySelectorAll('a')).toHaveLength(0) - // The labels survive as plain text, so the reader still sees what the model wrote. + await expect(canvasElement.querySelectorAll('img')).toHaveLength(0) + // The labels and the raw mention syntax survive as plain text, so the reader still sees what the + // model wrote. A chip would show the member's name in its place. await expect(canvasElement.textContent).toContain('click here') + await expect(canvasElement.textContent).toContain('@member:1 and @role:1') }, } diff --git a/products/replay_vision/frontend/components/CitedMarkdown.tsx b/products/replay_vision/frontend/components/CitedMarkdown.tsx index 45a27ce4a127..4996e5296e0f 100644 --- a/products/replay_vision/frontend/components/CitedMarkdown.tsx +++ b/products/replay_vision/frontend/components/CitedMarkdown.tsx @@ -12,8 +12,9 @@ import { TimestampCitation } from './TimestampCitation' * and that structure only reads as structure once it is rendered. * * The text is model output derived from a customer recording, so it is treated as hostile: an image must - * not get to fire a request from the reader's browser, and no link may be clickable. A citation is the - * one interactive thing here, and it seeks a player rather than navigating anywhere. + * not get to fire a request from the reader's browser, no link may be clickable, and an `@member:` in + * it must stay text rather than become a chip naming a real colleague. A citation is the one interactive + * thing here, and it seeks a player rather than navigating anywhere. */ export function CitedMarkdown({ text, @@ -31,8 +32,9 @@ export function CitedMarkdown({ className="text-sm" // A heading inside one card's body is a lead-in, not a section of the page. lowKeyHeadings - disableImages + disableImages="all" disableLinks + disableMentions renderTimestampRef={(timestampMs) => } > {markdown} diff --git a/products/replay_vision/frontend/utils/citations.test.ts b/products/replay_vision/frontend/utils/citations.test.ts index 2bf8d2a322ec..46083a73a49b 100644 --- a/products/replay_vision/frontend/utils/citations.test.ts +++ b/products/replay_vision/frontend/utils/citations.test.ts @@ -118,6 +118,16 @@ describe('citedMarkdown', () => { ], expected: 'Saw [an offer](https://evil.example)[00:05](t:5000)', }, + { + // `t:` takes digits only, so anything else in the target would render as a dead label. + name: 'rounds a fractional offset and clamps a negative one so the target stays parseable', + text: '', + segments: [ + { kind: 'chip', timestamp_ms: 92_000.6 }, + { kind: 'chip', timestamp_ms: -5 }, + ], + expected: '[01:32](t:92001)[00:00](t:0)', + }, { name: 'keeps text without citations untouched', text: 'No citations here.', diff --git a/products/replay_vision/frontend/utils/citations.ts b/products/replay_vision/frontend/utils/citations.ts index 42ca39706764..838855aa4233 100644 --- a/products/replay_vision/frontend/utils/citations.ts +++ b/products/replay_vision/frontend/utils/citations.ts @@ -82,8 +82,11 @@ export function citedMarkdown(text: string, segments: unknown): string { } // Glued to the preceding word, matching how the chips render outside markdown — except after a // `!`, where gluing a link turns the pair into an image. One space there reads as normal prose. - const label = colonDelimitedDuration(Math.max(0, Math.floor(segment.timestamp_ms / 1000)), null) - out += `${out.endsWith('!') ? ' ' : ''}[${label}](${TIMESTAMP_REF_PREFIX}${segment.timestamp_ms})` + // The target has to be a whole non-negative number: LemonMarkdown reads `t:` and nothing + // else, so a fractional or negative offset would silently render as a dead label. + const timestampMs = Math.max(0, Math.round(segment.timestamp_ms)) + const label = colonDelimitedDuration(Math.floor(timestampMs / 1000), null) + out += `${out.endsWith('!') ? ' ' : ''}[${label}](${TIMESTAMP_REF_PREFIX}${timestampMs})` } return out } diff --git a/products/replay_vision/frontend/utils/markdown.test.ts b/products/replay_vision/frontend/utils/markdown.test.ts index b4c69e8da04f..405d9a68a264 100644 --- a/products/replay_vision/frontend/utils/markdown.test.ts +++ b/products/replay_vision/frontend/utils/markdown.test.ts @@ -10,6 +10,12 @@ describe('flattenMarkdownToLine', () => { text: '## Checkout blocked\nThe form rejected it.', expected: 'Checkout blocked. The form rejected it.', }, + { + // Only spaces indent a block marker: a tab makes it literal text, in the backend flattener too. + name: 'leaves a tab-indented hash as text', + text: '\t# literal hash', + expected: '# literal hash', + }, { name: 'reads a bullet list as prose', text: '- Reached payment\n- Card rejected', From 394e72c82465e7d3d543d295af0d4df5fa0a20bf Mon Sep 17 00:00:00 2001 From: Hayne Ryu Date: Mon, 7 Sep 2026 18:59:55 +0100 Subject: [PATCH 4/7] resolve story test --- .../backend/observation_formatting.py | 12 +++++------ .../tests/test_observation_formatting.py | 4 ++-- .../components/CitedMarkdown.stories.tsx | 20 ++++++++++++------- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/products/replay_vision/backend/observation_formatting.py b/products/replay_vision/backend/observation_formatting.py index 8a588ff8961f..cb333d78fa26 100644 --- a/products/replay_vision/backend/observation_formatting.py +++ b/products/replay_vision/backend/observation_formatting.py @@ -13,7 +13,7 @@ # Markdown a scanner can emit in its free-text fields. Block markers are matched per line, the way a # markdown parser reads them; inline markers are matched anywhere on the line. Kept identical to the # frontend's `flattenMarkdownToLine` (products/replay_vision/frontend/utils/markdown.ts): the two have to -# agree on what is syntax, or a snippet flattens differently in a Slack message and in the seekbar. +# agree on what is syntax, or a snippet flattens differently in a search result and on the seekbar. _FENCE_RE = re.compile(r"^ {0,3}(?:```|~~~)") _RULE_RE = re.compile(r"^ {0,3}(?:[-*_]\s*){3,}$") _HEADING_RE = re.compile(r"^ {0,3}#{1,6}\s+") @@ -67,11 +67,11 @@ def flatten_markdown(text: str) -> str: def plain_snippet(text: str, *, limit: int | None = SEARCH_SNIPPET_LIMIT) -> str: """One line of readable plain text from a model's free-text field. - The single line is a safety property, not tidiness. These snippets are embedded inside markdown the - reader trusts — a Slack alert's bullet list, the untrusted-data fence in the synthesis prompt — where a - newline lets recording-derived text forge a row or a header. Flattening runs first so a markdown bullet - cannot survive as a literal `-` at the start of the folded line either. Blocks are joined with a - sentence break so a flattened list still reads as prose rather than one run-on clause. + The single line is a safety property, not tidiness. These snippets go into markdown someone else reads + as structure — the search-result bullet list inside a Max tool's untrusted-data fence — where a newline + lets recording-derived text forge a row or a header. Flattening runs first so a markdown bullet cannot + survive as a literal `-` at the start of the folded line either. Blocks are joined with a sentence + break so a flattened list still reads as prose rather than one run-on clause. """ flat = flatten_markdown(EVENT_ID_CITATION_RE.sub("", text)) out = "" diff --git a/products/replay_vision/backend/tests/test_observation_formatting.py b/products/replay_vision/backend/tests/test_observation_formatting.py index d4db6f2b8121..160ce3559ffd 100644 --- a/products/replay_vision/backend/tests/test_observation_formatting.py +++ b/products/replay_vision/backend/tests/test_observation_formatting.py @@ -66,8 +66,8 @@ def test_leaves_literal_punctuation_alone(self, _label: str, text: str) -> None: ] ) def test_never_emits_a_line_break_or_a_leading_block_marker(self, _label: str, text: str) -> None: - # Alert messages and the synthesis feed embed this inside markdown the reader trusts, so a newline - # or a leading marker would let recording-derived text forge a row or a header there. + # The search-result feed embeds this inside markdown that is read as structure, so a newline or a + # leading marker would let recording-derived text forge a row or a header there. snippet = plain_snippet(text) assert "\n" not in snippet assert not snippet.startswith(("-", "*", "#", ">")) diff --git a/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx b/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx index fd02af40eee3..bfa5f6e5a6d4 100644 --- a/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx +++ b/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx @@ -1,7 +1,5 @@ import type { Meta, StoryObj } from '@storybook/react' -import { expect } from 'storybook/test' - import { CitedMarkdown } from './CitedMarkdown' const meta: Meta = { @@ -61,12 +59,20 @@ export const HostileLinks: Story = { ].join('\n'), segments: [], }, - play: async ({ canvasElement }) => { - await expect(canvasElement.querySelectorAll('a')).toHaveLength(0) - await expect(canvasElement.querySelectorAll('img')).toHaveLength(0) + // `storybook/test` is a dependency of the frontend workspace, which a file under `products/` cannot + // resolve, so the assertions are plain throws. A play function that throws is a failed story. + play: ({ canvasElement }) => { + const check = (ok: boolean, failure: string): void => { + if (!ok) { + throw new Error(failure) + } + } + const text = canvasElement.textContent ?? '' + check(canvasElement.querySelectorAll('a').length === 0, 'a clickable link reached the reader') + check(canvasElement.querySelectorAll('img').length === 0, 'an image fired a request') // The labels and the raw mention syntax survive as plain text, so the reader still sees what the // model wrote. A chip would show the member's name in its place. - await expect(canvasElement.textContent).toContain('click here') - await expect(canvasElement.textContent).toContain('@member:1 and @role:1') + check(text.includes('click here'), 'a link label was dropped instead of kept as text') + check(text.includes('@member:1 and @role:1'), 'a mention resolved into a chip') }, } From 8debc5f8dd9627c099e16daea1865f99396cdfe6 Mon Sep 17 00:00:00 2001 From: Hayne Ryu Date: Mon, 7 Sep 2026 19:08:57 +0100 Subject: [PATCH 5/7] cleanup --- .../lemon-ui/LemonMarkdown/LemonMarkdown.tsx | 54 ++++--------------- .../tool/widgets/ReplayVisionScanWidget.tsx | 3 +- .../backend/observation_formatting.py | 22 ++++---- .../tests/test_observation_formatting.py | 6 --- .../components/CitedMarkdown.stories.tsx | 15 +----- .../frontend/components/CitedMarkdown.tsx | 13 +---- .../frontend/utils/citations.test.ts | 4 -- .../replay_vision/frontend/utils/citations.ts | 18 ++----- .../frontend/utils/markdown.test.ts | 3 -- .../replay_vision/frontend/utils/markdown.ts | 24 ++------- .../frontend/utils/observation.ts | 3 +- 11 files changed, 30 insertions(+), 135 deletions(-) diff --git a/frontend/src/lib/lemon-ui/LemonMarkdown/LemonMarkdown.tsx b/frontend/src/lib/lemon-ui/LemonMarkdown/LemonMarkdown.tsx index 29e177822867..72cddb8c8df9 100644 --- a/frontend/src/lib/lemon-ui/LemonMarkdown/LemonMarkdown.tsx +++ b/frontend/src/lib/lemon-ui/LemonMarkdown/LemonMarkdown.tsx @@ -31,18 +31,12 @@ function chartRefId(href: unknown): string | null { return typeof href === 'string' ? (CHART_REF_TARGET.exec(href)?.[1] ?? null) : null } -/** - * Link target that marks a moment in a recording, e.g. `[01:32](t:92000)` — the offset in milliseconds. - * Same reasoning as `chart:`: a link rather than a bespoke token, so the same markdown degrades to a - * readable timestamp in the renderers that have no player to seek. - */ +/** Link target for a moment in a recording, e.g. `[01:32](t:92000)`, the offset in milliseconds. */ export const TIMESTAMP_REF_PREFIX = 't:' -// Bounded digits so an overlong target is an ordinary link rather than a reference, keeping it subject -// to the usual URL sanitizing. 12 digits is ~31 years of recording, well past any real session. +// Bounded digits, so an overlong target stays an ordinary link and keeps the usual URL sanitizing. const TIMESTAMP_REF_TARGET = new RegExp(`^${TIMESTAMP_REF_PREFIX}(\\d{1,12})$`) -/** The offset in milliseconds behind a `t:` link target, or null when `href` is an ordinary link. */ function timestampRefMs(href: unknown): number | null { const digits = typeof href === 'string' ? TIMESTAMP_REF_TARGET.exec(href)?.[1] : undefined return digits === undefined ? null : parseInt(digits, 10) @@ -87,29 +81,14 @@ export interface LemonMarkdownProps { * URL — leaking the viewer's IP, acting as a tracking pixel, or probing internal addresses. * When set, only images served from PostHog (same-origin or a `posthog.com` host) render inline * as ; every other image is rendered as a plain click-to-open link instead. - * - * `'all'` drops that exception too, so no is emitted at all. Use it for content that has no - * business carrying an image: a same-origin `src` is still a credentialed GET the content gets to - * aim at any path on this host, which is not something to hand model output. + * `'all'` emits no at all: a same-origin `src` is still a credentialed GET on this host. */ disableImages?: boolean | 'all' - /** - * Whether to leave `@member:` / `@role:` as plain text rather than resolving them into - * mention chips. Use for content we don't control: a chip names a real colleague, tooltip with - * their email included, so without this the content gets to fake a mention of anyone in the org. - */ + /** Whether `@member:` / `@role:` stay plain text instead of chips naming a real colleague. */ disableMentions?: boolean /** - * Whether to render every link as plain text rather than an anchor, keeping its label. Use for - * content we don't control (e.g. LLM/agent output derived from a page a stranger wrote), where a - * link the reader can click is a phishing vector and the content has no reason to carry one. - * - * Set this rather than stripping link syntax from the source: markdown reaches a link through - * inline `[x](url)`, reference `[x][ref]` with a definition elsewhere in the document, a bare URL - * autolinked by remark-gfm, or an image that falls back to a link — so a sanitizer upstream of the - * parser has to out-guess every one of those forms, while this simply never emits an anchor. - * `renderChartRef` / `renderTimestampRef` targets are unaffected: those are rendered by the - * caller, not linked. + * Whether every link renders as its label rather than an anchor. Preferred over stripping link syntax + * from the source, which has to out-guess the inline, reference, autolink and image-fallback forms. */ disableLinks?: boolean className?: string @@ -137,12 +116,7 @@ export interface LemonMarkdownProps { * is a scheme no browser can follow. */ renderChartRef?: (chartId: string, sourceOffset?: number) => React.ReactNode - /** - * Optional renderer for `t:` link targets (see `TIMESTAMP_REF_PREFIX`). Returning null or - * undefined renders the link's own label as plain text, which is the fallback for a reference no - * player is on screen to seek. Omitting this prop renders every timestamp target as its label — - * never as a link, since `t:` is a scheme no browser can follow. - */ + /** Without it, or on a nullish return, a `t:` target renders as its label. */ renderTimestampRef?: (timestampMs: number) => React.ReactNode } @@ -193,8 +167,6 @@ const LemonMarkdownRenderer = memo(function LemonMarkdownRenderer({ a: ({ href, children, node }: any): JSX.Element => { const timestampMs = timestampRefMs(href) if (timestampMs !== null) { - // Falls back to its own label — the timestamp itself — which is what a reader with no - // player on screen needs anyway. return <>{renderTimestampRef?.(timestampMs) ?? children} } const chartId = chartRefId(href) @@ -289,8 +261,7 @@ const LemonMarkdownRenderer = memo(function LemonMarkdownRenderer({ if (disableImages !== 'all' && isTrustedPostHogUrl(src)) { return {alt} } - // The click-to-open fallback is itself an anchor, so it has to answer to - // `disableLinks` too — otherwise an image is a way to reach a live link. + // The click-to-open fallback is an anchor, so it answers to `disableLinks` too. return disableLinks ? ( <>{alt || src} ) : ( @@ -358,12 +329,7 @@ const LemonMarkdownRenderer = memo(function LemonMarkdownRenderer({ ] ) - // Neither `chart:` nor `t:` is a protocol the default URL sanitizer knows, so it strips the href - // before the `a` override ever sees it — and an emptied href is what `Link` turns into a focusable - // target-blank stub that goes nowhere. Let both schemes through whether or not this caller can draw - // charts or seek a player, so the `a` override can recognize the reference and render its label as - // text. Only on `href`: the same transform runs for an image `src`, where nothing downstream reads a - // reference, so `![x](chart:y)` would carry an unsanitized scheme into a broken image. + // The default sanitizer strips both schemes, so let them through, on `href` only. const urlTransform = useMemo( () => (url: string, key: string): string => @@ -376,8 +342,6 @@ const LemonMarkdownRenderer = memo(function LemonMarkdownRenderer({ // remark-breaks: a single newline becomes a line break, so prose authored without the arcane // two-trailing-spaces hard-break rule (e.g. agent-written report summaries) renders with the // line breaks the author intended. - // Mentions are gated at the plugin: with it out of the pipeline no `ph-mention` node is ever - // produced, and `skipHtml` already keeps a hand-written one out. const remarkPlugins = useMemo( () => (disableMentions ? [remarkGfm, remarkBreaks] : [remarkGfm, remarkBreaks, remarkMentions]), [disableMentions] diff --git a/products/posthog_ai/frontend/components/tool/widgets/ReplayVisionScanWidget.tsx b/products/posthog_ai/frontend/components/tool/widgets/ReplayVisionScanWidget.tsx index 77d5616838ce..65432b376c13 100644 --- a/products/posthog_ai/frontend/components/tool/widgets/ReplayVisionScanWidget.tsx +++ b/products/posthog_ai/frontend/components/tool/widgets/ReplayVisionScanWidget.tsx @@ -99,8 +99,7 @@ function ObservationRow({ observation }: { observation: ReplayObservationApi }): ) } - // Summarizers write a title and a body; a monitor scan on the same widget has reasoning instead. The - // reasoning can carry markdown, and this widget has one paragraph of plain text to show it in. + // Summarizers write a title and a body; a monitor scan has reasoning, which can carry markdown. const title = readTitle(observation) const reasoning = readReasoning(observation) const body = readSummary(observation) ?? (reasoning ? flattenMarkdownToLine(reasoning) : null) diff --git a/products/replay_vision/backend/observation_formatting.py b/products/replay_vision/backend/observation_formatting.py index cb333d78fa26..065b780e4483 100644 --- a/products/replay_vision/backend/observation_formatting.py +++ b/products/replay_vision/backend/observation_formatting.py @@ -10,10 +10,6 @@ SEARCH_SNIPPET_LIMIT = 600 -# Markdown a scanner can emit in its free-text fields. Block markers are matched per line, the way a -# markdown parser reads them; inline markers are matched anywhere on the line. Kept identical to the -# frontend's `flattenMarkdownToLine` (products/replay_vision/frontend/utils/markdown.ts): the two have to -# agree on what is syntax, or a snippet flattens differently in a search result and on the seekbar. _FENCE_RE = re.compile(r"^ {0,3}(?:```|~~~)") _RULE_RE = re.compile(r"^ {0,3}(?:[-*_]\s*){3,}$") _HEADING_RE = re.compile(r"^ {0,3}#{1,6}\s+") @@ -21,22 +17,13 @@ _BULLET_RE = re.compile(r"^\s*(?:[-*+]|\d{1,9}[.)])\s+") _IMAGE_RE = re.compile(r"!\[([^\]]*)\]\([^)]*\)") _LINK_RE = re.compile(r"\[([^\]]*)\]\([^)]*\)") -# A link reference definition is a whole line of pure syntax, so the line goes. Its `[text][ref]` and -# `[text][]` usages keep their label. Bare `[text]` is left alone: in prose it is far more often a real -# bracket ("clicked [Save]") than a shortcut reference. -# The definition has to be the entire line — a space-free destination and an optional title, nothing -# after. Matching just the prefix would eat a sentence that merely opens the same way, such as -# "[Save]: clicked twice before it took". _LINK_DEFINITION_RE = re.compile(r"^ {0,3}\[[^\]]+\]:\s*\S+(?:\s+(?:\"[^\"]*\"|'[^']*'|\([^)]*\)))?\s*$") _REFERENCE_RE = re.compile(r"!?\[([^\]]*)\]\[[^\]]*\]") _CODE_SPAN_RE = re.compile(r"`+([^`]*)`+") _STRONG_RE = re.compile(r"(\*\*|__)(?=\S)(.+?)(?<=\S)\1") _STRIKE_RE = re.compile(r"~~(?=\S)(.+?)(?<=\S)~~") -# The lookarounds keep `snake_case` and `2 * 3 * 4` intact: an underscore or star touching a word -# character on the outside is punctuation the model meant literally, not an emphasis delimiter. _EMPHASIS_RE = re.compile(r"(?~|])") -# Terminal punctuation, after which one flattened block runs into the next without a sentence break. _SENTENCE_ENDINGS = ".!?:;," @@ -47,6 +34,15 @@ def flatten_markdown(text: str) -> str: it as prose, where the syntax is noise at best (`**` and `##` inside a search snippet) and a forged list row at worst. Flattening here rather than at each consumer keeps the stored field the one canonical copy. `plain_snippet` is what folds the result onto a single line. + + The regexes above read a line the way a markdown parser does: block markers only at the start of a + line and only space-indented, inline markers anywhere. Three carry a reason worth knowing: + `_LINK_DEFINITION_RE` has to match a whole line, or it eats a sentence that merely opens the same way + ("[Save]: clicked twice before it took"); bare `[text]` is deliberately left alone, being far more + often a real bracket in prose than a shortcut reference; and `_EMPHASIS_RE`'s lookarounds keep + `snake_case` and `2 * 3 * 4` intact. The frontend's `flattenMarkdownToLine` + (products/replay_vision/frontend/utils/markdown.ts) is the same table and has to stay in step, or a + snippet flattens differently in a search result and on the seekbar. """ lines: list[str] = [] for raw in text.split("\n"): diff --git a/products/replay_vision/backend/tests/test_observation_formatting.py b/products/replay_vision/backend/tests/test_observation_formatting.py index 160ce3559ffd..4dc205819798 100644 --- a/products/replay_vision/backend/tests/test_observation_formatting.py +++ b/products/replay_vision/backend/tests/test_observation_formatting.py @@ -34,13 +34,10 @@ class TestPlainSnippet: ("blockquote", "> The user gave up.", "The user gave up."), ("link", "Landed on [the pricing page](https://example.com/p).", "Landed on the pricing page."), ("reference_link", "Landed on [pricing][p].\n\n[p]: https://example.com/p", "Landed on pricing."), - # Brackets in prose are far more often literal than a shortcut reference, so they stay. ("bracketed_prose", "The user clicked [Save].", "The user clicked [Save]."), - # Opens like a reference definition but is a sentence, so the line survives. ("prose_after_bracket", "[Save]: clicked twice before it took", "[Save]: clicked twice before it took"), ("image", "![a screenshot](https://example.com/x.png) followed.", "a screenshot followed."), ("escaped_star", r"Priced at 5\* the usual.", "Priced at 5* the usual."), - # Only spaces indent a block marker: a tab makes it literal text, in the browser flattener too. ("tab_indented_heading", "\t# literal hash", "# literal hash"), ("already_one_sentence_per_line", "Reached payment.\nCard rejected.", "Reached payment. Card rejected."), ("existing_terminal_punctuation", "Two problems:\n- one\n- two", "Two problems: one. two"), @@ -66,8 +63,6 @@ def test_leaves_literal_punctuation_alone(self, _label: str, text: str) -> None: ] ) def test_never_emits_a_line_break_or_a_leading_block_marker(self, _label: str, text: str) -> None: - # The search-result feed embeds this inside markdown that is read as structure, so a newline or a - # leading marker would let recording-derived text forge a row or a header there. snippet = plain_snippet(text) assert "\n" not in snippet assert not snippet.startswith(("-", "*", "#", ">")) @@ -79,5 +74,4 @@ def test_caps_length_and_can_opt_out(self) -> None: def test_flatten_markdown_keeps_line_structure_for_embeddings() -> None: - # Embeddings take the flattened text whole, so the paragraph break has to survive the strip. assert flatten_markdown("## Title\n\n- **one**\n- two") == "Title\n\none\ntwo" diff --git a/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx b/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx index bfa5f6e5a6d4..2a8276803d59 100644 --- a/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx +++ b/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx @@ -27,7 +27,6 @@ export const Structured: Story = { }, } -/** Reasoning written before the prompt asked for structure still has to render as the paragraph it is. */ export const PlainProse: Story = { args: { text: 'The user opened the pricing page, scrolled to the comparison table, and left without starting a trial.', @@ -35,14 +34,7 @@ export const PlainProse: Story = { }, } -/** - * Reasoning is model output derived from a page a stranger wrote, so a link in it is a phishing vector, an - * image is a request it gets to aim from the reader's browser, and a mention is a colleague it gets to - * name. Every markdown form that reaches one of those is here, including the reference forms that a regex - * over the source misses and the same-origin image that the ordinary `disableImages` lets through. The - * assertion runs in a real browser under `test-storybook`, which is the only place this can be checked: - * `react-markdown` is ESM-only and mocked out under Jest. - */ +/** Only `test-storybook` checks this: `react-markdown` is ESM-only and mocked out under Jest. */ export const HostileLinks: Story = { args: { text: [ @@ -59,8 +51,7 @@ export const HostileLinks: Story = { ].join('\n'), segments: [], }, - // `storybook/test` is a dependency of the frontend workspace, which a file under `products/` cannot - // resolve, so the assertions are plain throws. A play function that throws is a failed story. + // `storybook/test` belongs to the frontend workspace and does not resolve from `products/`. play: ({ canvasElement }) => { const check = (ok: boolean, failure: string): void => { if (!ok) { @@ -70,8 +61,6 @@ export const HostileLinks: Story = { const text = canvasElement.textContent ?? '' check(canvasElement.querySelectorAll('a').length === 0, 'a clickable link reached the reader') check(canvasElement.querySelectorAll('img').length === 0, 'an image fired a request') - // The labels and the raw mention syntax survive as plain text, so the reader still sees what the - // model wrote. A chip would show the member's name in its place. check(text.includes('click here'), 'a link label was dropped instead of kept as text') check(text.includes('@member:1 and @role:1'), 'a mention resolved into a chip') }, diff --git a/products/replay_vision/frontend/components/CitedMarkdown.tsx b/products/replay_vision/frontend/components/CitedMarkdown.tsx index 4996e5296e0f..93c4a0066bd1 100644 --- a/products/replay_vision/frontend/components/CitedMarkdown.tsx +++ b/products/replay_vision/frontend/components/CitedMarkdown.tsx @@ -5,17 +5,7 @@ import { LemonMarkdown } from 'lib/lemon-ui/LemonMarkdown' import { citedMarkdown } from '../utils/citations' import { TimestampCitation } from './TimestampCitation' -/** - * A scanner's cited free-text output, rendered as markdown with its citations as seek controls. - * - * Scanners write reasoning that carries structure — a lead-in naming what they found, then the evidence — - * and that structure only reads as structure once it is rendered. - * - * The text is model output derived from a customer recording, so it is treated as hostile: an image must - * not get to fire a request from the reader's browser, no link may be clickable, and an `@member:` in - * it must stay text rather than become a chip naming a real colleague. A citation is the one interactive - * thing here, and it seeks a player rather than navigating anywhere. - */ +/** Cited scanner output as markdown. The text is hostile, so it renders no request, link or mention. */ export function CitedMarkdown({ text, segments, @@ -23,7 +13,6 @@ export function CitedMarkdown({ }: { text: string segments: unknown - /** Called with timestamp_ms when a citation is clicked. If omitted, citations render as plain timestamps. */ onSeek?: (timestampMs: number) => void }): JSX.Element { const markdown = useMemo(() => citedMarkdown(text, segments), [text, segments]) diff --git a/products/replay_vision/frontend/utils/citations.test.ts b/products/replay_vision/frontend/utils/citations.test.ts index 46083a73a49b..a17717091076 100644 --- a/products/replay_vision/frontend/utils/citations.test.ts +++ b/products/replay_vision/frontend/utils/citations.test.ts @@ -98,7 +98,6 @@ describe('citedMarkdown', () => { expected: '- **Reached payment**[01:32](t:92000)\n- Card rejected', }, { - // `![label](target)` is an image, so a citation glued to a `!` would stop being a citation. name: 'does not let a trailing exclamation mark turn the citation into an image', text: '', segments: [ @@ -108,8 +107,6 @@ describe('citedMarkdown', () => { expected: 'Something broke! [00:05](t:5000)', }, { - // Sanitizing is the renderer's job (`CitedMarkdown` sets `disableLinks`), so the prose is - // passed through whole — a link here must arrive intact rather than half-stripped. name: 'passes the prose through untouched, including anything link-shaped', text: '', segments: [ @@ -119,7 +116,6 @@ describe('citedMarkdown', () => { expected: 'Saw [an offer](https://evil.example)[00:05](t:5000)', }, { - // `t:` takes digits only, so anything else in the target would render as a dead label. name: 'rounds a fractional offset and clamps a negative one so the target stays parseable', text: '', segments: [ diff --git a/products/replay_vision/frontend/utils/citations.ts b/products/replay_vision/frontend/utils/citations.ts index 838855aa4233..f7f0941165a2 100644 --- a/products/replay_vision/frontend/utils/citations.ts +++ b/products/replay_vision/frontend/utils/citations.ts @@ -58,16 +58,8 @@ export function parseCitedSegments(text: string, segments: unknown): Segment[] { } /** - * Markdown source for a cited field, with each citation chip written back as a `t:` link target - * LemonMarkdown hands to `renderTimestampRef`. - * - * The chips were split out of the prose at scan time, so a renderer that parses the text as markdown has - * to see one string, not a segment list — a bullet or a bold run that spans a citation would otherwise be - * cut in half and parsed as two documents. - * - * Nothing here sanitizes the prose. The renderer is what holds the line on links and images (see - * `CitedMarkdown`): stripping syntax at this layer would have to out-guess every markdown form that - * reaches a link, and reference-style `[x][ref]` alone already defeats that. + * Markdown source for a cited field, each chip written back as a `t:` target. One string rather than + * a segment list, or a bullet spanning a citation parses as two documents. The renderer sanitizes. */ export function citedMarkdown(text: string, segments: unknown): string { const list = parseCitedSegments(text, segments) @@ -80,10 +72,8 @@ export function citedMarkdown(text: string, segments: unknown): string { out += segment.value continue } - // Glued to the preceding word, matching how the chips render outside markdown — except after a - // `!`, where gluing a link turns the pair into an image. One space there reads as normal prose. - // The target has to be a whole non-negative number: LemonMarkdown reads `t:` and nothing - // else, so a fractional or negative offset would silently render as a dead label. + // Glued to the preceding word, except after a `!`, where that would make the pair an image. + // Whole non-negative digits only: LemonMarkdown reads nothing else, and would leave a dead label. const timestampMs = Math.max(0, Math.round(segment.timestamp_ms)) const label = colonDelimitedDuration(Math.floor(timestampMs / 1000), null) out += `${out.endsWith('!') ? ' ' : ''}[${label}](${TIMESTAMP_REF_PREFIX}${timestampMs})` diff --git a/products/replay_vision/frontend/utils/markdown.test.ts b/products/replay_vision/frontend/utils/markdown.test.ts index 405d9a68a264..4120afcd1dd2 100644 --- a/products/replay_vision/frontend/utils/markdown.test.ts +++ b/products/replay_vision/frontend/utils/markdown.test.ts @@ -11,7 +11,6 @@ describe('flattenMarkdownToLine', () => { expected: 'Checkout blocked. The form rejected it.', }, { - // Only spaces indent a block marker: a tab makes it literal text, in the backend flattener too. name: 'leaves a tab-indented hash as text', text: '\t# literal hash', expected: '# literal hash', @@ -37,13 +36,11 @@ describe('flattenMarkdownToLine', () => { expected: 'Landed on pricing.', }, { - // Brackets in prose are far more often literal than a shortcut reference, so they stay. name: 'leaves bracketed prose alone', text: 'The user clicked [Save].', expected: 'The user clicked [Save].', }, { - // Opens like a reference definition but is a sentence, so the line survives. name: 'keeps a sentence that merely opens like a reference definition', text: '[Save]: clicked twice before it took', expected: '[Save]: clicked twice before it took', diff --git a/products/replay_vision/frontend/utils/markdown.ts b/products/replay_vision/frontend/utils/markdown.ts index 5fda1b9ba2ce..2aaff7711460 100644 --- a/products/replay_vision/frontend/utils/markdown.ts +++ b/products/replay_vision/frontend/utils/markdown.ts @@ -1,7 +1,4 @@ -// Markdown a scanner can emit in its free-text fields. Block markers are matched per line, the way a -// markdown parser reads them; inline markers are matched anywhere on the line. Mirrors the backend's -// `flatten_markdown` (products/replay_vision/backend/observation_formatting.py), which does the same job -// for the surfaces that never reach a browser. +// Kept in step with `flatten_markdown` (backend/observation_formatting.py), which documents the table. const FENCE_RE = /^ {0,3}(?:```|~~~)/ const RULE_RE = /^ {0,3}(?:[-*_]\s*){3,}$/ const HEADING_RE = /^ {0,3}#{1,6}\s+/ @@ -9,32 +6,17 @@ const QUOTE_RE = /^ {0,3}>\s?/ const BULLET_RE = /^\s*(?:[-*+]|\d{1,9}[.)])\s+/ const IMAGE_RE = /!\[([^\]]*)\]\([^)]*\)/g const LINK_RE = /\[([^\]]*)\]\([^)]*\)/g -// A link reference definition is a whole line of pure syntax, so the line goes. Its `[text][ref]` and -// `[text][]` usages keep their label. Bare `[text]` is left alone: in prose it is far more often a real -// bracket ("clicked [Save]") than a shortcut reference. -// The definition has to be the entire line — a space-free destination and an optional title, nothing -// after. Matching just the prefix would eat a sentence that merely opens the same way, such as -// "[Save]: clicked twice before it took". const LINK_DEFINITION_RE = /^ {0,3}\[[^\]]+\]:\s*\S+(?:\s+(?:"[^"]*"|'[^']*'|\([^)]*\)))?\s*$/ const REFERENCE_RE = /!?\[([^\]]*)\]\[[^\]]*\]/g const CODE_SPAN_RE = /`+([^`]*)`+/g const STRONG_RE = /(\*\*|__)(\S(?:.*?\S)?)\1/g const STRIKE_RE = /~~(\S(?:.*?\S)?)~~/g -// The captured leading character and the lookahead keep `snake_case` and `2 * 3 * 4` intact: an -// underscore or star touching a word character on the outside is punctuation the model meant literally, -// not an emphasis delimiter. Captured rather than a lookbehind, which breaks chunk parsing. +// Captured rather than a lookbehind, which breaks chunk parsing. const EMPHASIS_RE = /(^|[^\w*_])([*_])(\S(?:.*?\S)?)\2(?![\w*_])/g const ESCAPE_RE = /\\([\\`*_{}[\]()#+\-.!>~|])/g -// Terminal punctuation, after which one flattened block runs into the next without a sentence break. const SENTENCE_ENDINGS = '.!?:;,' -/** - * Markdown-bearing model text folded onto a single line of readable plain prose. - * - * Reasoning renders as markdown wherever there is room for it, but the places that only have room for a - * fragment — a seekbar hover snippet, the dock's collapsed preview — would show the raw `**` and `-` - * instead. Blocks join with a sentence break so a flattened list still reads as prose. - */ +/** Markdown-bearing model text on one line, for surfaces with room for a fragment, not a rendered block. */ export function flattenMarkdownToLine(text: string): string { let out = '' for (const raw of text.split('\n')) { diff --git a/products/replay_vision/frontend/utils/observation.ts b/products/replay_vision/frontend/utils/observation.ts index 6da70952526c..b687d8cbf98b 100644 --- a/products/replay_vision/frontend/utils/observation.ts +++ b/products/replay_vision/frontend/utils/observation.ts @@ -132,8 +132,7 @@ export interface ObservationSeekbarMark { const SNIPPET_MAX_LENGTH = 160 function lastSentence(text: string): string | null { - // Flattened first: a seekbar mark has room for one line, so markdown syntax would reach the reader as - // literal `**` and `-`, and a bullet's marker would read as the start of the sentence. + // Flattened first: a mark has room for one line, where a bullet marker reads as the sentence start. const flat = flattenMarkdownToLine(text) if (!flat) { return null From 6f25d21540ebdcce125ec0389425148e24a9189d Mon Sep 17 00:00:00 2001 From: "posthog[bot]" <206114724+posthog[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 08:05:12 +0000 Subject: [PATCH 6/7] chore(visual): update storybook baselines 6 updated Run: 2713d41a-b311-4426-b3f5-cb1a4be02dfb --- frontend/snapshots.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/frontend/snapshots.yml b/frontend/snapshots.yml index 5d11c795ec34..5e3ceebbdf95 100644 --- a/frontend/snapshots.yml +++ b/frontend/snapshots.yml @@ -5996,6 +5996,18 @@ snapshots: hash: v1.k794b7964.e21450a1b2054a36fab2a183fd4485190e7161c07830a04008b734f83ecabb1c.Hj2ygumZlfd2zAIz7tC6m7XoM-G15hY_amDmI_m-cJo replay-vision-analysis-nudge--default--light: hash: v1.k794b7964.7eff340ad050dbd7cf07813207e989e33fccb29ef9767e38f89d16b1d567017c.Kr8YaJd8YlDg3tPWTMYWt4vAxTb_HKQuhGUe4IKACDU + replay-vision-cited-markdown--hostile-links--dark: + hash: v1.k794b7964.d98b1851e8c4c695b87856379b85fd225d554c843086706f8d51fe5c060ad2d9.Nu73wfxltRa0Loo3i2L0DYIY_5NgIZwuP5RMTMyQGts + replay-vision-cited-markdown--hostile-links--light: + hash: v1.k794b7964.362d39ed28755489dbf7a698e8115a98141f601b8db1364bc44c8558d488ee1c.AIbcwFSazoZiB1_qtsq7cOH1rV6XPryL3zckEZWetEA + replay-vision-cited-markdown--plain-prose--dark: + hash: v1.k794b7964.c31ceb73ee19d59911337cca2b583f30b7bfdc69349088088811998d357c87f3.y2qdje-cpHJgB9yhNVGjvi8_bsTMVyuR98LWJrnT9Lk + replay-vision-cited-markdown--plain-prose--light: + hash: v1.k794b7964.1a196e580088555e900e84c0162ae4c523feef638540838b541827654ae507ca.beJnsabMlBBuFJexvb0HtCu-cO0W1Md_3b2ucWGA72I + replay-vision-cited-markdown--structured--dark: + hash: v1.k794b7964.b8136fc7ff4b2794158921d793938bdd536bccbc0a66eeadf2d988386e27a2f0.5BXQa3xxRK-9zihaQbWZ4rHtM0lmb0uFKZaNcLbKiX8 + replay-vision-cited-markdown--structured--light: + hash: v1.k794b7964.380bf20693c1029dce9ff64afc117eb5fd831449e81d541f4463ade2ec32b1e0.BBhrMSTk8RqH29BEuHYQeMRRlEos0ZQxgB1wPgvSMxQ scenes-app-ai-observability-clustercard--default--dark: hash: v1.k794b7964.745c8ac41b251f329b890d56d7b07fb31b4939a440a40eea7932f1081d3de702.ESnQVAOfySBfRJWfJsURYu-prgU3EUvQZCD62MyLlX4 scenes-app-ai-observability-clustercard--default--light: From 261e133994c4280287de849918c757a9efb4175d Mon Sep 17 00:00:00 2001 From: Hayne Ryu Date: Tue, 8 Sep 2026 10:31:33 +0100 Subject: [PATCH 7/7] wait for the render before asserting in story --- .../components/CitedMarkdown.stories.tsx | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx b/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx index 2a8276803d59..46a5d18e9776 100644 --- a/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx +++ b/products/replay_vision/frontend/components/CitedMarkdown.stories.tsx @@ -51,17 +51,26 @@ export const HostileLinks: Story = { ].join('\n'), segments: [], }, - // `storybook/test` belongs to the frontend workspace and does not resolve from `products/`. - play: ({ canvasElement }) => { + // `storybook/test` belongs to the frontend workspace and does not resolve from `products/`, so the + // wait and the assertions are by hand. An empty canvas passes the element counts vacuously, so the + // prose has to arrive before anything is asserted. + play: async ({ canvasElement }) => { + let text = '' + for (let attempt = 0; attempt < 40; attempt++) { + text = canvasElement.textContent ?? '' + if (text.includes('click here')) { + break + } + await new Promise((resolve) => setTimeout(resolve, 50)) + } const check = (ok: boolean, failure: string): void => { if (!ok) { throw new Error(failure) } } - const text = canvasElement.textContent ?? '' + check(text.includes('click here'), 'the prose never rendered, or a link label was dropped') + check(text.includes('@member:1 and @role:1'), 'a mention resolved into a chip') check(canvasElement.querySelectorAll('a').length === 0, 'a clickable link reached the reader') check(canvasElement.querySelectorAll('img').length === 0, 'an image fired a request') - check(text.includes('click here'), 'a link label was dropped instead of kept as text') - check(text.includes('@member:1 and @role:1'), 'a mention resolved into a chip') }, }