Document compatibility shims for cleanup tracking #11
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| mems = _apply_filters(memories, namespace, entity_labels, cfg) | ||
| ranked = hybrid_search(query, mems, cfg) | ||
| # Return only Memory objects | ||
| return [m for m, _ in ranked] | ||
| baseline = [m for m, _ in ranked] | ||
| if not baseline: | ||
| return [] | ||
|
|
||
| if reranker is None: | ||
| return baseline[: cfg.top_k] | ||
|
|
||
| subset = mems[: cfg.rerank_k] | ||
| reranked_subset = apply_reranker(query, subset, cfg.rerank_k, reranker) |
There was a problem hiding this comment.
The rerank path builds subset = mems[: cfg.rerank_k] before invoking apply_reranker, but mems is just the filtered input sequence in its original order. Candidates that scored highest in hybrid_search but appear later in the input are never passed to the reranker, so the reranking step can ignore the best matches and return a degraded result set. The reranker should receive the top items from the hybrid ranking (e.g. the head of baseline) rather than the raw, unsorted list.
Useful? React with 👍 / 👎.
| def upsert_entity(self, label: str, name: str, props: Dict[str, Any]) -> None: | ||
| payload = dict(props) | ||
| uid = str(payload.get("uuid")) | ||
| if not uid: | ||
| raise ValueError("Memory props must include a UUID for SQLiteGraphDriver") | ||
| payload.setdefault("entity_label", label) | ||
| payload.setdefault("name", name) | ||
| namespace = payload.get("namespace") | ||
| cur = self._conn.cursor() | ||
| cur.execute( | ||
| """ | ||
| INSERT INTO entities (uuid, label, name, namespace, props) | ||
| VALUES (:uuid, :label, :name, :namespace, :props) | ||
| ON CONFLICT(uuid) DO UPDATE SET | ||
| label=excluded.label, | ||
| name=excluded.name, | ||
| namespace=excluded.namespace, | ||
| props=excluded.props | ||
| """, | ||
| { | ||
| "uuid": uid, | ||
| "label": payload.get("entity_label", label), | ||
| "name": payload.get("name", name), | ||
| "namespace": namespace, | ||
| "props": json.dumps(payload), | ||
| }, |
There was a problem hiding this comment.
The SQLite driver writes entity properties by calling json.dumps(payload) before inserting the row. A normal Memory.model_dump() includes datetime instances (e.g. created_at), which are not JSON serializable, so storing a newly created memory raises TypeError: Object of type datetime is not JSON serializable and the insert fails. The driver needs to coerce non‑serializable values (ISO‑format datetimes, lists, etc.) before dumping to JSON to make the backend usable.
Useful? React with 👍 / 👎.
Summary
Testing
https://chatgpt.com/codex/tasks/task_b_68ee1c9ee92c83218cbbebce8b0667b8