-
Notifications
You must be signed in to change notification settings - Fork 13.9k
Cleanup collaboration mode variants #10404
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
Changes from all commits
d298a6d
0f5230b
63d6910
50d81bd
01dd8a0
8c54e07
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -370,7 +370,7 @@ impl Codex { | |
| // TODO (aibrahim): Consolidate config.model and config.model_reasoning_effort into config.collaboration_mode | ||
| // to avoid extracting these fields separately and constructing CollaborationMode here. | ||
|
Comment on lines
370
to
371
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just want to point out that we need to think hard about migration path forward if we need to do that. Currently we are not even tracking them. |
||
| let collaboration_mode = CollaborationMode { | ||
| mode: ModeKind::Custom, | ||
| mode: ModeKind::Default, | ||
| settings: Settings { | ||
| model: model.clone(), | ||
| reasoning_effort: config.model_reasoning_effort, | ||
|
|
@@ -2628,7 +2628,7 @@ mod handlers { | |
| } => { | ||
| let collaboration_mode = collaboration_mode.or_else(|| { | ||
| Some(CollaborationMode { | ||
| mode: ModeKind::Custom, | ||
| mode: ModeKind::Default, | ||
| settings: Settings { | ||
| model: model.clone(), | ||
| reasoning_effort: effort, | ||
|
|
@@ -4942,7 +4942,7 @@ mod tests { | |
| let model_info = ModelsManager::construct_model_info_offline(model.as_str(), &config); | ||
| let reasoning_effort = config.model_reasoning_effort; | ||
| let collaboration_mode = CollaborationMode { | ||
| mode: ModeKind::Custom, | ||
| mode: ModeKind::Default, | ||
| settings: Settings { | ||
| model, | ||
| reasoning_effort, | ||
|
|
@@ -5025,7 +5025,7 @@ mod tests { | |
| let model_info = ModelsManager::construct_model_info_offline(model.as_str(), &config); | ||
| let reasoning_effort = config.model_reasoning_effort; | ||
| let collaboration_mode = CollaborationMode { | ||
| mode: ModeKind::Custom, | ||
| mode: ModeKind::Default, | ||
| settings: Settings { | ||
| model, | ||
| reasoning_effort, | ||
|
|
@@ -5292,7 +5292,7 @@ mod tests { | |
| let model_info = ModelsManager::construct_model_info_offline(model.as_str(), &config); | ||
| let reasoning_effort = config.model_reasoning_effort; | ||
| let collaboration_mode = CollaborationMode { | ||
| mode: ModeKind::Custom, | ||
| mode: ModeKind::Default, | ||
| settings: Settings { | ||
| model, | ||
| reasoning_effort, | ||
|
|
@@ -5412,7 +5412,7 @@ mod tests { | |
| let model_info = ModelsManager::construct_model_info_offline(model.as_str(), &config); | ||
| let reasoning_effort = config.model_reasoning_effort; | ||
| let collaboration_mode = CollaborationMode { | ||
| mode: ModeKind::Custom, | ||
| mode: ModeKind::Default, | ||
| settings: Settings { | ||
| model, | ||
| reasoning_effort, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -121,7 +121,7 @@ pub enum Feature { | |
| SkillEnvVarDependencyPrompt, | ||
| /// Steer feature flag - when enabled, Enter submits immediately instead of queuing. | ||
| Steer, | ||
| /// Enable collaboration modes (Plan, Code, Pair Programming, Execute). | ||
| /// Enable collaboration modes (Plan, Default). | ||
| CollaborationModes, | ||
|
Comment on lines
124
to
125
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we just remove this? I am not sure if we are going to turn this off at anytime in the future.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess we have to also remove the enable flag on app side as we do that.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense, will do in followup PR |
||
| /// Enable personality selection in the TUI. | ||
| Personality, | ||
|
|
||
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.
Defaults to unset- is this still accurate?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.