-
Notifications
You must be signed in to change notification settings - Fork 1.5k
chore(wren-ai-service): refine sql generation #784
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
Conversation
| api_results += [ | ||
| AskResult(**result) for result in valid_sql_summary_results | ||
| ] | ||
| self._ask_results[query_id] = AskResultResponse( | ||
| status="generating", | ||
| response=api_results, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think the results might be more than three. Do we need to remove any after the third candidate?
| api_results += [ | ||
| AskResult(**result) for result in valid_sql_summary_results | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the reference is already pointed to the attribute of response object, thus add new results into this variable, it might be occurring more than three candidates back. i think we need to check how many candidates had before putting them into the array.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the example to show the case
from dataclasses import dataclass
@dataclass
class Demo:
id: str
@dataclass
class Resp:
demos: list[Demo]
results = []
results += [Demo(id="1")]
resp = Resp(demos=results)
print(resp)
results += [Demo(id="2")]
print(resp)
paopa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Uh oh!
There was an error while loading. Please reload this page.