Skip to content

fix: add stop sequence support to ChatOpenAI custom#6434

Open
nikhil-shukl wants to merge 2 commits into
FlowiseAI:mainfrom
nikhil-shukl:bugfix-chatopenai-custom-stop-sequence
Open

fix: add stop sequence support to ChatOpenAI custom#6434
nikhil-shukl wants to merge 2 commits into
FlowiseAI:mainfrom
nikhil-shukl:bugfix-chatopenai-custom-stop-sequence

Conversation

@nikhil-shukl
Copy link
Copy Markdown

Description

Adds Stop Sequence support to the ChatOpenAI Custom node so users can pass comma-separated stop words such as <|im_end|> from Additional Parameters.

Fixes #2499

Changes

  • Adds a stopSequence additional parameter to ChatOpenAI Custom
  • Converts comma-separated stop sequences into the stop array passed to ChatOpenAI
  • Adds focused tests for the new field and request config behavior

Tests

  • pnpm --filter flowise-components test -- ChatOpenAICustom.test.ts
  • pnpm exec prettier --check packages/components/nodes/chatmodels/ChatOpenAICustom/ChatOpenAICustom.ts packages/components/nodes/chatmodels/ChatOpenAICustom/ChatOpenAICustom.test.ts

Note: local environment used Node v23.5.0, so pnpm printed an engine warning because the repo expects Node ^20, but the focused test passed.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a stopSequence input parameter to the ChatOpenAICustom model, allowing users to define comma-separated stop words, and includes unit tests verifying this behavior. A review comment suggests filtering out empty strings from the parsed stop sequences to prevent potential 400 Bad Request errors from the OpenAI API when handling consecutive or trailing commas.

Comment on lines +160 to +163
if (stopSequence) {
const stopSequenceArray = stopSequence.split(',').map((item) => item.trim())
obj.stop = stopSequenceArray
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If the stopSequence contains empty segments (e.g., trailing commas or multiple consecutive commas like foo,,bar), splitting and mapping will result in empty strings in the stop array. The OpenAI API will reject requests containing empty strings in the stop parameter with a 400 Bad Request error. Filtering out empty strings using simple, chained operations ensures robustness and maintains readability.

Suggested change
if (stopSequence) {
const stopSequenceArray = stopSequence.split(',').map((item) => item.trim())
obj.stop = stopSequenceArray
}
if (stopSequence) {
const stopSequenceArray = stopSequence
.split(',')
.map((item) => item.trim())
.filter((item) => item !== '')
if (stopSequenceArray.length > 0) {
obj.stop = stopSequenceArray
}
}
References
  1. Prioritize code readability and understandability over conciseness. A series of simple, chained operations can be preferable to a single, more complex one if it improves understandability and reduces the potential for future errors.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the review. I’ve addressed this in 0c594c8 by filtering empty stop sequence entries and added a regression test for consecutive/trailing commas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Additional Parameters did not take effect.

1 participant