Listing loops, insights or the review queue adopts hand deletions and says how many it forgot

cmd/logos/dream.go

@@ -111,6 +111,9 @@ func dreamReview() error {
 	if err := dream.InitQueue(ix.DB); err != nil {
 		return err
 	}
+	if err := announceInsightEdits(ix.DB); err != nil {
+		return err
+	}
 
 	ins, err := dream.List(ix.DB, dream.Pending)
 	if err != nil {
@@ -151,6 +154,9 @@ func dreamDecide(args []string, accept bool) error {
 	if err := dream.InitQueue(ix.DB); err != nil {
 		return err
 	}
+	if err := announceInsightEdits(ix.DB); err != nil {
+		return err
+	}
 
 	if !accept {
 		if err := dream.SetStatus(ix.DB, id, dream.Rejected); err != nil {
@@ -189,3 +195,14 @@ func memText(db *sql.DB, id int64) string {
 	}
 	return t
 }
+
+// announceInsightEdits adopts hand edits to the insight queue before review,
+// accept or reject reads it, and says what they did. See announceAdoption.
+func announceInsightEdits(db *sql.DB) error {
+	restored, removed, err := dream.Reconcile(db)
+	if err != nil {
+		return err
+	}
+	announceAdoption(dream.InsightsFile, "insight", "discarded", restored, removed)
+	return nil
+}

cmd/logos/hand_edits_test.go

@@ -0,0 +1,142 @@
+package main
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/Coder8124/logos/internal/dream"
+	"github.com/Coder8124/logos/internal/memory"
+	"github.com/Coder8124/logos/internal/secretary"
+)
+
+// deleteLineBy removes every line of path that contains needle, the way a
+// person deletes a bullet in their editor.
+func deleteLineBy(t *testing.T, path, needle string) {
+	t.Helper()
+	raw, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var kept []string
+	for _, line := range strings.Split(string(raw), "\n") {
+		if !strings.Contains(line, needle) {
+			kept = append(kept, line)
+		}
+	}
+	if len(kept) == len(strings.Split(string(raw), "\n")) {
+		t.Fatalf("%s has no line containing %q:\n%s", path, needle, raw)
+	}
+	if err := os.WriteFile(path, []byte(strings.Join(kept, "\n")), 0o644); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// #240: listing read the cache and never the file, so a loop deleted from
+// loops.md was still listed — at the one moment a person checks whether their
+// edit took.
+func TestALoopDeletedFromTheFileIsNotListed(t *testing.T) {
+	vaultDir := t.TempDir()
+	t.Setenv("LOGOS_VAULT", vaultDir)
+	t.Setenv("LOGOS_EMBED", "off")
+
+	for _, text := range []string{"send the optics quote", "renew the lab badge"} {
+		if err := commitmentCmd([]string{"add", text}); err != nil {
+			t.Fatal(err)
+		}
+	}
+	deleteLineBy(t, secretary.LoopsPath(vaultDir), "send the optics quote")
+
+	out := captureStdout(t, func() {
+		if err := commitmentCmd(nil); err != nil {
+			t.Fatal(err)
+		}
+	})
+	if strings.Contains(out, "send the optics quote") {
+		t.Errorf("`logos loop` still lists a loop deleted from loops.md:\n%s", out)
+	}
+	if !strings.Contains(out, "renew the lab badge") {
+		t.Errorf("`logos loop` lost the loop that was not deleted:\n%s", out)
+	}
+	if !strings.Contains(out, "1 loop forgotten") {
+		t.Errorf("`logos loop` adopted the deletion without saying so:\n%s", out)
+	}
+}
+
+// #241: a write adopts the file before it rewrites it, and that adoption was
+// silent — `loop add` printed only "tracked" while it also forgot a loop.
+func TestAddingALoopAfterAHandDeletionSaysTheDeletedLoopWasForgotten(t *testing.T) {
+	vaultDir := t.TempDir()
+	t.Setenv("LOGOS_VAULT", vaultDir)
+	t.Setenv("LOGOS_EMBED", "off")
+
+	if err := commitmentCmd([]string{"add", "send the optics quote"}); err != nil {
+		t.Fatal(err)
+	}
+	deleteLineBy(t, secretary.LoopsPath(vaultDir), "send the optics quote")
+
+	out := captureStdout(t, func() {
+		if err := commitmentCmd([]string{"add", "renew the lab badge"}); err != nil {
+			t.Fatal(err)
+		}
+	})
+	if !strings.Contains(out, "1 loop forgotten") || !strings.Contains(out, "tracked") {
+		t.Errorf("`loop add` after a hand deletion should say one loop was forgotten, then tracked:\n%s", out)
+	}
+}
+
+// #240 for the review queue: a proposal deleted from pending.md is a
+// rejection, and `logos review` offered it for review anyway.
+func TestAProposalDeletedFromTheQueueFileIsNotOfferedForReview(t *testing.T) {
+	queueForReview(t, "kestrel-one", "Staging DB is on port 5433.")
+	vaultDir := os.Getenv("LOGOS_VAULT")
+	deleteLineBy(t, filepath.Join(vaultDir, memory.Dir, memory.PendingFile), "5433")
+
+	out := captureStdout(t, func() {
+		if err := runReview(nil); err != nil {
+			t.Fatal(err)
+		}
+	})
+	if !strings.Contains(out, "review queue is empty") {
+		t.Errorf("`logos review` offered a proposal deleted from the queue file:\n%s", out)
+	}
+	if !strings.Contains(out, "1 proposal rejected") {
+		t.Errorf("`logos review` adopted the deletion without saying so:\n%s", out)
+	}
+}
+
+// #240 for dreamed insights: the same, for `logos dream review`.
+func TestAnInsightDeletedFromTheQueueFileIsNotOfferedForReview(t *testing.T) {
+	vaultDir := t.TempDir()
+	t.Setenv("LOGOS_VAULT", vaultDir)
+	t.Setenv("LOGOS_EMBED", "off")
+
+	ix, err := openEvents()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := dream.InitQueue(ix.DB); err != nil {
+		t.Fatal(err)
+	}
+	if err := dream.Enqueue(ix.DB, &dream.Insight{
+		Kind: dream.Connection, Text: "the badge renewal and the optics quote share a vendor",
+		EndpointA: 1, EndpointB: 2, Conf: 0.7, Model: "test",
+	}); err != nil {
+		t.Fatal(err)
+	}
+	ix.Close()
+	deleteLineBy(t, dream.InsightsPath(vaultDir), "share a vendor")
+
+	out := captureStdout(t, func() {
+		if err := dreamReview(); err != nil {
+			t.Fatal(err)
+		}
+	})
+	if strings.Contains(out, "share a vendor") {
+		t.Errorf("`logos dream review` offered an insight deleted from the queue file:\n%s", out)
+	}
+	if !strings.Contains(out, "1 insight discarded") {
+		t.Errorf("`logos dream review` adopted the deletion without saying so:\n%s", out)
+	}
+}

cmd/logos/handedit.go

@@ -0,0 +1,30 @@
+package main
+
+import (
+	"fmt"
+	"strings"
+)
+
+// announceAdoption says what a hand edit to one of the vault's list files —
+// loops.md, the insight queue, the review queue — did to the cache, before the
+// command's own output.
+//
+// The files tell the user that deleting a line forgets the record, and the
+// stores honour it by adopting the file before they read or rewrite it. That
+// adoption was silent: `loop add` printed only "tracked" on a run that also
+// forgot a loop, which is the silent background work invariant 3 rules out.
+// Printed only when something changed, so the common case — a file nobody
+// touched — adds nothing to the output.
+func announceAdoption(file, noun, removedVerb string, restored, removed int) {
+	var parts []string
+	if removed > 0 {
+		parts = append(parts, fmt.Sprintf("%d %s %s", removed, plural(removed, noun), removedVerb))
+	}
+	if restored > 0 {
+		parts = append(parts, fmt.Sprintf("%d %s taken from the file", restored, plural(restored, noun)))
+	}
+	if len(parts) == 0 {
+		return
+	}
+	fmt.Printf("adopted your edit to %s: %s\n", file, strings.Join(parts, ", "))
+}

cmd/logos/loop.go

@@ -23,6 +23,13 @@ func commitmentCmd(args []string) error {
 	if err := secretary.Init(ix.DB); err != nil {
 		return err
 	}
+	// Before the list as well as the writes: listing is when a person checks
+	// that deleting a line took, and it read the cache alone.
+	restored, removed, err := secretary.Reconcile(ix.DB)
+	if err != nil {
+		return err
+	}
+	announceAdoption(secretary.LoopsFile, "loop", "forgotten", restored, removed)
 
 	// `list` is a synonym for the bare form, not a subcommand of its own. Every
 	// other listing verb here takes it, the help line named only the three verbs

cmd/logos/review.go

@@ -47,6 +47,13 @@ func runReview(args []string) error {
 	if err := memory.Init(ix.DB); err != nil {
 		return err
 	}
+	// Before the queue is read: a proposal whose line the user deleted is one
+	// they already rejected, and offering it again asks them twice.
+	restored, rejected, err := memory.ReconcilePending(ix.DB)
+	if err != nil {
+		return err
+	}
+	announceAdoption(memory.PendingFile, "proposal", "rejected", restored, rejected)
 
 	memories, err := memory.Pending(ix.DB)
 	if err != nil {

internal/dream/insightstore.go

@@ -104,37 +104,58 @@ func withQueue(db *sql.DB, fn func(dir string) error) error {
 		return err
 	}
 	defer g.Unlock()
-	if err := reconcileLocked(db, dir); err != nil {
+	// The counts are dropped here, not lost: a command that announces the
+	// adoption calls Reconcile first, and this pass then finds the file ours.
+	if _, _, err := reconcileLocked(db, dir); err != nil {
 		return err
 	}
 	return fn(dir)
 }
 
+// Reconcile adopts hand edits to the queue file now, and reports how many
+// insights the file put back and how many it discarded.
+//
+// Exported for `logos dream review` and its accept/reject: only writes used to
+// reconcile, so the review listed an insight the user had deleted from the
+// file, and a write that did adopt the deletion said nothing about it.
+func Reconcile(db *sql.DB) (restored, removed int, err error) {
+	dir := vaultFor(db)
+	if dir == "" {
+		return 0, 0, nil
+	}
+	g, err := vault.Lock(dir, lockName)
+	if err != nil {
+		return 0, 0, err
+	}
+	defer g.Unlock()
+	return reconcileLocked(db, dir)
+}
+
 // reconcileLocked adopts hand edits to the queue file, with the lock held. A
 // file we wrote ourselves is skipped on its hash; an absent file says nothing
 // about what is queued, as Import explains.
-func reconcileLocked(db *sql.DB, dir string) error {
+func reconcileLocked(db *sql.DB, dir string) (restored, removed int, err error) {
 	raw, err := os.ReadFile(InsightsPath(dir))
 	if os.IsNotExist(err) {
-		return nil
+		return 0, 0, nil
 	}
 	if err != nil {
-		return err
+		return 0, 0, err
 	}
 	if stamps.Ours(db, raw) {
-		return nil
+		return 0, 0, nil
 	}
 	if looksTruncated(string(raw)) {
 		// Import refuses this file and says so on `logos index`. Here it is a
 		// reason not to adopt it, not to fail the write: the rewrite that
 		// follows replaces the torn file with the cache's complete copy.
-		return nil
+		return 0, 0, nil
 	}
-	if _, err := adoptLocked(db, raw); err != nil {
-		return err
+	if restored, removed, err = adoptLocked(db, raw); err != nil {
+		return restored, removed, err
 	}
 	stamps.Adopted(db, raw)
-	return nil
+	return restored, removed, nil
 }
 
 // flushLocked rewrites the whole file from the database, with the lock held.
@@ -395,7 +416,7 @@ func Import(db *sql.DB, dir string) (int, error) {
 				"restore the file or delete the partial line to accept it as-is", InsightsFile)
 	}
 
-	restored, err := adoptLocked(db, raw)
+	restored, _, err := adoptLocked(db, raw)
 	if err != nil {
 		return restored, err
 	}
@@ -405,11 +426,10 @@ func Import(db *sql.DB, dir string) (int, error) {
 
 // adoptLocked makes the cache match the file: every valid line is upserted at
 // its id, and every row with no line is deleted. Returns how many rows it had
-// to create.
-func adoptLocked(db *sql.DB, raw []byte) (int, error) {
+// to create and how many it deleted, so a command can say both.
+func adoptLocked(db *sql.DB, raw []byte) (restored, removed int, err error) {
 	parsed := parseInsights(string(raw))
 	keep := map[int64]bool{}
-	restored := 0
 	for _, in := range parsed {
 		// Validate, not trust: the file is editable by hand, and an insight
 		// that cannot name the two memories it bridges is the fabrication the
@@ -421,7 +441,7 @@ func adoptLocked(db *sql.DB, raw []byte) (int, error) {
 		}
 		created, err := upsertInsight(db, in)
 		if err != nil {
-			return restored, err
+			return restored, removed, err
 		}
 		keep[in.ID] = true
 		if created {
@@ -434,14 +454,14 @@ func adoptLocked(db *sql.DB, raw []byte) (int, error) {
 	// vault, and leaving the row would put the text back on the next flush.
 	rows, err := db.Query("SELECT id FROM dream_insights")
 	if err != nil {
-		return restored, err
+		return restored, removed, err
 	}
 	var gone []int64
 	for rows.Next() {
 		var id int64
 		if err := rows.Scan(&id); err != nil {
 			rows.Close()
-			return restored, err
+			return restored, removed, err
 		}
 		if !keep[id] {
 			gone = append(gone, id)
@@ -450,10 +470,11 @@ func adoptLocked(db *sql.DB, raw []byte) (int, error) {
 	rows.Close()
 	for _, id := range gone {
 		if _, err := db.Exec("DELETE FROM dream_insights WHERE id = ?", id); err != nil {
-			return restored, err
+			return restored, removed, err
 		}
+		removed++
 	}
-	return restored, nil
+	return restored, removed, nil
 }
 
 // upsertInsight restores one insight, keeping the id the file carries so

internal/memory/memory.go

@@ -361,7 +361,7 @@ func storeLocked(db *sql.DB, p *provider.Provider, embedModel string, m *Memory)
 		// insert, and the queue lock held from there through the write.
 		var rec Receipt
 		err := withPending(db, func(dir string) error {
-			if err := reconcilePendingLocked(db, dir); err != nil {
+			if _, _, err := reconcilePendingLocked(db, dir); err != nil {
 				return err
 			}
 			var err error