From e797e065efbd5370cf0f08ff3c8ce8c648f4be96 Mon Sep 17 00:00:00 2001 From: rrs <276464689+robotrocketscience@users.noreply.github.com> Date: Tue, 12 May 2026 15:54:17 -0700 Subject: [PATCH 1/5] feat(store): list_canonical_orphans set-based query (#725) Add MemoryStore.list_canonical_orphans(limit) that returns (belief_id, content_hash) tuples for beliefs where every ingest_log row has source_kind='legacy_unknown' or no row exists at all. Single SQL pass: NOT IN (SELECT je.value FROM ingest_log il, json_each(il.derived_belief_ids) je WHERE source_kind != 'legacy_unknown') replaces the N+1 per-belief iteration. Results ordered ORDER BY b.id ASC to preserve sample stability matching list_belief_ids(). --- src/aelfrice/store.py | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/aelfrice/store.py b/src/aelfrice/store.py index ebe2d042..18619145 100644 --- a/src/aelfrice/store.py +++ b/src/aelfrice/store.py @@ -1473,6 +1473,49 @@ def list_belief_ids(self) -> list[str]: cur = self._conn.execute("SELECT id FROM beliefs ORDER BY id ASC") return [str(r["id"]) for r in cur.fetchall()] + def list_canonical_orphans( + self, + limit: int | None = None, + ) -> list[tuple[str, str | None]]: + """Return `(belief_id, content_hash)` for every canonical orphan. + + A canonical belief is an orphan when every ingest_log row that + points at it has ``source_kind = 'legacy_unknown'``, or when no + ingest_log row points at it at all (pre-#205 stores). + + The set-based query avoids the N+1 per-belief iteration that + ``_compute_replay_drift_report`` previously performed. A single + SQL pass over ``ingest_log`` builds the set of belief ids that + have at least one non-legacy row; the outer ``NOT IN`` selects + the complement. + + Results are ordered ``ORDER BY b.id ASC`` to preserve the same + sample-stability guarantee as ``list_belief_ids()``. + + Args: + limit: When given, cap the returned list. ``None`` returns + all orphans (used for the total count). + """ + limit_clause = f" LIMIT {int(limit)}" if limit is not None else "" + cur = self._conn.execute( + f""" + SELECT b.id, b.content_hash + FROM beliefs b + WHERE b.id NOT IN ( + SELECT je.value + FROM ingest_log il, json_each(il.derived_belief_ids) je + WHERE il.derived_belief_ids IS NOT NULL + AND il.source_kind != ? + ) + ORDER BY b.id ASC{limit_clause} + """, + (INGEST_SOURCE_LEGACY_UNKNOWN,), + ) + return [ + (str(r["id"]), str(r["content_hash"]) if r["content_hash"] is not None else None) + for r in cur.fetchall() + ] + # --- Belief CRUD ------------------------------------------------------ def insert_belief(self, b: Belief) -> None: From 8c7731602fea95044bc1bcbdd51f45ed94cf2cb8 Mon Sep 17 00:00:00 2001 From: rrs <276464689+robotrocketscience@users.noreply.github.com> Date: Tue, 12 May 2026 15:56:06 -0700 Subject: [PATCH 2/5] test(store): unit tests for list_canonical_orphans (#725) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nine tests in test_store_crud.py covering: - empty store, no log rows (pre-#205), all-legacy rows - non-legacy row excludes belief - mixed beliefs (orphans vs non-orphans) - mixed log rows on same belief (both legacy + non-legacy → not orphan) - limit= cap, ORDER BY id ASC ordering, content_hash tuple field --- tests/test_store_crud.py | 128 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 128 insertions(+) diff --git a/tests/test_store_crud.py b/tests/test_store_crud.py index ee01df24..21e0dfc2 100644 --- a/tests/test_store_crud.py +++ b/tests/test_store_crud.py @@ -7,6 +7,8 @@ BELIEF_FACTUAL, BELIEF_SCOPE_GLOBAL, EDGE_SUPPORTS, + INGEST_SOURCE_CLI_REMEMBER, + INGEST_SOURCE_LEGACY_UNKNOWN, LOCK_NONE, RETENTION_FACT, RETENTION_TRANSIENT, @@ -207,3 +209,129 @@ def test_stamp_retrieved_overwrites_prior_timestamp() -> None: s.stamp_retrieved(["b1"], ts="2026-04-28T01:00:00Z") s.stamp_retrieved(["b1"], ts="2026-04-28T02:00:00Z") assert s.get_belief("b1").last_retrieved_at == "2026-04-28T02:00:00Z" # type: ignore[union-attr] + + +# --------------------------------------------------------------------------- +# list_canonical_orphans (#725) +# --------------------------------------------------------------------------- + +_TS = "2026-05-12T00:00:00+00:00" + + +def test_list_canonical_orphans_empty_store() -> None: + """Empty store → empty list.""" + s = MemoryStore(":memory:") + assert s.list_canonical_orphans() == [] + + +def test_list_canonical_orphans_no_log_rows() -> None: + """Belief with zero ingest_log rows is a canonical orphan (pre-#205 case).""" + s = MemoryStore(":memory:") + s.insert_belief(_mk_belief("b1")) + result = s.list_canonical_orphans() + assert result == [("b1", "h_b1")] + + +def test_list_canonical_orphans_all_legacy_rows() -> None: + """Belief whose only log rows are legacy_unknown → canonical orphan.""" + s = MemoryStore(":memory:") + s.insert_belief(_mk_belief("b1")) + s.record_ingest( + source_kind=INGEST_SOURCE_LEGACY_UNKNOWN, + source_path=None, + raw_text="content b1", + derived_belief_ids=["b1"], + ts=_TS, + ) + result = s.list_canonical_orphans() + assert result == [("b1", "h_b1")] + + +def test_list_canonical_orphans_non_legacy_row_excludes_belief() -> None: + """Belief with a non-legacy log row is not an orphan.""" + s = MemoryStore(":memory:") + s.insert_belief(_mk_belief("b1")) + s.record_ingest( + source_kind=INGEST_SOURCE_CLI_REMEMBER, + source_path=None, + raw_text="content b1", + derived_belief_ids=["b1"], + ts=_TS, + ) + assert s.list_canonical_orphans() == [] + + +def test_list_canonical_orphans_mixed_beliefs() -> None: + """Only all-legacy beliefs surface; non-legacy-covered beliefs are excluded.""" + s = MemoryStore(":memory:") + s.insert_belief(_mk_belief("b1")) # will be orphan + s.insert_belief(_mk_belief("b2")) # has a real log row → not orphan + s.insert_belief(_mk_belief("b3")) # no log rows at all → orphan + s.record_ingest( + source_kind=INGEST_SOURCE_LEGACY_UNKNOWN, + source_path=None, + raw_text="content b1", + derived_belief_ids=["b1"], + ts=_TS, + ) + s.record_ingest( + source_kind=INGEST_SOURCE_CLI_REMEMBER, + source_path=None, + raw_text="content b2", + derived_belief_ids=["b2"], + ts=_TS, + ) + result = s.list_canonical_orphans() + assert result == [("b1", "h_b1"), ("b3", "h_b3")] + + +def test_list_canonical_orphans_mixed_log_rows_same_belief() -> None: + """A belief with both a legacy and a non-legacy row is NOT an orphan.""" + s = MemoryStore(":memory:") + s.insert_belief(_mk_belief("b1")) + s.record_ingest( + source_kind=INGEST_SOURCE_LEGACY_UNKNOWN, + source_path=None, + raw_text="content b1", + derived_belief_ids=["b1"], + ts=_TS, + ) + s.record_ingest( + source_kind=INGEST_SOURCE_CLI_REMEMBER, + source_path=None, + raw_text="content b1", + derived_belief_ids=["b1"], + ts=_TS, + ) + assert s.list_canonical_orphans() == [] + + +def test_list_canonical_orphans_limit() -> None: + """limit= caps the returned list.""" + s = MemoryStore(":memory:") + for i in range(5): + s.insert_belief(_mk_belief(f"b{i}")) + result = s.list_canonical_orphans(limit=3) + assert len(result) == 3 + + +def test_list_canonical_orphans_order_by_id_asc() -> None: + """Results are returned in id ASC order (same as list_belief_ids).""" + s = MemoryStore(":memory:") + for bid in ["bz", "ba", "bm"]: + s.insert_belief(_mk_belief(bid)) + ids_orphans = [r[0] for r in s.list_canonical_orphans()] + ids_list = s.list_belief_ids() + assert ids_orphans == sorted(ids_orphans) + assert ids_orphans == ids_list + + +def test_list_canonical_orphans_no_log_rows_returns_content_hash() -> None: + """Tuples carry the correct content_hash from the beliefs row.""" + s = MemoryStore(":memory:") + s.insert_belief(_mk_belief("bx")) + result = s.list_canonical_orphans() + assert len(result) == 1 + bid, ch = result[0] + assert bid == "bx" + assert ch == "h_bx" From 3355d23fb387bf308cbb074436e62d64471999e7 Mon Sep 17 00:00:00 2001 From: rrs <276464689+robotrocketscience@users.noreply.github.com> Date: Tue, 12 May 2026 15:57:26 -0700 Subject: [PATCH 3/5] perf(replay): use list_canonical_orphans in _compute_replay_drift_report (#725) Replace the N+1 per-belief loop (list_belief_ids + iter_ingest_log_for_belief per id + get_belief per orphan) with a single call to the new set-based list_canonical_orphans(). Removes the TODO(perf) comment. Output is byte-identical: canonical_orphan count and examples_canonical_orphan sample are produced from the same ordered set, capped at drift_examples. --- src/aelfrice/replay.py | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/src/aelfrice/replay.py b/src/aelfrice/replay.py index 2988f3ed..410a7fb6 100644 --- a/src/aelfrice/replay.py +++ b/src/aelfrice/replay.py @@ -338,26 +338,14 @@ def replay_full_equality( # --- Canonical orphans ------------------------------------------------- # A canonical belief is an orphan when every log row pointing at it is # legacy_unknown (or there are no log rows at all, pre-#205). - # TODO(perf): replace N+1 iteration with set-based store query — see - # follow-up issue. - belief_ids = store.list_belief_ids() - canonical_orphan = 0 + all_orphans = store.list_canonical_orphans() + canonical_orphan = len(all_orphans) examples_canonical_orphan: list[dict] = [] # type: ignore[type-arg] - - for bid in belief_ids: - all_rows = store.iter_ingest_log_for_belief(bid) - has_non_legacy = any( - str(r.get("source_kind", "")) != INGEST_SOURCE_LEGACY_UNKNOWN - for r in all_rows - ) - if not has_non_legacy: - canonical_orphan += 1 - if len(examples_canonical_orphan) < drift_examples: - b = store.get_belief(bid) - examples_canonical_orphan.append({ - "belief_id": bid, - "content_hash": b.content_hash if b is not None else None, - }) + for bid, content_hash in all_orphans[:drift_examples]: + examples_canonical_orphan.append({ + "belief_id": bid, + "content_hash": content_hash, + }) # feedback_derived_edges: always 0 — edges table has no source column. # See docstring for explanation. From 699cb8300f13e69f93c270c15d7ab5b9ea5416ef Mon Sep 17 00:00:00 2001 From: rrs <276464689+robotrocketscience@users.noreply.github.com> Date: Tue, 12 May 2026 15:57:55 -0700 Subject: [PATCH 4/5] =?UTF-8?q?docs(changelog):=20#725=20N+1=20=E2=86=92?= =?UTF-8?q?=20set-based=20query=20in=20replay=20drift=20report?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add Performance entry under [3.0.0] - Unreleased describing the list_canonical_orphans optimization and its byte-identical contract. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e4a5467..edb863ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,8 @@ installable release; see the roadmap in [README.md](README.md). ### Performance +- **N+1 → set-based query for canonical-orphan count in replay drift report** ([#725](https://github.com/robotrocketscience/aelfrice/issues/725)). `_compute_replay_drift_report` previously iterated every belief id in the store, issuing one `iter_ingest_log_for_belief` full-table scan per belief to test for any non-legacy log row, then a `get_belief` fetch per orphan for the example payload — an O(N²) pattern. Replaced by a single `MemoryStore.list_canonical_orphans(limit)` call that pushes the classification into SQLite: a `NOT IN (SELECT je.value FROM ingest_log il, json_each(il.derived_belief_ids) je WHERE source_kind != 'legacy_unknown')` subquery builds the covered-belief set once; the outer SELECT walks only the orphan complement, ordered `BY b.id ASC` to preserve sample stability. Output is byte-identical (same count, same examples in the same order). Nine unit tests in `test_store_crud.py` cover the empty-store, no-log-rows, all-legacy, non-legacy-excludes, mixed-beliefs, mixed-log-rows-same-belief, limit, ordering, and content-hash-field cases. + - **Update-check cache TTL: 6h → 15min** (`src/aelfrice/lifecycle.py:CACHE_TTL_SECONDS`). The PyPI version-check cache used to expire after six hours, so a freshly-published release could lag the user-visible "update available" banner by up to that long on a busy machine (longer on a quiet one — the check is gated behind the next CLI call or `UserPromptSubmit` hook fire). PyPI's JSON endpoint is CDN-cached and unauthenticated, so a 15-minute cadence is well within the polling-etiquette band and shrinks the worst-case banner-lag window from 6h to ~15min. Detached-subprocess + on-disk-cache architecture is unchanged. ### Added From a8d0b8dde9392460ca41b74c4079c558f2222dbd Mon Sep 17 00:00:00 2001 From: rrs <276464689+robotrocketscience@users.noreply.github.com> Date: Tue, 12 May 2026 18:05:22 -0700 Subject: [PATCH 5/5] refactor(store): parameterize LIMIT in list_canonical_orphans (#725 sourcery) opengrep flagged the f-string LIMIT interpolation as a SQL-injection shape even though int(limit) made it runtime-safe. Switch to bound parameter for static-analysis cleanliness and consistency with the rest of the store query layer. No behaviour change. Sourcery's adjacent suggestions to COALESCE source_kind for NULL parity and switch NOT IN to NOT EXISTS for NULL-safe je.value are moot under the current schema: source_kind is TEXT NOT NULL and derived_belief_ids holds list[str]. Not adding defensive code for guaranteed invariants. --- src/aelfrice/store.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/aelfrice/store.py b/src/aelfrice/store.py index 18619145..e560326a 100644 --- a/src/aelfrice/store.py +++ b/src/aelfrice/store.py @@ -1496,9 +1496,7 @@ def list_canonical_orphans( limit: When given, cap the returned list. ``None`` returns all orphans (used for the total count). """ - limit_clause = f" LIMIT {int(limit)}" if limit is not None else "" - cur = self._conn.execute( - f""" + sql = """ SELECT b.id, b.content_hash FROM beliefs b WHERE b.id NOT IN ( @@ -1507,10 +1505,13 @@ def list_canonical_orphans( WHERE il.derived_belief_ids IS NOT NULL AND il.source_kind != ? ) - ORDER BY b.id ASC{limit_clause} - """, - (INGEST_SOURCE_LEGACY_UNKNOWN,), - ) + ORDER BY b.id ASC + """ + params: tuple[object, ...] = (INGEST_SOURCE_LEGACY_UNKNOWN,) + if limit is not None: + sql += " LIMIT ?" + params = (INGEST_SOURCE_LEGACY_UNKNOWN, int(limit)) + cur = self._conn.execute(sql, params) return [ (str(r["id"]), str(r["content_hash"]) if r["content_hash"] is not None else None) for r in cur.fetchall()