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

Attach images to conversations as URLs #424

Merged
merged 12 commits into from
Nov 5, 2024
3 changes: 3 additions & 0 deletions .github/workflows/rubyonrails.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ jobs:
env:
RAILS_ENV: test
DATABASE_URL: "postgres://rails:password@localhost:5432/hostedgpt_test"
APP_URL_PROTOCOL: "http"
APP_URL_HOST: "localhost"
APP_URL_PORT: "3000"
DISPLAY: "=:99"
CHROME_VERSION: "127.0.6533.119"

Expand Down
3 changes: 2 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,14 @@ gem "ffi", "~> 1.15.5" # explicitly requiring 15.5 until this is resolved: https
gem "amatch", "~> 0.4.1" # enables fuzzy comparison of strings, a tool uses this
gem "rails_heroicon", "~> 2.2.0"
gem "ruby-openai", "~> 7.0.1"
gem "anthropic", "~> 0.1.0"
gem "anthropic", "~> 0.1.0" # TODO update to the latest version
gem "tiktoken_ruby", "~> 0.0.9"
gem "solid_queue", "~> 1.0.0"
gem "name_of_person"
gem "actioncable-enhanced-postgresql-adapter" # longer paylaods w/ postgresql actioncable
gem "aws-sdk-s3", require: false
gem "postmark-rails"
gem "ostruct"

gem "omniauth", "~> 2.1"
gem "omniauth-google-oauth2", "~> 1.1"
Expand Down
2 changes: 2 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ GEM
omniauth-rails_csrf_protection (1.0.2)
actionpack (>= 4.2)
omniauth (~> 2.0)
ostruct (0.6.0)
parallel (1.24.0)
parser (3.2.2.4)
ast (~> 2.4.1)
Expand Down Expand Up @@ -452,6 +453,7 @@ DEPENDENCIES
omniauth (~> 2.1)
omniauth-google-oauth2 (~> 1.1)
omniauth-rails_csrf_protection (~> 1.0.2)
ostruct
pg (~> 1.1)
postmark-rails
pry-rails
Expand Down
20 changes: 20 additions & 0 deletions app/models/document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,26 @@ class Document < ApplicationRecord
validates :purpose, :filename, :bytes, presence: true
validate :file_present

def has_image?(variant = nil)
if variant.present?
return has_file_variant_processed?(variant)
end

file.attached?
end

def image_url(variant, fallback: nil)
return nil unless has_image?

if has_file_variant_processed?(variant)
fully_processed_url(variant)
elsif fallback.nil?
redirect_to_processed_path(variant)
else
fallback
end
end

def file_data_url(variant = :large)
return nil if !file.attached?

Expand Down
4 changes: 1 addition & 3 deletions app/models/message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ def finished?
(content_text.present? || content_tool_calls.present?)
end

def not_finished?
!finished?
end
def not_finished? = !finished?

private

Expand Down
17 changes: 3 additions & 14 deletions app/models/message/document_image.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,12 @@ module Message::DocumentImage
end

def has_document_image?(variant = nil)
has_image = documents.present? && documents.first.file.attached?
if has_image && variant
has_image = documents.first.has_file_variant_processed?(variant)
end

!!has_image
documents.present? && documents.first.has_image?(variant)
end

def document_image_path(variant, fallback: nil)
def document_image_url(variant, fallback: nil)
return nil unless has_document_image?

if documents.first.has_file_variant_processed?(variant)
documents.first.fully_processed_url(variant)
elsif fallback.nil?
documents.first.redirect_to_processed_path(variant)
else
fallback
end
documents.first.image_url(variant, fallback: fallback)
end
end
2 changes: 1 addition & 1 deletion app/services/ai_backend/open_ai.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def preceding_conversation_messages

content_with_images = [{ type: "text", text: message.content_text }]
content_with_images += message.documents.collect do |document|
{ type: "image_url", image_url: { url: document.file_data_url(:large) }}
{ type: "image_url", image_url: { url: document.image_url(:large) }}
end

{
Expand Down
8 changes: 4 additions & 4 deletions app/views/messages/_message.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,10 @@ end %>
role: "image-preview",
controller: "image-loader",
image_loader_message_scroller_outlet: "[data-role='inner-message']",
image_loader_url_value: message.document_image_path(:small),
image_loader_url_value: message.document_image_url(:small),
action: "modal#open",
} do %>
<%= image_tag message.document_image_path(:small, fallback: ""),
<%= image_tag message.document_image_url(:small, fallback: ""),
class: %|
my-0
mx-auto
Expand Down Expand Up @@ -254,10 +254,10 @@ end %>
class="flex flex-col md:flex-row justify-center"
data-controller="image-loader"
data-image-loader-message-scroller-outlet="[data-role='inner-message']"
data-image-loader-url-value="<%= message.document_image_path(:large) %>"
data-image-loader-url-value="<%= message.document_image_url(:large) %>"
data-turbo-permanent
>
<%= image_tag message.document_image_path(:large, fallback: ""),
<%= image_tag message.document_image_url(:large, fallback: ""),
class: "w-full h-auto",
data: {
image_loader_target: "image",
Expand Down
7 changes: 7 additions & 0 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,17 @@ class Application < Rails::Application
config.time_zone = "Central Time (US & Canada)"
config.eager_load_paths << Rails.root.join("lib")

Setting.require_keys!(:app_url_protocol, :app_url_host, :app_url_port)
config.app_url_protocol = Setting.app_url_protocol
config.app_url_host = Setting.app_url_host
config.app_url_port = Setting.app_url_port
config.app_url = "#{Setting.app_url_protocol}://#{Setting.app_url_host}:#{Setting.app_url_port}"

# Active Storage
if Feature.cloudflare_storage?
config.active_storage.service = :cloudflare
else
puts "using database service"
jlvallelonga marked this conversation as resolved.
Show resolved Hide resolved
config.active_storage.service = :database
end

Expand Down
3 changes: 2 additions & 1 deletion config/environments/development.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,9 @@
log_polling = ENV["SOLID_QUEUE_LOG_POLLING_ON"] != "false"
config.solid_queue.silence_polling = log_polling # NOTE: this is backwards, true means silence

config.web_console.permissions = ["192.168.0.0/16", "172.17.0.0/16"]
config.web_console.permissions = ["192.168.0.0/16", "172.17.0.0/16", "172.18.0.0/16"]

config.hosts << Setting.app_url_host
config.hosts << ENV["DEV_HOST"] if ENV["DEV_HOST"].present?

stdout_logger = ActiveSupport::Logger.new(STDOUT)
Expand Down
1 change: 1 addition & 0 deletions config/environments/production.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@
# /.*\.example\.com/ # Allow requests from subdomains like `www.example.com`
# ]
config.hosts = ENV["PRODUCTION_HOST"].split(",").map(&:strip) if ENV["PRODUCTION_HOST"]
# config.hosts << Setting.app_url_host

# Skip DNS rebinding protection for the default health check endpoint.
config.host_authorization = { exclude: ->(request) { request.path == "/up" } }
Expand Down
3 changes: 3 additions & 0 deletions config/options.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ shared:
password_reset_email: <%= ENV["PASSWORD_RESET_EMAIL_FEATURE"] || default_to(false, except_env_test: true) %>
settings:
# Be sure to add these ENV to docker-compose.yml
app_url_protocol: <%= ENV["APP_URL_PROTOCOL"] %>
app_url_host: <%= ENV["APP_URL_HOST"] %>
app_url_port: <%= ENV["APP_URL_PORT"] %>
product_name: <%= ENV["PRODUCT_NAME"] || "HostedGPT" %>
default_openai_key: <%= ENV["DEFAULT_OPENAI_KEY"] %>
default_anthropic_key: <%= ENV["DEFAULT_ANTHROPIC_KEY"] %>
Expand Down
3 changes: 3 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ services:
target: development
environment:
# Be sure to add environment variables to config/options.yml
- APP_URL_PROTOCOL
- APP_URL_HOST
- APP_URL_PORT
- DATABASE_URL=postgres://app:secret@postgres/app_development
- DEV_HOST=${DEV_HOST:-localhost} # Set if you want to use a different hostname
- OVERMIND_COLORS=2,3,5
Expand Down
11 changes: 8 additions & 3 deletions lib/active_storage/service/postgresql_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,15 @@ def url_helpers

def url_options
if ActiveStorage::Current.respond_to?(:url_options)
ActiveStorage::Current.url_options
else
{ host: ActiveStorage::Current.host }
url_opts = ActiveStorage::Current.url_options
return url_opts if url_opts.is_a?(Hash)
end

return {
protocol: Rails.application.config.app_url_protocol,
host: Rails.application.config.app_url_host,
port: Rails.application.config.app_url_port,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

fixes this: #398 (comment)

the problem is that the ActiveStorage::Current.url_options are set only during a request lifecycle (with ActiveStorage::SetCurrent) and workers don't run within the context of a request. This fix also allows you to get the url from within the rails console.

see: https://github.com/rails/rails/blob/main/activestorage/app/controllers/concerns/active_storage/set_current.rb
and: rails/rails#40855 (comment)
and: rails/rails#42520

Copy link
Contributor Author

Choose a reason for hiding this comment

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

on dang, nice debugging on this. i couldn’t figure out what the difference was within the worker

end
end
end
10 changes: 5 additions & 5 deletions test/models/message/document_image_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ class Message::DocumentImageTest < ActiveSupport::TestCase
refute messages(:examine_this).has_document_image?(:small)
end

test "document_image_path" do
assert messages(:examine_this).document_image_path(:small).is_a?(String)
assert messages(:examine_this).document_image_path(:small).starts_with?("/rails/active_storage/representations/redirect")
test "document_image_url" do
assert messages(:examine_this).document_image_url(:small).is_a?(String)
assert messages(:examine_this).document_image_url(:small).starts_with?("/rails/active_storage/representations/redirect")
end

test "document_image_path with fallback" do
assert_equal "", messages(:examine_this).document_image_path(:small, fallback: "")
test "document_image_url with fallback" do
assert_equal "", messages(:examine_this).document_image_url(:small, fallback: "")
end
end
Loading