refactor(mcp): extract shared persistence helper for entity writes

[gaodan-fang]

Summary

• Add _persist_entities helper in altk_evolve/frontend/mcp/mcp_server.py that centralizes update_entities plus the NamespaceNotFoundException → evict → re-resolve → retry pattern.
• Refactor store_user_facts, save_trajectory, and create_entity to call the helper, removing three copies of the retry block.
• New entity-writing tools can now reuse the helper for consistent error handling, as requested in refactor: extract shared persistence helper for entity writes #241.

Test plan

• pytest tests/unit/test_mcp_server.py — 20/20 passed
• pytest tests/unit/test_entity_sharing.py — 14/14 passed
• ruff check clean
• mypy clean (pre-commit hook)

Closes #241

Summary by CodeRabbit

• Refactor
  • Unified entity persistence logic with enhanced namespace-cache retry behavior for improved robustness across user facts, trajectories, and entity creation operations.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR extracts a shared _persist_entities() helper in mcp_server.py that centralizes namespace resolution, entity writing, and automatic retry on stale namespace cache. The helper is then integrated into three callers: store_user_facts, save_trajectory, and create_entity, eliminating duplication of retry-and-evict logic.

Changes

Entity Persistence Refactor

Layer / File(s)	Summary
Namespace-persist helper contract and implementation altk_evolve/frontend/mcp/mcp_server.py	New EntityUpdate import and _persist_entities(namespace_id, entities, enable_conflict_resolution) helper that resolves the effective namespace, writes entities via update_entities, and on NamespaceNotFoundException evicts the namespace cache, re-resolves, and retries the write once; returns both the entity update records and the resolved namespace string.
Integrate helper into all entity-write callers altk_evolve/frontend/mcp/mcp_server.py	store_user_facts removes prior direct namespace resolution and replaces manual update_entities+retry with _persist_entities(namespace_id=None). save_trajectory replaces its manual retry wrapper with _persist_entities(namespace_id). create_entity adds explicit logger.info to log creation intent and provided namespace, then replaces inline update_entities+retry with _persist_entities() for consistent behavior.

Sequence Diagram

No sequence diagram; this is a refactoring that consolidates inline retry logic into a helper without changing the observable data flow or component interactions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• AgentToolkit/altk-evolve#252: Both PRs modify the user-facts persistence path and namespace cache retry logic in mcp_server.py; this PR's new _persist_entities helper refactors the inline retry pattern that was introduced or adjusted in the earlier PR.

Suggested reviewers

• vinodmut
• illeatmyhat
• visahak

Poem

🐰 A tangled thicket of retries tamed,
Three paths unified, namespaces reclaimed—
One helper born to spare us from the pain,
Of catching, evicting, then trying again! ✨

🚥 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 'refactor(mcp): extract shared persistence helper for entity writes' directly summarizes the main change—extracting a new _persist_entities helper to centralize entity write logic.
Linked Issues check	✅ Passed	The PR fully implements the coding requirements from issue #241: extracting _persist_entities helper that centralizes entity persistence and error handling, refactoring store_user_facts and create_entity to eliminate duplication.
Out of Scope Changes check	✅ Passed	All changes are scoped to the entity persistence refactoring objective; the addition of namespace logging in create_entity is minimal and directly supports the refactoring goal.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 issue-241

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

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 and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

altk_evolve/frontend/mcp/mcp_server.py (1)

510-548: ⚡ Quick win

Guideline write still bypasses the new helper — inconsistent retry behavior within the same tool.

The trajectory write now goes through _persist_entities, but the guideline write a few lines below at 543-548 is still a raw get_client().update_entities(...) call with no namespace-eviction/retry path. If the namespace is evicted (or deleted externally) between the trajectory write and the guideline write, this second call will raise NamespaceNotFoundException and bubble up unhandled, defeating the consistency this PR is trying to establish. Since the helper already supports enable_conflict_resolution=True, the swap is mechanical.

♻️ Proposed fix to route the guideline write through `_persist_entities`

     if guideline_entities:
-        get_client().update_entities(
-            namespace_id=resolved_ns,
-            entities=guideline_entities,
-            enable_conflict_resolution=True,
-        )
+        _, resolved_ns = _persist_entities(
+            namespace_id=resolved_ns,
+            entities=guideline_entities,
+            enable_conflict_resolution=True,
+        )

🤖 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 `@altk_evolve/frontend/mcp/mcp_server.py` around lines 510 - 548, The guideline
write currently calls get_client().update_entities(...) directly which bypasses
the retry/namespace-eviction handling in _persist_entities; replace that raw
update with a call into _persist_entities using the same resolved_ns and
guideline_entities and enable_conflict_resolution=True (e.g., call
_persist_entities(namespace_id=resolved_ns, entities=guideline_entities,
enable_conflict_resolution=True) and ignore/handle its returned (namespace,
resolved_ns) tuple as done earlier) so guideline writes get the same
eviction/retry behavior as the trajectory write.

🤖 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 `@altk_evolve/frontend/mcp/mcp_server.py`:
- Around line 510-548: The guideline write currently calls
get_client().update_entities(...) directly which bypasses the
retry/namespace-eviction handling in _persist_entities; replace that raw update
with a call into _persist_entities using the same resolved_ns and
guideline_entities and enable_conflict_resolution=True (e.g., call
_persist_entities(namespace_id=resolved_ns, entities=guideline_entities,
enable_conflict_resolution=True) and ignore/handle its returned (namespace,
resolved_ns) tuple as done earlier) so guideline writes get the same
eviction/retry behavior as the trajectory write.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8c8e0edf-418e-455f-a0d3-ee395e1b7867

📥 Commits

Reviewing files that changed from the base of the PR and between 003bd53 and ab99aed.

📒 Files selected for processing (1)

• altk_evolve/frontend/mcp/mcp_server.py