radial
A collaborative coding-agent orchestrator for atproto radl.app
Star 14
Fork 0
Watch

add ability to include images in review findings #101

Merged disnetdev.com
targeting main from radial/impl-b21b85d4edb3
Merged opened by and assigned to disnetdev.com
targeting main from radial/impl-b21b85d4edb3

Review finding images #

Human reviewers can now paste, drop or pick a PNG/JPEG/WebP/GIF in any review finding, and a saved finding renders its markdown — screenshot included — under the severity that judges it.

No lexicon change was needed and none was made. The finding body already owns the relationship to the picture: it names a com.disnetdev.radial.image record with a radial-image: locator, exactly as a goal body does. So old and new findings are the same shape, com.disnetdev.radial.review is untouched, and pnpm codegen produces no diff.

What changed #

packages/ui/src/lib/verdicts.ts — new FindingRow extends FindingDraft, adding key (row identity) and uploading (a picture on the wire). blankFinding() returns one. findingsOf() still takes FindingDraft, so the serialization boundary cannot see the transient fields even by accident.

packages/ui/src/lib/components/Verdict.svelte

  • Each finding editor gets images and bind:uploading.
  • {#each rows … (row.key)} instead of (index), and drop() filters by key — removing a row above an editor no longer hands its undo history, caret and in-flight insertion anchor to a different finding.
  • A card-level attaching derived: while any finding is uploading, both verdict buttons say "Adding image…" and are disabled, and post() refuses. The card is several markdown fields and one signed record, so the hold is the card's, not the row's.
  • The uploading row's ✕ is disabled with an explaining title. Destroying that editor would leave a successfully anchored, public, permanent image record with nothing referring to it and nowhere to insert its locator. Every other row stays editable throughout — MarkdownEditor carries the insertion point across concurrent edits.
  • Upload validation and errors come from MarkdownEditor's existing path; no second upload path was added.

packages/ui/src/lib/components/UnitDetail.svelte — a finding body renders through the existing Prose component instead of <p>{finding.body}</p>. Prose/Runs remains the only rendering path: parsed tokens interpolated, never {@html}, only strict radial-image: locators resolved (through the author's current PDS), anything else left as a non-fetching named placeholder.

packages/ui/src/app.css — .finding p becomes .finding .body, keeping the severity/location grid and the finding's type scale while dropping the prose measure and adding min-width: 0 so fenced blocks scroll inside the column rather than widening a narrow drawer.

Docs — packages/ui/README.md (images now on for findings, why the body owns the locator, why templates and agent turns stay off, the aggregate hold), MarkdownEditor.svelte's header comment, the replaced comment in Verdict.svelte, and the markdown-editor bullet in DESIGN.md.

Coverage added #

  • packages/ui/src/lib/components/Verdict.svelte.test.ts (new, 7 tests, real mounted CodeMirror + the mocked uploadImage seam): picker/paste/drop enabled in every finding; both verdicts disabled and labelled while uploading, with write never called; the uploading row's ✕ disabled; the image landing in the finding it was pasted into while another is edited and a third row is added; an in-flight upload surviving the removal of the row above it; --findings-json containing exactly {severity, path, body} with the locator and no key/uploading; a refused upload leaving the body and the postability unchanged with an actionable message.
  • packages/ui/src/lib/components/UnitDetail.svelte.test.ts (new, 4 tests, mounted over the real fixture fold — records through MemoryRecordStore → materialize() → timeline()): a valid locator rendering RadialImage's <img> with the body's alt while severity and path:line stay put; markdown rendered as markdown and a raw <img …onerror> reaching the screen as its own source; a malformed locator, a wrong-collection at:// and a relative path all left as placeholders with resolveImage never called; an unresolvable locator degrading to the named placeholder.
  • packages/daemon/test/bundle-writer.test.mjs — a locator that appears only in a review finding is still listed once under ## Images in bundle.md, and bundle.json round-trips the exact finding body. Locks in the existing scan without teaching the serializer to fetch bytes.

Checks #

Run from the repo root on Node 24 / pnpm 10, all passing:

  • pnpm codegen → no diff (no lexicon change)
  • pnpm lint (typecheck + type-strip lint + generated-file check) → clean; svelte-check 600 files, 0 errors, 0 warnings
  • pnpm test → all packages green; @radial/ui 553 vitest tests across pure + components, plus 48 node --test bundle/serve assertions
  • pnpm build

Remaining risks #

  • Cancel is not held during an upload, deliberately. Abandoning the whole review is the reader's decision, and the leftover image record is the same outcome as writing markdown and then deleting it — existing, accepted behaviour. Blocking Cancel would trap them, and it matches every other composer, whose Cancel is disabled only while writing.
  • Unreferenced image records. Deleting an inserted locator still leaves the record standing. Nothing here deletes immutable public data implicitly; retention is a separate design.
  • Agent turns still cannot attach images — structurally, not by omission: a turn container holds no protocol credential and its socket command takes JSON findings, not file bytes.
  • Availability follows the author's PDS. An unreachable image is a named placeholder, never a blank or an error; bytes are never proxied or replicated.
  • The verdict card now shows an "Add image" affordance per finding, so a card with several findings is a little busier than before. It is the same affordance every other markdown surface carries.

Radial artifact

Commits 0