-
Notifications
You must be signed in to change notification settings - Fork 3
Feature/llm reranker #278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Feature/llm reranker #278
Conversation
|
❌ Tests failed (exit code: 1) 📊 Test Results
Branch: 📋 Full coverage report and logs are available in the workflow run. |
NISH1001
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial review
| agent_system_prompt: str = Field( | ||
| default=( | ||
| "You are an expert at evaluating search results. " | ||
| "Analyze the provided result for the query against all given criteria and " | ||
| "select the most appropriate category for each. Provide clear reasoning." | ||
| ), | ||
| description="System prompt for the internal scoring agent", | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have this get from os.getenv as well so that we can globally modify whenever we want...and default to this value you have. something like AKD_LLM_RERANKER_SYSTEM_PROMPT
| ScoringCriterion( | ||
| name="Processing Level", | ||
| description="How well does this result match the required processing level?", | ||
| weight=0.5, | ||
| ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems too specific to data search agent. we need to find some general criteria that can be globally applied to any usecase...
| debug: Enable debug logging | ||
| """ | ||
| super().__init__(config=config, debug=debug) | ||
| self.config: LLMRerankerToolConfig = self.config # type hint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need this...it will automatically be done through super()...
|
|
||
| class ScoringAgent(LiteLLMInstructorBaseAgent): | ||
| input_schema = DummyInput | ||
| output_schema = dynamic_scoring_model |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this to anotehr internal method like _setup_scoring_agent or something
and we can just do `self.scoring_agent = self._setup_scoring_agent(model....)
| Similar to relevancy-ranker.py approach - creates explicit named fields | ||
| for each criterion so LLM can see them in the JSON schema. | ||
| """ | ||
| from pydantic import create_model |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to top-level import
| ) | ||
|
|
||
| if self.debug: | ||
| print(formatted_prompt) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logger.debug(...)
| }, | ||
| ] | ||
|
|
||
| print(messages) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove print stattement...or add if self.debug: logger.debug(...)
| def create_reranker( | ||
| reranker_type: RerankerType, | ||
| config: RerankerToolConfig | None = None, | ||
| config: RerankerToolConfig | LLMRerankerToolConfig | None = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically don't need LLMRerankerToolConfig because it's also a type of RerankerToolConfig. Redundant type hint
|
|
||
| response = await self.scoring_agent.get_response_async( | ||
| messages=messages, | ||
| ) | ||
|
|
||
| results = {} | ||
| response_dict = response.model_dump() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if we do this at agent.arun level? since it's one-level higher abstraction. is it possible to do it with the formatted_prompt? you have to convert it to ap ydantic input schema
7819464 to
93ea754
Compare
|
❌ Tests failed (exit code: 1) 📊 Test Results
Branch: 📋 Full coverage report and logs are available in the workflow run. |
|
❌ Tests failed (exit code: 1) 📊 Test Results
Branch: 📋 Full coverage report and logs are available in the workflow run. |
Summary
- Single LLM call per result evaluates all criteria simultaneously commit hash (02eb523)
- Multiple LLM call per result (for per criteria) commit hash (166f475)