Skip to content

Reconcile chat pacing and queued follow-ups - #254

Closed
0xSero wants to merge 10 commits into
codex/chat-performance-goalfrom
codex/pi-chat-w00-m00-reconcile
Closed

Reconcile chat pacing and queued follow-ups#254
0xSero wants to merge 10 commits into
codex/chat-performance-goalfrom
codex/pi-chat-w00-m00-reconcile

Conversation

@0xSero

@0xSero 0xSero commented Aug 12, 2026

Copy link
Copy Markdown
Owner

Purpose

Complete Wave 0 Mission 00 by reconciling the existing Local Studio fidelity/performance candidate stack onto the supervised chat-performance plan.

Key changes

  • preserve the first streaming delta immediately, then coalesce sustained deltas at display cadence
  • keep accumulated deltas on close and surface lag through a pending full resync
  • serialize rapid Local Studio follow-ups and protect the local reservation from stale terminal events
  • retain the iOS required-activity display preference migration
  • reject release-only metadata and revert lifecycle normalization that lacked captured production frames
  • record the commit-by-commit disposition and remaining acceptance surfaces

Delegated mission

  • Pi session: 019ff2c5-ca5b-7659-9c70-c62941da9a94
  • provider/model proof: homelab/glm-5.2
  • Codex verdict: ACCEPTED after ten review/repair turns using the same persistent session

Verification

  • cargo test -p codex-mobile-client: 778 passed, 7 ignored; integration and doc tests passed
  • cargo test -p codex-slingshot: 14 passed; doc tests passed
  • focused app_store::tests: 16 passed after pacing repairs
  • focused mobile_client::store_listener::tests: 6 passed after queue repairs
  • git diff --check passed
  • worktree and Codex submodule are clean

Full crate suites were intentionally not repeated after focused repairs. Complete command output is retained in the ignored mission log directory; the evidence document records the exact surfaces and remaining risks.

Stacked on #240 so the implementation diff contains only Mission 00.

0xSero added 10 commits August 11, 2026 18:20
… wire)

Cherry-pick of 025f7f8 from codex/local-studio-fidelity-performance.
Resolved conflict in json_line_wire.rs: kept the current async-fn
next_message signature and ported the legacy lifecycle normalization
(parse to serde_json::Value, normalize, from_value) into it. The
candidate's impl-Future form was a pre-async-fn rewrite no longer
needed on this base.

Wave 0 disposition: accept. Captured in
work/chat-performance/evidence/task-00-disposition.md.
Cherry-pick of 96e77de from codex/local-studio-fidelity-performance.
Fixes a latent compile defect in the candidate's coalesce_streaming_window:
merge_app_update returns Err(Box<AppStoreUpdateRecord>), so the buffered
push_front must unbox with *next, matching coalesce_ready_updates.

Wave 0 disposition: accept (with the one-line unbox fix). Captured in
work/chat-performance/evidence/task-00-disposition.md.
Cherry-pick of f1946f4 from codex/local-studio-fidelity-performance.
Applied cleanly onto the reconciled base (025f7f8 + 96e77de).

Adds turn-start reservation, queued follow-up drain in store_listener,
reducer/snapshot queue state, and four queue concurrency/restore-order
tests under mobile_client::tests.

Wave 0 disposition: accept. Captured in
work/chat-performance/evidence/task-00-disposition.md.
Task 00 evidence: accept 025f7f8 (rewrite, async-fn wire conflict
resolved), accept 96e77de (rewrite, one-line unbox compile fix),
accept f1946f4 (clean cherry-pick), reject a4cbf2d (release-only
metadata, out of performance scope). Documents mobile-parity analysis,
build/test economy, and acceptance mapping.
…covery)

Review 04-05 follow-up to 0c95716 (96e77de reconciled).

- should_flush_streaming_now rearms next_flush_at = now + WINDOW when a
  same-identity delta arrives at/after the passed deadline, so an
  immediately-following delta coalesces instead of flushing again.
- Add pending_full_resync flag on AppStoreSubscriptionState. On lag while
  an accumulated streaming update exists, coalesce_streaming_window returns
  the accumulated exact text and defers FullResync; receive_next_update
  consumes the flag, clears pacing, and returns FullResync on the next
  receive. Close still surfaces accumulated text; the following receive
  reports closed normally.
- Remove the scheduler-sensitive elapsed < WINDOW wall-clock assertion
  from the isolated-first-delta test; the deterministic helper tests are
  the regression proof.
- Extend pacing_flushes_first_after_idle_immediately_and_arms_deadline:
  after a passed-deadline (old+5ms) flush, assert rearmed ==
  flush_time + WINDOW, a 2ms-later same-identity delta returns false, and
  rearmed != old_deadline + WINDOW.
- Add app_store_subscription_surfaces_accumulated_text_then_full_resync_after_lag:
  deterministic, no sleeps/reducer — capacity-2 broadcast channel forced
  laggy by three sends, current delta passed directly into
  coalesce_streaming_window, asserts exact text first then FullResync next.
Review 06-08 follow-up to f07ce92 (f1946f4 reconciled).

Reducer invariant: an old or duplicate terminal event is a no-op for a
newer in-flight turn.
- ThreadStatusChanged(Idle|SystemError): upsert_or_merge_thread's
  pre-merge status guard now preserves Idle for any active_turn_id
  (pre-existing behavior) and preserves SystemError only for a
  local-queued-turn reservation. The event closure additionally
  preserves active_turn_id, plan progress, and agent status for a local
  reservation. SystemError semantics for a normal authoritative active
  turn are unchanged.
- TurnCompleted: terminal cleanup runs only when the completion is
  authoritative (matches active_turn_id, or no active turn for tolerant
  legacy completion). A nonmatching completion (including one for the
  old turn while a local reservation is in flight) is a true no-op:
  closure returns false and the caller checks .unwrap_or(false), so no
  active state mutates and no metadata-changed signal fires.

Controlled race test (multi-thread tokio):
stale_terminal_event_cannot_clear_newer_local_queued_reservation queues
two drafts, parks the first turn/start after the reservation installs,
applies an old TurnCompleted plus duplicate Idle and SystemError while
parked, attempts a second drain (sends nothing), asserts the
reservation stays authoritative and status stays Active with one draft
queued, then releases the first request and asserts it resolves to the
returned turn id with no duplicate send.
Review 09 follow-up to cc8efcd (025f7f8 reconciled).

Wave rules require captured evidence for compatibility normalization at a
wire boundary. The only committed tests for the legacy lifecycle
normalizer (missing startedAtMs/completedAtMs, empty-string
commandExecution.cwd) were synthetic inline JSON authored alongside the
normalizer; they prove the normalizer does what it says, not that real
Pi/Local Studio bridges emit the legacy shape.

Search of the repository and Task 00 mission artifacts found no real
scrubbed Pi/Local Studio item/started or item/completed JSON-RPC frame.
.pi-missions/task-00/*.jsonl matches are agent tool-call records from
running git show on the candidate, not captured server wire frames.

Revert json_line_wire.rs to the frozen-base strict decode path
(serde_json::from_str::<JSONRPCMessage> with no Value parse/normalize/
from_value indirection). git diff 314271a -- json_line_wire.rs is empty.

The independently valid iOS required-activity preference migration from
cc8efcd is retained. Classification: 025f7f8 is partial/superseded
pending capture; the concrete capture task is recorded in
work/chat-performance/evidence/task-00-disposition.md.
Reflect the final commits and remove overclaims:
- pacing: first delta immediate, sustained cadence-batched, deadline
  rearm/lag/close behavior; app_store::tests 16 passed.
- queued follow-ups: controlled parked-request race + failure restoration;
  store_listener::tests 6 passed; explain restore/reapply rebuild cost
  and the patch-once/restore-once wave policy.
- lifecycle candidate: partial/superseded pending capture; record the
  search performed (no real scrubbed frame found) and the concrete
  capture task; remove the stale claim that synthetic tests alone prove
  production acceptance.
- current commit hashes, commands actually run, surfaces not proven,
  remaining risks.
Review 10 follow-up: documentation-only.
- Remove stale claimed origin/main SHA e42ce...; moving-tip reconciliation
  is coordinator-owned, represented by branch history not a prose SHA.
- Fix 'What the candidate did,two parts):' to grammatical prose.
- Replace final-lifecycle-commit placeholders with actual commits
  15e094a (wire reversal) and 400f130 (evidence update).
- Split the clean/pushed/reproducible acceptance criterion honestly:
  clean/reproducible/no-release-metadata/submodule-clean are proven
  here; push and latest-origin/main reconciliation remain coordinator
  gates.
- Label version-field facts as observed during Task 00 review, not a
  durable invariant.
@0xSero

0xSero commented Aug 18, 2026

Copy link
Copy Markdown
Owner Author

Closing as superseded by #319 (merged as 0824605d), which fixes the same stalls at the Rust store boundary rather than at display cadence. This branch has been conflicting against main for some time and carries stale release metadata. If any specific UI change here is still wanted, please re-cut it against post-#319 main.

@0xSero 0xSero closed this Aug 18, 2026
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