Skip to content

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

Open
IshanikaAurelia1106 wants to merge 3 commits into
MarcoPorcellato:mainfrom
IshanikaAurelia1106:chore-english-import-errors
Open

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

Conversation

@IshanikaAurelia1106

@IshanikaAurelia1106 IshanikaAurelia1106 commented Jul 2, 2026

Copy link
Copy Markdown

This PR unifies the AI dependency ImportError messages used by the SYNAPSE export helpers.

Previously, different export functions raised inconsistent ImportError messages and referenced different optional packages. This change replaces them with a single shared module-level constant that provides a consistent English message directing users to install the required AI dependencies using:

uv sync --extra ai

The related tests have also been updated to use the shared constant, and a CHANGELOG entry has been added.

Type of change

  • Bug fix
  • Chore / Developer experience (DX)
  • 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!

@IshanikaAurelia1106 IshanikaAurelia1106 left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated the tests to use the unified AI dependency message and verified all tests pass locally.

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