-
Notifications
You must be signed in to change notification settings - Fork 259
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
refactor(2665): send system message once in InferTypeName generation #2676
refactor(2665): send system message once in InferTypeName generation #2676
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2676 +/- ##
==========================================
- Coverage 86.09% 85.94% -0.16%
==========================================
Files 254 254
Lines 24486 24529 +43
==========================================
Hits 21081 21081
- Misses 3405 3448 +43 ☔ View full report in Codecov by Sentry. |
…ntaining previous API
|
||
async fn ask_with_context( | ||
&self, | ||
system_messages: &[ChatMessage], |
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.
can remove system_messages
if we are not using it.
question: &Question, | ||
) -> Result<Answer> { | ||
// Remove the conversion to ChatRequest | ||
self.wizard.as_ref().unwrap().ask(question.clone()).await |
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.
our goal was to send system message only once but question itself contains the system message.
@@ -10,6 +10,7 @@ use crate::core::config::Config; | |||
#[derive(Default)] | |||
pub struct InferTypeName { | |||
secret: Option<String>, | |||
wizard: Option<Wizard<Question, Answer>>, |
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.
why it's optional?
Moving to draft to reduce noise and improve CI efficiency. Once you are ready just mark it as "ready to review". Feel free to give a shoutout on the #contributors channel on Discord if you want immediate attention. |
Action required: PR inactive for 5 days. |
Action required: PR inactive for 5 days. |
PR closed after 10 days of inactivity. |
Summary:
Refactored the InferTypeName logic to optimize the system message handling. System messages are now sent only once during the type name generation process, and user messages are sent individually for each type, reducing redundant message transmission.
Issue Reference(s):
Fixes #2665
/claim #2665
Build & Testing:
cargo test
successfully../lint.sh --mode=fix
to fix all linting issues raised by./lint.sh --mode=check
.Checklist:
<type>(<optional scope>): <title>