feat: <shiny-aside> markup for source pills + popovers - #278
Conversation
Introduces a reusable sidenote pill + popover component: any assistant
message can carry inline <shiny-sidenote label="..." url="..." icon="...">
tags anywhere in its markdown, with the popover body as the tag's own
markdown children (supporting both a simple inline body and, via a
blank-line-separated block body, richer content like lists and
paragraphs). Entries are grouped per end-of-block into a labeled chip
(or count-fallback pill) that expands into a hover/click/focus popover
with prev/next navigation across grouped entries. This is a
general-purpose building block, not tied to any particular content
source (e.g. chatlas web search/fetch citations, a follow-up PR, will
build on top of it).
Anonymous sidenotes are numbered sequentially across the whole message,
and that message-scoped index is shown on the pill. The popover opens
after a short grace period and is rendered through a @floating-ui/react
portal so it escapes the message list's scrolling container, flipping
and shifting into view rather than clipping.
Accessibility and safety details:
- Multi-source labeled pills include the overflow count ("+N more") in
their accessible name.
- The `icon` attribute is URL-sanitized the same way `url` is.
- rehypeGroupSidenotes runs in both the markdown and HTML content
pipelines, so sidenotes also work under content_type="html".
- A literal `"` inside label/url/body must be HTML-entity-escaped, since
these are ordinary HTML attributes parsed by rehype-raw/parse5.
The component was originally built and named <shiny-footnote>, then
renamed to <shiny-sidenote> across the JS component, markdown pipeline,
Python/R packages, and docs/tests before landing.
Includes JS/CSS unit tests, a Python Playwright app + test, R docs, and
rebuilt web assets for the Python and R packages.
…closing tags While a response streams, a sidenote pill no longer flashes mid-sentence and jitters as text streams past it. It now appears only once its surrounding block has settled — either a later block has started or the stream has ended. Mirrors the existing streaming suggestion-card pattern: a new rehypeMarkTrailingSidenotes marks groups in the still-open trailing block as data-pending (SidenoteGroup renders nothing while pending), and finalizePendingSidenotes clears the markers on the non-streaming render path in hastToReact via an immutable path-copy that never mutates the cached HAST. Also fixes a bug where a self-closing <shiny-sidenote/> was rewritten to a lone <template …/>, which — like any non-void element — ignores the slash and swallows the text after it into the popover body. Normalizes <shiny-sidenote …/> to an open/close pair before the template rewrite, skipping over quoted attribute values so a slash inside url="https://…" isn't mistaken for the self-close. Also corrects the broken-icon test to assert the icon <img> unmounts (matching the intentional switch away from display:none hiding), and documents in the Python/R sidenote help that the favicon is fetched at render time from DuckDuckGo and that an explicit `icon` bypasses that third-party request.
Previously, grouping same-label sidenotes within a block kept only the first entry and discarded the rest. Now every entry is kept and the group pages through them (prev/next), matching the existing multi-source popover behavior. The overflow "+N more" badge is now hidden when all grouped entries share a single label, since paging already conveys that there's more than one entry — the badge is only shown for mixed-label groups where the count isn't otherwise visible. Updates the Python/R chat_append docs to describe same-label paging and the now-conditional overflow badge, corrects CHANGELOG/NEWS entries that still described the old dedup behavior, adds a test covering entry-count overflow in a mixed-label group, and rebuilds the JS assets for both packages.
"Aside" better describes the convention: a small pill/popover attached to a paragraph or list item, not limited to citation-style notes.
Replace hand-rolled hover/pin/blur/keydown handling with useHover, useFocus, useClick, useDismiss, useRole, and FloatingFocusManager so open/close/pin state, outside-click dismissal, and focus trapping are handled by Floating UI instead of bespoke DOM containment checks.
7b961a2 to
aa2f1a1
Compare
There was a problem hiding this comment.
This is a useful, well-tested annotation primitive, and the streaming/floating interaction work is thoughtful. I have a few comments I think we should resolve before merging — feel free to resolve at your discretion and merge.
The HTML content path currently loses unwrapped asides and corrupts rich aside bodies, which is a blocking correctness issue. The popover also needs bounded scrolling and text-resize/long-content handling before rich model-generated content is safe to ship.
At the design level, I would like us to clarify whether paragraph-tail aggregation intentionally gives up claim-level citation mapping, and whether <shiny-aside> is meant to be a compatibility-bound public wire format. My preference is to keep markup as an authoring adapter into a shared normalized aside/citation model, rather than making serialized markup canonical for future structured web results.
The remaining comments cover source actionability and privacy, documentation accuracy, accessibility, scoped theming, and noisy interaction tests.
| * markdown parsers wrapping inline HTML in <p> tags, which doesn't apply to a | ||
| * parse5-parsed HTML fragment. | ||
| */ | ||
| export const htmlProcessor = unified() |
There was a problem hiding this comment.
Required: The HTML path does not apply the pre-parse template disguise used by the Markdown path, while aside grouping happens only after parse5 has interpreted the markup. This causes two concrete failures: a root-level inline <shiny-aside> is never grouped and renders as null, and block content inside an aside can cause parse5 to close the element early, leaving an empty pill and leaking the body into the message. Please make HTML input preserve and group asides before browser-style parsing can alter them, and add coverage for both unwrapped inline asides and rich/block aside bodies.
There was a problem hiding this comment.
Fixed in aec8b838.
Raw HTML now protects <shiny-aside> before parse5 runs and restores it before HAST processing. Regression tests cover root-level asides and rich block content.
|
|
||
| function transform(tree: Root): void { | ||
| let asideIndex = 0 | ||
| visit(tree, "element", (node: Element) => { |
There was a problem hiding this comment.
Question: How committed are we to moving every aside in a paragraph to the paragraph tail? For citations, this loses the authored claim-to-source position and can make several independent claims look like one citation cluster. I recommend retaining a group at each authored location, grouping only adjacent/co-located asides, and deduplicating identical sources within that local group (prefer normalized URL identity, with exact attributes/body as a fallback).
There was a problem hiding this comment.
I have kept paragraph-tail aggregation for the web search and fetch use case. Multiple citations within one sentence are common, and a pill at every authored position makes the response harder to read.
#278 now restores claim-level context through the public grounded-span attribute. Any <shiny-aside> can identify exact preceding text in the same paragraph or list item. Opening or paging that aside highlights the most recent match. Missing or unmatched values have no effect.
#280 maps upstream ContentCitation.grounded_span values onto that public attribute. Citation occurrences remain distinct locally, including repeated URLs. Only the message-wide Sources summary deduplicates URLs.
Implemented on #278 in 9bf5da3 and integrated into #280 in 12fae80.
| return typeof v === "number" ? v : undefined | ||
| } | ||
|
|
||
| export function parseAsideEntries(node?: Element): AsideEntry[] { |
There was a problem hiding this comment.
Question: Can we establish a shared normalized AsideEntry boundary here? The rehype element should be an authoring adapter into that model, while future structured citation/web-result data should feed the same renderer directly. That avoids making serialized <shiny-aside> markup the canonical citation representation or requiring structured data to be escaped, serialized, and reparsed.
There was a problem hiding this comment.
Addressed in 1fb8d8d4.
<shiny-aside> now acts as an adapter from HAST into normalized AsideEntry objects. The exported AsideGroupView renders normalized entries directly, and its regression test does not use a HAST node. Future structured citation data can use the same renderer without serializing <shiny-aside> markup.
There was a problem hiding this comment.
The normalized boundary now separates generic aside state from citation metadata. AsideEntry.groundingId is top-level state that any grounded aside can use. Citation metadata contains only citation-specific fields, such as title and cited_quote, and the Sources collector consumes normalized citation entries.
The current Python and R transports map upstream ContentCitation.grounded_span values to the public grounded-span attribute. Markup remains an adapter at the client boundary. A future structured transport can populate normalized AsideGroupView entries directly.
Implemented on #278 in 9bf5da3 and integrated into #280 in 12fae80.
| })) | ||
| } | ||
|
|
||
| export function faviconUrl(url: string): string | undefined { |
There was a problem hiding this comment.
Required: Supplying a URL currently causes the client to contact DuckDuckGo for a favicon. That leaks source domains and user activity to a third party and may violate deployment CSP/privacy requirements. Please provide an app/chat-level way to disable or replace the resolver, with a no-icon fallback; this should not require each generated aside to opt out individually.
There was a problem hiding this comment.
Addressed in 3a631e92 as a deployment-wide policy rather than a new per-chat argument.
SHINYCHAT_ASIDE_FAVICON=false prevents derived DuckDuckGo requests from every chat in the process, which gives operators one privacy and CSP policy. Python and R validate the setting. Explicit icon= values still render, so apps can use controlled assets.
This setting does not provide different favicon policies for multiple chats in one process. If that use case is required, it needs a separate public API.
| </span> | ||
| </div> | ||
| )} | ||
| {current.label && ( |
There was a problem hiding this comment.
Required: url fetches an icon but does not provide a clickable destination: the source heading remains a plain div. A supplied source URL should produce an actual link inside the popover, using the project’s existing external-link behavior and accessible naming. Otherwise the API exposes the source location while preventing users from visiting it.
There was a problem hiding this comment.
Fixed in aec8b838.
Safe source URLs now render as external links in the popover heading and use the existing link handling. Unsafe URLs remain non-clickable.
| // (.shiny-chat-lightbox at 1080, fullscreen tool card at 1050) — now | ||
| // that this lives at the body level rather than inline with the message. | ||
| z-index: 1040; | ||
| min-width: 14em; |
There was a problem hiding this comment.
Required: Model-generated content can make this popover unusable: a long body produced a roughly 1,990px-tall fixed popover with no internal scrolling; at 200% text size it overflowed a 320px viewport; and an unbroken label expanded the pill to roughly 1,292px. Please add a compact viewport-aware max height with internal scrolling, cap width to the available viewport, and apply safe wrapping/overflow rules to labels and URLs. Add regression coverage at 200% text size and with long unbroken content.
There was a problem hiding this comment.
Fixed in aec8b838.
The popover now has viewport-aware size limits, internal scrolling, and long-content wrapping. Playwright covers a 320px viewport at 200% text size.
| text-align: start; | ||
| white-space: normal; | ||
|
|
||
| .shiny-aside-popover__nav { |
There was a problem hiding this comment.
Required: Please raise the carousel control opacity to at least 0.65 and verify contrast in both light and dark themes. The current 0.6 measures only about 4.37:1 in the tested light theme, leaving little margin. The measured previous/next buttons are also only about 26 x 22px; increase the interactive hit area to at least 24px in both dimensions, preferably 32px, while keeping the glyph visually compact.
There was a problem hiding this comment.
Fixed in aec8b838.
Carousel buttons now have a minimum 24px by 24px hit area and opacity of 0.65. Playwright verifies both hit-area dimensions. The opacity threshold is set directly on the navigation container in CSS.
This change does not add a theme-specific contrast assertion because scoped theme propagation remains a separate review thread.
| ) | ||
| }) | ||
|
|
||
| it("closes on mouse-leave when not pinned", () => { |
There was a problem hiding this comment.
Required: These interaction tests emit repeated React act(...) warnings even though they pass. That means state updates are escaping the test synchronization boundary and can hide real failures in CI noise. Please use userEvent, await observable updates with findBy/waitFor, and correctly advance/flush any timers so the new suite runs warning-free.
There was a problem hiding this comment.
Fixed in aec8b838.
The delayed hover tests now use awaited user interactions and real timers. The focused AsideGroup suite passes without React act(...) warnings.
| `url`, unless `icon` overrides it); without one, it falls back to a | ||
| plain numbered/count marker. The body is ordinary markdown: inline for a | ||
| one-liner, or — by separating it with blank lines — a rich block body | ||
| (paragraphs, lists, code) shown in the popover. Multiple asides in |
There was a problem hiding this comment.
Required: This documentation says all asides in a message collapse into a single element, but the implementation only groups labeled asides; anonymous asides remain separate numbered pills. Please document that distinction consistently in the Python and R APIs, generated Rd, and release notes (or change the implementation if one-group behavior is the intended contract).
There was a problem hiding this comment.
Fixed in 84b1dd21.
The Python and R API documentation, generated Rd, and release notes now distinguish labeled aggregation from anonymous notes. Labeled asides in one block share a carousel pill. Each unlabeled aside remains a separate numbered pill.
# Conflicts: # js/dist/shinychat.css # js/dist/shinychat.css.map # js/dist/shinychat.js # js/dist/shinychat.js.map # js/src/chat/ChatApp.tsx # js/src/chat/chat-entry.ts # js/src/chat/chat-tools.scss # js/tests/markdown/MarkdownContent.test.tsx # pkg-py/CHANGELOG.md # pkg-py/src/shinychat/www/GIT_VERSION # pkg-py/src/shinychat/www/shinychat.css # pkg-py/src/shinychat/www/shinychat.css.map # pkg-py/src/shinychat/www/shinychat.js # pkg-py/src/shinychat/www/shinychat.js.map # pkg-r/NEWS.md # pkg-r/inst/lib/shiny/GIT_VERSION # pkg-r/inst/lib/shiny/shinychat.css # pkg-r/inst/lib/shiny/shinychat.css.map # pkg-r/inst/lib/shiny/shinychat.js # pkg-r/inst/lib/shiny/shinychat.js.map # pkg-r/tests/testthat/test-chat.R
Why this matters
Assistant responses increasingly need to cite their sources — a web search result, a fetched page, a methodology note — without derailing the prose. This PR adds a general-purpose way to do exactly that: drop an inline
<shiny-aside>tag anywhere in a message's markdown and it renders as a small pill at the end of the paragraph (or list item) it belongs to. Hover, click, or keyboard-focus the pill and a popover shows the note's body.The key design decision is that this is a plain markup convention, not an API. Any content that reaches the chat as markdown can carry these tags — hand-authored, templated, or (the intended path) emitted by an LLM you've prompted to cite its sources. There is no new Python/R function to call and nothing source-specific baked in:
Chat.append_message()/chat_append()already stream markdown, and asides ride along inside it.This is the rendering primitive that the upcoming web search / web fetch citation UI is built on top of (see "Foundation for web citations" below).
What it looks like
Asides render at the end of the block they belong to, regardless of where in the sentence the tag was authored — the markup pass pulls each tag out of its inline position and appends the resulting pill(s) to the tail of the paragraph or list item.
A labeled aside renders as an identity chip — a favicon (derived from
url) plus the source name:When several distinct sources are cited in the same block, their pills collapse into one — the first source becomes the face, the rest become a
+Noverflow, and the popover turns into a carousel with prev/next controls:An aside with no
labelfalls back to a plain numbered marker ([1],[2], … numbered across the whole message), so anonymous notes still work:A single block can carry more than one pill: all labeled asides merge into one source-chip, while each label-less aside becomes its own numbered marker — so the tail of a paragraph might show a chip and a couple of numbered pills together.
The convention
label(optional) — source name shown on the pill/chip. Asides sharing a label within a block are grouped under one chip and paged through (prev/next) rather than deduped — every entry is kept. Without a label, the pill is a numbered marker.url(optional) — makes the source heading in the popover a link and supplies a derived favicon. The URL is sanitized.icon(optional) — overrides the favicon; also URL-sanitized.grounded-span(optional) — identifies answer text that is related to the aside. The value must exactly match preceding text in the same paragraph or list item. When the popover opens, shinychat highlights the most recent match. An unmatched value has no effect.Examples
Labeled, one-line body:
Two sources in one sentence collapse into a single pill:
Label-less aside with a rich block body (blank line ⇒ block body), falls back to a numbered pill:
The same markup works in R via
chat_append()— see the Asides section in the?chat_append/append_message()docs for the full reference.Try it locally
These example apps aren't part of this PR's diff (kept local, not committed) but are the easiest way to see every scenario at once.
sandbox/demo_asides.py— exhaustive manual scenarios (Python, no API key needed)Feature checklist
<shiny-aside>tag anywhere in a block → trailing pill at end-of-block+Noverflow and a prev/next carousel popover; anonymous notes stay as separate numbered pillslabelwithin a block page through every entry (prev/next) instead of deduping; the+Noverflow badge is shown only for mixed-label groups, since paging already conveys the same-label countgrounded-spanhighlights the most recent exact preceding match in the same block while its aside is open<shiny-aside…opening tag is withheld rather than shown as raw markupcontent_type="html"messagesurlandiconare URL-sanitized (defense-in-depth in the HAST pass + React render)SHINYCHAT_ASIDE_FAVICON=falseto disable third-party derived-favicon requests across a deployment; expliciticonURLs still workConsistency with suggestion cards
The aside streaming machinery is deliberately modeled on the existing suggestion-card code, so this reuses a proven pattern rather than inventing new streaming logic. Three concrete parallels:
SUGGESTION_PENDING_ATTR+rehypeSuggestionCards+finalizePendingSuggestionLists; asides mirror it withASIDE_PENDING_ATTR+rehypeMarkTrailingAsides+finalizePendingAsides. Both refuse to commit anything in the still-open last block, and both clear the marker at end-of-stream via the same immutable path-copy strategy (mirroringwithStreamingDot) so the cached Stage-1 HAST is never mutated.hastToReactcalls the two finalizers together.hideTrailingPartialTag'sTRAILING_PARTIAL_ASIDE_REmirrors suggestions'PARTIAL_SPAN_OPENING_RE(the nested-optional regex matching a tag that hasn't received its>yet). Both stop a partially-arrived tag from flashing as raw text mid-stream.Key distinction: suggestions promote in place (the list stays where it sits, just restructured), whereas asides relocate to end-of-block. The shared DNA is the streaming/finalization plumbing, not the placement semantics.
Foundation for web search / web fetch citation UI
This is deliberately the substrate for the citations work, not the whole thing. The public
grounded-spanattribute restores claim-level context after a pill moves to the block tail. Opening or paging to an aside highlights the most recent exact preceding match in the same paragraph or list item.Follow-up #280 maps structured
ContentCitation.grounded_spanmetadata onto this public attribute. It also adds citation-only behavior, including the message-wide Sources summary. Thedata-citationmarker controls that citation layer; it is not required for grounded aside behavior.Implementation notes (for reviewers)
The heavy lifting is a set of rehype passes wired into both the markdown and HTML processors (
js/src/markdown/processors.ts):rewriteAsideTemplate— round-trips<shiny-aside>through a<template>disguise across rehype-raw so parse5 keeps block-level children instead of applying the<p>-can't-contain-blocks auto-close rule.rehypeGroundedAsides— connectsgrounded-spanto the most recent exact preceding text in the same paragraph or tight list item, including matches that cross inline formatting.rehypeGroupAsides— collects asides per paragraph/tight-list-item, splices each out of its inline position, and appends the resulting group(s) to the end of the block; labeled asides collapse into one group, while anonymous ones each become their own group stamped with a message-scoped index.markTrailingAsides— marks groups in the open trailing block as pending during streaming;finalizePendingAsidesclears them on the settled render via an immutable path-copy (mirrors the suggestion-card pattern).AsideGroup.tsx— adapts grouped markup into normalized entries, then renders the pill + floating-ui popover/carousel.Tests: JS unit tests for each plugin + the component, plus a Python Playwright app/e2e at
pkg-py/tests/playwright/chat/web_asides/.