feat: port OpenAI ChatKit Python SDK to C#/MAF#7
Conversation
1:1 port of the ChatKit protocol into Qyl.ChatKit targeting Microsoft.Extensions.AI instead of the OpenAI Python Agents SDK. 27 source files, ~4300 lines: - Contracts: Page, ThreadItem (10 variants), ThreadStreamEvent (12), ThreadItemUpdate (10), ChatKitRequest (15), IStore, Actions, Sources, Messages, Attachments, Workflows, Errors, IconName, FeedbackKind - Widgets: 11 enums, 25+ components, 3 root types, chart types - Runtime: ChatKitServer<TContext>, WidgetDiff, AgentContext, ThreadItemConverter, ResponseStreamConverter, StreamingResult Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Deleted duplicate BitNet test fixtures — now consumed from ANcpLua.Roslyn.Utilities.Testing.AI package instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary by CodeRabbitRelease Notes
WalkthroughNew ChatKit library introduced with request routing, SSE streaming pipeline, thread/item/widget management, and persistence abstractions. Dependabot and package version configuration updated to support OpenAI and Microsoft AI extensions. Changes
Sequence DiagramsequenceDiagram
actor Client
participant ChatKitServer
participant EventLoop
participant Store
participant Handler
Client->>ChatKitServer: ProcessAsync(request)
alt Streaming Request
ChatKitServer->>ChatKitServer: ProcessStreamingAsync()
ChatKitServer->>EventLoop: ProcessEventsAsync(stream from Handler)
Handler->>Handler: RespondAsync() produces ThreadStreamEvent
EventLoop->>EventLoop: RunEventLoopAsync()
loop For each event
EventLoop->>Store: Persist(ThreadItem/Metadata changes)
EventLoop->>Client: Emit SSE frame
end
opt On cancellation
EventLoop->>Handler: HandleStreamCancelledAsync()
EventLoop->>Store: Persist pending assistant content
end
EventLoop->>Client: Complete stream
else Non-Streaming Request
ChatKitServer->>Store: Load/Save thread or items
ChatKitServer->>Client: Return JSON response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 3 warnings)
✅ Passed checks (2 passed)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the Qyl.ChatKit library, a framework for building threaded chat applications with support for streaming, UI widgets, and Microsoft.Extensions.AI integration. The feedback highlights a critical bug in ThreadItemConverter where tool call results are not correctly returned to the model, effectively breaking tool-calling functionality. Additionally, there are suggestions to improve the robustness of ID generation to avoid collisions and to refactor type-checking logic in ChatKitServer and ChatKitRequest using interfaces to enhance maintainability and extensibility.
| public virtual ValueTask<ChatMessage?> ConvertClientToolCallAsync(ClientToolCallItem item) | ||
| { | ||
| if (item.Status == "pending") | ||
| return new ValueTask<ChatMessage?>((ChatMessage?)null); | ||
|
|
||
| var args = JsonSerializer.Serialize(item.Arguments); | ||
| var output = JsonSerializer.Serialize(item.Output); | ||
|
|
||
| var message = new ChatMessage(ChatRole.Assistant, | ||
| [ | ||
| new FunctionCallContent(item.CallId, item.Name, new Dictionary<string, object?> { ["args"] = args }), | ||
| ]); | ||
|
|
||
| // The result message follows | ||
| var resultMessage = new ChatMessage(ChatRole.Tool, | ||
| [ | ||
| new FunctionResultContent(item.CallId, output), | ||
| ]); | ||
|
|
||
| // Return the function call; the caller collects both via ToAgentInputAsync | ||
| return new ValueTask<ChatMessage?>(message); | ||
| } |
There was a problem hiding this comment.
There is a critical bug here. The resultMessage (containing the tool output, created on line 99) is never returned or used. The method only returns the message with the function call. This means the tool's result is not passed to the model, which breaks the tool-calling functionality. The implementation needs to be corrected to ensure the tool result is included in the agent's input. This might require changing this method's signature to return multiple messages and updating ToAgentInputAsync to handle them.
| public static string GenerateId(StoreItemType itemType) | ||
| { | ||
| var prefix = Prefixes[itemType]; | ||
| return $"{prefix}_{Guid.NewGuid():N}"[..12]; | ||
| } |
There was a problem hiding this comment.
The generated ID is truncated to 12 characters, which is quite short. For example, a thr_ prefix followed by 8 characters from a GUID provides only 32 bits of randomness. With a large number of items, this could lead to ID collisions (see the birthday problem). I recommend using a longer ID to ensure uniqueness in a production environment.
public static string GenerateId(StoreItemType itemType)
{
var prefix = Prefixes[itemType];
return $"{prefix}_{Guid.NewGuid():N}"[..24];
}| int targetIndex = evt.Update switch | ||
| { | ||
| AssistantMessageContentPartAdded u => u.ContentIndex, | ||
| AssistantMessageContentPartTextDelta u => u.ContentIndex, | ||
| AssistantMessageContentPartAnnotationAdded u => u.ContentIndex, | ||
| AssistantMessageContentPartDone u => u.ContentIndex, | ||
| _ => -1, | ||
| }; |
There was a problem hiding this comment.
The when clause on line 547 already constrains evt.Update to one of the four AssistantMessageContentPart... types. This switch expression to get targetIndex is therefore repetitive. To improve maintainability, you could introduce a common interface for these update types that exposes a ContentIndex property. This would allow you to simplify the when clause and access the index directly, making the code more concise and easier to extend if more such update types are added.
| public static bool IsStreamingRequest(ChatKitRequest request) => request is | ||
| ThreadsCreateReq or | ||
| ThreadsAddUserMessageReq or | ||
| ThreadsRetryAfterItemReq or | ||
| ThreadsAddClientToolOutputReq or | ||
| ThreadsCustomActionReq; |
There was a problem hiding this comment.
This static method for checking if a request is streaming is not easily extensible. Every time a new streaming request type is added, this method must be manually updated, which is error-prone. A more robust and object-oriented approach would be to define an IStreamingRequest interface that all streaming request types implement. The check would then become a simple request is IStreamingRequest.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: faef40a364
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| } | ||
|
|
||
| var userMessage = new ChatMessage(ChatRole.User, string.Concat(textParts)); |
There was a problem hiding this comment.
ConvertUserMessageAsync builds the outgoing user message from string.Concat(textParts) and never calls AttachmentToContentAsync or TagToContentAsync, so attachment payloads and rich tag data are dropped before reaching the model. This breaks prompts that depend on uploaded files/images or tag metadata even though the converter exposes hooks for those content types.
Useful? React with 👍 / 👎.
| var resultMessage = new ChatMessage(ChatRole.Tool, | ||
| [ | ||
| new FunctionResultContent(item.CallId, output), | ||
| ]); |
There was a problem hiding this comment.
For completed ClientToolCallItems, the method constructs resultMessage with FunctionResultContent but never returns or appends it, so only the function-call half is included in agent history. In follow-up turns the model sees that a tool was invoked but not what it returned, which can corrupt tool-dependent reasoning and response quality.
Useful? React with 👍 / 👎.
| [JsonPolymorphic] | ||
| [JsonDerivedType(typeof(CustomSummary), typeDiscriminator: "custom")] | ||
| [JsonDerivedType(typeof(DurationSummary), typeDiscriminator: "duration")] |
There was a problem hiding this comment.
WorkflowSummary uses [JsonPolymorphic] without specifying TypeDiscriminatorPropertyName, so serialization falls back to the default discriminator key instead of the protocol's type field used by the rest of ChatKit unions. This will make summary payloads incompatible with clients expecting summary.type (custom/duration).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Ports the OpenAI ChatKit Python SDK concepts into a new .NET library (Qyl.ChatKit), adding the core wire-protocol types, a streaming server abstraction, and widget/workflow models to integrate with the existing agents/MAF stack.
Changes:
- Added new
Qyl.ChatKitproject with ChatKit protocol models (threads/items/events/requests), server-side streaming orchestration, and agent context helpers. - Introduced widget model types (components/roots/enums/chart types) plus a widget diff engine for incremental UI updates.
- Updated central package versions and test dependencies to include OpenAI +
Microsoft.Extensions.AI.OpenAI, and adjusted dependabot to ignore dotnet-sdk 11 prereleases.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| Version.props | Adds central versions for OpenAI + Microsoft.Extensions.AI.OpenAI. |
| Directory.Packages.props | Registers OpenAI + Microsoft.Extensions.AI.OpenAI for central package management. |
| tests/Qyl.Agents.Tests/Qyl.Agents.Tests.csproj | Adds OpenAI-related package refs to test project. |
| .github/dependabot.yml | Ignores dotnet-sdk 11.x prereleases. |
| netagents.slnx | Adds src/Qyl.ChatKit/Qyl.ChatKit.csproj to the solution. |
| src/Qyl.ChatKit/Qyl.ChatKit.csproj | Introduces the new ChatKit library project targeting net10.0. |
| src/Qyl.ChatKit/ChatKitServer.cs | Adds the abstract server implementation for streaming/non-streaming ChatKit requests over SSE. |
| src/Qyl.ChatKit/ChatKitJsonOptions.cs | Adds shared JSON options (snake_case, ignore nulls) for wire serialization. |
| src/Qyl.ChatKit/StreamingResult.cs | Adds wrappers for streaming SSE bytes vs non-streaming JSON payloads. |
| src/Qyl.ChatKit/Requests.cs | Adds request DTOs + streaming/non-streaming routing metadata. |
| src/Qyl.ChatKit/ThreadEvents.cs | Adds streaming event union and event payloads. |
| src/Qyl.ChatKit/ThreadItemUpdates.cs | Adds update/delta union types for incremental item updates. |
| src/Qyl.ChatKit/ThreadTypes.cs | Adds thread metadata + thread status union. |
| src/Qyl.ChatKit/ThreadItems.cs | Adds thread item union + item payload models (messages, widgets, workflow, tool calls, etc.). |
| src/Qyl.ChatKit/Messages.cs | Adds user/assistant message content models and inference options. |
| src/Qyl.ChatKit/ThreadItemConverter.cs | Adds ChatKit→MAF conversion layer for feeding thread history into an IChatClient. |
| src/Qyl.ChatKit/AgentContext.cs | Adds an agent-facing context object with helpers to emit ChatKit events/widgets/workflows. |
| src/Qyl.ChatKit/WidgetDiff.cs | Adds widget diffing + widget streaming helpers producing incremental deltas. |
| src/Qyl.ChatKit/Workflows.cs | Adds workflow + task models with polymorphic task/summary support. |
| src/Qyl.ChatKit/Sources.cs | Adds source/citation models (file/url/entity). |
| src/Qyl.ChatKit/ResponseStreamConverter.cs | Adds hooks for adapting streamed model output into ChatKit thread items/annotations. |
| src/Qyl.ChatKit/Page.cs | Adds a generic pagination container type. |
| src/Qyl.ChatKit/IStore.cs | Adds store interfaces + ID generation helpers and store item type enum. |
| src/Qyl.ChatKit/Errors.cs | Adds stream error types and a wire error-code enum. |
| src/Qyl.ChatKit/FeedbackKind.cs | Adds feedback sentiment enum for item feedback. |
| src/Qyl.ChatKit/IconName.cs | Adds icon-name constants for UI-related payloads. |
| src/Qyl.ChatKit/Attachments.cs | Adds attachment models + transcription input/result types. |
| src/Qyl.ChatKit/Actions.cs | Adds action payload types and related enums for widget actions. |
| src/Qyl.ChatKit/Widgets/WidgetComponentBase.cs | Defines widget polymorphic base types and shared widget DTOs. |
| src/Qyl.ChatKit/Widgets/WidgetComponents.cs | Adds widget component DTOs (Text/Markdown/Button/Form/etc.). |
| src/Qyl.ChatKit/Widgets/WidgetRoots.cs | Adds widget root DTOs (Card/ListView/BasicRoot) + dynamic component type. |
| src/Qyl.ChatKit/Widgets/WidgetEnums.cs | Adds enums for widget styling/alignment tokens. |
| src/Qyl.ChatKit/Widgets/ChartTypes.cs | Adds chart DTOs and polymorphic series types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| using var aEnum = a.EnumerateArray(); | ||
| using var bEnum = b.EnumerateArray(); | ||
|
|
||
| while (aEnum.MoveNext() && bEnum.MoveNext()) | ||
| { | ||
| if (!JsonElementEquals(aEnum.Current, bEnum.Current)) | ||
| return false; | ||
| } |
There was a problem hiding this comment.
ArrayEquals uses using var with JsonElement.ArrayEnumerator (and bEnum likewise). JsonElement.ArrayEnumerator does not implement IDisposable, so this won’t compile. Drop the using and just iterate the enumerators (or use EnumerateArray() with index-based comparison).
| public virtual ValueTask<ChatMessage?> ConvertUserMessageAsync( | ||
| UserMessageItem item, bool isLastMessage = true) | ||
| { | ||
| List<string> textParts = []; | ||
| List<UserMessageTagContent> rawTags = []; | ||
|
|
||
| foreach (var part in item.Content) | ||
| { | ||
| switch (part) | ||
| { | ||
| case UserMessageTextContent text: | ||
| textParts.Add(text.Text); | ||
| break; | ||
| case UserMessageTagContent tag: | ||
| textParts.Add($"@{tag.Text}"); | ||
| rawTags.Add(tag); | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
ConvertUserMessageAsync defines AttachmentToContentAsync / TagToContentAsync hooks, but the method never calls them. As a result, attachments and tag metadata are silently omitted from the model input even when present on UserMessageItem (only @text is inlined). Consider converting attachments/tags into AIContent entries on the ChatMessage so the model receives the structured context.
| ]); | ||
|
|
||
| // The result message follows | ||
| var resultMessage = new ChatMessage(ChatRole.Tool, | ||
| [ | ||
| new FunctionResultContent(item.CallId, output), | ||
| ]); | ||
|
|
||
| // Return the function call; the caller collects both via ToAgentInputAsync |
There was a problem hiding this comment.
ConvertClientToolCallAsync builds both a function-call message and a tool-result message, but returns only the function-call ChatMessage. Since ToAgentInputAsync only adds the returned message, the tool result is currently dropped from the model context. Return both messages (e.g., change the API to return a sequence) or otherwise ensure the tool-result ChatMessage is included.
| ]); | |
| // The result message follows | |
| var resultMessage = new ChatMessage(ChatRole.Tool, | |
| [ | |
| new FunctionResultContent(item.CallId, output), | |
| ]); | |
| // Return the function call; the caller collects both via ToAgentInputAsync | |
| new FunctionResultContent(item.CallId, output), | |
| ]); |
| /// <summary>Summary variants available for workflows.</summary> | ||
| [JsonPolymorphic] | ||
| [JsonDerivedType(typeof(CustomSummary), typeDiscriminator: "custom")] | ||
| [JsonDerivedType(typeof(DurationSummary), typeDiscriminator: "duration")] | ||
| public abstract record WorkflowSummary; |
There was a problem hiding this comment.
WorkflowSummary is marked [JsonPolymorphic] but doesn’t specify TypeDiscriminatorPropertyName = "type" like the rest of the wire unions in this project. The default discriminator name (e.g., "$type") is unlikely to match the ChatKit protocol, so summary polymorphic (de)serialization will fail. Set the discriminator property name explicitly to match the protocol.
| [JsonDerivedType(typeof(ListViewItem), "ListViewItem")] | ||
| [JsonDerivedType(typeof(Card), "Card")] | ||
| [JsonDerivedType(typeof(ListView), "ListView")] | ||
| [JsonDerivedType(typeof(BasicRoot), "Basic")] |
There was a problem hiding this comment.
DynamicWidgetComponent derives from WidgetComponentBase, but it isn’t registered in the [JsonDerivedType(...)] list for WidgetComponentBase. With System.Text.Json polymorphism, serializing/deserializing a runtime DynamicWidgetComponent through the base type can throw at runtime. Either register it with the correct discriminator or remove the type if it’s not intended to be used polymorphically.
| [JsonDerivedType(typeof(BasicRoot), "Basic")] | |
| [JsonDerivedType(typeof(BasicRoot), "Basic")] | |
| [JsonDerivedType(typeof(DynamicWidgetComponent), "DynamicWidgetComponent")] |
| catch (StreamError e) | ||
| { | ||
| await writer.WriteAsync(new ErrorEvent | ||
| { | ||
| Code = e.Code.ToString(), | ||
| AllowRetry = e.AllowRetry, | ||
| }, CancellationToken.None); | ||
| } |
There was a problem hiding this comment.
In the StreamError catch, ErrorEvent.Code is set to e.Code.ToString(), which yields the enum identifier (e.g., StreamError) rather than the intended wire value (e.g., stream.error). This will break client-side error code handling. Map the enum to its serialized string (or switch on the enum) instead of calling ToString().
| if (thread != lastThread) | ||
| { | ||
| lastThread = thread; | ||
| await Store.SaveThreadAsync(thread, context); | ||
| await writer.WriteAsync( | ||
| new ThreadUpdatedEvent { Thread = ToThreadResponse(thread) }, ct); | ||
| } |
There was a problem hiding this comment.
RunEventLoopAsync checks if (thread != lastThread) to persist/send ThreadUpdatedEvent, but thread is never reassigned inside the loop and ThreadMetadata uses init setters, so it can’t be mutated in-place either. As written, this branch is effectively dead and thread updates during streaming won’t be persisted. If thread updates are part of the protocol, handle ThreadUpdatedEvent (or a dedicated update event) by updating a local ThreadMetadata variable and saving it.
| case WorkflowItem workflow when evt.Update is WorkflowTaskUpdated or WorkflowTaskAdded: | ||
| { | ||
| var tasks = workflow.Workflow.Tasks.ToList(); | ||
|
|
||
| switch (evt.Update) | ||
| { | ||
| case WorkflowTaskUpdated u: | ||
| tasks[u.TaskIndex] = u.Task; | ||
| break; | ||
| case WorkflowTaskAdded u: | ||
| tasks.Add(u.Task); | ||
| break; | ||
| } |
There was a problem hiding this comment.
UpdatePendingItems applies WorkflowTaskUpdated via tasks[u.TaskIndex] = ... with no bounds checks. If an agent emits an out-of-range index (or tasks were added/removed unexpectedly), the stream loop will throw and terminate the connection. Guard the index and fail gracefully (e.g., treat as replace/full item update or emit an error event).
| public static string GenerateId(StoreItemType itemType) | ||
| { | ||
| var prefix = Prefixes[itemType]; | ||
| return $"{prefix}_{Guid.NewGuid():N}"[..12]; |
There was a problem hiding this comment.
StoreIdGenerator.GenerateId truncates the GUID-based id to 12 characters (including the prefix), leaving only ~7–8 hex chars of entropy depending on prefix length. This increases collision risk and can cause hard-to-debug store corruption at scale. Consider returning the full prefixed GUID (or at least significantly more characters).
| return $"{prefix}_{Guid.NewGuid():N}"[..12]; | |
| return $"{prefix}_{Guid.NewGuid():N}"; |
| /// Integration-only metadata stored with the attachment. | ||
| /// Ignored by ChatKit and not returned in server responses. | ||
| /// </summary> | ||
| [JsonPropertyName("metadata")] |
There was a problem hiding this comment.
The XML doc on AttachmentBase.Metadata says it is “ignored by ChatKit and not returned in server responses,” but the property is currently serialized as metadata (and ProcessNonStreamingAsync returns Serialize(attachment) for attachment creation). If this metadata must never go over the wire, mark it [JsonIgnore] (and/or use a separate internal model) to prevent accidental exposure.
| [JsonPropertyName("metadata")] | |
| [JsonIgnore] |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 35
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/dependabot.yml:
- Around line 8-10: The ignore rule for dependency-name "dotnet-sdk" currently
uses the versions pattern ">=11.0.0-0"; change this to the prerelease-only
pattern "11.0.0-*" so the rule explicitly ignores only .NET SDK 11 prereleases.
Edit the ignore entry that contains dependency-name: "dotnet-sdk" and replace
the versions array value with ["11.0.0-*"] to make the intent explicit and
idiomatic.
In `@src/Qyl.ChatKit/Actions.cs`:
- Around line 9-30: Add missing XML documentation for the public enum members
and any public properties in this file: provide a /// <summary> comment for the
enum members Client and Server (the client/server enum) and for each
LoadingBehavior member Auto, None, Self, and Container, describing their
purpose/behavior; also add XML docs to any other newly public properties in the
file. Use triple-slash comments (/// <summary>...</summary>) immediately above
each enum member or public property so the public API surface complies with the
project's XML doc requirements.
In `@src/Qyl.ChatKit/AgentContext.cs`:
- Around line 64-78: StartWorkflowAsync currently defers emitting
ThreadItemAddedEvent for empty non-"reasoning" workflows but EndWorkflowAsync
always emits ThreadItemDoneEvent, causing clients to see completions for items
they never saw; modify the logic so EndWorkflowAsync only emits
ThreadItemDoneEvent for WorkflowItem instances that were previously announced.
Concretely, add a Boolean flag/property (e.g., WasAnnounced) to WorkflowItem
when you call StreamAsync(new ThreadItemAddedEvent {...}) in StartWorkflowAsync
(or set it to true when workflow.Type == "reasoning" or workflow.Tasks.Count >
0), and in EndWorkflowAsync check that flag (or the same condition) before
sending ThreadItemDoneEvent; apply the same pattern to the analogous block
referenced at the 141-165 region.
- Line 22: The field _events currently uses
Channel.CreateUnbounded<ThreadStreamEvent>(), which allows unlimited queue
growth; change it to a bounded channel to apply backpressure (replace
Channel.CreateUnbounded with Channel.CreateBounded using a BoundedChannelOptions
instance), pick a sensible capacity (e.g., 100) and set SingleReader = true /
SingleWriter = true plus BoundedChannelFullMode.Wait so slow/disconnected SSE
clients block the producer rather than exhausting memory; update any
producer/consumer usage of _events to handle the bounded write semantics (e.g.,
awaiting WriteAsync) in the ThreadStreamEvent producers and the SSE consumer.
In `@src/Qyl.ChatKit/Attachments.cs`:
- Around line 73-74: Change the Size property's type from int to long to support
files >2GB: update the declaration public required int Size { get; init; } to
use long (public required long Size { get; init; }), then search for and update
all usages, method signatures, serialization/deserialization expectations, and
any comparisons or math that assume int (e.g., code referencing Size,
constructors, DTO mappings, and unit tests) so they accept long and preserve
JSON name "size" via the existing JsonPropertyName attribute.
In `@src/Qyl.ChatKit/ChatKitServer.cs`:
- Around line 12-23: Add OpenTelemetry instrumentation to ChatKitServer by
introducing an ActivitySource and Meter for the service and initializing them
for use by request-routing and streaming methods; specifically, add
protected/static readonly ActivitySource (e.g., ActivitySource named
"Qyl.ChatKit.ChatKitServer") and protected/static readonly Meter (e.g., Meter
named "Qyl.ChatKit.ChatKitServer.Metrics") fields to the ChatKitServer class,
ensure they are initialized before any tracing/metrics usage (constructor or
static initializer), and expose them as protected properties so methods that
handle lifecycle, routing, and streaming (the class constructor
ChatKitServer<TContext> and the methods in the 109-121 and 421-534 regions) can
create Activities and record metrics; also ensure disposal is handled if
required and that any created Activity uses the shared ActivitySource and metric
instruments (Counters/Histograms) from the Meter.
- Around line 489-523: The non-cancellation exception handlers
(CustomStreamError, StreamError, and the generic catch) don’t flush or persist
pendingItems, so partial item updates can be lost; before writing the ErrorEvent
in each of these catch blocks, call the same cleanup used for cancellations:
await HandleStreamCancelledAsync(thread, pendingItems.Values.ToList(), context,
CancellationToken.None); then complete the writer (writer.Complete()) and return
(or ensure the method exits) so pending state is persisted/retracted
consistently for CustomStreamError, StreamError, and the generic exception path;
keep existing logging (Logger.LogError) in the generic handler.
- Around line 341-347: The code deletes persisted items via
Store.DeleteThreadItemAsync (inside the itemsToRemove loop) but never emits
corresponding ThreadItemRemovedEvent notifications, so incremental clients stay
stale; after each successful DeleteThreadItemAsync call (and also in the
parallel block at the other location around RespondAsync/ProcessEventsAsync),
construct and emit a ThreadItemRemovedEvent for that item and propagate it
through the same event pipeline used by ProcessEventsAsync (or yield the event
before/after yielding other events) so clients receive the removal; update the
code paths that iterate itemsToRemove (including the other occurrence mentioned)
to create the ThreadItemRemovedEvent with the deleted item id and thread id and
ensure it is yielded/sent to subscribers.
- Around line 242-267: The try/catch in ProcessStreamingAsync currently only
wraps the assignment of events (ProcessStreamingImplAsync) but not the
enumeration, so exceptions thrown during await foreach escape and drop the SSE
stream; move the try/catch to wrap the await foreach that iterates events (the
WithCancellation loop) instead: assign IAsyncEnumerable<ThreadStreamEvent>
events = ProcessStreamingImplAsync(request, context, ct) first, then perform the
await foreach inside the try block and keep the catch (Exception ex) when (ex is
not OperationCanceledException) to Logger.LogError and rethrow.
- Around line 405-413: Replace the unbounded channel in ProcessEventsAsync with
a bounded one to apply backpressure (e.g., use
Channel.CreateBounded<ThreadStreamEvent>(capacity: 100) or a configurable
capacity) so writer.WriteAsync on channel.Writer will await when the buffer is
full; also stop fire-and-forgetting RunEventLoopAsync — either await its Task or
store and propagate it so exceptions surface and it can be cancelled/disposed
properly (ensure RunEventLoopAsync is awaited or its Task tracked and observed,
and that cancellation token ct is passed through).
- Line 428: The checks comparing thread != lastThread in RunEventLoopAsync are
dead because ThreadMetadata is immutable and thread is never reassigned; either
remove those unreachable branches or implement real update semantics: update the
local thread variable (e.g., reassign thread or use thread = thread with {...})
when metadata changes or re-fetch from storage, and then call SaveThreadAsync
and write ThreadUpdatedEvent only when the new thread differs from lastThread;
update references to lastThread and the equality checks accordingly so
SaveThreadAsync and ThreadUpdatedEvent are executed only when a genuine thread
change occurs.
In `@src/Qyl.ChatKit/Errors.cs`:
- Around line 12-13: The public enum member StreamError (and the other public
enum members/properties in the error contract) lack XML documentation; add
member-level XML doc comments (e.g., /// <summary>...</summary>) for StreamError
and each public enum member/property in the same enum/class so the public API
surface is documented—ensure the summary explains the meaning/when it is used
and include optional <remarks> or <value> tags where helpful to clarify
behavior.
- Around line 35-36: The CustomStreamError constructor currently accepts null or
whitespace messages; update the CustomStreamError(string message, bool
allowRetry = false) constructor to validate message is not null or whitespace
and throw an ArgumentException (or ArgumentNullException) when invalid,
preserving the allowRetry parameter and base Exception initialization only after
validation so no empty user-facing errors are created.
In `@src/Qyl.ChatKit/FeedbackKind.cs`:
- Around line 9-13: The public enum FeedbackKind exposes members without XML
docs; add member-level XML documentation for the FeedbackKind enum members
Positive and Negative describing their semantics (e.g., "Represents positive
feedback" and "Represents negative feedback") so the public API has proper XML
comments; update the FeedbackKind enum in src/Qyl.ChatKit/FeedbackKind.cs to
include /// <summary>...</summary> on both Positive and Negative.
In `@src/Qyl.ChatKit/IconName.cs`:
- Around line 12-75: The public icon constants in IconName (e.g., Agent,
Analytics, Atom, BookOpen, CheckCircleFilled, WriteAlt2, etc.) lack XML
documentation; add XML doc comments for the IconName class and each public const
string with concise /// <summary> descriptions that state the purpose or visual
meaning of the icon (for example "/// <summary>Agent icon
identifier.</summary>"). Ensure comments are added immediately above each
constant and the class definition so the generated API docs include
human-readable descriptions for all these public symbols.
In `@src/Qyl.ChatKit/IStore.cs`:
- Around line 59-62: The public async store interface methods (e.g.,
DeleteAttachmentAsync and CreateAttachmentAsync) must accept a CancellationToken
so callers can cancel DB/blob work; update these method signatures to include a
CancellationToken parameter (preferably as the last parameter with a default of
CancellationToken cancellationToken = default), apply the same change to all
other async members on both store interfaces, and then update all implementing
classes and any call sites to pass/forward the token through to underlying async
operations.
- Around line 45-48: The GenerateId method currently truncates the GUID
substring ("return $"{prefix}_{Guid.NewGuid():N}"[..12];"), which reduces
entropy and increases collision risk; change GenerateId in IStore (use
Prefixes[itemType]) to return the full GUID-based id without slicing—e.g.,
combine the prefix and the full Guid.NewGuid():N (or another sufficiently long
unique token) so persisted IDs are not truncated.
In `@src/Qyl.ChatKit/Messages.cs`:
- Around line 43-44: Change the mutable Data property to an immutable contract
by updating its type from Dictionary<string, object?> to
IReadOnlyDictionary<string, object?> on the record (property name: Data) and
keep the existing empty Dictionary as the default initializer (e.g., = new
Dictionary<string, object?>()), so callers see an IReadOnlyDictionary while JSON
serialization still has a concrete instance to serialize/deserialize; ensure the
JsonPropertyName attribute and using directives remain unchanged.
In `@src/Qyl.ChatKit/Page.cs`:
- Around line 8-15: Add XML documentation comments to the public API on Page<T>:
document the Data property (IReadOnlyList<T> Data) describing what items it
contains and that it's read-only, document HasMore (bool HasMore) to indicate
whether more pages are available, and document After (string? After) to explain
the cursor/token semantics and when it will be null; place these triple-slash
summaries immediately above the properties in the Page<T> type so generated API
docs include them.
In `@src/Qyl.ChatKit/ResponseStreamConverter.cs`:
- Around line 16-47: Add a CancellationToken parameter to all public async
virtual methods so overrides can cancel I/O: update Base64ImageToUrlAsync,
FileCitationToAnnotationAsync, and UrlCitationToAnnotationAsync to accept a
CancellationToken (e.g., CancellationToken cancellationToken = default) and
propagate it into their ValueTask results (no-op here but required for signature
compatibility); do not change PartialImageIndexToProgress (it's synchronous).
Ensure method signatures remain virtual and that callers/overrides are updated
to accept the new token parameter.
In `@src/Qyl.ChatKit/Sources.cs`:
- Around line 12-23: The public record/type in Sources.cs exposes properties
(e.g., Title, Description, Timestamp, Group and the other public properties
referenced in the review) without XML documentation; add concise /// <summary>
XML comments for each public property and any other public members in the same
file (the properties at the other ranges mentioned) describing their purpose,
nullability, and format expectations (for example note that Timestamp is an
ISO-8601 string if applicable), and include <remarks> or <example> where useful
to clarify usage; ensure every public API member in this file has a member-level
XML comment to satisfy the project's documentation rules.
In `@src/Qyl.ChatKit/StreamingResult.cs`:
- Around line 4-7: The public primary-constructor parameter stream on
StreamingResult is not null-guarded and can cause later failures; update the
constructor to validate stream (throw ArgumentNullException(nameof(stream)) when
null) and store it in a readonly backing field, then have GetAsyncEnumerator
call that validated field (e.g., use a private readonly IAsyncEnumerable<byte[]>
_stream) so the null check occurs at construction time.
- Around line 11-13: The Json property on NonStreamingResult exposes the
original byte[] allowing callers to mutate internal state; instead make a
defensive copy: in the constructor of NonStreamingResult(byte[] json) copy the
incoming array into a new private array (e.g., using Array.Copy or
json.ToArray()) and have the Json accessor return either that internal copy or
expose an immutable type (ReadOnlyMemory<byte>/ReadOnlySpan<byte>) so callers
cannot mutate the original buffer; update the Json property and constructor
accordingly to always use the copied/readonly buffer.
In `@src/Qyl.ChatKit/ThreadEvents.cs`:
- Around line 21-135: Several public DTO properties lack XML documentation; add
concise XML doc comments to each public property to explain semantics,
optionality and allowed values. Specifically, document Thread (in
ThreadCreatedEvent/ThreadUpdatedEvent) and Item/ItemId
(ThreadItemAddedEvent/ThreadItemDoneEvent/ThreadItemRemovedEvent/ThreadItemReplacedEvent),
Update (ThreadItemUpdatedEvent), StreamOptions.AllowCancel,
StreamOptionsEvent.StreamOptions, ProgressUpdateEvent.Icon/Text,
ClientEffectEvent.Name/Data, ErrorEvent.Code/Message/AllowRetry (explain default
"custom" code and when retry is allowed), and NoticeEvent.Level/Message/Title
(enumerate allowed level values). Place the comments on the properties within
the corresponding records (ThreadCreatedEvent, ThreadUpdatedEvent,
ThreadItemAddedEvent, ThreadItemUpdatedEvent, ThreadItemDoneEvent,
ThreadItemRemovedEvent, ThreadItemReplacedEvent, StreamOptions,
StreamOptionsEvent, ProgressUpdateEvent, ClientEffectEvent, ErrorEvent,
NoticeEvent) so consumers can read intent and constraints in IntelliSense.
In `@src/Qyl.ChatKit/ThreadItemConverter.cs`:
- Around line 174-178: The code in ThreadItemConverter (the return constructing
new ChatMessage) hardcodes "image/png" when creating UriContent(new
Uri(item.Image.Url), "image/png"); change this to determine the actual MIME type
instead: add a mime property to GeneratedImageItem (e.g., item.Image.Mime) or
infer the MIME from the image URL/extension (check item.Image.Url extension like
.png/.jpg/.jpeg/.webp and map to image/png, image/jpeg, image/webp) and pass
that value into UriContent; ensure UriContent creation uses the resolved MIME
string and handle missing/unknown extensions by falling back to a safe default
like "application/octet-stream" or performing a HEAD request to obtain
Content-Type if available.
- Around line 187-188: Add a CancellationToken parameter to the public async
method ToAgentInputAsync (e.g., change signature to accept CancellationToken)
and thread that token through every internal async conversion call (the
Convert*Async methods called inside ToAgentInputAsync) by passing the token into
each call; also update any callers of ToAgentInputAsync to accept/forward a
CancellationToken so the token is propagated end-to-end.
- Around line 85-106: ConvertClientToolCallAsync currently discards the created
resultMessage (FunctionResultContent) and only returns the function call
message, losing tool output; change the method to return both messages (e.g.,
update signature from ValueTask<ChatMessage?> to
ValueTask<IEnumerable<ChatMessage>?> or ValueTask<ChatMessage[]?>) and return a
collection containing both message and resultMessage (preserving the early
return for pending). Also update callers such as ToAgentInputAsync to consume
the new collection format so the FunctionResultContent reaches the agent.
In `@src/Qyl.ChatKit/ThreadItems.cs`:
- Around line 15-17: The filtering logic currently in shouldSwallow (which is
only applied for ThreadItemDoneEvent) must be applied to all streaming/event
paths: ensure ThreadItemAddedEvent and ThreadItemReplacedEvent are passed
through the same shouldSwallow check so HiddenContextItem and
SdkHiddenContextItem are dropped before they reach clients; additionally, in
ProcessSyncCustomActionAsync sanitize the SyncCustomActionResponse.UpdatedItem
(or run it through the same filter/mapper) before serializing/returning so
subclass implementations cannot return hidden items directly. Alternatively,
make HiddenContextItem and SdkHiddenContextItem part of an internal-only base
type excluded from the public JsonDerivedType polymorphic union so they cannot
be serialized to clients.
In `@src/Qyl.ChatKit/ThreadTypes.cs`:
- Around line 23-24: The Metadata property is declared as a mutable Dictionary
on an otherwise immutable record; change its type to IReadOnlyDictionary<string,
object?> (i.e., public IReadOnlyDictionary<string, object?> Metadata { get;
init; } ) and initialize it with a concrete dictionary or a ReadOnlyDictionary
wrapper (e.g., assign new Dictionary<string, object?>() or new
ReadOnlyDictionary<string, object?>(...)) so the public API is immutable while
preserving the underlying storage for construction; update the declaration of
Metadata in ThreadTypes.cs accordingly.
- Around line 14-15: The CreatedAt property in ThreadTypes.cs is declared as
DateTime which loses timezone info; change its type to DateTimeOffset (i.e.,
public required DateTimeOffset CreatedAt { get; init; }) and update any related
uses/assignments, deserialization expectations and tests to accept
DateTimeOffset values so JSON round-trips preserve offsets; ensure any code
constructing ThreadTypes instances or mapping from strings/parsers uses
DateTimeOffset.Parse/ParseExact or DateTimeOffset.UtcNow as appropriate and
adjust any API contracts that serialize/deserialize this property.
In `@src/Qyl.ChatKit/WidgetDiff.cs`:
- Around line 86-89: Remove the redundant trailing await Task.CompletedTask in
the async IAsyncEnumerable method (the method that yields results and currently
ends with "await Task.CompletedTask"); simply delete that statement so the
method returns by completing the iterator after its yields—no other changes
needed.
- Around line 302-313: GetChildren currently skips BasicRoot and
DynamicWidgetComponent and doesn't handle Children typed as object, so add cases
for BasicRoot and DynamicWidgetComponent in the GetChildren method and handle
their Children by runtime-inspecting the object: if component is BasicRoot br or
DynamicWidgetComponent d then examine br.Children/d.Children and if it is
IEnumerable<WidgetComponentBase> return it, if it is a single
WidgetComponentBase return a single-item enumerable, otherwise return null;
preserve the existing Transition single-child logic and reuse the same
pattern-matching/casting approach to avoid missing nested streaming text
components.
In `@src/Qyl.ChatKit/Widgets/ChartTypes.cs`:
- Around line 68-69: The XAxis property currently typed as object should be
changed to XAxisConfig so System.Text.Json preserves the concrete shape; update
the declaration in ChartTypes (the property named XAxis) to use XAxisConfig
instead of object (keeping the [JsonPropertyName("xAxis")] attribute and the
required init accessor) and adjust any callers/tests that construct or access
ChartTypes.XAxis to use XAxisConfig instances or casts as needed.
In `@src/Qyl.ChatKit/Widgets/WidgetComponents.cs`:
- Around line 5-637: The new public widget DTOs (e.g., BoxLayoutProps, Text,
Title, Image, Button, Select, Input, DatePicker, Checkbox, Transition,
ListViewItem and their public properties like Children, Align, Flex, Gap, Width,
Color, Size, Src, Label, OnClickAction, DefaultValue, etc.) are missing XML
documentation for many properties (notably object? and string? typed props); add
concise XML doc comments for each public record and every public property
explaining intent, allowed values/formats, units/semantics for ambiguous
object/string fields, and examples or <remarks> where applicable, and for those
uncertain fields add a TODO/remarks tag to flag for API review per the coding
guideline about flagging new public surface without docs. Ensure summaries
appear on the record types (e.g., BoxLayoutProps, Text, Image, Button) and for
ambiguous properties (Flex, Gap, Height, Width, Background, Color, Variant,
Size, InputType, Align, Direction, etc.) so IntelliSense consumers get clear
guidance.
In `@src/Qyl.ChatKit/Workflows.cs`:
- Around line 23-26: WorkflowSummary's JsonPolymorphic attribute uses the
default discriminator ("$type") which is inconsistent with other polymorphic
models and will break deserialization; update the attribute on the abstract
record WorkflowSummary to specify TypeDiscriminatorPropertyName = "type" so its
derived types (CustomSummary, DurationSummary) serialize/deserialize with the
"type" discriminator (ensure this change aligns with Workflow.Summary usage).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c2c1462f-5446-4cbf-a977-64c6aa71d900
⛔ Files ignored due to path filters (1)
netagents.slnxis excluded by none and included by none
📒 Files selected for processing (32)
.github/dependabot.ymlDirectory.Packages.propsVersion.propssrc/Qyl.ChatKit/Actions.cssrc/Qyl.ChatKit/AgentContext.cssrc/Qyl.ChatKit/Attachments.cssrc/Qyl.ChatKit/ChatKitJsonOptions.cssrc/Qyl.ChatKit/ChatKitServer.cssrc/Qyl.ChatKit/Errors.cssrc/Qyl.ChatKit/FeedbackKind.cssrc/Qyl.ChatKit/IStore.cssrc/Qyl.ChatKit/IconName.cssrc/Qyl.ChatKit/Messages.cssrc/Qyl.ChatKit/Page.cssrc/Qyl.ChatKit/Qyl.ChatKit.csprojsrc/Qyl.ChatKit/Requests.cssrc/Qyl.ChatKit/ResponseStreamConverter.cssrc/Qyl.ChatKit/Sources.cssrc/Qyl.ChatKit/StreamingResult.cssrc/Qyl.ChatKit/ThreadEvents.cssrc/Qyl.ChatKit/ThreadItemConverter.cssrc/Qyl.ChatKit/ThreadItemUpdates.cssrc/Qyl.ChatKit/ThreadItems.cssrc/Qyl.ChatKit/ThreadTypes.cssrc/Qyl.ChatKit/WidgetDiff.cssrc/Qyl.ChatKit/Widgets/ChartTypes.cssrc/Qyl.ChatKit/Widgets/WidgetComponentBase.cssrc/Qyl.ChatKit/Widgets/WidgetComponents.cssrc/Qyl.ChatKit/Widgets/WidgetEnums.cssrc/Qyl.ChatKit/Widgets/WidgetRoots.cssrc/Qyl.ChatKit/Workflows.cstests/Qyl.Agents.Tests/Qyl.Agents.Tests.csproj
📜 Review details
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test (windows-latest)
- GitHub Check: Agent
🧰 Additional context used
📓 Path-based instructions (3)
.github/**
⚙️ CodeRabbit configuration file
GitHub Actions workflows. Review for: action version pinning (use SHA not tags for third-party actions), proper secret handling (no secrets in logs, use GITHUB_TOKEN where possible), unnecessary workflow triggers, and job dependency correctness. Flag missing concurrency groups on push-triggered workflows. Ensure matrix strategies cover the supported .NET TFMs.
Files:
.github/dependabot.yml
**/*.props
⚙️ CodeRabbit configuration file
MSBuild property files (Directory.Build.props, Directory.Packages.props, Version.props). Review for: Central Package Management correctness, version consistency, and that new packages are added with explicit version pins. Flag transitive dependency promotions that aren't justified. Verify TFM targeting is correct (.NET 10).
Files:
Version.propsDirectory.Packages.props
src/**/*.cs
⚙️ CodeRabbit configuration file
src/**/*.cs: C# 14 / .NET 10 codebase. Review for: idiomatic modern C#, proper async/await (no sync-over-async, no fire-and-forget without justification), correct IDisposable/IAsyncDisposable, null safety (NRTs enabled), and adherence to existing patterns. Flag new public API surface that lacks XML doc comments. Check DI lifetime correctness (scoped vs singleton vs transient).
ARCHITECTURAL INVARIANTS — flag violations as blocking: 1. Every new injectable service must register OpenTelemetry instrumentation
(ActivitySource or Meter).
2. Every new DuckDB write path must handle backpressure (bounded channel or semaphore). 3. No hardcoded connection strings, paths, or magic strings — use IOptions or
IConfiguration.
4. No new dependencies on Sentry-specific types in core/ — Sentry is a comparison
target, not an identity.
5. CancellationToken must be threaded through all async public methods. 6. No sync-over-async (.Result, .GetAwaiter().GetResult()) outside of
well-documented infrastructure code.
Files:
src/Qyl.ChatKit/ChatKitJsonOptions.cssrc/Qyl.ChatKit/FeedbackKind.cssrc/Qyl.ChatKit/Page.cssrc/Qyl.ChatKit/StreamingResult.cssrc/Qyl.ChatKit/Sources.cssrc/Qyl.ChatKit/Errors.cssrc/Qyl.ChatKit/Widgets/WidgetEnums.cssrc/Qyl.ChatKit/IconName.cssrc/Qyl.ChatKit/Actions.cssrc/Qyl.ChatKit/Messages.cssrc/Qyl.ChatKit/ResponseStreamConverter.cssrc/Qyl.ChatKit/Attachments.cssrc/Qyl.ChatKit/ThreadTypes.cssrc/Qyl.ChatKit/WidgetDiff.cssrc/Qyl.ChatKit/Widgets/WidgetRoots.cssrc/Qyl.ChatKit/ThreadItemConverter.cssrc/Qyl.ChatKit/ThreadItemUpdates.cssrc/Qyl.ChatKit/IStore.cssrc/Qyl.ChatKit/Workflows.cssrc/Qyl.ChatKit/Widgets/ChartTypes.cssrc/Qyl.ChatKit/ThreadItems.cssrc/Qyl.ChatKit/ThreadEvents.cssrc/Qyl.ChatKit/Widgets/WidgetComponentBase.cssrc/Qyl.ChatKit/ChatKitServer.cssrc/Qyl.ChatKit/Requests.cssrc/Qyl.ChatKit/Widgets/WidgetComponents.cssrc/Qyl.ChatKit/AgentContext.cs
🔇 Additional comments (6)
Version.props (1)
9-10: No actionable issues in this segment.tests/Qyl.Agents.Tests/Qyl.Agents.Tests.csproj (1)
12-13: No actionable issues in this segment.Directory.Packages.props (1)
16-17: No actionable issues in this segment.src/Qyl.ChatKit/Qyl.ChatKit.csproj (1)
1-17: No actionable issues in this segment.src/Qyl.ChatKit/Widgets/WidgetEnums.cs (1)
1-292: LGTM. Clean enum definitions with proper JSON serialization attributes and XML documentation.src/Qyl.ChatKit/Widgets/WidgetRoots.cs (1)
1-77: LGTM. Widget root records with appropriate use ofJsonExtensionDatafor extensibility.
| ignore: | ||
| - dependency-name: "dotnet-sdk" | ||
| versions: [">=11.0.0-0"] |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Dependabot ignore.versionssemantics: does>=11.0.0-0match stable11.0.0 and later, and what is the recommended pattern to ignore only .NET SDK 11 prereleases?
💡 Result:
No, the Dependabot ignore.versions pattern >=11.0.0-0 does not match stable 11.0.0 and later versions. NuGet versioning uses SemVer with four parts (major.minor.patch.suffix), where stable versions like 11.0.0 are equivalent to 11.0.0.0. Prereleases append a dash suffix, e.g., 11.0.0-preview.1 or 11.0.0-0 (possibly a preview build). The >=11.0.0-0 range targets versions lexicographically greater than or equal to 11.0.0-0, which excludes stable 11.0.0.0 (since 11.0.0.0 < 11.0.0-0 in NuGet comparison, as the dash suffix sorts after numeric). It would match prereleases like 11.0.0-preview.1 (>= 11.0.0-0) but not stables like 11.0.0, 11.0.1, or 12.0.0. To ignore only .NET SDK 11 prereleases in Dependabot (for package-ecosystem: "nuget" or "dotnet-sdk"), use NuGet range syntax in ignore.versions. Since regex/glob is unsupported (open issue #7885), target known prerelease patterns explicitly: ignore: - dependency-name: "Microsoft.NETCore.App.Ref" # or specific SDK package/global.json versions: ["11.0.0-"] - dependency-name: "Microsoft.NETCore.App.Runtime" versions: ["11.0.0-"] For dotnet-sdk ecosystem (global.json updates), add allow-prerelease: false or ignore via update-types if prereleases are classified as such. For broad prerelease ignoring across ecosystems, list common tags like ["preview", "alpha", "beta", "rc"] (per issue #7885 comments), but test as NuGet specifics vary. Official docs recommend package-manager syntax like 11.* for ranges (e.g., ignore all 11.x), confirmed working for NuGet (7.*).
Citations:
- 1: https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/controlling-dependencies-updated
- 2: https://docs.github.com/en/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file
- 3: https://docs.github.com/en/code-security/reference/supply-chain-security/dependabot-options-reference
- 4: https://docs.github.com/en/enterprise-server@3.17/code-security/reference/supply-chain-security/dependabot-options-reference
- 5: NuGet PackageVersion uses version range brackets which are ignored by Dependabot dependabot/dependabot-core#11856
- 6: Dependabot does not respect
ignoreversion syntax for Nuget dependabot/dependabot-core#4075 - 7: Dependabot doesn't seem to ignore NuGet versions dependabot/dependabot-core#6873
- 8: Nuget does not work with the "Ignore" option of dependabot.yml dependabot/dependabot-core#2531
- 9: https://github.com/dotnet/core/blob/main/release-notes/11.0/preview/preview2/11.0.0-preview.2.md
🏁 Script executed:
cat -n .github/dependabot.ymlRepository: ANcpLua/netagents
Length of output: 960
Use the clearer prerelease-only pattern 11.0.0-*.
The pattern >=11.0.0-0 correctly targets only .NET SDK 11 prereleases; it does not exclude stable 11.x releases (NuGet/SemVer semantics: stable 11.0.0 sorts before 11.0.0-0). However, the pattern 11.0.0-* is more idiomatic and explicit for ignoring only prerelease versions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/dependabot.yml around lines 8 - 10, The ignore rule for
dependency-name "dotnet-sdk" currently uses the versions pattern ">=11.0.0-0";
change this to the prerelease-only pattern "11.0.0-*" so the rule explicitly
ignores only .NET SDK 11 prereleases. Edit the ignore entry that contains
dependency-name: "dotnet-sdk" and replace the versions array value with
["11.0.0-*"] to make the intent explicit and idiomatic.
| [JsonStringEnumMemberName("client")] | ||
| Client, | ||
|
|
||
| [JsonStringEnumMemberName("server")] | ||
| Server | ||
| } | ||
|
|
||
| /// <summary>Visual loading behavior when the action executes.</summary> | ||
| [JsonConverter(typeof(JsonStringEnumConverter<LoadingBehavior>))] | ||
| public enum LoadingBehavior | ||
| { | ||
| [JsonStringEnumMemberName("auto")] | ||
| Auto, | ||
|
|
||
| [JsonStringEnumMemberName("none")] | ||
| None, | ||
|
|
||
| [JsonStringEnumMemberName("self")] | ||
| Self, | ||
|
|
||
| [JsonStringEnumMemberName("container")] | ||
| Container |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add XML docs for public enum members and public properties.
Line 9-30 and Line 36-60 add public API members/properties without member-level XML comments.
As per coding guidelines, src/**/*.cs: "Flag new public API surface that lacks XML doc comments."
Also applies to: 36-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Qyl.ChatKit/Actions.cs` around lines 9 - 30, Add missing XML
documentation for the public enum members and any public properties in this
file: provide a /// <summary> comment for the enum members Client and Server
(the client/server enum) and for each LoadingBehavior member Auto, None, Self,
and Container, describing their purpose/behavior; also add XML docs to any other
newly public properties in the file. Use triple-slash comments (///
<summary>...</summary>) immediately above each enum member or public property so
the public API surface complies with the project's XML doc requirements.
| public WorkflowItem? WorkflowItem { get; set; } | ||
| public GeneratedImageItem? GeneratedImageItem { get; set; } | ||
|
|
||
| private readonly Channel<ThreadStreamEvent> _events = Channel.CreateUnbounded<ThreadStreamEvent>(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In System.Threading.Channels, does Channel.CreateUnbounded provide backpressure when the consumer is slower than the producer?
💡 Result:
No, Channel.CreateUnbounded does not provide backpressure when the consumer is slower than the producer. Unbounded channels have no capacity limit, so writes (including WriteAsync) always succeed synchronously without blocking or waiting, allowing the queue to grow indefinitely in memory until consumed or the process runs out of memory.
Citations:
- 1: https://learn.microsoft.com/en-us/dotnet/api/system.threading.channels.channel.createunbounded?view=net-10.0
- 2: https://learn.microsoft.com/en-us/dotnet/core/extensions/channels
- 3: https://www.dotnet-guide.com/articles/dotnet-channels-queue-backpressure/
- 4: https://zeeshan01.medium.com/understanding-bounded-and-unbounded-channels-in-net-7550698933f5
Bind the event channel to prevent unbounded memory growth.
Channel.CreateUnbounded() removes backpressure between agent callbacks and SSE delivery. A slow or disconnected client will cause the queue to grow without limit, exhausting memory. Use a bounded channel with backpressure semantics instead.
Suggested fix
- private readonly Channel<ThreadStreamEvent> _events = Channel.CreateUnbounded<ThreadStreamEvent>();
+ private readonly Channel<ThreadStreamEvent> _events = Channel.CreateBounded<ThreadStreamEvent>(
+ new BoundedChannelOptions(256)
+ {
+ SingleReader = true,
+ SingleWriter = false,
+ FullMode = BoundedChannelFullMode.Wait,
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private readonly Channel<ThreadStreamEvent> _events = Channel.CreateUnbounded<ThreadStreamEvent>(); | |
| private readonly Channel<ThreadStreamEvent> _events = Channel.CreateBounded<ThreadStreamEvent>( | |
| new BoundedChannelOptions(256) | |
| { | |
| SingleReader = true, | |
| SingleWriter = false, | |
| FullMode = BoundedChannelFullMode.Wait, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Qyl.ChatKit/AgentContext.cs` at line 22, The field _events currently uses
Channel.CreateUnbounded<ThreadStreamEvent>(), which allows unlimited queue
growth; change it to a bounded channel to apply backpressure (replace
Channel.CreateUnbounded with Channel.CreateBounded using a BoundedChannelOptions
instance), pick a sensible capacity (e.g., 100) and set SingleReader = true /
SingleWriter = true plus BoundedChannelFullMode.Wait so slow/disconnected SSE
clients block the producer rather than exhausting memory; update any
producer/consumer usage of _events to handle the bounded write semantics (e.g.,
awaiting WriteAsync) in the ThreadStreamEvent producers and the SSE consumer.
| public async ValueTask StartWorkflowAsync(Workflow workflow, CancellationToken ct = default) | ||
| { | ||
| WorkflowItem = new WorkflowItem | ||
| { | ||
| Id = GenerateId(StoreItemType.Workflow), | ||
| CreatedAt = TimeProvider.GetUtcNow().UtcDateTime, | ||
| Workflow = workflow, | ||
| ThreadId = Thread.Id, | ||
| }; | ||
|
|
||
| if (workflow.Type != "reasoning" && workflow.Tasks.Count == 0) | ||
| return; // Defer sending added event until we have tasks | ||
|
|
||
| await StreamAsync(new ThreadItemAddedEvent { Item = WorkflowItem }, ct); | ||
| } |
There was a problem hiding this comment.
Do not emit thread.item.done for workflows the client never saw.
StartWorkflowAsync defers ThreadItemAddedEvent for empty non-reasoning workflows, but EndWorkflowAsync always sends ThreadItemDoneEvent. An empty custom workflow will therefore complete an unknown item on the client.
One way to fix it
public sealed class AgentContext<TContext>
{
+ private bool _workflowAdded;
+
public async ValueTask StartWorkflowAsync(Workflow workflow, CancellationToken ct = default)
{
WorkflowItem = new WorkflowItem
{
Id = GenerateId(StoreItemType.Workflow),
@@
ThreadId = Thread.Id,
};
- if (workflow.Type != "reasoning" && workflow.Tasks.Count == 0)
+ _workflowAdded = workflow.Type == "reasoning" || workflow.Tasks.Count > 0;
+ if (!_workflowAdded)
return; // Defer sending added event until we have tasks
await StreamAsync(new ThreadItemAddedEvent { Item = WorkflowItem }, ct);
}
@@
public async ValueTask EndWorkflowAsync(
WorkflowSummary? summary = null, bool expanded = false, CancellationToken ct = default)
{
if (WorkflowItem is null)
return;
@@
WorkflowItem = WorkflowItem with
{
Workflow = WorkflowItem.Workflow with
{
Summary = finalSummary,
Expanded = expanded,
},
};
+ if (!_workflowAdded)
+ await StreamAsync(new ThreadItemAddedEvent { Item = WorkflowItem }, ct);
+
await StreamAsync(new ThreadItemDoneEvent { Item = WorkflowItem }, ct);
+ _workflowAdded = false;
WorkflowItem = null;
}
}Also applies to: 141-165
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Qyl.ChatKit/AgentContext.cs` around lines 64 - 78, StartWorkflowAsync
currently defers emitting ThreadItemAddedEvent for empty non-"reasoning"
workflows but EndWorkflowAsync always emits ThreadItemDoneEvent, causing clients
to see completions for items they never saw; modify the logic so
EndWorkflowAsync only emits ThreadItemDoneEvent for WorkflowItem instances that
were previously announced. Concretely, add a Boolean flag/property (e.g.,
WasAnnounced) to WorkflowItem when you call StreamAsync(new ThreadItemAddedEvent
{...}) in StartWorkflowAsync (or set it to true when workflow.Type ==
"reasoning" or workflow.Tasks.Count > 0), and in EndWorkflowAsync check that
flag (or the same condition) before sending ThreadItemDoneEvent; apply the same
pattern to the analogous block referenced at the 141-165 region.
| [JsonPropertyName("size")] | ||
| public required int Size { get; init; } |
There was a problem hiding this comment.
int Size limits attachments to ~2GB.
File sizes routinely exceed 2GB. Use long for size fields to match standard .NET file APIs (FileInfo.Length, Stream.Length).
Proposed fix
[JsonPropertyName("size")]
- public required int Size { get; init; }
+ public required long Size { get; init; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [JsonPropertyName("size")] | |
| public required int Size { get; init; } | |
| [JsonPropertyName("size")] | |
| public required long Size { get; init; } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Qyl.ChatKit/Attachments.cs` around lines 73 - 74, Change the Size
property's type from int to long to support files >2GB: update the declaration
public required int Size { get; init; } to use long (public required long Size {
get; init; }), then search for and update all usages, method signatures,
serialization/deserialization expectations, and any comparisons or math that
assume int (e.g., code referencing Size, constructors, DTO mappings, and unit
tests) so they accept long and preserve JSON name "size" via the existing
JsonPropertyName attribute.
| }; | ||
|
|
||
| await Task.CompletedTask; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove dead await Task.CompletedTask.
In an async IAsyncEnumerable method that already yield returns, the trailing await Task.CompletedTask is a no-op.
Proposed fix
};
-
- await Task.CompletedTask;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }; | |
| await Task.CompletedTask; | |
| } | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Qyl.ChatKit/WidgetDiff.cs` around lines 86 - 89, Remove the redundant
trailing await Task.CompletedTask in the async IAsyncEnumerable method (the
method that yields results and currently ends with "await Task.CompletedTask");
simply delete that statement so the method returns by completing the iterator
after its yields—no other changes needed.
| private static IEnumerable<WidgetComponentBase>? GetChildren(WidgetComponentBase component) => | ||
| component switch | ||
| { | ||
| Card card => card.Children, | ||
| ListView listView => listView.Children, | ||
| BoxLayoutProps box => box.Children, | ||
| Transition transition => transition.Children is not null | ||
| ? [transition.Children] | ||
| : null, | ||
| ListViewItem lvi => lvi.Children, | ||
| _ => null, | ||
| }; |
There was a problem hiding this comment.
BasicRoot and DynamicWidgetComponent children not traversed.
GetChildren omits BasicRoot (confirmed in WidgetRoots.cs:57-64) and DynamicWidgetComponent. Streaming text components nested inside these roots won't be detected, causing DiffWidget to miss deltas or incorrectly trigger full replacement.
Since Children is object?, you'll need to check if it's IEnumerable<WidgetComponentBase> at runtime or serialize/deserialize.
Proposed fix sketch
private static IEnumerable<WidgetComponentBase>? GetChildren(WidgetComponentBase component) =>
component switch
{
Card card => card.Children,
ListView listView => listView.Children,
+ BasicRoot basic => basic.Children as IEnumerable<WidgetComponentBase>,
+ DynamicWidgetComponent dyn => dyn.Children as IEnumerable<WidgetComponentBase>,
BoxLayoutProps box => box.Children,
Transition transition => transition.Children is not null
? [transition.Children]
: null,
ListViewItem lvi => lvi.Children,
_ => null,
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Qyl.ChatKit/WidgetDiff.cs` around lines 302 - 313, GetChildren currently
skips BasicRoot and DynamicWidgetComponent and doesn't handle Children typed as
object, so add cases for BasicRoot and DynamicWidgetComponent in the GetChildren
method and handle their Children by runtime-inspecting the object: if component
is BasicRoot br or DynamicWidgetComponent d then examine br.Children/d.Children
and if it is IEnumerable<WidgetComponentBase> return it, if it is a single
WidgetComponentBase return a single-item enumerable, otherwise return null;
preserve the existing Transition single-child logic and reuse the same
pattern-matching/casting approach to avoid missing nested streaming text
components.
| [JsonPropertyName("xAxis")] | ||
| public required object XAxis { get; init; } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
When System.Text.Json deserializes a property declared as object, what runtime type is produced for a JSON object value?
💡 Result:
When System.Text.Json deserializes a JSON object value into a property declared as object, the runtime type produced is JsonElement.
Citations:
- 1: https://ryansouthgate.com/custom-json-serialisation-for-specific-property/
- 2: https://stackoverflow.com/questions/75871653/how-to-deserialize-an-anonymous-object-using-c-system-text-json-and-access-its
- 3: https://stackoverflow.com/questions/78844869/system-text-json-deserializing-to-a-class-with-an-object-property-always-yields
- 4: https://stackoverflow.com/questions/59799836/how-to-get-value-of-object-inside-object-using-system-text-json-in-net-core-3-1
- 5: https://www.reddit.com/r/dotnet/comments/w1f920/net_6_rest_api_is_deserializing_property_of/
🏁 Script executed:
cat -n src/Qyl.ChatKit/Widgets/ChartTypes.cs | head -80Repository: ANcpLua/netagents
Length of output: 3006
🏁 Script executed:
rg "XAxis" src/Qyl.ChatKit/ -B 2 -A 2Repository: ANcpLua/netagents
Length of output: 691
🏁 Script executed:
rg "new Chart\(|Chart {|ChartTypes" . --type cs -B 1 -A 3 | head -50Repository: ANcpLua/netagents
Length of output: 192
🏁 Script executed:
rg "new Chart" . --type cs -B 1 -A 3Repository: ANcpLua/netagents
Length of output: 43
🏁 Script executed:
find . -type f -name "*.cs" -path "*/test*" -o -name "*.json" | head -20Repository: ANcpLua/netagents
Length of output: 1028
🏁 Script executed:
rg "xAxis" . -B 1 -A 1Repository: ANcpLua/netagents
Length of output: 261
Change XAxis from object to XAxisConfig.
The object type causes System.Text.Json to deserialize JSON objects into JsonElement, losing the XAxisConfig shape. Since XAxisConfig is the only defined axis type, use it directly instead of weakening the public contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Qyl.ChatKit/Widgets/ChartTypes.cs` around lines 68 - 69, The XAxis
property currently typed as object should be changed to XAxisConfig so
System.Text.Json preserves the concrete shape; update the declaration in
ChartTypes (the property named XAxis) to use XAxisConfig instead of object
(keeping the [JsonPropertyName("xAxis")] attribute and the required init
accessor) and adjust any callers/tests that construct or access ChartTypes.XAxis
to use XAxisConfig instances or casts as needed.
| /// <summary>Shared layout properties for flexible container widgets (Box, Row, Col, Form).</summary> | ||
| public abstract record BoxLayoutProps : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("children")] | ||
| public IReadOnlyList<WidgetComponentBase>? Children { get; init; } | ||
|
|
||
| [JsonPropertyName("align")] | ||
| public Alignment? Align { get; init; } | ||
|
|
||
| [JsonPropertyName("justify")] | ||
| public Justification? Justify { get; init; } | ||
|
|
||
| [JsonPropertyName("wrap")] | ||
| public string? Wrap { get; init; } | ||
|
|
||
| [JsonPropertyName("flex")] | ||
| public object? Flex { get; init; } | ||
|
|
||
| [JsonPropertyName("gap")] | ||
| public object? Gap { get; init; } | ||
|
|
||
| [JsonPropertyName("height")] | ||
| public object? Height { get; init; } | ||
|
|
||
| [JsonPropertyName("width")] | ||
| public object? Width { get; init; } | ||
|
|
||
| [JsonPropertyName("size")] | ||
| public object? Size { get; init; } | ||
|
|
||
| [JsonPropertyName("minHeight")] | ||
| public object? MinHeight { get; init; } | ||
|
|
||
| [JsonPropertyName("minWidth")] | ||
| public object? MinWidth { get; init; } | ||
|
|
||
| [JsonPropertyName("minSize")] | ||
| public object? MinSize { get; init; } | ||
|
|
||
| [JsonPropertyName("maxHeight")] | ||
| public object? MaxHeight { get; init; } | ||
|
|
||
| [JsonPropertyName("maxWidth")] | ||
| public object? MaxWidth { get; init; } | ||
|
|
||
| [JsonPropertyName("maxSize")] | ||
| public object? MaxSize { get; init; } | ||
|
|
||
| [JsonPropertyName("padding")] | ||
| public object? Padding { get; init; } | ||
|
|
||
| [JsonPropertyName("margin")] | ||
| public object? Margin { get; init; } | ||
|
|
||
| [JsonPropertyName("border")] | ||
| public object? Border { get; init; } | ||
|
|
||
| [JsonPropertyName("radius")] | ||
| public RadiusValue? Radius { get; init; } | ||
|
|
||
| [JsonPropertyName("background")] | ||
| public object? Background { get; init; } | ||
|
|
||
| [JsonPropertyName("aspectRatio")] | ||
| public object? AspectRatio { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Plain text with typography controls.</summary> | ||
| public sealed record Text : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("value")] | ||
| public required string Value { get; init; } | ||
|
|
||
| [JsonPropertyName("streaming")] | ||
| public bool? Streaming { get; init; } | ||
|
|
||
| [JsonPropertyName("italic")] | ||
| public bool? Italic { get; init; } | ||
|
|
||
| [JsonPropertyName("lineThrough")] | ||
| public bool? LineThrough { get; init; } | ||
|
|
||
| [JsonPropertyName("color")] | ||
| public object? Color { get; init; } | ||
|
|
||
| [JsonPropertyName("weight")] | ||
| public string? Weight { get; init; } | ||
|
|
||
| [JsonPropertyName("width")] | ||
| public object? Width { get; init; } | ||
|
|
||
| [JsonPropertyName("size")] | ||
| public TextSize? Size { get; init; } | ||
|
|
||
| [JsonPropertyName("textAlign")] | ||
| public TextAlign? TextAlign { get; init; } | ||
|
|
||
| [JsonPropertyName("truncate")] | ||
| public bool? Truncate { get; init; } | ||
|
|
||
| [JsonPropertyName("minLines")] | ||
| public int? MinLines { get; init; } | ||
|
|
||
| [JsonPropertyName("maxLines")] | ||
| public int? MaxLines { get; init; } | ||
|
|
||
| [JsonPropertyName("editable")] | ||
| public object? Editable { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Prominent headline text.</summary> | ||
| public sealed record Title : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("value")] | ||
| public required string Value { get; init; } | ||
|
|
||
| [JsonPropertyName("color")] | ||
| public object? Color { get; init; } | ||
|
|
||
| [JsonPropertyName("weight")] | ||
| public string? Weight { get; init; } | ||
|
|
||
| [JsonPropertyName("size")] | ||
| public TitleSize? Size { get; init; } | ||
|
|
||
| [JsonPropertyName("textAlign")] | ||
| public TextAlign? TextAlign { get; init; } | ||
|
|
||
| [JsonPropertyName("truncate")] | ||
| public bool? Truncate { get; init; } | ||
|
|
||
| [JsonPropertyName("maxLines")] | ||
| public int? MaxLines { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Supporting caption text.</summary> | ||
| public sealed record Caption : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("value")] | ||
| public required string Value { get; init; } | ||
|
|
||
| [JsonPropertyName("color")] | ||
| public object? Color { get; init; } | ||
|
|
||
| [JsonPropertyName("weight")] | ||
| public string? Weight { get; init; } | ||
|
|
||
| [JsonPropertyName("size")] | ||
| public CaptionSize? Size { get; init; } | ||
|
|
||
| [JsonPropertyName("textAlign")] | ||
| public TextAlign? TextAlign { get; init; } | ||
|
|
||
| [JsonPropertyName("truncate")] | ||
| public bool? Truncate { get; init; } | ||
|
|
||
| [JsonPropertyName("maxLines")] | ||
| public int? MaxLines { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Markdown content, optionally streamed.</summary> | ||
| public sealed record Markdown : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("value")] | ||
| public required string Value { get; init; } | ||
|
|
||
| [JsonPropertyName("streaming")] | ||
| public bool? Streaming { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Small badge indicating status or categorization.</summary> | ||
| public sealed record Badge : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("label")] | ||
| public required string Label { get; init; } | ||
|
|
||
| [JsonPropertyName("color")] | ||
| public string? Color { get; init; } | ||
|
|
||
| [JsonPropertyName("variant")] | ||
| public string? Variant { get; init; } | ||
|
|
||
| [JsonPropertyName("size")] | ||
| public string? Size { get; init; } | ||
|
|
||
| [JsonPropertyName("pill")] | ||
| public bool? Pill { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Generic flex container with direction control.</summary> | ||
| public sealed record Box : BoxLayoutProps | ||
| { | ||
| [JsonPropertyName("direction")] | ||
| public string? Direction { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Horizontal flex container.</summary> | ||
| public sealed record Row : BoxLayoutProps; | ||
|
|
||
| /// <summary>Vertical flex container.</summary> | ||
| public sealed record Col : BoxLayoutProps; | ||
|
|
||
| /// <summary>Form wrapper capable of submitting onSubmitAction.</summary> | ||
| public sealed record Form : BoxLayoutProps | ||
| { | ||
| [JsonPropertyName("onSubmitAction")] | ||
| public ActionConfig? OnSubmitAction { get; init; } | ||
|
|
||
| [JsonPropertyName("direction")] | ||
| public string? Direction { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Visual divider separating content sections.</summary> | ||
| public sealed record Divider : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("color")] | ||
| public object? Color { get; init; } | ||
|
|
||
| [JsonPropertyName("size")] | ||
| public object? Size { get; init; } | ||
|
|
||
| [JsonPropertyName("spacing")] | ||
| public object? Spacing { get; init; } | ||
|
|
||
| [JsonPropertyName("flush")] | ||
| public bool? Flush { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Icon component referencing a built-in icon name.</summary> | ||
| public sealed record Icon : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("name")] | ||
| public required string Name { get; init; } | ||
|
|
||
| [JsonPropertyName("color")] | ||
| public object? Color { get; init; } | ||
|
|
||
| [JsonPropertyName("size")] | ||
| public IconSize? Size { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Image component with sizing and fitting controls.</summary> | ||
| public sealed record Image : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("src")] | ||
| public required string Src { get; init; } | ||
|
|
||
| [JsonPropertyName("alt")] | ||
| public string? Alt { get; init; } | ||
|
|
||
| [JsonPropertyName("fit")] | ||
| public string? Fit { get; init; } | ||
|
|
||
| [JsonPropertyName("position")] | ||
| public string? Position { get; init; } | ||
|
|
||
| [JsonPropertyName("radius")] | ||
| public RadiusValue? Radius { get; init; } | ||
|
|
||
| [JsonPropertyName("frame")] | ||
| public bool? Frame { get; init; } | ||
|
|
||
| [JsonPropertyName("flush")] | ||
| public bool? Flush { get; init; } | ||
|
|
||
| [JsonPropertyName("height")] | ||
| public object? Height { get; init; } | ||
|
|
||
| [JsonPropertyName("width")] | ||
| public object? Width { get; init; } | ||
|
|
||
| [JsonPropertyName("size")] | ||
| public object? Size { get; init; } | ||
|
|
||
| [JsonPropertyName("minHeight")] | ||
| public object? MinHeight { get; init; } | ||
|
|
||
| [JsonPropertyName("minWidth")] | ||
| public object? MinWidth { get; init; } | ||
|
|
||
| [JsonPropertyName("minSize")] | ||
| public object? MinSize { get; init; } | ||
|
|
||
| [JsonPropertyName("maxHeight")] | ||
| public object? MaxHeight { get; init; } | ||
|
|
||
| [JsonPropertyName("maxWidth")] | ||
| public object? MaxWidth { get; init; } | ||
|
|
||
| [JsonPropertyName("maxSize")] | ||
| public object? MaxSize { get; init; } | ||
|
|
||
| [JsonPropertyName("margin")] | ||
| public object? Margin { get; init; } | ||
|
|
||
| [JsonPropertyName("background")] | ||
| public object? Background { get; init; } | ||
|
|
||
| [JsonPropertyName("aspectRatio")] | ||
| public object? AspectRatio { get; init; } | ||
|
|
||
| [JsonPropertyName("flex")] | ||
| public object? Flex { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Button component optionally wired to an action.</summary> | ||
| public sealed record Button : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("submit")] | ||
| public bool? Submit { get; init; } | ||
|
|
||
| [JsonPropertyName("label")] | ||
| public string? Label { get; init; } | ||
|
|
||
| [JsonPropertyName("onClickAction")] | ||
| public ActionConfig? OnClickAction { get; init; } | ||
|
|
||
| [JsonPropertyName("iconStart")] | ||
| public string? IconStart { get; init; } | ||
|
|
||
| [JsonPropertyName("iconEnd")] | ||
| public string? IconEnd { get; init; } | ||
|
|
||
| [JsonPropertyName("style")] | ||
| public string? Style { get; init; } | ||
|
|
||
| [JsonPropertyName("iconSize")] | ||
| public string? IconSize { get; init; } | ||
|
|
||
| [JsonPropertyName("color")] | ||
| public string? Color { get; init; } | ||
|
|
||
| [JsonPropertyName("variant")] | ||
| public ControlVariant? Variant { get; init; } | ||
|
|
||
| [JsonPropertyName("size")] | ||
| public ControlSize? Size { get; init; } | ||
|
|
||
| [JsonPropertyName("pill")] | ||
| public bool? Pill { get; init; } | ||
|
|
||
| [JsonPropertyName("uniform")] | ||
| public bool? Uniform { get; init; } | ||
|
|
||
| [JsonPropertyName("block")] | ||
| public bool? Block { get; init; } | ||
|
|
||
| [JsonPropertyName("disabled")] | ||
| public bool? Disabled { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Flexible spacer used to push content apart.</summary> | ||
| public sealed record Spacer : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("minSize")] | ||
| public object? MinSize { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Select dropdown component.</summary> | ||
| public sealed record Select : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("name")] | ||
| public required string Name { get; init; } | ||
|
|
||
| [JsonPropertyName("options")] | ||
| public required IReadOnlyList<SelectOption> Options { get; init; } | ||
|
|
||
| [JsonPropertyName("onChangeAction")] | ||
| public ActionConfig? OnChangeAction { get; init; } | ||
|
|
||
| [JsonPropertyName("placeholder")] | ||
| public string? Placeholder { get; init; } | ||
|
|
||
| [JsonPropertyName("defaultValue")] | ||
| public string? DefaultValue { get; init; } | ||
|
|
||
| [JsonPropertyName("variant")] | ||
| public ControlVariant? Variant { get; init; } | ||
|
|
||
| [JsonPropertyName("size")] | ||
| public ControlSize? Size { get; init; } | ||
|
|
||
| [JsonPropertyName("pill")] | ||
| public bool? Pill { get; init; } | ||
|
|
||
| [JsonPropertyName("block")] | ||
| public bool? Block { get; init; } | ||
|
|
||
| [JsonPropertyName("clearable")] | ||
| public bool? Clearable { get; init; } | ||
|
|
||
| [JsonPropertyName("disabled")] | ||
| public bool? Disabled { get; init; } | ||
|
|
||
| [JsonPropertyName("searchable")] | ||
| public bool? Searchable { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Date picker input component.</summary> | ||
| public sealed record DatePicker : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("name")] | ||
| public required string Name { get; init; } | ||
|
|
||
| [JsonPropertyName("onChangeAction")] | ||
| public ActionConfig? OnChangeAction { get; init; } | ||
|
|
||
| [JsonPropertyName("placeholder")] | ||
| public string? Placeholder { get; init; } | ||
|
|
||
| [JsonPropertyName("defaultValue")] | ||
| public DateTimeOffset? DefaultValue { get; init; } | ||
|
|
||
| [JsonPropertyName("min")] | ||
| public DateTimeOffset? Min { get; init; } | ||
|
|
||
| [JsonPropertyName("max")] | ||
| public DateTimeOffset? Max { get; init; } | ||
|
|
||
| [JsonPropertyName("variant")] | ||
| public ControlVariant? Variant { get; init; } | ||
|
|
||
| [JsonPropertyName("size")] | ||
| public ControlSize? Size { get; init; } | ||
|
|
||
| [JsonPropertyName("side")] | ||
| public string? Side { get; init; } | ||
|
|
||
| [JsonPropertyName("align")] | ||
| public string? Align { get; init; } | ||
|
|
||
| [JsonPropertyName("pill")] | ||
| public bool? Pill { get; init; } | ||
|
|
||
| [JsonPropertyName("block")] | ||
| public bool? Block { get; init; } | ||
|
|
||
| [JsonPropertyName("clearable")] | ||
| public bool? Clearable { get; init; } | ||
|
|
||
| [JsonPropertyName("disabled")] | ||
| public bool? Disabled { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Checkbox input component.</summary> | ||
| public sealed record Checkbox : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("name")] | ||
| public required string Name { get; init; } | ||
|
|
||
| [JsonPropertyName("label")] | ||
| public string? Label { get; init; } | ||
|
|
||
| [JsonPropertyName("defaultChecked")] | ||
| public bool? DefaultChecked { get; init; } | ||
|
|
||
| [JsonPropertyName("onChangeAction")] | ||
| public ActionConfig? OnChangeAction { get; init; } | ||
|
|
||
| [JsonPropertyName("disabled")] | ||
| public bool? Disabled { get; init; } | ||
|
|
||
| [JsonPropertyName("required")] | ||
| public bool? Required { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Single-line text input component.</summary> | ||
| public sealed record Input : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("name")] | ||
| public required string Name { get; init; } | ||
|
|
||
| [JsonPropertyName("inputType")] | ||
| public string? InputType { get; init; } | ||
|
|
||
| [JsonPropertyName("defaultValue")] | ||
| public string? DefaultValue { get; init; } | ||
|
|
||
| [JsonPropertyName("required")] | ||
| public bool? Required { get; init; } | ||
|
|
||
| [JsonPropertyName("pattern")] | ||
| public string? Pattern { get; init; } | ||
|
|
||
| [JsonPropertyName("placeholder")] | ||
| public string? Placeholder { get; init; } | ||
|
|
||
| [JsonPropertyName("allowAutofillExtensions")] | ||
| public bool? AllowAutofillExtensions { get; init; } | ||
|
|
||
| [JsonPropertyName("autoSelect")] | ||
| public bool? AutoSelect { get; init; } | ||
|
|
||
| [JsonPropertyName("autoFocus")] | ||
| public bool? AutoFocus { get; init; } | ||
|
|
||
| [JsonPropertyName("disabled")] | ||
| public bool? Disabled { get; init; } | ||
|
|
||
| [JsonPropertyName("variant")] | ||
| public string? Variant { get; init; } | ||
|
|
||
| [JsonPropertyName("size")] | ||
| public ControlSize? Size { get; init; } | ||
|
|
||
| [JsonPropertyName("gutterSize")] | ||
| public string? GutterSize { get; init; } | ||
|
|
||
| [JsonPropertyName("pill")] | ||
| public bool? Pill { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Form label associated with a field.</summary> | ||
| public sealed record Label : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("value")] | ||
| public required string Value { get; init; } | ||
|
|
||
| [JsonPropertyName("fieldName")] | ||
| public required string FieldName { get; init; } | ||
|
|
||
| [JsonPropertyName("size")] | ||
| public TextSize? Size { get; init; } | ||
|
|
||
| [JsonPropertyName("weight")] | ||
| public string? Weight { get; init; } | ||
|
|
||
| [JsonPropertyName("textAlign")] | ||
| public TextAlign? TextAlign { get; init; } | ||
|
|
||
| [JsonPropertyName("color")] | ||
| public object? Color { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Grouped radio input control.</summary> | ||
| public sealed record RadioGroup : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("name")] | ||
| public required string Name { get; init; } | ||
|
|
||
| [JsonPropertyName("options")] | ||
| public IReadOnlyList<RadioOption>? Options { get; init; } | ||
|
|
||
| [JsonPropertyName("ariaLabel")] | ||
| public string? AriaLabel { get; init; } | ||
|
|
||
| [JsonPropertyName("onChangeAction")] | ||
| public ActionConfig? OnChangeAction { get; init; } | ||
|
|
||
| [JsonPropertyName("defaultValue")] | ||
| public string? DefaultValue { get; init; } | ||
|
|
||
| [JsonPropertyName("direction")] | ||
| public string? Direction { get; init; } | ||
|
|
||
| [JsonPropertyName("disabled")] | ||
| public bool? Disabled { get; init; } | ||
|
|
||
| [JsonPropertyName("required")] | ||
| public bool? Required { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Multiline text input component.</summary> | ||
| public sealed record Textarea : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("name")] | ||
| public required string Name { get; init; } | ||
|
|
||
| [JsonPropertyName("defaultValue")] | ||
| public string? DefaultValue { get; init; } | ||
|
|
||
| [JsonPropertyName("required")] | ||
| public bool? Required { get; init; } | ||
|
|
||
| [JsonPropertyName("pattern")] | ||
| public string? Pattern { get; init; } | ||
|
|
||
| [JsonPropertyName("placeholder")] | ||
| public string? Placeholder { get; init; } | ||
|
|
||
| [JsonPropertyName("autoSelect")] | ||
| public bool? AutoSelect { get; init; } | ||
|
|
||
| [JsonPropertyName("autoFocus")] | ||
| public bool? AutoFocus { get; init; } | ||
|
|
||
| [JsonPropertyName("disabled")] | ||
| public bool? Disabled { get; init; } | ||
|
|
||
| [JsonPropertyName("variant")] | ||
| public string? Variant { get; init; } | ||
|
|
||
| [JsonPropertyName("size")] | ||
| public ControlSize? Size { get; init; } | ||
|
|
||
| [JsonPropertyName("gutterSize")] | ||
| public string? GutterSize { get; init; } | ||
|
|
||
| [JsonPropertyName("rows")] | ||
| public int? Rows { get; init; } | ||
|
|
||
| [JsonPropertyName("autoResize")] | ||
| public bool? AutoResize { get; init; } | ||
|
|
||
| [JsonPropertyName("maxRows")] | ||
| public int? MaxRows { get; init; } | ||
|
|
||
| [JsonPropertyName("allowAutofillExtensions")] | ||
| public bool? AllowAutofillExtensions { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Wrapper enabling transitions for a child component.</summary> | ||
| public sealed record Transition : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("children")] | ||
| public WidgetComponentBase? Children { get; init; } | ||
| } | ||
|
|
||
| /// <summary>Single row inside a ListView component.</summary> | ||
| public sealed record ListViewItem : WidgetComponentBase | ||
| { | ||
| [JsonPropertyName("children")] | ||
| public required IReadOnlyList<WidgetComponentBase> Children { get; init; } | ||
|
|
||
| [JsonPropertyName("onClickAction")] | ||
| public ActionConfig? OnClickAction { get; init; } | ||
|
|
||
| [JsonPropertyName("gap")] | ||
| public object? Gap { get; init; } | ||
|
|
||
| [JsonPropertyName("align")] | ||
| public Alignment? Align { get; init; } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Document the widget DTO properties.
This adds a large public contract, but most component properties have no XML docs. That is especially rough for the many object? and string-valued props, where IntelliSense is the only discoverability most consumers will have.
As per coding guidelines, "Flag new public API surface that lacks XML doc comments."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Qyl.ChatKit/Widgets/WidgetComponents.cs` around lines 5 - 637, The new
public widget DTOs (e.g., BoxLayoutProps, Text, Title, Image, Button, Select,
Input, DatePicker, Checkbox, Transition, ListViewItem and their public
properties like Children, Align, Flex, Gap, Width, Color, Size, Src, Label,
OnClickAction, DefaultValue, etc.) are missing XML documentation for many
properties (notably object? and string? typed props); add concise XML doc
comments for each public record and every public property explaining intent,
allowed values/formats, units/semantics for ambiguous object/string fields, and
examples or <remarks> where applicable, and for those uncertain fields add a
TODO/remarks tag to flag for API review per the coding guideline about flagging
new public surface without docs. Ensure summaries appear on the record types
(e.g., BoxLayoutProps, Text, Image, Button) and for ambiguous properties (Flex,
Gap, Height, Width, Background, Color, Variant, Size, InputType, Align,
Direction, etc.) so IntelliSense consumers get clear guidance.
| [JsonPolymorphic] | ||
| [JsonDerivedType(typeof(CustomSummary), typeDiscriminator: "custom")] | ||
| [JsonDerivedType(typeof(DurationSummary), typeDiscriminator: "duration")] | ||
| public abstract record WorkflowSummary; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the default type discriminator property name for System.Text.Json JsonPolymorphicAttribute when TypeDiscriminatorPropertyName is omitted?
💡 Result:
The default type discriminator property name for System.Text.Json's JsonPolymorphicAttribute, when TypeDiscriminatorPropertyName is omitted, is "$type".
Citations:
- 1: https://learn.microsoft.com/en-us/dotnet/api/system.text.json.serialization.jsonpolymorphicattribute.typediscriminatorpropertyname?view=net-10.0
- 2: https://learn.microsoft.com/en-us/dotnet/api/system.text.json.serialization.metadata.jsonpolymorphismoptions.typediscriminatorpropertyname?view=net-10.0
- 3: https://docs.microsoft.com/en-us/dotnet/api/system.text.json.serialization.jsonpolymorphicattribute
- 4: https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/polymorphism
- 5: https://learn.microsoft.com/en-us/dotnet/api/system.text.json.serialization.metadata.jsonpolymorphismoptions?view=net-10.0
🏁 Script executed:
# First, let's see the context around WorkflowSummary and understand its usage
find . -name "Workflows.cs" -type f | head -5Repository: ANcpLua/netagents
Length of output: 91
🏁 Script executed:
# Search for other JsonPolymorphic attributes in the codebase
rg -B 2 -A 5 "\[JsonPolymorphic\]" --type csRepository: ANcpLua/netagents
Length of output: 615
🏁 Script executed:
# Search for usages of WorkflowSummary to understand where it's serialized
rg "WorkflowSummary" --type cs | head -20Repository: ANcpLua/netagents
Length of output: 503
🏁 Script executed:
# Check if there's a Workflow class that contains Summary property
rg -B 5 -A 10 "class Workflow|record Workflow" src/Qyl.ChatKit/Workflows.cs | head -40Repository: ANcpLua/netagents
Length of output: 1184
🏁 Script executed:
# Search for JsonPolymorphic usage more broadly in the entire codebase
rg "JsonPolymorphic|JsonDerivedType" --type csRepository: ANcpLua/netagents
Length of output: 10380
🏁 Script executed:
# Check if there's any JsonSerializerOptions configuration that sets a default discriminator
rg "TypeDiscriminatorPropertyName|JsonSerializerOptions|JsonSerializerContext" --type cs | head -20Repository: ANcpLua/netagents
Length of output: 2224
🏁 Script executed:
# Look for other polymorphic types or serialization patterns to understand the convention
rg "JsonDerivedType|typeDiscriminator" --type cs -B 2 -A 2Repository: ANcpLua/netagents
Length of output: 13067
Add TypeDiscriminatorPropertyName = "type" to WorkflowSummary.
[JsonPolymorphic] defaults to "$type", while all other polymorphic models in ChatKit (ChatKitTask, ThreadStreamEvent, ThreadItem, etc.) use "type". Without fixing this, Workflow.Summary will serialize with the wrong discriminator and fail deserialization.
Suggested fix
-[JsonPolymorphic]
+[JsonPolymorphic(TypeDiscriminatorPropertyName = "type")]
[JsonDerivedType(typeof(CustomSummary), typeDiscriminator: "custom")]
[JsonDerivedType(typeof(DurationSummary), typeDiscriminator: "duration")]
public abstract record WorkflowSummary;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [JsonPolymorphic] | |
| [JsonDerivedType(typeof(CustomSummary), typeDiscriminator: "custom")] | |
| [JsonDerivedType(typeof(DurationSummary), typeDiscriminator: "duration")] | |
| public abstract record WorkflowSummary; | |
| [JsonPolymorphic(TypeDiscriminatorPropertyName = "type")] | |
| [JsonDerivedType(typeof(CustomSummary), typeDiscriminator: "custom")] | |
| [JsonDerivedType(typeof(DurationSummary), typeDiscriminator: "duration")] | |
| public abstract record WorkflowSummary; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Qyl.ChatKit/Workflows.cs` around lines 23 - 26, WorkflowSummary's
JsonPolymorphic attribute uses the default discriminator ("$type") which is
inconsistent with other polymorphic models and will break deserialization;
update the attribute on the abstract record WorkflowSummary to specify
TypeDiscriminatorPropertyName = "type" so its derived types (CustomSummary,
DurationSummary) serialize/deserialize with the "type" discriminator (ensure
this change aligns with Workflow.Summary usage).
Summary
Qyl.ChatKitprojectTest plan
dotnet buildpasses forQyl.ChatKitQyl.Agents.Testsstill pass🤖 Generated with Claude Code