Skip to content

Commit fa60c79

Browse files
committed
fix(layers): fetch the most recent drawers for L1 wake-up
PR #1630 fixed the L1 wake-up ordering by adding filed_at as a secondary sort key, and documented what it could not fix: "The fetch still scans up to MAX_SCAN=2000 drawers in collection (insertion) order, so on a very large unscoped wing the most-recent drawers may fall outside the cap before sorting... A SQL-side most-recent-N by filed_at fetch would fix the unscoped case but is a larger change left for a follow-up." This is that follow-up. On a 149k-drawer palace the sort was correct and the input was not: the 2000 drawers Layer1 scored were the oldest backfill slice, so wake-up permanently opened on the first files ever mined and never on this week's sessions. Backend capability rather than a pgvector special case: - BaseCollection.get_recent(limit, where, order_field, include) returns up to limit records newest-first on an ISO-8601 metadata field. The ABC default pages through get() and sorts the window locally, which is exactly what Layer1 did inline, so every backend that does not override it behaves as before. - PgVectorCollection overrides it with ORDER BY metadata->>%s DESC NULLS LAST, id pushed into the scan, and PgVectorBackend advertises the supports_recency_order capability token. That is exact at any table size. Filters that pgvector cannot push down exactly keep the existing local post-filter path. - EmbeddingCollection forwards get_recent explicitly. Without the forwarder, MRO would resolve the concrete ABC default on the wrapper and shadow the inner backend's pushdown (the invariant test_wrapper_forwards_all_concrete_basecollection_methods guards). - Layer1._fetch_candidates uses the capability and falls back to the previous paged scan when a collection predates get_recent or the backend errors, so wake-up degrades instead of failing. recency_sort_key is shared so every local sort orders identically: records missing the field, holding an empty string, or holding a non-string sort last instead of raising on a str/None comparison. No ordering semantics change for Layer1 itself. importance stays the primary key and filed_at the tiebreak; only the candidate window changes, from "the first 2000 rows the backend hands back" to "the 2000 most recently filed". Measured on the 149k-drawer palace this was written for: wake-up now leads with the newest sessions and renders in 0.72s. Tests: base default (ordering, missing/odd timestamps, window cap, where passthrough, dict-shaped get), pgvector pushdown (SQL text and bind order, filter pushdown, local-filter fallback, include, zero limit, custom order field), and Layer1 (capability used, wing filter forwarded, fallback when the capability is missing or raises).
1 parent 639c69a commit fa60c79

9 files changed

Lines changed: 709 additions & 25 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
1313
- **`sweep` books a failed `stat` as a failure again.** The non-regular-file gate added in 3.7.1 reads `stat.S_ISREG(f.stat().st_mode)` inside a `try`, and its `except OSError` printed `SKIP` and moved on. A dangling symlink, a symlink loop and a file unlinked between `rglob` and the gate all raise there, and before the gate existed every one of them reached `sweep()` and was booked in `failures` — so `sweep` went from reporting a transcript it could not read to reporting success. A failed probe is now an error, not a benign file type: it is logged, printed as `WARNING`, and appended to `failures`, while a probe that succeeds and says "not regular" still skips silently. (#2221)
1414
- **`mempalace init` no longer tracebacks on a directory it cannot enter.** `_parse_gradle`'s `is_file()` gate sat in front of the `try` that the parser's own `except OSError` provides, so a manifest under a directory with `r` but no `x` raised `PermissionError` out of a call that used to answer "no manifest name". The gate moved inside that `try`, and `_collect_manifest_names` stats through `os.path.isfile`, which reports rather than raises. (#2221)
1515
- **`split` no longer blocks on a FIFO at its own output name, nor writes through a broken link.** The type gate in `main` covers the files the glob listed; `split_file` builds its output names itself, so a pre-existing named pipe at one of them wedged `write_text` in the kernel waiting for a reader. The check asks about the link itself rather than its target, because a dangling symlink at an output name reads as "nothing there" and `write_text` would create the target — landing a chunk outside the output directory. Output names that are anything but a regular file are now skipped with a `SKIP` line. (#2221)
16+
- **Layer 1 wake-up fetches the most recent drawers instead of an arbitrary scan window.** `BaseCollection.get_recent()` returns the newest N by `filed_at`; pgvector overrides it with an `ORDER BY ... DESC LIMIT n` pushdown (capability token `supports_recency_order`), and backends without pushdown keep the previous scan-and-sort behavior. On palaces larger than the 2,000-drawer scan cap, wake-up no longer leads with the oldest backfill. (#1630)
1617

1718
---
1819

mempalace/backends/base.py

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,30 @@ class LexicalResult:
354354
hits: list[LexicalHit]
355355

356356

357+
def recency_sort_key(meta: Optional[dict], order_field: str = "filed_at") -> tuple[int, str]:
358+
"""Sort key for newest-first ordering on an ISO-8601 metadata field.
359+
360+
Returns ``(1, value)`` for a usable timestamp string and ``(0, "")``
361+
otherwise, so that with ``reverse=True`` records missing the field sort
362+
last instead of raising on a str/None comparison. Backends that implement
363+
:meth:`BaseCollection.get_recent` with a local sort MUST use this key so
364+
every backend orders identically.
365+
"""
366+
value = (meta or {}).get(order_field)
367+
if not isinstance(value, str) or not value:
368+
return (0, "")
369+
return (1, value)
370+
371+
372+
def _recency_order(metadatas: list[dict], order_field: str) -> list[int]:
373+
"""Indices into ``metadatas``, newest first, missing timestamps last."""
374+
return sorted(
375+
range(len(metadatas)),
376+
key=lambda i: recency_sort_key(metadatas[i], order_field),
377+
reverse=True,
378+
)
379+
380+
357381
# ---------------------------------------------------------------------------
358382
# Collection contract
359383
# ---------------------------------------------------------------------------
@@ -504,6 +528,74 @@ def get_all_metadata(self, where: Optional[dict] = None) -> list[dict]:
504528
offset += len(batch_meta)
505529
return all_meta
506530

531+
def get_recent(
532+
self,
533+
*,
534+
limit: int,
535+
where: Optional[dict] = None,
536+
order_field: str = "filed_at",
537+
include: Optional[list[str]] = None,
538+
) -> GetResult:
539+
"""Return up to ``limit`` records, newest first by ``order_field``.
540+
541+
``order_field`` names a metadata key holding an ISO-8601 timestamp
542+
string (``filed_at`` for drawers). Ordering is descending on that
543+
string, which is chronological for ISO-8601. Records whose value is
544+
missing, empty, or not a string sort last.
545+
546+
The default implementation pages through :meth:`get` in storage order
547+
up to ``limit`` records and sorts that window locally. That is exact
548+
for a collection no larger than ``limit`` and *approximate* above it:
549+
the window is whatever the backend hands back first, so the genuinely
550+
newest records can fall outside it (issue #1630's known limitation for
551+
Layer 1 wake-up). Backends able to push the ordering into storage MUST
552+
override this and advertise the ``supports_recency_order`` capability
553+
token, which promises exactness at any collection size.
554+
555+
Callers that need the guarantee should check the token rather than
556+
assume it; the default is always available so no backend breaks.
557+
"""
558+
if limit <= 0:
559+
return GetResult.empty()
560+
include = ["documents", "metadatas"] if include is None else list(include)
561+
562+
ids: list[str] = []
563+
documents: list[str] = []
564+
metadatas: list[dict] = []
565+
offset = 0
566+
fetched = 0
567+
page_size = min(500, limit)
568+
while fetched < limit:
569+
kwargs: dict = {"include": include, "limit": page_size, "offset": offset}
570+
if where:
571+
kwargs["where"] = where
572+
batch = self.get(**kwargs)
573+
batch_ids = list(batch.get("ids") or [])
574+
batch_docs = list(batch.get("documents") or [])
575+
batch_metas = list(batch.get("metadatas") or [])
576+
page_len = max(len(batch_ids), len(batch_docs), len(batch_metas))
577+
if not page_len:
578+
break
579+
# Pad the projections the caller did not request so the three
580+
# lists stay index-aligned for the sort below.
581+
ids.extend(batch_ids or [""] * page_len)
582+
documents.extend(batch_docs or [""] * page_len)
583+
metadatas.extend(batch_metas or [{}] * page_len)
584+
offset += page_len
585+
fetched += page_len
586+
if page_len < page_size:
587+
break
588+
589+
n = min(len(ids), len(documents), len(metadatas))
590+
ids, documents, metadatas = ids[:n], documents[:n], metadatas[:n]
591+
order = _recency_order(metadatas, order_field)[:limit]
592+
return GetResult(
593+
ids=[ids[i] for i in order],
594+
documents=[documents[i] for i in order],
595+
metadatas=[metadatas[i] for i in order],
596+
embeddings=None,
597+
)
598+
507599
def facet_counts(
508600
self,
509601
field: str,

mempalace/backends/embedding_wrapper.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,21 @@ def health(self):
167167
def lexical_search(self, *, query: str, n_results: int = 10, where: Optional[dict] = None):
168168
return self._inner.lexical_search(query=query, n_results=n_results, where=where)
169169

170+
def get_recent(
171+
self,
172+
*,
173+
limit: int,
174+
where: Optional[dict] = None,
175+
order_field: str = "filed_at",
176+
include: Optional[list[str]] = None,
177+
):
178+
# Concrete on ``BaseCollection`` (the scan-and-sort default), so MRO
179+
# would resolve it here and shadow a backend that pushes the ordering
180+
# into storage. Forward explicitly.
181+
return self._inner.get_recent(
182+
limit=limit, where=where, order_field=order_field, include=include
183+
)
184+
170185
def facet_counts(
171186
self, field: str, where: Optional[dict] = None, limit: int = 1000
172187
) -> dict[str, int]:

mempalace/backends/pgvector.py

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,7 @@ def scroll_rows(
678678
with_document: bool = True,
679679
limit: Optional[int] = None,
680680
offset: Optional[int] = None,
681+
order_field: Optional[str] = None,
681682
) -> list[dict]:
682683
qi = _quote_identifier(table)
683684
params: list = []
@@ -695,7 +696,21 @@ def scroll_rows(
695696
# primary key gives OFFSET a stable order (an unordered scan may skip
696697
# or repeat rows across pages); callers that scroll the whole table
697698
# pass neither bound, leaving their SQL unchanged.
698-
if limit is not None or offset:
699+
if order_field is not None:
700+
# Newest-first on an ISO-8601 metadata field. ISO-8601 sorts
701+
# chronologically as text, so ``metadata->>field DESC`` needs no
702+
# timestamp cast (a cast would also fail hard on one malformed
703+
# value). NULLS LAST keeps records without the field at the end;
704+
# ``id`` breaks ties so the order is total and stable.
705+
params.append(order_field)
706+
sql += " ORDER BY metadata->>%s DESC NULLS LAST, id"
707+
if limit is not None:
708+
params.append(int(limit))
709+
sql += " LIMIT %s"
710+
if offset:
711+
params.append(int(offset))
712+
sql += " OFFSET %s"
713+
elif limit is not None or offset:
699714
sql += " ORDER BY id"
700715
if limit is not None:
701716
params.append(int(limit))
@@ -893,6 +908,7 @@ def _scroll(
893908
with_document=True,
894909
limit=None,
895910
offset=None,
911+
order_field=None,
896912
) -> list[dict]:
897913
self._ensure_open()
898914
if not self._table_exists():
@@ -906,6 +922,7 @@ def _scroll(
906922
with_document=with_document,
907923
limit=limit,
908924
offset=offset,
925+
order_field=order_field,
909926
)
910927

911928
def get_all_metadata(self, where=None) -> list[dict]:
@@ -1194,6 +1211,42 @@ def get(
11941211
embeddings=[row["embedding"] or [] for row in rows] if spec.embeddings else None,
11951212
)
11961213

1214+
def get_recent(self, *, limit, where=None, order_field="filed_at", include=None):
1215+
"""Newest-first fetch with the ordering pushed into SQL.
1216+
1217+
The base implementation scans a window in storage order and sorts it
1218+
locally, so on a collection larger than ``limit`` the genuinely newest
1219+
records can be missing from the window entirely (#1630). Postgres can
1220+
do the whole thing: ``ORDER BY metadata->>'filed_at' DESC ... LIMIT n``
1221+
is exact at any table size, which is why this backend advertises
1222+
``supports_recency_order``.
1223+
1224+
Filters that ``metadata @> ...`` cannot express exactly fall back to
1225+
the local post-filter path (same correctness contract as ``get``), and
1226+
there the SQL order is still applied before the filter, so the result
1227+
is the newest ``limit`` rows *that match* only when the pushdown was
1228+
exact. The inexact case re-sorts locally after filtering.
1229+
"""
1230+
if limit is not None and limit <= 0:
1231+
return GetResult.empty()
1232+
_validate_where(where)
1233+
spec = _IncludeSpec.resolve(include, default_distances=False)
1234+
local_filter = _requires_local_filter(where)
1235+
rows = self._scroll(
1236+
where=None if local_filter else where,
1237+
with_embedding=spec.embeddings,
1238+
limit=None if local_filter else limit,
1239+
order_field=order_field,
1240+
)
1241+
if local_filter:
1242+
rows = [row for row in rows if _matches_where(row["metadata"], where)][:limit]
1243+
return GetResult(
1244+
ids=[row["id"] for row in rows],
1245+
documents=[row["document"] for row in rows] if spec.documents else [],
1246+
metadatas=[row["metadata"] for row in rows] if spec.metadatas else [],
1247+
embeddings=[row["embedding"] or [] for row in rows] if spec.embeddings else None,
1248+
)
1249+
11971250
def delete(self, *, ids=None, where=None):
11981251
_validate_where(where)
11991252
if not self._table_exists():
@@ -1336,6 +1389,7 @@ class PgVectorBackend(BaseBackend):
13361389
"supports_metadata_filters",
13371390
"supports_lexical_search",
13381391
"supports_metadata_facets",
1392+
"supports_recency_order",
13391393
"supports_namespace_isolation",
13401394
"supports_server_side_indexes",
13411395
"server_mode",

mempalace/layers.py

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,28 +87,47 @@ class Layer1:
8787

8888
MAX_DRAWERS = 15 # at most 15 moments in wake-up
8989
MAX_CHARS = 3200 # hard cap on total L1 text (~800 tokens)
90-
MAX_SCAN = 2000 # don't scan more than this for L1 generation
90+
MAX_SCAN = 2000 # size of the candidate window pulled for L1 generation
9191

9292
def __init__(self, palace_path: str = None, wing: str = None):
9393
cfg = MempalaceConfig()
9494
self.palace_path = palace_path or cfg.palace_path
9595
self.wing = wing
9696

97-
def generate(self) -> str:
98-
"""Pull top drawers from ChromaDB and format as compact L1 text."""
99-
try:
100-
col = _get_collection(self.palace_path, create=False)
101-
except Exception:
102-
return "## L1 — No palace found. Run: mempalace mine <dir>"
97+
def _fetch_candidates(self, col) -> tuple[list, list]:
98+
"""Fetch the L1 candidate window: the MAX_SCAN most recently filed drawers.
10399
104-
# Fetch all drawers in batches to avoid SQLite variable limit (~999)
100+
Uses the backend's ``get_recent`` capability, which pushes
101+
``ORDER BY filed_at DESC LIMIT n`` into storage where the backend can
102+
(pgvector today) and otherwise falls back to the scan-then-sort default
103+
in ``BaseCollection``. That default is what this method used to do
104+
inline, so backends without pushdown behave exactly as before.
105+
106+
Third-party collections predating ``get_recent`` (or any backend error)
107+
degrade to the inline paged scan below rather than failing wake-up.
108+
"""
109+
where = {"wing": self.wing} if self.wing else None
110+
getter = getattr(col, "get_recent", None)
111+
if getter is not None:
112+
try:
113+
result = getter(
114+
limit=self.MAX_SCAN,
115+
where=where,
116+
order_field="filed_at",
117+
include=["documents", "metadatas"],
118+
)
119+
return list(result.documents or []), list(result.metadatas or [])
120+
except Exception:
121+
pass # capability missing or backend hiccup — page it manually
122+
123+
# Fetch in batches to avoid SQLite variable limit (~999)
105124
_BATCH = 500
106125
docs, metas = [], []
107126
offset = 0
108127
while True:
109128
kwargs = {"include": ["documents", "metadatas"], "limit": _BATCH, "offset": offset}
110-
if self.wing:
111-
kwargs["where"] = {"wing": self.wing}
129+
if where:
130+
kwargs["where"] = where
112131
try:
113132
batch = col.get(**kwargs)
114133
except Exception:
@@ -122,6 +141,16 @@ def generate(self) -> str:
122141
offset += len(batch_docs)
123142
if len(batch_docs) < _BATCH or len(docs) >= self.MAX_SCAN:
124143
break
144+
return docs, metas
145+
146+
def generate(self) -> str:
147+
"""Pull top drawers from the palace and format as compact L1 text."""
148+
try:
149+
col = _get_collection(self.palace_path, create=False)
150+
except Exception:
151+
return "## L1 — No palace found. Run: mempalace mine <dir>"
152+
153+
docs, metas = self._fetch_candidates(col)
125154

126155
if not docs:
127156
return "## L1 — No memories yet."
@@ -136,6 +165,10 @@ def generate(self) -> str:
136165
# newest first. This keeps importance as the primary key for the day a
137166
# scoring pass populates it, while making the "recent filing" half of
138167
# the promise true today with data we already have.
168+
# The candidate window this sorts is now the MAX_SCAN *most recently
169+
# filed* drawers rather than the first MAX_SCAN the backend happened to
170+
# hand back, so on a palace larger than MAX_SCAN the newest drawers are
171+
# actually in the running (#1630's known limitation).
139172
scored = []
140173
for doc, meta in zip(docs, metas):
141174
meta = meta or {}

0 commit comments

Comments
 (0)