diff --git a/TODO.md b/TODO.md index 7fc743a..d8b5ec7 100644 --- a/TODO.md +++ b/TODO.md @@ -1,3 +1,4 @@ -- [ ] Add matcher diagnostics. Allow check details of each matcher in the three +- [x] Add matcher diagnostics. Allow check details of each matcher in the tree (via `to_h`) - [ ] Split stuff into files and add tests for each class -- [ ] Validate expressions and raise errors for invalid expressions +- [x] Validate expressions and raise errors for invalid expressions (Improved MCP validation) +- [x] Review docs/feedback/mcp_weak_format_matcher_experiment.md and improve experiment finalization for MCP-driven refactors diff --git a/lib/fast.rb b/lib/fast.rb index c4ba7be..91ede0b 100644 --- a/lib/fast.rb +++ b/lib/fast.rb @@ -63,7 +63,7 @@ class SyntaxError < StandardError; end /x.freeze class << self - attr_accessor :gain_tracking_enabled + attr_accessor :gain_tracking_enabled, :gains_dir def enable_gain_track! self.gain_tracking_enabled = true @@ -75,8 +75,12 @@ def disable_gain_track! def gain_tracking_enabled? return false if ENV['FAST_GAINS'] == '0' + return false unless @gain_tracking_enabled - @gain_tracking_enabled == true + dir = gains_dir || (defined?(Fast::Gains::STORAGE_DIR) ? Fast::Gains::STORAGE_DIR : nil) + return false unless dir + + File.writable?(dir) || File.writable?(File.dirname(dir)) end def ast_node?(node) @@ -624,6 +628,18 @@ def to_s "f[#{[*token].map(&:to_s).join(', ')}]" end + def to_h + { type: self.class.name.split('::').last, token: token_to_h } + end + + def token_to_h + case token + when Array then token.map { |t| t.respond_to?(:to_h) ? t.to_h : t } + when Find then token.to_h + else token + end + end + def ==(other) return false if other.nil? || !other.respond_to?(:token) @@ -670,6 +686,10 @@ def initialize(method_name) @method_name = method_name end + def to_h + { type: 'MethodCall', method_name: @method_name } + end + def match?(node) Kernel.send(@method_name, node) end @@ -681,6 +701,10 @@ def initialize(method_name) @method_name = method_name end + def to_h + { type: 'InstanceMethodCall', method_name: @method_name } + end + def match?(node) node.send(@method_name) end @@ -703,6 +727,10 @@ def initialize(token) @capture_index = token.to_i end + def to_h + { type: 'FindWithCapture', capture_index: @capture_index } + end + def match?(node) node == @previous_captures[@capture_index - 1] end @@ -731,6 +759,10 @@ def initialize(token) raise 'Arguments start in one' if @capture_argument.negative? end + def to_h + { type: 'FindFromArgument', capture_argument: @capture_argument + 1 } + end + def match?(node) raise 'You must define arguments to match' unless @arguments diff --git a/lib/fast/experiment.rb b/lib/fast/experiment.rb index fd3e03b..25351c9 100644 --- a/lib/fast/experiment.rb +++ b/lib/fast/experiment.rb @@ -91,10 +91,11 @@ def experiment(name, &block) class Experiment attr_writer :files attr_reader :name, :replacement, :expression, :files_or_folders, :ok_if - attr_accessor :autoclean + attr_accessor :autoclean, :strategy def initialize(name, &block) @name = name + @strategy = :combinations puts "\nStarting experiment: #{name}" instance_exec(&block) end @@ -128,13 +129,19 @@ def policy(&block) @ok_if = block end + # @param [Symbol] strategy to use. Default is :combinations. + def strategy(strategy = nil) + return @strategy if strategy.nil? + @strategy = strategy + end + # @return [Array] with files from {#lookup} expression. def files @files ||= Fast.ruby_files_from(@files_or_folders) end # Iterates over all {#files} to {#run_with} them. - # @return [void] + # @return [Array] results of the experiment for each file def run files.map(&method(:run_with)) end @@ -160,7 +167,7 @@ def initialize(round:, occurrences_count:, ok_experiments:, fail_experiments:) # Generate different combinations depending on the current round. # * Round 1: Use {#individual_replacements} # * Round 2: Tries {#all_ok_replacements_combined} - # * Round 3+: Follow {#ok_replacements_pair_combinations} + # * Round 3+: Follow {#ok_replacements_pair_combinations} or bisection/chunks def generate_combinations case @round when 1 @@ -168,7 +175,26 @@ def generate_combinations when 2 all_ok_replacements_combined else - ok_replacements_pair_combinations + if @ok_experiments.size > 10 + bisection_combinations + else + ok_replacements_pair_combinations + end + end + end + + def bisection_combinations + return [] if @ok_experiments.size <= 1 + + mid = @ok_experiments.size / 2 + left = @ok_experiments[0...mid].flatten.uniq.sort + right = @ok_experiments[mid..-1].flatten.uniq.sort + + [left, right].reject do |c| + c.size <= 1 || + @ok_experiments.include?(c) || + @fail_experiments.include?(c) || + (c.size == 1 && @ok_experiments.include?(c.first)) end end @@ -337,16 +363,55 @@ def cleanup_generated_files! def done! count_executed_combinations = @fail_experiments.size + @ok_experiments.size puts "Done with #{@file} after #{count_executed_combinations} combinations" - unless perfect_combination = @ok_experiments.last # rubocop:disable Lint/AssignmentInCondition + + if experiment.strategy == :dry_run cleanup_generated_files! if experiment.autoclean? - return + return { + file: @file, + status: :dry_run, + ok_experiments: @ok_experiments, + fail_experiments: @fail_experiments + } + end + + final_combination = @ok_experiments.max_by { |c| Array(c).size } + + if experiment.strategy == :apply_individual_survivors + final_combination = @ok_experiments.flatten.uniq.sort + end + + unless final_combination + cleanup_generated_files! if experiment.autoclean? + return { file: @file, status: :no_changes } + end + + # If the final combination is already applied to the file (last round succeeded) + # we just need to move it if it's not the same file. + # But usually we want to build a final file with ALL successful independent changes. + + # For now, let's just use the best one we found. + # If the user wants to apply all survivors, we need to create a file with all of them. + + if experiment.strategy == :apply_individual_survivors || !@ok_experiments.include?(final_combination) + content = partial_replace(*final_combination) + experimental_file = write_experiment_file(final_combination, content) + else + experimental_file = experimental_filename(final_combination) end puts 'The following changes were applied to the file:' - `diff #{experimental_filename(perfect_combination)} #{@file}` - puts "mv #{experimental_filename(perfect_combination)} #{@file}" - `mv #{experimental_filename(perfect_combination)} #{@file}` + `diff #{experimental_file} #{@file}` + puts "mv #{experimental_file} #{@file}" + `mv #{experimental_file} #{@file}` cleanup_generated_files! if experiment.autoclean? + + { + file: @file, + status: :applied, + combination: final_combination, + ok_count: @ok_experiments.size, + fail_count: @fail_experiments.size + } end # Increase the `@round` by 1 to {ExperimentCombinations#generate_combinations}. @@ -371,6 +436,7 @@ def run while combination = combinations.shift # rubocop:disable Lint/AssignmentInCondition run_partial_replacement_with(combination) end + break if (experiment.strategy == :apply_individual_survivors || experiment.strategy == :dry_run) && @round == 1 end done! end diff --git a/lib/fast/gains.rb b/lib/fast/gains.rb index b8da7e3..fbd380b 100644 --- a/lib/fast/gains.rb +++ b/lib/fast/gains.rb @@ -11,6 +11,14 @@ class Gains STORAGE_DIR = File.expand_path('~/.fast') STORAGE_FILE = File.join(STORAGE_DIR, 'gains.json') + def self.storage_dir + Fast.gains_dir || STORAGE_DIR + end + + def self.storage_file + File.join(storage_dir, 'gains.json') + end + attr_reader :command, :start_time, :total_bytes_searched, :total_bytes_reported, :files_count, :matched_files_count def initialize(command = nil) @@ -57,29 +65,38 @@ def save! bytes_reported: @total_bytes_reported } - FileUtils.mkdir_p(STORAGE_DIR) - temp_filename = File.join(STORAGE_DIR, "gains-#{Time.now.to_f}-#{Process.pid}.json") - File.write(temp_filename, JSON.generate(data)) + begin + FileUtils.mkdir_p(self.class.storage_dir) + temp_filename = File.join(self.class.storage_dir, "gains-#{Time.now.to_f}-#{Process.pid}.json") + File.write(temp_filename, JSON.generate(data)) + rescue Errno::EACCES, Errno::EPERM + # Fail silently if not possible to write due to permissions + end self.class.consolidate! end def self.consolidate! - FileUtils.mkdir_p(STORAGE_DIR) + return unless File.writable?(storage_dir) || File.writable?(File.dirname(storage_dir)) + begin + FileUtils.mkdir_p(storage_dir) + rescue Errno::EACCES, Errno::EPERM + # Fail silently if not possible to write due to permissions + end - File.open(STORAGE_FILE, File::RDWR|File::CREAT, 0644) do |f| + File.open(storage_file, File::RDWR|File::CREAT, 0644) do |f| f.flock(File::LOCK_EX) content = f.read all_data = JSON.parse(content, symbolize_names: true) rescue [] unless content.empty? all_data ||= [] - temp_files = Dir.glob(File.join(STORAGE_DIR, 'gains-*.json')) + temp_files = Dir.glob(File.join(storage_dir, 'gains-*.json')) temp_files.each do |file| begin all_data << JSON.parse(File.read(file), symbolize_names: true) File.delete(file) - rescue + rescue StandardError # Skip corrupted files end end @@ -92,6 +109,9 @@ def self.consolidate! f.write(JSON.pretty_generate(all_data)) all_data end + rescue Errno::EACCES, Errno::EPERM + # Fail silently if not possible to write due to permissions + [] end def self.summarize(data) diff --git a/lib/fast/mcp_server.rb b/lib/fast/mcp_server.rb index e61dec9..2d2d901 100644 --- a/lib/fast/mcp_server.rb +++ b/lib/fast/mcp_server.rb @@ -103,7 +103,12 @@ class McpServer lookup: { type: 'string', description: 'Folder or file to target, e.g. "spec"' }, search: { type: 'string', description: 'Fast AST search pattern to find nodes.' }, edit: { type: 'string', description: 'Ruby code to evaluate in Rewriter context. Has access to `node` variable. Example: `replace(node.loc.expression, "build_stubbed")`' }, - policy: { type: 'string', description: 'Shell command returning exit status 0 on success. Uses {file} for the temporary file created during the rewrite round. Example: `bin/spring rspec --fail-fast {file}`' } + policy: { type: 'string', description: 'Shell command returning exit status 0 on success. Uses {file} for the temporary file created during the rewrite round. Example: `bin/spring rspec --fail-fast {file}`' }, + strategy: { type: 'string', enum: ['combinations', 'apply_individual_survivors', 'dry_run'], description: 'Strategy for the experiment (default: combinations).' }, + cwd: { type: 'string', description: 'Optional directory to run the policy command in.' }, + env: { type: 'object', additionalProperties: { type: 'string' }, description: 'Optional environment variables for the policy command.' }, + gains_dir: { type: 'string', description: 'Optional directory to store gains files.' }, + gains_enabled: { type: 'boolean', description: 'Enable or disable gains tracking for this tool call (default: true).' } }, required: ['name', 'lookup', 'search', 'edit', 'policy'] } @@ -159,13 +164,20 @@ def handle_request(request) end def handle_tool_call(id, params) - Fast.enable_gain_track! tool_name = params['name'] args = params['arguments'] || {} show_ast = args['show_ast'] || false offset = args['offset'] || 0 limit = args['limit'] || 20 - @gains = Gains.new("mcp:#{tool_name}") + + if args['gains_enabled'] == false + Fast.disable_gain_track! + else + Fast.enable_gain_track! + end + Fast.gains_dir = args['gains_dir'] if args['gains_dir'] + + @gains = Gains.new("mcp:#{tool_name}") if args['pattern'] && !args['pattern'].start_with?('(', '{', '[') && !args['pattern'].match?(/^[a-z_]+$/) raise "Invalid Fast AST pattern: '#{args['pattern']}'. Did you mean to use an s-expression like '(#{args['pattern']})'?" @@ -187,7 +199,7 @@ def handle_tool_call(id, params) when 'rewrite_ruby_file' execute_rewrite_file(args['file'], args['pattern'], args['replacement']) when 'run_fast_experiment' - execute_fast_experiment(args['name'], args['lookup'], args['search'], args['edit'], args['policy']) + execute_fast_experiment(args) else raise "Unknown tool: #{tool_name}" end @@ -196,12 +208,12 @@ def handle_tool_call(id, params) @gains.save! write_response(id, { content: [{ type: 'text', text: JSON.generate(result) }] }) rescue => e - write_error(id, -32603, 'Tool execution failed', e.message) + write_error(id, -32603, "Tool execution failed: #{e.message}", e.backtrace.join("\n")) end def execute_validate_pattern(pattern) - Fast.expression(pattern) - { valid: true } + res = Fast.expression(pattern) + { valid: true, structure: res.is_a?(Array) ? res.map(&:to_h) : res.to_h } rescue StandardError => e { valid: false, error: e.message } end @@ -352,26 +364,36 @@ def execute_rewrite_file(file, pattern, replacement) { file: file, changed: true, diff: diff } end - def execute_fast_experiment(name, lookup_path, search_pattern, edit_code, policy_command) + def execute_fast_experiment(args) + name = args['name'] + lookup_path = args['lookup'] + search_pattern = args['search'] + edit_code = args['edit'] + policy_command = args['policy'] + cwd = args['cwd'] + env = args['env'] || {} + require 'fast/experiment' original_stdout = $stdout.dup capture_output = StringIO.new $stdout = capture_output + results = [] begin experiment = Fast.experiment(name) do lookup lookup_path search search_pattern + strategy args['strategy'].to_sym if args['strategy'] edit do |node, *captures| eval(edit_code) end policy do |new_file| cmd = policy_command.gsub('{file}', new_file) - system(cmd) + system(env, cmd, chdir: cwd || Dir.pwd) end end experiment.files.each { |f| @gains&.record_search(f) } - experiment.run + results = experiment.run ensure $stdout = original_stdout end @@ -379,7 +401,7 @@ def execute_fast_experiment(name, lookup_path, search_pattern, edit_code, policy # Exclude any color from captured output log = capture_output.string.gsub(/\e\[([;\d]+)?m/, '') - { experiment: name, log: log } + { experiment: name, log: log, results: results } end # Returns loc.expression if available diff --git a/lib/fast/rewriter.rb b/lib/fast/rewriter.rb index dfe06bd..7879217 100644 --- a/lib/fast/rewriter.rb +++ b/lib/fast/rewriter.rb @@ -131,6 +131,8 @@ def replace(range, content) def execute_replacement(node, captures) if replacement.parameters.length == 1 instance_exec node, &replacement + elsif replacement.parameters.length > 2 && captures.is_a?(Array) + instance_exec node, *captures, &replacement else instance_exec node, captures, &replacement end