Skip to content
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

[llm unify 5/n] llm_filter #1154

Merged
merged 19 commits into from
Feb 11, 2025
Merged

[llm unify 5/n] llm_filter #1154

merged 19 commits into from
Feb 11, 2025

Conversation

HenryL27
Copy link
Collaborator

@HenryL27 HenryL27 commented Feb 4, 2025

No description provided.

@HenryL27 HenryL27 requested a review from baitsguy February 4, 2025 23:54
@@ -317,7 +323,7 @@ def __init__(self, question: str, field: str, format: Optional[str], discrete: b
)


class LlmFilterMessagesPrompt(SimplePrompt):
class _LlmFilterMessagesPrompt(SimplePrompt):
Copy link
Contributor

Choose a reason for hiding this comment

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

delete?

LlmFilterDocValuePrompt = FieldValuePrompt(
system="You are a helpful classifier that generously filters database entries based on questions.",
user=(
"Given a field value and a question, you will answer the question relating "
Copy link
Contributor

Choose a reason for hiding this comment

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

We can probably combine these? It's okay to rewrite this prompt to something better too since we're editing. I can help test

def __init__(self):
super().__init__()


class JinjaDie(Exception):
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe name this differently :D

@@ -219,6 +221,7 @@ def test_extract_entity_with_similarity_sorting(self, mocker):
assert (taken[0].elements[1]["properties"]["_element_index"]) == 2

# Element order should be changed in the prompt
print(taken[1])
Copy link
Contributor

Choose a reason for hiding this comment

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

remove

)
comptransform = entity_extractor.as_llm_map(child)
assert isinstance(comptransform, CompositeTransform)
llm_map = comptransform.nodes[-2]
Copy link
Contributor

Choose a reason for hiding this comment

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

mind adding a couple comments to clarify what the expectation here is/why

Signed-off-by: Henry Lindeman <[email protected]>
@HenryL27 HenryL27 merged commit d43af0c into main Feb 11, 2025
11 of 15 checks passed
@HenryL27 HenryL27 deleted the hml-llm-unify branch February 11, 2025 21:56
austintlee pushed a commit that referenced this pull request Feb 14, 2025
* convert extract_properties to use LLMMap

Signed-off-by: Henry Lindeman <[email protected]>

* use _schema_name or _schema_class as output field if present otherwise use entity

Signed-off-by: Henry Lindeman <[email protected]>

* drop _prompts

Signed-off-by: Henry Lindeman <[email protected]>

* address pr comments

Signed-off-by: Henry Lindeman <[email protected]>

* make mypy happy

Signed-off-by: Henry Lindeman <[email protected]>

* put back original exception whoops

Signed-off-by: Henry Lindeman <[email protected]>

* initial llm filter translate

Signed-off-by: Henry Lindeman <[email protected]>

* ..source_element_index -> ..source_element_indices

Signed-off-by: Henry Lindeman <[email protected]>

* oh yeah this was supposed to get deleted

Signed-off-by: Henry Lindeman <[email protected]>

* deprecate docset.llm_filter

Signed-off-by: Henry Lindeman <[email protected]>

* add source indices for single batch element batcher

Signed-off-by: Henry Lindeman <[email protected]>

* get llm_filter uts working while using EntityExtractor.as_llm_map to plan as map

Signed-off-by: Henry Lindeman <[email protected]>

* fix uts that broke

Signed-off-by: Henry Lindeman <[email protected]>

* ruff

Signed-off-by: Henry Lindeman <[email protected]>

* mypy

Signed-off-by: Henry Lindeman <[email protected]>

* pr comments

Signed-off-by: Henry Lindeman <[email protected]>

---------

Signed-off-by: Henry Lindeman <[email protected]>
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