Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for rendering of mathematical formulas #515

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ceicke
Copy link
Contributor

@ceicke ceicke commented Oct 2, 2024

Better support the rendering of mathematical formulas.

Better support the rendering of mathematical formulas.
@ceicke
Copy link
Contributor Author

ceicke commented Oct 2, 2024

@krschacht I really don't know if this is a good way of doing this... what do you think? One test is already failing which already says a lot. Feels very hacky.

@krschacht
Copy link
Contributor

It's a good idea to add support for this! But hmm, that's tricky. The failing tests are simply because of removing html_escape() on the text that comes back. I tried adding it back and the tests pass, but then the math equation is not displayed properly—I just see a bunch of HTML.

The reason I had to add this html_escape was because sometimes the LLM will reply with actual HTML when it's explaining something to you. If this HTML is rendered by the browser rather than being escaped and displayed to the user, sometimes the text doesn't make sense. For example, you might ask "how do I make a table in HTML" and the LLM is trying to show you the HTML but the browser is rendering the HTML instead of displaying it. In addition, the LLM could return HTML that would cause the overall page layout to break. Somehow we'll have to differentiate between math and other html that the LLM may return.

Also, it's not impossible to use a JS library to do final rendering on the front-end, but I've been trying to avoid it. Parts of the page refresh a lot during streaming and I ran into some odd issues with render cycles. This is what led me to do serverside converting from markdown into HTML, using Redcarpet.

The easiest would be if Redcarpet supported some markup for math. I'm not very familiar with the options for math markup, but I did a little bit of digging and it seems like Redcarpet does not. However, in their forums they recommended a different ruby markdown parser: Kramdown. Maybe the best route to go would be to switch from Redcarpet to Kramdown and then lean into their support for math? e.g. https://kramdown.gettalong.org/documentation.html

@krschacht krschacht marked this pull request as draft November 5, 2024 16:53
@buithehien1991
Copy link

buithehien1991 commented Nov 21, 2024

Dear @ceicke,

I'm trying to your PR, but it's only display math after refresh page, New message isn't display with MathJax. I try using other event as turbo:load but it also only display for previous message, latest message isn't display while F5 too.

Ảnh màn hình 2024-11-21 lúc 13 05 40

@buithehien1991
Copy link

buithehien1991 commented Nov 21, 2024

Dear @ceicke,

I found latest message alway update by Connection.rb inside ApplicationCable

module ApplicationCable
  class Connection < ActionCable::Connection::Base
    def connect
      ActiveSupport::Notifications.subscribe "transmit_subscription_confirmation.action_cable" do |*args|
        # Sometimes the conversation page loads and it doesn't get the message to appear, even though
        # the logs show it was broadcasted. This is because there is a small race condition where right
        # in between when find() of the message and the client establishing the turbo stream connection,
        # the broadcast happens.
        #
        # The fix is to re-broadcast the last message to the client after the connection is established.
        #
        # FIXME: See if there is any conclusion on this Rails Issue: https://github.com/rails/rails/issues/52420
        event = ActiveSupport::Notifications::Event.new(*args)

        if event.payload[:channel_class] == "Turbo::StreamsChannel"
          identifier = JSON.parse(event.payload[:identifier])
          signed_stream_name = identifier["signed_stream_name"]
          stream_name = Base64.urlsafe_decode64(Turbo.signed_stream_verifier.verify(signed_stream_name))
          class_name, id = stream_name.split("/").last(2)
          conversation = class_name.classify.constantize.find(id)
          message = conversation.latest_message_for_version

          GetNextAIMessageJob.broadcast_updated_message(message)
        end
      end
    end
  end
end

I disabled this code, everything is OK.

@krschacht could you tell me how to call javascript after load final message? Here is code inside application.

document.addEventListener('turbo:load', (event) => {
  console.log('Document: load', event.detail.url)

  if (typeof MathJax !== 'undefined') {
    MathJax.typeset();
  }
})

Thank you so much!!

@buithehien1991
Copy link

Now, I'm hacky by

document.addEventListener('turbo:before-stream-render', (event) => {
  console.log(`Document: stream render (${event.target.getAttribute('action')} event)`, event.target)
  setTimeout(() => {
    console.log('Document: stream render done')
    if (typeof MathJax !== 'undefined') {
      MathJax.typeset();
    }
  }, 1000)
})

I think it's not good, but now I can't find other solution. if you have, please let me know

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.

3 participants