Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 103 additions & 0 deletions docs/ui/advisor-consult-log.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
# Advisor Consult Log

Status: **implemented (session-only)**

Supersedes [`advisor-interaction-map.md`](./advisor-interaction-map.md), which
was drawn for the preflight-era Advisor and plans a graph around prompt
injection. That injection step does not exist in the on-demand Advisor, so that
document is historical and must not be read as live guidance.

## The problem

The on-demand Advisor lets the primary model consult a separate read-only model
mid-turn via the `stave_consult_advisor` Local MCP tool. Before this surface,
none of it was reviewable:

- `advisorExchangeByTask` holds **one** snapshot per task. A second consult in
the same turn overwrote the first; only a `settledConsults` counter survived.
- Provider events are rAF-batched, and rAF is paused while the window is hidden
or occluded, so one flush routinely carries several *complete* consults. Any
"diff the map after the flush" archive would have kept only the last one.
- The floating exchange card auto-hides after 6s settled / 20s attention.

So the question text and the advice existed for a few seconds and were then
unrecoverable.

## What the surface does and does not claim

It shows, per consult: what was asked, what came back, the lifecycle, the
isolation and effort the runtime actually applied, what the consult cost, and
which tool calls of the same turn started after it settled.

It does **not** infer impact. Advice returns as an MCP tool result and the
primary is free to ignore it; there is no injection step and no `applied` phase.
Two lines of copy carry that limit and are asserted verbatim in
`tests/advisor-consult-log-render.test.tsx`:

- *"Tool calls in this turn that started after the consult settled, in order.
Sequence only — Stave cannot tell whether the advice caused them."*
- *"Reported by the runtime for the advisor call only. Stave reports usage per
message, not per turn, so this is not a share of the turn's total."*

The second exists because `ChatMessage.usage` is per message and carries no
`turnId`, and `buildUsageMetric` sums over the *paged* message list — so any
"% of the turn" denominator the renderer could build would silently drift.

Effectiveness is therefore a **user-set verdict** (Helpful / Not helpful /
Ignored), aggregated per advisor model rather than per task so it survives ring
eviction. The aggregate line has no denominator for the same reason.

## Shape

| Piece | Where |
| --- | --- |
| Pure state, ring, verdict tally | `src/lib/providers/advisor-consult-log.ts` |
| Archive hook inside the fold loop | `applyAdvisorActivityEvents`, `src/lib/providers/advisor-activity.ts` |
| Store slices and four actions | `app-store.types.ts`, `app-store-provider-interaction-actions.ts` |
| Presentation projection | `src/components/session/advisor-consult-log.utils.ts` |
| Dialog + store-connected host | `src/components/session/AdvisorConsultLogDialog.tsx` |

No new provider event, IPC channel, schema, or SQLite table: `advisor_activity`
already carries every field.

`foldAdvisorEvent` returns its input snapshot **by reference** to mean "nothing
changed", which is what lets the archive run once per folded *step* instead of
once per flush. That is the whole fix for the batching loss above.

The host is mounted in `ChatArea`, not inside either trigger, because both
triggers are short-lived: the exchange card clears on its linger timer and the
turn activity shelf is keyed `${taskId}:${activeTurnId}`. A dialog owned by
either would vanish mid-read. Store-held open state is also what lets the shelf
trigger exist without touching `ChatInput.tsx`.

The shelf's advisor row uses `detailSurface: "advisor-consult-log"` rather than
borrowing `toolUseId`: that field asserts "the transcript can reveal this call",
and a consult has nothing to reveal. `data-turn-activity-revealable` stays
tool-only; the advisor row carries `data-turn-activity-opens` instead.

## Bounds

- `ADVISOR_CONSULT_LOG_LIMIT = 24` per task. Must stay **≥
`MAX_ADVISOR_CONSULT_LIMIT` (20)**, or a turn that spends its whole consult
budget evicts its own earliest consults — the exact failure being fixed.
- `ADVISOR_CONSULT_LOG_TASK_LIMIT = 8`, matching
`RETAINED_TURN_ACTIVITY_LIMIT`.
- Worst case ≈ 4 MB, in memory, shed with the task.

## Known limits

- **"What ran after" is a lossy sample.** Work items exist only for the live
turn and the last finished turn per task, and are capped
(`PROVIDER_TURN_WORK_ITEM_LIMIT = 12`,
`PROVIDER_TURN_GENERAL_TOOL_LIMIT = 3`). Older consults render the empty
state, which says so rather than implying nothing ran.
- **Verdicts outlive their entries** by design; the "this session" wording
carries that.
- **`consultIndex` is not unique** — a recoverable provider retry can repeat
it. Entries key on `exchangeId` (falling back to `startedAt`), so no code may
assume index uniqueness.
- **A reload erases the log.** A durable SQLite-backed log is the follow-up;
`selectAdvisorConsultLog` plus a hydrate action is the only swap needed, with
no component changes.
- `ADVISOR_STAGE_LIMIT = 12` still truncates the lifecycle inside an archived
entry.
19 changes: 12 additions & 7 deletions docs/ui/advisor-interaction-map.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
# Advisor Interaction Map

Status: **proposed / not implemented — partially superseded**
Status: **historical — superseded, do not implement**

> ⚠️ This plan was drawn for the preflight-era Advisor (one blocking call
> before the turn, advice injected into the primary prompt). The Advisor has
> since become **on-demand**: the primary consults it mid-turn via the
> `stave_consult_advisor` Local MCP tool, advice returns as the tool result,
> and the `applied`/`primary_started` phases no longer exist. The injection
> and handoff panels below would need to be redesigned per-consult before this
> is implemented.
> before the turn, advice injected into the primary prompt). The Advisor is now
> **on-demand**: the primary consults it mid-turn via the
> `stave_consult_advisor` Local MCP tool, advice returns as the tool result, and
> the `applied` / `primary_started` phases and the injection step do not exist.
> The graph below is therefore drawn around a relationship the runtime cannot
> report.
>
> The shipped surface is [`advisor-consult-log.md`](./advisor-consult-log.md),
> which shows sequence and cost and asks the user for the effectiveness call
> rather than inferring causality. This file is kept only as a record of the
> preflight-era design.

This document is the durable visual and implementation plan for Advisor UX
prototype 3. The existing Handoff Monitor remains the ambient surface, and its
Expand Down
24 changes: 24 additions & 0 deletions src/components/session/AdvisorCheckIcon.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { Check, CircleDashed, Minus, X } from "lucide-react";

import type { AdvisorCheck } from "@/components/session/advisor-exchange.utils";

/**
* Status mark for one `buildAdvisorChecks` row.
*
* Shared by the floating exchange card and the consult log dialog so the same
* check never renders as a tick in one surface and a dash in the other.
*/
export function AdvisorCheckIcon(props: { status: AdvisorCheck["status"] }) {
if (props.status === "pass") {
return <Check className="mt-0.5 size-3.5 shrink-0 text-success" />;
}
if (props.status === "fail") {
return <X className="mt-0.5 size-3.5 shrink-0 text-destructive" />;
}
if (props.status === "pending") {
return (
<CircleDashed className="mt-0.5 size-3.5 shrink-0 text-muted-foreground motion-safe:animate-spin" />
);
}
return <Minus className="mt-0.5 size-3.5 shrink-0 text-muted-foreground/60" />;
}
Loading
Loading