fix: unify history writes and optimize streaming updates - #460
Conversation
Move the history storage rule next to its schema and tests so the root AGENTS.md stays within the documentation CI size limit. Preserve the rule and add the required CLAUDE.md symlink. Model: gpt-6
Clarify note triggers for implementation, research, and design; preserve read-only task restrictions and existing note ownership. Record the policy in bilingual notes and link the repository map to keep root instructions within budget. Model: gpt-6
![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: 9eb42c0a8d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // including unrelated user messages and control-field updates. | ||
| // Preserve the opaque item without extending MessageContentSchema's | ||
| // accepted input types. Known variants keep their existing guards. | ||
| return true; |
There was a problem hiding this comment.
When a newer peer introduces an unknown discriminator whose payload includes a non-string text field, such as structured rich-text metadata, this return true does not provide forward compatibility: every item is still independently validated against the declared text: schema.LoroText() property above, so validation rejects the future item and subsequent Mirror writes remain blocked. Make known-field schemas conditional on the discriminator, or otherwise prevent them from validating unknown variants, so their data is genuinely opaque.
AGENTS.md reference: packages/shared/AGENTS.md:L7-L9
Useful? React with 👍 / 👎.
Make reading, Spec approval, Agent Note updates, and completion checks explicit in root AGENTS.md. Move catalog explanations into a linked guide while preserving binding rules and update the bilingual process note. Model: gpt-6
Disable session Mirror update validation in the renderer and CLI while preserving external input parsing. Cover malformed historical items and both construction sites, and document the temporary tradeoff and replacement contract. Model: gpt-6
Extract the schema-aware HistoryWriter from #376 and route renderer and CLI history changes through one shared path. Parse new and changed inputs before CRDT operations while preserving untouched incompatible history and existing containers. Keep the full Mirror read path, add compatibility and compile-time regressions, and document the contract. Model: gpt-6
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0c55094c3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| timestamp: z.string(), | ||
| userTurnId: z.string().optional(), | ||
| acpTurnId: z.string().optional(), | ||
| items: z.array(MessageContentSchema).optional(), |
There was a problem hiding this comment.
Every session fork reaches SessionDocument.updateHistory with the session_fork_origin notice that cloneHistoryThroughTurn unconditionally appends, but routing all new items through MessageContentSchema rejects that notice because SystemNoticeSchema has no session_fork_origin variant. Consequently both regular and worktree forks throw HistoryWriteError while committing their cloned history and surface TARGET_WRITE_FAILED; add the existing notice and its metadata to the shared parser.
AGENTS.md reference: packages/shared/AGENTS.md:L17-L19
Useful? React with 👍 / 👎.
zxch3n
left a comment
zxch3n
left a comment
There was a problem hiding this comment.
Found one P1 regression at exact head f0c55094c314697494b48955c06582a7302b0ff6. GitHub does not allow the PR author's account to submit a Request changes review, so this is recorded as a blocking review comment.
| timestamp: z.string(), | ||
| userTurnId: z.string().optional(), | ||
| acpTurnId: z.string().optional(), | ||
| items: z.array(MessageContentSchema).optional(), |
There was a problem hiding this comment.
[P1] Accept the existing session_fork_origin notice here. MessageContent and cloneHistoryThroughTurn() still produce { type: 'system_notice', name: 'session_fork_origin', ... }, but SystemNoticeSchema has no matching branch. Both normal and worktree forks later call targetDoc.updateHistory(() => historyResult.history), so this new boundary throws HistoryWriteError: Invalid history write: items.0: invalid_union; the target fork is rolled back or marked failed. I reproduced it through createSessionMirror().historyWriter.append(...). Please add the correlated notice schema and a real writer-path fork regression, since the current fork-service mocks pass without exercising this boundary.
Preserve the shared HistoryWriter owner and the upstream no-op progress guard together. Model: gpt-6
Add the correlated fork-origin schema and bidirectional notice type coverage. Exercise regular and worktree forks through the real SessionDocument and HistoryWriter; retain malformed metadata rejection and document the prior test gap. Model: gpt-6
Separate writer-authored input from writer-captured stored history. Preserve opaque content during forks and guarded edit rollback, with real Loro integration coverage. Model: gpt-6
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3f91aad64
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let historyBeforeWrite: SessionHistoryInput[] | null = null; | ||
| let previousUserId: string | undefined; | ||
| await sessionDoc.updateHistory((currentHistory) => { | ||
| const rollbackHistory = await sessionDoc.updateHistoryWithRollback((currentHistory) => { |
There was a problem hiding this comment.
When persistPendingChanges('session-edit-and-resend-commit') rejects, this rollback is deterministically stale in a real SessionDocument: committing the pending replacement schedules attachAutoMarkLatestUserHistoryAsRead's microtask, and the await here lets that microtask change the replacement to seen before the persistence call begins. The receipt then observes history different from its captured after value and refuses to restore it, leaving the old conversation tail deleted even though edit-and-resend reports failure. Coordinate or suppress this expected auto-read mutation while the rollback transaction is active without allowing unrelated intervening edits to be overwritten.
Useful? React with 👍 / 👎.
zxch3n
left a comment
zxch3n
left a comment
There was a problem hiding this comment.
Re-review at d3f91aad: the previous session_fork_origin parser failure is fixed, including real writer-path coverage for regular/worktree forks and opaque stored history. One blocking P1 remains: the normal auto-read microtask changes the pending replacement before a failed edit-and-resend commit can roll back, so the rollback receipt becomes stale and the old history tail stays deleted. I reproduced this deterministically with the real history writer plus attachAutoMarkLatestUserHistoryAsRead; see the inline thread on session-edit-and-resend-service.ts:370.
Retain fork setup logs, allow replacement read acknowledgements during rollback, and validate tool state changes without reparsing untouched payloads. Model: gpt-6
Keep HistoryWriter-specific contracts and repair notes here; the general documentation workflow is reviewed independently in #466. No runtime code changes. Model: gpt-6
Preserve steer delivery markers through history writes and reads, and validate permission metadata updates independently of legacy tool payloads. Add real-writer regression coverage and update the related specs and notes. Model: gpt-6
Keep source hashes and turn identities stable while validating refreshed history against a cursor-bound stored-content baseline. Cover legacy storage, concurrent edits, delayed cursors and conflict resolution with real writer regressions, and document compatibility limits. Model: gpt-6
Resolve task decisions against the latest history item through HistoryWriter instead of replacing stale entries. Add deterministic replica regressions and document the remaining structural-edit limit. Model: gpt-6
Accept JSON protocol extensions without relaxing known message validation, preserve unchanged legacy tool blocks, and isolate deterministic write failures from subsequent ACP output. Add nested schema contracts, regressions, and implementation notes. Model: gpt-6
Preserve concurrent edits to untouched history rows during edit-and-resend compensation. Add real replica and service regressions and update rollback contracts and notes. Model: gpt-6
Index discriminated schemas, avoid redundant copies and unchanged-container reads, narrow field updates, and batch initial history writes. Add synthetic benchmarks and document the remaining full-Mirror performance gap. Model: gpt-6
Route targeted text and thought batches through turn-local writes, derive cached single-pass input parsers with refinements retained, and reduce item and stored-copy matching overhead. Add compatibility regressions and benchmarks documenting remaining seed and commit costs. Model: gpt-6
Patch the pinned Mirror reader for single existing text updates while preserving descriptors, snapshots, and notifications. Add real-replica regressions, isolated reader benchmarks, and documentation of remaining bulk-import limits. Model: gpt-6
Resolve session validation hotfix conflicts while preserving the shared HistoryWriter entrypoints and text-event optimization. Retain non-session validation coverage and record integrated verification. Model: gpt-5
zxch3n
changed the title
Normalize legacy selectors on new writes, validate independent field edits locally, retain queue items until history acceptance, and capture only the rollback range while preserving later appends. Add regressions and document remaining recovery limits. Model: gpt-5
Preserve the target branch fork-operation clear/reuse fix with the shared SessionMirror facade. Adapt its real-Loro regression and record integrated validation. Model: gpt-5
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 123e913242
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (history.some((entry) => entry.id === queuedTurnId)) { | ||
| await sessionDoc.removeMessageQueueItem(queuedItem.$cid); |
There was a problem hiding this comment.
When appendUserTurn commits the history entry but its subsequent repo.upsertDocMeta call fails, the queue row remains for retry; the next promotion then enters this duplicate-history branch and deletes that row without restoring latestUserMsgId. Because session metadata is the activation index and history is not scanned to infer work, the accepted message is left permanently undispatched. Republish the queued turn's pointer before removing the retry row.
AGENTS.md reference: apps/cli/src/session/AGENTS.md:L52-L54
Useful? React with 👍 / 👎.
Consolidate incremental notes into the bilingual owning record, repair references, remove source-string construction tests, and require behavior-focused tests and compact PR documentation in AGENTS.md. Model: gpt-5
Retain queued messages until history and activation publication succeed. Recover history-only commits without duplicating turns, replaying terminal messages, or replacing another pending activation. Cover metadata failures with a real SessionDocument regression and update the owning documentation. Model: gpt-5
Traverse both sides of schema pipes while preserving preprocessing, transforms, and refinements. Cover nested config filtering, legacy normalization, stored-history preservation, and invalid-write atomicity with real writer tests. Model: gpt-5
Use the released Mirror text-event optimization and remove the temporary 2.3.1 patch. Update the paired React/Jotai packages and lockfile, and record the upstreamed reader path in the owning notes. Model: gpt-5
1 participant
Summary
One shared HistoryWriter for CLI and renderer, preserving incompatible stored history while validating newly authored data. This is a writer extraction/integration and streaming optimization, not the original unknown-item-only hotfix.
Scope:
35f00a82, including main throughe12cb225. An unresolved queue-activation P1 remains; passing checks are not merge approval.Data flow
Performance and scope limits
Cached schema-derived single-pass parsers, target-local text/thought updates, field-local scalar/fileDiff writes, and a pinned Mirror 2.3.1 single-text-event path-copy patch reduce repeated work. The patch preserves descriptors, old snapshots and notifications; structural/mixed/tree paths retain the original reader. Loro remains 1.15.1; no storage migration.
Recorded at
76d0e9be: same-writer old/new-reader isolated measurements were 6.067 → 0.180 ms/chunk at 400 entries in Bun and 5.225 → 0.181 ms/chunk at 200 entries in Node. These are NOT whole-app speedups or current-head benchmark reruns. Full-Mirror reads, multi-event tool updates, bulk fork/capture and atomic baseline growth remain concerns. No real 3000-user-round desktop/mobile acceptance or overall 10x claim.Validation
pnpm check, formatting and docs checks passed for35f00a82.Review comments and remaining risks
appendUserTurnwrites history beforeupsertDocMeta(latestUserMsgId). If the latter fails, retry sees the existing turn and removes the queue row without repairing its activation pointer. The accepted message can remain undispatched. Review thread. This needs a repair that also avoids replaying already-handled queued turns.stale_rollbackcan still leave restoration incomplete after overlapping/structural edits; the caller logs the error. No durable recovery-copy/crash-recovery workflow is provided.storedHistoryBaselinecontains all per-turn hashes; no bounded-size hash-chain replacement is implemented. Generic turn reorder remains unsupported. Arbitrary item structural concurrency is not proven safe.Relationship to #376
#460 extracted the writer/materializer from #376; #376 should remain open for ConversationView, viewport hydration/rendering and cache ownership. After #460 lands, #376 must integrate it and remove its duplicate writer. That integration has NOT happened yet. Read-path rollback must continue using the shared writer.
#443 primitive/Text hints and #359 skeleton/hash-v2 work remain separate. Future Mirror patches must compose with this patch.
Documentation
Incremental notes were consolidated into one bilingual owning record, not erased from Git history. Design and evidence · Draft write contract.