Skip to content

perf(swift-ios): skip validated stale detail replay - #10762

Closed
saphid wants to merge 319 commits into
pingdotgg:t3code/rebuild-mobile-app-swiftfrom
saphid:pr/swiftui-stale-replay-20260908
Closed

saphid wants to merge 319 commits into
pingdotgg:t3code/rebuild-mobile-app-swiftfrom
saphid:pr/swiftui-stale-replay-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: +123 / −2 lines across 2 files; app code +12 / −2, the rest is tests and docs.

Replayed detail events needlessly reduce already-applied state. Validate the shared event envelope and owning thread before skipping stale sequences, preserving malformed/foreign-event handling. This avoids replay reduction; no measured phone speedup is claimed.

Verification: all current-head GitHub checks pass, including contract fixtures and native tests. Skipped checks are not claimed as executed proof.

The completion fixture repair is carried from #10759. Current-head native CI passes.

Delivery: stacked. This branch contains #10758, #10759, #10761; 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).

Base: Theo’s t3code/rebuild-mobile-app-swift, 157476f1fb.

Verification scope: Both stale-detail regressions passed on this head: preserve newer messages/explicit markers, and skip reduction only after envelope validation. This proves replay handling; no measured phone speedup is claimed. The native CI job linked above contains the passing receipts. The tests exercise the protocol, state, or accessibility-value boundary changed here.

Independent cross-provider review was unavailable: claude auth status returned exit 1 with loggedIn: false. No Claude review or maintainer approval is claimed.

Refresh onto 157476f1fb (2026-09-27)

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

  • Focused tests at this layer, measured before the review fixes below (NativeMultiEnvironmentTests, NativeThreadCatchUpTests, NativeRetryIdentityTests, FeatureRootModelTests, iOS Simulator, per-test time limits): 87 XCTest + 100 Swift Testing passed.
  • 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)

  • Upstream's refreshThread replaces the active page with its initial-page snapshot when its read lands after an older-page load. Detaching the send refresh (fix(swift-ios): release accepted commands before optional refreshes #10758) moves that window rather than creating it. Invalidating the refresh on a history-epoch change would close it.
  • Overlapping adoptEnvironment calls: an older one can cancel aggregate workers started by a newer one before returning at its bootstrap guard; the next adoption or aggregate refresh restarts them (Macroscope).

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

t3dotgg and others added 30 commits August 18, 2026 14:32
- Cache EnvironmentStore's decoded document; publishes read it several
  times per second and previously hit disk + JSON decode every time
- Memoize parsed ISO8601 dates in NativeFeatureClient; every publish
  re-parsed ~10 timestamps per thread through two formatters
- Cache the mapped provider catalog per environment, invalidated on
  server config writes, instead of rebuilding hundreds of structs per
  publish
- Sort activity logs by raw ISO string instead of parsing dates inside
  the comparator (O(n log n) formatter calls per detail rebuild)
- Skip attachment hydration entirely for text-only threads
- Dedupe emitConnection so per-event yields only publish transitions
- Early-out acknowledgeDeliveredMessages when the outbox is empty
- Cap terminal buffers at 256KB; unbounded append could OOM on verbose
  commands and re-layout megabyte strings per chunk
- Key the markdown document cache by content fingerprint instead of the
  whole source string, stop inserting streaming intermediates (which
  churned completed messages out), and promote the final streamed render
  on completion instead of reparsing on the main thread
- Throttle streaming markdown renders (150ms leading-edge) instead of a
  200ms trailing debounce that never fired at an 80ms publish cadence,
  which left streaming messages as plain text for whole turns
- Share JSONEncoder/Decoder.t3 instances (were rebuilt per call) and
  skip sortedKeys on throwaway JSONValue bridging encodes
- Weak-capture the RPC connection loop and keepalive so released
  clients can actually deinit; snapshot dictionary keys before
  reentrant iteration in connected(); scope subscription Interrupts to
  the connection that assigned the request ID; add reconnect jitter
- Cache DPoP JWK + thumbprint (SHA-256 per managed request before) and
  the proof encoder; reuse ISO8601 formatter in command builders
- Replace crash-prone Dictionary(uniqueKeysWithValues:) on
  server-supplied IDs with first-wins reduces (model picker, agent
  awareness); retain per-thread states instead of full FeatureThread
  copies for transition signals; idle the home list timer to 60s when
  nothing is working

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Move table column-width estimation into the background render pass
  and store it on MarkdownRenderedTable; the view measured every cell's
  AttributedString per body evaluation on the main thread
- Make rendered markdown blocks Equatable (inline runs compare by
  reference thanks to the streaming run cache) and wrap block views in
  .equatable(), so only the changed tail of a streaming message
  re-renders instead of every block
- Gate the thread header's per-second TimelineView on working status;
  idle threads render a static status
- Decode local attachment previews off-main through the shared
  thumbnail cache; UIImage(data:) ran in body per reconfigure
- Memoize composer trigger parsing (was parsed 4x per keystroke)
- Drop markdown caches on memory warnings
- Stop the QR capture session on viewDidDisappear/backgrounding and
  resume on reappear; dismantle was the only stop path
- Retry pending workspace navigation requests when snapshot data lands
  so cold-start deep links are not silently dropped
- Scope approval/user-input resolution to details that contain the
  request instead of deep-comparing every cached transcript
- Binary-search diff hydration anchors (was O(n^2) on large files)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Move streaming markdown rendering out of .task(id:) into a renderer
  that survives revision changes; the per-revision task cancelled any
  in-flight parse, so messages that parse slower than the publish
  cadence never rendered. One render runs at a time, latest revision
  wins, 150ms throttle. Keep showing the previous streamed document
  between renders instead of flashing back to plain text.
- Cap the terminal buffer in UTF-8 bytes (the unit the limit is defined
  in) instead of Character count, snapping to a character boundary.
- Start the home list timer after items are populated so an initially
  working thread gets the 1Hz interval; re-evaluate on every update.
- Sort activities by memoized parsed dates (stable, wire-order ties);
  raw ISO strings sort wrong across mixed fractional representations.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A cancelled drain unwinding after a replacement started could clear the
replacement's task slot, allowing two concurrent drains to deliver out
of order. Generation-stamp each drain; only the current generation may
clear the shared slot, loop, or deliver.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The stale-document fallback kept any revision-mismatched render while
streaming, so a reused transcript cell whose @State survived recycling
could briefly show another message's markdown. Only accept a stale
document whose source is a prefix of the current message.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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-stale-replay-20260908 branch from 0a7b700 to 6cc67cd 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-stale-replay-20260908 branch from 6cc67cd to a2f3053 Compare September 27, 2026 20:29
Comment thread apps/swift-ios/Features/Root/FeatureRootModel.swift Outdated
Comment thread apps/swift-ios/Features/Root/FeatureRootModel.swift Outdated
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-stale-replay-20260908 branch from a2f3053 to 7a9d0de Compare September 27, 2026 21:14
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:2518

The detached refreshThread started at line 2518 can overwrite a page loaded by loadEarlierThreadTurns: its initial-page snapshot replaces activeRawThread and activeThreadPage, dropping the prepended older messages and pagination state after sendMessage has returned. Cancel or invalidate this refresh when threadHistoryEpoch changes, or merge its result without discarding paginated history.

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

The detached `refreshThread` started at line 2518 can overwrite a page loaded by `loadEarlierThreadTurns`: its initial-page snapshot replaces `activeRawThread` and `activeThreadPage`, dropping the prepended older messages and pagination state after `sendMessage` has returned. Cancel or invalidate this refresh when `threadHistoryEpoch` changes, or merge its result without discarding paginated history.

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's refreshThread already replaces the active page with its initial-page snapshot when its read lands after an older-page load; detaching the send refresh moves that window rather than creating it, and the window is one thread read. Listed as a follow-up (invalidate on history epoch).

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 5 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>
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-stale-replay-20260908 branch from 7a9d0de to 71c1375 Compare September 27, 2026 22:06
github-actions Bot and others added 2 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>
@saphid
saphid force-pushed the pr/swiftui-stale-replay-20260908 branch from 71c1375 to dca5698 Compare September 27, 2026 22:27
_ environment: Environment,
client newClient: T3Client
) async {
cancelAggregateRefresh()

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:1163

An older adoptEnvironment invocation can cancel aggregate refresh workers started by a newer invocation, then return at the bootstrapID guard, leaving passive environments without background shell/catalog refreshes. Move cancelAggregateRefresh() below that guard so a stale adoption cannot tear down the current workers.

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

An older `adoptEnvironment` invocation can cancel aggregate refresh workers started by a newer invocation, then return at the `bootstrapID` guard, leaving passive environments without background shell/catalog refreshes. Move `cancelAggregateRefresh()` below that guard so a stale adoption cannot tear down the current workers.

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 as a narrow ordering between overlapping adoptEnvironment calls; the next adoption or aggregate refresh restarts the workers. Listed as a follow-up rather than changed in this stack.

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.

@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 dca56985ba:

  • 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.

Copy link
Copy Markdown
Member

Note

This comment is posted by Julius' dot

The stale-replay fast path is focused, but the current comparison also contains the hydration and command-recovery stack. Which of those changes are required for this optimization? Please isolate it or explain the dependency under the one problem rule.

@t3dotgg
t3dotgg force-pushed the t3code/rebuild-mobile-app-swift branch 2 times, most recently from 1637f70 to cf79028 Compare October 1, 2026 21:36
t3dotgg pushed a commit that referenced this pull request Oct 2, 2026
Port the focused fix from #10762 onto the current SwiftUI branch.

Source-Commit: dca5698

Ported by GPT-6.1-Sol through the Codex harness in T3 Code.
@t3dotgg

t3dotgg commented Oct 2, 2026

Copy link
Copy Markdown
Member

Note

🤖 GPT-6.1-Sol responding on behalf of Theo

Applied the focused fix to t3code/rebuild-mobile-app-swift in e4eaa597ab. Closing this source PR after the port.

The source branch includes old rewrite history that conflicts with the current target. Only the intended fix and its focused tests were carried over.

The combined focused native checks passed: 241 tests, one existing skip, no failures.

@t3dotgg t3dotgg closed this Oct 2, 2026
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.

5 participants