Skip to content

refactor: refactor prompt builder #30

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 14, 2024
Merged

refactor: refactor prompt builder #30

merged 3 commits into from
May 14, 2024

Conversation

micpst
Copy link
Collaborator

@micpst micpst commented May 14, 2024

This PR removes hard-coded PromptBuilder in LLMClient and fixes issues with prompt builder for new LLM models integrations.

@micpst micpst self-assigned this May 14, 2024
@github-actions GitHub Actions
Copy link

Trivy scanning results.

@github-actions GitHub Actions
Copy link

github-actions bot commented May 14, 2024 •
edited
Loading

badge

Code Coverage Summary

Filename                                                  Stmts    Miss  Cover    Missing
------------------------------------------------------  -------  ------  -------  -------------------------------------------------
dbally/_main.py                                              12       0  100.00%
dbally/_types.py                                              8       1  87.50%   24
dbally/collection.py                                         92       1  98.91%   263
dbally/assistants/base.py                                    23       0  100.00%
dbally/assistants/openai.py                                  59       2  96.61%   59-76
dbally/audit/event_span.py                                    8       0  100.00%
dbally/audit/event_tracker.py                                34       6  82.35%   35, 48, 59, 69, 86, 92
dbally/audit/event_handlers/base.py                          15       0  100.00%
dbally/audit/event_handlers/cli_event_handler.py             42      27  35.71%   9-12, 38-39, 42-46, 56-58, 70-78, 90-93, 104-107
dbally/audit/event_handlers/langsmith_event_handler.py       25      21  16.00%   6-92
dbally/data_models/audit.py                                  22       0  100.00%
dbally/data_models/execution_result.py                       14       0  100.00%
dbally/embedding_client/base.py                               5       0  100.00%
dbally/embedding_client/openai.py                            19      14  26.32%   20-30, 42-52
dbally/iql/_exceptions.py                                    24       1  95.83%   33
dbally/iql/_processor.py                                     78       7  91.03%   15, 43, 60, 66, 72, 81, 87
dbally/iql/_query.py                                         12       1  91.67%   7
dbally/iql/_type_validators.py                               39       2  94.87%   24, 28
dbally/iql/syntax.py                                         36       9  75.00%   6-9, 27, 36, 60, 63-66
dbally/iql_generator/iql_generator.py                        30       0  100.00%
dbally/iql_generator/iql_prompt_template.py                  12       1  91.67%   38
dbally/llm_client/base.py                                    44       3  93.18%   52-53, 74
dbally/llm_client/openai_client.py                           37      37  0.00%    1-98
dbally/nl_responder/nl_responder.py                          31       5  83.87%   79, 86-95
dbally/nl_responder/nl_responder_prompt_template.py           7       0  100.00%
dbally/nl_responder/query_explainer_prompt_template.py        7       0  100.00%
dbally/nl_responder/token_counters.py                        23      10  56.52%   24-25, 53-63
dbally/prompts/common_validation_utils.py                    15       0  100.00%
dbally/prompts/prompt_builder.py                             21       3  85.71%   7, 27-28
dbally/prompts/prompt_template.py                            28       2  92.86%   27, 35
dbally/similarity/chroma_store.py                            35       0  100.00%
dbally/similarity/detector.py                                73       6  91.78%   21, 200-204
dbally/similarity/faiss_store.py                             36      33  8.33%    5-94
dbally/similarity/fetcher.py                                  5       0  100.00%
dbally/similarity/index.py                                   18       0  100.00%
dbally/similarity/sqlalchemy_base.py                         40      21  47.50%   17, 35-37, 48-50, 59, 68-71, 81-87, 105-108
dbally/similarity/store.py                                    7       0  100.00%
dbally/utils/errors.py                                        2       0  100.00%
dbally/view_selection/base.py                                 7       0  100.00%
dbally/view_selection/llm_view_selector.py                   19       0  100.00%
dbally/view_selection/random_view_selector.py                10      10  0.00%    1-35
dbally/view_selection/view_selector_prompt_template.py       11       2  81.82%   32-33
dbally/views/base.py                                          8       0  100.00%
dbally/views/decorators.py                                    6       0  100.00%
dbally/views/exposed_functions.py                            33       1  96.97%   24
dbally/views/methods_base.py                                 34       2  94.12%   75, 83
dbally/views/pandas_base.py                                  33       1  96.97%   64
dbally/views/sqlalchemy_base.py                              37       7  81.08%   48, 63-65, 83-87
dbally/views/structured.py                                   34       0  100.00%
dbally/views/freeform/text2sql/_autodiscovery.py            113      18  84.07%   107-108, 272, 275, 288-300, 304, 319-321, 333-334
dbally/views/freeform/text2sql/_config.py                    20       0  100.00%
dbally/views/freeform/text2sql/_errors.py                     6       3  50.00%   10-12
dbally/views/freeform/text2sql/_view.py                      48       6  87.50%   84, 88-91, 94
dbally_cli/main.py                                            5       5  0.00%    1-11
dbally_cli/similarity.py                                     21       0  100.00%
TOTAL                                                      1483     268  81.93%

Diff against main

Filename                                      Stmts    Miss  Cover
------------------------------------------  -------  ------  --------
dbally/iql_generator/iql_generator.py            -2       0  +100.00%
dbally/llm_client/base.py                        +3      -3  +7.81%
dbally/view_selection/llm_view_selector.py       -2       0  +100.00%
TOTAL                                            -1      -3  +0.19%

Results for commit: 4a9cdd1

Minimum allowed coverage is 60%

♻️ This comment has been updated with latest results

ludwiktrammer
ludwiktrammer reviewed May 14, 2024
View reviewed changes
src/dbally/prompts/prompt_builder.py
@@ -13,19 +13,19 @@ class PromptBuilder:
def __init__(self, model_name: Optional[str] = None) -> None:
"""
Args:
model_name: Name of the model to load a tokenizer for.
Tokenizer is used to append special tokens to the prompt. If empty, no tokens will be added.
model_name: name of the tokenizer model to use. If provided, the tokenizer will convert the prompt to the
Copy link
Contributor

Choose a reason for hiding this comment

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

nit (non-blocking): maybe the name hg_model_name would make the fact that this is only for Hugging Face models (and one shouldn't put the names of other models here) more explicit? Or maybe even we should have a separate HuggingFacePromptBuilder (probably inheriting from PromptBuilder) so the general PromptBuilder doesn't containg HG-specific stuff?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I like the idea of a separate class. We don't use Hugging Face stuff yet, but we could keep it for future integrations.

@micpst micpst merged commit 61f1066 into main May 14, 2024
@micpst micpst deleted the mp/refactor-prompt-builder branch May 14, 2024 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@ludwiktrammer ludwiktrammer ludwiktrammer approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@micpst micpst

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@micpst @ludwiktrammer