one gesture beside the reply corrects a turn (V-630)

Two buttons' worth of cost: wrong, or wrong and it should have been this.
The second is worth much more and is not required to give the first, so a
turn marked wrong with no target still lands as a usable negative.

The target is one of the seven intents, never free text: an unroutable label
would enter the one table V-632 fits prototypes from.

/api/correct is not behind the step-up gate. It reaches no router, no model
and no act path, and a correction that costs a passkey tap is one that does
not get made.

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")
+	}
+}