Skip to content

Migrate web coach to AG-UI and Blazor AI components - #215

Draft
danroth27 wants to merge 4 commits into
davidortinau:mainfrom
danroth27:copilot/web-coach-agui-blazor-ai
Draft

danroth27 wants to merge 4 commits into
davidortinau:mainfrom
danroth27:copilot/web-coach-agui-blazor-ai

Conversation

@danroth27

@danroth27 danroth27 commented Nov 2, 2026 •

Copy link
Copy Markdown

Summary

Migrate the web learning coach to AG-UI turn transport and the experimental Blazor AI components, without changing MAUI client defaults.

  • Add an authenticated AG-UI endpoint and SDK client adapter over the existing application services. Preserve ownership checks, validated output, durable history, idempotency, approvals, and recovery; keep REST for resource operations.
  • Use UIAgent, AgentContext, MessageList, MessageInput, and typed domain block rendering. A thin boundary owns the context lifetime to clear stale preview-library retry state without remounting the application shell.
  • Handle incomplete or malformed AG-UI streams as recoverable failures, releasing the workspace busy state while preserving turn and operation identities for retry.
  • Add optional shared UI slots so native clients retain their existing transcript, composer, and REST transport.
  • Share domain timeline rendering through a typed RenderFragment and ordinary EventCallback/parameter flow. Remove duplicate web message/input wrappers, the transcript-action record, and the history event bus. The API references only AGUI.Abstractions and uses ASP.NET Core's SSE result.
  • Include focused regressions and an evaluation in docs/agui-blazor-ai-evaluation.md.

Also fix the coach host's YouTube handler dependency to use the registered interface, and release fixture-specific SQLite pools before deleting temporary test databases on Windows.

Validation

90 focused regression cases passed: 79 UI/workspace/client cases, four AG-UI client cases, and seven API cases. Overlapping tests were consolidated into mounted tests of the real transcript/composer composition. Recovery regressions exercise conversation switching, deletion, external recovery, stale error removal, subsequent submission, and incomplete-stream retry. API/web builds used existing SDKs with mobile targets disabled; no MAUI workloads were installed.

A broader rendering selection passed 226 cases and encountered 47 existing fixture failures: several fixtures omit the NavigationManager required by CoachVocabularySetCard, which the transcript already rendered on main. Those fixtures are unchanged by this PR.

Browser checks covered submission, intentional repeated questions, refresh/restored history, response-report controls, conversation switching, and a narrow viewport.

Live verification used an approved existing Foundry gpt-5-mini deployment and the Azure CLI identity, with real local HTTP web/API hosts and the production cookie-to-JWT forwarding path. Three learner turns made four successful inference HTTP requests; history survived full refresh. Only disposable local data was used, and temporary resources were cleaned up.

Repeated browser/live verification after simplification at 7418226e: seven persisted learner turns across two conversations made eight successful inference requests. Library retry and workspace retry recovered deliberately truncated responses with identical operation handles, no duplicate learner turns, and no extra inference calls. Injected HTTP failures verified cross-conversation error isolation and external dismissal; subsequent live submissions and refreshed history worked. Also rechecked report-panel cancellation/focus, 390-pixel layout, and browser console errors. Disposable hosts, databases, container, and generated sign-in artifacts were cleaned up.

Limitations and findings

Validated answers remain buffered until the reducer completes; this is not model-token streaming. Blazor AI is experimental, has composer-customization gaps, and currently requires remounting only the message list after canonical history replacement.

The live checks exposed an evidence-window validation rejection (EvidenceWindow:window_length) and an incorrect Korean grammatical-form explanation. The relevant coach instructions and validation were not changed by this migration, so these appear to be existing coach/model issues; an unmodified-baseline reproduction has not been performed. They are documented rather than fixed here.

The post-simplification recheck reproduced the empty-vocabulary validation rejection, but the model correctly explained the grammatical form this time. A minor transcript UX limitation was also observed: an abandoned local question remained visible after dismissal and the next successful turn until refresh; it was not saved and had no actionable retry. Whether that behavior predates the migration has not been established.

Local live verification is not a deployed-environment check, a production database/key-storage check, a latency benchmark, or a broad model-quality sign-off.

Preserve the application reducer, durable history, protected write flows, and native client defaults. Add focused regression coverage and an evaluation including live Foundry verification.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f0757936-2562-4fe8-9414-8e987c148e1f
@danroth27
danroth27 marked this pull request as draft November 2, 2026 01:03
danroth27 and others added 3 commits November 1, 2026 17:08
Replace stale agent contexts without remounting the app shell, reject cross-conversation retries, and map incomplete AG-UI responses to recoverable workspace failures. Add mounted-component and transport retry regressions and update the evaluation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f0757936-2562-4fe8-9414-8e987c148e1f
Share domain timeline rendering through Blazor templates and EventCallback parameters. Remove duplicate web wrappers, the history event bus, and overlapping tests. Use only the AG-UI protocol dependency on the API and retain the context reset needed by the experimental library.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f0757936-2562-4fe8-9414-8e987c148e1f
Record post-simplification inference, history, recovery, layout, and remaining coach/transcript findings.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: f0757936-2562-4fe8-9414-8e987c148e1f
@danroth27

Copy link
Copy Markdown
Author

Review summary: appropriate scenario, but request changes on the current integration

Reviewed commit 773741553e0345fc35f0c86fad25550cb1138aa5 and docs/agui-blazor-ai-evaluation.md.

The coach is a legitimate use case for AG-UI and Blazor AI components. The concern is not that SentenceStudio lacks an agentic UI; it is that this implementation introduces regressions and has not yet established a net improvement over the existing integration.

Scope and architectural conclusions

  • SentenceStudio already has a tool-using learning coach, exposed through /coach and the Sam overlay. It is one logical Microsoft Agent Framework agent with alternative baseline and harness implementations, not a multi-agent system. The application owns validation, history, approvals, execution, receipts, and recovery.
  • The separate /conversation practice activity is a custom chat UI using ordinary typed model requests. It reconstructs history in a prompt sent through /api/v1/ai/chat; it does not use MAF and is not migrated by this PR.
  • This PR replaces the web coach's composer, transcript primitives, and conversation lifecycle integration with MessageInput, MessageList, BlockRenderer, UIAgent, and AgentContext. The rich domain cards and substantial workspace behavior remain application-owned. Native clients retain their existing defaults.
  • AG-UI transport, Blazor AI lifecycle/rendering, and replacement of the custom composer are three separable adoption decisions. A problem with the composer does not invalidate AG-UI, and adopting Blazor AI does not require adopting AG-UI.

Implementation findings

Priority Finding Evidence
P1 Malformed AG-UI event ordering can leave the workspace permanently busy, with ordinary library Retry unable to recover. Reproduced: a snapshot before RUN_STARTED causes the SDK to throw InvalidOperationException. CoachAGUITransport does not normalize that failure; the workspace remains Running, CanSubmit is false, and library Retry makes no second transport call.
P2 An over-limit submission loses the learner's text. Reproduced with the mounted real input: submitting 501 characters clears the library textarea before adapter validation. The workspace draft and timeline remain empty and no API submission occurs. Adapter/API length enforcement is not equivalent to preserving the original composer UX.
P2 Unsent drafts can disappear when the input is unmounted, including collapsing Sam or switching to the Plan pane. Source-traced: the library owns the unsent draft; it is not bound to Workspace.Draft. This path was not independently browser-reproduced in this review.
P2 The replacement drops the existing IME-composition and narrow-screen Enter handling. Source-traced: the original composer has explicit composition-aware Enter handling and a mobile newline policy; the host replacement does not retain those protections. Browser/IME reproduction remains necessary.
P2 Sam's existing composer-focus integration no longer targets the replacement input. Source-traced: focus still targets coach-composer, but the replacement MessageInput is not given that ID.
P2 The default message-list footer adds English error/retry UI alongside application-owned recovery controls, with ineffective Retry in some states. Source-traced, with the nonrecovering malformed-stream case reproduced above. Recovery and localization ownership need to be explicit.

Relevant integration points: transport, workspace adapter, component boundary, and original composer/fallback.

Additional hardening observations: streamed RUN_ERROR events are flattened to synthetic HTTP 400 exceptions, and response run/thread identities are not consistently checked. I did not establish a current user-visible status-mapping regression or a current-server trigger for the identity mismatch, so these should not be presented as demonstrated merge-blocking failures.

What the evaluation establishes, and what it does not

The buffering is an application integration choice, not a limitation of AG-UI or Blazor AI. The existing agent runner awaits a complete typed CoachTurnIntent; application validation follows. The new endpoint emits RUN_STARTED, waits for the application operation, and then emits the result snapshot and whole-message text. The web transport waits through completion, and the workspace adapter subsequently yields timeline entries. This is standardized SSE, but not incremental model-answer delivery. Safe progress events could still add value without exposing unvalidated model output.

The request contract remains application-specific. The AG-UI outer request carries the actual command in state.turn or state.durableTurn; ordinary messages do not drive the endpoint. An authenticated messages-only request without that state is rejected. Custom state is valid AG-UI, but another client needs a coach-specific request adapter even for basic submission, not merely custom rendering for rich cards. The documentation should also distinguish rejected client tool declarations from tool-role messages, which the probe found were ignored when valid custom state was supplied.

The evaluation appropriately disclaims token-latency improvement, production verification, and broad model-quality sign-off. Those disclaimers should remain. However, successful integration demonstrates feasibility, not comparative superiority: the added context resets, history projection, remounting, and retry synchronization need to be weighed against responsibilities actually removed. Future extensibility is a reasonable motivation, but not evidence of an already improved user experience. The observed language-quality/grounding failures should not be attributed to this transport change without a baseline comparison.

Where AG-UI is appropriate across the API

API category Recommendation
Coach turn submission Strongest candidate. Use AG-UI for the interactive run boundary while preserving the authoritative application services.
Conversation practice Possible separate adoption. Streaming replies and structured feedback could benefit, but this requires a deliberate contract; wrapping the same completed Reply in a snapshot adds little UX value. MAF is not a prerequisite.
Generic /ai/chat and /ai/chat-messages Do not replace wholesale. Most typed callers need a completed DTO. /chat-messages is a plausible streaming-chat starting point, but no application callers of its wrappers were found under src.
Image analysis, speech, AI-assisted feedback, imports/channel checks Retain specialized request/response or job APIs. An agent may invoke or observe these services without replacing them with AG-UI.
Coach history, operations, approvals/confirmation/undo, memories, and reports Retain durable resource/action contracts. Protocol events do not substitute for authorization, idempotency, persistence, or recovery after disconnection.
Plan generation No current protocol motivation: the LLM-named generator still delegates to deterministic generation.

Recommended disposition

Request changes before merge, rather than reject the technology choice. Fix the demonstrated recovery and text-loss bugs, preserve the existing composer behavior (or retain the custom composer initially), and add committed regression coverage for these cases. Evaluate transport, rendering/lifecycle, and composer replacement independently. Revise the evaluation to distinguish standards-based framing from generic-client interoperability, and integration feasibility from demonstrated maintenance or UX gains.

Review evidence included existing API/client regressions, an authenticated API probe, SDK fault probes, and a 45-case UI/workspace selection containing two mounted adversarial probes. Those probes passed by asserting the defects described above; they do not mean the defects are fixed. The probes were session-only, not committed. No additional browser/live-model verification was performed by this review; the PR's reported live checks are separate evidence.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant