Skip to content

Commit f52b324

Browse files
author
gluck
committed
store, cli: the door is bounded; repairing history is a verb you run
Gluck's ruling, and it corrects the complexity I reported an hour ago: THE FIG IR IS CORRECT BY CONSTRUCTION. Every append through the door closes what it opened, so the only invokes that can be open belong to the LAST round -- nothing may scan history to discover that. ir_door.go roundLookback = 64 bounds the door's tail read. My previous commit made it O(history) on the path EVERY APPEND takes, to answer a question about the tail. doctor.go RepairToolCalls / 'figaro doctor toolcalls' closes every unanswered invoke in a whole history, on demand. That damage is history written before the door existed -- a one-time migration, so it is paid once rather than by every append forever. A BURIED ORPHAN THEREFORE SURVIVES THE DOOR, DELIBERATELY, and the test says so from both sides: the door closes NOTHING ('the door scanned past the current round') and the repair then closes exactly one. Sibling review (240b8f5e) flagged that the buried-orphan property looked uncovered after the rename -- the old test name is gone. It is covered, at lines 80-85 of the same test rather than in a second one. Their instinct is right that two properties want two tests; the coverage is real either way.
1 parent cbe63bf commit f52b324

4 files changed

Lines changed: 166 additions & 35 deletions

File tree

‎internal/cli/cli.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1416,7 +1416,7 @@ flow writes both through the daemon, so a client never has to know the path.`,
14161416
},
14171417
Run: func(ctx *cmdkit.RunContext) error {
14181418
if len(ctx.Args) != 1 {
1419-
return fmt.Errorf("usage: doctor <gc [--dry-run] | schema | term | mem [-j] | provider [--id <aria>] [-c N] [-j] | librettos [--dry-run] | skills [-j] | ttl [-j]>")
1419+
return fmt.Errorf("usage: doctor <gc [--dry-run] | schema | term | mem [-j] | provider [--id <aria>] [-c N] [-j] | librettos [--dry-run] | toolcalls [--dry-run] | skills [-j] | ttl [-j]>")
14201420
}
14211421
switch ctx.Args[0] {
14221422
case "gc":
@@ -1431,12 +1431,14 @@ flow writes both through the daemon, so a client never has to know the path.`,
14311431
return runDoctorProvider(ctx.Flag("id"), ctx.Flag("count"), ctx.BoolFlag("json"))
14321432
case "librettos":
14331433
return runDoctorLibrettos(ctx.BoolFlag("dry-run"))
1434+
case "toolcalls":
1435+
return runDoctorToolCalls(ctx.BoolFlag("dry-run"))
14341436
case "skills":
14351437
return runDoctorSkills(ctx.BoolFlag("json"))
14361438
case "ttl":
14371439
return runDoctorTTL(ctx.BoolFlag("json"))
14381440
}
1439-
return fmt.Errorf("usage: doctor <gc [--dry-run] | schema | term | mem [-j] | provider [--id <aria>] [-c N] [-j] | librettos [--dry-run] | skills [-j] | ttl [-j]>")
1441+
return fmt.Errorf("usage: doctor <gc [--dry-run] | schema | term | mem [-j] | provider [--id <aria>] [-c N] [-j] | librettos [--dry-run] | toolcalls [--dry-run] | skills [-j] | ttl [-j]>")
14401442
},
14411443
})
14421444

‎internal/cli/doctor.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,3 +341,48 @@ func runDoctorLibrettos(dryRun bool) error {
341341
map[bool]string{true: "; run without --dry-run", false: ""}[dryRun])
342342
return nil
343343
}
344+
345+
// runDoctorToolCalls closes tool invokes that no result ever answered.
346+
//
347+
// The door keeps this from happening going forward, so what this finds is
348+
// history written by a figaro older than the door. An aria carrying one is
349+
// refused by every provider on every send -- "tool_use ids were found without
350+
// tool_result blocks" -- and cannot be used again until it is closed.
351+
func runDoctorToolCalls(dryRun bool) error {
352+
if cli, err := sdk.DialAngelus(transport.UnixEndpoint(angelusSocketPath())); err == nil {
353+
cli.Close()
354+
return fmt.Errorf("angelus is running; stop it first (figaro stop)")
355+
}
356+
be, err := store.NewXwalBackend(ariaRoot(), 0)
357+
if err != nil {
358+
return err
359+
}
360+
defer be.Close()
361+
362+
arias, repaired := 0, 0
363+
for _, cv := range be.Conversations() {
364+
n, err := be.UnmatchedToolCalls(cv.ID)
365+
if err != nil || n == 0 {
366+
continue
367+
}
368+
arias++
369+
fmt.Printf("%-10s %d unanswered tool call(s)\n", cv.ID, n)
370+
if dryRun {
371+
continue
372+
}
373+
if fixed, err := be.RepairToolCalls(cv.ID); err != nil {
374+
fmt.Fprintf(os.Stderr, " %s: %v\n", cv.ID, err)
375+
} else {
376+
repaired += fixed
377+
}
378+
}
379+
switch {
380+
case arias == 0:
381+
fmt.Println("no aria carries an unanswered tool call")
382+
case dryRun:
383+
fmt.Printf("\n%d aria(s) would be repaired; run without --dry-run\n", arias)
384+
default:
385+
fmt.Printf("\nclosed %d call(s) across %d aria(s)\n", repaired, arias)
386+
}
387+
return nil
388+
}

‎internal/store/buried_orphan_test.go‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,19 +28,19 @@ func unmatchedInvokes(rows []Entry[message.Message]) []string {
2828
return open
2929
}
3030

31-
// AN ORPHANED INVOKE BURIED IN HISTORY MUST STILL BE CLOSABLE.
31+
// THE DOOR IS BOUNDED, AND REPAIRING HISTORY IS NOT ITS JOB.
3232
//
33-
// Gluck, 0.28.1: "messages.98: tool_use ids were found without tool_result
34-
// blocks immediately after", on an aria whose tail was past 440. The door
35-
// stops NEW orphans -- it completes a set the moment anything else lands --
36-
// so a buried one can only come from history written before the door
37-
// existed, which every aria upgraded from an older figaro has.
33+
// Gluck's rule: the fig IR is correct BY CONSTRUCTION -- every append closes
34+
// what it opened -- so nothing may scan the history to find out. The only
35+
// invokes that can be open belong to the last round, and that is all the door
36+
// looks at.
3837
//
39-
// outstandingInvokes scanned BACKWARD FOR THE LAST invoke-bearing message and
40-
// examined only that one. With a well-formed round after it, the buried
41-
// orphan was invisible to the door, to CloseOpenToolCalls, and therefore to
42-
// every repair figaro has -- and the aria could never send again.
43-
func TestABuriedOrphanedInvokeIsClosed(t *testing.T) {
38+
// A buried orphan therefore SURVIVES the door, deliberately. It can only come
39+
// from history written before the door existed (Gluck, 0.28.1: "messages.98:
40+
// tool_use ids were found without tool_result blocks", on an aria past 440),
41+
// and that is damage, repaired once by RepairToolCalls -- not a tax on every
42+
// append forever.
43+
func TestTheDoorDoesNotScanHistoryForOrphans(t *testing.T) {
4444
be, err := NewXwalBackend(t.TempDir(), 0)
4545
require.NoError(t, err)
4646
t.Cleanup(func() { be.Close() })
@@ -70,9 +70,16 @@ func TestABuriedOrphanedInvokeIsClosed(t *testing.T) {
7070
}
7171
require.Equal(t, []string{"toolu_buried"}, unmatchedInvokes(raw.Read()), "fixture")
7272

73+
// The door sees the last round, which is well formed, and closes nothing.
7374
n, err := be.CloseOpenToolCalls(aria)
7475
require.NoError(t, err)
75-
require.Equal(t, 1, n, "the repair reported closing nothing")
76+
require.Zero(t, n, "the door scanned past the current round")
77+
require.Equal(t, []string{"toolu_buried"}, unmatchedInvokes(raw.Read()))
78+
79+
// The explicit repair is what heals damage, and it reports what it did.
80+
fixed, err := be.RepairToolCalls(aria)
81+
require.NoError(t, err)
82+
require.Equal(t, 1, fixed, "the repair closed nothing")
7683
require.Empty(t, unmatchedInvokes(raw.Read()),
7784
"a buried orphan survived the repair, so this aria is refused by every "+
7885
"provider on every send, forever")

‎internal/store/ir_door.go‎

Lines changed: 98 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,14 @@ type figIRLog struct {
3434
loaded bool
3535
}
3636

37+
// roundLookback bounds the tail read that resolves outstanding invokes. THE
38+
// FIG IR IS CORRECT BY CONSTRUCTION: every append through this door closes
39+
// what it opened, so the only invokes that can be open when a door is created
40+
// belong to the LAST round -- an assistant message and the results after it.
41+
// A whole-history read to find them would be O(history) on a path every
42+
// append takes, to answer a question about the tail.
43+
const roundLookback = 64
44+
3745
const (
3846
// toolClosedNotice is the result text for an invoke the door had to close.
3947
toolClosedNotice = "tool call closed without a result (interrupt, fork, or fault)"
@@ -45,10 +53,7 @@ const (
4553
func (l *figIRLog) Append(e Entry[message.Message]) (Entry[message.Message], error) {
4654
l.mu.Lock()
4755
defer l.mu.Unlock()
48-
if !l.loaded {
49-
l.pending = outstandingInvokes(l.Log.Read())
50-
l.loaded = true
51-
}
56+
l.resolvePending()
5257

5358
results, others := splitToolResults(e.Payload)
5459
var late []message.Content
@@ -133,10 +138,7 @@ func GuardIR(inner Log[message.Message]) Log[message.Message] {
133138
func (l *figIRLog) CloseOpenToolCalls() (int, error) {
134139
l.mu.Lock()
135140
defer l.mu.Unlock()
136-
if !l.loaded {
137-
l.pending = outstandingInvokes(l.Log.Read())
138-
l.loaded = true
139-
}
141+
l.resolvePending()
140142
if len(l.pending) == 0 {
141143
return 0, nil
142144
}
@@ -179,28 +181,42 @@ func (l *figIRLog) isPending(id string) bool {
179181

180182
// outstandingInvokes reports the invokes of the last assistant message that
181183
// still lack results, which is the door's state after a restart or a fork.
182-
// outstandingInvokes is every invoke in the history with no result anywhere
183-
// after it. It reads the WHOLE history: it used to examine only the last
184-
// invoke-bearing message, so an orphan with a well-formed round after it was
185-
// invisible to every repair figaro has, and the aria was refused by every
186-
// provider on every send, forever (Gluck, 0.28.1: "messages.98: tool_use ids
187-
// were found without tool_result blocks"). The door prevents new ones, so
188-
// these come from history written before the door existed.
184+
// resolvePending reads what is open, ONCE, from the tail. Never at load: a
185+
// door that is created and never written costs nothing, and one that is
186+
// written resolves the current round rather than the history.
187+
func (l *figIRLog) resolvePending() {
188+
if l.loaded {
189+
return
190+
}
191+
l.pending = outstandingInvokes(TailSnapshot(l.Log, roundLookback))
192+
l.loaded = true
193+
}
194+
195+
// outstandingInvokes is the invokes of the last invoke-bearing message that
196+
// nothing after it answered. rows is a TAIL, not a history.
189197
func outstandingInvokes(rows []Entry[message.Message]) []message.Content {
198+
at := -1
199+
for i := len(rows) - 1; i >= 0; i-- {
200+
if len(assistantInvokes(rows[i].Payload)) > 0 {
201+
at = i
202+
break
203+
}
204+
}
205+
if at < 0 {
206+
return nil
207+
}
190208
answered := map[string]bool{}
191-
for _, e := range rows {
209+
for _, e := range rows[at+1:] {
192210
for _, c := range e.Payload.Content {
193211
if c.Type == message.ContentToolResult {
194212
answered[c.ToolCallID] = true
195213
}
196214
}
197215
}
198216
var open []message.Content
199-
for _, e := range rows {
200-
for _, inv := range assistantInvokes(e.Payload) {
201-
if !answered[inv.ToolCallID] {
202-
open = append(open, inv)
203-
}
217+
for _, inv := range assistantInvokes(rows[at].Payload) {
218+
if !answered[inv.ToolCallID] {
219+
open = append(open, inv)
204220
}
205221
}
206222
return open
@@ -239,3 +255,64 @@ func keepContent(in []message.Content, keep func(message.Content) bool) []messag
239255
}
240256
return out
241257
}
258+
259+
// UnmatchedToolCalls counts the invokes in an aria's history that no result
260+
// answered. Read-only; the audit half of RepairToolCalls.
261+
func (b *XwalBackend) UnmatchedToolCalls(ariaID string) (int, error) {
262+
b.mu.Lock()
263+
h, err := b.handleLocked(ariaID)
264+
b.mu.Unlock()
265+
if err != nil {
266+
return 0, err
267+
}
268+
return len(unansweredInvokes(h.ir.Read())), nil
269+
}
270+
271+
// unansweredInvokes is every invoke in rows with no result anywhere after it.
272+
func unansweredInvokes(rows []Entry[message.Message]) []message.Content {
273+
answered := map[string]bool{}
274+
for _, e := range rows {
275+
for _, c := range e.Payload.Content {
276+
if c.Type == message.ContentToolResult {
277+
answered[c.ToolCallID] = true
278+
}
279+
}
280+
}
281+
var open []message.Content
282+
for _, e := range rows {
283+
for _, inv := range assistantInvokes(e.Payload) {
284+
if !answered[inv.ToolCallID] {
285+
open = append(open, inv)
286+
}
287+
}
288+
}
289+
return open
290+
}
291+
292+
// RepairToolCalls closes every unanswered invoke in an aria's whole history
293+
// and reports how many it closed. It is the ONE place that reads the history
294+
// to find them: the door maintains the invariant going forward, so this
295+
// exists for arias whose records predate it. Run by `figaro doctor toolcalls`,
296+
// never on a read path.
297+
func (b *XwalBackend) RepairToolCalls(ariaID string) (int, error) {
298+
b.mu.Lock()
299+
h, err := b.handleLocked(ariaID)
300+
b.mu.Unlock()
301+
if err != nil {
302+
return 0, err
303+
}
304+
open := unansweredInvokes(h.ir.Read())
305+
if len(open) == 0 {
306+
return 0, nil
307+
}
308+
closing := message.Message{Role: message.RoleInput}
309+
for _, inv := range open {
310+
closing.Content = append(closing.Content,
311+
message.ToolResultContent(inv.ToolCallID, inv.ToolName, toolClosedNotice, true))
312+
}
313+
guard := &figIRLog{Log: h.ir, backend: b, ariaID: ariaID}
314+
if _, err := guard.write(Entry[message.Message]{Payload: closing}); err != nil {
315+
return 0, err
316+
}
317+
return len(open), nil
318+
}

0 commit comments

Comments
 (0)