add agent variant picker to profiles

[humanagent]

Summary

• add API/session support for updating an existing agent variant
• add a Variant row and picker sheet to the agent profile/settings page
• reuse the existing dev variant registry picker data and support clearing back to the default variant

Why

• internal builds need to switch existing agents to a different registered variant from the same profile surface used for agent settings.

Validation

• xcodebuild -project Convos.xcodeproj -scheme ConvosCore -destination "generic/platform=iOS Simulator" build
• full Convos (Dev) app build still fails locally at link time because LibXMTPSwiftFFI.xcframework is missing the x86_64 simulator slice for the NotificationService target.

---

^{Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.}

Note

Add agent variant picker to contact profiles

• Adds a 'Variant' row to ContactDetailActions that opens a new AgentVariantPickerSheet, allowing users to select or clear an agent variant for an agent instance.
• ContactDetailView loads available variants on appear via SessionManager.listAgentVariants() and applies changes via a new PATCH endpoint v2/agents/{instanceId}/variant. Errors surface as an alert.
• The variant picker and row are gated to internal builds and only shown when the contact has an agentInstanceId and at least one variant is available.
• Adds updateAgentVariant to ConvosAPIClient, SessionManager, and their protocols/mocks to support the new endpoint.

📊 Macroscope summarized 204cf7e. 6 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted

🗂️ Filtered Issues

No issues evaluated.

[macroscopeapp]

🟡 Medium

convos-ios/Convos/Contacts/ContactDetailView.swift

Line 1047 in 204cf7e

accessibilityLabel: "Share \(contactDisplayName)",

The variantRow displays selectedVariantLabel, but selectedAgentVariantId is only updated after a successful variant-change call and is never initialized from the contact's existing variantStamp on open. So when you first open a profile for an agent already on a non-default variant, the row shows "Default variant", and the picker sheet marks the default option as selected. The row and picker reflect the wrong variant until the user changes it. Initialize selectedAgentVariantId from variantStamp (e.g. in .onAppear or when the contact is set) so the existing variant is reflected on first display.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file @Convos/Contacts/ContactDetailView.swift around line 1047:

The `variantRow` displays `selectedVariantLabel`, but `selectedAgentVariantId` is only updated after a successful variant-change call and is never initialized from the contact's existing `variantStamp` on open. So when you first open a profile for an agent already on a non-default variant, the row shows "Default variant", and the picker sheet marks the default option as selected. The row and picker reflect the wrong variant until the user changes it. Initialize `selectedAgentVariantId` from `variantStamp` (e.g. in `.onAppear` or when the contact is set) so the existing variant is reflected on first display.

[claude]

Claude finished @humanagent's task —— View job

---

Code Review: Agent Variant Picker

Todos

• Read and analyze ContactDetailView.swift changes
• Review API client and session manager changes
• Check test coverage
• Evaluate against CLAUDE.md conventions
• Provide comprehensive review feedback

---

Summary

This PR adds the ability to switch an existing agent instance to a different variant from the profile card. The implementation follows clean architectural patterns and integrates well with the existing variant system. However, there are two bugs and several areas for improvement before merging.

---

🔴 Critical Issues (Must Fix)

1. Variant Not Displayed on Initial Open

Location: ContactDetailView.swift:102-103, 464-469

The selectedAgentVariantId state is never initialized from the contact's existing variantStamp. This causes:

• The variant row always shows "Default variant" when first opened, even for agents already on a non-default variant
• The picker sheet marks the wrong option as selected
• Only after the user manually changes the variant does the UI reflect the correct state

Fix: Initialize selectedAgentVariantId from variantStamp?.slug during view initialization or in .onAppear:

// Option 1: In init
init(contact: Contact, variantStamp: AgentVariantStamp? = nil, ...) {
    // ... existing init code ...
    _selectedAgentVariantId = State(initialValue: variantStamp?.slug)
}

// Option 2: In .onAppear (less ideal but workable)
.onAppear {
    if selectedAgentVariantId == nil {
        selectedAgentVariantId = variantStamp?.slug
    }
    ensureNavigator()
    navState.markScreenAppeared()
}

2. Missing Error Recovery After Failed Update

Location: ContactDetailView.swift:641-659

When updateAgentVariant throws an error, the picker sheet remains open and isUpdatingAgentVariant stays true in the defer block, but there's no mechanism to reset state or allow retry. The user is stuck with:

• A spinner that never stops (if the error happens before the defer)
• No way to dismiss the sheet and try again
• An error alert that doesn't provide actionable recovery

Fix: The error alert should dismiss the picker sheet so the user can retry:

} catch {
    Log.error("Failed to update agent variant \(instanceId): \(error.localizedDescription)")
    agentVariantErrorMessage = "We couldn't update this agent's variant. Please try again."
    presentingAgentVariantPicker = false  // Add this line
}

---

⚠️ Code Quality Issues

3. Inconsistent Nil Encoding Pattern

Location: ConvosAPIClient+Models.swift:238-244

The custom encode(to:) implementation explicitly encodes nil as a JSON null, but this is already the default Codable behavior. The custom encoder adds complexity without benefit:

// Current (unnecessary complexity)
public func encode(to encoder: Encoder) throws {
    var container = encoder.container(keyedBy: CodingKeys.self)
    if let variantId {
        try container.encode(variantId, forKey: .variantId)
    } else {
        try container.encodeNil(forKey: .variantId)
    }
}

// Simpler (default Codable handles this)
// Just remove the custom encode method entirely

Why this matters: The PATCH endpoint needs {"variantId": null} to clear the variant. Swift's default Codable already encodes Optional.none as null, so the custom implementation is redundant. If there's a specific backend requirement that necessitates this (e.g., distinguishing between missing key and null value), add a comment explaining it.

4. Sheet Dismissal Race Condition

Location: ContactDetailView.swift:654

The picker sheet is dismissed with presentingAgentVariantPicker = false while isUpdatingAgentVariant may still be true (before the defer executes). SwiftUI may still render the sheet with the loading spinner for a frame or two, creating a visual glitch.

Better pattern:

do {
    let response = try await session.updateAgentVariant(...)
    selectedAgentVariantId = response.variantId
    await MainActor.run {
        isUpdatingAgentVariant = false
        presentingAgentVariantPicker = false
    }
} catch {
    // ... error handling ...
    await MainActor.run {
        isUpdatingAgentVariant = false
        presentingAgentVariantPicker = false
    }
}

Or use defer but ensure it runs synchronously on the same actor:

defer {
    Task { @MainActor in
        isUpdatingAgentVariant = false
    }
}

---

📋 Missing Test Coverage

5. No Unit Tests for Variant Update Flow

Location: Test files

The PR adds default stub implementations in TestStubAPIClientDefaults.swift but no actual tests for:

• SessionManager.updateAgentVariant() success/failure paths
• API client error handling
• Response parsing for AgentVariantUpdateResponse
• Nil variant (clear to default) behavior

Recommendation: Add tests in ConvosCoreTests covering:

func testUpdateAgentVariant_Success() async throws {
    // Given an agent instance and a variant
    // When updateAgentVariant is called
    // Then the API client receives the correct PATCH request
    // And the response is parsed correctly
}

func testUpdateAgentVariant_ClearToDefault() async throws {
    // Given an agent on a non-default variant
    // When updateAgentVariant(variantId: nil) is called
    // Then the request body contains {"variantId": null}
}

func testUpdateAgentVariant_NetworkError() async throws {
    // Given a network error
    // When updateAgentVariant is called
    // Then the error propagates to the caller
}

6. No UI Tests for Picker Interaction

Location: N/A

There are no UI tests verifying:

• The variant row only appears for agents with agentInstanceId on internal builds
• The picker sheet displays all available variants
• Tapping a variant triggers the update call
• The "Default variant" option clears the variant
• Error alerts display correctly

Recommendation: Add UI tests or document that manual testing was performed.

---

🎨 SwiftUI Best Practices

7. Button Action Pattern Not Followed

Location: ContactDetailView.swift:870, 880, 894

The CLAUDE.md conventions require extracting button actions to avoid closure compilation errors, but the picker sheet uses inline closures:

// Current (violates convention)
Button(action: onClear) {
    variantRow(...)
}

Button(action: { onSelect(variant.slug) }) {
    variantRow(...)
}

Button("Done") {
    dismiss()
}

// Convention (extract actions)
Button(action: handleClear) {
    variantRow(...)
}

private func handleClear() {
    onClear()
}

While these simple closures may not hit type-check timeouts, consistency with project conventions improves maintainability.

8. Missing Accessibility Identifiers

Location: ContactDetailView.swift:858-924

The AgentVariantPickerSheet lacks accessibility identifiers for UI testing:

.accessibilityIdentifier("variant-picker-sheet")
.accessibilityIdentifier("variant-option-\(variant.slug)")
.accessibilityIdentifier("variant-picker-done")

This makes automated testing and accessibility verification harder.

---

🔒 Security & Performance

9. No Input Validation

Location: ConvosAPIClient.swift:949-955

The updateAgentVariant method URL-encodes the instanceId but doesn't validate it's non-empty or within expected length bounds. While the backend should validate, client-side checks improve error messages:

guard !instanceId.isEmpty else {
    throw APIClientError.invalidInput("instanceId cannot be empty")
}

10. Potential Memory Leak in Error Alert

Location: ContactDetailView.swift:203-209

The alert captures agentVariantErrorMessage directly, but the binding in line 472-480 is more complex. This pattern is fine, but consider using Binding<String?> directly:

.alert("Variant update failed", isPresented: $isShowingVariantError) {
    Button("OK", role: .cancel) {}
} message: {
    if let message = agentVariantErrorMessage {
        Text(message)
    }
}

private var isShowingVariantError: Binding<Bool> {
    Binding(
        get: { agentVariantErrorMessage != nil },
        set: { if !$0 { agentVariantErrorMessage = nil } }
    )
}

Current implementation is acceptable but slightly over-engineered.

---

✅ What's Good

1. Clean Architecture: The change follows the existing layering (API client → SessionManager → View)
2. Protocol Conformance: Mock implementations added to all test stubs
3. Internal Build Gating: Variant picker correctly hidden in production
4. Reusable Components: AgentVariantPickerSheet is a well-structured, reusable component
5. Error Handling: Network errors are caught and surfaced to the user
6. Consistent Naming: updateAgentVariant, listAgentVariants follow project conventions
7. Documentation: The API models are well-commented

---

📝 Recommendations Summary

Must fix before merge:

1. Initialize selectedAgentVariantId from variantStamp?.slug
2. Dismiss picker sheet on error to allow retry

Should fix:
3. Remove custom encoder or document why it's needed
4. Fix sheet dismissal race condition
5. Add unit tests for variant update flow
6. Follow button action extraction pattern
7. Add accessibility identifiers

Nice to have:
8. Add input validation for instanceId
9. Simplify error alert binding (or keep current)
10. Add UI tests for picker flow

---

Final Verdict

Approve with changes: The core implementation is solid, but the initialization bug (#1) will cause user-visible incorrect state on every profile open. Fix that and the error recovery issue (#2), and this is ready to ship. The other issues are quality improvements that can be addressed in follow-ups.

---