amadan

oficina / memoria Public

Retire previous passages when sibling returns successful empty replacement

Paul Campbell pushed by paul@keymail.dev 465a74bc83535e61e95caf626b2df762317be1a2 parent 762a8b7
3 files changed, +118 −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 +95 −0

CI passed — run details

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 f2833d6..02ccc94 100644
--- a/internal/memoria/siblingsync.go
+++ b/internal/memoria/siblingsync.go
@@ -39,7 +39,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.
@@ -154,22 +154,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
}
@@ -274,7 +267,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
index b81777f..15aa4ca 100644
--- a/internal/memoria/siblingsync_empty_test.go
+++ b/internal/memoria/siblingsync_empty_test.go
@@ -1,7 +1,10 @@
package memoria
import (
+ "amadan.net/oficina/memoria/internal/memoria/sibling"
"context"
+ "net/http"
+ "net/http/httptest"
"testing"
"amadan.net/oficina/suite"
@@ -9,6 +12,8 @@ import (
// 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) {
@@ -50,3 +55,93 @@ func TestEmptySiblingReplacementRetiresPriorPassages(t *testing.T) {
})
}
}
+
+// 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)
+ }
+}