Keep the operator's corrections as reading lessons at once: correct, lessons, undo (issue 1428)

This commit is contained in:
jochen
2026-10-21 00:27:47 +02:00
parent a13a6c5898
commit e87269b0d2
4 changed files with 454 additions and 41 deletions
@@ -0,0 +1,64 @@
package main
// Correcting one mail entry (novox/hq ADR 0459 §8, issue 1428): a pure function, no store, that applies a
// correction to an entry and says what changed, from and to, in one line — what the kept reading lesson's
// own Change (store.go) carries. tools.go's correctTool does the store work: fetching the entry, resolving a
// group's label, calling this, then writing both the updated entry and the new lesson.
import (
"fmt"
"strings"
"time"
)
// Correction is what correct changes about one entry: any of mark, mail group (by label — correctTool has
// already resolved it to GroupID by the time this runs), why or due date. A field left unset (Mark == "",
// GroupLabel == "", !WhySet, !DueDateSet) is left as the entry already has it.
type Correction struct {
Mark Mark
GroupLabel string
GroupID int64
Why string
WhySet bool
DueDate time.Time
DueDateSet bool
}
// ApplyCorrection applies c to entry, answering the updated entry and a change description naming each
// field actually changed, its own from and to (1428/AC1); "" when c changed nothing. oldGroupLabel is the
// entry's own mail group's label before the correction, for the change line alone (the entry itself keeps
// only the id).
func ApplyCorrection(entry Entry, c Correction, oldGroupLabel string) (Entry, string) {
var changes []string
if c.Mark != "" && c.Mark != entry.Mark {
changes = append(changes, fmt.Sprintf("mark: %s -> %s", orNone(string(entry.Mark)), c.Mark))
entry.Mark = c.Mark
}
if c.GroupLabel != "" && c.GroupID != entry.Group {
changes = append(changes, fmt.Sprintf("mail group: %s -> %s", orNone(oldGroupLabel), c.GroupLabel))
entry.Group = c.GroupID
}
if c.WhySet && c.Why != entry.Why {
changes = append(changes, fmt.Sprintf("why: %s -> %s", orNone(entry.Why), orNone(c.Why)))
entry.Why = c.Why
}
if c.DueDateSet && !c.DueDate.Equal(entry.DueDate) {
changes = append(changes, fmt.Sprintf("due date: %s -> %s", dateOrNone(entry.DueDate), dateOrNone(c.DueDate)))
entry.DueDate = c.DueDate
}
return entry, strings.Join(changes, "; ")
}
func orNone(s string) string {
if s == "" {
return "none"
}
return s
}
func dateOrNone(t time.Time) string {
if t.IsZero() {
return "none"
}
return t.Format("2006-01-02")
}
@@ -0,0 +1,197 @@
package main
import (
"context"
"strings"
"testing"
"time"
)
func testBundle(store Store, caller string) *Bundle {
return &Bundle{
Caller: func(map[string]any) string { return caller },
callersOverride: func() (Callers, error) { return Callers{"mesh-mail-web"}, nil },
Store: func(context.Context) (Store, error) { return store, nil },
}
}
// 1428/AC1: correct with a mail's id and a new mark or mail group changes that entry and keeps a reading
// lesson at once: the mail's sender name and address, subject, account and folder, the change (from and
// to), and when; no body text; no Approve asked.
func TestCorrectChangesTheEntryAndKeepsALessonAtOnce(t *testing.T) {
ctx := context.Background()
store := newMemStore()
id, err := store.AddEntry(ctx, Entry{
Account: "bob", Folder: "INBOX", SenderName: "Alice", SenderAddress: "alice@example.org",
Subject: "pay up", Mark: MarkNeither, Date: time.Now(),
})
if err != nil {
t.Fatal(err)
}
b := testBundle(store, "mesh-mail-web")
out, err := b.correctTool(map[string]any{"id": float64(id), "mark": "matters", "group": "bills"})
if err != nil {
t.Fatal(err)
}
m := out.(map[string]any)
if m["mark"] != "matters" || m["group"] != "bills" {
t.Fatalf("correct's answer: %v", m)
}
updated, ok, err := store.Entry(ctx, id)
if err != nil || !ok {
t.Fatalf("Entry(%d) = %v, %v, %v", id, updated, ok, err)
}
if updated.Mark != MarkMatters {
t.Fatalf("entry's mark = %q, want matters", updated.Mark)
}
lessons, err := store.Lessons(ctx)
if err != nil {
t.Fatal(err)
}
if len(lessons) != 1 {
t.Fatalf("lessons = %v, want exactly one kept at once", lessons)
}
l := lessons[0]
if l.Account != "bob" || l.Folder != "INBOX" || l.SenderName != "Alice" || l.SenderAddress != "alice@example.org" ||
l.Subject != "pay up" {
t.Fatalf("the lesson's own headers: %+v", l)
}
if !strings.Contains(l.Change, "neither") || !strings.Contains(l.Change, "matters") {
t.Fatalf("the lesson's change = %q, want it to name the mark's from and to", l.Change)
}
if l.Made.IsZero() {
t.Fatal("the lesson carries no when")
}
}
// 1428/AC5: correct refuses an unknown mail id or mark.
func TestCorrectRefusesUnknownIDOrMark(t *testing.T) {
ctx := context.Background()
store := newMemStore()
id, _ := store.AddEntry(ctx, Entry{Account: "bob", Mark: MarkNeither})
b := testBundle(store, "mesh-mail-web")
if _, err := b.correctTool(map[string]any{"id": float64(id + 999), "mark": "matters"}); err == nil {
t.Fatal("correct with an unknown id: no error")
}
if _, err := b.correctTool(map[string]any{"id": float64(id), "mark": "urgent"}); err == nil {
t.Fatal("correct with an unknown mark: no error")
}
}
// 1428/AC2: lessons answers the reading lessons newest first, undone ones marked or left out as asked.
func TestLessonsNewestFirstAndUndoneAsAsked(t *testing.T) {
ctx := context.Background()
store := newMemStore()
old, _ := store.AddLesson(ctx, Lesson{SenderAddress: "a@x.org", Made: time.Date(2026, 10, 1, 0, 0, 0, 0, time.UTC)})
newer, _ := store.AddLesson(ctx, Lesson{SenderAddress: "b@x.org", Made: time.Date(2026, 10, 2, 0, 0, 0, 0, time.UTC)})
store.UndoLesson(ctx, old)
b := testBundle(store, "mesh-mail-web")
out, err := b.lessonsTool(map[string]any{})
if err != nil {
t.Fatal(err)
}
lessons := out.(map[string]any)["lessons"].([]map[string]any)
if len(lessons) != 1 || lessons[0]["id"] != newer {
t.Fatalf("lessons (undone left out) = %v, want just the newer, not-undone one", lessons)
}
out, err = b.lessonsTool(map[string]any{"include-undone": true})
if err != nil {
t.Fatal(err)
}
lessons = out.(map[string]any)["lessons"].([]map[string]any)
if len(lessons) != 2 || lessons[0]["id"] != newer || lessons[1]["id"] != old || lessons[1]["undone"] != true {
t.Fatalf("lessons (include-undone) = %v, want newest first, old marked undone", lessons)
}
}
// 1428/AC3: undo of a lesson removes it from what later batches are given (selectLessons, question.go) and
// leaves the corrected entry as it is.
func TestUndoExcludesFromLaterBatchesAndLeavesTheEntry(t *testing.T) {
ctx := context.Background()
store := newMemStore()
id, _ := store.AddEntry(ctx, Entry{Account: "bob", Mark: MarkMatters, SenderAddress: "a@x.org"})
lessonID, _ := store.AddLesson(ctx, Lesson{SenderAddress: "a@x.org", Change: "mark: neither -> matters", Made: time.Now()})
b := testBundle(store, "mesh-mail-web")
if _, err := b.undoTool(map[string]any{"id": float64(lessonID)}); err != nil {
t.Fatal(err)
}
lessons, _ := store.Lessons(ctx)
selected := selectLessons(lessons, []QuestionMail{{From: []Person{{Address: "a@x.org"}}}})
if len(selected) != 0 {
t.Fatalf("selectLessons after undo = %v, want none (undone is never selected)", selected)
}
entry, ok, err := store.Entry(ctx, id)
if err != nil || !ok || entry.Mark != MarkMatters {
t.Fatalf("the entry changed after undo: %+v, %v, %v", entry, ok, err)
}
}
// 1428/AC5: undo refuses an unknown lesson id.
func TestUndoRefusesUnknownID(t *testing.T) {
store := newMemStore()
b := testBundle(store, "mesh-mail-web")
if _, err := b.undoTool(map[string]any{"id": float64(999)}); err == nil {
t.Fatal("undo with an unknown id: no error")
}
}
// 1428/AC4: a lesson given for a sender is in the next batch's question for a mail of that sender, before
// newer lessons of other senders; once undone, it is not — through 1427's own question builder.
func TestLessonForASenderReachesTheNextBatchsQuestionUntilUndone(t *testing.T) {
ctx := context.Background()
store := newMemStore()
id, _ := store.AddEntry(ctx, Entry{Account: "bob", SenderAddress: "a@x.org", Mark: MarkNeither})
b := testBundle(store, "mesh-mail-web")
older := time.Now().Add(-time.Hour)
store.AddLesson(ctx, Lesson{SenderAddress: "other@x.org", Change: "unrelated", Made: time.Now()}) // newer, other sender
if _, err := b.correctTool(map[string]any{"id": float64(id), "mark": "matters"}); err != nil {
t.Fatal(err)
}
lessons, _ := store.Lessons(ctx)
// Force the correction's own lesson to be the OLDER one, so a naive "newest first" selection would put
// the other sender's lesson ahead of it; matching-first (1427/AC1) must still put it first.
for i := range lessons {
if lessons[i].SenderAddress == "a@x.org" {
lessons[i].Made = older
}
}
mails := []QuestionMail{{ID: 1, From: []Person{{Address: "a@x.org"}}}}
q := BuildQuestion(mails, nil, lessons)
// Search by each lesson's own Change text (unique), never by the sender address alone: the mail's own
// header block in the question always names its sender, lesson or no lesson, so that would prove nothing.
matchingIdx := strings.Index(q.Text, "mark: neither -> matters")
otherIdx := strings.Index(q.Text, "unrelated")
if matchingIdx == -1 {
t.Fatalf("the matching sender's lesson is not in the question at all:\n%s", q.Text)
}
if otherIdx != -1 && matchingIdx > otherIdx {
t.Fatalf("the matching sender's lesson came after the other sender's, even though it is older")
}
// Undone, it is no longer in the next batch's question.
lessonID := int64(0)
allLessons, _ := store.Lessons(ctx)
for _, l := range allLessons {
if l.SenderAddress == "a@x.org" {
lessonID = l.ID
}
}
if _, err := b.undoTool(map[string]any{"id": float64(lessonID)}); err != nil {
t.Fatal(err)
}
allLessons, _ = store.Lessons(ctx)
q = BuildQuestion(mails, nil, allLessons)
if strings.Contains(q.Text, "mark: neither -> matters") {
t.Fatalf("the undone lesson is still in the question:\n%s", q.Text)
}
}
@@ -7,15 +7,14 @@ package main
// asked by the node-engine itself and carries no caller check.
//
// The six verbs' own behaviour — reading and writing mail entries, mail groups and reading lessons — is
// 1521 (entries, groups, status) and 1428 (correct, lessons, undo). Until each lands, its tool answers that
// it is not built yet, after the caller check has already passed or refused, so a caller never learns
// whether a verb is built before learning whether it is granted. mail_reader_health's own answer is built
// (1519): it reaches the kept health state (store.go, health.go) instead.
// 1521 (entries, groups, status) and 1428 (correct, lessons, undo): all six now answer for real.
// mail_reader_health's own answer is 1519's: it reaches the kept health state (store.go, health.go).
import (
"context"
"errors"
"fmt"
"sort"
"time"
stdio "git.novox.be/novox/mesh-sdk/go"
@@ -74,13 +73,6 @@ func (b *Bundle) checkCaller(args map[string]any) (string, error) {
return caller, nil
}
// notBuiltYet is what a mail-reading verb answers once its caller is granted, until its own issue lands
// (1426/AC3's note: this is not acceptable at the end of the milestone, by which every verb answers; it is
// acceptable now, while 1521 and 1428 are still to come).
func notBuiltYet(tool, issue string) error {
return fmt.Errorf("%s is not built yet (issue %s)", tool, issue)
}
// schema, text, integer and boolean mirror mesh-mail's own tools.go helpers (modules/mesh-mail/cmd/mesh-mail,
// copied in miniature so mesh-mail-reader never imports mesh-mail's tools package for its schemas alone).
func schema(required []string, props map[string]any) map[string]any {
@@ -93,6 +85,7 @@ func schema(required []string, props map[string]any) map[string]any {
func text(d string) map[string]any { return map[string]any{"type": "string", "description": d} }
func integer(d string) map[string]any { return map[string]any{"type": "integer", "description": d} }
func boolean(d string) map[string]any { return map[string]any{"type": "boolean", "description": d} }
// number reads an optional whole-number argument, otherwise when left out.
func number(args map[string]any, key string, otherwise int) (int, error) {
@@ -148,22 +141,26 @@ func (b *Bundle) Tools() []stdio.Tool {
}),
Run: b.groupsTool},
{Name: CorrectTool,
Description: "Correct one mail entry's mark, why or due date (novox/hq ADR 0459 §8): kept at once as " +
"a reading lesson (its headers, the change, when), no operator Approve needed. Answered only to a " +
"caller the setting callers names.",
Description: "Correct one mail entry's mark, mail group, why or due date (novox/hq ADR 0459 §8): " +
"kept at once as a reading lesson (the mail's own headers, the change made, when), no operator " +
"Approve needed, and outranking that mail's own words in any later batch. Refused for an unknown " +
"mail id or mark. Answered only to a caller the setting callers names.",
Input: schema([]string{"id"}, map[string]any{
"id": integer("the mail entry's id, as entries answered it"),
"mark": text("the corrected mark: matters, scam warning or neither"),
"group": text("the corrected mail group, by its label (made if none yet carries it)"),
"why": text("the corrected reason, one line"),
"due-date": text("the corrected due date (YYYY-MM-DD), or empty to clear it"),
}),
Run: b.correctTool},
{Name: LessonsTool,
Description: "The reading lessons kept (novox/hq ADR 0459 §9): at most 40, the sender or group's own " +
"first, then newest; a lesson outranks a mail it would otherwise contradict. Answered only to a " +
"caller the setting callers names. Read-only.",
Description: "The reading lessons kept (novox/hq ADR 0459 §9): newest first, at most 40 (fewer when " +
"asked); undone ones left out unless include-undone asks for them, each then marked undone. A " +
"lesson outranks a mail it would otherwise contradict. Answered only to a caller the setting " +
"callers names. Read-only.",
Input: schema(nil, map[string]any{
"limit": integer("how many to answer; a default bounds it, at most 40, when left out"),
"limit": integer("how many to answer, at most 40; a default bounds it when left out"),
"include-undone": boolean("also answer undone lessons, each marked undone; left out by default"),
}),
Run: b.lessonsTool},
{Name: UndoTool,
@@ -287,24 +284,162 @@ func (b *Bundle) groupsTool(args map[string]any) (any, error) {
return map[string]any{"groups": out}, nil
}
// 1428/AC5: correct refuses an unknown mark, before the store is ever written to.
func validMark(s string) (Mark, error) {
m := Mark(s)
if m != MarkMatters && m != MarkScamWarning && m != MarkNeither {
return "", fmt.Errorf("%s needs mark to be matters, scam warning or neither", CorrectTool)
}
return m, nil
}
func (b *Bundle) correctTool(args map[string]any) (any, error) {
if _, err := b.checkCaller(args); err != nil {
return nil, err
}
if err := only(args, "id", "mark", "why", "due-date"); err != nil {
if err := only(args, "id", "mark", "group", "why", "due-date"); err != nil {
return nil, err
}
return nil, notBuiltYet(CorrectTool, "1428")
id, err := number(args, "id", 0)
if err != nil {
return nil, err
}
if id <= 0 {
return nil, fmt.Errorf("%s needs id", CorrectTool)
}
ctx := context.Background()
store, err := b.Store(ctx)
if err != nil {
return nil, err
}
entry, ok, err := store.Entry(ctx, int64(id))
if err != nil {
return nil, err
}
if !ok {
// 1428/AC5: an unknown mail id is refused.
return nil, fmt.Errorf("%s: no mail entry %d is held; entries answers the ids held", CorrectTool, id)
}
var c Correction
if markStr, has := args["mark"]; has {
s, _ := markStr.(string)
m, err := validMark(s)
if err != nil {
return nil, err
}
c.Mark = m
}
labels, err := groupLabels(ctx, store)
if err != nil {
return nil, err
}
oldGroupLabel := labels[entry.Group]
if groupStr, has := args["group"]; has {
c.GroupLabel, _ = groupStr.(string)
if c.GroupLabel != "" {
g, err := store.FindOrCreateGroup(ctx, c.GroupLabel)
if err != nil {
return nil, err
}
c.GroupID = g.ID
}
}
if whyStr, has := args["why"]; has {
c.Why, _ = whyStr.(string)
c.WhySet = true
}
if dueStr, has := args["due-date"]; has {
s, _ := dueStr.(string)
if s != "" {
d, err := time.Parse("2006-01-02", s)
if err != nil {
return nil, fmt.Errorf("%s needs due-date to be YYYY-MM-DD, or empty to clear it", CorrectTool)
}
c.DueDate = d
}
c.DueDateSet = true
}
updated, change := ApplyCorrection(entry, c, oldGroupLabel)
if change == "" {
return nil, fmt.Errorf("%s needs a mark, mail group, why or due-date that actually changes the entry", CorrectTool)
}
if err := store.UpdateEntry(ctx, updated); err != nil {
return nil, err
}
// Kept at once, no operator Approve (ADR 0459 §8): the mail's own headers, never its body, the change made
// and when. 1428/AC1.
lesson := Lesson{
Account: entry.Account, Folder: entry.Folder, SenderName: entry.SenderName, SenderAddress: entry.SenderAddress,
Subject: entry.Subject, Group: updated.Group, Change: change, Made: time.Now(),
}
if _, err := store.AddLesson(ctx, lesson); err != nil {
return nil, err
}
newLabels, err := groupLabels(ctx, store)
if err != nil {
return nil, err
}
return entryOut(updated, newLabels), nil
}
func groupLabels(ctx context.Context, store Store) (map[int64]string, error) {
groups, err := store.Groups(ctx)
if err != nil {
return nil, err
}
out := map[int64]string{}
for _, g := range groups {
out[g.ID] = g.Label
}
return out, nil
}
func (b *Bundle) lessonsTool(args map[string]any) (any, error) {
if _, err := b.checkCaller(args); err != nil {
return nil, err
}
if err := only(args, "limit"); err != nil {
if err := only(args, "limit", "include-undone"); err != nil {
return nil, err
}
return nil, notBuiltYet(LessonsTool, "1428")
limit, err := number(args, "limit", MaxLessonsInBatch)
if err != nil {
return nil, err
}
if limit <= 0 || limit > MaxLessonsInBatch {
limit = MaxLessonsInBatch
}
includeUndone, _ := args["include-undone"].(bool)
ctx := context.Background()
store, err := b.Store(ctx)
if err != nil {
return nil, err
}
lessons, err := store.Lessons(ctx)
if err != nil {
return nil, err
}
var kept []Lesson
for _, l := range lessons {
if l.Undone && !includeUndone {
continue
}
kept = append(kept, l)
}
sort.SliceStable(kept, func(i, j int) bool { return kept[i].Made.After(kept[j].Made) }) // 1428/AC2: newest first
if len(kept) > limit {
kept = kept[:limit]
}
out := make([]map[string]any, len(kept))
for i, l := range kept {
out[i] = map[string]any{
"id": l.ID, "account": l.Account, "folder": l.Folder, "sender-name": l.SenderName,
"sender-address": l.SenderAddress, "subject": l.Subject, "change": l.Change,
"made": l.Made.UTC().Format(time.RFC3339), "undone": l.Undone,
}
}
return map[string]any{"lessons": out}, nil
}
func (b *Bundle) undoTool(args map[string]any) (any, error) {
@@ -314,7 +449,27 @@ func (b *Bundle) undoTool(args map[string]any) (any, error) {
if err := only(args, "id"); err != nil {
return nil, err
}
return nil, notBuiltYet(UndoTool, "1428")
id, err := number(args, "id", 0)
if err != nil {
return nil, err
}
if id <= 0 {
return nil, fmt.Errorf("%s needs id", UndoTool)
}
ctx := context.Background()
store, err := b.Store(ctx)
if err != nil {
return nil, err
}
// 1428/AC3: undo removes the lesson from what later batches are given (selectLessons, question.go, skips
// an Undone lesson) and leaves the corrected entry exactly as it is — nothing here touches mail_entry.
if err := store.UndoLesson(ctx, int64(id)); err != nil {
if err == ErrNotFound {
return nil, fmt.Errorf("%s: no reading lesson %d is held; lessons answers the ids held", UndoTool, id)
}
return nil, err
}
return map[string]any{"undone": id}, nil
}
func (b *Bundle) statusTool(args map[string]any) (any, error) {
@@ -34,9 +34,9 @@ func TestToolsListsTheSixVerbsAndHealth(t *testing.T) {
}
}
// 1426/AC4, 1521/AC4: a mail-reading verb answers a caller the setting callers names, and refuses any other,
// naming why. entries, groups and status now answer for real (1521); correct, lessons and undo still answer
// not built yet (1428).
// 1426/AC4, 1428/AC5: a mail-reading verb answers a caller the setting callers names, and refuses any other,
// naming why — before anything else about the verb (business errors such as an unknown id are not caller
// refusals, and are exercised by correct_test.go and 1521's own tests, not here).
func TestAMailReadingVerbChecksItsCaller(t *testing.T) {
granted := func(map[string]any) string { return "mesh-mail-web" }
refused := func(map[string]any) string { return "person.someoneelse" }
@@ -44,27 +44,24 @@ func TestAMailReadingVerbChecksItsCaller(t *testing.T) {
storeOf := func(context.Context) (Store, error) { return newMemStore(), nil }
for _, tool := range []struct {
name string
run func(*Bundle) (any, error)
built bool // whether its issue has landed: true answers for real, false answers "not built yet"
name string
run func(*Bundle) (any, error)
}{
{EntriesTool, func(b *Bundle) (any, error) { return b.entriesTool(map[string]any{}) }, true},
{GroupsTool, func(b *Bundle) (any, error) { return b.groupsTool(map[string]any{}) }, true},
{CorrectTool, func(b *Bundle) (any, error) { return b.correctTool(map[string]any{"id": float64(1)}) }, false},
{LessonsTool, func(b *Bundle) (any, error) { return b.lessonsTool(map[string]any{}) }, false},
{UndoTool, func(b *Bundle) (any, error) { return b.undoTool(map[string]any{"id": float64(1)}) }, false},
{StatusTool, func(b *Bundle) (any, error) { return b.statusTool(map[string]any{}) }, true},
{EntriesTool, func(b *Bundle) (any, error) { return b.entriesTool(map[string]any{}) }},
{GroupsTool, func(b *Bundle) (any, error) { return b.groupsTool(map[string]any{}) }},
{CorrectTool, func(b *Bundle) (any, error) {
return b.correctTool(map[string]any{"id": float64(1), "mark": "matters"})
}},
{LessonsTool, func(b *Bundle) (any, error) { return b.lessonsTool(map[string]any{}) }},
{UndoTool, func(b *Bundle) (any, error) { return b.undoTool(map[string]any{"id": float64(1)}) }},
{StatusTool, func(b *Bundle) (any, error) { return b.statusTool(map[string]any{}) }},
} {
callers := func() (Callers, error) { return Callers{"mesh-mail-web"}, nil }
b := &Bundle{Caller: granted, callersOverride: callers, Store: storeOf}
_, err := tool.run(b)
if tool.built {
if err != nil {
t.Errorf("%s granted: %v, want it to pass the caller check and answer", tool.name, err)
}
} else if err == nil || !strings.Contains(err.Error(), "not built yet") {
t.Errorf("%s granted: %v, want it to pass the caller check and answer not built yet", tool.name, err)
if err != nil && strings.Contains(err.Error(), "is not named in the setting callers") {
t.Errorf("%s granted: %v, want it to pass the caller check (any other error is its own business, not a caller refusal)", tool.name, err)
}
b = &Bundle{Caller: refused, callersOverride: callers, Store: storeOf}