Skip to content

perf(chat): cache optional preparation and identify slow send steps - #5127

Open
reachjalil wants to merge 2 commits into
devfrom
perf/cached-send-preparation
Open

reachjalil wants to merge 2 commits into
devfrom
perf/cached-send-preparation

Conversation

@reachjalil

Copy link
Copy Markdown
Collaborator

Problem

A message can remain in Starting while optional preparation finishes, and the console does not identify which pre-send request is waiting. Environment-key hints were cached per conversation, so a new thread repeated the same lookup; a stalled lookup could hold dispatch. Native Cloud skill materialization also staged every body before checking whether the existing file was identical.

The current send route already bypasses the broad Cloud MCP readiness gate. This change does not add another bypass or claim to identify the root cause of every Starting stall.

Changes

  • Cache optional environment-key hints by authenticated client object and runtime, sharing successful and in-flight reads across conversations without sharing across credential/client changes. Bound the cache to 100 entries and invalidate late results when environment state changes.
  • Stop awaiting optional environment-key hints after 1 second. Send with fresh date/time/locale and session-specific context; let the lookup warm the cache in the background. Subsequent sends do not duplicate an outstanding lookup. Failed lookups remain retryable. Only key names are involved, never credential values or authentication checks.
  • Reuse unchanged native skill files before staging; stage and atomically replace only changed/new bodies. Continue fresh authorized Cloud index/body reads, obsolete-file removal, scope cleanup, and native registration checks.
  • Warn once while a required send step is still pending after 2 seconds. Labels cover history, interruption, archive validation, attachment preparation, v1 admission, and v2 skill catalog/permission/model/context/prompt requests. Environment-key hints have their own 1-second warning. Output includes only static step labels and elapsed/threshold milliseconds—not prompts, IDs, endpoints, key names, tokens, or errors. Required operations retain their existing wait/error behavior; warnings do not retry or time out a send.
  • Extend the existing send-isolation test selection with the diagnostics suite.

No visual/layout changes. Archive checks, authentication, authorization, Stop ordering, and fresh per-message context are retained.

Validation

Commands below ran from apps/app unless noted:

  • bun test --isolate tests/session-stop-refresh.test.ts tests/session-send-isolation.test.ts tests/env-context.test.ts tests/send-step-diagnostics.test.ts tests/session-ownership.test.ts tests/opencode-session-native.test.ts tests/safe-edit-resend.test.ts tests/session-history.test.tsx tests/session-scroll.test.tsx324 passed. React act warnings remain in the history/Stop harnesses.
  • bun test --isolate tests/opencode-v2-adapter.test.ts --test-name-pattern 'send-step diagnostics'5 passed, 101 non-selected tests filtered out. These exercise each instrumented native request: warning fires while blocked, no duplicate request, subsequent requests wait, and original completion is preserved.
  • bun test --isolate ./tests/env-context.test.ts ./tests/send-step-diagnostics.test.ts ./tests/session-send-isolation.test.ts ./tests/cloud-mcp-submit-readiness.test.ts45 passed.
  • From apps/server: bun --conditions=development test src/cloud-native-skills.test.ts12 passed, including no writes/renames for unchanged bodies, atomic changed-body replacement, scope changes, removals, and failed staging.
  • pnpm run typecheck in apps/app and server TypeScript check — passed.
  • git diff --check — passed.

Known failures and limits

  • The complete v2 adapter/helper run had 112 passes and one provider-catalog failure: v2 provider catalog retains display names and advertised effort without exposing provider credentials receives an extra leading null variant. The exact same unchanged test fails on base 8c18c952abfad9fda39adba9bce29f7de8b2b817, independently checked with the same dependencies.
  • Root pnpm evals:pr specs/session-send-isolation.test.ts could not launch because the evals workspace lacks Vitest. Its nine underlying app suites were run directly and all passed; this is not a successful eval artifact receipt.
  • No live desktop/cloud end-to-end run, screenshot, or production latency benchmark. Tests prove eliminated duplicate work and bounded optional waiting, not a measured end-to-end speedup or resolution of the reported incident.
  • Warnings cover foreground preparation/admission, not post-admission first-token delays or lost UI events. Background queued sends retain their existing instrumentation coverage.
  • Cloud skill network freshness is intentionally not cached: permission changes/removals can happen remotely without local configuration changing.

Manual check

  1. Send in two new threads using the same client/runtime: only the first should fetch environment-key hints.
  2. Delay the environment-key response beyond one second: one environment_keys warning appears and sending can proceed without the optional hints. Let the response finish; the next thread reuses it.
  3. Delay a required archive/model/prompt request beyond two seconds: one [send-step] Still pending warning names it. The request is not replayed and normal error/Stop handling remains in effect.

@reachjalil
reachjalil requested a review from a team September 17, 2026 21:50
@vercel

vercel Bot commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
openwork-app Ready Ready Preview Sep 18, 2026 5:56pm UTC
3 Skipped Deployments
Project Deployment Actions Updated
openwork-den Skipped Skipped Sep 18, 2026 5:56pm UTC
openwork-diagnostics Skipped Skipped Sep 18, 2026 5:56pm UTC
openwork-landing Skipped Skipped Sep 18, 2026 5:56pm UTC

@github-actions

Copy link
Copy Markdown
Contributor

Selected evidence: Incomplete · 12/12 tests · 18/18 assertions · 8 images

Commit 136db1cf70c90d7466f81cc4cdf6ab5e5057d7ab · selected evidence
Required verification is reported separately by the current-head Required verification check.

Open review report

Coverage gaps: Required verification: incomplete. Selected evidence does not satisfy all required specs. No authenticated current-head required plan is available.

@vercel
vercel Bot temporarily deployed to Preview – openwork-den September 18, 2026 17:55 Inactive
@vercel
vercel Bot temporarily deployed to Preview – openwork-landing September 18, 2026 17:55 Inactive
@vercel
vercel Bot temporarily deployed to Preview – openwork-diagnostics September 18, 2026 17:55 Inactive
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