Skip to content

Support Bedrock application inference profile ARNs as model ids#803

Open
mattwebbio wants to merge 1 commit into
crmne:mainfrom
mattwebbio:bedrock-application-inference-profile-arn
Open

Support Bedrock application inference profile ARNs as model ids#803
mattwebbio wants to merge 1 commit into
crmne:mainfrom
mattwebbio:bedrock-application-inference-profile-arn

Conversation

@mattwebbio

@mattwebbio mattwebbio commented Jun 11, 2026

Copy link
Copy Markdown

What this does

Fixes Bedrock so an application inference profile ARN can be passed as the model id.

Application inference profiles (used for cost-allocation tagging) are invoked by passing their ARN as the modelId. An ARN contains a /:

arn:aws:bedrock:us-west-2:123456789012:application-inference-profile/<profile-id>

The Converse protocol builds the request path with the id raw — "/model/#{@model.id}/converse" — and the SigV4 signer (Bedrock::Auth#canonical_uri) splits the path on / to build the canonical URI. So the ARN's internal / is treated as a path separator in both the request URL and the signed canonical path, truncating the modelId to .../application-inference-profile. Bedrock then rejects the call:

The provided model identifier is invalid

The fix percent-encodes the model id's / (%2F) so the ARN stays a single path segment. canonical_uri then re-encodes each segment, double-encoding it to %252F — which is exactly what SigV4 requires for non-S3 services — so the signed path and the sent path stay consistent.

This is a no-op for ordinary model ids, which contain no /. A : version suffix (e.g. ...-v1:0) is a valid path-segment character and is unaffected.

Files (rebased onto current main after the Converse refactor, so the escaping now lives in the shared Protocols::Converse rather than the old Bedrock::Chat/Streaming):

  • lib/ruby_llm/protocols/converse/chat.rbcompletion_url now escapes the id via a new escape_model_id helper.
  • lib/ruby_llm/protocols/converse/streaming.rbstream_url escapes the id too (converse-stream).
  • spec/ruby_llm/providers/bedrock_spec.rb — new unit spec covering the converse URL, the converse-stream URL, an unchanged ordinary id, and the SigV4 canonical-path encoding.

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Performance improvement

Scope check

  • I read the Contributing Guide
  • This aligns with RubyLLM's focus on LLM communication
  • This isn't application-specific logic that belongs in user code
  • This benefits most users, not just my specific use case

Required for new features

N/A — bug fix in an existing provider, no new public API.

Quality check

  • I ran overcommit --install and all hooks pass
  • I tested my changes thoroughly
    • For provider changes: Re-recorded VCR cassettes with bundle exec rake vcr:record[provider_name]N/A: this change only affects URL/path string construction; it touches no HTTP request body or response handling, and the new spec is a pure unit test (no cassette), consistent with the other top-level provider specs (vertex_ai_auth_spec.rb, ollama_spec.rb, etc.).
    • All tests pass: bundle exec rspec spec/ruby_llm/providers/bedrock_spec.rb (14 examples, 0 failures — the 4 new path-encoding examples alongside the existing Bedrock specs); bundle exec rubocop on the changed files reports no offenses.
  • I updated documentation if needed — none needed.
  • I didn't modify auto-generated files manually (models.json, aliases.json)

AI-generated code

  • I used AI tools to help write this code
  • I have reviewed and understand all generated code (required if above is checked)

API changes

  • Breaking change
  • New public methods/classes
  • Changed method signatures
  • No API changes

@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.89%. Comparing base (8595adb) to head (528c2df).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #803      +/-   ##
==========================================
- Coverage   81.90%   81.89%   -0.01%     
==========================================
  Files         169      169              
  Lines        7713     7715       +2     
  Branches     1284     1284              
==========================================
+ Hits         6317     6318       +1     
  Misses        884      884              
- Partials      512      513       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mattwebbio mattwebbio marked this pull request as ready for review June 11, 2026 03:20
@mattwebbio mattwebbio force-pushed the bedrock-application-inference-profile-arn branch from 8ef5bb5 to 5bfc269 Compare June 24, 2026 20:27
Application inference profiles (used for cost-allocation tagging) are invoked by
passing their ARN as the modelId. The Bedrock provider built the request path as
"/model/#{@model.id}/converse" with the id raw, so an ARN's internal "/"
(".../application-inference-profile/<id>") was parsed as a path separator in both
the request URL and the SigV4 canonical path — AWS then rejected it with
"The provided model identifier is invalid".

Percent-encode the model id's "/" so the ARN stays a single path segment.
canonical_uri re-encodes per segment, which double-encodes it as SigV4 requires for
non-S3 services, keeping the signed and sent paths consistent. No-op for ordinary
model ids (which contain no "/"); ":" version suffixes are valid path-segment
characters and are unaffected.
@mattwebbio mattwebbio force-pushed the bedrock-application-inference-profile-arn branch from 5bfc269 to 528c2df Compare June 24, 2026 20:30
@mattwebbio

Copy link
Copy Markdown
Author

Rebased this onto current main (post the Converse refactor) — CI is green across the matrix. The change moved with the refactor: the /-escaping now lives in Protocols::Converse (completion_url/stream_url via a small escape_model_id helper) rather than the old Bedrock::Chat/Streaming, and the canonical-path double-encoding it relies on is in Bedrock::Auth#canonical_uri. Still a no-op for ordinary model ids; the spec covers the converse + converse-stream URLs and the SigV4 canonical path. I've updated the PR description to match the new locations.

Happy to adjust anything — e.g. fold the escaping somewhere else or tighten the tests — if there's a shape you'd prefer.

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.

1 participant