From bcba9667bdfb85daff575daeca25267a028c6bf6 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Fri, 21 Aug 2026 23:20:11 -0400 Subject: [PATCH 01/10] fix(agent): recover the answer behind a bare "above" pointer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mort issue #1611: run 8eea3e82 front-loaded a 2,245-char analysis into its cite-call turn and closed with 220 bytes — "Done — that's the full chain above. Short version: …". The user got the 220 bytes and a pointer at a chain that was never posted. finalOutput already had three shapes for this pathology, and the closer matched none of them: too long for the weak-final cap (220 > 120), no citations heading, and no "Citations are logged." ack to open the summary-closer class. So it was delivered verbatim. The three shapes were each a separate vocabulary of ack phrases, which is why a fourth phrasing walked straight through. Two of them are really one signal — the terminal DEFERS, telling us the answer is somewhere the user cannot see — differing only in whether the terminal also carries content of its own. That signal is now isBackRef, shared by both, so a new phrasing is added once and covered in the bare and the "+ compression" variant at the same time. Its open-ended half is aboveRefRe: a DEICTIC "above", separated from the preposition by what follows the word. The deictic use ends its clause ("that's the full chain above.", "as shown above,"); the preposition always continues into a noun phrase ("above 100°C", "above the fold", "above all, …"). pointsAbove additionally requires the reference in the terminal's first 120 bytes — with almost no text before it in THIS message, it cannot be pointing at the message's own content. isSummaryCloser now opens on either the citations ack or pointsAbove. Recovery is unchanged: the mandatory dwarf ratio and the user-message scan boundary still gate it, so a closer only loses to a prior turn in the same user turn that is clearly the fuller original. Break-checked: reverting either classifier, dropping the clause-final rule, or dropping the offset bound each kills a named test; the unmutated control survives. --- agent/finalize.go | 92 +++++++++++++++++++++------ agent/finalize_test.go | 140 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 213 insertions(+), 19 deletions(-) diff --git a/agent/finalize.go b/agent/finalize.go index 72b9e77..1ffc9a8 100644 --- a/agent/finalize.go +++ b/agent/finalize.go @@ -25,14 +25,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 (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 // 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 // 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."). +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 { + return backRefRe.MatchString(t) || pointsAbove(t) +} + // 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 @@ -180,6 +217,13 @@ 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 + // 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 // answer regardless of how it opens (the preamble filter is not applied at // this length — see isSubstantiveAnswer). @@ -203,13 +247,16 @@ 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. func isWeakFinal(s string) bool { t := strings.TrimSpace(s) if t == "" { 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 @@ -238,19 +285,26 @@ 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 "citations are logged" +// -style ack sentence (summaryCloserRe), and a deictic back-reference in the +// terminal's opening (pointsAbove — mort issue #1611's "Done — that's the full +// chain above. Short version: …", which the ack shape alone did not cover). +// 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 { t := strings.TrimSpace(s) if t == "" || len(t) > summaryCloserMaxChars { return false } - return summaryCloserRe.MatchString(t) + return summaryCloserRe.MatchString(t) || pointsAbove(t) } // lastSubstantiveAssistantText scans msgs newest→oldest (skipping the terminal diff --git a/agent/finalize_test.go b/agent/finalize_test.go index 61e6586..d2bb61a 100644 --- a/agent/finalize_test.go +++ b/agent/finalize_test.go @@ -26,6 +26,12 @@ 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 (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 { 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}, + + // 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) { cases := []struct { name string @@ -102,6 +155,13 @@ 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}, + {"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 +207,11 @@ 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. + 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 { name string @@ -387,6 +452,50 @@ 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(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 // 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) } } + +// 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) + } +} -- 2.54.0 From f97c2b78c26f55ec7842c3298638b62f717ef3b4 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Fri, 21 Aug 2026 23:41:46 -0400 Subject: [PATCH 02/10] =?UTF-8?q?fix(agent):=20gadfly=20round=201=20?= =?UTF-8?q?=E2=80=94=20the=20hyphen=20made=20"above-board"=20a=20back-refe?= =?UTF-8?q?rence?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- agent/finalize.go | 45 +++++++++++++++++++++++++----------------- agent/finalize_test.go | 44 ++++++++++++++++++++++++++++++++++------- 2 files changed, 64 insertions(+), 25 deletions(-) diff --git a/agent/finalize.go b/agent/finalize.go index 1ffc9a8..ed08094 100644 --- a/agent/finalize.go +++ b/agent/finalize.go @@ -36,9 +36,9 @@ import ( // 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 +// 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 @@ -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 // 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|$)`) +// +// 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 // 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 { 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 { - return backRefRe.MatchString(t) || pointsAbove(t) -} - // 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 @@ -219,11 +226,13 @@ const ( 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 + // 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, 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 // answer regardless of how it opens (the preamble filter is not applied at // this length — see isSubstantiveAnswer). @@ -256,7 +265,7 @@ func isWeakFinal(s string) bool { if t == "" { 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 diff --git a/agent/finalize_test.go b/agent/finalize_test.go index d2bb61a..e300523 100644 --- a/agent/finalize_test.go +++ b/agent/finalize_test.go @@ -51,6 +51,13 @@ func TestIsWeakFinal(t *testing.T) { // 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 @@ -72,13 +79,24 @@ func TestPointsAbove(t *testing.T) { {"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 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}, {"empty", "", false}, // 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}, + // 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}, + {"reference-at-the-bound", strings.Repeat("x", 100) + " above.", true}, } for _, c := range cases { 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) { @@ -208,7 +238,7 @@ func TestFinalOutput(t *testing.T) { // 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. - 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 // 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." @@ -460,12 +490,12 @@ func TestFinalOutput(t *testing.T) { 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...), + asst(analysis, cite...), llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}), asst(closer1611), }, terminal: closer1611, - want: analysis1611, + want: analysis, }, { // 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 // 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)) + analysis := analysis1611() fp := fake.New("fp") fp.Enqueue("test-model", fake.ReplyWith(llm.Response{ -- 2.54.0 From 9534e442fb975747ef57c3578aa602438f559750 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Fri, 21 Aug 2026 23:44:52 -0400 Subject: [PATCH 03/10] =?UTF-8?q?test(agent):=20gadfly=20round=201b=20?= =?UTF-8?q?=E2=80=94=20pin=20the=20exact=20bound=20and=20the=20bar=20asymm?= =?UTF-8?q?etry?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sonnet's three findings, all on the tests rather than the rule: - The "at the bound" case sat at index 115 (and 101 after the first round), never at 120 itself, so <= vs < was never exercised. Both sides of the boundary are now pinned — index 120 accepted, 121 rejected — and an off-by-one mutation is killed by the new case. - A SHORT (<=120-byte) deictic closer takes the modeBackRef bar, not the summary closer's mandatory dwarf ratio, even though it can carry a scrap of answer content. That asymmetry is deliberate and pre-existing (a "see above" closer has always had it): inside 120 bytes there is no room for both a pointer and a real answer. Pinned with a case sized so the two bars actually DISAGREE — 3x98 = 294 > the 275-byte prior turn, so the summary bar would reject what the back-ref bar accepts. The first draft of this fixture was 83 bytes and passed under either bar, proving nothing. - The isBackRef doc finding was sonnet's fourth independent report of it; already fixed in the previous commit. Break-check now runs eight mutations, including the off-by-one and a swap of the classifier ordering; each is killed by a named test and the control survives. --- agent/finalize_test.go | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/agent/finalize_test.go b/agent/finalize_test.go index e300523..76ae38e 100644 --- a/agent/finalize_test.go +++ b/agent/finalize_test.go @@ -96,7 +96,12 @@ func TestPointsAbove(t *testing.T) { // 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}, + // 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) { @@ -239,6 +244,12 @@ func TestFinalOutput(t *testing.T) { 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 98-byte deictic closer: inside the weak-final cap (120), and sized so + // the two bars actually DISAGREE about it — 3x98 = 294 > longAnswer's 275, + // so the summary closer's dwarf ratio would reject longAnswer while the + // modeBackRef bar (>=200 bytes, no ratio) accepts it. A shorter closer + // would pass under either bar and prove nothing. + shortAbovePointer := "Done. The whole chain is above, so there is no point repeating all of that detail down here again." // 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." @@ -526,6 +537,25 @@ func TestFinalOutput(t *testing.T) { terminal: prepositionalTerminal, want: prepositionalTerminal, }, + { + // A SHORT deictic closer takes the modeBackRef bar, not the summary + // closer's mandatory dwarf ratio — even though it carries a scrap + // of answer content ("Done."). Deliberate, and the same contract a + // short "see above" closer has always had: within the 120-byte cap + // there is no room for both a pointer and a real answer, so a + // >=200-byte prior turn wins without having to be 3x. Here the + // ratio would demand ~330 bytes and wrongly keep the pointer. + // (gadfly/sonnet flagged the asymmetry; this pins it.) + name: "short above-pointer closer uses the back-ref bar, not the dwarf ratio", + msgs: []llm.Message{ + llm.UserText("q?"), + asst(longAnswer, cite...), + llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}), + asst(shortAbovePointer), + }, + terminal: shortAbovePointer, + want: longAnswer, + }, { // A closer matching BOTH the ack shape and a back-reference // carries no answer content, so the back-ref test must win and the -- 2.54.0 From 7dccb233adf3a931e15ed843ad311ef6689ec9cc Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Fri, 21 Aug 2026 23:52:25 -0400 Subject: [PATCH 04/10] docs(agent): drop the dangling isBackRef reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2 (glm): the isSummaryCloser comment still explained itself in terms of "the whole isBackRef class" — a symbol deleted in f97c2b7. It named the right distinction with the wrong noun. Reworded to name backRefRe directly; no isBackRef occurrences remain in the package. --- agent/finalize.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/agent/finalize.go b/agent/finalize.go index ed08094..64c348d 100644 --- a/agent/finalize.go +++ b/agent/finalize.go @@ -304,10 +304,10 @@ func isCitationsOnly(s string) bool { // 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. +// 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 { -- 2.54.0 From 3f1c016e749f96b2ca0707c856b794bb6c53be74 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sat, 22 Aug 2026 00:03:15 -0400 Subject: [PATCH 05/10] =?UTF-8?q?fix(agent):=20gadfly=20round=202=20?= =?UTF-8?q?=E2=80=94=20a=20pointer=20is=20not=20a=20compression?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings, all real. **A deictic closer now needs a compression marker too** (opus, correctness). The pointsAbove branch of isSummaryCloser required only a back-reference, so a terminal like "Given the analysis above, I recommend option B because X" — a pointer followed by a CONCLUSION the earlier turn never contained — would be discarded in favour of that turn, throwing away the answer. The ack shape does not have this problem because "Citations are logged." carries nothing; a bare pointer does not carry that guarantee. Both halves are now required: the pointer says the full answer is elsewhere, and compressionMarkerRe ("Short version:", "TL;DR", "In short") is the model saying what sits beside it is a condensation rather than new reasoning. #1611's closer has both. A marker without a pointer stays out of scope for the reason already documented — a user who asked for brevity gets exactly that shape. Unmatched closers keep today's behaviour, so the narrowing fails closed. **CRLF** (opus, error-handling): "above\r\n" was not clause-final, so a CRLF transcript quietly lost every line-final deictic. CR joins LF in the terminator set. **A comment wrapped mid-phrase** so that "// -style" read as a list marker (sonnet) — reflowed as part of rewriting that doc block. **Process provenance in a test comment** (sonnet): "(gadfly, 3 models)" is an execution log, not an invariant. The lesson survives, the attribution does not. Break-check is ten mutations now — dropping CR and dropping the compression requirement are each killed by their own named cases — with the control surviving. --- agent/finalize.go | 44 +++++++++++++++++++++++++++++++++++------- agent/finalize_test.go | 33 +++++++++++++++++++++++++++++-- 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/agent/finalize.go b/agent/finalize.go index 64c348d..7bc385d 100644 --- a/agent/finalize.go +++ b/agent/finalize.go @@ -136,8 +136,9 @@ var backRefRe = regexp.MustCompile(`(?i)(already answered|see above|as (i )?(sai // 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|$)`) +// 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 @@ -157,6 +158,23 @@ func pointsAbove(t string) bool { return loc != nil && loc[0] <= backRefHeadChars } +// 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. +var compressionMarkerRe = regexp.MustCompile(`(?i)\b(short version|shorter version|short answer|tl;?dr|in short|in brief|in summary|in sum|bottom line|net[- ]net|the gist)\b`) + // 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 @@ -296,10 +314,21 @@ func isCitationsOnly(s string) bool { // 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 "citations are logged" -// -style ack sentence (summaryCloserRe), and a deictic back-reference in the -// terminal's opening (pointsAbove — mort issue #1611's "Done — that's the full -// chain above. Short version: …", which the ack shape alone did not cover). +// 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. @@ -313,7 +342,8 @@ func isSummaryCloser(s string) bool { if t == "" || len(t) > summaryCloserMaxChars { return false } - return summaryCloserRe.MatchString(t) || pointsAbove(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 76ae38e..f69f555 100644 --- a/agent/finalize_test.go +++ b/agent/finalize_test.go @@ -70,6 +70,8 @@ func TestPointsAbove(t *testing.T) { {"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}, @@ -80,8 +82,9 @@ func TestPointsAbove(t *testing.T) { {"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). + // '-' 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}, @@ -196,6 +199,13 @@ func TestIsSummaryCloser(t *testing.T) { {"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}, + + // 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 { @@ -250,6 +260,10 @@ func TestFinalOutput(t *testing.T) { // modeBackRef bar (>=200 bytes, no ratio) accepts it. A shorter closer // would pass under either bar and prove nothing. shortAbovePointer := "Done. The whole chain is above, so there is no point repeating all of that detail down here again." + // 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." @@ -556,6 +570,21 @@ func TestFinalOutput(t *testing.T) { terminal: shortAbovePointer, want: longAnswer, }, + { + // 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 -- 2.54.0 From 1756910ef02c27ae7f34a4e5035ef205652ff0d6 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sat, 22 Aug 2026 00:16:47 -0400 Subject: [PATCH 06/10] =?UTF-8?q?fix(agent):=20gadfly=20round=203=20?= =?UTF-8?q?=E2=80=94=20the=20pointer=20must=20be=20all=20there=20is?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2 fixed this class on isSummaryCloser and left its twin standing on isWeakFinal. Three of the four findings are that twin, and every one of them reproduced: "That's the chain above. Ship Tuesday." weak=true "See the summary above. Option B wins." weak=true "Anything above 100 boils. See the note above." weak=true "Anything above, say, 40 degrees is a problem." weak=true Each of those is 37-58 bytes with the answer sitting right next to the pointer, and each was disposable — a >=200-byte earlier turn replaced it and "Ship Tuesday" went in the bin. pointsAbove asks whether a pointer is PRESENT; disposability needs it to be ALL THERE IS. bareAbovePointer cuts the reference's own clause — from the end of the previous sentence through the match — and requires what remains to be filler: nothing, punctuation, or a throat-clearing "Done —". Cutting the clause rather than testing position is what makes the other two shapes safe for free: a mixed terminal keeps its first sentence, and a comparative "above," with an interjection keeps the rest of its own. The fourth finding (kimi) is the same idea one level up: compressionMarkerRe matched anywhere, so "Given the analysis above, the bottom line is that we need a different vendor" read as an announced summary when it is a conclusion. The marker must now OPEN a sentence. Both classes now enforce one rule from opposite ends: a terminal is disposable only when it carries no answer of its own — proved in isWeakFinal by nothing standing beside the pointer, and in isSummaryCloser by the model declaring what stands beside it a compression. One of my own round-1b fixtures had to change: "…is above, so there is no point repeating it" is a pure pointer to a human, but prose after the reference is indistinguishable from an answer, so the rule correctly stops treating it as disposable. Replaced with a genuinely bare 96-byte pointer, still sized so the two recovery bars disagree about it. Break-check: thirteen mutations, each killed by a named test, control survives. M12 initially "passed" by failing to compile — reformulated so it builds, and TestIsWeakFinal/prepositional-then-deictic-with-content kills it properly. --- agent/finalize.go | 62 ++++++++++++++++++++++++++++++++++++++++-- agent/finalize_test.go | 51 +++++++++++++++++++++++++++++----- 2 files changed, 103 insertions(+), 10 deletions(-) diff --git a/agent/finalize.go b/agent/finalize.go index 7bc385d..88220af 100644 --- a/agent/finalize.go +++ b/agent/finalize.go @@ -158,6 +158,46 @@ func pointsAbove(t string) bool { return loc != nil && loc[0] <= backRefHeadChars } +// 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 := aboveRefRe.FindStringIndex(t) + if loc == nil || loc[0] > backRefHeadChars { + return false + } + start := 0 + if k := strings.LastIndexAny(t[:loc[0]], ".!?\n"); k >= 0 { + start = k + 1 + } + rest := strings.TrimSpace(t[:start]) + " " + strings.TrimSpace(t[loc[1]:]) + return bareRemainderRe.MatchString(strings.Trim(rest, pointerResidueCutset)) +} + +// 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 + `[\s,.!:—-]*)*$`) + // 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 @@ -173,7 +213,12 @@ func pointsAbove(t string) bool { // 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. -var compressionMarkerRe = regexp.MustCompile(`(?i)\b(short version|shorter version|short answer|tl;?dr|in short|in brief|in summary|in sum|bottom line|net[- ]net|the gist)\b`) +// The marker must OPEN a sentence. 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 — and treating that as disposable is the very failure the +// two-part test exists to prevent. +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)\b`) // summaryCloserRe matches a terminal turn that OPENS with a bookkeeping // acknowledgment of the citation round — "Citations are logged.", "Sources @@ -190,7 +235,11 @@ var compressionMarkerRe = regexp.MustCompile(`(?i)\b(short version|shorter versi // 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)?)` + summaryPreface = `(` + fillerWords + `[\s,.!:—-]+)?` // 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 @@ -278,12 +327,19 @@ const ( // 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) || pointsAbove(t)) + return len(t) <= weakFinalMaxChars && (backRefRe.MatchString(t) || bareAbovePointer(t)) } // isCitationsOnly reports whether a terminal turn is essentially just a diff --git a/agent/finalize_test.go b/agent/finalize_test.go index f69f555..3a999f1 100644 --- a/agent/finalize_test.go +++ b/agent/finalize_test.go @@ -27,9 +27,20 @@ func TestIsWeakFinal(t *testing.T) { {"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 (aboveRefRe), inside the length cap. + // 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}, + {"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 } @@ -200,6 +211,13 @@ func TestIsSummaryCloser(t *testing.T) { {"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. @@ -254,12 +272,16 @@ func TestFinalOutput(t *testing.T) { 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 98-byte deictic closer: inside the weak-final cap (120), and sized so - // the two bars actually DISAGREE about it — 3x98 = 294 > longAnswer's 275, - // so the summary closer's dwarf ratio would reject longAnswer while the - // modeBackRef bar (>=200 bytes, no ratio) accepts it. A shorter closer - // would pass under either bar and prove nothing. - shortAbovePointer := "Done. The whole chain is above, so there is no point repeating all of that detail down here again." + // A 96-byte BARE deictic closer: nothing but filler and the pointer's own + // clause, inside the weak-final cap (120), and sized so the two bars + // actually DISAGREE — 3x96 = 288 > longAnswer's 275, so the summary + // closer's dwarf ratio would reject longAnswer while the modeBackRef bar + // (>=200 bytes, no ratio) accepts it. A shorter closer would pass under + // either bar and prove nothing. It must also stay BARE: an earlier draft + // ended "…so there is no point repeating it", and prose after the + // reference is indistinguishable from an answer, so bareAbovePointer + // correctly stopped treating it as disposable. + shortAbovePointer := "Done — that is the complete chain, start to finish, exactly as I worked it out for you, above." // 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. @@ -570,6 +592,21 @@ func TestFinalOutput(t *testing.T) { terminal: shortAbovePointer, want: longAnswer, }, + { + // 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 — -- 2.54.0 From 184627c5704063c7f2fff4f1aa893c48bf5069fd Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sat, 22 Aug 2026 00:28:53 -0400 Subject: [PATCH 07/10] =?UTF-8?q?fix(agent):=20gadfly=20round=204=20?= =?UTF-8?q?=E2=80=94=20the=20answer=20can=20come=20BEFORE=20the=20pointer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My round-3 fix has the same defect it fixed, mirrored. bareAbovePointer cut back to the previous FULL STOP, not the previous clause, so anything sharing the reference's sentence ahead of it was swallowed: "Ship Tuesday, as shown above." bare=true "OK. The verdict is guilty, as detailed above." bare=true "The answer is sixty minutes, as computed above." bare=true All three read as pure pointers and were replaced by an earlier turn. The comment called it a clause cut; the code cut a sentence. It survived a round because every case I had written put the answer AFTER the pointer — I fixed the direction I had thought of and tested only that. clauseBoundaryChars now includes , ; : and the em dash. Three more, all real: - pointsAbove and bareAbovePointer each did their own FindStringIndex plus offset check, so the two could disagree about what a pointer is. One aboveRefLoc now answers that. - fillerWords was extracted to stop drift while its separator class was still copy-pasted beside it — the same defect one token over. fillerSep shares it too. - A missing blank comment line ran two paragraphs together. The padded 96-byte "bare pointer" fixture is gone rather than bent a third time. Its premise does not survive the rule: a genuinely bare pointer is its own clause and nothing else, so it is SHORT, and the two recovery bars cannot disagree about a short terminal. I had twice reshaped that string to keep a case alive whose scenario the code no longer admits. The modeBackRef path stays covered by the bare-pointer cases in TestIsWeakFinal and the "(Already answered above.)" end-to-end. Break-check: fifteen mutations including a regression to the sentence cut, each killed by a named test; the harness now also fails a mutation that only breaks the build. --- agent/finalize.go | 38 ++++++++++++++++++++++++++++++-------- agent/finalize_test.go | 42 ++++++++++++++++++------------------------ 2 files changed, 48 insertions(+), 32 deletions(-) diff --git a/agent/finalize.go b/agent/finalize.go index 88220af..1b5fd50 100644 --- a/agent/finalize.go +++ b/agent/finalize.go @@ -153,9 +153,17 @@ var aboveRefRe = regexp.MustCompile(`(?i)\babove\b[ \t]*([.,;:!?)\]"'’”—]| // 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 { 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) - return loc != nil && loc[0] <= backRefHeadChars + if loc == nil || loc[0] > backRefHeadChars { + return nil + } + return loc } // bareAbovePointer reports whether the terminal is a deictic back-reference and @@ -176,18 +184,28 @@ func pointsAbove(t string) bool { // 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 := aboveRefRe.FindStringIndex(t) - if loc == nil || loc[0] > backRefHeadChars { + loc := aboveRefLoc(t) + if loc == nil { return false } start := 0 - if k := strings.LastIndexAny(t[:loc[0]], ".!?\n"); k >= 0 { + if k := strings.LastIndexAny(t[:loc[0]], clauseBoundaryChars); k >= 0 { start = k + 1 } 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 only to the +// previous full stop swallowed the decision and made the whole thing look +// bare. That is the same defect as treating a pointer as disposable because +// something followed it — mirrored, and it survived a round of review because +// every case I had written put the answer AFTER the pointer. +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.)"). @@ -196,7 +214,7 @@ 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 + `[\s,.!:—-]*)*$`) +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, @@ -213,6 +231,7 @@ var bareRemainderRe = regexp.MustCompile(`(?i)^(` + fillerWords + `[\s,.!:—-]* // 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. 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 @@ -238,8 +257,11 @@ const ( // 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)?)` - summaryPreface = `(` + fillerWords + `[\s,.!:—-]+)?` // optional "Done —" style opener + 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 diff --git a/agent/finalize_test.go b/agent/finalize_test.go index 3a999f1..821a3b7 100644 --- a/agent/finalize_test.go +++ b/agent/finalize_test.go @@ -37,6 +37,14 @@ func TestIsWeakFinal(t *testing.T) { // 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}, @@ -272,16 +280,6 @@ func TestFinalOutput(t *testing.T) { 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 96-byte BARE deictic closer: nothing but filler and the pointer's own - // clause, inside the weak-final cap (120), and sized so the two bars - // actually DISAGREE — 3x96 = 288 > longAnswer's 275, so the summary - // closer's dwarf ratio would reject longAnswer while the modeBackRef bar - // (>=200 bytes, no ratio) accepts it. A shorter closer would pass under - // either bar and prove nothing. It must also stay BARE: an earlier draft - // ended "…so there is no point repeating it", and prose after the - // reference is indistinguishable from an answer, so bareAbovePointer - // correctly stopped treating it as disposable. - shortAbovePointer := "Done — that is the complete chain, start to finish, exactly as I worked it out for you, above." // 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. @@ -574,23 +572,19 @@ func TestFinalOutput(t *testing.T) { want: prepositionalTerminal, }, { - // A SHORT deictic closer takes the modeBackRef bar, not the summary - // closer's mandatory dwarf ratio — even though it carries a scrap - // of answer content ("Done."). Deliberate, and the same contract a - // short "see above" closer has always had: within the 120-byte cap - // there is no room for both a pointer and a real answer, so a - // >=200-byte prior turn wins without having to be 3x. Here the - // ratio would demand ~330 bytes and wrongly keep the pointer. - // (gadfly/sonnet flagged the asymmetry; this pins it.) - name: "short above-pointer closer uses the back-ref bar, not the dwarf ratio", + // 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("q?"), - asst(longAnswer, cite...), + llm.UserText("when do we ship?"), + asst(hugeAnswer, cite...), llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}), - asst(shortAbovePointer), + asst("Ship Tuesday, as shown above."), }, - terminal: shortAbovePointer, - want: longAnswer, + terminal: "Ship Tuesday, as shown above.", + want: "Ship Tuesday, as shown above.", }, { // The isWeakFinal twin of the case below: a SHORT pointer that -- 2.54.0 From 285789c89b823e8af6e0081b1b21cf702bd6dcff Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sat, 22 Aug 2026 00:32:45 -0400 Subject: [PATCH 08/10] test(agent): enumerate the orientations instead of patching them one by one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- agent/finalize.go | 8 +++++- agent/finalize_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/agent/finalize.go b/agent/finalize.go index 1b5fd50..1d89bba 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" ) @@ -190,7 +191,12 @@ func bareAbovePointer(t string) bool { } start := 0 if k := strings.LastIndexAny(t[:loc[0]], clauseBoundaryChars); k >= 0 { - start = k + 1 + // 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)) diff --git a/agent/finalize_test.go b/agent/finalize_test.go index 821a3b7..7ba434d 100644 --- a/agent/finalize_test.go +++ b/agent/finalize_test.go @@ -146,6 +146,71 @@ func TestPointsAbove(t *testing.T) { } } +// 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 -- 2.54.0 From bb2196ecb2f77151ccc78d26e4c1c36dee7f8047 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sat, 22 Aug 2026 00:37:48 -0400 Subject: [PATCH 09/10] test(agent): the orientation matrix iterated maps, so its 36 cells shuffled Caught on the sibling mort PR (gadfly/sonnet) and the same mistake was here: map iteration is randomised, so the matrix printed its cells in a different order every run and two failing runs could not be diffed. The "which placements are bare" lookup was a second map keyed by name, which also let a placement and its expectation drift apart by a typo. Both are now one slice of {name, bare, build}, so the expectation sits on the row it belongs to and the order is fixed. --- agent/finalize_test.go | 70 ++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/agent/finalize_test.go b/agent/finalize_test.go index 7ba434d..71ae120 100644 --- a/agent/finalize_test.go +++ b/agent/finalize_test.go @@ -161,50 +161,52 @@ func TestPointsAbove(t *testing.T) { // // 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 + // 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. - 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"}, + 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" - 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, + // 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 pname, p := range pointers { - for placement, build := range placements { - t.Run(pname+"/"+placement, func(t *testing.T) { - in := build(p) + 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) } - want := bare[placement] - if got := bareAbovePointer(in); got != want { - t.Errorf("bareAbovePointer(%q) = %v, want %v", in, got, want) + if got := bareAbovePointer(in); got != pl.bare { + t.Errorf("bareAbovePointer(%q) = %v, want %v", in, got, pl.bare) } }) } -- 2.54.0 From 7fd239c8cd3acbd9d24f0c88935ea38a6c6df0a3 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sat, 22 Aug 2026 00:42:22 -0400 Subject: [PATCH 10/10] fix(agent): "In short supply" is not a short version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 5. Two of the eight findings are the em-dash mid-rune slice, which the orientation matrix had already caught and fixed in 285789c before this round returned — the harness doing the reviewers' job, which is the point of it. Of the rest: **A marker phrase can open a sentence and still be prose** (kimi). "That's the chain above. In short supply of alternatives, we went with B." opens a sentence with "In short" and announces nothing. The marker must now be followed by the delimiter a model puts after a real announcement — the colon in "Short version:", the comma in "In short,". No delimiter means no match, which keeps the terminal: fails closed. Adding that made the sentence-opening anchor look redundant — a break-check that removed it survived, because every mid-sentence case in the table was by then rejected for want of a delimiter. It is not redundant: "…, and in short, we went with B" is a clause continuation with a comma. The missing thing was a case for what the anchor alone does, not the anchor. **backRefHeadChars was named for the wrong family** (opus; sonnet raised the same constant from the other side). It is applied only to the deictic offset test, never to backRefRe. Renamed abovePointerHeadChars. It stays defined as `= weakFinalMaxChars` rather than inlined, because an earlier round asked for exactly that linkage and the reason still holds. **The clause-boundary comment narrated my review history** (opus) — the same class of finding I took two rounds ago and then reintroduced one comment over. Rewritten as behaviour, pointing at the matrix. **analysis1611 was a function returning a constant string** (sonnet) — now a var. Accepted, not fixed: a comparative "above," plus a sentence-opening marker can still reach isSummaryCloser. That needs the comparative false positive, a real marker, a terminal under 300 bytes, and a same-turn prior at least 3x longer, all at once; the dwarf ratio is the backstop and no lexical test separates the comparative from the deictic. Break-check: seventeen mutations, each killed by a named test. --- agent/finalize.go | 51 ++++++++++++++++++++++++++---------------- agent/finalize_test.go | 37 +++++++++++++++++++++--------- 2 files changed, 58 insertions(+), 30 deletions(-) diff --git a/agent/finalize.go b/agent/finalize.go index 1d89bba..d06cdc3 100644 --- a/agent/finalize.go +++ b/agent/finalize.go @@ -161,7 +161,7 @@ func pointsAbove(t string) bool { return aboveRefLoc(t) != nil } // bareAbovePointer cannot disagree about what counts as a pointer. func aboveRefLoc(t string) []int { loc := aboveRefRe.FindStringIndex(t) - if loc == nil || loc[0] > backRefHeadChars { + if loc == nil || loc[0] > abovePointerHeadChars { return nil } return loc @@ -204,12 +204,11 @@ func bareAbovePointer(t string) bool { // 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 only to the -// previous full stop swallowed the decision and made the whole thing look -// bare. That is the same defect as treating a pointer as disposable because -// something followed it — mirrored, and it survived a round of review because -// every case I had written put the answer AFTER the pointer. +// 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 @@ -238,12 +237,22 @@ var bareRemainderRe = regexp.MustCompile(`(?i)^(` + fillerWords + fillerSep + `* // comment explains: a user who asked for brevity is answered with exactly that // shape. // -// The marker must OPEN a sentence. 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 — and treating that as disposable is the very failure the -// two-part test exists to prevent. -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)\b`) +// 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 @@ -319,15 +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 - // backRefHeadChars bounds how far into the terminal a deictic "above" may - // sit and still read as pointing OUTSIDE this message (see pointsAbove). - // It IS weakFinalMaxChars — the same guard ("there is not enough text + // 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, not by repeating the literal: tuning the weak cap without the - // offset following it would split one rule into two. - backRefHeadChars = weakFinalMaxChars + // 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). diff --git a/agent/finalize_test.go b/agent/finalize_test.go index 71ae120..ac35341 100644 --- a/agent/finalize_test.go +++ b/agent/finalize_test.go @@ -73,9 +73,7 @@ const closer1611 = "Done — that's the full chain above. Short version: it's no // 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)) -} +var analysis1611 = strings.TrimSpace(strings.Repeat("The break was the Iran strikes, then the Epstein files. ", 12)) func TestPointsAbove(t *testing.T) { cases := []struct { @@ -112,9 +110,9 @@ func TestPointsAbove(t *testing.T) { {"no-above-at-all", "42", false}, {"empty", "", false}, - // Offset bound: past backRefHeadChars there IS enough text before the + // 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 backRefHeadChars +/- n: a case sized from + // 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}, @@ -133,12 +131,12 @@ func TestPointsAbove(t *testing.T) { }) } - // The two bounds are ONE guard expressed two ways (see backRefHeadChars). + // 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 backRefHeadChars != weakFinalMaxChars { - t.Errorf("backRefHeadChars = %d, weakFinalMaxChars = %d: the offset bound and the "+ + 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", - backRefHeadChars, weakFinalMaxChars) + abovePointerHeadChars, weakFinalMaxChars) } if weakFinalMaxChars != 120 { t.Errorf("weakFinalMaxChars = %d, want 120: the literal-length cases in this table "+ @@ -291,6 +289,23 @@ func TestIsSummaryCloser(t *testing.T) { // 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 @@ -346,7 +361,7 @@ func TestFinalOutput(t *testing.T) { // 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() + 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. @@ -839,7 +854,7 @@ func TestRun_RecoversFrontLoadedAnswerWithCitations(t *testing.T) { // compression. The delivered output must be the front-loaded analysis, with no // extra model call. func TestRun_RecoversFrontLoadedAnswerOverAboveRefCloser(t *testing.T) { - analysis := analysis1611() + analysis := analysis1611 fp := fake.New("fp") fp.Enqueue("test-model", fake.ReplyWith(llm.Response{ -- 2.54.0