Skip to content

Conversation

reatang
Copy link
Contributor

@reatang reatang commented Jun 12, 2025

Although the ag-ui output library has been added, I think this protocol is not a mature protocol and even lacks many functions.

Completed issue:#227

Copy link

codecov bot commented Jun 13, 2025

Codecov Report

❌ Patch coverage is 79.33884% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.96%. Comparing base (ae82e7e) to head (7281133).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/service/service.py 74.39% 21 Missing ⚠️
src/core/settings.py 40.00% 3 Missing ⚠️
src/core/llm.py 50.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
src/schema/models.py 100.00% <100.00%> (ø)
src/schema/schema.py 86.20% <100.00%> (+0.24%) ⬆️
src/service/utils.py 83.33% <100.00%> (+10.83%) ⬆️
src/core/llm.py 75.00% <50.00%> (-0.87%) ⬇️
src/core/settings.py 76.02% <40.00%> (-1.28%) ⬇️
src/service/service.py 77.41% <74.39%> (-1.48%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JoshuaC215
Copy link
Owner

Looks good besides the comments. The protocol seems a little immature and I'm not sure about the long term maintenance (seems like maybe a startup created it?). But, it's not a huge addition after refactors so I don't think it's a high cost to have it in this project, and several people upvoted in the issue.

@reatang
Copy link
Contributor Author

reatang commented Jul 9, 2025

Looks good besides the comments. The protocol seems a little immature and I'm not sure about the long term maintenance (seems like maybe a startup created it?). But, it's not a huge addition after refactors so I don't think it's a high cost to have it in this project, and several people upvoted in the issue.

This protocol is a protocol layer hatched from the CopilotKit Agent front-end library. In theory, it can be directly used in CopilotKit, but I have not studied the CopilotKit front-end library in depth.

jump to:https://github.com/CopilotKit/CopilotKit

@reatang
Copy link
Contributor Author

reatang commented Jul 10, 2025

I have provided a single page to run a simple example, but this front-end page will not be a production-level feature.

Comment on lines +1274 to +1288
const response = await fetch('/chatbot/stream', {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'Accept': 'text/event-stream',
},
body: JSON.stringify({
message: userMessage,
stream_protocol: 'agui',
thread_id: threadId,
user_id: generateUUID(),
model: selectedModel,
stream_tokens: true
})
});
Copy link
Owner

Choose a reason for hiding this comment

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

So a few concerns about this and the handleAgUIEvent method below:

  • Seems to just re-implement the client part of the ag-ui protocol from scratch
  • Doesn't support tool messages which are important
  • Chatbot is hard coded

To prove it works as expected, seems like it would be much better to use the provided agui typescript sdk, such as with the HttpAgent and AgentSubscriber classes to manage the handling.

I think it might mean the shape of the server endpoint needs to change, but that's probably desirable if we're trying to make something that's actually compatible with the protocol? Otherwise, I'm not sure this PR makes sense if it only implements the protocol halfway.

An alternate approach to this html page - I saw one of the provided ag-ui examples just writes a CLI in typescript. If it's easier to just provide a CLI that uses the typescript sdk instead of a full website, that seems sufficient to me.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The official one does provide one typescript-sdk


@app.get("/agui/")
async def agui_root():
with open("web-agui/index.html", encoding="utf-8") as file:
Copy link
Owner

Choose a reason for hiding this comment

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

Just noting that this only works currently if the service is run from the root of the repo, and does not work if the service is in docker. If we move to a CLI or a website that just be launched directly from browser (no server interaction) we could just remove this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is indeed a difficult decision. If you want the agent to be a pure API service, then the web-agui function should also be separated into a rendering service like streamlit_app.

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