Skip to content

Top level ask method#242

Closed
tpaulshippy wants to merge 7 commits intocrmne:mainfrom
tpaulshippy:why-not-ask
Closed

Top level ask method#242
tpaulshippy wants to merge 7 commits intocrmne:mainfrom
tpaulshippy:why-not-ask

Conversation

@tpaulshippy
Copy link
Copy Markdown
Contributor

@tpaulshippy tpaulshippy commented Jun 12, 2025

What this does

Provide a simple way to call the LLM with a single prompt for scenarios where there isn't additional conversation needed.

Type of change

  • New feature

Scope check

  • I read the Contributing Guide
  • This aligns with RubyLLM's focus on LLM communication
  • This isn't application-specific logic that belongs in user code
  • This benefits most users, not just my specific use case

Quality check

  • I ran overcommit --install and all hooks pass
  • I tested my changes thoroughly
  • I updated documentation if needed
  • I didn't modify auto-generated files manually (models.json, aliases.json)

API changes

  • New public methods/classes

Related issues

Comment thread lib/ruby_llm.rb Outdated

def ask(message = nil, **options, &)
chat_options = options.slice(:model, :provider, :assume_model_exists, :context)
chat = Chat.new(**chat_options)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like ask is a pure one-shot. if chat is changed to an instance variable and we @chat ||= Chat.new(..) it keeps the simplicy of the API but extends the method into into a chat session. The only thing special about a chat session is its context. Adding a new method RubyLLM.clear to clear the context would be handy. OR maybe all that is required to clear the context is RubyLLM.ask("hello", context: [])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a look at sharing the instance variable across calls and ran into all sorts of issues in the specs with switching models. Leaving it at instance per call for now, but open to further feedback.

@tpaulshippy tpaulshippy marked this pull request as ready for review June 12, 2025 17:39
@cllns
Copy link
Copy Markdown

cllns commented Jun 13, 2025

I think it's fine to add the convenience class method for one-off requests (and convenience in the console), but I think creating an instance is better (i.e. more object-oriented) than relying on global state via the class method. So, I think the documentation should only be updated to include that RubyLLM.ask is an option for those who prefer it, but keep the constructor/instance as the default way to use the library.

And, along those lines, I think RubyLLM.ask should be a new session with a new context each time.

@tpaulshippy
Copy link
Copy Markdown
Contributor Author

I agree. Updated docs accordingly.

@crmne
Copy link
Copy Markdown
Owner

crmne commented Jul 16, 2025

This adds that complicated method which adds a ton of dependencies to all the other methods in the Chat interface, plus a ton of tests to pass without a clear advantage.

Is seriously adding .chat after RubyLLM such a big deal?

Closing.

@crmne crmne closed this Jul 16, 2025
@tpaulshippy
Copy link
Copy Markdown
Contributor Author

It's five characters, so no not a huge deal. But if I just want a single shot why do I have to type ".chat"? That's the argument. I know we don't need it. But I still want it. And I think it would be convenient in that situation.

@tpaulshippy
Copy link
Copy Markdown
Contributor Author

I guess if anyone really wants to use it they can add the wrapper method themselves either in their project or in a fork.

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.

4 participants