fix(agent): gadfly round 1 — the hyphen made "above-board" a back-reference
CI / Tidy (pull_request) Successful in 9m25s
CI / Build & Test (pull_request) Successful in 10m43s

Three of four reviewers independently found the same defect: `\b` holds
between "above" and "-", so the literal hyphen in aboveRefRe's terminator
class made every hyphenated compound clause-final. "above-average",
"above-board", "above-ground" all read as deictic pointers, putting a
legitimate short closer at risk of being discarded. My own test used the
SPACE-separated "above average" — it tested the neighbour, not the named
path, which is exactly why the class survived a round of review. Hyphen
dropped from the class; the four compounds are now table cases.

Two more findings, both real:

- backRefHeadChars and weakFinalMaxChars were two unlinked 120 literals
  that the comment called "the same guard". Now defined by reference,
  with a test pinning the identity.
- isBackRef claimed to be the shared extension point for both shapes but
  had a single caller — isSummaryCloser deliberately uses pointsAbove
  directly, because backRefRe's fixed phrases match ANYWHERE and are only
  safe under the 120-byte weak cap. Folding the two together for
  tidiness would widen the gate, not deduplicate it. Helper deleted, the
  doc moved onto pointsAbove where the real sharing is, and it now says
  why backRefRe is not shared.

Also shared the front-loaded-analysis fixture between the table and the
end-to-end test (kimi), and de-hollowed the offset-bound cases: they were
sized as backRefHeadChars±n, so they moved with the constant they were
meant to pin — a break-check that widened the bound to 100000 sailed
through. Literal lengths now, plus an explicit identity assertion.

Break-check: six mutations, each killed by a named test; control survives.
This commit is contained in:
2026-08-21 23:41:46 -04:00
parent bcba9667bd
commit f97c2b78c2
2 changed files with 64 additions and 25 deletions
+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
// 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
@@ -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
@@ -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{