Skip to content

feat(simplex): treat an edited inbound message as a correction that supersedes the queued/in-flight original - #97317

Open
DavidMetcalfe wants to merge 4 commits into
NousResearch:mainfrom
DavidMetcalfe:feat/simplex-edit-supersede-35535
Open

DavidMetcalfe wants to merge 4 commits into
NousResearch:mainfrom
DavidMetcalfe:feat/simplex-edit-supersede-35535

Conversation

@DavidMetcalfe

Copy link
Copy Markdown
Contributor

Summary

Implements edited-message supersede for the SimpleX gateway adapter, closing #35535.

Today the SimpleX adapter handles only newChatItems; a chatItemUpdated event (the terminal-API notification emitted when a user edits a sent message) falls into the unhandled-event log, so the agent acts on the original — mistaken — text. This PR treats an inbound edit as "I meant this instead", correlated to the specific original message by platform message id:

  • Original still queued (primary pending slot or overflow FIFO) → the queued MessageEvent's text is replaced in place. FIFO order, identity, and metadata are preserved; the correction is silent (no busy-ack).
  • Original in-flight (the turn's originating message id, now recorded on TurnState.active_message_id) → the running agent is redirected with the corrected text, framed as [User edited their earlier message. Corrected message: "..."]. Falls back to steer() when the agent doesn't support active-turn redirect, and to normal queueing when neither primitive is available. Neither primitive cancels in-flight tools — that is the best available behavior without mutating the transcript (see design notes).
  • Uncorrelated edits (no queued or in-flight match) → dropped with an info log rather than dispatched as a new message. Rationale in Open questions below.

Changes

plugins/platforms/simplex/adapter.py

  • _handle_chat_item now extracts meta.itemId and sets message_id=str(itemId) on every inbound MessageEvent (the prerequisite correlation key — previously never populated).
  • New chatItemUpdated branch routes the event's AChatItem through the same parse path with is_edit=True, tagging metadata={"is_edit": True}. The existing outgoing-direction guard means edits of the bot's own messages are ignored.

gateway/session_state.py

  • TurnState.active_message_id records the message id of the event that started the turn; cleared by the existing TurnState.clear() at every turn boundary (no new lifecycle to maintain, no leak path).

gateway/run.py

  • _replace_queued_message(): first-match, in-place text replacement across both queue levels, modeled on the existing _clear_goal_pending_continuations pattern.
  • _handle_edit_supersede(): the correlation decision — queued replace → in-flight redirect/steer → drop+log. Fully synchronous (no await between the queue check, the in-flight check, and the decision), so there is no interleaving window with queue promotion. Inserted in _handle_message after authorization, ignored-channel, and plugin-hook guards; adapter producers other than SimpleX get the same behavior free by tagging metadata["is_edit"] + message_id.

Design notes

  • Why not rewrite the transcript: the stale original text stays in history; per the project's prompt-caching constraint, past context is never mutated. The correction reaches the live turn via redirect/steer, mirroring existing mid-turn steering semantics.
  • Why gateway-core, not adapter-only: correlating an edit to the specific queued/in-flight original requires the queue/turn structures the adapter can't see — as the issue describes. Any future adapter that tags edits (Signal and Discord both surface them today in other forms) inherits the behavior.
  • Startup-resume path: the synthetic resume event carries no message id; its active_message_id stays None, so edits during a resumed turn fall to the uncorrelated policy rather than misfiring.

Open questions

  • Uncorrelated-edit policy: dropping (with a log) rather than queueing as a new message is a judgment call — Pro-side reasoning in the design review was that an isolated out-of-context correction ("the") injected into a fresh turn confuses the model; the counter-argument is that a drop silently loses user intent. If maintainers prefer "queue uncorrelated edits as normal messages", it's a two-line change in _handle_edit_supersede.

Tests

  • tests/gateway/test_simplex_plugin.py: 7 new tests — direct/group edit correlation, outgoing-direction suppression, message_id population on the normal path, image-content edits, missing-itemId fallback.
  • tests/gateway/test_edit_supersede.py (new): 15 tests — primary-slot replace, overflow-FIFO replace, in-flight redirect, steer fallback, sentinel-window handling, stale-correlation drop, uncorrelated drop, non-edit pass-through.
  • All 38 pass locally; the full tests/gateway/ suite shows only pre-existing environment flakes present on clean main.

Closes #35535

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 28, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Overall: Thoughtful edit-supersede implementation that treats a chatItemUpdated edit as a correction correlated by itemId/message_id. Queued text is replaced in-place (order preserved), in-flight turns are redirect/steer-framed, and uncorrelated edits are dropped — all synchronous on the gateway's single event loop with no await gaps (so no lock needed).

What it does

  • plugins/platforms/simplex/adapter.py:173 adds chatItemUpdated branch (stable itemId → message_id=str(itemId), metadata={"is_edit":True}), guards outgoing edits via existing direction check, and tags new newChatItems similarly; file-caption-only edit intentional.
  • gateway/session_state.py:73 adds TurnState.active_message_id (cleared with clear()).
  • gateway/run.py:9262 adds _replace_queued_message(session_key, adapter, message_id, new_text) (primary _pending_messages[session_key] then overflow FIFO queued_events, mutates .text in place, preserves FIFO) and _handle_edit_supersede(event, session_key, adapter) (edit+id guard, queued replace silent, in-flight active_message_id match → redirect(framed)steer fallback, both best-effort, no tool cancellation, uncorrelated → info log + drop; explicitly notes single-loop synchrony avoids interleaving).
  • Tests tests/gateway/test_edit_supersede.py:234 and tests/gateway/test_simplex_plugin.py:411 cover queued primary/overflow replacement, no-match, in-flight redirect/steer, dispatcher wiring, and simplex direct/group/outgoing/message_id propagation.

Non-blocking notes

  • redirect vs steer is best-available: neither cancels in-flight tools — documented as without transcript mutation; mid-tool correction will interleave — acceptable tradeoff.
  • uncorrelated edit → dropped is deliberate to avoid out-of-context model confusion — ensure operator expectation documented (edit of already-consumed turn is not replayed).
  • Session-scoped correlation prevents cross-chat overwrite in groups (key derived from platform+chat_id) — correct.

Non-blocking — please use your judgment.

@DavidMetcalfe

Copy link
Copy Markdown
Contributor Author

@Enough1122 Thanks for the review.

One correction on the summary: the newChatItems path is not tagged as an edit — only the chatItemUpdated branch sets metadata={"is_edit": True} (plugins/platforms/simplex/adapter.py:676-677, gated on the is_edit parameter that branch passes); normal inbound messages gain only message_id=str(itemId). The gateway's supersede logic keys off is_edit + message_id, so ordinary messages are unaffected.

(Note: the review's line references had drifted — the chatItemUpdated branch is at adapter.py:453, TurnState.active_message_id at session_state.py:78, _replace_queued_message at run.py:9265.)

On the three non-blocking notes — all as designed: the redirect/steer no-tool-cancellation tradeoff is documented in the PR body's design notes, the uncorrelated-edit drop policy is under "Open questions" in the PR body awaiting maintainer preference, and group sessions are keyed per-chat so cross-user supersede can't happen.

@Enough1122

Copy link
Copy Markdown
Contributor

Thanks for the correction — you're right. Re-reading at head b300eda6e9, only the chatItemUpdated branch sets metadata={"is_edit": True} (adapter.py:676-677, gated on is_edit), while newChatItems just gains message_id=str(itemId) — ordinary inbound messages are unaffected and the gateway's is_edit + message_id correlation is clean. My summary line conflated the two paths; noted and corrected.

The per-chat session keying and the documented redirect/steer and uncorrelated-drop tradeoffs are as you say — no action needed from me.

@DavidMetcalfe

Copy link
Copy Markdown
Contributor Author

@Enough1122 Appreciate the follow-up — glad the distinction is clear now. No further action needed on this thread.

@Enough1122

Copy link
Copy Markdown
Contributor

Ack — correction noted and accepted. Thanks for clarifying the is_edit gating (only chatItemUpdated branch tags metadata={"is_edit": True}) and the documented tradeoffs. No further items from me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(simplex): treat an edited inbound message as a correction that supersedes the in-flight/queued original

3 participants