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

Fix agent stream callback #614

Merged
merged 6 commits into from
Mar 6, 2024
Merged

Conversation

devinyf
Copy link
Contributor

@devinyf devinyf commented Feb 18, 2024

I did a little change in this PR to handled 2 problems when using AgentFinalStreamHandler:

I followed this example to study using agent
https://github.com/tmc/langchaingo/blob/main/examples/mrkl-agent-example/mrkl_agent.go
and use a handler to get streaming result like this:

	callbackHandler := callbacks.NewFinalStreamHandler()
	// callbackHandler.PrintOutput = true

	executor, err := agents.Initialize(
		llm,
		agentTools,
		agents.ZeroShotReactDescription,
		agents.WithMaxIterations(5),
		agents.WithCallbacksHandler(callbackHandler),
	)

problem1. Detect keyword failed:

reason: cut out slice handler.LastTokens before doing detect keyword will cause Detection can’t success

for example:
before the handler.LastTokens string value is

"result Final Answer: As of February 18,"

after the reassign the slice value, it's become

"Answer: As of February 18,"

Then it can't pass the keywords check process which should contained Final Answer: keyword in LastTokens.

So I changed code order, check keywords first, then reassign slice

problem2. Final Answer sometimes contain colon symbol : at the beginning

Add few code to filter the colon if it exist

May this be a positive contribution.

PR Checklist

  • Read the Contributing documentation.
  • Read the Code of conduct documentation.
  • Name your Pull Request title clearly, concisely, and prefixed with the name of the primarily affected package you changed according to Good commit messages (such as memory: add interfaces for X, Y or util: add whizzbang helpers).
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. Fixes #123).
  • Describes the source of new concepts.
  • References existing implementations as appropriate.
  • Contains test coverage for new functions.
  • Passes all golangci-lint checks.

Copy link
Owner

@tmc tmc left a comment

Choose a reason for hiding this comment

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

can you flesh out some more expected cases and use a table driven test in the test method?

@devinyf
Copy link
Contributor Author

devinyf commented Feb 19, 2024

I've updated test case in this PR and #612 , Both using table driven

extraStrBefore := fmt.Sprintf(" another text. Final Answer: %s", correctStr)
extraColonWithSpacesBefore := fmt.Sprintf(` : %s`, correctStr)

testCases := []filterFinalStringTestCase{
Copy link
Owner

@tmc tmc Feb 19, 2024

Choose a reason for hiding this comment

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

please place this at the top of the test (after t.Parallel()) and approach it like:

cases := []struct { ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated all test case inline with:

cases := []struct { ...

please review

Copy link
Owner

@tmc tmc left a comment

Choose a reason for hiding this comment

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

LGTM

@tmc tmc merged commit b74cceb into tmc:main Mar 6, 2024
3 checks passed
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.

2 participants