feat: add support for custom embedding model#56
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
VladimirKadlec
force-pushed
the
embedding-config
branch
2 times, most recently
from
5b529a6 to
52c9984
Compare
VladimirKadlec
force-pushed
the
embedding-config
branch
from
52c9984 to
d7fd99e
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughAdds embedding support across config, models, and evaluation: new EmbeddingConfig, EmbeddingManager, and RagasEmbeddingManager; defaults added to constants. Refactors LLM env validation into module-level functions. Updates SystemConfig loader and usage. Modifies metrics (Ragas accepts embeddings, removes from_system_config). Adjusts evaluator/pipeline construction and dependencies. Adds dependencies. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant ConfigLoader
participant SystemConfig
participant LLMManager
participant EmbeddingManager
participant RagasEmbeddingManager
participant MetricsEvaluator
User->>ConfigLoader: load()
ConfigLoader->>SystemConfig: construct (includes embedding)
User->>MetricsEvaluator: __init__(config_loader, metric_manager)
MetricsEvaluator->>SystemConfig: get()
MetricsEvaluator->>LLMManager: from_system_config(SystemConfig)
note right of LLMManager: Runs provider env validators
MetricsEvaluator->>EmbeddingManager: from_system_config(SystemConfig)
note right of EmbeddingManager: Validates embedding provider/env
MetricsEvaluator->>RagasEmbeddingManager: __init__(EmbeddingManager)
note right of RagasEmbeddingManager: Build embeddings backend
sequenceDiagram
autonumber
participant MetricsEvaluator
participant RagasMetrics
participant LLMManager
participant RagasEmbeddingManager
participant Metric
participant Dataset
MetricsEvaluator->>RagasMetrics: evaluate(metric_name, dataset)
RagasMetrics->>LLMManager: get_llm()
RagasMetrics->>RagasEmbeddingManager: embeddings
RagasMetrics->>Metric: construct(llm, embeddings?, metric_kwargs)
RagasMetrics->>Dataset: compute()
Metric-->>RagasMetrics: result
RagasMetrics-->>MetricsEvaluator: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 review |
1 similar comment
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lightspeed_evaluation/core/metrics/custom.py (1)
58-64: Broaden exception handling in evaluate() to avoid crashing the pipeline.
_call_llm()raisesRuntimeError, which isn’t caught here; evaluation will bubble up and abort runs.Apply:
- except (ValueError, AttributeError, KeyError) as e: + except Exception as e: # includes RuntimeError from _call_llm return None, f"Custom {metric_name} evaluation failed: {str(e)}"
🧹 Nitpick comments (14)
src/lightspeed_evaluation/core/metrics/custom.py (3)
29-29: OK to drop factory; minor style nit.Class-level pylint disable is fine. Consider documenting construction via LLMManager in module docstring for discoverability.
46-46: Use logging instead of print for library code.Apply:
+import logging +logger = logging.getLogger(__name__) @@ - print(f"✅ Custom Metrics initialized: {self.model_name}") + logger.info("Custom Metrics initialized: %s", self.model_name)
23-25: Tighten type hints for contexts.
Optional[list]is too generic.Apply:
- contexts: Optional[list] = Field( + contexts: Optional[list[dict | str]] = Field( None, description="Context information if available" )src/lightspeed_evaluation/core/metrics/deepeval.py (1)
70-76: Guard against unexpected DeepEval errors.Broaden catch to prevent a single bad metric run from aborting evaluation.
Apply:
- except (ValueError, AttributeError, KeyError) as e: + except Exception as e: return None, f"DeepEval {metric_name} evaluation failed: {str(e)}"src/lightspeed_evaluation/core/constants.py (1)
14-15: Sensible defaults; confirm provider/model align with LiteLLM config.If OpenAI env isn’t set, ensure callers fail fast with clear errors or switch to HF defaults.
config/system.yaml (1)
20-33: Embedding config docs are clear; consider adding OpenAI example.Example:
# embedding: # provider: "openai" # model: "text-embedding-3-small" # provider_kwargs: {}tests/unit/core/llm/test_manager.py (1)
135-145: Add a couple more assertions for robustness.
- Case-insensitive provider for embeddings.
- Default embedding config behavior when
embeddingis absent.If helpful, I can add tests for these.
src/lightspeed_evaluation/core/llm/ragas.py (1)
1-9: Optional: avoid heavy imports at module import timeLangChain providers are heavy; consider lazy-importing them inside RagasEmbeddingManager.init to reduce startup cost when Ragas metrics aren’t used.
src/lightspeed_evaluation/core/metrics/ragas.py (2)
69-89: Derive result column from metric instance to avoid version driftHard-coding
result_keyrisks KeyError across ragas versions (e.g., context relevance). Use the metric instance’snameand guard missing columns.- def _evaluate_metric( # pylint: disable=too-many-positional-arguments,too-many-arguments + def _evaluate_metric( # pylint: disable=too-many-positional-arguments,too-many-arguments self, metric_class: Any, metric_kwargs: dict[str, Any] | None, dataset_dict: dict[str, Any], - result_key: str, metric_name: str, ) -> tuple[Optional[float], str]: @@ - metric_instance = metric_class(llm=self.llm_manager.get_llm(), **metric_kwargs) + metric_instance = metric_class(llm=self.llm_manager.get_llm(), **metric_kwargs) result = evaluate(dataset, metrics=[metric_instance]) df = result.to_pandas() - score = df[result_key].iloc[0] + col = getattr(metric_instance, "name", None) + if not col: + return None, f"Ragas {metric_name} missing metric name for result column" + try: + score = df[col].iloc[0] + except KeyError: + return None, f"Ragas {metric_name} result column '{col}' not found. Available: {list(df.columns)}" return score, f"Ragas {metric_name}: {score:.2f}"Note: Update callers to drop the
result_keyargument.
279-284: Verify column name for ContextRelevance or rely on dynamic name
"nv_context_relevance"may not match ragas’ column in some versions. Safer to use the metric instance’snameper suggestion above.src/lightspeed_evaluation/core/llm/manager.py (4)
53-59: Prefer logger over print in validatorsUse logging for
_validate_ollama_env(info) instead of printing.+import logging @@ +logger = logging.getLogger(__name__) @@ - if not os.environ.get("OLLAMA_HOST"): - print("ℹ️ OLLAMA_HOST not set, using default localhost:11434") + if not os.environ.get("OLLAMA_HOST"): + logger.info("OLLAMA_HOST not set, using default localhost:11434")
74-76: Replace constructor print with loggerEmits to stdout; prefer logger.
- print( - f"✅ LLM Manager: {self.config.provider}/{self.config.model} -> {self.model_name}" - ) + logger.info( + "LLM Manager: %s/%s -> %s", + self.config.provider, self.config.model, self.model_name + )
167-169: Replace EmbeddingManager print with loggerSame logging consistency nit.
- print( - f""" -✅ Embedding Manager: {self.config.provider} -- {self.config.model} {self.config.provider_kwargs}""" - ) + logger.info( + "Embedding Manager: %s -- %s %s", + self.config.provider, self.config.model, self.config.provider_kwargs + )
189-192: Fix docstring: EmbeddingManager factoryMinor wording nit.
@classmethod def from_system_config(cls, system_config: SystemConfig) -> "EmbeddingManager": - """Create LLM Manager from system configuration.""" + """Create Embedding Manager from system configuration.""" return cls(system_config.embedding)
📜 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 (14)
config/system.yaml(1 hunks)pyproject.toml(1 hunks)src/lightspeed_evaluation/core/constants.py(1 hunks)src/lightspeed_evaluation/core/llm/manager.py(3 hunks)src/lightspeed_evaluation/core/llm/ragas.py(2 hunks)src/lightspeed_evaluation/core/metrics/custom.py(1 hunks)src/lightspeed_evaluation/core/metrics/deepeval.py(1 hunks)src/lightspeed_evaluation/core/metrics/ragas.py(9 hunks)src/lightspeed_evaluation/core/models/__init__.py(2 hunks)src/lightspeed_evaluation/core/models/system.py(3 hunks)src/lightspeed_evaluation/core/system/loader.py(2 hunks)src/lightspeed_evaluation/pipeline/evaluation/evaluator.py(3 hunks)src/lightspeed_evaluation/pipeline/evaluation/pipeline.py(1 hunks)tests/unit/core/llm/test_manager.py(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-10T15:48:14.671Z
Learnt from: asamal4
PR: lightspeed-core/lightspeed-evaluation#47
File: src/lightspeed_evaluation/core/output/generator.py:43-49
Timestamp: 2025-09-10T15:48:14.671Z
Learning: In the lightspeed-evaluation framework, system configuration uses Pydantic data models (SystemConfig, OutputConfig, LoggingConfig, etc.) rather than plain dictionaries. Components like OutputHandler receive properly structured Pydantic models, so direct attribute access (e.g., system_config.output.enabled_outputs) is the correct approach.
Applied to files:
src/lightspeed_evaluation/core/models/__init__.pysrc/lightspeed_evaluation/pipeline/evaluation/evaluator.py
🔇 Additional comments (12)
src/lightspeed_evaluation/core/metrics/deepeval.py (1)
18-18: OK to remove from_system_config; class-level pylint disable is fine.src/lightspeed_evaluation/core/models/__init__.py (2)
13-13: Re-export of EmbeddingConfig looks good.
30-30: all update is consistent.config/system.yaml (1)
11-19: Helpful Gemini example.Comment-only change improves discoverability.
tests/unit/core/llm/test_manager.py (2)
7-13: Public API import path changes look correct.
106-126: SystemConfig-based construction test reads well.Covers nested
provider_kwargsshape.src/lightspeed_evaluation/pipeline/evaluation/pipeline.py (1)
55-55: Constructor change: ensure MetricsEvaluator signature updated everywhere.Found call at src/lightspeed_evaluation/pipeline/evaluation/pipeline.py:55 —
metrics_evaluator = MetricsEvaluator(self.config_loader). ast-grep did not locate the MetricsEvaluator class/constructor in the repo; confirm MetricsEvaluator.init no longer requiresllm_managerand update the constructor or any remaining call sites that passllm_manager.pyproject.toml (1)
30-31: Make embedding deps optional & verify sentence-transformers version compatibility
- pyproject.toml (lines 30–31): "langchain[huggingface]>=0.3.27" and "sentence-transformers>=5.1.0" — verification failed (SSL certificate error when querying PyPI). Confirm that sentence-transformers 5.1.0 exists and is compatible with pinned torch==2.7.0 (5.1.0 may be too new).
- Move heavy embedding deps to an optional extra to avoid pulling them when embeddings aren’t used. Suggested change (apply after verifying versions):
@@ - "tenacity>=9.1.2", - "langchain[huggingface]>=0.3.27", - "sentence-transformers>=5.1.0", + "tenacity>=9.1.2", ] @@ +[project.optional-dependencies] +embeddings = [ + "langchain[huggingface]>=0.3.27", + # pick a known-good floor; adjust after verification + "sentence-transformers>=3.0.0", +]src/lightspeed_evaluation/core/models/system.py (1)
225-227: Good addition: defaulted embedding config on SystemConfigWiring EmbeddingConfig with default_factory is correct and consistent with LLM. No issues.
Please confirm DEFAULT_EMBEDDING_PROVIDER/MODEL defaults point to actually installed providers/models in pyproject to avoid runtime ImportError.
src/lightspeed_evaluation/pipeline/evaluation/evaluator.py (2)
37-38: Correct dependency injection into RagasMetricsPassing both LLM and Embedding managers is the right move.
26-35: Constructor call sites updated — no action requiredSearch found a single instantiation: MetricsEvaluator(self.config_loader) at src/lightspeed_evaluation/pipeline/evaluation/pipeline.py:55; no callers pass llm_manager.
src/lightspeed_evaluation/core/metrics/ragas.py (1)
147-153: Nice: embeddings passed for ResponseRelevancyCorrectly wiring
embeddingsintoResponseRelevancy.
| "openai": _validate_openai_env, | ||
| # "google": _validate_gemini_env, # Google embeddings are not supported at the moment |
There was a problem hiding this comment.
Do we really need to support openai embedding model ??
For embedding models I would rather prefer just the open source models support.
It is very unlikely that someone would use openai embedding model, when opensource models are equally capable
There was a problem hiding this comment.
Yes, because this is the default for Ragas. Also Huggingface embeddings runs locally which might not be ideal for some scenarios.
There was a problem hiding this comment.
because this is the default for Ragas
I don't think we have to keep same default or use same model.
runs locally which might not be ideal for some scenarios
not sure about the scenario (unless system resource is unreasonably low)..
openAI adds cost, limited options and similar quality models are available free..
with huggingface most model gets downloaded easily, if it is not present in the system. To avoid this, team using eval tool can provide model path (the way it is done in OLS rag)
For local run, it is one time download.
But I guess, no harm in keeping it as an option.
| DEFAULT_EMBEDDING_PROVIDER = "openai" | ||
| DEFAULT_EMBEDDING_MODEL = "text-embedding-3-small" |
There was a problem hiding this comment.
This is just for reference, still I would suggest to keep an opensource model as default.
There was a problem hiding this comment.
Note that choosing some huggingface model requires downloading and running it locally, IMHO a big change from the current setup.
There was a problem hiding this comment.
Fo default, I would personally prefer a free model..
with huggingface most model gets downloaded easily, if it is not present in the system.
For openai, team needs to have the access/key..
I know we have openai as default LLM, but majorly people use gemini, so they won't have embedding model.
| return cls(llm_config) | ||
|
|
||
|
|
||
| class EmbeddingManager: # pylint: disable=too-few-public-methods |
There was a problem hiding this comment.
Please keep embedding model initialization code outside llm. in a folder named embedding..
There was a problem hiding this comment.
Have to say that the current llm configuration is slightly over complicated. But I can create another module for sure :)
There was a problem hiding this comment.
Thank you..
Have to say that the current llm configuration is slightly over complicated
it seems complicated, but we can support new providers very easily. Perhaps a separate discussion..
Please ping me or create a story with details, so that we can improve.
|
|
||
| import litellm | ||
| from langchain_huggingface import HuggingFaceEmbeddings | ||
| from langchain_openai import OpenAIEmbeddings |
There was a problem hiding this comment.
Same as earlier, I would rather prefer just the opensource embedding models.
config/system.yaml
Outdated
| # Example Google Gemini LLM | ||
| # llm: | ||
| # provider: "gemini" | ||
| # model: "gemini-1.5-pro" | ||
| # temperature: 0.0 | ||
| # max_tokens: 512 | ||
| # timeout: 120 | ||
| # num_retries: 3 |
There was a problem hiding this comment.
One example is already in place for llm (openai)..
Can we remove gemini specific example from this file.. we are not going to add every llm provider example here.
There was a problem hiding this comment.
Agreed, I'll move it to some docs.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/lightspeed_evaluation/core/embedding/ragas.py (1)
18-24: Add error handling for unsupported providers.The current implementation raises a generic RuntimeError for unknown providers, but this should align with the validation pattern established elsewhere in the codebase.
Consider creating a specific EmbeddingError and importing it from the manager module:
+from ..embedding.manager import EmbeddingError + embedding_class = { "openai": OpenAIEmbeddings, "huggingface": HuggingFaceEmbeddings, }.get(config.provider) if not embedding_class: - raise RuntimeError(f"Unknown embedding provider {config.provider}") + raise EmbeddingError(f"Unknown embedding provider {config.provider}")src/lightspeed_evaluation/core/models/system.py (1)
72-85: Consider validation of provider_kwargs structure.While the typing is correct, consider adding validation to ensure provider_kwargs contains expected keys for each provider to catch configuration errors early.
provider_kwargs: Optional[dict[str, Any]] = Field( default=None, description="Embedding provider arguments, e.g. model_kwargs: device:cpu", ) +@field_validator("provider_kwargs") +@classmethod +def _validate_provider_kwargs(cls, v: Optional[dict[str, Any]], info) -> Optional[dict[str, Any]]: + if v is None: + return v + + provider = info.data.get("provider", "") + if provider == "huggingface" and "model_kwargs" in v: + # Validate model_kwargs is a dict + if not isinstance(v["model_kwargs"], dict): + raise ValueError("model_kwargs must be a dictionary") + return vsrc/lightspeed_evaluation/core/embedding/manager.py (2)
29-40: Potential validation redundancy with EmbeddingConfig.The manager validates providers again, but EmbeddingConfig already has field validation. This could create inconsistency if the allowed sets diverge.
Consider leveraging the EmbeddingConfig validation rather than duplicating it:
def _validate_config(self) -> None: """Validate config and env variables.""" def empty_check() -> None: pass - env_validator = { - "openai": validate_openai_env, - # "google": _validate_gemini_env, # Google embeddings are not supported at the moment - "huggingface": empty_check, - }.get(self.config.provider) - - if env_validator is None: - raise EmbeddingError( - f"Unsupported embedding provider {self.config.provider}" - ) - - env_validator() + # Note: Provider validation is already handled by EmbeddingConfig + env_validators = { + "openai": validate_openai_env, + "huggingface": empty_check, + } + + validator = env_validators.get(self.config.provider, empty_check) + validator()
18-21: Consider more informative success message.The current print statement formatting could be cleaner and more consistent with other manager outputs.
-print( - f""" -✅ Embedding Manager: {self.config.provider} -- {self.config.model} {self.config.provider_kwargs}""" -) +print(f"✅ Embedding Manager: {self.config.provider}/{self.config.model}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
README.md(1 hunks)config/system.yaml(1 hunks)src/lightspeed_evaluation/core/embedding/manager.py(1 hunks)src/lightspeed_evaluation/core/embedding/ragas.py(1 hunks)src/lightspeed_evaluation/core/llm/__init__.py(2 hunks)src/lightspeed_evaluation/core/llm/manager.py(3 hunks)src/lightspeed_evaluation/core/metrics/ragas.py(9 hunks)src/lightspeed_evaluation/core/models/system.py(3 hunks)src/lightspeed_evaluation/core/system/loader.py(2 hunks)src/lightspeed_evaluation/pipeline/evaluation/evaluator.py(2 hunks)src/lightspeed_evaluation/pipeline/evaluation/pipeline.py(1 hunks)tests/unit/core/llm/test_manager.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- config/system.yaml
- src/lightspeed_evaluation/pipeline/evaluation/pipeline.py
- src/lightspeed_evaluation/pipeline/evaluation/evaluator.py
- src/lightspeed_evaluation/core/system/loader.py
🧰 Additional context used
🧬 Code graph analysis (6)
src/lightspeed_evaluation/core/llm/__init__.py (1)
src/lightspeed_evaluation/core/llm/manager.py (4)
LLMError(9-10)LLMManager(61-156)validate_gemini_env(43-50)validate_openai_env(13-18)
src/lightspeed_evaluation/core/embedding/ragas.py (1)
src/lightspeed_evaluation/core/embedding/manager.py (1)
EmbeddingManager(11-45)
tests/unit/core/llm/test_manager.py (2)
src/lightspeed_evaluation/core/llm/manager.py (1)
LLMManager(61-156)src/lightspeed_evaluation/core/models/system.py (2)
LLMConfig(33-64)SystemConfig(228-255)
src/lightspeed_evaluation/core/embedding/manager.py (2)
src/lightspeed_evaluation/core/llm/manager.py (1)
validate_openai_env(13-18)src/lightspeed_evaluation/core/models/system.py (2)
EmbeddingConfig(67-95)SystemConfig(228-255)
src/lightspeed_evaluation/core/metrics/ragas.py (5)
src/lightspeed_evaluation/core/embedding/manager.py (1)
EmbeddingManager(11-45)src/lightspeed_evaluation/core/embedding/ragas.py (1)
RagasEmbeddingManager(10-31)src/lightspeed_evaluation/core/llm/manager.py (3)
LLMManager(61-156)get_model_name(130-132)get_litellm_params(134-142)src/lightspeed_evaluation/core/llm/ragas.py (2)
RagasLLMManager(84-111)get_llm(102-104)src/lightspeed_evaluation/core/models/data.py (2)
EvaluationScope(211-222)TurnData(34-126)
src/lightspeed_evaluation/core/llm/manager.py (2)
src/lightspeed_evaluation/core/models/system.py (2)
LLMConfig(33-64)SystemConfig(228-255)src/lightspeed_evaluation/core/embedding/manager.py (1)
from_system_config(43-45)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: mypy
- GitHub Check: tests (3.13)
- GitHub Check: tests (3.11)
🔇 Additional comments (14)
README.md (1)
159-184: LGTM: Well-documented embedding configuration example.The non-OpenAI configuration example provides clear documentation of the new embedding integration, including HuggingFace embeddings with provider_kwargs. The examples align with the implementation changes in the codebase.
tests/unit/core/llm/test_manager.py (2)
7-11: LGTM: Imports aligned with new architecture.The imports now correctly include SystemConfig and utilize the centralized import structure from the core models module.
105-114: LGTM: Test updated to use SystemConfig validation.The test correctly constructs a SystemConfig using model_validate, which ensures proper validation of the configuration structure including the new embedding configuration.
src/lightspeed_evaluation/core/llm/__init__.py (1)
5-5: LGTM: Environment validators exposed for reuse.Making the environment validators publicly available through the module's API allows them to be reused by the embedding manager and other components, promoting code reuse.
Also applies to: 14-16
src/lightspeed_evaluation/core/embedding/ragas.py (1)
25-31: LGTM: Proper kwargs handling and embedding wrapper construction.The implementation correctly handles None provider_kwargs and constructs the LangchainEmbeddingsWrapper as expected by Ragas, based on the web search results showing this is the proper integration pattern.
src/lightspeed_evaluation/core/models/system.py (2)
87-95: LGTM: Provider validation addresses previous feedback.The field validator properly restricts embedding providers to the supported set (openai, huggingface) and provides clear error messages, addressing the past review comment about documentation misalignment.
235-237: Address reviewer's question about API embedding requirement.Based on the past review comment questioning why embedding is required for API functionality, consider if the embedding field should be optional or have conditional validation based on API configuration.
src/lightspeed_evaluation/core/llm/manager.py (3)
13-19: LGTM: Proper environment validator extraction.Moving the environment validators to module level enables reuse by the embedding manager and maintains consistent validation patterns across the codebase.
149-151: LGTM: SystemConfig integration properly implemented.The updated from_system_config method correctly uses the structured SystemConfig object and accesses the llm field, aligning with the new configuration architecture.
53-59: Informational message for Ollama is appropriate.The informational message for missing OLLAMA_HOST provides helpful guidance without failing validation, which is appropriate for local development scenarios.
src/lightspeed_evaluation/core/metrics/ragas.py (4)
28-28: LGTM: Clean embedding manager integration.The constructor properly accepts both LLM and embedding managers, and the RagasEmbeddingManager initialization follows the established pattern.
Also applies to: 33-41
71-85: LGTM: Enhanced metric evaluation with embeddings support.The updated _evaluate_metric method signature and implementation properly supports metric-specific configuration, enabling embedding injection for response relevancy metrics.
176-178: LGTM: Consistent metric_kwargs parameter passing.All metric evaluation methods now consistently pass metric_kwargs, maintaining API consistency even when no specific kwargs are needed (empty dict for most metrics).
Also applies to: 200-205, 230-236, 261-263, 281-286
149-155: Approve — embedding integration aligns with Ragas. ResponseRelevancy accepts an "embeddings" kwarg expecting a LangChain-style embeddings object; RagasEmbeddingManager provides a LangchainEmbeddingsWrapper, so passing self.embedding_manager.embeddings is correct.
3 participants
LCORE-649
Summary by CodeRabbit
New Features
Documentation
Refactor
Chores
Tests