Review a long record within a bound sized to it, and let a proposal waiting on its author leave the knowledge base healthy (hq issue 508)

A judgement over one bus call's 30 seconds is collected by its ticket from
claude-code, so an eleven-clause record can be reviewed; a proposal out of
review tries is told to the operator, not raised as the module's fault, so
it no longer holds the module's own rollouts.

modules/mesh-knowledge/README.md

@@ -99,7 +99,10 @@ as text. `get` serves each, and any other field a record of hq wrote under `fiel
    the record, the records it links and the rules, and answers `approve` or `changes` with findings. The rules it sees
    name every kind of link `propose` accepts, from the one list `propose` holds a link to, and the record it sees
    shows its state in the knowledge base apart from a research effort's own status (hq issue 492). The background
-   work asks it at once, before the next step of any drafting, one judgement at a time.
+   work asks it at once, before the next step of any drafting, one judgement at a time. Every judgement asks a bound
+   sized to its question: 25 seconds and 2 more per KiB, at most 120 (hq issue 508: an eleven-clause record was never
+   reviewed in 25). A judgement not done within one bus call is answered pending with a ticket, and collected from
+   the same node with `claude-code.claude_code_judged`.
    - On `changes` the record stays proposed, the findings kept with the version they judged (`get` shows them).
      Its author revises it by proposing it again, and that version is reviewed again.
    - A judgement that cannot be had (the judge unreachable, or an answer that is not `approve`, or `changes` naming
@@ -108,8 +111,9 @@ as text. `get` serves each, and any other field a record of hq wrote under `fiel
      version in the store and said in its history, so the background work takes it up at that time, also after a
      restart of the tool runner (hq issue 497). After 3 tries the record says it was the last and its author
      proposes it again to have it reviewed again; the operator is told once on the operator's channel (keyed by
-     record and version, and cleared once the record is proposed again, accepted, withdrawn or superseded), and
-     the module is unhealthy, naming it, so the mesh raises a condition.
+     record and version, cleared once the record is proposed again, accepted, withdrawn or superseded, and sent
+     again at the next step when it could not be sent). The module's health names the record and stays healthy:
+     the record waits on its author, not on the module, so it never holds the module's own rollouts (hq issue 508).
      An ask of the operator that cannot be made counts as a try the same way.
    - A review whose outcome the database refused to keep is held back for that record alone (10, then 20
      minutes, 3 tries, logged as given up), so it never blocks the records behind it.

modules/mesh-knowledge/cmd/mesh-knowledge/judge_bound_test.go

@@ -0,0 +1,113 @@
+package main
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"strings"
+	"testing"
+	"time"
+)
+
+// novox/hq issue 508: ADR 0297, eleven clauses and eleven links, could not be reviewed within the judgement's 25
+// seconds five times running, while short records were reviewed in 14 to 25. A review's bound scales with what is
+// judged, up to a cap, and a judgement past one bus call's time is collected by its ticket.
+
+// aRecord is a decision of n clauses, each about size bytes.
+func aRecord(n, size int) *Record {
+	r := &Record{Kind: KindDecision, Number: 297, Title: "Issues are kept as a log of operations", Status: StatusProposed}
+	for i := 1; i <= n; i++ {
+		r.Clauses = append(r.Clauses, Clause{N: i, Text: fmt.Sprintf("**Clause %d.** %s", i, strings.Repeat("A word of reasoning. ", size/20))})
+	}
+	r.Sections = []Section{{Name: "Context", Text: strings.Repeat("Why it was decided. ", n*size/40)}}
+	return r
+}
+
+// asked is the agent module, played over the calls the knowledge base makes to it.
+type asked struct {
+	keys    []string
+	bodies  []map[string]any
+	pending int // how many times a ticketed judgement answers pending before its answer
+	refuse  bool
+}
+
+func (a *asked) ask(key string, body any) (json.RawMessage, error) {
+	b, _ := body.(map[string]any)
+	a.keys, a.bodies = append(a.keys, key), append(a.bodies, b)
+	if strings.HasPrefix(key, "claude-code.claude_code_judge@") {
+		if _, long := b["within"]; long && a.refuse {
+			return nil, errors.New("claude_code_judge takes question, schema and model; not within")
+		}
+		if _, long := b["within"]; long && a.pending > 0 {
+			return json.RawMessage(`{"node":"novox","ticket":"t1","pending":true}`), nil
+		}
+		return json.RawMessage(`{"answer":{"verdict":"approve","findings":[]},"model":"sonnet","node":"novox"}`), nil
+	}
+	if key == "claude-code.claude_code_judged@novox" && b["ticket"] == "t1" {
+		if a.pending--; a.pending > 0 {
+			return json.RawMessage(`{"node":"novox","ticket":"t1","pending":true}`), nil
+		}
+		return json.RawMessage(`{"answer":{"verdict":"approve","findings":[]},"model":"sonnet","node":"novox","ticket":"t1"}`), nil
+	}
+	return nil, fmt.Errorf("nothing answers %s", key)
+}
+
+func withinOf(t *testing.T, b map[string]any) time.Duration {
+	t.Helper()
+	v, ok := b["within"]
+	if !ok {
+		return 0
+	}
+	s, ok := v.(int)
+	if !ok {
+		t.Fatalf("within is not whole seconds: %#v", v)
+	}
+	return time.Duration(s) * time.Second
+}
+
+func TestALongRecordsReviewGetsTheLargerBound(t *testing.T) {
+	short, long := ReviewQuestion(aRecord(2, 200), nil), ReviewQuestion(aRecord(12, 2000), nil)
+	a := &asked{}
+	j := judgementOver(a.ask, "novox")
+	for _, q := range []string{short, long} {
+		if _, _, err := j(q, reviewSchema, "sonnet"); err != nil {
+			t.Fatal(err)
+		}
+	}
+	s, l := withinOf(t, a.bodies[0]), withinOf(t, a.bodies[1])
+	if l <= s || l <= 25*time.Second || l > MostJudgeWithin {
+		t.Fatalf("a %d-byte review is bounded %s and a %d-byte one %s: the longer must get the larger bound, past 25s and at most %s",
+			len(short), s, len(long), l, MostJudgeWithin)
+	}
+	if got := JudgeWithin(strings.Repeat("x", MostReviewQuestion)); got != MostJudgeWithin {
+		t.Fatalf("the largest question is bounded %s, not the cap %s", got, MostJudgeWithin)
+	}
+}
+
+// A judgement answered pending is collected by its ticket from the node that holds it, and its answer is the
+// judgement's.
+func TestAPendingJudgementIsCollectedByItsTicket(t *testing.T) {
+	a := &asked{pending: 3}
+	answer, by, err := judgementOver(a.ask, "novox")(ReviewQuestion(aRecord(12, 2000), nil), reviewSchema, "sonnet")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(answer) != `{"verdict":"approve","findings":[]}` || by != "the agent on novox (sonnet)" {
+		t.Fatalf("%s by %s", answer, by)
+	}
+	if strings.Join(a.keys, " ") != "claude-code.claude_code_judge@novox claude-code.claude_code_judged@novox claude-code.claude_code_judged@novox claude-code.claude_code_judged@novox" {
+		t.Fatalf("asked %q", a.keys)
+	}
+}
+
+// An agent module from before issue 508, which takes no within, is asked again without it: the review is had when
+// it fits the old bound, and fails as before when it does not.
+func TestAnAgentModuleThatTakesNoWithinIsAskedWithoutIt(t *testing.T) {
+	a := &asked{refuse: true}
+	if _, _, err := judgementOver(a.ask, "novox")(ReviewQuestion(aRecord(12, 2000), nil), reviewSchema, "sonnet"); err != nil {
+		t.Fatal(err)
+	}
+	if len(a.bodies) != 2 || withinOf(t, a.bodies[1]) != 0 {
+		t.Fatalf("asked %v", a.bodies)
+	}
+}

modules/mesh-knowledge/cmd/mesh-knowledge/main.go

@@ -202,25 +202,76 @@ func busReserver(kind, title string) (int, error) {
 }
 
 // busJudgement asks the agent module on this node for one judgement (ADR 0278).
-func busJudgement(node string) Judgement {
+func busJudgement(node string) Judgement { return judgementOver(stdio.Ask, node) }
+
+// The bound a judgement asks scales with its question (novox/hq issue 508): ADR 0297, eleven clauses and eleven
+// links, was never reviewed within the agent module's 25 seconds, while short records took 14 to 25. Each KiB of
+// question adds JudgePerKiB to JudgeFloor, up to MostJudgeWithin, which the agent module takes at most. A sized
+// bound was chosen over asking clause by clause: every judgement pays the agent's start, which alone is most of
+// those 14 to 25 seconds, so a dozen clause-sized judgements would each risk the same 25 seconds, take minutes
+// together, and still need a judgement of the whole record for what binds its clauses together.
+const (
+	JudgeFloor      = 25 * time.Second
+	JudgePerKiB     = 2 * time.Second
+	MostJudgeWithin = 120 * time.Second
+)
+
+// JudgeWithin is the bound a judgement of this question asks.
+func JudgeWithin(question string) time.Duration {
+	return min(JudgeFloor+time.Duration(len(question)>>10)*JudgePerKiB, MostJudgeWithin)
+}
+
+// judgementOver asks the agent module through ask. A bound past one bus call's 30 seconds is answered pending with
+// a ticket when the judgement is not done in the call, and collected from the node that holds it with
+// claude_code_judged (novox/hq issue 508).
+func judgementOver(ask func(key string, body any) (json.RawMessage, error), node string) Judgement {
 	return func(question string, schema map[string]any, model string) (json.RawMessage, string, error) {
 		key := "claude-code.claude_code_judge"
 		if node != "" {
 			key += "@" + node
 		}
-		raw, err := stdio.Ask(key, map[string]any{"question": question, "schema": schema, "model": model})
-		if err != nil {
-			return nil, "", err
+		within := JudgeWithin(question)
+		body := map[string]any{"question": question, "schema": schema, "model": model}
+		if within > JudgeFloor {
+			body["within"] = int(within / time.Second)
 		}
-		var out struct {
-			Answer json.RawMessage `json:"answer"`
-			Model  string          `json:"model"`
-			Node   string          `json:"node"`
+		raw, err := ask(key, body)
+		if err != nil && body["within"] != nil && strings.Contains(err.Error(), "not within") {
+			// An agent module from before issue 508 takes no within: asked again without it, under its own bound.
+			delete(body, "within")
+			raw, err = ask(key, body)
 		}
-		if err := json.Unmarshal(raw, &out); err != nil || len(out.Answer) == 0 {
-			return nil, "", fmt.Errorf("the agent module's answer is not a judgement: %.200s", raw)
+		// Past the judgement's bound the agent module ends it and says so; the deadline only guards against a
+		// module that answers pending for ever.
+		deadline := time.Now().Add(within + time.Minute)
+		for {
+			if err != nil {
+				return nil, "", err
+			}
+			var out struct {
+				Answer  json.RawMessage `json:"answer"`
+				Model   string          `json:"model"`
+				Node    string          `json:"node"`
+				Ticket  string          `json:"ticket"`
+				Pending bool            `json:"pending"`
+			}
+			if err := json.Unmarshal(raw, &out); err != nil {
+				return nil, "", fmt.Errorf("the agent module's answer is not a judgement: %.200s", raw)
+			}
+			if !out.Pending {
+				if len(out.Answer) == 0 {
+					return nil, "", fmt.Errorf("the agent module's answer is not a judgement: %.200s", raw)
+				}
+				return out.Answer, fmt.Sprintf("the agent on %s (%s)", out.Node, out.Model), nil
+			}
+			if out.Ticket == "" || out.Node == "" {
+				return nil, "", fmt.Errorf("the agent module answered pending with no ticket or node: %.200s", raw)
+			}
+			if time.Now().After(deadline) {
+				return nil, "", fmt.Errorf("the agent on %s still had not answered %s after its bound of %s", out.Node, out.Ticket, within)
+			}
+			raw, err = ask("claude-code.claude_code_judged@"+out.Node, map[string]any{"ticket": out.Ticket})
 		}
-		return out.Answer, fmt.Sprintf("the agent on %s (%s)", out.Node, out.Model), nil
 	}
 }
 
@@ -501,9 +552,10 @@ type HealthAnswer struct {
 	At      time.Time `json:"at,omitempty"`
 }
 
-// Health is how the knowledge base is: unhealthy when its database cannot serve, when a rule that judges the
-// whole record set fails, or when a proposed record ran out of review tries — the mesh raises a condition naming
-// the rule, the records, or the records not reviewed.
+// Health is whether the knowledge base works: unhealthy when its database cannot serve, or when a rule that judges
+// the whole record set fails — the mesh raises a condition naming the rule and the records. A proposal that ran out
+// of review tries waits on its author, not on the module: it is told to its author and the operator, and named
+// here, but leaves the module healthy, so it never holds the module's own rollouts (novox/hq issue 508).
 func (k *KB) Health(ready Healther) HealthAnswer {
 	ctx, cancel := context.WithTimeout(context.Background(), 8*time.Second)
 	defer cancel()
@@ -525,33 +577,38 @@ func (k *KB) Health(ready Healther) HealthAnswer {
 	if err != nil {
 		return HealthAnswer{State: "unhealthy", Why: "the records cannot be read: " + err.Error()}
 	}
-	unreviewed := Unreviewed(all)
-	if len(findings)+len(unreviewed) > 0 {
-		var parts []string
-		if len(unreviewed) > 0 {
-			if len(unreviewed) > 8 {
-				unreviewed = append(unreviewed[:8], fmt.Sprintf("and %d more", len(unreviewed)-8))
-			}
-			parts = append(parts, fmt.Sprintf("%d proposed record(s) could not be reviewed in %d tries and wait for their authors to propose them again: %s",
-				len(Unreviewed(all)), MostReviewTries, strings.Join(unreviewed, "; ")))
+	waiting := ""
+	if unreviewed := Unreviewed(all); len(unreviewed) > 0 {
+		n := len(unreviewed)
+		if n > 8 {
+			unreviewed = append(unreviewed[:8], fmt.Sprintf("and %d more", n-8))
 		}
-		if len(says) > 0 {
-			if len(says) > 8 {
-				says = append(says[:8], fmt.Sprintf("and %d more", len(says)-8))
-			}
-			parts = append(parts, fmt.Sprintf("%d rule(s) that judge the whole record set fail: %s", len(findings), strings.Join(says, "; ")))
-		}
-		return HealthAnswer{State: "unhealthy", Why: strings.Join(parts, ". ")}
+		waiting = fmt.Sprintf("%d proposed record(s) could not be reviewed in %d tries and wait for their authors to propose them again: %s",
+			n, MostReviewTries, strings.Join(unreviewed, "; "))
 	}
-	return HealthAnswer{State: "healthy", Healthy: true, Why: "the database serves, every rule that judges the whole record set holds, and no proposal ran out of review tries"}
+	if len(says) > 0 {
+		if len(says) > 8 {
+			says = append(says[:8], fmt.Sprintf("and %d more", len(says)-8))
+		}
+		why := fmt.Sprintf("%d rule(s) that judge the whole record set fail: %s", len(findings), strings.Join(says, "; "))
+		if waiting != "" {
+			why += ". " + waiting
+		}
+		return HealthAnswer{State: "unhealthy", Why: why}
+	}
+	why := "the database serves and every rule that judges the whole record set holds"
+	if waiting != "" {
+		why += "; " + waiting
+	}
+	return HealthAnswer{State: "healthy", Healthy: true, Why: why}
 }
 
 func healthTool(status string, now func() time.Time) stdio.Tool {
 	return stdio.Tool{Name: HealthTool,
 		Description: "How the knowledge base is, as its watch last saw it: healthy when its database serves this version and every " +
 			"rule that judges the whole record set holds; unhealthy otherwise, naming the rule and the records — a decision accepted " +
-			"that nothing cites, a supersession or a change said on one end only, a record extending a superseded one — or when a " +
-			"proposed record ran out of review tries, naming it.",
+			"that nothing cites, a supersession or a change said on one end only, a record extending a superseded one. A " +
+			"proposed record that ran out of review tries is named but leaves it healthy: it waits on its author (novox/hq issue 508).",
 		Run: func(map[string]any) (any, error) { return ReadStatus(status, now()), nil }}
 }
 

modules/mesh-knowledge/cmd/mesh-knowledge/manifest_test.go

@@ -76,7 +76,7 @@ func TestItDeclaresAndHoldsTheKnowledgeBaseSeatOnceForTheMesh(t *testing.T) {
 	if !reflect.DeepEqual(m.Uses, []string{"operator-channel"}) || len(m.Consumes) != 0 {
 		t.Fatalf("uses %v, consumes %v", m.Uses, m.Consumes)
 	}
-	if !reflect.DeepEqual(m.Invokes, []string{"seat:issue-tracker.reserve", "claude-code.claude_code_judge", "records.records_status",
+	if !reflect.DeepEqual(m.Invokes, []string{"seat:issue-tracker.reserve", "claude-code.claude_code_judge", "claude-code.claude_code_judged", "records.records_status",
 		"records.records_list", "records.records_read", "seat:operator-channel.notify"}) {
 		t.Fatalf("invokes %v", m.Invokes)
 	}

modules/mesh-knowledge/cmd/mesh-knowledge/review.go

@@ -12,8 +12,9 @@ package main
 //     and says so on it. It is never taken as an approval. It is owed: the try count and when the next try is due
 //     are kept with the version in the store, so the background work takes it up at that time, also after a restart
 //     of the tool runner (novox/hq issue 497), up to MostReviewTries. After the last try the record says so, its
-//     author proposes it again to have it reviewed again, the module's health names it, and the operator is told
-//     once on the operator's channel.
+//     author proposes it again to have it reviewed again, and the operator is told once on the operator's channel.
+//     The module's health names it and stays healthy: the record waits on its author, and the module works
+//     (novox/hq issue 508).
 //   - A record with a version whose writer the runtime did not prove is not reviewed: who wrote it cannot be told,
 //     and no one but its proven author revises it, so its author proposes it again under a new number.
 //   - On approve, a record that does not need the operator is accepted at once; one its author marked
@@ -278,7 +279,7 @@ func (k *KB) ReviewStep() (string, error) {
 }
 
 // Unreviewed are the proposed records that ran out of review tries: no judgement could be had, or the operator
-// could not be asked. They make the module unhealthy, so the mesh raises a condition naming them.
+// could not be asked. The module's health names them and stays healthy (novox/hq issue 508); the operator is told.
 func Unreviewed(all []*Entry) []string {
 	var out []string
 	for _, e := range all {
@@ -326,19 +327,23 @@ func (k *KB) tellUnreviewed(e *Entry) {
 		version = e.PendingVersion
 	}
 	key := fmt.Sprintf("%sv%d", unreviewedOf(w.Key()), version)
-	// Kept as told before the notice is sent, as for drafted clauses: a notice that fails is logged and not sent
-	// again, and the module's health still names the record, so the mesh's condition stands for it.
+	// Kept as told before the notice is sent, so two steps never send it twice; a notice that fails is no longer
+	// kept as told, and is sent again at the next step: the module's health stays healthy for it (novox/hq issue
+	// 508), so this notice is the only loud word of it.
 	if first, err := k.Store.Told(ctx, key+" "+at); err != nil || !first {
 		return
 	}
 	summary := fmt.Sprintf("%s was never reviewed: its review could not be had %d times (last: %s). It stays proposed, and nothing of it is in force.",
 		w.Key(), w.Judging.Tries, w.Judging.Reason)
 	explanation := "No review that could not be had counts as an approval. Its author proposes it again to have it reviewed again; " +
-		"until then the knowledge base's health names it. The knowledge base's get and history show it."
+		"the knowledge base's get and history show it."
 	// The channel edits a cleared message with the resolved line given here, not the one sent with the clear.
 	resolved := w.Key().String() + " no longer waits for its review"
 	if err := k.Notify(key, summary, "A proposal was never reviewed: "+w.Key().String(), explanation, resolved); err != nil {
-		k.logf("[mesh-knowledge] the operator could not be told that %s was never reviewed: %v; the module's health still names it", w.Key(), err)
+		k.logf("[mesh-knowledge] the operator could not be told that %s was never reviewed: %v; told again at the next step", w.Key(), err)
+		if uerr := k.Store.Untold(ctx, key+" "+at); uerr != nil {
+			k.logf("[mesh-knowledge] and that it was not told could not be kept: %v", uerr)
+		}
 	}
 }
 

modules/mesh-knowledge/cmd/mesh-knowledge/review_bounds_test.go

@@ -73,21 +73,36 @@ func TestARecordWhoseReviewCannotBeKeptBlocksNoOtherAndIsTriedBoundedly(t *testi
 	}
 }
 
-// A record that ran out of review tries makes the module unhealthy, naming it: the mesh raises a condition.
-func TestARecordThatCouldNotBeReviewedMakesTheModuleUnhealthy(t *testing.T) {
+// A record that ran out of review tries waits on its author, not on the module: it is told to the operator and leaves
+// the module healthy, so it never holds the module's own rollouts (novox/hq issue 508). A notice that could not be
+// sent is sent again at the next step, since health no longer stands for it.
+func TestARecordThatCouldNotBeReviewedLeavesTheModuleHealthyAndIsTold(t *testing.T) {
 	w := imported(t, newMemStore())
 	n := proposed(t, w, "author@g14", false)
 	w.judge.reviewFail = true
-	for i := 0; i < MostReviewTries; i++ {
+	notify := w.k.Notify
+	failing := true
+	w.k.Notify = func(key, summary, headline, explanation, resolved string) error {
+		if failing {
+			failing = false
+			return errors.New("the operator's channel did not answer")
+		}
+		return notify(key, summary, headline, explanation, resolved)
+	}
+	for i := 0; i < MostReviewTries+1; i++ {
 		w.reviewAll(t)
-		if h := w.k.Health(nil); i < MostReviewTries-1 && !h.Healthy {
-			t.Fatalf("unhealthy while tries remain: %s", h.Why)
+		if h := w.k.Health(nil); !h.Healthy {
+			t.Fatalf("unhealthy after try %d: %s", i+1, h.Why)
 		}
 		w.clock = w.clock.Add(time.Hour)
 	}
+	w.reviewAll(t)
 	h := w.k.Health(nil)
-	if h.Healthy || !strings.Contains(h.Why, itoaKey(n)) || !strings.Contains(h.Why, "could not be reviewed") {
-		t.Fatalf("%+v", h)
+	if !h.Healthy || h.State != "healthy" || !strings.Contains(h.Why, itoaKey(n)) || !strings.Contains(h.Why, "author") {
+		t.Fatalf("not healthy, or the record waiting on its author not named: %+v", h)
+	}
+	if len(w.told) != 1 || !strings.Contains(w.told[0], itoaKey(n)) || !strings.Contains(w.told[0], "never reviewed") {
+		t.Fatalf("told %q", w.told)
 	}
 }
 

modules/mesh-knowledge/module.json

@@ -71,6 +71,7 @@
   "invokes": [
     "seat:issue-tracker.reserve",
     "claude-code.claude_code_judge",
+    "claude-code.claude_code_judged",
     "records.records_status",
     "records.records_list",
     "records.records_read",