Skip to content

fix(client): remove reverted prompts from live history - #9857

Open
juliusmarminge wants to merge 2 commits into
mainfrom
audit/reverted-prompt-retention
Open

juliusmarminge wants to merge 2 commits into
mainfrom
audit/reverted-prompt-retention

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 5, 2026 •

Copy link
Copy Markdown
Member

After a successful revert, the live client keeps user prompts without a turn ID even when the server has removed them. Reloading the thread produces different history.

Human-held: the ordinary two-turn case is fixed, but additional receipt tests confirm a pre-existing server/client disagreement for steering messages and turnless assistant commentary. This candidate preserves those earlier rows in the live client while the server drops them. Choosing preservation versus matching the server's removal needs a maintainer decision; this PR is not ready for automatic merge.

Bound those prompts by the latest retained checkpoint's assistant message, falling back to its completion time when that message is outside the loaded window. Preserve messages bound to retained turns and system messages. This carries forward only Amit Ray's retention correction and regression tests from closed #6044, with an additional no-surviving-checkpoint control. The author invited reuse, and maintainer edits on that closed branch are unavailable.

Addresses the live-history portion of #5685. Restoring prompts, images or existing drafts into the composer is not included; the issue remains open for those recovery choices.

Before and after

A disposable SQLite orchestration engine generated two turns and checkpoint records at 82f64cd8. After its committed revert-completion receipt:

History Before After
Persisted server snapshot Prompt 1, Answer 1 Prompt 1, Answer 1
Actual shared client reducer, given that exact persisted event Prompt 1, Answer 1, Prompt 2 Prompt 1, Answer 1

The receipt fixture waits on engine receipts and worker drains, without sleeps. This verifies post-revert history projection, not a Git/provider rewind or browser interaction. No UI component, provider behavior, schema, dependency or stored-data migration changes.

Additional receipt-derived controls use real persisted snapshots, not reducer-only expected arrays:

Case Result at this candidate
Ordinary two-turn history Live and persisted histories match
Missing checkpoint assistant ID, completion-time fallback Match
Loaded client window starts after the retained checkpoint Match for the loaded message IDs
Same-turn steering before the retained checkpoint Live keeps the steering prompt; persisted history drops it
Turnless assistant commentary before the retained checkpoint Live keeps the commentary; persisted history drops it

The last two are unresolved history-policy limits, not claims of complete parity. Server retention and contracts remain unchanged.

Checks

  • 49 focused shared-client reducer and pagination tests pass. The original, paginated-window and steering-history regressions failed on main before the correction.
  • Checkpoint-zero and no-retained-checkpoint controls preserve system messages and remove turnless prompts.
  • Client-runtime typecheck, targeted lint and diff checks pass.
  • Web, desktop and mobile consume this shared reducer. No native-device run is claimed.

The earlier CI attempt was infrastructure-blocked by Ubuntu HTTP mirror timeouts. This branch is now refreshed onto ce4712d5, which includes the upstream HTTPS mirror repair. All 49 focused tests, client-runtime typecheck and targeted lint pass again. A new current-head CI cycle is running. No extra workflow change or check bypass was made. The human history-policy hold above remains in place.

Original correction by Amit Ray. Prepared and verified with GPT 6 Astra via Codex in T3 Code.


Note

Medium Risk
Changes shared revert projection logic used by web/desktop/mobile; wrong bounds could drop or keep the wrong messages in chat history, though scope is limited to thread.reverted handling and is heavily regression-tested.

Overview
Fixes live thread history after a thread.reverted event so reverted user prompts (and related rows) disappear without a full reload.

retainMessagesAfterRevert no longer keeps every message with a null turnId. Turn-bound messages still follow retained checkpoints; system messages stay. Turnless user/assistant rows are kept only through the latest retained checkpoint—by index through that checkpoint’s assistantMessageId when it’s in the loaded window, otherwise by createdAt vs the checkpoint’s completedAt.

The existing two-turn revert test is updated (explicit second prompt + assistant id), with new reducer tests for paginated windows, no surviving checkpoint, and pre-checkpoint steering/commentary retention.

Reviewed by Cursor Bugbot for commit 576461e. Bugbot is set up for automated code reviews on this repo. Configure here.

Carry forward the focused retention correction from #6044.

Co-authored-by: Amit Ray <51674969+amitray007@users.noreply.github.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 5, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 5, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 956e4b5

Macroscope's review found this PR approvable — This is a focused client-runtime correction that removes turnless prompts beyond a retained checkpoint after a revert while preserving valid prior history. The implementation is confined to the reducer and includes targeted coverage for pagination, missing checkpoints, and steering-message cases.

No code changes detected at 576461e. Prior analysis still applies.

You can add or adjust custom eligibility rules. Learn more.

@github-actions

github-actions Bot commented Sep 5, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.5 KiB 13.5 KiB −8 B (−0.1%) 15.1 KiB ✅
Codex Thread snapshot wire 7.0 KiB 7.0 KiB −3 B (−0.0%) 7.3 KiB ✅
Codex Live turn WebSocket wire 6.5 KiB 6.5 KiB −5 B (−0.1%) 7.8 KiB ✅
Codex Live turn WebSocket decoded 57.0 KiB 57.0 KiB 0 B (0.0%) 66.4 KiB ✅
Codex Live turn messages 8 8 0 (0.0%) 21 ✅
Claude Total thread wire 13.8 KiB 13.5 KiB −225 B (−1.6%) 15.1 KiB ✅
Claude Thread snapshot wire 7.0 KiB 7.0 KiB +4 B (+0.1%) 7.3 KiB ✅
Claude Live turn WebSocket wire 6.7 KiB 6.5 KiB −229 B (−3.3%) 7.8 KiB ✅
Claude Live turn WebSocket decoded 59.3 KiB 57.8 KiB −1.5 KiB (−2.5%) 66.4 KiB ✅
Claude Live turn messages 10 8 −2 (−20.0%) 21 ✅

Baseline: ce4712d · PR result: 576461e · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 113.8 KiB
  • Claude decoded thread snapshot: 114.5 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@shivamhwp

Copy link
Copy Markdown
Collaborator

Note: GPT-6 on behalf of shivam (@shivamhwp).

Current main already removes the ordinary second prompt through #5362, but the loaded-pagination case still needs a fix. Rebasing this retention rule must preserve main's imported-history exception: at checkpoint zero it currently removes both imported prompts and imported answers, while the persisted server history keeps them. Retain isImportedAgentSessionMessageId rows alongside system messages.

The completion-time fallback also needs compareDateTimeStrings, rather than lexical <=. For a 09:00Z checkpoint, a message at 10:30+02:00 belongs before the cutoff but is dropped, while 07:30-02:00 belongs after it but is kept.

The existing history-policy hold still applies. There is also a current fallback disagreement when no checkpoint survives in the loaded client state and turnCount is positive: this candidate drops all turnless rows, while main retains fallback messages. Resolve that alongside the steering/commentary policy before treating live and persisted history as consistent.

This branch has not been deployed

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

Labels

size:M 30-99 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.

2 participants