From 396a34ad710860b44f19349a7f74aebce3dffebe Mon Sep 17 00:00:00 2001 From: Esity Date: Sun, 29 Mar 2026 18:13:10 -0500 Subject: [PATCH 1/8] add multi-provider model diversity to adversarial PR review (closes #2) --- .../actors/lifecycle_subscriber.rb | 4 +- .../runners/extension_lifecycle.rb | 56 ++++++++++++-- .../runners/pull_request_reviewer.rb | 11 ++- .../actors/lifecycle_subscriber_spec.rb | 15 +++- .../runners/extension_lifecycle_spec.rb | 77 +++++++++++++++++++ .../runners/pull_request_reviewer_spec.rb | 27 +++++++ 6 files changed, 175 insertions(+), 15 deletions(-) diff --git a/lib/legion/extensions/swarm_github/actors/lifecycle_subscriber.rb b/lib/legion/extensions/swarm_github/actors/lifecycle_subscriber.rb index d10e7c6..b6743cf 100644 --- a/lib/legion/extensions/swarm_github/actors/lifecycle_subscriber.rb +++ b/lib/legion/extensions/swarm_github/actors/lifecycle_subscriber.rb @@ -22,8 +22,10 @@ def action(payload) generation = payload[:generation] || {} review = payload.except(:generation) review_k = payload[:review_k] + review_models = payload[:review_models] - Runners::ExtensionLifecycle.run_lifecycle(generation: generation, review: review, review_k: review_k) + Runners::ExtensionLifecycle.run_lifecycle(generation: generation, review: review, + review_k: review_k, review_models: review_models) rescue StandardError => e log.warn("LifecycleSubscriber failed: #{e.message}") { success: false, error: e.message } diff --git a/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb b/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb index ca2cdde..88cb094 100644 --- a/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb +++ b/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb @@ -7,7 +7,7 @@ module Runners module ExtensionLifecycle extend self - def run_lifecycle(generation:, review:, review_k: nil) + def run_lifecycle(generation:, review:, review_k: nil, review_models: nil) config = github_config return { success: false, error: :github_not_enabled } unless config[:enabled] return { success: false, error: :target_repo_missing } unless config[:target_repo] @@ -32,8 +32,9 @@ def run_lifecycle(generation:, review:, review_k: nil) labels: config[:pr_labels]) k = review_k || default_review_k + models = review_models || default_review_models review_result = run_adversarial_review(owner: owner, repo: repo, - pull_number: pr[:pull_number], k: k) + pull_number: pr[:pull_number], k: k, models: models) handle_auto_merge(owner: owner, repo: repo, pull_number: pr[:pull_number], config: config, review: review, review_result: review_result) @@ -98,13 +99,54 @@ def default_review_k 1 end - def run_adversarial_review(owner:, repo:, pull_number:, k:) # rubocop:disable Naming/MethodParameterName + def default_review_models + return [] unless defined?(Legion::Settings) + + Legion::Settings.dig(:codegen, :self_generate, :github, :review_models) || [] + rescue StandardError => e + log.warn(e.message) + [] + end + + def provider_available?(provider_sym) + return false unless defined?(Legion::Settings) + + Legion::Settings.dig(:llm, :providers, provider_sym, :enabled) == true + rescue StandardError => e + log.warn(e.message) + false + end + + def build_model_assignments(count, models) + return Array.new(count) { nil } if models.nil? || models.empty? + + available = models.select do |spec| + provider_sym = spec[:provider]&.to_sym + if provider_sym && !provider_available?(provider_sym) + log.warn("review provider #{provider_sym} not available, skipping") + false + else + true + end + end + + return Array.new(count) { nil } if available.empty? + + Array.new(count) { |i| available[i % available.size] } + end + + def run_adversarial_review(owner:, repo:, pull_number:, k:, models: []) # rubocop:disable Naming/MethodParameterName return { success: true, skipped: true, reason: :reviewer_unavailable } unless pr_reviewer_available? - reviews = Array.new(k) do - Legion::Extensions::SwarmGithub::Runners::PullRequestReviewer.review_pull_request( - owner: owner, repo: repo, pull_number: pull_number - ) + assignments = build_model_assignments(k, models) + + reviews = assignments.map do |spec| + kwargs = { owner: owner, repo: repo, pull_number: pull_number } + if spec + kwargs[:model] = spec[:model] if spec[:model] + kwargs[:provider] = spec[:provider] if spec[:provider] + end + Legion::Extensions::SwarmGithub::Runners::PullRequestReviewer.review_pull_request(**kwargs) end approvals = reviews.count do |r| diff --git a/lib/legion/extensions/swarm_github/runners/pull_request_reviewer.rb b/lib/legion/extensions/swarm_github/runners/pull_request_reviewer.rb index 5f8729e..d5f0519 100644 --- a/lib/legion/extensions/swarm_github/runners/pull_request_reviewer.rb +++ b/lib/legion/extensions/swarm_github/runners/pull_request_reviewer.rb @@ -5,11 +5,11 @@ module Extensions module SwarmGithub module Runners module PullRequestReviewer - def review_pull_request(owner:, repo:, pull_number:) + def review_pull_request(owner:, repo:, pull_number:, model: nil, provider: nil) files = fetch_pr_files(owner: owner, repo: repo, pull_number: pull_number) return { status: 'skipped', reason: 'no files' } if files.empty? - review = generate_review(files) + review = generate_review(files, model: model, provider: provider) { status: 'reviewed', @@ -33,12 +33,15 @@ def fetch_pr_files(owner:, repo:, pull_number:) [] end - def generate_review(files) + def generate_review(files, model: nil, provider: nil) chunks = Helpers::DiffChunker.chunk_files(files) reviews = chunks.map do |chunk| diff_text = chunk.map { |f| "--- #{f[:filename]} ---\n#{f[:patch]}" }.join("\n\n") prompt = code_review_prompt(diff_text) - response = Legion::LLM.chat(message: prompt, caller: { extension: 'lex-swarm-github' }) + llm_kwargs = { message: prompt, caller: { extension: 'lex-swarm-github' } } + llm_kwargs[:model] = model if model + llm_kwargs[:provider] = provider if provider + response = Legion::LLM.chat(**llm_kwargs) parse_review_response(response) end merge_chunk_reviews(reviews) diff --git a/spec/legion/extensions/swarm_github/actors/lifecycle_subscriber_spec.rb b/spec/legion/extensions/swarm_github/actors/lifecycle_subscriber_spec.rb index 91ca9d7..0e71382 100644 --- a/spec/legion/extensions/swarm_github/actors/lifecycle_subscriber_spec.rb +++ b/spec/legion/extensions/swarm_github/actors/lifecycle_subscriber_spec.rb @@ -80,12 +80,21 @@ class Subscription # rubocop:disable Lint/EmptyClass actor.action(payload) expect(Legion::Extensions::SwarmGithub::Runners::ExtensionLifecycle) .to have_received(:run_lifecycle).with( - generation: generation, - review: hash_including(verdict: 'approve'), - review_k: nil + generation: generation, + review: hash_including(verdict: 'approve'), + review_k: nil, + review_models: nil ) end + it 'forwards review_models from payload' do + models = [{ provider: :bedrock, model: 'claude' }] + payload = { verdict: 'approve', generation: generation, review_models: models } + actor.action(payload) + expect(Legion::Extensions::SwarmGithub::Runners::ExtensionLifecycle) + .to have_received(:run_lifecycle).with(hash_including(review_models: models)) + end + it 'returns the lifecycle result' do result = actor.action({ verdict: 'approve', generation: generation }) expect(result).to eq(lifecycle_result) diff --git a/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb b/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb index 06bdfdd..17a5084 100644 --- a/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb +++ b/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb @@ -208,5 +208,82 @@ expect(result[:review_k]).to eq(3) end end + + describe 'multi-provider adversarial review' do + let(:mod) { described_class } + + before do + allow(mod).to receive(:github_config).and_return( + enabled: true, target_repo: 'Test/repo', target_branch: 'main', + auto_merge: false, pr_labels: [], branch_prefix: 'feature/auto-gen' + ) + allow(mod).to receive(:create_lifecycle_branch).and_return({ success: true }) + allow(mod).to receive(:commit_generated_files).and_return({ success: true }) + allow(mod).to receive(:open_pull_request).and_return({ success: true, pull_number: 1, html_url: 'url' }) + allow(mod).to receive(:label_pull_request).and_return({ success: true }) + allow(mod).to receive(:handle_auto_merge) + end + + it 'forwards review_models to adversarial review' do + allow(mod).to receive(:run_adversarial_review).and_return({ success: true, consensus: :approve, k: 2 }) + models = [{ provider: :bedrock, model: 'claude' }] + mod.run_lifecycle(generation: generation, review: review, review_k: 2, review_models: models) + expect(mod).to have_received(:run_adversarial_review).with(hash_including(models: models)) + end + + it 'uses default_review_models when review_models is nil' do + allow(mod).to receive(:run_adversarial_review).and_return({ success: true, consensus: :approve, k: 1 }) + allow(mod).to receive(:default_review_models).and_return([]) + mod.run_lifecycle(generation: generation, review: review) + expect(mod).to have_received(:default_review_models) + end + end + + describe '#build_model_assignments' do + let(:mod) { described_class } + + it 'returns all nils when models is nil' do + result = mod.send(:build_model_assignments, 3, nil) + expect(result).to eq([nil, nil, nil]) + end + + it 'returns all nils when models is empty' do + result = mod.send(:build_model_assignments, 2, []) + expect(result).to eq([nil, nil]) + end + + it 'cycles available models across K slots' do + allow(mod).to receive(:provider_available?).and_return(true) + models = [{ provider: :bedrock, model: 'a' }, { provider: :openai, model: 'b' }] + result = mod.send(:build_model_assignments, 3, models) + expect(result).to eq([{ provider: :bedrock, model: 'a' }, { provider: :openai, model: 'b' }, + { provider: :bedrock, model: 'a' }]) + end + + it 'skips unavailable providers and falls back to nil assignments' do + allow(mod).to receive(:provider_available?).and_return(false) + models = [{ provider: :unavailable, model: 'x' }] + result = mod.send(:build_model_assignments, 2, models) + expect(result).to eq([nil, nil]) + end + end + + describe '#provider_available?' do + let(:mod) { described_class } + + it 'returns false when Legion::Settings is not defined' do + hide_const('Legion::Settings') if defined?(Legion::Settings) + expect(mod.send(:provider_available?, :bedrock)).to be false + end + end + + describe '#default_review_models' do + let(:mod) { described_class } + + it 'returns empty array when Legion::Settings is not defined' do + hide_const('Legion::Settings') if defined?(Legion::Settings) + expect(mod.send(:default_review_models)).to eq([]) + end + end end end diff --git a/spec/legion/extensions/swarm_github/runners/pull_request_reviewer_spec.rb b/spec/legion/extensions/swarm_github/runners/pull_request_reviewer_spec.rb index 9ee34a8..9482e3f 100644 --- a/spec/legion/extensions/swarm_github/runners/pull_request_reviewer_spec.rb +++ b/spec/legion/extensions/swarm_github/runners/pull_request_reviewer_spec.rb @@ -98,5 +98,32 @@ expect(result[:comments].first[:severity]).to eq('error') end end + + context 'with model: and provider: kwargs' do + let(:client) { Class.new { include Legion::Extensions::SwarmGithub::Runners::PullRequestReviewer }.new } + + before do + stub_const('Legion::LLM', Module.new) + allow(Legion::LLM).to receive(:chat).and_return('{"summary":"ok","comments":[]}') + allow(client).to receive(:fetch_pr_files).and_return( + [{ filename: 'test.rb', patch: '+x' }] + ) + end + + it 'forwards model and provider to Legion::LLM.chat' do + client.review_pull_request(owner: 'o', repo: 'r', pull_number: 1, model: 'gpt-4o', provider: :openai) + expect(Legion::LLM).to have_received(:chat).with(hash_including(model: 'gpt-4o', provider: :openai)) + end + + it 'omits model key when model is nil' do + client.review_pull_request(owner: 'o', repo: 'r', pull_number: 1) + expect(Legion::LLM).to have_received(:chat).with(hash_not_including(:model)) + end + + it 'omits provider key when provider is nil' do + client.review_pull_request(owner: 'o', repo: 'r', pull_number: 1) + expect(Legion::LLM).to have_received(:chat).with(hash_not_including(:provider)) + end + end end end From 8507f715847038fe6a4a7c6be106d3f6b82aadbd Mon Sep 17 00:00:00 2001 From: Esity Date: Sun, 29 Mar 2026 20:07:38 -0500 Subject: [PATCH 2/8] bump version to 0.3.2 and add changelog for multi-provider review --- CHANGELOG.md | 14 ++++++++++++-- lib/legion/extensions/swarm_github/version.rb | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79613ec..7fcfdd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,21 @@ # Changelog -## [0.3.1] - 2026-03-28 +## [0.3.2] - 2026-03-29 + +### Added +- Multi-provider model diversity for adversarial K-factor PR review (`review_models:` kwarg) +- `build_model_assignments` cycles available providers, skips unavailable with log warning, backfills with default +- `provider_available?` checks `Settings[:llm][:providers][sym][:enabled]` +- `PullRequestReviewer` accepts `model:` and `provider:` kwargs, forwarded to `Legion::LLM.chat` +- `LifecycleSubscriber` forwards `review_models:` from AMQP payload +- Settings fallback: `codegen.self_generate.github.review_models` (default `[]`) + +## [0.3.1] - 2026-03-29 ### Added - `Runners::ExtensionLifecycle` — orchestrates autonomous extension github pipeline: branch creation, file commit, PR open, label, optional auto-merge - `Actor::LifecycleSubscriber` — subscription actor that triggers lifecycle runner when generation review verdict is `approve` and github lifecycle is enabled -- Added K-factor adversarial PR review (review_k: kwarg, default 1, settings-configurable) +- K-factor adversarial PR review (`review_k:` kwarg, default 1, settings-configurable) ## [0.3.0] - 2026-03-24 diff --git a/lib/legion/extensions/swarm_github/version.rb b/lib/legion/extensions/swarm_github/version.rb index 0935a4b..e84f1f0 100644 --- a/lib/legion/extensions/swarm_github/version.rb +++ b/lib/legion/extensions/swarm_github/version.rb @@ -3,7 +3,7 @@ module Legion module Extensions module SwarmGithub - VERSION = '0.3.1' + VERSION = '0.3.2' end end end From a868b7613e64534a4a46eca73539896d5fcf2d63 Mon Sep 17 00:00:00 2001 From: Esity Date: Sun, 29 Mar 2026 20:19:45 -0500 Subject: [PATCH 3/8] address copilot review: string key normalization, spec coercion, changelog fixes --- CHANGELOG.md | 4 ++-- .../actors/lifecycle_subscriber.rb | 19 +++++++++++++++---- .../runners/extension_lifecycle.rb | 15 +++++++++++---- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fcfdd5..267e6aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,13 +4,13 @@ ### Added - Multi-provider model diversity for adversarial K-factor PR review (`review_models:` kwarg) -- `build_model_assignments` cycles available providers, skips unavailable with log warning, backfills with default +- `build_model_assignments` cycles available providers, skips unavailable; falls back to default only when review_models is nil/empty or all providers unavailable - `provider_available?` checks `Settings[:llm][:providers][sym][:enabled]` - `PullRequestReviewer` accepts `model:` and `provider:` kwargs, forwarded to `Legion::LLM.chat` - `LifecycleSubscriber` forwards `review_models:` from AMQP payload - Settings fallback: `codegen.self_generate.github.review_models` (default `[]`) -## [0.3.1] - 2026-03-29 +## [0.3.1] - 2026-03-28 ### Added - `Runners::ExtensionLifecycle` — orchestrates autonomous extension github pipeline: branch creation, file commit, PR open, label, optional auto-merge diff --git a/lib/legion/extensions/swarm_github/actors/lifecycle_subscriber.rb b/lib/legion/extensions/swarm_github/actors/lifecycle_subscriber.rb index b6743cf..73802d6 100644 --- a/lib/legion/extensions/swarm_github/actors/lifecycle_subscriber.rb +++ b/lib/legion/extensions/swarm_github/actors/lifecycle_subscriber.rb @@ -19,10 +19,11 @@ def action(payload) return { skipped: true, reason: verdict == 'approve' ? :github_disabled : :not_approved } end - generation = payload[:generation] || {} - review = payload.except(:generation) - review_k = payload[:review_k] - review_models = payload[:review_models] + generation = payload[:generation] || payload['generation'] || {} + review = payload.except(:generation, 'generation') + review_k = payload[:review_k] || payload['review_k'] + raw_models = payload[:review_models] || payload['review_models'] + review_models = normalize_review_models(raw_models) Runners::ExtensionLifecycle.run_lifecycle(generation: generation, review: review, review_k: review_k, review_models: review_models) @@ -33,6 +34,16 @@ def action(payload) private + def normalize_review_models(raw) + return nil unless raw.is_a?(Array) + + raw.filter_map do |spec| + next unless spec.is_a?(Hash) + + spec.transform_keys { |k| k.respond_to?(:to_sym) ? k.to_sym : k } + end + end + def github_lifecycle_enabled? return false unless defined?(Legion::Settings) diff --git a/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb b/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb index 88cb094..98bca76 100644 --- a/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb +++ b/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb @@ -120,14 +120,21 @@ def provider_available?(provider_sym) def build_model_assignments(count, models) return Array.new(count) { nil } if models.nil? || models.empty? - available = models.select do |spec| + available = models.filter_map do |raw_spec| + unless raw_spec.is_a?(Hash) + log.warn('review model spec is not a Hash, skipping') + next + end + + spec = raw_spec.transform_keys { |k| k.respond_to?(:to_sym) ? k.to_sym : k } provider_sym = spec[:provider]&.to_sym + if provider_sym && !provider_available?(provider_sym) log.warn("review provider #{provider_sym} not available, skipping") - false - else - true + next end + + spec end return Array.new(count) { nil } if available.empty? From 5924d823e36197178c6fc868423553a7a6389153 Mon Sep 17 00:00:00 2001 From: Esity Date: Sun, 29 Mar 2026 20:35:22 -0500 Subject: [PATCH 4/8] normalize provider to symbol in build_model_assignments for consistent downstream typing --- .../extensions/swarm_github/runners/extension_lifecycle.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb b/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb index 98bca76..e815500 100644 --- a/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb +++ b/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb @@ -128,6 +128,7 @@ def build_model_assignments(count, models) spec = raw_spec.transform_keys { |k| k.respond_to?(:to_sym) ? k.to_sym : k } provider_sym = spec[:provider]&.to_sym + spec[:provider] = provider_sym if provider_sym if provider_sym && !provider_available?(provider_sym) log.warn("review provider #{provider_sym} not available, skipping") From 6b95900cda0e0eea0045328e59b542c9cb501399 Mon Sep 17 00:00:00 2001 From: Esity Date: Sun, 29 Mar 2026 20:39:34 -0500 Subject: [PATCH 5/8] address copilot review feedback on pr #3 change build_model_assignments to use each available spec at most once then backfill remaining slots with nil (default provider), instead of cycling; update spec to match new use-once-then-nil semantics --- .../extensions/swarm_github/runners/extension_lifecycle.rb | 3 ++- .../swarm_github/runners/extension_lifecycle_spec.rb | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb b/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb index e815500..8d21200 100644 --- a/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb +++ b/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb @@ -140,7 +140,8 @@ def build_model_assignments(count, models) return Array.new(count) { nil } if available.empty? - Array.new(count) { |i| available[i % available.size] } + assignments = available.first(count) + assignments + Array.new([count - assignments.length, 0].max) { nil } end def run_adversarial_review(owner:, repo:, pull_number:, k:, models: []) # rubocop:disable Naming/MethodParameterName diff --git a/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb b/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb index 17a5084..649e242 100644 --- a/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb +++ b/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb @@ -252,12 +252,11 @@ expect(result).to eq([nil, nil]) end - it 'cycles available models across K slots' do + it 'uses each available model at most once, then fills remaining slots with nil' do allow(mod).to receive(:provider_available?).and_return(true) models = [{ provider: :bedrock, model: 'a' }, { provider: :openai, model: 'b' }] result = mod.send(:build_model_assignments, 3, models) - expect(result).to eq([{ provider: :bedrock, model: 'a' }, { provider: :openai, model: 'b' }, - { provider: :bedrock, model: 'a' }]) + expect(result).to eq([{ provider: :bedrock, model: 'a' }, { provider: :openai, model: 'b' }, nil]) end it 'skips unavailable providers and falls back to nil assignments' do From 1b5ec73375b5221c96af3caf27942cb75a642e93 Mon Sep 17 00:00:00 2001 From: Esity Date: Sun, 29 Mar 2026 20:58:22 -0500 Subject: [PATCH 6/8] address copilot review round 3 on pr #3 - add Array type guard in build_model_assignments with log.warn for non-Array input - add extend self to PullRequestReviewer so module-level call in run_adversarial_review works - stub pr_reviewer_available? in integration contexts that don't exercise adversarial review path - add spec coverage for non-Array models guard --- .../swarm_github/runners/extension_lifecycle.rb | 5 +++++ .../swarm_github/runners/pull_request_reviewer.rb | 2 ++ .../swarm_github/runners/extension_lifecycle_spec.rb | 8 ++++++++ 3 files changed, 15 insertions(+) diff --git a/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb b/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb index 8d21200..c84224c 100644 --- a/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb +++ b/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb @@ -120,6 +120,11 @@ def provider_available?(provider_sym) def build_model_assignments(count, models) return Array.new(count) { nil } if models.nil? || models.empty? + unless models.is_a?(Array) + log.warn("review_models must be an Array, got #{models.class}; using defaults") + return Array.new(count) { nil } + end + available = models.filter_map do |raw_spec| unless raw_spec.is_a?(Hash) log.warn('review model spec is not a Hash, skipping') diff --git a/lib/legion/extensions/swarm_github/runners/pull_request_reviewer.rb b/lib/legion/extensions/swarm_github/runners/pull_request_reviewer.rb index d5f0519..cccd8c9 100644 --- a/lib/legion/extensions/swarm_github/runners/pull_request_reviewer.rb +++ b/lib/legion/extensions/swarm_github/runners/pull_request_reviewer.rb @@ -5,6 +5,8 @@ module Extensions module SwarmGithub module Runners module PullRequestReviewer + extend self + def review_pull_request(owner:, repo:, pull_number:, model: nil, provider: nil) files = fetch_pr_files(owner: owner, repo: repo, pull_number: pull_number) return { status: 'skipped', reason: 'no files' } if files.empty? diff --git a/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb b/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb index 649e242..3734271 100644 --- a/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb +++ b/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb @@ -79,6 +79,7 @@ { result: { 'number' => 42, 'html_url' => 'https://github.com/org/repo/pull/42' } } ) allow(github_client).to receive(:add_labels).and_return({ success: true }) + allow(runner).to receive(:pr_reviewer_available?).and_return(false) end it 'returns success with PR details' do @@ -154,6 +155,7 @@ { result: { 'number' => 7, 'html_url' => 'https://github.com/org/repo/pull/7' } } ) allow(github_client).to receive(:merge_pull_request).and_return({ success: true }) + allow(runner).to receive(:pr_reviewer_available?).and_return(false) end it 'calls merge_pull_request' do @@ -265,6 +267,12 @@ result = mod.send(:build_model_assignments, 2, models) expect(result).to eq([nil, nil]) end + + it 'returns all nils and warns when models is not an Array' do + expect(mod).to receive(:log).at_least(:once).and_return(double(warn: nil)) + result = mod.send(:build_model_assignments, 2, 'not-an-array') + expect(result).to eq([nil, nil]) + end end describe '#provider_available?' do From ba205d6e944883ea93fdebdced3bc51859dd5c37 Mon Sep 17 00:00:00 2001 From: Esity Date: Sun, 29 Mar 2026 21:07:36 -0500 Subject: [PATCH 7/8] address copilot review round 4 on pr #3 move is_a?(Array) guard before models.empty? to avoid NoMethodError on non-Array types; add spec coverage for Hash and Integer inputs (#3) --- .../swarm_github/runners/extension_lifecycle.rb | 4 +++- .../runners/extension_lifecycle_spec.rb | 14 +++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb b/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb index c84224c..cc58155 100644 --- a/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb +++ b/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb @@ -118,13 +118,15 @@ def provider_available?(provider_sym) end def build_model_assignments(count, models) - return Array.new(count) { nil } if models.nil? || models.empty? + return Array.new(count) { nil } if models.nil? unless models.is_a?(Array) log.warn("review_models must be an Array, got #{models.class}; using defaults") return Array.new(count) { nil } end + return Array.new(count) { nil } if models.empty? + available = models.filter_map do |raw_spec| unless raw_spec.is_a?(Hash) log.warn('review model spec is not a Hash, skipping') diff --git a/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb b/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb index 3734271..0c74828 100644 --- a/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb +++ b/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb @@ -268,11 +268,23 @@ expect(result).to eq([nil, nil]) end - it 'returns all nils and warns when models is not an Array' do + it 'returns all nils and warns when models is a String' do expect(mod).to receive(:log).at_least(:once).and_return(double(warn: nil)) result = mod.send(:build_model_assignments, 2, 'not-an-array') expect(result).to eq([nil, nil]) end + + it 'returns all nils and warns when models is a Hash' do + expect(mod).to receive(:log).at_least(:once).and_return(double(warn: nil)) + result = mod.send(:build_model_assignments, 2, { provider: :bedrock, model: 'claude' }) + expect(result).to eq([nil, nil]) + end + + it 'returns all nils and warns when models is an Integer' do + expect(mod).to receive(:log).at_least(:once).and_return(double(warn: nil)) + result = mod.send(:build_model_assignments, 2, 42) + expect(result).to eq([nil, nil]) + end end describe '#provider_available?' do From 0fce72bf5ecf7509dca83220a171cffd77987d97 Mon Sep 17 00:00:00 2001 From: Esity Date: Sun, 29 Mar 2026 21:20:46 -0500 Subject: [PATCH 8/8] address copilot review round 5 on pr #3 guard non-symbolizable provider values in build_model_assignments (#3) --- .../swarm_github/runners/extension_lifecycle.rb | 9 ++++++++- .../swarm_github/runners/extension_lifecycle_spec.rb | 10 ++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb b/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb index cc58155..95ff3bb 100644 --- a/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb +++ b/lib/legion/extensions/swarm_github/runners/extension_lifecycle.rb @@ -134,7 +134,14 @@ def build_model_assignments(count, models) end spec = raw_spec.transform_keys { |k| k.respond_to?(:to_sym) ? k.to_sym : k } - provider_sym = spec[:provider]&.to_sym + provider_value = spec[:provider] + + if provider_value && !provider_value.respond_to?(:to_sym) + log.warn("review provider value #{provider_value.inspect} (#{provider_value.class}) cannot be symbolized, skipping") + next + end + + provider_sym = provider_value&.to_sym spec[:provider] = provider_sym if provider_sym if provider_sym && !provider_available?(provider_sym) diff --git a/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb b/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb index 0c74828..2da3330 100644 --- a/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb +++ b/spec/legion/extensions/swarm_github/runners/extension_lifecycle_spec.rb @@ -285,6 +285,16 @@ result = mod.send(:build_model_assignments, 2, 42) expect(result).to eq([nil, nil]) end + + it 'skips specs with a non-symbolizable provider value and warns' do + logger = double(warn: nil) + allow(mod).to receive(:log).and_return(logger) + models = [{ provider: [1, 2], model: 'x' }, { provider: :openai, model: 'y' }] + allow(mod).to receive(:provider_available?).with(:openai).and_return(true) + result = mod.send(:build_model_assignments, 2, models) + expect(logger).to have_received(:warn).with(/cannot be symbolized/) + expect(result).to eq([{ provider: :openai, model: 'y' }, nil]) + end end describe '#provider_available?' do