Skip to content

chore(synapse): unify import error messages for missing AI dependencies#84

Open
IshanikaAurelia1106 wants to merge 1 commit into
MarcoPorcellato:mainfrom
IshanikaAurelia1106:chore-english-import-errors
Open

chore(synapse): unify import error messages for missing AI dependencies#84
IshanikaAurelia1106 wants to merge 1 commit into
MarcoPorcellato:mainfrom
IshanikaAurelia1106:chore-english-import-errors

Conversation

@IshanikaAurelia1106

@IshanikaAurelia1106 IshanikaAurelia1106 commented Jul 2, 2026

Copy link
Copy Markdown

This PR improves and unifies import error messages when AI-related dependencies are missing.

Previously, different modules raised inconsistent ImportError messages, making debugging harder.

This PR standardizes those messages to provide a clearer and more user-friendly explanation when optional AI dependencies are not installed.

Before / After Example

Before:
ModuleNotFoundError: No module named 'xyz'

After:
AI dependency missing: please install optional package 'xyz'

Type of change

  • Bug fix
  • New feature (non-breaking change which adds functionality)
  • Breaking change
  • Documentation update

Checklist

  • I have tested my changes locally
  • My code follows the project's style guidelines
  • I have not introduced breaking changes

@IshanikaAurelia1106

Copy link
Copy Markdown
Author

This PR standardizes ImportError messages for missing AI dependencies to improve clarity and debugging consistency. Happy to adjust wording if needed.

@MarcoPorcellato

Copy link
Copy Markdown
Owner

Hi @IshanikaAurelia1106 !

Thanks for your PR and welcome to "Logseq Matryca Parser"

I'm studying your contribution and I'll keep you updated.

Marco

@MarcoPorcellato

Copy link
Copy Markdown
Owner

Hi @IshanikaAurelia1106 — thank you for opening this PR and welcome to the project!

Unifying the SYNAPSE ImportError messages in English and pointing users to uv sync --extra ai is a sensible improvement. The Italian strings in synapse.py were inconsistent with the rest of the codebase (including the KINETIC CLI), and the old LlamaIndex hint referenced llama-index while our [ai] extra installs llama-index-core.

CI is currently failing (3 tests in tests/test_synapse.py). The production change is fine at runtime, but the test suite still expects the previous message text (match="LangChain" / match="LlamaIndex"). Please update those assertions to match the new wording (or assert against a shared module constant — see below).

Before this is merge-ready, could you please:

  1. Fix the tests — update:

    • test_to_langchain_documents_raises_when_dependency_missing
    • test_to_llamaindex_nodes_raises_when_dependency_missing
    • test_to_context_enriched_chunks_raises_when_dependency_missing
      in tests/test_synapse.py (e.g. match="Missing AI export dependencies").
  2. DRY the message — extract a single module-level constant in synapse.py (e.g. _MISSING_AI_EXPORT_DEPS_MSG) and reuse it in all three raise ImportError(...) sites instead of repeating the same string.

  3. Trim unrelated diff noise — revert or avoid the extra Ruff line-wrapping changes that are not part of this chore (e.g. LangChainVisitor, frozenset, etc.). A focused PR is easier to review.

  4. CHANGELOG — add a short bullet under ## [Unreleased]### Changed in CHANGELOG.md (user-visible error message improvement).

  5. Align wording with KINETIC (optional but recommended)kinetic.py already prints "Missing AI export dependencies. Please install them using: uv sync --extra ai" when it catches this error. Consider matching that phrasing in the library message (without Rich markup) so "unify" is true end-to-end.

  6. PR description — this is a chore/DX change, not a bug fix. The body mentions ModuleNotFoundError and an example string that does not match the actual message; please update the description to reflect what the code really does.

  7. Verify locally — run make all and ensure CI is green before the next push.

Branch hygiene: one PR per concern is perfect here — please keep this scoped to SYNAPSE import errors only.

Happy to re-review once CI is green. Thanks again for contributing!

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