Feat/discord summarizer prompt update #47
Conversation
Still errors on retrieve_similar_nodes.py in case of group_by. We need to fix that. Commiting the work to be saved.
|
Warning Rate Limit Exceeded@amindadgar has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 11 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe recent updates enhance the system's ability to handle data aggregation and filtering. New functionalities allow for grouping records by metadata and adding more specific filters in queries. Adjustments in query logic and additional logging improve flexibility and traceability. A new suite of tests ensures the robustness of these enhancements. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| and_filters: dict[str, str] | None = kwargs.get("and_filters", None) | ||
| filters: list[dict[str, str]] = [] | ||
|
|
||
| for node in nodes: | ||
| # the filter made by given node | ||
| filter: dict[str, str] = {} | ||
| filter[metadata_group1_key] = node.metadata[metadata_group1_key] | ||
| filter[metadata_group2_key] = node.metadata[metadata_group2_key] | ||
| # date filter | ||
| filter[metadata_date_key] = node.metadata[metadata_date_key] | ||
|
|
||
| # if more and filters were given | ||
| if and_filters: | ||
| for key, value in and_filters.items(): | ||
| filter[key] = value | ||
|
|
There was a problem hiding this comment.
The integration of and_filters from kwargs is handled correctly. However, consider adding error handling for cases where and_filters might not be a dictionary as expected.
+ if not isinstance(and_filters, dict):
+ raise ValueError("Expected 'and_filters' to be a dictionary.")Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| and_filters: dict[str, str] | None = kwargs.get("and_filters", None) | |
| filters: list[dict[str, str]] = [] | |
| for node in nodes: | |
| # the filter made by given node | |
| filter: dict[str, str] = {} | |
| filter[metadata_group1_key] = node.metadata[metadata_group1_key] | |
| filter[metadata_group2_key] = node.metadata[metadata_group2_key] | |
| # date filter | |
| filter[metadata_date_key] = node.metadata[metadata_date_key] | |
| # if more and filters were given | |
| if and_filters: | |
| for key, value in and_filters.items(): | |
| filter[key] = value | |
| and_filters: dict[str, str] | None = kwargs.get("and_filters", None) | |
| if not isinstance(and_filters, dict): | |
| raise ValueError("Expected 'and_filters' to be a dictionary.") | |
| filters: list[dict[str, str]] = [] | |
| for node in nodes: | |
| filter: dict[str, str] = {} | |
| filter[metadata_group1_key] = node.metadata[metadata_group1_key] | |
| filter[metadata_group2_key] = node.metadata[metadata_group2_key] | |
| filter[metadata_date_key] = node.metadata[metadata_date_key] | |
| # if more and filters were given | |
| if and_filters: | |
| for key, value in and_filters.items(): | |
| filter[key] = value | |
| aggregate_records : bool | ||
| aggregate records and group by a given term in `group_by_metadata` | ||
| group_by_metadata : list[str] | ||
| do grouping by some property of `metadata_` | ||
| """ | ||
| ignore_sort = kwargs.get("ignore_sort", False) | ||
| aggregate_records = kwargs.get("aggregate_records", False) | ||
| group_by_metadata = kwargs.get("group_by_metadata") |
There was a problem hiding this comment.
The handling of aggregate_records and group_by_metadata is implemented correctly. Ensure that group_by_metadata is always a list to avoid type errors during iteration.
+ if not isinstance(group_by_metadata, list):
+ raise ValueError("Expected 'group_by_metadata' to be a list.")Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| aggregate_records : bool | |
| aggregate records and group by a given term in `group_by_metadata` | |
| group_by_metadata : list[str] | |
| do grouping by some property of `metadata_` | |
| """ | |
| ignore_sort = kwargs.get("ignore_sort", False) | |
| aggregate_records = kwargs.get("aggregate_records", False) | |
| group_by_metadata = kwargs.get("group_by_metadata") | |
| aggregate_records : bool | |
| aggregate records and group by a given term in `group_by_metadata` | |
| group_by_metadata : list[str] | |
| do grouping by some property of `metadata_` | |
| """ | |
| ignore_sort = kwargs.get("ignore_sort", False) | |
| aggregate_records = kwargs.get("aggregate_records", False) | |
| group_by_metadata = kwargs.get("group_by_metadata") | |
| if not isinstance(group_by_metadata, list): | |
| raise ValueError("Expected 'group_by_metadata' to be a list.") |
| summary_nodes_filters : list[dict[str, str]] | ||
| a list of filters to fetch the summary nodes | ||
| for default, not passing this would mean to use previous nodes | ||
| but if passed we would re-fetch nodes. | ||
| This could be benefitial in case we want to do some manual | ||
| processing with nodes |
There was a problem hiding this comment.
The addition of summary_nodes_filters allows for flexible filtering of summary nodes. Ensure that this parameter is properly validated to be a list of dictionaries before use.
+ if summary_nodes_filters is not None and not all(isinstance(f, dict) for f in summary_nodes_filters):
+ raise ValueError("Each filter in 'summary_nodes_filters' must be a dictionary.")Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| summary_nodes_filters : list[dict[str, str]] | |
| a list of filters to fetch the summary nodes | |
| for default, not passing this would mean to use previous nodes | |
| but if passed we would re-fetch nodes. | |
| This could be benefitial in case we want to do some manual | |
| processing with nodes | |
| summary_nodes_filters : list[dict[str, str]] | |
| a list of filters to fetch the summary nodes | |
| for default, not passing this would mean to use previous nodes | |
| but if passed we would re-fetch nodes. | |
| This could be benefitial in case we want to do some manual | |
| processing with nodes | |
| if summary_nodes_filters is not None and not all(isinstance(f, dict) for f in summary_nodes_filters): | |
| raise ValueError("Each filter in 'summary_nodes_filters' must be a dictionary.") |
Getting the metadata_ in a way no duplicate data could happen.
note: this test case was just testing each module and no db conection was made, so we moved it to unit tests.
Add group_by_metadata validation Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Tests
Refactor