Refactor Qdrant client factory to resolve circular dependencies

[google-labs-jules]

This change extracts the Qdrant client factory logic into a dedicated module src/domains/memory/vector/client_factory.py. This breaks a circular dependency cycle where qdrant.py imported tools, which in turn imported the client factory from the same module (or a related one causing the cycle).

The original locations src/domains/memory/qdrant_provider.py and src/domains/memory/vector/qdrant/domains/qdrant_provider.py have been retained as deprecated shims to maintain backward compatibility for any untracked consumers, but all internal usages have been updated to use the new factory directly.

This architectural change aligns with the "Sovereign Architect" directive for robust and decoupled code.

---

PR created automatically by Jules for task 6831929615835677485 started by @Giftedx

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

---

For security, I will only act on instructions from the user who triggered this task.

New to Jules? Learn more at jules.google/docs.

[github-actions]

🤖 Hi @google-labs-jules[bot], I've received your request, and I'm working on it now! You can track my progress in the logs for more details.

[github-actions]

🤖 I'm sorry @google-labs-jules[bot], but I was unable to process your request. Please see the logs for more details.

[claude]

PR Review: Refactor Qdrant Client Factory

Overview

This refactoring successfully extracts the Qdrant client factory logic into a dedicated module to resolve circular dependencies. The approach is sound and aligns with the project's architectural principles.

---

✅ Strengths

1. Architecture & Design

• Clean Separation: Moving get_qdrant_client() to src/domains/memory/vector/client_factory.py is the right architectural choice
• Circular Dependency Resolution: Successfully breaks the import cycle between qdrant.py and tools
• Backward Compatibility: Deprecation shims maintain compatibility for external consumers
• Consistent with CLAUDE.md: Aligns with the "Sovereign Architect" directive for robust and decoupled code

2. Code Quality

• Proper use of deprecation warnings in shim files with clear migration path
• TYPE_CHECKING guard for proper type annotation without runtime overhead
• Good fallback mechanisms with _DummyClient for testing environments
• Comprehensive _DummyClient implementation supporting multiple Qdrant operations

3. Migration Coverage

All 18 import sites have been updated consistently:

• src/domains/memory/hybrid_retriever.py
• src/domains/memory/unified_graph_store.py
• src/domains/memory/vector/memory_*.py tools
• And 15 other files

---

⚠️ Issues & Concerns

1. Unused Pool Configuration Variables (Medium Priority)

Location: client_factory.py:225-228

_pool_size = int(os.getenv("QDRANT_POOL_SIZE", "10"))
_max_overflow = int(os.getenv("QDRANT_MAX_OVERFLOW", "5"))
_pool_timeout = int(os.getenv("QDRANT_POOL_TIMEOUT", "30"))
_pool_recycle = int(os.getenv("QDRANT_POOL_RECYCLE", "3600"))

Problem: These environment variables are read but never used. The QdrantClient constructor doesn't accept these parameters.

Impact:

• Misleading configuration - users might set these expecting them to work
• Code maintenance burden
• Missing from .env.example (proper environment-driven config per CLAUDE.md)

Recommendation: Either:

1. Remove these unused variables, OR
2. Implement actual connection pooling using these parameters, OR
3. Document why they're reserved for future use and add to .env.example

2. Test Import Path Not Updated (High Priority)

Location: tests/unit/core/test_memory_dummy_qdrant.py:1

from memory.qdrant_provider import _DummyClient, get_qdrant_client

Problem: Test still imports from old memory.qdrant_provider instead of new domains.memory.vector.client_factory

Impact:

• Tests may be exercising the deprecated shim rather than the actual implementation
• Test coverage metrics may be misleading
• Future removal of shims will break tests

Recommendation: Update to:

from domains.memory.vector.client_factory import _DummyClient, get_qdrant_client

3. RuntimeError Documentation Mismatch (Low Priority)

Location: client_factory.py:214

Raises
------
RuntimeError
    If the optional dependency *qdrant-client* is not installed at runtime.

Problem: The code doesn't raise RuntimeError - it gracefully returns _DummyClient when qdrant_client is missing

Recommendation: Update docstring to reflect actual behavior:

Returns
-------
QdrantClient | _DummyClient
    Real QdrantClient if available and configured, otherwise _DummyClient for testing

4. Import Organization (Low Priority)

Location: client_factory.py:20

The import os is separated from other stdlib imports (lines 16-19). Per PEP 8, it should be grouped with other standard library imports at the top.

---

🧪 Test Coverage Concerns

1. Missing Export Test: No test verifies that _DummyClient is properly exported in __all__
2. Deprecation Warning Test: No test validates that the deprecation warnings are triggered correctly
3. Secure Fallback Logic: The ENABLE_SECURE_QDRANT_FALLBACK path at line 246-250 appears untested

---

🔒 Security Review

✅ No security concerns identified:

• Proper API key handling via settings
• No hardcoded credentials
• Graceful exception handling doesn't leak sensitive info

---

⚡ Performance Considerations

Positive:

• @lru_cache ensures singleton client reuse (good for connection pooling)
• Lazy imports reduce startup time when Qdrant isn't needed

Neutral:

• The unused pool configuration variables suggest incomplete connection pool implementation

---

📋 Recommendations Summary

Must Fix (Before Merge)

1. ✅ Update test import in test_memory_dummy_qdrant.py

Should Fix (This PR or Next)

2. ⚠️ Clarify pool configuration variables - use them, remove them, or document them
3. ⚠️ Fix docstring to match actual exception behavior

Nice to Have

4. 📝 Group import os with other stdlib imports
5. 📝 Add test coverage for deprecation warnings
6. 📝 Add pool configuration to .env.example if they're intended for future use

---

✨ Final Assessment

Overall: ✅ Approve with requested changes

This is a well-executed refactoring that successfully resolves the circular dependency issue. The backward compatibility approach is commendable. The main concerns are:

1. The broken test import (must fix)
2. Unused configuration variables creating confusion (should address)

Once the test import is updated, this PR will be ready to merge. The pool configuration issue can be addressed in a follow-up if time is tight.

Diff Impact: +300/-519 lines (net -219) - good consolidation!

---

Review generated in accordance with CLAUDE.md guidelines