Merge pull request 'Knowledge base: withdraw a clause's 'changed by' mark once no accepted record changes it (issue 466)' (#489) from fix/kb-stale-marks into main

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

@@ -515,6 +515,9 @@ func (k *KB) accept(ctx context.Context, held *Entry, w *Record, by, why, warran
 
 // judgeSet holds the whole record set to its 4 rules and keeps what fails, which the module's health answers.
 func (k *KB) judgeSet(ctx context.Context) []Finding {
+	if _, err := k.RecomputeMarks(ctx); err != nil {
+		k.logf("[mesh-knowledge] %v", err)
+	}
 	all, err := k.Store.All(ctx)
 	if err != nil {
 		k.logf("[mesh-knowledge] the whole record set cannot be read to judge it: %v", err)

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

@@ -99,7 +99,7 @@ func serve(k *KB, status string) {
 	k.kick = make(chan struct{}, 1)
 	go hear(k)
 	go background(k)
-	if err := stdio.Serve("", append(Tools(k, callerOf), healthTool(status, k.now), highestTool(k))); err != nil {
+	if err := stdio.Serve("", append(Tools(k, callerOf), healthTool(status, k.now), highestTool(k), marksTool(k))); err != nil {
 		logf("%v", err)
 		os.Exit(1)
 	}

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

@@ -119,7 +119,7 @@ func TestItsProcessesAreThePreparationTheActAndTheJudge(t *testing.T) {
 	}
 	h, _ := watch["health"].(map[string]any)
 	if watch == nil || watch["run"].([]any)[1] != "watch" || h["kind"] != "tool" || h["tool"] != HealthTool ||
-		!reflect.DeepEqual(m.Tools, []string{HealthTool, HighestTool}) {
+		!reflect.DeepEqual(m.Tools, []string{HealthTool, HighestTool, MarksTool}) {
 		t.Fatalf("watch %v, tools %v", watch, m.Tools)
 	}
 	// The watch's verdict is whether the database serves, so its check names the provision it exercises: while the

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

@@ -0,0 +1,137 @@
+package main
+
+// The marks on a clause (changed, and by what) follow the accepted links, both ways: an acceptance that adds a
+// changes link sets the mark (accept), and a later accepted version that drops the link withdraws it here. Without
+// this a mark outlived its link, so holds and the whole-set rule `partial` said "changed" while links and get said
+// nothing (ADR 0536 v6 changed ADR 0525 §10; v12 only cited it).
+
+import (
+	"context"
+	"fmt"
+	"strconv"
+	"strings"
+
+	stdio "git.novox.be/novox/mesh-sdk/go"
+)
+
+// MarksTool is the module's tool that recomputes every mark now.
+const MarksTool = "knowledge_marks"
+
+// markedBy reads "ADR 0536 §4" or "research 045" as the record it names.
+func markedBy(s string) (Key, bool) {
+	f := strings.Fields(s)
+	if len(f) < 2 {
+		return Key{}, false
+	}
+	n, err := strconv.Atoi(f[1])
+	if err != nil {
+		return Key{}, false
+	}
+	switch strings.ToLower(f[0]) {
+	case "adr":
+		return Key{KindDecision, n}, true
+	case "research":
+		return Key{KindResearch, n}, true
+	}
+	return Key{}, false
+}
+
+// StaleMarks drops, on every record of the set, each "changed by" mark whose changing record is accepted and no
+// longer links the clause with a changes link; a clause left with no mark is in force again. A mark of a record
+// not accepted (a change confirmed ahead of its acceptance) or of a record not held stays. It changes the
+// records in place and answers the keys it changed with what it did.
+func StaleMarks(set []*Record) map[Key][]string {
+	byKey := map[Key]*Record{}
+	for _, r := range set {
+		byKey[r.Key()] = r
+	}
+	did := map[Key][]string{}
+	for _, t := range set {
+		if t.Status == StatusSuperseded {
+			continue
+		}
+		for i := range t.Clauses {
+			c := &t.Clauses[i]
+			if c.Status != ClauseChanged {
+				continue
+			}
+			var kept []string
+			for _, by := range c.ChangedBy {
+				from, ok := markedBy(by)
+				src := byKey[from]
+				if !ok || src == nil || !src.Accepted() || changesClause(src, t.Key(), c.N) {
+					kept = append(kept, by)
+					continue
+				}
+				did[t.Key()] = append(did[t.Key()], fmt.Sprintf("§%d no longer changed by %s", c.N, by))
+			}
+			if len(kept) != len(c.ChangedBy) {
+				c.ChangedBy = kept
+				if len(kept) == 0 {
+					c.Status = ClauseInForce
+				}
+			}
+		}
+	}
+	return did
+}
+
+func changesClause(src *Record, to Key, clause int) bool {
+	for _, l := range src.Links {
+		if k, ok := l.To.Record(); ok && l.Kind == LinkChanges && k == to && l.To.Clause == clause {
+			return true
+		}
+	}
+	return false
+}
+
+// RecomputeMarks withdraws every stale mark of the record set now, each a new version of the record it frees, and
+// says what it withdrew. judgeSet runs it first, so an acceptance, the background work every few minutes and a
+// start all agree with the links.
+func (k *KB) RecomputeMarks(ctx context.Context) ([]string, error) {
+	all, err := k.Store.All(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("the record set cannot be read to recompute its marks: %v", err)
+	}
+	set := make([]*Record, len(all))
+	for i, e := range all {
+		set[i] = e.Current.Clone()
+	}
+	did := StaleMarks(set)
+	var said []string
+	for i, e := range all {
+		what := did[e.Current.Key()]
+		if len(what) == 0 {
+			continue
+		}
+		why := strings.Join(what, "; ") + " (its changing record's accepted links no longer change it)"
+		if err := k.Store.Save(ctx, Version{By: "the knowledge base", At: k.now(), Why: why, Record: set[i]}, true, false, e.Revision); err != nil {
+			k.logf("[mesh-knowledge] the marks of %s could not be recomputed (%v): the next judgement tries again", e.Current.Key(), err)
+			continue
+		}
+		k.emit(EventChanged, set[i], "the knowledge base", why, e.Versions+1)
+		said = append(said, fmt.Sprintf("%s: %s", e.Current.Key(), why))
+	}
+	return said, nil
+}
+
+func marksTool(k *KB) stdio.Tool {
+	return stdio.Tool{Name: MarksTool,
+		Description: "Recompute every clause's changed and changed-by marks from the accepted links now, withdrawing each mark whose " +
+			"changing record no longer links the clause (the clause is in force again), and answer what it withdrew. The " +
+			"knowledge base does this on every acceptance and every few minutes; this runs it at once.",
+		Input: schema(nil, map[string]any{}),
+		Run: func(m map[string]any) (any, error) {
+			if _, err := readArgs(MarksTool, m); err != nil {
+				return nil, err
+			}
+			k.mu.Lock()
+			said, err := k.RecomputeMarks(context.Background())
+			k.mu.Unlock()
+			if err != nil {
+				return nil, err
+			}
+			findings := k.judgeSet(context.Background())
+			return map[string]any{"withdrawn": said, "findings": len(findings)}, nil
+		}}
+}

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

@@ -117,6 +117,37 @@ func TestARecordAcceptedCitingANewDecisionLetsItBeAcceptedAndDroppingTheLinkIsLo
 	})
 }
 
+// A mark follows the accepted links: a later accepted version that drops a changes link withdraws the clause's
+// mark, so holds, get and the rule `partial` agree with links (ADR 0536 v6 changed ADR 0525 §10; v12 only cited it).
+func TestACorrectionThatDropsAChangesLinkWithdrawsTheClausesMark(t *testing.T) {
+	eachStore(t, func(t *testing.T, s Store) {
+		w := imported(t, s)
+		n := w.reserved(t, "author@g14")
+		a := valid(n) // changes 0276 §1
+		w.accepted(t, "author@g14", a)
+		if c := mustGet(t, s, Key{KindDecision, 276}).Current.Clause(1); c.Status != ClauseChanged || len(c.ChangedBy) != 1 {
+			t.Fatalf("0276 §1 is not marked changed by the acceptance: %+v", c)
+		}
+		a.Links = a.Links[:1] // only extends 0276 now
+		a.Insight = "it no longer changes 0276 §1"
+		w.accepted(t, "author@g14", a)
+		c := mustGet(t, s, Key{KindDecision, 276}).Current.Clause(1)
+		if c.Status != ClauseInForce || len(c.ChangedBy) != 0 {
+			t.Fatalf("0276 §1 keeps a mark no accepted link makes: %+v", c)
+		}
+		f, _ := s.Findings(context.Background())
+		for _, x := range f {
+			if x.Rule == "partial" {
+				t.Fatalf("a stale mark is still a finding: %+v", x)
+			}
+		}
+		// Recomputing again changes nothing.
+		if said, err := w.k.RecomputeMarks(context.Background()); err != nil || len(said) != 0 {
+			t.Fatalf("a second recompute withdrew %v (%v)", said, err)
+		}
+	})
+}
+
 // Each of the 4 whole-set rules, failing on a record set made to break it.
 func TestEachWholeSetRuleFails(t *testing.T) {
 	acc := func(n int, links ...Link) *Record {

modules/mesh-knowledge/module.json

@@ -58,7 +58,8 @@
   ],
   "tools": [
     "knowledge_health",
-    "knowledge_highest"
+    "knowledge_highest",
+    "knowledge_marks"
   ],
   "emits": [
     "reserved",