fix(agent): recover the answer behind a bare "above" pointer #28

Merged
steve merged 10 commits from fix/backref-above-closer into main 2026-08-22 04:53:04 +00:00
2 changed files with 213 additions and 19 deletions
Showing only changes of commit bcba9667bd - Show all commits
+73 -19
View File
@@ -25,14 +25,21 @@ import (
// glm-5.2 "cite" pattern behind mort issue #1418). The citations are real, // 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 // useful content — unlike a back-reference — so recover the prior answer and
// KEEP the citations, appended below it. // KEEP the citations, appended below it.
// - a bookkeeping closer ("Citations are logged. Short version: …"): the // - a deferring closer ("Citations are logged. Short version: …", "Done —
// model acknowledged the citation round and compressed the answer it had // that's the full chain above. Short version: …"): the model points at the
// already written into a one-liner (mort run b3cb9ee9 — a 2,089-char answer // answer it had already written and compresses it into a one-liner (mort
// shrank to a 153-byte closer at delivery). The compression is strictly // run b3cb9ee9 — a 2,089-char answer shrank to a 153-byte closer; mort
// poorer than the front-loaded answer, so recover the prior turn and // issue #1611 — a 2,245-char analysis shrank to 220 bytes). The
// DISCARD the closer — but only when the prior turn clearly dwarfs it, // compression is strictly poorer than the front-loaded answer, so recover
// because unlike a back-reference this closer DOES carry answer content // the prior turn and DISCARD the closer — but only when the prior turn
// (see modeSummary). // 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 // 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. // 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 // backRefRe matches a terminal turn that merely points back to an earlier
// message instead of stating the answer ("(Already answered above.)", // 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))`) 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 {
Review

Rationale for not sharing backRefRe with isSummaryCloser is duplicated verbatim in pointsAbove and isSummaryCloser comments

maintainability · flagged by 1 model

2. Identical rationale stated in two placesagent/finalize.go:149-155 and agent/finalize.go:413-417

🪰 Gadfly · advisory

⚪ **Rationale for not sharing backRefRe with isSummaryCloser is duplicated verbatim in pointsAbove and isSummaryCloser comments** _maintainability · flagged by 1 model_ **2. Identical rationale stated in two places** — `agent/finalize.go:149-155` and `agent/finalize.go:413-417` <sub>🪰 Gadfly · advisory</sub>
return backRefRe.MatchString(t) || pointsAbove(t)
}
// summaryCloserRe matches a terminal turn that OPENS with a bookkeeping // summaryCloserRe matches a terminal turn that OPENS with a bookkeeping
// acknowledgment of the citation round — "Citations are logged.", "Sources // acknowledgment of the citation round — "Citations are logged.", "Sources
// cited.", "Logged the citations." — the shape a model produces when it // cited.", "Logged the citations." — the shape a model produces when it
2
@@ -180,6 +217,13 @@ const (
// genuine final answer that merely contains "as I said" mid-sentence is // genuine final answer that merely contains "as I said" mid-sentence is
// longer than this, so it is never treated as weak. // longer than this, so it is never treated as weak.
weakFinalMaxChars = 120 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 // 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 // answer regardless of how it opens (the preamble filter is not applied at
// this length — see isSubstantiveAnswer). // this length — see isSubstantiveAnswer).
@@ -203,13 +247,16 @@ const (
) )
// isWeakFinal reports whether a terminal turn's text fails to stand on its own // 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 { func isWeakFinal(s string) bool {
t := strings.TrimSpace(s) t := strings.TrimSpace(s)
if t == "" { if t == "" {
return true 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 // 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 return len(residue) <= len(t)/citationDominatedDivisor
} }
// isSummaryCloser reports whether a terminal turn is a bookkeeping closer: it // isSummaryCloser reports whether a terminal turn defers to an earlier answer
// opens with a complete "citations are logged"-style ack sentence (see // and is short enough that whatever follows the deferral can only be a
// summaryCloserRe) and is short enough that whatever follows the ack can only // compression of it. Two openers qualify: a complete "citations are logged"
// be a compression of an earlier, fuller answer. Whether that fuller answer // -style ack sentence (summaryCloserRe), and a deictic back-reference in the
// actually exists is modeSummary's job — the dwarf ratio in // terminal's opening (pointsAbove — mort issue #1611's "Done — that's the full
// isSubstantiveAnswer keeps a matching closer in place when nothing earlier // chain above. Short version: …", which the ack shape alone did not cover).
// clearly outweighs it. // 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 { func isSummaryCloser(s string) bool {
t := strings.TrimSpace(s) t := strings.TrimSpace(s)
if t == "" || len(t) > summaryCloserMaxChars { if t == "" || len(t) > summaryCloserMaxChars {
return false return false
} }
return summaryCloserRe.MatchString(t) return summaryCloserRe.MatchString(t) || pointsAbove(t)
} }
// lastSubstantiveAssistantText scans msgs newest→oldest (skipping the terminal // lastSubstantiveAssistantText scans msgs newest→oldest (skipping the terminal
+140
View File
@@ -26,6 +26,12 @@ func TestIsWeakFinal(t *testing.T) {
{"crisp-yes", "Yes.", false}, {"crisp-yes", "Yes.", false},
{"crisp-status", "It's down, restarting now.", false}, {"crisp-status", "It's down, restarting now.", false},
{"long-with-as-i-said", long, false}, // >120 chars: not weak despite the phrase {"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 { for _, c := range cases {
t.Run(c.name, func(t *testing.T) { 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},
Outdated
Review

analysis1611 is a function rather than a var — inconsistent with other test fixtures and recomputes on each call

maintainability · flagged by 1 model

3. analysis1611() is a function rather than a varagent/finalize_test.go:76-78

🪰 Gadfly · advisory

⚪ **analysis1611 is a function rather than a var — inconsistent with other test fixtures and recomputes on each call** _maintainability · flagged by 1 model_ **3. `analysis1611()` is a function rather than a `var`** — `agent/finalize_test.go:76-78` <sub>🪰 Gadfly · advisory</sub>
// 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) { func TestIsCitationsOnly(t *testing.T) {
cases := []struct { cases := []struct {
name string name string
@@ -102,6 +155,13 @@ func TestIsSummaryCloser(t *testing.T) {
{"mentions-citations-midsentence", "The paper's citations are what got it retracted.", false}, {"mentions-citations-midsentence", "The paper's citations are what got it retracted.", false},
{"crisp-number", "42", 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 {"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 { for _, c := range cases {
t.Run(c.name, func(t *testing.T) { 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, // 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. // 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." 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 { tests := []struct {
name string name string
@@ -387,6 +452,50 @@ func TestFinalOutput(t *testing.T) {
terminal: b3cb9ee9Closer, terminal: b3cb9ee9Closer,
want: hugeAnswer, 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 // A closer matching BOTH the ack shape and a back-reference
// carries no answer content, so the back-ref test must win and the // 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) 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)
}
}