diff --git a/agent/finalize.go b/agent/finalize.go index 72b9e77..d06cdc3 100644 --- a/agent/finalize.go +++ b/agent/finalize.go @@ -3,6 +3,7 @@ package agent import ( "regexp" "strings" + "unicode/utf8" "gitea.stevedudenhoeffer.com/steve/majordomo/llm" ) @@ -25,14 +26,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 (pointsAbove, or the bookkeeping +// ack). They differ only in whether the terminal also carries content of its +// own, so a new deferral phrasing added to pointsAbove 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 +118,142 @@ 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."). +// +// The terminator set deliberately excludes the ASCII hyphen. `\b` holds +// between "above" and "-", so a literal '-' in the class made every +// hyphenated compound — "above-average", "above-board", "above-ground" — +// read as a clause-final deictic and put a legitimate short answer at risk of +// being discarded. The em dash stays: a model writes "…above — see the +// links", never "above-" as a separator. CR is listed alongside LF so a +// CRLF transcript does not quietly lose every line-final "above". +var aboveRefRe = regexp.MustCompile(`(?i)\babove\b[ \t]*([.,;:!?)\]"'’”—]|\r|\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). +// +// This is the DEFERRAL SIGNAL SHARED by both recovery shapes — isWeakFinal and +// isSummaryCloser both call it, so a widening here reaches the bare closer and +// the "+ compression" closer at once. Its sibling backRefRe is deliberately +// NOT shared: those fixed phrases are matched anywhere in the text, which is +// only safe under isWeakFinal's 120-byte cap. Folding the two into one +// predicate for tidiness would hand isSummaryCloser an unanchored match across +// 300 bytes — widening the gate, not deduplicating it. +func pointsAbove(t string) bool { return aboveRefLoc(t) != nil } + +// aboveRefLoc returns the span of the qualifying deictic reference, or nil. +// One place performs the match and the offset test, so pointsAbove and +// bareAbovePointer cannot disagree about what counts as a pointer. +func aboveRefLoc(t string) []int { + loc := aboveRefRe.FindStringIndex(t) + if loc == nil || loc[0] > abovePointerHeadChars { + return nil + } + return loc +} + +// bareAbovePointer reports whether the terminal is a deictic back-reference and +// essentially NOTHING ELSE — the form isWeakFinal may discard outright. +// +// pointsAbove alone is not enough for that, because a pointer can share a short +// terminal with the answer: "That's the chain above. Ship Tuesday." is 37 bytes +// and the decision is the four words the pointer is not. Discarding it in +// favour of an earlier turn throws away the only thing the user needed. Same +// shape as the summary-closer fix — a pointer says where the detail is, it does +// not say the text beside it is disposable. +// +// So the reference's own clause is cut out and whatever remains must be filler: +// nothing, punctuation, or a throat-clearing "Done —". Cutting the CLAUSE (from +// the end of the previous sentence through the reference) rather than testing +// position is also what makes a mixed terminal safe — "Anything above 100 +// boils. See the note above." keeps its first sentence and is correctly not +// bare, and a comparative "above," with an interjection after it keeps the rest +// of its own sentence for the same reason. +func bareAbovePointer(t string) bool { + loc := aboveRefLoc(t) + if loc == nil { + return false + } + start := 0 + if k := strings.LastIndexAny(t[:loc[0]], clauseBoundaryChars); k >= 0 { + // k is the BYTE index of the boundary rune's first byte, and one of + // those runes is a 3-byte em dash — k+1 would slice into the middle of + // it and leave a stray continuation byte in the remainder, which then + // never trims away and makes a genuinely bare pointer look occupied. + _, w := utf8.DecodeRuneInString(t[k:]) + start = k + w + } + rest := strings.TrimSpace(t[:start]) + " " + strings.TrimSpace(t[loc[1]:]) + return bareRemainderRe.MatchString(strings.Trim(rest, pointerResidueCutset)) +} + +// clauseBoundaryChars ends the clause the reference belongs to. Commas, +// semicolons, colons and dashes are in it, not just sentence terminators, +// because the answer can share the reference's SENTENCE — "Ship Tuesday, as +// shown above." is a decision plus a pointer, and cutting back to the previous +// full stop would swallow the decision and make the whole terminal look bare. +// Content on either side of the pointer disqualifies it equally; see +// TestBareAbovePointerOrientations, which checks every placement. +const clauseBoundaryChars = ".!?\n,;:—" + +// pointerResidueCutset is trimmed from both ends of what survives cutting the +// pointer's clause — the brackets, quotes, and punctuation a model wraps a +// back-reference in ("(Already answered above.)"). +const pointerResidueCutset = " \t\r\n.,;:!?()[]{}\"'“”‘’*_-—" + +// bareRemainderRe matches a remainder that carries no answer: empty, or only +// the filler a closer opens with. Shares fillerWords with summaryPreface so the +// two lists cannot drift. +var bareRemainderRe = regexp.MustCompile(`(?i)^(` + fillerWords + fillerSep + `*)*$`) + +// compressionMarkerRe matches a model announcing that what follows is the +// short form of something longer ("Short version: …", "TL;DR: …", "In short, +// …"). It is the second half of the deictic summary-closer test: the pointer +// says the full answer is elsewhere, and this says the text beside it is a +// condensation rather than new reasoning. +// +// Both halves are required, because a deictic pointer alone does not mean the +// terminal is disposable. "Given the analysis above, I recommend option B +// because X" opens with a pointer and then states a CONCLUSION the earlier +// turn never contained — discarding it in favour of that turn would throw away +// the answer. A compression marker is the model telling us the opposite. +// +// A marker WITHOUT a pointer stays out of scope, as summaryCloserRe's own +// comment explains: a user who asked for brevity is answered with exactly that +// shape. +// +// The marker must OPEN a sentence AND be followed by a delimiter — the colon +// or comma a model puts after it when it is genuinely introducing the short +// form. Both halves are needed against a different failure each: +// +// - Mid-sentence the same words are ordinary prose carrying new content. +// "Given the analysis above, the bottom line is that we need a different +// vendor" states a conclusion; it does not announce a condensation. +// - At a sentence opening the words can still run on into ordinary prose. +// "In short supply of alternatives, we went with B" opens a sentence with +// "In short" and is not a summary at all. +// +// A marker with no delimiter ("In short we chose B") is not matched, which +// fails closed: the terminal is kept, which is today's behaviour. +var compressionMarkerRe = regexp.MustCompile(`(?i)(^|[.!?:;—]\s*|\n\s*)` + + `(short version|shorter version|short answer|tl;?dr|in short|in brief|in summary|in sum|bottom line|net[- ]net|the gist)` + + `\s*([:,—-]|$)`) + // 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 @@ -128,7 +269,14 @@ var backRefRe = regexp.MustCompile(`(?i)(already answered|see above|as (i )?(sai // behavior (fail closed). Assembled from named fragments so the alternations // stay legible and extendable. const ( - summaryPreface = `((done|all set|ok(ay)?)[\s,.!:—-]+)?` // optional "Done —" style opener + // fillerWords are the throat-clearing tokens a closer opens with. Shared + // with bareRemainderRe, which has to recognise exactly the same set as + // "not answer content". + fillerWords = `(done|all set|ok(ay)?)` + // fillerSep is the punctuation a filler word trails. Shared for the same + // reason fillerWords is: two copies of one separator class drift. + fillerSep = `[\s,.!:—-]` + summaryPreface = `(` + fillerWords + fillerSep + `+)?` // optional "Done —" style opener summaryNouns = `(citations?|sources?|references?|claims?)` // "all" appears here AND in summaryArticle on purpose: as a quantifier // between noun and verb ("Citations all logged.") and as a determiner @@ -180,6 +328,19 @@ 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 + // abovePointerHeadChars bounds how far into the terminal a deictic "above" + // may sit and still read as pointing OUTSIDE this message (see + // pointsAbove). It applies to the deictic family only — backRefRe's fixed + // phrases are matched anywhere under the weak cap — so it is named for the + // pointer, not for the back-reference family as a whole. + // + // It IS weakFinalMaxChars: the same guard ("there is not enough text + // before the reference for it to be pointing at content inside this + // turn"), expressed as an offset because a summary closer carries a + // compression AFTER the pointer and so is not itself short. Defined by + // reference rather than by repeating the literal, because tuning the weak + // cap without the offset following it would split one rule into two. + abovePointerHeadChars = weakFinalMaxChars // 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 +364,23 @@ 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. +// +// The deictic half additionally demands that the pointer be the WHOLE message +// (bareAbovePointer), not merely present in it. Both classes now enforce the +// same rule from opposite ends: a terminal is only disposable when it carries +// no answer of its own — proved here by there being nothing beside the pointer, +// and in isSummaryCloser by the model declaring what is beside it a +// compression. func isWeakFinal(s string) bool { t := strings.TrimSpace(s) if t == "" { return true } - return len(t) <= weakFinalMaxChars && backRefRe.MatchString(t) + return len(t) <= weakFinalMaxChars && (backRefRe.MatchString(t) || bareAbovePointer(t)) } // isCitationsOnly reports whether a terminal turn is essentially just a @@ -238,19 +409,38 @@ 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 bookkeeping ack sentence — "Citations are logged." — which is +// summaryCloserRe, and carries no answer content of its own; +// - a deictic back-reference in the opening PLUS a compression marker — +// mort issue #1611's "Done — that's the full chain above. Short version: +// …" — which the ack shape alone did not cover. +// +// The second opener needs both halves. A pointer on its own does not make a +// terminal disposable: "Given the analysis above, I recommend option B because +// X" points backwards and then states a conclusion the earlier turn never +// contained, and replacing it with that turn would discard the answer. The +// compression marker is the model saying the opposite — that what follows is +// the short form of something it already wrote. +// +// 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. +// +// pointsAbove is used here; its sibling backRefRe is NOT. Those fixed phrases +// are matched 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) && compressionMarkerRe.MatchString(t)) } // lastSubstantiveAssistantText scans msgs newest→oldest (skipping the terminal diff --git a/agent/finalize_test.go b/agent/finalize_test.go index 61e6586..ac35341 100644 --- a/agent/finalize_test.go +++ b/agent/finalize_test.go @@ -26,6 +26,31 @@ 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, inside the length cap. A pointer only + // counts when it is ALL there is — see bareAbovePointer. + {"bare-above-pointer", "That's the full chain above.", true}, + {"above-pointer-in-parens", "(the breakdown is above)", true}, + {"bare-pointer-with-filler-opener", "Done. See the chain above.", true}, + + // A pointer sharing the terminal with real content is NOT weak: the + // content beside it is the answer, and discarding the terminal would + // throw it away. + {"pointer-plus-a-decision", "That's the chain above. Ship Tuesday.", false}, + + // The answer can also sit BEFORE the pointer, in the same sentence. + // Cutting back to the previous full stop rather than the previous + // CLAUSE swallowed it and made these look bare. + {"answer-before-pointer-same-sentence", "Ship Tuesday, as shown above.", false}, + {"filler-then-answer-before-pointer", "OK. The verdict is guilty, as detailed above.", false}, + {"answer-before-pointer-no-filler", "The answer is sixty minutes, as computed above.", false}, + {"answer-before-pointer-semicolon", "We are going with B; the rationale is above.", false}, + {"pointer-plus-a-choice", "See the summary above. Option B wins.", false}, + {"pointer-mid-sentence-then-answer", "As shown above, the answer is sixty minutes.", false}, + {"prepositional-then-deictic-with-content", "Anything above 100 boils. See the note above.", false}, + {"comparative-with-an-interjection", "Anything above, say, 40 degrees is a problem for the pump.", false}, + {"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 +61,156 @@ 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." + +// analysis1611 stands in for that run's front-loaded analysis: long enough to +// dwarf closer1611 (>3x its 220 bytes). Shared by the finalOutput table and +// the end-to-end Run test so the two cannot drift apart. +var analysis1611 = strings.TrimSpace(strings.Repeat("The break was the Iran strikes, then the Epstein files. ", 12)) + +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}, + {"line-final-crlf", "Everything is above\r\nShort version: yes.", true}, + {"crlf-at-end", "The full breakdown is above\r\n", 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}, + + // Hyphenated compounds. \b holds between "above" and "-", so a literal + // '-' in the terminator class makes all of these read as deictic. The + // space-separated cases above do NOT cover this: they are a different + // character, and one passed while the other was broken. + {"hyphen-above-average", "Turnout was above-average in three counties.", false}, + {"hyphen-above-board", "The deal was above-board from the start.", false}, + {"hyphen-above-ground", "Run the above-ground cable along the fence.", false}, + {"hyphen-above-mentioned", "The above-mentioned findings are attached.", false}, + + {"no-above-at-all", "42", false}, + {"empty", "", false}, + + // Offset bound: past abovePointerHeadChars there IS enough text before the + // reference for it to be pointing inside this same message. The + // lengths are LITERALS, not abovePointerHeadChars +/- n: a case sized from + // the constant it is meant to pin moves with it, and a break-check + // that widened the bound to 100000 sailed straight through. + {"late-reference-not-a-pointer", strings.Repeat("x", 200) + " as shown above.", false}, + // The exact boundary, both sides: "above" starts at index 120 (allowed, + // the bound is <=) and at 121 (rejected). 119/120 x's plus the space + // puts the 'a' on 120/121 — the separator is needed because \b will not + // hold between "x" and "above". + {"reference-exactly-at-the-bound", strings.Repeat("x", 119) + " above.", true}, + {"reference-one-past-the-bound", strings.Repeat("x", 120) + " above.", false}, + } + 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) + } + }) + } + + // The two bounds are ONE guard expressed two ways (see abovePointerHeadChars). + // Pinned here so decoupling them is a test failure, not a silent drift. + if abovePointerHeadChars != weakFinalMaxChars { + t.Errorf("abovePointerHeadChars = %d, weakFinalMaxChars = %d: the offset bound and the "+ + "weak-final cap are the same guard and must stay equal", + abovePointerHeadChars, weakFinalMaxChars) + } + if weakFinalMaxChars != 120 { + t.Errorf("weakFinalMaxChars = %d, want 120: the literal-length cases in this table "+ + "pin the bound at 120 and must be resized with it", weakFinalMaxChars) + } +} + +// TestBareAbovePointerOrientations is the anti-drift harness for the one rule +// that kept coming back: a deictic pointer only makes a terminal disposable +// when the pointer is ALL there is. +// +// Three consecutive review rounds found that rule broken in a DIFFERENT +// orientation — content after the pointer, then content before it in the same +// sentence — because each round's cases only covered the orientation that +// round was about, and each fix was tested only in the direction I had just +// thought of. Enumerating the placements is the fix a fourth patch would not +// have been: every pointer form is now checked against every position content +// can occupy, so a new pointer form or a new placement covers the whole grid +// rather than one cell of it. +// +// The rule under test is a single line: bare IFF there is no content. +func TestBareAbovePointerOrientations(t *testing.T) { + // Slices, not maps: map iteration is randomised, so the 36 cells would + // print in a different order every run and two failing runs could not be + // diffed against each other. + // + // Each pointer form is written so it reads naturally both as a whole + // sentence and as a trailing clause. + type pointerForm struct{ name, sentence, clause string } + pointers := []pointerForm{ + {"demonstrative", "That's the chain above.", "as shown above"}, + {"imperative", "See the note above.", "per the note above"}, + {"copular", "The breakdown is above.", "which is above"}, + } + // Where the answer can sit relative to the pointer. "" = nowhere: the + // pointer is alone, which is the only bare case. + const answer = "Ship Tuesday" + // bare records whether the built terminal contains NO answer — the only + // case the rule may treat as disposable. + placements := []struct { + name string + bare bool + build func(p pointerForm) string + }{ + {"alone", true, func(p pointerForm) string { return p.sentence }}, + {"alone as a clause", true, func(p pointerForm) string { return p.clause }}, + {"filler then pointer", true, func(p pointerForm) string { return "OK. " + p.sentence }}, + {"done-dash then pointer", true, func(p pointerForm) string { return "Done — " + p.clause + "." }}, + + {"before, same sentence", false, func(p pointerForm) string { return answer + ", " + p.clause + "." }}, + {"before, own sentence", false, func(p pointerForm) string { return answer + ". " + p.sentence }}, + {"before, own line", false, func(p pointerForm) string { return answer + "\n" + p.sentence }}, + {"before, list item", false, func(p pointerForm) string { return "- " + answer + "\n- " + p.sentence }}, + {"after, same sentence", false, func(p pointerForm) string { return p.clause + ", " + answer + "." }}, + {"after, own sentence", false, func(p pointerForm) string { return p.sentence + " " + answer + "." }}, + {"after, own line", false, func(p pointerForm) string { return p.sentence + "\n" + answer + "." }}, + {"after a filler opener", false, func(p pointerForm) string { return "OK. " + answer + ", " + p.clause + "." }}, + } + + for _, p := range pointers { + for _, pl := range placements { + t.Run(p.name+"/"+pl.name, func(t *testing.T) { + in := pl.build(p) + if !pointsAbove(in) { + t.Fatalf("setup is not a pointer at all, so the case proves nothing: %q", in) + } + if got := bareAbovePointer(in); got != pl.bare { + t.Errorf("bareAbovePointer(%q) = %v, want %v", in, got, pl.bare) + } + }) + } + } +} + func TestIsCitationsOnly(t *testing.T) { cases := []struct { name string @@ -102,6 +277,44 @@ 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}, + + // The compression marker has to OPEN a sentence. Mid-sentence the same + // words are prose carrying NEW content, not an announcement that what + // follows is a condensation. + {"marker-mid-sentence-is-new-content", "Given the analysis above, the bottom line is that we need a different vendor entirely.", false}, + {"marker-mid-sentence-in-short", "That is the chain above, and in short supply of alternatives we went with B.", false}, + // A marker phrase can OPEN a sentence and still be ordinary prose: + // "In short supply" is not an announcement of a summary. The delimiter + // after the marker is what separates the two. + {"marker-word-opens-but-runs-on", "That's the chain above. In short supply of alternatives, we went with B.", false}, + {"marker-word-opens-but-runs-on-summary", "That's the chain above. In summary meetings we agreed to ship on Tuesday.", false}, + {"marker-with-a-comma", "That's the chain above. In short, the merger fell through.", true}, + {"marker-with-a-colon", "That's the chain above. Short answer: no.", true}, + // No delimiter at all: not matched, so the terminal is kept. Fails + // closed, which is today's behaviour rather than a wrong recovery. + {"marker-without-a-delimiter", "That's the chain above. In short we went with B.", false}, + // The delimiter alone is not enough either: mid-sentence, "…, and in + // short, we went with B" is a clause continuation, not an announced + // summary. This is the case the sentence-opening anchor exists for — + // without it a break-check that removed the anchor survived, because + // every other mid-sentence case was already rejected for want of a + // delimiter. + {"marker-mid-sentence-with-a-delimiter", "That is the chain above, and in short, we went with B.", false}, + {"marker-opening-after-a-colon", "That's the chain above: in short, the merger fell through.", true}, + + // A deictic pointer WITHOUT a compression marker is not a summary + // closer: these state a conclusion the earlier turn never contained, + // so replacing them with that turn would discard the answer. + {"pointer-then-a-recommendation", "Given the analysis above, I recommend option B: it is the only one that survives a regional outage.", false}, + {"pointer-then-a-decision", "Based on everything above, we should ship Tuesday and hold the migration until the following sprint.", false}, + {"pointer-then-a-new-caveat", "That is the chain above. One thing it misses: the Senate vote is scheduled before any of this takes effect.", 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 +360,15 @@ 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. + analysis := analysis1611 + // A deictic pointer followed by a NEW conclusion (no compression marker): + // >120 bytes so isWeakFinal cannot claim it, and it must not be treated as + // a summary closer either. + pointerThenConclusion := "Given the analysis above, I recommend option B: it is the only one that survives a regional outage without a manual failover step." + // 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 +609,95 @@ 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(analysis, cite...), + llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}), + asst(closer1611), + }, + terminal: closer1611, + want: analysis, + }, + { + // 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, + }, + { + // The mirror of the case below: the answer sits BEFORE the pointer, + // in the same sentence. Every case in this file put it after, which + // is how a cut-back-to-the-previous-full-stop swallowed "Ship + // Tuesday" and shipped. + name: "answer before the pointer in the same sentence is not discarded", + msgs: []llm.Message{ + llm.UserText("when do we ship?"), + asst(hugeAnswer, cite...), + llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}), + asst("Ship Tuesday, as shown above."), + }, + terminal: "Ship Tuesday, as shown above.", + want: "Ship Tuesday, as shown above.", + }, + { + // The isWeakFinal twin of the case below: a SHORT pointer that + // shares its terminal with the decision. Before bareAbovePointer + // this was weak, so a >=200-byte prior turn replaced it and "Ship + // Tuesday" — the only thing the user needed — was discarded. + name: "short above-pointer sharing the terminal with the answer is not discarded", + msgs: []llm.Message{ + llm.UserText("when do we ship?"), + asst(hugeAnswer, cite...), + llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}), + asst("That's the chain above. Ship Tuesday."), + }, + terminal: "That's the chain above. Ship Tuesday.", + want: "That's the chain above. Ship Tuesday.", + }, + { + // A pointer plus a NEW conclusion is not a compression, so it must + // survive verbatim even though a much longer prior turn exists — + // otherwise the recommendation is thrown away in favour of the + // analysis it was drawn from. + name: "above-pointer closer with a new conclusion is not discarded", + msgs: []llm.Message{ + llm.UserText("which option?"), + asst(hugeAnswer, cite...), + llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}), + asst(pointerThenConclusion), + }, + terminal: pointerThenConclusion, + want: pointerThenConclusion, + }, { // 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 +847,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 := analysis1611 + 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) + } +}