fix(agent): recover the answer behind a bare "above" pointer #28
+32
-19
@@ -161,7 +161,7 @@ func pointsAbove(t string) bool { return aboveRefLoc(t) != nil }
|
|||||||
// bareAbovePointer cannot disagree about what counts as a pointer.
|
// bareAbovePointer cannot disagree about what counts as a pointer.
|
||||||
func aboveRefLoc(t string) []int {
|
func aboveRefLoc(t string) []int {
|
||||||
loc := aboveRefRe.FindStringIndex(t)
|
loc := aboveRefRe.FindStringIndex(t)
|
||||||
if loc == nil || loc[0] > backRefHeadChars {
|
if loc == nil || loc[0] > abovePointerHeadChars {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
return loc
|
return loc
|
||||||
@@ -204,12 +204,11 @@ func bareAbovePointer(t string) bool {
|
|||||||
|
|
||||||
// clauseBoundaryChars ends the clause the reference belongs to. Commas,
|
// clauseBoundaryChars ends the clause the reference belongs to. Commas,
|
||||||
|
|
|||||||
// semicolons, colons and dashes are in it, not just sentence terminators,
|
// semicolons, colons and dashes are in it, not just sentence terminators,
|
||||||
// because the answer can share the reference's SENTENCE: "Ship Tuesday, as
|
// 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
|
// shown above." is a decision plus a pointer, and cutting back to the previous
|
||||||
// previous full stop swallowed the decision and made the whole thing look
|
// full stop would swallow the decision and make the whole terminal look bare.
|
||||||
// bare. That is the same defect as treating a pointer as disposable because
|
// Content on either side of the pointer disqualifies it equally; see
|
||||||
// something followed it — mirrored, and it survived a round of review because
|
// TestBareAbovePointerOrientations, which checks every placement.
|
||||||
// every case I had written put the answer AFTER the pointer.
|
|
||||||
const clauseBoundaryChars = ".!?\n,;:—"
|
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
|
||||||
@@ -238,12 +237,22 @@ var bareRemainderRe = regexp.MustCompile(`(?i)^(` + fillerWords + fillerSep + `*
|
|||||||
// 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 AND be followed by a delimiter — the colon
|
||||||
// prose carrying new content — "Given the analysis above, the bottom line is
|
// or comma a model puts after it when it is genuinely introducing the short
|
||||||
// that we need a different vendor" states a conclusion, it does not announce a
|
// form. Both halves are needed against a different failure each:
|
||||||
// condensation — and treating that as disposable is the very failure the
|
//
|
||||||
// two-part test exists to prevent.
|
// - Mid-sentence the same words are ordinary prose carrying new content.
|
||||||
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`)
|
// "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
|
// summaryCloserRe matches a terminal turn that OPENS with a bookkeeping
|
||||||
// acknowledgment of the citation round — "Citations are logged.", "Sources
|
// 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
|
// genuine final answer that merely contains "as I said" mid-sentence is
|
||||||
// longer than this, so it is never treated as weak.
|
// longer than this, so it is never treated as weak.
|
||||||
weakFinalMaxChars = 120
|
weakFinalMaxChars = 120
|
||||||
// backRefHeadChars bounds how far into the terminal a deictic "above" may
|
// abovePointerHeadChars bounds how far into the terminal a deictic "above"
|
||||||
// sit and still read as pointing OUTSIDE this message (see pointsAbove).
|
// may sit and still read as pointing OUTSIDE this message (see
|
||||||
// It IS weakFinalMaxChars — the same guard ("there is not enough text
|
// 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
|
// before the reference for it to be pointing at content inside this
|
||||||
// turn"), expressed as an offset because a summary closer carries a
|
// turn"), expressed as an offset because a summary closer carries a
|
||||||
// compression AFTER the pointer and so is not itself short. Defined by
|
// compression AFTER the pointer and so is not itself short. Defined by
|
||||||
// reference, not by repeating the literal: tuning the weak cap without the
|
// reference rather than by repeating the literal, because tuning the weak
|
||||||
// offset following it would split one rule into two.
|
// cap without the offset following it would split one rule into two.
|
||||||
backRefHeadChars = weakFinalMaxChars
|
abovePointerHeadChars = weakFinalMaxChars
|
||||||
// recoverMinChars: a prior assistant turn this long is treated as a real
|
// 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
|
// answer regardless of how it opens (the preamble filter is not applied at
|
||||||
// this length — see isSubstantiveAnswer).
|
// this length — see isSubstantiveAnswer).
|
||||||
|
|||||||
+26
-11
@@ -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
|
// 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
|
// dwarf closer1611 (>3x its 220 bytes). Shared by the finalOutput table and
|
||||||
// the end-to-end Run test so the two cannot drift apart.
|
// the end-to-end Run test so the two cannot drift apart.
|
||||||
func analysis1611() string {
|
var analysis1611 = strings.TrimSpace(strings.Repeat("The break was the Iran strikes, then the Epstein files. ", 12))
|
||||||
return strings.TrimSpace(strings.Repeat("The break was the Iran strikes, then the Epstein files. ", 12))
|
|
||||||
}
|
|
||||||
|
|
||||||
func TestPointsAbove(t *testing.T) {
|
func TestPointsAbove(t *testing.T) {
|
||||||
cases := []struct {
|
cases := []struct {
|
||||||
@@ -112,9 +110,9 @@ func TestPointsAbove(t *testing.T) {
|
|||||||
{"no-above-at-all", "42", false},
|
{"no-above-at-all", "42", false},
|
||||||
{"empty", "", 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
|
// 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
|
// the constant it is meant to pin moves with it, and a break-check
|
||||||
// that widened the bound to 100000 sailed straight through.
|
// that widened the bound to 100000 sailed straight through.
|
||||||
{"late-reference-not-a-pointer", strings.Repeat("x", 200) + " as shown above.", false},
|
{"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.
|
// Pinned here so decoupling them is a test failure, not a silent drift.
|
||||||
if backRefHeadChars != weakFinalMaxChars {
|
if abovePointerHeadChars != weakFinalMaxChars {
|
||||||
t.Errorf("backRefHeadChars = %d, weakFinalMaxChars = %d: the offset bound and the "+
|
t.Errorf("abovePointerHeadChars = %d, weakFinalMaxChars = %d: the offset bound and the "+
|
||||||
"weak-final cap are the same guard and must stay equal",
|
"weak-final cap are the same guard and must stay equal",
|
||||||
backRefHeadChars, weakFinalMaxChars)
|
abovePointerHeadChars, weakFinalMaxChars)
|
||||||
}
|
}
|
||||||
if weakFinalMaxChars != 120 {
|
if weakFinalMaxChars != 120 {
|
||||||
t.Errorf("weakFinalMaxChars = %d, want 120: the literal-length cases in this table "+
|
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.
|
// 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-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-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},
|
{"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
|
// 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.
|
// 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."
|
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 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.
|
||||||
@@ -839,7 +854,7 @@ func TestRun_RecoversFrontLoadedAnswerWithCitations(t *testing.T) {
|
|||||||
// compression. The delivered output must be the front-loaded analysis, with no
|
// compression. The delivered output must be the front-loaded analysis, with no
|
||||||
// extra model call.
|
// extra model call.
|
||||||
func TestRun_RecoversFrontLoadedAnswerOverAboveRefCloser(t *testing.T) {
|
func TestRun_RecoversFrontLoadedAnswerOverAboveRefCloser(t *testing.T) {
|
||||||
analysis := analysis1611()
|
analysis := analysis1611
|
||||||
fp := fake.New("fp")
|
fp := fake.New("fp")
|
||||||
fp.Enqueue("test-model",
|
fp.Enqueue("test-model",
|
||||||
fake.ReplyWith(llm.Response{
|
fake.ReplyWith(llm.Response{
|
||||||
|
|||||||
Reference in New Issue
Block a user
🟡 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🪰 Gadfly · advisory