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 64 additions and 25 deletions
Showing only changes of commit f97c2b78c2 - Show all commits
+27 -18
View File
@@ -36,9 +36,9 @@ import (
// carry answer content (see modeSummary). // carry answer content (see modeSummary).
// //
// The last two shapes share one signal — the terminal DEFERS: it tells us the // 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). // answer is somewhere the user cannot see (pointsAbove, or the bookkeeping
// They differ only in whether the terminal also carries content of its own, so // ack). They differ only in whether the terminal also carries content of its
// a new deferral phrase added to that shared signal is covered in both the // own, so a new deferral phrasing added to pointsAbove is covered in both the
// bare and the "+ compression" variant at once. // 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
@@ -130,26 +130,33 @@ var backRefRe = regexp.MustCompile(`(?i)(already answered|see above|as (i )?(sai
// back-reference family is covered without enumerating every phrasing a model // back-reference family is covered without enumerating every phrasing a model
// might invent — backRefRe's fixed list kept missing new ones (mort issue // might invent — backRefRe's fixed list kept missing new ones (mort issue
// #1611: "Done — that's the full chain above."). // #1611: "Done — that's the full chain above.").
var aboveRefRe = regexp.MustCompile(`(?i)\babove\b[ \t]*([.,;:!?)\]"'’”—-]|\n|$)`) //
// 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.
var aboveRefRe = regexp.MustCompile(`(?i)\babove\b[ \t]*([.,;:!?)\]"'’”—]|\n|$)`)
// pointsAbove reports whether a deictic "above" appears in the terminal's // 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 // 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 // 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 // at the message's own content, so it must point at a turn the user never saw
// (the harness delivers only the final turn). // (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
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>
// 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 { func pointsAbove(t string) bool {
loc := aboveRefRe.FindStringIndex(t) loc := aboveRefRe.FindStringIndex(t)
return loc != nil && loc[0] <= backRefHeadChars return loc != nil && loc[0] <= backRefHeadChars
} }
// isBackRef reports whether a terminal turn defers to earlier content instead
// of stating the answer — the signal shared by the back-reference and
// summary-closer shapes (see finalOutput). Extend the class here, once, rather
// than in either caller.
func isBackRef(t string) bool {
return backRefRe.MatchString(t) || pointsAbove(t)
}
// summaryCloserRe matches a terminal turn that OPENS with a bookkeeping // 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
@@ -219,11 +226,13 @@ const (
weakFinalMaxChars = 120 weakFinalMaxChars = 120
// backRefHeadChars bounds how far into the terminal a deictic "above" may // backRefHeadChars bounds how far into the terminal a deictic "above" may
// sit and still read as pointing OUTSIDE this message (see pointsAbove). // sit and still read as pointing OUTSIDE this message (see pointsAbove).
// Same guard as weakFinalMaxChars — "there is not enough text before the // It IS weakFinalMaxChars — the same guard ("there is not enough text
// reference for it to be pointing at content inside this turn" — but // before the reference for it to be pointing at content inside this
// expressed as an offset, because a summary closer carries a compression // turn"), expressed as an offset because a summary closer carries a
// AFTER the pointer and so is not itself short. // compression AFTER the pointer and so is not itself short. Defined by
backRefHeadChars = 120 // reference, not by repeating the literal: tuning the weak cap without the
// offset following it would split one rule into two.
backRefHeadChars = weakFinalMaxChars
// 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).
@@ -256,7 +265,7 @@ func isWeakFinal(s string) bool {
if t == "" { if t == "" {
return true return true
} }
return len(t) <= weakFinalMaxChars && isBackRef(t) return len(t) <= weakFinalMaxChars && (backRefRe.MatchString(t) || pointsAbove(t))
} }
// isCitationsOnly reports whether a terminal turn is essentially just a // isCitationsOnly reports whether a terminal turn is essentially just a
+37 -7
View File
@@ -51,6 +51,13 @@ func TestIsWeakFinal(t *testing.T) {
// that had never been posted. // 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." 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) { func TestPointsAbove(t *testing.T) {
cases := []struct { cases := []struct {
name string name string
1
@@ -72,13 +79,24 @@ func TestPointsAbove(t *testing.T) {
{"above-all", "Above all, keep the deploy green.", false}, {"above-all", "Above all, keep the deploy green.", false},
{"above-average", "Turnout was above average in three counties.", 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 made all of these read as deictic — the
// space-separated cases above did NOT cover it (gadfly, 3 models).
{"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}, {"no-above-at-all", "42", false},
{"empty", "", false}, {"empty", "", false},
// Offset bound: past backRefHeadChars there IS enough text before the // Offset bound: past backRefHeadChars there IS enough text before the
// reference for it to be pointing inside this same message. // reference for it to be pointing inside this same message. The
{"late-reference-not-a-pointer", strings.Repeat("x", backRefHeadChars+1) + " as shown above.", false}, // lengths are LITERALS, not backRefHeadChars +/- n: a case sized from
{"reference-at-the-bound", strings.Repeat("x", backRefHeadChars-6) + " above.", true}, // 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},
{"reference-at-the-bound", strings.Repeat("x", 100) + " above.", true},
} }
for _, c := range cases { for _, c := range cases {
t.Run(c.name, func(t *testing.T) { t.Run(c.name, func(t *testing.T) {
@@ -87,6 +105,18 @@ func TestPointsAbove(t *testing.T) {
} }
}) })
} }
// 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)
}
} }
func TestIsCitationsOnly(t *testing.T) { func TestIsCitationsOnly(t *testing.T) {
@@ -208,7 +238,7 @@ func TestFinalOutput(t *testing.T) {
// 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. // 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 analysis := analysis1611()
// A terminal using "above" as a PREPOSITION — not a back-reference, so it // A terminal using "above" as a PREPOSITION — not a back-reference, so it
// must survive verbatim next to a dwarfing prior turn. // 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." prepositionalTerminal := "Anything above 100 degrees boils off, which is exactly why the sample evaporated overnight in the unsealed tray."
@@ -460,12 +490,12 @@ func TestFinalOutput(t *testing.T) {
name: "above-pointer closer discarded when the front-loaded answer dwarfs it", name: "above-pointer closer discarded when the front-loaded answer dwarfs it",
msgs: []llm.Message{ msgs: []llm.Message{
llm.UserText("what set off the Truth Social rant?"), llm.UserText("what set off the Truth Social rant?"),
asst(analysis1611, cite...), asst(analysis, cite...),
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}), llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
asst(closer1611), asst(closer1611),
}, },
terminal: closer1611, terminal: closer1611,
want: analysis1611, want: analysis,
}, },
{ {
// The dwarf ratio governs the new opener too: a prior turn that is // The dwarf ratio governs the new opener too: a prior turn that is
@@ -652,7 +682,7 @@ func TestRun_RecoversFrontLoadedAnswerWithCitations(t *testing.T) {
// compression. The delivered output must be the front-loaded analysis, with no // compression. The delivered output must be the front-loaded analysis, with no
// extra model call. // extra model call.
func TestRun_RecoversFrontLoadedAnswerOverAboveRefCloser(t *testing.T) { func TestRun_RecoversFrontLoadedAnswerOverAboveRefCloser(t *testing.T) {
analysis := strings.TrimSpace(strings.Repeat("The break was the Iran strikes, then the Epstein files. ", 12)) analysis := analysis1611()
fp := fake.New("fp") fp := fake.New("fp")
fp.Enqueue("test-model", fp.Enqueue("test-model",
fake.ReplyWith(llm.Response{ fake.ReplyWith(llm.Response{