ai: compute opportinistic summary of command execution by hugoShaka · Pull Request #28033 · gravitational/teleport

hugoShaka · 2023-06-19T14:50:56Z

Fixes https://github.com/gravitational/teleport.e/issues/1466

This is the MVP for the command execution summary. During the implementation, I encountered the following issues:

the execution endpoint is different from the conversation one. The Chat from the conversation endpoint will not be notified that messages were inserted by the execution. I worked around by flagging the history as stale after a command suggestion and rebuilding the Chat object. This is not ideal but will work at our current scale. We might want to merge the discussion and execution endpoint later.
When asking followup question the model makes mistakes when invoking the command suggestion tool. This is not related directly to the feature but harms the UX a bit. I think we'll want to prioritize fixing this.

I have not implemented posthog events reporting token consumption and command execution as part of the MVP.

The tests in command_test.go required mocking openAI to avoid summary generation warnings. I copied some code from #28077 and we'll want to cleanup once the two branches are merged.

hugoShaka · 2023-06-20T22:02:39Z

@ryanclark could you review the web/ part? I have 0 experience with TypeScript, please don't assume I know what I'm doing 🙃

Edit: also the test are not passing, I'll fix them tomorrow and mark the branch ready

jakule · 2023-06-20T22:17:09Z

This command is getting popular 😆

$ grep -HR "echo txlxport | sed 's/x/e/g'" . | wc -l 4

I changed everything to use this new constant instead of the string. However this makes the flaky test detector run on Terminal tests (and they seem racy). I might have to undo this change to merge the branch.

+1 for reverting the change. Fixing the tests shouldn't be part of this PR.

xacrimon · 2023-06-21T15:26:01Z

Doesn't this just perform a generic completion request? Shouldn't this function be responsible for injecting the summary prompt?

ryanclark

web changes look good to me!

jakule

As I mentioned in the review, the WS close issue must be solved before this PR can be merged.

jakule · 2023-06-22T16:41:56Z

Did you find any message with an empty role? Curious about the first part of this statement.

COMMAND_RESULT has an empty role for example. Empty role is used in a few places as "this message should not be sent to the model"

jakule

I don't think the fix proposed for the WS close issue in the latest change is correct.

* Revert "fixup! Attempt to tame the stream close monster" This reverts commit 8537aa2. * Revert "Attempt to tame the stream close monster" This reverts commit e0c861d. * Do not close the WS after command execution

jakule

LGTM. Tests need to be fixed.

* ai: compute opportinistic summary of command execution * ai: add streaming summary back after rebase on new front-end * Lint and fix tests pt.1 * reference nodes by name and add tests * Lint, fix tests and address feedback * Attempt to tame the stream close monster * fixup! Attempt to tame the stream close monster * [Assist] Do not close the WS after command execution (#28246) * Revert "fixup! Attempt to tame the stream close monster" This reverts commit 8537aa2. * Revert "Attempt to tame the stream close monster" This reverts commit e0c861d. * Do not close the WS after command execution * Fix tests and lint * fixup! Fix tests and lint * undo put web test command into constant --------- Co-authored-by: Jakub Nyckowski <jakub.nyckowski@goteleport.com>

* [Assist] Scaffold the chat-loop onto a multi-step thinking model (#27075) * agent scaffold conversion * command input validation * rename Agent.Think and replace debug logs with trace logs * doc * action docs * godocs * clarify * remove unused code * remove tests which relied on the old non-agent model interaction with the llm * fix broken e * Add node name to the Assist execution result (#27635) * Add node name to the Assist execution result Currently, only node ID is returned on the command execution result in Assist. For better UX we want to display Node name which id more human friendly rather than a node ID which is a UUID. Adding the value to returned payload sounds cheaper than calling an API to get node names. * Add test * Extract commandExecResult struct * Fix test after rebase * Fix command execution test flakiness (#27704) Fix ``` --- FAIL: TestExecuteCommand (1.46s) testing.go:1206: TempDir RemoveAll cleanup: unlinkat /tmp/TestExecuteCommand3553793052/002/log/upload/streaming/default: directory not empty FAIL ``` error * [Assist] Fix panic when writing to one WS from multiple threads (#27828) * [Assist] Fix panic when writing to one WS from multiple threads Fixes https://github.com/gravitational/teleport.e/issues/1650 * Remove mutex on SetReadDeadline * Move SetPongHandler * Fix typos * Fix command output showing when running on multiple nodes (#27936) * ai: Add a node embedding watcher (#27204) * ai: add embeddings basic support - add Embeddings service and its local implementation - add Embedding type and proto message - add nodeEmbeddingCollector tracking nodes - add NodeEmbeddingWatcher watching for events adn sending them to the collector - add the Embedder interface and its openai implementation * ai: adapt embeddings to the vector index * fixup! ai: adapt embeddings to the vector index * fixup! fixup! ai: adapt embeddings to the vector index * Update lib/service/service.go Co-authored-by: Jakub Nyckowski <jakub.nyckowski@goteleport.com> * address feedback pt.1 * address feedback pt.2: store protobuf message in backend * address feedback pt.3: have GetEmbeddings return a stream * Update lib/services/embeddings.go Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com> * address feedback pt.4: extract embedding logic out of Embeddings service * fixup! address feedback pt.4: extract embedding logic out of Embeddings service * address feedback pt.5: simpler error handling when embedding fails * fix tests pt.1 * fix tests pt.2 * fix tests pt.3 * [Assist] Replace embedding watcher (#27953) Change the way how the embeddings are calculated. Instead of creating a watcher in Auth, we will process all nodes every hour and process embeddings if any embeddings are missing or any node has been updated. --------- Co-authored-by: Jakub Nyckowski <jakub.nyckowski@goteleport.com> Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com> * Restore `lib/ai` tests (#28077) * Restore `lib/ai` tests The tests were removed as a part of #27075. This PR updates the tests to use the new logic. * Fix tests * Restore lib/web tests * GCI * Move test handler to a common place * Fix used token test * Add comment * Remove duplicate imports (#27886) * [Assist] Remove the empty assist message (#28125) * [Assist] Remove the empty assist message Assist shows an empty message at the beginning of each conversation when reading it from DB. This PR fixes that behavior and adds a test to prevent this from happening in the future. * Address code review comments * Address code review comments * Skip embedding processor on Cloud Non-Team plan (#28197) * ai: compute opportinistic summary of command execution (#28033) * ai: compute opportinistic summary of command execution * ai: add streaming summary back after rebase on new front-end * Lint and fix tests pt.1 * reference nodes by name and add tests * Lint, fix tests and address feedback * Attempt to tame the stream close monster * fixup! Attempt to tame the stream close monster * [Assist] Do not close the WS after command execution (#28246) * Revert "fixup! Attempt to tame the stream close monster" This reverts commit 8537aa2. * Revert "Attempt to tame the stream close monster" This reverts commit e0c861d. * Do not close the WS after command execution * Fix tests and lint * fixup! Fix tests and lint * undo put web test command into constant --------- Co-authored-by: Jakub Nyckowski <jakub.nyckowski@goteleport.com> * [Assist] Include embeddings in the prompt (#28116) * [Assist] Include embeddings in the prompt * Add comments GCI Minor fixes * Move stuff * Fix tests * Fix tests * Fixes after rebase Apply code review suggestions. * Address review comments * After rebase fix * Improve error handling and embedding prompts; fix typos (#28403) * "Improve error handling and embedding prompts; fix typos" This commit encompasses several changes. First, an error handling routine has been added in AssistContext.tsx to properly close a WebSocket connection and finish all results. The intent is to ensure that execution fails gracefully when a session doesn't end normally. In tool.go, user instructions have been made more explicit to ensure users check access to nodes before generating any commands. It warns them that not checking access will cause error. Also, some minor typos were corrected in agent.go and messages.go for better readability. * "Refactor 'hosts' to 'nodes' in AI Tool Descriptions" This commit refactors the language from 'host' terminology to 'node' terminology in the AI tool's generated responses as the LLM seems to be confused when generating queries with embeddings. * Update expected test values in chat_test.go The expected values in three different tests in chat_test.go have been updated. This change was required because the underlying algorithm has been adjusted and these modifications will keep the tests aligned with the current algorithm's behavior. * Add missing imports * Introduce user preferences (#28291) * Add user preferences feature * Add missing license header * Fix the order of arguments to require.Equal * Update lib/web/userpreferences.go Co-authored-by: Michelle Bergquist <11967646+michellescripts@users.noreply.github.com> * Add a `GetUserPreferencesResponse` message * Remove unused logger * Use .Put instead of .Create/.Update * Add missing godoc * trace.Wrap the happy path --------- Co-authored-by: Michelle Bergquist <11967646+michellescripts@users.noreply.github.com> * Shut down embedding processor on graceful exit (#28356) * Refactor websocket termination and stream handling (#28452) * Refactor websocket termination and stream handling Refactored websocket stream shutdown and error handling. Replaced `Close()` with `SendCloseMessage()` for better control over the websocket connection termination process. Added checks for the validity of channels to prevent reading from closed channels. The commit also includes minor typo fixes. * Remove unused completedC * Remove unnecessary select blocks in terminal.go The select blocks used in terminal.go for reading data from channels were unnecessary as we were just pulling from a single channel. Removed the select block and directly attempted to read from the channel. These changes increase code readability and integrity by removing unnecessary select blocks. In the command_test.go, an explanatory comment was added for clarity. * Remove commented code * Replace trace.NewAggregate with trace.Wrap as aggregation is not needed. * Add the UI for Assist's settings (#28413) * Add the UI for Assist's settings * Add typing * Fix test by wrapping render in LayoutContextProvider * Run prettier * Assist: fix summary logic (#28487) * Update command.go * simplify export signature * assist: add classification code (#28221) * [Assist] Provide interactive updates during agent execution (#27893) * send progress update messages during agent thoughts * handle new output format * define json tags for serialized fields * use streaming api * fan streaming from model loop * fix streaming * stream progress updates * Update lib/assist/assist.go Co-authored-by: Jakub Nyckowski <jakub.nyckowski@goteleport.com> * remove useless mute * nits * Update lib/ai/model/agent.go Co-authored-by: Jakub Nyckowski <jakub.nyckowski@goteleport.com> * fix merge * fix misc * more misc fixes * what * what2 * weird eof errors? * Fix tests UI integration * Fix other tests * Linter fixes * Comment out token counting for assist streams to avoid race condition. * Fix more tests --------- Co-authored-by: Jakub Nyckowski <jakub.nyckowski@goteleport.com> * Remove console.log in AssistContext (#28607) --------- Co-authored-by: Joel <jwejdenstal@goteleport.com> Co-authored-by: Ryan Clark <ryan.clark@goteleport.com> Co-authored-by: Hugo Shaka <hugo.hervieux@goteleport.com> Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> Co-authored-by: Edoardo Spadolini <edoardo.spadolini@goteleport.com> Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com> Co-authored-by: Justinas Stankevičius <justinas@users.noreply.github.com> Co-authored-by: Michelle Bergquist <11967646+michellescripts@users.noreply.github.com>

hugoShaka force-pushed the hugo/ai-command-output branch from 7e78ad6 to 39362b5 Compare June 19, 2023 18:12

hugoShaka requested review from jakule, ryanclark and xacrimon June 20, 2023 21:59

jakule reviewed Jun 20, 2023

View reviewed changes

hugoShaka force-pushed the hugo/ai-command-output branch from f9270ec to 5866b0d Compare June 21, 2023 15:31

xacrimon reviewed Jun 21, 2023

View reviewed changes

hugoShaka force-pushed the hugo/ai-command-output branch from 5866b0d to 25a9c00 Compare June 21, 2023 15:45

ryanclark approved these changes Jun 21, 2023

View reviewed changes

hugoShaka marked this pull request as ready for review June 21, 2023 18:14

github-actions Bot added size/lg ui labels Jun 21, 2023

github-actions Bot requested review from avatus and ravicious June 21, 2023 18:14

hugoShaka requested review from jakule and xacrimon June 21, 2023 18:14

hugoShaka force-pushed the hugo/ai-command-output branch from 9f46e62 to e43bcf5 Compare June 22, 2023 13:28

jakule reviewed Jun 22, 2023

View reviewed changes

hugoShaka added 5 commits June 22, 2023 15:48

ai: compute opportinistic summary of command execution

c0a16a4

ai: add streaming summary back after rebase on new front-end

b5f75ca

Lint and fix tests pt.1

a2ed33c

reference nodes by name and add tests

e68c9e0

Lint, fix tests and address feedback

9b7e89c

hugoShaka force-pushed the hugo/ai-command-output branch from 7faf03c to 9b7e89c Compare June 22, 2023 20:16

hugoShaka added 2 commits June 22, 2023 17:29

Attempt to tame the stream close monster

e0c861d

fixup! Attempt to tame the stream close monster

8537aa2

jakule suggested changes Jun 23, 2023

View reviewed changes

[Assist] Do not close the WS after command execution (#28246)

99f1c38

* Revert "fixup! Attempt to tame the stream close monster" This reverts commit 8537aa2. * Revert "Attempt to tame the stream close monster" This reverts commit e0c861d. * Do not close the WS after command execution

hugoShaka requested a review from jakule June 26, 2023 22:40

jakule approved these changes Jun 27, 2023

View reviewed changes

public-teleport-github-review-bot Bot removed request for avatus and ravicious June 27, 2023 00:39

xacrimon approved these changes Jun 27, 2023

View reviewed changes

hugoShaka added 3 commits June 27, 2023 10:43

Fix tests and lint

1c7bb82

fixup! Fix tests and lint

c81b14d

undo put web test command into constant

5f26ba5

hugoShaka added this pull request to the merge queue Jun 27, 2023

Merged via the queue into master with commit d386c42 Jun 27, 2023

hugoShaka deleted the hugo/ai-command-output branch June 27, 2023 16:32

Conversation

hugoShaka commented Jun 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hugoShaka commented Jun 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jakule Jun 20, 2023

Choose a reason for hiding this comment

Uh oh!

hugoShaka Jun 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakule Jun 27, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xacrimon Jun 21, 2023

Choose a reason for hiding this comment

Uh oh!

hugoShaka Jun 21, 2023

Choose a reason for hiding this comment

Uh oh!

ryanclark left a comment

Choose a reason for hiding this comment

Uh oh!

jakule left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jakule Jun 22, 2023

Choose a reason for hiding this comment

Uh oh!

hugoShaka Jun 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jakule left a comment

Choose a reason for hiding this comment

Uh oh!

jakule left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hugoShaka commented Jun 19, 2023 •

edited

Loading

hugoShaka commented Jun 20, 2023 •

edited

Loading

hugoShaka Jun 21, 2023 •

edited

Loading

hugoShaka Jun 22, 2023 •

edited

Loading