diff --git a/CHANGELOG.md b/CHANGELOG.md index 79613ec..267e6aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,21 @@ # Changelog +## [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; 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-28 ### 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/actors/lifecycle_subscriber.rb b/lib/legion/extensions/swarm_github/actors/lifecycle_subscriber.rb index d10e7c6..73802d6 100644 --- a/lib/legion/extensions/swarm_github/actors/lifecycle_subscriber.rb +++ b/lib/legion/extensions/swarm_github/actors/lifecycle_subscriber.rb @@ -19,11 +19,14 @@ 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] + 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) + 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 } @@ -31,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 ca2cdde..95ff3bb 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,77 @@ 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? + + 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') + next + end + + spec = raw_spec.transform_keys { |k| k.respond_to?(:to_sym) ? k.to_sym : k } + 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) + log.warn("review provider #{provider_sym} not available, skipping") + next + end + + spec + end + + return Array.new(count) { nil } if available.empty? + + 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 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..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,11 +5,13 @@ module Extensions module SwarmGithub module Runners module PullRequestReviewer - def review_pull_request(owner:, repo:, pull_number:) + 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? - review = generate_review(files) + review = generate_review(files, model: model, provider: provider) { status: 'reviewed', @@ -33,12 +35,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/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 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..2da3330 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 @@ -208,5 +210,109 @@ 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 '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' }, nil]) + 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 + + 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 + + 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 + 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