diff --git a/internal/loki/agents/builder.md b/internal/loki/agents/builder.md index e1426b6..1bb5f84 100644 --- a/internal/loki/agents/builder.md +++ b/internal/loki/agents/builder.md @@ -10,7 +10,7 @@ Method — in this order: Rules: - PLAN ONCE. If the task needs a plan, write it to PLAN.md in ONE short write, then follow it. Between tool calls, think one or two sentences at most — NEVER restate or re-derive the plan: it is in PLAN.md and in your criteria, read them instead. - Write each file COMPLETE in a single write call. Many small writes waste turns. -- You may NOT mark a criterion passed — the verification pass does that. +- When a criterion is done AND checked, mark it completed (criteria set). Verification starts once none is left pending; only it may mark passed. - If a command fails, read the error and fix the cause; do not retry the same command unchanged. - Ask the user (ask tool) only for decisions that are genuinely theirs; decide the rest yourself. - Making the change is your job; shipping it is not. Without an explicit request, never commit, push, reset or rebase, never deploy, restart a service or replace a running binary. Read-only inspection (git_status, git_diff) is fine and encouraged. diff --git a/internal/loki/chat_conversation.go b/internal/loki/chat_conversation.go index b143e42..b3aa038 100644 --- a/internal/loki/chat_conversation.go +++ b/internal/loki/chat_conversation.go @@ -785,6 +785,9 @@ func (c *Conversation) generate(ctx context.Context, caps Caps, temperature floa // l'usage est arrivé ; sinon on retombe sur l'estimation (celle qui pilote // déjà la compaction), approximative mais jamais absente. sawUsage := false + // asked : le tour s'est terminé sur une question à l'utilisateur (outil ask) — + // pas de vérification du mode Code avant sa réponse. + asked := false extra, _ := runChat(ctx, InjectSkills(final, caps), temperature, caps, func(ev StreamEvent) bool { switch { case ev.Err != nil: @@ -851,6 +854,7 @@ func (c *Conversation) generate(ctx context.Context, caps Caps, temperature floa c.appendDelta(epoch, map[string]any{"stats": ev.Stats}) case ev.Ask != nil: // Question structurée (outil ask) : carte à boutons dans l'UI. + asked = true c.appendDelta(epoch, map[string]any{"ask": ev.Ask}) case ev.DropReasoning: c.appendDelta(epoch, map[string]any{"drop_reasoning": true}) @@ -900,7 +904,7 @@ func (c *Conversation) generate(ctx context.Context, caps Caps, temperature floa // corrections, re-vérification — AVANT de rendre la main. Voir // code_verify.go. No-op hors mode code ou sans critères. if !stale && ctx.Err() == nil { - c.codeVerifyLoop(ctx, caps, temperature, epoch) + c.codeVerifyLoop(ctx, caps, temperature, epoch, asked) c.mu.Lock() msgs = append([]Message(nil), c.Messages...) ctxUsed = c.CtxUsed diff --git a/internal/loki/code_criteria.go b/internal/loki/code_criteria.go index 62ea6ef..ae19082 100644 --- a/internal/loki/code_criteria.go +++ b/internal/loki/code_criteria.go @@ -13,6 +13,7 @@ package loki import ( "encoding/json" "fmt" + "strconv" "strings" "time" ) @@ -20,7 +21,7 @@ import ( type Criterion struct { ID int `json:"id"` Text string `json:"text"` - Status string `json:"status"` // pending | passed | failed + Status string `json:"status"` // pending | completed (le builder dit « fait ») | passed | failed Note string `json:"note,omitempty"` } @@ -74,6 +75,18 @@ func critPending(list []Criterion) int { return n } +// critOpen : les critères que le builder n'a pas encore déclarés faits — +// pending, ou failed et pas encore repris (la reprise les remet à completed). +func critOpen(list []Criterion) []Criterion { + var out []Criterion + for _, c := range list { + if c.Status == "pending" || c.Status == "failed" || c.Status == "" { + out = append(out, c) + } + } + return out +} + // critRender : la liste formatée pour un prompt (builder ou verifier). func critRender(list []Criterion) string { var b strings.Builder @@ -84,6 +97,8 @@ func critRender(list []Criterion) string { mark = "[✓]" case "failed": mark = "[✗]" + case "completed": + mark = "[~]" // fait selon le builder, pas encore vérifié } fmt.Fprintf(&b, "%s #%d %s", mark, c.ID, c.Text) if c.Note != "" { @@ -100,15 +115,15 @@ func criteriaTool() Tool { Function: ToolFunction{ Name: "criteria", Description: "Manage the acceptance criteria of the current task (the contract that defines DONE). " + - "action=add posits new criteria (texts[]), action=set updates one (id + status passed|failed|pending, optional note), " + - "action=list shows them, action=clear removes them all. Set criteria BEFORE building; only the verification pass may mark passed.", + "action=add posits new criteria (texts[]), action=set updates one (id + status, optional note), " + + "action=list shows them, action=clear removes them all. Set criteria BEFORE building. Builder: mark each criterion completed once you have done AND checked it — verification only starts when nothing is left pending. Only the verification pass may mark passed or failed.", Parameters: map[string]any{ "type": "object", "properties": map[string]any{ "action": map[string]any{"type": "string", "enum": []string{"add", "set", "list", "clear"}}, "texts": map[string]any{"type": "array", "items": map[string]any{"type": "string"}, "description": "add: one entry per criterion, short and testable"}, "id": map[string]any{"type": "integer", "description": "set: criterion id"}, - "status": map[string]any{"type": "string", "enum": []string{"pending", "passed", "failed"}}, + "status": map[string]any{"type": "string", "enum": []string{"pending", "completed", "passed", "failed"}}, "note": map[string]any{"type": "string", "description": "set: why it failed / how it was verified"}, }, "required": []string{"action"}, @@ -158,8 +173,15 @@ func toolCriteria(args map[string]any, allowPass bool) string { critNotify(list) return fmt.Sprintf("[ok] %d critère(s) ajouté(s)\n%s", added, critRender(list)) case "set": - idf, _ := args["id"].(float64) - id := int(idf) + // id numérique ou chaîne (« "2" », « "#2" ») : les petits modèles + // écrivent volontiers l'id entre guillemets, qui tombait sinon à #0. + id := 0 + switch v := args["id"].(type) { + case float64: + id = int(v) + case string: + id, _ = strconv.Atoi(strings.TrimPrefix(strings.TrimSpace(v), "#")) + } status, _ := args["status"].(string) note, _ := args["note"].(string) if status == "passed" && !allowPass { diff --git a/internal/loki/code_verify.go b/internal/loki/code_verify.go index 76afae1..c987165 100644 --- a/internal/loki/code_verify.go +++ b/internal/loki/code_verify.go @@ -25,10 +25,19 @@ import ( // en 2 corrections tourne en rond, l'utilisateur doit reprendre la main. const codeMaxFixTurns = 2 +// codeMaxContinue : relances du builder qui a rendu la main avec des critères +// encore ouverts (« ensuite je vais… ») avant de vérifier quand même. Reprise +// du buildAgentNudge d'OpenFox : une passe de vérification sur un travail +// inachevé, c'est un prefill complet, le cache KV du fil évincé, et des +// « failed » qui ne disent que « pas encore fait ». +const codeMaxContinue = 2 + // codeVerifyLoop est appelé par generate() à la FIN d'un tour de build en mode -// code. Il ne fait rien s'il n'y a pas de contrat (aucun critère). -func (c *Conversation) codeVerifyLoop(ctx context.Context, caps Caps, temperature float64, epoch int) { - if !caps.Code || caps.Role == "verifier" || ctx.Err() != nil { +// code. Il ne fait rien s'il n'y a pas de contrat (aucun critère), ni si le +// tour s'est terminé sur une question à l'utilisateur (asked) : sa réponse +// passe avant toute vérification. +func (c *Conversation) codeVerifyLoop(ctx context.Context, caps Caps, temperature float64, epoch int, asked bool) { + if !caps.Code || caps.Role == "verifier" || ctx.Err() != nil || asked { return } convID := convEnsureActive() @@ -40,6 +49,24 @@ func (c *Conversation) codeVerifyLoop(ctx context.Context, caps Caps, temperatur if caps.Role == "planner" { return } + // Le builder vérifie lui-même puis marque « completed » ce qu'il estime + // fait. Tant qu'il reste des critères ouverts, on le relance plutôt que de + // vérifier un travail inachevé. + for n := 0; n < codeMaxContinue; n++ { + open := critOpen(list) + if len(open) == 0 { + break + } + msg := "Not done yet — these acceptance criteria are still open:\n" + critRender(open) + + "\n\nContinue working on them. Once one is done and checked, mark it completed (criteria action=set, status=completed). If you need the user's decision, use ask." + if c.runBuilderTurn(ctx, caps, temperature, epoch, msg) || ctx.Err() != nil { + return // question posée à l'utilisateur, ou arrêt + } + list = critList(convID) + if critAllPassed(list) { + return + } + } for fix := 0; ; fix++ { if ctx.Err() != nil { return @@ -59,7 +86,9 @@ func (c *Conversation) codeVerifyLoop(ctx context.Context, caps Caps, temperatur fmt.Sprint(codeMaxFixTurns) + " corrections — voir le panneau des critères)_"}) return } - c.runFixTurn(ctx, caps, temperature, epoch, list) + if c.runFixTurn(ctx, caps, temperature, epoch, list) { + return // le builder attend une réponse de l'utilisateur + } } } @@ -91,24 +120,33 @@ func (c *Conversation) runVerifyPass(ctx context.Context, caps Caps, temperature } // runFixTurn relance le BUILDER sur l'historique normal avec la liste des -// critères en échec. Sa trace est persistée comme un tour ordinaire. -func (c *Conversation) runFixTurn(ctx context.Context, caps Caps, temperature float64, epoch int, list []Criterion) { +// critères en échec. Sa trace est persistée comme un tour ordinaire. Renvoie +// true si le builder a posé une question à l'utilisateur (ask). +func (c *Conversation) runFixTurn(ctx context.Context, caps Caps, temperature float64, epoch int, list []Criterion) bool { var failed []Criterion for _, cr := range list { if cr.Status != "passed" { failed = append(failed, cr) } } + return c.runBuilderTurn(ctx, caps, temperature, epoch, "Verification failed on these criteria:\n"+critRender(failed)+ + "\n\nFix the code so they pass. Address the notes precisely; do not touch what already passed. Mark each fixed criterion completed again.") +} + +// runBuilderTurn relance le BUILDER sur l'historique normal avec une consigne +// (correction après vérification, ou relance sur des critères ouverts). Sa +// trace est persistée comme un tour ordinaire. Renvoie true si le builder a +// posé une question à l'utilisateur (ask) : la boucle doit alors s'arrêter. +func (c *Conversation) runBuilderTurn(ctx context.Context, caps Caps, temperature float64, epoch int, instruction string) (asked bool) { c.appendDelta(epoch, map[string]any{"role": "builder"}) defer c.appendDelta(epoch, map[string]any{"role": ""}) - fixMsg := Message{Role: "user", Content: "Verification failed on these criteria:\n" + critRender(failed) + - "\n\nFix the code so they pass. Address the notes precisely; do not touch what already passed."} + fixMsg := Message{Role: "user", Content: instruction} c.mu.Lock() if c.epoch != epoch { c.mu.Unlock() - return + return false } c.Messages = append(c.Messages, fixMsg) msgs := append([]Message(nil), c.Messages...) @@ -123,6 +161,12 @@ func (c *Conversation) runFixTurn(ctx context.Context, caps Caps, temperature fl if ev.Content != "" { content.WriteString(ev.Content) } + if ev.ToolUsed != nil { + content.Reset() // déjà dans le message tool_calls (voir generate) + } + if ev.Ask != nil { + asked = true + } c.forwardStream(ev, epoch) return true }) @@ -135,6 +179,7 @@ func (c *Conversation) runFixTurn(ctx context.Context, caps Caps, temperature fl } c.mu.Unlock() c.persist() + return asked } // forwardStream relaie les événements d'un runChat secondaire (vérification, diff --git a/internal/loki/code_verify_gate_test.go b/internal/loki/code_verify_gate_test.go new file mode 100644 index 0000000..708004b --- /dev/null +++ b/internal/loki/code_verify_gate_test.go @@ -0,0 +1,110 @@ +package loki + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync" + "testing" +) + +// moteurScripte : faux llama-server qui répond selon le DERNIER message reçu. +// Un message `tool` en queue → texte de clôture ; sinon la règle dont le motif +// figure dans le dernier message utilisateur → appel d'outil criteria. +func moteurScripte(t *testing.T, regles map[string]string) (port string, vus *[]string) { + t.Helper() + var mu sync.Mutex + var log []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body struct { + Messages []Message `json:"messages"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + last := body.Messages[len(body.Messages)-1] + w.Header().Set("Content-Type", "text/event-stream") + w.WriteHeader(200) + args := "" + if last.Role == "user" { + txt := msgText(last) + mu.Lock() + log = append(log, txt) + mu.Unlock() + for motif, a := range regles { + if strings.Contains(txt, motif) { + args = a + } + } + } + if args != "" { + esc := strings.ReplaceAll(args, `"`, `\"`) + _, _ = w.Write([]byte(`data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"c1","type":"function","function":{"name":"criteria","arguments":"` + esc + `"}}]},"finish_reason":null}]}` + "\n\n")) + _, _ = w.Write([]byte(`data: {"choices":[{"delta":{},"finish_reason":"tool_calls"}]}` + "\n\n")) + } else { + _, _ = w.Write([]byte(sseChunk("ok"))) + _, _ = w.Write([]byte(`data: {"choices":[{"delta":{},"finish_reason":"stop"}]}` + "\n\n")) + } + _, _ = w.Write([]byte("data: [DONE]\n\n")) + })) + t.Cleanup(srv.Close) + u, _ := url.Parse(srv.URL) + return u.Port(), &log +} + +// Le builder rend la main avec un critère encore ouvert : on le RELANCE (il le +// marque completed) avant de vérifier, au lieu de vérifier un travail +// inachevé (repris du buildAgentNudge d'OpenFox). +func TestVerificationAttendQueLeBuilderAitFini(t *testing.T) { + withWorkspace(t) + id := convEnsureActive() + toolCriteria(map[string]any{"action": "add", "texts": []any{"le build compile"}}, false) + port, vus := moteurScripte(t, map[string]string{ + "Not done yet": `{"action":"set","id":"1","status":"completed"}`, + "Verify each non-pass": `{"action":"set","id":1,"status":"passed"}`, + }) + if err := SetConfigKey("PORT", port); err != nil { + t.Fatal(err) + } + c := newTestConv() + c.Messages = []Message{{Role: "user", Content: "fais le build"}} + c.codeVerifyLoop(context.Background(), Caps{Agent: true, Code: true}, 0.2, c.epoch, false) + + if len(*vus) < 2 || !strings.Contains((*vus)[0], "Not done yet") || !strings.Contains((*vus)[1], "Verify each non-pass") { + t.Fatalf("ordre des passes = %q, attendu relance du builder PUIS vérification", *vus) + } + if l := critList(id); len(l) != 1 || l[0].Status != "passed" { + t.Fatalf("critère final : %+v", l) + } +} + +// Tour terminé sur une question à l'utilisateur : aucune passe ne part avant +// sa réponse. +func TestPasDeVerificationApresUneQuestion(t *testing.T) { + withWorkspace(t) + toolCriteria(map[string]any{"action": "add", "texts": []any{"le build compile"}}, false) + port, vus := moteurScripte(t, nil) + if err := SetConfigKey("PORT", port); err != nil { + t.Fatal(err) + } + c := newTestConv() + c.Messages = []Message{{Role: "user", Content: "fais le build"}} + c.codeVerifyLoop(context.Background(), Caps{Agent: true, Code: true}, 0.2, c.epoch, true) + if len(*vus) != 0 { + t.Fatalf("%d requête(s) au moteur alors qu'une question attend l'utilisateur", len(*vus)) + } +} + +// Un id de critère écrit entre guillemets (« "2" », « "#2" ») vise bien #2. +func TestCritereIdEnChaine(t *testing.T) { + withWorkspace(t) + id := convEnsureActive() + toolCriteria(map[string]any{"action": "add", "texts": []any{"a", "b"}}, false) + if out := toolCriteria(map[string]any{"action": "set", "id": "#2", "status": "completed"}, false); !strings.HasPrefix(out, "[ok]") { + t.Fatalf("id en chaîne refusé : %s", out) + } + if l := critList(id); l[1].Status != "completed" || l[0].Status != "pending" { + t.Fatalf("mauvais critère modifié : %+v", l) + } +} diff --git a/internal/loki/code_verify_test.go b/internal/loki/code_verify_test.go new file mode 100644 index 0000000..e151d54 --- /dev/null +++ b/internal/loki/code_verify_test.go @@ -0,0 +1,70 @@ +package loki + +import ( + "context" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync/atomic" + "testing" +) + +// fauxMoteurOutil : un llama-server de comédie qui, à la PREMIÈRE requête, +// répond par un appel d'outil (name/args), puis par un texte à la suivante. +func fauxMoteurOutil(t *testing.T, name, args string) (port string, requêtes *int32) { + t.Helper() + var n int32 + esc := strings.ReplaceAll(args, `"`, `\"`) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/event-stream") + w.WriteHeader(200) + if atomic.AddInt32(&n, 1) == 1 { + _, _ = w.Write([]byte(`data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"c1","type":"function","function":{"name":"` + name + `","arguments":"` + esc + `"}}]},"finish_reason":null}]}` + "\n\n")) + _, _ = w.Write([]byte(`data: {"choices":[{"delta":{},"finish_reason":"tool_calls"}]}` + "\n\n")) + } else { + _, _ = w.Write([]byte(sseChunk("Verdict enregistré."))) + _, _ = w.Write([]byte(`data: {"choices":[{"delta":{},"finish_reason":"stop"}]}` + "\n\n")) + } + _, _ = w.Write([]byte("data: [DONE]\n\n")) + })) + t.Cleanup(srv.Close) + u, _ := url.Parse(srv.URL) + return u.Port(), &n +} + +// La passe de vérification (Role=verifier) doit pouvoir marquer un critère +// passed via l'outil criteria — c'est tout son rôle. Vécu : « en mode code, +// le système de vérification ne coche plus les critères ». +func TestPasseDeVerificationCocheLesCriteres(t *testing.T) { + withWorkspace(t) + id := convEnsureActive() + toolCriteria(map[string]any{"action": "add", "texts": []any{"le build compile"}}, false) + + port, n := fauxMoteurOutil(t, "criteria", `{"action":"set","id":1,"status":"passed"}`) + if err := SetConfigKey("PORT", port); err != nil { + t.Fatal(err) + } + + caps := Caps{Agent: true, Code: true, Role: "verifier"} + var tools []string + _, err := runChat(context.Background(), []Message{ + {Role: "system", Content: rolePrompt("verifier")}, + {Role: "user", Content: "Verify."}, + }, 0.2, caps, func(ev StreamEvent) bool { + if ev.ToolUsed != nil && ev.ToolUsed.Done { + tools = append(tools, ev.ToolUsed.Name+" → "+ev.ToolUsed.Result) + } + return true + }) + if err != nil { + t.Fatal(err) + } + if *n < 2 { + t.Fatalf("%d requête(s) au moteur : l'appel d'outil n'a pas été suivi d'un second tour", *n) + } + list := critList(id) + if len(list) != 1 || list[0].Status != "passed" { + t.Fatalf("critère non coché par le vérificateur : %+v — outils : %v", list, tools) + } +} diff --git a/internal/loki/ui/index.html b/internal/loki/ui/index.html index ee407c7..62c8aa1 100644 --- a/internal/loki/ui/index.html +++ b/internal/loki/ui/index.html @@ -2143,6 +2143,7 @@ html[data-files="1"] #files-btn{color:var(--accent)} #criteria-list li.crit-passed{color:var(--accent)} #criteria-list li.crit-failed{color:var(--err,#b3564d)} #criteria-list li.crit-pending{color:var(--dim)} +#criteria-list li.crit-completed{color:var(--text)} .rolebadge{display:inline-block;margin-left:6px;padding:0 6px;border:1px solid var(--accent);border-radius:3px;color:var(--accent);font-size:9px;letter-spacing:.05em;text-transform:uppercase;vertical-align:middle} @@ -9894,7 +9895,7 @@ function renderCriteria(list){ for(const c of list){ const li = document.createElement('li'); li.className = 'crit-'+(c.status||'pending'); - const mark = c.status==='passed' ? '✓' : (c.status==='failed' ? '✗' : '○'); + const mark = c.status==='passed' ? '✓' : (c.status==='failed' ? '✗' : (c.status==='completed' ? '◐' : '○')); li.textContent = mark+' '+c.text + (c.note ? ' — '+c.note : ''); ul.appendChild(li); } diff --git a/internal/loki/ui/src/js/20-mode.js b/internal/loki/ui/src/js/20-mode.js index 40fd94e..dcc0f49 100644 --- a/internal/loki/ui/src/js/20-mode.js +++ b/internal/loki/ui/src/js/20-mode.js @@ -110,7 +110,7 @@ function renderCriteria(list){ for(const c of list){ const li = document.createElement('li'); li.className = 'crit-'+(c.status||'pending'); - const mark = c.status==='passed' ? '✓' : (c.status==='failed' ? '✗' : '○'); + const mark = c.status==='passed' ? '✓' : (c.status==='failed' ? '✗' : (c.status==='completed' ? '◐' : '○')); li.textContent = mark+' '+c.text + (c.note ? ' — '+c.note : ''); ul.appendChild(li); } diff --git a/internal/loki/ui/src/styles.css b/internal/loki/ui/src/styles.css index e4536fc..44f568f 100644 --- a/internal/loki/ui/src/styles.css +++ b/internal/loki/ui/src/styles.css @@ -2108,6 +2108,7 @@ html[data-files="1"] #files-btn{color:var(--accent)} #criteria-list li.crit-passed{color:var(--accent)} #criteria-list li.crit-failed{color:var(--err,#b3564d)} #criteria-list li.crit-pending{color:var(--dim)} +#criteria-list li.crit-completed{color:var(--text)} .rolebadge{display:inline-block;margin-left:6px;padding:0 6px;border:1px solid var(--accent);border-radius:3px;color:var(--accent);font-size:9px;letter-spacing:.05em;text-transform:uppercase;vertical-align:middle}