diff --git a/docs/changes/unreleased/1067-no-change-keeps-the-answer.md b/docs/changes/unreleased/1067-no-change-keeps-the-answer.md new file mode 100644 index 0000000000..cfef3e7958 --- /dev/null +++ b/docs/changes/unreleased/1067-no-change-keeps-the-answer.md @@ -0,0 +1,18 @@ +--- +kind: changed +title: a wrong completion check is answered with [no change], and the answer before it stays in view +pr: 1067 +surface: [chat, engine, docs] +invalidates: + - "The end-of-turn reader was shown the first 600 bytes of the turn's last message (`checkpointSaidBytes = checkpointSketchBytes`), cut with a bare `…`. It is shown up to 8 KB now (`checkpointSaidBytes = 8 * 1024`), less when the ask fills the digest's head, and a cut it still has to make ends with `[clipped by codeaf: N of M bytes]` (`checkpointClipSaid`, which takes the room left). The remains ask (`checkpointRemainsAsk`) now says clipped text is never evidence the person saw a cut-off answer." + - "`checkpointCarryOnLead` told a model that found the note mistaken to `explain the evidence briefly, and finish`. It now says codeaf's completion check wrote the note, the person does not see it, and a wrong note is answered with exactly `[no change]` (`session.NoChangeReply`). The old lead is kept as `explainingCheckpointCarryOnLead` only so old journals still hide it." + - "A carry-on the model answered was always read again until `checkpointCarryOnCap` (3) or the standstill rule stopped it. A carry-on answered with `[no change]` now ends the turn at once, with no further reader call. It is honoured only as the very next reply to the reader's note (`answeredNoteUnchanged` reads the transcript): after tool calls, after a person's steer, after a load or ask nudge, or with other words beside it, the turn is read as before. A turn ending on the token neither starts the route judge nor hands the token to the memory reflex, which reads the answer before it." + - "Every assistant reply was a row a person could read back, and the fold's answer was the last one. A tool-less reply that is only `[no change]` is now left out of replay (`entryRows` answers 0) and emptied in the live feed at its response boundary (`feed.confirmResponse`), so the settled answer before it is what `deriveWorkfolds` leaves standing. `bin/codeaf chat --once` prints deltas as they stream and can still print the token." + - "`internal/session/prompts/system.md` had `# Tone` and `# Delivery`. It has `# The answer`, `# Answer or change`, `# When corrected` and `# Messages from codeaf` instead, all four kept on the lean page, and the rules the old two carried — match the person's task and vocabulary, choose the safe option under uncertainty, evidence and blocking details complete, never a compiling scaffold or a narrowed test, never half-solved work — live inside them. `# The answer` says that once a turn ends only its last message stays in view (a plan and the per-batch sentence are still shown while it runs), and `# Answer or change` says an answer asked for in chat is written in the reply, not a file, unless the person asks for a file (`Messages from codeaf` is an explicit `keeps: true` row). That section names the user-role tags codeaf writes to the conversation — `[carry on]`, `[taking stock]`, `[silent]`, `[stuck]`, `[folded …]`, `[context compacted]` — while `[image #N]` marks the person's attachment; it does not list the private reader's `[still asked]`, the tool result's `[held]`, or standing news's `[something you set up fired]`, whose line carries its own instruction under the event and is not repeated on the page (`TestTheSteeringLineReadsAsNewsAndNotAsARequest`). `TestMessagesFromCodeafNamesEveryUserRoleOpening` pins the list from the production constants. `for reversible work act and offer to unwind it` now ends `when the person asked for a change`, and `NEVER yield while actionable work remains` is `Don't end the turn while work the person asked for remains`." + - "The prefix byte waivers were `fixedPrefixWaiver = 5_291` and `leanPrefixWaiver = 13_566`. After the new sections and the provenance-list correction they are 7_280 and 15_555: fixed measures 55,280 bytes and lean 47,055, 2,004 bytes over `origin/dev` on both arms, and both waivers sit exactly on the measurement. The page's overall length is round two of #1065." +--- + +Round one of #1065. A person asked for a per-seat model table; the reader, +shown 600 bytes of it, said three times that it was cut off; the model +reprinted it twice and then argued, and the argument was the only answer left +standing under the `worked` fold. diff --git a/internal/manual/chat/starting-codeaf.md b/internal/manual/chat/starting-codeaf.md index af67c6aa99..3757525fcf 100644 --- a/internal/manual/chat/starting-codeaf.md +++ b/internal/manual/chat/starting-codeaf.md @@ -504,6 +504,36 @@ correctly reported that a file did not exist, and the reader claimed three times running that the missing file had not been reported: the reply spent three turns explaining that the observation was mistaken, and all three were on the screen. +## It said my answer was cut off when it was not · it argued with the check instead of answering · my table disappeared under worked · [no change] + +When a reply ends, codeaf's completion check can read a summary of the turn and send the +model a note that starts with `[carry on]`. You never see that note, and you did not +write it. The model compares it with your request and its own work, then does one of two +things: + +- **The note found a real gap.** The model closes the gap and ends with a complete final + answer. Only that last message stays in view when the turn folds under its `worked` + line, so it has to carry the whole deliverable again. +- **The note is wrong.** The model replies with exactly `[no change]` and nothing else. + That ends the reply. The check is not asked again, the same note is not sent again, + and the answer it gave before the note stays in view, in the chat and when the + conversation is opened again. It counts only as the model's very next reply to the + note. After tool calls, after your own message typed in between, or with any other + words beside it, the reply is checked again as usual. A reply that is only + `[no change]` is never drawn, whether or not it counted. + Headless `--once` output is printed as it streams, so if a check runs there the token + can appear in it. + +The summary shows the check up to 8 KB of the reply's last message, less when your request +itself fills most of the summary. A longer message is +cut to fit and ends with `[clipped by codeaf: N of M bytes]`, and the check is told that +this cut belongs to the summary and is never a sign that you saw a cut-off answer. A +table or report under that size reaches the check whole. + +Before this, the check saw only the first 600 bytes and the model was told to explain +itself when the note was wrong. A long table read as "cut off", the note was sent three +times, and the model's explanation ended up as the only visible answer. + ## A task waiting on one that did not finish · work that will never start · it says something is still running **Work that is still going is never a standstill, and never finished either.** A diff --git a/internal/session/agent.go b/internal/session/agent.go index 7e52b3d4a5..8116f4b0e2 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -4177,6 +4177,15 @@ const legacyCheckpointCarryOnLead = "[carry on] You stopped, but what was asked "Somebody reading the work against the request says this is what is left. " + "Carry on with it, and do not summarise what you have already done:\n" +// explainingCheckpointCarryOnLead is the continuation prefix journals wrote +// before [NoChangeReply] existed, when a model that found the note mistaken was +// told to explain itself (#1065). Its complete prefix is matched for the same +// reason as [legacyCheckpointCarryOnLead]'s. +const explainingCheckpointCarryOnLead = "[carry on] A reader of a bounded account of the work raised the observation below. " + + "Check it against the actual current work and the person's request before changing anything. " + + "Fix any confirmed gap. If the observation is mistaken or already satisfied, preserve the correct work, " + + "explain the evidence briefly, and finish; do not invent a change to satisfy the observation.\n" + // entryRows is how many rows the shaping above makes of ONE message: none at // all for system messages and private continuation context, and otherwise the // message's own row plus one for every tool call riding it. It is the rule the loop appends by, written @@ -4197,10 +4206,17 @@ func entryRows(msg ai.Message) int { // The continuation's full reserved lead identifies older journals too: // they wrote it as an unmarked user message even though nobody typed it. // Keep the model's record intact; only its display projection omits it. - if isVolatileNote(text) || strings.HasPrefix(text, checkpointCarryOnLead) || strings.HasPrefix(text, legacyCheckpointCarryOnLead) { + if isVolatileNote(text) || strings.HasPrefix(text, checkpointCarryOnLead) || + strings.HasPrefix(text, explainingCheckpointCarryOnLead) || strings.HasPrefix(text, legacyCheckpointCarryOnLead) { return 0 } } + // A CARRY-ON ANSWERED [NoChangeReply] WITHDREW ITSELF AS THE ANSWER. The model + // needs the token in its record; a person reading the history needs the + // settled answer before it, which is what a missing row leaves standing. + if msg.Role == "assistant" && len(msg.ToolCalls) == 0 && IsNoChangeReply(messageContentText(msg)) { + return 0 + } return 1 + len(msg.ToolCalls) } diff --git a/internal/session/carryonwait_test.go b/internal/session/carryonwait_test.go index dfe67fe452..409abb4c3f 100644 --- a/internal/session/carryonwait_test.go +++ b/internal/session/carryonwait_test.go @@ -257,8 +257,9 @@ func TestCarryingOnAnAskHasACeilingOfItsOwn(t *testing.T) { // the same piece said twice. // // The model's continuation answer is scripted as a rebuttal rather than a -// change, which is what a model does with an observation that is already -// satisfied: the lead itself tells it to explain and finish. +// change, which is what a model did with an observation that was already +// satisfied while the lead told it to explain and finish — and what a model +// that ignores the lead's [NoChangeReply] still does. func TestAReaderObservationThatWasAnsweredIsNotRaisedAgain(t *testing.T) { const answered = "there is no zeta.txt, so nothing was written about it; the check ran and reported the miss" const observation = "the missing zeta.txt has not been reported" diff --git a/internal/session/checkpoint.go b/internal/session/checkpoint.go index 65feb75f5b..88061efc3a 100644 --- a/internal/session/checkpoint.go +++ b/internal/session/checkpoint.go @@ -423,12 +423,54 @@ const ( // checkpointSaidBytes is how much of the turn's last words the reader is // shown. It is the one part of the digest that is the model's own account of - // where it has got to, and a paragraph of it is the whole of what a reader - // needs to place the ledger above it — the same bound a sketch's own halves - // are held to, for the same reason. - checkpointSaidBytes = checkpointSketchBytes + // where it has got to — and, on a turn that answered in words, it IS the + // deliverable. + // + // IT WAS A SKETCH'S SIX HUNDRED BYTES, AND THAT WAS MEASURED WRONG (#1065). A + // person asked for a per-seat model table, the model wrote it, and the reader + // — shown its first six hundred bytes — said three times running that the + // table was cut off. The model reprinted it twice and then argued, and the + // argument was the only thing left standing under the fold. Eight kilobytes + // holds any table or report a turn ends on and is still under half of + // [checkpointDigestBytes], so the ledger keeps its room. + checkpointSaidBytes = 8 * 1024 ) +// checkpointClippedMark is the line that ends last words the digest had to +// shorten. A bare `…` was the old marker and a reader took it for the answer +// itself breaking off; this one says in words that the cut is the SUMMARY'S, who +// made it, and how much was kept. +const checkpointClippedMark = "[clipped by codeaf: %d of %d bytes]" + +// checkpointClipSaid fits the turn's last words into room bytes — never more +// than [checkpointSaidBytes] — marker included, and names the cut with +// [checkpointClippedMark] when it makes one. Words that already fit are returned +// untouched, and room too small to hold even the mark keeps nothing rather than +// a mark with no words in front of it. +// +// THE ROOM IS THE CALLER'S BECAUSE THE ASK SHARES THE PAGE. An ask up to the +// whole digest bound rides in the digest's head ([checkpointCompletionPage]), +// and eight kilobytes of last words behind a twelve-kilobyte ask would reach +// the digest's backstop clip — which cuts from the end with a bare `…`, the very +// marker this function exists to replace. +func checkpointClipSaid(said string, room int) string { + room = min(room, checkpointSaidBytes) + if len(said) <= room { + return said + } + // The room for the mark is measured at its WIDEST — the kept count can never + // exceed the room — so saying how much was kept cannot push past it. + widest := "\n" + fmt.Sprintf(checkpointClippedMark, max(room, 0), len(said)) + cut := room - len(widest) + if cut <= 0 { + return "" + } + for cut > 0 && !utf8RuneStart(said[cut]) { + cut-- + } + return said[:cut] + "\n" + fmt.Sprintf(checkpointClippedMark, cut, len(said)) +} + // The digest's headings. They are SHOUTED and they are few, because what they // have to do is let a reader tell four kinds of evidence apart at a glance in // one message that also carries the ask. @@ -694,6 +736,7 @@ const checkpointRemainsAsk = "[still asked] Above is what the person asked for a "A successful write confirms only that bytes were written; it does not prove their contents meet the request. " + "Tool failures and exact data mismatches outweigh an assistant claim of success. " + "Abbreviated or omitted content is unknown, not evidence of a defect. " + + "Text marked as clipped was shortened for this summary only; it is never evidence that the person saw a cut-off answer. " + "Ground any claimed defect in the evidence shown; if a necessary check is missing, name that check instead of inventing its result. " + "In one line, say what of the ASK is still not done. " + "If everything they asked for is done, answer with the single line " + checkpointNothingLeft + @@ -909,10 +952,53 @@ func partsText(message ai.Message) string { return text.String() } -const checkpointCarryOnLead = "[carry on] A reader of a bounded account of the work raised the observation below. " + - "Check it against the actual current work and the person's request before changing anything. " + - "Fix any confirmed gap. If the observation is mistaken or already satisfied, preserve the correct work, " + - "explain the evidence briefly, and finish; do not invent a change to satisfy the observation.\n" +const checkpointCarryOnLead = "[carry on] codeaf's completion check read a clipped summary of this turn and raised the note below.\n" + + "The person did not write it and does not see it. Compare it with the person's request and your work.\n" + + "If there is a real gap, close it and end with a complete final answer.\n" + + "If the note is wrong, reply with exactly " + NoChangeReply + " and nothing else.\n" + +// answeredNoteUnchanged reads the transcript for the one shape [NoChangeReply] +// is an answer to: the newest message is a tool-less reply that is only the +// token, and the message it answers is the reader's continuation +// ([checkpointCarryOnLead]). A volatile note that landed between the two is the +// harness's own context and is walked through; anything else is not. +func answeredNoteUnchanged(messages []ai.Message) bool { + last := len(messages) - 1 + if last < 1 || messages[last].Role != "assistant" || len(messages[last].ToolCalls) > 0 || + !IsNoChangeReply(partsText(messages[last])) { + return false + } + for index := last - 1; index >= 0; index-- { + text := partsText(messages[index]) + if messages[index].Role == "user" && isVolatileNote(text) { + continue + } + return messages[index].Role == "user" && strings.HasPrefix(text, checkpointCarryOnLead) + } + return false +} + +// NoChangeReply is the whole of the model's answer to a carry-on it judges +// mistaken, and the harness's token for "the answer already given stands". +// +// IT REPLACED "EXPLAIN THE EVIDENCE BRIEFLY, AND FINISH" (#1065). An explanation +// is words at the end of a turn, and words at the end of a turn are what a +// surface folds the turn down to — so a model that rightly refused a false +// "the table was cut off" left the person a rebuttal where the table had been. +// A token is a thing the model chose to say rather than a phrase the harness +// thought it heard, for [checkpointNothingLeft]'s reason, and it ends two things +// at once: carrying on for this ask ([Agent.checkpointReopen]), and its own +// place as the answer — the transcript keeps it for the model, and every surface +// leaves it undrawn ([IsNoChangeReply]) so the settled answer before it stands. +const NoChangeReply = "[no change]" + +// IsNoChangeReply says whether a response's words are [NoChangeReply] and +// nothing else. It is the ONE reading of the token, used by the harness and by +// every surface that draws an answer, so the two can never disagree about which +// reply was withdrawn. +func IsNoChangeReply(text string) bool { + return strings.TrimSpace(text) == NoChangeReply +} // ── the meter ─────────────────────────────────────────────────────────────── @@ -2385,7 +2471,8 @@ func checkpointDigest(asked string, messages []ai.Message) string { tail.WriteString(line) tail.WriteString("\n\n") } - if said := clip(checkpointLastSaid(messages), checkpointSaidBytes); said != "" { + room := checkpointDigestBytes - head.Len() - tail.Len() - len(checkpointDigestSaid) - len("\n\n") + if said := checkpointClipSaid(checkpointLastSaid(messages), room); said != "" { tail.WriteString(checkpointDigestSaid) tail.WriteString("\n") tail.WriteString(said) @@ -2767,7 +2854,10 @@ func checkpointLastSaid(messages []ai.Message) string { for _, part := range messages[index].Content { said.WriteString(part.Text) } - if text := strings.TrimSpace(said.String()); text != "" { + // A WITHDRAWN REPLY IS NOT THE LAST THING SAID. [NoChangeReply] left the + // answer before it standing, and that answer is what a later reading is + // about. + if text := strings.TrimSpace(said.String()); text != "" && !IsNoChangeReply(text) { return text } } @@ -3198,6 +3288,21 @@ func (a *Agent) checkpointReopen(ctx context.Context, hub *eventHub, user userMe return false, false } said := response.Text() + // A CARRY-ON ANSWERED WITH [NoChangeReply] ENDS CARRYING ON, AND IT COSTS NO + // READER. The model was handed the reader's note, compared it with the ask and + // the work, and said the note was wrong — so the turn ends on the answer it + // already gave, and nothing below may spend a reading to re-raise what was + // just rejected. Ending the turn is what keeps the note from coming back: the + // meter that counts carry-ons dies with it. + // + // IT IS HONOURED ONLY AS THE VERY NEXT REPLY TO THE NOTE ([answeredNoteUnchanged]). + // A token after calls made in answer to the note would end the turn on an + // answer those calls may have made stale, unread; a token after the person's + // own words, or after a load or ask nudge, answers something that is not the + // reader's note at all. + if meter != nil && answeredNoteUnchanged(a.snapshot()) { + return false, false + } // A TURN THAT LOADED A TOOL AND STOPPED WITHOUT USING IT IS SENT BACK ONCE, // AND THIS COSTS NO READER. It stands ahead of every gate below because it is // not a reading of the work at all: it is the harness finishing something it diff --git a/internal/session/loop.go b/internal/session/loop.go index f7b375a63d..3a1690bee0 100644 --- a/internal/session/loop.go +++ b/internal/session/loop.go @@ -1143,7 +1143,19 @@ func (a *Agent) runTurn(ctx context.Context, hub *eventHub, user userMessage) bo // [Agent.applyRouteJudge] below is the only thing with an effect in it // (route_judge.go). A reading that starts work from inside its own // goroutine is this mechanism used in name and broken in fact. - judge := a.judgeAhead(ctx, user, usedTools, response.Text()) + // A REPLY THAT ANSWERS THE READER'S NOTE WITH [NoChangeReply] IS NO ANSWER + // TO JUDGE OR LEARN FROM. The transcript decides whether the token really + // answered that note; elsewhere it is an ordinary reply, even though every + // surface still leaves a reply containing only the token undrawn. + answer := response.Text() + withdrawn := IsNoChangeReply(answer) && answeredNoteUnchanged(a.snapshot()) + if withdrawn { + answer = checkpointLastSaid(a.snapshot()) + } + var judge *judgeRace + if !withdrawn { + judge = a.judgeAhead(ctx, user, usedTools, answer) + } a.tellPhase(provider.PhaseChecking, "whether the work is finished", time.Now()) again, over := a.checkpointReopen(ctx, hub, user, meter, &turn, started, model, response, marked) a.endPhase() @@ -1184,7 +1196,7 @@ func (a *Agent) runTurn(ctx context.Context, hub *eventHub, user userMessage) bo // goroutine entirely and on the session's own lifetime rather than // the turn's (memory.go). Nobody is waiting for it, nothing it finds // reaches this turn, and it says nothing whatever happens to it. - a.learnFromTurn(user.text(), response.Text()) + a.learnFromTurn(user.text(), answer) return true } diff --git a/internal/session/messagesfromcodeaf_test.go b/internal/session/messagesfromcodeaf_test.go new file mode 100644 index 0000000000..1c67466d3d --- /dev/null +++ b/internal/session/messagesfromcodeaf_test.go @@ -0,0 +1,100 @@ +package session + +import ( + "os" + "strings" + "testing" +) + +// THE PAGE NAMES EVERY USER-ROLE VOICE THE SESSION PUTS IN THE CONVERSATION. +// +// A bracket alone cannot say who wrote a message: a person's attachment wears +// one too. This list follows the production append sites by name so adding a +// new spelling, or moving a private reader/tool note onto the conversation, +// requires an explicit provenance decision in the page the model reads. +func TestMessagesFromCodeafNamesEveryUserRoleOpening(t *testing.T) { + page, err := os.ReadFile("prompts/system.md") + if err != nil { + t.Fatalf("read system page: %v", err) + } + section := topLevelPromptSection(t, string(page), "Messages from codeaf") + + openings := map[string]string{ + "checkpointCarryOnLead": checkpointCarryOnLead, + "checkpointChoiceLead": checkpointChoiceLead, + "foldMarkerPrefix": foldMarkerPrefix, + "legacyFramesNote": legacyFramesNote, + "silent loop note": "[silent]", + "stuck loop note": "[stuck]", + } + for source, text := range openings { + tag := bracketedOpening(t, source, text) + if !strings.Contains(section, tag) { + t.Errorf("# Messages from codeaf forgets %s's %s tag", source, tag) + } + } + + words := strings.Join(strings.Fields(section), " ") + for _, instruction := range []string{ + "Follow them.", + "Never answer as if the person wrote them", + "argue with them", + "mention them in your answer", + } { + if !strings.Contains(words, instruction) { + t.Errorf("# Messages from codeaf forgets the instruction %q", instruction) + } + } + if strings.Contains(strings.ToLower(words), "any other opening") { + t.Error("# Messages from codeaf assigns every unlisted opening to the person") + } + if !strings.Contains(words, "[image #N] marks the person's attachment") { + t.Error("# Messages from codeaf does not identify the person's attachment marker") + } + + // These voices never enter the running conversation as user messages. A + // tool result and a private reader instruction must not be taught as though + // they were turns the conversation model receives. + // + // AND A NOTE THAT CARRIES ITS OWN INSTRUCTION IS NOT REPEATED ON THE PAGE. + // Standing news opens with [standingNewsFrame] and says under the news + // itself what it is and what not to do about it, so the page's copy would + // be one law bought on every request of every turn for a turn most + // sessions never have — the WITH THE EVENT class of the prompt diet, which + // TestTheSteeringLineReadsAsNewsAndNotAsARequest pins from the other side. + for source, tag := range map[string]string{ + "process-rule tool result": "[held]", + "completion reader": "[still asked]", + "standing news": bracketedOpening(t, "standingNewsFrame", standingNewsFrame), + } { + if strings.Contains(section, tag) { + t.Errorf("# Messages from codeaf names %s's private %s tag", source, tag) + } + } +} + +func topLevelPromptSection(t *testing.T, page, heading string) string { + t.Helper() + lead := "# " + heading + "\n" + start := strings.Index(page, lead) + if start < 0 { + t.Fatalf("system page has no %q section", lead[:len(lead)-1]) + } + section := page[start+len(lead):] + if end := strings.Index(section, "\n# "); end >= 0 { + section = section[:end] + } + return section +} + +func bracketedOpening(t *testing.T, source, text string) string { + t.Helper() + if !strings.HasPrefix(text, "[") { + t.Fatalf("%s no longer opens with a bracketed tag: %q", source, text) + } + end := strings.IndexByte(text, ']') + if end < 0 { + return strings.TrimSpace(text) + " …]" + } + return text[:end+1] +} diff --git a/internal/session/nochange_test.go b/internal/session/nochange_test.go new file mode 100644 index 0000000000..3355b372eb --- /dev/null +++ b/internal/session/nochange_test.go @@ -0,0 +1,307 @@ +package session + +// A COMPLETION CHECK THAT WAS WRONG IS TOLD SO IN ONE TOKEN, AND THE ANSWER STANDS. +// +// THE MEASURED FAILURE (#1065): a person asked for a per-seat model table, the +// model wrote it, and the end-of-turn reader — shown the first six hundred bytes +// of it — said three times running that the table was cut off. The model +// reprinted it twice and then argued, and a finished turn folds to its last +// words, so the argument was the only thing the person was left reading. +// +// Three contracts close it and each is pinned here: the reader is shown a real +// answer whole and told in words when it is not; a carry-on the model answers +// with [NoChangeReply] ends carrying on for the ask; and that reply is never +// the answer a person reads back. + +import ( + "context" + "fmt" + "strings" + "sync/atomic" + "testing" + "unicode/utf8" + + "github.com/Agent-Field/agentfield/sdk/go/ai" +) + +// perSeatTable is an answer well past the six hundred bytes the reader used to +// be shown, in the shape the measured turn ended on. +func perSeatTable() string { + var table strings.Builder + table.WriteString("| seat | model | why |\n| --- | --- | --- |\n") + for seat := 1; seat <= 30; seat++ { + fmt.Fprintf(&table, "| seat %02d | deepseek/deepseek-v4.1-flash | cheap and fast for this seat's work |\n", seat) + } + return strings.TrimSpace(table.String()) +} + +// THE READER SEES A TABLE WHOLE, AND A CUT IT CANNOT AVOID SAYS WHOSE CUT IT IS. +func TestTheReaderSeesALongAnswerWholeAndACutNamesItself(t *testing.T) { + table := perSeatTable() + if len(table) <= 600 { + t.Fatalf("the fixture is %d bytes, which the old bound already showed whole", len(table)) + } + digest := checkpointDigest("a per-seat model table", []ai.Message{textMessage("assistant", table)}) + if !strings.Contains(digest, table) { + t.Fatalf("a %d-byte answer did not reach the reader whole:\n%s", len(table), digest) + } + if strings.Contains(digest, "[clipped by codeaf:") { + t.Fatalf("an answer that fit was marked clipped:\n%s", digest) + } + + // AN ASK THAT FILLS MOST OF THE PAGE LEAVES THE LAST WORDS LESS ROOM, AND THEY + // ARE STILL CUT BY NAME. The digest's backstop cuts from the end with a bare + // `…`, which is exactly the marker a reader took for a broken answer. + long := strings.Repeat("keep every seat's constraints in mind. ", 14_000/39) + report := strings.Repeat("| seat | model | why this seat |\n", 8_000/32) + page := checkpointCompletionPage(long, []ai.Message{textMessage("assistant", report)}) + if len(page) > checkpointDigestBytes { + t.Errorf("the page is %d bytes, over the %d digest bound", len(page), checkpointDigestBytes) + } + if !strings.Contains(page, strings.TrimSpace(long)) { + t.Error("the ask did not reach the reader whole") + } + if strings.HasSuffix(page, "…") || !strings.Contains(page, "[clipped by codeaf:") { + t.Errorf("last words squeezed by the ask were not cut by name; the page ends %q", page[len(page)-80:]) + } + + // EVERY BYTE OFFSET OF A TWO-BYTE RUN, so whichever parity the mark's width + // leaves the cut on, one of these lands it inside a rune. + for _, lead := range []string{"", "a"} { + huge := lead + strings.Repeat("é", checkpointSaidBytes) + said := checkpointClipSaid(huge, checkpointSaidBytes) + if len(said) > checkpointSaidBytes { + t.Errorf("clipped last words are %d bytes, over the %d bound", len(said), checkpointSaidBytes) + } + if !utf8.ValidString(said) { + t.Errorf("the cut split a character: %q", said[len(said)-60:]) + } + kept := strings.LastIndex(said, "\n[clipped by codeaf: ") + if kept < 0 { + t.Fatalf("a cut was not named in words: %q", said[len(said)-80:]) + } + if want := "\n" + fmt.Sprintf(checkpointClippedMark, kept, len(huge)); said[kept:] != want { + t.Errorf("the mark miscounts the cut: got %q, want %q", said[kept:], want) + } + if !strings.HasPrefix(huge, said[:kept]) { + t.Error("the clipped words are not the head of what was said") + } + } + // AND THE READER IS TOLD WHAT THE MARK MEANS, in the ask it is read with. + if !strings.Contains(checkpointRemainsAsk, "never evidence that the person saw a cut-off answer") { + t.Error("the remains ask no longer tells the reader a clipped summary is not a cut-off answer") + } +} + +// A CARRY-ON ANSWERED [no change] ENDS CARRYING ON, AND THE ANSWER BEFORE IT STANDS. +// +// The reader here says the same false thing every time it is asked, in words +// that change each time so the standstill rule is not what ends the turn. +func TestACarryOnAnsweredNoChangeEndsItAndKeepsTheAnswer(t *testing.T) { + table := perSeatTable() + var remainsAsks, carriedOn, rounds atomic.Int64 + steps := make([]step, checkpointMarkAt(1)+40) + for index := range steps { + steps[index] = func(_ context.Context, messages []ai.Message) (*ai.Response, error) { + if askedForRemains(messages) { + return textResponse(fmt.Sprintf("the table was cut off mid-row (look %d)", remainsAsks.Add(1))), nil + } + if last := messages[len(messages)-1]; last.Role == "user" && strings.HasPrefix(partsText(last), checkpointCarryOnLead) { + carriedOn.Add(1) + return textResponse(NoChangeReply), nil + } + if call := rounds.Add(1); call <= int64(checkpointMarkAt(1)) { + return toolResponseWithText(fmt.Sprintf("call-%d", call), "ls", + fmt.Sprintf(`{"path":"./%d"}`, call), ""), nil + } + return textResponse(table), nil + } + } + completer := &scriptedCompleter{steps: steps} + agent := checkpointAgent(t, completer, func(config *Config) { config.Memory = openTestBrain(t) }) + learned := observeMemoryExtraction(completer) + stubbedGraph(agent, func(node *TaskNode) {}) + + events, err := agent.Submit(context.Background(), "compare the models per seat in a table; do not change any files") + if err != nil { + t.Fatalf("Submit: %v", err) + } + notices := noticeTexts(collect(t, events)) + + if got := carriedOn.Load(); got != 1 { + t.Errorf("the model was carried on %d times, want once; notices were %q", got, notices) + } + if got := remainsAsks.Load(); got != 1 { + t.Errorf("the reader was spent %d times, want once: a rejected note is not read for again", got) + } + if got := strings.Count(transcriptText(agent), checkpointCarryOnLead); got != 1 { + t.Errorf("%d continuations were written into the turn, want one", got) + } + if got := saidHowOften(notices, checkpointCarryOnNote); got != 1 || len(notices) != 1 { + t.Errorf("want exactly the one carry-on notice and nothing after it; notices were %q", notices) + } + + // THE MODEL KEEPS ITS TOKEN; THE PERSON'S HISTORY DOES NOT. + if !strings.Contains(transcriptText(agent), NoChangeReply) { + t.Error("the model's own record lost the reply it made") + } + entries, _, stop := agent.AttachReplay() + stop() + last := "" + for _, entry := range entries { + if IsNoChangeReply(entry.Text) { + t.Errorf("a %q row reached the person's history", NoChangeReply) + } + if entry.Role == "assistant" && strings.TrimSpace(entry.Text) != "" { + last = entry.Text + } + } + if last != table { + t.Errorf("the answer left standing is not the table:\n%s", last) + } + if err := agent.Close(); err != nil { + t.Fatalf("close: %v", err) + } + exchange := learnedExchange(t, learned) + if !strings.Contains(exchange, "ASSISTANT:\n| seat | model | why |") || + !strings.Contains(exchange, "| seat 01 |") || strings.Contains(exchange, "ASSISTANT:\n"+NoChangeReply) { + t.Errorf("the memory reflex did not receive the answer before the withdrawn token:\n%s", exchange) + } +} + +// THE TOKEN IS AN ANSWER TO A CARRY-ON AND NOTHING ELSE. A turn that says it +// without having been carried on is read for what remains like any other, so +// the token cannot become a way to end a turn unread. +func TestNoChangeWithoutACarryOnDoesNotEndTheReading(t *testing.T) { + var remainsAsks, rounds atomic.Int64 + steps := make([]step, checkpointMarkAt(1)+10) + for index := range steps { + steps[index] = func(_ context.Context, messages []ai.Message) (*ai.Response, error) { + if askedForRemains(messages) { + remainsAsks.Add(1) + return textResponse(checkpointNothingLeft), nil + } + if call := rounds.Add(1); call <= int64(checkpointMarkAt(1)) { + return toolResponseWithText(fmt.Sprintf("call-%d", call), "ls", + fmt.Sprintf(`{"path":"./%d"}`, call), ""), nil + } + return textResponse(NoChangeReply), nil + } + } + agent := checkpointAgent(t, &scriptedCompleter{steps: steps}) + stubbedGraph(agent, func(node *TaskNode) {}) + + events, err := agent.Submit(context.Background(), "look through these paths") + if err != nil { + t.Fatalf("Submit: %v", err) + } + collect(t, events) + if got := remainsAsks.Load(); got != 1 { + t.Errorf("the reader was spent %d times on a turn never carried on, want once", got) + } +} + +// A TOKEN AFTER WORK IS NOT AN ANSWER TO THE NOTE. The model was carried on, +// made calls, and then said [no change]: those calls may have changed what the +// answer before the note says, so the turn is read again rather than ended on +// it unread. +func TestNoChangeAfterCallsMadeForTheNoteIsReadAgain(t *testing.T) { + var remainsAsks, rounds, afterNote atomic.Int64 + steps := make([]step, checkpointMarkAt(1)+40) + for index := range steps { + steps[index] = func(_ context.Context, messages []ai.Message) (*ai.Response, error) { + if askedForRemains(messages) { + if remainsAsks.Add(1) == 1 { + return textResponse("seat 3 has no model named"), nil + } + return textResponse(checkpointNothingLeft), nil + } + if last := messages[len(messages)-1]; last.Role == "user" && strings.HasPrefix(partsText(last), checkpointCarryOnLead) { + afterNote.Add(1) + return toolResponseWithText("call-note", "ls", `{"path":"./seat-3"}`, ""), nil + } + if afterNote.Load() > 0 { + return textResponse(NoChangeReply), nil + } + if call := rounds.Add(1); call <= int64(checkpointMarkAt(1)) { + return toolResponseWithText(fmt.Sprintf("call-%d", call), "ls", + fmt.Sprintf(`{"path":"./%d"}`, call), ""), nil + } + return textResponse(perSeatTable()), nil + } + } + completer := &scriptedCompleter{steps: steps} + agent := checkpointAgent(t, completer, func(config *Config) { config.Memory = openTestBrain(t) }) + learned := observeMemoryExtraction(completer) + stubbedGraph(agent, func(node *TaskNode) {}) + + events, err := agent.Submit(context.Background(), "compare the models per seat in a table") + if err != nil { + t.Fatalf("Submit: %v", err) + } + collect(t, events) + if got := remainsAsks.Load(); got != 2 { + t.Errorf("the reader was spent %d times, want twice: a token after calls must not end the turn unread", got) + } + if err := agent.Close(); err != nil { + t.Fatalf("close: %v", err) + } + if exchange := learnedExchange(t, learned); !strings.Contains(exchange, "ASSISTANT:\n"+NoChangeReply) { + t.Errorf("the memory reflex was not handed the non-withdrawn token reply:\n%s", exchange) + } +} + +// observeMemoryExtraction reads the existing reflex seam without adding a +// production hook. The checkpoint fixture already routes concurrent readers +// through scriptedCompleter.aside; this wraps that route so the post-turn +// extractor can answer without consuming one of the conversation's steps. +func observeMemoryExtraction(completer *scriptedCompleter) <-chan string { + learned := make(chan string, 1) + prior := completer.aside + completer.aside = func(messages []ai.Message) (*ai.Response, bool) { + if len(messages) > 1 && strings.Contains(messageText(messages[0]), "worth remembering after this session ends") { + learned <- messageText(messages[1]) + return textResponse(`{"mem":0}`), true + } + if prior != nil { + return prior(messages) + } + return nil, false + } + return learned +} + +func learnedExchange(t *testing.T, learned <-chan string) string { + t.Helper() + select { + case exchange := <-learned: + return exchange + default: + t.Fatal("the memory reflex never read the exchange") + return "" + } +} + +// THE TOKEN ANSWERS THE READER'S NOTE AND NOTHING ELSE THAT OPENS A TURN BACK UP. +// A load or ask nudge also wears [carry on], and a person can steer in between; +// neither is the note, so neither is ended by the token. +func TestNoChangeAnswersOnlyTheReadersNote(t *testing.T) { + note := textMessage("user", checkpointCarryOnLead+"seat 3 has no model named") + token := textMessage("assistant", NoChangeReply) + for _, shape := range []struct { + name string + messages []ai.Message + want bool + }{ + {"the note, then the token", []ai.Message{note, token}, true}, + {"the note, a volatile note, then the token", []ai.Message{note, textMessage("user", volatileNoteOpening+" the clock moved"), token}, true}, + {"a load nudge, then the token", []ai.Message{textMessage("user", checkpointLoadNudgeLead([]string{"ask"})), token}, false}, + {"the note, the person steering, then the token", []ai.Message{note, textMessage("user", "leave seat 3 blank"), token}, false}, + {"the note, then the token with more words", []ai.Message{note, textMessage("assistant", NoChangeReply+" the table is whole")}, false}, + {"the token with nothing before it", []ai.Message{token}, false}, + } { + if got := answeredNoteUnchanged(shape.messages); got != shape.want { + t.Errorf("%s: honoured %v, want %v", shape.name, got, shape.want) + } + } +} diff --git a/internal/session/prefixbudget_test.go b/internal/session/prefixbudget_test.go index acf13ab834..c3cad388b5 100644 --- a/internal/session/prefixbudget_test.go +++ b/internal/session/prefixbudget_test.go @@ -427,9 +427,30 @@ const fixedPrefixTarget = 48_000 // figure here, in this diff, on purpose: the rule says a raise is a decision // with a name on it, and the alternative was cutting the sentence the merge // just bought. +// +// 2026-09-16, #1065 round one. `# Tone` and `# Delivery` became `# The answer`, +// `# Answer or change`, `# When corrected` and `# Messages from codeaf`, in the +// wording the issue fixed and with the five rules the old two carried restored +// inside them, and that paid 2,035 bytes on both arms (fixed 53,276 to 55,311, +// lean 45,051 to 47,086). Fifteen of them came out of room both arms already had, +// so both waivers rise by 2,020 and now sit exactly on the measurement, on +// purpose and with this name on it: the issue that +// bought the sections is the one that puts the page's overall length to its +// second round, and cutting other laws in the first would be that round done +// early and unreviewed. +// +// 2026-09-16, #1067 review. The provenance list had to grow because compaction +// also writes user-role tags into the conversation: `[folded …]` and `[context +// compacted]`. It also stopped calling the tool-only `[held]` a user message or +// declaring every unlisted tag the person's, and it does NOT name standing +// news's `[something you set up fired]`: that line carries its own instruction +// under the event, and TestTheSteeringLineReadsAsNewsAndNotAsARequest refuses +// the page's copy as one law paid for twice. Tightening the surrounding +// sentence paid for the truth with room to spare: fixed is 55,280 and lean is +// 47,055, so both waivers fall by 31 and again sit exactly on the measurement. const ( - fixedPrefixWaiver = 5_291 - leanPrefixWaiver = 13_566 + fixedPrefixWaiver = 7_280 + leanPrefixWaiver = 15_555 ) // THE LEAN PROFILE GETS A BUDGET OF ITS OWN (2026-09-10, the prompt diet's lane diff --git a/internal/session/promptprofile.go b/internal/session/promptprofile.go index 7633c286db..bc636c473d 100644 --- a/internal/session/promptprofile.go +++ b/internal/session/promptprofile.go @@ -280,6 +280,14 @@ var leanPageSections = []leanSection{{ // heading at all. A row naming it would now decide nothing, which // promptprofile_test.go refuses — and it is the right outcome: a law the lean // arm did not need turned out to be a law nobody needed on the page. +}, { + heading: "Messages from codeaf", + keeps: true, + why: "THE ONE PLACE THE PAGE IDENTIFIES SESSION-AUTHORED BRACKETED MESSAGES, and it is not the " + + "section below. Each live nudge carries its own instruction, but none of them can " + + "say that the person never sees it: a model that answered `[carry on]` as though the " + + "person had typed it left an argument with the completion check standing as the " + + "answer (#1065), and a small model is the likeliest to do it.", }, { heading: "Interrupts and steering", keeps: false, diff --git a/internal/session/promptprofile_test.go b/internal/session/promptprofile_test.go index 04967e6ce9..240f0c83a6 100644 --- a/internal/session/promptprofile_test.go +++ b/internal/session/promptprofile_test.go @@ -243,7 +243,7 @@ func TestALeanPageDropsExactlyTheSectionsTheTableNames(t *testing.T) { // A SECTION NOBODY RULED ON IS KEPT. A law written into the page tomorrow // reaches both arms until somebody decides otherwise; dropping by default is // how the lean arm would quietly lose everything written after this file. - for _, kept := range []string{"# Tone", "# Tool Policy", "# Workflow", "# Delivery", "# Critical", "# Session facts"} { + for _, kept := range []string{"# The answer", "# Answer or change", "# When corrected", "# Messages from codeaf", "# Tool Policy", "# Workflow", "# Critical", "# Session facts"} { if !strings.Contains(lean, "\n"+kept+"\n") { t.Errorf("a lean page dropped %q, which no row rules on", kept) } diff --git a/internal/session/prompts/system.md b/internal/session/prompts/system.md index cd6bed9dc9..cd97c5783f 100644 --- a/internal/session/prompts/system.md +++ b/internal/session/prompts/system.md @@ -8,19 +8,58 @@ your tools to ground your answers. Keep continuity while delegated work runs. - Consider compiled code: NEVER avoidably allocate, copy or compute. - Unexpected repo changes are the user's work; adapt. -# Tone -- Fragments when clearer; no ceremony, hedging, summaries, filler, marketing. -- Match the person's task and vocabulary; include technical detail when useful. -- Concrete: exact files, symbols, APIs, state fields, edge cases. -- Conclusion first, evidence next: facts, constraints, tradeoffs, checks. -- Uncertainty: state it at the claim, name the tradeoff, choose the safe option. +# The answer +Once a turn ends, only its last message stays in view. Everything before it +(notes, plans, tool calls, results, thinking) folds into a closed "worked" line. +- The last message carries the whole deliverable, even if an earlier message in + the turn already had it. Never write "as above", "already given" or "see my + previous message": the person cannot see those. +- Line one answers the question or states the outcome. Then the thing asked for + (the table, the diff, the command) in the form asked. Then, only if needed, a + few lines of why. Evidence and blocking details stay complete. +- An answer is a few sentences; a deliverable is as long as the work needs. If you + cannot tell which, it is an answer. "Explain", "why" or "walk me through" lift + the limit. +- Structure only where the content has it: a table for comparisons, numbered steps + for sequences, prose otherwise. No headers on short answers, no emoji, no + decorative bold. +- Match the person's task and vocabulary. Concrete: exact files, symbols, values, + commands. State uncertainty at the claim it affects and choose the safe option. +- No opener ("Sure", "Great question", "You're right"), no recap of what you did, + no closing offer ("Want me to…", "Say the word and I'll…"). Stop when the content + stops. A decision you need goes through `ask`, with your pick. +- "Done" means the specified behavior end to end plus every named acceptance + check, never a compiling scaffold or a narrowed test; never hand back half-solved + work. Say plainly what you did not do or could not verify. + +# Answer or change +- Questions, options, comparisons, tables, plans, reviews and "not yet" are + answered in words, in the reply itself and not in a file unless the person asks + for a file. Make no edits until the person asks for the change: an edit they did + not ask for is work they must inspect and undo. +- Fix, change, add, build: act, at the size asked. No extra scope, no easier + substitute. If you think the ask is mistaken, say so in one sentence, then do it + as asked. + +# When corrected +- A correction or restated ask is the new ask: deliver it. No apology, no defense + of the earlier answer, no "as I said". +- If you believe the person is factually wrong, give the evidence in one or two + lines, then still deliver what they asked. + +# Messages from codeaf +codeaf writes user-role tags, not the person: [carry on], [taking stock], +[silent], [stuck], [folded …], [context compacted]. Follow them. Never answer as +if the person wrote them, argue with them, or mention them in your answer. +[image #N] marks the person's attachment. # Tool Policy ## General - Compute exact calculations and text transformations with a tool, and check the result against the requested format. - Before asking, climb the decision ladder: read the record; state a reasonable - assumption; for reversible work act and offer to unwind it; show concrete + assumption; for reversible work act and offer to unwind it when the person + asked for a change; show concrete outcomes; offer structured choices before free text. - Use `ask` only as the last rung, with why the decision is needed now, its stakes, and your pick, and ask THROUGH `ask`, never in prose: a typed-out @@ -123,11 +162,6 @@ material shows the width is real. STANDING_FACTS -# Delivery -- No extra scope or easier substitute; never punt half-solved work. -- "Done": the specified behavior end to end plus every named acceptance criterion; not a compiling scaffold or a narrowed test. -- Format MUST match the ask; prose brief; evidence and blocking details complete. - # Interrupts and steering A person's message arriving mid-turn means the generation before it was cut: keep the partial work already in the transcript, then answer the correction or @@ -157,5 +191,5 @@ BELT_FACTS BEFORE RUNNING A COMMAND, CHECK THE TRANSCRIPT. If its answer is already here, use it. Re-deriving a settled fact is a defect, not diligence. # Critical -- NEVER yield while actionable work remains; phase boundary/todo flip/sub-step never stops: same turn. +- Don't end the turn while work the person asked for remains; phase boundary/todo flip/sub-step never stops: same turn. - HANDED-OFF WORK IS NOT WORK THAT REMAINS: end your reply once nothing independent of it is left. A task of your own still owes its deliverable whatever it hands out. diff --git a/internal/session/task_ending_e2e_test.go b/internal/session/task_ending_e2e_test.go index 708579880a..eafdfa7445 100644 --- a/internal/session/task_ending_e2e_test.go +++ b/internal/session/task_ending_e2e_test.go @@ -233,7 +233,7 @@ func TestATaskThatWorkedQuietlyDoesNotGoInCircles(t *testing.T) { // all: the tree moved on every one of them, which resets the silent ladder // and answers the "read nothing new" rule in the same reading. for _, note := range []string{"[silent]", "[stuck]"} { - if completer.childSaw(note) { + if completer.childWasHanded(note) { t.Fatalf("a quiet landing was handed a %s note", note) } } diff --git a/internal/session/task_repair_test.go b/internal/session/task_repair_test.go index bb6cdb2a63..9c62d0165e 100644 --- a/internal/session/task_repair_test.go +++ b/internal/session/task_repair_test.go @@ -106,6 +106,23 @@ func (c *routedCompleter) childSaw(needle string) bool { return false } +// childWasHanded is [routedCompleter.childSaw] for a NOTE: it reads the +// conversation a worker was sent and never its system page, because the page +// names every bracketed note by its tag to say who writes them (prompts/system.md's +// `# Messages from codeaf`) and a page is not a note anybody handed it. +func (c *routedCompleter) childWasHanded(needle string) bool { + c.mu.Lock() + defer c.mu.Unlock() + for _, request := range c.childRequests { + for _, message := range request { + if message.Role != "system" && strings.Contains(messageText(message), needle) { + return true + } + } + } + return false +} + // childWorkTurns is how many times the node's lane was asked to WORK — the // harness's own title call rides the same lane and is not a turn anybody took on // the task. It is where "the work was handed back" and "it never was" are told diff --git a/internal/tui3/feed.go b/internal/tui3/feed.go index 71ba819a68..62171fd6d0 100644 --- a/internal/tui3/feed.go +++ b/internal/tui3/feed.go @@ -1572,6 +1572,15 @@ func (f *feed) confirmResponse() { } e := &f.entries[last] e.text, e.replyTags, e.demoted = text.String(), tags, false + // A RESPONSE THAT IS ONLY [session.NoChangeReply] WITHDREW ITSELF AS THE + // ANSWER. It is the model telling the completion check its note was wrong, + // never words for the person, so the confirmed block is emptied in place + // like every other discarded fragment — and the settled answer above it is + // the last words the turn's fold finds (workfold.go), exactly as replay + // finds it with the row left out (#1065). + if session.IsNoChangeReply(e.text) { + e.text, e.stale = "", true + } } f.touch() } diff --git a/internal/tui3/nochange_test.go b/internal/tui3/nochange_test.go new file mode 100644 index 0000000000..07816eb98e --- /dev/null +++ b/internal/tui3/nochange_test.go @@ -0,0 +1,75 @@ +package tui3 + +import ( + "strings" + "testing" + + "github.com/Agent-Field/codeaf/internal/session" +) + +// A CARRY-ON ANSWERED [session.NoChangeReply] LEAVES THE ANSWER BEFORE IT +// STANDING (#1065). The measured turn folded down to its last words, and its +// last words were the model arguing with the completion check — so the table +// the person asked for sat inside the chip, and only the argument stood. +// +// The events are the session's own order on that road: the answer and its +// boundary, the carry-on's notice, the token and its boundary. It drives the +// reducer directly, as the caption tests do, because what is pinned is what a +// confirmed response leaves in the transcript and what the fold then finds. +func TestANoChangeReplyLeavesThePriorAnswerAsTheFoldsAnswer(t *testing.T) { + const table = "| seat | model |\n| --- | --- |\n| one | deepseek/deepseek-v4.1-flash |\n" + f := &feed{live: -1, think: -1, turn: 1} + f.entries = append(f.entries, entry{kind: entryUser, text: "compare the models per seat", turn: 1}) + runBatch(f, "read", "c1") + f.ingest(session.Event{Kind: session.EventTextDelta, Text: table}) + f.ingest(session.Event{Kind: session.EventAssistantDone}) + f.ingest(session.Event{Kind: session.EventNotice, Text: "the ask is not finished · carrying on rather than stopping here"}) + f.ingest(session.Event{Kind: session.EventTextDelta, Text: session.NoChangeReply}) + f.ingest(session.Event{Kind: session.EventAssistantDone}) + + answer := -1 + for i, e := range f.entries { + if e.kind != entryAssistant || strings.TrimSpace(e.text) == "" { + continue + } + if session.IsNoChangeReply(e.text) { + t.Fatalf("the %q reply is still a block the person reads", session.NoChangeReply) + } + answer = i + } + if answer < 0 || f.entries[answer].text != table { + t.Fatalf("the last words left standing are not the table: %+v", f.entries) + } + folds := deriveWorkfolds(f.entries, 0) + if len(folds) != 1 { + t.Fatalf("the carried-on turn derived %d folds, want 1", len(folds)) + } + for _, fold := range folds { + if fold.answer != answer { + t.Fatalf("the fold answers with block %d, want the table at %d", fold.answer, answer) + } + } + if workEntry(f.entries, folds, answer) { + t.Fatal("the table was folded away as work") + } +} + +// ONLY THE TOKEN ALONE IS WITHDRAWN. Words that merely open with it, or quote it, +// are an answer like any other and stay exactly as they arrived. +func TestWordsThatOnlyMentionNoChangeStayTheAnswer(t *testing.T) { + for _, said := range []string{ + session.NoChangeReply + " — the table above is whole.", + "Reply with `" + session.NoChangeReply + "` when the note is wrong.", + } { + f := &feed{live: -1, think: -1, turn: 1} + f.ingest(session.Event{Kind: session.EventTextDelta, Text: said}) + f.ingest(session.Event{Kind: session.EventAssistantDone}) + kept := false + for _, e := range f.entries { + kept = kept || (e.kind == entryAssistant && e.text == said) + } + if !kept { + t.Errorf("an answer that only mentions the token was dropped: %q", said) + } + } +} diff --git a/test/remote/stub/PROBE-NOCHANGE.md b/test/remote/stub/PROBE-NOCHANGE.md new file mode 100644 index 0000000000..5f4fd778d5 --- /dev/null +++ b/test/remote/stub/PROBE-NOCHANGE.md @@ -0,0 +1,35 @@ +# Replaying the no-change fold + +From the repository root, build the real binary and the local model stub, then +start the stub with its log kept beside it: + +```sh +export GOFLAGS=-buildvcs=false +make build +go build -o /tmp/modelstub ./test/remote/stub +/tmp/modelstub -addr 127.0.0.1:8080 2>/tmp/modelstub-nochange.log & +stub_pid=$! +``` + +Open codeaf in tmux with a fresh, short state path and the one model served by +the stub: + +```sh +probe_home=/tmp/caf-nc-$$ +tmux new-session -s codeaf-nochange \ + "cd '$PWD' && env CODEAF_BASE_URL=http://127.0.0.1:8080/api/v1 OPENROUTER_API_KEY=stub CODEAF_HOME=$probe_home bin/codeaf chat --model stub/scripted --one-model" +``` + +Type `PROBE-NOCHANGE ./go.mod` and press enter. When the turn settles, the stub +log has exactly one request whose last user message starts with `[carry on]`, +one `PROBE-NOCHANGE no change` branch, and one reader branch: + +```sh +grep -c 'ask="\[carry on\]' /tmp/modelstub-nochange.log +grep -c 'PROBE-NOCHANGE no change' /tmp/modelstub-nochange.log +grep -c 'PROBE-NOCHANGE reader ' /tmp/modelstub-nochange.log +``` + +Each command prints `1`. On screen, the folded turn still ends with the +`seat 30` row of the table. `[no change]` appears nowhere on screen. Stop the +stub after leaving codeaf with `kill "$stub_pid"`. diff --git a/test/remote/stub/main.go b/test/remote/stub/main.go index 87105aff65..5ee29c11e5 100644 --- a/test/remote/stub/main.go +++ b/test/remote/stub/main.go @@ -21,6 +21,7 @@ // PROBE-PWD calls `bash` with `pwd`, then quotes the answer back // PROBE-READ calls `read` on the path, then quotes the answer back // PROBE-SLOW a reply spread over seconds, for the link that dies +// PROBE-NOCHANGE replays the completion-check rejection from #1065 // // QUOTING THE TOOL OUTPUT BACK IS THE WHOLE TRICK. `--once` prints the model's // reply on stdout and the tool's own output nowhere at all, so a scenario that @@ -106,9 +107,10 @@ func models(w http.ResponseWriter, r *http.Request) { // ignored: this is a script, not a model, and a field it pretended to honour // would be a second opinion about a protocol it does not own. type request struct { - Model string `json:"model"` - Stream bool `json:"stream"` - Messages []message `json:"messages"` + Model string `json:"model"` + Stream bool `json:"stream"` + Messages []message `json:"messages"` + Tools json.RawMessage `json:"tools"` } // message holds Content as a raw message because the dialect sends it two ways @@ -160,7 +162,12 @@ func completions(w http.ResponseWriter, r *http.Request) { log.Printf("POST /chat/completions model=%q stream=%v messages=%d ask=%q phase=%s", body.Model, body.Stream, len(body.Messages), shorten(ask, 120), phaseWord(answered)) - reply := compose(ask, answered) + reply, branch, handled := noChangeReply(body) + if !handled { + reply = compose(ask, answered) + } else { + log.Printf("%s %s", markerNoChange, branch) + } if !body.Stream { // Nothing in codeaf asks for a whole completion today, but a stub that // only spoke one of the two shapes would fail mysteriously the day @@ -226,17 +233,19 @@ type reply struct { // pause is held between chunks. pause time.Duration // tool, when named, makes this a tool call instead of an answer. - tool string - args string + tool string + args string + callID string } // The markers. They are spelled in the person's own message and nowhere else, // so a scenario reads as a sentence somebody could actually type. const ( - markerEcho = "PROBE-ECHO" - markerPwd = "PROBE-PWD" - markerRead = "PROBE-READ" - markerSlow = "PROBE-SLOW" + markerEcho = "PROBE-ECHO" + markerPwd = "PROBE-PWD" + markerRead = "PROBE-READ" + markerSlow = "PROBE-SLOW" + markerNoChange = "PROBE-NOCHANGE" // markerJob starts the command after it in the BACKGROUND, which is the one // thing the other four markers cannot reach: every one of them is a turn that // finishes, and a background job is by definition the work that outlives the @@ -251,6 +260,12 @@ const ( markerJob = "PROBE-JOB" ) +// noChangeToolRounds is checkpointMarkAt(1) + 1. The stub cannot import an +// internal package, so this value moves by hand if the first checkpoint does. +const noChangeToolRounds = 11 + +var noChangeReaderCalls atomic.Int64 + // The prefixes the harness asserts on. They are constants here and constants in // the test, because a string that appears in two places drifts. const ( @@ -262,6 +277,78 @@ const ( sayJob = "ENGINE-JOB: " ) +// noChangeReply is the deterministic live replay of #1065. The main +// conversation carries tools; the completion reader does not, so the tools +// field keeps a title or another private errand that quotes the person's marker +// from accidentally running the scenario. +func noChangeReply(body request) (reply, string, bool) { + markerAt, markerText := -1, "" + for index, message := range body.Messages { + text := message.text() + if strings.Contains(text, markerNoChange) { + markerAt, markerText = index, text + break + } + } + if markerAt < 0 { + return reply{}, "", false + } + + for _, message := range body.Messages { + if strings.Contains(message.text(), "[still asked]") { + call := noChangeReaderCalls.Add(1) + return reply{chunks: []string{fmt.Sprintf("the table was cut off mid-row (look %d)", call)}}, + fmt.Sprintf("reader %d", call), true + } + } + if !requestHasTools(body.Tools) { + return reply{}, "", false + } + + for index := len(body.Messages) - 1; index >= 0; index-- { + message := body.Messages[index] + if !strings.EqualFold(message.Role, "user") { + continue + } + if strings.HasPrefix(strings.TrimSpace(message.text()), "[carry on]") { + return reply{chunks: []string{"[no change]"}}, "no change", true + } + break + } + + rounds := 0 + for _, message := range body.Messages[markerAt+1:] { + if strings.EqualFold(message.Role, "tool") { + rounds++ + } + } + if rounds < noChangeToolRounds { + round := rounds + 1 + path := pathAfter(markerText, markerNoChange) + return reply{ + chunks: []string{fmt.Sprintf("Reading replay round %d.", round)}, + tool: "read", + args: fmt.Sprintf(`{"path":"%s","offset":%d,"limit":1}`, jsonEscape(path), round), + callID: fmt.Sprintf("call_nochange_%02d", round), + }, fmt.Sprintf("round %d tool", round), true + } + return reply{chunks: []string{noChangeTable()}}, "table", true +} + +func requestHasTools(tools json.RawMessage) bool { + trimmed := strings.TrimSpace(string(tools)) + return trimmed != "" && trimmed != "null" && trimmed != "[]" +} + +func noChangeTable() string { + var table strings.Builder + table.WriteString("| seat | model | why |\n| --- | --- | --- |\n") + for seat := 1; seat <= 30; seat++ { + fmt.Fprintf(&table, "| seat %02d | stub/scripted | deterministic replay row %02d |\n", seat, seat) + } + return strings.TrimSpace(table.String()) +} + // compose is the script itself. func compose(ask, answered string) reply { switch { @@ -370,9 +457,16 @@ func writeStream(w http.ResponseWriter, answer reply) { send(chunk(id, `{"role":"assistant","content":""}`, "")) if answer.tool != "" { + callID := answer.callID + if callID == "" { + callID = "call_1" + } + for _, text := range answer.chunks { + send(chunk(id, fmt.Sprintf(`{"content":%q}`, text), "")) + } send(chunk(id, fmt.Sprintf( - `{"tool_calls":[{"index":0,"id":"call_1","type":"function","function":{"name":%q,"arguments":%q}}]}`, - answer.tool, answer.args), "")) + `{"tool_calls":[{"index":0,"id":%q,"type":"function","function":{"name":%q,"arguments":%q}}]}`, + callID, answer.tool, answer.args), "")) send(finish(id, "tool_calls")) send("[DONE]") return @@ -413,8 +507,12 @@ func finish(id, reason string) string { func writeWhole(w http.ResponseWriter, answer reply) { w.Header().Set("Content-Type", "application/json") if answer.tool != "" { - fmt.Fprintf(w, `{"id":"stub","object":"chat.completion","model":"stub/scripted","choices":[{"index":0,"message":{"role":"assistant","content":"","tool_calls":[{"id":"call_1","type":"function","function":{"name":%q,"arguments":%q}}]},"finish_reason":"tool_calls"}],"usage":{"prompt_tokens":1,"completion_tokens":1,"total_tokens":2,"cost":0}}`, - answer.tool, answer.args) + callID := answer.callID + if callID == "" { + callID = "call_1" + } + fmt.Fprintf(w, `{"id":"stub","object":"chat.completion","model":"stub/scripted","choices":[{"index":0,"message":{"role":"assistant","content":%q,"tool_calls":[{"id":%q,"type":"function","function":{"name":%q,"arguments":%q}}]},"finish_reason":"tool_calls"}],"usage":{"prompt_tokens":1,"completion_tokens":1,"total_tokens":2,"cost":0}}`, + strings.Join(answer.chunks, ""), callID, answer.tool, answer.args) return } fmt.Fprintf(w, `{"id":"stub","object":"chat.completion","model":"stub/scripted","choices":[{"index":0,"message":{"role":"assistant","content":%q},"finish_reason":"stop"}],"usage":{"prompt_tokens":1,"completion_tokens":1,"total_tokens":2,"cost":0}}`,