Skip to content

Defer Typesense client setup for conversation search#7662

Open
tianmind-studio wants to merge 5 commits into
BasedHardware:mainfrom
tianmind-studio:codex/lazy-typesense-client
Open

Defer Typesense client setup for conversation search#7662
tianmind-studio wants to merge 5 commits into
BasedHardware:mainfrom
tianmind-studio:codex/lazy-typesense-client

Conversation

@tianmind-studio

@tianmind-studio tianmind-studio commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • lazily initialize the conversation-search Typesense client so importing backend routers does not require TYPESENSE_* env vars
  • validate missing Typesense config only when search is actually used, preserving config errors as RuntimeError
  • guard first-use initialization with a lock so concurrent callers share one client
  • add unit coverage for import-time safety, missing config, first-use construction, and concurrent initialization

Verification

  • python -m pytest backend\tests\unit\test_conversation_search_typesense_config.py -q --tb=short -> 4 passed
  • python -m py_compile backend\utils\conversations\search.py backend\tests\unit\test_conversation_search_typesense_config.py
  • python -m black --line-length 120 --skip-string-normalization --check backend\utils\conversations\search.py backend\tests\unit\test_conversation_search_typesense_config.py
  • PYTHONUTF8=1 python backend\scripts\scan_async_blockers.py -> no new blockers from this PR; existing backlog only
  • git diff --check origin/main...HEAD
  • scripts/pre-commit via Git for Windows sh.exe with the backend venv on PATH

Notes

  • Rebased onto current main (9ed12aea07).
  • Existing Greptile feedback remains addressed: first-use initialization is lock-guarded, missing config keeps its RuntimeError, and concurrent initialization coverage is included.

@greptile-apps

greptile-apps Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR defers Typesense client construction from module-import time to first use, so importing utils.conversations.search (and any router that pulls it in) no longer requires TYPESENSE_* environment variables to be present. Focused unit tests and a test.sh entry accompany the change.

  • _get_typesense_client() replaces the module-level typesense.Client(...) call with a lazy singleton, validating all three env vars only when search is first invoked.
  • Three new unit tests verify that import succeeds without env vars, that a missing TYPESENSE_API_KEY raises at call time, and that a successful first call constructs the client with the expected config.
  • The singleton is not guarded by a lock, so concurrent first-calls can create duplicate typesense.Client instances (minor, benign in CPython due to the GIL, but worth hardening). The existing broad except Exception catch also repackages the config RuntimeError as a generic Exception, which may complicate error-type-based handling upstream.

Confidence Score: 4/5

Safe to merge — the lazy-init change is self-contained, the tests cover the main scenarios, and the identified concerns are non-blocking quality improvements.

The core change is straightforward and achieves its goal. The singleton lacks a threading lock (benign in practice but not future-proof), and the broad exception handler silently changes the type of config errors, making them harder to distinguish from runtime search failures. Neither issue causes incorrect behavior today.

backend/utils/conversations/search.py — the singleton initialization pattern and exception-wrapping logic are the two spots worth a second look before this pattern is replicated elsewhere.

Important Files Changed

Filename Overview
backend/utils/conversations/search.py Replaces module-level Typesense client construction with a lazy singleton; _get_typesense_client() defers env-var validation to first use. The singleton is not guarded by a lock (minor race under concurrency), and config RuntimeError is swallowed by the pre-existing broad except block.
backend/tests/unit/test_conversation_search_typesense_config.py New unit tests cover import-time safety, missing-env-var error on first use, and successful client construction. Module isolation via sys.modules.pop is correct. Missing a test asserting that the second call reuses the cached client rather than constructing a new one.
backend/test.sh Adds the new test file to the CI test script in the correct position alongside similar unit tests; no issues.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant search_conversations
    participant _get_typesense_client
    participant typesense.Client

    Caller->>search_conversations: search_conversations(uid, query)
    search_conversations->>_get_typesense_client: _get_typesense_client()
    alt _typesense_client is None
        _get_typesense_client->>_get_typesense_client: _get_required_env_var(TYPESENSE_HOST)
        _get_typesense_client->>_get_typesense_client: _get_required_env_var(TYPESENSE_HOST_PORT)
        _get_typesense_client->>_get_typesense_client: _get_required_env_var(TYPESENSE_API_KEY)
        note over _get_typesense_client: raises RuntimeError if any var missing
        _get_typesense_client->>typesense.Client: Client(config)
        typesense.Client-->>_get_typesense_client: client instance
        _get_typesense_client->>_get_typesense_client: "_typesense_client = client (cached)"
    else _typesense_client already initialized
        note over _get_typesense_client: return cached singleton
    end
    _get_typesense_client-->>search_conversations: client
    search_conversations->>typesense.Client: collections['conversations'].documents.search(params)
    typesense.Client-->>search_conversations: results
    search_conversations-->>Caller: "{items, total_pages, current_page, per_page}"
Loading

Comments Outside Diff (1)

  1. backend/utils/conversations/search.py, line 87-88 (link)

    P2 Config RuntimeError is repackaged as a generic Exception

    When _get_required_env_var raises RuntimeError (missing env vars), the outer except Exception catch-all wraps it into a plain Exception("Failed to search conversations: ..."). This changes the exception type, so callers cannot use except RuntimeError to distinguish a misconfiguration from a network/search error. The tests pass because pytest.raises matches the substring, but any production error-handling code that inspects exception type will treat a missing TYPESENSE_API_KEY identically to a Typesense timeout. Letting the RuntimeError propagate uncaught (or re-raising it explicitly before the broad catch) would preserve the distinction.

Reviews (1): Last reviewed commit: "Defer Typesense client setup for convers..." | Re-trigger Greptile

Comment thread backend/utils/conversations/search.py Outdated

@kodjima33 kodjima33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lazy Typesense client init — backend refactor with tests, approve only per backend policy.

@tianmind-studio tianmind-studio force-pushed the codex/lazy-typesense-client branch from b3a0c51 to a02fc14 Compare June 8, 2026 13:45

Copy link
Copy Markdown
Contributor Author

I rebased and force-pushed this branch onto the current main. The latest commit includes the review follow-ups: lazy initialization is lock-guarded, missing config remains a RuntimeError, and there is a concurrent first-use test. I reran python -m pytest tests\unit\test_conversation_search_typesense_config.py -q: 4 passed.

@kodjima33 kodjima33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-approved on new sha: backend bug fix (lazy Typesense client init + concurrency lock) — approve only

@kodjima33 kodjima33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approved on new sha: lazy Typesense init — has merge conflicts, please rebase

@tianmind-studio tianmind-studio force-pushed the codex/lazy-typesense-client branch from 3bd28ca to 361a79c Compare June 12, 2026 07:13

@kodjima33 kodjima33 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lazy Typesense init w/ tests — re-approved on new sha

@tianmind-studio tianmind-studio force-pushed the codex/lazy-typesense-client branch 2 times, most recently from 7242fc7 to 703c6b6 Compare June 16, 2026 14:42
@tianmind-studio tianmind-studio force-pushed the codex/lazy-typesense-client branch from 703c6b6 to d266b5b Compare June 16, 2026 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants