Conversation
BenchmarksBenchmark execution time: 2026-03-09 16:33:09 Comparing candidate commit 90db32d in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 228 metrics, 32 unstable metrics. |
crysmags
force-pushed
the
crysmags/langgraph-w/-orch-update
branch
from
60ec132 to
f4dc3d3
Compare
Overall package sizeSelf size: 4.97 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.0.0 | 81.15 kB | 815.98 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
This comment has been minimized.
This comment has been minimized.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7567 +/- ##
==========================================
+ Coverage 80.32% 80.34% +0.02%
==========================================
Files 739 746 +7
Lines 31960 32269 +309
==========================================
+ Hits 25671 25928 +257
- Misses 6289 6341 +52
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
These changes were made by the llm obs workflow to improve test robustness but are not needed for the LangGraph integration itself. The changes have been saved to /tmp/util-js-llmobs-improvements.patch for future reference and can be applied later if needed. Changes reverted: - Mock value handling for timing fields (span_id, duration, start_ns) - Better error handling in test assertions - Input validation improvements - Edge case handling for outputValue === 0
- Add trailing commas to all arrays and objects per @stylistic/comma-dangle - Break long line in llmobs plugin to comply with max-len rule - Remove unused 'llmobs' variable in test spec All langgraph-related linting errors have been fixed.
crysmags
force-pushed
the
crysmags/langgraph-w/-orch-update
branch
from
3961fdf to
9ac81a6
Compare
Add langgraph and its dependencies (@langchain/core, zod, zod-to-json-schema) to externals.js. This was missing, causing dependency installation to fail. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add zod-to-json-schema@3.23.1 - Update @langchain/core from 1.1.15 to 1.1.16 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
sabrenner
left a comment
sabrenner
left a comment
There was a problem hiding this comment.
really only one actual question regarding usage of asyncEnd that does seem to work in tests and is probably fine but I just want to check locally first. but otherwise looks good to go % some comments on things i think either a bad rebase and/or the toolkit added/changed
docs/add-redirects.sh
Outdated
There was a problem hiding this comment.
looks like the addition of this file might be unrelated?
There was a problem hiding this comment.
Yeah, I deleted this file recently, it must've been added accidentally during a rebase. The file needs to be deleted in this PR.
There was a problem hiding this comment.
This file, keeps on reappearing, yes will delete
| this._tagger.tagTextIO(span, input, output) | ||
| } | ||
|
|
||
| formatIO (data) { |
There was a problem hiding this comment.
nit for a follow-up: we can probably move this to a util file/share it with the general langchain integration
| static id = 'llmobs_langgraph_pregel_stream' | ||
| static prefix = 'tracing:orchestrion:@langchain/langgraph:Pregel_stream' | ||
|
|
||
| asyncEnd (ctx) { |
There was a problem hiding this comment.
i gotta try this out locally, and i know tests are passing but it seems like this could be overwriting the asyncEnd from the LlmObsPlugin of the BaseLangGraphLLMObsPlugin above? although since tests pass this is a bit confusing - just leaving this as a note for myself.
| static id = 'llmobs_langgraph_pregel_stream' | ||
| static prefix = 'tracing:orchestrion:@langchain/langgraph:Pregel_stream' | ||
|
|
||
| asyncEnd (ctx) { |
There was a problem hiding this comment.
i gotta try this out locally, and i know tests are passing but it seems like this could be overwriting the asyncEnd from the LlmObsPlugin of the BaseLangGraphLLMObsPlugin above? although since tests pass this is a bit confusing - just leaving this as a note for myself.
| } | ||
|
|
||
| error (ctx) { | ||
| this.#tagOnComplete(ctx) |
There was a problem hiding this comment.
is this needed? will we not hit the asyncEnd of the parent BaseLangGraphLLMObsPlugin plugin and tag what we have on the end there?
| inputValue: JSON.stringify({ | ||
| messages: [{ role: 'user', content: 'Stream test' }], | ||
| }), | ||
| outputValue: MOCK_STRING, // Final streamed output |
There was a problem hiding this comment.
since this should be deterministic, let's assert the actual aggregated output
| spanKind: 'workflow', | ||
| name: 'foobarzoo', | ||
| inputValue: JSON.stringify({ count: 0 }), | ||
| outputValue: MOCK_STRING, |
There was a problem hiding this comment.
ditto on asserting real output
There was a problem hiding this comment.
this file can be removed
There was a problem hiding this comment.
looks like there are some unrelated version updates in this file (prisma stuff, langchain core) - if we could only add langgraph, and any zod packages, i think that's best!
docs/add-redirects.sh
Outdated
There was a problem hiding this comment.
Yeah, I deleted this file recently, it must've been added accidentally during a rebase. The file needs to be deleted in this PR.
5 participants
What does this PR do?
Motivation
Additional Notes