From e7eba423c835f6a165ad1bd9acd4a7d010103b68 Mon Sep 17 00:00:00 2001 From: Keith Schacht Date: Tue, 5 Nov 2024 09:37:36 -0600 Subject: [PATCH 01/43] Revert "Attach images to conversations as URLs (#424)" This reverts commit 5e99b819142eeaf3745f9fa2f64d9a86bbdf809d. --- .github/workflows/rubyonrails.yml | 6 ------ Dockerfile | 4 ++-- Gemfile | 3 +-- app/models/document.rb | 20 ------------------- app/models/message.rb | 4 +++- app/models/message/document_image.rb | 17 +++++++++++++--- app/services/ai_backend/open_ai.rb | 2 +- app/views/messages/_message.html.erb | 8 ++++---- config/application.rb | 8 -------- config/environments/development.rb | 3 +-- config/environments/production.rb | 1 - config/options.yml | 4 ---- docker-compose.yml | 4 ---- .../service/postgresql_service.rb | 11 +++------- test/models/message/document_image_test.rb | 10 +++++----- 15 files changed, 34 insertions(+), 71 deletions(-) diff --git a/.github/workflows/rubyonrails.yml b/.github/workflows/rubyonrails.yml index b4cbf6c66..9b9fe16f0 100644 --- a/.github/workflows/rubyonrails.yml +++ b/.github/workflows/rubyonrails.yml @@ -28,9 +28,6 @@ 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" steps: - name: Checkout code uses: actions/checkout@v4 @@ -71,9 +68,6 @@ 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" diff --git a/Dockerfile b/Dockerfile index 8a3c68ba2..f00aedb32 100644 --- a/Dockerfile +++ b/Dockerfile @@ -53,7 +53,7 @@ RUN bundle exec bootsnap precompile app/ lib/ RUN grep -l '#!/usr/bin/env ruby' /rails/bin/* | xargs sed -i '/^#!/aDir.chdir File.expand_path("..", __dir__)' # Precompiling assets for production without requiring secret RAILS_MASTER_KEY -RUN SECRET_KEY_BASE_DUMMY=1 VALIDATE_ENV_VARS=0 ./bin/rails assets:precompile +RUN SECRET_KEY_BASE_DUMMY=1 ./bin/rails assets:precompile # Final stage for app image @@ -134,7 +134,7 @@ COPY . . # Precompiling assets for production without requiring secret RAILS_MASTER_KEY RUN bundle exec bootsnap precompile --gemfile app/ lib/ -RUN SECRET_KEY_BASE_DUMMY=1 VALIDATE_ENV_VARS=0 ./bin/rails assets:precompile +RUN SECRET_KEY_BASE_DUMMY=1 ./bin/rails assets:precompile RUN mkdir -p log tmp bin diff --git a/Gemfile b/Gemfile index 7879c5ded..3013be528 100644 --- a/Gemfile +++ b/Gemfile @@ -41,14 +41,13 @@ 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" # TODO update to the latest version +gem "anthropic", "~> 0.1.0" 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" diff --git a/app/models/document.rb b/app/models/document.rb index 68ab2f5b9..6fb523877 100644 --- a/app/models/document.rb +++ b/app/models/document.rb @@ -19,26 +19,6 @@ 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? diff --git a/app/models/message.rb b/app/models/message.rb index 315b2b97e..36148bedb 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -43,7 +43,9 @@ def finished? (content_text.present? || content_tool_calls.present?) end - def not_finished? = !finished? + def not_finished? + !finished? + end private diff --git a/app/models/message/document_image.rb b/app/models/message/document_image.rb index c778cea54..0b6358c94 100644 --- a/app/models/message/document_image.rb +++ b/app/models/message/document_image.rb @@ -8,12 +8,23 @@ module Message::DocumentImage end def has_document_image?(variant = nil) - documents.present? && documents.first.has_image?(variant) + has_image = documents.present? && documents.first.file.attached? + if has_image && variant + has_image = documents.first.has_file_variant_processed?(variant) + end + + !!has_image end - def document_image_url(variant, fallback: nil) + def document_image_path(variant, fallback: nil) return nil unless has_document_image? - documents.first.image_url(variant, fallback: fallback) + 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 end end diff --git a/app/services/ai_backend/open_ai.rb b/app/services/ai_backend/open_ai.rb index 3930a7d0e..f00fc713f 100644 --- a/app/services/ai_backend/open_ai.rb +++ b/app/services/ai_backend/open_ai.rb @@ -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.image_url(:large) }} + { type: "image_url", image_url: { url: document.file_data_url(:large) }} end { diff --git a/app/views/messages/_message.html.erb b/app/views/messages/_message.html.erb index 794d00101..4ba6a954a 100644 --- a/app/views/messages/_message.html.erb +++ b/app/views/messages/_message.html.erb @@ -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_url(:small), + image_loader_url_value: message.document_image_path(:small), action: "modal#open", } do %> - <%= image_tag message.document_image_url(:small, fallback: ""), + <%= image_tag message.document_image_path(:small, fallback: ""), class: %| my-0 mx-auto @@ -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_url(:large) %>" + data-image-loader-url-value="<%= message.document_image_path(:large) %>" data-turbo-permanent > - <%= image_tag message.document_image_url(:large, fallback: ""), + <%= image_tag message.document_image_path(:large, fallback: ""), class: "w-full h-auto", data: { image_loader_target: "image", diff --git a/config/application.rb b/config/application.rb index 2f53f0fa0..4bc219189 100644 --- a/config/application.rb +++ b/config/application.rb @@ -37,14 +37,6 @@ class Application < Rails::Application config.time_zone = "Central Time (US & Canada)" config.eager_load_paths << Rails.root.join("lib") - if Setting.validate_env_vars == "1" - Setting.require_keys!(:app_url_protocol, :app_url_host, :app_url_port) - end - 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 diff --git a/config/environments/development.rb b/config/environments/development.rb index 53b35c62f..499c3c2ba 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -85,9 +85,8 @@ 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", "172.18.0.0/16"] + config.web_console.permissions = ["192.168.0.0/16", "172.17.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) diff --git a/config/environments/production.rb b/config/environments/production.rb index f5f5c9b1b..a43f85e4c 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -98,7 +98,6 @@ # /.*\.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" } } diff --git a/config/options.yml b/config/options.yml index 44ee2ebe8..ee007b39c 100644 --- a/config/options.yml +++ b/config/options.yml @@ -47,10 +47,6 @@ 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"] %> - validate_env_vars: <%= ENV["VALIDATE_ENV_VARS"] || "1" %> product_name: <%= ENV["PRODUCT_NAME"] || "HostedGPT" %> default_openai_key: <%= ENV["DEFAULT_OPENAI_KEY"] %> default_anthropic_key: <%= ENV["DEFAULT_ANTHROPIC_KEY"] %> diff --git a/docker-compose.yml b/docker-compose.yml index 28d6e1c26..012612c85 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -22,10 +22,6 @@ services: target: development environment: # Be sure to add environment variables to config/options.yml - - APP_URL_PROTOCOL - - APP_URL_HOST - - APP_URL_PORT - - VALIDATE_ENV_VARS - 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 diff --git a/lib/active_storage/service/postgresql_service.rb b/lib/active_storage/service/postgresql_service.rb index c906b0ef5..e4d46646a 100644 --- a/lib/active_storage/service/postgresql_service.rb +++ b/lib/active_storage/service/postgresql_service.rb @@ -148,15 +148,10 @@ def url_helpers def url_options if ActiveStorage::Current.respond_to?(:url_options) - url_opts = ActiveStorage::Current.url_options - return url_opts if url_opts.is_a?(Hash) + ActiveStorage::Current.url_options + else + { host: ActiveStorage::Current.host } end - - return { - protocol: Rails.application.config.app_url_protocol, - host: Rails.application.config.app_url_host, - port: Rails.application.config.app_url_port, - } end end end diff --git a/test/models/message/document_image_test.rb b/test/models/message/document_image_test.rb index 0ceca2088..1b0c8c675 100644 --- a/test/models/message/document_image_test.rb +++ b/test/models/message/document_image_test.rb @@ -16,12 +16,12 @@ class Message::DocumentImageTest < ActiveSupport::TestCase refute messages(:examine_this).has_document_image?(:small) end - 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") + 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") end - test "document_image_url with fallback" do - assert_equal "", messages(:examine_this).document_image_url(:small, fallback: "") + test "document_image_path with fallback" do + assert_equal "", messages(:examine_this).document_image_path(:small, fallback: "") end end From 452da3ed0bd6412298b4b736eb05127ae838da0f Mon Sep 17 00:00:00 2001 From: David Patnode Date: Tue, 5 Nov 2024 10:48:37 -0600 Subject: [PATCH 02/43] Better support local dev - local DB documentation & env vars (#456) --- README.md | 3 ++- config/database.yml | 8 +------- docker-compose.yml | 2 +- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 78c3a5fbc..fba7919f4 100644 --- a/README.md +++ b/README.md @@ -296,7 +296,8 @@ Every time you pull new changes down, kill docker (if it's running) and re-run: HostedGPT requires these services to be running: -- Postgres ([installation instructions](https://www.postgresql.org/download/)) +- Postgres (`brew install postgresql@16` or other [install instructions](https://www.postgresql.org/download/)) + - By default postgres will create a default user and following the instructions below to run the app should just work without any additional config, but if you want to set a specific username and password for your database then set the environment variable `DATABASE_URL=postgres://username:password@localhost/hostedgpt_development` (replacing `username`, `password`, and `hostedgpt_development` (database name) as appropriate - rbenv ([installation instructions](https://github.com/rbenv/rbenv)) - ImageMagick (`brew install imagemagick` should work on Mac ) diff --git a/config/database.yml b/config/database.yml index 6fa635417..f30a6c41c 100644 --- a/config/database.yml +++ b/config/database.yml @@ -1,24 +1,18 @@ default: &default adapter: postgresql encoding: unicode - host: localhost pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %> port: <%= ENV['HOSTEDGPT_DATABASE_PORT'] || 5432 %> <% if RUBY_PLATFORM =~ /darwin/ %> gssencmode: disable <% end %> + url: <%= ENV['DATABASE_URL'] %> development: <<: *default - database: <%= ENV['HOSTEDGPT_DEV_DB'] || "hostedgpt_development" %> test: <<: *default - database: <%= ENV['HOSTEDGPT_TEST_DB'] || "hostedgpt_test" %> production: <<: *default - database: hostedgpt_production - username: hostedgpt - password: <%= ENV["HOSTEDGPT_DATABASE_PASSWORD"] %> - \ No newline at end of file diff --git a/docker-compose.yml b/docker-compose.yml index 012612c85..3366bfb0c 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -22,7 +22,7 @@ services: target: development environment: # Be sure to add environment variables to config/options.yml - - DATABASE_URL=postgres://app:secret@postgres/app_development + - DATABASE_URL - DEV_HOST=${DEV_HOST:-localhost} # Set if you want to use a different hostname - OVERMIND_COLORS=2,3,5 - ALLOWED_REQUEST_ORIGINS From 7f2e4fc3d4574ae5b0c4983b6c27ca38b08c9247 Mon Sep 17 00:00:00 2001 From: Keith Schacht Date: Tue, 5 Nov 2024 10:53:05 -0600 Subject: [PATCH 03/43] Make the database config more resilient --- README.md | 6 +++--- config/database.yml | 9 +++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index fba7919f4..2ef1e6e2a 100644 --- a/README.md +++ b/README.md @@ -297,14 +297,14 @@ Every time you pull new changes down, kill docker (if it's running) and re-run: HostedGPT requires these services to be running: - Postgres (`brew install postgresql@16` or other [install instructions](https://www.postgresql.org/download/)) - - By default postgres will create a default user and following the instructions below to run the app should just work without any additional config, but if you want to set a specific username and password for your database then set the environment variable `DATABASE_URL=postgres://username:password@localhost/hostedgpt_development` (replacing `username`, `password`, and `hostedgpt_development` (database name) as appropriate - rbenv ([installation instructions](https://github.com/rbenv/rbenv)) - ImageMagick (`brew install imagemagick` should work on Mac ) 1. `cd` into your local repository clone 1. `rbenv install` to install the correct ruby version (it reads the .ruby-version in the repo) -1. `bin/dev` starts up all the services, installs gems, and inits database (don't run **db:setup** as it will not configure encryption properly) -1. Open [http://localhost:3000](http://localhost:3000) and register as a new user +1. Do NOT run db:setup as it will not configure encryption properly. Proceed to the next step and it will automatically configure the database. +1. `bin/dev` starts up all the services, installs gems, and handles db. Note: The app should automatically configure a database, but if you get any database errors or want to change the default configuration, set the environment variable `DATABASE_URL=postgres://username:password@localhost:5432/hostedgpt_development` (replacing `username`, `password`, `hostedgpt_development` with your database name, and 5432 with your database port number). +1. Open [http://localhost:3000](http://localhost:3000) and register as a new user. 1. `bin/rails test` and `bin/rails test:system` to run the comprehensive tests 1. The project root has an `.editorconfig` file to help eliminate whitespace differences in pull requests. It's nice if you install an extension in your IDE to utilize this (e.g. VS Code has "EditorConfig for VS Code"). 1. If you want a few fake users and a bunch of conversations and other data pre-populated in the database, you can load fixtures into the development database. This can be helpful, for example, if you want to test a migration and save yourself the time manually creating a bunch of data: `bin/rails db:fixtures:load` diff --git a/config/database.yml b/config/database.yml index f30a6c41c..b657cbe2b 100644 --- a/config/database.yml +++ b/config/database.yml @@ -7,12 +7,21 @@ default: &default gssencmode: disable <% end %> url: <%= ENV['DATABASE_URL'] %> + # url format: postgres://username:password@localhost:5432/database_name development: <<: *default + database: <%= ENV['HOSTEDGPT_DEV_DB'] || "hostedgpt_development" %> + # It's best to set DATABASE_URL to configure database details. That overrides this "database" name. Note: Docker sets DATABASE_URL inside docker-compose.yml test: <<: *default + database: <%= ENV['HOSTEDGPT_TEST_DB'] || "hostedgpt_test" %> + # It's best to set DATABASE_URL to configure database details. That overrides this "database" name. production: <<: *default + database: hostedgpt_production + username: hostedgpt + password: <%= ENV["HOSTEDGPT_DATABASE_PASSWORD"] %> + # It's best to set DATABASE_URL to configure database details. That overrides this "database" name. From edbbffd8cb809eaae290507f76b1307aff2f3647 Mon Sep 17 00:00:00 2001 From: Keith Schacht Date: Tue, 5 Nov 2024 11:46:39 -0600 Subject: [PATCH 04/43] Update some fly instructions & commands (#534) --- Gemfile.lock | 6 ++++++ README.md | 15 ++++++++------- app/services/fly.rb | 8 +++++--- fly.toml | 2 +- 4 files changed, 20 insertions(+), 11 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 83d95e829..5e67b9ff8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -201,6 +201,8 @@ GEM net-smtp (0.5.0) net-protocol nio4r (2.7.3) + nokogiri (1.16.7-arm64-darwin) + racc (~> 1.4) nokogiri (1.16.7-x86_64-linux) racc (~> 1.4) oauth2 (2.0.9) @@ -398,9 +400,12 @@ GEM railties (>= 6.0.0) stringio (3.1.1) sync (0.5.0) + tailwindcss-rails (2.7.2-arm64-darwin) + railties (>= 7.0.0) tailwindcss-rails (2.7.2-x86_64-linux) railties (>= 7.0.0) thor (1.3.2) + tiktoken_ruby (0.0.9-arm64-darwin) tiktoken_ruby (0.0.9-x86_64-linux) timecop (0.9.8) timeout (0.4.1) @@ -431,6 +436,7 @@ GEM zeitwerk (2.7.1) PLATFORMS + arm64-darwin-23 x86_64-linux DEPENDENCIES diff --git a/README.md b/README.md index 2ef1e6e2a..e8c6b33a9 100644 --- a/README.md +++ b/README.md @@ -81,18 +81,19 @@ If you encountered an error while waiting for the services to be deployed on Ren Deploying to Fly.io is another great option. It's not quite one-click like Render and it's not 100% free. But we've made the configuration really easy for you and the cost should be about $2 per month, and Render costs $7 per month after 90 days of free service so Fly is actually less expensive over the long term. -1. Click Fork > Create New Fork at the top of this repository. Pull your forked repository down to your computer (the usual git clone ...). +1. Click Fork > Create New Fork at the top of this repository. **Pull your forked repository down to your computer (the usual git clone ...)**. +1. Go into the directory you just created with your git clone and run `bundle` 1. Install the Fly command-line tool on Mac with `brew install flyctl` otherwise `curl -L https://fly.io/install.sh | sh` ([view instructions](https://fly.io/docs/hands-on/install-flyctl/)) -1. Think of an internal Fly name for your app, it has to be unique to all of Fly, and then in the root directory of the repository you pulled down, run `fly launch --build-only --copy-config --name=APP_NAME_YOU_CHOSE` +1. Think of an internal Fly name for your app, it has to be unique to all of Fly. You'll use this **APP_NAME** three times in the steps below. First, in the root directory of the repository you pulled down, run `fly launch --build-only --copy-config --name=APP_NAME` - Say "Yes" when it asks if you want to tweak these settings -1. When it opens your browser, change the Database to `Fly Postgres` with a unique name such as `[APP_NAME]-db` and you can set the configuration to `Development`. +1. When it opens your browser, (i) change the Database to `Fly Automated Postgres`, (ii) set the name to be `[APP_NAME]-db`, (iii) and you can set the configuration to `Development`. 1. Click `Confirm Settings` at the bottom of the page and close the browser. -1. The app will do a bunch of build steps and then return to the command line. Scroll through the output and save the Postgres username & password somewhere as you'll never be able to see those again. -1. Next run `bin/rails db:setup_encryption[true]`. This will initialize some private keys for your app and send them to Fly. +1. The app will do a bunch of build steps and then return to the command line. Scroll through the output and **save the Postgres username & password somewhere as you'll never be able to see those again**. +1. Next run `bin/rails db:setup_encryption[true]`. This will initialize some private keys for your app and send them to Fly. (If you get an error you may have forgotten to run `bundle`). 1. Run `fly deploy --ha=false` -1. Assuming you chose `Development` as the DB size in the step above, now you should run `bin/rails db:fly[APP_NAME_FROM_EARLIER,swap,512]` This will increase the swap on your database machine so that it doesn't crash. +1. Assuming you chose `Development` as the DB size in the step above, now you should run `bin/rails db:fly[APP_NAME,swap,512]` This will increase the swap on your database machine so that it doesn't crash since the Development database has less ram. You may want to read about [configuring optional features](#configure-optional-features). @@ -303,7 +304,7 @@ HostedGPT requires these services to be running: 1. `cd` into your local repository clone 1. `rbenv install` to install the correct ruby version (it reads the .ruby-version in the repo) 1. Do NOT run db:setup as it will not configure encryption properly. Proceed to the next step and it will automatically configure the database. -1. `bin/dev` starts up all the services, installs gems, and handles db. Note: The app should automatically configure a database, but if you get any database errors or want to change the default configuration, set the environment variable `DATABASE_URL=postgres://username:password@localhost:5432/hostedgpt_development` (replacing `username`, `password`, `hostedgpt_development` with your database name, and 5432 with your database port number). +1. `bin/dev` starts up all the services, installs gems, and handles db. Note: The app should automatically configure a database, but if you get any database errors or want to change the default configuration, set the environment variable DATABASE_URL=postgres://username:password@localhost:5432/hostedgpt_development (replacing username, password, hostedgpt_development with your database name, and 5432 with your database port number). 1. Open [http://localhost:3000](http://localhost:3000) and register as a new user. 1. `bin/rails test` and `bin/rails test:system` to run the comprehensive tests 1. The project root has an `.editorconfig` file to help eliminate whitespace differences in pull requests. It's nice if you install an extension in your IDE to utilize this (e.g. VS Code has "EditorConfig for VS Code"). diff --git a/app/services/fly.rb b/app/services/fly.rb index d82d2415e..b47e96f99 100644 --- a/app/services/fly.rb +++ b/app/services/fly.rb @@ -7,9 +7,11 @@ def change_db_swap(app:, swap:) end swap = swap.to_i - app_id = get_apps.find { |m| m.name == app_name }&.id + apps = get_apps + app_id = apps.find { |m| m.name == app_name }&.id if app_id.nil? puts "Could not find the app named #{app_name}. Aborting." + puts "These are all the app names on your Fly account: #{apps.map(&:name).join(", ")}" return end @@ -38,13 +40,13 @@ def get_machines(app_name) end def patch_machine(app_name, id, config) - patch("https://api.machines.dev/v1/apps/#{app_name}/machines/#{id}").param(config: config) + post("https://api.machines.dev/v1/apps/#{app_name}/machines/#{id}").param(config: config) end private def bearer_token - `fly auth token`.chop + @bearer_token ||= `fly auth token`.chop.split.last end def header diff --git a/fly.toml b/fly.toml index 976295258..da24b141f 100644 --- a/fly.toml +++ b/fly.toml @@ -23,7 +23,7 @@ console_command = '/rails/bin/rails console' [http_service] internal_port = 3000 force_https = true - auto_stop_machines = false + auto_stop_machines = 'off' auto_start_machines = true min_machines_running = 1 processes = ['app'] From e7776cab2576741e103252bc537463077869dd7e Mon Sep 17 00:00:00 2001 From: Dr Nic Williams Date: Thu, 7 Nov 2024 13:09:15 +1000 Subject: [PATCH 05/43] Bump rubocop TargetRubyVersion: 3.3 (#537) --- .rubocop.yml | 2 +- app/models/message.rb | 12 ++++++------ app/models/user/registerable.rb | 14 +++++++------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 6fad3fd68..c60afbe88 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -9,7 +9,7 @@ require: - rubocop-performance AllCops: - TargetRubyVersion: 3.0 + TargetRubyVersion: 3.3 # RuboCop has a bunch of cops enabled by default. This setting tells RuboCop # to ignore them, so only the ones explicitly set in this file are enabled. DisabledByDefault: true diff --git a/app/models/message.rb b/app/models/message.rb index 36148bedb..a89a01e1b 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -50,7 +50,7 @@ def not_finished? private def create_conversation - self.conversation = Conversation.create!(user: Current.user, assistant: assistant) + self.conversation = Conversation.create!(user: Current.user, assistant:) end def validate_conversation @@ -62,12 +62,12 @@ def validate_assistant end def start_assistant_reply - m = conversation.messages.create!( - assistant: assistant, + conversation.messages.create!( + assistant:, role: :assistant, content_text: nil, - version: version, - index: index+1 + version:, + index: index + 1 ) end @@ -77,7 +77,7 @@ def set_last_assistant_message def update_assistant_on_conversation return if conversation.assistant == assistant - conversation.update!(assistant: assistant) + conversation.update!(assistant:) end def update_input_token_cost diff --git a/app/models/user/registerable.rb b/app/models/user/registerable.rb index 0ddfcb322..3b12714f5 100644 --- a/app/models/user/registerable.rb +++ b/app/models/user/registerable.rb @@ -54,13 +54,13 @@ def create_initial_assistants_etc output_token_cost_cents = output_token_cost_per_million/million language_models.create!( - api_name: api_name, - api_service: api_service, - name: name, + api_name:, + api_service:, + name:, supports_tools: true, - supports_images: supports_images, - input_token_cost_cents: input_token_cost_cents, - output_token_cost_cents: output_token_cost_cents, + supports_images:, + input_token_cost_cents:, + output_token_cost_cents:, ) end @@ -73,7 +73,7 @@ def create_initial_assistants_etc input_token_cost_cents = input_token_cost_per_million/million output_token_cost_cents = output_token_cost_per_million/million - language_models.create!(api_name: api_name, api_service: api_service, name: name, supports_tools: false, supports_images: supports_images, input_token_cost_cents: input_token_cost_cents, output_token_cost_cents: output_token_cost_cents) + language_models.create!(api_name:, api_service:, name:, supports_tools: false, supports_images:, input_token_cost_cents:, output_token_cost_cents:) end assistants.create! name: "GPT-4o", language_model: language_models.find_by(api_name: LanguageModel::BEST_GPT) From c3520cc2d2a4dd1f8793c518998d6fc2fba5d4a1 Mon Sep 17 00:00:00 2001 From: Keith Schacht Date: Wed, 6 Nov 2024 21:15:00 -0600 Subject: [PATCH 06/43] Hotfix: Correct broken docker-compose config for D --- docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index 3366bfb0c..012612c85 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -22,7 +22,7 @@ services: target: development environment: # Be sure to add environment variables to config/options.yml - - DATABASE_URL + - 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 - ALLOWED_REQUEST_ORIGINS From 692ff03a395b2b7ecf3a310fa3c23c41fe6187e0 Mon Sep 17 00:00:00 2001 From: Dr Nic Williams Date: Sun, 10 Nov 2024 11:24:52 +1000 Subject: [PATCH 07/43] Dynamic "best" Language Model (#538) --- .../settings/language_models_controller.rb | 4 +- app/models/language_model.rb | 38 ++++++------------- app/models/run.rb | 2 +- app/models/user/registerable.rb | 12 +++--- app/services/ai_backend/anthropic.rb | 2 +- app/services/ai_backend/open_ai.rb | 2 +- .../settings/language_models/_form.html.erb | 6 +++ .../settings/language_models/index.html.erb | 2 + ...41106213119_add_best_to_language_models.rb | 5 +++ db/schema.rb | 3 +- .../language_models_controller_test.rb | 33 ++++++++++++++++ test/fixtures/language_models.yml | 2 + .../get_next_ai_message_job_anthropic_test.rb | 10 +++-- .../get_next_ai_message_job_openai_test.rb | 10 +++-- test/models/language_model_test.rb | 20 ---------- 15 files changed, 83 insertions(+), 68 deletions(-) create mode 100644 db/migrate/20241106213119_add_best_to_language_models.rb diff --git a/app/controllers/settings/language_models_controller.rb b/app/controllers/settings/language_models_controller.rb index 5ff7f919d..74610bc4e 100644 --- a/app/controllers/settings/language_models_controller.rb +++ b/app/controllers/settings/language_models_controller.rb @@ -3,7 +3,7 @@ class Settings::LanguageModelsController < Settings::ApplicationController before_action :set_system_language_model, only: [:show] def index - @language_models = LanguageModel.for_user(Current.user).order(updated_at: :desc) + @language_models = LanguageModel.for_user(Current.user).ordered end def edit @@ -53,6 +53,6 @@ def set_system_language_model end def language_model_params - params.require(:language_model).permit(:api_name, :name, :supports_images, :supports_tools, :api_service_id) + params.require(:language_model).permit(:api_name, :name, :best, :supports_images, :supports_tools, :api_service_id) end end diff --git a/app/models/language_model.rb b/app/models/language_model.rb index 72e26c348..e813e307f 100644 --- a/app/models/language_model.rb +++ b/app/models/language_model.rb @@ -1,27 +1,5 @@ # We don"t care about large or not class LanguageModel < ApplicationRecord - BEST_GPT = "gpt-best" - BEST_CLAUDE = "claude-best" - BEST_GROQ = "groq-best" - - BEST_MODELS = { - BEST_GPT => "gpt-4o-2024-08-06", - BEST_CLAUDE => "claude-3-5-sonnet-20240620", - BEST_GROQ => "llama3-70b-8192", - } - - BEST_MODEL_INPUT_PRICES = { - BEST_GPT => 250, - BEST_CLAUDE => 300, - BEST_GROQ => 59, - } - - BEST_MODEL_OUTPUT_PRICES = { - BEST_GPT => 1000, - BEST_CLAUDE => 1500, - BEST_GROQ => 79, - } - belongs_to :user belongs_to :api_service @@ -33,16 +11,14 @@ class LanguageModel < ApplicationRecord validates :api_name, :name, :position, presence: true before_save :soft_delete_assistants, if: -> { has_attribute?(:deleted_at) && deleted_at && deleted_at_changed? && deleted_at_was.nil? } + after_save :update_best_language_model_for_api_service - scope :ordered, -> { order(:position) } + scope :ordered, -> { order(Arel.sql("CASE WHEN best THEN 0 ELSE position END")).order(:position) } scope :for_user, ->(user) { where(user_id: user.id).not_deleted } + scope :best_for_api_service, ->(api_service) { where(best: true, api_service: api_service) } delegate :ai_backend, to: :api_service - def provider_name - BEST_MODELS[api_name] || api_name - end - def created_by_current_user? user == Current.user end @@ -61,4 +37,12 @@ def populate_position def soft_delete_assistants assistants.update_all(deleted_at: Time.current) end + + # Only one best language model per API service + def update_best_language_model_for_api_service + if best? + api_service.language_models.update_all(best: false) + update_column(:best, true) + end + end end diff --git a/app/models/run.rb b/app/models/run.rb index 9e6976a04..438ba938e 100644 --- a/app/models/run.rb +++ b/app/models/run.rb @@ -14,6 +14,6 @@ class Run < ApplicationRecord private def set_model - self.model = assistant&.language_model&.provider_name + self.model = assistant&.language_model&.api_name end end diff --git a/app/models/user/registerable.rb b/app/models/user/registerable.rb index 3b12714f5..5b4340db4 100644 --- a/app/models/user/registerable.rb +++ b/app/models/user/registerable.rb @@ -12,11 +12,8 @@ def create_initial_assistants_etc anthropic_api_service = api_services.create!(url: APIService::URL_ANTHROPIC, driver: :anthropic, name: "Anthropic") groq_api_service = api_services.create!(url: APIService::URL_GROQ, driver: :openai, name: "Groq") + best_models = %w[gpt-4o claude-3-5-sonnet-20240620 llama3-70b-8192] [ - [LanguageModel::BEST_GPT, "Best OpenAI Model", true, open_ai_api_service, LanguageModel::BEST_MODEL_INPUT_PRICES[LanguageModel::BEST_GPT], LanguageModel::BEST_MODEL_OUTPUT_PRICES[LanguageModel::BEST_GPT]], - [LanguageModel::BEST_CLAUDE, "Best Anthropic Model", true, anthropic_api_service, LanguageModel::BEST_MODEL_INPUT_PRICES[LanguageModel::BEST_CLAUDE], LanguageModel::BEST_MODEL_OUTPUT_PRICES[LanguageModel::BEST_CLAUDE]], - [LanguageModel::BEST_GROQ, "Best Open-Source Model", true, groq_api_service, LanguageModel::BEST_MODEL_INPUT_PRICES[LanguageModel::BEST_GROQ], LanguageModel::BEST_MODEL_OUTPUT_PRICES[LanguageModel::BEST_GROQ]], - ["gpt-4o", "GPT-4o (latest)", true, open_ai_api_service, 250, 1000], ["gpt-4o-2024-08-06", "GPT-4o Omni Multimodal (2024-08-06)", true, open_ai_api_service, 250, 1000], ["gpt-4o-2024-05-13", "GPT-4o Omni Multimodal (2024-05-13)", true, open_ai_api_service, 500, 1500], @@ -57,6 +54,7 @@ def create_initial_assistants_etc api_name:, api_service:, name:, + best: best_models.include?(api_name), supports_tools: true, supports_images:, input_token_cost_cents:, @@ -76,8 +74,8 @@ def create_initial_assistants_etc language_models.create!(api_name:, api_service:, name:, supports_tools: false, supports_images:, input_token_cost_cents:, output_token_cost_cents:) end - assistants.create! name: "GPT-4o", language_model: language_models.find_by(api_name: LanguageModel::BEST_GPT) - assistants.create! name: "Claude 3.5 Sonnet", language_model: language_models.find_by(api_name: LanguageModel::BEST_CLAUDE) - assistants.create! name: "Meta Llama 3 70b", language_model: language_models.find_by(api_name: LanguageModel::BEST_GROQ) + assistants.create! name: "GPT-4o", language_model: language_models.best_for_api_service(open_ai_api_service).first + assistants.create! name: "Claude 3.5 Sonnet", language_model: language_models.best_for_api_service(anthropic_api_service).first + assistants.create! name: "Meta Llama 3 70b", language_model: language_models.best_for_api_service(groq_api_service).first end end diff --git a/app/services/ai_backend/anthropic.rb b/app/services/ai_backend/anthropic.rb index 18f004189..18a829fb0 100644 --- a/app/services/ai_backend/anthropic.rb +++ b/app/services/ai_backend/anthropic.rb @@ -39,7 +39,7 @@ def set_client_config(config) super(config) @client_config = { - model: @assistant.language_model.provider_name, + model: @assistant.language_model.api_name, system: config[:instructions], messages: config[:messages], parameters: { diff --git a/app/services/ai_backend/open_ai.rb b/app/services/ai_backend/open_ai.rb index f00fc713f..0109d7ca5 100644 --- a/app/services/ai_backend/open_ai.rb +++ b/app/services/ai_backend/open_ai.rb @@ -40,7 +40,7 @@ def set_client_config(config) @client_config = { parameters: { - model: @assistant.language_model.provider_name, + model: @assistant.language_model.api_name, messages: system_message(config[:instructions]) + config[:messages], stream: config[:streaming] && @response_handler || nil, max_tokens: 2000, # we should really set this dynamically, based on the model, to the max diff --git a/app/views/settings/language_models/_form.html.erb b/app/views/settings/language_models/_form.html.erb index efcfa1f91..a10b86b6a 100644 --- a/app/views/settings/language_models/_form.html.erb +++ b/app/views/settings/language_models/_form.html.erb @@ -39,6 +39,12 @@ %> <% end %> + +
+ <%= form.check_box :best %> + <%= form.label :best, "Best?" %> +
+
<%= form.check_box :supports_images %> <%= form.label :supports_images, "Supports Images?" %> diff --git a/app/views/settings/language_models/index.html.erb b/app/views/settings/language_models/index.html.erb index 22047244e..d870ba453 100644 --- a/app/views/settings/language_models/index.html.erb +++ b/app/views/settings/language_models/index.html.erb @@ -15,6 +15,7 @@ Name + Best? Description API Service @@ -25,6 +26,7 @@ <%= turbo_frame_tag dom_id(language_model) do %> <%= language_model.api_name %> + <%= language_model.best? ? "Yes" : "No" %> <%= language_model.name %> <%= n_a_if_blank(language_model.api_service.name) %> diff --git a/db/migrate/20241106213119_add_best_to_language_models.rb b/db/migrate/20241106213119_add_best_to_language_models.rb new file mode 100644 index 000000000..fe7d290fe --- /dev/null +++ b/db/migrate/20241106213119_add_best_to_language_models.rb @@ -0,0 +1,5 @@ +class AddBestToLanguageModels < ActiveRecord::Migration[7.2] + def change + add_column :language_models, :best, :boolean, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index ed71cceaa..1bbc15e74 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_10_22_053212) do +ActiveRecord::Schema[7.2].define(version: 2024_11_06_213119) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -172,6 +172,7 @@ t.boolean "supports_tools", default: false t.decimal "input_token_cost_cents", precision: 30, scale: 15 t.decimal "output_token_cost_cents", precision: 30, scale: 15 + t.boolean "best", default: false t.index ["api_service_id"], name: "index_language_models_on_api_service_id" t.index ["user_id", "deleted_at"], name: "index_language_models_on_user_id_and_deleted_at" t.index ["user_id"], name: "index_language_models_on_user_id" diff --git a/test/controllers/settings/language_models_controller_test.rb b/test/controllers/settings/language_models_controller_test.rb index 4fdf67f2e..90c24cc8f 100644 --- a/test/controllers/settings/language_models_controller_test.rb +++ b/test/controllers/settings/language_models_controller_test.rb @@ -33,6 +33,28 @@ class Settings::LanguageModelsControllerTest < ActionDispatch::IntegrationTest assert_equal @user, LanguageModel.last.user end + test "create new best language model replaces existing best" do + api_service = api_services(:keith_anthropic_service) + assert_equal api_service.language_models.best.pluck(:api_name), ["claude-best"] + + params = { + api_name: "new-claude-service", + api_service_id: api_service.id, + name: "new best service", + best: true, + supports_images: false, + supports_tools: false + } + + assert_difference("LanguageModel.count") do + post settings_language_models_url, params: { language_model: params } + end + + assert_redirected_to settings_language_models_url + + assert_equal api_service.language_models.best.pluck(:api_name), ["new-claude-service"] + end + test "should get edit" do get edit_settings_language_model_url(@language_model) assert_response :success @@ -130,6 +152,17 @@ class Settings::LanguageModelsControllerTest < ActionDispatch::IntegrationTest assert_equal params, @language_model.reload.slice(:api_name, :name, :supports_images) end + test "update should update best language model" do + api_service = api_services(:keith_anthropic_service) + assert_equal api_service.language_models.best.pluck(:api_name), ["claude-best"] + + another_language_model = language_models(:claude_3_opus_20240229) + + patch settings_language_model_url(another_language_model), params: { language_model: {best: true}} + assert_redirected_to settings_language_models_url + assert_equal api_service.language_models.best.pluck(:api_name), ["claude-3-opus-20240229"] + end + test "destroy should soft-delete language_model" do assert_no_difference "LanguageModel.count" do delete settings_language_model_url(@language_model) diff --git a/test/fixtures/language_models.yml b/test/fixtures/language_models.yml index ac21c32f1..339f3b882 100644 --- a/test/fixtures/language_models.yml +++ b/test/fixtures/language_models.yml @@ -135,6 +135,7 @@ gpt_best: position: 1 user: keith name: Best OpenAI Model + best: true api_name: gpt-best api_service: keith_openai_service supports_images: true @@ -146,6 +147,7 @@ claude_best: position: 2 user: keith name: Best Anthropic Model + best: true api_service: keith_anthropic_service api_name: claude-best supports_images: true diff --git a/test/jobs/get_next_ai_message_job_anthropic_test.rb b/test/jobs/get_next_ai_message_job_anthropic_test.rb index 60af1827c..570c7b966 100644 --- a/test/jobs/get_next_ai_message_job_anthropic_test.rb +++ b/test/jobs/get_next_ai_message_job_anthropic_test.rb @@ -37,11 +37,13 @@ class GetNextAIMessageJobAnthropicTest < ActiveJob::TestCase end test "when anthropic key is blank, a nice error message is displayed" do - api_service = @conversation.assistant.language_model.api_service - api_service.update!(token: "") + stub_features(default_llm_keys: false) do + api_service = @conversation.assistant.language_model.api_service + api_service.update!(token: "") - assert GetNextAIMessageJob.perform_now(@user.id, @message.id, @conversation.assistant.id) - assert_includes @conversation.latest_message_for_version(:latest).content_text, "need to enter a valid API key for Anthropic" + assert GetNextAIMessageJob.perform_now(@user.id, @message.id, @conversation.assistant.id) + assert_includes @conversation.latest_message_for_version(:latest).content_text, "need to enter a valid API key for Anthropic" + end end test "when API response key is, a nice error message is displayed" do diff --git a/test/jobs/get_next_ai_message_job_openai_test.rb b/test/jobs/get_next_ai_message_job_openai_test.rb index aac8fe17c..86edf5db4 100644 --- a/test/jobs/get_next_ai_message_job_openai_test.rb +++ b/test/jobs/get_next_ai_message_job_openai_test.rb @@ -75,11 +75,13 @@ class GetNextAIMessageJobOpenaiTest < ActiveJob::TestCase end test "when openai key is blank, a nice error message is displayed" do - api_service = @assistant.language_model.api_service - api_service.update!(token: "") + stub_features(default_llm_keys: false) do + api_service = @assistant.language_model.api_service + api_service.update!(token: "") - assert GetNextAIMessageJob.perform_now(@user.id, @message.id, @assistant.id) - assert_includes @conversation.latest_message_for_version(:latest).content_text, "need to enter a valid API key for OpenAI" + assert GetNextAIMessageJob.perform_now(@user.id, @message.id, @assistant.id) + assert_includes @conversation.latest_message_for_version(:latest).content_text, "need to enter a valid API key for OpenAI" + end end test "when API response key is missing, a nice error message is displayed" do diff --git a/test/models/language_model_test.rb b/test/models/language_model_test.rb index f1d8d551b..31944055e 100644 --- a/test/models/language_model_test.rb +++ b/test/models/language_model_test.rb @@ -22,26 +22,6 @@ class LanguageModelTest < ActiveSupport::TestCase assert_equal AIBackend::OpenAI, language_models(:gpt_best).ai_backend end - test "provider_name for Anthropic models" do - assert_equal "claude-3-sonnet-20240229", language_models(:claude_3_sonnet_20240229).provider_name - assert_equal "claude-3-opus-20240229", language_models(:claude_3_opus_20240229).provider_name - end - - test "provider_name for OpenAI models" do - assert_equal "gpt-3.5-turbo-0125", language_models(:gpt_3_5_turbo_0125).provider_name - assert_equal "gpt-4o", language_models(:gpt_4o).provider_name - end - - test "provider_name for best models" do - assert_equal "gpt-4o-2024-08-06", language_models(:gpt_best).provider_name - assert_equal "claude-3-5-sonnet-20240620", language_models(:claude_best).provider_name - end - - test "provider_name for non-best models" do - assert_equal "gpt-4o", language_models(:gpt_4o).provider_name - assert_equal "claude-3-opus-20240229", language_models(:claude_3_opus_20240229).provider_name - end - test "validates api_name" do record = LanguageModel.new(api_name: "") refute record.valid? From 5e967ece48b27e8ed13e1e299c9778ebab961465 Mon Sep 17 00:00:00 2001 From: Keith Schacht Date: Sat, 9 Nov 2024 19:23:24 -0600 Subject: [PATCH 08/43] Hotfix: Ensure db:migrate still runs from scratch --- ..._change_column_assistant_id_on_messages.rb | 2 +- .../20240523080700_create_language_models.rb | 11 ++++++-- ...dd_claude_3_5_sonnet_to_language_models.rb | 11 ++++++-- ...te_existing_language_models_with_prices.rb | 8 ++++-- .../20241022053212_update_model_prices.rb | 28 +++++++++++-------- 5 files changed, 39 insertions(+), 21 deletions(-) diff --git a/db/migrate/20240128211815_change_column_assistant_id_on_messages.rb b/db/migrate/20240128211815_change_column_assistant_id_on_messages.rb index a5eb9294d..36192eae8 100644 --- a/db/migrate/20240128211815_change_column_assistant_id_on_messages.rb +++ b/db/migrate/20240128211815_change_column_assistant_id_on_messages.rb @@ -1,6 +1,6 @@ class ChangeColumnAssistantIdOnMessages < ActiveRecord::Migration[7.1] def up - Message.all.each { |m| m.update!(assistant: m.conversation.assistant) } + # Message.all.each { |m| m.update!(assistant: m.conversation.assistant) } change_column :messages, :assistant_id, :bigint, null: false end diff --git a/db/migrate/20240523080700_create_language_models.rb b/db/migrate/20240523080700_create_language_models.rb index 0405c4b64..79ee59f5e 100644 --- a/db/migrate/20240523080700_create_language_models.rb +++ b/db/migrate/20240523080700_create_language_models.rb @@ -75,9 +75,14 @@ def down end def create_without_validation!(attributes) - record = LanguageModel.new(attributes) - if !record.save(validate: false) - raise "Could not create LanguageModel record for #{attributes.inspect}" + LanguageModel.skip_callback(:save, :after, :update_best_language_model_for_api_service) + begin + record = LanguageModel.new(attributes) + if !record.save(validate: false) + raise "Could not create LanguageModel record for #{attributes.inspect}" + end + ensure + LanguageModel.set_callback(:save, :after, :update_best_language_model_for_api_service) end record end diff --git a/db/migrate/20240620100000_add_claude_3_5_sonnet_to_language_models.rb b/db/migrate/20240620100000_add_claude_3_5_sonnet_to_language_models.rb index dac7f4793..e8e07abfd 100644 --- a/db/migrate/20240620100000_add_claude_3_5_sonnet_to_language_models.rb +++ b/db/migrate/20240620100000_add_claude_3_5_sonnet_to_language_models.rb @@ -33,9 +33,14 @@ def down end def create_without_validation!(attributes) - record = LanguageModel.new(attributes) - if !record.save(validate: false) - raise "Could not create LanguageModel record for #{attributes.inspect}" + LanguageModel.skip_callback(:save, :after, :update_best_language_model_for_api_service) + begin + record = LanguageModel.new(attributes) + if !record.save(validate: false) + raise "Could not create LanguageModel record for #{attributes.inspect}" + end + ensure + LanguageModel.set_callback(:save, :after, :update_best_language_model_for_api_service) end record end diff --git a/db/migrate/20240911114059_update_existing_language_models_with_prices.rb b/db/migrate/20240911114059_update_existing_language_models_with_prices.rb index 8116e8a57..f5e9939db 100644 --- a/db/migrate/20240911114059_update_existing_language_models_with_prices.rb +++ b/db/migrate/20240911114059_update_existing_language_models_with_prices.rb @@ -1,9 +1,11 @@ class UpdateExistingLanguageModelsWithPrices < ActiveRecord::Migration[7.1] def up [ - [LanguageModel::BEST_GPT, LanguageModel::BEST_MODEL_INPUT_PRICES[LanguageModel::BEST_GPT], LanguageModel::BEST_MODEL_OUTPUT_PRICES[LanguageModel::BEST_GPT]], - [LanguageModel::BEST_CLAUDE, LanguageModel::BEST_MODEL_INPUT_PRICES[LanguageModel::BEST_CLAUDE], LanguageModel::BEST_MODEL_OUTPUT_PRICES[LanguageModel::BEST_CLAUDE]], - [LanguageModel::BEST_GROQ, LanguageModel::BEST_MODEL_INPUT_PRICES[LanguageModel::BEST_GROQ], LanguageModel::BEST_MODEL_OUTPUT_PRICES[LanguageModel::BEST_GROQ]], + # Constant was removed in a later PR + # + # [LanguageModel::BEST_GPT, LanguageModel::BEST_MODEL_INPUT_PRICES[LanguageModel::BEST_GPT], LanguageModel::BEST_MODEL_OUTPUT_PRICES[LanguageModel::BEST_GPT]], + # [LanguageModel::BEST_CLAUDE, LanguageModel::BEST_MODEL_INPUT_PRICES[LanguageModel::BEST_CLAUDE], LanguageModel::BEST_MODEL_OUTPUT_PRICES[LanguageModel::BEST_CLAUDE]], + # [LanguageModel::BEST_GROQ, LanguageModel::BEST_MODEL_INPUT_PRICES[LanguageModel::BEST_GROQ], LanguageModel::BEST_MODEL_OUTPUT_PRICES[LanguageModel::BEST_GROQ]], ["gpt-4o", 500, 1500], ["gpt-4o-2024-05-13", 500, 1500], diff --git a/db/migrate/20241022053212_update_model_prices.rb b/db/migrate/20241022053212_update_model_prices.rb index 0d2bd98b0..e229bf0bc 100644 --- a/db/migrate/20241022053212_update_model_prices.rb +++ b/db/migrate/20241022053212_update_model_prices.rb @@ -1,7 +1,8 @@ class UpdateModelPrices < ActiveRecord::Migration[7.1] def up [ - [LanguageModel::BEST_GPT, LanguageModel::BEST_MODEL_INPUT_PRICES[LanguageModel::BEST_GPT], LanguageModel::BEST_MODEL_OUTPUT_PRICES[LanguageModel::BEST_GPT]], + # Constant was removed in a later PR + # [LanguageModel::BEST_GPT, LanguageModel::BEST_MODEL_INPUT_PRICES[LanguageModel::BEST_GPT], LanguageModel::BEST_MODEL_OUTPUT_PRICES[LanguageModel::BEST_GPT]], ["gpt-4o", 250, 1000], ].each do |api_name, input_token_cost_per_million, output_token_cost_per_million| @@ -27,16 +28,21 @@ def up input_token_cost_cents = input_token_cost_per_million/million output_token_cost_cents = output_token_cost_per_million/million - - user.language_models.create!( - api_name: api_name, - name: name, - supports_images: supports_images, - api_service: api_service, - supports_tools: true, - input_token_cost_cents: input_token_cost_cents, - output_token_cost_cents: output_token_cost_cents, - ) + LanguageModel.skip_callback(:save, :after, :update_best_language_model_for_api_service) + + begin + user.language_models.create!( + api_name: api_name, + name: name, + supports_images: supports_images, + api_service: api_service, + supports_tools: true, + input_token_cost_cents: input_token_cost_cents, + output_token_cost_cents: output_token_cost_cents, + ) + ensure + LanguageModel.set_callback(:save, :after, :update_best_language_model_for_api_service) + end end end end From 4f7984d2f44fd1a6fb4d6955be2f9f11eaac45e4 Mon Sep 17 00:00:00 2001 From: Dr Nic Williams Date: Mon, 11 Nov 2024 02:13:10 +1000 Subject: [PATCH 09/43] Fix 'Test is missing assertions' warnings (#541) --- .../active_storage/postgresql_controller_test.rb | 1 + test/models/api_service_test.rb | 2 +- test/models/message/version_test.rb | 2 +- test/services/ai_backend/anthropic_test.rb | 12 ++++++------ 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/test/controllers/active_storage/postgresql_controller_test.rb b/test/controllers/active_storage/postgresql_controller_test.rb index 365641294..275a7150a 100644 --- a/test/controllers/active_storage/postgresql_controller_test.rb +++ b/test/controllers/active_storage/postgresql_controller_test.rb @@ -52,6 +52,7 @@ class ActiveStorage::PostgresqlControllerTest < ActionDispatch::IntegrationTest blob.delete get blob.send(url_method) + assert_response :not_found end test "showing blob with invalid key" do diff --git a/test/models/api_service_test.rb b/test/models/api_service_test.rb index da2ab9198..71b566851 100644 --- a/test/models/api_service_test.rb +++ b/test/models/api_service_test.rb @@ -57,7 +57,7 @@ class APIServiceTest < ActiveSupport::TestCase end test "can create record" do - APIService.create!(create_params) + assert APIService.create!(create_params) end test "soft delete also soft deletes language_models" do diff --git a/test/models/message/version_test.rb b/test/models/message/version_test.rb index 018144269..44439825a 100644 --- a/test/models/message/version_test.rb +++ b/test/models/message/version_test.rb @@ -66,7 +66,7 @@ class Message::VersionTest < ActiveSupport::TestCase test "creating a message with branched true AND branched_from_version specified SUCCEEDS" do Current.user = users(:keith) - conversations(:versioned).messages.create!(assistant: assistants(:samantha), content_text: "What is your name?", index: 2, version: 3, branched: true, branched_from_version: 2) + assert conversations(:versioned).messages.create!(assistant: assistants(:samantha), content_text: "What is your name?", index: 2, version: 3, branched: true, branched_from_version: 2) end test "creating a new messages for a SPECIFIC INDEX and SPECIFIC VERSION fails if the VERSION is SKIPPING a number" do diff --git a/test/services/ai_backend/anthropic_test.rb b/test/services/ai_backend/anthropic_test.rb index 7791ae20d..8ed468dc8 100644 --- a/test/services/ai_backend/anthropic_test.rb +++ b/test/services/ai_backend/anthropic_test.rb @@ -48,11 +48,11 @@ class AIBackend::AnthropicTest < ActiveSupport::TestCase end end - test "preceding_conversation_messages only considers messages on the intended conversation version and includes the correct names" do - # TODO - end + # TODO + # test "preceding_conversation_messages only considers messages on the intended conversation version and includes the correct names" do + # end - test "preceding_conversation_messages includes the appropriate tool details" do - # TODO - end + # TODO + # test "preceding_conversation_messages includes the appropriate tool details" do + # end end From b003c454e2d0d5892e88d9c836036d563206d329 Mon Sep 17 00:00:00 2001 From: Dr Nic Williams Date: Mon, 11 Nov 2024 02:18:07 +1000 Subject: [PATCH 10/43] Ruby {key: key} -> {key:} simplification (#540) --- .../google_oauth_controller.rb | 2 +- app/helpers/application_helper.rb | 8 ++-- app/jobs/get_next_ai_message_job.rb | 2 +- app/jobs/send_reset_password_email_job.rb | 2 +- app/mailers/password_mailer.rb | 2 +- app/models/message/version.rb | 4 +- app/services/ai_backend.rb | 6 +-- app/services/ai_backend/open_ai.rb | 2 +- app/services/sdk.rb | 24 +++++----- app/services/toolbox/gmail/message.rb | 14 +++--- .../toolbox/google_tasks/api_helpers.rb | 12 ++--- .../service/postgresql_service.rb | 46 +++++++++---------- .../password_credentials_controller_test.rb | 8 ++-- test/models/message_test.rb | 10 ++-- test/services/toolbox/google_tasks_test.rb | 18 ++++---- test/support/sdk_helper.rb | 10 ++-- 16 files changed, 85 insertions(+), 85 deletions(-) diff --git a/app/controllers/authentications/google_oauth_controller.rb b/app/controllers/authentications/google_oauth_controller.rb index af070aa1e..3d8a83fd3 100644 --- a/app/controllers/authentications/google_oauth_controller.rb +++ b/app/controllers/authentications/google_oauth_controller.rb @@ -97,7 +97,7 @@ def initialize_google_person def add_person_credentials(type) p = Current.person || @person c = p.user.credentials.build( - type: type, + type:, oauth_id: auth[:uid], oauth_email: auth[:info][:email], oauth_token: auth[:credentials][:token], diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 1e2f0d02e..e6e15f20f 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -70,15 +70,15 @@ def div_tag(content_or_options_with_block = nil, options = nil, &block) end def meta_tag(name, content) - tag.meta(name: name, content: content) + tag.meta(name:, content:) end def charset_tag(charset) - tag.meta(charset: charset) + tag.meta(charset:) end def viewport_tag(content) - tag.meta(name: "viewport", content: content) + tag.meta(name: "viewport", content:) end def n_a_if_blank(value, n_a = "Not Available") @@ -86,6 +86,6 @@ def n_a_if_blank(value, n_a = "Not Available") end def to_dollars(cents, precision: 2) - number_to_currency(cents / 100.0, precision: precision) + number_to_currency(cents / 100.0, precision:) end end diff --git a/app/jobs/get_next_ai_message_job.rb b/app/jobs/get_next_ai_message_job.rb index 74a68b1d9..013831249 100644 --- a/app/jobs/get_next_ai_message_job.rb +++ b/app/jobs/get_next_ai_message_job.rb @@ -118,7 +118,7 @@ def self.broadcast_updated_message(message, locals = {}) html = ApplicationController.render( partial: "messages/message", locals: { - message: message, + message:, only_scroll_down_if_was_bottom: true, streamed: true, message_counter: message.index diff --git a/app/jobs/send_reset_password_email_job.rb b/app/jobs/send_reset_password_email_job.rb index 371de716b..e7fc1f5bd 100644 --- a/app/jobs/send_reset_password_email_job.rb +++ b/app/jobs/send_reset_password_email_job.rb @@ -7,7 +7,7 @@ def perform(email, os, browser) Rails.logger.info "Sending reset password email to #{email} from #{os} with #{browser}" if person&.user # make sure the user exists (i.e. user has not become a tombstone) - PasswordMailer.with(person: person, os: os, browser: browser).reset.deliver_now + PasswordMailer.with(person:, os:, browser:).reset.deliver_now end end end diff --git a/app/mailers/password_mailer.rb b/app/mailers/password_mailer.rb index 0360a5352..5b729f8b8 100644 --- a/app/mailers/password_mailer.rb +++ b/app/mailers/password_mailer.rb @@ -11,7 +11,7 @@ def reset purpose: Email::PasswordReset::TOKEN_PURPOSE, expires_in: @token_ttl ) - @edit_url = edit_password_credential_url(token: token) + @edit_url = edit_password_credential_url(token:) mail( to: person.email, diff --git a/app/models/message/version.rb b/app/models/message/version.rb index 5baa5fa9e..8c1659ab9 100644 --- a/app/models/message/version.rb +++ b/app/models/message/version.rb @@ -81,9 +81,9 @@ def set_next_conversation_index_and_version else if version.negative? || version > max_version errors.add(:version, "#{version} is invalid for this index") - elsif conversation.messages.exists?(index: index, version: version) + elsif conversation.messages.exists?(index:, version:) errors.add(:version, "#{version} already exists for this index") - elsif versions.present? && version < versions.max && !conversation.messages.exists?(index: index-1, version: version) + elsif versions.present? && version < versions.max && !conversation.messages.exists?(index: index-1, version:) errors.add(:version, "#{version} is invalid for this index") end end diff --git a/app/services/ai_backend.rb b/app/services/ai_backend.rb index 42e4fd7d8..d4204f084 100644 --- a/app/services/ai_backend.rb +++ b/app/services/ai_backend.rb @@ -14,9 +14,9 @@ def initialize(user, assistant, conversation = nil, message = nil) def get_oneoff_message(instructions, messages, params = {}) set_client_config( - instructions: instructions, + instructions:, messages: preceding_messages(messages), - params: params, + params:, ) response = @client.send(client_method_name, ** @client_config) @@ -77,7 +77,7 @@ def preceding_messages(messages = []) role = (i % 2).zero? ? "user" : "assistant" { - role: role, + role:, content: msg } end diff --git a/app/services/ai_backend/open_ai.rb b/app/services/ai_backend/open_ai.rb index 0109d7ca5..5f7146c80 100644 --- a/app/services/ai_backend/open_ai.rb +++ b/app/services/ai_backend/open_ai.rb @@ -88,7 +88,7 @@ def stream_handler(&chunk_handler) def system_message(content) [{ role: "system", - content: content, + content:, }] end diff --git a/app/services/sdk.rb b/app/services/sdk.rb index d073c81bd..238a038c8 100644 --- a/app/services/sdk.rb +++ b/app/services/sdk.rb @@ -1,40 +1,40 @@ class SDK def get(url, token = nil) SDK::Get.new( - url: url, + url:, bearer_token: token || bearer_token, - expected_status: expected_status, - header: header, + expected_status:, + header:, calling_method: calling_method(__method__), ) end def post(url, token = nil) SDK::Post.new( - url: url, + url:, bearer_token: token || bearer_token, - expected_status: expected_status, - header: header, + expected_status:, + header:, calling_method: calling_method(__method__), ) end def patch(url, token = nil) SDK::Patch.new( - url: url, + url:, bearer_token: token || bearer_token, - expected_status: expected_status, - header: header, + expected_status:, + header:, calling_method: calling_method(__method__), ) end def delete(url, token = nil) SDK::Delete.new( - url: url, + url:, bearer_token: token || bearer_token, - expected_status: expected_status, - header: header, + expected_status:, + header:, calling_method: calling_method(__method__), ) end diff --git a/app/services/toolbox/gmail/message.rb b/app/services/toolbox/gmail/message.rb index 5a26a5040..20468e0a3 100644 --- a/app/services/toolbox/gmail/message.rb +++ b/app/services/toolbox/gmail/message.rb @@ -55,13 +55,13 @@ def body_html def to_h { - id: id, - thread_id: thread_id, - date: date, - from: from, - to: to, - subject: subject, - snippet: snippet, + id:, + thread_id:, + date:, + from:, + to:, + subject:, + snippet:, body: body || body_html, } end diff --git a/app/services/toolbox/google_tasks/api_helpers.rb b/app/services/toolbox/google_tasks/api_helpers.rb index 8a56bbf11..4e6bfb86a 100644 --- a/app/services/toolbox/google_tasks/api_helpers.rb +++ b/app/services/toolbox/google_tasks/api_helpers.rb @@ -41,8 +41,8 @@ def get_tasks_for_list(list, due_min: nil, due_max: nil) def create_task_for_list(list, title:, notes: nil, due: nil) refresh_token_if_needed do post("https://tasks.googleapis.com/tasks/v1/lists/#{list.id}/tasks").param( - title: title, - notes: notes, + title:, + notes:, due: format_time(due), status: "needsAction", ) @@ -58,7 +58,7 @@ def move_task_to_list(task, list, keep_due: true) delete_task(task) create_task_for_list(list, title: task.title, - notes: notes, + notes:, due: keep_due.presence && task.try(:due), ) end @@ -73,12 +73,12 @@ def update_task(task, title: nil, notes: nil, due: nil, completed: nil, deleted: refresh_token_if_needed do patch("https://tasks.googleapis.com/tasks/v1/lists/#{list_id_of(task)}/tasks/#{task.id}").param( { id: task.id, - title: title, - notes: notes, + title:, + notes:, due: format_time(due), status: !!completed ? "completed" : "needsAction", completed: completed.presence && format_time(completed), - deleted: deleted, + deleted:, }.compact) # w/o this it updates values to nil end end diff --git a/lib/active_storage/service/postgresql_service.rb b/lib/active_storage/service/postgresql_service.rb index e4d46646a..cb382d12c 100644 --- a/lib/active_storage/service/postgresql_service.rb +++ b/lib/active_storage/service/postgresql_service.rb @@ -7,14 +7,14 @@ def initialize(public: false, **options) end def upload(key, io, checksum: nil, **) - instrument :upload, key: key, checksum: checksum do - ActiveStorage::Postgresql::File.create!(key: key, io: io, checksum: checksum) + instrument :upload, key:, checksum: do + ActiveStorage::Postgresql::File.create!(key:, io:, checksum:) end end def download(key) if block_given? - instrument :streaming_download, key: key do + instrument :streaming_download, key: do ActiveStorage::Postgresql::File.open(key) do |file| while data = file.read(5.megabytes) yield data @@ -22,7 +22,7 @@ def download(key) end end else - instrument :download, key: key do + instrument :download, key: do ActiveStorage::Postgresql::File.open(key) do |file| file.read end @@ -31,7 +31,7 @@ def download(key) end def download_chunk(key, range) - instrument :download_chunk, key: key, range: range do + instrument :download_chunk, key:, range: do ActiveStorage::Postgresql::File.open(key) do |file| file.seek(range.first) file.read(range.size) @@ -40,14 +40,14 @@ def download_chunk(key, range) end def delete(key) - instrument :delete, key: key do - ActiveStorage::Postgresql::File.find_by(key: key).try(&:destroy) + instrument :delete, key: do + ActiveStorage::Postgresql::File.find_by(key:).try(&:destroy) end end def exist?(key) - instrument :exist, key: key do |payload| - answer = ActiveStorage::Postgresql::File.where(key: key).exists? + instrument :exist, key: do |payload| + answer = ActiveStorage::Postgresql::File.where(key:).exists? payload[:exist] = answer answer end @@ -60,11 +60,11 @@ def delete_prefixed(prefix) end def private_url(key, expires_in:, filename:, content_type:, disposition:, **) - generate_url(key, expires_in: expires_in, filename: filename, content_type: content_type, disposition: disposition) + generate_url(key, expires_in:, filename:, content_type:, disposition:) end def public_url(key, filename:, content_type: nil, disposition: :attachment, **) - generate_url(key, expires_in: nil, filename: filename, content_type: content_type, disposition: disposition) + generate_url(key, expires_in: nil, filename:, content_type:, disposition:) end def url(key, **options) @@ -90,23 +90,23 @@ def generate_url(key, expires_in:, filename:, disposition:, content_type:) counter = counter.to_i # End hack - instrument :url, key: key do |payload| - content_disposition = content_disposition_with(type: disposition, filename: filename) + instrument :url, key: do |payload| + content_disposition = content_disposition_with(type: disposition, filename:) verified_key_with_expiration = ActiveStorage.verifier.generate( { - key: key, + key:, disposition: content_disposition, - content_type: content_type + content_type: }, - expires_in: expires_in, + expires_in:, purpose: :blob_key ) generated_url = url_helpers.rails_postgresql_service_url(verified_key_with_expiration, **url_options, disposition: content_disposition, - content_type: content_type, - filename: filename, + content_type:, + filename:, retry_count: counter ) payload[:url] = generated_url @@ -116,15 +116,15 @@ def generate_url(key, expires_in:, filename:, disposition:, content_type:) end def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:, custom_metadata: {}) - instrument :url, key: key do |payload| + instrument :url, key: do |payload| verified_token_with_expiration = ActiveStorage.verifier.generate( { - key: key, - content_type: content_type, + key:, + content_type:, content_length: content_length, - checksum: checksum + checksum: }, - expires_in: expires_in, + expires_in:, purpose: :blob_token ) diff --git a/test/controllers/password_credentials_controller_test.rb b/test/controllers/password_credentials_controller_test.rb index c33e5cf0c..8e79d5e5b 100644 --- a/test/controllers/password_credentials_controller_test.rb +++ b/test/controllers/password_credentials_controller_test.rb @@ -19,7 +19,7 @@ class PasswordCredentialsControllerTest < ActionDispatch::IntegrationTest test "should get edit" do token = get_test_user_token(@user) - get edit_password_credential_url, params: { token: token } + get edit_password_credential_url, params: { token: } assert_response :success assert assigns(:user).is_a?(User) @@ -37,7 +37,7 @@ class PasswordCredentialsControllerTest < ActionDispatch::IntegrationTest token = get_test_user_token(@user) @user.destroy # make sure the user doesn't exist when we try to find it - get edit_password_credential_url, params: { token: token } + get edit_password_credential_url, params: { token: } assert_response :not_found end @@ -46,7 +46,7 @@ class PasswordCredentialsControllerTest < ActionDispatch::IntegrationTest token = get_test_user_token(@user) new_password = "new_password" - patch password_credential_url, params: { token: token, password: new_password } + patch password_credential_url, params: { token:, password: new_password } assert_response :redirect assert_redirected_to root_path @@ -61,7 +61,7 @@ class PasswordCredentialsControllerTest < ActionDispatch::IntegrationTest token = get_test_user_token(@user) new_password = "new_password" - patch password_credential_url, params: { token: token, password: new_password } + patch password_credential_url, params: { token:, password: new_password } assert_response :redirect assert_redirected_to root_path diff --git a/test/models/message_test.rb b/test/models/message_test.rb index fd27ed03f..cc240dbbc 100644 --- a/test/models/message_test.rb +++ b/test/models/message_test.rb @@ -123,7 +123,7 @@ class MessageTest < ActiveSupport::TestCase assert_raises ActiveRecord::RecordInvalid do Message.create!( - assistant: assistant, + assistant:, conversation: conversation_owned_by_someone_else, content_text: "This should fail" ) @@ -137,8 +137,8 @@ class MessageTest < ActiveSupport::TestCase assert_nothing_raised do message = Message.create!( - assistant: assistant, - conversation: conversation, + assistant:, + conversation:, content_text: "This works since Conversation is owned by Current.user" ) assert_equal Current.user, message.conversation.user @@ -154,8 +154,8 @@ class MessageTest < ActiveSupport::TestCase assert_nothing_raised do message = Message.create!( - assistant: assistant, - conversation: conversation, + assistant:, + conversation:, content_text: "This works since Current.user not set" ) end diff --git a/test/services/toolbox/google_tasks_test.rb b/test/services/toolbox/google_tasks_test.rb index e2b82b931..f2f32bc2f 100644 --- a/test/services/toolbox/google_tasks_test.rb +++ b/test/services/toolbox/google_tasks_test.rb @@ -389,13 +389,13 @@ def lists_data def task_data(title: "Title of task", notes: "notes for task", deleted: false, due: nil, completed: false) { id: "mockid", - title: title, + title:, selfLink: "https://www.googleapis.com/tasks/v1/lists/SjBhVUFfLWtEVElJRnJ2Yg/tasks/NHhjQUY3bGlHVHdPUEwwaA", - notes: notes, + notes:, status: completed ? "completed" : "needsAction", completed: completed.presence && Time.zone.today.strftime("%Y-%m-%dT00:00:00.000Z"), due: due == "clear" ? nil : due, - deleted: deleted, + deleted:, links: [], webViewLink: "https://tasks.google.com/task/4xcAF7liGTwOPL0h" } @@ -403,7 +403,7 @@ def task_data(title: "Title of task", notes: "notes for task", deleted: false, d def tasks_data(deleted: false) { items: [ - task_data(deleted: deleted).merge(position: "00001"), + task_data(deleted:).merge(position: "00001"), { id: "mockid2", position: "00002", @@ -411,7 +411,7 @@ def tasks_data(deleted: false) selfLink: "https://www.googleapis.com/tasks/v1/lists/SjBhVUFfLWtEVElJRnJ2Yg/tasks/NHhjQUY3bGlHVHdPUEwwaA", status: "needsAction", due: nil, - deleted: deleted, + deleted:, links: [], webViewLink: "https://tasks.google.com/task/4xcAF7liGTwOPL0h" },{ @@ -421,7 +421,7 @@ def tasks_data(deleted: false) selfLink: "https://www.googleapis.com/tasks/v1/lists/SjBhVUFfLWtEVElJRnJ2Yg/tasks/NHhjQUY3bGlHVHdPUEwwaA", status: "needsAction", due: nil, - deleted: deleted, + deleted:, links: [], webViewLink: "https://tasks.google.com/task/4xcAF7liGTwOPL0h" }] @@ -432,13 +432,13 @@ def create_data(title:, notes: nil, due: nil) { kind: "tasks#task", id: "mockid", etag: "\"NzU5Mjc2NTA2\"", - title: title, + title:, updated: "2024-06-18T21:29:54.000Z", selfLink: "https://www.googleapis.com/tasks/v1/lists/SjBhVUFfLWtEVElJRnJ2Yg/tasks/NHhjQUY3bGlHVHdPUEwwaA", position: "00000000000000000000", - notes: notes, + notes:, status: "needsAction", - due: due, + due:, links: [], webViewLink: "https://tasks.google.com/task/4xcAF7liGTwOPL0h" }.compact diff --git a/test/support/sdk_helper.rb b/test/support/sdk_helper.rb index ad6cdd7b1..fec305809 100644 --- a/test/support/sdk_helper.rb +++ b/test/support/sdk_helper.rb @@ -20,24 +20,24 @@ def stub_response(verb, method, status:, response: {}, &block) end def stub_get_response(method, status:, response: {}, &block) - stub_response(:get, method, status: status, response: response, &block) + stub_response(:get, method, status:, response:, &block) end def stub_post_response(method, status:, response: {}, &block) - stub_response(:post, method, status: status, response: response, &block) + stub_response(:post, method, status:, response:, &block) end def stub_patch_response(method, status:, response: {}, &block) - stub_response(:patch, method, status: status, response: response, &block) + stub_response(:patch, method, status:, response:, &block) end def stub_delete_response(method, status:, response: {}, &block) - stub_response(:delete, method, status: status, response: response, &block) + stub_response(:delete, method, status:, response:, &block) end def response_for(status, hash) OpenData.new( - status: status, + status:, body: hash.to_json, ) end From 8c87c14c5be19c6c5a2c6014409e65f881ba087c Mon Sep 17 00:00:00 2001 From: Keith Schacht Date: Tue, 12 Nov 2024 17:48:32 -0600 Subject: [PATCH 11/43] Fix one pixel of unclickable border in nav column (#543) --- app/views/assistants/_assistant.html.erb | 4 ++-- app/views/layouts/_settings_item.erb | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/views/assistants/_assistant.html.erb b/app/views/assistants/_assistant.html.erb index d36f297f6..3e04f9fea 100644 --- a/app/views/assistants/_assistant.html.erb +++ b/app/views/assistants/_assistant.html.erb @@ -6,7 +6,7 @@ <%# This extra div ^ is needed because of the absolute positioning. It doesn't lay out properly if added to the div below. %>
" data-transition-target="<%= !visible && 'transitionable' %>" > - <%= link_to new_assistant_message_path(assistant), class: "flex-1 flex items-center text-gray-950 dark:text-gray-100 font-medium truncate", data: { role: "name" } do %> + <%= link_to new_assistant_message_path(assistant), class: "flex-1 flex py-1 items-center text-gray-950 dark:text-gray-100 font-medium truncate", data: { role: "name" } do %> <%= render partial: "layouts/assistant_avatar", locals: { assistant: assistant, size: 7, classes: "mr-2" } %> <%= assistant.name %> <% end %> diff --git a/app/views/layouts/_settings_item.erb b/app/views/layouts/_settings_item.erb index 5ec2e5ecc..085de0b37 100644 --- a/app/views/layouts/_settings_item.erb +++ b/app/views/layouts/_settings_item.erb @@ -3,7 +3,7 @@ <% selected = selected && params[:id] == item.try(:id)&.to_s if controller.to_s == "assistants" %> +
+ <%= form.check_box :supports_system_message %> + <%= form.label :supports_system_message, "Supports System Message (Instructions)?" %> +
<%= form.submit "Save", class: %| diff --git a/db/migrate/20241111131751_add_supports_system_message_to_language_models.rb b/db/migrate/20241111131751_add_supports_system_message_to_language_models.rb new file mode 100644 index 000000000..e1e0315b8 --- /dev/null +++ b/db/migrate/20241111131751_add_supports_system_message_to_language_models.rb @@ -0,0 +1,17 @@ +class AddSupportsSystemMessageToLanguageModels < ActiveRecord::Migration[7.1] + def change + add_column :language_models, :supports_system_message, :boolean, default: false + + ActiveRecord::Base.connection.execute <<-END_SQL + UPDATE + language_models + SET supports_system_message = true + WHERE + (api_name like 'gpt%' and api_name <> 'gpt-3.5-turbo-instruct' and api_name <> 'gpt-3.5-turbo-16k-0613') OR + api_name like 'claude%' OR + api_name like 'groq%' OR + api_name like 'gemma%' OR + api_name IN ('mixtral-8x7b-32768', 'llama3-70b-8192', 'llama3-8b-8192'); + END_SQL + end +end \ No newline at end of file diff --git a/db/schema.rb b/db/schema.rb index 1bbc15e74..44ceb9d69 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_11_06_213119) do +ActiveRecord::Schema[7.1].define(version: 2024_11_11_131751) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -172,6 +172,7 @@ t.boolean "supports_tools", default: false t.decimal "input_token_cost_cents", precision: 30, scale: 15 t.decimal "output_token_cost_cents", precision: 30, scale: 15 + t.boolean "supports_system_message", default: false t.boolean "best", default: false t.index ["api_service_id"], name: "index_language_models_on_api_service_id" t.index ["user_id", "deleted_at"], name: "index_language_models_on_user_id_and_deleted_at" diff --git a/test/controllers/settings/language_models_controller_test.rb b/test/controllers/settings/language_models_controller_test.rb index 90c24cc8f..184cfdb3c 100644 --- a/test/controllers/settings/language_models_controller_test.rb +++ b/test/controllers/settings/language_models_controller_test.rb @@ -68,7 +68,7 @@ class Settings::LanguageModelsControllerTest < ActionDispatch::IntegrationTest assert_response :success assert_select "form" do assert_select 'input[name="language_model[supports_tools]"]' - assert_select 'input[name="language_model[supports_tools]"][checked="checked"]', false # Not checked by default, from schema + assert_select 'input[name="language_model[supports_tools]"][checked="checked"]', false, "Checkbox should default to false within DB" end end @@ -173,4 +173,30 @@ class Settings::LanguageModelsControllerTest < ActionDispatch::IntegrationTest assert flash[:notice].present?, "There should have been a success message" refute flash[:alert].present?, "There should NOT have been an error message" end + + test "create should not have supports_system_message checkbox" do + get new_settings_language_model_url + assert_response :success + assert_select "form" do + assert_select 'input[name="language_model[supports_system_message]"]' + assert_select 'input[name="language_model[supports_system_message]"][checked="checked"]', false, "Checkbox should default to false within DB" + end + end + + test "edit should have supports_system_message checkbox checked" do + get edit_settings_language_model_url(@language_model) + assert_response :success + assert_select "form" do + assert_select 'input[name="language_model[supports_system_message]"][checked="checked"]' + end + end + + test "edit should have supports_system_message checkbox unchecked" do + get edit_settings_language_model_url(language_models(:guanaco)) + assert_response :success + assert_select "form" do + assert_select 'input[name="language_model[supports_system_message]"]' + assert_select 'input[name="language_model[supports_system_message]"][checked="checked"]', false + end + end end diff --git a/test/fixtures/language_models.yml b/test/fixtures/language_models.yml index 339f3b882..7ebfa15fb 100644 --- a/test/fixtures/language_models.yml +++ b/test/fixtures/language_models.yml @@ -140,6 +140,7 @@ gpt_best: api_service: keith_openai_service supports_images: true supports_tools: true + supports_system_message: true input_token_cost_cents: 0.0001 output_token_cost_cents: 0.0001 From d445dde1e3b7fa75ec11b2a4eed151c986cabc0b Mon Sep 17 00:00:00 2001 From: Dr Nic Williams Date: Sat, 16 Nov 2024 12:41:19 +1000 Subject: [PATCH 17/43] Introduce login with Microsoft (#539) --- Gemfile | 1 + Gemfile.lock | 5 + README.md | 56 ++++++ .../microsoft_graph_oauth_controller.rb | 106 +++++++++++ .../concerns/authenticate/login_logout.rb | 2 +- app/models/feature.rb | 1 + app/models/microsoft_graph_credential.rb | 10 + app/models/user.rb | 1 + app/views/authentications/new.html.erb | 21 ++ app/views/users/new.html.erb | 21 ++ config/initializers/omniauth.rb | 6 + config/options.yml | 8 +- config/routes.rb | 1 + .../authentications/microsoft_auth_test.rb | 19 ++ .../microsoft_graph_oauth_controller_test.rb | 179 ++++++++++++++++++ .../authentications/registration_test.rb | 2 +- .../authentications_controller_test.rb | 2 +- .../authenticate/by_http_header_test.rb | 3 +- test/controllers/users/microsoft_auth_test.rb | 19 ++ test/controllers/users/registration_test.rb | 2 +- test/fixtures/credentials.yml | 6 + test/models/feature_test.rb | 8 +- .../models/microsoft_graph_credential_test.rb | 13 ++ test/models/user_test.rb | 4 + test/system/sessions_test.rb | 14 ++ 25 files changed, 502 insertions(+), 8 deletions(-) create mode 100644 app/controllers/authentications/microsoft_graph_oauth_controller.rb create mode 100644 app/models/microsoft_graph_credential.rb create mode 100644 test/controllers/authentications/microsoft_auth_test.rb create mode 100644 test/controllers/authentications/microsoft_graph_oauth_controller_test.rb create mode 100644 test/controllers/users/microsoft_auth_test.rb create mode 100644 test/models/microsoft_graph_credential_test.rb diff --git a/Gemfile b/Gemfile index 3013be528..e354992ea 100644 --- a/Gemfile +++ b/Gemfile @@ -51,6 +51,7 @@ gem "postmark-rails" gem "omniauth", "~> 2.1" gem "omniauth-google-oauth2", "~> 1.1" +gem "omniauth-microsoft_graph", "~> 2.0" gem "omniauth-rails_csrf_protection", "~> 1.0.2" group :development, :test do diff --git a/Gemfile.lock b/Gemfile.lock index 5af6aa2d5..1b42dc0c0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -221,6 +221,10 @@ GEM oauth2 (~> 2.0) omniauth (~> 2.0) omniauth-oauth2 (~> 1.8) + omniauth-microsoft_graph (2.0.1) + jwt (~> 2.0) + omniauth (~> 2.0) + omniauth-oauth2 (~> 1.8.0) omniauth-oauth2 (1.8.0) oauth2 (>= 1.4, < 3) omniauth (~> 2.0) @@ -458,6 +462,7 @@ DEPENDENCIES name_of_person omniauth (~> 2.1) omniauth-google-oauth2 (~> 1.1) + omniauth-microsoft_graph (~> 2.0) omniauth-rails_csrf_protection (~> 1.0.2) ostruct pg (~> 1.1) diff --git a/README.md b/README.md index e8c6b33a9..2cbf9bd43 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,7 @@ This project is led by an experienced rails developer, but I'm actively looking - [Authentication](#authentication) - [Password authentication](#password-authentication) - [Google OAuth authentication](#google-oauth-authentication) + - [Microsoft Graph OAuth authentication](#microsoft-graph-oauth-authentication) - [HTTP header authentication](#http-header-authentication) - [Contribute as a developer](#contribute-as-a-developer) - [Running locally](#Running-locally) @@ -196,6 +197,7 @@ HostedGPT supports multiple authentication methods: - [Password authentication](#password-authentication) - [Google OAuth authentication](#google-oauth-authentication) +- [Microsoft Graph OAuth authentication](#microsoft-graph-oauth-authentication) #### Password authentication @@ -210,6 +212,14 @@ To enable Google OAuth authentication, you need to set up Google OAuth in the Go - `GOOGLE_AUTH_CLIENT_ID` - Google OAuth client ID - `GOOGLE_AUTH_CLIENT_SECRET` - Google OAuth client secret +Alternately, add the following to your encrypted credentials file: + +```yaml +google: + auth_client_id: + auth_client_secret: +``` + **Steps to set up:** 1. **Go to the Google Cloud Console and Create a New Project:** @@ -248,6 +258,52 @@ To enable Google OAuth authentication, you need to set up Google OAuth in the Go - `GOOGLE_AUTH_CLIENT_ID`: Your Client ID - `GOOGLE_AUTH_CLIENT_SECRET`: Your Client Secret +#### Microsoft Graph OAuth authentication + +Microsoft Graph OAuth authentication is disabled by default. You can enable it by setting `MICROSOFT_GRAPH_AUTHENTICATION_FEATURE` to `true`. + +To enable Microsoft Graph OAuth authentication, you need to set up Microsoft Graph OAuth in the Microsoft Azure portal. It's a bit involved but we've outlined the steps below. After you follow these steps you will set the following environment variables: + +- `MICROSOFT_GRAPH_AUTH_CLIENT_ID` - Microsoft Graph OAuth client ID +- `MICROSOFT_GRAPH_AUTH_CLIENT_SECRET` - Microsoft Graph OAuth client secret +- `MICROSOFT_GRAPH_SCOPE` - Space separated list of scopes to request. This defaults to `openid profile email offline_access user.read mailboxsettings.read`. + +Alternately, add the following to your encrypted credentials file: + +```yaml +microsoft_graph: + auth_client_id: + auth_client_secret: + scope: openid profile email offline_access user.read mailboxsettings.read +``` + +Users will need to have setup their full name in their Microsoft account before they can use this authentication method, via , otherwise they will see a login/registration error like "First name can't be blank and last name can't be blank". + +Users can remotely remove the connection between their Microsoft account and HostedGPT by going to and clicking "Don't Allow" on the corresponding application. However, this will not sign out the user from HostedGPT until the session expires. + +**Steps to set up:** + +1. **Go to the Microsoft Azure portal and create a new application:** + + - Navigate to [Register an application](https://portal.azure.com/#view/Microsoft_AAD_RegisteredApps/CreateApplicationBlade/quickStartType) + - Give it a name + - Select the Supported account types + - Select the Redirect URI for "Web" (e.g., `https://example.com/auth/microsoft/callback` or `http://localhost:3000/auth/microsoft/callback`) + - Click Register + +2. **Create OAuth Credentials:** + + - The client ID ("Application (client) ID") is displayed on the Overview page + - To generate a client secret, click on "Add a certificate or secret" > "New client secret" + - Give it a name and pick an expiration date + - Back on the "Certificates & secrets" page, the new client secret will be listed under "Value" + +3. **Set Environment Variables:** + - Set the Client ID and Client Secret as environment variables in your application: + - `MICROSOFT_GRAPH_AUTH_CLIENT_ID`: Your Client ID + - `MICROSOFT_GRAPH_AUTH_CLIENT_SECRET`: Your Client Secret + - `MICROSOFT_GRAPH_SCOPE` - Space separated list of scopes to request. This defaults to `openid profile email offline_access user.read mailboxsettings.read`. + #### HTTP header authentication Note: Enabling this automatically disables Password-based and Google-auth based authentication. diff --git a/app/controllers/authentications/microsoft_graph_oauth_controller.rb b/app/controllers/authentications/microsoft_graph_oauth_controller.rb new file mode 100644 index 000000000..49cce2d28 --- /dev/null +++ b/app/controllers/authentications/microsoft_graph_oauth_controller.rb @@ -0,0 +1,106 @@ +class Authentications::MicrosoftGraphOauthController < ApplicationController + allow_unauthenticated_access + + # GET /auth/microsoft_graph/callback + def create + if params[:error] + if Current.user + redirect_to(edit_settings_person_path, alert: params[:error_description]) + else + redirect_to(login_path, alert: params[:error_description]) + end + return + end + + if Current.user + Current.user.microsoft_graph_credential&.destroy + _, cred = add_person_credentials("MicrosoftGraphCredential") + cred.save! && redirect_to(edit_settings_person_path, notice: "Saved") && return + + elsif (credential = MicrosoftGraphCredential.find_by(oauth_id: auth[:uid])) + @person = credential.user.person + + elsif Feature.disabled?(:registration) + redirect_to(root_path, alert: "Registration is disabled") && return + + elsif auth_email && (user = Person.find_by(email: auth_email)&.user) + @person = init_for_user(user) + + elsif auth_email && (@person = Person.find_by(email: auth_email)) + @person = init_for_person(@person) + + else + @person = initialize_microsoft_person + end + + if @person&.save + login_as(@person, credential: @person.user.reload.microsoft_graph_credential) + redirect_to root_path + else + @person&.errors&.delete :personable + msg = @person.errors.full_messages.map { |m| m.gsub(/Personable |credentials /, "") }.to_sentence.capitalize + if msg.downcase.include?("oauth refresh token can't be blank") + msg += " " + helpers.link_to("Microsoft third-party connections", "https://account.microsoft.com/privacy/app-access", class: "underline") + " search for website, and delete all it's connections. Then try again." + end + + redirect_to new_user_path, alert: msg + end + rescue => e + warn e.message + warn e.backtrace.join("\n") + redirect_to edit_settings_person_path, alert: "Error. #{e.message}", status: :see_other + end + + private + + def auth + request.env["omniauth.auth"]&.deep_symbolize_keys || {} + end + + def auth_email + auth.dig(:info, :email) + end + + def init_for_user(user) + user.microsoft_graph_credential&.destroy + + user.first_name = auth[:info][:first_name] + user.last_name = auth[:info][:last_name] + @person = user.person + add_person_credentials("MicrosoftGraphCredential").first + end + + def init_for_person(person) + @person.personable_type = "User" + @person.personable_attributes = { + first_name: auth[:info][:first_name], + last_name: auth[:info][:last_name] + } + add_person_credentials("MicrosoftGraphCredential").first + end + + def initialize_microsoft_person + @person = Person.new({ + personable_type: "User", + email: auth_email, + personable_attributes: { + first_name: auth[:info][:first_name], + last_name: auth[:info][:last_name], + } + }) + add_person_credentials("MicrosoftGraphCredential").first + end + + def add_person_credentials(type) + p = Current.person || @person + c = p.user.credentials.build( + type: type, + oauth_id: auth[:uid], + oauth_email: auth[:info][:email], + oauth_token: auth[:credentials][:token], + oauth_refresh_token: auth[:credentials][:refresh_token], + properties: auth[:credentials].except(:token, :refresh_token) + ) + [p, c] + end +end diff --git a/app/controllers/concerns/authenticate/login_logout.rb b/app/controllers/concerns/authenticate/login_logout.rb index 3d54a5366..b589cb3ed 100644 --- a/app/controllers/concerns/authenticate/login_logout.rb +++ b/app/controllers/concerns/authenticate/login_logout.rb @@ -35,6 +35,6 @@ def reset_authentication end def manual_login_allowed? - Feature.password_authentication? || Feature.google_authentication? + Feature.password_authentication? || Feature.google_authentication? || Feature.microsoft_graph_authentication? end end diff --git a/app/models/feature.rb b/app/models/feature.rb index 2841773f6..89125c8f9 100644 --- a/app/models/feature.rb +++ b/app/models/feature.rb @@ -21,6 +21,7 @@ def features if @@features_hash[:http_header_authentication] @@features_hash[:password_authentication] = false @@features_hash[:google_authentication] = false + @@features_hash[:microsoft_graph_authentication] = false end @@features_hash diff --git a/app/models/microsoft_graph_credential.rb b/app/models/microsoft_graph_credential.rb new file mode 100644 index 000000000..fadcf6140 --- /dev/null +++ b/app/models/microsoft_graph_credential.rb @@ -0,0 +1,10 @@ +class MicrosoftGraphCredential < Credential + alias_attribute :oauth_id, :external_id + + validates :oauth_token, presence: true + validates :oauth_refresh_token, presence: true + validates :oauth_id, presence: true, uniqueness: true + validates :oauth_email, presence: true, uniqueness: true, format: { with: URI::MailTo::EMAIL_REGEXP } + + normalizes :oauth_email, with: -> email { email.downcase.strip } +end diff --git a/app/models/user.rb b/app/models/user.rb index 5f8b2df0a..c822d839b 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -18,6 +18,7 @@ class User < ApplicationRecord has_one :google_credential, -> { type_is("GoogleCredential") }, class_name: "Credential", inverse_of: :user has_one :gmail_credential, -> { type_is("GmailCredential") }, class_name: "Credential", inverse_of: :user has_one :google_tasks_credential, -> { type_is("GoogleTasksCredential") }, class_name: "Credential", inverse_of: :user + has_one :microsoft_graph_credential, -> { type_is("MicrosoftGraphCredential") }, class_name: "Credential", inverse_of: :user has_one :http_header_credential, -> { type_is("HttpHeaderCredential") }, class_name: "Credential", inverse_of: :user belongs_to :last_cancelled_message, class_name: "Message", optional: true diff --git a/app/views/authentications/new.html.erb b/app/views/authentications/new.html.erb index f88b7a5ba..f5389ca9e 100644 --- a/app/views/authentications/new.html.erb +++ b/app/views/authentications/new.html.erb @@ -26,6 +26,27 @@ <% end %> +<% if Feature.microsoft_graph_authentication? %> + <%= button_to "Log In with Microsoft", "/auth/microsoft_graph", + method: "post", + class: %| + text-white font-medium + bg-brand-blue dark:bg-gray-900 + border border-white dark:border-gray-400 + rounded-lg p-4 text-center + cursor-pointer + hover:opacity-95 + |, + form_class: "flex flex-col space-y-4 w-80", + data: { + turbo: false, + } + %> + + - Or - + +<% end %> + <% if Feature.password_authentication? %> <%= form_with(url: login_path, method: :post, class: "flex flex-col space-y-4 w-80") do |f| %> <%= f.email_field :email, diff --git a/app/views/users/new.html.erb b/app/views/users/new.html.erb index fe5f98f62..d8754299a 100644 --- a/app/views/users/new.html.erb +++ b/app/views/users/new.html.erb @@ -30,6 +30,27 @@ <% end %> +<% if Feature.microsoft_graph_authentication? %> + <%= button_to "Sign Up with Microsoft", "/auth/microsoft_graph", + method: "post", + class: %| + text-white font-medium + bg-brand-blue dark:bg-gray-900 + border border-white dark:border-gray-400 + rounded-lg p-4 text-center + cursor-pointer + hover:opacity-95 + |, + form_class: "flex flex-col space-y-4 w-80", + data: { + turbo: false, + } + %> + + - Or - + +<% end %> + <% if Feature.password_authentication? %> <%= form_with method: :post, model: @person, diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 48eadec41..95a227561 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -26,4 +26,10 @@ include_granted_scopes: true } + provider :microsoft_graph, Setting.microsoft_graph_auth_client_id, Setting.microsoft_graph_auth_client_secret, { + name: "microsoft_graph", + scope: Setting.microsoft_graph_scope.presence || %| + openid profile email offline_access user.read mailboxsettings.read + | + } end diff --git a/config/options.yml b/config/options.yml index ee007b39c..8c2ab07ca 100644 --- a/config/options.yml +++ b/config/options.yml @@ -40,6 +40,7 @@ shared: google_tools: <%= ENV["GOOGLE_TOOLS_FEATURE"] || false %> password_authentication: <%= ENV["PASSWORD_AUTHENTICATION_FEATURE"] || true %> google_authentication: <%= ENV["GOOGLE_AUTHENTICATION_FEATURE"] || false %> + microsoft_graph_authentication: <%= ENV["MICROSOFT_GRAPH_AUTHENTICATION_FEATURE"] || false %> http_header_authentication: <%= ENV["HTTP_HEADER_AUTHENTICATION_FEATURE"] || false %> voice: <%= ENV["VOICE_FEATURE"] || false %> default_to_voice: <%= ENV["DEFAULT_TO_VOICE_FEATURE"] || false %> @@ -55,8 +56,11 @@ shared: cloudflare_access_key_id: <%= ENV["CLOUDFLARE_ACCESS_KEY_ID"] || default_to(cloudflare: :access_key_id) %> cloudflare_secret_access_key: <%= ENV["CLOUDFLARE_SECRET_ACCESS_KEY"] || default_to(cloudflare: :secret_access_key) %> cloudflare_bucket: <%= ENV["CLOUDFLARE_BUCKET"] || default_to(cloudflare: :bucket) %> - google_auth_client_id: <%= ENV["GOOGLE_AUTH_CLIENT_ID"] || default_to(:google_auth_client_id) %> - google_auth_client_secret: <%= ENV["GOOGLE_AUTH_CLIENT_SECRET"] || default_to(:google_auth_client_secret) %> + google_auth_client_id: <%= ENV["GOOGLE_AUTH_CLIENT_ID"] || default_to(google: :auth_client_id) || default_to(:google_auth_client_id) %> + google_auth_client_secret: <%= ENV["GOOGLE_AUTH_CLIENT_SECRET"] || default_to(google: :auth_client_secret) || default_to(:google_auth_client_secret) %> + microsoft_graph_auth_client_id: <%= ENV["MICROSOFT_GRAPH_AUTH_CLIENT_ID"] || default_to(microsoft_graph: :auth_client_id) %> + microsoft_graph_auth_client_secret: <%= ENV["MICROSOFT_GRAPH_AUTH_CLIENT_SECRET"] || default_to(microsoft_graph: :auth_client_secret) %> + microsoft_graph_scope: <%= ENV["MICROSOFT_GRAPH_SCOPE"] || default_to(microsoft_graph: :scope) %> http_header_auth_email: <%= ENV["HTTP_HEADER_AUTH_EMAIL"] || "X-WEBAUTH-EMAIL" %> http_header_auth_name: <%= ENV["HTTP_HEADER_AUTH_NAME"] || "X-WEBAUTH-NAME" %> http_header_auth_uid: <%= ENV["HTTP_HEADER_AUTH_UID"] || "X-WEBAUTH-USER" %> diff --git a/config/routes.rb b/config/routes.rb index 810de522f..6c8b4733f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -33,6 +33,7 @@ resource :password_credential, only: [:edit, :update] end + get "/auth/microsoft_graph/callback" => "authentications/microsoft_graph_oauth#create", as: :microsoft_graph_oauth, provider: "microsoft_graph" get "/auth/:provider/callback" => "authentications/google_oauth#create", as: :google_oauth get "/auth/failure" => "authentications/google_oauth#failure" # connected in omniauth.rb diff --git a/test/controllers/authentications/microsoft_auth_test.rb b/test/controllers/authentications/microsoft_auth_test.rb new file mode 100644 index 000000000..1da9ba72f --- /dev/null +++ b/test/controllers/authentications/microsoft_auth_test.rb @@ -0,0 +1,19 @@ +require "test_helper" + +class AuthenticationsController::MicrosoftAuthTest < ActionDispatch::IntegrationTest + test "should NOT display a Microsoft button when the feature is DISABLED" do + stub_features(microsoft_graph_authentication: false) do + get login_url + assert_response :success + assert_no_match "Log In with Microsoft", response.body + end + end + + test "should SHOW the Microsoft button when the feature is ENABLED" do + stub_features(microsoft_graph_authentication: true) do + get login_url + assert_response :success + assert_match "Log In with Microsoft", response.body + end + end +end diff --git a/test/controllers/authentications/microsoft_graph_oauth_controller_test.rb b/test/controllers/authentications/microsoft_graph_oauth_controller_test.rb new file mode 100644 index 000000000..50f55225a --- /dev/null +++ b/test/controllers/authentications/microsoft_graph_oauth_controller_test.rb @@ -0,0 +1,179 @@ +require "test_helper" + +class Authentications::MicrosoftGraphOauthControllerTest < ActionDispatch::IntegrationTest + setup do + OmniAuth.config.test_mode = true + end + + test "cancelling a microsoft oauth flow redirects to the settings page" do + login_as users(:keith) + get "/auth/microsoft_graph/callback?error=access_denied&error_description=The%20user%20has%20denied%20access%20to%20the%20scope%20requested%20by%20the%20client%20application" + assert_redirected_to edit_settings_person_path + end + + test "cancelling a microsoft oauth flow redirects to the login" do + get "/auth/microsoft_graph/callback?error=access_denied&error_description=The%20user%20has%20denied%20access%20to%20the%20scope%20requested%20by%20the%20client%20application" + assert_redirected_to login_path + end + + test "existing user can add a MicrosoftGraphCredential and redirect to edit_settings" do + # rob has no microsoft credential + user = users(:rob) + login_as user + details = details_for_email(email: user.email, first_name: "Rob", last_name: nil) + OmniAuth.config.add_mock(:microsoft_graph, details) + + refute user.microsoft_graph_credential.present? + assert_difference "Credential.count", 1 do + assert_no_difference "User.count" do + get microsoft_graph_oauth_path + end + end + + assert_redirected_to edit_settings_person_path + assert_equal "Saved", flash[:notice] + assert_credential_matches_details(user.reload.microsoft_graph_credential, details) + end + + test "should log you in for a user that exists" do + OmniAuth.config.add_mock(:microsoft_graph, { uid: credentials(:keith_microsoft_graph).oauth_id }) + assert_no_difference "Credential.count" do + assert_difference "Authentication.count", 1 do + assert_difference "Client.count", 1 do + get microsoft_graph_oauth_path + end + end + end + + assert_redirected_to root_path + assert_logged_in(users(:keith)) + end + + test "registration is disabled" do + OmniAuth.config.add_mock(:microsoft_graph, details_for_email(email: "john@gmail.com")) + + stub_features(microsoft_graph_authentication: true, registration: false) do + get microsoft_graph_oauth_path + end + assert_redirected_to root_path + assert_equal "Registration is disabled", flash[:alert] + end + + test "should UPDATE A USER when authing with PASSWORD SIGNED UP USER that we find by matching email" do + user = users(:rob) + + assert user.password_credential.present? + refute user.microsoft_graph_credential.present? + assert_not_equal "John", user.first_name + assert_not_equal "Doe", user.last_name + + details = details_for_email(email: user.email) + OmniAuth.config.add_mock(:microsoft_graph, details) + + stub_features(microsoft_graph_authentication: true, registration: true) do + assert_no_difference "Person.count" do + assert_no_difference "User.count" do + assert_difference "Client.count", 1 do + assert_difference "Credential.count", 1 do + assert_difference "Authentication.count", 1 do + get microsoft_graph_oauth_path + end + end + end + end + end + end + user.reload + + assert_redirected_to root_path + assert_logged_in(user) + assert_credential_matches_details(user.microsoft_graph_credential, details) + assert_equal details[:info][:email], user.email + assert user.clients.ordered.last.authenticated? + end + + test "should CREATE A USER and everything else when authing with a NOT SIGNED UP person" do + assert people(:ali_invited).email.present? + assert_nil people(:ali_invited).user + + details = details_for_email(email: people(:ali_invited).email) + OmniAuth.config.add_mock(:microsoft_graph, details) + + stub_features(microsoft_graph_authentication: true, registration: true) do + assert_no_difference "Person.count" do + assert_difference "User.count", 1 do + assert_difference "Client.count", 1 do + assert_difference "Credential.count", 1 do + assert_difference "Authentication.count", 1 do + get microsoft_graph_oauth_path + end + end + end + end + end + end + user = people(:ali_invited).reload.user + assert user.present? + + assert_redirected_to root_path + assert_logged_in(user) + assert_credential_matches_details(user.microsoft_graph_credential, details) + assert_equal details[:info][:email], user.email + assert user.clients.ordered.last.authenticated? + end + + test "should CREATE A USER when google authing and NOTHING EXISTS" do + details = details_for_email(email: "john@gmail.com") + OmniAuth.config.add_mock(:microsoft_graph, details) + + stub_features(microsoft_graph_authentication: true, registration: true) do + assert_difference "Person.count", 1 do + assert_difference "User.count", 1 do + assert_difference "Client.count", 1 do + assert_difference "Credential.count", 1 do + assert_difference "Authentication.count", 1 do + get microsoft_graph_oauth_path + end + end + end + end + end + end + user = User.last + + assert_redirected_to root_path + assert_logged_in(user) + assert_credential_matches_details(user.microsoft_graph_credential, details) + assert_equal details[:info][:email], user.email + assert user.clients.ordered.last.authenticated? + end + + private + + def details_for_email(email:, first_name: "John", last_name: "Doe") + { + uid: "new_abc123", + info: { + email: email, + first_name:, + last_name: + }, + credentials: { + token: "new_token", + refresh_token: "new_refresh_token", + scope: "openid profile email offline_access user.read mailboxsettings.read" + } + } + end + + def assert_credential_matches_details(credential, details) + user = credential.user + assert_equal details[:info][:first_name], user.first_name if details.dig(:info, :first_name) + assert_equal details[:info][:last_name], user.last_name if details.dig(:info, :last_name) + assert_equal details[:uid], credential.oauth_id + assert_equal details[:info][:email], credential.oauth_email + assert_equal details[:credentials][:token], credential.oauth_token + assert_equal details[:credentials][:refresh_token], credential.oauth_refresh_token + assert_equal details[:credentials].except(:token, :refresh_token), credential.properties + end +end diff --git a/test/controllers/authentications/registration_test.rb b/test/controllers/authentications/registration_test.rb index 74f30a8eb..c5d77c9e4 100644 --- a/test/controllers/authentications/registration_test.rb +++ b/test/controllers/authentications/registration_test.rb @@ -9,7 +9,7 @@ class AuthenticationsController::RegistrationTest < ActionDispatch::IntegrationT end test "should return not_found when all auth schemes are disabled" do - stub_features(password_authentication: false, google_authentication: false) do + stub_features(password_authentication: false, google_authentication: false, microsoft_graph_authentication: false) do get register_url assert_response :not_found end diff --git a/test/controllers/authentications_controller_test.rb b/test/controllers/authentications_controller_test.rb index a9e2c0ac3..db9c3de00 100644 --- a/test/controllers/authentications_controller_test.rb +++ b/test/controllers/authentications_controller_test.rb @@ -56,7 +56,7 @@ class AuthenticationsControllerTest < ActionDispatch::IntegrationTest end test "should return not_found when all auth schemes are disabled" do - stub_features(password_authentication: false, google_authentication: false) do + stub_features(password_authentication: false, google_authentication: false, microsoft_graph_authentication: false) do get login_path assert_response :not_found end diff --git a/test/controllers/concerns/authenticate/by_http_header_test.rb b/test/controllers/concerns/authenticate/by_http_header_test.rb index 59a51de43..c4e04d4ee 100644 --- a/test/controllers/concerns/authenticate/by_http_header_test.rb +++ b/test/controllers/concerns/authenticate/by_http_header_test.rb @@ -54,7 +54,8 @@ class Authenticate::ByHttpHeaderTest < ActionDispatch::IntegrationTest stub_features( http_header_authentication: true, password_authentication: false, - google_authentication: false + google_authentication: false, + microsoft_graph_authentication: false ) do get root_url end diff --git a/test/controllers/users/microsoft_auth_test.rb b/test/controllers/users/microsoft_auth_test.rb new file mode 100644 index 000000000..a490797c9 --- /dev/null +++ b/test/controllers/users/microsoft_auth_test.rb @@ -0,0 +1,19 @@ +require "test_helper" + +class UsersController::MicrosoftAuthTest < ActionDispatch::IntegrationTest + test "should NOT display a Microsoft button when the feature is DISABLED" do + stub_features(microsoft_graph_authentication: false) do + get register_url + assert_response :success + assert_no_match "Sign Up with Microsoft", response.body + end + end + + test "should SHOW the Microsoft button when the feature is ENABLED" do + stub_features(microsoft_graph_authentication: true) do + get register_url + assert_response :success + assert_match "Sign Up with Microsoft", response.body + end + end +end diff --git a/test/controllers/users/registration_test.rb b/test/controllers/users/registration_test.rb index cd2b922c9..8abc4dc6f 100644 --- a/test/controllers/users/registration_test.rb +++ b/test/controllers/users/registration_test.rb @@ -9,7 +9,7 @@ class UsersController::RegistrationTest < ActionDispatch::IntegrationTest end test "should return not_found when all auth schemes are disabled" do - stub_features(password_authentication: false, google_authentication: false) do + stub_features(password_authentication: false, google_authentication: false, microsoft_graph_authentication: false) do get register_url assert_response :not_found end diff --git a/test/fixtures/credentials.yml b/test/fixtures/credentials.yml index 0be77dad9..d0dd88417 100644 --- a/test/fixtures/credentials.yml +++ b/test/fixtures/credentials.yml @@ -34,6 +34,12 @@ keith_google_tasks: properties: {"expires_at":1717004605,"expires":true,"scope":"https://www.googleapis.com/auth/gmail.modify https://www.googleapis.com/auth/userinfo.email"} last_authenticated_at: 2023-12-19 08:40:05 +keith_microsoft_graph: + user: keith + type: MicrosoftGraphCredential + external_id: microsoft_id123 + oauth_email: keith@hostedgpt.com + rob_password: user: rob type: PasswordCredential diff --git a/test/models/feature_test.rb b/test/models/feature_test.rb index e78a6ab07..b0cca2c33 100644 --- a/test/models/feature_test.rb +++ b/test/models/feature_test.rb @@ -60,6 +60,7 @@ class FeatureTest < ActiveSupport::TestCase http_header_authentication: true, password_authentication: true, google_authentication: true, + microsoft_graph_authentication: true ) do assert Feature.enabled?(:http_header_authentication) assert Feature.http_header_authentication? @@ -67,6 +68,8 @@ class FeatureTest < ActiveSupport::TestCase refute Feature.password_authentication? refute Feature.enabled?(:google_authentication) refute Feature.google_authentication? + refute Feature.enabled?(:microsoft_graph_authentication) + refute Feature.microsoft_graph_authentication? end end @@ -76,13 +79,16 @@ class FeatureTest < ActiveSupport::TestCase http_header_authentication: false, password_authentication: true, google_authentication: false, - ) do + microsoft_graph_authentication: false + ) do refute Feature.enabled?(:http_header_authentication) refute Feature.http_header_authentication? assert Feature.enabled?(:password_authentication) assert Feature.password_authentication? refute Feature.enabled?(:google_authentication) refute Feature.google_authentication? + refute Feature.enabled?(:microsoft_graph_authentication) + refute Feature.microsoft_graph_authentication? end end diff --git a/test/models/microsoft_graph_credential_test.rb b/test/models/microsoft_graph_credential_test.rb new file mode 100644 index 000000000..d82701567 --- /dev/null +++ b/test/models/microsoft_graph_credential_test.rb @@ -0,0 +1,13 @@ +require "test_helper" + +class MicrosoftGraphCredentialTest < ActiveSupport::TestCase + # See credential_test for tests in common to all credentials + + test "confirm STI is working" do + assert_instance_of MicrosoftGraphCredential, credentials(:keith_microsoft_graph) + end + + test "confirm that one of the MicrosoftGraphApp included changes is working" do + assert_equal credentials(:keith_microsoft_graph).external_id, credentials(:keith_microsoft_graph).oauth_id + end +end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 2d37ac0c2..b64cf217a 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -29,6 +29,10 @@ class UserTest < ActiveSupport::TestCase assert_instance_of GoogleTasksCredential, users(:keith).google_tasks_credential end + test "has an associated microsoft_graph_credential" do + assert_instance_of MicrosoftGraphCredential, users(:keith).microsoft_graph_credential + end + test "has an associated http_header_credential" do assert_instance_of HttpHeaderCredential, users(:rob).http_header_credential end diff --git a/test/system/sessions_test.rb b/test/system/sessions_test.rb index 65aa0e2ab..08fb88e7e 100644 --- a/test/system/sessions_test.rb +++ b/test/system/sessions_test.rb @@ -43,4 +43,18 @@ class SessionsTest < ApplicationSystemTestCase assert_text "Log In with Google" end end + + test "should SHOW the Microsoft button when the feature is ENABLED" do + stub_features(microsoft_graph_authentication: true) do + visit root_url + assert_text "Log In with Microsoft" + end + end + + test "should NOT display a Microsoft button when the feature is DISABLED" do + stub_features(microsoft_graph_authentication: false) do + visit root_url + assert_no_text "Log In with Microsoft" + end + end end From 751d103363a9e1e466bf916c7f10b1f3068e08af Mon Sep 17 00:00:00 2001 From: Paul Vudmaska Date: Sat, 16 Nov 2024 12:56:24 -0600 Subject: [PATCH 18/43] truncate long names so nav-container does not grow (#550) --- app/helpers/application_helper.rb | 4 ++++ app/views/layouts/application.html.erb | 2 +- db/schema.rb | 4 ++-- test/helpers/application_helper_test.rb | 13 +++++++++++++ 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index a088b0899..99f6a6554 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,5 +1,9 @@ module ApplicationHelper + def truncate_long_name(name) + truncate(name, length: 20) + end + def at_most_two_initials(initials) return initials if initials.nil? || initials.length <= 2 initials[0] + initials[-1] diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 6f67f5e7f..949d9707d 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -36,7 +36,7 @@
<%= render partial: "layouts/user_avatar", locals: { user: Current.user, size: 7, classes: "ml-2 mr-2" } %>
- <%= Current.user.name.full || "Profile" %> + <%= truncate_long_name(Current.user.name.full) || "Profile" %>
<%= icon "chevron-up", variant: :mini, class: 'text-gray-500 ml-[2px]' %>
diff --git a/db/schema.rb b/db/schema.rb index 44ceb9d69..bbd39ec21 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_11_11_131751) do +ActiveRecord::Schema[7.2].define(version: 2024_11_11_131751) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -172,8 +172,8 @@ t.boolean "supports_tools", default: false t.decimal "input_token_cost_cents", precision: 30, scale: 15 t.decimal "output_token_cost_cents", precision: 30, scale: 15 - t.boolean "supports_system_message", default: false t.boolean "best", default: false + t.boolean "supports_system_message", default: false t.index ["api_service_id"], name: "index_language_models_on_api_service_id" t.index ["user_id", "deleted_at"], name: "index_language_models_on_user_id_and_deleted_at" t.index ["user_id"], name: "index_language_models_on_user_id" diff --git a/test/helpers/application_helper_test.rb b/test/helpers/application_helper_test.rb index 8d702b208..680fed592 100644 --- a/test/helpers/application_helper_test.rb +++ b/test/helpers/application_helper_test.rb @@ -29,4 +29,17 @@ class ApplicationHelperTest < ActionView::TestCase test "can have spaces" do assert_equal "pQ", at_most_two_initials("p v Q") end + + test "truncates long names" do + assert_equal "John D. Z. Smith ...", truncate_long_name("John D. Z. Smith Jane Doe") + end + + test "short names are not truncated" do + assert_equal "John D. Doe", truncate_long_name("John D. Doe") + end + + test "handles nil" do + assert_nil truncate_long_name(nil) + end + end From 7a58fe95e23ed4e4abe3ab74c7600254b2cf8ca0 Mon Sep 17 00:00:00 2001 From: Paul Vudmaska Date: Mon, 18 Nov 2024 16:05:48 -0600 Subject: [PATCH 19/43] Truncate profile name using CSS instead of rails helper (#554) --- app/helpers/application_helper.rb | 4 ---- app/views/layouts/application.html.erb | 4 ++-- test/helpers/application_helper_test.rb | 12 ------------ 3 files changed, 2 insertions(+), 18 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 99f6a6554..a088b0899 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -1,9 +1,5 @@ module ApplicationHelper - def truncate_long_name(name) - truncate(name, length: 20) - end - def at_most_two_initials(initials) return initials if initials.nil? || initials.length <= 2 initials[0] + initials[-1] diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 949d9707d..86d06f452 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -35,8 +35,8 @@