Fix callback persistence chaining using Fanout pattern #509
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
When using the Rails integration (
acts_as_chat), persistence callbacks are set up on the underlyingRubyLLM::Chatinstance. However, because the original implementation of@oncallbacks inChatsimply replaced the existing proc, users setting their own callbacks (e.g.,chat.to_llm.on_new_message { ... }) would accidentally overwrite and remove the persistence logic. This resulted in lost chat history.Solution
This PR implements a Callback Fanout pattern directly in
RubyLLM::Chat.ChatMethodsandActsAsLegacy) was previously trying to manually wrap and chain callbacks to prevent this issue. This brittle logic has been removed in favor of the robust native support inChat.on_new_message(and others) appends the new callback rather than replacing the existing one. This ensures that both system callbacks (persistence) and user callbacks run successfully.Testing
spec/ruby_llm/active_record/acts_as_spec.rbspecifically checking that chaining callbacks onto_llmpreserves persistence.