fix(agent): recover the answer behind a bare "above" pointer #28

Merged
steve merged 10 commits from fix/backref-above-closer into main 2026-08-22 04:53:04 +00:00
2 changed files with 48 additions and 32 deletions
Showing only changes of commit 184627c570 - Show all commits
+30 -8
View File
1
@@ -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 // only safe under isWeakFinal's 120-byte cap. Folding the two into one
// predicate for tidiness would hand isSummaryCloser an unanchored match across // predicate for tidiness would hand isSummaryCloser an unanchored match across
// 300 bytes — widening the gate, not deduplicating it. // 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) 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 // 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 // bare, and a comparative "above," with an interjection after it keeps the rest
// of its own sentence for the same reason. // of its own sentence for the same reason.
func bareAbovePointer(t string) bool { func bareAbovePointer(t string) bool {
loc := aboveRefRe.FindStringIndex(t) loc := aboveRefLoc(t)
if loc == nil || loc[0] > backRefHeadChars { if loc == nil {
return false return false
} }
start := 0 start := 0
if k := strings.LastIndexAny(t[:loc[0]], ".!?\n"); k >= 0 { if k := strings.LastIndexAny(t[:loc[0]], clauseBoundaryChars); k >= 0 {
Review

🟠 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.

🪰 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>
start = k + 1 start = k + 1
} }
rest := strings.TrimSpace(t[:start]) + " " + strings.TrimSpace(t[loc[1]:]) rest := strings.TrimSpace(t[:start]) + " " + strings.TrimSpace(t[loc[1]:])
return bareRemainderRe.MatchString(strings.Trim(rest, pointerResidueCutset)) 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
Review

🟡 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 commentagent/finalize.go:205-206

🪰 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>
// 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 // pointerResidueCutset is trimmed from both ends of what survives cutting the
// pointer's clause — the brackets, quotes, and punctuation a model wraps a // pointer's clause — the brackets, quotes, and punctuation a model wraps a
// back-reference in ("(Already answered above.)"). // 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 // bareRemainderRe matches a remainder that carries no answer: empty, or only
// the filler a closer opens with. Shares fillerWords with summaryPreface so the // the filler a closer opens with. Shares fillerWords with summaryPreface so the
// two lists cannot drift. // 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 // compressionMarkerRe matches a model announcing that what follows is the
// short form of something longer ("Short version: …", "TL;DR: …", "In short, // 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 // 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 // comment explains: a user who asked for brevity is answered with exactly that
// shape. // shape.
//
// The marker must OPEN a sentence. Mid-sentence the same words are ordinary // 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 // 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 // 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 // fillerWords are the throat-clearing tokens a closer opens with. Shared
// with bareRemainderRe, which has to recognise exactly the same set as // with bareRemainderRe, which has to recognise exactly the same set as
// "not answer content". // "not answer content".
fillerWords = `(done|all set|ok(ay)?)` fillerWords = `(done|all set|ok(ay)?)`
summaryPreface = `(` + fillerWords + `[\s,.!:—-]+)?` // optional "Done —" style opener // 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?)` summaryNouns = `(citations?|sources?|references?|claims?)`
// "all" appears here AND in summaryArticle on purpose: as a quantifier // "all" appears here AND in summaryArticle on purpose: as a quantifier
// between noun and verb ("Citations all logged.") and as a determiner // between noun and verb ("Citations all logged.") and as a determiner
+18 -24
View File
@@ -37,6 +37,14 @@ func TestIsWeakFinal(t *testing.T) {
// content beside it is the answer, and discarding the terminal would // content beside it is the answer, and discarding the terminal would
// throw it away. // throw it away.
{"pointer-plus-a-decision", "That's the chain above. Ship Tuesday.", false}, {"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-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}, {"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}, {"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." bothMatchCloser := "Citations are logged. As I mentioned above, the full detail on the money sources is in my earlier message."
// The #1611 pair: a front-loaded analysis that dwarfs its 220-byte closer. // The #1611 pair: a front-loaded analysis that dwarfs its 220-byte closer.
analysis := analysis1611() 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): // 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 // >120 bytes so isWeakFinal cannot claim it, and it must not be treated as
// a summary closer either. // a summary closer either.
@@ -574,23 +572,19 @@ func TestFinalOutput(t *testing.T) {
want: prepositionalTerminal, want: prepositionalTerminal,
}, },
{ {
// A SHORT deictic closer takes the modeBackRef bar, not the summary // The mirror of the case below: the answer sits BEFORE the pointer,
// closer's mandatory dwarf ratio — even though it carries a scrap // in the same sentence. Every case in this file put it after, which
// of answer content ("Done."). Deliberate, and the same contract a // is how a cut-back-to-the-previous-full-stop swallowed "Ship
// short "see above" closer has always had: within the 120-byte cap // Tuesday" and shipped.
// there is no room for both a pointer and a real answer, so a name: "answer before the pointer in the same sentence is not discarded",
// >=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{ msgs: []llm.Message{
llm.UserText("q?"), llm.UserText("when do we ship?"),
asst(longAnswer, cite...), asst(hugeAnswer, cite...),
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}), llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
asst(shortAbovePointer), asst("Ship Tuesday, as shown above."),
}, },
terminal: shortAbovePointer, terminal: "Ship Tuesday, as shown above.",
want: longAnswer, want: "Ship Tuesday, as shown above.",
}, },
{ {
// The isWeakFinal twin of the case below: a SHORT pointer that // The isWeakFinal twin of the case below: a SHORT pointer that