refactor(memory): remove facade test shims

[yyhhyyyyyy]

• Move memory presenter tests from facade private getters to service-level test seams
• Remove legacy MemoryPresenter runtime accessor shims
• Keep lifecycle, cleanup, cold-path, cooldown, and dispose coverage intact

Summary by CodeRabbit

• Refactor
  • Simplified memory presentation internals by removing legacy test-only access paths and cleanup hooks.
• Tests
  • Standardized memory-related tests around a shared helper for inspecting runtime state, improving consistency across lifecycle, maintenance, and concurrency coverage.
  • Updated verification steps and architecture notes for the new testing approach.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds architecture docs (plan, spec, tasks) for an agent-memory test-seams refactor, removes MemoryPresenter's private test-only compat/runtime accessor methods, and refactors memoryPresenter.test.ts to use a new memoryRuntimeForTests helper exposing service-level runtime state instead of inline type casts. No production behavior changes.

Changes

Agent memory test seams refactor

Layer / File(s)	Summary
Architecture plan, spec, and tasks docs docs/architecture/agent-memory-test-seams/plan.md, spec.md, tasks.md	New documents describing the test-only refactor strategy, acceptance criteria, constraints, and checklist for migrating tests off facade-level runtime accessors.
MemoryPresenter compat accessor removal src/main/presenter/memoryPresenter/index.ts	Removes the unused IMemoryVectorStore import and deletes private compat/runtime accessor methods and the constructor call retaining them, leaving public runtime APIs unchanged.
Runtime test seams helper test/main/presenter/memoryPresenter.test.ts	Adds MemoryPresenterRuntimeTestSeams type and memoryRuntimeForTests(presenter) helper exposing internal tracking state (embedding, vector store, maintenance, persona, orphan reconcile, error retry) and small wiring utilities.
Lifecycle/cleanup, recall/search, and error-retry test migrations test/main/presenter/memoryPresenter.test.ts	Refactors numerous tests (cleanup, embedding drains/warmups, backfill, persona locks, cold/warm recall, semantic neighbors, dispose, error retry cooldowns, orphan reconcile) to use memoryRuntimeForTests/internals instead of inline presenter as unknown as {...} casts.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• ThinkInAIXYZ/deepchat#1770: Introduced the original MemoryPresenter and memoryPresenter.test.ts implementations that this PR refactors.
• ThinkInAIXYZ/deepchat#1873: Introduced the service-level runtime state helpers/contracts (getMutableRuntimeStateForTests()) that this PR migrates tests onto.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly describes the main change: removing facade test shims from the MemoryPresenter, which is the core of this refactoring effort.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/memory-test-seams

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

test/main/presenter/memoryPresenter.test.ts (1)

57-123: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Flat-spread runtime seam risks silent key collisions.

memoryRuntimeForTests merges six independently-typed getMutableRuntimeStateForTests() results into one flat object via spread. This currently works because none of the exposed keys collide, but nothing prevents a future addition (e.g. a new field on maintenance or persona) from silently shadowing an existing key from another service, with no compiler error to catch it since each source object is spread rather than namespaced.

Consider namespacing by service to make the seam self-documenting and collision-proof:

♻️ Suggested restructuring

 function memoryRuntimeForTests(presenter: MemoryPresenter) {
   const internals = presenter as unknown as MemoryPresenterRuntimeTestSeams
   return {
     embeddingService: internals.embedding,
     vectorStoreService: internals.vectorStore,
-    ...internals.embedding.getMutableRuntimeStateForTests(),
-    ...internals.vectorStore.getMutableRuntimeStateForTests(),
-    ...internals.maintenance.getMutableRuntimeStateForTests(),
-    ...internals.reflection.getMutableRuntimeStateForTests(),
-    ...internals.persona.getMutableRuntimeStateForTests(),
-    ...internals.workingMemory.getMutableRuntimeStateForTests(),
+    embedding: internals.embedding.getMutableRuntimeStateForTests(),
+    vectorStore: internals.vectorStore.getMutableRuntimeStateForTests(),
+    maintenance: internals.maintenance.getMutableRuntimeStateForTests(),
+    reflection: internals.reflection.getMutableRuntimeStateForTests(),
+    persona: internals.persona.getMutableRuntimeStateForTests(),
+    workingMemory: internals.workingMemory.getMutableRuntimeStateForTests(),
     warmEmbeddingConnection: (...)
   }
 }

This is a larger mechanical change touching every call site in this ~7000-line test file, so it's optional given the current lack of collisions.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/main/presenter/memoryPresenter.test.ts` around lines 57 - 123,
`memoryRuntimeForTests` currently flattens multiple
`getMutableRuntimeStateForTests()` results into one object with spreads, which
can silently shadow keys if any service adds a duplicate field. Update the seam
in `memoryPresenter.test.ts` to keep each service’s runtime state namespaced
under its service key (for example, embedding/vectorStore/maintenance/etc.)
instead of merging them into a single flat shape, and adjust any test helpers or
call sites that read those properties to use the namespaced access.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@test/main/presenter/memoryPresenter.test.ts`:
- Around line 57-123: `memoryRuntimeForTests` currently flattens multiple
`getMutableRuntimeStateForTests()` results into one object with spreads, which
can silently shadow keys if any service adds a duplicate field. Update the seam
in `memoryPresenter.test.ts` to keep each service’s runtime state namespaced
under its service key (for example, embedding/vectorStore/maintenance/etc.)
instead of merging them into a single flat shape, and adjust any test helpers or
call sites that read those properties to use the namespaced access.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 72c2fe94-2d67-4e0e-ba76-0b9740a97575

📥 Commits

Reviewing files that changed from the base of the PR and between c2540c6 and 4a37eab.

📒 Files selected for processing (5)

• docs/architecture/agent-memory-test-seams/plan.md
• docs/architecture/agent-memory-test-seams/spec.md
• docs/architecture/agent-memory-test-seams/tasks.md
• src/main/presenter/memoryPresenter/index.ts
• test/main/presenter/memoryPresenter.test.ts