Skip to content

fix(swift-ios): compact consecutive tool groups with relative ages - #10767

Open
saphid wants to merge 31 commits into
pingdotgg:t3code/rebuild-mobile-app-swiftfrom
saphid:pr/swiftui-compact-tools-20260908
Open

saphid wants to merge 31 commits into
pingdotgg:t3code/rebuild-mobile-app-swiftfrom
saphid:pr/swiftui-compact-tools-20260908

Conversation

@saphid

@saphid saphid commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Review this layer

GitHub shows this PR against t3code/rebuild-mobile-app-swift, so its diff includes every earlier PR in the chain. Review only this layer's change:

This layer's diff: +290 / −77 lines across 6 files; app code +198 / −64, the rest is tests and docs.

Group consecutive tool calls and updates into one expandable transcript row, preserving boundaries at text, notices, and new turns. Display the group’s most recent event age. The inherited activity row now shows last-message age at the end of the transcript; the thread-header receipt label is removed.

Delivery: stacked. This branch contains #10758, #10759, #10761, #10762, #10763, #10764, #10765, #10766, #14021; merge the chain in order (#10758 → #10759 → #10761 → #10762 → #10763 → #10764 → #10765 → #10766 → #14021 → #10767). Each PR's own change is its top commit(s).

Fresh combined layout at current head a92cb54a2:

Current compact tool group with last-message age below agent activity

Actual native WorkspaceView recording: a snapshot receipt, a new assistant message, and completion. Synthetic conversation and four tool events; iPhone 17 Pro Simulator / iOS 26.5, dark mode, full 402×874-point viewport scaled to 390 pixels wide. Real-time playback, GIF sampled at 12 fps, plus a disclosed three-second final-frame hold. The idle transition retains all transcript text and removes the activity row.

Current head: message age advances and activity clears on completion

Clean current-head recording · Captioned recording · Idle screenshot · Capture/edit receipt

The tool-group comparison and expansion/collapse footage below were captured at prerequisite 9083a8378 and candidate 9d2e2fed9. This older idle-thread fixture has no activity row or receipt label; the tool-group implementation it exercises is unchanged at current head. Both capture tests passed (1 test each, exit 0). The fresh combined recording above verifies the changed activity/header layout. The matching before/after activity comparison is in #10766.

Still-state comparison: two actual screenshots held for 3.5 seconds each; this is not a timing measurement.

Before separate tool rows; after an expandable group

Actual candidate interaction: expand, retain all four calls, age advances from 1m to 2m, collapse. GIF samples 12 fps with real-time playback; no sped-up or generated transitions.

Actual tool-group interaction

Clean recording · Captioned recording · Capture and editing receipt

Current-head verification: native WorkspaceView capture and two message-age tests passed (3 tests, xcodebuild exit 0). CI passed, including contract fixtures and native tests. Earlier captures had incomplete redraws and were rejected; replacement current-head frames were inspected through idle. Xcode compiler-probe stalls required retries. No live-backend or phone performance measurement is claimed.

Independent Claude review was unavailable: claude auth status returned exit 1 with loggedIn: false. Visual inspection is the author’s assessment, not maintainer approval.

Refresh onto 157476f1fb (2026-09-27)

Rebased with the thread-sync chain onto the current t3code/rebuild-mobile-app-swift (157476f1fb). Head 68d92c2752.

  • Focused tests at this layer, measured before the review fixes below (NativeMultiEnvironmentTests, NativeThreadCatchUpTests, NativeRetryIdentityTests, FeatureRootModelTests, iOS Simulator, per-test time limits): see full suite below.
  • Full T3CodeTests at the chain top (68d92c2752): 343 XCTest cases passed (1 skipped). Swift Testing ran 892 tests; the only 2 failures are UsageModelsTests currency formatting, which fail identically on unmodified 157476f1fb.
  • Independent review: devin -p --sandbox --permission-mode smart --model swe-2-max over the whole chain diff, exit 0. No blocker, high or medium findings. A dead-state finding was fixed. A report that Stop state outlives a vanished thread was not changed, because stopSurvivesCloseReopenAndMissingReconnectSeed requires that behaviour.

Follow-ups (minor, not blocking)

  • When reconciliation finds the thread unchanged it keeps the previous page metadata, so hasMore can stay stale after older turns are deleted (Macroscope).

Model and harness: GPT-6 / Codex. Rebase onto 157476f1fb: Claude Opus 5.5 / Claude Code.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL 1,000+ changed lines (additions + deletions). labels Sep 8, 2026
@saphid
saphid marked this pull request as ready for review September 8, 2026 21:57
@macroscopeapp

macroscopeapp Bot commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Macroscope has since reviewed this pull request. An earlier review was skipped by a cost limit; a review has now completed, so that notice no longer applies.

@macroscopeapp

macroscopeapp Bot commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR substantially changes default Swift iOS behavior across transport reconciliation, thread stopping, transcript rendering, navigation, and background lifecycle handling, with extensive new asynchronous coordination. Several concrete unresolved state and race conditions are also documented, including an acknowledged High-severity stop-path issue.

Not approved because:

  • 5 blocking correctness issues found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

@cursor

cursor Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@saphid

saphid commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@t3dotgg Alex has approved the updated UI and media here. Please take a look at the compact tool groups with the updated activity-row message age. Current-head screenshots, GIFs, and videos are in the description. This remains chained after #10766.

@saphid
saphid force-pushed the pr/swiftui-compact-tools-20260908 branch from a92cb54 to 85ae1dc Compare September 26, 2026 23:47
}

private static func lifecycleKey(_ activity: OrchestrationActivity) -> String {
if let id = activity.payload["toolCallId"]?.stringValue

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium Core/NativeTranscriptTimeline.swift:210

Activities with an empty toolCallId are all assigned the same id: lifecycle key, so parallel calls merge into one callOrder entry and a completion can clear another call's active label. Only use toolCallId as the stable identity when it is non-empty; otherwise use the fallback identity.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/Core/NativeTranscriptTimeline.swift around line 210:

Activities with an empty `toolCallId` are all assigned the same `id:` lifecycle key, so parallel calls merge into one `callOrder` entry and a completion can clear another call's active label. Only use `toolCallId` as the stable identity when it is non-empty; otherwise use the fallback identity.

Comment thread apps/swift-ios/Features/Root/FeatureRootModel.swift
Comment on lines 1215 to 1219

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High Root/FeatureRootModel.swift:1215

The pending-creation path can cancel a newer turn than the one the user claimed. After awaiting outbox drain/discard work, the unguarded client.cancelTurn(threadID:) cancels whichever turn is current; pass key.turnID as expectedTurnID, as the normal path does.

Suggested change
try await client.cancelTurn(threadID: threadID)
try await client.cancelTurn(threadID: threadID, expectedTurnID: key.turnID)
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/Features/Root/FeatureRootModel.swift around line 1215:

The pending-creation path can cancel a newer turn than the one the user claimed. After awaiting outbox drain/discard work, the unguarded `client.cancelTurn(threadID:)` cancels whichever turn is current; pass `key.turnID` as `expectedTurnID`, as the normal path does.

A detached accepted-command refresh could finish after deleteThread and
publish the deleted thread again. Cancel it once the server accepts the
delete.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@saphid
saphid force-pushed the pr/swiftui-compact-tools-20260908 branch from 85ae1dc to 456fc91 Compare September 27, 2026 00:56
github-actions Bot and others added 3 commits September 27, 2026 18:36
…ng older state

A detail read that started before a newer send was accepted could publish
after it and hide the newer message until the next read. Skip publishing a
superseded read (its refresh loop reads again), and stop a cancelled shell
refresh from writing a shell read before the change.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ives

A Stop accepted during a send's detail read discarded that read without
requesting another, and an older detail could replace the transcript before
the accepted message reached it. Re-read detail after a discarded read, and
keep delivered messages until a server transcript includes them.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Do not retain a delivered message that a server transcript already
confirmed, keep only its display copy, and drop retained messages when
their thread details are removed or cleared.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@saphid
saphid force-pushed the pr/swiftui-compact-tools-20260908 branch from 456fc91 to 9003d71 Compare September 27, 2026 20:29
Comment thread apps/swift-ios/Features/Root/FeatureRootModel.swift Outdated
@saphid
saphid force-pushed the pr/swiftui-compact-tools-20260908 branch from 9003d71 to 5f96389 Compare September 27, 2026 20:32
Evicting a cached thread detail forgot its accepted messages, so a stale
reopen hid them. A restored outbox or a new task's local detail was also
recorded as the server transcript, so its accepted prompt was treated as
confirmed and disappeared on the next older read. Only server transcripts
now confirm delivery, and eviction keeps retained messages.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@saphid
saphid force-pushed the pr/swiftui-compact-tools-20260908 branch from 5f96389 to 45a5e94 Compare September 27, 2026 21:14
github-actions Bot and others added 2 commits September 28, 2026 07:26
A newer send can be delivered while an older one waits to retry. Delivered
copies were placed before every queued copy, which reversed them. Merge
both by send time.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
github-actions Bot and others added 3 commits September 28, 2026 07:26
A transient environment catalog read failure ended the active client's
configuration subscription, so later provider and settings updates were
ignored. Skip that publish instead.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…e probe

A passive peer's catalogue probe started at bootstrap could finish after
the user saved a preference there and replace it with the older value.
Discard the probe when a config already arrived.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@saphid
saphid force-pushed the pr/swiftui-compact-tools-20260908 branch from 45a5e94 to 9ec65b8 Compare September 27, 2026 22:06
guard try await currentEnvironments(for: environment) != nil else { return }
// Never disconnect the shared client if this peer becomes selected.
let probe = await runtime.ephemeralClient(for: environment)
let config = try? await probe.serverConfig()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium App/NativeFeatureClient.swift:4729

refreshCatalogue retries a rejected serverConfig() request every failureInterval, continuing to create ephemeral clients and make requests even after the peer requires re-pairing. try? discards the authorization failure, so the loop never applies the shell worker’s 24-hour backoff or exits; handle rejected credentials explicitly and stop this worker.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/App/NativeFeatureClient.swift around line 4729:

`refreshCatalogue` retries a rejected `serverConfig()` request every `failureInterval`, continuing to create ephemeral clients and make requests even after the peer requires re-pairing. `try?` discards the authorization failure, so the loop never applies the shell worker’s 24-hour backoff or exits; handle rejected credentials explicitly and stop this worker.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #10761 (fix(swift-ios): stop probing a peer's catalogue once it needs pairing). The probe's own error cannot carry the 401 (the socket ticket rejection surfaces as a timeout), so the loop stops once the peer's shell read marks it as needing pairing. Managed (T3 Connect) peers are a listed follow-up.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

self?.acceptedCommandRefreshes[threadID]?.needsDetail = false
self?.acceptedCommandRefreshes[threadID]?.pending = false
if needsDetail {
try? await self?.refreshThread(id: threadID, client: client) { [weak self] in

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium App/NativeFeatureClient.swift:2551

An accepted send can resurrect a thread deleted by another client: the in-flight refreshThread publishes .detail after the shell has removed the thread, and FeatureRootModel handles that event by upsert(value.thread). clearRemovedThreadDetail clears selection but does not cancel this task, so cancel the accepted-command refresh there or revalidate that the thread still exists before emitting .detail.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/App/NativeFeatureClient.swift around line 2551:

An accepted send can resurrect a thread deleted by another client: the in-flight `refreshThread` publishes `.detail` after the shell has removed the thread, and `FeatureRootModel` handles that event by `upsert(value.thread)`. `clearRemovedThreadDetail` clears selection but does not cancel this task, so cancel the accepted-command refresh there or revalidate that the thread still exists before emitting `.detail`.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changing in this PR: upstream awaited the same refresh inside send and has the same race with a remote delete. Cancelling accepted-command refreshes on shell removal would close it; listed as a follow-up on #10758.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

Comment on lines +5952 to +5955
if reconcile, pendingHistory == nil, let current = activeRawThread,
snapshot.snapshotSequence >= (activeThreadSequence ?? 0),
snapshot.thread == current {
return

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium App/NativeFeatureClient.swift:5952

Reconciliation returns without updating activeThreadPage when snapshot.thread is unchanged, so page metadata such as hasMore remains stale and the detail can continue advertising “load earlier” after older turns are deleted. Include snapshot.page in this equality check so changed pagination state is published.

Suggested change
if reconcile, pendingHistory == nil, let current = activeRawThread,
snapshot.snapshotSequence >= (activeThreadSequence ?? 0),
snapshot.thread == current {
return
if reconcile, pendingHistory == nil, let current = activeRawThread,
snapshot.snapshotSequence >= (activeThreadSequence ?? 0),
snapshot.thread == current,
snapshot.page == activeThreadPage {
return
}
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/App/NativeFeatureClient.swift around lines 5952-5955:

Reconciliation returns without updating `activeThreadPage` when `snapshot.thread` is unchanged, so page metadata such as `hasMore` remains stale and the detail can continue advertising “load earlier” after older turns are deleted. Include `snapshot.page` in this equality check so changed pagination state is published.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged: with an unchanged thread, reconciliation keeps the previous page metadata, so hasMore can stay stale until the next full load. Listed as a follow-up in the PR body.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

github-actions Bot and others added 14 commits September 28, 2026 08:23
A peer whose credential was rejected before its catalogue loaded kept
opening a probe client every 20 seconds. The probe's own error cannot show
the rejection (the socket ticket 401 surfaces as a timeout), so stop once
the peer's shell read has marked it as needing pairing; re-pairing starts
a new worker.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ismissed early

Reopening a thread in the compact split view can report the new thread view
as disappeared right after it appears while it stays on screen. That
cancelled the draft restore, so the composer showed a busy send button
until relaunch. Restore outside the view task's lifetime.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…cted

Pairing rejection cancelled the polling tasks but left the reconciliation
loop issuing rejected shell reads every 30 seconds.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A passive peer whose socket stayed open kept its 30-second quiet shell
reads after a 401, and showed as connected. Handle the rejected credential
before the live-stream shortcut and switch to the long back-off.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The open socket's updates marked a rejected peer connected again, and stream
repairs woke its HTTP loop before the back-off. Keep the rejection in the
peer's shared state.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The live-peer fixture this test needs now lands below the Stop layer, so
the regression moved out of the command-acknowledgement layer returns here.
Stop status refreshes the shell cache and reads it back. A live turn
advancing the shell during that read made the refresh discard itself as
superseded, so the status check failed and Stop could not be confirmed or
retried. While the environment's stream is authoritative on the current
socket, the cache it keeps current answers instead; after a disconnect or
lost authority the check still fails.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@saphid
saphid force-pushed the pr/swiftui-compact-tools-20260908 branch from 9ec65b8 to 70258b6 Compare September 27, 2026 22:27
@saphid

saphid commented Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

@t3dotgg this is ready for your review and merge.

Merge order: stacked chain, merge in order: #10758 → #10759 → #10761 → #10762 → #10763 → #10764 → #10765 → #10766 → #10767. #12655 (composer focus freeze) is independent and can merge at any time.

Evidence at head 70258b6a6b:

  • CI: all checks pass on this head.
  • Full T3CodeTests at the chain top (70258b6a6b, iOS Simulator): 343 XCTest cases passed (1 skipped). Swift Testing ran 892 tests; the only 2 failures are UsageModelsTests currency formatting, which fail identically on unmodified 157476f1fb.
  • Each review fix has a focused regression test that fails without the fix, except where the description says why not.
  • An earlier build of this chain (before the review rounds) fixed the stuck-send behaviour on a physical iPhone, as confirmed by the user. The current heads have not been re-verified on a device.

Independent review: No layer-specific Astra review. Covered by the SWE-2 Max review of the whole chain (exit 0, no blocker/high/medium findings).

Macroscope findings on the latest heads have been verified against the source: real ones are fixed, and the rest have a written reason and are listed under Follow-ups in the description.

github-actions Bot and others added 6 commits September 28, 2026 10:32
The receipt line in the thread header showed a snapshot or update receipt
time, which read as message freshness and stayed static while a quiet
thread was connected. Show the age of the last sent message on the
activity row while the agent works instead, advancing only at displayed
unit boundaries, and remove the header receipt line.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

This branch has not been deployed

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

Labels

size:XXL 1,000+ changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant