steve
285789c89b
Three consecutive rounds found the same 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 direction that round was about, and each fix was then tested only in the direction I had just thought of. A fourth patch was not the answer; the missing thing was a harness. TestBareAbovePointerOrientations crosses every pointer form with every position content can occupy: alone, before in the same sentence, before in its own sentence, on its own line, as a list item, after in each of those, and behind a filler opener. 36 cells, one assertion — bare IFF there is no content — and each cell first asserts the input really is a pointer, so a mistyped fixture fails loudly instead of passing vacuously. It earned itself immediately: three cells failed on the first run, and the bug was mine and shipped. clauseBoundaryChars gained the em dash last round, and the cut did start = k + 1 — but LastIndexAny returns the BYTE index of the boundary rune, and an em dash is three bytes. "Done — as shown above." sliced mid-rune, left a stray continuation byte in the remainder that no trim removes, and a genuinely bare pointer stopped being recovered. Now advances by the rune's width. Break-check: sixteen mutations, each killed by a named test, control survives. Two had to be reformulated after the harness started failing mutations that were only "killed" by breaking the build — including the one for this very fix.
864 lines
39 KiB
Go
864 lines
39 KiB
Go
package agent
|
||
|
||
import (
|
||
"context"
|
||
"encoding/json"
|
||
"strings"
|
||
"testing"
|
||
|
||
"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
|
||
"gitea.stevedudenhoeffer.com/steve/majordomo/provider/fake"
|
||
)
|
||
|
||
func TestIsWeakFinal(t *testing.T) {
|
||
long := strings.Repeat("As I said, this is the full answer. ", 6) // >120, contains "as i said"
|
||
cases := []struct {
|
||
name string
|
||
in string
|
||
want bool
|
||
}{
|
||
{"empty", "", true},
|
||
{"whitespace", " \n\t ", true},
|
||
{"already-answered", "(Already answered above.)", true},
|
||
{"see-above", "see above", true},
|
||
{"as-i-said-short", "As I said, it's 60 minutes.", true},
|
||
{"crisp-number", "42", false},
|
||
{"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) {
|
||
if got := isWeakFinal(c.in); got != c.want {
|
||
t.Errorf("isWeakFinal(%q) = %v, want %v", c.in, got, c.want)
|
||
}
|
||
})
|
||
}
|
||
}
|
||
|
||
// 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.
|
||
func analysis1611() string {
|
||
return 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 backRefHeadChars there IS enough text before the
|
||
// reference for it to be pointing inside this same message. The
|
||
// lengths are LITERALS, not backRefHeadChars +/- 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 backRefHeadChars).
|
||
// Pinned here so decoupling them is a test failure, not a silent drift.
|
||
if backRefHeadChars != weakFinalMaxChars {
|
||
t.Errorf("backRefHeadChars = %d, weakFinalMaxChars = %d: the offset bound and the "+
|
||
"weak-final cap are the same guard and must stay equal",
|
||
backRefHeadChars, 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) {
|
||
// Each pointer form, written so it reads naturally both as a whole
|
||
// sentence and as a trailing clause.
|
||
pointers := map[string]struct{ sentence, clause string }{
|
||
"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"
|
||
placements := map[string]func(p struct{ sentence, clause string }) string{
|
||
"alone": func(p struct{ sentence, clause string }) string { return p.sentence },
|
||
"alone as a clause": func(p struct{ sentence, clause string }) string { return p.clause },
|
||
"before, same sentence": func(p struct{ sentence, clause string }) string { return answer + ", " + p.clause + "." },
|
||
"before, own sentence": func(p struct{ sentence, clause string }) string { return answer + ". " + p.sentence },
|
||
"before, own line": func(p struct{ sentence, clause string }) string { return answer + "\n" + p.sentence },
|
||
"before, list item": func(p struct{ sentence, clause string }) string { return "- " + answer + "\n- " + p.sentence },
|
||
"after, same sentence": func(p struct{ sentence, clause string }) string { return p.clause + ", " + answer + "." },
|
||
"after, own sentence": func(p struct{ sentence, clause string }) string { return p.sentence + " " + answer + "." },
|
||
"after, own line": func(p struct{ sentence, clause string }) string { return p.sentence + "\n" + answer + "." },
|
||
"after a filler opener": func(p struct{ sentence, clause string }) string { return "OK. " + answer + ", " + p.clause + "." },
|
||
"filler then pointer": func(p struct{ sentence, clause string }) string { return "OK. " + p.sentence },
|
||
"done-dash then pointer": func(p struct{ sentence, clause string }) string {
|
||
return "Done — " + p.clause + "."
|
||
},
|
||
}
|
||
// The only terminals with no answer in them.
|
||
bare := map[string]bool{
|
||
"alone": true,
|
||
"alone as a clause": true,
|
||
"filler then pointer": true,
|
||
"done-dash then pointer": true,
|
||
}
|
||
|
||
for pname, p := range pointers {
|
||
for placement, build := range placements {
|
||
t.Run(pname+"/"+placement, func(t *testing.T) {
|
||
in := build(p)
|
||
if !pointsAbove(in) {
|
||
t.Fatalf("setup is not a pointer at all, so the case proves nothing: %q", in)
|
||
}
|
||
want := bare[placement]
|
||
if got := bareAbovePointer(in); got != want {
|
||
t.Errorf("bareAbovePointer(%q) = %v, want %v", in, got, want)
|
||
}
|
||
})
|
||
}
|
||
}
|
||
}
|
||
|
||
func TestIsCitationsOnly(t *testing.T) {
|
||
cases := []struct {
|
||
name string
|
||
in string
|
||
want bool
|
||
}{
|
||
{"sources-md-links", "Sources: [pcprice.watch](https://pcprice.watch/x), [ebay](https://ebay.com/1).", true},
|
||
{"lowercase-bare-url", "sources: see https://example.com/a", true},
|
||
{"bold-label-colon-inside", "**Sources:** [a](https://a), [b](https://b)", true},
|
||
{"bold-label-colon-outside", "**Sources**: [a](https://a), [b](https://b)", true}, // colon after the closing **
|
||
{"references-dash", "References — [a](https://a)", true},
|
||
{"citations-label", "Citations: https://x/y", true},
|
||
{"leading-list-marker", "- Sources: [a](https://a)", true},
|
||
{"atx-heading", "## Sources: [a](https://a), [b](https://b)", true}, // ATX heading marker + its trailing space
|
||
{"further-reading", "Further reading: https://example.com/deep-dive", true},
|
||
{"annotated-multi-source", "Sources: [pcprice.watch](https://a) (tracker), [eBay](https://b) (sold), [bestvaluegpu](https://c) (retail), [resaleprices](https://d) (asking).", true}, // the reported issue-1418 shape
|
||
{"backref-plus-links-is-citations", "References: as noted above, [pcprice.watch](https://pcprice.watch/x).", true}, // a back-ref phrase inside a real sources list is still citations
|
||
|
||
{"empty", "", false},
|
||
{"label-but-no-link", "Source: internal analysis, no URL here", false},
|
||
{"prose-then-sources", "It sells for ~$2,700. Sources: [a](https://a)", false}, // answer first → not a pure addendum
|
||
{"source-led-prose-answer", "Source: According to https://cdc.gov the flu vaccine is 40-60% effective, and the CDC recommends annual vaccination for everyone over six months old.", false}, // a prose answer that merely opens with a "Source:" label
|
||
{"mentions-sources-midsentence", "The sources of the leak were never confirmed.", false},
|
||
{"link-without-label", "Here is the link you asked for: [a](https://a)", false},
|
||
{"bare-domains-out-of-scope", "Sources: pcprice.watch (used ~$200), ebay.com (sold listings)", false}, // bare domains: no scheme or markdown link to key on
|
||
{"crisp-number", "42", false},
|
||
}
|
||
for _, c := range cases {
|
||
t.Run(c.name, func(t *testing.T) {
|
||
if got := isCitationsOnly(c.in); got != c.want {
|
||
t.Errorf("isCitationsOnly(%q) = %v, want %v", c.in, got, c.want)
|
||
}
|
||
})
|
||
}
|
||
}
|
||
|
||
// 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 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) {
|
||
cases := []struct {
|
||
name string
|
||
in string
|
||
want bool
|
||
}{
|
||
{"b3cb9ee9-verbatim", b3cb9ee9Closer, true},
|
||
{"ack-only", "Citations are logged.", true},
|
||
{"ack-no-copula", "Citations logged.", true},
|
||
{"claims-cited", "All claims cited.", true},
|
||
{"verb-first", "Logged the citations.", true},
|
||
{"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},
|
||
{"ack-verb-then-clause", "Citations are logged in Zotero whenever you click the save button.", false},
|
||
{"compression-without-ack", "Short version: yes.", false}, // deliberately out of scope
|
||
{"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},
|
||
{"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) {
|
||
if got := isSummaryCloser(c.in); got != c.want {
|
||
t.Errorf("isSummaryCloser(%q) = %v, want %v", c.in, got, c.want)
|
||
}
|
||
})
|
||
}
|
||
}
|
||
|
||
func asst(text string, tools ...llm.ToolCall) llm.Message {
|
||
m := llm.Message{Role: llm.RoleAssistant}
|
||
if text != "" {
|
||
m.Parts = []llm.Part{llm.Text(text)}
|
||
}
|
||
m.ToolCalls = tools
|
||
return m
|
||
}
|
||
|
||
func TestFinalOutput(t *testing.T) {
|
||
cite := []llm.ToolCall{{ID: "c1", Name: "cite", Arguments: json.RawMessage(`{}`)}}
|
||
longAnswer := strings.TrimSpace(strings.Repeat("Free group calls are capped at sixty minutes. ", 6)) // >200
|
||
hugeAnswer := strings.TrimSpace(strings.Repeat("Free group calls are capped at sixty minutes. ", 12)) // >3x the b3cb9ee9 closer
|
||
// A sources/citations-only terminal — the glm-5.2 "cite" shape behind mort
|
||
// issue #1418: the prose answer was front-loaded into the tool-call turn and
|
||
// the terminal turn carried only the citations.
|
||
sources := "Sources: [pcprice.watch](https://pcprice.watch/x), [ebay](https://www.ebay.com/itm/1)."
|
||
answerWithSources := longAnswer + "\n\n" + sources
|
||
// A concise (>80, <200 byte) front-loaded answer + a long citations terminal:
|
||
// the ratio arm can't be met against the long terminal, so citations mode
|
||
// must fall back to the floor.
|
||
conciseAnswer := "It sells for about $2,700 used on eBay, typically $2,400 to $2,900 depending on condition and bundle."
|
||
longSources := "Sources: [pcprice.watch](https://pcprice.watch/gpu/rtx5090) (tracker), [ebay](https://www.ebay.com/sch/rtx5090) (sold), [newegg](https://newegg.com/rtx5090) (retail), [pcpartpicker](https://pcpartpicker.com/rtx5090) (history)."
|
||
// A substantive answer that merely OPENS with "Source:" (not a bare list).
|
||
sourceLedAnswer := "Source: https://nvd.nist.gov/vuln/detail/CVE-2024-1234 — this is the authoritative NVD entry for the vulnerability, rated CVSS 9.8 critical."
|
||
// A borderline-band (80–200 byte) turn that opens like a planning preamble:
|
||
// it clears the floor, but the preamble filter still vetoes it (the filter
|
||
// applies only in the borderline band; a >=200-byte turn is accepted as-is).
|
||
preambleTurn := "Let me look that up across a few different sites and then compile the full comparison for you here."
|
||
// 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."
|
||
// 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
|
||
msgs []llm.Message
|
||
terminal string
|
||
want string
|
||
}{
|
||
{
|
||
name: "front-loaded answer recovered over back-ref closer",
|
||
msgs: []llm.Message{
|
||
llm.UserText("q?"),
|
||
asst(longAnswer, cite...),
|
||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||
asst("(Already answered above.)"),
|
||
},
|
||
terminal: "(Already answered above.)",
|
||
want: longAnswer,
|
||
},
|
||
{
|
||
name: "empty terminal recovers prior substantive answer",
|
||
msgs: []llm.Message{
|
||
llm.UserText("q?"),
|
||
asst(longAnswer, cite...),
|
||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||
asst(""),
|
||
},
|
||
terminal: "",
|
||
want: longAnswer,
|
||
},
|
||
{
|
||
name: "healthy terminal answer is unchanged",
|
||
msgs: []llm.Message{
|
||
llm.UserText("q?"),
|
||
asst("Let me check.", cite...),
|
||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||
asst(longAnswer),
|
||
},
|
||
terminal: longAnswer,
|
||
want: longAnswer,
|
||
},
|
||
{
|
||
name: "short crisp answer not overridden by a short preamble prior",
|
||
msgs: []llm.Message{
|
||
llm.UserText("is it up?"),
|
||
asst("Let me check the server status.", cite...),
|
||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||
asst("It's down, restarting now."),
|
||
},
|
||
terminal: "It's down, restarting now.", // not weak → returned as-is
|
||
want: "It's down, restarting now.",
|
||
},
|
||
{
|
||
name: "weak terminal but only a preamble prior: no recovery",
|
||
msgs: []llm.Message{
|
||
llm.UserText("q?"),
|
||
asst("Let me look that up for you.", cite...),
|
||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||
asst("(see above)"),
|
||
},
|
||
terminal: "(see above)",
|
||
want: "(see above)", // preamble excluded; falls back to terminal
|
||
},
|
||
{
|
||
name: "citations-only terminal recovers front-loaded answer and keeps sources",
|
||
msgs: []llm.Message{
|
||
llm.UserText("q?"),
|
||
asst(longAnswer, cite...),
|
||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||
asst(sources),
|
||
},
|
||
terminal: sources,
|
||
want: answerWithSources, // answer recovered, citations appended
|
||
},
|
||
{
|
||
name: "citations-only terminal but only a preamble prior: keeps the sources",
|
||
msgs: []llm.Message{
|
||
llm.UserText("q?"),
|
||
asst("Let me gather the sources.", cite...),
|
||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||
asst(sources),
|
||
},
|
||
terminal: sources,
|
||
want: sources, // nothing substantive to recover → keep the addendum
|
||
},
|
||
{
|
||
name: "citations already in the recovered answer are not duplicated",
|
||
msgs: []llm.Message{
|
||
llm.UserText("q?"),
|
||
asst(answerWithSources, cite...),
|
||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||
asst(sources),
|
||
},
|
||
terminal: sources,
|
||
want: answerWithSources, // recovered turn already carries the sources
|
||
},
|
||
{
|
||
// #1418 persisted for CONCISE answers: a <200-char front-loaded
|
||
// answer must still be recovered against a long citations terminal
|
||
// (the ratio arm is skipped in citations mode).
|
||
name: "concise front-loaded answer recovered against a long citations terminal",
|
||
msgs: []llm.Message{
|
||
llm.UserText("q?"),
|
||
asst(conciseAnswer, cite...),
|
||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||
asst(longSources),
|
||
},
|
||
terminal: longSources,
|
||
want: conciseAnswer + "\n\n" + longSources,
|
||
},
|
||
{
|
||
// A substantive answer that merely OPENS with "Source:" and cites a
|
||
// URL mid-sentence is NOT a citations addendum — return it verbatim,
|
||
// never prepend the prior planning turn.
|
||
name: "source-led substantive answer is not hijacked by a prior turn",
|
||
msgs: []llm.Message{
|
||
llm.UserText("what's the authoritative URL?"),
|
||
asst("I'll look up the CVE in the NVD database, cross-reference the vendor advisory, and confirm the canonical URL before I answer.", cite...),
|
||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||
asst(sourceLedAnswer),
|
||
},
|
||
terminal: sourceLedAnswer,
|
||
want: sourceLedAnswer,
|
||
},
|
||
{
|
||
// A borderline-length turn that opens like a preamble is vetoed
|
||
// during recovery; the older real answer is recovered instead. (A
|
||
// >=200-byte turn would be accepted verbatim — see the next case.)
|
||
name: "borderline preamble is skipped; older real answer recovered",
|
||
msgs: []llm.Message{
|
||
llm.UserText("q?"),
|
||
asst(conciseAnswer, cite...),
|
||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||
asst(preambleTurn, cite...),
|
||
llm.ToolResultsMessage(llm.ToolResult{ID: "c2", Name: "cite", Content: "ok"}),
|
||
asst(sources),
|
||
},
|
||
terminal: sources,
|
||
want: conciseAnswer + "\n\n" + sources,
|
||
},
|
||
{
|
||
// Guards the gadfly regression: a LONG (>=200-byte) front-loaded
|
||
// answer that merely opens with a conversational word ("Sure, …")
|
||
// must still be recovered — the preamble filter must not veto it.
|
||
name: "long answer opening with a conversational word is still recovered",
|
||
msgs: []llm.Message{
|
||
llm.UserText("q?"),
|
||
asst(longConversationalAnswer, cite...),
|
||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||
asst(sources),
|
||
},
|
||
terminal: sources,
|
||
want: longConversationalAnswer + "\n\n" + sources,
|
||
},
|
||
{
|
||
// The b3cb9ee9 shape: full answer front-loaded into the cite turn,
|
||
// then a summary closer. The closer is discarded — its content is a
|
||
// strict compression of the recovered answer.
|
||
name: "summary closer discarded when the front-loaded answer dwarfs it",
|
||
msgs: []llm.Message{
|
||
llm.UserText("where did the $64M come from?"),
|
||
asst(hugeAnswer, cite...),
|
||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||
asst(b3cb9ee9Closer),
|
||
},
|
||
terminal: b3cb9ee9Closer,
|
||
want: hugeAnswer,
|
||
},
|
||
{
|
||
// The dwarf ratio is mandatory for a summary closer at EVERY length:
|
||
// a prior turn that is longer but not clearly the fuller original
|
||
// (here ~275 chars vs a 151-char closer, under the 3x bar) must not
|
||
// displace a closer that carries real answer content.
|
||
name: "summary 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(b3cb9ee9Closer),
|
||
},
|
||
terminal: b3cb9ee9Closer,
|
||
want: b3cb9ee9Closer,
|
||
},
|
||
{
|
||
// An ack-only closer ("Citations are logged.") is tiny, so even a
|
||
// modest front-loaded answer clears the ratio and replaces it.
|
||
name: "ack-only summary closer recovered over a modest answer",
|
||
msgs: []llm.Message{
|
||
llm.UserText("q?"),
|
||
asst(longAnswer, cite...),
|
||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||
asst("Citations are logged."),
|
||
},
|
||
terminal: "Citations are logged.",
|
||
want: longAnswer,
|
||
},
|
||
{
|
||
name: "summary closer with only a preamble prior keeps the closer",
|
||
msgs: []llm.Message{
|
||
llm.UserText("q?"),
|
||
asst("Let me gather the numbers.", cite...),
|
||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||
asst(b3cb9ee9Closer),
|
||
},
|
||
terminal: b3cb9ee9Closer,
|
||
want: b3cb9ee9Closer,
|
||
},
|
||
{
|
||
// 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,
|
||
},
|
||
{
|
||
// 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
|
||
// 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) {
|
||
if got := finalOutput(tc.msgs, tc.terminal); got != tc.want {
|
||
t.Errorf("finalOutput = %q, want %q", got, tc.want)
|
||
}
|
||
})
|
||
}
|
||
}
|
||
|
||
func citeToolbox(t *testing.T) *llm.Toolbox {
|
||
t.Helper()
|
||
return llm.NewToolbox("sources", llm.Tool{
|
||
Name: "cite",
|
||
Description: "Record a citation.",
|
||
Parameters: json.RawMessage(`{"type":"object","properties":{}}`),
|
||
Handler: func(_ context.Context, _ json.RawMessage) (any, error) {
|
||
return map[string]bool{"ok": true}, nil
|
||
},
|
||
})
|
||
}
|
||
|
||
// TestRun_RecoversFrontLoadedAnswer reproduces the glm-5.2 shape end-to-end: a
|
||
// turn carrying the full answer text AND a tool call, then a degenerate
|
||
// terminal turn. The recovered answer must be delivered with no extra model
|
||
// call (zero-cost recovery from the transcript).
|
||
func TestRun_RecoversFrontLoadedAnswer(t *testing.T) {
|
||
longAnswer := strings.TrimSpace(strings.Repeat("Free group calls are capped at sixty minutes. ", 6))
|
||
fp := fake.New("fp")
|
||
fp.Enqueue("test-model",
|
||
fake.ReplyWith(llm.Response{
|
||
Parts: []llm.Part{llm.Text(longAnswer)},
|
||
ToolCalls: []llm.ToolCall{{ID: "c1", Name: "cite", Arguments: json.RawMessage(`{}`)}},
|
||
FinishReason: llm.FinishToolCalls,
|
||
Usage: llm.Usage{InputTokens: 10, OutputTokens: 5},
|
||
}),
|
||
fake.Reply("(Already answered above.)"),
|
||
)
|
||
|
||
a := New(newModel(t, fp), "sys", WithToolbox(citeToolbox(t)))
|
||
res, err := a.Run(context.Background(), "is there a meet time limit?")
|
||
if err != nil {
|
||
t.Fatalf("Run: %v", err)
|
||
}
|
||
if res.Output != longAnswer {
|
||
t.Errorf("Output = %q, want recovered front-loaded answer", res.Output)
|
||
}
|
||
if n := len(fp.Calls()); n != 2 {
|
||
t.Errorf("model calls = %d, want 2 (no extra nudge turn)", n)
|
||
}
|
||
}
|
||
|
||
// TestRun_HealthyTerminalUnchanged guards against regressing the normal case:
|
||
// a deferred answer in the terminal turn is delivered verbatim.
|
||
func TestRun_HealthyTerminalUnchanged(t *testing.T) {
|
||
fp := fake.New("fp")
|
||
fp.Enqueue("test-model",
|
||
toolCallReply("c1", "cite", `{}`),
|
||
fake.Reply("The limit is 60 minutes for free group calls."),
|
||
)
|
||
a := New(newModel(t, fp), "sys", WithToolbox(citeToolbox(t)))
|
||
res, err := a.Run(context.Background(), "q?")
|
||
if err != nil {
|
||
t.Fatalf("Run: %v", err)
|
||
}
|
||
if res.Output != "The limit is 60 minutes for free group calls." {
|
||
t.Errorf("Output = %q, want terminal answer unchanged", res.Output)
|
||
}
|
||
}
|
||
|
||
// TestRun_RecoversFrontLoadedAnswerOverSummaryCloser reproduces mort run
|
||
// b3cb9ee9 end-to-end: the model front-loads its full answer into the
|
||
// cite-call turn, the cite results come back, and the terminal turn is only a
|
||
// bookkeeping ack plus a one-line compression. The delivered output must be
|
||
// the front-loaded answer, with no extra model call.
|
||
func TestRun_RecoversFrontLoadedAnswerOverSummaryCloser(t *testing.T) {
|
||
hugeAnswer := strings.TrimSpace(strings.Repeat("Free group calls are capped at sixty minutes. ", 12))
|
||
fp := fake.New("fp")
|
||
fp.Enqueue("test-model",
|
||
fake.ReplyWith(llm.Response{
|
||
Parts: []llm.Part{llm.Text(hugeAnswer)},
|
||
ToolCalls: []llm.ToolCall{{ID: "c1", Name: "cite", Arguments: json.RawMessage(`{}`)}},
|
||
FinishReason: llm.FinishToolCalls,
|
||
Usage: llm.Usage{InputTokens: 10, OutputTokens: 5},
|
||
}),
|
||
fake.Reply(b3cb9ee9Closer),
|
||
)
|
||
|
||
a := New(newModel(t, fp), "sys", WithToolbox(citeToolbox(t)))
|
||
res, err := a.Run(context.Background(), "where did the $64M come from?")
|
||
if err != nil {
|
||
t.Fatalf("Run: %v", err)
|
||
}
|
||
if res.Output != hugeAnswer {
|
||
t.Errorf("Output = %q, want recovered front-loaded answer", res.Output)
|
||
}
|
||
if n := len(fp.Calls()); n != 2 {
|
||
t.Errorf("model calls = %d, want 2 (no extra nudge turn)", n)
|
||
}
|
||
}
|
||
|
||
// TestRun_RecoversFrontLoadedAnswerWithCitations reproduces mort issue #1418
|
||
// end-to-end: the model front-loads the prose answer into the tool-call turn
|
||
// and closes with a sources-only terminal turn. The delivered output must be
|
||
// the recovered answer with the citations appended (not the bare sources list),
|
||
// with no extra model call.
|
||
func TestRun_RecoversFrontLoadedAnswerWithCitations(t *testing.T) {
|
||
longAnswer := strings.TrimSpace(strings.Repeat("Free group calls are capped at sixty minutes. ", 6))
|
||
sources := "Sources: [docs](https://example.com/docs), [pricing](https://example.com/pricing)."
|
||
fp := fake.New("fp")
|
||
fp.Enqueue("test-model",
|
||
fake.ReplyWith(llm.Response{
|
||
Parts: []llm.Part{llm.Text(longAnswer)},
|
||
ToolCalls: []llm.ToolCall{{ID: "c1", Name: "cite", Arguments: json.RawMessage(`{}`)}},
|
||
FinishReason: llm.FinishToolCalls,
|
||
Usage: llm.Usage{InputTokens: 10, OutputTokens: 5},
|
||
}),
|
||
fake.Reply(sources),
|
||
)
|
||
|
||
a := New(newModel(t, fp), "sys", WithToolbox(citeToolbox(t)))
|
||
res, err := a.Run(context.Background(), "is there a meet time limit?")
|
||
if err != nil {
|
||
t.Fatalf("Run: %v", err)
|
||
}
|
||
want := longAnswer + "\n\n" + sources
|
||
if res.Output != want {
|
||
t.Errorf("Output = %q, want recovered answer + citations %q", res.Output, want)
|
||
}
|
||
if n := len(fp.Calls()); n != 2 {
|
||
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)
|
||
}
|
||
}
|