[OPIK-6425] [FE] feat: Explain (Ollie) on trace cells — bridge, store, button & popover

[awkoy]

Details

Adds the Ollie "Explain" feature (OPIK-6425) on the frontend, end to end: the host-side bridge contract, the plugin-scoped explain store, and the user-facing Explain button + popover wired into trace-table cells. Frontend-only; no backend, migrations, or v1 changes. Everything explain-specific lives inside the comet plugin, so OSS builds get nothing.

Bridge contract

Adds the 7 explain/chat/console events (explain:run/cancel, chat:continue, console:ready, explain:chunk/done/error) + ExplainTarget to assistant-sidebar.ts, bumps BRIDGE_PROTOCOL_VERSION 1→2, registers the new host events in createHostListeners(), and routes the shell→host events into the explain store. Bridge machinery is extracted into plugins/comet/assistantBridge.ts (host listeners, createBridge, emitHostEvent, emitter registration) and must stay in lockstep with ollie-console/src/bridge.ts (separate repo / PR).

Explain store (plugins/comet/explain/explainStore.ts)

• Caches per cell (projectId:kind:entityId) — reopening a popover shows the cached answer (or its still-streaming text), not a refetch; entries persist across close.
• Multiplexes parallel streams — an explainId → cell route map fans incoming explain:chunk deltas back to the right cell, so N concurrent explains = N entries + N routes over the single bridge. In-flight concurrency cap of 3; oldest settled cells evicted past MAX_CACHED.
• capabilities + bridge version captured from console:ready; ownership-guarded host→shell emit registration.
• Retry, mid-stream continueChat (hands the partial answer to the sidebar chat and cancels the cell stream), and cancel.

Stream resilience & bridge robustness

• Per-stream watchdog (streamWatchdog.ts): a waking hint at 10s then a retryable timeout at 30s, so a request that never reaches a live pod can't hang on "Thinking…". A streamed chunk retires the watchdog.
• Recovery-after-error: a transient console error keeps the route alive so a same-explainId retry can stream chunks in and resurrect the cell; a terminal error→done stays an error rather than blanking.
• Orphan-subscriber eviction: a pod readiness flap remounts the Ollie <iframe> into a fresh JS realm without running the old realm's React cleanup, leaving its host→shell subscription parked in the listener set. The host then double-handled each event — the dead-realm handler failing fast and surfacing a spurious "Couldn't load…" / "Couldn't continue…" — so subscribe() now evicts the orphan for the single-subscriber events (explain:run/explain:cancel/chat:continue/conversation:start) before wiring the fresh one. Multi-subscriber events (context:changed/runner:state-changed/visibility:changed) are deliberately excluded.

UI

• ExplainButton — owl pill pinned to the cell's right edge, fail-closed when the pod isn't ready / lacks the explain capability / the kind is unknown. Resting circular icon button expands to the Figma pill on hover/open (gradient rotates −45°→−18°, "Explain" label slides in). Opening dispatches the stream and only counts a click that actually surfaces an explanation; closing mid-stream cancels the unseen (paid) generation.
• ExplainPopover — streamed output via the shared MarkdownPreview, Thinking… / waking / error / empty states, Retry and Continue conversation actions, and an aria-live region so streamed text is announced. Aligned 1:1 with Figma (nodes 351:17617 / 351:31659 / 351:31623).
• ExplainableCell + a shared createExplainTargetBuilder factory build per-kind, null-safe payloads, wiring Explain into the Error, Duration and Cost cells across the Traces/Spans and Threads tabs from one place.

BI

EXPLAIN_CLICKED, EXPLAIN_COMPLETED, EXPLAIN_ERRORED, EXPLAIN_CONTINUE_CLICKED, EXPLAIN_RETRIED.

Change checklist

• User facing
• Documentation update

Issues

• OPIK-6425

AI-WATERMARK

AI-WATERMARK: yes

• Tools: Claude-Code
• Model: Opus-4.8
• Scope: All files in this PR
• Human verification: Yes — design and pixel-level UI reviewed and iterated with the author against Figma; the stream-resilience / orphan-eviction fixes were diagnosed from test-env capture and verified live, then re-reviewed (xhigh multi-agent) for merge-readiness.

Testing

Commands (from apps/opik-frontend):

• npm run typecheck — clean (incl. after merging latest main)
• npx eslint on the changed files — 0 problems
• npx vitest run (explain + bridge suites) — 58 tests passing across ExplainButton, ExplainPopover, explainStore, useCanExplain, registry, and assistantBridge (the single-subscriber eviction invariant + the route-lifecycle / recovery / error-code-hardening regressions).

Runtime: button + popover verified in the running app and iterated against Figma (hover → expand → popover, streamed markdown, Retry, Continue conversation); the orphan-eviction fix was confirmed against the test environment's pod-flap repro.

Documentation

N/A — internal host bridge + plugin UI only; no user-facing docs change.

🤖 Generated with Claude Code

[baz-reviewer]

keyOf() drops projectId, so explain state from different projects collides in the same kind:entityId bucket; should we scope the key/route by projectId or clear project-scoped explain state on switch?

Want Baz to fix this for you? Activate Fixer

Other fix methods

Prompt for AI Agents

Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/plugins/comet/explain/explainStore.ts around lines 26-26, the
`keyOf(t: ExplainTarget)` function builds `${t.kind}:${t.entityId}` but drops
`projectId`. Refactor `keyOf` (and anything that depends on it, like `entries`,
`routes`, `useExplainEntry`, `explain`, and `retry`) to include `projectId` in the
composite key so cached answers and streaming routes cannot collide across projects.
Additionally, if the UI does not automatically reset explain state on
`context.projectId` changes, add/trigger a store reset/clear when the active project
changes (or otherwise scope the in-flight counting to the current project) so stale
loading entries from a previous project don’t consume the shared `MAX_IN_FLIGHT`
budget.

[baz-reviewer]

Commit 6504577 addressed this comment by scoping keyOf() to projectId (${t.projectId}:${t.kind}:${t.entityId}), which prevents explain-state and stream-route collisions across projects. The dependent entry lookup and retry routing now inherit that project-scoped key as well.

[baz-reviewer]

routes never drops finished explainIds, so the map grows unbounded as explain keeps creating fresh UUIDs. Should we mirror retry and delete routes[explainId] when the stream completes or errors?

Want Baz to fix this for you? Activate Fixer

Other fix methods

Prompt for AI Agents

Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/plugins/comet/explain/explainStore.ts around lines 112-131, the
`onDone` and `onError` handlers update `entries` but never remove `routes[explainId]`.
Refactor these handlers so that when a stream finishes successfully or with an error,
they also delete the corresponding `explainId` entry from `routes` inside the same `set`
call, mirroring the cleanup done in `retry`. Be careful to handle cases where
`entries[key]` is missing (unknown/cleared explain) but still remove `routes[explainId]`
so the `routes` map cannot grow unbounded over time.

[baz-reviewer]

Commit 6504577 addressed this comment by deleting routes[explainId] in both onDone and onError, matching the cleanup already used in retry. It also keeps the route removal even when the cached entry is missing, preventing the routes map from growing unbounded.

[aadereiko]

LGTM

[baz-reviewer]

The ExplainButton accent classes (border-[#F46E41], gradient backgrounds, and shadow-[...rgba(244,110,65,0.1)]) hard-code colors, so they bypass theme tokens and won't adapt in light/dark or future theme changes — should we swap them for theme-aware values?

Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]

Other fix methods

Prompt for AI Agents

Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/plugins/comet/explain/ExplainButton.tsx around lines 58-70 (the
`className` passed to the Explain button in `ExplainButton`), remove the hard-coded
accent hex/RGBA values (`#F46E41`, `#F59E0B`, and the rgba(...) shadows/gradients).
Refactor by replacing them with theme-aware design tokens (e.g., existing Tailwind theme
colors or CSS variables for the accent and accent-shadow), and update the
gradient/shadow utilities to reference those tokens so the styling adapts to light/dark
and other theme changes. After refactoring, verify the popover trigger still uses the
same sizing/behavior while visually matching the previous accent in the default theme.

[baz-reviewer]

Commit 0e6ef91 addressed this comment by replacing the hard-coded hex/RGBA values in ExplainButton with CSS variables (--color-ollie, --color-ollie-amber, --shadow-ollie). It also defines those tokens in main.scss, so the button styling now comes from theme-aware values instead of inline constants.

[baz-reviewer]

The popover header icon and loading dot use hard-coded text-[#F46E41] and bg-[#00D14C], so they stay the same in dark mode instead of following the design system — should we swap them for the :root/.dark CSS variables and utility classes from main.css, as .agents/skills/opik-frontend/ui-components.md requires?

Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]

Other fix methods

Prompt for AI Agents

Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/plugins/comet/explain/ExplainPopover.tsx around lines 40-56 (the
header icon <OllieOwl ...> and the loading dot <span className="... bg-[#00D14C]">),
replace the hard-coded hex colors text-[#F46E41] and bg-[#00D14C] with theme-aware
classes/variables from the design system. Update main.css to define matching CSS
variables under :root and .dark (or reuse existing ones if already present), then switch
the component to reference those variables via utility classes (so dark mode
automatically changes). Make sure the visual intent (owl icon orange and
“Thinking...” dot green) remains unchanged aside from theme adaptation.

[baz-reviewer]

Commit 0e6ef91 addressed this comment by replacing the hard-coded hex colors in ExplainPopover.tsx with CSS-variable-based Tailwind classes (text-[var(--color-ollie)] and bg-[var(--color-ollie-live)]). It also adds those variables in main.scss under both :root and .dark, so the icon and loading dot now follow the design system in dark mode.

[baz-reviewer]

The explain store's inFlight >= MAX_IN_FLIGHT guard rejects the 4th concurrent request while MAX_IN_FLIGHT is still 3, so Too many explanations in progress fires one request early — should we bump the cap to 4, or, if 3 is intended according to the PR description, align the spec and tests?

Want Baz to fix this for you? Activate Fixer

Other fix methods

Prompt for AI Agents

Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/plugins/comet/explain/explainStore.ts around lines 30 and 197-206
inside the `explain: (target) => boolean` logic, the in-flight throttling guard
currently rejects when `inFlight >= MAX_IN_FLIGHT`, which makes the 4th concurrent
request fail if MAX_IN_FLIGHT is still 3. Update MAX_IN_FLIGHT (and any related PR/test
expectations) to match the intended concurrency spec (either set MAX_IN_FLIGHT to 4, or
if 3 is correct, adjust the PR description and any tests/assertions that assume 4). Also
ensure the AT_CAPACITY_MESSAGE triggers exactly when the cap is exceeded (one request
after the allowed max).

[awkoy]

Working as intended — 3 is the spec (PR description: "In-flight concurrency cap of 3"). inFlight >= MAX_IN_FLIGHT with MAX_IN_FLIGHT = 3 allows exactly 3 concurrent streams (when 3 are already in flight, the next is rejected), so the cap is 3, not 2 — the 4th request is correctly the one that's throttled. The test caps in-flight streams at MAX_IN_FLIGHT (3) asserts this directly: 3 loading entries, the 4th explain() returns false and surfaces the retryable "Too many" message. No change needed.

[baz-reviewer]

Thanks for the clarification — you're right that the current check enforces a cap of exactly 3 concurrent streams. I'll save this to memory once the PR is merged.

[baz-reviewer]

buildThreadDurationTarget repeats buildThreadCostTarget's guard/payload flow, so future payload changes have to be synced in two places and the duration/cost targets can drift — should we factor a shared helper that takes the positive field and payload builder and keeps the number_of_messages branch in one place?

Want Baz to fix this for you? Activate Fixer

[baz-reviewer]

Commit 84a53a8 addressed this comment by replacing the two separate thread target builders with a shared threadTargetBuilder factory. That centralizes the common number_of_messages handling and payload/scaffold logic so duration and cost stay in sync.

[awkoy]

Done in 84a53a8 — factored a shared threadTargetBuilder(kind, basePayload) that owns the project_id guard and the number_of_messages branch, with buildThreadDurationTarget/buildThreadCostTarget now just supplying their kind + value field. Mirrors the errorTargetBuilder/durationTargetBuilder/costTargetBuilder factory pattern in the Traces/Spans explainTargets.ts, so the two can't drift.

[baz-reviewer]

threadTargetBuilder repeats the same project_id guard/payload scaffolding used by the TracesSpansTab/explainTargets.ts builders, so changes to the guard, number_of_messages, or ExplainTarget shape can drift between the thread and trace/span branches — should we factor a shared createExplainTargetBuilder(kind, payloadFn) helper?

Want Baz to fix this for you? Activate Fixer

[baz-reviewer]

Commit 3191775 addressed this comment by extracting a shared createExplainTargetBuilder helper and using it from both the Threads and Traces/Spans explain target builders. That removes the duplicated project_id guard and shared payload envelope so the builders stay in sync.

[baz-reviewer]

Null explain payloads may be rejected

finiteOrNull now converts missing duration/cost values to explicit null, so the explain builders send { duration: null } / { total_estimated_cost: null } over the bridge instead of omitting the key, which will fail if the assistant backend validator still expects numeric-or-absent fields — should we confirm null is accepted there or keep the omitted-key shape for N/A rows?

Want Baz to fix this for you? Activate Fixer

Other fix methods

Prompt for AI Agents

Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/v2/pages/LogsPage/createExplainTargetBuilder.ts around lines
16-17, in the `finiteOrNull` helper, you’re converting all non-finite inputs
(including missing cost/duration) into explicit `null`, which forces `{ duration: null
}` / `{ total_estimated_cost: null }` to be sent. Please verify the backend
explain-payload schema/validator for the affected `ExplainKind`s to confirm whether
these fields explicitly allow `null`; if they do not, change the N/A behavior to
preserve the old “omit the key” payload shape (so JSON.stringify drops the field) by
returning `undefined` for those cases from `finiteOrNull` (or by doing a per-field
conditional in the payload builder). Then add/update a unit test that asserts the exact
payload shape generated for an N/A/missing duration/cost row matches what the backend
accepts.

[aadereiko]

The PR looks good! Thank you for covering a lot of functionality with tests, this is useful. The only thing that I noticed is that the comments are too verbose sometimes, but it's up to you whether you want to fix

Great job and nice feature!

[baz-reviewer]

React import grouped after vitest

This file puts vitest before import type { MutableRefObject } from "react";, so it breaks the repo's react → external → internal import order, including type-only React imports — can we reorder it to match .agents/skills/opik-frontend/code-quality.md?

Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]

Other fix methods

Prompt for AI Agents

Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/plugins/comet/assistantBridge.test.ts around lines 1-10, the
import ordering violates the frontend convention: the `vitest` imports are placed before
the type-only React import. Reorder the top-level imports so they follow `react`
(including `import type { MutableRefObject } from "react"`) first, then external
libraries (`vitest`), and finally internal imports from `./assistantBridge`. Keep the
import specifiers the same; only change the ordering to match the repo’s code-quality
rules.

[aadereiko]

LGTM!