[Groundedness] Handle edge cases #43809
Conversation
github-actions
bot
added
the
Evaluation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the context validation logic in the GroundednessEvaluator by extracting it into a separate _validate_context method and conditionally applying file search result filtering based on context validity.
- Extracts context validation logic from
_has_contextinto a reusable_validate_contextmethod - Adds conditional filtering: only filters file search results when context is valid
...valuation/azure-ai-evaluation/azure/ai/evaluation/_evaluators/_groundedness/_groundedness.py
Outdated
Show resolved
Hide resolved
...valuation/azure-ai-evaluation/azure/ai/evaluation/_evaluators/_groundedness/_groundedness.py
Show resolved
Hide resolved
ahibrahimm
changed the title
| ) | ||
| self._flow = AsyncPrompty.load(source=self._prompty_file, model=prompty_model_config) | ||
| self._flow = AsyncPrompty.load( | ||
| source=self._prompty_file, model=prompty_model_config |
There was a problem hiding this comment.
should you add credential and is_reasoning_model to args here?
...valuation/azure-ai-evaluation/azure/ai/evaluation/_evaluators/_groundedness/_groundedness.py
Show resolved
Hide resolved
| else: | ||
| return EVALUATION_PASS_FAIL_MAPPING[False] | ||
|
|
||
| @override |
There was a problem hiding this comment.
no need for override tag
|
|
||
| @override | ||
| async def _do_eval(self, eval_input: Dict) -> Dict[str, Union[float, str]]: # type: ignore[override] | ||
| async def _do_eval_wflow(self, eval_input: Dict, flow) -> Dict[str, Union[float, str]]: # type: ignore[override] |
There was a problem hiding this comment.
I don't think the comment "# type: ignore[override]" is needed too
|
|
||
| @override | ||
| async def _do_eval(self, eval_input: Dict) -> Dict[str, Union[float, str]]: # type: ignore[override] | ||
| async def _do_eval_wflow(self, eval_input: Dict, flow) -> Dict[str, Union[float, str]]: # type: ignore[override] |
There was a problem hiding this comment.
Optional: It's better to add type for flow argument. And can you change the name to _with_flow
| whatever inputs are needed for the _flow method, including context | ||
| and other fields depending on the child class. | ||
| :type eval_input: Dict | ||
| :return: The evaluation result. |
There was a problem hiding this comment.
add documentation for flow param
| :return: The evaluation result. | ||
| :rtype: Dict | ||
| """ | ||
|
|
There was a problem hiding this comment.
Optional: Remove redundant new line after doc string.
| self._prompty_file = None | ||
|
|
||
| self._flow_wquery = self._load_flow(self._PROMPTY_FILE_WITH_QUERY, credential=credential) | ||
| self._flow_woquery = self._load_flow(self._PROMPTY_FILE_NO_QUERY, credential=credential) |
There was a problem hiding this comment.
Optional: Can you rename it to with_query and _flow_without_query?
Description
Please add an informative description that covers that changes made by the pull request and link all relevant issues.
If an SDK is being regenerated based on a new API spec, a link to the pull request containing these API spec changes should be included above.
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines