Refine optional provider handling and expand CI coverage #6
Conversation
WalkthroughRefactors search orchestration to use SearchService, adds AddEpisodePayload and payload-based add flow, introduces FastAPI app-scoped GraphiumClient lifecycle and request-based dependency, improves optional-provider handling with missing-provider shims and clearer import errors, enhances LLM settings resolution, adds pre-commit (ruff), broadens CI triggers and test guards, and updates many tests to skip when optional SDKs are absent. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as HTTP Client
participant FastAPI as FastAPI App
participant Lifespan as Lifespan Context
participant State as app.state
participant Graphium as GraphiumClient
rect rgb(235, 245, 255)
note over FastAPI,Lifespan: App startup
FastAPI->>Lifespan: enter lifespan(app)
Lifespan->>Graphium: construct GraphiumClient(settings...)
Graphium-->>Lifespan: instance
Lifespan->>State: state.graphium_client = Graphium
Lifespan-->>FastAPI: yield
end
rect rgb(240, 255, 240)
note over Client,FastAPI: Request handling
Client->>FastAPI: Route call
FastAPI->>State: get graphium_client
State-->>FastAPI: GraphiumClient
FastAPI->>Graphium: perform operation (search / add_episode / ingest)
Graphium-->>FastAPI: result
FastAPI-->>Client: response
end
rect rgb(255, 240, 240)
note over FastAPI,Lifespan: App shutdown
FastAPI->>Lifespan: exit lifespan
Lifespan->>State: get graphium_client
Lifespan->>Graphium: close()
Graphium-->>Lifespan: closed
end
sequenceDiagram
autonumber
participant Caller as API/Service
participant Graphium as Graphium
participant Payload as AddEpisodePayload
participant Orchestrator as EpisodeOrchestrator
Caller->>Graphium: add_episode(args...)
Graphium->>Payload: construct AddEpisodePayload from args
Graphium->>Graphium: add_episode_payload(payload)
Graphium->>Orchestrator: orchestrator.add_episode(payload fields...)
Orchestrator-->>Graphium: AddEpisodeResults
Graphium-->>Caller: AddEpisodeResults
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas to focus review on:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
graphium_core/embedder/providers/__init__.py (1)
8-17: Extract_missing_providerto eliminate duplication.This is identical to the
_missing_providerhelper ingraphium_core/cross_encoder/rerankers/__init__.py(lines 9-18). Please see my comment on that file for the suggested refactor to a shared utility module.
🧹 Nitpick comments (4)
conftest.py (1)
58-63: LGTM! The unusedconfigparameter is expected for pytest hooks.The static analysis hint (ARG001) is a false positive. The
configparameter is part of the standardpytest_collection_modifyitemshook signature and must be present even if unused. Pytest uses introspection to call hooks, so the signature must match the expected interface.The hook logic correctly provides a fallback for unit tests with async behavior when pytest-asyncio is unavailable, complementing the stricter requirement in
tests/integration/__init__.py.
Optional refinement: If you prefer to eliminate the linter warning, you could omit
configfrom the signature entirely—pytest's hook system tolerates a subset of parameters. However, including it documents the full hook interface, so the current approach is perfectly acceptable.graphium_core/cross_encoder/rerankers/gemini.py (1)
14-22: Simplify the typing.cast pattern for clarity.The pattern
typing.cast(typing.Any, None)is unusual and potentially confusing. When the import fails, these variables are simplyNoneat runtime, and castingNonetoAnydoesn't provide meaningful type information.Consider these alternatives for cleaner type hints:
Option 1: Use direct None assignment
if TYPE_CHECKING: # pragma: no cover - typing only try: from google import genai as genai # type: ignore[import-not-found] from google.genai import types as types # type: ignore[import-not-found] except ImportError: # pragma: no cover - optional dependency missing at type time - genai = typing.cast(typing.Any, None) - types = typing.cast(typing.Any, None) + genai = None # type: ignore[assignment] + types = None # type: ignore[assignment] else: # pragma: no cover - runtime placeholders - genai = typing.cast(typing.Any, None) - types = typing.cast(typing.Any, None) + genai = None + types = NoneOption 2: Use a proper typing.Any annotation
if TYPE_CHECKING: # pragma: no cover - typing only try: from google import genai as genai # type: ignore[import-not-found] from google.genai import types as types # type: ignore[import-not-found] except ImportError: # pragma: no cover - optional dependency missing at type time - genai = typing.cast(typing.Any, None) - types = typing.cast(typing.Any, None) + genai: typing.Any = None + types: typing.Any = None else: # pragma: no cover - runtime placeholders - genai = typing.cast(typing.Any, None) - types = typing.cast(typing.Any, None) + genai: typing.Any = None + types: typing.Any = NoneBoth options are clearer than using
cast(Any, None), which conflates the concepts of casting and type annotation.server/graph_service/main.py (1)
14-18: Consider error handling for client initialization.If
GraphiumClientconstruction orbuild_indices_and_constraintsfails during startup, the application will fail to start. While this is appropriate for critical initialization, consider whether explicit error logging or a more informative startup failure message would help operators diagnose issues.ENGINEERING_PLAN.md (1)
1-150: Well-structured engineering plan with clear milestones.The document provides an excellent roadmap for the refactoring efforts, with:
- Clear goals and acceptance criteria for each milestone
- Concrete code examples for implementation
- Progress tracking via task checkboxes
- Validation steps to ensure quality
The plan aligns well with the actual code changes in this PR (Milestones 2-5) and provides valuable context for future work.
Consider adding a note about the required extras (e.g.,
[openai],[gemini]) needed to run the full test suite, as mentioned in Milestone 4's validation section.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (37)
.github/workflows/ci.yml(2 hunks).pre-commit-config.yaml(1 hunks)CODE_REVIEW.md(1 hunks)ENGINEERING_PLAN.md(1 hunks)conftest.py(2 hunks)docs/reports/code-complexity.md(1 hunks)graphium_core/cross_encoder/rerankers/__init__.py(1 hunks)graphium_core/cross_encoder/rerankers/gemini.py(1 hunks)graphium_core/embedder/providers/__init__.py(1 hunks)graphium_core/graphium.py(6 hunks)graphium_core/llm_client/pydantic_ai_adapter.py(1 hunks)graphium_core/orchestration/__init__.py(0 hunks)graphium_core/orchestration/payloads.py(1 hunks)graphium_core/orchestration/search_orchestrator.py(0 hunks)graphium_core/providers/factory.py(3 hunks)graphium_core/settings.py(2 hunks)mcp_server/graphium_mcp/state.py(1 hunks)mcp_server/graphium_mcp/tools.py(1 hunks)mcp_server/tests/test_health_endpoint.py(1 hunks)mcp_server/tests/test_server_smoke.py(1 hunks)mcp_server/tests/test_streamable_http.py(1 hunks)pyproject.toml(3 hunks)server/graph_service/graphium_client.py(3 hunks)server/graph_service/main.py(1 hunks)server/graph_service/routers/ingest.py(4 hunks)server/graph_service/routers/retrieve.py(3 hunks)tests/integration/__init__.py(1 hunks)tests/integration/conftest.py(0 hunks)tests/unit/cross_encoder/test_bge_reranker_client.py(1 hunks)tests/unit/cross_encoder/test_gemini_reranker_client.py(1 hunks)tests/unit/embedder/test_embeddinggemma.py(1 hunks)tests/unit/embedder/test_openai.py(1 hunks)tests/unit/embedder/test_voyage.py(1 hunks)tests/unit/graphium/test_add_episode_payload.py(1 hunks)tests/unit/mcp/test_episode_queue.py(1 hunks)tests/unit/providers/test_factory.py(1 hunks)tests/unit/settings/test_llm_settings.py(1 hunks)
💤 Files with no reviewable changes (3)
- tests/integration/conftest.py
- graphium_core/orchestration/search_orchestrator.py
- graphium_core/orchestration/init.py
🧰 Additional context used
📓 Path-based instructions (15)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use 4-space indentation, 100-character line length, and prefer single quotes as configured in pyproject.toml
Modules, files, and functions should use snake_case names
Only use# type: ignoreas a last resort and include a justification comment
Files:
tests/unit/embedder/test_voyage.pytests/unit/graphium/test_add_episode_payload.pygraphium_core/providers/factory.pymcp_server/graphium_mcp/state.pytests/unit/settings/test_llm_settings.pygraphium_core/cross_encoder/rerankers/__init__.pygraphium_core/cross_encoder/rerankers/gemini.pyserver/graph_service/main.pytests/unit/cross_encoder/test_bge_reranker_client.pyserver/graph_service/graphium_client.pytests/unit/mcp/test_episode_queue.pytests/unit/providers/test_factory.pytests/unit/embedder/test_embeddinggemma.pytests/integration/__init__.pygraphium_core/llm_client/pydantic_ai_adapter.pygraphium_core/graphium.pyserver/graph_service/routers/retrieve.pymcp_server/tests/test_streamable_http.pygraphium_core/embedder/providers/__init__.pytests/unit/cross_encoder/test_gemini_reranker_client.pymcp_server/graphium_mcp/tools.pymcp_server/tests/test_health_endpoint.pygraphium_core/settings.pytests/unit/embedder/test_openai.pyserver/graph_service/routers/ingest.pymcp_server/tests/test_server_smoke.pygraphium_core/orchestration/payloads.pyconftest.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Name test functions as test_
Use @pytest.mark.integration for database-reliant tests so CI can gate them
Prefer @pytest.mark.parametrize over manual loops for clearer, isolated assertions
Seed random fixtures so test runs are deterministic
Reuse shared constants (e.g., GROUP_ID) instead of hard-coding values in tests
Register fixtures viapytest_plugins = [...]rather than importing solely for side effects
Remove unnecessary noqa directives once unused imports are gone
For provider-specific integration fixtures, parameterize with sentinels and skip inside the fixture so CI reports a skip when services are disabled
Files:
tests/unit/embedder/test_voyage.pytests/unit/graphium/test_add_episode_payload.pytests/unit/settings/test_llm_settings.pytests/unit/cross_encoder/test_bge_reranker_client.pytests/unit/mcp/test_episode_queue.pytests/unit/providers/test_factory.pytests/unit/embedder/test_embeddinggemma.pytests/integration/__init__.pytests/unit/cross_encoder/test_gemini_reranker_client.pytests/unit/embedder/test_openai.py
{server/**/*.py,graphium_core/**/*.py}
📄 CodeRabbit inference engine (AGENTS.md)
Use ruff for formatting/lint and pyright for type checks (
make lint), with pyright standard mode in server and basic in core
Files:
graphium_core/providers/factory.pygraphium_core/cross_encoder/rerankers/__init__.pygraphium_core/cross_encoder/rerankers/gemini.pyserver/graph_service/main.pyserver/graph_service/graphium_client.pygraphium_core/llm_client/pydantic_ai_adapter.pygraphium_core/graphium.pyserver/graph_service/routers/retrieve.pygraphium_core/embedder/providers/__init__.pygraphium_core/settings.pyserver/graph_service/routers/ingest.pygraphium_core/orchestration/payloads.py
graphium_core/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
graphium_core/**/*.py: In graphium_core use absolute imports (e.g., from graphium_core.prompts.models import Message) instead of deep relative imports
Check for None (e.g.,driver is None) before use to satisfy type-narrowing and avoid possibly-unbound warnings
Files:
graphium_core/providers/factory.pygraphium_core/cross_encoder/rerankers/__init__.pygraphium_core/cross_encoder/rerankers/gemini.pygraphium_core/llm_client/pydantic_ai_adapter.pygraphium_core/graphium.pygraphium_core/embedder/providers/__init__.pygraphium_core/settings.pygraphium_core/orchestration/payloads.py
{graphium_core/**/*.py,server/**/*.py,mcp_server/**/*.py}
📄 CodeRabbit inference engine (AGENTS.md)
Import the public facade via
from graphium_core.graphium import Graphiuminstead of importing from graphium_core.init
Files:
graphium_core/providers/factory.pymcp_server/graphium_mcp/state.pygraphium_core/cross_encoder/rerankers/__init__.pygraphium_core/cross_encoder/rerankers/gemini.pyserver/graph_service/main.pyserver/graph_service/graphium_client.pygraphium_core/llm_client/pydantic_ai_adapter.pygraphium_core/graphium.pyserver/graph_service/routers/retrieve.pymcp_server/tests/test_streamable_http.pygraphium_core/embedder/providers/__init__.pymcp_server/graphium_mcp/tools.pymcp_server/tests/test_health_endpoint.pygraphium_core/settings.pyserver/graph_service/routers/ingest.pymcp_server/tests/test_server_smoke.pygraphium_core/orchestration/payloads.py
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Tag fenced Markdown code blocks with an explicit language (
text,ini, etc.) to satisfy linting
Files:
docs/reports/code-complexity.mdENGINEERING_PLAN.mdCODE_REVIEW.md
mcp_server/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
mcp_server/**/*.py: Avoid static state bindings in the MCP server; import runtime state dynamically
Use Literal types for transport strings ("streamable-http"|"http"|"stdio") in MCP-related code
MCP server code should live under mcp_server/ with Docker compose service defined there
For MCP server state, import asfrom . import stateand access attributes (e.g., state.graphium_client) rather than binding names from state
Files:
mcp_server/graphium_mcp/state.pymcp_server/tests/test_streamable_http.pymcp_server/graphium_mcp/tools.pymcp_server/tests/test_health_endpoint.pymcp_server/tests/test_server_smoke.py
{graphium_core/settings.py,server/**/*.py,mcp_server/**/*.py}
📄 CodeRabbit inference engine (AGENTS.md)
Document defaulted environment flags and let caller-provided values take precedence
Files:
mcp_server/graphium_mcp/state.pyserver/graph_service/main.pyserver/graph_service/graphium_client.pyserver/graph_service/routers/retrieve.pymcp_server/tests/test_streamable_http.pymcp_server/graphium_mcp/tools.pymcp_server/tests/test_health_endpoint.pygraphium_core/settings.pyserver/graph_service/routers/ingest.pymcp_server/tests/test_server_smoke.py
{server/**/*.py,mcp_server/**/*.py}
📄 CodeRabbit inference engine (AGENTS.md)
Mirror core environment requirements in services: set LLM_PROVIDER and database credentials for server and MCP
Files:
mcp_server/graphium_mcp/state.pyserver/graph_service/main.pyserver/graph_service/graphium_client.pyserver/graph_service/routers/retrieve.pymcp_server/tests/test_streamable_http.pymcp_server/graphium_mcp/tools.pymcp_server/tests/test_health_endpoint.pyserver/graph_service/routers/ingest.pymcp_server/tests/test_server_smoke.py
server/graph_service/main.py
📄 CodeRabbit inference engine (AGENTS.md)
FastAPI service entry point is server/graph_service/main.py
Files:
server/graph_service/main.py
graphium_core/{llm_client,embedder}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Place LLM clients under graphium_core/llm_client/ and embedders under graphium_core/embedder/
Files:
graphium_core/llm_client/pydantic_ai_adapter.pygraphium_core/embedder/providers/__init__.py
graphium_core/graphium.py
📄 CodeRabbit inference engine (AGENTS.md)
Treat graphium_core/graphium.py as the core library facade and delegate to orchestrators
Files:
graphium_core/graphium.py
mcp_server/tests/test_health_endpoint.py
📄 CodeRabbit inference engine (AGENTS.md)
Health endpoint must return 503 until Graphium initialization completes, then 200 with a success message
Files:
mcp_server/tests/test_health_endpoint.py
graphium_core/orchestration/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Bulk ingest, maintenance, and search orchestration should be under graphium_core/orchestration/
Files:
graphium_core/orchestration/payloads.py
{pyproject.toml,Makefile,pytest.ini,conftest.py}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain tooling manifests and test configuration at their canonical locations (repo root for pyproject.toml/Makefile/pytest.ini; conftest.py under tests as needed)
Files:
pyproject.tomlconftest.py
🧠 Learnings (2)
📚 Learning: 2025-10-12T05:55:20.133Z
Learnt from: CR
PR: CaliLuke/graphium#0
File: AGENTS.md:0-0
Timestamp: 2025-10-12T05:55:20.133Z
Learning: Applies to server/graph_service/main.py : FastAPI service entry point is server/graph_service/main.py
Applied to files:
server/graph_service/main.py
📚 Learning: 2025-10-12T05:55:20.133Z
Learnt from: CR
PR: CaliLuke/graphium#0
File: AGENTS.md:0-0
Timestamp: 2025-10-12T05:55:20.133Z
Learning: Applies to graphium_core/orchestration/**/*.py : Bulk ingest, maintenance, and search orchestration should be under graphium_core/orchestration/
Applied to files:
graphium_core/graphium.py
🧬 Code graph analysis (13)
tests/unit/graphium/test_add_episode_payload.py (3)
graphium_core/graphium.py (2)
Graphium(41-256)add_episode_payload(123-139)graphium_core/nodes.py (1)
EpisodeType(26-59)graphium_core/orchestration/payloads.py (1)
AddEpisodePayload(16-33)
graphium_core/providers/factory.py (4)
graphium_core/embedder/providers/gemini.py (2)
GeminiEmbedder(39-131)GeminiEmbedderConfig(34-36)graphium_core/embedder/providers/embeddinggemma.py (2)
EmbeddingGemmaConfig(18-21)EmbeddingGemmaEmbedder(24-46)graphium_core/cross_encoder/rerankers/gemini.py (1)
GeminiReranker(29-141)graphium_core/cross_encoder/rerankers/bge.py (1)
BGEReranker(20-39)
tests/unit/settings/test_llm_settings.py (1)
graphium_core/settings.py (6)
LLMProvider(45-51)LLMSettings(64-185)resolved_api_key(140-156)resolved_api_key(237-268)resolved_base_url(117-138)resolved_base_url(270-287)
graphium_core/cross_encoder/rerankers/__init__.py (3)
graphium_core/embedder/providers/__init__.py (2)
_missing_provider(8-17)_MissingProvider(9-14)graphium_core/cross_encoder/rerankers/gemini.py (1)
GeminiReranker(29-141)graphium_core/cross_encoder/rerankers/bge.py (1)
BGEReranker(20-39)
server/graph_service/main.py (3)
server/graph_service/graphium_client.py (1)
GraphiumClient(16-70)server/graph_service/routers/ingest.py (1)
lifespan(42-45)server/graph_service/config.py (1)
get_settings(22-23)
server/graph_service/graphium_client.py (2)
graphium_core/graphium.py (1)
Graphium(41-256)server/graph_service/dto/retrieve.py (1)
FactResult(16-26)
graphium_core/llm_client/pydantic_ai_adapter.py (1)
graphium_core/settings.py (2)
resolved_api_key(140-156)resolved_api_key(237-268)
graphium_core/graphium.py (4)
graphium_core/orchestration/payloads.py (1)
AddEpisodePayload(16-33)graphium_core/orchestration/services/search_service.py (2)
SearchService(21-92)search_with_config(60-77)graphium_core/orchestration/models.py (1)
AddEpisodeResults(9-15)graphium_core/orchestration/episode_orchestrator.py (1)
add_episode(132-270)
server/graph_service/routers/retrieve.py (3)
server/graph_service/graphium_client.py (2)
get_fact_result_from_edge(80-89)get_entity_edge(31-36)graphium_core/graphium.py (1)
search(203-214)server/graph_service/dto/retrieve.py (1)
SearchQuery(8-13)
graphium_core/embedder/providers/__init__.py (4)
graphium_core/cross_encoder/rerankers/__init__.py (2)
_missing_provider(9-18)_MissingProvider(10-15)graphium_core/embedder/providers/gemini.py (2)
GeminiEmbedder(39-131)GeminiEmbedderConfig(34-36)graphium_core/embedder/providers/voyage.py (2)
VoyageAIEmbedder(33-64)VoyageAIEmbedderConfig(28-30)graphium_core/embedder/providers/embeddinggemma.py (2)
EmbeddingGemmaConfig(18-21)EmbeddingGemmaEmbedder(24-46)
mcp_server/graphium_mcp/tools.py (2)
mcp_server/graphium_mcp/state.py (1)
EpisodeProcessor(16-20)mcp_server/graphium_mcp/queues.py (1)
enqueue_episode(107-120)
server/graph_service/routers/ingest.py (2)
server/graph_service/graphium_client.py (3)
delete_entity_edge(58-63)delete_group(38-56)delete_episodic_node(65-70)server/graph_service/dto/common.py (1)
Result(8-10)
graphium_core/orchestration/payloads.py (1)
graphium_core/nodes.py (1)
EpisodeType(26-59)
🪛 Ruff (0.13.3)
graphium_core/providers/factory.py
146-149: Avoid specifying long messages outside the exception class
(TRY003)
165-168: Avoid specifying long messages outside the exception class
(TRY003)
214-217: Avoid specifying long messages outside the exception class
(TRY003)
227-230: Avoid specifying long messages outside the exception class
(TRY003)
graphium_core/cross_encoder/rerankers/__init__.py
11-11: Unused method argument: args
(ARG002)
11-11: Unused method argument: kwargs
(ARG002)
12-15: Avoid specifying long messages outside the exception class
(TRY003)
graphium_core/embedder/providers/__init__.py
10-10: Unused method argument: args
(ARG002)
10-10: Unused method argument: kwargs
(ARG002)
11-14: Avoid specifying long messages outside the exception class
(TRY003)
42-42: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
graphium_core/settings.py
133-135: Avoid specifying long messages outside the exception class
(TRY003)
151-153: Avoid specifying long messages outside the exception class
(TRY003)
conftest.py
58-58: Unused function argument: config
(ARG001)
🔇 Additional comments (52)
mcp_server/tests/test_streamable_http.py (1)
10-14: LGTM! Module-level skip guard follows pytest best practices.The skip guard correctly prevents smoke tests from running unless explicitly enabled via
RUN_MCP_SMOKE_TESTS=1. The implementation follows pytest conventions withallow_module_level=Trueand includes an appropriate pragma comment to prevent coverage false negatives.tests/unit/embedder/test_openai.py (1)
10-11: Approve import guard
pytest.importorskip in test_openai.py correctly mirrors other provider tests, cleanly skipping the module when the OpenAI SDK is missing.tests/unit/embedder/test_embeddinggemma.py (1)
5-6: LGTM! Correct placement for optional dependency guard.The
importorskipis correctly placed before thegraphium_coreimports, ensuring the entire test module is skipped at collection time whenlitellmis unavailable. This prevents import errors and aligns with the PR's objective to guard optional provider dependencies.tests/unit/mcp/test_episode_queue.py (1)
7-8: LGTM! Correct optional dependency handling.The import guard is correctly placed before any imports that require fastmcp, allowing the test suite to gracefully skip when the optional MCP extra is not installed. This aligns with the PR objective to tolerate skipped integration suites in CI.
mcp_server/tests/test_server_smoke.py (1)
4-6: LGTM! Consistent optional dependency handling.The import guard follows the same pattern as other MCP tests and correctly prevents import failures when fastmcp is unavailable. The reason string appropriately describes the context.
mcp_server/tests/test_health_endpoint.py (1)
4-6: LGTM! Dual-guard pattern is intentional and correct.The new
importorskipguard for fastmcp complements the existing try/except guard for fastapi (lines 7-12). This dual-guard approach is intentional: fastmcp is required by themcp_server.graphium_mcpimports at lines 15-17, while fastapi is required byTestClientat line 8. The ordering ensures early detection of missing dependencies before subsequent imports fail.tests/integration/__init__.py (1)
1-13: LGTM! Robust integration test gating.The two-level guard is well-designed:
- Early exit if
RUN_INTEGRATION_TESTS != '1'(lines 7-11)- Dependency check for pytest-asyncio only if tests are enabled (line 13)
This prevents unnecessary import checks when integration tests are disabled and provides clear skip messages in both scenarios.
conftest.py (3)
7-9: LGTM! Appropriate test environment defaults.Disabling optional graph database services by default is a sensible choice for the test environment, allowing tests to run without external dependencies unless explicitly enabled.
14-14: LGTM! Correct use of pytest_plugins.Registering integration fixtures as a plugin is the recommended approach per the coding guidelines.
16-20: LGTM! Clean optional dependency guard.The try/except pattern with appropriate pragma comments correctly establishes whether pytest-asyncio is available for use in the collection hook.
mcp_server/graphium_mcp/state.py (2)
15-20: LGTM: Protocol definition enhances type safety.The migration from a simple type alias to a
runtime_checkableProtocol is a solid improvement:
- Explicitly documents the
queue_metadataattribute requirement- Enables runtime
isinstance()checks for validation- Provides better IDE support and type checking
The Protocol contract clearly communicates that implementations need both the callable signature and the metadata attribute, which aligns well with the dynamic attribute assignment pattern used in
tools.py.
6-7: LGTM: Import updates support the Protocol definition.The import changes correctly add
Awaitablefromcollections.abcandProtocol,runtime_checkablefromtyping, which are necessary for the new Protocol-based definition. The removal of theCallableimport (previously used for the type alias) is appropriate.mcp_server/graphium_mcp/tools.py (1)
109-116: Keep metadata assignment – it’s required
The manualqueue_metadataset at lines 110–114 seeds values that_metadata()reads and augments withattempts, so it’s not overwritten and must be retained.Likely an incorrect or invalid review comment.
pyproject.toml (2)
109-110: LGTM! Type-checking mode upgrade improves code quality.The upgrade from
basictostandardmode and Python 3.12 target aligns with the PR's goal of stricter type enforcement and is consistent with the updatedrequires-python = ">=3.12".
38-38: Verify dependency versions align with latest stable releases
- Upgrade
voyageaito>=0.3.5- Upgrade
sentence-transformersto>=5.1.1fastmcp>=2.12.4is up-to-date ✓- Bump
pytest-asyncioto>=1.2.0; no references to the removedevent_loopfixture found in testsEnsure these match the latest PyPI releases.
tests/unit/embedder/test_voyage.py (1)
10-11: LGTM! Proper optional dependency guard.The
pytest.importorskipguard ensures tests are skipped gracefully when thevoyageaiSDK is not installed, preventing import-time failures and aligning with the PR's optional dependency handling strategy.tests/unit/cross_encoder/test_gemini_reranker_client.py (1)
10-11: LGTM! Consistent optional dependency guard.The
pytest.importorskipguard forgoogle.genaifollows the same pattern as other optional provider tests, ensuring graceful skips when the dependency is unavailable..pre-commit-config.yaml (1)
1-7: LGTM! Pre-commit configuration properly set up.The configuration uses Ruff v0.7.1 (matching
pyproject.tomldev dependencies) with appropriate hooks:
ruff-formatfor consistent code formattingruff --fix --exit-non-zero-on-fixto apply fixes and ensure reviewThis aligns with the PR's goal of stricter tooling and CI hygiene.
graphium_core/llm_client/pydantic_ai_adapter.py (1)
120-127: LGTM! Sensible early exit when API key unavailable.The early API key check prevents unnecessary Pydantic AI model creation and execution when credentials are missing, with appropriate debug logging. The
getattrpattern safely handles both enum and string provider values.This optimization aligns with the provider-aware key resolution introduced in
settings.pyand ensures graceful fallback to the legacy flow.tests/unit/cross_encoder/test_bge_reranker_client.py (1)
6-9: LGTM! Consistent optional dependency guard.The
pytest.importorskipguard forsentence_transformersfollows the established pattern for optional dependencies, ensuring tests skip gracefully when the package is unavailable..github/workflows/ci.yml (2)
6-6: LGTM! Broader CI coverage on all branches.Changing the trigger from
mainto**ensures CI runs on all branches, catching issues earlier in development. This aligns with the PR's goal of expanding CI coverage.
44-55: Verify exit code 4 handling is intentional.The script allows exit codes 4 and 5 to be treated as skip/unavailable. However:
- Exit code 5: "No tests were collected" (expected for optional integration tests)
- Exit code 4: "pytest command line usage error" (indicates a configuration problem)
Allowing exit code 4 might mask actual pytest usage errors.
Consider restricting to only exit code 5:
- if [ "$status" -eq 5 ] || [ "$status" -eq 4 ]; then + if [ "$status" -eq 5 ]; then echo "Integration test suite skipped or unavailable (status $status)." elif [ "$status" -ne 0 ]; then exit "$status" fiIf exit code 4 is intentional (e.g., handling specific pytest configuration scenarios), please document why it's allowed.
CODE_REVIEW.md (1)
1-144: LGTM! Well-structured architectural guidance.This documentation provides clear, actionable recommendations for improving maintainability and performance. The examples are concrete and the prioritization (Critical/High/Medium severity) helps guide implementation.
A few observations:
- The singleton pattern recommendation (Issue 1) aligns with the PR's introduction of app-scoped lifecycle management.
- The payload-based API pattern (Issue 2) matches the
AddEpisodePayloadaddition referenced in the PR objectives.- The tooling recommendations (pre-commit hooks, stricter type checking) are solid best practices.
graphium_core/cross_encoder/rerankers/__init__.py (1)
21-33: LGTM! Clean optional dependency handling.The try/except pattern correctly handles optional dependencies, providing shim types when imports fail and only exposing real implementations in
__all__when they succeed. This aligns with the PR objective of guarding optional embedder/reranker imports.graphium_core/embedder/providers/__init__.py (2)
20-42: LGTM! Optional dependency handling is correctly implemented.The pattern correctly handles optional embedder providers, with proper fallback to shim types when dependencies are missing.
Regarding Ruff RUF022 (unsorted
__all__): This is a false positive. Since__all__is built dynamically viaextend()calls based on successful imports, sorting it statically would break the conditional export logic. This warning can be safely ignored.
36-40: Verify EmbeddingGemma extra name in pyproject.toml
The fallback uses'sentence-transformers', butembeddinggemma.pydoesn’t import that package. Ensure the extra defined under[tool.poetry.extras]matches this name; update the dependency string if it differs.docs/reports/code-complexity.md (1)
83-83: LGTM! Complexity metrics look healthy.The
server/graph_service/graphium_client.pyfile shows good maintainability metrics (MI: 60.5/C, CC: 2.2/A), which aligns with the PR's goal of improving code quality.tests/unit/providers/test_factory.py (1)
4-5: LGTM! Proper test guards for optional dependencies.The
pytest.importorskipguards correctly prevent test failures when optional dependencies are missing, aligning with the PR's objective of handling optional providers gracefully.Minor note: Line 5's reason mentions
'google-generativeai'but the import check is for'google.genai'. While these refer to the same package (the import name vs. the PyPI package name), consider being consistent. Either use:
reason='google-genai SDK required...'(matches the extra name), or- Keep as-is if referencing the PyPI package name is preferred.
graphium_core/providers/factory.py (4)
143-149: LGTM: Clear guidance for missing optional dependency.The import guard provides actionable installation instructions for users attempting to use the Gemini embedder without the required dependency.
162-168: LGTM: Clear guidance for missing optional dependency.The import guard provides actionable installation instructions for users attempting to use the EmbeddingGemma embedder without the required dependency.
211-217: LGTM: Clear guidance for missing optional dependency.The import guard provides actionable installation instructions for users attempting to use the Gemini reranker without the required dependency.
224-230: LGTM: Clear guidance for missing optional dependency.The import guard provides actionable installation instructions for users attempting to use the BGE reranker without the required dependency.
Note: Static analysis flags TRY003 (long messages outside exception class) for all four import guards in this file. For user-facing error messages with installation instructions, inline strings are acceptable despite the style warning.
server/graph_service/graphium_client.py (1)
73-77: LGTM: Appropriate app-scoped client lifecycle.The runtime lookup pattern correctly retrieves the app-scoped GraphiumClient and provides appropriate error handling when the client is not initialized. This aligns with the lifespan-based initialization in
server/graph_service/main.py.tests/unit/settings/test_llm_settings.py (3)
8-30: LGTM: Comprehensive test isolation.The autouse fixture ensures clean environment state for each test, preventing environment variable pollution between tests.
33-41: LGTM: Well-designed test helper.The
make_settingshelper properly constructs test instances with defaults while allowing overrides, making test cases more readable and maintainable.
44-109: LGTM: Comprehensive test coverage.The test suite thoroughly exercises LLMSettings resolution logic, including edge cases like ambiguous provider configurations. Test names clearly describe the behavior under test.
server/graph_service/routers/retrieve.py (1)
12-12: LGTM: Consistent dependency updates.All route handlers have been updated to use
GraphiumClientDep, aligning with the client lifecycle changes. The refactor is complete and consistent across the file.Also applies to: 18-18, 31-31, 37-37, 47-47
server/graph_service/main.py (1)
19-24: Ensure LLM client config is safely mutable at runtime
Directly mutatingclient.llm_client.config.base_url/api_keyand settingclient.llm_client.modelassumes the LLM client is always initialized with a mutable config; verify GraphiumClient (and its Graphium base) supports runtime updates to these properties or expose a dedicated configuration method.tests/unit/graphium/test_add_episode_payload.py (1)
1-143: LGTM! Comprehensive test coverage for payload-based episode addition.The test suite thoroughly validates the AddEpisodePayload integration:
- Minimal payload flow with default values
- Full payload flow with all optional fields populated
- Delegation from legacy
add_episodetoadd_episode_payloadThe use of
Graphium.__new__to bypass initialization and inject mock dependencies is an appropriate unit testing pattern that isolates the code under test.graphium_core/graphium.py (5)
25-26: LGTM! Clean import additions for payload-based API.The imports of
AddEpisodePayloadandSearchServicealign with the architectural changes documented in the engineering plan and enable the new payload-based episode addition flow.
90-93: LGTM! SearchService instantiation follows existing patterns.The replacement of
SearchOrchestratorwithSearchServicemaintains the same initialization pattern (clients and max_coroutines) and successfully simplifies the search layer as documented in Milestone 4 of the engineering plan.
123-139: LGTM! Clean payload delegation to orchestrator.The
add_episode_payloadmethod correctly unpacks all payload fields and forwards them to the existingepisode_orchestrator.add_episodemethod, maintaining consistency with the orchestrator's signature.
158-174: LGTM! Backward-compatible delegation to payload method.The refactored
add_episodemaintains the existing signature and delegates to the new payload-based API, ensuring backward compatibility while adopting the cleaner payload pattern internally.
212-246: LGTM! Search methods correctly delegate to SearchService.All three search methods (
search,search_, andget_nodes_and_edges_by_episode) have been correctly updated to delegate to the newsearch_service, maintaining their existing signatures and behavior.graphium_core/settings.py (4)
38-42: LGTM! Sensible model config update.Adding
populate_by_name=Trueenables field access via both the Python attribute name and the environment variable alias, improving ergonomics for programmatic settings construction while maintaining backward compatibility.
102-115: LGTM! Clean helper methods for provider-specific configurations.The
_provider_api_keys()and_provider_base_urls()methods centralize the mapping of provider-specific environment variables, making the resolution logic more maintainable and reducing repetition.Note: Currently only OpenAI has a provider-specific base URL. This is intentional and appropriate given most providers use their default endpoints.
117-138: LGTM! Explicit base URL resolution with ambiguity detection.The refactored logic correctly handles:
- Direct
base_urlorlitellm_proxy_base_urloverrides (highest priority)- Provider-specific URLs when
provideris explicitly set- Single provider auto-detection
- Clear error for ambiguous multi-provider configurations
- Fallback to provider defaults
This significantly improves configuration clarity and reduces the risk of silent misconfigurations.
140-156: LGTM! Explicit API key resolution with ambiguity detection.The refactored logic mirrors the base URL resolution approach and correctly handles:
- Direct
api_keyoverride (highest priority)- Provider-specific keys when
provideris explicitly set- Single provider auto-detection
- Clear error for ambiguous multi-provider configurations
This ensures users receive helpful guidance when their configuration is ambiguous rather than experiencing silent fallback behavior.
graphium_core/orchestration/payloads.py (3)
1-1: LGTM! Updated docstring reflects module purpose.The docstring accurately describes that this module contains typed payload models shared across orchestration components.
6-13: LGTM! Clean imports and exports.The import of
EpisodeTypesupports thesourcefield default, and the updated__all__list correctly exports the newAddEpisodePayloadmodel alongside existing payload types.
16-34: LGTM! Well-structured payload model with appropriate defaults.The
AddEpisodePayloadmodel correctly captures all parameters from theGraphium.add_episodemethod:
- Required fields:
name,episode_body,source_description,reference_time- Sensible default:
source=EpisodeType.message- Optional fields:
group_id,uuid,update_communities, type configurations, edge configurationsThe
ConfigDict(extra='allow', arbitrary_types_allowed=True)is appropriate given:
entity_typesandedge_typescontaintype[BaseModel]valuesedge_type_mapuses tuple keys which may not be directly serializable- The payload is used for internal API bridging rather than external serialization
server/graph_service/routers/ingest.py (1)
10-10: LGTM—GraphiumClientDep provides all invoked methods.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
graphium_core/search/nodes.py (1)
401-416: Restore thesemaphore_gatherimport.
_gather_episode_search_resultsand_gather_community_search_resultsstill callsemaphore_gather, but this module no longer imports it. At runtime those code paths will raiseNameError. Please reintroduce the import.-from graphium_core.search.shared import gather_search_results +from graphium_core.search.shared import gather_search_results +from graphium_core.utils.async_utils import semaphore_gather
🧹 Nitpick comments (2)
graphium_core/search/edges.py (1)
312-312: Sort__all__to appease Ruff.Ruff’s RUF022 still fires here; swapping to an alphabetic order (e.g.
['_build_edge_search_tasks', 'edge_search']) will keep lint green.-__all__ = ['edge_search', '_build_edge_search_tasks'] +__all__ = ['_build_edge_search_tasks', 'edge_search']graphium_core/embedder/providers/__init__.py (1)
26-33: Keep__all__sorted after optional extends.Ruff’s RUF022 complains once we append optional exports; calling
__all__.sort()(or reassigning the sorted list) after the try/except blocks keeps the dynamic additions lint-compliant.else: # pragma: no cover __all__.extend(['EmbeddingGemmaEmbedder', 'EmbeddingGemmaConfig']) +__all__.sort()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
graphium_core/cross_encoder/rerankers/__init__.py(1 hunks)graphium_core/embedder/providers/__init__.py(1 hunks)graphium_core/search/edges.py(3 hunks)graphium_core/search/nodes.py(2 hunks)graphium_core/search/shared.py(1 hunks)graphium_core/utils/__init__.py(1 hunks)graphium_core/utils/optional_dependencies.py(1 hunks)pyproject.toml(3 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use 4-space indentation, 100-character line length, and prefer single quotes as configured in pyproject.toml
Modules, files, and functions should use snake_case names
Only use# type: ignoreas a last resort and include a justification comment
Files:
graphium_core/utils/optional_dependencies.pygraphium_core/utils/__init__.pygraphium_core/search/nodes.pygraphium_core/cross_encoder/rerankers/__init__.pygraphium_core/search/shared.pygraphium_core/embedder/providers/__init__.pygraphium_core/search/edges.py
{graphium_core/driver/**,graphium_core/utils/**}
📄 CodeRabbit inference engine (AGENTS.md)
Place side-effectful code only in drivers or adapters (graphium_core/driver, graphium_core/utils); keep helpers elsewhere pure
Files:
graphium_core/utils/optional_dependencies.pygraphium_core/utils/__init__.py
{server/**/*.py,graphium_core/**/*.py}
📄 CodeRabbit inference engine (AGENTS.md)
Use ruff for formatting/lint and pyright for type checks (
make lint), with pyright standard mode in server and basic in core
Files:
graphium_core/utils/optional_dependencies.pygraphium_core/utils/__init__.pygraphium_core/search/nodes.pygraphium_core/cross_encoder/rerankers/__init__.pygraphium_core/search/shared.pygraphium_core/embedder/providers/__init__.pygraphium_core/search/edges.py
graphium_core/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
graphium_core/**/*.py: In graphium_core use absolute imports (e.g., from graphium_core.prompts.models import Message) instead of deep relative imports
Check for None (e.g.,driver is None) before use to satisfy type-narrowing and avoid possibly-unbound warnings
Files:
graphium_core/utils/optional_dependencies.pygraphium_core/utils/__init__.pygraphium_core/search/nodes.pygraphium_core/cross_encoder/rerankers/__init__.pygraphium_core/search/shared.pygraphium_core/embedder/providers/__init__.pygraphium_core/search/edges.py
{graphium_core/**/*.py,server/**/*.py,mcp_server/**/*.py}
📄 CodeRabbit inference engine (AGENTS.md)
Import the public facade via
from graphium_core.graphium import Graphiuminstead of importing from graphium_core.init
Files:
graphium_core/utils/optional_dependencies.pygraphium_core/utils/__init__.pygraphium_core/search/nodes.pygraphium_core/cross_encoder/rerankers/__init__.pygraphium_core/search/shared.pygraphium_core/embedder/providers/__init__.pygraphium_core/search/edges.py
{pyproject.toml,Makefile,pytest.ini,conftest.py}
📄 CodeRabbit inference engine (AGENTS.md)
Maintain tooling manifests and test configuration at their canonical locations (repo root for pyproject.toml/Makefile/pytest.ini; conftest.py under tests as needed)
Files:
pyproject.toml
graphium_core/search/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Search pipeline components belong under graphium_core/search/
Files:
graphium_core/search/nodes.pygraphium_core/search/shared.pygraphium_core/search/edges.py
graphium_core/{llm_client,embedder}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Place LLM clients under graphium_core/llm_client/ and embedders under graphium_core/embedder/
Files:
graphium_core/embedder/providers/__init__.py
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: CaliLuke/graphium PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-12T05:55:20.133Z
Learning: Applies to graphium_core/orchestration/**/*.py : Bulk ingest, maintenance, and search orchestration should be under graphium_core/orchestration/
📚 Learning: 2025-10-12T05:55:20.133Z
Learnt from: CR
Repo: CaliLuke/graphium PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-12T05:55:20.133Z
Learning: Applies to {graphium_core/**/*.py,server/**/*.py,mcp_server/**/*.py} : Import the public facade via `from graphium_core.graphium import Graphium` instead of importing from graphium_core.__init__
Applied to files:
graphium_core/utils/__init__.pygraphium_core/embedder/providers/__init__.py
📚 Learning: 2025-10-12T05:55:20.133Z
Learnt from: CR
Repo: CaliLuke/graphium PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-12T05:55:20.133Z
Learning: Applies to {server/**/*.py,graphium_core/**/*.py} : Use ruff for formatting/lint and pyright for type checks (`make lint`), with pyright standard mode in server and basic in core
Applied to files:
pyproject.toml
📚 Learning: 2025-10-12T05:55:20.133Z
Learnt from: CR
Repo: CaliLuke/graphium PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-12T05:55:20.133Z
Learning: Applies to graphium_core/models/**/*.py : Pydantic models in graphium_core/models must use PascalCase with explicit type hints
Applied to files:
pyproject.toml
📚 Learning: 2025-10-12T05:55:20.133Z
Learnt from: CR
Repo: CaliLuke/graphium PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-12T05:55:20.133Z
Learning: Applies to graphium_core/driver/**/*.py : If Ty reports unresolved imports for optional extras (Kuzu/FalkorDB/Neptune), install the extra or gate imports behind runtime-only paths
Applied to files:
graphium_core/cross_encoder/rerankers/__init__.py
🧬 Code graph analysis (6)
graphium_core/utils/__init__.py (1)
graphium_core/utils/optional_dependencies.py (1)
missing_provider(8-32)
graphium_core/search/nodes.py (2)
graphium_core/search/shared.py (1)
gather_search_results(11-29)graphium_core/search/search_utils.py (1)
node_bfs_search(873-959)
graphium_core/cross_encoder/rerankers/__init__.py (1)
graphium_core/utils/optional_dependencies.py (1)
missing_provider(8-32)
graphium_core/search/shared.py (1)
graphium_core/utils/async_utils.py (1)
semaphore_gather(22-52)
graphium_core/embedder/providers/__init__.py (1)
graphium_core/utils/optional_dependencies.py (1)
missing_provider(8-32)
graphium_core/search/edges.py (2)
graphium_core/search/shared.py (1)
gather_search_results(11-29)graphium_core/search/search_utils.py (1)
edge_bfs_search(529-651)
🪛 Ruff (0.14.3)
graphium_core/utils/optional_dependencies.py
28-28: Unused method argument: args
(ARG002)
28-28: Unused method argument: kwargs
(ARG002)
graphium_core/embedder/providers/__init__.py
32-32: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
graphium_core/search/edges.py
312-312: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
🔇 Additional comments (5)
pyproject.toml (3)
64-66: Verify intentionality: optional providers now duplicated in dev extra.Lines 64 and 66 add
sentence-transformersandvoyageaito thedevextra, though they're already listed in[project.optional-dependencies](lines 40 and 38). This redundancy aligns with the PR objective to move heavy test-only dependencies intodev, but confirm this dual-listing is intentional rather than an oversight.
110-111: Pyright typeCheckingMode set to "standard" for core; verify against coding guidelines.Lines 110–111 configure Pyright for core with
pythonVersion = "3.12"andtypeCheckingMode = "standard". Per learnings, pyright should use standard mode in server and basic mode in core. Confirm whether this change to standard mode is intentional to improve type checking rigor, or if it should revert to basic to align with guidelines.If this is intentional to tighten type checking, add a note to ADRs or commit message explaining the rationale. If it should remain basic, apply this diff:
[tool.pyright] include = ["graphium_core"] pythonVersion = "3.12" - typeCheckingMode = "standard" + typeCheckingMode = "basic"
69-69: No breaking changes found—upgrade to pytest-asyncio 1.0.0 is safe.Verification shows the codebase uses no deprecated APIs. The major v1.0.0 breaking change is removal of the
event_loopfixture, which is not used anywhere in this codebase. All async tests use@pytest.mark.asyncio(compatible with v1.0.0) and async fixtures use@pytest_asyncio.fixture(also compatible). Thepytest_pluginsdeclarations in integration tests are redundant but harmless in v1.0.0 since the plugin is auto-discovered. No migration work is needed.graphium_core/search/edges.py (1)
283-295: Shared gather flow looks good.Centralizing the BFS fallback via
gather_search_resultskeeps this path consistent with node search and drops the bespoke semaphore plumbing—nice tightening.graphium_core/embedder/providers/__init__.py (1)
10-31: Optional provider shims read cleanly.Thanks for routing the optional backends through
missing_provider; the shims should keep import-time UX consistent when extras are absent.
| class _MissingProvider: # pragma: no cover - shim for optional extras | ||
| def __init__(self, *args, **kwargs): | ||
| raise ImportError(message) | ||
|
|
||
| _MissingProvider.__name__ = f'Missing{name}' |
There was a problem hiding this comment.
Silence Ruff ARG002 on the shim constructor.
The new shim trips Ruff’s ARG002 because *args/**kwargs are unused. That will fail ruff check. Rename the parameters to throwaway identifiers so lint passes.
- class _MissingProvider: # pragma: no cover - shim for optional extras
- def __init__(self, *args, **kwargs):
+ class _MissingProvider: # pragma: no cover - shim for optional extras
+ def __init__(self, *_args, **_kwargs):
raise ImportError(message)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class _MissingProvider: # pragma: no cover - shim for optional extras | |
| def __init__(self, *args, **kwargs): | |
| raise ImportError(message) | |
| _MissingProvider.__name__ = f'Missing{name}' | |
| class _MissingProvider: # pragma: no cover - shim for optional extras | |
| def __init__(self, *_args, **_kwargs): | |
| raise ImportError(message) | |
| _MissingProvider.__name__ = f'Missing{name}' |
🧰 Tools
🪛 Ruff (0.14.3)
28-28: Unused method argument: args
(ARG002)
28-28: Unused method argument: kwargs
(ARG002)
🤖 Prompt for AI Agents
In graphium_core/utils/optional_dependencies.py around lines 27 to 31, the shim
class _MissingProvider defines __init__(self, *args, **kwargs) which triggers
Ruff ARG002 because those parameters are unused; rename them to throwaway
identifiers (e.g., *_, **__) so the linter ignores them and keep the body
unchanged (raise ImportError(message)) and retain the __name__ assignment.
Summary
LLMSettings(provider=...)construction by enabling name populationAddEpisodePayloadentry point, new unit coverage, and tighten Pydantic AI fallback behaviourTesting
uv run pytest tests/unit/providers/test_factory.pyuv run pytest tests/unit/settings/test_llm_settings.pyuv run pytest tests/unit/llm_client/test_litellm_client.py::test_litellm_client_validates_response_modelty check .Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Tests
Chores