Repository navigation
fix: echo submitted user messages into the TUI transcript - #2
Merged
Merged
Conversation
Approval lives in the agent loop, not the tool, per CLAUDE.md invariant 5: tools declare NeedsApproval via an optional interface, the loop consults an injected Approver before executing, and a denial becomes a normal IsError ToolResult so the loop continues.
Atomic temp+rename write with a line-diff Preview surfaced to the loop's approval prompt; wires the --allow-write flag CLAUDE.md's CLI skeleton task deferred to this phase.
StreamEvents centralizes the close-exactly-once/exit-on-cancel contract in one place so 6.2/6.3's adapters don't each reimplement it; CollectStream is the shared consumer-side counterpart that reassembles a canonical Response from an event stream, reused by both adapters' equivalence tests.
Accumulates input_json_delta fragments as raw text per content-block index and unmarshals once at content_block_stop, never parsing a partial chunk. Verified against a 7-chunk fixture that splits a tool argument's \u escape mid-character, asserting byte-identical reassembly and Response-level equivalence with the non-streaming path.
Unlike Anthropic, Ollama doesn't fragment a tool call's arguments across lines, so each line's tool_calls become an immediate ToolUseStart/ ToolUseDone pair rather than accumulated deltas. Reuses the existing string-vs-object argument normalization per line.
One shared suite (canonical round-trip, tool call, error surfaces as an error, cancellation, stream/non-stream equivalence) driven by a small Harness each adapter supplies; the suite itself never imports a provider SDK, so a third provider only needs its own Harness.
Renderer consumes a provider's llm.Event stream and returns the assembled Response, mirroring llm.CollectStream but with the side effect of printing as text arrives. PlainRenderer is the default, non-interactive implementation, plus an optional ToolStatusReporter hook renderers can implement to learn about tool runs the loop performs after a round's events finish. Agent gains RunStream, sharing round bookkeeping (guards, repetition, history) with Run via a new runRound helper so the two entry points can't drift on policy. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1KJRVUrFgkDupk6NzkHKq
Model implements the interactive TUI's state machine (idle / streaming / tool running / approval modal) over bubbles' viewport, textarea, and spinner, with glamour rendering finished assistant text. TUIRenderer and TUIApprover bridge the agent loop's Renderer/Approver contracts to a running *tea.Program via Send, keeping Model itself free of any agent or provider dependency. docs/tui-check.md is the manual walkthrough 7.2's AC calls for; it can't be automated. runChat is no longer a placeholder: it resolves a real provider via llm.New, builds a tool registry (read_file/list_dir, write_file behind --allow-write — run_command is left out, since it needs a binary allowlist config has no flag for yet), and drives either the plain or TUI path through Agent.RunStream. It's now a package var so cmd/pico's tests can substitute a stub instead of exercising a real provider. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1KJRVUrFgkDupk6NzkHKq
Agent records each completed turn's total Usage (summed across that turn's provider rounds, since a turn can span several tool-calling rounds) and exposes TurnUsages/CumulativeUsage. The Ollama adapter estimates InputTokens/OutputTokens from character length whenever a response's real counts come back 0, in both Chat and Stream, so cumulative tracking keeps making progress against a version/model that omits them. cmd/pico gains a small slash-command dispatcher in the plain REPL; /usage is its first command, printing cumulative and last-turn totals. Split out runREPL so it's testable without a real terminal. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1KJRVUrFgkDupk6NzkHKq
history.CompactionBoundary finds a safe cut point by construction: it always lands at the start of a genuine user turn (one carrying a Text block), and a new turn can only start once every earlier ToolUse has been answered (CLAUDE.md invariant 3) — so the boundary can never split a pair. History.Compact replaces everything before it with one synthetic assistant message and refuses to commit a result that fails Validate(), as a backstop rather than the primary safety mechanism. Agent.maybeCompact is the trigger policy: once EstimateTokens(history) crosses a configured fraction of the context window, it asks the provider to summarize the elided turns (prompt pinned by a golden test) and calls Compact. It's best-effort — a failed summarization call just skips compaction for this turn rather than failing it. cmd/pico wires a default policy (Ollama's num_ctx, or a constant for Anthropic since config has no per-model window size yet) into both the plain and TUI paths. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1KJRVUrFgkDupk6NzkHKq
History gains LoadInto (replace a shared *History's messages in place, validating before committing so a corrupt file never leaves it partially loaded) and Reset. cmd/pico's new session type resolves a sessions directory and a named session's file path, used both at startup (--session resumes an existing file or starts a fresh one) and by the plain REPL's /new, /save, and /load commands. History auto-saves after every turn in both the plain REPL and the TUI driver, so killing the process mid-session and resuming replays the full prior context. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1KJRVUrFgkDupk6NzkHKq
recordutil.Recorder wraps an http.RoundTripper to capture a live request/response pair without altering how it's sent; Scrub redacts exact secret values plus any sk-shaped literal. Each adapter gets a TestRecordLiveExchange, skipped unless RECORD=1 and real credentials are present, that drives one real Chat call and writes the scrubbed exchange to testdata/golden/live_exchange — never touching the network in normal `make test` runs. TestGoldenFixturesContainNoSecrets statically greps every checked-in fixture for sk- and the live API key env var. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1KJRVUrFgkDupk6NzkHKq
config.LogLevel is now a validated type (ErrUnknownLogLevel joins ErrUnknownProvider/ErrUnknownToolsMode), and cmd/pico's setupLogging points the default slog logger at stderr with that severity, so piped chat output on stdout stays clean regardless of level. recordutil gains LogJSON/LogBytes: a debug-only, level-gated dump that scrubs and truncates a request/response before it ever reaches slog. Both adapters call it around their Chat and Stream exchanges. Anthropic's Provider keeps its API key only for exact-value redaction — the SDK never puts it in the request body, only a header, so recordutil's generic sk-shaped pattern already covers the realistic case. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1KJRVUrFgkDupk6NzkHKq
lint-test runs make lint, make test, and a separate go test -race pass on Go 1.26 and 1.27 (the floor ollama/ollama/api's own go.mod raised it to in phase 3.1). build cross-compiles for linux/darwin amd64/arm64 with CGO_ENABLED=0 — verified locally for all four combinations before adding this. Neither job needs any secret: the suite is offline by construction, and 9.1's RECORD=1-gated live tests stay skipped without it set. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1KJRVUrFgkDupk6NzkHKq
Covers what the project is, install/quickstart for both backends, the full flag table, the plain REPL's slash commands vs the TUI, the safety model, a mermaid diagram of the agent loop, and an honest section on what building against a small local model actually exposed (invented tool names, repeated calls, string-vs-object arguments, the UTF-8/JSON decoder gap found while designing 6.2's SSE fixture). Writing this surfaced two real gaps that weren't visible before: --tools and --stream are parsed and validated but never read by cmd/pico, so selecting prompted-tools mode or turning off streaming currently does nothing. Documented under Known Limitations alongside the run_command and TUI-slash-command gaps already noted in earlier commits, rather than silently fixed under this task's banner. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1KJRVUrFgkDupk6NzkHKq
resolveProvider wraps the resolved provider with prompted.Wrap when --tools=prompted, rejecting that combined with --tui up front (a clear startup error) rather than letting it fail confusingly on the first turn — prompted.Provider.Stream always errors, and the TUI always drives the agent through RunStream. newTurnRunner picks Run or RunStream per --stream, printing Run's result itself since PlainRenderer only ever consumes a stream. --tools=prompted forces non-streaming regardless of --stream, since the same Stream limitation applies there too; a log line says so when it overrides an explicit --stream=true. Updates the README's Known limitations section to drop the two entries this closes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1KJRVUrFgkDupk6NzkHKq
config.AllowCommands carries a comma-separated binary allowlist from the new --allow-commands flag. buildRegistry registers run_command only when it's non-empty, using the same defaultToolTimeout the agent loop already applies to every other tool — an empty allowlist would leave the tool present but unconditionally useless, so it stays unregistered instead. Updates the README: documents the flag, drops the now-closed run_command entry from Known limitations, and fixes a stale cross- reference to it left over from the --tools/--stream wiring commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1KJRVUrFgkDupk6NzkHKq
ui gains CommandOutput/commandOutputMsg: a way to append text to the transcript that, unlike a turn, never touches the spinner/streaming state machine (cmd/pico's driver sends it without ever sending TurnStarted). cmd/pico's TUI driver goroutine now recognizes a leading slash the same way the plain REPL's runREPL does, via the same slashCommand/handleCommand pair — /usage, /new, /save, /load work identically in both. Since the TUI never echoes what was typed, runTUICommand prefixes the output with the command line itself so it's clear what ran. Split out from the driver goroutine so it's testable without a running *tea.Program, mirroring why runREPL was split out from runPlainChat. Updates README (both surfaces now documented as understanding the same commands; the "TUI doesn't parse slash commands" Known Limitation is gone) and docs/tui-check.md with a manual check for the four commands. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1KJRVUrFgkDupk6NzkHKq
config.ContextWindow carries a new --context-window flag (default 200,000, same value the hardcoded constant used) into compactionPolicy for every provider except Ollama, which keeps using NumCtx — that's already an explicit, required value, so compaction reuses it rather than tracking a second number that could drift from it. Removes the README's now-empty Known Limitations section: this was the last entry in it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P1KJRVUrFgkDupk6NzkHKq
Draw a home screen at startup: a white ASCII skull beside a red-framed box with the greeting, provider/model/user, launch directory, recent sessions and the context budget. The frame is width-responsive, so NewModel now takes a BannerInfo and renders on WindowSizeMsg rather than taking a pre-rendered string; the plain REPL sizes it from term.GetSize. cmd/pico projects config, cwd and sessions onto BannerInfo so internal/ui reads neither config nor disk. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013Wf5VDWF6MBSW5ATG9ND1f
Model.Update has a value receiver, so bubbletea's real dispatch gives
each incoming message a fresh copy of Model. liveText was a
strings.Builder value field, which panics ("illegal use of non-zero
Builder copied by value") the moment a second copy writes to it -
meaning any turn that streamed more than one text chunk would crash
the TUI. Switched liveText to a plain string, matching the already-
string completed field.
10.1: the TUI model now tracks elapsed time (a self-rescheduling once-per-second tick), an estimated count of tokens streamed so far, and wires Esc to interrupt the turn alongside Ctrl+C, matching the "esc to interrupt" hint. Counters reset at the start of every turn and never leak into the next one. 10.2: the static "thinking"/"running tools" word is replaced with one drawn from a whimsical vocabulary, rotated every few seconds and never repeating itself back-to-back. Randomness is injectable (wordRand) so rotation boundaries are deterministic in tests.
Two issues corrupted the screen before the first prompt:
- An info-level startup log ("ollama: context window configured")
printed to stderr before program.Run() took over the terminal,
since stderr shares the same screen as the TUI's alt-screen buffer.
--tui now defaults to a quieter log level unless --log-level is
passed explicitly.
- The glamour renderer was built with WithAutoStyle(), which queries
the terminal's background color via an OSC 11 escape sequence at
render time. Once bubbletea owns the terminal in raw mode, that
query races with its own input reader and leaks raw escape bytes
onto the screen - on every resize, not just startup. The style is
now resolved once, via lipgloss.HasDarkBackground(), before
tea.Program takes over, and handleResize builds the renderer from
that fixed style instead of re-querying.
fencedJSONRe matched a fence with any label, not just json or none: "(?:json)?" is optional, and the following "\s*\n?" was also fully optional, so a label like "plaintext" or "python" was silently absorbed into the capture instead of rejecting the match. A model's genuine final answer wrapped in an unrelated language fence (common for code-focused models) was misread as a malformed tool call, producing a bogus "tool not found" error sent back to the model. Found live while debugging why --tools=prompted's final answer sometimes triggered an extra, confusing round-trip with qwen2.5-coder:7b: debug logs showed the model's correctly-fenced ```python answer being reinterpreted as a parse error. Fixed by requiring a real line break immediately after the optional "json" label, so only a json-or-unlabeled fence can match.
Some Ollama models that report native tool support (reproduced live
against qwen2.5-coder:7b) still write the call as plain JSON text in
Content instead of populating the structured ToolCalls field -
CLAUDE.md's "sometimes narrate a tool call in prose instead of
calling it", previously unhandled in native mode. This left --tui
with no working tool-calling path for such a model, since
--tools=prompted (which already recovers this) is incompatible with
the TUI's streaming requirement.
Adds narratedToolCall: recognizes content that is *exactly* one JSON
object shaped {"name":string,"arguments":object} whose name matches a
tool offered this turn, narrow enough that a model's legitimate
"output JSON" answer isn't misread as a call. Wired into both the
non-streaming path (fromMessage) and streaming (Stream): the streaming
side withholds TextDelta for content that still looks like it could
resolve into a bare JSON object, flushing it unchanged the moment
that stops being true (an ordinary reply pays no cost - buffering
never begins unless the first non-whitespace character is '{'), and
converts it into a real ToolUse if the full response matches once
sent.
Verified live: read_file now genuinely executes under --tui with
qwen2.5-coder:7b (✓ status marker, zero raw JSON reaching the
screen), where it previously just printed the JSON as text.
Local models frequently advertise native tool support (passing the /api/show capability probe) but still narrate a fenced tool call as prose instead of using Ollama's structured tool_calls field, which --tools=native silently accepts as a final answer. Default ollama sessions to --tools=prompted, which parses that fenced-JSON shape directly, unless --tools is passed explicitly. --provider=anthropic is unaffected. --tui + ollama with no explicit --tools now hits the existing prompted/--tui guard error, since prompted mode can't stream.
Drives handleCommand and /help off one commandTable of {name, args,
summary, run} so adding a command means adding a table entry. An
unknown command now suggests its closest match by edit distance
instead of a bare error.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M5Rus6L14ygYTQwnMxwcFD
/clear resets the terminal scrollback (an ANSI clear in the plain REPL, the transcript state in the TUI) without touching the agent's history. /compact forces phase 8.2's summarization pass immediately via a new agent.ForceCompact, sharing its boundary/summarize/commit logic with the automatic maybeCompact, and reports the estimated token count before and after. /exit saves the active session, then signals the same clean return path Ctrl+D already takes (runREPL returns nil; the TUI quits the bubbletea program) via two sentinel errors the caller reacts to, since neither action can be expressed by writing text to a command's output. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M5Rus6L14ygYTQwnMxwcFD
/cd re-roots the workspace sandbox via a new Sandbox.Reroot, reached through the always-registered read_file tool's new Sandbox() getter rather than threading a sandbox parameter through every command-dispatch call site. Reroot refuses (leaving the sandbox untouched) a target that doesn't exist or can't be resolved, the same way NewSandbox already does at startup. /model switches the active provider's model, validated first via a new llm.ModelSwitcher interface (ValidateModel/SetModel) that both adapters implement: Anthropic through a real Models API lookup, Ollama by probing /api/show for the candidate name (refactored out of the existing tool- capability probe). It's checked via a type assertion rather than added to llm.Provider, per CLAUDE.md invariant 2. An unknown model errors without touching the provider or the compaction policy; an explicit smaller context window updates the policy so the existing maybeCompact check fires naturally on the next turn. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M5Rus6L14ygYTQwnMxwcFD
Enter in the Model's handleKey sent the input to the submit channel and reset the textarea, but never wrote the text anywhere the viewport could render it — only the assistant's reply and command output ever reached m.completed. The user's own message vanished the instant they hit Enter. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M5Rus6L14ygYTQwnMxwcFD
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
m.completed, so the viewport never showed what the user typed — only assistant replies and command output ever reached the transcript.m.completedon submit, before clearing the textarea.Test plan
make testmake lintTestModelEnterSubmitsInputOnlyWhenIdlethat the submitted text lands inm.completed🤖 Generated with Claude Code
https://claude.ai/code/session_01M5Rus6L14ygYTQwnMxwcFD