-
Notifications
You must be signed in to change notification settings - Fork 8.2k
feat: add variant support for subagents (#7138) [alt of #7140] #7156
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
base: dev
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
|
|
@@ -247,6 +247,9 @@ function App() { | |
| }) | ||
| local.model.set({ providerID, modelID }, { recent: true }) | ||
| } | ||
| if (args.variant) { | ||
| local.model.variant.set(args.variant) | ||
| } | ||
| if (args.sessionID) { | ||
|
Comment on lines
+250
to
253
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. Let's do this part separately if at all, this doesnt have error handling for if the model actually supports the variants does it? Let's try to stay scoped to what we need to do to get mvp working |
||
| route.navigate({ | ||
| type: "session", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,10 +9,12 @@ export function DialogAgent() { | |
|
|
||
| const options = createMemo(() => | ||
| local.agent.list().map((item) => { | ||
| const desc = item.native ? "native" : item.description | ||
| const variant = item.variant ? ` (${item.variant})` : "" | ||
| return { | ||
| value: item.name, | ||
| title: item.name, | ||
| description: item.native ? "native" : item.description, | ||
| description: desc ? desc + variant : variant.trim(), | ||
|
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. not necessary to include variant here i dont think |
||
| } | ||
| }), | ||
| ) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1278,7 +1278,13 @@ function AssistantMessage(props: { message: AssistantMessage; parts: Part[]; las | |
| ▣{" "} | ||
| </span>{" "} | ||
| <span style={{ fg: theme.text }}>{Locale.titlecase(props.message.mode)}</span> | ||
| <span style={{ fg: theme.textMuted }}> · {props.message.modelID}</span> | ||
| <span style={{ fg: theme.textMuted }}> | ||
| {" "} | ||
| · {props.message.providerID}/{props.message.modelID} | ||
| </span> | ||
| <Show when={props.message.variant}> | ||
| <span style={{ fg: theme.textMuted }}> · {props.message.variant}</span> | ||
|
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. Not necessary |
||
| </Show> | ||
| <Show when={duration()}> | ||
| <span style={{ fg: theme.textMuted }}> · {Locale.duration(duration())}</span> | ||
| </Show> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,6 +71,10 @@ export const TuiThreadCommand = cmd({ | |
| .option("agent", { | ||
| type: "string", | ||
| describe: "agent to use", | ||
| }) | ||
| .option("variant", { | ||
| type: "string", | ||
| describe: "model variant (provider-specific reasoning effort, e.g., none, low, medium, high, xhigh, max)", | ||
| }), | ||
| handler: async (args) => { | ||
| // Resolve relative paths against PWD to preserve behavior when using --cwd flag | ||
|
Comment on lines
+74
to
80
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. not necessary |
||
|
|
@@ -149,6 +153,7 @@ export const TuiThreadCommand = cmd({ | |
| sessionID: args.session, | ||
| agent: args.agent, | ||
| model: args.model, | ||
| variant: args.variant, | ||
|
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. ditto |
||
| prompt, | ||
| }, | ||
| onExit: async () => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,8 +98,8 @@ export namespace LLM { | |
| system.push(header, rest.join("\n")) | ||
| } | ||
|
|
||
| const variant = | ||
| !input.small && input.model.variants && input.user.variant ? input.model.variants[input.user.variant] : {} | ||
| const selectedVariant = input.user.variant ?? input.agent.variant | ||
| const variant = !input.small && input.model.variants && selectedVariant ? input.model.variants[selectedVariant] : {} | ||
|
Comment on lines
+101
to
+102
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. this looks pretty messy now, prolly switch to iife invocation w/ if statements |
||
| const base = input.small | ||
| ? ProviderTransform.smallOptions(input.model) | ||
| : ProviderTransform.options({ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -386,6 +386,7 @@ export namespace MessageV2 { | |
| }), | ||
| }), | ||
| finish: z.string().optional(), | ||
| variant: z.string().optional(), | ||
|
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. shouldnt be necessary |
||
| }).meta({ | ||
| ref: "AssistantMessage", | ||
| }) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -339,6 +339,7 @@ export namespace SessionPrompt { | |
| }, | ||
| modelID: taskModel.id, | ||
| providerID: taskModel.providerID, | ||
| variant: lastUser.variant, | ||
|
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. assistant message shouldnt need variants?
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. right? |
||
| time: { | ||
| created: Date.now(), | ||
| }, | ||
|
|
@@ -542,6 +543,7 @@ export namespace SessionPrompt { | |
| }, | ||
| modelID: model.id, | ||
| providerID: model.providerID, | ||
| variant: lastUser.variant, | ||
|
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. ditto |
||
| time: { | ||
| created: Date.now(), | ||
| }, | ||
|
|
@@ -837,7 +839,7 @@ export namespace SessionPrompt { | |
| agent: agent.name, | ||
| model: input.model ?? agent.model ?? (await lastModel(input.sessionID)), | ||
| system: input.system, | ||
| variant: input.variant, | ||
| variant: input.variant ?? agent.variant, | ||
| } | ||
|
|
||
| const parts = await Promise.all( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,8 @@ const parameters = z.object({ | |
| subagent_type: z.string().describe("The type of specialized agent to use for this task"), | ||
| session_id: z.string().describe("Existing Task session to continue").optional(), | ||
| command: z.string().describe("The command that triggered this task").optional(), | ||
| model: z.string().describe("Override the subagent's model (format: providerID/modelID)").optional(), | ||
|
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 don't want to introduce new things to task tool |
||
| variant: z.string().describe("Override the subagent's variant (e.g. 'low', 'medium', 'high')").optional(), | ||
| }) | ||
|
|
||
| export const TaskTool = Tool.define("task", async (ctx) => { | ||
|
|
@@ -99,10 +101,23 @@ export const TaskTool = Tool.define("task", async (ctx) => { | |
| const msg = await MessageV2.get({ sessionID: ctx.sessionID, messageID: ctx.messageID }) | ||
| if (msg.info.role !== "assistant") throw new Error("Not an assistant message") | ||
|
|
||
| const model = agent.model ?? { | ||
| modelID: msg.info.modelID, | ||
| providerID: msg.info.providerID, | ||
| } | ||
| // Model priority: param override > agent config > parent context | ||
| const model = (() => { | ||
| if (params.model) { | ||
| const [providerID, ...rest] = params.model.split("/") | ||
| const modelID = rest.join("/") | ||
| if (!providerID || !modelID) { | ||
| throw new Error(`Invalid model format: ${params.model}. Expected: providerID/modelID`) | ||
| } | ||
| return { providerID, modelID } | ||
| } | ||
| return ( | ||
| agent.model ?? { | ||
| modelID: msg.info.modelID, | ||
| providerID: msg.info.providerID, | ||
| } | ||
| ) | ||
| })() | ||
|
|
||
| ctx.metadata({ | ||
| title: params.description, | ||
|
|
@@ -152,6 +167,10 @@ export const TaskTool = Tool.define("task", async (ctx) => { | |
| providerID: model.providerID, | ||
| }, | ||
| agent: agent.name, | ||
| // Variant priority: param override > subagent's configured variant > undefined. | ||
| // Model priority: param override > subagent's configured model > parent's model. | ||
| // This allows orchestrator agents to dynamically assign model+variant per task. | ||
| variant: params.variant ?? agent.variant, | ||
| tools: { | ||
| todowrite: false, | ||
| todoread: false, | ||
|
|
||
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.
not necessary