Skip to content

refactor: address PR review feedback and remove deprecated THINKING events#23

Merged
paduh merged 19 commits into
mainfrom
fix/pr-1512-review-gaps
May 1, 2026
Merged

refactor: address PR review feedback and remove deprecated THINKING events#23
paduh merged 19 commits into
mainfrom
fix/pr-1512-review-gaps

Conversation

@paduh

@paduh paduh commented May 1, 2026

Copy link
Copy Markdown
Owner

Summary

  • Addressed 14 PR review items: license headers, dead code, naming, A2UI routing, buffer safety, and more
  • Removed all deprecated THINKING_* event types from the public API; added a transparent backward-compat decoder layer (mirrors TypeScript SDK's BackwardCompatibility_0_0_45 approach) that silently remaps wire-level THINKING_* JSON to REASONING_* events

Changes

Breaking (internal cleanup)

  • THINKING_* event types, DTOs, registry, and tests deleted from AGUICore
  • ThinkingTelemetryState and all thinking fields removed from AgentState, AbstractAgent, HttpAgent
  • AgentBuilders free functions replaced by public enum AgentBuilders namespace

Fixes

  • License headers across 292 files reduced from 23-line MIT block to single copyright line
  • Deleted dead AGUICore placeholder struct and deprecated AGUITools struct (+ its test)
  • Fixed SwiftLint large_tuple: 3 shorthand → proper nested YAML config
  • Fixed unused colIdx variable in A2UIComponentView
  • Moved A2UISurfaceView JSON decode from body to init
  • Replaced fragile "[A2UI Action] {json}" string prefix with typed A2UIActionEnvelope: Encodable
  • Added SseParser.maxBufferByteCount (10 MB) guard to prevent unbounded buffer growth

Additions

  • ToolExecutionResult.decode<T: Decodable>(as:using:) typed convenience accessor
  • Shared MockToolRegistry actor extracted to Tests/AGUIAgentSDKTests/Mocks/
  • .claude/worktrees/ added to .gitignore

Type of Change

  • refactor - Code refactoring
  • fix - Bug fix
  • chore - Build/tooling changes

Testing

  • All tests pass (swift test)
  • Code formatted (swift package plugin swiftformat)
  • SwiftLint passes (swiftlint lint)

Breaking Changes

  • This PR includes breaking changes

Breaking changes:

  • THINKING_* event cases removed from EventType enum — callers pattern-matching on these must migrate to REASONING_* equivalents
  • AgentBuilders free functions replaced by static methods on enum AgentBuilders — call sites must be updated to AgentBuilders.xxx(...)
  • AgentState.thinking, AbstractAgent.thinking, HttpAgent.thinking removed — no replacement (use REASONING_* events directly)

Related Issues

Closes #


Reviewer Checklist

  • Code follows project standards (CLAUDE.md)
  • Tests are comprehensive and pass
  • No unnecessary changes or scope creep
  • Commit messages follow conventions
  • CI checks pass

paduh added 13 commits May 1, 2026 14:07
- Delete AgentConfig and HttpAgentConfig structs (no callsites in
  Sources/ or Tests/; ChatApp has its own separate AgentConfig type)
- Move RunAgentParameters to RunAgentParameters.swift (still used by
  AbstractAgent and HttpAgent — not dead code)
- Remove connectTimeout from AgUiAgentConfig (never read by AgUiAgent;
  only requestTimeout is wired into HttpAgentConfiguration)
- Update plan: remove Bug 5 (RetryPolicy is fully wired end-to-end via
  HttpAgentTransport.shouldRetry/retryDelay — not dead code)
Add mimeType: String? (defaulted nil) to ImageInputContent,
AudioInputContent, and VideoInputContent — matching TypeScript's
InputContentSource.mimeType field. DocumentInputContent already had it.

- Update the three corresponding DTOs to decode and pass mimeType
  through toDomain()
- Update MessageEncoder.encodeUserMessage() to include mimeType in
  the JSON output for image, audio, and video content blocks
- All existing call sites unaffected (new param defaults to nil)
MessageEncoder.defaultRegistry() covered 6 roles; calling encode() on
a ReasoningMessage threw unsupportedRole(.reasoning) at runtime.

- Add private encodeReasoningMessage() (mirrors encodeAssistantMessage:
  serialises id, role, content?, encryptedValue? — omits name, which
  is always nil for ReasoningMessage per spec)
- Register under .reasoning in defaultRegistry()
- Update doc comment: "all 6 message types" → "all 7 message types"
- New MessageEncoderTests: correct JSON output, encryptedValue round-
  trip, name omission, unsupportedRole error path
Covers all 7 reasoning event types (ReasoningStart, ReasoningEnd,
ReasoningMessageStart, ReasoningMessageContent, ReasoningMessageEnd,
ReasoningMessageChunk, ReasoningEncryptedValue) and ReasoningMessage
model — 71 tests, 0 failures.
Mirrors StepStartedEventTests exactly — decode, timestamp, rawEvent,
error cases, model behaviors, and 6 standard EventDecodingErrorTests.
Seven DTO shim files that were pure field-passthrough with no
wire-to-domain transformation are deleted. Their domain types now
conform to Decodable directly:

Events (rawEvent set via withRawEvent helper in registry):
- TextMessageStartEvent, TextMessageContentEvent
- ToolCallStartEvent, ToolCallArgsEvent

Messages (hardcode role in init(from:)):
- AssistantMessage, SystemMessage, DeveloperMessage

Registries updated; 1338 tests pass.
Remove `content: String?` from the Message protocol (breaking change).
Rename `ActivityMessage.activityContent: Data` to `content: Data` to
align the property name with its AG-UI wire-format key. Update all
callers: ActivityMessageDTO.toDomain(), MessageEncoder, RunAgentInput
hash/equality, and the full test suite.
Introduces a typed RunFinishedOutcome enum (COMPLETED / CANCELLED /
MAX_ITERATIONS_REACHED) on RunFinishedEvent. The DTO decoder defaults to
.completed for absent or unrecognised outcome values, keeping forward-
compatibility with future protocol versions. Nine new tests cover each
case, the missing-field default, and the unknown-value fallback.
Covers event passthrough, tool execution lifecycle, arg-delta
concatenation, response handler routing, failure path, execution
events stream ordering, cancellation, and error propagation.
Previous suite covered only .reasoning (4 tests). Added tests for
.developer, .system, .user, .assistant, .tool, .activity, and
.reasoning, verifying mandatory fields, optional field omission,
multimodal content array encoding, toolCalls array shape,
activity content inlining, and invalidMessageType error path.
Replace fire-and-forget Task wrappers with async signatures.
AbstractAgent: use task.result pattern for cleanup; make abortRun
and dispose async. HttpAgent/AgUiAgent: propagate async signatures.
Replace verbose opt-in ruleset with minimal only_rules config
(force_cast, force_try, force_unwrapping, custom_rules, large_tuple)
matching the League iOS SDK baseline. Add no_assume_isolated custom
rule. Re-enable SwiftLint step in CI lint job. Add test-linux job
using swift:latest container on ubuntu-latest.
Add internal transport-injecting init to StatefulAgUiAgent to enable
unit testing without HTTP. Refactor sendMessage to branch between the
injected transport path (tests) and the httpAgent SSE path (production).

Tests cover: RunAgentInput construction, system prompt lifecycle,
multi-round history accumulation, per-thread isolation, history trimming,
clearHistory, event passthrough, text message assembly, tool call
flushing/deduplication, and StateSnapshot/StateDelta tracking.
@paduh paduh self-assigned this May 1, 2026
paduh and others added 6 commits May 1, 2026 17:17
On Linux, URLSession lives in FoundationNetworking, not Foundation,
causing it to resolve as AnyObject. URLSession.AsyncBytes and
session.bytes(for:) are also Apple-only.

Add `import FoundationNetworking` guard in URLSessionHTTPClient and
HttpTransport. On Linux, replace the AsyncBytes streaming path with a
dataTask-based fallback that buffers the full response; true SSE
streaming remains an Apple-platform-only feature.
The SDK targets iOS 15+ / macOS 13+ only. SSE streaming relies on
URLSession.AsyncBytes which is Apple-platform-only. A Linux CI job
adds no value and would require either a stub or an incompatible
third-party HTTP library.
- Add ClientError.encodingError; HttpTransport now throws it on encode
  failure instead of the misleading decodingError
- ClientToolResponseHandler surfaces RunErrorEvent as streamError
  instead of silently discarding all events from the tool-response run
- AgentFormView: use SecureField for API key input
- StatefulAgUiAgent.init(baseURL:) is now a convenience init delegating
  to init(configuration:), removing ~15 lines of duplicated setup
- Extract shared JSONPrimitiveWrapper from 6 private DTO copies into
  AGUICore/Utilities/JSONPrimitiveWrapper.swift (internal, one source)
- Replace 23-line MIT license blocks with 1-line copyright notice (292 files)
- Delete dead AGUICore placeholder struct and deprecated AGUITools struct
- Fix SwiftLint large_tuple config to use proper nested YAML syntax
- Fix unused colIdx variable in A2UIComponentView (replace with _)
- Wrap builder free functions in AgentBuilders enum namespace
- Add 10 MB max-buffer guard to SseParser to prevent unbounded growth
- Decode A2UISurfaceView surfaceData once in init instead of every body evaluation
- Replace fragile "[A2UI Action] {json}" string prefix with typed A2UIActionEnvelope
- Extract duplicate MockToolRegistry into shared Tests/AGUIAgentSDKTests/Mocks/
- Add ToolExecutionResult.decode<T>(as:using:) convenience for typed result access
- Remove all deprecated THINKING_* event types, DTOs, registry, state, and tests
- Add AGUIEventDecoder.remapThinkingEvent() for transparent THINKING→REASONING
  backward compat at the wire level (mirrors TypeScript BackwardCompatibility_0_0_45)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@paduh paduh changed the title Fix Pr Feedback refactor: address PR review feedback and remove deprecated THINKING events May 1, 2026
@paduh
paduh merged commit 0c438bb into main May 1, 2026
9 checks passed
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