The knowledge base's review knows its round: from round 3 only a wrong record asks changes, at round 5 the operator decides (issue 784, ADR 0665)

Claude-Session: https://claude.ai/code/session_01JG73KK8HD1gufnqyFxMFdW
This commit is contained in:
2026-11-09 23:13:55 +01:00
parent 5e537f851f
commit 62644aa012
7 changed files with 296 additions and 9 deletions
@@ -364,6 +364,10 @@ func (k *KB) Propose(caller string, a ProposeAsk) (*Proposed, error) {
current := true
why := strings.TrimSpace(a.Why)
if held != nil {
if w := held.Working(); w.Status == StatusProposed && w.Ask != nil && w.Ask.Said == AskWaiting && w.Ask.Round > 0 {
return nil, fmt.Errorf("%s is at round %d of its review, which asked changes: the operator decides it, and no review runs before the "+
"answer. Nothing was proposed; propose again after the operator's answer", key, w.Ask.Round)
}
switch w := held.Working(); {
case held.Current.Status == StatusReserved:
r.ReservedBy, r.ReservedAt, r.ReservedFor = held.Current.ReservedBy, held.Current.ReservedAt, held.Current.ReservedFor
@@ -380,7 +384,7 @@ func (k *KB) Propose(caller string, a ProposeAsk) (*Proposed, error) {
"Nothing was proposed", key, w.Author, caller)
}
r.Author, r.Reviews, r.ReservedBy, r.ReservedAt, r.ReservedFor = w.Author, w.Reviews, w.ReservedBy, w.ReservedAt, w.ReservedFor
r.Parts = w.Parts
r.Parts, r.CountFrom = w.Parts, w.CountFrom
r.Writers = w.Writers
if a.Insight != "" {
refusals = append(refusals, Refusal{Rule: "reserved", Says: "a proposal not yet accepted is revised, not corrected by an insight"})
@@ -411,6 +415,10 @@ func (k *KB) Propose(caller string, a ProposeAsk) (*Proposed, error) {
}
r.AcceptedAt = held.Current.AcceptedAt
r.Parts = held.Current.Parts
if held.Pending != nil {
// A revision of a pending version keeps its earlier reviews, so the round count goes on (ADR 0665 §1).
r.Reviews, r.CountFrom = held.Pending.Reviews, held.Pending.CountFrom
}
r.Pending = &PendingState{Insight: Insight{Date: k.now().Format("2006-01-02"), Before: strings.TrimSpace(a.Insight), By: caller}}
r.Date = held.Current.Date
r.Effort = a.Status
@@ -92,6 +92,9 @@ const AskGlance = 1
// again to hold the warrant to it — in the form it was asked in, so an ask made before a form changed is still
// held to the words the operator was shown.
func composeAsk(r *Record, a *OperatorAsk) oc.Ask {
if a.Round > 0 {
return changesAsk(r, a)
}
if a.Form == AskGlance {
return glanceAsk(r, a)
}
@@ -124,6 +127,44 @@ func glanceAsk(r *Record, a *OperatorAsk) oc.Ask {
return withOptions(r, a, oc.Ask{ID: a.ID, Headline: headline, Explanation: explanation, Details: details})
}
// changesAsk is the ask of a record whose round-5 review asked changes (ADR 0665 §3): the findings, read at a glance,
// and the two answers. It is composed from the record and its ask alone: the findings are the last review's.
func changesAsk(r *Record, a *OperatorAsk) oc.Ask {
name := r.Key().String()
var b strings.Builder
fmt.Fprintf(&b, "The knowledge base's review of %s asked changes at round %d, the last round: you decide. Approve accepts the version as written; "+
"Decline leaves it proposed for its author, and its next review starts at round 1.", name, a.Round)
if t := strings.TrimSuffix(strings.TrimSpace(r.Title), "."); shown(t) {
fmt.Fprintf(&b, "\n\nIts title: %s.", t)
}
if n := len(r.Reviews); n > 0 {
b.WriteString("\n\nThe findings:")
size, hidden := 0, 0
for _, f := range strings.Split(r.Reviews[n-1].Findings, "\n") {
if f = strings.TrimSpace(f); f == "" {
continue
}
if rs := []rune(f); len(rs) > 300 {
f = string(rs[:300]) + "..."
}
if !shown(f) || size+len(f) > 1500 {
hidden++
continue
}
size += len(f)
fmt.Fprintf(&b, "\n- %s", f)
}
if hidden > 0 {
fmt.Fprintf(&b, "\n(%d finding(s) not shown here)", hidden)
}
}
fmt.Fprintf(&b, "\n\nRead it whole with the knowledge base's get, number %d; its fingerprint is %s.", r.Number, fingerprint(r.Digest()))
out := withOptions(r, a, oc.Ask{ID: a.ID, Headline: fmt.Sprintf("%s: accept it as written after round %d?", name, a.Round), Explanation: b.String()})
out.Options[0].Does = "the version is accepted as written and its clauses are in force"
out.Options[1].Does = "the record stays proposed for its author; its next review is round 1"
return out
}
// listedAsk is the ask as it was composed before the brief: every clause's lead listed. Composed only to hold
// the warrant of an ask made then.
func listedAsk(r *Record, a *OperatorAsk) oc.Ask {
@@ -180,7 +221,11 @@ func newAskID(r *Record) string {
// it. The caller holds the write lock. An ask that could not be made or published is taken back, and the review is
// asked again after a pause, counted with the tries before it (tries): the record stays proposed, and is reviewed
// at most MostReviewTries times.
func (k *KB) askOperator(ctx context.Context, held *Entry, w *Record, by string, tries int) (string, error) {
//
// round is 0 for a record the review approved. For round MostReviewRounds it is that round: the review asked changes,
// and the operator is asked to accept the version as written or leave it proposed (ADR 0665 §3). An ask not made
// then takes the round's review back, so its retry is the same round.
func (k *KB) askOperator(ctx context.Context, held *Entry, w *Record, by string, tries int, round int) (string, error) {
version := held.CurrentVersion
if held.Pending != nil {
version = held.PendingVersion
@@ -189,6 +234,10 @@ func (k *KB) askOperator(ctx context.Context, held *Entry, w *Record, by string,
w.Ask = nil
w.Judging = failedJudging(tries+1, why, k.now())
said := fmt.Sprintf("approved by the knowledge base's review, and %s (%s", why, w.Judging.owedSays())
if round > 0 {
w.Reviews = w.Reviews[:len(w.Reviews)-1]
said = fmt.Sprintf("round %d asked changes, and %s (%s", round, why, w.Judging.owedSays())
}
e, err := k.Store.Get(ctx, w.Key())
if err != nil || e == nil {
return "", fmt.Errorf("%s: %s, and that could not be kept (%v)", w.Key(), said, err)
@@ -203,17 +252,21 @@ func (k *KB) askOperator(ctx context.Context, held *Entry, w *Record, by string,
if k.Asks == nil {
return unasked("the operator cannot be asked from here")
}
oa := &OperatorAsk{ID: newAskID(w), At: k.now(), Version: version, Said: AskWaiting, Form: AskGlance}
oa := &OperatorAsk{ID: newAskID(w), At: k.now(), Version: version, Said: AskWaiting, Form: AskGlance, Round: round}
ask := composeAsk(w, oa)
if err := occ.Check(ask, k.now()); err != nil {
return unasked(fmt.Sprintf("the ask could not be made (%v)", err))
}
w.Ask = oa
if err := k.Store.Save(ctx, Version{By: by, At: k.now(), Why: "reviewed: approved; the operator is asked", Record: w, Insight: held.Pending != nil && held.Pending.IsInsight()},
asked := "approved; the operator is asked"
if round > 0 {
asked = fmt.Sprintf("round %d asked changes; the operator is asked", round)
}
if err := k.Store.Save(ctx, Version{By: by, At: k.now(), Why: "reviewed: " + asked, Record: w, Insight: held.Pending != nil && held.Pending.IsInsight()},
held.Pending == nil, false, held.Revision); err != nil {
return "", fmt.Errorf("%s: the review could not be kept (%v); the operator was not asked", w.Key(), err)
}
k.emit(EventReviewed, w, by, "approved; the operator is asked", held.Versions+1)
k.emit(EventReviewed, w, by, asked, held.Versions+1)
if err := k.Asks.Publish(oc.AcceptSubjectAsk(Module), ask, ask.ID); err != nil {
k.logf("[mesh-knowledge] the ask for %s could not be published: %v", w.Key(), err)
return unasked(fmt.Sprintf("the operator could not be asked (%v)", err))
@@ -351,6 +404,9 @@ func (k *KB) decide(ctx context.Context, e *Entry, w *Record, warrant oc.Warrant
case OptionDecline:
w = w.Clone()
w.Ask.Warrant = occ.Says(warrant)
if w.Ask.Round > 0 {
w.CountFrom = len(w.Reviews) // the revision's review starts a new count at round 1 (ADR 0665 §3)
}
return k.endAsk(ctx, e, w, "declined by the operator")
}
return k.endAsk(ctx, e, w, "was answered with "+option.ID+", which it does not offer")
@@ -264,6 +264,9 @@ type OperatorAsk struct {
// Held is why the router holds the ask while it waits: no channel can carry it yet (ADR 0397, issue 926).
// Empty once the router offers it, or when it ended.
Held string `json:"held,omitempty"`
// Round is the review round whose changes this ask lays before the operator (ADR 0665 §3); 0 for an ask of
// the approval of a record the review approved.
Round int `json:"round,omitempty"`
}
// Drafting is where a prose record stands in the drafting and confirming of its clauses (to-be 55 §7).
@@ -329,6 +332,9 @@ type Record struct {
// AcceptedAt is when it was accepted here; zero for one imported accepted.
AcceptedAt time.Time `json:"accepted-at,omitempty"`
Reviews []Review `json:"reviews,omitempty"`
// CountFrom is how many of Reviews came before the round count last restarted: the operator's Decline of a
// round-5 ask restarts it (ADR 0665 §1, §3), so the next review is round 1 (review_round.go).
CountFrom int `json:"count-from,omitempty"`
// Parts are the latest verdict of each part of the version (the head, each section), kept for a new version whose
// part is unchanged (review_parts.go).
Parts []PartReview `json:"parts,omitempty"`
@@ -484,7 +490,7 @@ func (r *Record) Clone() *Record {
// it.
func (r *Record) Digest() string {
c := r.Clone()
c.Reviews, c.Ask, c.Drafting, c.Judging, c.Parts, c.FormOnly = nil, nil, nil, nil, nil, false
c.Reviews, c.Ask, c.Drafting, c.Judging, c.Parts, c.FormOnly, c.CountFrom = nil, nil, nil, nil, nil, false, 0
raw, _ := json.Marshal(c)
sum := sha256.Sum256(raw)
return "sha256:" + hex.EncodeToString(sum[:])
@@ -121,7 +121,8 @@ func ReviewQuestion(r *Record, linked []*Record) string {
"what it decides is checked; and every link fits: a record it changes says which clause and really changes that "+
"clause, a record it supersedes is really replaced whole, and no clause contradicts a clause in force of a record it "+
"links without changing or superseding it. Answer changes otherwise, with findings, each naming the section or "+
"clause and saying what to change. Do not ask for changes of taste or wording that leave the meaning as it is.\n\n", what, r.Key())
"clause and saying what to change. Do not ask for changes of taste or wording that leave the meaning as it is.\n", what, r.Key())
b.WriteString(roundText(r) + "\n")
b.WriteString("The rules every record was already checked against when it was proposed:\n")
for _, rule := range PerRecordRules {
fmt.Fprintf(&b, "- %s\n", rule.Holds)
@@ -536,7 +537,12 @@ func (k *KB) reviewOne(key Key) (string, error) {
}
}
}
round := w.Round()
w.Reviews = append(w.Reviews, Review{By: by, At: k.now(), Verdict: verdict, Findings: strings.Join(findings, "\n"), Version: version})
if verdict == VerdictChanges && round >= MostReviewRounds {
// Round 5's changes are the operator's to decide (ADR 0665 §3): no review runs before the answer.
return k.askOperator(ctx, now, w, by, tries, round)
}
if verdict == VerdictChanges {
why := fmt.Sprintf("reviewed: changes asked (%d finding(s)); it stays proposed for its author to revise", len(findings))
if err := save(w, why); err != nil {
@@ -546,7 +552,7 @@ func (k *KB) reviewOne(key Key) (string, error) {
return key.String() + ": " + why, nil
}
if w.NeedsOperator {
return k.askOperator(ctx, now, w, by, tries)
return k.askOperator(ctx, now, w, by, tries, 0)
}
k.emit(EventReviewed, w, by, "approved", now.Versions+1)
r, err := k.accept(ctx, now, w, by, "accepted after the knowledge base's review", "")
@@ -71,7 +71,8 @@ func introFor(r *Record, what string) string {
"what it decides is checked; and every link fits: a record it changes says which clause and really changes that "+
"clause, a record it supersedes is really replaced whole, and no clause contradicts a clause in force of a record it "+
"links without changing or superseding it. Answer changes otherwise, with findings, each naming the section or "+
"clause and saying what to change. Do not ask for changes of taste or wording that leave the meaning as it is.\n\n", about, r.Key(), what)
"clause and saying what to change. Do not ask for changes of taste or wording that leave the meaning as it is.\n", about, r.Key(), what)
b.WriteString(roundText(r) + "\n")
b.WriteString("The rules every record was already checked against when it was proposed:\n")
for _, rule := range PerRecordRules {
fmt.Fprintf(&b, "- %s\n", rule.Holds)
@@ -0,0 +1,66 @@
package main
// The review knows its round (novox/hq ADR 0665, issue 784). A record's round is the number of its reviews since the
// last of: it was accepted, it was first proposed, the operator declined it at round MostReviewRounds, plus one.
// Reviews belong to the version being worked (a pending version starts with none) and CountFrom marks the Decline,
// so the count is what is on the record. The question names the round and lists the earlier findings of the count;
// from round OnlyWrongFrom only a finding that makes the record wrong asks changes; at round MostReviewRounds a
// review that asks changes becomes an ask of the operator (askOperator with a round), and no review runs before the
// answer (Propose refuses a revision meanwhile).
import (
"fmt"
"strings"
)
const (
// MostReviewRounds is the round at which the operator decides.
MostReviewRounds = 5
// OnlyWrongFrom is the first round in which only a finding that makes the record wrong asks changes.
OnlyWrongFrom = 3
// MostEarlierFindings bounds the earlier findings a question lists, the latest rounds kept.
MostEarlierFindings = 6 << 10
)
// Round is the round of the next review of r.
func (r *Record) Round() int {
from := r.CountFrom
if from > len(r.Reviews) {
from = len(r.Reviews)
}
return len(r.Reviews) - from + 1
}
// roundText is what every part's question says of the round: its number, what the round allows, and the findings
// of the earlier rounds of the count.
func roundText(r *Record) string {
round := r.Round()
var b strings.Builder
fmt.Fprintf(&b, "\nThis is round %d of its review. At round %d the operator decides, so name every finding at once; "+
"a finding an earlier round named and the record answered is not named again.\n", round, MostReviewRounds)
if round >= OnlyWrongFrom {
b.WriteString("From round 3 only a finding that makes the record wrong asks changes: two of its parts contradict each other, " +
"or it contradicts a clause in force of a record it links without changing or superseding that clause, or it breaks one of the " +
"rules above, or a link the whole-set rule needs is missing. Anything else, such as wording, a further case, a nicer form or a " +
"detail its design can hold, is a remark: answer approve and give it as a finding under approve.\n")
}
var earlier []string
for i := r.CountFrom; i < len(r.Reviews) && i >= 0; i++ {
if rv := r.Reviews[i]; rv.Verdict == VerdictChanges && strings.TrimSpace(rv.Findings) != "" {
earlier = append(earlier, fmt.Sprintf("Round %d (version %d) asked changes:\n%s\n", i-r.CountFrom+1, rv.Version, rv.Findings))
}
}
size, from := 0, len(earlier)
for from > 0 && size+len(earlier[from-1]) <= MostEarlierFindings {
from--
size += len(earlier[from])
}
if from < len(earlier) {
b.WriteString("\nThe findings of the earlier rounds:\n")
if from > 0 {
fmt.Fprintf(&b, "(%d earlier round(s) are not shown: the question has no room for them.)\n", from)
}
b.WriteString(strings.Join(earlier[from:], "\n"))
}
return b.String()
}
@@ -0,0 +1,144 @@
package main
import (
"fmt"
"strings"
"testing"
"time"
occ "git.novox.be/novox/mesh-contracts/operator-and-conversation/go/clients/operatorchannelclient"
)
// ADR 0665: the review knows its round; from round 3 only a record that is wrong asks changes; at round 5 changes
// become an ask of the operator, no review runs before the answer, and a Decline restarts the count.
func changes(i int) string {
return fmt.Sprintf(`{"verdict":"changes","findings":["finding of review %d"]}`, i)
}
// review is the i'th review of record n: its author revises it first (but for the first), then every part is
// judged with answer. It answers the questions the judge was asked.
func (w *world) review(t *testing.T, n, i int, answer string) string {
t.Helper()
p := valid(n)
p.Sections[2].Text = fmt.Sprintf("Merges land sooner; revision %d.", i)
if _, err := w.k.Propose("author@g14", p); err != nil {
t.Fatal(err)
}
w.judge.review, w.judge.reviews = answer, nil
w.reviewAll(t)
return strings.Join(w.judge.reviews, "\n")
}
func TestTheReviewIsToldItsRoundAndRoundFiveIsTheOperators(t *testing.T) {
w := imported(t, newMemStore())
n := w.reserved(t, "author@g14")
key := Key{KindDecision, n}
for i := 1; i <= 4; i++ {
q := w.review(t, n, i, changes(i))
if !strings.Contains(q, fmt.Sprintf("This is round %d of its review", i)) || !strings.Contains(q, "At round 5 the operator decides") ||
strings.Contains(q, "From round 3 only") != (i >= 3) {
t.Fatalf("round %d's question: %.600s", i, q)
}
for j := 1; j < i; j++ { // every earlier round's findings, none of this round's
if !strings.Contains(q, fmt.Sprintf("Round %d (version", j)) || !strings.Contains(q, fmt.Sprintf("finding of review %d", j)) {
t.Errorf("round %d's question lacks the findings of round %d", i, j)
}
}
if strings.Contains(q, fmt.Sprintf("finding of review %d", i)) {
t.Errorf("round %d's question holds its own finding", i)
}
}
if e := mustGet(t, w.k.Store, key); e.Current.Status != StatusProposed || len(e.Current.Reviews) != 4 || e.Current.Round() != 5 || len(w.router.asked) != 0 {
t.Fatalf("after four rounds: %s, %d reviews, %d asks", e.Current.Status, len(e.Current.Reviews), len(w.router.asked))
}
// Round 5 asks changes: the operator is asked, showing the findings; no review runs, and no revision is taken, before the answer.
if q := w.review(t, n, 5, changes(5)); !strings.Contains(q, "This is round 5 of its review") {
t.Fatalf("round 5's question: %.300s", q)
}
if len(w.router.asked) != 1 {
t.Fatalf("asks after round 5's changes: %d", len(w.router.asked))
}
ask := w.router.asked[0]
if err := occ.Check(ask, w.clock); err != nil || !strings.Contains(ask.Explanation, "finding of review 5") || !strings.Contains(ask.Explanation, "round 5") {
t.Fatalf("the ask: %v %+v", err, ask)
}
if e := mustGet(t, w.k.Store, key); e.Current.Ask == nil || e.Current.Ask.Round != 5 || e.Current.Status != StatusProposed {
t.Fatalf("after round 5: %+v", e.Current.Ask)
}
w.judge.reviews = nil
if said := w.reviewAll(t); len(said) != 0 || len(w.judge.reviews) != 0 {
t.Fatalf("a review ran before the operator answered: %v", said)
}
p := valid(n)
p.Sections[2].Text = "Merges land sooner; revision 6."
if _, err := w.k.Propose("author@g14", p); err == nil || !strings.Contains(err.Error(), "the operator decides") {
t.Fatalf("a revision before the answer: %v", err)
}
// Decline: it stays proposed, and the revision's review is round 1.
w.router.answer(ask.ID, OptionDecline, w.clock.Add(time.Minute))
if _, err := w.k.Reconcile(); err != nil {
t.Fatal(err)
}
if q := w.review(t, n, 6, changes(6)); !strings.Contains(q, "This is round 1 of its review") || strings.Contains(q, "finding of review") {
t.Fatalf("after the Decline: %.400s", q)
}
if len(w.router.asked) != 1 {
t.Fatalf("round 1 asked the operator")
}
}
func TestRoundFiveApprovedByTheOperatorAcceptsTheVersionAsWritten(t *testing.T) {
w := imported(t, newMemStore())
n := w.reserved(t, "author@g14")
for i := 1; i <= 5; i++ {
w.review(t, n, i, changes(i))
}
w.router.answer(w.router.asked[0].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{KindDecision, n}); e.Current.Status != StatusAccepted || e.Current.Ask.Said != AskApproved {
t.Fatalf("%s %+v", e.Current.Status, e.Current.Ask)
}
}
// From round 3 a finding that is only a remark sits under approve, and the record is accepted.
func TestARemarkUnderApproveAtRoundFourAcceptsTheRecord(t *testing.T) {
w := imported(t, newMemStore())
n := w.reserved(t, "author@g14")
for i := 1; i <= 3; i++ {
w.review(t, n, i, changes(i))
}
q := w.review(t, n, 4, `{"verdict":"approve","findings":["wording: the second section could say it shorter"]}`)
if !strings.Contains(q, "answer approve and give it as a finding under approve") {
t.Fatalf("the question: %.400s", q)
}
if e := mustGet(t, w.k.Store, Key{KindDecision, n}); e.Current.Status != StatusAccepted || len(e.Current.Reviews) != 4 {
t.Fatalf("%s, %d reviews", e.Current.Status, len(e.Current.Reviews))
}
}
// The rounds of a pending version (an insight to an accepted record) go on across its revisions.
func TestTheRoundCountGoesOnAcrossRevisionsOfAPendingInsight(t *testing.T) {
w := imported(t, newMemStore())
n := w.reserved(t, "author@g14")
w.accepted(t, "author@g14", valid(n))
var q string
for i := 1; i <= 3; i++ {
p := valid(n)
p.Insight = "it said only that merges land sooner"
p.Sections[2].Text = fmt.Sprintf("Merges land sooner, revision %d.", i)
if _, err := w.k.Propose("author@g14", p); err != nil {
t.Fatal(err)
}
w.judge.review, w.judge.reviews = changes(i), nil
w.reviewAll(t)
q = strings.Join(w.judge.reviews, "\n")
}
if !strings.Contains(q, "This is round 3 of its review") {
t.Fatalf("the third revision of the insight: %.300s", q)
}
}