Skip to content

fix(subagents): show that a model or effort pin costs each subagent its own runtime - #23510

Merged
bolichen97 merged 1 commit into
mainfrom
fix/subagent-pin-memory-23484
Oct 23, 2026
Merged

bolichen97 merged 1 commit into
mainfrom
fix/subagent-pin-memory-23484

Conversation

@bolichen97

@bolichen97 bolichen97 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Problem / Motivation

Goal: Tell users and agents that a Subagent Model or Subagent Effort pin starts every subagent in its own runtime, about 0.55 GB of memory each, where they choose the pin and where they spawn.

On a 16 GB Mac, four pinned subagents added about 2.2 GB and pushed the host into swap (#23484). Any model or effort pin turns session sharing off for every subagent. Each one then starts its own kiro-cli-chat plus three MCP servers. Nothing in Settings or in the spawn_run reply says so. The role-model help even recommends a pin to save credits.

Why it matters

A user who pins a cheaper subagent model to save credits pays a hidden ~0.55 GB per subagent. On 16 GB machines that makes KiroCrew close to unusable while subagents run, and the user only finds the cause in Activity Monitor.

Not a goal

What changed (motivation → approach → change)

Users pick a pin without seeing its memory cost, because sharing silently turns off. This PR states the cost at the two places a pin is chosen or used.

Where What it says now
Settings → Chat → Models → Subagents a line under the heading: setting Subagent Model or Subagent Effort off its default runs each subagent in its own process, about 0.55 GB each, which can slow a 16 GB machine; on both defaults they can share the chat's process (new key in all 13 website/src/i18n catalogs)
agent.role_models / agent.role_efforts help (config/sections_agent.py, config-baseline.json) the same cost for a subagent pin
POST /api/spawn reply an additive runtime_note key when a per-call or role pin applies
spawn_run receipt (mcp_tools/spawn.py) one ℹ <ids>: … line per batch from that key

The reply reads the run's own _sharing_plan, the one decision the start and admission already take: the note is added when that plan carries a model or effort, so it covers per-call values, role pins and routed models, and cannot drift from the rule. Its text, PINNED_RUNTIME_NOTE, lives in subagent_parts/model_choice.py, reaching the handler (dashboard/messaging_api/spawn.py) through src/kiro_crew/subagent.py and the dashboard/handlers/messaging.py facade. It is reporting only and never changes a start.

Backwards compatibility

Compatible: the runtime_note key is additive and optional, and an older client ignores it. No start, share decision or default changes.

Removes nothing: the diff only adds text, one optional reply key and one receipt line.

Tests

  • test/test_spawn_pinned_runtime_note.py: POST /api/spawn adds the key when the run's sharing plan carries a model, an effort or both, omits it for a plan with neither (shared or dedicated for another reason), reads the plan of the run it just spawned, and never fails the spawn on an unreadable plan; the receipt renders it once per batch and never ahead of an Error: first line.
  • website/src/test/ChatPanel.subagentPinCost.test.tsx: the Subagents section shows the cost line after its heading.

Proof: not provable -- other: the fix adds a new helper, so reverting every production hunk makes the new test file fail at import (prove.py INCONCLUSIVE, collection only)

Manual verification

N/A for a live run: unit coverage pins the reply key and the receipt line, and nothing changes how a run starts. The Settings line was checked by eye in the built SPA, dark and light (see Screenshots).

Screenshots / video

Settings → Chat → Models → Subagents, captured from the built SPA with the shared gateway-free API stub (website/scripts/lib/stub-dashboard-api.mjs). The new line is the second muted line under the heading.

Dark:

Subagents section, dark theme

Light:

Subagents section, light theme

Related Issues

Part of #23484

Sibling sweep

Swept: git grep -n "role_model_auto_hint\|role_effort_hint\|model_for_spawned_sub_agents" -- website/src src
Sibling: website/src/pages/settings/ChatPanel.tsx -- changed here -- the Background section shares the hints but a background pin starts no subagent runtime
Sibling: src/kiro_crew/mcp_tools/spawn_parts/schemas.py -- not affected -- the reasoning_effort field already says it forces the dedicated process

Pattern harvest

Not generalizable: the cost is specific to the subagent sharing plan; the general memory fix is tracked in #23509.

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|style|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable)
  • No secrets, credentials, or internal references in the diff

@bolichen97
bolichen97 requested review from a team and KiroCrewCaptain as code owners October 23, 2026 17:28
@bolichen97
bolichen97 requested a review from CrysisDeu October 23, 2026 17:28
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Oct 23, 2026
@bolichen97
bolichen97 enabled auto-merge (squash) October 23, 2026 17:35
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Design Review (Opus 5.5) — ✅ PASS

Design-level review of 93d1f9718368cfca8bc5837da92a33f58916ca39 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

Design-Verdict: PASS

No design-level concerns: the change only reports, it never changes how a run starts, and the note is derived from the run's own sharing plan.

Notes

  • api_spawn builds its "would it share without the pin" answer from private manager methods (_sharing_plan, _should_use_session_sharing on a copy.copy(info)). That means the HTTP handler holds sharing rules that belong to the manager. One public manager method, such as pin_forced_dedicated(info) -> bool, would keep that rule in one owner.
  • The description says the note appears "when that plan carries a model or effort". The code also skips the note for kept runs and member runs, and when the run could not share even without the pin. The code's rule is the better one; the description describes a looser rule than the one shipped.

[DESIGN-REVIEWED] 93d1f97

Description read

[DESCRIPTION-READ] 102941c1e977d78a656c190c13b9fdfb88ffb4f95de07e5558ff0d51daa702c8

That sha256 names everything this verdict read: the PR description, plus the 1 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

UX Review (Opus 5.5) — ✅ PASS

UX-level review of 93d1f9718368cfca8bc5837da92a33f58916ca39 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

UX-Verdict: PASS

Nothing blocks. The only change you can see is one static line of text, both screenshots show it, and the blind reader understood the main point: it is "a warning that changing either setting below uses more of my computer's memory and could slow it down."

Notes

  • In subagent_pin_runtime_cost, name the default values instead of saying "its default". The reader "can't tell whether 'Auto (provider picks)' counts as the default", and if it doesn't, they think they are already paying the memory cost. Suggested text: "Setting Subagent Model off Auto, or Subagent Effort off Model default, runs each subagent in its own process…"
  • Replace "the chat's process" with user words such as "can share the chat's memory". The reader said "process" means nothing to them.

[UX-REVIEWED] 93d1f97

Description read

[DESCRIPTION-READ] b9c089b305b62b2140cb0360bf6290618aa4e291d84669f7d3cd59724dbac586

That sha256 names everything this verdict read: the PR description, plus the 5 evidence file(s) the workflow collected from it and handed this review. It covers that evidence because the media strip replaces every attachment URL with the same placeholder, so an edit swapping one attachment for another would leave the description's captured text identical while what this review judged changed. Recompute it with .github/scripts/pr-description-capture.sh over the same inputs; if it differs, the description or that evidence changed after this verdict was formed and any finding drawn from either is unproven.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

First Principles Review (Opus 5.5) — ✅ PASS

Premise-level review of 93d1f9718368cfca8bc5837da92a33f58916ca39 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS does not.

First-Principles-Verdict: PASS

The spawn handler copies part of the share rule into a second place. That is a note, not a block.

Notes

  • Item 5 — oversized: the description says the note "reads the run's own _sharing_plan … and cannot drift from the rule". The handler instead re-checks three things by hand: _selection_kind(info) != "template", info.keep, and a copy with model/routed_model cleared, passed back to _should_use_session_sharing. Those checks restate parts of _sharing_plan_impl (src/kiro_crew/subagent_manager/run_start.py:93) and _should_use_session_sharing_impl (same file, line 64), so if that rule changes, the note can drift from it. Subtraction: drop the selection-kind, keep and counterfactual-copy checks, and gate only on plan.shared is False and (plan.eff_model or plan.eff_effort). The note's text already hedges with "without one it would share".

What this change ships

Inventory (5 items) — 4 justified

Intent: let users and agents see that a subagent model or effort pin costs about 0.55 GB per subagent, before the host starts swapping (#23484). FIX (it fills a missing disclosure).

  1. Settings → Chat → Models → Subagents shows a new line on what a pin costs in memory, in all 13 locales — justified
  2. The help text for agent.role_models and agent.role_efforts, and config-baseline.json, now names the per-subagent runtime cost — justified
  3. The POST /api/spawn reply gains an optional runtime_note key — justified
  4. The spawn_run receipt prints one ℹ <ids>: … line per batch — justified
  5. The spawn handler re-derives when a pin "took the share away" — oversized, hand-copies conditions from _sharing_plan_impl

[FIRST-PRINCIPLES-REVIEWED] 93d1f97

Description read

[DESCRIPTION-READ] 8f24e01301f163ce341908bf96e36db2803866dc16790acbab33d946d545ff52

That sha256 names the PR description this verdict read. Recompute it with .github/scripts/pr-description-capture.sh; if it differs, the description changed after this verdict was formed and any finding drawn from the description is unproven.

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Opus 5.5 Review — ✅ no blocking findings

Reviewed 93d1f9718368cfca8bc5837da92a33f58916ca39 — this comment is updated in place on each push.

Review details

No findings.

Notes:

  • docs/system-specs/modules/subagent.md does not document the new runtime_note response key or its receipt line. The spec covers effort_dropped, effort_applied and effort_pending, but this PR does not update it.

[OPUS-REVIEWED] 93d1f97

Verdict parsed from the review's SHA-scoped output markers for commit 93d1f9718368cfca8bc5837da92a33f58916ca39.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 93d1f9718368cfca8bc5837da92a33f58916ca39: <one-sentence reason>

@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

GPT 6.1 Review — ✅ no blocking findings (all downgraded on adjudication)

GPT 6.1 flagged blocking issues on 93d1f9718368cfca8bc5837da92a33f58916ca39; Opus 5.5 adjudication downgraded every one of them. Downgraded: the remedy each one needs is disproportionate to the harm it prevents. Adjudication downgraded all 1 blocking finding(s): the remedy each one requires is disproportionate to the harm it prevents.

This comment is updated in place on each push.

Review details

BLOCKING -- src/kiro_crew/dashboard/messaging_api/spawn.py:525 -- Runtime note promises sharing despite an AWS profile split

if manager._should_use_session_sharing(unpinned) is not True:
Trigger: A template spawn_run supplies a model pin and a cwd bound to a different AWS profile from its sharing-eligible parent. /api/spawn → _runtime_note clears the model and checks preliminary eligibility, which excludes the profile comparison.
Outcome: The receipt promises “without one it would share the parent chat's runtime,” but _sharing_plan_impl still selects a dedicated runtime without the pin because the profiles differ. Removing the pin does not deliver the advertised memory saving.
Anchor: defect
Fix: Say the run “may otherwise share” rather than promising sharing, or account for the profile-split decision before making that promise.

[BLOCK-MERGE-DOWNGRADED] 93d1f97
[GPT-REVIEWED] 93d1f97

Adjudication (Opus 5.5) — is blocking on each finding proportionate?

F1 is LOW. The only thing wrong is the wording of an informational receipt string. The run itself still picks the correct runtime and is priced correctly.

  • Conditions, all confirmed in code:

    • It must be a template run with a model or effort pin (spawn.py:511-515).
    • The note checks the unpinned copy only against _should_use_session_sharing (spawn.py:521-525), and that check does not compare AWS profiles.
    • The child's cwd must be bound in aws.project_profiles to a different profile from the parent's. That split is applied only in _sharing_plan_impl (run_start.py:136-145), so the run still gets its own runtime and the note's promise of sharing is wrong.
    • Recovery: none is needed. The run goes to its own runtime and is priced as one, as it should be; only the advice in the text is wrong.
  • Cost of the real fix: making the note accurate means calling a second private manager check (the profile-split check), which needs the filesystem lookup run off the loop and its own tests. All of that would exist only to make an advisory sentence exact in a rare setup. That costs more than the harm.

    [ADJUDICATION] 93d1f97 total=1 uphold=0 downgrade=1
    DOWNGRADE F1 src/kiro_crew/dashboard/messaging_api/spawn.py:525 reason=disproportionate-remedy
    [GPT-ADJUDICATED] 93d1f97

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 93d1f9718368cfca8bc5837da92a33f58916ca39: <one-sentence reason>

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Oct 23, 2026
@bolichen97
bolichen97 force-pushed the fix/subagent-pin-memory-23484 branch from a019a81 to ec68b7c Compare October 23, 2026 18:47
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Oct 23, 2026
@bolichen97
bolichen97 force-pushed the fix/subagent-pin-memory-23484 branch 2 times, most recently from 0e226d0 to b3db0c7 Compare October 23, 2026 20:24
@bolichen97

Copy link
Copy Markdown
Contributor Author
  • UX PASS punchline: wording of the new Subagents line

fixed in b3db0c7: the line no longer says "pin". It names the two controls ("Setting Subagent Model or Subagent Effort to anything but its default …") and states the consequence ("which can slow a machine with 16 GB or less"), in all 13 catalogs using each catalog's own field labels. New dark/light screenshots are in the body.
rebutted (visual weight): the line keeps the section's muted 12px style on purpose. It sits directly above both selects it describes, and it covers both of them, so putting it under only the Subagent Model select would hide its Effort half.

@bolichen97

Copy link
Copy Markdown
Contributor Author
  • First Principles PASS: item 4 (copy of the sharing-plan pin rule) and item 6 (PINNED_RUNTIME_NOTE re-export)

Item 6 rebutted: the re-export has a consumer. pinned_runtime_note is an owner function of the kiro_crew.subagent facade and runs on that facade's globals, so PINNED_RUNTIME_NOTE must be bound there. Deleting the line fails test/test_subagent_parts_composition_contract.py::test_every_global_an_owner_function_reads_is_bound_on_the_facade (it reported {'model_choice.pinned_runtime_note': ['PINNED_RUNTIME_NOTE']} before the line was added).
Item 4 accepted-and-deferred: the note reads the same helpers the plan reads (_subagent_default_model / _subagent_default_effort), and api_spawn has no SubagentInfo or _SharingPlan at reply time, so reusing the plan needs a shared pin predicate in run_start.py. That belongs with the runtime-per-pin change in #23509, which rewrites exactly that decision; it is noted there.

@bolichen97

Copy link
Copy Markdown
Contributor Author
  • Design PASS notes: routed_model not covered; predicate duplicates the sharing plan

rebutted (routed_model): info.routed_model is chosen by _route_unpinned_start when the run starts, after POST /api/spawn has already answered, so the reply cannot know it. As the note says, this can only leave a warning out; it never gives a wrong one. A routed start is not a user's pin, which is what this text explains.
accepted-and-deferred (shared predicate): moving the pin rule into one helper that both _sharing_plan_impl and the note call belongs with #23509, which rewrites that same decision for runtime-per-pin sharing.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Oct 23, 2026
@bolichen97

Copy link
Copy Markdown
Contributor Author
  • A second spelling of the sharing plan's pin check

fixed in d62a3d0: pinned_runtime_note is deleted. api_spawn now reads the spawned run's own state.subagents._sharing_plan(info, cfg=config) off the loop and adds runtime_note when that plan's eff_model or eff_effort is set, so the note follows the one decision the start and admission take (routed_model included when it is already known). The re-export of the helper is gone; PINNED_RUNTIME_NOTE reaches the handler through the kiro_crew.subagent and messaging facades, which is its only consumer.

@bolichen97
bolichen97 force-pushed the fix/subagent-pin-memory-23484 branch from b3db0c7 to d62a3d0 Compare October 23, 2026 21:37
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Oct 23, 2026
@bolichen97

bolichen97 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author
  • span=cf6fd1e675ff runtime note credits the pin where subagents can never share

fixed in 93d1f97: _runtime_note now names the pin only when it is what took the share away. It needs a template run, a model or effort in the plan, plan.shared is False, no keep, and _should_use_session_sharing answering True for a copy of the run without its own model. So a non-sharing backend, session_sharing off, an ineligible parent, a kept run and a member run get no note (tests in test/test_spawn_pinned_runtime_note.py). The receipt text and the config help say "its own agent process" instead of kiro-cli.
rebutted (Settings copy): the Settings line says a non-default setting "runs each subagent in its own process", which is true on every backend, and that with both on defaults subagents "can share" the chat's process. It makes no promise on a backend that never shares. The cost it names was measured on the default kiro backend.

@bolichen97
bolichen97 force-pushed the fix/subagent-pin-memory-23484 branch from d62a3d0 to 93d1f97 Compare October 23, 2026 22:24
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Oct 23, 2026

@cruisercohen cruisercohen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at 93d1f97.

The change is reporting only: help text, one Settings line, an optional runtime_note key on the POST /api/spawn reply, and one receipt line in spawn_run. Nothing changes how a run starts or whether it shares. The note reads the run's own _sharing_plan off the loop, asks the "would it share without the pin" question on a shallow copy (the run's own info is never mutated), and any exception in that path drops the note rather than failing the spawn.

Checked on this head merged with current main (b769bf7c99, clean merge):

  • Backend: 24 files around spawn, session sharing, role pins, AWS project profiles, the messaging facade and config baseline, 1037 passed.
  • Frontend: the 22 ChatPanel test files plus the new cost-line test, 301 passed.
  • The new tests pin the change. With the note never returned, 3 cases fail; with the "pin is what took the share away" check removed, 4 fail; with the receipt line dropped, 1 fails.

One non-blocking note: the "would it share without the pin" check calls _should_use_session_sharing, which does not look at the aws.project_profiles split that _sharing_plan also applies. So a pinned run whose cwd binds a different AWS profile from its parent's gets a note saying it would otherwise share the parent's runtime, although it would still run in its own process. It is rare and only affects the wording, so it is fine as a follow-up: the check could also skip the note when the run's profile can differ from its parent's, the same _aws_profile_split_possible test _sharing_plan applies.

@bolichen97
bolichen97 merged commit d3e689c into main Oct 23, 2026
107 checks passed
@bolichen97
bolichen97 deleted the fix/subagent-pin-memory-23484 branch October 23, 2026 23:44
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants