Skip to content

0.7.0: remove :adapter_error default retry, narrow AdapterCaller rescue#13

Merged
justi merged 1 commit intomainfrom
feat/remove-adapter-error-default
Apr 21, 2026
Merged

0.7.0: remove :adapter_error default retry, narrow AdapterCaller rescue#13
justi merged 1 commit intomainfrom
feat/remove-adapter-error-default

Conversation

@justi
Copy link
Copy Markdown
Owner

@justi justi commented Apr 21, 2026

Summary

0.7.0 — breaking changes targeting redundancy between ruby_llm-contract and upstream ruby_llm 1.14.1.

  1. :adapter_error removed from DEFAULT_RETRY_ON. New default: [:validation_failed, :parse_error]. ruby_llm's Faraday middleware already retries transport errors (RateLimitError, ServerError, ServiceUnavailableError, OverloadedError, timeouts) with backoff (connection.rb:77), so the previous default re-ran the same model on errors the HTTP layer had already retried — retry × retry with no change in context. Opt in explicitly if needed, paired with escalate for meaningful model fallback.
  2. AdapterCaller narrows rescue StandardErrorrescue ::RubyLLM::Error. Programmer errors (NoMethodError, ArgumentError, config bugs) now propagate instead of being silently coerced into :adapter_error and retried. Provider errors still produce :adapter_error as before.

Why not a 0.6.x deprecation first?

Closed the previous PR #12 (which added a deprecation warning in 0.6.5) because this gem has no real external user base. A deprecation → grace period → breaking pattern is enterprise theater when nobody is between releases to see the warning. Ship the change, document migration clearly.

Migration

Restore pre-0.7 behavior (if you were relying on :adapter_error retry):

retry_policy do
  attempts 3
  retry_on :validation_failed, :parse_error, :adapter_error
end

Preferred pattern — pair with a model fallback chain:

retry_policy do
  escalate "gpt-4.1-nano", "gpt-4.1-mini"
  retry_on :validation_failed, :parse_error, :adapter_error
end

Test plan

  • bundle exec rspec — 1336 examples, 0 failures, 8 pending (all pending are API-key-gated live LLM tests)
  • bundle exec rubocop lib/ruby_llm/contract/step/retry_policy.rb lib/ruby_llm/contract/step/adapter_caller.rb — no offenses
  • Updated existing specs: :adapter_error retry tests now use explicit retry_on :adapter_error opt-in; adapter failure fixtures raise RubyLLM::Error instead of StandardError; adversarial test inverted (:adapter_error is NOT retryable by default + new test for explicit opt-in).
  • Version bump 0.6.40.7.0; Gemfile.lock updated.

Copilot AI review requested due to automatic review settings April 21, 2026 12:08
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces the 0.7.0 breaking changes to retry behavior and adapter error handling to reduce redundant retries and avoid masking programmer/configuration errors.

Changes:

  • Remove :adapter_error from RetryPolicy::DEFAULT_RETRY_ON (now defaults to [:validation_failed, :parse_error]).
  • Narrow AdapterCaller rescue from StandardError to ::RubyLLM::Error, allowing non-provider errors to propagate.
  • Update specs, changelog, and version/lockfile for the 0.7.0 release and new opt-in behavior.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
spec/ruby_llm/contract/step/runner_spec.rb Updates adapter failure fixture to raise RubyLLM::Error.
spec/ruby_llm/contract/step/retry_policy_spec.rb Updates default retryable statuses expectation to exclude :adapter_error.
spec/ruby_llm/contract/step/retry_integration_spec.rb Makes :adapter_error retry behavior explicit via retry_on :adapter_error and updates fixtures.
spec/ruby_llm/contract/edge_cases_spec.rb Updates adapter failure fixture to raise RubyLLM::Error.
spec/ruby_llm/contract/adversarial_round10_spec.rb Inverts default retryability expectation for :adapter_error and adds explicit opt-in test.
spec/integration/step_end_to_end_spec.rb Updates adapter failure fixture to raise RubyLLM::Error.
lib/ruby_llm/contract/version.rb Bumps gem version to 0.7.0.
lib/ruby_llm/contract/step/retry_policy.rb Changes retry defaults and documents the breaking rationale.
lib/ruby_llm/contract/step/adapter_caller.rb Narrows rescue to ::RubyLLM::Error and preserves :adapter_error mapping for provider errors.
Gemfile.lock Updates lockfile entry for ruby_llm-contract to 0.7.0.
CHANGELOG.md Adds 0.7.0 release notes and migration guidance.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +12 to +15
# Breaking (0.7.0): was `rescue StandardError`. Now only catches provider
# errors from ruby_llm (RubyLLM::Error hierarchy). Programmer errors
# (NoMethodError, ArgumentError, etc.) propagate instead of being silently
# coerced into :adapter_error and retried — bugs should be fixed, not retried.
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

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

The change to only rescue ::RubyLLM::Error is a breaking behavioral change, but there doesn’t appear to be a spec that asserts a non-RubyLLM::Error raised by an adapter (e.g., ArgumentError/NoMethodError) now propagates instead of being coerced into :adapter_error. Adding an explicit regression spec for this new contract would help prevent accidental broadening of the rescue in the future.

Copilot uses AI. Check for mistakes.
… rescue

Breaking changes

1. DEFAULT_RETRY_ON = [:validation_failed, :parse_error]. :adapter_error
   removed from the default. ruby_llm's Faraday middleware already retries
   transport errors (RateLimitError, ServerError, ServiceUnavailableError,
   OverloadedError, timeouts) with backoff, so the old default re-ran the
   same model on errors transport had already retried — retry x retry with
   no change in context. Opt in explicitly with retry_on :adapter_error,
   meaningful primarily paired with escalate "model_a", "model_b" where a
   different model/provider can bypass what transport could not.

2. AdapterCaller rescues ::RubyLLM::Error instead of StandardError.
   Programmer errors (NoMethodError, ArgumentError, config bugs) propagate
   rather than being silently converted into :adapter_error and retried.
   Provider errors still produce :adapter_error as before.

Migration: add retry_on :validation_failed, :parse_error, :adapter_error
to restore pre-0.7 behavior, or preferably combine with escalate for a
real fallback chain.

Full rationale: analysis in internal overlap study (H20) vs ruby_llm 1.14.1
Faraday retry scope. Codex-verified.
@justi justi force-pushed the feat/remove-adapter-error-default branch from 8d0af56 to f9eae16 Compare April 21, 2026 12:21
@justi
Copy link
Copy Markdown
Owner Author

justi commented Apr 21, 2026

Codex review findings addressed in the amended commit.

P1 (transport errors leaking) — fixed. AdapterCaller#rescue now catches ::RubyLLM::Error and ::Faraday::Error. Faraday re-raises Faraday::TimeoutError / Faraday::ConnectionFailed after its retry middleware exhausts, and these aren't in the RubyLLM::Error hierarchy — the narrower rescue would have leaked them. Added 3 regression specs in retry_integration_spec.rb under "transport error handling (Codex review regression)":

  • Faraday::TimeoutError:adapter_error (Codex verified this leaked without the fix)
  • Faraday::ConnectionFailed:adapter_error
  • NoMethodError (programmer bug) → propagates as NoMethodError

P2 (ArgumentError still coerced to :input_error) — acknowledged as known limitation. Step::Base#run_once rescues ArgumentError for input-type validation (dry-types raises this path). Disambiguating adapter-ArgumentError vs input-validation-ArgumentError requires refactoring run_once to scope the rescue narrowly around the type-check boundary, not around the whole Runner. That's a distinct change in scope; called out in the CHANGELOG as known limitation and left as a follow-up.

Full suite: 1339 examples, 0 failures, 8 pending.

@justi
Copy link
Copy Markdown
Owner Author

justi commented Apr 21, 2026

@copilot-pull-request-reviewer Review was on the pre-amend commit. The current HEAD (f9eae16) already has the regression spec you requested: spec/ruby_llm/contract/step/retry_integration_spec.rb:508it "propagates NoMethodError from adapter (programmer bug, not transport)" — asserts that non-RubyLLM::Error / non-Faraday::Error adapter exceptions propagate via raise_error(NoMethodError) instead of being coerced into :adapter_error. Two sibling specs also cover the Faraday::TimeoutError and Faraday::ConnectionFailed paths that Codex review found were leaking; all three live under the "transport error handling (Codex review regression)" describe block.

@justi justi merged commit 0d6ed4b into main Apr 21, 2026
2 checks passed
@justi justi deleted the feat/remove-adapter-error-default branch April 21, 2026 13:49
justi added a commit that referenced this pull request Apr 22, 2026
ADR-0021 deliverable 2. Follow-up to Codex review finding P2 on PR #13
(v0.7.0), acknowledged there as known limitation.

Before: Step::Base#run_once had a blanket `rescue ArgumentError` around
the entire Runner chain. Intent was to catch DSL misconfiguration such
as `prompt has not been set` and return it as :input_error. Side effect:
any ArgumentError raised from adapter code during Runner#call — wrong
arity, bad config arg, programmer bug — was silently coerced into
:input_error and re-tried as if the user had supplied bad input.

After: rescue is scoped to the Runner-construction phase only. DSL
configuration errors still produce :input_error (regression test for
`prompt has not been set` kept passing). ArgumentError raised during
Runner#call propagates to the caller. Input-type validation failures
continue to produce :input_error via InputValidator's own scoped rescue,
unchanged.

Two new regression specs under "transport error handling" describe:
- adapter code raising ArgumentError now propagates (was: :input_error)
- DSL misconfiguration (missing prompt) still returns :input_error

Full suite: 1341 examples, 0 failures. Codex review on v0.7.0 explicitly
flagged this case under P2; this closes that follow-up.
justi added a commit that referenced this pull request Apr 22, 2026
#15)

ADR-0021 deliverable 2. Follow-up to Codex review finding P2 on PR #13
(v0.7.0), acknowledged there as known limitation.

Before: Step::Base#run_once had a blanket `rescue ArgumentError` around
the entire Runner chain. Intent was to catch DSL misconfiguration such
as `prompt has not been set` and return it as :input_error. Side effect:
any ArgumentError raised from adapter code during Runner#call — wrong
arity, bad config arg, programmer bug — was silently coerced into
:input_error and re-tried as if the user had supplied bad input.

After: rescue is scoped to the Runner-construction phase only. DSL
configuration errors still produce :input_error (regression test for
`prompt has not been set` kept passing). ArgumentError raised during
Runner#call propagates to the caller. Input-type validation failures
continue to produce :input_error via InputValidator's own scoped rescue,
unchanged.

Two new regression specs under "transport error handling" describe:
- adapter code raising ArgumentError now propagates (was: :input_error)
- DSL misconfiguration (missing prompt) still returns :input_error

Full suite: 1341 examples, 0 failures. Codex review on v0.7.0 explicitly
flagged this case under P2; this closes that follow-up.
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