diff --git a/cmd/mavweb/chat.go b/cmd/mavweb/chat.go index 383245c..e78a9ff 100644 --- a/cmd/mavweb/chat.go +++ b/cmd/mavweb/chat.go @@ -2,12 +2,15 @@ package main import ( _ "embed" + "errors" "log" "net/http" "net/url" + "strconv" "strings" "github.com/kami/maven/internal/ipc" + "github.com/kami/maven/internal/router" "github.com/kami/maven/internal/webauthn" ) @@ -25,6 +28,21 @@ type chatMsg struct { // Source — the query source that claimed the turn, shown as a badge beside // the reply. Empty for a turn no source claimed (V-539). Source string + // TraceID anchors the correction gesture (V-630). Non-zero ⇒ the turn was + // persisted and can be corrected in one click. 0 ⇒ no correction is offered, + // which is honest: a box with no database has no turn to correct. + TraceID int64 + // Corrected — the owner already corrected this turn, so the page says thank + // you instead of offering the buttons again. + Corrected string +} + +// correctionTargets — the seven public intents, in the order the buttons are +// shown. Read from internal/router rather than typed out, so a new intent cannot +// exist without a way to correct a turn into it. +var correctionTargets = []router.Intent{ + router.IntentFact, router.IntentNote, router.IntentReminder, + router.IntentQuery, router.IntentAct, router.IntentChat, router.IntentSystem, } // handleChatPage renders the chat conversation page. @@ -38,12 +56,21 @@ func handleChatPage(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI) { msgs = append(msgs, chatMsg{Role: "user", Text: q}) } if reply := r.URL.Query().Get("r"); reply != "" { - msgs = append(msgs, chatMsg{Role: "assistant", Text: reply, Source: r.URL.Query().Get("s")}) + id, _ := strconv.ParseInt(r.URL.Query().Get("t"), 10, 64) + msgs = append(msgs, chatMsg{ + Role: "assistant", Text: reply, Source: r.URL.Query().Get("s"), + TraceID: id, Corrected: r.URL.Query().Get("c"), + }) } + // UserText rides beside the messages so the correction form can hand the + // conversation back on the redirect: this page has no session and no JS, so + // what is on screen is what the query params carry. renderPage(w, chatTmpl, struct { Error string Messages []chatMsg - }{Messages: msgs}) + Targets []router.Intent + UserText string + }{Messages: msgs, Targets: correctionTargets, UserText: r.URL.Query().Get("q")}) } // handleChatAPI processes a chat message POST and redirects back to /chat. @@ -88,5 +115,77 @@ func handleChatAPI(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI, ses if reply.Source != "" { dest += "&s=" + url.QueryEscape(reply.Source) } + // The trace id rides along so the reply can carry a correction gesture + // (V-630). Absent when nothing persisted, and the page then offers none. + if reply.TraceID != 0 { + dest += "&t=" + strconv.FormatInt(reply.TraceID, 10) + } http.Redirect(w, r, dest, http.StatusSeeOther) } + +// handleCorrectAPI records that the last turn was routed wrongly (V-630). +// +// A correction is the only supervised signal this box gets, and everything else +// in the trace accumulates on its own. So the gesture has to cost nothing: one +// POST from the reply he is already looking at, carrying the trace id and +// optionally the intent it should have been. An unstated target is accepted, +// because a turn marked wrong with no target is still a usable negative. +// +// Not step-up gated, unlike POST /api/chat. Writing a label reaches no router, +// no model and no act path; it writes one row nothing executes from. Gating it +// would price the gesture out of being used, which is the one thing that makes +// it worthless. +func handleCorrectAPI(w http.ResponseWriter, r *http.Request, core ipc.CoreAPI) { + if r.Method != http.MethodPost { + http.Error(w, "POST only", http.StatusMethodNotAllowed) + return + } + if !requireCore(w, core, "correct") { + return + } + id, err := strconv.ParseInt(strings.TrimSpace(r.FormValue("trace_id")), 10, 64) + if err != nil || id <= 0 { + http.Error(w, "trace_id required", http.StatusBadRequest) + return + } + shouldBe := strings.TrimSpace(r.FormValue("should_be")) + // Only one of the seven, or nothing. Free text here would put an unroutable + // label in the one table V-632 fits prototypes from. + if shouldBe != "" && !isCorrectionTarget(shouldBe) { + http.Error(w, "should_be must be one of the seven intents", http.StatusBadRequest) + return + } + if err := core.CorrectTurn(r.Context(), id, shouldBe); err != nil { + log.Printf("correct turn %d: %v", id, err) + // A turn past the retention bound is gone, and saying so is different + // from saying the write broke. + if errors.Is(err, ipc.ErrNoSuchTrace) { + http.Error(w, "that turn is no longer stored", http.StatusNotFound) + return + } + http.Error(w, "correction failed", http.StatusBadGateway) + return + } + stamp := shouldBe + if stamp == "" { + stamp = "wrong" + } + // Back to the conversation he was in, with the turn still on screen. The + // query params carry it, so the correction is preserved by re-sending them. + dest := "/chat?q=" + url.QueryEscape(r.FormValue("q")) + + "&r=" + url.QueryEscape(r.FormValue("rep")) + "&c=" + url.QueryEscape(stamp) + if s := r.FormValue("s"); s != "" { + dest += "&s=" + url.QueryEscape(s) + } + http.Redirect(w, r, dest, http.StatusSeeOther) +} + +// isCorrectionTarget — one of the seven, and nothing else. +func isCorrectionTarget(s string) bool { + for _, t := range correctionTargets { + if string(t) == s { + return true + } + } + return false +} diff --git a/cmd/mavweb/chat.html b/cmd/mavweb/chat.html index ac35c0e..88dccc1 100644 --- a/cmd/mavweb/chat.html +++ b/cmd/mavweb/chat.html @@ -5,6 +5,21 @@
{{range .Messages}}
{{if eq .Role "user"}}you{{else}}maven{{end}}: {{.Text}}{{if .Source}} {{.Source}}{{end}}
+ {{if and (eq .Role "assistant") .TraceID}} + {{if .Corrected}} +
corrected: {{.Corrected}}
+ {{else}} +
+ + + + + + should have been: + {{range $.Targets}}{{end}} +
+ {{end}} + {{end}} {{else}}
diff --git a/cmd/mavweb/correct_test.go b/cmd/mavweb/correct_test.go new file mode 100644 index 0000000..20ecd59 --- /dev/null +++ b/cmd/mavweb/correct_test.go @@ -0,0 +1,146 @@ +package main + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/kami/maven/internal/ipc" +) + +// correctCore records the correction the handler sends. +type correctCore struct { + ipc.UnimplementedCoreAPI + traceID int64 + shouldBe string + called bool + err error +} + +func (c *correctCore) CorrectTurn(_ context.Context, traceID int64, shouldBe string) error { + c.called, c.traceID, c.shouldBe = true, traceID, shouldBe + return c.err +} + +func postCorrect(form url.Values) *http.Request { + req := httptest.NewRequest(http.MethodPost, "/api/correct", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + return req +} + +// The full gesture: wrong, and it should have been a fact. +func TestCorrectAPIWithTarget(t *testing.T) { + core := &correctCore{} + rr := httptest.NewRecorder() + handleCorrectAPI(rr, postCorrect(url.Values{ + "trace_id": {"42"}, "should_be": {"fact"}, "q": {"поужинал"}, "rep": {"поняла"}, + }), core) + + if rr.Code != http.StatusSeeOther { + t.Fatalf("status %d, want 303; body=%s", rr.Code, rr.Body.String()) + } + if core.traceID != 42 || core.shouldBe != "fact" { + t.Errorf("corrected trace %d to %q", core.traceID, core.shouldBe) + } + // The turn stays on screen, and the page says it was corrected. + loc := rr.Header().Get("Location") + if !strings.Contains(loc, "c=fact") || !strings.Contains(loc, "q=") { + t.Errorf("redirect %q loses the turn or the correction", loc) + } +} + +// The cheap half. A turn marked wrong with no target is still a usable negative, +// and it must not cost more to give than the full answer. +func TestCorrectAPIWithNoTarget(t *testing.T) { + core := &correctCore{} + rr := httptest.NewRecorder() + handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"7"}}), core) + + if rr.Code != http.StatusSeeOther { + t.Fatalf("status %d, want 303", rr.Code) + } + if !core.called || core.shouldBe != "" { + t.Errorf("called=%v shouldBe=%q, want an untargeted negative recorded", core.called, core.shouldBe) + } + if !strings.Contains(rr.Header().Get("Location"), "c=wrong") { + t.Errorf("redirect %q does not say the turn was marked wrong", rr.Header().Get("Location")) + } +} + +// Free text here would put an unroutable label in the one table V-632 fits +// prototypes from. +func TestCorrectAPIRejectsUnknownTarget(t *testing.T) { + core := &correctCore{} + rr := httptest.NewRecorder() + handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"7"}, "should_be": {"погода"}}), core) + + if rr.Code != http.StatusBadRequest { + t.Fatalf("status %d, want 400", rr.Code) + } + if core.called { + t.Error("wrote a label for a target that is not one of the seven") + } +} + +func TestCorrectAPINeedsTraceID(t *testing.T) { + for _, form := range []url.Values{{}, {"trace_id": {"0"}}, {"trace_id": {"nope"}}} { + core := &correctCore{} + rr := httptest.NewRecorder() + handleCorrectAPI(rr, postCorrect(form), core) + if rr.Code != http.StatusBadRequest { + t.Errorf("form %v: status %d, want 400", form, rr.Code) + } + if core.called { + t.Errorf("form %v: reached the core", form) + } + } +} + +// A write that broke is not a turn that expired, and the two must not read the +// same to the owner deciding whether to correct again. +func TestCorrectAPIReportsFailure(t *testing.T) { + core := &correctCore{err: errors.New("disk is full")} + rr := httptest.NewRecorder() + handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"9"}, "should_be": {"note"}}), core) + if rr.Code != http.StatusBadGateway { + t.Fatalf("status %d, want 502", rr.Code) + } +} + +// A trace past the retention bound is gone, and the surface says that. +func TestCorrectAPIExpiredTurn(t *testing.T) { + core := &correctCore{err: ipc.ErrNoSuchTrace} + rr := httptest.NewRecorder() + handleCorrectAPI(rr, postCorrect(url.Values{"trace_id": {"9"}, "should_be": {"note"}}), core) + if rr.Code != http.StatusNotFound { + t.Fatalf("status %d, want 404", rr.Code) + } +} + +func TestCorrectAPIPostOnly(t *testing.T) { + rr := httptest.NewRecorder() + handleCorrectAPI(rr, httptest.NewRequest(http.MethodGet, "/api/correct", nil), &correctCore{}) + if rr.Code != http.StatusMethodNotAllowed { + t.Fatalf("status %d, want 405", rr.Code) + } +} + +// Every one of the seven intents has a button, so a new intent cannot exist with +// no way to correct a turn into it. +func TestCorrectionTargetsAreTheSeven(t *testing.T) { + if len(correctionTargets) != 7 { + t.Fatalf("%d targets, want the seven public intents", len(correctionTargets)) + } + for _, want := range []string{"fact", "note", "reminder", "query", "act", "chat", "system"} { + if !isCorrectionTarget(want) { + t.Errorf("%s is not offered", want) + } + } + if isCorrectionTarget("") { + t.Error("empty is not a target: it is the absence of one, handled separately") + } +} diff --git a/cmd/mavweb/main.go b/cmd/mavweb/main.go index ea913b9..15fa0d9 100644 --- a/cmd/mavweb/main.go +++ b/cmd/mavweb/main.go @@ -210,6 +210,13 @@ func main() { mux.HandleFunc("/routines", gatedPage(handleRoutines)) mux.HandleFunc("/api/chat", gatedPage(handleChatAPI)) mux.HandleFunc("/api/revert", gatedPage(handleRevert)) + // POST /api/correct is deliberately NOT on the step-up list (V-630). It + // reaches no router, no model and no act path: it writes one label row that + // nothing executes from. A correction that costs a passkey tap is a + // correction the owner does not make, and then the table stays empty. + mux.HandleFunc("/api/correct", func(w http.ResponseWriter, r *http.Request) { + handleCorrectAPI(w, r, core) + }) mux.HandleFunc("/models", func(w http.ResponseWriter, r *http.Request) { handleModels(w, r, core, swapConn, stepUpSession, *requireStepUp) }) diff --git a/cmd/mavweb/static/ui.css b/cmd/mavweb/static/ui.css index ee1eb0b..d7029c1 100644 --- a/cmd/mavweb/static/ui.css +++ b/cmd/mavweb/static/ui.css @@ -702,6 +702,11 @@ details[open] > summary { margin-bottom: var(--space-1); } .chat-form { display: flex; gap: var(--space-2); } .chat-form input { flex: 1; } .chat-scroll { max-height: 60vh; overflow-y: auto; margin-bottom: var(--space-4); } +/* The correction gesture (V-630). Wraps on a phone rather than scrolling: it is + one row of small buttons, and a gesture that has to be panned to is not one. */ +.chat-correct { display: flex; flex-wrap: wrap; align-items: center; gap: var(--space-1); + padding: 0 var(--space-3) var(--space-2); margin-top: calc(-1 * var(--space-1)); margin-bottom: var(--space-2); } +.chat-correct-label { font-size: var(--fs-xs); color: var(--text-machine); margin-left: var(--space-2); } /* ── Key-value grid ── */ .kv { display: grid; grid-template-columns: auto 1fr; gap: var(--space-1) var(--space-3); font-size: var(--fs-sm); }