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

Fixes associated with feedback endpoint #593

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

eyurtsev
Copy link
Collaborator

@eyurtsev eyurtsev commented Apr 3, 2024

  • Fixes langsmith client not being enabled if token feedback was specified
  • Add unit test for token feedback endpoint
  • Fix schema for token feedback to accept a str in addition to a UUID
  • Add metadata even to astream log with token feedback information

@cla-bot cla-bot bot added the cla-signed label Apr 3, 2024
@eyurtsev eyurtsev requested a review from bracesproul April 3, 2024 20:06
@eyurtsev eyurtsev merged commit 8439937 into main Apr 3, 2024
10 of 11 checks passed
@eyurtsev eyurtsev deleted the eugene/update_tokenfeedbcak branch April 3, 2024 20:11
@@ -1229,6 +1243,18 @@ async def _stream_log() -> AsyncIterator[dict]:
"data": self._serializer.dumps(data).decode("utf-8"),
"event": "data",
}

# Send a metadata event as soon as possible
if not has_sent_metadata and self._enable_feedback_endpoint:
Copy link

Choose a reason for hiding this comment

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

If _token_feedback_enabled is False but _enable_feedback_endpoint is True, task will be None, causing AssertionError("Feedback token task was not created.") always raise. Do you mean to check
_token_feedback_enabled here?
Can be reproduced by using the default example in

langserve/README.md

Lines 399 to 406 in 109445b

add_routes(
app,
chain.with_types(input_type=InputChat),
enable_feedback_endpoint=True,
enable_public_trace_link_endpoint=True,
playground_type="chat",
)
```

Copy link

Choose a reason for hiding this comment

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

Fixed at #620

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

Successfully merging this pull request may close these issues.

2 participants