-
Notifications
You must be signed in to change notification settings - Fork 482
feat(tools): add per-call llm_profile override to the task tool #4510
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
Open
georgeglarson
wants to merge
9
commits into
OpenHands:main
Choose a base branch
from
georgeglarson:feat/task-tool-llm-profile
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
df136cf
feat(tools): add per-call llm_profile override to the task tool
georgeglarson f1c1cb1
refactor(sdk): share cipher-aware profile loading
georgeglarson 09b6f76
fix(task): preserve isolated per-call profile behavior
georgeglarson 38a858e
docs(task): record per-call profile design
georgeglarson d715cba
Merge branch 'main' into feat/task-tool-llm-profile
georgeglarson 10cac63
Merge branch 'main' into feat/task-tool-llm-profile
georgeglarson 7fe2de3
fix(task): retain subscription worker condenser
georgeglarson 61c3464
ci: retrigger checks after GitHub Actions outage
georgeglarson d99d9f0
fix(task): make worker profile selection explicit
georgeglarson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| # Per-call `llm_profile` on the task tool | ||
|
|
||
| ## Problem | ||
|
|
||
| The task tool can delegate work to a subagent, but an inherited subagent always | ||
| uses the parent conversation's LLM. A parent that wants a mixed-model workflow | ||
| must either switch its own model or rely on a static `model:` value in an agent | ||
| definition. Neither supports choosing a worker profile for an individual task | ||
| while the parent keeps its current model. | ||
|
|
||
| ## User-facing behavior | ||
|
|
||
| `TaskAction` gains an optional `llm_profile` field naming a saved LLM profile. | ||
|
|
||
| An inherited agent uses the task call's `llm_profile` when supplied and otherwise | ||
| uses the parent model. Supplying `llm_profile` for an agent definition that already | ||
| selects a model is a configuration conflict and fails explicitly; neither choice | ||
| is silently discarded. | ||
|
|
||
| For resumable tasks, a bare resume retains the task's effective per-call profile; | ||
| an explicit profile on a resume replaces it. Resume remains scoped to the current | ||
| in-memory `TaskManager` lifecycle. | ||
|
|
||
| Unknown profiles fail loudly and become an error `TaskObservation`. The task is | ||
| not partially registered and the worker never silently falls back to the parent | ||
| model. | ||
|
|
||
| ## Shared profile-loading boundary | ||
|
|
||
| `LocalConversation.load_profile_llm()` is the common cipher-aware loading | ||
| primitive for conversation-owned profile operations. It selects the | ||
| conversation's default store or a caller-supplied profile directory, then calls | ||
| `LLMProfileStore.load(profile_name, cipher=self._cipher)`. | ||
|
|
||
| The operation only loads. It does not activate the returned LLM, add it to the | ||
| parent conversation's registry, or bind parent conversation context. | ||
|
|
||
| The existing higher-level methods add their own behavior: | ||
|
|
||
| - `switch_profile()` loads through the primitive, assigns the canonical | ||
| `profile:<name>` usage ID, and activates the LLM; | ||
| - `get_or_create_profile_llm()` returns a registry hit or loads through the | ||
| primitive, assigns the caller's usage ID, registers the LLM, and binds the | ||
| conversation context; | ||
| - `TaskManager` loads through the primitive, resets metrics, and passes the LLM | ||
| into the worker factory. The worker `LocalConversation` then owns registration | ||
| and context binding. | ||
|
|
||
| Using `get_or_create_profile_llm()` directly for a worker would register an | ||
| otherwise unused template LLM in the parent conversation and give it the wrong | ||
| metrics/context owner. Adding a mode flag to that method would also make its | ||
| registry-oriented contract ambiguous. | ||
|
|
||
| `LLMProfileStore.load()` already performs persisted subscription restoration | ||
| through `LLM.from_persisted()`. The loading primitive therefore returns the | ||
| runtime subscription LLM while still avoiding parent registration and | ||
| activation. | ||
|
|
||
| The store's native error contract is preserved: a missing profile raises | ||
| `FileNotFoundError` with the available profile files, while invalid or corrupted | ||
| profiles raise `ValueError`. There is no separate list-before-load operation, so | ||
| validation and reading happen through the store's locked load path. | ||
|
|
||
| ## Worker construction and metrics | ||
|
|
||
| The selected LLM is injected before `factory_func` runs. This matters because a | ||
| factory may derive a default condenser from its input LLM; swapping afterward | ||
| would leave the condenser on the parent model. | ||
|
|
||
| Subscription-backed LLMs retain the factory-created condenser. Subscription | ||
| completion dispatch supports condenser calls, matching top-level agent creation | ||
| and profile switching. | ||
|
|
||
| The loaded LLM gets a fresh metrics object before factory construction. It is | ||
| never registered in the parent's `llm_registry`. The worker conversation tracks | ||
| its own agent and condenser usage, and the existing task completion path copies | ||
| the worker's combined metrics into the parent under `task:<task-id>` exactly | ||
| once. The parent's active model is unchanged. | ||
|
|
||
| ## Profile discovery and confidentiality | ||
|
|
||
| The task tool description lists saved profile names using the same public | ||
| `get_llm_profile_names()` and `format_llm_profiles()` helpers as `switch_llm`. | ||
| It exposes names only: profile model IDs, provider URLs, API keys, and persisted | ||
| JSON are not included. When no profiles exist, the section is omitted. | ||
|
|
||
| The list is a creation-time snapshot of the default profile store. When any | ||
| registered agent specifies a custom `profile_store_dir`, the task tool omits the | ||
| list rather than advertising names from a store that the selected agent may not | ||
| use. | ||
|
|
||
| ## Persistence and compatibility | ||
|
|
||
| `Task.llm_profile` stores the effective per-call profile for resume. Existing task | ||
| calls omit the optional field and continue inheriting the parent model. | ||
|
|
||
| The additive action field has the repository's normal version-skew caveat: an old | ||
| SDK that forbids unknown action fields cannot deserialize a new event containing | ||
| `llm_profile`. | ||
|
|
||
| ## Verification matrix | ||
|
|
||
| Focused tests cover: | ||
|
|
||
| - two sequential tasks choosing different saved profiles; | ||
| - parent model unchanged and no worker profile entry in the parent registry; | ||
| - independent worker metrics and `task:<id>` merge-back; | ||
| - encrypted-secret loading through the conversation cipher; | ||
| - persisted subscription restoration; | ||
| - subscription workers retain their supported LLM-backed condenser; | ||
| - custom profile directories; | ||
| - native missing-profile errors with no partial task state; | ||
| - explicit conflicts between definition-owned models and per-call profiles; | ||
| - bare-resume retention and explicit resume replacement; | ||
| - a clear missing-profile failure when a stored profile is removed before resume; | ||
| - inherited behavior when no override is supplied; | ||
| - pre-factory injection and `stream=False` worker behavior; | ||
| - profile names present while model IDs, provider URLs, and API keys remain absent | ||
| from the tool description; | ||
| - omission of the profile section when no profiles exist. | ||
| - omission of default-store profile names when a registered agent uses a custom | ||
| profile store. | ||
|
|
||
| Current verification on the rebased branch: | ||
|
|
||
| - 235 task, subagent, profile-switch, and conversation-switch tests pass; | ||
| - pre-commit passes on every revised file, including Pyright; | ||
| - a live encrypted-profile delegation completed with a MiniMax M3 parent and a | ||
| MiMo v2.5 Pro worker. The parent remained on MiniMax, its registry contained no | ||
| worker profile entry, its only metrics key was `task:task_00000001`, and a bare | ||
| resume retained the encrypted worker profile and MiMo model. | ||
|
|
||
| ## Out of scope | ||
|
|
||
| - changing the `switch_llm` or subagent defaults; | ||
| - enabling parallel task execution by default; | ||
| - adding frontend worker-model reporting; | ||
| - changing definition-level profile loading; | ||
| - changing the conversation-spawn workflow. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.