Agent: a turn that changed nothing cannot say it did

Live, asked to delete a journal entry and later a seed lot, the model answered "Done — I've deleted it" both times having deleted nothing; each was still there a turn later. The prompt already forbade that. Now the run catches it: honestReply appends a correction when the reply claims a change ("Done", "I've deleted…") and no non-read-only tool call succeeded, and logs the steps so the mechanism can be read off the log next time. Alongside: the id-taking tools turn a bare "not found" into a message that names what was missing and which tool lists the ids ("nothing was changed"), the two delete descriptions say to look the id up in THIS turn, and the prompt says an error result means the thing did not happen. Co-Authored-By: Claude Fable 5 <[email protected]>
2026-08-23 02:37:06 -04:00
parent 07f33e62db
commit d0ca56b79b
4 changed files with 179 additions and 12 deletions
@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"regexp"
 	"strings"
 	"time"

@@ -168,6 +169,12 @@ func (r *Runner) Run(ctx context.Context, actorID, gardenID int64, message, toda
 	if result != nil {
 		turn.Reply = result.Output
 		turn.Steps = len(result.Steps)
+		if corrected := honestReply(turn.Reply, result); corrected != turn.Reply {
+			// The steps are logged so the mechanism can be read off the log
+			// next time — which tool it tried, what came back, what it said.
+			slog.Warn("agent: reply claimed a change no tool made", "run", runID, "garden", gardenID, "steps", describeSteps(result))
+			turn.Reply = corrected
+		}
 	}
 	if turn.Reply == "" {
 		turn.Reply = fallbackReply(turn)
@@ -175,6 +182,78 @@ func (r *Runner) Run(ctx context.Context, actorID, gardenID int64, message, toda
 	return turn, nil
 }

+// readOnlyTools are the tools whose success changes nothing — a turn made of
+// these alone has not done anything, whatever its reply says.
+var readOnlyTools = map[string]bool{
+	"list_gardens": true, "describe_garden": true, "list_years": true, "list_plantings": true,
+	"find_plant": true, "read_journal": true, "read_history": true, "list_seed_lots": true,
+	"list_shares": true,
+}
+
+// changeClaim matches a reply that reports a change as made: a "Done"/"Fixed"
+// opener, or a first-person past-tense claim ("I've deleted", "I moved"). A
+// question or an offer ("want me to delete it?", "I'll remove it") does not
+// match — only a claim of something already done.
+var changeClaim = regexp.MustCompile(`(?i)(?:^\s*(?:done|fixed|undone|deleted|removed|updated|changed|added|saved|moved|planted|filled|cleared|corrected|recorded|marked|shared|renamed|reverted)\b` +
+	`|\bI(?:'ve| have)? (?:just |now |also |already )?(?:deleted|removed|updated|changed|added|saved|moved|planted|filled|cleared|corrected|recorded|marked|shared|renamed|reverted|undone|set|put|pulled|replaced|swapped|rotated|rewrote|rewritten|edited|created|started|attached|restored|made)\b)`)
+
+// unbackedClaim is what the person reads under a claim no tool backs up.
+const unbackedClaim = "\n\n_Correction: nothing actually changed in this turn — no tool call that changes anything succeeded. Ask again and I'll do it properly._"
+
+// describeSteps summarizes a run for a log line: per step, the tools it
+// called (with ! on a failure) and the start of what the model said.
+func describeSteps(r *agent.Result) string {
+	parts := make([]string, 0, len(r.Steps))
+	for _, st := range r.Steps {
+		var b strings.Builder
+		fmt.Fprintf(&b, "%d:", st.Index)
+		for _, res := range st.Results {
+			b.WriteString(" " + res.Name)
+			if res.IsError {
+				b.WriteString("!")
+			}
+		}
+		if st.Response != nil {
+			if text := strings.Join(strings.Fields(st.Response.Text()), " "); text != "" {
+				if len(text) > 80 {
+					text = text[:80] + "…"
+				}
+				fmt.Fprintf(&b, " %q", text)
+			}
+		}
+		parts = append(parts, b.String())
+	}
+	return strings.Join(parts, " | ")
+}
+
+// acted reports whether any tool call in the run succeeded at something that
+// is not read-only.
+func acted(r *agent.Result) bool {
+	for _, st := range r.Steps {
+		for _, res := range st.Results {
+			if !res.IsError && !readOnlyTools[res.Name] {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// honestReply appends a correction to a reply that claims a change when no
+// tool call in the run made one. The prompt already forbids this, and the
+// live model did it anyway: asked to delete a journal entry it answered
+// "Done — I've deleted it" having deleted nothing, and the entry was found
+// still there a turn later. The person should hear that from the app, not
+// discover it. A reply that claims nothing, or a run in which some change
+// succeeded, passes through unchanged — this cannot tell a true claim from a
+// false one once anything at all was done, so it only speaks when nothing was.
+func honestReply(reply string, r *agent.Result) string {
+	if r == nil || acted(r) || !changeClaim.MatchString(reply) {
+		return reply
+	}
+	return reply + unbackedClaim
+}
+
 // isLoopLimit reports whether an error is one of majordomo's loop guards firing
 // rather than a genuine failure. Those runs have a partial result worth keeping.
 func isLoopLimit(err error) bool {
@@ -318,7 +397,10 @@ How to work:
 How to behave:
 - Only claim what a tool actually did. If a tool failed, or there is no tool for what was asked,
  say so plainly — never describe a change you did not make, and never say something is undone
-  unless undo_change did it.
+  unless undo_change did it. A tool result that is an error means the thing did not happen: say
+  it failed and why, and what you will try instead. Before you say you deleted, changed or added
+  something, there must be a successful tool result for it in THIS turn — an earlier turn does
+  not count, and neither does meaning to.
 - Every reply of yours that changed the garden has an "Undo this" button under it, and the
  History panel can revert any change; mention that when it helps.
 - When a request could mean materially different things — "swap the cucumbers and the melons"
@@ -591,3 +591,63 @@ func TestSystemPromptCarriesTheGardenersNotes(t *testing.T) {
 		t.Error("a garden without notes doesn't say so")
 	}
 }
+
+// TestAClaimedChangeNoToolMadeIsCorrected — the live model, asked to delete a
+// journal entry, answered "Done — I've deleted it" having called nothing, and
+// the entry was found still there a turn later. The prompt forbids that; the
+// run now catches it too: a reply that claims a change, in a turn where no
+// tool call changed anything, gets a correction the person can read.
+func TestAClaimedChangeNoToolMadeIsCorrected(t *testing.T) {
+	ctx := context.Background()
+	svc, owner := newAgentTestService(t)
+	g, err := svc.CreateGarden(ctx, owner, service.GardenInput{Name: "Plot", WidthCM: 2000, HeightCM: 2000})
+	if err != nil {
+		t.Fatalf("garden: %v", err)
+	}
+	entry, err := svc.CreateJournalEntry(ctx, owner, g.ID, service.JournalInput{Body: "Aphids on the cucumbers."})
+	if err != nil {
+		t.Fatalf("journal: %v", err)
+	}
+
+	run := func(steps ...fake.Step) string {
+		t.Helper()
+		turn, err := scriptedRunner(t, svc, steps...).Run(ctx, owner, g.ID, "delete that note", "", nil, nil)
+		if err != nil {
+			t.Fatalf("Run: %v", err)
+		}
+		return turn.Reply
+	}
+	corrected := func(reply string) bool { return strings.Contains(reply, "nothing actually changed") }
+
+	// No tool at all, a confident claim: corrected.
+	if r := run(fake.Reply("Done — I've deleted the journal entry about the aphids.")); !corrected(r) {
+		t.Errorf("a claim with no tool call passed uncorrected: %q", r)
+	}
+	// Only reads, then a claim: corrected.
+	if r := run(toolCall("read_journal", map[string]any{"gardenId": g.ID}), fake.Reply("I've deleted it.")); !corrected(r) {
+		t.Errorf("a claim over read-only calls passed uncorrected: %q", r)
+	}
+	// A tool that FAILED, then a claim: corrected — and the failure names the
+	// entry and where the ids come from, so a model that reads it has no
+	// excuse to guess again.
+	box := NewToolbox(svc, owner, "")
+	res := box.Execute(ctx, llm.ToolCall{ID: "1", Name: "delete_journal_entry", Arguments: mustJSON(t, map[string]any{"entryId": 999})})
+	if !res.IsError || !strings.Contains(res.Content, "read_journal") || !strings.Contains(res.Content, "nothing was changed") {
+		t.Errorf("deleting a missing entry = %q, want a refusal naming read_journal and saying nothing changed", res.Content)
+	}
+	if r := run(toolCall("delete_journal_entry", map[string]any{"entryId": 999}), fake.Reply("Done — it's gone.")); !corrected(r) {
+		t.Errorf("a claim over a failed call passed uncorrected: %q", r)
+	}
+	// No claim: nothing appended, whatever the tools did.
+	if r := run(fake.Reply("That note is still there — want me to delete it?")); corrected(r) {
+		t.Errorf("an offer was corrected as if it were a claim: %q", r)
+	}
+	// A real deletion: the claim stands.
+	r := run(toolCall("delete_journal_entry", map[string]any{"entryId": entry.ID}), fake.Reply("Done — I've deleted the journal entry."))
+	if corrected(r) {
+		t.Errorf("a true claim was corrected: %q", r)
+	}
+	if _, _, err := svc.ListJournal(ctx, owner, g.ID, service.JournalQuery{}); err != nil {
+		t.Fatalf("journal after: %v", err)
+	}
+}
@@ -157,7 +157,8 @@ func newToolbox(svc *service.Service, actorID int64, today string) (*llm.Toolbox
 			a.updateJournalEntry),
 		llm.DefineTool("delete_journal_entry",
 			"Delete a journal entry, by its id from read_journal. This is permanent — the journal is "+
-				"not in the undo history — so delete only the entry the user pointed at.",
+				"not in the undo history — so delete only the entry the user pointed at. Get the id "+
+				"from read_journal in this turn: ids from earlier turns are not in front of you.",
 			a.deleteJournalEntry),
 		llm.DefineTool("read_history",
 			"Read the garden's change history: every change anyone made — by hand in the editor, or "+
@@ -227,7 +228,8 @@ func newToolbox(svc *service.Service, actorID int64, today string) (*llm.Toolbox
 		llm.DefineTool("delete_seed_lot",
 			"Delete a seed lot the user recorded, by its id from list_seed_lots. Plantings attributed "+
 				"to it stay in the garden, just no longer linked to a purchase. Permanent — seed lots "+
-				"are not in the undo history — so delete only the lot the user pointed at.",
+				"are not in the undo history — so delete only the lot the user pointed at. Get the id "+
+				"from list_seed_lots in this turn: ids from earlier turns are not in front of you.",
 			a.deleteSeedLot),
 		llm.DefineTool("delete_plant",
 			"Delete a plant from the user's own catalog — a duplicate or a mistake. Refused while "+
@@ -468,8 +470,9 @@ func (a *adapter) movePlanting(ctx context.Context, args struct {
 	YCM        float64 `json:"yCm" description:"new center y in the destination object's local frame (cm)"`
 	ToObjectID *int64  `json:"toObjectId" description:"optional: another plantable object in the same garden to move it into; omit to move within its current object"`
 }) (any, error) {
-	return a.svc.MovePlanting(ctx, a.actor, args.PlantingID,
+	pl, err := a.svc.MovePlanting(ctx, a.actor, args.PlantingID,
 		service.MoveInput{ToObjectID: args.ToObjectID, XCM: args.XCM, YCM: args.YCM}, args.Version)
+	return pl, whenMissing(err, "planting", args.PlantingID, "list_plantings")
 }

 func (a *adapter) findPlant(ctx context.Context, args struct {
@@ -608,7 +611,19 @@ func (a *adapter) updatePlanting(ctx context.Context, args struct {
 	case args.SeedLotID != nil:
 		patch.SetSeedLotID, patch.SeedLotID = true, args.SeedLotID
 	}
-	return a.svc.UpdatePlanting(ctx, a.actor, args.PlantingID, patch, args.Version)
+	pl, err := a.svc.UpdatePlanting(ctx, a.actor, args.PlantingID, patch, args.Version)
+	return pl, whenMissing(err, "planting", args.PlantingID, "list_plantings")
+}
+
+// whenMissing turns the store's bare "not found" into a message that says what
+// was not found and where the ids come from, so the model re-reads instead of
+// guessing at another id — or, as the live one did, reporting success over
+// the error.
+func whenMissing(err error, what string, id int64, from string) error {
+	if errors.Is(err, domain.ErrNotFound) {
+		return fmt.Errorf("%w: no %s with id %d that you can act on — %s lists the ids; nothing was changed", domain.ErrNotFound, what, id, from)
+	}
+	return err
 }

 // parseDay checks a date the model typed, so a malformed one fails with a
@@ -660,15 +675,16 @@ func (a *adapter) updateJournalEntry(ctx context.Context, args struct {
 		}
 		observed = &on
 	}
-	return a.svc.UpdateJournalEntry(ctx, a.actor, args.EntryID,
+	e, err := a.svc.UpdateJournalEntry(ctx, a.actor, args.EntryID,
 		service.JournalPatch{Body: args.Body, ObservedAt: observed}, args.Version)
+	return e, whenMissing(err, "journal entry", args.EntryID, "read_journal")
 }

 func (a *adapter) deleteJournalEntry(ctx context.Context, args struct {
 	EntryID int64 `json:"entryId" description:"journal entry to delete (its id from read_journal)"`
 }) (any, error) {
 	if err := a.svc.DeleteJournalEntry(ctx, a.actor, args.EntryID); err != nil {
-		return nil, err
+		return nil, whenMissing(err, "journal entry", args.EntryID, "read_journal")
 	}
 	return map[string]any{"deleted": args.EntryID}, nil
 }
@@ -694,7 +710,7 @@ func (a *adapter) undoChange(ctx context.Context, args struct {
 	}
 	cs, conflicts, err := a.svc.RevertChangeSet(ctx, a.actor, args.ChangeSetID, domain.SourceAgent)
 	if err != nil {
-		return nil, err
+		return nil, whenMissing(err, "change", args.ChangeSetID, "read_history")
 	}
 	res := undoResult{UndoneID: args.ChangeSetID, Conflicts: conflicts}
 	if cs == nil {
@@ -806,7 +822,8 @@ func (a *adapter) removePlanting(ctx context.Context, args struct {
 	if err != nil {
 		return nil, err
 	}
-	return a.svc.RemovePlanting(ctx, a.actor, args.PlantingID, args.Version, on)
+	pl, err := a.svc.RemovePlanting(ctx, a.actor, args.PlantingID, args.Version, on)
+	return pl, whenMissing(err, "planting", args.PlantingID, "list_plantings")
 }

 func (a *adapter) listSeedLots(ctx context.Context, args struct {
@@ -934,14 +951,15 @@ func (a *adapter) updateSeedLot(ctx context.Context, args struct {
 		SetPackedForYear: args.PackedForYear != nil, PackedForYear: args.PackedForYear,
 		SetGerminationPct: args.GerminationPct != nil, GerminationPct: args.GerminationPct,
 	}
-	return a.svc.UpdateSeedLot(ctx, a.actor, args.LotID, patch, args.Version)
+	l, err := a.svc.UpdateSeedLot(ctx, a.actor, args.LotID, patch, args.Version)
+	return l, whenMissing(err, "seed lot", args.LotID, "list_seed_lots")
 }

 func (a *adapter) deleteSeedLot(ctx context.Context, args struct {
 	LotID int64 `json:"lotId" description:"seed lot to delete (its id from list_seed_lots)"`
 }) (any, error) {
 	if err := a.svc.DeleteSeedLot(ctx, a.actor, args.LotID); err != nil {
-		return nil, err
+		return nil, whenMissing(err, "seed lot", args.LotID, "list_seed_lots")
 	}
 	return map[string]any{"deleted": args.LotID}, nil
 }
@@ -965,7 +983,7 @@ func (a *adapter) deletePlanting(ctx context.Context, args struct {
 	PlantingID int64 `json:"plantingId" description:"plop to delete outright (its id from describe_garden or list_plantings)"`
 }) (any, error) {
 	if err := a.svc.DeletePlanting(ctx, a.actor, args.PlantingID); err != nil {
-		return nil, err
+		return nil, whenMissing(err, "planting", args.PlantingID, "list_plantings")
 	}
 	return map[string]any{"deleted": args.PlantingID}, nil
 }