Generate a returns doc comment for cs::encodedReturn service operations - #4932
Conversation
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new generated documentation lacks automated regression assertions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds generated <returns> documentation for service operations using cs::encodedReturn.
Changes:
- Documents encoded payloads and streamed return tuples.
- Shares tuple-return documentation formatting.
- Adds encoded-return Slice fixtures.
File summaries
| File | Description |
|---|---|
ServiceGenerator.cs |
Emits encoded-return documentation. |
OperationExtensions.cs |
Builds encoded and tuple return comments. |
DocumentationTests.slice |
Adds documentation fixtures. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| /// @param user: The user. | ||
| /// @returns count: The number of entries. | ||
| /// @returns entries: The log entries. | ||
| [cs::encodedReturn] streamLog(user: string) -> (count: int32, entries: stream string) |
There was a problem hiding this comment.
Added XML documentation tests for both operations in 678b20f.
InsertCreativityHere
left a comment
There was a problem hiding this comment.
Looks good, but I also agree, the testing here seems lackluster. Just making sure the doc-comment doesn't emit any warnings could let many kinds of failures through silently.
| var items = new List<(string, string)> { (op.EncodedReturnPayloadName, payloadDescription) }; | ||
| if (DocCommentFormatter.FormatOverview(streamReturn.Comment, currentNamespace) is string overview) | ||
| { | ||
| items.Add((streamReturn.Name, overview)); | ||
| } | ||
| return TupleReturnsDocComment(items); |
There was a problem hiding this comment.
In the case where the user hasn't documented their stream parameter, this will generate a doc-comment describing a tuple of one element, which is better than the current state, but feels incomplete.
Maybe we could add a fallback for when FormatOverview returns null, so we always generate 2 items in the list?
There was a problem hiding this comment.
Done in 678b20f: the stream item is always listed, with "The streamed return value." when it has no @returns.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
InsertCreativityHere
left a comment
There was a problem hiding this comment.
Looks good to me!
Fixes #4927.
The service-side declaration of a
cs::encodedReturnoperation now gets a<returns>doc comment. Its return valueis a
PipeReaderholding the encoded return value, or a tuple of that reader and the streamed return, so the Slice@returnstags don't describe it directly:Payloadelement and that fixed text, followed by thestream element with its
@returnstext, or "The streamed return value." when it is not documented.The "A tuple containing" list formatting is now shared with the existing
<returns>comment for tuple returns.Two documented
cs::encodedReturnoperations were added toDocumentationTests.slice, andDocumentationTests.cschecks their generated comments in the XML documentation file. Generated output for the streamed case:
Follow-up to #4926.
What's Changed
Area: Slice codec
<returns>doc comment for the service-side declaration of operations with thecs::encodedReturnattribute.🤖 Generated with Claude Code