amadan

oficina / memoria Public

Retire previous passages when sibling returns successful empty replacement

Paul Campbell pushed by paul@keymail.dev b22c65da413775f7c66015282a9bfd94a00e479c parent 1365bcb
3 files changed, +170 −17
  • docs/superpowers/specs/2026-09-07-empty-sibling-replacement.md +13 −0
  • internal/memoria/siblingsync.go +10 −17
  • internal/memoria/siblingsync_empty_test.go +147 −0
diff --git a/docs/superpowers/specs/2026-09-07-empty-sibling-replacement.md b/docs/superpowers/specs/2026-09-07-empty-sibling-replacement.md
new file mode 100644
index 0000000..952384d
--- /dev/null
+++ b/docs/superpowers/specs/2026-09-07-empty-sibling-replacement.md
@@ -0,0 +1,13 @@
+# Successful empty sibling replacement
+
+Status: bounded draft implementation, independent Fable review pending. Base a012c8f. No merge/deploy approval.
+
+Authority: current Memoria design §4.1 revision replacement and decision8. Reset and post-restore reconciliation remain governed by §4.1's completed-marker/count requirement; this change implements no reset handling.
+
+Reproduced before implementation in commit762a8b7: Poll ingests revision7, then consumes a non-gone revision8 row and successful empty/whitespace S3 response. It reports Empty and advances the cursor while retaining revision7 in Document, Chunk and the index. A complete current body with no passages cannot justify retaining the previous passages.
+
+Change only applyRow's successful empty-body branch: invoke existing deleteBySiblingItem, scoped by configured source plus keyed external item ID, and count actual retirements in Deleted alongside Empty. An unseen/repeated empty item deletes zero records. Index-removal or DB errors abort the page before cursor advancement. Existing 404 Missing and other errors never enter this branch. Do not manufacture a title, empty passage or source deletion.
+
+Existing Store.Delete removes index entries before its DB transaction, and refuses DB deletion on index failure. This draft uses that established path; it does not claim to implement the future durable tombstone log, derived-artifact storage, atomic multi-item feed application or reset reconciliation. Those are independent missing capabilities. A DB failure after index removal retains stored rows but may temporarily reduce index results until rebuild; this is existing deletion behavior, not newly asserted atomicity.
+
+Tests exercise actual Poll through its real HTTP client: nonempty-to-empty and whitespace replacement, prior index removal, cross-source same-ID isolation, repeated replacement,404/403/500 retention, index failure with unchanged cursor, and409 reset retention. Regression ran red before implementation; removing retirement through a Go overlay reproduces red after implementation. No browser/UI copy changes.
diff --git a/internal/memoria/siblingsync.go b/internal/memoria/siblingsync.go
index 1b7af1e..7367242 100644
--- a/internal/memoria/siblingsync.go
+++ b/internal/memoria/siblingsync.go
@@ -42,7 +42,7 @@ type PollResult struct {
Missing int
// Empty counts items the sibling served with no body at all.
//
- // Skipped, not stored: a stub page has no passages, so there is
+ // Not stored; any previous copy is retired. A stub has no passages, so there is
// nothing to retrieve and nothing to cite. Counted rather than passed
// over in silence, because a source that is suddenly all stubs is a
// fact about the sibling worth being able to see.
@@ -159,22 +159,15 @@ func (s *Store) applyRow(ctx context.Context, c *sibling.Client, sib sibling.Sib
return err
}
- // **An empty item is skipped and counted, not stored and not failed.**
- //
- // This said `AllowTitleOnly: true` and claimed it stopped one empty
- // document blocking a catch-up. It did the opposite: no Title is
- // passed here — a sibling's S3 gives a body and nothing else — so
- // Ingest refused with "neither a body nor a title", the row failed,
- // the cursor stayed put, and **every subsequent poll of that sibling
- // failed on the same row forever**. One stub page in Docs and nothing
- // from Docs is ever ingested again. A review found it.
- //
- // Skipping is right rather than a fallback title: a stub page has no
- // passages, so there is nothing to retrieve and nothing to cite, and
- // inventing a title from an empty body would put a document in the
- // corpus whose only content is its own name. The count is what keeps
- // it from being silent.
+ // A successful empty replacement has no passages. Retire any previous
+ // copy of this item; merely skipping would leave its old revision indexed.
+ // Refusals and failures returned above never enter this replacement path.
if strings.TrimSpace(string(body)) == "" {
+ n, err := s.deleteBySiblingItem(ctx, sib.Name, row.ItemID)
+ if err != nil {
+ return err
+ }
+ res.Deleted += n
res.Empty++
return nil
}
@@ -279,7 +272,7 @@ func (s *Store) upsertFeed(ctx context.Context, app string, fields map[string]an
})
}
-// deleteBySiblingItem applies one `gone` row.
+// deleteBySiblingItem retires one gone or successfully emptied item.
//
// It resolves the sibling's item id the same way Ingest stores it — the
// keyed fingerprint over (source, key) — because that is the only handle
diff --git a/internal/memoria/siblingsync_empty_test.go b/internal/memoria/siblingsync_empty_test.go
new file mode 100644
index 0000000..15aa4ca
--- /dev/null
+++ b/internal/memoria/siblingsync_empty_test.go
@@ -0,0 +1,147 @@
+package memoria
+
+import (
+ "amadan.net/oficina/memoria/internal/memoria/sibling"
+ "context"
+ "net/http"
+ "net/http/httptest"
+ "testing"
+
+ "amadan.net/oficina/suite"
+)
+
+// Regression: a successful empty replacement used to leave the prior
+// revision in storage and the index after Poll advanced its cursor.
+// Mutation checked 2026-09-07: removing the empty-path retirement makes
+// both cases fail with the prior document still held.
+func TestEmptySiblingReplacementRetiresPriorPassages(t *testing.T) {
+ for _, body := range []string{"", " \n\t "} {
+ t.Run(body, func(t *testing.T) {
+ ctx := context.Background()
+ s := storeForTest(t)
+ f := &feedServer{t: t, pages: map[string]suite.Feed{
+ "": {Cursor: "c1", Rows: []suite.FeedRow{row("item", 7, false)}},
+ "c1": {Cursor: "c1", Rows: []suite.FeedRow{}},
+ }, bodies: map[string]string{"item": "Synthetic retained passage from an earlier revision."}}
+ sib, client := f.start(t)
+ if _, err := s.Poll(ctx, client, sib); err != nil {
+ t.Fatal(err)
+ }
+ assertHeld(t, s, "Docs", "item", 7)
+ n, err := s.Count()
+ if err != nil || n == 0 {
+ t.Fatalf("initial index count=%d error=%v", n, err)
+ }
+ f.pages["c1"] = suite.Feed{Cursor: "c2", Rows: []suite.FeedRow{row("item", 8, false)}}
+ f.pages["c2"] = suite.Feed{Cursor: "c2", Rows: []suite.FeedRow{}}
+ f.bodies["item"] = body
+ res, err := s.Poll(ctx, client, sib)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if res.Empty != 1 {
+ t.Fatalf("empty=%d", res.Empty)
+ }
+ if heldCount(t, s, "Docs") != 0 {
+ t.Fatal("successful empty replacement retained the old document")
+ }
+ n, err = s.Count()
+ if err != nil || n != 0 {
+ t.Fatalf("stale index count=%d error=%v", n, err)
+ }
+ if cursorOf(t, s, "Docs") != "c2" {
+ t.Fatal("replacement cursor not applied")
+ }
+ })
+ }
+}
+
+// The successful replacement must stay scoped to its source and remain
+// retryable if index removal fails; an HTTP refusal is never an empty body.
+func TestEmptySiblingReplacementBoundaries(t *testing.T) {
+ for _, mode := range []string{"empty", "missing", "forbidden", "outage", "index-failure"} {
+ t.Run(mode, func(t *testing.T) {
+ ctx := context.Background()
+ s := storeForTest(t)
+ for _, source := range []string{"Docs", "Sheets"} {
+ if _, err := s.Ingest(ctx, IngestSpec{Source: source, Key: "item", ItemID: "item", SiblingRevision: 7, Body: "Synthetic old indexed content."}); err != nil {
+ t.Fatal(err)
+ }
+ }
+ f := &feedServer{t: t, pages: map[string]suite.Feed{
+ "": {Cursor: "next", Rows: []suite.FeedRow{row("item", 8, false)}},
+ "next": {Cursor: "next", Rows: []suite.FeedRow{}},
+ }, bodies: map[string]string{"item": ""}}
+ switch mode {
+ case "missing":
+ f.failS3 = map[string]bool{"item": true}
+ case "forbidden":
+ f.forbidS3 = map[string]bool{"item": true}
+ case "outage":
+ f.brokeS3 = map[string]bool{"item": true}
+ case "index-failure":
+ if err := s.ix.Close(); err != nil {
+ t.Fatal(err)
+ }
+ }
+ sib, client := f.start(t)
+ res, err := s.Poll(ctx, client, sib)
+ wantsError := mode == "forbidden" || mode == "outage" || mode == "index-failure"
+ if (err != nil) != wantsError {
+ t.Fatalf("error=%v, wantsError=%v", err, wantsError)
+ }
+ assertHeld(t, s, "Sheets", "item", 7)
+ if mode == "empty" {
+ if heldCount(t, s, "Docs") != 0 || res.Deleted != 1 || res.Empty != 1 {
+ t.Fatalf("replacement result=%+v", res)
+ }
+ // Replaying the same successful replacement cannot report a second deletion.
+ if err := s.setCursor(ctx, "Docs", ""); err != nil {
+ t.Fatal(err)
+ }
+ again, err := s.Poll(ctx, client, sib)
+ if err != nil || again.Deleted != 0 || again.Empty != 1 {
+ t.Fatalf("replay=%+v error=%v", again, err)
+ }
+ } else {
+ assertHeld(t, s, "Docs", "item", 7)
+ if res.Deleted != 0 || res.Empty != 0 {
+ t.Fatalf("failure counted as replacement: %+v", res)
+ }
+ if wantsError && cursorOf(t, s, "Docs") != "" {
+ t.Fatal("failed replacement advanced cursor")
+ }
+ }
+ })
+ }
+}
+
+// A reset response is a failed poll until a complete reconciliation protocol
+// exists; it cannot be interpreted as a successful empty replacement.
+func TestSiblingResetRetainsCorpus(t *testing.T) {
+ ctx := context.Background()
+ s := storeForTest(t)
+ if _, err := s.Ingest(ctx, IngestSpec{Source: "Docs", Key: "item", ItemID: "item", SiblingRevision: 7, Body: "Synthetic retained source content."}); err != nil {
+ t.Fatal(err)
+ }
+ if err := s.setCursor(ctx, "Docs", "old"); err != nil {
+ t.Fatal(err)
+ }
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.WriteHeader(http.StatusConflict)
+ _, _ = w.Write([]byte(`{"code":"CURSOR_RESET"}`))
+ }))
+ defer srv.Close()
+ res, err := s.Poll(ctx, sibling.NewForTest(), sibling.Sibling{Name: "Docs", BaseURL: srv.URL, Token: "synthetic"})
+ if err == nil || res.Deleted != 0 {
+ t.Fatalf("reset result=%+v error=%v", res, err)
+ }
+ assertHeld(t, s, "Docs", "item", 7)
+ if cursorOf(t, s, "Docs") != "old" {
+ t.Fatal("reset advanced cursor")
+ }
+ n, err := s.Count()
+ if err != nil || n == 0 {
+ t.Fatalf("reset removed index: count=%d error=%v", n, err)
+ }
+}