feat: add comments to task artifacts - #3970
Draft
puemos wants to merge 28 commits into
Draft
Conversation
Add inline and document-level artifact comments with threads, image regions, resilient text anchors, sanitized HTML previews, version drift handling, and live query refresh.
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
React Doctor found 2 issues in 1 file · 2 warnings. 2 warnings
Reviewed by React Doctor for commit |
puemos
marked this pull request as draft
July 29, 2026 19:37
Replace sanitized shadow-DOM rendering with a Peek-inspired opaque-origin iframe bridge. Preserve authored CSS/resources, show a visible selection comment button in-frame, synchronize highlights over postMessage, and retain restrictive sandbox capabilities.
Scroll each renderer to the selected comment anchor, move the document composer to the sidebar footer, and replace custom sidebar cards, badges, separators and empty state with Quill primitives.
Contributor
Author
|
Addressed review feedback:
|
Show comment counts in artifact rows, make resolve PAT-compatible through thread-state events, move image annotation controls into the header, render Quill callouts with comment previews, and pulse sidebar threads selected from artifact highlights.
Contributor
Author
|
Final UX pass complete:
Validation: API/core/UI typechecks, Biome, host boundaries, and 33 focused tests all pass. |
Replace image labels with yellow circular comment pins, add Open/Resolved filtering with counts, and reuse the Thread mention composer across document, reply, and inline artifact comments while sending mention IDs for notifications.
Contributor
Author
|
Implemented the latest design update:
Validation: API/core/UI typechecks, Biome, host boundaries, and 35 focused tests pass. |
Filter resolved threads before artifact rendering, centralize thread grouping and resolution, reduce comment context to anchor/state, reuse the generated comments client and ZoomableImage overlay, remove duplicate component helpers, and clear React Doctor errors.
Contributor
Author
|
Lean/resolved-visibility pass complete:
Validation: all package typechecks, 36 focused tests, Biome, host boundaries, and React Doctor pass. |
Comments lived in two places: a sidebar inside the artifact pane (which ate half the document's width) and a second, read-only list at the bottom of the Artifacts tab. Now there is one home — the activity sidebar reads Timeline · Artifacts · Comments, and that tab is the comments pane itself. The tab carries everything the sidebar had: the Open/Resolved filter with counts, replies, resolve/reopen, the changed-anchor warning, and selection highlighting. Each row names the artifact it came from, and clicking one opens that artifact and scrolls the document to the anchor. The list and the artifact live in sibling React trees, so a small view store carries the focus request between them, replacing the commentId that used to ride along in the panel tab's persisted data. That also fixes repeat navigation: reopening an already-open artifact never updated its tab data, so picking a second thread on it silently did nothing. Comment mutations now patch and invalidate both cache shapes (a single resource's query and the task-wide fan-out), so a reply or resolve from the tab shows up without waiting for a poll. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
The list and the artifact talk through a store because they sit in sibling React trees, and that seam had no test — the two directions were only ever checked by hand. jsdom has no layout, so the highlight rectangles and the scroll call are stubbed outright, which is enough to drive both directions: clicking a highlight publishes the pick, and a pick published elsewhere scrolls this artifact to the anchor and marks the thread active. A request aimed at another artifact is ignored. Also covers the panel itself: the third tab renders its list without the conversation composer, a pick made on an artifact brings the tab forward, requests for other tasks (and ones that predate the task on a reused panel) are left alone, and only the timeline gets scrolled to the bottom. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
CI was already red on this branch, and the cause was a scroll: jsdom implements no scrolling at all, so the old artifact preview's `querySelector(...).scrollIntoView` threw from inside an animation frame — after the test that scheduled it had passed. Vitest reported every test green and then exited 1 on one unhandled error, which reads as an unrelated flake. That call is gone with the sidebar, but the trap isn't: the comment list reveals a thread the same way, and the activity panel scrolls the timeline with `scrollTo`. Both are now no-oped in the shared setup, beside the ResizeObserver and getAnimations stubs that exist for the same reason, and the two tests that assert on a scroll spy on them rather than assigning their own. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
The Comments tab listed only artifact and canvas threads. This makes it the one place every comment on a task lives: - PR comments, both kinds. Inline review threads keep their replies and can be replied to and resolved from the tab (via the existing PR actions); the conversation comments — issue chatter and review summaries — show as single read-only cards. Clicking a thread opens the PR in the review pane at its file. Threads from several PRs load in parallel and cache, so revisiting the tab is free. - Comments on the task itself, from a composer pinned under the list, for the discussion that belongs to the work rather than to any one deliverable. - A source filter beside the open/resolved one. It follows whatever artifact is open in the main pane until you pick a source by hand, after which it stays where you put it. All three fold into one normalized thread model, so the list renders and sorts them together and only reply/resolve/open still care which backend a thread came from. Also the styling notes from the screenshot: the pulse ring is inset so a clipping pane can't shave it, the redundant "Comments" title is gone, and replies sit at the root's indentation instead of stepping further in each time. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
PR threads were labelled "PR #1234". They now use the PR's real title, fetched off the same cache the review pane already warms, and fall back to the number until it loads. Titles run long, so the card label, the source-filter button, and the filter menu all truncate and show the full name on hover. Also a bit more top padding on the list so the first card clears the sticky filter header. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
…t act GitHub comment bodies are GFM with raw HTML in them, and the comment card rendered them without the raw-HTML pipeline, so tags leaked through as stray `</a>` fragments. The review pane already solved this with a rehype-raw + sanitize plugin set; that's now extracted to a shared module and used by both the review pane and the comment card, so PR bodies render the same way in both. And a conversation comment can't be replied to or resolved from the tab — it only exists on GitHub — so instead of a dead Reply button it now shows a "View on GitHub" link to the comment. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
…nned The source dropdown truncated PR titles to "feat(ch…", so you couldn't tell one source from another. The label span had no flex sizing and the menu was capped narrow, which collapsed the title to a few characters. Now the menu is a fixed comfortable width and each title wraps in full with the count pinned to the right, using the same min-w/ml-auto idioms the other quill dropdowns in the app already use. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
The wrapping attempt centred and stacked the labels — worse than before. The quill dropdown radio item is a left-aligned flex row with an inset for the indicator, so the model picker's shape is the one to follow: a single-line label that truncates at the end (full name on hover) with the count pushed right via ml-auto, on a comfortably wide menu. Titles read left-to-right and line up again. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
Each source row in the filter menu now leads with the same icon the comment card uses for that source — the PR icon for pull requests, the file-type icon for artifacts, the canvas icon, a chat icon for the task — so the sources are scannable at a glance rather than read letter by letter. The icon mapping is extracted into one helper the card and the menu share, and the option list carries each source's kind so the menu knows which to draw. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
An SVG artifact opened to an empty pane. SVG is deliberately kept out of the <img> allowlist — its scripts would run from a data URL — so it falls to the generic preview, but the blob kept whatever type the download carried (often none), and the iframe won't draw an untyped blob. The generic preview is an opaque-origin iframe with scripts already blocked, so SVG is safe there; it just needed the right type. Now SVG blobs are typed image/svg+xml and render. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
The source filter listed everything newest-first, so "This task" landed wherever its last comment fell. It's the one source every task has, so it now sits at the top of the options, right under "All sources", with the rest still newest-first. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
Four things from the screenshot round: - SVG artifacts now open on the same surface as raster images, so they get the zoom controls and can be commented on with pins, instead of sitting in the bare iframe. An <img> renders SVG in a secure static mode that never runs scripts, so this doesn't loosen the data-URL rule the shared allowlist keeps. - Opening a composer puts the caret in it — both the new-comment popover and a thread's Reply — so you can type straight away. The task composer pinned at the bottom of the list is left alone; focusing that on tab open would steal the caret from whatever you were doing. - An image comment pin now holds its on-screen size as you zoom: the overlay lives inside the transformed content, so it takes the scale and counters it. - And the pin's dark halo is gone — that was shadow-md reading as a thick black ring on a pale image. A thin white ring separates it from the artwork instead. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
Both complaints traced to the same cause: the pin was a quill Button, which is built for a control in a toolbar, not for something stuck to a picture. It sits a pixel high and drops on :active — that was the click-nudge — and it keeps a dark focus ring after the click, which is the heavy black circle in the screenshot (the thin white gap was our own ring underneath it). So the pin is now a plain marker: a yellow disc with a dark icon, one hairline ring in a theme alpha grey instead of a black-and-white double edge, no press translate, and a yellow focus outline for keyboard users only. A thicker ring still marks the selected thread. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
The pin now says who left the comment instead of just that one exists: a teardrop shell around that person's avatar, reusing UserAvatar so the hue and the Gravatar are the same ones they have everywhere else in the app. Squaring the bottom-left corner of a circle makes the teardrop, and that corner is the anchor point — so the pin hangs off the spot it marks rather than covering it, which also means several pins near each other stay legible. The shell deliberately ignores the theme: it sits on someone's artwork, which has a palette of its own, so it stays a dark disc with a white hairline that reads on both black and white pixels. The selected thread gets a thicker ring. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
The thread cards were drawing their own grey initials bubble while the image pins showed the person's real face, so the same comment looked like it came from two different people. The cards now use UserAvatar too — Gravatar and the person's app-wide hue — with the plain url path kept for GitHub authors, who never have a PostHog user behind them. The pin also had alpha in it, on both the shell and the avatar, so a Gravatar with its own transparency let the artwork bleed through and muddied both. Shell and the disc behind the avatar are now fully opaque. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
The shell was still see-through because `bg-black` and `ring-white` were doing nothing here — this app styles from the Radix scales, so the literal palette utilities never landed and the pin came out as a bare outline around the avatar. Shell and ring are now opposite ends of the grey scale (--gray-12 against --gray-1), which are opaque and carry their own contrast onto artwork of any colour, in either theme. The shell also got a pixel of breathing room around the avatar, so the teardrop reads as a pin rather than as a rim. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
puemos
changed the title
A comment on an artifact or canvas points at an id that lives in a run's JSON, not in a table the server can join against, so nothing on the backend could work out which task the comment belonged to. Mentions on those comments therefore reached people by email and by the web inbox but never by this app's Activity page — the one surface where the comment is actually readable. The task id now rides along in item_context, which is the only piece the server was missing. It is client-supplied, so the backend checks it against the team before acting on it, and re-checks visibility when the feed is read. Needs the matching PostHog/posthog change to have any effect; harmless before it lands, since item_context is a free-form JSON field. Generated-By: PostHog Code Task-Id: ec8afa68-0d3b-417a-b9aa-4804b7429d76
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds Google Docs/Figma-style commenting to task artifact tabs:
Resolved threads remain available under the Resolved filter but are not rendered on the artifact: no text highlight, click target, or image pin.
Architecture
CommentAPI withscope=task_artifact; no new persistence model.MentionComposer/MentionTextand the API's existingmentionsfield.{ anchor, threadState? }initem_context; artifact identity remains the existingitem_id./completeaction rejects PAT access.HTML artifacts
Inspired by Peek's open-source annotation bridge:
postMessageallow-same-origindisabledImages
The existing
ZoomableImageprimitive now accepts an optional overlay. Pins and placement transform with the image automatically, without geometry polling or format-specific zoom logic.Validation