diff --git a/app/controllers/commands_controller.rb b/app/controllers/commands_controller.rb index b697fb826..679e1219b 100644 --- a/app/controllers/commands_controller.rb +++ b/app/controllers/commands_controller.rb @@ -12,7 +12,7 @@ class CommandsController < ApplicationController result = command.execute respond_with_execution_result(result) else - render plain: command.title, status: :conflict + respond_with_needs_confirmation(command) end else head :unprocessable_entity @@ -21,7 +21,11 @@ class CommandsController < ApplicationController private def parse_command(string) - Command::Parser.new(parsing_context).parse(string) + command_parser.parse(string) + end + + def command_parser + @command_parser ||= Command::Parser.new(parsing_context) end def parsing_context @@ -37,9 +41,30 @@ class CommandsController < ApplicationController when Command::Result::Redirection redirect_to result.url when Command::Result::ChatResponse - render json: result.content, status: :accepted + respond_with_chat_response(result) else redirect_back_or_to root_path end end + + def respond_with_chat_response(result) + command = chat_response_to_command(result) + + if confirmed?(command) + command.execute + redirect_back_or_to root_path + else + respond_with_needs_confirmation(command.commands, redirect_to: result.context_url) + end + end + + def respond_with_needs_confirmation(commands, redirect_to: nil) + render json: { commands: Array(commands).collect(&:title), redirect_to: redirect_to }, status: :conflict + end + + def chat_response_to_command(chat_response) + context = Command::Parser::Context.new(Current.user, url: chat_response.context_url || request.referrer) + parser = Command::Parser.new(context) + Command::Composite.new(chat_response.command_lines.collect { parser.parse it }) + end end diff --git a/app/javascript/controllers/terminal_controller.js b/app/javascript/controllers/terminal_controller.js index 1191371f5..ce40fdde1 100644 --- a/app/javascript/controllers/terminal_controller.js +++ b/app/javascript/controllers/terminal_controller.js @@ -1,14 +1,18 @@ import { Controller } from "@hotwired/stimulus" import { HttpStatus } from "helpers/http_helpers" -import { delay } from "helpers/timing_helpers" +import { delay, nextFrame } from "helpers/timing_helpers" import { marked } from "marked" export default class extends Controller { static targets = [ "input", "form", "confirmation" ] static classes = [ "error", "confirmation", "help" ] + static values = { originalInput: String, waitingForConfirmation: Boolean } - disconnect() { - if (this.waitingForConfirmation) { this.#reset() } + connect() { + if (this.waitingForConfirmationValue) { + this.inputTarget.setSelectionRange(this.inputTarget.value.length, this.inputTarget.value.length) + this.inputTarget.focus() + } } // Actions @@ -33,7 +37,7 @@ export default class extends Controller { } handleKeyPress(event) { - if (this.waitingForConfirmation) { + if (this.waitingForConfirmationValue) { this.#handleConfirmationKey(event.key.toLowerCase()) event.preventDefault() } @@ -41,12 +45,7 @@ export default class extends Controller { handleCommandResponse(event) { if (event.detail.success) { - const response = event.detail.fetchResponse - if (response && response.response.headers.get("Content-Type")?.includes("application/json")) { - this.#handleJsonCommandResponse(response) - } else { - this.#reset() - } + this.#reset() } else { const response = event.detail.fetchResponse.response this.#handleErrorResponse(response) @@ -79,31 +78,20 @@ export default class extends Controller { async #handleErrorResponse(response) { const status = response.status - const message = await response.text() if (status === HttpStatus.UNPROCESSABLE) { this.#showError() } else if (status === HttpStatus.CONFLICT) { - this.#requestConfirmation(message) + const jsonResponse = await response.json() + this.#requestConfirmation(jsonResponse) } } - #handleJsonCommandResponse(response) { - response.response.json().then((commands) => { - if (commands.reply) { - this.element.querySelector("#chat-insight").innerHTML = marked.parse(commands.reply) - } else { - this.element.querySelector("#chat-responses").textContent = JSON.stringify(commands, null, 2) - } - this.#executeCommands(commands) - }) - } - #reset(inputValue = "") { - this.formTarget.reset() + console.debug("RESET!") this.inputTarget.value = inputValue this.confirmationTarget.value = "" - this.waitingForConfirmation = false + this.waitingForConfirmationValue = false this.originalInputValue = null this.element.classList.remove(this.errorClass) @@ -114,12 +102,20 @@ export default class extends Controller { this.element.classList.add(this.errorClass) } - async #requestConfirmation(message) { + async #requestConfirmation(jsonResponse) { + const message = jsonResponse.commands[0] + console.debug("request confirmation", this.inputTarget.value); this.originalInputValue = this.inputTarget.value + + this.element.classList.add(this.confirmationClass) this.inputTarget.value = `${message}? [Y/n] ` - this.waitingForConfirmation = true + this.waitingForConfirmationValue = true + + if (jsonResponse.redirect_to) { + Turbo.visit(jsonResponse.redirect_to, { frame: "cards" }) + } } #handleConfirmationKey(key) { @@ -135,32 +131,4 @@ export default class extends Controller { this.confirmationTarget.value = "confirmed" this.formTarget.requestSubmit() } - - async #executeCommands(commands) { - for (const command of commands) { - if (command.command == "/search") { - // Clunky example, for PoC purposes - Turbo.visit(`/cards?${toQueryString(command)}`) - await delay(2000) - } else { - this.inputTarget.value = command.command - this.formTarget.requestSubmit() - } - } - } -} - -function toQueryString(obj) { - const params = new URLSearchParams() - for (const key in obj) { - const value = obj[key] - if (Array.isArray(value)) { - for (const item of value) { - params.append(`${key}[]`, item) - } - } else if (value !== undefined && value !== null) { - params.append(key, value) - } - } - return params.toString() } diff --git a/app/models/command/chat_query.rb b/app/models/command/chat_query.rb index 2b6540634..0b47cea67 100644 --- a/app/models/command/chat_query.rb +++ b/app/models/command/chat_query.rb @@ -7,8 +7,8 @@ class Command::ChatQuery < Command def execute response = chat.ask query - response = replace_names_with_ids(JSON.parse(response.content)) - Command::Result::ChatResponse.new(JSON.pretty_generate(response)) + generated_commands = replace_names_with_ids(JSON.parse(response.content)) + build_chat_response_with generated_commands end private @@ -17,6 +17,8 @@ class Command::ChatQuery < Command chat.with_instructions(prompt) end + # TODO: + # - Don't generate initial /search if not requested. "Assign to JZ" should def prompt <<~PROMPT You are a helpful assistant that translates natural language into commands that Fizzy understand. @@ -76,7 +78,9 @@ class Command::ChatQuery < Command ] Unless asking for explicit filtering, always prefer /insight over /search. When asking about "cards" with properties that can - be satisfied with /search, then use /search + be satisfied with /search, then use /search. + + A response can contain at most one /search command. Please combine commands to satisfy what the user needs. E.g: search with keywords and filters and then apply as many commands as needed. Make sure you don't leave actions mentioned in the query needs unattended.' @@ -111,4 +115,19 @@ class Command::ChatQuery < Command User.all.find { |user| user.mentionable_handles.include?(string_without_at) } end + def build_chat_response_with(generated_commands) + Command::Result::ChatResponse.new \ + command_lines: response_command_lines_from(generated_commands), + context_url: response_context_url_from(generated_commands) + end + + def response_command_lines_from(generated_commands) + generated_commands.filter { it["command"] != "/search" }.collect { it["command"] } + end + + def response_context_url_from(generated_commands) + if search_command = generated_commands.find { it["command"] == "/search" } + cards_path(**search_command.without("command")) + end + end end diff --git a/app/models/command/composite.rb b/app/models/command/composite.rb new file mode 100644 index 000000000..237e5b636 --- /dev/null +++ b/app/models/command/composite.rb @@ -0,0 +1,33 @@ +# A composite of commands +class Command::Composite + attr_reader :commands + + def initialize(commands) + @commands = commands + end + + def title + @commands.collect(&:title).join(", ") + end + + def execute + ApplicationRecord.transaction do + commands.each(&:execute) + end + end + + def undo + ApplicationRecord.transaction do + undoable_commands.reverse.each(&:undo) + end + end + + def needs_confirmation? + commands.any?(&:needs_confirmation?) + end + + private + def undoable_commands + commands.filter(&:undoable?) + end +end diff --git a/app/models/command/get_insight.rb b/app/models/command/get_insight.rb index 5d08e0731..c43380b9c 100644 --- a/app/models/command/get_insight.rb +++ b/app/models/command/get_insight.rb @@ -9,7 +9,7 @@ class Command::GetInsight < Command def execute response = chat.ask query - Command::Result::ChatResponse.new({ reply: response.content }) + Command::Result::InsightResponse.new({ reply: response.content }) end def undoable? diff --git a/app/models/command/result/chat_response.rb b/app/models/command/result/chat_response.rb index 4e1ea2f67..d6a0cd2e3 100644 --- a/app/models/command/result/chat_response.rb +++ b/app/models/command/result/chat_response.rb @@ -1 +1,12 @@ -Command::Result::ChatResponse = Struct.new(:content) +class Command::Result::ChatResponse + attr_reader :command_lines, :context_url + + def initialize(context_url: nil, command_lines:) + @context_url = context_url + @command_lines = command_lines + end + + def has_context_url? + context_url.present? + end +end diff --git a/app/models/command/result/insight_response.rb b/app/models/command/result/insight_response.rb new file mode 100644 index 000000000..ebf76ea75 --- /dev/null +++ b/app/models/command/result/insight_response.rb @@ -0,0 +1 @@ +Command::Result::InsightResponse = Struct.new(:content) diff --git a/app/views/commands/_form.html.erb b/app/views/commands/_form.html.erb index 239dc6263..a1d775036 100644 --- a/app/views/commands/_form.html.erb +++ b/app/views/commands/_form.html.erb @@ -4,7 +4,6 @@ data: { controller: "form", terminal_target: "form", - turbo_permanent: true, action: "turbo:submit-end->terminal#focus keydown.meta+k@document->terminal#focus keydown.enter->terminal#executeCommand keydown.esc->terminal#hideHelpMenu turbo:submit-end->terminal#handleCommandResponse" } do %> diff --git a/app/views/commands/_terminal.html.erb b/app/views/commands/_terminal.html.erb index 6efee7eff..1d4c3480c 100644 --- a/app/views/commands/_terminal.html.erb +++ b/app/views/commands/_terminal.html.erb @@ -1,10 +1,14 @@ -<%= tag.div class: "terminal full-width flex flex-column gap justify-end", data: { - controller: "toggle-class terminal navigable-list", - terminal_error_class: "terminal--error", - terminal_confirmation_class: "terminal--confirmation", - terminal_help_class: "terminal--showing-help", - toggle_class_toggle_class: "terminal--open", - action: "keydown->terminal#handleKeyPress keydown->navigable-list#navigate focusin->navigable-list#select keydown.esc->toggle-class#remove:stop keydown.esc->terminal#focus keydown.esc->navigable-list#selectLast keydown.esc@document->terminal#hideHelpMenu" } do %> +<%= tag.div \ + id: "command-terminal", + class: "terminal full-width flex flex-column gap justify-end", + data: { + controller: "toggle-class terminal navigable-list", + terminal_error_class: "terminal--error", + terminal_confirmation_class: "terminal--confirmation", + terminal_help_class: "terminal--showing-help", + toggle_class_toggle_class: "terminal--open", + turbo_permanent: true, + action: "keydown->terminal#handleKeyPress keydown->navigable-list#navigate focusin->navigable-list#select keydown.esc->toggle-class#remove:stop keydown.esc->terminal#focus keydown.esc->navigable-list#selectLast keydown.esc@document->terminal#hideHelpMenu" } do %> <%= turbo_frame_tag :recent_commands, src: commands_path, refresh: "morph" %> <%= render "commands/form" %>