Skip to content

[8.15] [Elastic Assistant] Update default assistant graph (#190686)#191386

Merged
P1llus merged 3 commits intoelastic:8.15from
P1llus:backport/8.15/pr-190686
Aug 27, 2024
Merged

[8.15] [Elastic Assistant] Update default assistant graph (#190686)#191386
P1llus merged 3 commits intoelastic:8.15from
P1llus:backport/8.15/pr-190686

Conversation

@P1llus
Copy link
Member

@P1llus P1llus commented Aug 27, 2024

Backport

This will backport the following commits from main to 8.15:

Questions ?

Please refer to the Backport tool documentation

## Summary

**NOTE** I will need help testing this before we merge it!

I spoke with @spong about an upcoming PR we have here:
elastic#190426 which bumps the langgraph
version from 0.0.31 to 0.0.34, unfortunately this caused a lot of type
errors in the default assistant.

After some more discussion we proposed to open a PR that removes some of
the more complex layers and to fix up the type issues. Though I have not
worked on this graph before, the changes hopefully makes sense 👍

Graph flow:

![image](https://github.com/user-attachments/assets/911190c1-2cdc-429f-bd1b-2b4a6a343729)

The PR changes the below items to remove some of the abstractions and
resolve some of the type issues, also adds a few improvements in
general:

- Moves `llmType`, `bedrockChatEnabled`, `isStream` and `conversationId`
to be invoke parameters rather than compile parameters. This allows them
to be used in state, and removes the need to pass them everywhere as
parameters. Adding them to the state also allows them to be available in
langsmith.
- Removes the constants defining each node with wrappers and rather
expose them directly as async functions. This removes a lot of the
boilerplate code and it makes reading the stacktraces much easier.
- Moved to a single `stepRouter` used for the current conditional edges.
This allows one to very easily extend the routing between either
existing or new nodes, and makes it much easier to understand what
conditions are routed where.
- Exports a common `NodeType` object constant (no need for the extra
compile overhead of Enums here, we are only using strings), to make the
node name strings auto-complete and prevent hardcoded names for the
router.
- Added a `modelInput` node to be the starter node. This was first
because adding nodes inside if conditions usually create errors, so it
was created to be able to set the `hasRespondStep` state. However this
node is nice to have as an entrypoint in which you find yourself wanting
to change the state based on the invoke parameters or other conditions
retrieved from other parts of the stack etc before it continues to any
of the other nodes.
- Added a `yarn draw-graph` command, that outputs to
`docs/img/default_assistant_graph.png`. This is then also included in
the readme. This makes it better for changes by other teams (like me) to
understand the intended graph workflows easier.

### Checklist

Delete any items that are not applicable to this PR.

- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
(cherry picked from commit b660d42)

# Conflicts:
#	x-pack/plugins/elastic_assistant/server/lib/langchain/graphs/default_assistant_graph/nodes/execute_tools.ts
#	x-pack/plugins/elastic_assistant/server/lib/langchain/graphs/default_assistant_graph/nodes/generate_chat_title.ts
#	x-pack/plugins/elastic_assistant/server/lib/langchain/graphs/default_assistant_graph/nodes/run_agent.ts
#	x-pack/plugins/elastic_assistant/server/lib/langchain/graphs/default_assistant_graph/nodes/should_continue.ts
@P1llus P1llus added the backport This PR is a backport of another PR label Aug 27, 2024
@P1llus P1llus enabled auto-merge (squash) August 27, 2024 08:13
@P1llus P1llus requested a review from spong August 27, 2024 08:16
@P1llus P1llus disabled auto-merge August 27, 2024 08:16
@kibana-ci
Copy link

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@P1llus P1llus merged commit ee067b6 into elastic:8.15 Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This PR is a backport of another PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants