feat(cli,tui): issue #12 — presenter contract semantics amendment (v0.2.0)
Replaces the stateless _render_event / _render_event_to_log helpers with stateful per-turn presenters (CliPresenterState / TuiPresenterState). Coalesces thinking-event deltas into a single growing display per run; demotes telemetry events with editorial hierarchy; formats duration + usage for human reading. Headline behavior change: a 50-token thinking phase now renders as ONE coalesced growing line in CLI (or one closed RichLog entry + per-delta live Static widget in TUI), not 50 lines of [thinking] spam. Editorial promotion line (issue #12 INV-002): - Load-bearing (no demotion prefix): Text, Done, Error, Cancelled - Demoted telemetry (`. ` ASCII prefix in CLI; dim `· ` in TUI): WorkerPhase, Thinking, TextBoundary, ToolStart, ToolResult Stateful coalescing: - Thinking deltas accumulate into thinking_buffer; first non-thinking event closes the run with a single \n boundary in CLI / one closed dim RichLog entry in TUI. - TUI adds a dedicated Static(id="thinking-current") widget that shows the last ~200 chars of the active run, mirroring per-delta updates. Two-views-of-thinking decoupling per INV-004: chronological RichLog + always-visible widget. - CLI INV-005: when stdout text was streamed mid-line, text_written_since_newline triggers a stdout flush + \n before the next stderr terminal label — guarantees [done] / [error] / [cancelled] land on their own line in a TTY without breaking pipe-to-file scripted consumers. Formatting helpers (issue #12 INV-006 / INV-007): - _format_duration_ms — autoscale `347ms` / `5.5s` / `1.2m` - _format_usage — natural-language `6756 in -> 126 out (6882 total, 0 cached)` with arrow="->" CLI / "→" TUI Cross-frontier design pass (eitri-smithy-dev, althing 01KSBE52YZR5E3SPTKA672JE43) returned 16-of-16 confirmed decisions + 4 material divergences applied: - ASCII `. ` prefix in CLI (`·` is U+00B7, not ASCII) - RichLog one-closed-entry-per-run + Static per-delta updates (not inline-mirror as initially proposed) - presenter-state object instead of pure-function rendering - Framed as "contract semantics amendment", not "polish" Volva paraphrase round (5 prose-precision fixes applied to 12.contract.md): INV-001 "growing display" semantics; single hide mechanism for the Static widget (Textual reactive `display: bool`); [render_error] security clause (type-only, no exception message); text_written_since_newline `\n`-terminated text corner case; [create_session] integration path (bypasses state.render — not an SSE Event variant). Volva code-review round (5 findings applied): - F1 drift: render-exception fallback now writes BOTH a plain-label fallback line for the original event AND the `[render_error] <type>` line (was missing the fallback half). - F2 drift: dim Rich style applied to all demoted-telemetry RichLog writes via `rich.text.Text(..., style="dim")` (was plain str). - F3 drift: belt-and-braces widget clear+hide on EVERY terminal event (Done/Error/Cancelled), even when thinking_open was False. - F4 precision: _format_usage gains PRE-001 assertion on the four expected usage keys. - F5 precision: _run_turn signature amended in issue #3 contract to document the new `state: CliPresenterState | None = None` test- injection kwarg. [create_session] lifecycle line demoted to `. create_session:` (written directly by _amain; bypasses state.render since it's not a wire-level SSE Event variant). Pre-amendment _render_event / _render_event_to_log and their test classes removed under the no-backwards-compat rule. Issues #3 and #4 contracts amended in-place: #3 (CliPresenterState CLASS + FN block + helper FN blocks + _run_turn signature + _amain create_session demotion); #4 (TuiPresenterState CLASS + FN block + compose Static widget + _stream_turn_worker state construction). 209 tests GREEN; ruff clean. Bumps v0.1.0 → v0.2.0 (minor — output shape change breaks pre-amendment grep patterns like `[thinking] '`; no public API surface change beyond the rendering contract). Persistent-memory commit-along: captures the issue #12 decision, forward direction (require end_user_id for every access — declined worldtree-dev's requires_end_user_id offer because we'll send it universally), and the Heimdall scope-model foot-gun note (the "per-Tier-1-agent scope add" diagnosis was a phantom ask resolved by worldtree-dev's correction; agent.call:* baseline covers all Tier 1).
This commit is contained in:
@@ -321,7 +321,7 @@ BRIEF: Async orchestrator. Opens an authenticated httpx.AsyncClient, optionally
|
||||
PRE: [PRE-001 hard] args is a ParsedArgs (post-validation; PRE-002/PRE-003 of _parse_args hold) -- assert isinstance(args, ParsedArgs)
|
||||
POST: [POST-001 return_value] returns one of the documented exit codes (0, 2, 3, 12, 20, 21, 22)
|
||||
POST: [POST-002 side_effect] when args.new is True, exactly one POST /sessions was issued -- assert respx tracked the call
|
||||
POST: [POST-003 side_effect] when args.new is True, stderr contains "[create_session] session_id=... agent_id=..." before any stream events
|
||||
POST: [POST-003 side_effect] when args.new is True, stderr contains ". create_session: session_id=... agent_id=..." before any stream events (issue #12 amendment: `[create_session]` demoted to `. create_session:` to match the telemetry hierarchy; written directly by `_amain` — bypasses `state.render` since it is not a wire-level Event variant)
|
||||
POST: [POST-004 side_effect] the SIGINT handler is removed in cleanup (loop.remove_signal_handler called) -- verified via teardown probe in test fixtures
|
||||
ERROR_ROUTING:
|
||||
AgentNotFound:
|
||||
@@ -346,7 +346,7 @@ STEPS:
|
||||
WRITE stderr; RETURN 20
|
||||
ON httpx.ConnectError | httpx.ReadTimeout | httpx.TransportError as exc:
|
||||
WRITE stderr; RETURN 21
|
||||
WRITE f"[create_session] session_id={info.session_id} agent_id={info.agent_id}\n" to stderr
|
||||
WRITE f". create_session: session_id={info.session_id} agent_id={info.agent_id}\n" to stderr (issue #12: demoted prefix; direct write bypasses state.render)
|
||||
SET session_id = info.session_id
|
||||
ELSE:
|
||||
SET session_id = args.session_id # pre-validated non-None
|
||||
@@ -360,7 +360,7 @@ STEPS:
|
||||
loop.remove_signal_handler(signal.SIGINT)
|
||||
5. [cleanup] RETURN exit_code
|
||||
TESTS:
|
||||
happy_new_session_then_stream [happy,tracer]: respx mocks POST /sessions → 201 + the SSE POST → text+done; argv specifies --new --agent mimir → _amain returns 0; stderr has "[create_session]" before "[done]"
|
||||
happy_new_session_then_stream [happy,tracer]: respx mocks POST /sessions → 201 + the SSE POST → text+done; argv specifies --new --agent mimir → _amain returns 0; stderr has ". create_session:" before "[done]" (issue #12: demoted prefix; pre-amendment shape "[create_session]" forbidden)
|
||||
happy_existing_session [happy]: respx mocks the SSE POST only; argv specifies --session s-1 → _amain returns 0; respx tracked exactly 0 POST /sessions calls
|
||||
agent_not_found_exits_12 [error]: respx mocks POST /sessions → 404; --new → returns 12; stderr "[agent_not_found]"; stream_turn never invoked
|
||||
session_api_failed_exits_20 [error]: respx mocks POST /sessions → 500 with body → returns 20; stderr "[session_api_failed] status=500 body=..."
|
||||
@@ -370,58 +370,75 @@ TESTS:
|
||||
```
|
||||
|
||||
```contract
|
||||
FN _render_event(event: Event, *, stdout: TextIO, stderr: TextIO) -> None
|
||||
BRIEF: Pure event-to-output renderer. Routes `Text` deltas to stdout (with per-chunk flush per INV-010); routes every other Event variant to stderr with a labeled line. No I/O outside the two passed TextIO objects; no side effects on the event itself.
|
||||
PRE: [PRE-001 hard] event is an instance of one of the Event union variants -- assert isinstance(event, (WorkerPhase, Thinking, Text, TextBoundary, ToolStart, ToolResult, Done, Error, Cancelled))
|
||||
POST: [POST-001 side_effect] for Text events: stdout received event.content (no newline appended) AND stdout was flushed -- assert stdout.getvalue().endswith(event.content) and stdout.flush.called
|
||||
POST: [POST-002 side_effect] for non-Text-non-Done events (WorkerPhase, Thinking, TextBoundary, ToolStart, ToolResult, Error, Cancelled): stdout was NOT written to (INV-002); stderr received exactly one line ending in newline -- assert stdout.getvalue() == "" and stderr.getvalue().endswith("\n")
|
||||
POST: [POST-003 side_effect] for Done: stdout receives a single newline AND is flushed; stderr receives a single labeled line including `turn_id` (from event.sse_id.turn_id) + `model` + `duration_ms` (INV-002 carve-out — Done is the one non-Text variant that writes to stdout) -- assert stdout.getvalue() == "\n" and stderr.getvalue().startswith("[done]")
|
||||
ERROR_ROUTING:
|
||||
(none — pure function over the typed union; if an instance doesn't match any branch, PRE-001 catches it as an assertion failure)
|
||||
STEPS:
|
||||
1. [setup, flexibility=prescriptive] Match on `type(event)`:
|
||||
2. [branch, flexibility=prescriptive]
|
||||
CASE Text:
|
||||
stdout.write(event.content); stdout.flush()
|
||||
CASE Done:
|
||||
stdout.write("\n"); stdout.flush()
|
||||
stderr.write(f"[done] turn_id={event.sse_id.turn_id} model={event.model} duration_ms={event.duration_ms} usage={event.usage!r}\n")
|
||||
CASE Error:
|
||||
stderr.write(f"[error] turn_id={event.sse_id.turn_id} code={event.error_code} message={event.message!r}\n")
|
||||
CASE Cancelled:
|
||||
stderr.write(f"[cancelled] turn_id={event.turn_id} reason={event.reason!r} partial_message_id={event.partial_message_id}\n")
|
||||
CASE WorkerPhase:
|
||||
stderr.write(f"[worker_phase] phase={event.phase} turn_id={event.turn_id}\n")
|
||||
CASE Thinking:
|
||||
stderr.write(f"[thinking] {event.content[:200]!r}\n")
|
||||
CASE TextBoundary:
|
||||
stderr.write(f"[text_boundary] kind={event.kind} char_offset={event.char_offset}\n")
|
||||
CASE ToolStart:
|
||||
stderr.write(f"[tool_start] name={event.name} args={event.arguments!r}\n")
|
||||
CASE ToolResult:
|
||||
stderr.write(f"[tool_result] name={event.name} duration_ms={event.duration_ms} result={event.result!r:.200}\n")
|
||||
TESTS:
|
||||
text_to_stdout_only [happy,tracer]: Text(content="hello", sse_id=...) → stdout=="hello"; stderr==""; stdout.flush called once
|
||||
done_writes_newline_and_label [happy]: Done(sse_id=(42,5), model="glm5-turbo", duration_ms=1234, ...) → stdout=="\n"; stderr starts with "[done]" and contains "turn_id=42" + "model=glm5-turbo"
|
||||
error_to_stderr_only [happy]: Error(sse_id=(42,5), error_code="llm_output_invalid", message="m", ...) → stdout==""; stderr starts with "[error]"; contains "code=llm_output_invalid"; turn_id from sse_id
|
||||
cancelled_to_stderr_only [happy]: Cancelled(sse_id=(42,5), turn_id=42, reason="user", partial_message_id=7) → stderr starts with "[cancelled]" and contains "reason='user'" + "partial_message_id=7"; stdout==""
|
||||
worker_phase_to_stderr [happy]: WorkerPhase(phase="streaming", turn_id=42, ...) → stderr starts with "[worker_phase]"; stdout==""
|
||||
thinking_truncated [trace]: Thinking(content="a"*500, ...) → stderr line includes only first 200 chars of content
|
||||
tool_start_to_stderr [happy]: ToolStart(name="read_file", arguments={"path": "/x"}, ...) → stderr starts with "[tool_start] name=read_file args="
|
||||
tool_result_truncated [trace]: ToolResult(name="x", result="b"*500, duration_ms=42, ...) → stderr line repr truncated to ≤200 chars in result field
|
||||
text_boundary_to_stderr [happy]: TextBoundary(kind="sentence", char_offset=128, ...) → stderr starts with "[text_boundary]"
|
||||
invariant_inv003_stderr_only [scenario]: emit one of each non-Text variant in sequence; assert stdout buffer is empty after each (INV-003 verified by exhaustion of the non-Text union)
|
||||
CLASS CliPresenterState # issue #12 amendment
|
||||
BRIEF: Stateful per-turn presenter for `--send` mode. Replaces the stateless `_render_event` (removed). Owns `thinking_buffer`, `thinking_open`, `text_written_since_newline`; coalesces thinking-event deltas into one growing stderr line per run; demotes telemetry events with a `. ` prefix; guarantees a stdout `\n` boundary before terminal labels (`[done]`, `[error]`, `[cancelled]`) when assistant text has been streamed.
|
||||
PROPERTIES:
|
||||
thinking_buffer: list[str]
|
||||
thinking_open: bool
|
||||
text_written_since_newline: bool
|
||||
INV-WIRE-001: One instance per `_amain` call (issue #12 INV-008).
|
||||
```
|
||||
|
||||
```contract
|
||||
FN _run_turn(client: httpx.AsyncClient, session_id: str, content: str, sigint_event: asyncio.Event, *, stdout: TextIO, stderr: TextIO) -> int
|
||||
FN CliPresenterState.render(self, event: Event, *, stdout: TextIO, stderr: TextIO) -> None # issue #12 amendment
|
||||
BRIEF: Render one event into stdout/stderr with editorial hierarchy + thinking coalescing per issue #12 INV-001..INV-007. ASCII-only output (no Unicode in CLI). Demoted-telemetry events get `. ` prefix on stderr; load-bearing events (Text on stdout; Done/Error/Cancelled on stderr) get no prefix.
|
||||
PRE: [PRE-001 hard] event is an instance of one of the Event union variants
|
||||
POST: [POST-001 side_effect] for Thinking: append delta to thinking_buffer; write to stderr (with `. thinking: ` prefix on the first delta of the run, content-only on subsequent deltas); set thinking_open=True
|
||||
POST: [POST-002 side_effect] for non-Thinking when thinking_open: write `\n` to stderr; clear buffer; thinking_open=False; THEN render the new event
|
||||
POST: [POST-003 side_effect] for Text: write event.content to stdout (no forced newline); set text_written_since_newline = not event.content.endswith("\n") (Volva F4 fix)
|
||||
POST: [POST-004 side_effect] for Done/Error/Cancelled: if text_written_since_newline, write `\n` to stdout + flush + reset flag (INV-005); then write the load-bearing terminal label to stderr (no demotion prefix); for Done, format `duration=<autoscale>` + `usage <p> in -> <c> out (<t> total, <ci> cached)` via INV-006 / INV-007 helpers
|
||||
POST: [POST-005 side_effect] for demoted telemetry (WorkerPhase, TextBoundary, ToolStart, ToolResult): write `. <label>: <fields>\n` to stderr
|
||||
ERROR_ROUTING:
|
||||
(none at this level — pure dispatch over the typed union)
|
||||
TESTS:
|
||||
thinking_coalesce_single_run [happy,tracer]: Thinking("hello"), Thinking(" world"), Done → stderr has ". thinking: hello world\n" then "[done] ..."; no demotion prefix on [done]
|
||||
thinking_closes_on_first_non_thinking_event [happy]: Thinking, WorkerPhase → ". thinking: ...\n" then ". worker_phase: ..."
|
||||
thinking_closes_on_error [error]: Thinking, Error → thinking closes with \n; partial thinking preserved; "[error]" rendered (no demotion prefix)
|
||||
multiple_thinking_runs [scenario]: Thinking, Text, Thinking, Done → TWO ". thinking: " runs; stdout receives Text + INV-005 boundary before [done]
|
||||
text_then_done_newline_boundary [trace]: Text("answer"), Done → stdout=="answer\n"; stderr has [done]
|
||||
no_text_then_done_no_extra_newline [trace]: Done with no Text → stdout untouched
|
||||
newline_terminated_text_then_done [trace, Volva F4]: Text("answer\n"), Done → stdout="answer\n" exactly once (no double newline)
|
||||
cancelled_mid_thinking [scenario]: Thinking, Cancelled → thinking closes; "[cancelled]" without demotion prefix
|
||||
worker_phase_demoted [trace]: stderr line starts with ". worker_phase:" not "[worker_phase]"
|
||||
tool_start_demoted [trace]: ". tool_start:" prefix
|
||||
tool_result_truncated [trace]: ". tool_result:" + ≤200 chars of result repr
|
||||
text_boundary_demoted [trace]: ". text_boundary:" prefix
|
||||
duration_format_seconds [trace]: Done(duration_ms=5467) → "duration=5.5s" (not duration_ms=5467)
|
||||
duration_format_subsecond [trace]: Done(duration_ms=347) → "duration=347ms"
|
||||
duration_format_minutes [trace]: Done(duration_ms=72000) → "duration=1.2m"
|
||||
usage_format_ascii_arrow [trace]: Done → "usage 6756 in -> 126 out (6882 total, 0 cached)" (ASCII arrow, not Unicode)
|
||||
state_reset_per_amain [trace]: two independent CliPresenterState() instances; the second starts with thinking_open=False
|
||||
```
|
||||
|
||||
```contract
|
||||
FN _format_duration_ms(ms: int) -> str # issue #12 INV-006 helper
|
||||
BRIEF: Auto-scale duration formatting. ms<1000 → "{ms}ms"; ms<60_000 → "{s:.1f}s"; else "{m:.1f}m". Locale-blind.
|
||||
TESTS:
|
||||
subsecond: 347 → "347ms"
|
||||
exact_one_second: 1000 → "1.0s"
|
||||
fractional_seconds: 5467 → "5.5s"
|
||||
exact_one_minute: 60000 → "1.0m"
|
||||
fractional_minutes: 72000 → "1.2m"
|
||||
zero: 0 → "0ms"
|
||||
```
|
||||
|
||||
```contract
|
||||
FN _format_usage(usage: dict, *, arrow: str) -> str # issue #12 INV-007 helper
|
||||
BRIEF: Natural-language usage formatting. arrow="->" for CLI (ASCII), arrow="→" for TUI (Unicode).
|
||||
TESTS:
|
||||
ascii_arrow: arrow="->" → "6756 in -> 126 out (6882 total, 0 cached)"
|
||||
unicode_arrow: arrow="→" → "6756 in → 126 out (6882 total, 0 cached)"
|
||||
```
|
||||
|
||||
```contract
|
||||
FN _run_turn(client: httpx.AsyncClient, session_id: str, content: str, sigint_event: asyncio.Event, *, stdout: TextIO, stderr: TextIO, state: CliPresenterState | None = None) -> int # issue #12 amendment: `state` kwarg threaded by `_amain`; defaults to a fresh state when omitted so tests can construct standalone
|
||||
BRIEF: Drive `stream_turn`, render events, race each `__anext__()` against `sigint_event.wait()` so a SIGINT lands within one event boundary. On first SIGINT (with last_turn_id known), spawn `cancel_turn` as a background task and keep draining until the `Cancelled` terminal event arrives. Map terminal events and uncaught exceptions to exit codes per the Data flow table.
|
||||
PRE: [PRE-001 hard] client is not None -- assert client is not None
|
||||
PRE: [PRE-002 hard] session_id is a non-empty string -- assert session_id and isinstance(session_id, str)
|
||||
PRE: [PRE-003 hard] content is a non-empty string -- assert content and isinstance(content, str)
|
||||
PRE: [PRE-004 hard] sigint_event is an asyncio.Event -- assert isinstance(sigint_event, asyncio.Event)
|
||||
POST: [POST-001 return_value] returns one of (0, 2, 3, 20, 21, 22) — terminal-event-driven OR exception-mapped
|
||||
POST: [POST-002 side_effect] each yielded event passed through _render_event exactly once -- spy on _render_event call count == event count
|
||||
POST: [POST-002 side_effect] each yielded event passed through CliPresenterState.render exactly once -- spy on CliPresenterState.render call count == event count (issue #12 amendment: was _render_event)
|
||||
POST: [POST-003 side_effect] sigint mid-stream issues exactly one cancel_turn HTTP call -- assert respx tracked one POST /sessions/{id}/turns/{turn_id}/cancel
|
||||
POST: [POST-004 side_effect] sigint before any event yields zero cancel_turn calls -- INV-008: turn_id is unknown so cancel cannot be issued
|
||||
POST: [POST-005 side_effect] cancel_failed during sigint drains writes "[cancel_failed]" to stderr but does NOT raise -- INV-009: primary exit code is the stream's terminal-event code
|
||||
@@ -493,7 +510,7 @@ STEPS:
|
||||
# exception type → stderr label + exit_code per ERROR_ROUTING
|
||||
RETURN <mapped exit code>
|
||||
last_turn_id = event.sse_id.turn_id
|
||||
_render_event(event, stdout=stdout, stderr=stderr)
|
||||
state.render(event, stdout=stdout, stderr=stderr) # issue #12 amendment
|
||||
IF isinstance(event, Done):
|
||||
IF NOT cancelling: sigint_task.cancel()
|
||||
RETURN 0
|
||||
|
||||
Reference in New Issue
Block a user