Conversation
There was a problem hiding this comment.
Pull request overview
This PR narrows Step::Base#run_once’s ArgumentError handling so that only DSL/setup-time misconfiguration is converted into an :input_error result, while ArgumentError raised during adapter execution propagates (treating it as a programmer/runtime error rather than “bad input”).
Changes:
- Scope
rescue ArgumentErrorto the Runner construction phase inStep::Base#run_once, allowing adapter-phaseArgumentErrorto bubble up. - Add regression specs covering (a) propagating adapter
ArgumentErrorand (b) still converting missing-promptDSL misconfiguration to:input_error. - Bump version to
0.7.1and document the behavioral change in the changelog.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
lib/ruby_llm/contract/step/base.rb |
Refactors run_once to only rescue ArgumentError during Runner setup, not during Runner#call. |
spec/ruby_llm/contract/step/retry_integration_spec.rb |
Adds regression coverage for adapter-phase ArgumentError propagation and prompt-missing :input_error. |
lib/ruby_llm/contract/version.rb |
Version bump to 0.7.1. |
CHANGELOG.md |
Documents the behavioral change in 0.7.1. |
Gemfile.lock |
Updates the local gem version entry to 0.7.1. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
630b26d to
a528b19
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to the Codex review P2 finding on PR #13 (v0.7.0), acknowledged there as a known limitation and tracked for v0.8. This is the first v0.8-planned deliverable from the local v0.8 roadmap — small enough to ship as a 0.7.x patch rather than wait for the batch.
Before:
Step::Base#run_oncewrapped the entire Runner chain inrescue ArgumentErrorto convert DSL misconfiguration (e.g.prompt has not been set) into:input_error. Side effect: anyArgumentErrorraised from adapter code duringRunner#call— wrong arity, bad config arg, any programmer bug — was silently coerced into:input_errorand re-tried as if the user had supplied bad input.After: the rescue is scoped to the Runner-construction phase only. DSL configuration errors still produce
:input_error.ArgumentErrorraised duringRunner#callpropagates to the caller.The
InputValidatorinsideRunnercontinues to handleDry::Types::CoercionError, TypeError, ArgumentErrorin its own scoped rescue and returns:input_errorfor bad-input cases. That path is unchanged.Why it matters
The narrative of v0.7 (CHANGELOG + README + ADR-0020) was "programmer errors propagate, provider errors become
:adapter_error". TheAdapterCalleralready respected that — narrow rescue toRubyLLM::Error+Faraday::Error. Butrun_once's broaderrescue ArgumentErrorwas a backdoor that let adapter-codeArgumentErrorbugs slip back into:input_errorand become retry targets.This closes that backdoor. Programmer bugs raised during an adapter call now surface loudly instead of being disguised as "user gave bad input."
Test plan
bundle exec rspec— 1341 examples, 0 failures, 8 pending (all pending are API-key-gated live LLM tests)retry_integration_spec.rbunder "transport error handling":it "propagates ArgumentError from adapter code (programmer bug, not bad input)"— adapter raisingArgumentErrornow propagates instead of being coerced into:input_error.it "still converts DSL misconfiguration ArgumentError to :input_error (prompt missing)"—prompt has not been setstill becomes:input_error. Existing BUG 48 adversarial spec (step without prompt →:input_error) also still passes, verifying the narrow path works.0.7.0→0.7.1; CHANGELOG entry added.Semver positioning
Technically a behavioral change — callers previously relying on
ArgumentErrorfrom adapter code to produce:input_errorresults will now see the exception propagate. Shipped as patch (0.7.1) rather than minor (0.8.0) because:Chat#with_contractspike) per local ADR-0021.