From 6b759f708e0490ebd2d0bf0850934fe439b0f57b Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 19:41:17 +0200 Subject: [PATCH] issue 1857: judge at most 6 mails a batch (1857.3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR 0516 §3: the answer grows by each mail's tasks and promises, so a batch holds 6 mails, not 10, to stay within its 40 s; a test proves the largest question stays under 64 KiB with every mail whole. --- modules/mesh-mail-reader/README.md | 2 +- .../cmd/mesh-mail-reader/batch.go | 2 +- .../mesh-mail-reader/batch_gap_integration.go | 2 +- .../cmd/mesh-mail-reader/batch_test.go | 14 +++--- .../cmd/mesh-mail-reader/batchsize_test.go | 50 +++++++++++++++++++ .../cmd/mesh-mail-reader/question.go | 4 +- 6 files changed, 62 insertions(+), 12 deletions(-) create mode 100644 modules/mesh-mail-reader/cmd/mesh-mail-reader/batchsize_test.go diff --git a/modules/mesh-mail-reader/README.md b/modules/mesh-mail-reader/README.md index c6e06e2d..48350f75 100644 --- a/modules/mesh-mail-reader/README.md +++ b/modules/mesh-mail-reader/README.md @@ -11,7 +11,7 @@ account never does. It reads mail; it moves, flags, deletes and sends nothing (A mail seats, serving six verbs: `entries`, `groups`, `correct`, `lessons`, `undo`, `status` (ADR 0459 §2). Each answers only a caller the trusted setting `callers` names (default `["mesh-mail-web"]`); any other caller is refused, naming why. -- **Hears mesh-mail's added events** and judges a batch of up to 10 at a time against a question put to +- **Hears mesh-mail's added events** and judges a batch of up to 6 at a time (ADR 0516 §3) against a question put to `claude_code_judge`, keeping an entry per mail — never its body — in its own database (issue 1518, 1427; not yet built). - **Keeps a health of its own**, `mail_reader_health`: unhealthy once a judgement is refused, times out or diff --git a/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch.go b/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch.go index 6b94a872..bd40d673 100644 --- a/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch.go +++ b/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch.go @@ -294,7 +294,7 @@ func RunOneBatch(ctx context.Context, store Store, readText ReadTextFunc, judge // time, and after each one — win or lose — try the mail signal (SendSignal, tell.go, issue 1522; it is its // own no-op when there is nothing untold that matters, or the reader is unhealthy) — forever, until ctx is // done. gap is given explicitly (main.go's batchLoop passes the var BatchGap, which only a `-tags -// integration` build ever shortens; TestThirtyWaitingMailsRunAsThreeBatchesAtLeast160sApart passes +// integration` build ever shortens; TestThirtyWaitingMailsRunAsFiveBatchesAtLeast160sApart passes // MinBatchGap) so this function itself never reads that var — the try on 8000b637 found this test still // reading it (and so, compiled into the integration binary, pacing at the shortened 3s instead of 160s) // when RunBatches read BatchGap directly inside its own loop (review round 2, issue 1520). 1520/AC2-AC5's diff --git a/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch_gap_integration.go b/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch_gap_integration.go index 96a281a4..698b36ef 100644 --- a/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch_gap_integration.go +++ b/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch_gap_integration.go @@ -9,7 +9,7 @@ import "time" // operator or the manifest could change (novox/hq ADR 0459 §15 still holds there: at least 160s). This is // what lets an integration test (acceptance_integration_test.go, catchup_integration_test.go) exercise the // reader's real pacing loop without actually waiting out 160s for each batch; a unit test on a test clock -// (batch_test.go's TestNextBatchWait, TestThirtyWaitingMailsRunAsThreeBatchesAtLeast160sApart) still proves +// (batch_test.go's TestNextBatchWait, TestThirtyWaitingMailsRunAsFiveBatchesAtLeast160sApart) still proves // the real 160s value, unaffected by this file, which they are never built with. // // issue 1431 (named by the lead's merge-check feedback, which also named 1524 and 1517): the earlier diff --git a/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch_test.go b/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch_test.go index c882125b..21bfe180 100644 --- a/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch_test.go +++ b/modules/mesh-mail-reader/cmd/mesh-mail-reader/batch_test.go @@ -63,13 +63,13 @@ func TestNextBatchWait(t *testing.T) { } // 1520/AC1 (review round 1 finding 5): with 30 mails waiting, RunBatches ITSELF — never a hand-driven copy -// of its own loop — runs three batches of 10, their starts at least MinBatchGap (160s) apart. A test clock: +// of its own loop — runs five batches of 6 (MaxMailsPerBatch, ADR 0516 §3), their starts at least MinBatchGap (160s) apart. A test clock: // now reads a clock this test owns, and sleep (the only thing RunBatches calls to wait) advances that same // clock instead of actually waiting 320s+ of wall time. The judge stand-in records when each batch was // judged (synchronous with RunOneBatch's own lastStart, since nothing sleeps between them) and cancels ctx -// once the 3rd has run, so RunBatches returns on its own next turn through the loop — this fails the moment +// once the 5th has run, so RunBatches returns on its own next turn through the loop — this fails the moment // RunBatches stops pacing, not only when nextBatchWait's own math would (TestNextBatchWait, above). -func TestThirtyWaitingMailsRunAsThreeBatchesAtLeast160sApart(t *testing.T) { +func TestThirtyWaitingMailsRunAsFiveBatchesAtLeast160sApart(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() store := newMemStore() @@ -112,7 +112,7 @@ func TestThirtyWaitingMailsRunAsThreeBatchesAtLeast160sApart(t *testing.T) { mails[i] = map[string]any{"id": id, "mark": "neither"} } raw, _ := json.Marshal(map[string]any{"mails": mails}) - if len(starts) == 3 { + if len(starts) == 30/MaxMailsPerBatch { cancel() } return raw, OutcomeOK, "" @@ -128,11 +128,11 @@ func TestThirtyWaitingMailsRunAsThreeBatchesAtLeast160sApart(t *testing.T) { select { case <-done: case <-time.After(5 * time.Second): - t.Fatal("RunBatches did not stop after its own 3rd batch and this test's cancel") + t.Fatal("RunBatches did not stop after its own 5th batch and this test's cancel") } - if len(starts) != 3 { - t.Fatalf("judge was called %d time(s), want 3 (30 mails / %d per batch)", len(starts), MaxMailsPerBatch) + if len(starts) != 5 { + t.Fatalf("judge was called %d time(s), want 5 (30 mails / %d per batch)", len(starts), MaxMailsPerBatch) } for i := 1; i < len(starts); i++ { if gap := starts[i].Sub(starts[i-1]); gap < MinBatchGap { diff --git a/modules/mesh-mail-reader/cmd/mesh-mail-reader/batchsize_test.go b/modules/mesh-mail-reader/cmd/mesh-mail-reader/batchsize_test.go new file mode 100644 index 00000000..610f18fe --- /dev/null +++ b/modules/mesh-mail-reader/cmd/mesh-mail-reader/batchsize_test.go @@ -0,0 +1,50 @@ +package main + +// Issue 1857 (novox/hq ADR 0516 §3): a batch holds at most 6 mails, so the judgement's longer answer stays +// within its 40 s; each part of the question keeps its bound, so a question stays under 64 KiB. + +import ( + "context" + "encoding/json" + "strings" + "testing" + "time" +) + +// 1857/AC4: of 9 waiting mails a batch takes 6, judges them, and leaves 3 waiting. +func TestABatchHoldsSixMails(t *testing.T) { + ctx := context.Background() + store := newMemStore() + seedWaiting(t, store, 9) + waiting, _ := store.Waiting(ctx) + asked := 0 + judge := func(question string, _ map[string]any) (json.RawMessage, HealthOutcome, string) { + asked = strings.Count(question, "\nmail ") + return judgeAllNeither(waiting[:min(asked, len(waiting))])(question, nil) + } + at := func() time.Time { return time.Date(2026, 10, 22, 10, 0, 0, 0, time.UTC) } + if err := RunOneBatch(ctx, store, noopReadText, judge, waiting, at, noopLogf); err != nil { + t.Fatal(err) + } + entries, _ := store.Entries(ctx) + left, _ := store.Waiting(ctx) + if asked != 6 || len(entries) != 6 || len(left) != 3 { + t.Fatalf("the question held %d mails, %d entries kept, %d left waiting; want 6, 6, 3", asked, len(entries), len(left)) + } +} + +// 1857/AC4: with every part at its largest — the framing, groups and lessons overfull, 6 mails each at its +// own whole bound — the question is under 64 KiB and still holds all 6 mails whole: no part is cut by the +// question's own bound. +func TestTheLargestQuestionStaysUnder64KiB(t *testing.T) { + mails := manyMails(MaxMailsPerBatch, 10000) + q := BuildQuestion(mails, manyGroups(500), manyLessons(500)) + if len(q.Text) >= 64<<10 { + t.Fatalf("the largest question is %d bytes, want under 64 KiB", len(q.Text)) + } + for _, m := range mails { + if !strings.Contains(q.Text, mailBlock(m)) { + t.Fatalf("mail %d is not in the question whole: the question's bound cut a part", m.ID) + } + } +} diff --git a/modules/mesh-mail-reader/cmd/mesh-mail-reader/question.go b/modules/mesh-mail-reader/cmd/mesh-mail-reader/question.go index 81175222..3fc0d13b 100644 --- a/modules/mesh-mail-reader/cmd/mesh-mail-reader/question.go +++ b/modules/mesh-mail-reader/cmd/mesh-mail-reader/question.go @@ -17,7 +17,7 @@ import ( // Budgets (ADR 0459 §5), each a byte count on the UTF-8 text, never split mid-rune. const ( - MaxMailsPerBatch = 10 + MaxMailsPerBatch = 6 // ADR 0516 §3 (was 10), issue 1857 FramingBudget = 4 << 10 // 4 KiB GroupsBudget = 4 << 10 // 4 KiB LessonsBudget = 12 << 10 // 12 KiB @@ -230,7 +230,7 @@ type Question struct { // BuildQuestion assembles one batch's question (1427/AC1): framing, the existing mail groups (largest first), // the applicable reading lessons, then at most MaxMailsPerBatch mails (mails beyond that are simply not asked -// about here — the caller, 1520, is the one that takes at most 10 waiting mails to begin with). The whole +// about here — the caller, 1520, is the one that takes at most MaxMailsPerBatch waiting mails to begin with). The whole // text is bounded to QuestionBudget, dropping from the end (the mails, last) first, never splitting a mail's // own block or a mid-rune boundary. func BuildQuestion(mails []QuestionMail, groups []GroupSummary, lessons []Lesson) Question {