Skip to content

fix(bedrock): inherit modalities from foundation models for inference profiles - #34359

Open
JiaDe-Wu wants to merge 1 commit into
NousResearch:mainfrom
JiaDe-Wu:fix/bedrock-profile-modalities-v2
Open

fix(bedrock): inherit modalities from foundation models for inference profiles#34359
JiaDe-Wu wants to merge 1 commit into
NousResearch:mainfrom
JiaDe-Wu:fix/bedrock-profile-modalities-v2

Conversation

@JiaDe-Wu

Copy link
Copy Markdown
Contributor

discover_bedrock_models() hardcodes inference profiles (us.*, global.*) to TEXT-only modalities. Foundation models report their real inputModalities/outputModalities from ListFoundationModels, but ListInferenceProfiles doesn't return that field, so the profile entries were just stamped with ["TEXT"].

The practical effect: Claude profiles like us.anthropic.claude-sonnet-4-6 support IMAGE input, but they show up as text-only. Anything that filters models by vision capability in the /model picker silently drops them.

Fix

After foundation models are discovered, build a small {model_id: modalities} map. When walking the inference profiles, resolve each one back to its base foundation model and inherit those modalities:

  1. Pull the model ID out of the profile's ARN (arn:aws:bedrock:*::foundation-model/<id>)
  2. If there's no ARN, strip the regional prefix (us.anthropic.claude-v2 -> anthropic.claude-v2)
  3. If neither resolves, keep the TEXT-only default

Tests

Three new cases in TestInferenceProfileModalityInheritance -- ARN-based inheritance, the regional-prefix fallback, and the no-match default. Full test_bedrock_adapter.py passes (121 tests) on Python 3.11.

Reported by @ptlally in #7920.

… profiles

discover_bedrock_models() hardcoded inference profiles (us.*, global.*)
to TEXT-only input/output modalities. Claude inference profiles support
IMAGE input, but this was lost — excluding them from vision-capable
filtering in the /model picker.

Build a foundation-model modality lookup after foundation discovery, then
resolve each profile's underlying model via its ARN
(arn:aws:bedrock:*::foundation-model/<id>) and inherit those modalities.
Falls back to stripping the regional prefix (us./eu./global./apac.) when
the ARN is absent, and to TEXT-only when no foundation match exists.

3 new tests: ARN inheritance, regional-prefix fallback, TEXT-only default.
121 bedrock_adapter tests passing.

Ref: PR NousResearch#7920 feedback from @ptlally
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have provider/bedrock AWS Bedrock (boto3, IAM) comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels May 29, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused metadata fix. The current-head defect is real: inference profiles are still stamped TEXT-only in agent/bedrock_adapter.py:1206-1207.

Problems

  • The stated /model impact is not currently implemented. _model_flow_bedrock() filters live_models only by ID prefixes/substrings (hermes_cli/model_setup_flows.py:2269-2275) and reduces entries to m["id"] at :2315; it never reads modality metadata. The other production discovery consumers likewise extract IDs only (agent/bedrock_adapter.py:399-401, hermes_cli/models.py:4232-4234).
  • The new tests prove IMAGE input inheritance, but not output_modalities inheritance although the patch changes both fields.

Suggested changes

  • Wire the corrected metadata into the actual capability-filtering path and add a consumer-level regression test, or narrow the stated behavior if no such path currently exists.
  • Add an output-modality inheritance assertion.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/profiles Multi-profile isolation, HERMES_HOME scoping labels Jul 13, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This was generated by AI during triage.

Summary

Two PRs address the Bedrock inference-profile metadata defect: #11132 combines modality inheritance with two unrelated Bedrock fixes, while #34359 carries the remaining modality change as a focused, current-main replacement. Both replace hardcoded TEXT-only profile metadata by resolving the underlying foundation model, inheriting its input and output modalities, and retaining a TEXT fallback when resolution fails.

Related pull requests

  • #11132 [closed] duplicate — (+209/-6) — superseded by #34359: The diff fixes the reported hardcoded-modality cause, but also adds AWS credential detection and Bedrock auxiliary-client support; those two changes later landed separately, leaving its modality portion as the relevant reference implementation and motivating its closure in favor of the focused replacement.
  • #34359 related — (+135/-3) — keep open pending review fixes: The focused diff resolves profile ARNs or strips regional prefixes, copies both modality fields from matching foundation models, and defaults unmatched profiles to TEXT. The contributor keep_open review correctly notes that the claimed /model capability-filtering impact is not present in current consumers and that output-modality inheritance lacks an assertion, so the description should be narrowed or a real consumer path added and the missing assertion supplied before merge.

Duplicates

#11132 and #34359 implement substantially the same inference-profile modality inheritance; #34359 is the rebased, scoped successor, while #11132 also contains unrelated fixes.

Suggested consolidation

Merge #34359 after addressing its keep_open review by removing or implementing the unsupported /model filtering claim and adding an output-modality inheritance assertion; retain #11132 closed as superseded by #34359 rather than reopening it.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    subgraph Dup11132 ["PRs duplicating each other"]
        P11132["PR #11132 (closed)"]
        P34359["PR #34359 (open)"]
    end
    class P11132 closed
    class P34359 open
    class P34359 target
    click P11132 "https://github.com/NousResearch/hermes-agent/pull/11132"
    click P34359 "https://github.com/NousResearch/hermes-agent/pull/34359"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 23 kB of PR diffs, 3 kB of issue/PR text, 2 kB of discussion (4 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@JiaDe-Wu

Copy link
Copy Markdown
Contributor Author

Thanks for the review — both points hold up, and I've verified them against current main.

On the /model impact: correct, and the gap is still open. The flow has since moved from hermes_cli/main.py to hermes_cli/model_setup_flows.py, but the behavior is unchanged — _model_flow_bedrock() reduces discovery output to model_list = [m["id"] for m in deduped], so the metadata this PR corrects is discarded before it reaches the picker, and the summary line prints text model(s) unconditionally. As it stands the PR fixes the metadata with nothing consuming it, so the stated benefit isn't real yet.

What I'm adding:

  • partition_bedrock_models_by_vision() in agent/bedrock_adapter.py, which splits discovered models on IMAGE in input_modalities
  • _model_flow_bedrock() calls it, so the summary reports the actual split instead of asserting everything is text
  • _prompt_model_selection() gains a vision_model_ids argument and tags those rows (vision). Display-only — the returned value is still the bare model ID
  • consumer-level regression tests: an inherited-IMAGE profile classified as vision-capable, missing/empty modality keys treated as text-only, and case-insensitive matching

On the output-modality assertion: agreed, and it needs more care than it first looks. Discovery drops any foundation model whose outputModalities lacks TEXT (the if "TEXT" not in output_mods: continue guard in discover_bedrock_models), so a model that outputs only IMAGE never enters the lookup map at all — a profile pointing at it falls back to the ["TEXT"] default and the assertion would pass for the wrong reason. The test therefore uses a foundation model declaring ["TEXT", "IMAGE"] output: TEXT gets it past the filter, and the inherited IMAGE is what actually proves inheritance rather than the default.

One scope note: this branch predates the main.pymodel_setup_flows.py split, so I'll rebase onto current main and land the picker wiring in its new home rather than patching the old location. Will follow up here once that's pushed.

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

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have provider/bedrock AWS Bedrock (boto3, IAM) sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants