diff --git a/docs/changes/unreleased/975-task-name-is-a-label.md b/docs/changes/unreleased/975-task-name-is-a-label.md new file mode 100644 index 0000000000..ffa6782110 --- /dev/null +++ b/docs/changes/unreleased/975-task-name-is-a-label.md @@ -0,0 +1,30 @@ +--- +kind: fixed +title: A namer that answers with its plan is refused, so the task keeps the person's own words +pr: 975 +surface: [chat, engine] +invalidates: + - "A task whose namer answered with a sentence was named from the front of that sentence: the rail row and the task page read `I'll start by` while the node's own record carried the person's words right beside it. No longer true: an answer that opens as a clause about the speaker is refused at any length, and the row keeps the fallback — the person's own words, clipped by the column exactly as an unnamed task's title always was." + - "The repair both namers read an answer through (`cleanTitle` in `internal/session/title.go`) covered markup, quotes, announcements, instruction echoes and empty-subject answers, and a first-person preamble was none of those. No longer true: a first-person subject followed by an auxiliary — `I'll start by`, `We'll begin`, `Let me first`, `I am going to` — is refused before `firstWordsOf` cuts to three words, so the cut can never manufacture a label out of the front of a sentence. Both halves are required, so `we chat cutover` is still a name." + - "Throat-clearing a model puts in front of an answer was the agreement it opened on — `Sure,`, `Okay!`. It is now also the sequencing adverb a plan narrates itself with, so `First, I will write the four pages` and `Okay so I'll look at the brief` have their opener taken off by the same `stripInterjection` every namer already shared, and `First, the bakery pages` is minted as `the bakery pages`." +--- + +The namer is the cheapest call in the session, asked for two or three lowercase +words, and the prompt was what kept it to that — until a run pinned every tier +and role to a reasoning model and the namer answered its plan instead: 264 +completion tokens of "I'll start by creating the four bakery landing pages one +at a time." Nothing in the path refused a sentence. `firstWordsOf` cut it to +three words, `taskNameNeeded` asked only whether an answer was empty, an echo, a +path or over three words, and the task was titled `I'll start by` on the rail, on +the task page and in its own record — while `spec.title`, the person's own words, +sat there as the fallback the truncated preamble had overwritten. + +The refusal is grammar rather than a list of sentences, and it lives where both +namers already read an answer: a first-person subject followed by the word that +makes it the subject of something about to happen. A refusal is the empty string, +which is a road that already worked — the task namer leaves the row's title +exactly where it was, and that title is the person's own sentence clipped by the +column. A noun phrase that merely ran long is still cut to three words and is +still a name, which is the whole difficulty of the fix: `launch post for existing +users` was a label, and `I'll start by creating the four bakery landing pages` was +never one at any length. diff --git a/internal/manual/chat/tasks.md b/internal/manual/chat/tasks.md index 315234fae4..c9dcb4514a 100644 --- a/internal/manual/chat/tasks.md +++ b/internal/manual/chat/tasks.md @@ -462,7 +462,10 @@ field in an answer aforge was already paying for and already waiting on. all — only the sentence it was started from: a `/task` whose shaping could not run, work that started on its own after a words-only turn, an adaptive run's own row. That title is handed to the cheap `taskname` role, which reads the work and answers with two or three lowercase words. -Empty replies, instruction echoes and placeholders such as `nothing to name` are refused. +Empty replies, instruction echoes and placeholders such as `nothing to name` are refused. An answer +that is a sentence about what the model is about to do — `I'll start by …`, `Let me first …`, +`First, I will …` — is refused too: a name is a label for the work, not the front of a plan, so +the row keeps the fallback instead of being titled after somebody's opening words. The next configured naming model may answer within the same time limit; if it cannot, the existing fallback stays. A previously saved placeholder such as `nothing to name` is named again from its saved brief when you reopen the conversation; the work itself diff --git a/internal/session/taskname_test.go b/internal/session/taskname_test.go index 7bf5e5572f..2da668b725 100644 --- a/internal/session/taskname_test.go +++ b/internal/session/taskname_test.go @@ -87,6 +87,27 @@ func TestCleanTaskNameCutsToThreeWordsAndRefusesWhatIsNotAName(t *testing.T) { {"", ""}, {" ", ""}, {"/var/folders/j7/59f75dnd", ""}, + // THE THREE ANSWERS OBSERVED IN #942, and the shapes beside them. A + // reasoning namer answered its plan instead of a label and the reader took + // the first three words of it. A clause about the speaker is refused BY ITS + // SHAPE AND AT ANY LENGTH — the refusal runs before the cut to three words, + // so the cut can never manufacture a label out of the front of a sentence. + {"I'll start by creating the four bakery landing pages one at a time.", ""}, + {"Let me first look at the brief.", ""}, + {"First, I will write the four pages.", ""}, + {"I will write the four pages.", ""}, + {"We'll begin with the first page.", ""}, + {"Okay so I'll look at the brief.", ""}, + // AND A NAME THAT MERELY OPENS ON ONE OF THOSE WORDS IS NOT TOUCHED, which + // is what the second half of the test buys: the pronoun has to be the + // subject of something about to happen before the answer stops being a + // name. The four-word row above it is the other control — that answer was + // a label that ran long, so it is cut and kept. + {"we chat cutover", "we chat cutover"}, + // AND A SEQUENCING ADVERB THAT IS PART OF A NAME KEEPS ITS PLACE. It is + // throat-clearing only where it stands in front of an answer, punctuated + // off or carried by "so", which is [stripInterjection]'s whole test. + {"first pass on the brief", "first pass on"}, } { if got := cleanTaskName(c.raw); got != c.want { t.Errorf("cleanTaskName(%q) = %q, want %q", c.raw, got, c.want) @@ -96,49 +117,94 @@ func TestCleanTaskNameCutsToThreeWordsAndRefusesWhatIsNotAName(t *testing.T) { // THE WHOLE FEATURE, END TO END: a task admitted under a sentence is renamed by // one cheap call, the new name is what every surface is told, and it is in the -// checkpoint so a restart keeps it. +// checkpoint so a restart keeps it. THE SECOND ROW IS #942'S WHOLE DOOR: a +// namer that answered its plan instead of a label — 264 completion tokens of it, +// measured in the field — is refused, and the row keeps the person's own words, +// which is the title it was already drawn under. func TestWorkAdmittedUnderASentenceIsNamedByTheCheapModel(t *testing.T) { - client := &scriptedCompleter{steps: []step{func(_ context.Context, messages []ai.Message) (*ai.Response, error) { - if !isNameCall(messages) { - t.Errorf("the first call was not the namer's: %q", messageContentText(messages[0])) - } - return textResponse("parser recon"), nil - }}} - agent, _ := newTestAgent(t, client, func(c *Config) { c.RolesSource = nameSettings() }) - ran := make(chan uint64, 4) - graph := stubbedGraph(agent, func(node *TaskNode) { ran <- node.id }) - - updates := agent.TaskUpdates() - id := graph.reserve() - graph.admit(id, taskSpec{ - title: "read /Users/me/src and say what the parser does", - summary: "read the parser", - brief: "Read the parser under /Users/me/src and write down what it does.", - acceptance: "a note saying what the parser does", - }) - select { - case <-ran: - case <-time.After(5 * time.Second): - t.Fatal("the admitted node never ran") - } + const sentence = "read /Users/me/src and say what the parser does" + for _, c := range []struct { + name string + answer string + want string + renames bool + }{ + {"a namer that answers with a label", "parser recon", "parser recon", true}, + // THE OBSERVED ANSWER OF #942. A refusal is the empty string, and that is + // a road which already works: the caller's only response is to leave the + // title where it was, and where it was is the person's own words. Nothing + // is published, so no surface is ever told "I'll start by". + {"a namer that answers its plan", "I'll start by creating the four bakery landing pages one at a time.", sentence, false}, + } { + t.Run(c.name, func(t *testing.T) { + // EVERY RUNG THE LADDER HAS IS SCRIPTED, and the refusing row is why. + // An answer the reader refuses is not an answer, so the ladder asks the + // rung below it ([Agent.callRoleChecked]'s accept check) — a script one + // step long would be answered "(unscripted)" on that second ask and the + // fixture would be measuring the fixture. Both rungs say the same thing, + // which is what the model in the field did. + answer := func(_ context.Context, messages []ai.Message) (*ai.Response, error) { + if !isNameCall(messages) { + t.Errorf("a call that was not the namer's: %q", messageContentText(messages[0])) + } + return textResponse(c.answer), nil + } + steps := make([]step, roleFallThroughs+1) + for i := range steps { + steps[i] = answer + } + client := &scriptedCompleter{steps: steps} + agent, _ := newTestAgent(t, client, func(cfg *Config) { cfg.RolesSource = nameSettings() }) + ran := make(chan uint64, 4) + graph := stubbedGraph(agent, func(node *TaskNode) { ran <- node.id }) - // THE NAME LANDS ON THE NODE, and it is the title from then on. - if !nameLanded(func() bool { return graph.node(id).title() == "parser recon" }) { - t.Fatalf("the node is still called %q", graph.node(id).title()) - } - // AND THE WORLD IS TOLD. A row already on screen under the sentence learns - // the name from an ordinary update. - if !waitForNamedRow(updates, id, "parser recon") { - t.Fatal("no update carried the new name") - } - // AND IT IS KEPT. The checkpoint is what a resumed session reads its roster - // back out of, so a name that only lived in this process would be a row that - // went back to its sentence after a restart. - if got := graph.document().Nodes[0].Title; got != "parser recon" { - t.Fatalf("the checkpoint says %q", got) - } - if got := client.model(0); got != "cheap/model" { - t.Fatalf("the namer ran on %q, want the cheap tier", got) + updates := agent.TaskUpdates() + id := graph.reserve() + graph.admit(id, taskSpec{ + title: sentence, + summary: "read the parser", + brief: "Read the parser under /Users/me/src and write down what it does.", + acceptance: "a note saying what the parser does", + }) + select { + case <-ran: + case <-time.After(5 * time.Second): + t.Fatal("the admitted node never ran") + } + // THE NAMER REALLY WAS ASKED, in both rows: a title the call never + // touched reads the same as one a refusal left alone, and only one of + // those is this feature working. + if !nameLanded(func() bool { return client.requests() > 0 }) { + t.Fatal("the namer was never asked") + } + // THE NAME LANDS ON THE NODE, and it is the title from then on. In the + // refused row this is the row it never left. + if !nameLanded(func() bool { return graph.node(id).title() == c.want }) { + t.Fatalf("the node is called %q, want %q", graph.node(id).title(), c.want) + } + // AND THE WORLD IS TOLD. A row already on screen under the sentence + // learns the name from an ordinary update — and a refusal publishes + // nothing at all, which is the other half of the claim: no surface is + // ever handed the front of a sentence. + if c.renames && !waitForNamedRow(updates, id, c.want) { + t.Fatal("no update carried the new name") + } + // AND IT IS KEPT. The checkpoint is what a resumed session reads its + // roster back out of, so a name that only lived in this process would + // be a row that went back to its sentence after a restart. + if got := graph.document().Nodes[0].Title; got != c.want { + t.Fatalf("the checkpoint says %q, want %q", got, c.want) + } + if got := client.model(0); got != "cheap/model" { + t.Fatalf("the namer ran on %q, want the cheap tier", got) + } + // AND A REFUSAL NEEDS NO WINDOW WAITED OUT. [TaskGraph.rename] is only + // ever reached with a name that is not the empty string, so once every + // answer this fixture can give is one the reader refuses, there is no + // later moment at which the row could still move. The poll above is + // what proves the call was made at all, which is the half a sleep + // could not have proved either way. + }) } } diff --git a/internal/session/title.go b/internal/session/title.go index 08b978bac6..dea6585c09 100644 --- a/internal/session/title.go +++ b/internal/session/title.go @@ -583,10 +583,14 @@ func compactTitle(raw string) string { // cleanTitle takes the first line and strips the things a model adds against // the instruction: the throat-clearing it opens with ("Title:", "Sure, here is -// the name:"), the MARKDOWN it emphasises with, surrounding quotes, a trailing +// the name:"), the MARKUP it emphasises with, surrounding quotes, a trailing // full stop, and the separators of a name answered as a SLUG. Then it REFUSES an -// answer that is not a name at all — the instruction handed back, or an opener -// with nothing behind it. +// answer that is not a name at all — the instruction handed back, an opener with +// nothing behind it, or A CLAUSE ABOUT THE SPEAKER RATHER THAN A NAME FOR THE +// WORK ([opensAsPlan]). That last refusal is what keeps a namer that answered +// its plan from being shortened into a label: it runs HERE, before any caller +// cuts the answer to a few words, because the front of "I'll start by creating +// the four bakery landing pages" is not a name at any length. // // The slug is the one worth explaining. The instruction asks for words, and a // model that has spent its life reading identifiers sometimes answers @@ -621,7 +625,7 @@ func cleanTitle(raw string) string { title = strings.Join(strings.Fields(title), " ") } title = clip(strings.TrimSpace(title), titleLimit) - if namesTheInstruction(title) || unusableName(title) { + if namesTheInstruction(title) || unusableName(title) || opensAsPlan(title) { return "" } return title @@ -748,6 +752,21 @@ func namesTheInstruction(name string) bool { return false } +// isOpener reads the words in front of a colon and says whether they are an +// announcement. Either end decides it: an interjection at the front ("sure, +// here is the name"), or one of the words a label ends on at the back ("title", +// "session name", "the session is about", "the title is"). +func isOpener(words []string) bool { + if isInterjection(words[0]) { + return true + } + switch words[len(words)-1] { + case "title", "name", "is", "about", "called", "answer", "caption", "full", "tab": + return true + } + return false +} + // openerLimit and openerWords bound what may be read as throat-clearing. A // label is short and stands at the very front; anything longer is a sentence // the model meant, and cutting at a colon inside one would take half a name @@ -800,7 +819,16 @@ func stripOpener(title string) string { func stripInterjection(title string) string { if cut := strings.IndexAny(title, ",!"); cut > 0 && cut <= openerLimit { if words := normalizedWords(title[:cut]); len(words) == 1 && isInterjection(words[0]) { - title = strings.TrimSpace(title[cut+1:]) + return strings.TrimSpace(title[cut+1:]) + } + } + // AND IT NEEDS NO PUNCTUATION WHEN "SO" IS CARRYING IT. A model writing the + // way somebody talks opens "okay so the bakery pages" with no comma at all, + // and `so` after an agreement is that comma spelled as a word. Something has + // to be left behind it, or this would strip an answer down to nothing. + if fields := strings.Fields(title); len(fields) > 2 && strings.EqualFold(fields[1], "so") { + if words := normalizedWords(fields[0]); len(words) == 1 && isInterjection(words[0]) { + return strings.Join(fields[2:], " ") } } return title @@ -830,26 +858,80 @@ func allOpenerWords(words []string) bool { return true } -// isOpener reads the words in front of a colon and says whether they are an -// announcement. Either end decides it: an interjection at the front ("sure, -// here is the name"), or one of the words a label ends on at the back ("title", -// "session name", "the session is about", "the title is"). -func isOpener(words []string) bool { - if isInterjection(words[0]) { - return true - } - switch words[len(words)-1] { - case "title", "name", "is", "about", "called", "answer", "caption", "full", "tab": - return true - } - return false +// ── an answer that was never a name ───────────────────────────────────────── +// +// A REASONING MODEL NARRATES ITS PLAN BEFORE IT FOLLOWS ONE, and a call asking +// for a name does not stop it doing that. Measured in the field (#942), with +// every tier and role pinned to a reasoning model, the namer answered "I'll +// start by creating the four bakery landing pages one at a time." — 264 +// completion tokens about what it was about to do. The reader took the first +// three words of it and a task stood on the rail called `I'll start by`, while +// the node's own record still carried the person's words right beside the name +// it had been given. +// +// THE CUT IS WHAT MADE A SENTENCE LOOK LIKE A LABEL. Nothing refused an answer +// for being a sentence; it was shortened until it was the right length to pass +// for a name. So an answer is refused BY ITS SHAPE AND AT ANY LENGTH: a name is +// a noun phrase, and a clause about the speaker is not one. A noun phrase that +// merely ran long ("launch post for existing users") is still cut to three +// words and is still a name. +// +// THE TEST IS THE GRAMMAR AND NOT A LIST OF SENTENCES: a first-person subject, +// followed by the word that makes it the subject of something about to happen — +// an auxiliary, the remnant the fold in [normalizedWords] leaves of one written +// as a contraction, or the person `let` is addressed about. BOTH HALVES ARE +// REQUIRED, which is what keeps a real name that opens on one of those words +// ("I/O error fix") out of it. +// +// The throat-clearing a plan opens on — "First, I will …", "Okay so I'll …" — +// is NOT a second rule here. It is an opener, and [stripInterjection] is the one +// place this file takes an opener off an answer, so it is taken off there and +// this reads what is left. + +// planSubjects is who a plan is about. The fold in [normalizedWords] reads every +// mark as a space, so a contraction arrives as its stem and a remnant ("I'll" is +// "i ll", "let's" is "let s") and the pronoun itself is the only spelling that +// has to be written down. It sits beside [openerVocabulary] because the two are +// the same kind of thing: the words a model reaches for when it is not +// answering the question it was asked. +var planSubjects = map[string]bool{"i": true, "we": true, "let": true, "lets": true} + +// planPredicates is the word that follows that subject when the sentence is +// about what the speaker is about to do: the auxiliary or modal a finite clause +// hangs on, the remnant a contraction leaves of one, or the person `let` is +// addressed about. IT IS A CLOSED CLASS OF GRAMMAR AND NOT A LIST OF OBSERVED +// SENTENCES, which is why it is this short and why a preamble nobody has met yet +// is already refused. +var planPredicates = map[string]bool{ + "ll": true, "m": true, "ve": true, "d": true, "re": true, "s": true, + "will": true, "would": true, "shall": true, "should": true, "can": true, + "could": true, "may": true, "might": true, "must": true, "am": true, + "is": true, "are": true, "was": true, "were": true, "have": true, + "has": true, "had": true, "do": true, "does": true, "did": true, + "need": true, "going": true, "me": true, "us": true, +} + +// opensAsPlan reports whether an answer opens as a clause about the speaker +// rather than as a name for the work. It runs on the REPAIRED answer and before +// any caller cuts it, because the shape it refuses is the FRONT of a sentence: +// read after the cut to three words it would be handed "I'll start by", which +// is the same three words a label would have been. +func opensAsPlan(title string) bool { + words := normalizedWords(title) + return len(words) > 1 && planSubjects[words[0]] && planPredicates[words[1]] } -// isInterjection is the handful of words a model agrees with the request in -// before it answers it. +// isInterjection is the handful of words a model clears its throat with before +// it answers: the agreement it opens on, and the SEQUENCING ADVERB a plan +// narrates itself with ("First, I will write the four pages"). They are one list +// because they are one thing to a reader — a word about the answering rather +// than about the work — and because the plan a reasoning namer opens on (#942) +// has to have its opener taken off it HERE, before [opensAsPlan] reads what is +// left, or the pronoun that decides it is never the first word. func isInterjection(word string) bool { switch word { - case "sure", "ok", "okay", "certainly", "absolutely", "here", "heres": + case "sure", "ok", "okay", "certainly", "absolutely", "here", "heres", + "first", "next", "then", "now", "alright", "so": return true } return false diff --git a/internal/session/titleecho_test.go b/internal/session/titleecho_test.go index 5b12113c4d..97e611b4d3 100644 --- a/internal/session/titleecho_test.go +++ b/internal/session/titleecho_test.go @@ -71,6 +71,51 @@ func TestTheOpenerIsStrippedAndAnEmptyOneRefused(t *testing.T) { } } +// A NAMER THAT ANSWERED ITS PLAN HAS NOT NAMED ANYTHING EITHER, and #942 is +// where that was measured: a reasoning model on the task namer's call wrote a +// paragraph about what it was about to do, and the reader cut the first three +// words of it into a task called `I'll start by`. A clause about the speaker is +// refused at any length, in the ONE HAND BOTH NAMERS READ THROUGH — so the +// session's name and a task's name cannot drift apart on what a name is. +func TestANameThatIsAPlanIsRefusedByBothNamers(t *testing.T) { + for _, said := range []string{ + // The three observed in the field. + "I'll start by creating the four bakery landing pages one at a time.", + "Let me first look at the brief.", + "First, I will write the four pages.", + // And the same shape in the spellings around them: the opener taken off + // by punctuation or by "so", the plural speaker, the bare auxiliary. + "Okay so I'll look at the brief.", + "We'll begin with the first page.", + "I am going to write the four pages.", + "Then, we can start on the pages.", + } { + if got := cleanTitle(said); got != "" { + t.Errorf("cleanTitle(%q) = %q, want a plan refused", said, got) + } + if got := cleanTaskName(said); got != "" { + t.Errorf("cleanTaskName(%q) = %q, want a plan refused", said, got) + } + if got := cleanConversationTitle(said); got.full != "" || got.short != "" { + t.Errorf("cleanConversationTitle(%q) = %+v, want a plan refused", said, got) + } + } + // AND THE CONTROLS, which are the whole difficulty: a label that merely ran + // long is a label, and a name that opens on one of those words is a name. The + // refusal is the grammar of the opening and never the length of the answer. + for _, row := range []struct{ said, want string }{ + {"launch post for existing users", "launch post for existing users"}, + {"nil-map crash fix", "nil-map crash fix"}, + {"we chat cutover", "we chat cutover"}, + {"first pass on the brief", "first pass on the brief"}, + {"let go of the lock", "let go of the lock"}, + } { + if got := cleanTitle(row.said); got != row.want { + t.Errorf("cleanTitle(%q) = %q, want %q", row.said, got, row.want) + } + } +} + // THE INSTRUCTION IS ASKED WHERE A SMALL MODEL READS IT: last in the user // message, after the exchange, with the system line saying only who is asked. func TestTheNamerAsksAtTheEndOfTheUserMessage(t *testing.T) {