UI extension for new Corpus feature #499
Conversation
| if op not in EXCLUDED_OPERATORS | ||
| ] | ||
|
|
||
| DUNDER_METHODS = {f"__{op}__".lower(): op for op in ALLOWED_OPERATORS} |
There was a problem hiding this comment.
Can remove this validation if not helpful. It uses the type information passed by the source storage to infer a list of valid operators.
It's not super useful though as string will still allow 'GT' etc., which I don't think is helpful (e.g. path > /my_path is valid Python code, but I don't think it's what users will want).
See below (compute_valid_operator_options etc) for more details.
| return None | ||
| return MetadataFilter.and_(metadata_filters).to_primitive() | ||
|
|
||
| def validate(self): |
There was a problem hiding this comment.
Called in modal_configuration.py ...
| css_classes=["metadata-filter-row-collection"], | ||
| ) | ||
|
|
||
| def construct_metadata_filters(self): |
There was a problem hiding this comment.
Called in modal_configuration.py...
| corpus_name=self.metadata_filter_rows.corpus_names_select.value, | ||
| ) | ||
|
|
||
| async def did_finish_upload(self, uploaded_documents, corpus_name=None): |
There was a problem hiding this comment.
We now need to pass corpus_name as it is attached to the chat.
| @@ -146,22 +197,20 @@ async def did_finish_upload(self, uploaded_documents): | |||
| self.start_chat_button.disabled = False | |||
|
|
|||
| def change_upload_files_label(self, mode="normal"): | |||
| self.error = False | |||
There was a problem hiding this comment.
Required to introduce error so that we can watch it, and update the CSS whenever a user performs an invalid operation.
This was previously implicitly watched by other components, but now that we have added many more components to the modal, we need to introduce an explicit component to watch it (otherwise there are edge cases that arise where the CSS is not updated when a user performs an error).
| super().__init__(**params) | ||
|
|
||
| key_select_disabled = True | ||
| if valid_key_value_pairs: |
There was a problem hiding this comment.
This is ok for the EQ, NE, IN, and NOT_IN cases, but falls flat for the numeric ones. Imagine I have [1, 3, 5, 7] in my DB. < 4 should be valid although 4 is not inside the DB.
There was a problem hiding this comment.
Thanks Nick for all the hard work here! I found a few bugs, but I don't want to keep this PR open any longer. Let me send a small cleanup commit to fix any nits I might have and fix CI. I'll open issues for all of my comments so we can fix them in follow-up PRs.
| metadata_filters_readable = str( | ||
| MetadataFilter.from_primitive(self.current_chat["metadata"]["input"]) | ||
| ).replace("\n", "<br>") |
There was a problem hiding this comment.
This needs further processing than just the newlines. The child should be indented
form ragna.core import MetadataFilter
print(MetadataFilter.and_([MetadataFilter.in_("document_name", ["foo.md"])]))AND(
IN('document_name', ['foo.md']),
)
| ] | ||
|
|
||
| NO_CORPUS_KEY = "No corpuses available" | ||
| NO_FILTER_KEY = "Empty Filter" |
There was a problem hiding this comment.
- I don't like this "empty filter" idea. I'd prefer we just have no filter row by default and the user has to click the plus to add one. No filter row would just mean to use the full corpus and don't apply a
MetadataFilter.
If we don't go with 1. all the points below apply.
-
The operator and value field are not cleared when first selecting something and afterwards switching back to the "Empty filter"
-
When doing 2. with the
INorNOT_INoperator, the value field is not disabled
| disabled=True, | ||
| ) | ||
|
|
||
| self.multi_value_select = pn.widgets.MultiChoice.from_param( |
There was a problem hiding this comment.
-
I like the idea and the widget in general, but right now I'm only able to see a single row
Plus, it also leads to another vertical scroll bar so we now have two scroll bars on a modal, which is a big no-no according to Smera.
-
Not sure where this is handled, but the value is extracted wrong from the widget. For example if I select the following filter in the modal
I get the following output in the chat history
There are two issues here:
- It did pick a different document as I did select. In my test I couldn't make out a pattern other than it only ever picked a single one.
- The value is still a string rather than a list of strings. This leads to errors in the backed, e.g. Chroma, as reported in a previous round of review.
(Separate branch from #484 to ease initial review.)