Duplicate URL→backend detection in persistence doctor diverges from canonical resolver

[MervinPraison]

Summary

The persistence doctor CLI helpers in the wrapper hand-roll their own URL→backend detection instead of reusing the canonical, tested PraisonAIDB._detect_backend. The two implementations have already drifted, so the doctor mislabels (and probes with the wrong client) several backends that the live path resolves correctly. This is a structural duplication / maintenance-drift issue in the wrapper — not a feature cut.

Current behaviour

Three doctor helpers each inline substring-based scheme sniffing:

• src/praisonai/praisonai/cli/features/persistence.py:571-578 (_test_conversation_store) — postgres / mysql / sqlite, unknown → sqlite.
• src/praisonai/praisonai/cli/features/persistence.py:597-602 (_test_knowledge_store) — qdrant / chroma, unknown → qdrant.
• src/praisonai/praisonai/cli/features/persistence.py:621-624 (_test_state_store) — redis, unknown → memory.

The canonical single source of truth is PraisonAIDB._detect_backend at src/praisonai/praisonai/db/adapter.py:280-325, which the live persistence path uses and which is covered directly by tests (tests/unit/test_serverless_postgres.py, tests/unit/test_turso_store.py). It recognises schemes the doctor copies do not — e.g. libsql:// → turso, *.supabase.co → supabase, weaviate — and fails loudly on an unrecognised scheme rather than guessing.

Because the copies use loose substring checks with a silent default, e.g. a libsql://… conversation URL is reported as sqlite and a *.supabase.co URL falls through to the sqlite default, so persistence doctor probes the wrong store client and can report a misleading result for exactly the cloud/serverless backends _detect_backend was extended to support.

Why it matters

• Duplication / maintenance drift: backend-detection rules now live in two places; the canonical resolver has already gained libsql/supabase/weaviate support the doctor copies never received. Any future backend must be added in both, and the doctor silently lags.
• Diagnostic accuracy: the doctor is a trust surface for catching real misconfigurations; today it can mislabel or wrongly probe valid cloud backends, undermining the very reassurance it exists to give.

This is not merely "could be simpler" — there is a concrete, tested canonical owner and observable divergence.

Category

Duplicate

Capability preserved

• praisonai persistence doctor continues to probe all three store kinds (conversation, knowledge, state) with the same output shape and connectivity test.
• No backend, store, or doctor check is removed.
• The doctor's existing lenient defaults (memory for state, and the chroma/qdrant knowledge distinction that _detect_backend does not model) are retained via a CLI-local fallback.

Proposed approach

Route the recognised schemes through the canonical _detect_backend inside a try/except ValueError, keeping the doctor-specific fallbacks only in the except branch. Merge to one owner; keep behaviour. No public API or CLI surface change.

Resolution sketch

# Before (src/praisonai/praisonai/cli/features/persistence.py)
def _test_conversation_store(url: str) -> tuple:
    try:
        from praisonai.persistence.factory import create_conversation_store
        if "postgresql" in url or "postgres" in url:
            backend = "postgres"
        elif "mysql" in url:
            backend = "mysql"
        elif url.endswith(".db") or "sqlite" in url:
            backend = "sqlite"
        else:
            backend = "sqlite"
        store = create_conversation_store(backend, url=url)
        ...

# After — delegate to the canonical resolver, keep the lenient fallback
def _detect_store_backend(url: str, default: str) -> str:
    """Reuse the single canonical resolver; fall back leniently for the doctor."""
    from praisonai.db.adapter import PraisonAIDB
    try:
        return PraisonAIDB._detect_backend(PraisonAIDB, url)  # or a module-level helper
    except ValueError:
        return default

def _test_conversation_store(url: str) -> tuple:
    try:
        from praisonai.persistence.factory import create_conversation_store
        backend = _detect_store_backend(url, default="sqlite")
        store = create_conversation_store(backend, url=url)
        ...

_test_knowledge_store keeps its chroma special-case (unknown → qdrant) and _test_state_store keeps memory as its except default, since those two buckets model store kinds the unified resolver does not. If sharing _detect_backend as an instance method is awkward, the minimal alternative is to lift its scheme table into one module-level helper that both db/adapter.py and the doctor import.

Layer placement

• Primary layer: wrapper (praisonai)
• Touches core/tools/plugins: none — both sites are in the wrapper.
• 3-way surface (CLI + YAML + Python): preserved — CLI diagnostic only; no change to Python/YAML behaviour.

Severity

Low — diagnostic accuracy and maintenance drift on an opt-in doctor path; no hot-path or runtime-behaviour impact.

Validation

• Traced both implementations: doctor copies at persistence.py:571-578/597-602/621-624; canonical resolver at db/adapter.py:280-325 (tested by test_serverless_postgres.py / test_turso_store.py).
• Divergence is concrete: libsql:// and *.supabase.co are handled by the canonical resolver but silently defaulted by the doctor copies.
• The proposed fix consolidates behind the tested owner while preserving the doctor's lenient fallbacks, so there is no user-facing regression.

Keep unchanged

• The connectivity probes themselves (list_sessions, collection_exists, set/get/delete) and their output strings.
• The chroma/qdrant knowledge-store distinction and the memory state-store default (retained as CLI-local fallbacks).
• PraisonAIDB._detect_backend's fail-loud contract on the live path — the lenient behaviour stays confined to the doctor.
• All other doctor checks and the persistence factory/registry paths.

[praisonai-triage-agent]

Claude finished @MervinPraison's task —— View job • claude/issue-4523-20260827-1615 • [Create PR ➔](https://github.com/MervinPraison/PraisonAI/compare/main...claude/issue-4523-20260827-1615?quick_pull=1&title=Issue%20%234523%3A%20Changes%20from%20Claude&body=This%20PR%20addresses%20issue%20%234523%0A%0AGenerated%20with%20%5BClaude%20Code%5D(https%3A%2F%2Fclaude.ai%2Fcode)

---

Fixing duplicate URL→backend detection in persistence doctor

• Read AGENTS.md and relevant files
• Understand canonical _detect_backend and doctor copies
• Implement minimal fix delegating to canonical resolver
• Test the changes
• Commit, push, and create PR

PR created: #4524

Summary

Consolidated the persistence doctor's hand-rolled URL→backend detection behind the canonical, tested PraisonAIDB._detect_backend.

• Added a CLI-local _detect_store_backend(url, default) that delegates to the canonical resolver in a try/except, keeping the doctor's lenient fallbacks (sqlite / qdrant / memory) only in the except branch.
• _test_knowledge_store keeps its chroma special-case; _test_state_store keeps memory.
• No public API, CLI surface, or probe-behaviour change.

The doctor now recognises the same schemes as the live path — libsql:// → turso and *.supabase.co → supabase (previously silently mislabeled as sqlite), plus weaviate.

Validation: verified detection across libsql, supabase, postgres, mysql, sqlite, redis, qdrant, weaviate, and unknown-scheme fallbacks — all resolve as expected.
• Branch: claude/issue-4523-20260827-1615