feat(wren-ai-service): sql-expansion #667
Conversation
cyyeh
commented
Sep 13, 2024
•
cyyeh
commented
- refactoring
- remove global variables
- restructuring pipelines
- fix test command in Justfile
- sql-expansion
cyyeh
added
module/ai-service
cyyeh
force-pushed
the
feat/ai-service/sql-add-columns
branch
from
a33ebdc
to
201abcf
Compare
|
there is issue in openai api currently, and we can safely merge this PR if no other issues |
| @@ -16,7 +17,7 @@ | |||
|
|
|||
|
|
|||
| @component | |||
| class GenerationPostProcessor: | |||
| class SQLBreakdownGenerationPostProcessor: | |||
There was a problem hiding this comment.
nitpick: its name seems a bit long, I think might be we can consider a shorten name for it. such as SQLBreakdownGenPostProcessor, SQLBreakdownPostProcessor, SQLBreakdownProcessor, etc... wdyt?
There was a problem hiding this comment.
fixed, use SQLBreakdownGenPostProcessor. I also changed SQLGenerationPostProcessor to SQLGenPostProcessor
| ) | ||
| self.sql_regeneration_post_processor = GenerationPostProcessor(engine=engine) | ||
| self._components = { | ||
| "sql_regeneration_preprocesser": SQLRegenerationRreprocesser(), |
There was a problem hiding this comment.
here seems a typo.
| "sql_regeneration_preprocesser": SQLRegenerationRreprocesser(), | |
| "sql_regeneration_preprocessor": SQLRegenerationPreprocessor(), |
| "sql_regeneration_prompt_builder": PromptBuilder( | ||
| template=sql_regeneration_user_prompt_template | ||
| ), | ||
| "sql_regeneration_generator": llm_provider.get_generator( | ||
| system_prompt=sql_regeneration_system_prompt | ||
| ), | ||
| "sql_regeneration_post_processor": SQLBreakdownGenerationPostProcessor( |
There was a problem hiding this comment.
I think we could consider removing the prefix sql_regeneration to be more concise.
| self._score_filter = ScoreFilter() | ||
| # todo: add a llm filter to filter out low scoring document | ||
| self._output_formatter = OutputFormatter() | ||
| _store = store_provider.get_store(dataset_name="view_questions") |
There was a problem hiding this comment.
nitpick: I do believe there is you just forgot removing the underscore. 🤣
| @@ -96,7 +96,7 @@ async def execute_sql( | |||
| if response.status == 204: | |||
| return True, None, None | |||
|
|
|||
| res = await response.json() | |||
| res = await response.text() | |||
There was a problem hiding this comment.
curiously, why did you change to use text method to get the response?
