Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core[minor]: Add graph functionality to Runnable base #4566

Merged
merged 11 commits into from
Mar 12, 2024

Conversation

albertpurnama
Copy link
Contributor

@albertpurnama albertpurnama commented Feb 28, 2024

Description

  • Adds Graph class port over from Python's Graph implementation here
  • Adds getGraph to base Runnable to allow constructing graph representation of the Runnable. This should be exact port of this function in python

Failing test case

(Use `node --trace-warnings ...` to show where the warning was created)
@langchain/core:test: FAIL src/runnables/tests/runnable_graph.test.ts
@langchain/core:test:   ● Test graph sequence
@langchain/core:test: 
@langchain/core:test:     expect(received).toBe(expected) // Object.is equality
@langchain/core:test: 
@langchain/core:test:     - Expected  - 18
@langchain/core:test:     + Received  +  2
@langchain/core:test: 
@langchain/core:test:     @@ -17,20 +17,11 @@
@langchain/core:test:             "target": 3,
@langchain/core:test:           },
@langchain/core:test:         ],
@langchain/core:test:         "nodes": Array [
@langchain/core:test:           Object {
@langchain/core:test:     -       "data": Object {
@langchain/core:test:     -         "properties": Object {
@langchain/core:test:     -           "name": Object {
@langchain/core:test:     -             "title": "Name",
@langchain/core:test:     -             "type": "string",
@langchain/core:test:     -           },
@langchain/core:test:     -         },
@langchain/core:test:     -         "title": "PromptInput",
@langchain/core:test:     -         "type": "object",
@langchain/core:test:     -       },
@langchain/core:test:     +       "data": Object {},
@langchain/core:test:             "id": 0,
@langchain/core:test:             "type": "schema",
@langchain/core:test:           },
@langchain/core:test:           Object {
@langchain/core:test:             "data": Object {
@langchain/core:test:     @@ -49,11 +40,10 @@
@langchain/core:test:             "data": Object {
@langchain/core:test:               "id": Array [
@langchain/core:test:                 "langchain",
@langchain/core:test:                 "llms",
@langchain/core:test:                 "fake",
@langchain/core:test:     -           "llm",
@langchain/core:test:                 "FakeLLM",
@langchain/core:test:               ],
@langchain/core:test:               "name": "FakeLLM",
@langchain/core:test:             },
@langchain/core:test:             "id": 2,
@langchain/core:test:     @@ -71,17 +61,11 @@
@langchain/core:test:             },
@langchain/core:test:             "id": 3,
@langchain/core:test:             "type": "runnable",
@langchain/core:test:           },
@langchain/core:test:           Object {
@langchain/core:test:     -       "data": Object {
@langchain/core:test:     -         "items": Object {
@langchain/core:test:     -           "type": "string",
@langchain/core:test:     -         },
@langchain/core:test:     -         "title": "CommaSeparatedListOutputParserOutput",
@langchain/core:test:     -         "type": "array",
@langchain/core:test:     -       },
@langchain/core:test:     +       "data": Object {},
@langchain/core:test:             "id": 4,
@langchain/core:test:             "type": "schema",
@langchain/core:test:           },
@langchain/core:test:         ],
@langchain/core:test:       }
@langchain/core:test: 
@langchain/core:test:       33 |   // expect(Object.keys(graph.nodes).length).toBe(5);
@langchain/core:test:       34 |
@langchain/core:test:     > 35 |   expect(graph.toJson()).toBe({
@langchain/core:test:          |                          ^
@langchain/core:test:       36 |     nodes: [
@langchain/core:test:       37 |       {
@langchain/core:test:       38 |         id: 0,
@langchain/core:test: 
@langchain/core:test:       at Object.toBe (src/runnables/tests/runnable_graph.test.ts:35:26)
@langchain/core:test: 

This is because the implementation of base Runnable is still TBD. Specifically here (same for the last node)

The solution to this requires the base Runnable to be fully implemented

Fixes #4582

Copy link

vercel bot commented Feb 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
langchainjs-api-refs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 12, 2024 11:14pm
langchainjs-docs ✅ Ready (Inspect) Visit Preview Mar 12, 2024 11:14pm

Comment on lines 557 to 561
// TODO: what should the inputNode be?
const inputNode = graph.addNode({
type: "unknown",
config,
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jacoblee93 Here's where I'm a little bit lost,

I'm trying to find a way to put the schema of the input node of this runnable. I haven't been able to find it yet.

This should be similar to get_input_schema in Python's core lib.

I think for graphing purposes, we might be able to replace this with a placeholder and maybe implement this later when we're going to use Zod.

Comment on lines 565 to 569
// TODO: what should the outputNode be?
const outputNode = graph.addNode({
type: "unknown",
config,
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same case as the above.

@jacoblee93 jacoblee93 changed the title Add graph functionality to Runnable base core[minor]: Add graph functionality to Runnable base Feb 29, 2024

interface Node {
id: string;
data: any;
Copy link
Collaborator

Choose a reason for hiding this comment

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

we prob want to type this

return node.id;
} else {
// Assuming `node.data` has a similar structure to the Python version
let data = node.data.toString();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we want to use node.data.getName() here

@jacoblee93
Copy link
Collaborator

Hey sorry about the delay - will spend some time with this tomorrow and hopefully ship it!

@@ -550,6 +551,22 @@ export abstract class Runnable<
);
}

getGraph(_?: RunnableConfig): Graph {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't be any reason to preemptively make this async right?

@jacoblee93
Copy link
Collaborator

I will land @albertpurnama, thank you for this!

@jacoblee93 jacoblee93 marked this pull request as ready for review March 12, 2024 22:48
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. auto:improvement Medium size change to existing code to handle new use-cases labels Mar 12, 2024
@jacoblee93
Copy link
Collaborator

Thank you very much for your patience and the great work @albertpurnama!

@jacoblee93 jacoblee93 merged commit 7110065 into langchain-ai:main Mar 12, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto:improvement Medium size change to existing code to handle new use-cases size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port get_graph from Python
3 participants