Repository navigation
Conversation
Teingi
left a comment
Teingi
left a comment
There was a problem hiding this comment.
Four correctness issues need fixing before merge; details and reproductions are inline. I also exercised migration and HTTP/MCP lifecycle flows against real OceanBase 4.3.5.4 with live model calls. Provider account errors and timeouts required supplementary model configurations, so this does not establish an all-green run with the original model configuration. The migration finding reproduces on real OceanBase; the other findings use the controlled timing or model responses described in their comments.
| raise InvalidBaseAccessRequestError("mode", "must be auto, text, vector, or hybrid") | ||
| if kind is not None: | ||
| AtomicMemoryContent(kind=kind, text="validation") | ||
| filters = await application.security.filters(self.scope_id, self._context(context), tags=tag_filter) |
There was a problem hiding this comment.
[P1] Bind search authorization to the data snapshot
filters caches scope_read before query embedding, but retrieval uses a later database snapshot without rechecking that permission. In a SQLite HTTP reproduction, I paused a reader's vector query during embedding, revoked its scope.viewer grant, then created a new memory. Resuming the query returned that new body; a fresh query returned no hits and direct GET returned 403. This reproduces with both built-in and Casbin access providers. The returned memory did not exist while the reader was authorized. Resolve permission and eligible data within the same snapshot after embedding, rather than carrying the earlier scope-wide allow decision into retrieval.
There was a problem hiding this comment.
Fixed in a6d1c379. Query embedding completes before the read transaction; authorization now uses access.with_connection(connection) inside the same consistent snapshot as retrieval and returned-hit validation. Decision audit writes flush after that transaction closes. No Scope row lock was added.
Added HTTP race regressions for both Builtin and Casbin providers: revocation during embedding denies the subsequent search, and revocation plus creation during an established read snapshot can commit without blocking while the reader sees only its original snapshot. The new cases passed on SQLite and real SeekDB. The new and existing snapshot suites also passed on real OceanBase 4.3.5.6: 15 passed, 0 failed, 0 skipped.
|
|
||
| def _imported_refs(entry: _Entry, row: Mapping[str, Any]) -> tuple[dict[str, Any], ...]: | ||
| values = [ | ||
| {"family": "memory", "artifact_id": entry.memory_id, "revision": row["created_in_revision"]}, |
There was a problem hiding this comment.
[P2] Preserve per-entry evidence when importing legacy memory
Adding the whole collection revision to every imported entry's lineage makes EvidenceResolver traverse that collection's Sources too. I migrated a collection written with Sources A and B where entry A cited only Source A: resolving the imported atomic memory produced roots A+B and included B's unrelated body in the evidence projection used by Dream, while its legacy MemoryCitation still resolved only A. This reproduces on SQLite and real OceanBase 4.3.5.4, even though apply, verify and restart report ready. Preserve the selected entry's exact evidence; collection-level migration provenance must not become evidence for every entry. The reproduction exercised evidence resolution, not a Dream model call.
There was a problem hiding this comment.
Fixed in a6d1c379. The stored collection reference remains migration provenance. A shared reader now identifies the imported entry through its exact retained manifest/version and deterministic Atomic identity, then expands only that entry's Source/Artifact evidence and same-entry predecessor. Dream evidence resolution and automatic extraction both use it.
The provenance anchor is excluded from evidence-graph traversal, including when another selected root explicitly references the same collection. Ordinary explicit collection evidence retains its existing meaning. This also fixes already-imported databases without rewriting immutable content or lineage.
Added 13 SQLite regressions covering per-entry Sources and Artifacts, mixed-root selection, history, merge/restore, automatic extraction, and verify/repeated apply with unchanged imported rows.
| model = self.application.embedding_model | ||
| if model is None or profile is None or model.profile != profile: | ||
| raise AtomicMemoryIndexError("embedding-profile", "Related-memory vector profile is unavailable") | ||
| result = await model.embed((query,)) |
There was a problem hiding this comment.
[P2] Use query embeddings for related-memory retrieval
_query_vector() calls model.embed(), while ordinary memory search uses embed_query(). These differ for supported asymmetric embedding providers. With the MiniMax adapter and mocked HTTP responses containing asymmetric vectors, ordinary vector search emits type=query and retrieves the existing memory, but Source flush emits type=db, supplies an empty related set to reconciliation, and creates a second active identity instead of revising the existing one. Use the query embedding boundary here as well so related-memory retrieval has the same query semantics as ordinary search.
There was a problem hiding this comment.
Fixed in a6d1c379. Related-memory retrieval now calls the existing embed_query helper. Document writes still use document embeddings, and symmetric providers retain the helper's fallback to embed.
Added a Source-flush regression using the real MiniMax adapter with mocked asymmetric HTTP responses. It verifies type=query for recall and that an equivalent Source retains the existing memory identity instead of creating a duplicate. A symmetric-provider regression also passes.
| except AtomicMemoryIndexError: | ||
| if not self.config.related_fts_fallback: | ||
| raise | ||
| mode = "fts" |
There was a problem hiding this comment.
[P2] Apply the opt-in FTS fallback to transient embedding failures
The documented ATOMIC_MEMORY_RELATED_FTS_FALLBACK=true option permits full-text retrieval when the query vector is unavailable, but this handler only catches AtomicMemoryIndexError. A provider's InferenceUnavailableError therefore escapes before reconciliation. In a SQLite probe with an existing vectorized memory and a duplicate Source, injecting this temporary embedding failure leaves the cursor at 0 in hybrid mode with fallback enabled; switching to FTS performs a noop, advances the cursor to 2, and retains one active memory without needing new document embeddings. Handle the supported transient query-embedding errors when this fallback is enabled.
There was a problem hiding this comment.
Fixed in a6d1c379. The query-vector boundary now handles InferenceUnavailableError and InferenceTimeoutError when the FTS fallback is explicitly enabled. With fallback disabled, the error propagates and the cursor stays unchanged. Document-embedding failures still fail the write; programming errors and cancellation also propagate.
Source-flush regressions cover unavailable/timeout responses, duplicate noops, create/revise failures, and recovery: 12 passed.
Overall verification: all 25 CI checks pass; the full local E2E rerun has 407 passed, 0 failed, 96 skipped (4 deselected). The full local unit rerun retains one failure: 3,432 passed, 1 failed, 68 skipped. test_destructured_assignments_downgrade_outer_identifiers_and_members[js] exhausted the existing 5-second query deadline during repository revalidation. That case passed in isolation and in all four CI Python versions; the cause of the local timeout remains unconfirmed. No timeout or assertion was weakened.
This branch has not been deployed
2 participants
Which issue or RFC does this PR close?
Implements RFC #1809 and follows the retrieval contract in RFC #1803.
Rationale for this change
Memory collections couple unrelated entries to one revision and write boundary. This change makes each fact, preference or decision an independent Artifact, so content revision, lifecycle, authorization and exact evidence can be managed per memory.
What changes are included in this PR?
atomic-memorycontent with immutable revisions and separateactive,forgotten,mergedandretiredstate. Add explicit writes, exact historical reads, merge, forgetting, restoration preview, restoration and merge undo.create,revise,merge,noop). Recheck evidence, authorization, read dependencies and cursor generation before publishing memory changes and Source progress together.Are there any user-facing changes?
This is a breaking Memory upgrade.
memory: null. Route retention does not preserve the old response models. Collection CAS, citation revise/retire, capacity/compaction, collection Create/Replace, continuous changes and collection rollback are unsupported; HTTP rejects unsupported operations with422 legacy_memory_operation_unsupportedbefore writes.atomic-memory-migrate --action plan/apply/verify; startup verifies readiness instead of converting data automatically. Migration maps legacy entries to deterministic new Artifact IDs while retaining their version chains, historical evidence, lifecycle, ownership, tags, grants/receipts and Source/task progress. Invalid history/ownership, unsupported grants, unresolved candidates or task/cursor formats can block conversion.memory.extractPrompts require explicit migration toatomic_memory.extractandatomic_memory.reconcile. Legacy CandidatePipeline and MemoryWriteGate injection are incompatible with the Atomic runtime.How was this change tested?
Submission checks:
uv run --locked prek run -apassed all 11 hooks, including Ruff and ty;git diff --checkpassed. The seven follow-up files match the recorded validation material.Recorded validation used isolated, locked environments:
pnpm --dir integrations/dsh/plugins/powercontext testandtest:e2e— 266 unit tests and 9 E2E tests passed on each of macOS and Linux. Formal builds reproduced the checked-in bundles. OpenCodetest— 72 passed;typecheckandbuildpassed.text-embedding-3-small. SQLite's three raw XML reports were checked locally. OceanBase's three complete launcher records exited 0; their raw XML/events could not be exported after connectivity was lost. Across all model attempts, results were 9 passed / 8 failed over 17 executions of those six nodes: two provider errors, three model-output/decision failures and three configuration failures. These earlier failures remain recorded.2b903293: 3,798 passed / 152 skipped / 4 deselected. Across recorded implementation commits, all 90 distinct official OceanBase nodes have passing records after targeted fixes/replays. This is not a full-suite run of the submission's seven follow-up files; that full suite was not rerun.A historical Zcode Stop
unknownand one OceanBase FULLTEXT initialization lock timeout were not reproduced in bounded diagnostics; their causes remain unknown. Windows/macOS-specific cases, isolated native service checks and four controlled/live Zcode acceptance nodes remain deferred. Real-model OceanBase raw evidence export and the new validation container's stop operation remain pending connectivity. Migration evidence covers the tested schemas/scenarios, not production deployment acceptance.AI usage statement
GPT-6.1 Sol Ultra subagents performed implementation, validation and PR preparation; the Codex root agent reviewed the result. GPT-5.6 Luna was used as the generation/reranking model in the final real-model acceptance run. Validation scope and evidence limits are stated above.