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

MRKL output parser no longer breaks well formed queries #5432

Conversation

ninjapenguin
Copy link
Contributor

Handles the edge scenario in which the action input is a well formed SQL query which ends with a quoted column

There may be a cleaner option here (or indeed other edge scenarios) but this seems to robustly determine if the action input is likely to be a well formed SQL query in which we don't want to arbitrarily trim off " characters

Fixes #5423

Who can review?

Community members can review the PR once tests pass. Tag maintainers/contributors who might be interested:

For a quicker response, figure out the right person to tag with @

@hwchase17 - project lead

Agents / Tools / Toolkits

@eyurtsev
Copy link
Collaborator

@ninjapenguin this looks good to me. If you have time to add a unit test will be appreciated. If not, we'll merge as is.

@eyurtsev eyurtsev added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label May 30, 2023
@eyurtsev eyurtsev self-requested a review May 30, 2023 13:39
@ninjapenguin
Copy link
Contributor Author

@eyurtsev great shout, have added a covering test that validates change now 🙇

@eyurtsev
Copy link
Collaborator

Thank you @ninjapenguin

@eyurtsev eyurtsev merged commit 1d861dc into langchain-ai:master May 30, 2023
vowelparrot pushed a commit that referenced this pull request May 31, 2023
# Handles the edge scenario in which the action input is a well formed
SQL query which ends with a quoted column

There may be a cleaner option here (or indeed other edge scenarios) but
this seems to robustly determine if the action input is likely to be a
well formed SQL query in which we don't want to arbitrarily trim off `"`
characters

Fixes #5423

## Who can review?

Community members can review the PR once tests pass. Tag
maintainers/contributors who might be interested:

For a quicker response, figure out the right person to tag with @

  @hwchase17 - project lead

  Agents / Tools / Toolkits
  - @vowelparrot
@danielchalef danielchalef mentioned this pull request Jun 5, 2023
Undertone0809 pushed a commit to Undertone0809/langchain that referenced this pull request Jun 19, 2023
…#5432)

# Handles the edge scenario in which the action input is a well formed
SQL query which ends with a quoted column

There may be a cleaner option here (or indeed other edge scenarios) but
this seems to robustly determine if the action input is likely to be a
well formed SQL query in which we don't want to arbitrarily trim off `"`
characters

Fixes langchain-ai#5423

## Who can review?

Community members can review the PR once tests pass. Tag
maintainers/contributors who might be interested:

For a quicker response, figure out the right person to tag with @

  @hwchase17 - project lead

  Agents / Tools / Toolkits
  - @vowelparrot
This was referenced Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm PR looks good. Use to confirm that a PR is ready for merging.
Projects
None yet
2 participants