From 7fd239c8cd3acbd9d24f0c88935ea38a6c6df0a3 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sat, 22 Aug 2026 00:42:22 -0400 Subject: [PATCH] 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{