Skip to content

refactor(web): unify task and PR-review detail views on shared primitives#485

Merged
jonwiggins merged 9 commits into
mainfrom
worktree-magical-honking-bentley
Apr 23, 2026
Merged

refactor(web): unify task and PR-review detail views on shared primitives#485
jonwiggins merged 9 commits into
mainfrom
worktree-magical-honking-bentley

Conversation

@jonwiggins

Copy link
Copy Markdown
Owner

Summary

The two detail pages — /tasks/[id] and /reviews/[id] — told the same story with different building blocks: bespoke state strip vs. StateBadge, inline log <div> vs. LogViewer, hand-rolled header vs. factored header, separate chat code paths. A user moving between the views had to relearn the location of every familiar control.

This PR extracts shared primitives and adopts them in both pages.

What changed

New shared components / hooks

  • DetailHeader — title + state badge + subtitle + meta + actions
  • StatePipelineStrip — compact horizontal pipeline strip (Queued → CI → Reviewing → Ready → Submitted)
  • PrStatusBar — CI / review / PR-state row with optional right-aligned actions
  • ChatTranscript + ChatComposer — message list and input with optional interrupt button (auto-resize, Enter-to-send, Shift+Enter-for-newline)
  • usePrReviewLogs — WS-driven log tailing for PR reviews, same { logs, connected, capped, clear } shape as useLogs so it plugs straight into LogViewer's externalLogs prop

Extended primitives

  • StateBadge gains configs for waiting_ci, reviewing, ready, stale, submitted so both pages render the same badge component

Backend (live log streaming for PR reviews)

  • event-bus.publishEvent now also publishes to optio:pr-review:{id} for any event that carries a prReviewId — the existing pr_review_run:log and pr_review_run:state_changed publishes wire up for free
  • New WS endpoint /ws/pr-reviews/:id/logs mirroring /ws/logs/:taskId (catch-up + live subscription), registered in server.ts
  • createPrReviewLogClient helper in ws-client.ts

Page changes

  • /reviews/[id] adopts DetailHeader, StatePipelineStrip, PrStatusBar, ChatTranscript/ChatComposer, LogViewer (with full search / filtering / tool-use grouping), and the existing classifyError banner
  • /tasks/[id] adopts DetailHeader, PrStatusBar, and ChatComposer for the mid-task messaging bar (with Stop=interrupt)

Stays inline (these are the actual primitive of "review", not duplication):

  • Verdict picker (Approve / Request Changes / Comment)
  • Inline file-comment editor (path / line / body)
  • Split merge button with strategy dropdown
  • Draft dirty / Save Draft state machine

Test plan

  • Visit /reviews/<id> for a working review — header renders, state badge matches the rest of the app, pipeline strip shows correct stage
  • Open the runs/logs panel — logs stream in live (via WS) instead of polling; search and filter work
  • Send a chat message — composer behaves the same as the task page's mid-task bar
  • Visit /tasks/<id> for a running claude-code task — Stop and Send buttons in the message bar still work
  • CI status row on a PR-opened task still shows CI / review / cost / Request Review action
  • Failed review now renders the classified-error banner with retry button

…ives

The two detail pages — /tasks/[id] and /reviews/[id] — told the same story
with different building blocks: bespoke state strip vs. StateBadge, inline
log div vs. LogViewer, hand-rolled header vs. factored header, separate chat
code paths. A user moving between the views had to relearn the location of
every familiar control.

This pass extracts five shared primitives and adopts them in both pages:

  - DetailHeader: title + state badge + subtitle + action slot
  - StatePipelineStrip: compact horizontal pipeline strip
  - PrStatusBar: CI / review / PR-state row
  - ChatTranscript + ChatComposer: message list + input with optional interrupt
  - usePrReviewLogs: WS-driven log tailing for PR reviews, same shape as useLogs

StateBadge gains configs for review-specific states (waiting_ci, reviewing,
ready, stale, submitted) so both views use the same badge component.

The review page now streams logs live via a new WebSocket endpoint
(/ws/pr-reviews/:id/logs) — previously it polled every 3s while `showLogs`
was open. event-bus publishes to `optio:pr-review:{id}` for any event with
a prReviewId, so the existing pr_review_run:log publishes wire up for free.

The review page also gets the classified error banner (title + remedy +
retryable chip) that the task page already used, and a proper LogViewer with
search / filtering / tool-use grouping instead of the bespoke <pre>.

Unique-to-review UI (verdict picker, inline file comments, merge dropdown,
draft dirty/save state machine) stays inline — that's the actual primitive
of "review" and doesn't belong in a shared abstraction.
…draft widget at top

The reviews page now matches the same shape as /tasks/[id]:

  - LogViewer is the primary surface (full-height, streaming live)
  - Vertical pipeline timeline lives in a 320px right-hand sidebar
  - The verdict + summary widget appears at the top of the log column
    only when state is ready/stale/submitted
  - Chat composer pinned to the bottom of the log column when applicable

Implementation:

  - Export `PipelineStageRow` + `PipelineStage` type from pipeline-timeline so
    the visual primitive can be reused outside the task page
  - Add `ReviewPipelineTimeline` that derives 5 stages (Queued → Waiting CI →
    Reviewing → Ready → Submitted) from the review row + runs and renders via
    the shared `PipelineStageRow`. Surfaces re-run count and cost as detail
    on the Reviewing stage.
  - Restructure /reviews/[id] into the task-page flex shape: header → status
    bar → banners → flex row { log column [draft widget?, working hint?,
    timeline toggle, LogViewer, chat?] | Pipeline sidebar }
  - Drop the standalone working-state card and runs-collapsible — both are
    now reflected in the timeline + LogViewer.
A chevron toggle in the widget header lets the user collapse the verdict +
summary + comments editor when they want more vertical room for the logs.
The header strip stays visible while collapsed and shows the verdict pill,
comment count, "unsaved" marker, and the first line of the summary so the
gist of the draft remains glanceable.
The standalone timeline-toggle strip sat awkwardly between the collapsible
verdict draft and the logs. Moving the button into the PrStatusBar's actions
slot puts it on the same row as "CI: passing", which is where the user is
already scanning for status. The PR status row now always renders so the
Timeline control has a stable home even when there's no PR status loaded.
Mirror the change just made to /reviews/[id] — the Timeline button now sits
in the PrStatusBar's actions slot next to Cost / Request Review, instead of
in a standalone toolbar above the LogViewer. The PR status row always
renders to give the toggle a stable home even before a PR exists.
pr-review-worker transitioned the parent pr_review to FAILED on any run
failure, including chat runs. A chat is a follow-up turn on an
already-produced draft; its failure is a conversational hiccup and should
not invalidate the review. Skip the parent transition when run.kind is
'chat' so the draft, verdict, and log history stay intact. Both the
normal-failure path and the worker-exception catch are fixed.

Also route user chat turns through LogViewer's userMessages prop so they
render inline in the log stream (matching /tasks/[id] mid-task messaging),
and drop the now-redundant ChatTranscript bubble list. User turns are
hydrated from persisted chat history on load so past messages still appear
after a refresh.
opencode.ai's install script (curl-pipe-bash) intermittently fails with
"Failed to fetch version information", which kills the whole agent image
build. The same failure mode is already handled defensively for
@github/copilot and openclaw in this Dockerfile — apply the same || echo
WARN fallback to opencode so a transient upstream blip doesn't block CI.
…itives

Apply the same shared-primitive pattern used for /tasks/[id] and
/reviews/[id] to the standalone (workflow) task pages.

Run page (/jobs/[id]/runs/[runId]):
  - Drop the tabs (Logs / Output / Params) in favour of the side-by-side
    log column + timeline sidebar layout that tasks and reviews already use
  - Adopt DetailHeader (title + StateBadge + meta + actions)
  - PrStatusBar hosts the Timeline toggle + cost/tokens, mirroring the
    other two pages
  - Add WorkflowRunPipelineTimeline (Queued → Provisioning → Running →
    Done), built on the shared PipelineStageRow primitive
  - Output and Params become collapsible widgets at the top of the log
    column (output expanded by default, params collapsed) — same shape as
    the verdict draft widget on /reviews
  - Adopt the classified-error banner pattern (title + remedy + retry +
    category chip) instead of the inline error block

Blueprint page (/jobs/[id]):
  - Replace the hand-rolled header with DetailHeader
  - Use a new "enabled"/"disabled" StateBadge variant in place of the
    bespoke green/grey dot
  - Keep the stats grid, runs table, triggers list, and config panel
    unchanged — those are blueprint-specific and shouldn't unify
Params were rendered below Output and defaulted to collapsed, so they were
easy to miss after the side-by-side run-page rewrite — the previous
tabbed UI made them one click away. Lay Params and Output out as a
two-column grid (stacked on narrow screens) and default both expanded so
the most useful run context stays glanceable.
@jonwiggins jonwiggins merged commit e4feca2 into main Apr 23, 2026
7 checks passed
jplorier pushed a commit to jplorier/optio that referenced this pull request May 5, 2026
…ives (jonwiggins#485)

* refactor(web): unify task and PR-review detail views on shared primitives

The two detail pages — /tasks/[id] and /reviews/[id] — told the same story
with different building blocks: bespoke state strip vs. StateBadge, inline
log div vs. LogViewer, hand-rolled header vs. factored header, separate chat
code paths. A user moving between the views had to relearn the location of
every familiar control.

This pass extracts five shared primitives and adopts them in both pages:

  - DetailHeader: title + state badge + subtitle + action slot
  - StatePipelineStrip: compact horizontal pipeline strip
  - PrStatusBar: CI / review / PR-state row
  - ChatTranscript + ChatComposer: message list + input with optional interrupt
  - usePrReviewLogs: WS-driven log tailing for PR reviews, same shape as useLogs

StateBadge gains configs for review-specific states (waiting_ci, reviewing,
ready, stale, submitted) so both views use the same badge component.

The review page now streams logs live via a new WebSocket endpoint
(/ws/pr-reviews/:id/logs) — previously it polled every 3s while `showLogs`
was open. event-bus publishes to `optio:pr-review:{id}` for any event with
a prReviewId, so the existing pr_review_run:log publishes wire up for free.

The review page also gets the classified error banner (title + remedy +
retryable chip) that the task page already used, and a proper LogViewer with
search / filtering / tool-use grouping instead of the bespoke <pre>.

Unique-to-review UI (verdict picker, inline file comments, merge dropdown,
draft dirty/save state machine) stays inline — that's the actual primitive
of "review" and doesn't belong in a shared abstraction.

* refactor(reviews): mirror task layout — logs main, timeline sidebar, draft widget at top

The reviews page now matches the same shape as /tasks/[id]:

  - LogViewer is the primary surface (full-height, streaming live)
  - Vertical pipeline timeline lives in a 320px right-hand sidebar
  - The verdict + summary widget appears at the top of the log column
    only when state is ready/stale/submitted
  - Chat composer pinned to the bottom of the log column when applicable

Implementation:

  - Export `PipelineStageRow` + `PipelineStage` type from pipeline-timeline so
    the visual primitive can be reused outside the task page
  - Add `ReviewPipelineTimeline` that derives 5 stages (Queued → Waiting CI →
    Reviewing → Ready → Submitted) from the review row + runs and renders via
    the shared `PipelineStageRow`. Surfaces re-run count and cost as detail
    on the Reviewing stage.
  - Restructure /reviews/[id] into the task-page flex shape: header → status
    bar → banners → flex row { log column [draft widget?, working hint?,
    timeline toggle, LogViewer, chat?] | Pipeline sidebar }
  - Drop the standalone working-state card and runs-collapsible — both are
    now reflected in the timeline + LogViewer.

* feat(reviews): make the verdict draft widget collapsible

A chevron toggle in the widget header lets the user collapse the verdict +
summary + comments editor when they want more vertical room for the logs.
The header strip stays visible while collapsed and shows the verdict pill,
comment count, "unsaved" marker, and the first line of the summary so the
gist of the draft remains glanceable.

* refactor(reviews): move Timeline toggle into the PR status bar row

The standalone timeline-toggle strip sat awkwardly between the collapsible
verdict draft and the logs. Moving the button into the PrStatusBar's actions
slot puts it on the same row as "CI: passing", which is where the user is
already scanning for status. The PR status row now always renders so the
Timeline control has a stable home even when there's no PR status loaded.

* refactor(tasks): move Timeline toggle into the PR status bar row

Mirror the change just made to /reviews/[id] — the Timeline button now sits
in the PrStatusBar's actions slot next to Cost / Request Review, instead of
in a standalone toolbar above the LogViewer. The PR status row always
renders to give the toggle a stable home even before a PR exists.

* fix(reviews): chat run failure must not fail the parent review

pr-review-worker transitioned the parent pr_review to FAILED on any run
failure, including chat runs. A chat is a follow-up turn on an
already-produced draft; its failure is a conversational hiccup and should
not invalidate the review. Skip the parent transition when run.kind is
'chat' so the draft, verdict, and log history stay intact. Both the
normal-failure path and the worker-exception catch are fixed.

Also route user chat turns through LogViewer's userMessages prop so they
render inline in the log stream (matching /tasks/[id] mid-task messaging),
and drop the now-redundant ChatTranscript bubble list. User turns are
hydrated from persisted chat history on load so past messages still appear
after a refresh.

* ci(agent-image): make opencode install best-effort

opencode.ai's install script (curl-pipe-bash) intermittently fails with
"Failed to fetch version information", which kills the whole agent image
build. The same failure mode is already handled defensively for
@github/copilot and openclaw in this Dockerfile — apply the same || echo
WARN fallback to opencode so a transient upstream blip doesn't block CI.

* refactor(jobs): unify standalone task UI with shared task/review primitives

Apply the same shared-primitive pattern used for /tasks/[id] and
/reviews/[id] to the standalone (workflow) task pages.

Run page (/jobs/[id]/runs/[runId]):
  - Drop the tabs (Logs / Output / Params) in favour of the side-by-side
    log column + timeline sidebar layout that tasks and reviews already use
  - Adopt DetailHeader (title + StateBadge + meta + actions)
  - PrStatusBar hosts the Timeline toggle + cost/tokens, mirroring the
    other two pages
  - Add WorkflowRunPipelineTimeline (Queued → Provisioning → Running →
    Done), built on the shared PipelineStageRow primitive
  - Output and Params become collapsible widgets at the top of the log
    column (output expanded by default, params collapsed) — same shape as
    the verdict draft widget on /reviews
  - Adopt the classified-error banner pattern (title + remedy + retry +
    category chip) instead of the inline error block

Blueprint page (/jobs/[id]):
  - Replace the hand-rolled header with DetailHeader
  - Use a new "enabled"/"disabled" StateBadge variant in place of the
    bespoke green/grey dot
  - Keep the stats grid, runs table, triggers list, and config panel
    unchanged — those are blueprint-specific and shouldn't unify

* fix(jobs): show params next to output, expanded by default

Params were rendered below Output and defaulted to collapsed, so they were
easy to miss after the side-by-side run-page rewrite — the previous
tabbed UI made them one click away. Lay Params and Output out as a
two-column grid (stacked on narrow screens) and default both expanded so
the most useful run context stays glanceable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant