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 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. diff --git a/src/aelfrice/store.py b/src/aelfrice/store.py index ebe2d042..e560326a 100644 --- a/src/aelfrice/store.py +++ b/src/aelfrice/store.py @@ -1473,6 +1473,50 @@ 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). + """ + sql = """ + 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 + """ + 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() + ] + # --- Belief CRUD ------------------------------------------------------ def insert_belief(self, b: Belief) -> None: 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"