Skip to content

Conversation

zhephyn
Copy link
Contributor

@zhephyn zhephyn commented Jul 15, 2025

This is part of a dual PR which removes the max_llm_calls_per_minute option from the code base.

@zhephyn
Copy link
Contributor Author

zhephyn commented Jul 15, 2025

I had to remove one test which relied on the max_llm_calls_per_minute option.

        context "when throttling calls per minute" do
          let(:max_llm_calls_per_minute) { 2 }

          before do
            stub_const("Foobara::Agent::AccomplishGoal::SECONDS_PER_MINUTE", 1)
          end

          it "can fix the busted record", vcr: { record: :none } do
            expect {
              expect(outcome).to be_success
              expect(result[:result_data].name).to eq("Barbara")
            }.to change {
              Capybaras::Capybara.transaction do
                Capybaras::Capybara.find_by(name: "Barbara").year_of_birth
              end
            }.from(19).to(2019)
          end
        end

While the tests pass after the change, the code coverage is now down. Not sure how to proceed.

@azimux
Copy link
Contributor

azimux commented Jul 15, 2025

My guess is that the uncovered methods can be deleted. You can see them in coverage/index.html after running the test suite. If they are related to calls-per-minute stuff you can delete them.

One caveat is that we should merge removal of this stuff from foobara-agent-backed-command first since it depends on this gem and will break if we release removing it from here first.

@azimux
Copy link
Contributor

azimux commented Jul 15, 2025

Ohhh I just saw there was a PR in agent-backed-command and merged it! I apparently wasn't watching that repo so never saw that whoops

@zhephyn
Copy link
Contributor Author

zhephyn commented Jul 15, 2025

I'm gonna take a look at the uncovered lines in a few then push an update!

@zhephyn
Copy link
Contributor Author

zhephyn commented Jul 15, 2025

New update for this is now up.

@azimux azimux merged commit 5448595 into foobara:main Jul 15, 2025
1 check passed
@azimux
Copy link
Contributor

azimux commented Jul 15, 2025

Awesome! Thanks so much!

@zhephyn zhephyn deleted the remove-max_llm_calls_per_minute-option branch July 15, 2025 17:12
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