fix(cli): show a turn as running when it starts, not when it speaks

In transcript mode the status row said idle from submit until the model's
first token. Measured against a live aria, with a watcher that only
subscribes: the daemon broadcast the committed question 34.5ms after
submit and the first content delta at 2263.6ms. Two point two seconds of
honest signal, thrown away, and much worse on a cold turn.

Two separate causes.

THE GUARD WAS ONE LINE TOO WIDE. armThinking does two things, and they
have different owners. t.status.beginTurn() sets state the PAGER paints,
in its own footer, through footerStanza. t.in.OpenThinking() opens a live
region belonging to the inline renderer alone, which must not paint while
the pager holds the screen. f28e6e1 guarded both on t.tr.active and its
comment says why: "No-op in the pager (it renders the footer itself)".
That is true of the second half and false of the first, because what the
pager renders the footer FROM was inside the guard. So the guard is kept
and split rather than deleted: status is set in both views, the incipit
region stays inline-only.

THAT ALONE FIXES ONLY THE SENDER. A prompt typed into the pager goes
through commandSend, which never called armThinking; a listener never
submits at all; a queued prompt begins its turn long after its own call
returned. All three learn from the wire, so the wire is what arms them.
Client gains OnTurnOpen, beside OnClosed, OnLive and OnMetrics, fired
when a turn takes the open slot.

It belongs in the client and not in the caller because "which turn owns
the open slot" is already decided there, once, in claimsOpen, and that
file warns in its own comments what a second opinion computed upstream
costs: three branches disagreeing about the buffer is what let a page of
history destroy a live turn. Recomputing the same predicate in the CLI
would be that mistake again. Catch-up history arrives through Merge,
which fires no callbacks, and claimsOpen refuses a sealed part, so
neither reading old turns nor a mid-turn reconcile lights the spinner.
The callback is edge triggered on the turn id, since the inquiry rides
along on later frames of the same turn.

Four tests in internal/cli/thinking_armed_test.go, verified to fail
without the fix: three fail with the exact symptom, and the fourth is the
sealed-frame guard, which must pass either way.

Also documents the trap that ate the first draft of that file.
thinking_arm_test.go compiles only on GOARCH=arm, so it was not a test
that failed, it was a file that did not exist, and build, vet and test
were all clean while it did not run.

Author: gluck

internal/cli/livelog_bridge.go

@@ -179,6 +179,18 @@ func (t *livelogTurn) wireClient() {
 			}
 		}
 	}
+	t.client.OnTurnOpen = func(int) {
+		// THE WIRE, NOT THE SUBMIT. A session that sent the prompt already
+		// armed itself in armThinking; this is for every other way a turn
+		// starts under a live view: `:send` from inside the pager, a listener
+		// watching an aria someone else is driving, a queued prompt that only
+		// begins when the turn ahead of it ends. The daemon broadcasts the
+		// committed question before it calls a provider, so this is early.
+		t.status.beginTurn()
+		if t.tr.active {
+			t.tr.render()
+		}
+	}
 	t.client.OnLive = func(m aria.Message) {
 		newOpen := m.Turn != t.open.Turn
 		t.open = m
@@ -305,15 +317,25 @@ func (t *livelogTurn) openOverflows(nodes []livedoc.Node) bool {
 	return false
 }
 
-// armThinking pins the footer the instant a submit is accepted: before the
-// prompt has round-tripped, before the model's first token. The footer is a
-// permanent fixture of the view, so it must not wait on the stream.
+// armThinking shows the turn as running the instant a submit is accepted:
+// before the prompt has round-tripped, before the model's first token.
+//
+// The two halves have different owners, which is why the pager guard sits
+// where it does. status is shared: the pager paints it in its own footer
+// (transcript.go, footerStanza), so it must be set in BOTH views. The incipit
+// live region is the inline renderer's alone, and opening one while the pager
+// owns the screen would paint underneath it.
+//
+// This used to guard both halves on t.tr.active, which meant the pager was
+// never told a turn had started. It then had nothing to draw until the first
+// content frame, roughly two seconds of blank on a warm turn and much more on
+// a cold one.
 func (t *livelogTurn) armThinking() {
+	t.status.beginTurn()
 	if t.tr.active || t.thinkingOpen {
 		return
 	}
 	t.thinkingOpen = true
-	t.status.beginTurn()
 	t.in.OpenThinking(livedoc.RoleOutput)
 }
 

internal/cli/thinking_armed_test.go

@@ -0,0 +1,111 @@
+package cli
+
+import (
+	"bytes"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/jack-work/figaro/api/aria"
+	"github.com/jack-work/figaro/api/livedoc"
+)
+
+// THE SPINNER MUST NOT WAIT FOR THE MODEL.
+//
+// The daemon commits the user's question and broadcasts it BEFORE it calls a
+// provider (internal/figaro/turn.go, appendUserPrompt then ariaSrv.OpenInquiry),
+// so a client holds "this turn is running" while the model is still thinking.
+// Measured on a live aria: the inquiry frame landed 35ms after submit and the
+// first content delta 2264ms after it. Everything the pager drew in that gap
+// said idle.
+//
+// Two separate defects produced that gap, so there are two tests here.
+
+// One: the pager was never told. armThinking guarded BOTH of its halves on
+// t.tr.active, and one of those halves is the shared sessionStatus that the
+// pager paints in its own footer (transcript.go, footerStanza). The incipit
+// live region genuinely is inline-only and stays guarded.
+func TestArmThinkingMarksTheTurnRunningInThePager(t *testing.T) {
+	var out bytes.Buffer
+	status := newSessionStatus("aria1234", time.Now())
+	lt := newLivelogTurn(&out, 80, 20, &renderSettings{}, "aria1234", time.Now(), status, nil, nil)
+	lt.enterTranscript()
+
+	lt.armThinking()
+
+	if !status.turnRunning() {
+		t.Fatal("armThinking left the pager showing idle; the footer has nothing to draw until first content")
+	}
+	// The incipit region is the inline renderer's alone. Opening one under the
+	// pager paints beneath a screen we do not own.
+	if lt.thinkingOpen {
+		t.Fatal("armThinking opened an incipit live region while the pager held the screen")
+	}
+}
+
+// Two: nothing armed at all on the paths where the submit is not ours. A
+// `:send` typed into the pager goes through commandSend, a listener never
+// submits anything, and a queued prompt starts its turn long after its own
+// call returned. All three learn from the wire, so the wire is what arms them.
+func TestInquiryFrameMarksTheTurnRunning(t *testing.T) {
+	var out bytes.Buffer
+	status := newSessionStatus("aria1234", time.Now())
+	lt := newLivelogTurn(&out, 80, 20, &renderSettings{}, "aria1234", time.Now(), status, nil, nil)
+	lt.enterTranscript()
+
+	if status.turnRunning() {
+		t.Fatal("a fresh session claims a turn is running")
+	}
+	// The shape the daemon actually sends first: the question, no nodes, no
+	// live region. Nothing here says what the model will say.
+	lt.apply(inquiryPage(7, "WHAT IS THE GAP?"))
+
+	if !status.turnRunning() {
+		t.Fatal("the inquiry frame did not mark the turn running; the spinner waits out the model's latency")
+	}
+}
+
+// The companion that proves the two above can fail: content-bearing frames
+// arm the status by a different route (OnLive), so a test that fed one would
+// pass whether or not the inquiry path works at all.
+func TestInquiryArmingIsNotJustOnLive(t *testing.T) {
+	var out bytes.Buffer
+	status := newSessionStatus("aria1234", time.Now())
+	lt := newLivelogTurn(&out, 80, 20, &renderSettings{}, "aria1234", time.Now(), status, nil, nil)
+	lt.enterTranscript()
+
+	lt.apply(inquiryPage(7, "WHAT IS THE GAP?"))
+	armedByInquiry := status.turnRunning()
+
+	status.setTurn(turnStatusIdle)
+	lt.apply(aria.Page{Parts: []aria.TurnPart{{Turn: aria.Turn{
+		ID: 7, Live: &aria.Live{From: 0, Nodes: []aria.NodeDelta{{
+			ID: 0, Set: map[string]any{"type": "prose", "markdown": "the answer"},
+		}}},
+	}}}})
+	armedByContent := status.turnRunning()
+
+	if !armedByInquiry || !armedByContent {
+		t.Fatalf("inquiry=%v content=%v, want both true", armedByInquiry, armedByContent)
+	}
+}
+
+// A turn that is merely being READ is not a turn that is running. History
+// reaches the client through Merge, which fires no callbacks, but a sealed
+// turn can also arrive as a pushed frame (reconcileAriaServer fans one out
+// per turn on a mid-turn error path). Neither may light the spinner.
+func TestSealedFrameDoesNotMarkTheTurnRunning(t *testing.T) {
+	var out bytes.Buffer
+	status := newSessionStatus("aria1234", time.Now())
+	lt := newLivelogTurn(&out, 80, 20, &renderSettings{}, "aria1234", time.Now(), status, nil, nil)
+	lt.enterTranscript()
+
+	lt.apply(aria.Page{Parts: []aria.TurnPart{{Turn: aria.Turn{
+		ID: 4, Inquiry: "AN OLD QUESTION", Sealed: true,
+		Nodes: []livedoc.Node{{Type: livedoc.NodeProse, Markdown: "an old answer"}},
+	}}}})
+
+	if status.turnRunning() {
+		t.Fatalf("a sealed turn lit the spinner:\n%s", strings.TrimSpace(out.String()))
+	}
+}

internal/livelog/aria/client.go

@@ -50,6 +50,23 @@ type Client struct {
 	OnLive    func(Message)
 	OnDesync  func(sinceLT int)
 	OnMetrics func(Metrics)
+	// OnTurnOpen fires once per turn, when a turn takes the open slot. It is
+	// the counterpart of the server's turn.done: the daemon commits the user's
+	// question and broadcasts it before it calls a provider, so this lands
+	// while the model is still thinking rather than with its first token. A
+	// status surface that waits for OnLive instead waits out the whole of the
+	// model's latency, which on a warm turn is two seconds and on a cold one
+	// is much worse.
+	//
+	// It belongs here and not in a caller because "which turn owns the open
+	// slot" is decided here, once (see claimsOpen). A second opinion computed
+	// upstream is the exact mistake this file already warns about.
+	OnTurnOpen func(turn int)
+
+	// openedTurn is the turn id OnTurnOpen last reported. The inquiry rides
+	// along on later frames of the same turn, so the callback is edge
+	// triggered on the id rather than fired per frame.
+	openedTurn int
 }
 
 // heldInquiry is a turn's question and its per-sender split, parked together
@@ -266,6 +283,8 @@ func (c *Client) Apply(p Page) {
 	c.mu.Lock()
 	var finalized []Message
 	desync := -1
+	// opened is the turn that took the open slot on this page, 0 for none.
+	opened := 0
 	metrics := p.Metrics
 
 	for _, part := range p.Parts {
@@ -293,6 +312,13 @@ func (c *Client) Apply(p Page) {
 			// slot for it would destroy the live turn on its way past.
 			if staged && !part.ClippedHead {
 				c.store.ClaimOpen(id)
+				// This is the earliest a client can know a turn is running:
+				// the question is committed and the provider has not been
+				// called yet. Reported outside the lock with everything else.
+				if id > c.openedTurn {
+					c.openedTurn = id
+					opened = id
+				}
 			}
 		}
 
@@ -430,6 +456,11 @@ func (c *Client) Apply(p Page) {
 	if metrics != nil && c.OnMetrics != nil {
 		c.OnMetrics(*metrics)
 	}
+	// Ahead of the content callbacks: a surface that reacts to both should
+	// hear "a turn started" before it hears what the turn said.
+	if opened != 0 && c.OnTurnOpen != nil {
+		c.OnTurnOpen(opened)
+	}
 	for _, m := range finalized {
 		if c.OnClosed != nil {
 			c.OnClosed(m)

skills/figaro/contributing/maintaining.md

@@ -246,6 +246,32 @@ ran: same failure, and it has cost this project several cycles.
 
 Prove the fixture can fail, then trust what it says.
 
+### A test file can silently not exist
+
+Go decides which files compile from the FILENAME, before it reads a line of
+them. A file whose name ends in `_<goos>.go` or `_<goarch>.go` is constrained
+to that platform, and `_test` in front of the suffix does not exempt it:
+`thinking_arm_test.go` compiles only on GOARCH=arm, so on this machine it is
+not a file with a failing test in it, it is a file that is not there.
+
+The failure mode is the quiet one. `go build` is clean, `go vet` is clean,
+`go test ./internal/cli/` is clean, and `-run TestTheThing` answers
+`ok ... [no tests to run]`, which reads as a typo in the pattern rather than
+as a missing file. Nothing anywhere says the word "arm".
+
+The suffixes that bite are ordinary English: `_arm`, `_386`, `_ios`, `_js`,
+`_plan9`, `_windows`, `_linux`, `_darwin`, `_android`, `_wasm`. So do not name
+a file after a verb ending in "arm", or after a subsystem called "linux", and
+when a new test does not run, check that the compiler agrees it exists before
+touching the test itself:
+
+```bash
+go list -f '{{range .TestGoFiles}}{{.}}{{"\n"}}{{end}}' ./internal/cli/ | grep <name>
+```
+
+Empty output means the filename is the bug. Rename it (`thinking_armed_test.go`)
+and the test appears.
+
 ### An equivalence oracle is not scaffolding
 
 When you replace an implementation with a faster one that must behave