Repository navigation
馃Л fix: Normalize GraphInterrupt Langfuse Traces - #348
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
馃挕 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8d8f7606a
鈩癸笍 About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 馃憤.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| description: 'suspends to collect a human answer', | ||
| schema: z.object({}).passthrough(), | ||
| } | ||
| ) as unknown as StructuredToolInterface; |
There was a problem hiding this comment.
Replace this as unknown as StructuredToolInterface cast with a correctly typed tool or adapter. The double assertion suppresses structural checking in the real HITL integration test, so changes that make the generated tool incompatible with ToolNode can be accepted rather than diagnosed at the construction site; the repository guidance explicitly prohibits this assertion pattern.
AGENTS.md reference: AGENTS.md:L49-L49
Useful? React with 馃憤聽/ 馃憥.
There was a problem hiding this comment.
Fixed in 1e3a10a. The generated structured tool now flows directly into ToolNode without the prohibited double assertion, so TypeScript retains structural compatibility checking. Verified with 21/21 focused tests, tsc, ESLint, Prettier, and the import-sort check.
| @@ -1,5 +1,5 @@ | |||
| import { CallbackHandler } from '@langfuse/langchain'; | |||
| import { isParentCommand } from '@langchain/langgraph'; | |||
| import { isGraphInterrupt, isParentCommand } from '@langchain/langgraph'; | |||
There was a problem hiding this comment.
Running node scripts/sort-imports.ts --check src/langfuse.ts src/specs/langfuse-callbacks.test.ts reports that both changed files need sorting, so the repository's import-order check fails for this commit. Reorder the package and type imports using the project's sorter before merging.
AGENTS.md reference: AGENTS.md:L63-L69
Useful? React with 馃憤聽/ 馃憥.
There was a problem hiding this comment.
Fixed in 1e3a10a. Ran the repository sorter on both changed files and pinned it with node scripts/sort-imports.ts --check src/langfuse.ts src/specs/langfuse-callbacks.test.ts; the focused test, tsc, ESLint, Prettier, and diff checks also pass.
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: 鈩癸笍 About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 馃憤. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
I normalized expected LangGraph human-in-the-loop interruptions in Langfuse so
ask_user_questionpauses no longer inflate production error telemetry.GraphInterruptandNodeInterruptchain callbacks as successful control-flow endings.handleToolEndwith a sanitized{"controlFlow":"GraphInterrupt"}marker.StateGraph+MemorySaverpause/resume regression covering callback ancestry, checkpointed interrupt payloads, resumed tool output, sanitized telemetry, and balanced spans.NodeInterrupt, null errors, and genuine failures.Change Type
Testing
I exercised the real
ask_user_questionpause/resume path and verified the callback classification directly.npm test -- --runTestsByPath src/specs/langfuse-callbacks.test.ts --runInBandnpx eslint src/langfuse.ts src/specs/langfuse-callbacks.test.tsnpx prettier --check src/langfuse.ts src/specs/langfuse-callbacks.test.tsnpx tsc --noEmitnpm run buildgit diff --checkTest Configuration
@langchain/langgraph1.4.6@librechat/agents3.3.4 development checkoutChecklist