Persist Telegram bridge turns: nested after() never fires#77
Conversation
The Telegram bridge invokes the chat handler in-process from inside the webhook's own after() callback. An after() registered there never runs, so every bridge turn streamed its answer fine but silently dropped the usage rollup and the transcript (bumpUsage + saveTurn lived only in the chat route's after()). This dated back to the ack-first webhook redesign and explains Telegram conversations missing from the dashboard. Internal calls now run the persistence block inline before the stream closes — the bridge reads to EOF, so completion is guaranteed. Public calls keep after(), so widget/API clients see EOF without waiting on DB writes. Verified end-to-end: a webhook-driven DM turn now lands as a channel='telegram' conversation.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview The post-stream work is refactored into Reviewed by Cursor Bugbot for commit 91106ea. Configure here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe POST /v1/chat handler now derives an ChangesChat route persistence refactor
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ChatRoute
participant PersistTurn
participant After
Client->>ChatRoute: POST /v1/chat
ChatRoute->>ChatRoute: determine internal flag
ChatRoute->>ChatRoute: stream response, finally block
alt internal call
ChatRoute->>PersistTurn: persistTurn() inline
PersistTurn-->>ChatRoute: rollups + saveTurn complete
else public call
ChatRoute->>After: after(persistTurn)
After->>PersistTurn: persistTurn() deferred
PersistTurn-->>After: rollups + saveTurn complete
end
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91106eaebe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // INLINE before the stream closes: the bridge reads to EOF, so completion is | ||
| // guaranteed. Public calls keep after() below so their EOF isn't delayed by DB | ||
| // writes. | ||
| if (internal) await persistTurn(); |
There was a problem hiding this comment.
For internal Telegram turns, streamToTelegram only calls editFinal after the for await over res.body reaches EOF (apps/api/app/api/telegram/route.ts:364-388). Since this line waits for persistTurn() before controller.close(), any slow or hung transcript/usage/cache/telemetry write leaves users on “…” or a throttled draft even though the terminal answer has already been emitted; on cache-hit/refusal paths there are no deltas, so the placeholder is delayed entirely. Consider rendering the final answer before draining/persisting, while still keeping the invocation alive for persistence.
Useful? React with 👍 / 👎.
1 participant
What
Moves the chat route's persistence block (usage rollup + transcript save + spend meter + tier-1 cache write + telemetry flush) into a named
persistTurn()and runs it inline before the stream closes for internal (Telegram bridge) calls, keepingafter(persistTurn)for public calls.Why
The Telegram webhook acks first and runs the turn inside its own
after()callback, invoking the chat handler in-process. Anafter()registered from within anotherafter()callback never fires, so every bridge turn streamed its answer to the user but silently droppedbumpUsageandsaveTurn. Net effect: Telegram conversations never appeared in the dashboard (the owner-reported bug — the channel filters looked broken), and usage counters undercounted. This dates back to the ack-first redesign; it was masked until now by the schema drift that broke transcript writes for all channels.Inline persistence is safe for the bridge because it consumes the stream to EOF — the writes are guaranteed to complete before the webhook invocation ends. Public widget/API clients keep the
after()path, so their EOF is not delayed by DB writes.Verification
after()-only.channel='telegram'conversation with linked user/assistant messages.apps/api: typecheck, 85 tests, biome all green.Summary by CodeRabbit