issue 1857: judge at most 6 mails a batch (1857.3)

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.
This commit is contained in:
jochen
2026-10-22 19:41:17 +02:00
parent c3b504e26e
commit 6b759f708e
6 changed files with 62 additions and 12 deletions
+1 -1
View File
@@ -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
@@ -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
@@ -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
@@ -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 {
@@ -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)
}
}
}
@@ -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 {