Skip to content

fix(swift-ios): recover missing completed thread replies - #10759

Closed
saphid wants to merge 314 commits into
pingdotgg:t3code/rebuild-mobile-app-swiftfrom
saphid:pr/swiftui-completion-repair-20260908
Closed

saphid wants to merge 314 commits into
pingdotgg:t3code/rebuild-mobile-app-swiftfrom
saphid:pr/swiftui-completion-repair-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: +522 / −31 lines across 2 files; app code +75 / −20, the rest is tests and docs.

A shell can report completion while the selected transcript still lacks the final reply or displays streaming text. Require an authoritative detail snapshot without closing the live subscription, preserve newer detail state against stale shell metadata, and avoid repeatedly publishing already-synchronized legacy bursts.

Pre-restack CI verification: GitHub checks passed on the original head, including contract fixtures and native tests. Skipped checks are not claimed as executed proof.

The completion tests now use observable upstream snapshot events instead of a receipt type supplied only by the later bootstrap PR. Pre-restack native CI passed.

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

Historical verification scope (before this restack): Native tests passed for missing or streaming final replies, completion while required snapshots are held, newer running details against delayed shell completion, and warm cached opens. Tests assert authoritative replacement while preserving the live subscription. The native CI job linked above contains the passing receipts. The tests exercise the protocol, state, or accessibility-value boundary changed here.

Refresh onto 157476f1fb (2026-09-27)

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

  • Focused tests at this layer, measured before the review fixes below (NativeMultiEnvironmentTests, NativeThreadCatchUpTests, NativeRetryIdentityTests, FeatureRootModelTests, iOS Simulator, per-test time limits): 84 passed, 0 failures.
  • 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.
  • Rebase change: upstream efc8b08c58 (bounded catch-up batches) now publishes one legacy .live per received batch. This PR's detailWasSynchronized guard suppressed that and made upstream's testLegacyReplayPublishesOncePerReceivedBatchAndResumesAfterAppliedEvents hang, so the guard is dropped in favour of upstream's design, along with testLegacyBurstCoalescesUntilExplicitMarker and testLegacyMarkerlessFinalPublishesWhenClockReleases, which asserted the old behaviour.

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-completion-repair-20260908 branch from be60f78 to 4fd44f0 Compare September 27, 2026 00:56
try? await self?.refreshThread(id: threadID, client: client)
}
guard self?.isCurrentAcceptedCommandRefresh(threadID: threadID, id: id) == true else { break }
try? await self?.refresh(client: client)

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

A cancelled accepted-command refresh can restore a deleted thread, causing a later emitCachedSnapshot to publish it again. refresh(client:) resumes after shellSnapshot() and assigns shellsByEnvironmentID and latestShell without checking Task.isCancelled; add a cancellation/current-client guard before those assignments.

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

A cancelled accepted-command refresh can restore a deleted thread, causing a later `emitCachedSnapshot` to publish it again. `refresh(client:)` resumes after `shellSnapshot()` and assigns `shellsByEnvironmentID` and `latestShell` without checking `Task.isCancelled`; add a cancellation/current-client guard before those assignments.

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>
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-completion-repair-20260908 branch from f89db82 to ed9720a 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>
@saphid
saphid force-pushed the pr/swiftui-completion-repair-20260908 branch from ed9720a to 392394d Compare September 27, 2026 22:06
@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 392394d4a7:

  • 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. This layer was 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.

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 1, 2026 — with ChatGPT Codex Connector
@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

t3dotgg commented Oct 2, 2026

Copy link
Copy Markdown
Member

Note

🤖 GPT-6.1-Sol responding on behalf of Theo

Closing this implementation as part of the SwiftUI backlog cleanup. Missing completed replies and older shell metadata replacing newer detail are real problems. This patch adds forced snapshot recovery based on shell completion and changes stream/cache synchronization, which is broader than the small-fix scope of this pass.

For reconsideration, submit a focused stale-shell ordering fix or a current reproduction of missing final content with a small repair. The problems remain valid; this closure is a scope decision.

See the contribution scope guidance.

@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

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews 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