From 9f893b6305725351d695684f306bbaf6fc876977 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 14:05:37 +0200 Subject: [PATCH] mesh-knowledge: let a record change a drafted clause, and never redraft a record whose clauses others link by number A changes link onto a drafted clause was refused, so records that really change such a rule could only cite it (ADR 0294, 0301, 0320); and return renumbered ADR 0281's clauses, pointing other records' links at other clauses. The changed drafted clause keeps its number and stays unserved; return refuses a record whose clauses another record changed or links by number. Issue 494, ADR 0321. --- modules/mesh-knowledge/README.md | 10 +- .../cmd/mesh-knowledge/answers.go | 7 ++ .../cmd/mesh-knowledge/draft.go | 42 +++++++ .../cmd/mesh-knowledge/issue494_test.go | 108 ++++++++++++++++++ .../mesh-knowledge/cmd/mesh-knowledge/main.go | 3 +- .../cmd/mesh-knowledge/rules.go | 8 +- 6 files changed, 172 insertions(+), 6 deletions(-) create mode 100644 modules/mesh-knowledge/cmd/mesh-knowledge/issue494_test.go diff --git a/modules/mesh-knowledge/README.md b/modules/mesh-knowledge/README.md index 654c214f..8e0ec8fc 100644 --- a/modules/mesh-knowledge/README.md +++ b/modules/mesh-knowledge/README.md @@ -23,7 +23,7 @@ operator, it is accepted only on the operator's Approve, given on a channel that | `reserve` | `kind`, `for` | the next decision or research number, asked of the issue tracker (`issue-tracker.reserve`); spent whether or not its record is accepted | | `propose` | the record whole | proposed, or refused naming every rule it breaks; an accepted record changes only by a progressive insight (`insight`). Each version proposed is then reviewed by the knowledge base itself (below) | | `import` | `dry-run`, `commit` | hq's decision records and research efforts at the commit the records module stands at, each whole at its own number | -| `return` | `number`, `reason` | drafted clauses sent back to be drafted again | +| `return` | `number`, `reason` | drafted clauses sent back to be drafted again; refused for a record whose clauses another record changed or links by number | `ask`, `get`, `holds` and `history` stay within `bound` bytes (4096 unless asked for, at most 65536) and say what they cut and how to read the rest. The module's own tool `knowledge_health` answers its health (below). @@ -78,7 +78,7 @@ as text. `get` serves each, and any other field a record of hq wrote under `fiel - 25 on the structure: kind, reserved number, title, topic, date, deciders, research status, at least one clause, clause numbering, bold lead, clause size, the Context, Consequences and "How it is checked" sections, a research effort's overview, link kind, link target, link fit, no link to itself, what it extends, what it - supersedes or changes is accepted, a changed clause is in force, a superseded record cited beside its + supersedes or changes is accepted, a changed clause is in force or changed (a drafted one too), a superseded record cited beside its superseder, a graduated effort says what it became, and the links in its text resolve; - 4 on the prose: no word the glossary retired, no daemon's line, a deferral names its tracking issue, and a progressive insight in the dated, marked form. @@ -178,6 +178,12 @@ the older clause marked `changed`. When every prose record of a topic has been t operator is told once (`operator-channel.notify`, a warning) with the counts. `return` sends a record back to be drafted again. +A drafted clause may be changed by a later record (`changes 0281 §9`, novox/hq issue 494, ADR 0321): on its acceptance the +clause is marked `changed` with the change named, keeps its number and stays `drafted`, so it is still never served +as in force, and `get` says both. A redraft numbers a record's clauses anew, so `return` refuses a record any of +whose clauses another record changed or links by number, served or proposed, naming each link: a link never comes +to point at another clause. + ## Where it keeps its records In a database in the store: the module requires a `postgres-database` provision named `knowledge`, as diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/answers.go b/modules/mesh-knowledge/cmd/mesh-knowledge/answers.go index db2fe8fb..9a25ea0e 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/answers.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/answers.go @@ -70,6 +70,13 @@ func held(r *Record, c Clause) string { if c.Unconfirmed != "" { s += " (the second judgement did not confirm it: " + c.Unconfirmed + ")" } + if len(c.ChangedBy) > 0 { + if c.Status == ClauseSuperseded { + s += "; superseded by " + strings.Join(c.ChangedBy, ", ") + } else { + s += "; changed by " + strings.Join(c.ChangedBy, ", ") + ": read both" + } + } return s case r.Status == StatusProposed || c.Status == ClauseProposed: return "proposed: not in force" diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/draft.go b/modules/mesh-knowledge/cmd/mesh-knowledge/draft.go index 7e1bc2bb..2bc37c8e 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/draft.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/draft.go @@ -503,6 +503,15 @@ func (k *KB) Return(caller string, key Key, reason string) (map[string]any, erro if r.Drafting == nil { return nil, fmt.Errorf("%s's clauses were written by its author, not drafted: a written clause is changed by a new record", key) } + all, err := k.Store.All(ctx) + if err != nil { + return nil, err + } + if held := clauseHolds(r, all); len(held) > 0 { + return nil, fmt.Errorf("%s is not returned: drafting it again would number its clauses anew, and these hold them to their numbers: %s. "+ + "A clause keeps its number once another record links it (novox/hq issue 494). To say a drafted clause is wrong "+ + "or no longer holds, propose a record that changes it (changes %s §N)", key, strings.Join(held, "; "), strings.TrimPrefix(key.String(), "ADR ")) + } r.Clauses = nil r.Drafting = &Drafting{State: DraftWaiting, Returned: reason, At: k.now()} why := "returned to be drafted again: " + reason @@ -512,3 +521,36 @@ func (k *KB) Return(caller string, key Key, reason string) (map[string]any, erro k.emit(EventReturned, r, caller, why, e.Versions+1) return map[string]any{"record": key.String(), "says": fmt.Sprintf("%s's clauses are taken back and will be drafted again, with your reason; until then it serves none.", key)}, nil } + +// clauseHolds is what holds a record's clauses to their numbers: each clause another record changed or superseded, +// and each link of another record, served or proposed, that names one of its clauses. A redraft numbers the clauses +// anew, so a record any of these name is never returned: its links would point at other clauses. +func clauseHolds(r *Record, all []*Entry) []string { + var out []string + for _, c := range r.Clauses { + if len(c.ChangedBy) > 0 && (c.Status == ClauseChanged || c.Status == ClauseSuperseded) { + out = append(out, fmt.Sprintf("§%d is %s by %s", c.N, c.Status, strings.Join(c.ChangedBy, ", "))) + } + } + key := r.Key() + seen := map[string]bool{} + for _, e := range all { + for _, v := range []*Record{e.Current, e.Pending} { + if v == nil || v.Key() == key || v.Status == StatusWithdrawn { + continue + } + for _, l := range v.Links { + to, ok := l.To.Record() + if !ok || to != key || l.To.Clause == 0 { + continue + } + s := fmt.Sprintf("%s (%s) %s §%d", v.Key(), v.Status, l.Kind, l.To.Clause) + if !seen[s] { + seen[s] = true + out = append(out, s) + } + } + } + } + return out +} diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/issue494_test.go b/modules/mesh-knowledge/cmd/mesh-knowledge/issue494_test.go new file mode 100644 index 00000000..30343227 --- /dev/null +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/issue494_test.go @@ -0,0 +1,108 @@ +package main + +import ( + "context" + "strings" + "testing" +) + +// novox/hq issue 494: a record may change a clause the knowledge base holds only as drafted, and a record whose +// clauses another record links by number is never drafted again, so no link ever points at another clause. + +// draftedNotConfirmed drafts 0240 and has the second judgement reject its first clause: it stays drafted. +func draftedNotConfirmed(t *testing.T, w *world) Key { + t.Helper() + k := Key{KindDecision, 240} + draftUntil(t, w, k, DraftDrafted) + e := mustGet(t, w.k.Store, k) + r := e.Current.Clone() + r.Clauses[0].Text += " NOT EXACT" + if err := w.k.Store.Save(context.Background(), Version{By: "test", Why: "a draft the checker will reject", Record: r}, true, false, e.Revision); err != nil { + t.Fatal(err) + } + r = draftUntil(t, w, k, DraftDone) + if r.Clauses[0].Source != SourceDrafted || !r.Accepted() { + t.Fatalf("0240 §1 is not a drafted clause of an accepted record: %s %+v", r.Status, r.Clauses[0]) + } + return k +} + +func TestARecordMayChangeAClauseHeldOnlyAsDrafted(t *testing.T) { + eachStore(t, func(t *testing.T, s Store) { + w := imported(t, s) + k := draftedNotConfirmed(t, w) + n := w.reserved(t, "author@test") + p := valid(n) + p.Links = append(p.Links, LinkAsk{Kind: LinkChanges, To: "0240 §1", From: 2}) + if _, err := w.k.Propose("author@test", p); err != nil { + t.Fatalf("a change of a drafted clause was refused: %v", err) + } + w.reviewAll(t) + if e := mustGet(t, s, Key{KindDecision, n}); !e.Current.Accepted() { + t.Fatalf("not accepted: %s", e.Current.Status) + } + // The drafted clause is changed by it, keeps its number and stays drafted: still never served as in force. + r := mustGet(t, s, k).Current + c := r.Clause(1) + want := itoa4(n) + " §2" + if c == nil || c.Status != ClauseChanged || c.Source != SourceDrafted || len(c.ChangedBy) != 1 || !strings.HasSuffix(c.ChangedBy[0], want) { + t.Fatalf("0240 §1 after the change: %+v", c) + } + if Served(r, *c) { + t.Fatal("a drafted clause that was changed is served as holding") + } + g, err := w.k.Get(GetAsk{Kind: KindDecision, Number: 240}) + if err != nil { + t.Fatal(err) + } + if h := g.Clauses[0].Holds; !strings.HasPrefix(h, "drafted, not confirmed") || !strings.Contains(h, "changed by ADR "+want) { + t.Fatalf("get says of 0240 §1: %q", h) + } + // The whole-set rules find nothing wrong with the change. + f, _ := s.Findings(context.Background()) + for _, x := range f { + if x.Rule == "partial" { + t.Fatalf("a whole-set finding: %+v", x) + } + } + }) +} + +func TestARecordWhoseClausesAreLinkedByNumberIsNeverReturned(t *testing.T) { + eachStore(t, func(t *testing.T, s Store) { + w := imported(t, s) + k := draftedNotConfirmed(t, w) + // A proposed record cites 0240 §2: returning 0240 would point it at whatever is drafted second next time. + n := w.reserved(t, "author@test") + p := valid(n) + p.Links = append(p.Links, LinkAsk{Kind: LinkCites, To: "0240 §2", From: 1}) + w.judge.review = `{"verdict":"changes","findings":["not yet"]}` + if _, err := w.k.Propose("author@test", p); err != nil { + t.Fatal(err) + } + w.reviewAll(t) + before := mustGet(t, s, k) + _, err := w.k.Return("operator@shanks", k, "clause 1 says more than the section") + if err == nil { + t.Fatal("a record a proposed record links by clause was returned") + } + if !strings.Contains(err.Error(), "ADR "+itoa4(n)+" (proposed) cites §2") || !strings.Contains(err.Error(), "changes 0240 §N") { + t.Fatalf("the refusal does not say what holds the clauses or the way forward: %v", err) + } + after := mustGet(t, s, k) + if after.Versions != before.Versions || len(after.Current.Clauses) != len(before.Current.Clauses) || len(w.said.of(EventReturned)) != 0 { + t.Fatalf("a refused return changed 0240: %d versions (was %d), %d clauses", after.Versions, before.Versions, len(after.Current.Clauses)) + } + + // A clause another record changed holds the record to its numbers too, with no link left to scan. + w2 := imported(t, newMemStore()) + k2 := draftedNotConfirmed(t, w2) + n2 := w2.reserved(t, "author@test") + p2 := valid(n2) + p2.Links = append(p2.Links, LinkAsk{Kind: LinkChanges, To: "0240 §1", From: 2}) + w2.accepted(t, "author@test", p2) + if _, err := w2.k.Return("operator@shanks", k2, "again"); err == nil || !strings.Contains(err.Error(), "§1 is changed by ADR "+itoa4(n2)+" §2") { + t.Fatalf("a record with a changed clause was returned, or the refusal does not name the change: %v", err) + } + }) +} diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/main.go b/modules/mesh-knowledge/cmd/mesh-knowledge/main.go index c3573e2c..c1f1f2e3 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/main.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/main.go @@ -1010,7 +1010,8 @@ func Tools(k *KB, caller func(map[string]any) string) []stdio.Tool { {Name: Seat + ".return", Description: "Send the clauses drafted from a prose decision back to be drafted again, with your reason, which the next " + "drafting reads. Until they are drafted and confirmed again the record serves none. Refused: a record whose clauses its " + - "author wrote, an empty reason.", + "author wrote, an empty reason, and a record whose clauses another record changed or links by number (a redraft " + + "numbers them anew; a record that changes the drafted clause says it is wrong instead).", Input: schema([]string{"number", "reason"}, map[string]any{"kind": kindArg(), "number": integer("the record's number"), "reason": text("what is wrong with the drafted clauses")}), Run: func(m map[string]any) (any, error) { diff --git a/modules/mesh-knowledge/cmd/mesh-knowledge/rules.go b/modules/mesh-knowledge/cmd/mesh-knowledge/rules.go index 2b7c5b83..87e72f3c 100644 --- a/modules/mesh-knowledge/cmd/mesh-knowledge/rules.go +++ b/modules/mesh-knowledge/cmd/mesh-knowledge/rules.go @@ -48,7 +48,7 @@ var PerRecordRules = []Rule{ {"self-link", "no link points at the record itself", "records.py supersession"}, {"rests-on", "a record it extends is accepted or proposed, never superseded or withdrawn", "records.py rests-on"}, {"supersedes-accepted", "a record it supersedes or changes is accepted", "records.py supersession, partial-supersession"}, - {"changes-in-force", "a clause it changes is in force or changed, never superseded, proposed or drafted", "records.py partial-supersession"}, + {"changes-in-force", "a clause it changes is in force or changed, never superseded or proposed; a drafted one may be changed", "records.py partial-supersession"}, {"live-citation", "a superseded record it cites is cited beside the record that replaced it", "records.py live-citation"}, {"became", "a graduated research effort names what it became", "cycle.py research"}, {"text-links", "every link its text writes to a decision record or research effort names one the knowledge base holds", "records.py links"}, @@ -261,8 +261,10 @@ func JudgeRecord(r *Record, held *Entry, lookup Lookup, ref *Reference) []Refusa if !target.Accepted() || target.Status == StatusSuperseded { refuse("supersedes-accepted", "%s changes %s, which is %s, not accepted", self, to, target.Status) } else if c := target.Clause(l.To.Clause); c != nil { - if (c.Status != ClauseInForce && c.Status != ClauseChanged) || c.Source == SourceDrafted { - refuse("changes-in-force", "%s changes %s §%d, which is %s (%s)", self, to, l.To.Clause, c.Status, c.Source) + // A drafted clause may be changed (novox/hq issue 494, ADR 0321): the change is recorded against the clause the + // knowledge base holds, by its number, which nothing renumbers once another record links it (Return). + if c.Status != ClauseInForce && c.Status != ClauseChanged { + refuse("changes-in-force", "%s changes %s §%d, which is %s", self, to, l.To.Clause, c.Status) } } case LinkCites: