Skip to content

fix: address PR #4639 review feedback - seed currentContent and fix auto-save scheduling#4834

Merged
marinatrajk merged 1 commit into
mainfrom
swarm/task-3
Feb 19, 2026
Merged

fix: address PR #4639 review feedback - seed currentContent and fix auto-save scheduling#4834
marinatrajk merged 1 commit into
mainfrom
swarm/task-3

Conversation

@marinatrajk

@marinatrajk marinatrajk commented Feb 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Addresses review feedback on #4639 (feat: persist document widget in chat after app reload).

Fix 1: Seed currentContent in createDocument

createDocument now sets self.currentContent = initialContent before marking the document active. Previously, when a document was opened via document_load_response (with existing content) and a subsequent append-mode streaming update arrived, currentContent was nil, causing the buffer to drop the original text and lose data on reload.

Fix 2: Schedule auto-save before the coordinator guard

Moved scheduleAutoSave() to before the guard let coordinator = editorCoordinator else { return } block in updateDocument. Previously, if all streaming updates completed before the WebView coordinator finished loading, scheduleAutoSave() was never called and the document was never persisted. The auto-save now fires regardless of coordinator readiness.

Files Modified

  • clients/macos/vellum-assistant/Features/MainWindow/DocumentManager.swift

Notes

Fix 3 from the review (use selection = .panel(.documentEditor) instead of togglePanel) was already applied in commit d3f80158 ("fix: prevent document editor panel closing on second show") and is not included here.

🤖 Generated with Claude Code


Open with Devin

@marinatrajk marinatrajk self-assigned this Feb 19, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 3 additional findings in Devin Review.

Open in Devin Review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Duplicate scheduleAutoSave() call — original not removed after "move"

The PR description states scheduleAutoSave() was moved before the guard let coordinator check, but the original call at line 94 was left in place. When the coordinator IS available (the normal path), scheduleAutoSave() is called twice in the same synchronous execution: once at line 83 and again at line 94.

Root Cause and Impact

The second call at line 94 immediately cancels the autoSaveTask created by the first call at line 83 and schedules a brand new one. While this is functionally equivalent to calling it once (due to the cancel-and-reschedule pattern in scheduleAutoSave()), it is clearly unintentional leftover code from an incomplete move.

During rapid streaming updates, each updateDocument call resets the debounce timer twice instead of once per invocation. The practical impact is negligible since both calls happen synchronously, but the redundant call should be removed to match the stated intent of the fix.

(Refers to line 94)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

…ore guard

Addresses review feedback on #4639:
- createDocument now sets self.currentContent = initialContent so subsequent
  append-mode updateDocument calls do not lose the loaded content
- scheduleAutoSave() is now called before the guard let coordinator check so
  the document is persisted even when the WebView coordinator is not yet ready

Co-Authored-By: Claude <noreply@anthropic.com>
@marinatrajk marinatrajk merged commit 67a2d6b into main Feb 19, 2026
2 checks passed
@marinatrajk marinatrajk deleted the swarm/task-3 branch February 19, 2026 01:52
ashleeradka added a commit that referenced this pull request Apr 28, 2026
… state (LUM-1272) (#28681)

* fix(macos): flush pending autosave before clearing state in closeDocument() (LUM-1272)

- closeDocument() now calls save() before cancelling the autoSave task and
  clearing state, preventing silent data loss when the user closes within
  the 2-second debounce window.
- save() captures title, wordCount, and documentClient as locals before the
  Task to prevent reading stale/cleared values from self after close.
- save() uses [weak self] and a surfaceId scope guard so stale save responses
  from a previous document cannot clobber a newly-opened document's state.
- createDocument() and closeDocument() reset isSaving/lastSaveError for clean
  state at document boundaries.
- Remove redundant scheduleAutoSave() call in updateDocument() (became dead
  after PR #4834 moved the call before the coordinator guard).
- Add deinit to cancel autoSaveTask per @observable task lifecycle guidance.

Apple refs checked (2026-04-28):
- WWDC21 Protect mutable state with Swift actors
- swift#79551 @observable deinit actor isolation

Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>

* fix: flush active document in createDocument() before overwriting state

Callers like handleDocumentEditorShow and handleDocumentLoadResponse call
createDocument() directly without closeDocument(). If a document has unsaved
edits, the new document would silently overwrite them. Guard at the data
layer so all callers get the flush automatically.

Co-Authored-By: ashlee@vellum.ai <ashlee@vellum.ai>

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
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