diff --git a/internal/loki/chat_compact.go b/internal/loki/chat_compact.go index 7162595..ce8662e 100644 --- a/internal/loki/chat_compact.go +++ b/internal/loki/chat_compact.go @@ -338,7 +338,14 @@ func compactBounds(msgs []Message, tailBudget int) (head, tailStart int) { break } } - for tailStart > head && msgs[tailStart].Role != "user" && msgs[tailStart].Role != "assistant" { + // Jamais sur un rappel de loki (isLokiInjected) : il a le rôle `user` sans + // être une demande. En tête de queue, la vraie demande réinjectée juste + // avant (pending, voir compactMessages) lui serait collée — deux `user` + // d'affilée, que les gabarits à alternance stricte refusent à chaque tour. + // On recule jusqu'à l'appel d'outil qui le précède : la queue garde un + // groupe de plus, rien n'est perdu. + for tailStart > head && ((msgs[tailStart].Role != "user" && msgs[tailStart].Role != "assistant") || + isLokiInjected(msgs[tailStart])) { tailStart-- } return head, tailStart diff --git a/internal/loki/chat_conversation.go b/internal/loki/chat_conversation.go index 796c7b3..89ca3d6 100644 --- a/internal/loki/chat_conversation.go +++ b/internal/loki/chat_conversation.go @@ -920,6 +920,9 @@ func (c *Conversation) generate(ctx context.Context, caps Caps, temperature floa if s := content.String(); strings.TrimSpace(s) != "" { c.Messages = append(c.Messages, Message{Role: "assistant", Content: s}) } + // Rappel de loki resté sans réponse (stop, erreur) ou collé à un autre + // user : retiré, sinon deux `user` d'affilée au tour suivant. + c.Messages = dropStrayNudges(c.Messages) // Le dernier compte couvre tout ce qui vient d'être rangé : la dernière // requête portait le tour entier, sa génération en est la réponse. if sawUsage { diff --git a/internal/loki/code_verify.go b/internal/loki/code_verify.go index 3bfc854..6309fdc 100644 --- a/internal/loki/code_verify.go +++ b/internal/loki/code_verify.go @@ -188,6 +188,7 @@ func (c *Conversation) runBuilderTurn(ctx context.Context, caps Caps, temperatur if s := content.String(); strings.TrimSpace(s) != "" { c.Messages = append(c.Messages, Message{Role: "assistant", Content: s}) } + c.Messages = dropStrayNudges(c.Messages) // même règle que generate if sawUsage { c.ctxUsedLen = len(c.Messages) // même règle que generate } diff --git a/internal/loki/llm_client.go b/internal/loki/llm_client.go index 6510bcb..c2ccb7d 100644 --- a/internal/loki/llm_client.go +++ b/internal/loki/llm_client.go @@ -1713,8 +1713,9 @@ func runChat(ctx context.Context, messages []Message, temperature float64, caps // Relance tool_choice « none » sans réponse exploitable (appel tenté // malgré tout, balisage d'appel en texte, ou rien du tout) : repli UNE // fois sur le chemin historique, outils retirés du gabarit. Le prompt est - // alors recalculé, mais le tour aboutit comme avant. - if toolChoiceNone && (noneLeak || textualToolCallSnippet(assistantContent.String()) != "" || + // alors recalculé, mais le tour aboutit comme avant. Pas après un stop : + // la relance partirait sur un contexte annulé et afficherait une erreur. + if toolChoiceNone && ctx.Err() == nil && (noneLeak || textualToolCallSnippet(assistantContent.String()) != "" || strings.TrimSpace(assistantContent.String()) == "") { logCtx("relance tool_choice=none sans réponse exploitable : repli sans outils") toolChoiceNone = false diff --git a/internal/loki/llm_injected.go b/internal/loki/llm_injected.go index 9aedae1..d101c72 100644 --- a/internal/loki/llm_injected.go +++ b/internal/loki/llm_injected.go @@ -38,6 +38,17 @@ const retryCorrectiveLead = "Your last answer contained a TOOL CALL WRITTEN AS T // relance qui suit. const toolsOffHint = "Do not call any more tools. Answer now, directly, in the user's language, using only the information already gathered." +// budgetNudgeEnds : fins des trois rappels de budget (llm_budget.go). Le seul +// préfixe « [system] » ne suffit pas à les reconnaître : un utilisateur peut +// très bien coller un texte qui commence ainsi, et sa demande serait alors +// sautée par la compaction, le vérificateur ou le titre. Le test +// TestIsLokiInjected tient ces fins en phase avec budgetNudge. +var budgetNudgeEnds = []string{ + "unless exactly one specific call is genuinely still missing.", + "say so in the answer instead of investigating further.", + "Write your final answer in this message. Do not call another tool.", +} + // isLokiInjected dit si un message `user` vient de loki et non de // l'utilisateur. func isLokiInjected(m Message) bool { @@ -48,8 +59,48 @@ func isLokiInjected(m Message) bool { if !ok { return false } - return strings.HasPrefix(s, lokiNotePrefix) || s == thinkNudgeFirst || s == thinkNudgeStuck || - strings.HasPrefix(s, retryCorrectiveLead) + if s == thinkNudgeFirst || s == thinkNudgeStuck || s == lokiNotePrefix+toolsOffHint || + strings.HasPrefix(s, retryCorrectiveLead) { + return true + } + if !strings.HasPrefix(s, lokiNotePrefix) { + return false + } + for _, end := range budgetNudgeEnds { + if strings.HasSuffix(s, end) { + return true + } + } + return false +} + +// dropStrayNudges retire de l'historique à persister les rappels de loki +// restés en porte-à-faux : en toute fin (le tour a été arrêté ou a échoué avant +// que le modèle y réponde), ou collés à un autre message `user` (un ajout en +// cours de réponse arrivé juste derrière, ou un rappel éphémère qu'une +// compaction en cours de tour a fait entrer dans l'historique). Gardés, ils +// laisseraient deux `user` d'affilée au tour suivant — exception à chaque tour +// sur les gabarits à alternance stricte (voir appendNudge). Retirés, on +// retrouve l'historique d'avant leur persistance ; le cache ne perd que ce +// qui suivait le rappel, comme avant. Renvoie msgs tel quel s'il n'y a rien à +// retirer, une copie sinon. +func dropStrayNudges(msgs []Message) []Message { + for { + drop := -1 + for i, m := range msgs { + if !isLokiInjected(m) { + continue + } + if i == len(msgs)-1 || msgs[i+1].Role == "user" || (i > 0 && msgs[i-1].Role == "user") { + drop = i + break + } + } + if drop < 0 { + return msgs + } + msgs = append(append([]Message(nil), msgs[:drop]...), msgs[drop+1:]...) + } } // appendNudge ajoute un rappel de loki à la vue du modèle et, quand c'est sans diff --git a/internal/loki/llm_injected_test.go b/internal/loki/llm_injected_test.go index 57acda5..f762ad9 100644 --- a/internal/loki/llm_injected_test.go +++ b/internal/loki/llm_injected_test.go @@ -31,9 +31,13 @@ func scriptedServer(t *testing.T, steps ...string) func() []map[string]any { n := len(bodies) - 1 mu.Unlock() step := steps[min(n, len(steps)-1)] - if step == "500" { + switch step { + case "500": http.Error(w, "Failed to parse tool call", http.StatusInternalServerError) return + case "400": + http.Error(w, "Unsupported param: tool_choice", http.StatusBadRequest) + return } w.Header().Set("Content-Type", "text/event-stream") w.WriteHeader(200) @@ -71,6 +75,9 @@ func TestIsLokiInjected(t *testing.T) { {"budget 1", um(budgetNudge(24, 24, 0)), true}, {"budget 2", um(budgetNudge(48, 24, 1)), true}, {"budget 3", um(budgetNudge(72, 24, 2)), true}, + {"budget suivants", um(budgetNudge(150, 24, 5)), true}, + {"consigne du 500 en message à part", um(lokiNotePrefix + toolsOffHint), true}, + {"demande qui commence comme un rappel", um(lokiNotePrefix + "réécris ce prompt système"), false}, {"pensé sans agir", um(thinkNudgeFirst), true}, {"pensé sans agir, bis", um(thinkNudgeStuck), true}, {"appel écrit en texte", um(retryCorrective("x")), true}, @@ -224,6 +231,71 @@ func TestCompactionReinjecteLaVraieDemandePasLeRappel(t *testing.T) { if count(tailNudge) != 1 { t.Fatal("le rappel de la queue a disparu") } + assertAlternance(t, out) +} + +// assertAlternance : jamais deux `user` d'affilée (gabarits stricts). +func assertAlternance(t *testing.T, msgs []Message) { + t.Helper() + for i := 1; i < len(msgs); i++ { + if msgs[i].Role == "user" && msgs[i-1].Role == "user" { + t.Fatalf("deux user d'affilée en %d : %q puis %q", i, msgText(msgs[i-1]), msgText(msgs[i])) + } + } +} + +// La queue protégée ne commence jamais sur un rappel : la vraie demande, +// réinjectée juste devant elle, lui serait collée (deux user d'affilée). +func TestCompactBoundsPasSurUnRappel(t *testing.T) { + nudge := um(budgetNudge(24, 24, 0)) + msgs := []Message{um("cherche"), atc("web_read"), tm(strings.Repeat("x", 800)), nudge, atc("web_read"), tm("court")} + budget := msgTokens(msgs[3]) + msgTokens(msgs[4]) + msgTokens(msgs[5]) + _, tailStart := compactBounds(msgs, budget) + if tailStart != 1 { + t.Fatalf("tailStart = %d, attendu 1 (l'appel d'outil avant le rappel)", tailStart) + } + // Sans rappel, la frontière reste celle d'avant. + plain := []Message{um("cherche"), atc("web_read"), tm(strings.Repeat("x", 800)), um("et ensuite"), atc("web_read"), tm("court")} + if _, ts := compactBounds(plain, msgTokens(plain[3])+msgTokens(plain[4])+msgTokens(plain[5])); ts != 3 { + t.Fatalf("frontière sans rappel déplacée : %d", ts) + } +} + +// Ce qui est persisté en fin de tour ne garde aucun rappel en porte-à-faux. +func TestDropStrayNudges(t *testing.T) { + n := um(thinkNudgeFirst) + cases := []struct { + name string + in []Message + want []Message + }{ + {"rappel répondu", []Message{um("q"), atc("glob"), tm("r"), n, am("fini")}, nil}, + {"rappel resté sans réponse (stop)", []Message{um("q"), atc("glob"), tm("r"), n}, + []Message{um("q"), atc("glob"), tm("r")}}, + {"ajout en cours de réponse juste derrière", []Message{um("q"), atc("glob"), tm("r"), n, um("et aussi"), am("fini")}, + []Message{um("q"), atc("glob"), tm("r"), um("et aussi"), am("fini")}}, + {"rappel collé derrière la demande", []Message{um("q"), n, am("fini")}, []Message{um("q"), am("fini")}}, + {"correctif sans réponse", []Message{um("q"), am(""), um(retryCorrective(""))}, + []Message{um("q"), am("")}}, + {"deux rappels de suite en fin", []Message{um("q"), atc("glob"), tm("r"), n, um(thinkNudgeStuck)}, + []Message{um("q"), atc("glob"), tm("r")}}, + {"vraie demande en fin", []Message{um("q"), am("a"), um("suite")}, nil}, + } + for _, c := range cases { + orig, _ := json.Marshal(c.in) + got := dropStrayNudges(c.in) + if after, _ := json.Marshal(c.in); string(after) != string(orig) { + t.Errorf("%s : l'entrée a été modifiée", c.name) + } + want := c.want + if want == nil { + want = c.in + } + if !reflect.DeepEqual(got, want) { + t.Errorf("%s :\n got %+v\n want %+v", c.name, got, want) + } + assertAlternance(t, got) + } } // La réduction forcée coupe aux vraies demandes ; un rappel ne sert de coupe @@ -341,14 +413,22 @@ func TestRelance500GardeLePrompt(t *testing.T) { // tout) → chemin historique, outils retirés et consigne dans le système. func TestRelance500ReplieSurLeCheminHistorique(t *testing.T) { leak := `data: {"choices":[{"delta":{"content":""}}]}` + "\n\n" + sseStop - for name, second := range map[string]string{"second 500": "500", "appel en texte": leak, "réponse vide": sseStop} { + for name, second := range map[string]string{"second 500": "500", "refus 4xx": "400", "appel en texte": leak, + "appel par le protocole": sseGlobCall, "réponse vide": sseStop} { t.Run(name, func(t *testing.T) { testHome(t) reqs := scriptedServer(t, "500", second, sseChunk("voici")+sseStop) in := []Message{{Role: "system", Content: "SYS"}, um("question")} - if _, err := runChat(t.Context(), in, 0.7, Caps{Agent: true}, func(StreamEvent) bool { return true }); err != nil { + ran := false + if _, err := runChat(t.Context(), in, 0.7, Caps{Agent: true}, func(ev StreamEvent) bool { + ran = ran || ev.ToolUsed != nil + return true + }); err != nil { t.Fatal(err) } + if ran { + t.Fatal("un appel émis sous tool_choice none a été exécuté") + } bodies := reqs() if len(bodies) != 3 { t.Fatalf("%d requêtes, attendu 3", len(bodies))