π€ feat: no passage is spoken without its owner's answer, structurally
The one rule stops being a convention and becomes a signature. There is one method that returns renderable text, it takes the asker, and the ruling happens inside it β so "every path that would speak a sibling's passage asks first" is true of every path because there is no other way to obtain a passage. A version that returned raw hits and trusted its callers would be correct exactly as long as nobody added a second caller. **The model gains what S2 can actually be asked about.** `ExternalID` is a keyed fingerprint, deliberately one-way so the bucket does not confirm which documents a team holds β right for finding a row and useless for asking somebody about it. So a sibling document now carries its app of record's own item id, sealed under its own purpose, and the revision that app issued. Two corrections fell out of writing that down: - **`Document.Revision` is memoria's own counter and is not what S2 is asked about.** The model said it was. Two apps counting independently agree by accident, so a re-check keyed on it would have been a quiet misauthorization rather than a missing one. - **`seal.PurposeItemID` is separate from `PurposeCoordinate`** even though both seal "an id handed back to its source", because the sources differ. One purpose for both means a bug that opens a Notion page id on the sibling path sends a Notion coordinate to Docs and it decrypts cleanly on the way out. **Nobody to ask is not nobody objecting.** A sibling dropped from the roster, and an instance with no custody wired at all, both leave memoria holding passages whose owner it cannot reach. Both withhold loudly. The tempting reading β an unreachable owner imposes no restriction β is the disclosure Β§4.1 exists to prevent, and it is one `continue` away in either branch. **The no-match copy was about to lie.** "Nothing matched β¦ so this is a real absence rather than an empty index" is false the moment anything was withheld, and it would have been asserted on the very line written to prevent that failure. It now has a third arm. Ten mutations on the ruling path, all red, none surviving β including reading the item id from ExternalID, and asking S2 with memoria's revision instead of the sibling's. **And one bug found by mutating my own fix.** The first version built its notices by ranging a `map[int]string`, so a page with two unreachable apps ordered them differently on every reload. Go randomises map iteration precisely so that surfaces. Notices are now aggregated once, in result order β the order the reader meets the passages the lines are about β and `TestNoticesAreInResultOrder` runs twenty times.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H3VKtXnq6fSmM2F11Bjcsy
18 files changed,
+894
−47
AGENTS.md+20 −0internal/memoria/app.go+17 −1internal/memoria/custody.go+62 −0internal/memoria/custody_test.go+401 −0internal/memoria/delete_test.go+4 −4internal/memoria/handlers.go+34 −3internal/memoria/migrations/0005_alter_documents.sql+4 −0internal/memoria/migrations/schema.sql+1 −1internal/memoria/models.go+42 −3internal/memoria/seal/seal.go+12 −0internal/memoria/searchhelper_test.go+28 −0internal/memoria/store.go+199 −18internal/memoria/templates/results.html+24 −1internal/memoriatest/ancestry_test.go+5 −5internal/memoriatest/connector_test.go+9 −9internal/memoriatest/regress_test.go+1 −1internal/memoriatest/searchhelper_test.go+30 −0tools/seed/main.go+1 −1
diff --git a/AGENTS.md b/AGENTS.md| index 0b64747..fe46f20 100644 |
| --- a/AGENTS.md |
| +++ b/AGENTS.md |
| @@ -46,6 +46,26 @@ is consistent, not that the gate works β they are the cases already |
| defended against while writing it. **Have someone who did not write the |
| gate pick the mutations.** Β§Review's argument, one level down. (Β§1.9.) |
| +**Assert the state, not the copy β and if your own mutation cannot reach |
| +a guard, the guard is the finding.** Both of these were learnt on the same |
| +afternoon, from the two mutations out of twenty that survived while |
| +building Β§4.1's policy layer (2026-09-06). |
| + |
| +`Outcome.Record` filing the silent `NOT_VISIBLE` rulings alongside the |
| +loud ones rendered an identical page β nothing prints them β while |
| +quietly accumulating a per-app count of passages the reader may not see, |
| +one switch arm or debug print from being the leak Β§4.1 forbids. The test |
| +read `Lines()` and could not tell the two apart; the test that catches it |
| +reads the struct. |
| + |
| +And `scatter`'s short-slice fallback could not be reached by any fixture, |
| +because the function above it is total. Changing that fallback from |
| +`UNKNOWN` to `VISIBLE` left the suite green. A defence nothing can reach |
| +is the same object as a guard written to pass, whatever the intent β so |
| +either make it reachable (extract it, call it directly, as `scatter` now |
| +is) or delete it. Leaving it in place with a comment is the third option |
| +and it is the one that ships broken. |
| + |
| **A count that fails is not a count of zero.** `chunkCount` returned 0 on |
| error, and the page then told the reader "so this is a real absence rather |
| than an empty index" β the codebase's own worst failure shape, asserted on |
diff --git a/internal/memoria/app.go b/internal/memoria/app.go| index eb145ba..206cb24 100644 |
| --- a/internal/memoria/app.go |
| +++ b/internal/memoria/app.go |
| @@ -36,7 +36,12 @@ func App(d *db.DB, origin string, sealer *seal.Sealer, logger *slog.Logger) (*ht |
| return nil, nil, err |
| } |
| - store := NewStore(d, sealer, ix) |
| + // Custody starts nil and stays nil until UseSiblings supplies one. |
| + // nil withholds every sibling passage rather than rendering it, so |
| + // the un-wired state is the closed one β slice 0 stores nothing owned |
| + // by anybody else, which makes nil correct rather than a gap. See |
| + // Store.custody, and UseSiblings for why this is a setter. |
| + store := NewStore(d, sealer, ix, nil) |
| indexed, err := store.Rebuild(context.Background()) |
| if err != nil { |
| // A failed rebuild is fatal, not a warning. Serving with a |
| @@ -198,6 +203,17 @@ func (a *app) isOwner(addr string) bool { |
| return false |
| } |
| +// UseSiblings supplies the apps of record memoria may ask about |
| +// passages. Called by main from Config. |
| +// |
| +// A setter, in the shape UseAuth already has here, and with the same |
| +// hazard: production can forget to call it. What makes that survivable is |
| +// which way the omission fails β a store with no custody withholds every |
| +// sibling passage and says so on the page, where UseAuth's omission |
| +// served every route unauthenticated. The failure is loud and visible to |
| +// the first reader, not silent. |
| +func (a *app) UseSiblings(c Custody) { a.store.custody = c } |
| + |
| // SetOwners supplies the owner list. Called by main from Config. |
| func (a *app) SetOwners(owners []string) { a.owners = owners } |
diff --git a/internal/memoria/custody.go b/internal/memoria/custody.go| new file mode 100644 |
| index 0000000..36f4537 |
| --- /dev/null |
| +++ b/internal/memoria/custody.go |
| @@ -0,0 +1,62 @@ |
| +package memoria |
| + |
| +import ( |
| + "context" |
| + |
| + "amadan.net/oficina/suite" |
| + |
| + "amadan.net/oficina/memoria/internal/memoria/sibling" |
| +) |
| + |
| +// Custody is the app's route to the one rule: |
| +// |
| +// memoria never speaks a passage without asking whoever owns it whether |
| +// this person may see it, right now. |
| +// |
| +// An interface rather than the concrete client for one reason that is not |
| +// testing convenience: **a nil Custody must fail closed, and a type that |
| +// can be nil makes that a case somebody has to write down.** Slice 0 has |
| +// no siblings configured and every document is memoria-owned, so the S2 |
| +// path would otherwise be dead code that comes alive on the day the first |
| +// sibling is configured β which is the worst possible day for it to run |
| +// for the first time. |
| +type Custody interface { |
| + // Sibling resolves a Document.Source to a configured app of record. |
| + // |
| + // **A source that does not resolve is not a source that renders.** A |
| + // sibling removed from configuration leaves its documents in the |
| + // store, and there is then nobody to ask about them β which is |
| + // UNKNOWN, which withholds loudly. It is emphatically not "no |
| + // restriction applies". |
| + Sibling(source string) (sibling.Sibling, bool) |
| + |
| + // Rule answers for every candidate, in the order given. It does not |
| + // fail: Β§4.1's four values already cover every outcome. |
| + Rule(ctx context.Context, asker suite.Subject, cands []sibling.Candidate) ([]sibling.Disposition, *sibling.Outcome) |
| +} |
| + |
| +// Siblings is the production Custody: a roster of configured apps of |
| +// record and the client that asks them. |
| +type Siblings struct { |
| + client *sibling.Client |
| + byName map[string]sibling.Sibling |
| +} |
| + |
| +// NewSiblings builds the roster. `team` binds the transport to one team's |
| +// hosts; Β§6.4 is what that enforces and it is the module's to enforce. |
| +func NewSiblings(team string, dial suite.DialFunc, apps []sibling.Sibling) *Siblings { |
| + byName := make(map[string]sibling.Sibling, len(apps)) |
| + for _, s := range apps { |
| + byName[s.Name] = s |
| + } |
| + return &Siblings{client: sibling.New(team, dial), byName: byName} |
| +} |
| + |
| +func (s *Siblings) Sibling(source string) (sibling.Sibling, bool) { |
| + got, ok := s.byName[source] |
| + return got, ok |
| +} |
| + |
| +func (s *Siblings) Rule(ctx context.Context, asker suite.Subject, cands []sibling.Candidate) ([]sibling.Disposition, *sibling.Outcome) { |
| + return s.client.Rule(ctx, asker, cands) |
| +} |
diff --git a/internal/memoria/custody_test.go b/internal/memoria/custody_test.go| new file mode 100644 |
| index 0000000..dcd80ef |
| --- /dev/null |
| +++ b/internal/memoria/custody_test.go |
| @@ -0,0 +1,401 @@ |
| +package memoria |
| + |
| +import ( |
| + "context" |
| + "io" |
| + "log/slog" |
| + "path/filepath" |
| + "reflect" |
| + "strings" |
| + "testing" |
| + |
| + "github.com/carlosframework/rastrillo/db" |
| + "github.com/carlosframework/rastrillo/migrate" |
| + |
| + "amadan.net/oficina/suite" |
| + |
| + "amadan.net/oficina/memoria/internal/memoria/index" |
| + "amadan.net/oficina/memoria/internal/memoria/seal" |
| + "amadan.net/oficina/memoria/internal/memoria/sibling" |
| +) |
| + |
| +// fixedCustody rules every candidate the same way and records what it was |
| +// asked. It is the seam that lets Β§4.1's table be exercised end to end |
| +// without a live sibling. |
| +type fixedCustody struct { |
| + // apps is the configured roster. A source not in it does not resolve. |
| + apps map[string]sibling.Sibling |
| + give suite.Authorization |
| + // asked is what reached the client: the asker, and the pairs. |
| + asker suite.Subject |
| + pairs []suite.ItemRevision |
| +} |
| + |
| +func (f *fixedCustody) Sibling(source string) (sibling.Sibling, bool) { |
| + s, ok := f.apps[source] |
| + return s, ok |
| +} |
| + |
| +func (f *fixedCustody) Rule(ctx context.Context, asker suite.Subject, cands []sibling.Candidate) ([]sibling.Disposition, *sibling.Outcome) { |
| + f.asker = asker |
| + out := make([]sibling.Disposition, len(cands)) |
| + notices := &sibling.Outcome{} |
| + for i, c := range cands { |
| + f.pairs = append(f.pairs, c.Item) |
| + out[i] = sibling.Decide(f.give) |
| + notices.Record(c.Sibling.Name, out[i]) |
| + } |
| + return out, notices |
| +} |
| + |
| +func docsRoster() map[string]sibling.Sibling { |
| + return map[string]sibling.Sibling{ |
| + "Docs": {Name: "Docs", BaseURL: "https://docs.example", Token: "t"}, |
| + } |
| +} |
| + |
| +// storeWithASiblingDocument seeds one document owned by Docs and one owned |
| +// by memoria, both matching the same query. |
| +// |
| +// **Two documents, not one, and that is the presence assertion.** A test |
| +// with only a sibling document cannot tell "the ruling withheld it" from |
| +// "the search found nothing"; the memoria-owned row is the control that |
| +// stays on the page in every case below, so an empty result is always |
| +// attributable. |
| +func storeWithASiblingDocument(t *testing.T, custody Custody) *Store { |
| + t.Helper() |
| + s := storeForTest(t) |
| + s.custody = custody |
| + ctx := context.Background() |
| + if _, err := s.Ingest(ctx, IngestSpec{ |
| + Source: "Docs", Key: "docs/handbook", Title: "Handbook", |
| + Body: "The quarterly planning process runs in March.", |
| + ItemID: "docs-item-991", |
| + SiblingRevision: 7, |
| + }); err != nil { |
| + t.Fatalf("ingest sibling doc: %v", err) |
| + } |
| + if _, err := s.Ingest(ctx, IngestSpec{ |
| + Source: "upload", Key: "notes.txt", Title: "Notes", |
| + Body: "My own notes on the quarterly planning process.", |
| + }); err != nil { |
| + t.Fatalf("ingest owned doc: %v", err) |
| + } |
| + return s |
| +} |
| + |
| +// Β§4.1 end to end, through the only method that returns a renderable |
| +// passage. |
| +// |
| +// This is the gate the sibling package's unit tests cannot be: they prove |
| +// `Decide` maps values and `Rule` fans out, and neither proves that the |
| +// thing which actually renders a page consults either. The failure they |
| +// leave open is a `Search` that computes a disposition and ignores it. |
| +// |
| +// MUTATION-TESTED 2026-09-06, each verified red then reverted: |
| +// |
| +// Search returns `out` without calling rule() |
| +// -> every withholding case red |
| +// rule() keeps a passage when d.Notices() rather than d.Renders() |
| +// -> VISIBLE case red, NOT_VISIBLE case red |
| +// rule() treats an unresolvable source as memoria-owned |
| +// -> the unconfigured-sibling case red |
| +// rule() skips the nil-custody branch |
| +// -> nil case red (and it renders, which is the disclosure) |
| +// Search asks about the memoria-owned document too |
| +// -> the pairs assertion red |
| +func TestASiblingPassageIsNeverSpokenUnruled(t *testing.T) { |
| + ctx := context.Background() |
| + |
| + for _, c := range []struct { |
| + name string |
| + give suite.Authorization |
| + wantSibling bool |
| + wantNotice string |
| + }{ |
| + {"VISIBLE renders", suite.VISIBLE, true, ""}, |
| + {"NOT_VISIBLE omits silently", suite.NOT_VISIBLE, false, ""}, |
| + {"STALE withholds loudly", suite.STALE, false, "Docs has moved on from 1 result; refetching."}, |
| + {"UNKNOWN withholds loudly", suite.UNKNOWN, false, "Docs did not answer; 1 result withheld."}, |
| + } { |
| + t.Run(c.name, func(t *testing.T) { |
| + cus := &fixedCustody{apps: docsRoster(), give: c.give} |
| + s := storeWithASiblingDocument(t, cus) |
| + |
| + page, err := s.Search(ctx, testAsker, "quarterly planning", 10) |
| + if err != nil { |
| + t.Fatalf("search: %v", err) |
| + } |
| + |
| + var sawSibling, sawOwned bool |
| + for _, r := range page.Results { |
| + switch r.Source { |
| + case "Docs": |
| + sawSibling = true |
| + case "upload": |
| + sawOwned = true |
| + } |
| + } |
| + // The control. Without it, every "the sibling passage is not |
| + // here" assertion below is satisfied by a search that found |
| + // nothing at all. |
| + if !sawOwned { |
| + t.Fatal("memoria's own document did not render, so this test " + |
| + "cannot tell withholding from an empty search") |
| + } |
| + if sawSibling != c.wantSibling { |
| + t.Errorf("sibling passage rendered = %v, want %v", sawSibling, c.wantSibling) |
| + } |
| + |
| + switch c.wantNotice { |
| + case "": |
| + if len(page.Notices) != 0 { |
| + t.Errorf("notices = %v, want none β %q is not a loud value", |
| + page.Notices, c.give) |
| + } |
| + default: |
| + if len(page.Notices) != 1 || page.Notices[0] != c.wantNotice { |
| + t.Errorf("notices = %v, want [%q]", page.Notices, c.wantNotice) |
| + } |
| + } |
| + |
| + // The question that went out names the sibling's own |
| + // coordinate and this person β and names memoria's own |
| + // document to nobody, because Β§4.2 makes memoria its app of |
| + // record and there is nobody else to ask. |
| + if len(cus.pairs) != 1 { |
| + t.Fatalf("asked about %d items, want 1: memoria's own content "+ |
| + "has no app of record to ask", len(cus.pairs)) |
| + } |
| + want := suite.ItemRevision{ItemID: "docs-item-991", Revision: 7} |
| + if cus.pairs[0] != want { |
| + t.Errorf("asked about %+v, want %+v", cus.pairs[0], want) |
| + } |
| + if cus.asker != testAsker { |
| + t.Errorf("asked on behalf of %+v, want %+v", cus.asker, testAsker) |
| + } |
| + }) |
| + } |
| +} |
| + |
| +// The two ways there is nobody to ask, which must not become "nobody |
| +// objects". |
| +// |
| +// A sibling dropped from configuration leaves its documents in the store. |
| +// So does an instance wired without any custody at all. In both, memoria |
| +// holds a passage whose app of record it cannot reach β and the tempting |
| +// reading, that an unreachable owner imposes no restriction, is the |
| +// disclosure Β§4.1 exists to prevent. |
| +// |
| +// MUTATION-TESTED 2026-09-06: make either branch fall through to |
| +// rendering and the matching case goes red, showing the sibling passage on |
| +// the page. Reverted. |
| +func TestNobodyToAskIsNotNobodyObjecting(t *testing.T) { |
| + ctx := context.Background() |
| + |
| + t.Run("a source with no configured app of record", func(t *testing.T) { |
| + // The roster knows Sheets. The document came from Docs. |
| + cus := &fixedCustody{ |
| + apps: map[string]sibling.Sibling{"Sheets": {Name: "Sheets"}}, |
| + give: suite.VISIBLE, |
| + } |
| + s := storeWithASiblingDocument(t, cus) |
| + |
| + page, err := s.Search(ctx, testAsker, "quarterly planning", 10) |
| + if err != nil { |
| + t.Fatalf("search: %v", err) |
| + } |
| + assertOnlyOwnedRendered(t, page) |
| + if len(cus.pairs) != 0 { |
| + t.Errorf("asked %d questions of a sibling that is not configured", len(cus.pairs)) |
| + } |
| + }) |
| + |
| + t.Run("no custody at all", func(t *testing.T) { |
| + s := storeWithASiblingDocument(t, nil) |
| + page, err := s.Search(ctx, testAsker, "quarterly planning", 10) |
| + if err != nil { |
| + t.Fatalf("search: %v", err) |
| + } |
| + assertOnlyOwnedRendered(t, page) |
| + }) |
| +} |
| + |
| +func assertOnlyOwnedRendered(t *testing.T, page Page) { |
| + t.Helper() |
| + var sawOwned bool |
| + for _, r := range page.Results { |
| + if r.Source == "Docs" { |
| + t.Errorf("a passage from an app of record memoria cannot reach was "+ |
| + "spoken: %q. An unreachable owner is not an absent one", r.Title) |
| + } |
| + if r.Source == "upload" { |
| + sawOwned = true |
| + } |
| + } |
| + if !sawOwned { |
| + t.Fatal("memoria's own document did not render, so this proves nothing") |
| + } |
| + if len(page.Notices) != 1 || !strings.Contains(page.Notices[0], "did not answer") { |
| + t.Errorf("notices = %v, want one saying Docs did not answer β a passage "+ |
| + "dropped without a notice is a page that reads as complete", page.Notices) |
| + } |
| +} |
| + |
| +// A search with nobody on it is refused, not answered. |
| +// |
| +// Β§4.1's question is "may THIS PERSON read this passage", and there is no |
| +// answer to it for nobody. Refusing here rather than sending an empty |
| +// subject also keeps the failure on memoria's side of the wire, where it |
| +// can be read, instead of arriving at a sibling as an unanswerable call. |
| +// |
| +// MUTATION-TESTED 2026-09-06: drop the guard and the search returns |
| +// memoria-owned passages to an unidentified caller. Red. Reverted. |
| +func TestASearchWithNoAskerIsRefused(t *testing.T) { |
| + s := storeWithASiblingDocument(t, &fixedCustody{apps: docsRoster(), give: suite.VISIBLE}) |
| + page, err := s.Search(context.Background(), suite.Subject{}, "quarterly planning", 10) |
| + if err == nil { |
| + t.Fatalf("a search with no asker returned %d results", len(page.Results)) |
| + } |
| + if len(page.Results) != 0 { |
| + t.Errorf("a refused search still returned passages: %d", len(page.Results)) |
| + } |
| +} |
| + |
| +// Both or neither: a sibling document needs an item id and a revision. |
| +// |
| +// Either half alone is worse than neither. An item id with revision 0 asks |
| +// a sibling about a body it never issued; a revision with no item id is a |
| +// document nothing can be asked about, which would then render as |
| +// memoria's own. |
| +// |
| +// MUTATION-TESTED 2026-09-06: default the missing half instead of |
| +// refusing, and both cases go red β the second one by rendering a sibling |
| +// document with no ruling at all. Reverted. |
| +func TestASiblingDocumentNeedsBothHalvesOfItsCoordinate(t *testing.T) { |
| + ctx := context.Background() |
| + for _, c := range []struct { |
| + name string |
| + spec IngestSpec |
| + }{ |
| + {"an item id with no revision", IngestSpec{ |
| + Source: "Docs", Key: "k1", Title: "T", Body: "body", ItemID: "docs-1", |
| + }}, |
| + {"a revision with no item id", IngestSpec{ |
| + Source: "Docs", Key: "k2", Title: "T", Body: "body", SiblingRevision: 4, |
| + }}, |
| + } { |
| + t.Run(c.name, func(t *testing.T) { |
| + s := storeForTest(t) |
| + if _, err := s.Ingest(ctx, c.spec); err == nil { |
| + t.Error("stored a half-coordinated sibling document") |
| + } |
| + }) |
| + } |
| + |
| + t.Run("both halves store", func(t *testing.T) { |
| + // Without this the two refusals above are satisfied by an Ingest |
| + // that refuses every sibling document. |
| + s := storeForTest(t) |
| + if _, err := s.Ingest(ctx, IngestSpec{ |
| + Source: "Docs", Key: "k3", Title: "T", Body: "body", |
| + ItemID: "docs-3", SiblingRevision: 9, |
| + }); err != nil { |
| + t.Errorf("a fully-coordinated sibling document was refused: %v", err) |
| + } |
| + }) |
| +} |
| + |
| +// storeForTest is a store with a real database and a real index and no |
| +// custody set β the caller supplies whatever apps of record the test is |
| +// about. |
| +func storeForTest(t *testing.T) *Store { |
| + t.Helper() |
| + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) |
| + d, err := db.Open(filepath.Join(t.TempDir(), "store.db"), logger) |
| + if err != nil { |
| + t.Fatalf("db.Open: %v", err) |
| + } |
| + t.Cleanup(func() { d.Close() }) |
| + if _, err := migrate.Apply(context.Background(), d, BootSchema); err != nil { |
| + t.Fatalf("migrate: %v", err) |
| + } |
| + dek := make([]byte, 32) |
| + for i := range dek { |
| + dek[i] = byte(i*11 + 3) |
| + } |
| + sealer, err := seal.New(dek) |
| + if err != nil { |
| + t.Fatalf("seal.New: %v", err) |
| + } |
| + ix, err := index.Open() |
| + if err != nil { |
| + t.Fatalf("index.Open: %v", err) |
| + } |
| + t.Cleanup(func() { ix.Close() }) |
| + return NewStore(d, sealer, ix, nil) |
| +} |
| + |
| +// A page's notices are in a fixed order, and the order is the reader's. |
| +// |
| +// **This is here because the first version was not.** `rule()` built its |
| +// unresolved-source notices by ranging a `map[int]string`, so a page with |
| +// two unreachable apps listed them differently on every reload β Go |
| +// randomises map iteration exactly so that this surfaces, and it did. A |
| +// notice that moves is a notice a reader stops trusting. |
| +// |
| +// The order asserted is the order the results themselves are in, which is |
| +// the order the reader meets the passages the lines are about. |
| +// |
| +// MUTATION-TESTED 2026-09-06, each verified red then reverted: |
| +// |
| +// build the notices from a map keyed by result index |
| +// -> flaky, then red at -count=20 (the failure the fix is for) |
| +// sort the notices alphabetically |
| +// -> red: Sheets would precede Wiki regardless of the ranking |
| +// record only the dropped passages, not every result |
| +// -> green here, and that is fine β Record ignores a Render. Kept in |
| +// this list so the next reader knows it was tried and why it is |
| +// not a finding. |
| +func TestNoticesAreInResultOrder(t *testing.T) { |
| + // Two apps, neither on the roster, so both are unreachable and both |
| + // have something to say. Wiki's document scores first because the |
| + // query words are denser in it. |
| + cus := &fixedCustody{apps: map[string]sibling.Sibling{}, give: suite.VISIBLE} |
| + s := storeForTest(t) |
| + s.custody = cus |
| + ctx := context.Background() |
| + for _, d := range []struct { |
| + source, key, title, body, item string |
| + }{ |
| + {"Wiki", "wiki/1", "Wiki", "Quarterly planning, quarterly planning, quarterly planning.", "wiki-1"}, |
| + {"Sheets", "sheets/1", "Sheet", "A note about quarterly planning somewhere in a longer document that dilutes it considerably with many other words.", "sheets-1"}, |
| + } { |
| + if _, err := s.Ingest(ctx, IngestSpec{ |
| + Source: d.source, Key: d.key, Title: d.title, Body: d.body, |
| + ItemID: d.item, SiblingRevision: 3, |
| + }); err != nil { |
| + t.Fatalf("ingest %s: %v", d.source, err) |
| + } |
| + } |
| + |
| + want := []string{ |
| + "Wiki did not answer; 1 result withheld.", |
| + "Sheets did not answer; 1 result withheld.", |
| + } |
| + for i := 0; i < 20; i++ { |
| + page, err := s.Search(ctx, testAsker, "quarterly planning", 10) |
| + if err != nil { |
| + t.Fatalf("search: %v", err) |
| + } |
| + if len(page.Results) != 0 { |
| + t.Fatalf("run %d: %d passages rendered from two unreachable apps", |
| + i, len(page.Results)) |
| + } |
| + if !reflect.DeepEqual(page.Notices, want) { |
| + t.Fatalf("run %d: notices = %v, want %v β a page whose notices "+ |
| + "reorder between reloads is a page a reader learns to skip", |
| + i, page.Notices, want) |
| + } |
| + } |
| +} |
diff --git a/internal/memoria/delete_test.go b/internal/memoria/delete_test.go| index fa91176..0df53f3 100644 |
| --- a/internal/memoria/delete_test.go |
| +++ b/internal/memoria/delete_test.go |
| @@ -29,7 +29,7 @@ func storeOneDocument(t *testing.T, a *app, term string) int64 { |
| if err != nil { |
| t.Fatalf("Ingest: %v", err) |
| } |
| - hits, err := a.store.Search(ctx, term, 10) |
| + hits, err := searchAsReader(a.store, ctx, term, 10) |
| if err != nil { |
| t.Fatalf("Search: %v", err) |
| } |
| @@ -147,7 +147,7 @@ func TestASearchNeverCitesADocumentThatIsNotInTheDatabase(t *testing.T) { |
| "here would prove nothing at all") |
| } |
| - hits, err := a.store.Search(ctx, term, 10) |
| + hits, err := searchAsReader(a.store, ctx, term, 10) |
| if err != nil { |
| t.Fatalf("Search: %v", err) |
| } |
| @@ -240,7 +240,7 @@ func TestASearchNeverCitesAPassageTheDocumentNoLongerHas(t *testing.T) { |
| "nothing here would prove nothing") |
| } |
| - hits, err := a.store.Search(ctx, gone, 10) |
| + hits, err := searchAsReader(a.store, ctx, gone, 10) |
| if err != nil { |
| t.Fatalf("Search: %v", err) |
| } |
| @@ -353,7 +353,7 @@ func TestAnOrdinaryDeleteStillDeletes(t *testing.T) { |
| } else if n != 0 { |
| t.Errorf("%d passages are still indexed after a delete that reported no error", n) |
| } |
| - hits, err := a.store.Search(ctx, term, 10) |
| + hits, err := searchAsReader(a.store, ctx, term, 10) |
| if err != nil { |
| t.Fatalf("Search: %v", err) |
| } |
diff --git a/internal/memoria/handlers.go b/internal/memoria/handlers.go| index cea5f08..9e70f4e 100644 |
| --- a/internal/memoria/handlers.go |
| +++ b/internal/memoria/handlers.go |
| @@ -12,6 +12,8 @@ import ( |
| "strings" |
| "github.com/carlosframework/rastrillo/auth" |
| + |
| + "amadan.net/oficina/suite" |
| "github.com/carlosframework/rastrillo/db" |
| "github.com/carlosframework/rastrillo/flash" |
| "github.com/go-chi/chi/v5" |
| @@ -139,6 +141,17 @@ type page struct { |
| Documents []Summary |
| Drop dropZone |
| + // Notices is Β§6.5's loud withholding: what this page could not show |
| + // and which app it came from. **Never a count of what the reader may |
| + // not see** β a NOT_VISIBLE is omitted silently, and the only value |
| + // that may be silent (Β§4.1). |
| + // |
| + // A page with results and no notices is a complete answer. A page |
| + // with notices is an honest partial one, which Β§6.5 calls the normal |
| + // case rather than an error path. What must never happen is the third |
| + // thing: a short page that reads as complete. |
| + Notices []string |
| + |
| // SearchFailed is not "no results". A search that errored renders |
| // its own callout in the results region, because "nothing matched" |
| // and "the search broke" look identical to a reader and this |
| @@ -591,7 +604,24 @@ func (a *app) runSearch(r *http.Request, p *page) { |
| if !p.Searched { |
| return |
| } |
| - results, err := a.store.Search(r.Context(), p.Query, 25) |
| + // The asker, typed, in lone shape: a lowercased address (Β§4.1, suite |
| + // Β§3). The kind travels on the wire so a receiver keyed on person |
| + // refs answers UNKNOWN for the whole call rather than guessing β a |
| + // loud failure instead of a silent mismatch between an address and a |
| + // person ref that happen to collide. |
| + // |
| + // **No identity, no search.** Not an empty page: a page. A reader who |
| + // is not signed in has nobody for a sibling to rule on, and every |
| + // passage would have to be withheld anyway β so this is the same |
| + // refusal, said before the round trip rather than after it. |
| + addr, ok := a.identity(r) |
| + if !ok || addr == "" { |
| + a.logger.Error("search refused: no identity on the request") |
| + p.Searched, p.SearchFailed = false, true |
| + return |
| + } |
| + found, err := a.store.Search(r.Context(), |
| + suite.Subject{Kind: suite.SubjectAddress, Value: addr}, p.Query, 25) |
| if err != nil { |
| // No query text in the log line: AGENTS.md, and `carlos logs` is |
| // storage we cannot grep. |
| @@ -599,8 +629,9 @@ func (a *app) runSearch(r *http.Request, p *page) { |
| p.Searched, p.SearchFailed = false, true |
| return |
| } |
| - p.Results = results |
| - p.Primed = query.PrimesFirstResult(p.Query, len(results)) |
| + p.Results = found.Results |
| + p.Notices = found.Notices |
| + p.Primed = query.PrimesFirstResult(p.Query, len(found.Results)) |
| } |
| // count reads a non-negative integer out of a query parameter. Anything |
diff --git a/internal/memoria/migrations/0005_alter_documents.sql b/internal/memoria/migrations/0005_alter_documents.sql| new file mode 100644 |
| index 0000000..bdd4c6c |
| --- /dev/null |
| +++ b/internal/memoria/migrations/0005_alter_documents.sql |
| @@ -0,0 +1,4 @@ |
| +ALTER TABLE `documents` ADD `sibling_item_id` blob; |
| + |
| +ALTER TABLE `documents` ADD `sibling_revision` integer NOT NULL DEFAULT 0; |
| + |
diff --git a/internal/memoria/migrations/schema.sql b/internal/memoria/migrations/schema.sql| index e16c256..5e670b1 100644 |
| --- a/internal/memoria/migrations/schema.sql |
| +++ b/internal/memoria/migrations/schema.sql |
| @@ -5,7 +5,7 @@ CREATE TABLE `chunks` (`id` integer PRIMARY KEY AUTOINCREMENT,`document_id` inte |
| CREATE TABLE `connectors` (`id` integer PRIMARY KEY AUTOINCREMENT,`kind` text NOT NULL,`sealed_token` blob NOT NULL,`workspace_print` text,`last_verified` datetime,`created_at` datetime,`updated_at` datetime); |
| -CREATE TABLE `documents` (`id` integer PRIMARY KEY AUTOINCREMENT,`source` text NOT NULL,`external_id` text NOT NULL,`revision` integer NOT NULL DEFAULT 1,`sealed_title` blob NOT NULL,`sealed_body` blob NOT NULL,`bytes` integer,`created_at` datetime,`updated_at` datetime, `body_print` text NOT NULL DEFAULT "", `sealed_path` blob); |
| +CREATE TABLE `documents` (`id` integer PRIMARY KEY AUTOINCREMENT,`source` text NOT NULL,`external_id` text NOT NULL,`revision` integer NOT NULL DEFAULT 1,`sealed_title` blob NOT NULL,`sealed_body` blob NOT NULL,`bytes` integer,`created_at` datetime,`updated_at` datetime, `body_print` text NOT NULL DEFAULT "", `sealed_path` blob, `sibling_item_id` blob, `sibling_revision` integer NOT NULL DEFAULT 0); |
| CREATE TABLE `source_pages` (`id` integer PRIMARY KEY AUTOINCREMENT,`connector_id` integer NOT NULL,`page_print` text NOT NULL,`sealed_page_id` blob NOT NULL,`sealed_path` blob,`path_print` text,`seen_edited` text,`ingested_edited` text,`ingested_path` text,`state` text NOT NULL DEFAULT "pending",`error_code` text,`last_seen` datetime,`document_id` integer,`created_at` datetime,`updated_at` datetime); |
diff --git a/internal/memoria/models.go b/internal/memoria/models.go| index a83be78..a8fc2ec 100644 |
| --- a/internal/memoria/models.go |
| +++ b/internal/memoria/models.go |
| @@ -42,10 +42,49 @@ type Document struct { |
| // BodyPrint is a keyed fingerprint of the body β the change |
| // detector. Keyed for the same reason ExternalID is. |
| BodyPrint string `gorm:"not null;default:''"` |
| - // Revision increments on every change to the body. Slice 1's |
| - // authorization re-check is keyed on (id, revision), so a stale |
| - // passage can be refused rather than spoken. |
| + // Revision increments on every change to the body. **This is |
| + // memoria's own counter and it is not what S2 is asked about.** |
| + // |
| + // The model said the authorization re-check was keyed on it. That is |
| + // wrong for sibling content and would have been a quiet |
| + // misauthorization: the revision S2 rules on is a token the *sibling* |
| + // issued, memoria never invents one, and two apps counting |
| + // independently agree only by accident. What this number is for is |
| + // memoria-owned content under Β§4.2, where memoria is the app of |
| + // record and its own count is the truth β and for Β§5.2's provenance, |
| + // where a derived artefact built from revision 1 must not be served |
| + // after revision 2 lands. Sibling documents carry SiblingRevision |
| + // below and this counts their local edits alongside it. |
| Revision int `gorm:"not null;default:1"` |
| + // SiblingItemID is the app of record's own id for this document, |
| + // sealed and reversible (seal.PurposeItemID) because it goes back out |
| + // on the wire in every S2 question and S3 fetch. |
| + // |
| + // It cannot be recovered from ExternalID, which is a keyed |
| + // fingerprint β deliberately one-way, so the bucket does not confirm |
| + // which documents a team holds. A one-way id is right for finding a |
| + // row and useless for asking somebody about it, so both are here. |
| + // |
| + // Empty for memoria-owned content (Β§4.2), which has no app of record |
| + // to ask. |
| + SiblingItemID []byte |
| + // SiblingRevision is the revision token the sibling issued for the |
| + // body memoria holds. **Never compared, only matched for equality** |
| + // (Β§4.1): STALE and AHEAD are the sibling's verdict on its own log, |
| + // not memoria's arithmetic on two numbers, and a sibling restored to |
| + // an older revision must not be second-guessed by preferring the |
| + // larger one. |
| + // |
| + // int64 because that is what the contract module types it as. Β§4.1's |
| + // prose calls a revision an opaque token that need not be numeric; |
| + // the shipped wire shape is narrower than that sentence, and the |
| + // substantive rule β equality only, never ordering β is what memoria |
| + // actually depends on and is unaffected. |
| + // |
| + // Plaintext, like Revision, and for the same reason Β§3.2 gives: a |
| + // coordinate is not content, and the count of edits is already |
| + // inferable from row history. |
| + SiblingRevision int64 `gorm:"not null;default:0"` |
| // SealedTitle and SealedBody are sealed under distinct HKDF purposes. |
| SealedTitle []byte `gorm:"not null"` |
| SealedBody []byte `gorm:"not null"` |
diff --git a/internal/memoria/seal/seal.go b/internal/memoria/seal/seal.go| index f708e79..32dd479 100644 |
| --- a/internal/memoria/seal/seal.go |
| +++ b/internal/memoria/seal/seal.go |
| @@ -51,6 +51,18 @@ const ( |
| // reason this purpose exists: the fetch phase has to hand the id back |
| // to the API it came from, and an HMAC cannot be handed back. |
| PurposeCoordinate = "source.coordinate" |
| + // PurposeItemID seals a SIBLING's own id for a document β the value |
| + // that goes on the wire in an S2 question and an S3 fetch. |
| + // |
| + // Reversible, for the same reason PurposeCoordinate is, and separate |
| + // from it for a reason worth stating: these two are the ids memoria |
| + // hands back to somebody else, and they go to *different* somebodies. |
| + // One purpose for both would mean a bug that opened a Notion page id |
| + // under the sibling path would send a Notion coordinate to Docs and it |
| + // would decrypt cleanly on the way out. With two, it does not decrypt |
| + // at all. "One purpose per kind of thing sealed" earns its keep here: |
| + // the kinds differ by who receives them, not by what they look like. |
| + PurposeItemID = "sibling.item_id" |
| ) |
| func sha256New() hash.Hash { return sha256.New() } |
diff --git a/internal/memoria/searchhelper_test.go b/internal/memoria/searchhelper_test.go| new file mode 100644 |
| index 0000000..c3a3c6f |
| --- /dev/null |
| +++ b/internal/memoria/searchhelper_test.go |
| @@ -0,0 +1,28 @@ |
| +package memoria |
| + |
| +import ( |
| + "context" |
| + |
| + "amadan.net/oficina/suite" |
| +) |
| + |
| +// testAsker is the person every store-level test searches as. |
| +// |
| +// Β§4.1's typed subject in lone shape: a lowercased address, with the kind |
| +// on the value so a receiver keyed on person refs answers UNKNOWN for the |
| +// whole call rather than guessing. |
| +var testAsker = suite.Subject{Kind: suite.SubjectAddress, Value: "reader@example.com"} |
| + |
| +// searchAsReader runs a search as an ordinary signed-in reader and returns |
| +// the passages that survived Β§4.1's ruling. |
| +// |
| +// **It is not a way around the ruling and must never become one.** It |
| +// calls Store.Search like everything else does; all it supplies is the |
| +// asker the test would otherwise have to spell out at thirty call sites, |
| +// and all it drops is the notices, which these tests are not about. A |
| +// helper that reached past Search to the raw hits would be the seam |
| +// AGENTS.md warns about β the guard that exists only in a comment. |
| +func searchAsReader(s *Store, ctx context.Context, q string, n int) ([]Result, error) { |
| + p, err := s.Search(ctx, testAsker, q, n) |
| + return p.Results, err |
| +} |
diff --git a/internal/memoria/store.go b/internal/memoria/store.go| index bdb951a..b8aa526 100644 |
| --- a/internal/memoria/store.go |
| +++ b/internal/memoria/store.go |
| @@ -8,9 +8,12 @@ import ( |
| "github.com/carlosframework/rastrillo/db" |
| + "amadan.net/oficina/suite" |
| + |
| "amadan.net/oficina/memoria/internal/memoria/chunk" |
| "amadan.net/oficina/memoria/internal/memoria/index" |
| "amadan.net/oficina/memoria/internal/memoria/seal" |
| + "amadan.net/oficina/memoria/internal/memoria/sibling" |
| ) |
| // Store is the only thing that opens sealed rows. Everything above it |
| @@ -20,11 +23,21 @@ type Store struct { |
| db *db.DB |
| sealer *seal.Sealer |
| ix *index.Index |
| + // custody is who memoria asks about a sibling's passages. |
| + // |
| + // **nil is a configuration, not a bug**, and it is slice 0's: no |
| + // siblings, every document memoria-owned under Β§4.2. It is also the |
| + // state a misconfigured slice-1 instance is in, so nil withholds |
| + // every sibling passage rather than rendering it. See rule(). |
| + custody Custody |
| } |
| // NewStore wires the store. The index starts empty β call Rebuild. |
| -func NewStore(d *db.DB, sealer *seal.Sealer, ix *index.Index) *Store { |
| - return &Store{db: d, sealer: sealer, ix: ix} |
| +// |
| +// custody may be nil, and what that means is in the field's comment: no |
| +// app of record can be asked, so nothing owned by one is spoken. |
| +func NewStore(d *db.DB, sealer *seal.Sealer, ix *index.Index, custody Custody) *Store { |
| + return &Store{db: d, sealer: sealer, ix: ix, custody: custody} |
| } |
| // Result is one search hit, with its citation resolved to plaintext. |
| @@ -44,6 +57,29 @@ type Result struct { |
| Heading string |
| Snippet string |
| Score float64 |
| + // item is the app of record's coordinate for this passage's document, |
| + // zero for memoria-owned content (Β§4.2). |
| + // |
| + // Unexported: it goes to a sibling on the wire and never to a |
| + // template. A reader has no use for another app's internal id, and a |
| + // field a template can reach is a field a template eventually prints. |
| + item sibling.Candidate |
| +} |
| + |
| +// Page is one search, ruled on. |
| +// |
| +// **Results are the passages this person may read** β every one of them |
| +// either memoria's own (Β§4.2) or carrying a VISIBLE from its app of |
| +// record, asked for this person, for this revision, just now. |
| +// |
| +// **Notices are what could not be shown and why**, per Β§6.5, naming the |
| +// app and never the document. A short page with no notice is the failure |
| +// this whole design is arranged against; a short page WITH one is an |
| +// honest partial answer, and Β§6.5 says partial pages are the normal case |
| +// rather than an error path. |
| +type Page struct { |
| + Results []Result |
| + Notices []string |
| } |
| // Summary is a document as the list shows it. |
| @@ -95,6 +131,18 @@ type IngestSpec struct { |
| // an entire import, and skipping makes the page an absence with |
| // nothing to explain it. |
| AllowTitleOnly bool |
| + // ItemID and SiblingRevision are the app of record's coordinate, set |
| + // only for content ingested from a sibling (Β§4.1) and left empty for |
| + // everything memoria owns outright (Β§4.2). |
| + // |
| + // **Both or neither.** A document with an item id and no revision |
| + // would ask S2 about revision 0, which is a revision no sibling |
| + // issued β and a sibling asked about a revision it never issued |
| + // answers about a body memoria is not holding. Ingest refuses the |
| + // half-set case rather than storing it, because the failure it |
| + // produces is a wrong authorization rather than a missing one. |
| + ItemID string |
| + SiblingRevision int64 |
| } |
| // Ingest stores a document and indexes it. |
| @@ -133,6 +181,23 @@ func (s *Store) Ingest(ctx context.Context, spec IngestSpec) (int64, error) { |
| } |
| pathStr := chunk.Chunk{Path: spec.Path}.PathString() |
| + // Both or neither: see IngestSpec.ItemID. Refused here rather than |
| + // defaulted, because either default is a lie β an item id with |
| + // revision 0 asks about a body no sibling has, and a revision with no |
| + // item id is a document nothing can be asked about that would |
| + // otherwise render as memoria's own. |
| + if (spec.ItemID == "") != (spec.SiblingRevision == 0) { |
| + return 0, errors.New("ingest: a sibling document needs both an item id and a revision") |
| + } |
| + var sealedItemID []byte |
| + if spec.ItemID != "" { |
| + sealed, err := s.sealer.Seal(seal.PurposeItemID, []byte(spec.ItemID)) |
| + if err != nil { |
| + return 0, err |
| + } |
| + sealedItemID = sealed |
| + } |
| + |
| external, err := s.externalID(key) |
| if err != nil { |
| return 0, err |
| @@ -213,20 +278,27 @@ func (s *Store) Ingest(ctx context.Context, spec IngestSpec) (int64, error) { |
| "sealed_path": sealedPath, |
| "body_print": bodyPrint, |
| "bytes": len(body), |
| + // The sibling's own revision moves with the body, and it |
| + // is the sibling's number rather than an increment of |
| + // ours: Β§4.1 forbids memoria inventing or ordering one. |
| + "sibling_item_id": sealedItemID, |
| + "sibling_revision": spec.SiblingRevision, |
| }).Error; err != nil { |
| return err |
| } |
| default: |
| rechunk = true |
| d := Document{ |
| - Source: spec.Source, |
| - ExternalID: external, |
| - BodyPrint: bodyPrint, |
| - Revision: 1, |
| - SealedTitle: sealedTitle, |
| - SealedBody: sealedBody, |
| - SealedPath: sealedPath, |
| - Bytes: len(body), |
| + Source: spec.Source, |
| + ExternalID: external, |
| + BodyPrint: bodyPrint, |
| + Revision: 1, |
| + SealedTitle: sealedTitle, |
| + SealedBody: sealedBody, |
| + SealedPath: sealedPath, |
| + SiblingItemID: sealedItemID, |
| + SiblingRevision: spec.SiblingRevision, |
| + Bytes: len(body), |
| } |
| if err := tx.Create(&d).Error; err != nil { |
| return err |
| @@ -409,13 +481,30 @@ func (s *Store) Rebuild(ctx context.Context) (int, error) { |
| } |
| // Search runs the query and resolves each hit's citation. |
| -func (s *Store) Search(ctx context.Context, query string, limit int) ([]Result, error) { |
| +// Search finds passages this person may read. |
| +// |
| +// **The ruling happens in here, and that placement is the design.** Β§4.1 |
| +// binds every path that would speak a sibling's passage, and the way to |
| +// make that true of every path is to leave no way to obtain a passage |
| +// without it: there is one method that returns renderable text, it takes |
| +// the asker, and it does not return anything it has not asked about. A |
| +// version that returned raw hits and trusted its callers to ask would be |
| +// correct exactly as long as nobody added a second caller. |
| +// |
| +// An empty asker refuses outright. There is nobody to ask about, and a |
| +// question S2 cannot answer for a person is not a question worth sending |
| +// β Β§4.1's typed subject travels on the wire precisely so a receiver that |
| +// cannot make sense of it says UNKNOWN rather than guessing. |
| +func (s *Store) Search(ctx context.Context, asker suite.Subject, query string, limit int) (Page, error) { |
| + if asker.Value == "" { |
| + return Page{}, errors.New("search: no asker; nothing can be authorized") |
| + } |
| hits, err := s.ix.Search(query, limit) |
| if err != nil { |
| - return nil, err |
| + return Page{}, err |
| } |
| if len(hits) == 0 { |
| - return nil, nil |
| + return Page{}, nil |
| } |
| docIDs := make([]int64, 0, len(hits)) |
| seen := map[int64]bool{} |
| @@ -427,25 +516,42 @@ func (s *Store) Search(ctx context.Context, query string, limit int) ([]Result, |
| } |
| var docs []Document |
| if err := s.db.G.WithContext(ctx).Where("id IN ?", docIDs).Find(&docs).Error; err != nil { |
| - return nil, err |
| + return Page{}, err |
| } |
| titles := map[int64]string{} |
| sources := map[int64]string{} |
| paths := map[int64]string{} |
| + // items carries each document's app-of-record coordinate, absent for |
| + // memoria-owned rows. |
| + items := map[int64]sibling.Candidate{} |
| for _, d := range docs { |
| t, err := s.sealer.Open(seal.PurposeTitle, d.SealedTitle) |
| if err != nil { |
| - return nil, fmt.Errorf("open title %d: %w", d.ID, err) |
| + return Page{}, fmt.Errorf("open title %d: %w", d.ID, err) |
| } |
| titles[d.ID] = string(t) |
| sources[d.ID] = d.Source |
| if len(d.SealedPath) > 0 { |
| p, err := s.sealer.Open(seal.PurposePath, d.SealedPath) |
| if err != nil { |
| - return nil, fmt.Errorf("open path %d: %w", d.ID, err) |
| + return Page{}, fmt.Errorf("open path %d: %w", d.ID, err) |
| } |
| paths[d.ID] = string(p) |
| } |
| + if len(d.SiblingItemID) == 0 { |
| + continue |
| + } |
| + // A sibling document whose coordinate will not open is a document |
| + // memoria cannot ask about. It is left with a zero Candidate, |
| + // which rule() treats as unresolvable and withholds loudly β the |
| + // error is not swallowed into "no restriction applies". |
| + id, err := s.sealer.Open(seal.PurposeItemID, d.SiblingItemID) |
| + if err != nil { |
| + return Page{}, fmt.Errorf("open item id %d: %w", d.ID, err) |
| + } |
| + items[d.ID] = sibling.Candidate{ |
| + Item: suite.ItemRevision{ItemID: string(id), Revision: d.SiblingRevision}, |
| + } |
| } |
| // livePassage is the database's answer to "is there still such a |
| // passage", and the database is the only thing allowed to answer it. |
| @@ -456,7 +562,7 @@ func (s *Store) Search(ctx context.Context, query string, limit int) ([]Result, |
| } |
| var present []int64 |
| if err := s.db.G.WithContext(ctx).Model(&Chunk{}).Where("id IN ?", chunkIDs).Pluck("id", &present).Error; err != nil { |
| - return nil, err |
| + return Page{}, err |
| } |
| livePassage := map[int64]bool{} |
| for _, id := range present { |
| @@ -517,9 +623,84 @@ func (s *Store) Search(ctx context.Context, query string, limit int) ([]Result, |
| Heading: h.Heading, |
| Snippet: h.Snippet, |
| Score: h.Score, |
| + item: items[h.DocumentID], |
| }) |
| } |
| - return out, nil |
| + return s.rule(ctx, asker, out), nil |
| +} |
| + |
| +// rule asks every app of record about its own passages and keeps the ones |
| +// it may speak. |
| +// |
| +// **Withheld passages are dropped, not refilled.** The slot a withheld |
| +// passage occupied in top-k stays lost, and the page is shorter by one. |
| +// Β§12 lists over-fetching and refilling as deliberately not built, and |
| +// Β§6.5 says partial pages are the normal case β so the notice, and not a |
| +// backfill, is what makes the page honest. (Β§4.1's post-restore rule 1 is |
| +// the one exception and it acts earlier, by taking a reconciling source |
| +// out of the ranking before anything is scored.) |
| +func (s *Store) rule(ctx context.Context, asker suite.Subject, all []Result) Page { |
| + // One disposition per result, and **the zero value is Withhold**, so |
| + // a branch below that forgets to assign one withholds loudly rather |
| + // than speaking. See sibling.Disposition's const block for why that |
| + // is the first constant. |
| + disp := make([]sibling.Disposition, len(all)) |
| + var cands []sibling.Candidate |
| + var at []int |
| + for i, r := range all { |
| + if r.item.Item.ItemID == "" { |
| + // memoria-owned: Β§4.2 makes memoria the app of record, and |
| + // there is nobody else to ask. |
| + disp[i] = sibling.Render |
| + continue |
| + } |
| + if s.custody == nil { |
| + continue |
| + } |
| + sib, ok := s.custody.Sibling(r.Source) |
| + if !ok { |
| + // The document is here and its app of record is not |
| + // configured. Nobody can be asked, so nothing is spoken β |
| + // never "no restriction applies", which is how a sibling |
| + // removed from the roster would become a disclosure. |
| + continue |
| + } |
| + c := r.item |
| + c.Sibling = sib |
| + cands = append(cands, c) |
| + at = append(at, i) |
| + } |
| + |
| + if len(cands) > 0 { |
| + got, _ := s.custody.Rule(ctx, asker, cands) |
| + for j, d := range got { |
| + disp[at[j]] = d |
| + } |
| + } |
| + |
| + // **Notices are aggregated once, here, in result order.** The first |
| + // version of this built two Outcomes β one from the client, one for |
| + // the sources that could not be resolved β and filled the second by |
| + // ranging a `map[int]string`, so a page with two unreachable apps |
| + // listed them in a different order on every reload. Go randomises map |
| + // iteration precisely so that this shows up, and it showed up. |
| + // |
| + // Ranging `all` also means the lines arrive in the order the reader |
| + // meets the results they are about, which is the order that reads |
| + // like an explanation rather than a list. |
| + notices := &sibling.Outcome{} |
| + page := Page{Results: make([]Result, 0, len(all))} |
| + for i, r := range all { |
| + // Record before the keep check: a passage kept is a Render, which |
| + // Record ignores, and a passage dropped is the one with something |
| + // to say. Doing both from one loop is what keeps them in step. |
| + notices.Record(r.Source, disp[i]) |
| + if disp[i].Renders() { |
| + page.Results = append(page.Results, r) |
| + } |
| + } |
| + page.Notices = notices.Lines() |
| + return page |
| } |
| // Summaries lists every document, titles opened. |
diff --git a/internal/memoria/templates/results.html b/internal/memoria/templates/results.html| index 1c9ec8a..05b56ad 100644 |
| --- a/internal/memoria/templates/results.html |
| +++ b/internal/memoria/templates/results.html |
| @@ -18,10 +18,33 @@ |
| </div> |
| {{else if .Searched}} |
| <h2>{{len .Results}} result{{if ne (len .Results) 1}}s{{end}} for β{{.Query}}β</h2> |
| + {{/* Β§6.5's loud withholding, above the results rather than below them. |
| + |
| + A reader who scrolls past the last result has already decided what |
| + the page means; the notice has to arrive before the list it is |
| + about. What each line says is decided in Go (sibling.Outcome), so |
| + one wording lives in one place β and every line names an app and |
| + never a document, because naming the document would report the |
| + existence of a passage this person may not have. */}} |
| + {{if .Notices}} |
| + <div rst-callout="warn" rst-withheld> |
| + {{range .Notices}}<p rst-callout-body>{{.}}</p>{{end}} |
| + </div> |
| + {{end}} |
| {{if not .Results}} |
| <div rst-callout="quiet"> |
| <p rst-callout-body> |
| - {{if lt .Chunks 0}} |
| + {{/* Three sentences, and which one is true depends on why the |
| + page is empty. The third β "a real absence" β was the only |
| + one here, and the notices above have just made it a claim |
| + this template cannot support: a page emptied by withholding |
| + is not an absence, and saying so would be this codebase's |
| + own worst failure asserted on the line written to prevent |
| + it. Same rule as the count that fails not being a zero. */}} |
| + {{if .Notices}} |
| + Nothing that could be shown matched. Passages were withheld β |
| + see above β so this is not necessarily a real absence. |
| + {{else if lt .Chunks 0}} |
| Nothing matched β but the passage count could not be read, so |
| this may be an empty index rather than a real absence. |
| {{else}} |
diff --git a/internal/memoriatest/ancestry_test.go b/internal/memoriatest/ancestry_test.go| index 97ad601..8ca774e 100644 |
| --- a/internal/memoriatest/ancestry_test.go |
| +++ b/internal/memoriatest/ancestry_test.go |
| @@ -50,7 +50,7 @@ func TestTitleOnlyIsRefusedForPeopleAndAcceptedForConnectors(t *testing.T) { |
| } |
| // It must be FINDABLE by the only words it has. Storing it with an |
| // empty body would index nothing and hide it. |
| - hits, err := store.Search(ctx, "quarterly planning", 10) |
| + hits, err := searchAsReader(store, ctx, "quarterly planning", 10) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -130,7 +130,7 @@ func TestMoveReachesMemoriaAndReindexes(t *testing.T) { |
| if _, err := store.Ingest(ctx, spec); err != nil { |
| t.Fatal(err) |
| } |
| - if hits, _ := store.Search(ctx, "onboarding", 10); len(hits) == 0 { |
| + if hits, _ := searchAsReader(store, ctx, "onboarding", 10); len(hits) == 0 { |
| t.Fatal("the original ancestry is not searchable, so this test proves nothing") |
| } |
| @@ -139,10 +139,10 @@ func TestMoveReachesMemoriaAndReindexes(t *testing.T) { |
| if _, err := store.Ingest(ctx, spec); err != nil { |
| t.Fatal(err) |
| } |
| - if hits, _ := store.Search(ctx, "platform", 10); len(hits) == 0 { |
| + if hits, _ := searchAsReader(store, ctx, "platform", 10); len(hits) == 0 { |
| t.Error("the new ancestry is not searchable: the move did not reach the index") |
| } |
| - if hits, _ := store.Search(ctx, "onboarding", 10); len(hits) != 0 { |
| + if hits, _ := searchAsReader(store, ctx, "onboarding", 10); len(hits) != 0 { |
| t.Error("the OLD ancestry is still searchable after a move: a stale coordinate survives") |
| } |
| } |
| @@ -167,7 +167,7 @@ func TestAncestryIsSealedInTheDatabase(t *testing.T) { |
| } |
| // Vacuity guard: the ancestry must actually be indexed, or the scan |
| // below is a scan of nothing relevant. |
| - if hits, _ := store.Search(ctx, secret, 10); len(hits) == 0 { |
| + if hits, _ := searchAsReader(store, ctx, secret, 10); len(hits) == 0 { |
| t.Fatal("the ancestry is not searchable, so it may not have been stored at all") |
| } |
diff --git a/internal/memoriatest/connector_test.go b/internal/memoriatest/connector_test.go| index 180b4d3..11d394a 100644 |
| --- a/internal/memoriatest/connector_test.go |
| +++ b/internal/memoriatest/connector_test.go |
| @@ -200,7 +200,7 @@ func TestSyncImportsTheTreeWithAncestry(t *testing.T) { |
| t.Errorf("fetched %d, want 3", run.Fetched) |
| } |
| // The ancestry must be searchable, or carrying it was pointless. |
| - hits, err := app.Store().Search(context.Background(), "onboarding", 10) |
| + hits, err := searchAsReader(app.Store(), context.Background(), "onboarding", 10) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -285,7 +285,7 @@ func TestSyncNeverDeletesOnAbsence(t *testing.T) { |
| h, app, cid, _ := connectorApp(t, src) |
| sync(t, h, app, cid) |
| - if hits, _ := app.Store().Search(context.Background(), "expenses", 10); len(hits) == 0 { |
| + if hits, _ := searchAsReader(app.Store(), context.Background(), "expenses", 10); len(hits) == 0 { |
| t.Fatal("the page was never imported, so its survival proves nothing") |
| } |
| @@ -297,7 +297,7 @@ func TestSyncNeverDeletesOnAbsence(t *testing.T) { |
| if run.State != memoria.RunDone { |
| t.Errorf("state %q, want done β an absence is not an error either", run.State) |
| } |
| - if hits, _ := app.Store().Search(context.Background(), "expenses", 10); len(hits) == 0 { |
| + if hits, _ := searchAsReader(app.Store(), context.Background(), "expenses", 10); len(hits) == 0 { |
| t.Error("a page that vanished from Notion was deleted from memoria") |
| } |
| } |
| @@ -348,7 +348,7 @@ func TestEditAndAncestorRenameBothReachMemoria(t *testing.T) { |
| t.Run("an edit is re-ingested", func(t *testing.T) { |
| src.edit("kid", "Week one now covers parking permits.", "t2") |
| sync(t, h, app, cid) |
| - hits, _ := app.Store().Search(context.Background(), "parking permits", 10) |
| + hits, _ := searchAsReader(app.Store(), context.Background(), "parking permits", 10) |
| if len(hits) == 0 { |
| t.Error("an edited page was not re-ingested") |
| } |
| @@ -360,10 +360,10 @@ func TestEditAndAncestorRenameBothReachMemoria(t *testing.T) { |
| src.rename("root", "Handbook", "Playbook") |
| sync(t, h, app, cid) |
| - if hits, _ := app.Store().Search(context.Background(), "playbook", 10); len(hits) == 0 { |
| + if hits, _ := searchAsReader(app.Store(), context.Background(), "playbook", 10); len(hits) == 0 { |
| t.Error("the new ancestry is not searchable: an ancestor rename never reached memoria") |
| } |
| - if hits, _ := app.Store().Search(context.Background(), "handbook", 10); len(hits) != 0 { |
| + if hits, _ := searchAsReader(app.Store(), context.Background(), "handbook", 10); len(hits) != 0 { |
| t.Error("the old ancestry is still searchable after an ancestor rename") |
| } |
| }) |
| @@ -563,7 +563,7 @@ func TestDisconnectKeepsTheDocuments(t *testing.T) { |
| h, app, cid, _ := connectorApp(t, src) |
| sync(t, h, app, cid) |
| - if hits, _ := app.Store().Search(context.Background(), "expenses", 10); len(hits) == 0 { |
| + if hits, _ := searchAsReader(app.Store(), context.Background(), "expenses", 10); len(hits) == 0 { |
| t.Fatal("nothing was imported, so keeping it proves nothing") |
| } |
| @@ -584,7 +584,7 @@ func TestDisconnectKeepsTheDocuments(t *testing.T) { |
| if len(docs) == 0 { |
| t.Error("disconnecting deleted the stored documents; what memoria imported, memoria keeps") |
| } |
| - if hits, _ := app.Store().Search(context.Background(), "expenses", 10); len(hits) == 0 { |
| + if hits, _ := searchAsReader(app.Store(), context.Background(), "expenses", 10); len(hits) == 0 { |
| t.Error("disconnecting removed the documents from the index") |
| } |
| } |
| @@ -606,7 +606,7 @@ func TestEmptyPageIsImportedAndCounted(t *testing.T) { |
| if run.Empty != 1 { |
| t.Errorf("empty %d, want 1 (failed %d)", run.Empty, run.FailedPages) |
| } |
| - if hits, _ := app.Store().Search(context.Background(), "quarterly planning", 10); len(hits) == 0 { |
| + if hits, _ := searchAsReader(app.Store(), context.Background(), "quarterly planning", 10); len(hits) == 0 { |
| t.Error("a title-only page is not findable by the only words it has") |
| } |
| } |
diff --git a/internal/memoriatest/regress_test.go b/internal/memoriatest/regress_test.go| index 4b6c7e4..96d5c90 100644 |
| --- a/internal/memoriatest/regress_test.go |
| +++ b/internal/memoriatest/regress_test.go |
| @@ -53,7 +53,7 @@ func TestAFailureAfterImportKeepsWhatWeHold(t *testing.T) { |
| } |
| // The document is still held, and memoria still KNOWS it holds it. |
| - if hits, _ := app.Store().Search(context.Background(), "travel booking", 10); len(hits) == 0 { |
| + if hits, _ := searchAsReader(app.Store(), context.Background(), "travel booking", 10); len(hits) == 0 { |
| t.Error("the document was removed when its page 404'd; memoria does not delete on absence") |
| } |
| _, importedAfter, err := app.Store().CountPages(context.Background(), cid) |
diff --git a/internal/memoriatest/searchhelper_test.go b/internal/memoriatest/searchhelper_test.go| new file mode 100644 |
| index 0000000..d37a59d |
| --- /dev/null |
| +++ b/internal/memoriatest/searchhelper_test.go |
| @@ -0,0 +1,30 @@ |
| +package memoriatest |
| + |
| +import ( |
| + "context" |
| + |
| + "amadan.net/oficina/suite" |
| + |
| + "amadan.net/oficina/memoria/internal/memoria" |
| +) |
| + |
| +// testAsker is the person every store-level test searches as. |
| +// |
| +// Β§4.1's typed subject in lone shape: a lowercased address, with the kind |
| +// on the value so a receiver keyed on person refs answers UNKNOWN for the |
| +// whole call rather than guessing. |
| +var testAsker = suite.Subject{Kind: suite.SubjectAddress, Value: "reader@example.com"} |
| + |
| +// searchAsReader runs a search as an ordinary signed-in reader and returns |
| +// the passages that survived Β§4.1's ruling. |
| +// |
| +// **It is not a way around the ruling and must never become one.** It |
| +// calls Store.Search like everything else does; all it supplies is the |
| +// asker the test would otherwise have to spell out at thirty call sites, |
| +// and all it drops is the notices, which these tests are not about. A |
| +// helper that reached past Search to the raw hits would be the seam |
| +// AGENTS.md warns about β the guard that exists only in a comment. |
| +func searchAsReader(s *memoria.Store, ctx context.Context, q string, n int) ([]memoria.Result, error) { |
| + p, err := s.Search(ctx, testAsker, q, n) |
| + return p.Results, err |
| +} |
diff --git a/tools/seed/main.go b/tools/seed/main.go| index 2f2a1af..93f5df5 100644 |
| --- a/tools/seed/main.go |
| +++ b/tools/seed/main.go |
| @@ -78,7 +78,7 @@ func main() { |
| } |
| defer ix.Close() |
| - store := memoria.NewStore(d, sealer, ix) |
| + store := memoria.NewStore(d, sealer, ix, nil) |
| ctx := context.Background() |
| var n, skipped, indexed int |