Fix stale default-open doc comments per argus's review

ClassifyCtx's tool_use-id correlation gates markdown-vs-plain body
format for a recv result, not open/collapsed state — that's always
the operator's uniform preference now. 5 backend comments still
described it as controlling "default-open" rendering, contradicting
the actual render path and this PR's own rewritten docs.

Also fixed useAgentState.ts's stale comment promising an
SSE-triggered refresh model "in a later commit" — that's permanently
off the table now that the terminal stream's kind tag is gone by
design (argus flagged this as a drive-by, not blocking, but it's a
one-line cause-and-effect of this same PR so fixing it here).
This commit is contained in:
iris 2026-08-30 21:28:34 +02:00
commit 907567ef76
4 changed files with 32 additions and 29 deletions

View file

@ -1,16 +1,13 @@
// useAgentState — polls `GET /api/state` and exposes the latest
// snapshot + loading/error status. Mirrors app.js's old `refreshState`
// data fetch (not yet its exact re-poll cadence — see the interval
// comment below).
//
// Cadence note: the old page only re-polls on a timer while a login is
// in flight, and otherwise waits for an SSE `turn_end` event to trigger
// one-shot refreshes — this avoids clobbering the operator's half-typed
// message in the term-input field. That field doesn't exist in this
// rewrite yet (a later commit on this same PR), so there's nothing to
// clobber yet; this hook uses a flat interval for now and switches to
// the SSE-triggered model in the commit that adds TermInput + the live
// stream, matching the original behavior once it's actually needed.
// data fetch, not its exact re-poll cadence: the old page re-polled on a
// timer only while a login was in flight, and otherwise waited for an
// SSE `turn_end` event to trigger one-shot refreshes. That SSE-triggered
// model is permanently off the table now — the terminal stream's `kind`
// tag (what let a client single out a turn-boundary event) is gone by
// design, see `useLiveStream.ts`'s module doc — so this hook's flat 4s
// interval is the only refresh path, not a placeholder for a later
// commit.
import { useEffect, useRef, useState } from 'preact/hooks';
import type { AgentState } from '../types.js';

View file

@ -268,9 +268,11 @@ fn classify_assistant_content(content: &[Value], ctx: &mut ClassifyCtx) -> Vec<T
}
/// `_category === 'rich'` tools used to get an expandable row (diff body
/// for Edit, default-open markdown for send, plain for everything else with
/// a body). That's now just "does this row have a body" — `body.is_some()`
/// on the returned [`TermMsg`] *is* the expandable signal, no separate flag.
/// for Edit, markdown body for send, plain for everything else with a
/// body). That's now just "does this row have a body" — `body.is_some()`
/// on the returned [`TermMsg`] *is* the expandable signal, no separate
/// flag; whether it renders open or collapsed is a uniform client-side
/// preference, not something this function decides.
fn classify_tool_use(c: &Value) -> TermMsg {
let name = c.get("name").and_then(Value::as_str).unwrap_or("");
let input = c.get("input").cloned().unwrap_or_else(|| json!({}));
@ -418,12 +420,16 @@ fn classify_task_event(v: &Value) -> Option<TermMsg> {
/// Pre-compute the expandable body for rich tool entries.
///
/// Returns `Some((body, body_type))` where `body_type` tells the frontend
/// which renderer to use:
/// - `"diff"` → `api.detailsDiff` (colour-coded `+`/`-` lines)
/// - `"plain"` → `api.details` (plain `<pre>` block)
/// - `"markdown"` → `api.detailsOpenMd` (markdown rendered via marked + `DOMPurify`,
/// default-open; used for message-bearing tools: `send`)
/// Returns `Some((body, body_type))` where `body_type` becomes the
/// `TermMsg`'s `body_format` and tells the frontend which renderer to use:
/// - `"diff"` → colour-coded `+`/`-` lines
/// - `"plain"` (`None` on the wire) → a plain `<pre>` block
/// - `"markdown"` → rendered via `marked` + `DOMPurify`; used for
/// message-bearing tools: `send`
///
/// Whether the resulting row renders open or collapsed is a uniform
/// client-side preference (the operator's expand-tool-output setting),
/// not something this function or its `body_type` decides.
///
/// Returns `None` for tools that have no body at all.
///

View file

@ -101,14 +101,13 @@ impl TermMsg {
/// Per-connection/per-request classification state. A live SSE stream keeps
/// one of these alive for the connection's lifetime — `tool_use` id → name
/// correlation, so a `tool_result` can tell it's answering a `recv` call and
/// render as a default-open markdown message body. The history endpoint
/// render its body as markdown instead of plain text. The history endpoint
/// uses a fresh one per page: correlation only works within the page
/// actually returned, not across the live/history boundary. Accepted
/// degradation (same shape as the turn-timestamp fallback documented in
/// `docs/terminal-rendering.md`) — the only user-visible effect is a `recv`
/// result whose `tool_use` fell on the other side of a page/reconnect
/// boundary rendering as a plain block instead of default-open markdown,
/// not a functional loss.
/// degradation — the only user-visible effect is a `recv` result whose
/// `tool_use` fell on the other side of a page/reconnect boundary rendering
/// its body as plain text instead of markdown (open/collapsed state is a
/// uniform client-side preference either way, not affected by this).
#[derive(Default)]
pub struct ClassifyCtx {
tool_name_by_id: HashMap<String, String>,

View file

@ -79,9 +79,10 @@ pub(super) async fn events_history(
// replay and live tail deliver identical shapes. The DB stores raw
// events; classification is applied at read time here (see
// `crate::term_msg`). One `ClassifyCtx` for the whole page — tool_use→
// name correlation (for default-open `recv` results) only works within
// a single page/connection, not across the live/history boundary; see
// that module's doc for why that's an accepted degradation.
// name correlation (for markdown-vs-plain `recv` result bodies) only
// works within a single page/connection, not across the live/history
// boundary; see that module's doc for why that's an accepted
// degradation.
let mut ctx = ClassifyCtx::default();
let events: Vec<TermEnvelope> = events
.into_iter()