fix(agent): recover the answer behind a bare "above" pointer #28
@@ -3,6 +3,7 @@ package agent
|
||||
import (
|
||||
"regexp"
|
||||
"strings"
|
||||
"unicode/utf8"
|
||||
|
||||
"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
|
||||
)
|
||||
@@ -25,14 +26,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 (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
|
||||
// terminal can match more than one shape), so its links are never discarded.
|
||||
@@ -110,9 +118,142 @@ 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.").
|
||||
//
|
||||
// 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. 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
|
||||
// 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 { 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)
|
||||
if loc == nil || loc[0] > abovePointerHeadChars {
|
||||
return nil
|
||||
}
|
||||
return loc
|
||||
}
|
||||
|
||||
// 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 := aboveRefLoc(t)
|
||||
if loc == nil {
|
||||
return false
|
||||
}
|
||||
start := 0
|
||||
|
gitea-actions
commented
🟠 em dash in clauseBoundaryChars makes start=k+1 land mid-rune, so a bare "Done — … above." closer defeats bareRemainderRe and is not recovered correctness, error-handling · flagged by 1 model
🪰 Gadfly · advisory 🟠 **em dash in clauseBoundaryChars makes start=k+1 land mid-rune, so a bare "Done — … above." closer defeats bareRemainderRe and is not recovered**
_correctness, error-handling · flagged by 1 model_
**`agent/finalize.go:192` — em dash in `clauseBoundaryChars` makes `start = k+1` land mid-rune, so a short bare "Done — … above." closer is not recovered.**
<sub>🪰 Gadfly · advisory</sub>
|
||||
if k := strings.LastIndexAny(t[:loc[0]], clauseBoundaryChars); k >= 0 {
|
||||
// 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))
|
||||
}
|
||||
|
||||
// clauseBoundaryChars ends the clause the reference belongs to. Commas,
|
||||
|
gitea-actions
commented
🟡 First-person authorial narrative in permanent code comment ('every case I had written') violates project comment conventions maintainability · flagged by 2 models 1. First-person authorial voice in a permanent code comment — 🪰 Gadfly · advisory 🟡 **First-person authorial narrative in permanent code comment ('every case I had written') violates project comment conventions**
_maintainability · flagged by 2 models_
**1. First-person authorial voice in a permanent code comment** — `agent/finalize.go:205-206`
<sub>🪰 Gadfly · advisory</sub>
|
||||
// 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 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
|
||||
// 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 + fillerSep + `*)*$`)
|
||||
|
||||
// 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.
|
||||
//
|
||||
// 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
|
||||
// cited.", "Logged the citations." — the shape a model produces when it
|
||||
@@ -128,7 +269,14 @@ var backRefRe = regexp.MustCompile(`(?i)(already answered|see above|as (i )?(sai
|
||||
// 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)?)`
|
||||
// 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
|
||||
@@ -180,6 +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
|
||||
// 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 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).
|
||||
@@ -203,13 +364,23 @@ 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.
|
||||
//
|
||||
// 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)
|
||||
return len(t) <= weakFinalMaxChars && (backRefRe.MatchString(t) || bareAbovePointer(t))
|
||||
}
|
||||
|
||||
// isCitationsOnly reports whether a terminal turn is essentially just a
|
||||
@@ -238,19 +409,38 @@ 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 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.
|
||||
//
|
||||
// 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 {
|
||||
return false
|
||||
}
|
||||
return summaryCloserRe.MatchString(t)
|
||||
return summaryCloserRe.MatchString(t) ||
|
||||
(pointsAbove(t) && compressionMarkerRe.MatchString(t))
|
||||
}
|
||||
|
||||
// lastSubstantiveAssistantText scans msgs newest→oldest (skipping the terminal
|
||||
|
||||
@@ -26,6 +26,31 @@ 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, 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},
|
||||
|
||||
// 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},
|
||||
{"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
|
||||
}
|
||||
for _, c := range cases {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
@@ -36,6 +61,156 @@ 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."
|
||||
|
||||
// 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.
|
||||
var analysis1611 = strings.TrimSpace(strings.Repeat("The break was the Iran strikes, then the Epstein files. ", 12))
|
||||
|
gitea-actions
commented
⚪ analysis1611 is a function rather than a var — inconsistent with other test fixtures and recomputes on each call maintainability · flagged by 1 model 3. 🪰 Gadfly · advisory ⚪ **analysis1611 is a function rather than a var — inconsistent with other test fixtures and recomputes on each call**
_maintainability · flagged by 1 model_
**3. `analysis1611()` is a function rather than a `var`** — `agent/finalize_test.go:76-78`
<sub>🪰 Gadfly · advisory</sub>
|
||||
|
||||
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},
|
||||
{"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},
|
||||
|
||||
// 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},
|
||||
|
||||
// Hyphenated compounds. \b holds between "above" and "-", so a literal
|
||||
// '-' 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},
|
||||
{"hyphen-above-mentioned", "The above-mentioned findings are attached.", false},
|
||||
|
||||
{"no-above-at-all", "42", false},
|
||||
{"empty", "", false},
|
||||
|
||||
// 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 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},
|
||||
// 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) {
|
||||
if got := pointsAbove(c.in); got != c.want {
|
||||
t.Errorf("pointsAbove(%q) = %v, want %v", c.in, got, c.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// 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 abovePointerHeadChars != weakFinalMaxChars {
|
||||
t.Errorf("abovePointerHeadChars = %d, weakFinalMaxChars = %d: the offset bound and the "+
|
||||
"weak-final cap are the same guard and must stay equal",
|
||||
abovePointerHeadChars, 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)
|
||||
}
|
||||
}
|
||||
|
||||
// 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) {
|
||||
// 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.
|
||||
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"
|
||||
// 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 _, 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)
|
||||
}
|
||||
if got := bareAbovePointer(in); got != pl.bare {
|
||||
t.Errorf("bareAbovePointer(%q) = %v, want %v", in, got, pl.bare)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsCitationsOnly(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
@@ -102,6 +277,44 @@ 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},
|
||||
|
||||
// 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},
|
||||
// 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
|
||||
// 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 {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
@@ -147,6 +360,15 @@ 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.
|
||||
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.
|
||||
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."
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
@@ -387,6 +609,95 @@ 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(analysis, cite...),
|
||||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||||
asst(closer1611),
|
||||
},
|
||||
terminal: closer1611,
|
||||
want: analysis,
|
||||
},
|
||||
{
|
||||
// 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,
|
||||
},
|
||||
{
|
||||
// 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("when do we ship?"),
|
||||
asst(hugeAnswer, cite...),
|
||||
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
|
||||
asst("Ship Tuesday, as shown above."),
|
||||
},
|
||||
terminal: "Ship Tuesday, as shown above.",
|
||||
want: "Ship Tuesday, as shown above.",
|
||||
},
|
||||
{
|
||||
// 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 —
|
||||
// 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
|
||||
@@ -536,3 +847,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 := analysis1611
|
||||
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)
|
||||
}
|
||||
}
|
||||
|
||||
⚪ Rationale for not sharing backRefRe with isSummaryCloser is duplicated verbatim in pointsAbove and isSummaryCloser comments
maintainability · flagged by 1 model
2. Identical rationale stated in two places —
agent/finalize.go:149-155andagent/finalize.go:413-417🪰 Gadfly · advisory