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) + } +}