From bcba9667bdfb85daff575daeca25267a028c6bf6 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Fri, 21 Aug 2026 23:20:11 -0400 Subject: [PATCH] fix(agent): recover the answer behind a bare "above" pointer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mort issue #1611: run 8eea3e82 front-loaded a 2,245-char analysis into its cite-call turn and closed with 220 bytes — "Done — that's the full chain above. Short version: …". The user got the 220 bytes and a pointer at a chain that was never posted. finalOutput already had three shapes for this pathology, and the closer matched none of them: too long for the weak-final cap (220 > 120), no citations heading, and no "Citations are logged." ack to open the summary-closer class. So it was delivered verbatim. The three shapes were each a separate vocabulary of ack phrases, which is why a fourth phrasing walked straight through. Two of them are really one signal — the terminal DEFERS, telling us the answer is somewhere the user cannot see — differing only in whether the terminal also carries content of its own. That signal is now isBackRef, shared by both, so a new phrasing is added once and covered in the bare and the "+ compression" variant at the same time. Its open-ended half is aboveRefRe: a DEICTIC "above", separated from the preposition by what follows the word. The deictic use ends its clause ("that's the full chain above.", "as shown above,"); the preposition always continues into a noun phrase ("above 100°C", "above the fold", "above all, …"). pointsAbove additionally requires the reference in the terminal's first 120 bytes — with almost no text before it in THIS message, it cannot be pointing at the message's own content. isSummaryCloser now opens on either the citations ack or pointsAbove. Recovery is unchanged: the mandatory dwarf ratio and the user-message scan boundary still gate it, so a closer only loses to a prior turn in the same user turn that is clearly the fuller original. Break-checked: reverting either classifier, dropping the clause-final rule, or dropping the offset bound each kills a named test; the unmutated control survives. --- agent/finalize.go | 92 +++++++++++++++++++++------ agent/finalize_test.go | 140 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 213 insertions(+), 19 deletions(-) diff --git a/agent/finalize.go b/agent/finalize.go index 72b9e77..1ffc9a8 100644 --- a/agent/finalize.go +++ b/agent/finalize.go @@ -25,14 +25,21 @@ import ( // glm-5.2 "cite" pattern behind mort issue #1418). The citations are real, // useful content — unlike a back-reference — so recover the prior answer and // KEEP the citations, appended below it. -// - 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 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 deferring closer ("Citations are logged. Short version: …", "Done — +// that's the full chain above. Short version: …"): the model points at the +// answer it had already written and compresses it into a one-liner (mort +// run b3cb9ee9 — a 2,089-char answer shrank to a 153-byte closer; mort +// issue #1611 — a 2,245-char analysis shrank to 220 bytes). 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 bare back-reference this closer DOES +// carry answer content (see modeSummary). +// +// The last two shapes share one signal — the terminal DEFERS: it tells us the +// answer is somewhere the user cannot see (isBackRef, or the bookkeeping ack). +// They differ only in whether the terminal also carries content of its own, so +// a new deferral phrase added to that shared signal is covered in both the +// bare and the "+ compression" variant at once. // // 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. @@ -110,9 +117,39 @@ const ( // backRefRe matches a terminal turn that merely points back to an earlier // message instead of stating the answer ("(Already answered above.)", -// "see above", "as I said", ...). +// "see above", "as I said", ...). It is a list of fixed phrasings; aboveRefRe +// covers the open-ended half of the same family. var backRefRe = regexp.MustCompile(`(?i)(already answered|see above|as (i )?(said|mentioned|stated|noted)|answered (that )?above|per my (previous|earlier))`) +// aboveRefRe matches a DEICTIC "above" — one pointing at earlier text rather +// than serving as a preposition. What follows the word separates the two uses: +// the deictic use ends its clause ("that's the full chain above.", "as shown +// above,", a line that simply ends in "above"), while the preposition always +// continues into a noun phrase ("above 100°C", "above the fold", "above all, +// the ..."). Only the clause-final form matches, so the open-ended half of the +// back-reference family is covered without enumerating every phrasing a model +// might invent — backRefRe's fixed list kept missing new ones (mort issue +// #1611: "Done — that's the full chain above."). +var aboveRefRe = regexp.MustCompile(`(?i)\babove\b[ \t]*([.,;:!?)\]"'’”—-]|\n|$)`) + +// pointsAbove reports whether a deictic "above" appears in the terminal's +// OPENING. The offset bound is what makes a bare "above" safe to key on: with +// almost no text before it in THIS message, the reference cannot be pointing +// at the message's own content, so it must point at a turn the user never saw +// (the harness delivers only the final turn). +func pointsAbove(t string) bool { + loc := aboveRefRe.FindStringIndex(t) + return loc != nil && loc[0] <= backRefHeadChars +} + +// isBackRef reports whether a terminal turn defers to earlier content instead +// of stating the answer — the signal shared by the back-reference and +// summary-closer shapes (see finalOutput). Extend the class here, once, rather +// than in either caller. +func isBackRef(t string) bool { + return backRefRe.MatchString(t) || pointsAbove(t) +} + // summaryCloserRe matches a terminal turn that OPENS with a bookkeeping // acknowledgment of the citation round — "Citations are logged.", "Sources // cited.", "Logged the citations." — the shape a model produces when it @@ -180,6 +217,13 @@ const ( // genuine final answer that merely contains "as I said" mid-sentence is // longer than this, so it is never treated as weak. weakFinalMaxChars = 120 + // backRefHeadChars bounds how far into the terminal a deictic "above" may + // sit and still read as pointing OUTSIDE this message (see pointsAbove). + // Same guard as weakFinalMaxChars — "there is not enough text before the + // reference for it to be pointing at content inside this turn" — but + // expressed as an offset, because a summary closer carries a compression + // AFTER the pointer and so is not itself short. + backRefHeadChars = 120 // recoverMinChars: a prior assistant turn this long is treated as a real // answer regardless of how it opens (the preamble filter is not applied at // this length — see isSubstantiveAnswer). @@ -203,13 +247,16 @@ const ( ) // isWeakFinal reports whether a terminal turn's text fails to stand on its own -// as the answer: empty/whitespace, or a short pure back-reference. +// as the answer: empty/whitespace, or a short pure back-reference. The length +// cap is what keeps a genuine answer that merely contains a back-reference +// phrase mid-sentence out of the class; past the cap a deferring terminal is +// isSummaryCloser's business, under the stricter dwarf bar. func isWeakFinal(s string) bool { t := strings.TrimSpace(s) if t == "" { return true } - return len(t) <= weakFinalMaxChars && backRefRe.MatchString(t) + return len(t) <= weakFinalMaxChars && isBackRef(t) } // isCitationsOnly reports whether a terminal turn is essentially just a @@ -238,19 +285,26 @@ func isCitationsOnly(s string) bool { return len(residue) <= len(t)/citationDominatedDivisor } -// isSummaryCloser reports whether a terminal turn is a bookkeeping closer: it -// opens with a complete "citations are logged"-style ack sentence (see -// summaryCloserRe) and is short enough that whatever follows the ack can only -// be a compression of an earlier, fuller answer. Whether that fuller answer -// actually exists is modeSummary's job — the dwarf ratio in -// isSubstantiveAnswer keeps a matching closer in place when nothing earlier -// clearly outweighs it. +// isSummaryCloser reports whether a terminal turn defers to an earlier answer +// and is short enough that whatever follows the deferral can only be a +// compression of it. Two openers qualify: a complete "citations are logged" +// -style ack sentence (summaryCloserRe), and a deictic back-reference in the +// terminal's opening (pointsAbove — mort issue #1611's "Done — that's the full +// chain above. Short version: …", which the ack shape alone did not cover). +// Whether the fuller answer actually exists is modeSummary's job — the dwarf +// ratio in isSubstantiveAnswer keeps a matching closer in place when nothing +// earlier clearly outweighs it. +// +// Only pointsAbove is used here, not the whole isBackRef class: backRefRe's +// fixed phrases can appear anywhere in the text, and a 300-byte closer has +// room for a real answer that merely mentions "as I said" mid-sentence. +// pointsAbove is offset-bounded, so it stays anchored to the opening. func isSummaryCloser(s string) bool { t := strings.TrimSpace(s) if t == "" || len(t) > summaryCloserMaxChars { return false } - return summaryCloserRe.MatchString(t) + return summaryCloserRe.MatchString(t) || pointsAbove(t) } // lastSubstantiveAssistantText scans msgs newest→oldest (skipping the terminal diff --git a/agent/finalize_test.go b/agent/finalize_test.go index 61e6586..d2bb61a 100644 --- a/agent/finalize_test.go +++ b/agent/finalize_test.go @@ -26,6 +26,12 @@ func TestIsWeakFinal(t *testing.T) { {"crisp-yes", "Yes.", false}, {"crisp-status", "It's down, restarting now.", false}, {"long-with-as-i-said", long, false}, // >120 chars: not weak despite the phrase + + // The deictic half of the class (aboveRefRe), inside the length cap. + {"bare-above-pointer", "That's the full chain above.", true}, + {"above-pointer-in-parens", "(the breakdown is above)", true}, + {"prepositional-above-not-weak", "Anything above 100 degrees boils off.", false}, + {"1611-closer-too-long-for-weak", closer1611, false}, // 220 bytes: isSummaryCloser's job } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -36,6 +42,53 @@ func TestIsWeakFinal(t *testing.T) { } } +// closer1611 is the verbatim terminal turn from mort run +// 8eea3e82-b61b-4174-9750-9aa2f4bde4d4 (issue #1611): the model front-loaded a +// 2,245-char analysis into the cite-call turn and closed with this 220-byte +// pointer-plus-compression. Too long for the weak-final cap and carrying no +// citations ack, it matched none of the three original shapes and was +// delivered verbatim — the user saw a summary referring to a "chain above" +// that had never been posted. +const closer1611 = "Done — that's the full chain above. Short version: it's not one incident, it's the confluence of the Iran war, the Epstein files, and three ex-allies now openly plotting a third-party movement that finally set him off." + +func TestPointsAbove(t *testing.T) { + cases := []struct { + name string + in string + want bool + }{ + // Deictic: "above" ends its clause, so it points at earlier text. + {"1611-verbatim", closer1611, true}, + {"sentence-final", "That's the full chain above.", true}, + {"comma", "As shown above, the answer is 60 minutes.", true}, + {"end-of-string", "The full breakdown is above", true}, + {"line-final", "Everything is above\n\nShort version: yes.", true}, + {"closing-paren", "(the detail is above).", true}, + {"semicolon", "It's above; the short answer is no.", true}, + + // Prepositional: "above" continues into a noun phrase. + {"above-a-number", "Anything above 100 degrees boils off.", false}, + {"above-the-fold", "The banner sits above the fold on every page.", false}, + {"above-all", "Above all, keep the deploy green.", false}, + {"above-average", "Turnout was above average in three counties.", false}, + + {"no-above-at-all", "42", false}, + {"empty", "", false}, + + // Offset bound: past backRefHeadChars there IS enough text before the + // reference for it to be pointing inside this same message. + {"late-reference-not-a-pointer", strings.Repeat("x", backRefHeadChars+1) + " as shown above.", false}, + {"reference-at-the-bound", strings.Repeat("x", backRefHeadChars-6) + " above.", true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := pointsAbove(c.in); got != c.want { + t.Errorf("pointsAbove(%q) = %v, want %v", c.in, got, c.want) + } + }) + } +} + func TestIsCitationsOnly(t *testing.T) { cases := []struct { name string @@ -102,6 +155,13 @@ func TestIsSummaryCloser(t *testing.T) { {"mentions-citations-midsentence", "The paper's citations are what got it retracted.", false}, {"crisp-number", "42", false}, {"over-cap", "Citations are logged. " + strings.Repeat("The long version has many more details worth keeping. ", 6), false}, // >300: too substantial to replace + + // A deictic back-reference opener also qualifies — the #1611 shape, + // which carries no citations ack at all. + {"1611-verbatim", closer1611, true}, + {"above-pointer-plus-tldr", "That's the whole picture above. In short: the merger fell through.", true}, + {"prepositional-above-is-not-a-closer", "Anything above 100 degrees boils off, which is why the sample evaporated.", false}, + {"1611-over-cap", closer1611 + " " + strings.Repeat("Plenty more detail worth keeping here. ", 4), false}, // >300 } for _, c := range cases { t.Run(c.name, func(t *testing.T) { @@ -147,6 +207,11 @@ func TestFinalOutput(t *testing.T) { // 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." + // The #1611 pair: a front-loaded analysis that dwarfs its 220-byte closer. + analysis1611 := strings.TrimSpace(strings.Repeat("The break was the Iran strikes, then the Epstein files. ", 12)) // >3x closer1611 + // A terminal using "above" as a PREPOSITION — not a back-reference, so it + // must survive verbatim next to a dwarfing prior turn. + prepositionalTerminal := "Anything above 100 degrees boils off, which is exactly why the sample evaporated overnight in the unsealed tray." tests := []struct { name string @@ -387,6 +452,50 @@ func TestFinalOutput(t *testing.T) { terminal: b3cb9ee9Closer, want: hugeAnswer, }, + { + // mort issue #1611: a pointer-plus-compression closer with no + // citations ack. The 2,245-char analysis was front-loaded into the + // cite turn; the closer pointed at a "chain above" the user never + // saw. Recover the analysis and discard the closer. + name: "above-pointer closer discarded when the front-loaded answer dwarfs it", + msgs: []llm.Message{ + llm.UserText("what set off the Truth Social rant?"), + asst(analysis1611, cite...), + llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}), + asst(closer1611), + }, + terminal: closer1611, + want: analysis1611, + }, + { + // The dwarf ratio governs the new opener too: a prior turn that is + // longer but not clearly the fuller original (here ~275 bytes vs a + // 220-byte closer, under 3x) must not displace a closer that + // carries real answer content. + name: "above-pointer closer kept when the prior turn does not dwarf it", + msgs: []llm.Message{ + llm.UserText("q?"), + asst(longAnswer, cite...), + llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}), + asst(closer1611), + }, + terminal: closer1611, + want: closer1611, + }, + { + // A prepositional "above" is not a back-reference: this terminal + // stands on its own and must be returned verbatim even though a + // much longer prior turn exists. + name: "prepositional above is not hijacked by a longer prior turn", + msgs: []llm.Message{ + llm.UserText("why did the sample evaporate?"), + asst(hugeAnswer, cite...), + llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}), + asst(prepositionalTerminal), + }, + terminal: prepositionalTerminal, + want: prepositionalTerminal, + }, { // A closer matching BOTH the ack shape and a back-reference // carries no answer content, so the back-ref test must win and the @@ -536,3 +645,34 @@ func TestRun_RecoversFrontLoadedAnswerWithCitations(t *testing.T) { t.Errorf("model calls = %d, want 2 (no extra nudge turn)", n) } } + +// TestRun_RecoversFrontLoadedAnswerOverAboveRefCloser reproduces mort issue +// #1611 end-to-end: the model front-loads its analysis into the cite-call turn +// and closes with a pointer at that invisible text plus a one-line +// compression. The delivered output must be the front-loaded analysis, with no +// extra model call. +func TestRun_RecoversFrontLoadedAnswerOverAboveRefCloser(t *testing.T) { + analysis := strings.TrimSpace(strings.Repeat("The break was the Iran strikes, then the Epstein files. ", 12)) + fp := fake.New("fp") + fp.Enqueue("test-model", + fake.ReplyWith(llm.Response{ + Parts: []llm.Part{llm.Text(analysis)}, + ToolCalls: []llm.ToolCall{{ID: "c1", Name: "cite", Arguments: json.RawMessage(`{}`)}}, + FinishReason: llm.FinishToolCalls, + Usage: llm.Usage{InputTokens: 10, OutputTokens: 5}, + }), + fake.Reply(closer1611), + ) + + a := New(newModel(t, fp), "sys", WithToolbox(citeToolbox(t))) + res, err := a.Run(context.Background(), "what set off the Truth Social rant?") + if err != nil { + t.Fatalf("Run: %v", err) + } + if res.Output != analysis { + t.Errorf("Output = %q, want recovered front-loaded analysis %q", res.Output, analysis) + } + if n := len(fp.Calls()); n != 2 { + t.Errorf("model calls = %d, want 2 (no extra nudge turn)", n) + } +}