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 —