Skip to content

fix(chat): fork before edited user turns to keep model context and visible history consistent - #1729

Open
dakjdakd wants to merge 1 commit into
TencentCloud:developfrom
dakjdakd:feature/fix-chat-history-edit-context
Open

dakjdakd wants to merge 1 commit into
TencentCloud:developfrom
dakjdakd:feature/fix-chat-history-edit-context

Conversation

@dakjdakd

@dakjdakd dakjdakd commented Oct 6, 2026

Copy link
Copy Markdown

Summary

Editing an earlier dashboard user message currently truncates only the client's message array, then submits the replacement as a normal turn to the original thread. Its checkpoint still contains the original question, the corresponding answer, and the later conversation. The model therefore receives context that has disappeared from the UI.

This PR makes an edit continue in a new thread seeded before the selected user message. It reuses the existing fork infrastructure to copy the retained prefix into both the checkpoint and stored history, resends the replacement into that new thread, and navigates to the resulting conversation. The original thread remains available with its complete transcript.

Fixes #1728

This branch targets develop at 2571a04c30a8893d9dacf1bb169946824feeb5d2. The source links below intentionally refer to the earlier revision where the original behavior was inspected.

Before and after

Consider a completed conversation:

U1: The launch budget is $10,000.
A1: [Budget discussion.]
U2: Allocate it between marketing and engineering.
A2: [An allocation.]

If U1 is changed to The launch budget is $25,000., the existing implementation displays only the replacement locally but continues the original checkpoint:

original U1 → A1 → U2 → A2 → replacement U1

After this change, the new conversation's model input begins with:

replacement U1

If U2 is edited instead, the new conversation retains:

U1 → A1 → replacement U2

The old U2, its answer, and later turns are excluded. This is a conversation-context change, not an attempt to undo tool side effects that have already happened.

Backend boundary and storage

The existing POST /agents/{agent_id}/threads/{thread_id}/fork endpoint gains an optional user_turns_from_end locator. Existing assistant-fork locators and copy-through boundaries remain unchanged; forks now also inherit conversation mode and approval policy. A request cannot specify both user and assistant turn locators.

For an edit, fork_dashboard_thread finds the selected user message and copies messages[:index]. The selected user itself is excluded, so the edited text becomes a new user turn after the copied prefix. Editing the first user message produces an empty prefix and a fresh conversation.

The implementation reuses the existing full-history read and fork write paths:

  • Versioned sources are read through HistoryArchive, including any legacy segments.
  • Other sources are read from checkpoint history.
  • The prefix seeds the destination checkpoint and its versioned archive or legacy read projection through the existing fork machinery.
  • The source transcript and checkpoint are preserved.
  • The destination inherits model reference, reasoning settings, conversation mode, and thread-scoped approval policy. Completed history is copied; an outstanding approval is not transferred into a new continuation.

No schema migration, new storage format, or separate history-rewrite subsystem is introduced.

Message location

A persisted user message ID is preferred when it exists in checkpoint history. This avoids moving the edit boundary merely because another client appended a turn after the dashboard loaded.

Temporary UI IDs use a suffix count of user turns, not assistant bubbles or tool messages. Counting from the end also supports a dashboard that has loaded only the recent page and then prepended older pages. A multimodal HumanMessage remains one user turn, and intermediate tool traffic does not change the count.

When the supplied original text is non-empty, the fallback checks it and rejects an obvious mismatch instead of silently editing a different turn. It permits the original text followed by server-generated attachment hints. This is deliberately a fallback for UI IDs, not a claim to solve every possible concurrent-history ambiguity; a repeated identical prompt can still be indistinguishable without a persisted ID.

Dashboard continuation and recovery

The dashboard no longer rewrites the source's local message array before preparing the backend context. It requests the fork, reads the destination's server history, and waits if its projection is still loading. Only then does it append and send the replacement into the destination and navigate to it.

The replacement retains the original message's attachments and available composer context: model, connectors, knowledge bases, selected experts, and reasoning settings. The inherited thread settings provide conversation-mode and approval-policy continuity.

A failed fork or destination-history read leaves the source's messages and pagination cursor intact and does not send the replacement to the source. The UI reports an edit failure through an explicit English/Chinese message. A history-read failure after a successful fork can leave the newly created prefix-only conversation available; it does not destroy or rewrite the source.

The edit operation is guarded on both sides. The dashboard avoids editing a streaming conversation, active team speaker, or pending recoverable HITL pause, and prevents overlapping edit requests. The API continues to validate agent/thread ownership and rejects user-edit forks while the source turn or agent is active or a pending approval exists. Users must finish or stop the active work before editing.

The obsolete client-only truncateAndReplaceUserMessage helper is removed, and the user-visible behavior is recorded in CHANGELOG.md.

Relevant existing code

The affected original behavior can be inspected at the fixed base revision:

Target branch

  • Base is develop (feature / fix — default)
  • Base is main (release/* or hotfix/* only)

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Refactor / chore
  • Release / hotfix

Test plan

Backend regression coverage exercises the REST endpoint with real database-backed thread/history services, for both legacy and versioned history and for edits to the first and later user turns. It checks the retained checkpoint prefix, destination history, source preservation, composer settings, ownership denial, active-turn/HITL rejection, and ambiguous locator validation. Existing assistant-fork behavior remains covered.

The model-context regression uses a real LangGraph MessagesState graph and InMemorySaver. Its deterministic model node records the messages it receives after the replacement is invoked. For a first-turn edit it receives only edited; for a second-turn edit it receives first, first answer, edited. Neither case contains the original edited question, its old answer, or subsequent turns. This validates the context supplied to the model node with the actual checkpoint/reducer machinery; it does not require a live provider or depend on the wording of generated output.

Frontend regression coverage runs the actual edit hook and chat store with mocked API/transport boundaries. Seven edit tests cover successful destination hydration and resend arguments, attachments and composer selections, source preservation, failed fork, failed history load, empty first-turn prefix, active stream/pending approval, and waiting for projection readiness.

Validation completed locally on Windows with Python 3.13.2:

Check Result
uv run --no-sync pytest tests/unit/agents/test_thread_fork.py tests/integration/test_chat_ws.py -q 39 passed after rebasing onto the latest develop
Final test_thread_fork.py run after the additional multimodal locator regression 14 passed
History/gateway projection/API history/thread database/fork tests after rebasing 79 passed
Seven dashboard Vitest files, including the five related history/edit files, useSessions.test.ts, and TrajectoryInspector.test.tsx 38 passed after rebasing
uv run --no-sync pytest tests/unit/i18n -q 76 passed
uv run --no-sync ruff check src tests Passed
uv run --no-sync ruff format --check src tests Passed
uv run --no-sync mypy --strict src/octop Passed, 563 source files
TypeScript app and Node projects, each with a separate incremental cache Passed
npm run build Passed
git diff --check Passed
make all PYTEST_JOBS=2 Passed: 4,197 passed, 144 skipped; backend formatting, Ruff, strict mypy, and the full non-live suite

The updated session-list and trajectory tests are included in the frontend run against the current develop base. The model-context check uses a local deterministic graph; the reproduction and regression coverage do not require external provider credentials.

The repository pre-commit hook passed (make precommit: 121 passed, 35 skipped), and npm run build completed the TypeScript, Vite, and PWA build.

  • make all passes locally
  • Added/updated tests

Checklist

  • Updated CHANGELOG.md (if user-facing)
  • README / docs updated (if needed)

The API request model and endpoint summary describe the new edit boundary in the generated API documentation. No installation or configuration change is required.

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant