Skip to content

fix(delegation): resolve child fallback chains through the full pin x config matrix (#80450) - #80479

Open
ayushnangia wants to merge 3 commits into
NousResearch:mainfrom
ayushnangia:fix/80450-fallback-matrix
Open

fix(delegation): resolve child fallback chains through the full pin x config matrix (#80450)#80479
ayushnangia wants to merge 3 commits into
NousResearch:mainfrom
ayushnangia:fix/80450-fallback-matrix

Conversation

@ayushnangia

Copy link
Copy Markdown
Contributor

Summary

Completes the delegation fallback-chain matrix from the #80450 cross-PR map: one pure function, _resolve_child_fallback_chain, implementing all six cells of the pin × delegation.fallback_providers decision table — composing the pin semantics of #80465 (@teknium1) with the config-chain semantics of #80438 (@wz-heng) / #80421 (@andrexibiza) for #65038 (@mlahatte), and settling the composition cell that all three leave undefined.

Root cause (the class)

Every open PR on this seam answers "which fallback chain does a non-head agent get?" for one input combination, editing the same parent_fallback hunk. The combinations interact: #80465 nulls the chain on pin, #80438/#80421 install a configured chain — land them in either order and pinned-child semantics differ silently. The matrix needs to be decided once, in one place.

Changes

tools/delegate_tool.py_resolve_child_fallback_chain(parent_agent, delegation_cfg, pinned):

pin delegation.fallback_providers result
yes declared, non-empty declared chain — delegation-scoped recovery honors both explicit intents; the silent drag in #80450 is specifically the parent's chain substituting a pin
yes declared, empty [] none — explicit disable
yes absent none — pin fails loudly (exactly #80465's semantics)
no declared, non-empty declared chain (exactly #80438/#80421's semantics, #65038)
no declared, empty [] none — explicit disable
no absent inherit parent chain (historical behavior, byte-identical)

Malformed declared values log and fall back pin-aware (none when pinned, parent chain otherwise) — extending #80421's log-and-inherit contract so the config-error path can't reintroduce the drag. Entries normalize through the canonical hermes_cli.fallback_config.get_fallback_chain (ordering/dedupe identical to top-level chains). The two pre-existing inheritance tests are made hermetic against the real user config, as #80438 also did.

Validation

  • tests/tools/test_delegate_fallback_matrix.py (14 tests): all six matrix cells, empty-parent edge, normalizer dedupe, pin-aware malformed fallback (both arms), non-dict config, plus four end-to-end wiring tests through _build_child_agent including pin+declared. Red on main: the suite fails collection (resolver doesn't exist); the wiring behaviors are unreachable.
  • Neighbors: test_delegate.py + test_async_delegation.py + test_fallback_config.py — 88/88 via scripts/run_tests.sh.
  • Interplay verified: the executable class map in test(delegation): executable class map for fallback-chain resolution (#80450) #80474 flips its three xfails to XPASS against this branch (1 passed, 1 skipped, 3 xpassed) — the map and this fix compose as designed.

Scope notes

…n x config matrix

Composes NousResearch#80465's pin semantics with NousResearch#80438/NousResearch#80421's
delegation.fallback_providers semantics (NousResearch#65038) and settles the
composition cell the three PRs leave undefined (NousResearch#80450 map).

_resolve_child_fallback_chain implements the whole decision table in
one pure function: pinned children get no chain unless the delegation
section declares one (a delegation-scoped chain honors both explicit
intents; the silent drag in NousResearch#80450 is specifically the PARENT chain
substituting a pin); an explicit empty list disables fallback; absent
config preserves historical parent inheritance exactly. Malformed
declared values log and fall back PIN-AWARE — never the parent chain on
a pinned child, so the config-error path cannot reintroduce the drag.
Entries normalize through the canonical get_fallback_chain.

The two existing inheritance tests are made hermetic against the real
user config (as NousResearch#80438 also did).
Adopted from PR NousResearch#80421 with the author's explicit go-ahead on NousResearch#80450
('Please proceed!'): config_defaults entry, cli-config.yaml.example
block, and user-guide docs for the delegation-scoped fallback chain.

Co-authored-by: Andrex Ibiza, MBA <84248988+andrexibiza@users.noreply.github.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/delegate Subagent delegation area/config Config system, migrations, profiles sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 6, 2026
@ayushnangia

Copy link
Copy Markdown
Contributor Author

Update per @andrexibiza's go-ahead on #80450: absorbed the complementary, non-overlapping parts of #80421 — the config_defaults entry, cli-config.yaml.example block, and user-guide docs for delegation.fallback_providers — as 29bcac3 with Co-authored-by credit. This PR now carries the full surface of the config-chain member (behavior + defaults + docs) alongside the matrix; #80438 remains a compatible alternative for the behavior half.

@spfcraze

spfcraze commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary:
A delegation.model-only pin (provider inherited) still inherits the parent fallback chain under this matrix, because the resolver keys its pin cell on override_provider alone — the model arm of #80450, which this PR cites, stays outside the fix.

Problems:

  • The new call site passes pinned=bool(override_provider) to _resolve_child_fallback_chain (delegate_tool.py:1625 in head 29bcac3), so an explicit delegation.model without delegation.provider/base_url takes the unpinned column, where the resolver's default (default = None if pinned else parent_chain, :1334) hands the child the parent's chain.
  • On origin/main, _resolve_delegation_credentials returns provider: None for that configuration ("No provider override — child inherits everything from parent"), so a model-only pin is exactly the case where override_provider is empty; delegate_task: pinned delegation.provider/model silently replaced at runtime by inherited parent fallback chain #80450's proposed fix names this arm ("or an explicit delegation.model pin").

Solution:
Key the pin on the model as well — pinned=bool(override_provider or model) at the call site — or state in the description that the matrix covers provider pins only.


Checked against 29bcac3 — the tip of fix/80450-fallback-matrix when this was written — and 0957277, main at the same moment.

…ch#80450)

A delegation.model-only pin (provider inherited) took the unpinned
column because pinned was keyed on override_provider alone, so the
child inherited the parent chain and a mid-run failure could silently
swap the pinned model. model at the call site is creds["model"] --
delegation.model config -- at both call sites, so keying on it adds no
false pins. base_url-only pins already resolve to a provider override.

Caught by review on the PR; two matrix tests added (model-only pin with
absent and declared chains).
@ayushnangia

Copy link
Copy Markdown
Contributor Author

Correct catch — fixed in f536c90. The pin is now keyed on override_provider or model: at both call sites model is creds["model"], i.e. delegation.model config, so keying on it adds no false pins (per-task/LLM-chosen models never flow through that parameter), and base_url-only pins already resolve to a provider override in _resolve_delegation_credentials. Two matrix tests added for the model arm (absent chain → no inheritance; declared chain → declared), red before the fix. The resolver docstring now states the full pin definition: provider, base_url, or model-only.

@ayushnangia

Copy link
Copy Markdown
Contributor Author

Triage update: #80438 closed into this family (wz-heng's comment cites this PR as the canonical coverage) — and #65052 is the config-plumbing layer beneath this one. Landing order note posted on #65052; the three PRs are layered, not competing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/delegate Subagent delegation type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants