From 21b4775d16d51bcdbe640a9f97429f5e9341be3d Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Wed, 5 Aug 2026 20:17:56 -0400 Subject: [PATCH] =?UTF-8?q?fix(agent):=20gadfly=20round=201=20=E2=80=94=20?= =?UTF-8?q?user-boundary=20scan,=20back-ref=20precedence,=20regex=20legibi?= =?UTF-8?q?lity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two behavioral fixes from the review: - modeSummary's backward scan now stops at the most recent user message. With the dwarf ratio rejecting the current turn's 1x-3x answer, the old unbounded scan could walk into WithHistory content and resurrect a stale answer to a DIFFERENT question — strictly worse than keeping the closer (opus, correctness). Other modes keep their historical unbounded scan. - A terminal matching BOTH the ack shape and a back-reference is now classified back-ref: it carries no answer content, so the looser bar is the right one (opus, error-handling). Plus the nits: summaryCloserRe assembled from named fragments, the leading marker class gains '+' (parity with citationLabelRe), verb-first form takes 'all the', dwarf ratio hoisted into one named local, and the 151-vs-153 char/byte comment inaccuracy corrected. Co-Authored-By: Claude Fable 5 --- agent/finalize.go | 66 +++++++++++++++++++++++++++++++----------- agent/finalize_test.go | 60 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 107 insertions(+), 19 deletions(-) diff --git a/agent/finalize.go b/agent/finalize.go index c311b4a..88c20ac 100644 --- a/agent/finalize.go +++ b/agent/finalize.go @@ -28,15 +28,18 @@ import ( // - a bookkeeping closer ("Citations are logged. Short version: …"): the // model acknowledged the citation round and compressed the answer it had // already written into a one-liner (mort run b3cb9ee9 — a 2,089-char answer -// shrank to 151 chars at delivery). The compression is strictly poorer than -// the front-loaded answer, so recover the prior turn and DISCARD the -// closer — but only when the prior turn clearly dwarfs it, because unlike a -// back-reference this closer DOES carry answer content (see modeSummary). +// shrank to a 153-byte closer at delivery). The compression is strictly +// poorer than the front-loaded answer, so recover the prior turn and +// DISCARD the closer — but only when the prior turn clearly dwarfs it, +// because unlike a back-reference this closer DOES carry answer content +// (see modeSummary). // // A citations addendum is tested first and wins over the other two (a short -// terminal can match more than one shape), so its links are never discarded; -// the summary closer is tested before the plain weak-final test so the -// stricter recovery bar applies when both match. When the terminal text stands +// terminal can match more than one shape), so its links are never discarded. +// The back-reference test wins over the summary-closer test: a terminal +// matching both ("Citations are logged. As I said above…") carries no answer +// content of its own, so the looser back-ref recovery bar — not the summary +// closer's dwarf ratio — is the right one. When the terminal text stands // on its own it is returned unchanged; when it is degenerate but nothing // better can be recovered, it is returned as-is (a compressed answer still // beats nothing). @@ -48,9 +51,11 @@ func finalOutput(msgs []llm.Message, terminal string) string { switch { case isCitationsOnly(terminal): mode = modeCitations + case isWeakFinal(terminal): + // modeBackRef case isSummaryCloser(terminal): mode = modeSummary - case !isWeakFinal(terminal): + default: return terminal } rec, ok := lastSubstantiveAssistantText(msgs, terminal, mode) @@ -95,8 +100,11 @@ const ( // modeSummary: the terminal acknowledges the citation round and may carry // a short compression of the front-loaded answer. Unlike a back-reference // it DOES contain answer content, so it is only replaced when a prior turn - // clearly dwarfs it — the ratio is mandatory at every length, and the - // closer is discarded (its content is a strict subset of what it replaced). + // clearly dwarfs it — the ratio is mandatory at every length, the recovery + // scan stops at the most recent user message (a compression can only be of + // THIS turn's answer; never resurrect one from an earlier question), and + // the closer is discarded (its content is a strict subset of what it + // replaced). modeSummary ) @@ -117,8 +125,20 @@ var backRefRe = regexp.MustCompile(`(?i)(already answered|see above|as (i )?(sai // ("Short version: no.") is deliberately out of scope — a user who asked for // brevity would be answered with exactly that shape, and misclassifying it // would hijack a legitimate answer; an unmatched closer merely keeps today's -// behavior (fail closed). -var summaryCloserRe = regexp.MustCompile(`(?i)^[\s>#*_-]*((done|all set|ok(ay)?)[\s,.!:—-]+)?(((all|the)\s+)?(citations?|sources?|references?|claims?)\s+((are|were|have\s+been|all)\s+)*(logged|recorded|cited|saved|noted|captured|filed)|logged\s+((all|the)\s+)*(citations?|sources?|references?))[.!]`) +// behavior (fail closed). Assembled from named fragments so the alternations +// stay legible and extendable. +const ( + summaryLead = `[\s>#*_+-]*` // leading markdown/list markers, as in citationLabelRe + summaryPreface = `((done|all set|ok(ay)?)[\s,.!:—-]+)?` // optional "Done —" style opener + summaryNouns = `(citations?|sources?|references?|claims?)` + summaryCopulas = `((are|were|have\s+been|all)\s+)*` + summaryVerbs = `(logged|recorded|cited|saved|noted|captured|filed)` + summaryArticle = `((all|the)\s+)*` // star, not ?: "Logged all the citations." +) + +var summaryCloserRe = regexp.MustCompile(`(?i)^` + summaryLead + summaryPreface + + `(` + summaryArticle + summaryNouns + `\s+` + summaryCopulas + summaryVerbs + + `|logged\s+` + summaryArticle + summaryNouns + `)[.!]`) // preambleRe matches intent-announcing prefixes ("Let me search...", "I'll // check...") so a preamble is never mistaken for the answer during recovery. @@ -165,9 +185,10 @@ const ( // "Source:" and cites a URL mid-sentence is not mistaken for a bare list. citationDominatedDivisor = 3 // summaryCloserMaxChars bounds a summary closer: room for the ack sentence - // plus a couple of compression sentences (the b3cb9ee9 closer was 151). - // Beyond this the "short version" is substantial enough that replacing it - // risks losing content the front-loaded turn never had. + // plus a couple of compression sentences (the b3cb9ee9 closer was 153 + // bytes — Go len(), which is what every threshold here compares). Beyond + // this the "short version" is substantial enough that replacing it risks + // losing content the front-loaded turn never had. summaryCloserMaxChars = 300 ) @@ -230,6 +251,16 @@ func lastSubstantiveAssistantText(msgs []llm.Message, terminal string, mode reco tt := strings.TrimSpace(terminal) for i := len(msgs) - 1; i >= 0; i-- { m := msgs[i] + if mode == modeSummary && m.Role == llm.RoleUser { + // A summary closer compresses THIS turn's front-loaded answer, so + // the scan must not cross into an earlier question: once the dwarf + // ratio has rejected the current turn's text, walking further back + // would resurrect a stale answer to a DIFFERENT question — strictly + // worse than keeping the closer. (A mid-run steer message is also a + // user-role boundary; recovery then fails closed, which is fine.) + // The other modes keep their historical unbounded scan. + break + } if m.Role != llm.RoleAssistant { continue } @@ -261,7 +292,8 @@ func lastSubstantiveAssistantText(msgs []llm.Message, terminal string, mode reco // addendum is not a rival answer, so its length is irrelevant; a summary // closer already proved the ratio above). func isSubstantiveAnswer(txt, terminal string, mode recoveryMode) bool { - if mode == modeSummary && len(txt) < recoverRatio*len(terminal) { + dwarfs := len(txt) >= recoverRatio*len(terminal) + if mode == modeSummary && !dwarfs { return false } if len(txt) >= recoverMinChars { @@ -270,5 +302,5 @@ func isSubstantiveAnswer(txt, terminal string, mode recoveryMode) bool { if len(txt) < recoverFloorChars || preambleRe.MatchString(txt) { return false } - return mode != modeBackRef || len(txt) >= recoverRatio*len(terminal) + return mode != modeBackRef || dwarfs } diff --git a/agent/finalize_test.go b/agent/finalize_test.go index cef84ef..89703fb 100644 --- a/agent/finalize_test.go +++ b/agent/finalize_test.go @@ -73,8 +73,9 @@ func TestIsCitationsOnly(t *testing.T) { } // b3cb9ee9Closer is the verbatim terminal turn from mort run b3cb9ee9: a -// 2,089-char answer was front-loaded into the cite-call turn and this 151-char -// compression was all that got delivered. +// 2,089-char answer was front-loaded into the cite-call turn and this 153-byte +// compression (151 runes — the em dash is 3 bytes, and byte length is what the +// thresholds compare) was all that got delivered. const b3cb9ee9Closer = "Citations are logged. Short version: the bulk of that ~$64M was AIPAC and dark-money super PACs, not the party committees — and it still wasn't enough." func TestIsSummaryCloser(t *testing.T) { @@ -91,6 +92,8 @@ func TestIsSummaryCloser(t *testing.T) { {"done-prefix", "Done — citations logged.", true}, {"ack-then-tldr", "Sources have been recorded! TL;DR: the GPU was the bottleneck.", true}, {"references-noted", "References noted. In short: yes, it ships Tuesday.", true}, + {"plus-list-marker", "+ Citations are logged.", true}, + {"logged-all-the", "Logged all the citations.", true}, {"empty", "", false}, {"ack-continues-midsentence", "The citations are recorded in the court transcript, which shows the filing dates.", false}, @@ -141,6 +144,9 @@ func TestFinalOutput(t *testing.T) { // A >=200-byte real answer that merely OPENS with a conversational word // ("Sure,"). The preamble filter must NOT veto it (gadfly regression guard). longConversationalAnswer := "Sure, here's the rundown: it currently sells for about $2,700 used on eBay, typically $2,400 to $2,900 depending on condition and bundle, with the sealed Founders Edition commanding the top of that range while used AIB cards go a bit lower." + // Matches BOTH the summary ack and backRefRe, within the 120-byte weak cap, + // and long enough (>~92 bytes) that longAnswer would fail the summary bar. + bothMatchCloser := "Citations are logged. As I mentioned above, the full detail on the money sources is in my earlier message." tests := []struct { name string @@ -347,6 +353,56 @@ func TestFinalOutput(t *testing.T) { terminal: b3cb9ee9Closer, want: b3cb9ee9Closer, }, + { + // Gadfly (opus, correctness): the modeSummary scan must stop at the + // most recent user message. Here the current turn's answer sits in + // the 1x-3x band (rejected by the ratio) while a dwarfing answer to + // a DIFFERENT question sits in history — resurrecting it would be + // strictly worse than keeping the closer. + name: "summary closer never resurrects a stale answer across the user boundary", + msgs: []llm.Message{ + llm.UserText("earlier, unrelated question?"), + asst(hugeAnswer), + llm.UserText("q?"), + asst(conciseAnswer, cite...), + llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}), + asst(b3cb9ee9Closer), + }, + terminal: b3cb9ee9Closer, + want: b3cb9ee9Closer, + }, + { + // The boundary must not break the legitimate multi-turn case: the + // dwarfing front-loaded answer in THIS turn's window is recovered + // even with history behind it. + name: "summary closer recovery still works with history present", + msgs: []llm.Message{ + llm.UserText("earlier, unrelated question?"), + asst(longAnswer), + llm.UserText("q?"), + asst(hugeAnswer, cite...), + llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}), + asst(b3cb9ee9Closer), + }, + terminal: b3cb9ee9Closer, + want: hugeAnswer, + }, + { + // Gadfly (opus, error-handling): a closer matching BOTH the ack + // shape and a back-reference carries no answer content, so the + // back-ref test must win and the ordinary recovery bar apply — + // under the summary bar this ~106-byte terminal would demand a + // ~318-byte prior and wrongly keep the closer over longAnswer. + name: "back-reference wins over the summary ack when both match", + msgs: []llm.Message{ + llm.UserText("q?"), + asst(longAnswer, cite...), + llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}), + asst(bothMatchCloser), + }, + terminal: bothMatchCloser, + want: longAnswer, + }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) {