Files
mesh-catalog/modules/mesh-knowledge/cmd/mesh-knowledge/review_test.go
T
jschoubben c5dc10fa8c CAT-K6: a version is reviewed in parts: the head and each section, a part judged again only when its text or what it names changes
One judgement per part with no tools, at most two at once; a section is read with its frame (the clauses and records it
names and the digest of its Context), so a usual revision judges only what changed. The latest approval of each part is
kept in the head and carried to the next version; a part that could not be judged keeps the others' verdicts and is the
only one asked again; reviewed is said once per version and each verdict as part-reviewed on the stream reviews. A part
in flight is a key of the state asked. A version is accepted when every part approves; findings name their part.

Issue 1966.

Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig
2026-11-04 12:07:32 +01:00

472 lines
20 KiB
Go

package main
import (
"context"
"strings"
"testing"
"time"
"git.novox.be/novox/mesh-sdk/go/asks"
)
// to-be 55 §10: "A record is accepted only after the knowledge base's own judgement approves it, and, when it needs
// the operator, only on the warrant — the module's tests: a proposal whose judgement answers changes, or cannot be
// had, stays proposed; a needs operator record unaccepted without the warrant, accepted with it."
func proposed(t *testing.T, w *world, author string, needsOperator bool) int {
t.Helper()
n := w.reserved(t, author)
p := valid(n)
p.NeedsOperator = needsOperator
if needsOperator {
p.Brief = &Brief{Headline: "Merges land after ninety seconds of quiet",
Summary: "A merge window closes after ninety seconds without a merge, so merges land sooner."}
}
if _, err := w.k.Propose(author, p); err != nil {
t.Fatal(err)
}
return n
}
func TestAProposalIsReviewedByTheKnowledgeBaseItselfAndAcceptedOnApprove(t *testing.T) {
eachStore(t, func(t *testing.T, s Store) {
w := imported(t, s)
n := proposed(t, w, "author@g14", false)
// Proposed, waiting for its review: nothing of it holds yet.
e := mustGet(t, s, Key{KindDecision, n})
if e.Current.Status != StatusProposed || e.Current.Judging == nil || e.Current.Judging.State != JudgingDue || len(w.judge.reviews) != 0 {
t.Fatalf("after propose: %s, %+v", e.Current.Status, e.Current.Judging)
}
said := w.reviewAll(t)
if len(said) != 1 || len(w.judge.reviews) != 5 { // the head and its four sections
t.Fatalf("reviews: %v, %d judgements", said, len(w.judge.reviews))
}
// The judgements saw the record, the records it links and the rules — nothing else of the knowledge base.
q := strings.Join(w.judge.reviews, "\n")
for _, want := range []string{"The window closes after ninety seconds of quiet", "ADR 0276", "a decision has at least one clause",
"Merges land sooner."} {
if !strings.Contains(q, want) {
t.Errorf("the review's question lacks %q", want)
}
}
for _, never := range []string{"ADR 0131", "ADR 0011", "author@g14"} {
if strings.Contains(q, never) {
t.Errorf("the review's question holds %q, which the record does not link", never)
}
}
e = mustGet(t, s, Key{KindDecision, n})
if e.Current.Status != StatusAccepted || e.Current.Clauses[0].Status != ClauseInForce || e.Current.AcceptedAt.IsZero() ||
e.Current.Judging != nil || len(e.Current.Reviews) != 1 || e.Current.Reviews[0].Verdict != VerdictApprove ||
e.Current.Reviews[0].By != "the agent on test (sonnet)" || e.Current.Reviews[0].Version != 2 {
t.Fatalf("not accepted: %+v", e.Current)
}
// 0276 §1 is changed by the new record's clause 1, as its changes link says.
c := mustGet(t, s, Key{KindDecision, 276}).Current.Clause(1)
if c.Status != ClauseChanged || len(c.ChangedBy) != 1 || c.ChangedBy[0] != itoaKey(n)+" §1" {
t.Fatalf("0276 §1: %+v", c)
}
if len(w.said.of(EventReviewed)) != 1 || len(w.said.of(EventAccepted)) != 1 || len(w.said.of(EventChanged)) != 1 {
t.Fatalf("events: %v", w.said.events)
}
h, err := w.k.Holds("", &Key{KindDecision, 276}, 0)
if err != nil {
t.Fatal(err)
}
if !strings.Contains(h.Clauses[0].Holds, "changed by "+itoaKey(n)) {
t.Fatalf("holds: %+v", h.Clauses[0])
}
// Reviewed once: nothing waits any more.
if said := w.reviewAll(t); len(said) != 0 || len(w.judge.reviews) != 5 {
t.Fatalf("reviewed again: %v", said)
}
})
}
func itoaKey(n int) string { return Key{KindDecision, n}.String() }
// On changes the record stays proposed, the findings kept with the version they judged; only its author revises
// it, and the revision is reviewed again.
func TestOnChangesTheRecordStaysProposedAndItsAuthorsRevisionIsReviewedAgain(t *testing.T) {
eachStore(t, func(t *testing.T, s Store) {
w := imported(t, s)
n := proposed(t, w, "author@g14", false)
w.judge.review = `{"verdict":"changes","findings":["§2: name the bound","Consequences: say who waits"]}`
w.reviewAll(t)
e := mustGet(t, s, Key{KindDecision, n})
if e.Current.Status != StatusProposed || e.Current.Judging != nil || len(e.Current.Reviews) != 1 ||
e.Current.Reviews[0].Verdict != VerdictChanges || !strings.Contains(e.Current.Reviews[0].Findings, "[head] §2: name the bound\n[head] Consequences: say who waits") ||
e.Current.Reviews[0].Version != 2 {
t.Fatalf("after changes: %+v", e.Current)
}
g, _ := w.k.Get(GetAsk{Number: n})
if len(g.Reviews) != 1 || g.Clauses[0].Holds != "proposed: not in force" {
t.Fatalf("get after changes: %+v", g)
}
// Not judged again until it is revised.
if said := w.reviewAll(t); len(said) != 0 || len(w.judge.reviews) != 5 {
t.Fatalf("judged again unrevised: %v", said)
}
p := valid(n)
p.Sections[2].Text = "Merges land sooner; nobody waits longer than ninety seconds."
if _, err := w.k.Propose("reviewer@novox", p); err == nil || !strings.Contains(err.Error(), "only its author") {
t.Fatalf("a revision by someone else: %v", err)
}
w.judge.review = ""
if _, err := w.k.Propose("author@g14", p); err != nil {
t.Fatal(err)
}
w.reviewAll(t)
e = mustGet(t, s, Key{KindDecision, n})
if e.Current.Status != StatusAccepted || len(e.Current.Reviews) != 2 || e.Current.Reviews[1].Verdict != VerdictApprove ||
e.Current.Reviews[1].Version != 4 || e.Current.Reviews[0].Version != 2 {
t.Fatalf("after the revision: %s, %+v", e.Current.Status, e.Current.Reviews)
}
})
}
// A judgement that cannot be had — the judge unreachable, or its answer unreadable — leaves the record proposed,
// says so, and is never taken as an approval.
func TestAJudgementThatCannotBeHadNeverApproves(t *testing.T) {
for _, c := range []struct{ name, answer string }{
{"unreachable", ""},
{"not json", `the record looks fine to me`},
{"no verdict", `{"findings":[]}`},
{"another verdict", `{"verdict":"approved","findings":[]}`},
{"a verdict in capitals", `{"verdict":"APPROVE","findings":[]}`},
{"changes naming none", `{"verdict":"changes","findings":[" "]}`},
} {
t.Run(c.name, func(t *testing.T) {
w := imported(t, newMemStore())
n := proposed(t, w, "author@g14", false)
w.judge.review, w.judge.reviewFail = c.answer, c.answer == ""
for try := 1; try <= MostReviewTries; try++ {
if said := w.reviewAll(t); len(said) != 1 || !strings.Contains(said[0], "not reviewed") {
t.Fatalf("try %d: %v", try, said)
}
e := mustGet(t, w.k.Store, Key{KindDecision, n})
if e.Current.Status != StatusProposed || e.Current.Judging == nil || e.Current.Judging.State != JudgingFailed ||
e.Current.Judging.Tries != try || len(e.Current.Reviews) != 0 {
t.Fatalf("try %d: %s, %+v", try, e.Current.Status, e.Current.Judging)
}
g, _ := w.k.Get(GetAsk{Number: n})
if !strings.Contains(g.Review, "not reviewed") || !strings.Contains(g.Review, "counts as an approval") {
t.Fatalf("get does not say it: %q", g.Review)
}
// Not asked again at once: after a pause that grows with each try.
if said := w.reviewAll(t); len(said) != 0 {
t.Fatalf("asked again at once: %v", said)
}
w.clock = w.clock.Add(time.Duration(try) * 10 * time.Minute)
}
// Tried enough: it stays proposed, saying its author proposes it again.
w.clock = w.clock.Add(24 * time.Hour)
if said := w.reviewAll(t); len(said) != 0 {
t.Fatalf("asked after its last try: %v", said)
}
e := mustGet(t, w.k.Store, Key{KindDecision, n})
g, _ := w.k.Get(GetAsk{Number: n})
if e.Current.Status != StatusProposed || !strings.Contains(g.Review, "proposes it again") || len(w.said.of(EventAccepted)) != 0 {
t.Fatalf("after the last try: %s, %q", e.Current.Status, g.Review)
}
h, _ := w.k.Holds("", &Key{KindDecision, n}, 0)
if len(h.Clauses) != 0 {
t.Fatalf("a record never reviewed holds %v", h.Clauses)
}
// Its author proposes it again, and that version is reviewed.
w.judge.review, w.judge.reviewFail = "", false
w.accepted(t, "author@g14", valid(n))
})
}
}
// A record revised while its judgement was asked is not decided on the version judged: the newest is judged next.
func TestARecordRevisedWhileItIsJudgedIsJudgedAgain(t *testing.T) {
w := imported(t, newMemStore())
n := proposed(t, w, "author@g14", false)
p := valid(n)
p.Sections[2].Text = "Merges land sooner, and the walk is shorter."
w.judge.during = func() {
if _, err := w.k.Propose("author@g14", p); err != nil {
t.Error(err)
}
}
said := w.reviewAll(t)
if len(said) != 2 || !strings.Contains(said[0], "changed while it was judged") || len(w.judge.reviews) != 10 {
t.Fatalf("%v", said)
}
e := mustGet(t, w.k.Store, Key{KindDecision, n})
if e.Current.Status != StatusAccepted || e.Current.Section("Consequences") != p.Sections[2].Text || e.Current.Reviews[0].Version != 3 {
t.Fatalf("%s %+v", e.Current.Status, e.Current.Reviews)
}
}
// A record with a version written while the runtime proved no caller is never reviewed, and so never accepted:
// who wrote it cannot be told, and only its proven author revises it. Its author proposes it again.
func TestARecordProposedUnprovenIsNeverReviewed(t *testing.T) {
w := imported(t, newMemStore())
n := w.reserved(t, Unproven)
got, err := w.k.Propose(Unproven, valid(n))
if err != nil {
t.Fatal(err)
}
if !strings.Contains(got.Says, "proposes it again under a new number") {
t.Fatalf("propose says %q", got.Says)
}
if said := w.reviewAll(t); len(said) != 0 || len(w.judge.reviews) != 0 {
t.Fatalf("an unproven proposal was judged: %v", said)
}
g, _ := w.k.Get(GetAsk{Number: n})
if g.Status != StatusProposed || !strings.Contains(g.Review, "proved no caller") {
t.Fatalf("%+v", g)
}
}
// A record that supersedes another moves it and every clause of it to superseded.
func TestASupersedingRecordSupersedesTheOlderOnAcceptance(t *testing.T) {
w := imported(t, newMemStore())
n := w.reserved(t, "author@g14")
p := valid(n)
p.Links = []LinkAsk{{Kind: LinkSupersedes, To: "0276"}}
p.Sections[0].Text = "The merge window is replaced."
w.accepted(t, "author@g14", p)
old := mustGet(t, w.k.Store, Key{KindDecision, 276}).Current
if old.Status != StatusSuperseded || old.Clause(4).Status != ClauseSuperseded || old.Extra["superseded-by"] != itoaKey(n) {
t.Fatalf("0276: %s, §4 %+v", old.Status, old.Clause(4))
}
h, _ := w.k.Holds("", &Key{KindDecision, 276}, 0)
if len(h.Clauses) != 0 {
t.Fatalf("a superseded record holds %v", h.Clauses)
}
}
func TestARecordThatNeedsTheOperatorIsAcceptedOnlyOnTheWarrant(t *testing.T) {
eachStore(t, func(t *testing.T, s Store) {
w := imported(t, s)
n := proposed(t, w, "author@g14", true)
w.reviewAll(t)
if e := mustGet(t, s, Key{KindDecision, n}); e.Current.Status != StatusProposed || len(w.router.asked) != 1 {
t.Fatalf("after the review: %s, asks %d", e.Current.Status, len(w.router.asked))
}
ask := w.router.asked[0]
if err := ask.Check(w.clock); err != nil || ask.Options[0].Level != asks.Approve || ask.Options[1].Level != asks.Approve ||
!strings.Contains(ask.Details, "the knowledge base's own review") || ask.Headline != "Merges land after ninety seconds of quiet" ||
asks.Glance(ask.Explanation) != nil || strings.Contains(ask.Explanation, "The window closes") {
t.Fatalf("the ask: %v %+v", err, ask)
}
// Without the warrant: nothing is accepted, however often the router's record is read or the reviews run.
for i := 0; i < 2; i++ {
if _, err := w.k.Reconcile(); err != nil {
t.Fatal(err)
}
w.reviewAll(t)
}
if e := mustGet(t, s, Key{KindDecision, n}); e.Current.Status != StatusProposed || len(w.judge.reviews) != 5 {
t.Fatalf("accepted with no warrant: %s", e.Current.Status)
}
h, _ := w.k.Holds("", &Key{KindDecision, n}, 0)
if len(h.Clauses) != 0 {
t.Fatalf("a record waiting for the operator holds %v", h.Clauses)
}
// The operator's Approve, on a channel that proves who answered: accepted, the warrant kept.
w.clock = w.clock.Add(10 * time.Minute)
warrant := w.router.answer(ask.ID, OptionApprove, w.clock)
said, err := w.k.Reconcile()
if err != nil {
t.Fatal(err)
}
e := mustGet(t, s, Key{KindDecision, n})
if e.Current.Status != StatusAccepted || e.Current.Ask == nil || e.Current.Ask.Warrant != warrant.Says() ||
e.Current.Ask.Said != AskApproved || len(said) != 1 {
t.Fatalf("on the warrant: %+v, ask %+v, said %v", e.Current.Status, e.Current.Ask, said)
}
vs, _ := s.Versions(context.Background(), Key{KindDecision, n})
if last := vs[len(vs)-1]; !strings.Contains(last.Why, "warrant") || last.By != "the operator" {
t.Fatalf("the accepting version: %+v", last)
}
if said, _ := w.k.Reconcile(); len(said) != 0 {
t.Fatalf("decided twice: %v", said)
}
})
}
// An ask that cannot be published is taken back: the record stays proposed and is reviewed again later.
func TestAnAskThatCannotBePublishedLeavesTheRecordProposed(t *testing.T) {
w := imported(t, newMemStore())
w.k.Asks = nil
n := proposed(t, w, "author@g14", true)
said := w.reviewAll(t)
e := mustGet(t, w.k.Store, Key{KindDecision, n})
if len(said) != 1 || !strings.Contains(said[0], "cannot be asked") || e.Current.Status != StatusProposed || e.Current.Ask != nil ||
e.Current.Judging == nil || e.Current.Judging.State != JudgingFailed {
t.Fatalf("%v: %s %+v", said, e.Current.Status, e.Current.Judging)
}
}
// A warrant for an ask whose record changed after it was asked does not hold: the act it bound is another version.
func TestAWarrantForAnotherVersionDoesNotAccept(t *testing.T) {
w := imported(t, newMemStore())
n := proposed(t, w, "author@g14", true)
w.reviewAll(t)
// The record is changed behind the ask, keeping the ask: what the operator was shown is not what is held.
ctx := context.Background()
e := mustGet(t, w.k.Store, Key{KindDecision, n})
r := e.Current.Clone()
r.Clauses[1].Text = "**A batch is delivered by many walks.** One commit per repository."
if err := w.k.Store.Save(ctx, Version{By: "someone", Why: "changed behind the ask", Record: r}, true, false, e.Revision); err != nil {
t.Fatal(err)
}
w.router.answer(w.router.asked[0].ID, OptionApprove, w.clock.Add(time.Minute))
if _, err := w.k.Reconcile(); err != nil {
t.Fatal(err)
}
e = mustGet(t, w.k.Store, Key{KindDecision, n})
if e.Current.Status != StatusProposed || !strings.Contains(e.Current.Ask.Said, "does not hold") {
t.Fatalf("accepted on a warrant for another version: %s, %+v", e.Current.Status, e.Current.Ask)
}
}
func TestTheOperatorsDeclineLeavesTheRecordProposed(t *testing.T) {
w := imported(t, newMemStore())
n := proposed(t, w, "author@g14", true)
w.reviewAll(t)
w.router.answer(w.router.asked[0].ID, OptionDecline, w.clock.Add(time.Minute))
if _, err := w.k.Reconcile(); err != nil {
t.Fatal(err)
}
e := mustGet(t, w.k.Store, Key{KindDecision, n})
if e.Current.Status != StatusProposed || e.Current.Ask.Said != "declined by the operator" {
t.Fatalf("%s %+v", e.Current.Status, e.Current.Ask)
}
}
// An accepted record changes only by a progressive insight, which is reviewed like a proposal while the accepted
// version is served.
func TestAnAcceptedRecordChangesOnlyByAnInsightReviewedLikeAProposal(t *testing.T) {
w := imported(t, newMemStore())
n := w.reserved(t, "author@g14")
w.accepted(t, "author@g14", valid(n))
p := valid(n)
p.Sections[2].Text = "Merges land sooner, by about a minute."
if _, err := w.k.Propose("author@g14", p); err == nil || !strings.Contains(err.Error(), "progressive insight") {
t.Fatalf("an accepted record rewritten: %v", err)
}
p.Insight = "it said only that merges land sooner"
w.judge.review = `{"verdict":"changes","findings":["Consequences: say what a minute is measured from"]}`
if _, err := w.k.Propose("author@g14", p); err != nil {
t.Fatal(err)
}
w.reviewAll(t)
e := mustGet(t, w.k.Store, Key{KindDecision, n})
if e.Pending == nil || e.Current.Section("Consequences") != "Merges land sooner." || len(e.Pending.Reviews) != 1 ||
e.Pending.Reviews[0].Verdict != VerdictChanges {
t.Fatalf("the insight was served before its review approved it: %+v", e.Current.Sections)
}
w.judge.review = ""
p.Sections[2].Text = "Merges land sooner, by about a minute from the last merge."
if _, err := w.k.Propose("author@g14", p); err != nil {
t.Fatal(err)
}
w.reviewAll(t)
e = mustGet(t, w.k.Store, Key{KindDecision, n})
if e.Pending != nil || e.Current.Status != StatusAccepted || len(e.Current.Insights) != 1 ||
e.Current.Section("Consequences") != p.Sections[2].Text {
t.Fatalf("after the insight's review: %+v", e.Current)
}
// 0276 §1 was changed once, by the acceptance; the insight keeps the link and changes it no further.
if c := mustGet(t, w.k.Store, Key{KindDecision, 276}).Current.Clause(1); len(c.ChangedBy) != 1 {
t.Fatalf("0276 §1 changed again by an insight: %v", c.ChangedBy)
}
}
// No verb takes a review from anyone: the review is the knowledge base's own.
func TestNoVerbTakesAReview(t *testing.T) {
for _, tool := range Tools(&KB{}, callerOf) {
if strings.HasSuffix(tool.Name, ".review") || strings.Contains(tool.Name, "review") {
t.Errorf("%s is served", tool.Name)
}
}
for _, v := range Verbs {
if v == "review" {
t.Error("review is a verb")
}
}
}
func TestReserveAsksTheTrackerAndANumberNeverProposedIsWithdrawn(t *testing.T) {
w := newWorld(t, newMemStore(), fixture())
r, err := w.k.ReserveNumber("author@g14", KindResearch, "how a knowledge base is asked")
if err != nil {
t.Fatal(err)
}
if r.Number != 50 || len(w.tracker.asked) != 1 || w.tracker.asked[0]["kind"] != KindResearch ||
w.tracker.asked[0]["for"] != "how a knowledge base is asked" {
t.Fatalf("%+v %v", r, w.tracker.asked)
}
if _, err := w.k.ReserveNumber("author@g14", "design", "x"); err == nil {
t.Fatal("a design number reserved")
}
w.clock = w.clock.Add(DefaultReservedFor + time.Hour)
if n, err := w.k.WithdrawStale(); err != nil || n != 1 {
t.Fatalf("withdrew %d (%v)", n, err)
}
e := mustGet(t, w.k.Store, Key{KindResearch, 50})
if e.Current.Status != StatusWithdrawn || e.Current.ReservedBy != "author@g14" {
t.Fatalf("%+v", e.Current)
}
if _, err := w.k.Propose("author@g14", ProposeAsk{Kind: KindResearch, Number: 50, Title: "late", Status: StatusActive,
Sections: []Section{{Name: "00-overview.md", Text: "x"}}}); err == nil {
t.Fatal("a withdrawn number took a proposal")
}
}
// An ask the router holds because no channel can carry it yet is shown as held, with the reason, never as waiting
// for an answer nobody was shown (ADR 0397, issue 926); offered, it waits again; answered, it is decided as ever.
func TestAnAskTheRouterHoldsIsShownHeldWithTheReason(t *testing.T) {
w := imported(t, newMemStore())
n := proposed(t, w, "author@g14", true)
w.reviewAll(t)
id := w.router.asked[0].ID
key := Key{KindDecision, n}
waiting := func() string {
g, err := w.k.Get(GetAsk{Kind: key.Kind, Number: key.Number})
if err != nil {
t.Fatal(err)
}
return g.Waiting
}
if got := waiting(); !strings.Contains(got, AskWaiting) {
t.Fatalf("before it is held: %q", got)
}
w.router.hold(id, "Telegram cannot prove who answered")
if said, err := w.k.Reconcile(); err != nil || len(said) != 1 {
t.Fatalf("%v %v", said, err)
}
got := waiting()
if !strings.Contains(got, "held by the router") || !strings.Contains(got, "Telegram cannot prove who answered") || strings.Contains(got, AskWaiting) {
t.Fatalf("held: %q", got)
}
if e := mustGet(t, w.k.Store, key); e.Current.Status != StatusProposed || e.Current.Ask.Said != AskWaiting {
t.Fatalf("a held ask changed the record: %+v", e.Current.Ask)
}
if said, _ := w.k.Reconcile(); len(said) != 0 {
t.Fatalf("said again while nothing changed: %v", said)
}
w.router.offer(id)
if _, err := w.k.Reconcile(); err != nil {
t.Fatal(err)
}
if got := waiting(); got != "the operator's ask: "+AskWaiting {
t.Fatalf("offered: %q", got)
}
// Held, then answered: decided on the warrant, with nothing of the hold left.
w.router.hold(id, "Telegram cannot prove who answered")
_, _ = w.k.Reconcile()
w.router.answer(id, OptionApprove, w.clock.Add(time.Minute))
if _, err := w.k.Reconcile(); err != nil {
t.Fatal(err)
}
if e := mustGet(t, w.k.Store, key); e.Current.Status != StatusAccepted || e.Current.Ask.Held != "" {
t.Fatalf("%s %+v", e.Current.Status, e.Current.Ask)
}
}