-
Notifications
You must be signed in to change notification settings - Fork 6
Add documentation for Events feature (streaming responses via SSE) #32
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
Conversation
Co-authored-by: ddunkin <264744+ddunkin@users.noreply.github.com>
Co-authored-by: ddunkin <264744+ddunkin@users.noreply.github.com>
define/fsd.md
Outdated
| { | ||
| messages: ChatMessage[]; | ||
| status: ChatStatus; | ||
| usage: ChatUsage; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot omit this from the example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed from the example. (9b4e46f)
define/fsd.md
Outdated
| /// Token usage statistics (sent in final chunk) | ||
| usage: UsageInfo; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot omit from example, unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the usage field and UsageInfo data type. (9b4e46f)
define/fsd.md
Outdated
| **Events** return a stream of responses, allowing progressive updates. Each response in the stream is a complete response DTO. Events are ideal for: | ||
|
|
||
| * Long-running operations with progress updates | ||
| * Streaming AI/LLM responses where tokens are generated incrementally | ||
| * Real-time data feeds that push updates to clients | ||
| * Incremental results where partial data is useful before completion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot since these use SSE, what are the performance implications and how would they compare to web sockets or polling?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SSE (Server-Sent Events) provides a good balance for streaming responses:
vs WebSockets:
- SSE is simpler (HTTP-based, one-way server-to-client)
- WebSockets are bidirectional and more complex
- SSE automatically reconnects on disconnection
- SSE works better with standard HTTP infrastructure (proxies, load balancers)
- Use SSE for server-to-client streaming; use WebSockets when you need bidirectional communication
vs Polling:
- SSE maintains an open connection, eliminating polling overhead
- Lower latency - server pushes immediately vs polling intervals
- More efficient - no repeated request/response overhead
- Better for real-time updates
SSE is ideal for the streaming response pattern in Facility where the server pushes data to the client as it becomes available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot summarize in the docs
define/fsd.md
Outdated
| * The server yields multiple response chunks over time | ||
| * Each chunk is a complete response DTO, serialized and sent as an SSE event | ||
| * Clients can process chunks as they arrive, enabling progressive rendering |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot is this "chunking" description too specific to the example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Changed terminology from "chunks" to "responses" throughout and removed "progressive rendering" which was too UI-specific. The description is now more general and applicable to any use case. (9b4e46f)
Co-authored-by: ddunkin <264744+ddunkin@users.noreply.github.com>
ddunkin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot link to relevant SSE specifications.
Remove redundant or verbose new documentation.
define/fsd.md
Outdated
| **Events** return a stream of responses, allowing progressive updates. Each response in the stream is a complete response DTO. Events are ideal for: | ||
|
|
||
| * Long-running operations with progress updates | ||
| * Streaming AI/LLM responses where tokens are generated incrementally | ||
| * Real-time data feeds that push updates to clients | ||
| * Incremental results where partial data is useful before completion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot summarize in the docs
define/fsd.md
Outdated
|
|
||
| When a client invokes an event, it provides request field values just like a method. However, unlike a method that returns a single response, an event returns a stream where each item is a complete response DTO. The stream can yield multiple responses over time until it completes. | ||
|
|
||
| In C#, events return `Task<ServiceResult<IAsyncEnumerable<ServiceResult<TResponse>>>>`, allowing clients to process responses as they arrive using `await foreach`. Other languages provide similar async iterable constructs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot omit implementation specifics in generic docs
|
@copilot try again |
|
@copilot link to relevant SSE specifications (see https://developer.mozilla.org/en-US/docs/Web/API/Server-sent_events) Remove redundant or verbose new documentation. |
Co-authored-by: ddunkin <264744+ddunkin@users.noreply.github.com>
Added link to MDN SSE specification and significantly simplified the documentation by:
The Events section is now much more concise while still covering the essential information. (f672e8b) |
define/fsd.md
Outdated
|
|
||
| The `http` attribute works the same for events as for methods. Events support the `method` and `path` parameters. | ||
|
|
||
| The `method` parameter indicates the HTTP method to use, typically `POST` (the default). The `path` parameter specifies the HTTP path, defaulting to the event name with a leading slash (e.g., `/chatStream`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot POST is the default?
EventSource doesn't support POST, only GET? The specification isn't explicit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're correct - the standard EventSource API only supports GET requests. Events should default to GET, not POST, since they use SSE. I'll update the documentation to reflect this difference from methods.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot that doesn't mean events should default to GET necessarily, but it does mean we can't use EventSource in that case, right? Can we use fetch directly in the browser for these requests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct! Updated the documentation to clarify:
- Events default to POST (same as methods)
- EventSource only supports GET, so for POST or other methods, clients need custom SSE implementations (e.g., fetch with SSE parsing)
This gives the flexibility to use any HTTP method while documenting the EventSource limitation. (73fb843)
Co-authored-by: ddunkin <264744+ddunkin@users.noreply.github.com>
Co-authored-by: ddunkin <264744+ddunkin@users.noreply.github.com>
|
@copilot check FacilityApi/FacilityJavaScript#75 and confirm the documentation in this PR is correct |
I've reviewed the current documentation against the implementation patterns. The documentation states: Current documentation:
Key points to verify from PR #75:
Without direct access to PR #75, I cannot fully confirm all implementation details. Could you point me to specific aspects of the implementation I should verify or clarify what seems incorrect? Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot try again FacilityApi/FacilityJavaScript#75 and confirm the documentation in this PR is correct |
Co-authored-by: ddunkin <264744+ddunkin@users.noreply.github.com>
Verified against PR #75. Updated documentation to match the actual implementation: Key corrections:
The documentation now accurately reflects the implementation in PR #75. (3a25b50) |
Removed unnecessary blank lines before the SSE explanation.
Plan: Add Documentation for Events Feature
This PR adds comprehensive documentation for the Events feature that enables server-sent events (SSE) style streaming responses in Facility.
Checklist
define/fsd.mdafter "Methods" section (after line 149)generate/csharp.mdgenerate/javascript.mdChanges Made
define/fsd.md - Added concise Events section including:
generate/csharp.md - Added Events section with:
await foreachgenerate/javascript.md - Added Events section with:
for await...of(verified against actual implementation)Review Updates
HttpClientUtility.createFetchEventStream) that supports all HTTP methods, not just GET.okproperty instead of checking.valueto match actual ServiceResult APIAll documentation follows the existing style and formatting conventions and accurately reflects the implementations in all language targets.
Reference
Original prompt
This pull request was created as a result of the following prompt from Copilot chat.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.