Repository navigation
Flow → CRDT-first: live collaboration for .fl0 - #42
Open
adamhumphrey44 wants to merge 25 commits into
Open
adamhumphrey44 wants to merge 25 commits into
adamhumphrey44 wants to merge 25 commits into
Conversation
…-shape Conflict resolutions of note: - flowstate-flow: kept the branch's Loro-native rewrite (operations.rs replaces actions.rs/history.rs); ported main's persistence hardening into the new binary .fl0 format (64MB read cap, atomic tmp+rename save). Quarantine-on-corrupt deliberately NOT ported: both callers fail loud and leave the file intact, which is strictly safer. The old actions.rs dup-id guard / delete-inverse are subsumed by the rewrite (internal UUID minting; Loro owns undo). - migrated the flow rewrite onto the post-cutover document model: Document -> DocumentProjection, Arc<Vec<Paragraph>> -> ParagraphSeq / BlockSeq make_mut guards, db8_bytes -> document_to_loro + loro_db8_bytes (new flowstate_flow::cell_db8_bytes helper), replace_document_from_collaboration -> replace_document_projection, caret_bounds' new VisualGravity param (Neutral for zoom anchors). - editor caret paint: main's caret_color_rgb + default fallback wins (invisible-caret fix + collab peer colors); the branch's flow-cell caret tinting rides the fallback (config.caret_color first, then the theme default text color). - workspace: kept the branch's toolkit redesign (toggle_toolkit and toolkit_collapsed removed; dropped main's Toolkit menu item and the headless-test field init), branch's ctrl+scroll flow-zoom fallback folded into main's paint-phase canvas hook, unions for asset lists / ribbon imports / status bar (flow zoom + collab phase/roster). - exporter blocks.rs: main's byte-based run slicing is correct for the current model (TextRun.len is bytes) + force_bold threading. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NDcBhNZgW3kjkJSMQg3T7d
Parameterize paragraph_ids_by_boundary over its registry (paragraph_ids_by_boundary_in) and add materialize_single_flow — the body flow-walk law applied to one ensure_flow-shaped map carrying its own paragraphs_by_id child, returning RegionRows with block ids derived 1:1 from paragraph ids. Expose ensure_flow/ensure_sentinel so the .fl0 schema can seed cell flows with the identical canonical shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UFUYkPyp7R8gZEMA6EUjgW
…n law New CRDT-first modules alongside the record-era API (deleted when the editor rewires): format.rs (immutable format vocabulary), loro_schema.rs (flow.meta/sheet_order/sheets_by_id/cells_by_id/annotations containers; MovableList ordering; per-cell .db8-shaped flows with own paragraph registries; seed/replace/mark write helpers), loro_projection.rs (total board materializer with deterministic normalization: liveness, column clamp, dangling-parent orphan, greatest-uuid cycle break, adjacency orphan, canonical DFS re-linearization; CellSummary law), projection.rs (FlowBoardProjection/Sheet/Cell/CellSummary), board_ops.rs (pure drag preview/commit resolution law), intents.rs (FlowIntent vocabulary), FLOWFL0 v2 container with explicit v1 rejection. Gates: import→materialize identity, snapshot round trip, 3-peer raw- mutation determinism fuzz, cycle/orphan/run-split laws, and the two headline defects asserted dead at schema level (reorder-vs-edit, same-cell char merge). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UFUYkPyp7R8gZEMA6EUjgW
…ty, flow I/O The .db8 local_write/crdt_runtime split mirrored at board scale (src/flow/): FlowRuntime behind the SAME generic WriteGate (board projection maintained in place per intent, per-open-cell materialization cache, ordered board + per-cell streams, publish queue, Loro UndoManager configured verbatim — remote/repair excluded, meta inert, FlowUndoMeta board context riding undo items via on_push/on_pop with stack-transformed cursors); commit.rs holds ALL frontier logic (resolve → mutate → ONE 'local' commit with class message → in-place derive → publish, revert_to compensation under 'repair'); cell_text.rs is the cell-scoped resolution law + paragraph-only executors; FlowCellAuthority implements LocalWriteAuthority per cell (whole-cell Replace streams, cursor-exact caret survival via encode/resolve_selection_anchor); FlowIoHandle is the DocIoService sibling with the §6.4 coalescer + byte budget, fork-under-gate exports, and .fl0 v2 save/recovery bytes. Debug builds audit every committed intent's maintained board against a fresh materialization — the audit caught (and the fix codified) the Sibling::After placement law under canonical DFS ordering. Gates: single-peer intents of every class, no-op/rejection zero-mutation, undo/redo ping-pong across classes, undo-meta focus restore, publish-queue convergence onto a second runtime, summary fixture parity. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UFUYkPyp7R8gZEMA6EUjgW
…ariant flow_convergence.rs (N-peer random structural intents, self-consistency vs fresh materialization + cross-peer board equality, fixed CI seeds + ignored soak) and flow_cell_text_fuzz.rs (concurrent same-cell edits through real FlowCellAuthority paths, reorder-vs-edit arm, plus the two headline defects as deterministic tests). The fuzz surfaced a real law gap the debug audit caught: post-merge raw order lists sit in non-canonical interleavings, so index-based writes diverged from the materialized view. Fixes: (1) structural intents re-linearize canonically INSIDE their own commit, reconciling the raw list against liveness drift (dead/dup/missing entries); (2) the drag preview applies the identical canonicalization (one law, preview == commit); (3) imports run a capped 'repair'-origin order-canonicalization pass mirroring schedule_projection_repairs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UFUYkPyp7R8gZEMA6EUjgW
…w I/O pump gates
PROTOCOL_VERSION 3→4. SessionTicket carries document: DocumentKind
{RichText, Flow} INSIDE the HMAC tag (kind-flipping is an authentication
failure, not a silent reroute), with leading-varint version pre-decode so
cross-version tickets yield actionable UX messages ('outdated invite' /
'created by a newer Flowstate') instead of generic decode failures. The
kind threads through standing-access grants and discovery admission
grants into minted tickets. flow_io_pump mirrors the doc_io_pump drip
suite: held-gate coalescing bounds, duplicated/reversed delivery,
deferred non-import ordering, exactly-once publish pump, and .fl0
save/recovery round trips.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UFUYkPyp7R8gZEMA6EUjgW
New sync_io.rs: SyncIoHandle { RichText(DocIoHandle), Flow(FlowIoHandle) }
exposing the three transport-facing calls (oplog_version_vector /
export_updates_for / snapshot_bytes) + as_* accessors.
DirectSessionHandler.io retypes to Option<SyncIoHandle> (the only
in-crate retype; new_rich_text keeps the pre-flow call shape). Flow
sessions now serve direct snapshot/updates pulls straight off the flow
I/O service — verified by a flow-variant loopback pull test alongside the
untouched rich-text loopback regression.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UFUYkPyp7R8gZEMA6EUjgW
flowstate-flow drops the record-era API entirely (FlowDocument/ FlowProjection/entity deltas/v1 persistence deleted; lib.rs is v2-only). FlowEditor holds Arc<FlowDocHandle> + a drained FlowBoardProjection copy; all ~19 op call sites route through handle.apply(FlowIntent::…) (adds collapse into one CellPlacement path), drag commits through MoveCellSubtree with preview == commit via board_ops, undo/redo ride the whole-doc stack with FlowUndoMeta focus restore, and saves fork a v2 .fl0 snapshot. cell_editing.rs's observer/re-encode loop is DELETED — each open cell's RichTextEditor is driven by handle.cell_authority(id) through set_write_authority, and the flow editor only listens for Changed events to refresh its board copy. Inactive cells render from a materialize-on- miss preview cache invalidated by board stream items. create_flow_panel takes a FlowRuntimeSource, spawns the panel's FlowIoHandle into the new Workspace::flow_document_runtimes map; loads go read_fl0 → FlowRuntime::from_snapshot. Gate: headless flow edit/save/undo through the authority (sheet+cell intents, cell typing via FlowCellAuthority, MoveCellSubtree drag commit, undo reversal, .fl0 v2 save round trip) + full app lib suite green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UFUYkPyp7R8gZEMA6EUjgW
…e same CollabSession CollabSession keeps one type; its document-facing half becomes two-arm enums (CollabEditor / JoinedAuthority / JoinedDocumentPayload) keyed by the ticket's DocumentKind. Hosting on an open flow tab reuses the tab's existing gated authority + FlowIoHandle (start_flow_session_for_panel); the join completion match dispatches RichText attachments vs create_flow_panel_from_attachment, which adopts the session's I/O service instead of spawning a duplicate on the same gate. Workspace glue: share button + collaboration dialog gate on flow tabs too, and the window-close prompt cascade scans flow_panels (collaboration_close_panels), so leaving/closing a live flow session prompts exactly like a .db8 one. Gates: full headless suite green (66 tests) including the .db8 collab_glue regression cases, plus new flow cases — host start registers a live phase + self roster entry, leave keeps the tab editable through the same authority, and the close-prompt cascade intercepts live flow sessions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UFUYkPyp7R8gZEMA6EUjgW
… trip Headless gate for the join path: a snapshot-built flow authority + I/O service hand off into a pathless workspace tab that paints the host's board and stays editable through its own gate; autosave skips it; the debounced .fl0 recovery file is written after edits and removed by discard. The two-instance loopback join (snapshot + incremental updates over the direct protocol) is covered by swarm_loopback::flow_direct_snapshot_and_updates_pull. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UFUYkPyp7R8gZEMA6EUjgW
…act cell carets
PresenceState.selection becomes focus: PresenceFocus{Text,Flow} (wire
change covered by the protocol v4 bump; caps/impersonation guards apply
per-arm, and an oversized flow caret drops just the caret, keeping the
board focus). FlowPresenceFocus carries sheet/cell/editing plus the
caret as encoded Loro cursors, produced and resolved under the flow
gate (FlowDocHandle::presence_selection / resolve_presence_selection),
so remote carets land at exact positions and foreign-cell cursors fail
safely on the container check.
Session glue: the flow arm of refresh_own_presence reads the editor's
board focus synchronously; the external refresh maps roster entries to
FlowExternalPresence and hands them to the flow editor, which paints a
peer-color outline + name chip on the focused cell, colored dots on the
peer's sheet row in the outline, and forwards carets into open cell
editors via the existing ExternalCaret API (re-resolved after remote
imports). Cell caret motion emits CellSelectionChanged so presence
republishes on the same debounce as .db8 selection changes.
Gate: headless synthetic remote focus asserts the cell outline color,
sheet dot, exact forwarded caret offset, and clear-on-leave; presence
caps/impersonation suites and the full collab test set stay green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UFUYkPyp7R8gZEMA6EUjgW
…ures, clippy/fmt clean Dropbox: bound .fl0 files mirror on save/autosave via sync_bound_flow_file — raw v2 container upload against the stored rev; on conflict the remote merges through the flow I/O service (CRDT import), the local file rewrites, and the merged container re-uploads; a flow.meta document-id gate prevents cross-document clobbering, same as the .db8 checkpoint path. flowstate-soak gains a flow_soak bin: two peers drive structural intents + same-cell typing through the real gated write path with per-round update exchange, then prove convergence (boards byte-equal and equal a fresh materialization, no typed character lost) and print per-stage wall-clock distributions. Hotpath instrumentation lands on apply_flow_intent and the board/cell materializers (flowstate-flow grows the standard hotpath feature wiring). The v1-era flowstate-flow examples are ported to the v2 schema (ergonomics_fixture writes canonical-order boards and asserts a defect-free materialization; dump_flow reads .fl0 v2). cargo clippy --workspace --all-targets and cargo fmt --check are clean; new integration test files follow the repo's cfg(test)-module convention; JoinedDocumentPayload boxes its rich-text arm. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UFUYkPyp7R8gZEMA6EUjgW
Contributor
📝 WalkthroughWalkthroughFlowstate is migrated to a Loro-backed Flow v2 document model with collaborative intents, per-cell editing, Flow sessions, FL0 persistence, presence support, and new Flow editor UI. Rich-text zoom, search filtering, menus, and supporting workspace integrations are also updated. ChangesFlow v2 foundation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/gpui-flowtext/src/rich_text/editor/commands.rs (1)
457-473: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winParagraph-restricted paste bypasses equation/table-cell routing
Theallow_paragraph_breaksbranch returns before the equation/table-cell handlers, so clipboard paste in those selections falls back to the document body instead ofinsert_text_into_selected_equation/ table-cell paste logic. Move the selection-specific routing ahead of this gate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/gpui-flowtext/src/rich_text/editor/commands.rs` around lines 457 - 473, The paste method’s allow_paragraph_breaks guard returns before equation and table-cell selection handling, bypassing their specialized insertion paths. Move or invoke the selection-specific routing, including insert_text_into_selected_equation and table-cell paste logic, before the paragraph-restriction branch while preserving the existing restricted-paragraph flattening behavior for ordinary document selections.
🧹 Nitpick comments (6)
crates/flowstate-soak/src/bin/flow_soak.rs (1)
187-195: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConvergence proof only checks for lost keystrokes, not duplicated ones.
text_a.chars().count() >= rounds * keystrokes_per_round * 2only guards against losing characters. Since everyInsertTextcall in this soak is?-propagated (no rejection tolerance, unlike the fuzz tests), the exact count is always known ahead of time — an==check would also catch accidental duplication introduced by a merge bug, which>=would silently let through.🔎 Proposed tightening
- anyhow::ensure!( - text_a.chars().count() >= rounds * keystrokes_per_round * 2, - "typed characters were lost: {} < {}", - text_a.chars().count(), - rounds * keystrokes_per_round * 2, - ); + anyhow::ensure!( + text_a.chars().count() == rounds * keystrokes_per_round * 2, + "typed characters were lost or duplicated: {} != {}", + text_a.chars().count(), + rounds * keystrokes_per_round * 2, + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/flowstate-soak/src/bin/flow_soak.rs` around lines 187 - 195, Update the final character-count assertion in the soak convergence check around cell_text and the “typed characters were lost” ensure to require exact equality with rounds * keystrokes_per_round * 2 instead of a lower bound, so both lost and duplicated keystrokes are detected while preserving the existing shared-text convergence check.crates/flowstate/src/flow/cell_theme.rs (1)
22-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMagic style-slot numbers lack context.
[3, 4, 6]and1are hardcoded paragraph/semantic style slots with no comment or named constant explaining what they represent. A future change to the flow cell's custom style slot assignments elsewhere could silently desync from this list.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/flowstate/src/flow/cell_theme.rs` around lines 22 - 32, Replace the hardcoded style-slot values in the flow cell theme setup with named constants or shared definitions that identify the corresponding paragraph and semantic styles. Update the insertions in the theme initialization block to reuse those definitions, keeping the current slot assignments while ensuring future custom style changes cannot silently diverge.crates/flowstate-flow/Cargo.toml (1)
12-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared workspace pins for
loroandpostcardcrates/flowstate-flow/Cargo.toml:15-16already has these dependencies declared in the workspace; switching to{ workspace = true }here keeps the version source centralized and avoids drift with the other crates.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/flowstate-flow/Cargo.toml` around lines 12 - 21, Update the loro and postcard dependency declarations in the flowstate-flow manifest to use the shared workspace entries instead of local version pins. Preserve their existing dependency names and rely on the workspace-managed versions, matching the surrounding workspace dependency declarations.crates/flowstate-collab/src/presence.rs (1)
326-350: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd test coverage for the new Flow-focus caret-drop sanitize branch.
sanitize_presence_statenow has a distinct code path that keeps a Flow focus's sheet/cell/editing while dropping only an oversized caret, but no test in this module exercises it (existing tests only cover the display-name cap). Given this guards a wire-exposed cap (Part B), a targeted test would be cheap and valuable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/flowstate-collab/src/presence.rs` around lines 326 - 350, Add a targeted test in the presence module for sanitize_presence_state where a Flow focus has an oversized caret encoding. Assert that the sanitized result preserves the Flow focus’s sheet, cell, and editing fields while setting caret to None, covering the distinct caret-drop branch and Part B cap behavior.crates/flowstate/src/flow/editor.rs (2)
1061-1068: 🎯 Functional Correctness | 🔵 Trivial"Send to document" button has no click handler.
.on_click(|_, _, _| {})is a no-op stub — clicking currently does nothing. If this is an intentionally deferred feature, consider a tracking marker; happy to help wire this up if there's a concrete target action.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/flowstate/src/flow/editor.rs` around lines 1061 - 1068, Replace the no-op on_click handler on the “Send to document” Button in the flow editor with the intended send-to-document action; if the feature is intentionally deferred, add an explicit tracking marker instead of leaving an empty callback.
1119-1139: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftPer-cell restyle + theme transform recomputed every render, for every inactive cell.
rendered_documentis cloned fromself.cell_documentsand mutated fresh each render: conditionally restyling the first paragraph, rebuildingdocument.blocks, then runningapply_flow_cell_theme(which does OKLCH color transforms per custom style) — none of this is written back into the cache. For boards with many cells this repeats non-trivial work every frame. Note a naive "cache the themed result" fix would need to stay keyed onself.board_zoom/theme, sinceapply_flow_cell_themeintentionally re-applies live zoom scaling on every call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/flowstate/src/flow/editor.rs` around lines 1119 - 1139, Cache the post-restyle, themed document instead of cloning and recomputing it on every render in the cell rendering flow around rendered_document and apply_flow_cell_theme. Store the transformed result back in the appropriate cache, and invalidate or key it by the active client theme, side color, background, and board_zoom so live zoom scaling remains correct. Preserve uses_summary_projection and the existing first-paragraph restyling behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/flowstate-collab/src/flow/flow_io.rs`:
- Around line 157-231: Update io_loop’s deferred request storage from a Vec to a
FIFO queue such as VecDeque, retrieve pending requests from the front with
pop_front, and replace both deferred.push calls in the pre-lock and post-lock
import drains with push_back. Preserve arrival order for all non-import requests
deferred during coalescing.
In `@crates/flowstate-collab/src/flow/runtime.rs`:
- Around line 386-407: Treat refresh_all failures in import_remote_updates and
undo_redo_step as unrecoverable after self.doc mutation: invoke the existing
reload/poisoned-state handling path rather than returning a plain error.
Preserve refresh_all’s successful board and stream refresh behavior, and use the
same failure mechanism already applied to other unrecoverable write failures.
In `@crates/flowstate-flow/src/loro_projection.rs`:
- Around line 146-178: Update the cell normalization loop to emit the
appropriate FlowDefect when a cell record has a missing or mismatched “id”
instead of silently continuing, and update the per-sheet normalization loop
around sheet_keys to emit a defect when a key is not a valid UUID. Reuse the
existing FlowDefect variants and contextual identifiers used for similar
normalization failures, while preserving the current skip behavior after
recording each defect.
In `@crates/flowstate-flow/src/loro_schema.rs`:
- Around line 386-394: Update cell_flow_from_paragraphs to maintain a running
Unicode character count while building full, and push that counter for each
paragraph instead of calling full.chars().count() repeatedly. Increment the
counter as newline and run text are appended, preserving the existing boundary
positions and output behavior.
In `@crates/flowstate-flow/src/persistence.rs`:
- Around line 31-63: Update decode_fl0_snapshot to bound decompressed output,
not just the input checked against MAX_FL0_BYTES. Replace
zstd::stream::decode_all with a streaming decoder and read through a limited
buffer, rejecting the payload once the decoded byte count exceeds the hard
snapshot limit before returning the collected bytes.
In `@crates/flowstate/src/collab/manager.rs`:
- Around line 155-160: Update the FLOW discovery registration around
canonical_document_id to avoid converting a missing document_id() into 0. Mirror
collaboration_discovery_context by treating the unavailable ID as a skip
condition, and only continue registration when a real document ID can be
converted to its u128 fingerprint.
In `@crates/flowstate/src/collab/session.rs`:
- Around line 912-936: Update finish_join_flow_snapshot to load and bind the
joining collaborator’s author identity on the newly created FlowRuntime by
invoking its existing set_author_identity(...) flow before FlowDocHandle::new.
Reuse the same identity source and loading/binding behavior used by the other
join path, ensuring author_user_id is populated before later revisions are
processed.
In `@crates/flowstate/src/flow/editor.rs`:
- Around line 689-694: Update drop_cell_editor to handle errors returned by
self.handle.close_cell(cell) instead of discarding them. Log unexpected close
failures using the same error-reporting pattern as discard_recovery_file, while
preserving the existing cleanup of cell editors, themes, and subscriptions.
- Around line 719-739: Update save_to_path to capture the current save/edit
generation when taking the snapshot, then clear editor.dirty only if the write
succeeds and that generation still matches the editor’s latest generation.
Increment the generation wherever edits or remote intent make the document
dirty, preserving dirty state when newer changes arrive during the asynchronous
save.
In `@crates/flowstate/src/flow/ribbon.rs`:
- Around line 107-121: Update the root ribbon mouse-down handler in the view
containing the annotation tool controls so it does not reset the tool when the
event originates from a Marker or Eraser Button click. Preserve resetting to
AnnotationTool::None for other ribbon clicks, allowing active Marker/Eraser
toggles to reach their on_click handlers and turn off correctly.
- Around line 85-106: Update the active-sheet rename input flow around
SheetNameInputState so its displayed value synchronizes with the canonical sheet
name when that name changes while the sheet_id key remains active. Preserve
in-progress local edits by only applying the external value when the input is
not locally modified, and retain the existing Change subscription that calls
rename_active_sheet.
In `@crates/flowstate/src/workspace/document_search_overlay.rs`:
- Around line 547-601: Disambiguate the duplicate “Underline” entries in the
Styles dropdown by renaming the semantic-style label from SEMANTIC_STYLE_FILTERS
and/or the standalone toggle_underline item to clearly indicate “semantic”
versus “direct” formatting. Preserve both existing click handlers and behaviors.
In `@crates/flowstate/src/workspace/toolkit_panel.rs`:
- Around line 460-468: Update the UI that renders the toolkit Insert button to
disable or hide it whenever active_editor is absent, matching the guard in
insert_toolkit_hit. Keep the button enabled only when a rich-text editor is
active, while preserving the existing insertion behavior.
In `@crates/flowstate/src/workspace/workspace/documents.rs`:
- Around line 589-594: Update new_flow to handle FlowDocHandle::new_document
failure like add_document_panel and add_document_panel_with_title: log the
runtime-start error with tracing::error! and surface a critical user prompt
before returning. Preserve the existing add_flow_panel path for successful
document creation.
In `@crates/gpui-flowtext/src/rich_text/editor/scroll_anchor.rs`:
- Around line 48-58: The ParagraphRemainder fallback in
scroll_anchor_item_and_delta must convert delta back to the remainder item’s
local offset before returning, rather than returning the full delta. Update the
cache-miss path associated with valid_chunk_cache_entry and preserve the
cache-hit behavior, so restore_scroll_anchor does not add the chunk height twice
after cache invalidation.
In `@crates/gpui-flowtext/src/rich_text/editor/traits.rs`:
- Around line 40-47: Update the flow_cell_height calculation in the editor view
to sum item heights without the 1px initial offset, then clamp the resulting
height to at least px(1.0) using the existing flow_cell_height value before
applying h().
In `@crates/gpui-flowtext/src/rich_text/editor/zoom.rs`:
- Around line 65-94: Update restore_preserved_zoom_anchor so
zoom_anchor_apply_pending is cleared after every restore_zoom_anchor attempt,
including when restoration returns false or the anchor is unavailable. Preserve
the existing successful-restoration updates to zoom_anchor and scroll_y, while
preventing later layout passes from retrying an unsuccessful restore.
In `@vendor/gpui-component/src/menu/popup_menu.rs`:
- Around line 1080-1083: Update Menu's keep_open_confirmed method to emit
MenuChangedEvent after changing the keep-open state, while preserving the
existing cx.notify() behavior. Use the same event-dispatch mechanism and event
construction already used by nearby menu state-change methods so subscribers
such as dropdown_menu.rs receive the update.
---
Outside diff comments:
In `@crates/gpui-flowtext/src/rich_text/editor/commands.rs`:
- Around line 457-473: The paste method’s allow_paragraph_breaks guard returns
before equation and table-cell selection handling, bypassing their specialized
insertion paths. Move or invoke the selection-specific routing, including
insert_text_into_selected_equation and table-cell paste logic, before the
paragraph-restriction branch while preserving the existing restricted-paragraph
flattening behavior for ordinary document selections.
---
Nitpick comments:
In `@crates/flowstate-collab/src/presence.rs`:
- Around line 326-350: Add a targeted test in the presence module for
sanitize_presence_state where a Flow focus has an oversized caret encoding.
Assert that the sanitized result preserves the Flow focus’s sheet, cell, and
editing fields while setting caret to None, covering the distinct caret-drop
branch and Part B cap behavior.
In `@crates/flowstate-flow/Cargo.toml`:
- Around line 12-21: Update the loro and postcard dependency declarations in the
flowstate-flow manifest to use the shared workspace entries instead of local
version pins. Preserve their existing dependency names and rely on the
workspace-managed versions, matching the surrounding workspace dependency
declarations.
In `@crates/flowstate-soak/src/bin/flow_soak.rs`:
- Around line 187-195: Update the final character-count assertion in the soak
convergence check around cell_text and the “typed characters were lost” ensure
to require exact equality with rounds * keystrokes_per_round * 2 instead of a
lower bound, so both lost and duplicated keystrokes are detected while
preserving the existing shared-text convergence check.
In `@crates/flowstate/src/flow/cell_theme.rs`:
- Around line 22-32: Replace the hardcoded style-slot values in the flow cell
theme setup with named constants or shared definitions that identify the
corresponding paragraph and semantic styles. Update the insertions in the theme
initialization block to reuse those definitions, keeping the current slot
assignments while ensuring future custom style changes cannot silently diverge.
In `@crates/flowstate/src/flow/editor.rs`:
- Around line 1061-1068: Replace the no-op on_click handler on the “Send to
document” Button in the flow editor with the intended send-to-document action;
if the feature is intentionally deferred, add an explicit tracking marker
instead of leaving an empty callback.
- Around line 1119-1139: Cache the post-restyle, themed document instead of
cloning and recomputing it on every render in the cell rendering flow around
rendered_document and apply_flow_cell_theme. Store the transformed result back
in the appropriate cache, and invalidate or key it by the active client theme,
side color, background, and board_zoom so live zoom scaling remains correct.
Preserve uses_summary_projection and the existing first-paragraph restyling
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f18bff8e-9a3d-4284-b13f-9535a0e9e6a7
⛔ Files ignored due to path filters (8)
Cargo.lockis excluded by!**/*.lockcrates/flowstate/assets/icons/file-input.svgis excluded by!**/*.svgcrates/flowstate/assets/icons/message-square-reply.svgis excluded by!**/*.svgcrates/flowstate/assets/icons/remove-formatting.svgis excluded by!**/*.svgcrates/flowstate/assets/icons/replace-all.svgis excluded by!**/*.svgcrates/flowstate/assets/icons/replace.svgis excluded by!**/*.svgcrates/flowstate/assets/icons/text-search.svgis excluded by!**/*.svgmessage-square-reply.svgis excluded by!**/*.svg
📒 Files selected for processing (117)
crates/flowstate-collab/Cargo.tomlcrates/flowstate-collab/src/flow/cell_authority.rscrates/flowstate-collab/src/flow/cell_text.rscrates/flowstate-collab/src/flow/commit.rscrates/flowstate-collab/src/flow/flow_io.rscrates/flowstate-collab/src/flow/handle.rscrates/flowstate-collab/src/flow/mod.rscrates/flowstate-collab/src/flow/runtime.rscrates/flowstate-collab/src/io_util.rscrates/flowstate-collab/src/lib.rscrates/flowstate-collab/src/net/direct.rscrates/flowstate-collab/src/net/mod.rscrates/flowstate-collab/src/net/runtime.rscrates/flowstate-collab/src/presence.rscrates/flowstate-collab/src/proto_direct.rscrates/flowstate-collab/src/proto_gossip.rscrates/flowstate-collab/src/sync_io.rscrates/flowstate-collab/src/ticket.rscrates/flowstate-collab/tests/flow_cell_text_fuzz.rscrates/flowstate-collab/tests/flow_convergence.rscrates/flowstate-collab/tests/flow_io_pump.rscrates/flowstate-collab/tests/flow_runtime.rscrates/flowstate-collab/tests/swarm_loopback.rscrates/flowstate-document/src/lib.rscrates/flowstate-document/src/loro_projection.rscrates/flowstate-document/src/loro_schema.rscrates/flowstate-flow/Cargo.tomlcrates/flowstate-flow/examples/dump_flow.rscrates/flowstate-flow/examples/ergonomics_fixture.rscrates/flowstate-flow/src/actions.rscrates/flowstate-flow/src/board_ops.rscrates/flowstate-flow/src/document.rscrates/flowstate-flow/src/format.rscrates/flowstate-flow/src/history.rscrates/flowstate-flow/src/intents.rscrates/flowstate-flow/src/lib.rscrates/flowstate-flow/src/loro_projection.rscrates/flowstate-flow/src/loro_schema.rscrates/flowstate-flow/src/persistence.rscrates/flowstate-flow/src/projection.rscrates/flowstate-flow/src/styles.rscrates/flowstate-flow/tests/v2_board.rscrates/flowstate-soak/Cargo.tomlcrates/flowstate-soak/src/bin/flow_soak.rscrates/flowstate/Cargo.tomlcrates/flowstate/src/app.rscrates/flowstate/src/collab/asset_transfer.rscrates/flowstate/src/collab/dropbox_checkpoint.rscrates/flowstate/src/collab/manager.rscrates/flowstate/src/collab/mod.rscrates/flowstate/src/collab/presence_view.rscrates/flowstate/src/collab/session.rscrates/flowstate/src/collab/session_io.rscrates/flowstate/src/collab/session_presence.rscrates/flowstate/src/commands.rscrates/flowstate/src/commands/keymap.rscrates/flowstate/src/flow/cell_theme.rscrates/flowstate/src/flow/editor.rscrates/flowstate/src/flow/editor/annotation.rscrates/flowstate/src/flow/editor/cell_editing.rscrates/flowstate/src/flow/editor/connector.rscrates/flowstate/src/flow/editor/drag_drop.rscrates/flowstate/src/flow/editor/layout.rscrates/flowstate/src/flow/editor/telemetry.rscrates/flowstate/src/flow/editor/zoom.rscrates/flowstate/src/flow/mod.rscrates/flowstate/src/flow/panel.rscrates/flowstate/src/flow/ribbon.rscrates/flowstate/src/main.rscrates/flowstate/src/ribbon/editor_ribbon/core.rscrates/flowstate/src/ribbon/editor_ribbon/helpers.rscrates/flowstate/src/ribbon/editor_ribbon/menus.rscrates/flowstate/src/ribbon/editor_ribbon/modern_layout.rscrates/flowstate/src/ribbon/editor_ribbon/modern_types.rscrates/flowstate/src/rich_text_element/mod.rscrates/flowstate/src/workspace/document_panel.rscrates/flowstate/src/workspace/document_search_overlay.rscrates/flowstate/src/workspace/toolkit_panel.rscrates/flowstate/src/workspace/workspace/collab.rscrates/flowstate/src/workspace/workspace/collab_prompts.rscrates/flowstate/src/workspace/workspace/documents.rscrates/flowstate/src/workspace/workspace/headless_tests/collab_glue.rscrates/flowstate/src/workspace/workspace/headless_tests/documents.rscrates/flowstate/src/workspace/workspace/headless_tests/flows_and_keystrokes.rscrates/flowstate/src/workspace/workspace/keybindings.rscrates/flowstate/src/workspace/workspace/mod.rscrates/flowstate/src/workspace/workspace/render_body.rscrates/flowstate/src/workspace/workspace/render_outline.rscrates/flowstate/src/workspace/workspace/render_status.rscrates/flowstate/src/workspace/workspace/render_top_bar.rscrates/flowstate/src/workspace/workspace/theme.rscrates/flowstate/src/workspace/workspace/top_bar.rscrates/flowstate/src/workspace/workspace/traits.rscrates/flowstate/src/workspace/workspace/workspace_state.rscrates/flowstate/src/workspace/workspace/zoom_status.rscrates/gpui-flowtext/src/rich_text/editor/action_handlers.rscrates/gpui-flowtext/src/rich_text/editor/commands.rscrates/gpui-flowtext/src/rich_text/editor/item_sizes.rscrates/gpui-flowtext/src/rich_text/editor/layout_access.rscrates/gpui-flowtext/src/rich_text/editor/lifecycle.rscrates/gpui-flowtext/src/rich_text/editor/mod.rscrates/gpui-flowtext/src/rich_text/editor/mouse.rscrates/gpui-flowtext/src/rich_text/editor/scroll_anchor.rscrates/gpui-flowtext/src/rich_text/editor/traits.rscrates/gpui-flowtext/src/rich_text/editor/zoom.rscrates/gpui-flowtext/src/rich_text/element.rscrates/gpui-flowtext/src/rich_text/invisibility.rscrates/gpui-flowtext/src/rich_text/layout/prep.rsflow-ergonomics-fixture.annotated.fl0.corrupt.c6681a35-97cc-4d2a-9417-5ea172057cc1flow-ergonomics-fixture.fl0.corrupt.5e53ec12-30c5-4aff-a2d6-e51ceb4ca4e3flow-ergonomics-guide.mdflowplan.mdpaniconwritetopdf.mdvendor/gpui-component/src/icon.rsvendor/gpui-component/src/menu/dropdown_menu.rsvendor/gpui-component/src/menu/mod.rsvendor/gpui-component/src/menu/popup_menu.rs
💤 Files with no reviewable changes (8)
- crates/flowstate/src/ribbon/editor_ribbon/helpers.rs
- crates/flowstate-flow/src/actions.rs
- crates/flowstate-flow/src/document.rs
- crates/flowstate/src/workspace/workspace/headless_tests/documents.rs
- crates/flowstate-flow/src/history.rs
- crates/flowstate/src/workspace/workspace/theme.rs
- crates/flowstate-flow/src/styles.rs
- crates/flowstate/src/workspace/workspace/workspace_state.rs
Comment on lines
+157
to
+231
| fn io_loop(core: &Arc<WriteGate<FlowRuntime>>, receiver: &Receiver<FlowIoRequest>) { | ||
| let mut deferred: Vec<FlowIoRequest> = Vec::new(); | ||
| loop { | ||
| let request = if let Some(request) = deferred.pop() { | ||
| request | ||
| } else { | ||
| match receiver.recv_blocking() { | ||
| Ok(request) => request, | ||
| Err(_) => break, | ||
| } | ||
| }; | ||
| let kind = request_kind(&request); | ||
| let started = std::time::Instant::now(); | ||
| match request { | ||
| FlowIoRequest::ImportRemoteUpdate { bytes, reply } => { | ||
| // Spec §6.4: coalesce immediately-available import blobs into ONE gate | ||
| // acquisition, respecting the byte budget; non-import requests popped | ||
| // while draining are deferred, order preserved among themselves. | ||
| let mut coalesced_bytes = bytes.len(); | ||
| let mut chunk: Vec<(Vec<u8>, ReplySender<()>)> = vec![(bytes, reply)]; | ||
| while chunk.len() < IMPORT_COALESCE_MAX && coalesced_bytes < IMPORT_COALESCE_BYTE_BUDGET { | ||
| match receiver.try_recv() { | ||
| Ok(FlowIoRequest::ImportRemoteUpdate { bytes, reply }) => { | ||
| coalesced_bytes += bytes.len(); | ||
| chunk.push((bytes, reply)); | ||
| }, | ||
| Ok(other) => { | ||
| deferred.push(other); | ||
| break; | ||
| }, | ||
| Err(_) => break, | ||
| } | ||
| } | ||
| match core.lock(GateHolder::ImportChunk) { | ||
| Ok(mut guard) => { | ||
| // §act-five P1.A second drain: fold in blobs that arrived while | ||
| // this thread was blocked on the gate — no added latency. | ||
| while chunk.len() < IMPORT_COALESCE_MAX && coalesced_bytes < IMPORT_COALESCE_BYTE_BUDGET { | ||
| match receiver.try_recv() { | ||
| Ok(FlowIoRequest::ImportRemoteUpdate { bytes, reply }) => { | ||
| coalesced_bytes += bytes.len(); | ||
| chunk.push((bytes, reply)); | ||
| }, | ||
| Ok(other) => { | ||
| deferred.push(other); | ||
| break; | ||
| }, | ||
| Err(_) => break, | ||
| } | ||
| } | ||
| let blobs: Vec<&[u8]> = chunk.iter().map(|(bytes, _)| bytes.as_slice()).collect(); | ||
| match guard.import_remote_updates(&blobs) { | ||
| Ok(()) => { | ||
| for (_, reply) in &chunk { | ||
| send_reply(reply, Ok(())); | ||
| } | ||
| }, | ||
| Err(error) => { | ||
| let error = format!("{error:#}"); | ||
| for (_, reply) in &chunk { | ||
| send_reply(reply, Err(anyhow::anyhow!("batched flow import failed: {error}"))); | ||
| } | ||
| }, | ||
| } | ||
| if chunk.len() > 1 { | ||
| tracing::debug!(coalesced = chunk.len(), "coalesced flow import chunk under one gate hold"); | ||
| } | ||
| }, | ||
| Err(poisoned) => { | ||
| for (_, reply) in chunk { | ||
| send_reply(&reply, Err(anyhow::anyhow!(poisoned))); | ||
| } | ||
| }, | ||
| } | ||
| }, |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
deferred is a stack (LIFO), but the comment promises FIFO ordering.
deferred.push(other) appends and deferred.pop() (Line 160) removes from the same end, so if two different non-import requests get deferred across separate coalescing attempts (one from the pre-lock drain, one from the post-lock drain, or across two different import batches before deferred empties), the most-recently-deferred one is served first — reversing arrival order. That contradicts the stated invariant: "non-import requests popped while draining are deferred, order preserved among themselves." Under load (mixed BoardSnapshot/PumpPublish/ExportUpdatesFor traffic interleaved with import bursts), this can let a later request "jump the queue" ahead of an earlier one, so callers can observe out-of-order results.
🐛 Proposed fix — use a FIFO queue
+use std::collections::VecDeque;
+
fn io_loop(core: &Arc<WriteGate<FlowRuntime>>, receiver: &Receiver<FlowIoRequest>) {
- let mut deferred: Vec<FlowIoRequest> = Vec::new();
+ let mut deferred: VecDeque<FlowIoRequest> = VecDeque::new();
loop {
- let request = if let Some(request) = deferred.pop() {
+ let request = if let Some(request) = deferred.pop_front() {
request
} else {
match receiver.recv_blocking() {
Ok(request) => request,
Err(_) => break,
}
};And update both deferred.push(other) call sites (Lines 184, 201) to deferred.push_back(other).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/flowstate-collab/src/flow/flow_io.rs` around lines 157 - 231, Update
io_loop’s deferred request storage from a Vec to a FIFO queue such as VecDeque,
retrieve pending requests from the front with pop_front, and replace both
deferred.push calls in the pre-lock and post-lock import drains with push_back.
Preserve arrival order for all non-import requests deferred during coalescing.
Comment on lines
+386
to
+407
| /// Rebuild the board from canonical state, refresh every open cell, and push | ||
| /// Replace items on every affected stream. The import/undo derivation path. | ||
| pub(crate) fn refresh_all(&mut self) -> Result<()> { | ||
| let (board, defects) = materialize_board(&self.doc)?; | ||
| self.defects = defects; | ||
| self.board = board; | ||
| self.push_board_stream(); | ||
| let open: Vec<CellId> = self.cells.keys().copied().collect(); | ||
| for cell in open { | ||
| let live = self.board.cell(cell).is_some(); | ||
| if live { | ||
| // `refresh_cell` re-derives the summary; the fresh board already has | ||
| // it, so only the cell stream side effect matters here. | ||
| let _ = self.refresh_cell(cell)?; | ||
| } else { | ||
| // Cell deleted remotely: the board Replace already tells the editor; | ||
| // drop the dead stream + cache. | ||
| self.close_cell(cell); | ||
| } | ||
| } | ||
| Ok(()) | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether materialize_board failures are otherwise mitigated/tested,
# and whether callers of import_remote_updates/undo/redo handle this error path.
rg -nP -C4 '\bmaterialize_board\s*\(' crates/flowstate-flow/src crates/flowstate-collab/src
rg -nP -C4 'GatePoisoned' crates/flowstate-collab/src
rg -nP -C4 'refresh_all\s*\(\s*\)' crates/flowstate-collab/srcRepository: realartists-gitmo/flowstate
Length of output: 19733
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant sections around the affected functions.
sed -n '420,520p' crates/flowstate-collab/src/flow/runtime.rs
printf '\n--- commit.rs ---\n'
sed -n '680,730p' crates/flowstate-collab/src/flow/commit.rs
printf '\n--- handle.rs ---\n'
sed -n '1,140p' crates/flowstate-collab/src/flow/handle.rs
printf '\n--- local_write/handle.rs ---\n'
sed -n '200,260p' crates/flowstate-collab/src/local_write/handle.rsRepository: realartists-gitmo/flowstate
Length of output: 12463
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '420,520p' crates/flowstate-collab/src/flow/runtime.rs
printf '\n--- commit.rs ---\n'
sed -n '680,730p' crates/flowstate-collab/src/flow/commit.rs
printf '\n--- handle.rs ---\n'
sed -n '1,140p' crates/flowstate-collab/src/flow/handle.rs
printf '\n--- local_write/handle.rs ---\n'
sed -n '200,260p' crates/flowstate-collab/src/local_write/handle.rsRepository: realartists-gitmo/flowstate
Length of output: 12463
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where refresh_all failures are handled and where board audits run.
rg -n -C3 'refresh_all\(\)\?|audit_board_against_rebuild\(|CompensationFailed|GatePoisoned|import_remote_updates\(' crates/flowstate-collab/src
printf '\n--- flow runtime audit/invocation context ---\n'
sed -n '1,220p' crates/flowstate-collab/src/flow/runtime.rs
printf '\n--- flow commit call sites around apply/undo/remote import ---\n'
sed -n '620,760p' crates/flowstate-collab/src/flow/commit.rsRepository: realartists-gitmo/flowstate
Length of output: 33620
Handle refresh_all() failures as unrecoverable in import_remote_updates() and undo_redo_step(). Both paths mutate self.doc first and rebuild self.board afterward; if materialize_board() fails, the mutation has already landed, the derived board/streams stay stale, and the runtime can remain inconsistent until a later refresh happens. Route this through the same reload/poisoned-state path used for other unrecoverable write failures instead of returning a plain error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/flowstate-collab/src/flow/runtime.rs` around lines 386 - 407, Treat
refresh_all failures in import_remote_updates and undo_redo_step as
unrecoverable after self.doc mutation: invoke the existing reload/poisoned-state
handling path rather than returning a plain error. Preserve refresh_all’s
successful board and stream refresh behavior, and use the same failure mechanism
already applied to other unrecoverable write failures.
Comment on lines
+146
to
+178
| for key in cell_keys { | ||
| let Some(map) = child_map(&cell_registry, &key) else { | ||
| continue; | ||
| }; | ||
| let Some(id) = map_uuid(&map, "id").filter(|id| id.to_string() == key) else { | ||
| continue; | ||
| }; | ||
| let Some(sheet_id) = map_uuid(&map, "sheet_id") else { | ||
| defects.push(FlowDefect::CellRecordUnknownSheet { cell: id }); | ||
| continue; | ||
| }; | ||
| if !live_sheet_ids.contains(&sheet_id) { | ||
| defects.push(FlowDefect::CellRecordUnknownSheet { cell: id }); | ||
| continue; | ||
| } | ||
| cells_by_sheet.entry(sheet_id).or_default().push(RawCell { | ||
| id, | ||
| sheet_id, | ||
| column_id: map_uuid(&map, "column_id"), | ||
| parent_id: map_uuid(&map, "parent_id"), | ||
| map, | ||
| }); | ||
| } | ||
|
|
||
| // ---- Per-sheet normalization ---------------------------------------------- | ||
| let mut sheets = Vec::with_capacity(sheet_keys.len()); | ||
| for key in &sheet_keys { | ||
| let Some(sheet_map) = child_map(&sheet_registry, key) else { | ||
| continue; | ||
| }; | ||
| let Ok(sheet_id) = Uuid::parse_str(key) else { | ||
| continue; | ||
| }; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Malformed cell and sheet records should surface a defect.
The continue paths for a mismatched or missing cell id and a non-UUID sheet_keys entry silently drop corrupted input, while the rest of this materializer reports FlowDefects for similar normalization failures. These cases are likely to come from external .fl0 corruption, so emitting a defect here would preserve the quarantine/debuggability model.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/flowstate-flow/src/loro_projection.rs` around lines 146 - 178, Update
the cell normalization loop to emit the appropriate FlowDefect when a cell
record has a missing or mismatched “id” instead of silently continuing, and
update the per-sheet normalization loop around sheet_keys to emit a defect when
a key is not a valid UUID. Reuse the existing FlowDefect variants and contextual
identifiers used for similar normalization failures, while preserving the
current skip behavior after recording each defect.
Comment on lines
+386
to
+394
| let mut full = String::new(); | ||
| let mut boundaries = Vec::with_capacity(paragraphs.len()); | ||
| for paragraph in paragraphs { | ||
| boundaries.push(full.chars().count()); | ||
| full.push('\n'); | ||
| for run in ¶graph.runs { | ||
| full.push_str(&run.text); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
O(n²) boundary computation in cell_flow_from_paragraphs.
boundaries.push(full.chars().count()) re-scans the entire accumulated full string on every paragraph, making this loop quadratic in the number of paragraphs/total text length — exactly the case (paste/import of a whole cell) this function is designed to make efficient per its own docstring.
⚡ Track a running unicode length instead of rescanning
let mut full = String::new();
let mut boundaries = Vec::with_capacity(paragraphs.len());
+ let mut running_len = 0usize;
for paragraph in paragraphs {
- boundaries.push(full.chars().count());
+ boundaries.push(running_len);
full.push('\n');
+ running_len += 1;
for run in ¶graph.runs {
full.push_str(&run.text);
+ running_len += run.text.chars().count();
}
}
text.insert(0, &full)?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let mut full = String::new(); | |
| let mut boundaries = Vec::with_capacity(paragraphs.len()); | |
| for paragraph in paragraphs { | |
| boundaries.push(full.chars().count()); | |
| full.push('\n'); | |
| for run in ¶graph.runs { | |
| full.push_str(&run.text); | |
| } | |
| } | |
| let mut full = String::new(); | |
| let mut boundaries = Vec::with_capacity(paragraphs.len()); | |
| let mut running_len = 0usize; | |
| for paragraph in paragraphs { | |
| boundaries.push(running_len); | |
| full.push('\n'); | |
| running_len += 1; | |
| for run in ¶graph.runs { | |
| full.push_str(&run.text); | |
| running_len += run.text.chars().count(); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/flowstate-flow/src/loro_schema.rs` around lines 386 - 394, Update
cell_flow_from_paragraphs to maintain a running Unicode character count while
building full, and push that counter for each paragraph instead of calling
full.chars().count() repeatedly. Increment the counter as newline and run text
are appended, preserving the existing boundary positions and output behavior.
Comment on lines
+31
to
63
| pub fn decode_fl0_snapshot(bytes: &[u8]) -> anyhow::Result<Vec<u8>> { | ||
| if bytes.len() as u64 > MAX_FL0_BYTES { | ||
| bail!("refusing to decode oversized .fl0 payload ({} bytes)", bytes.len()); | ||
| } | ||
| let mut cursor = Cursor::new(bytes); | ||
| let mut magic = [0; 8]; | ||
| cursor.read_exact(&mut magic)?; | ||
| if &magic != MAGIC { | ||
| return Err(io::Error::new(io::ErrorKind::InvalidData, "invalid .fl0 signature").into()); | ||
| } | ||
| let version = read_u32(&mut cursor)?; | ||
| if version == 1 { | ||
| // v1 was a pre-release format that never shipped; there is nothing to | ||
| // migrate. Loud, specific message instead of a generic version error. | ||
| return Err( | ||
| io::Error::new( | ||
| io::ErrorKind::InvalidData, | ||
| "this .fl0 uses the pre-release v1 format, which never shipped and cannot be opened", | ||
| ) | ||
| .into(), | ||
| ); | ||
| } | ||
| if version == 0 { | ||
| return upgrade_0_1(data).map(|saveable| saveable.nodes); | ||
| if version != FL0_VERSION { | ||
| return Err(io::Error::new(io::ErrorKind::InvalidData, format!("unsupported .fl0 version {version}")).into()); | ||
| } | ||
| bail!("unsupported .fl0 save version {version}"); | ||
| let payload_len = read_u64(&mut cursor)? as usize; | ||
| if payload_len != bytes.len().saturating_sub(cursor.position() as usize) { | ||
| return Err(io::Error::new(io::ErrorKind::InvalidData, "invalid .fl0 payload length").into()); | ||
| } | ||
| let mut compressed = Vec::with_capacity(payload_len); | ||
| cursor.read_to_end(&mut compressed)?; | ||
| Ok(zstd::stream::decode_all(compressed.as_slice())?) | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does the Rust zstd crate's Decoder support limiting maximum decompressed output size to protect against decompression bombs?
💡 Result:
The popular Rust zstd crate (a wrapper around the C libzstd) does not provide a single, built-in parameter on its Decoder to automatically enforce a maximum decompressed output size for decompression bomb protection [1][2]. Because it is a streaming decoder, handling decompression bombs requires application-level implementation to monitor and limit the total bytes produced [3]. To protect against decompression bombs when using the zstd crate, you should implement one of the following patterns: 1. Use Read::take: You can wrap the Decoder in an std::io::Read::take(limit) adapter [1][3]. This ensures that once the specified limit of bytes has been read from the stream, no further data will be produced, effectively halting decompression [3]. 2. Monitor Output in a Loop: When using streaming decompression, read data into a buffer in fixed-size chunks and maintain a running counter of total bytes processed. If the counter exceeds your defined threshold, stop reading and return an error [3][4]. If you are performing bulk (single-shot) decompression, the zstd crate provides a decompress method that accepts a capacity argument; however, depending on the implementation version, this might only control the initial allocation rather than strictly enforcing a cap on the source data's potential output [5]. For scenarios requiring strict byte-limit guarantees, using streaming with Read::take is the standard and recommended approach [3]. Other crates in the ecosystem, such as ruzstd, also require similar manual management of output size to prevent memory exhaustion [6][4]. Users requiring bit-exact parity with C libzstd or explicit limit support often implement custom wrappers around the underlying C bindings or utilize specialized crates designed for hardened decompression [7].
Citations:
- 1: https://docs.rs/zstd/latest/zstd/stream/read/struct.Decoder.html
- 2: https://docs.rs/zstd/latest/zstd/?search=encoder
- 3: https://docs.rs/s4-codec/latest/src/s4_codec/cpu_zstd.rs.html
- 4: https://github.com/KillingSpark/zstd-rs
- 5: https://docs.rs/zstd/latest/src/zstd/bulk/decompressor.rs.html
- 6: https://docs.rs/ruzstd/latest/ruzstd/decoding/struct.FrameDecoder.html
- 7: https://github.com/danifunker/libzstd-bitexact-rs
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- persistence.rs ---\n'
ast-grep outline crates/flowstate-flow/src/persistence.rs --view expanded || true
printf '\n--- relevant lines ---\n'
sed -n '1,220p' crates/flowstate-flow/src/persistence.rs | cat -n
printf '\n--- dropbox checkpoint call site ---\n'
ast-grep outline crates/flowstate/src/collab/dropbox_checkpoint.rs --view expanded || true
printf '\n--- search decode/read helpers ---\n'
rg -n "decode_fl0_snapshot|read_fl0|load_flow_runtime_source|MAX_FL0_BYTES|decode_all|zstd::stream::Decoder|take\(" crates/flowstate crates/flowstate-flow -SRepository: realartists-gitmo/flowstate
Length of output: 256
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the exact decoder implementation and the Dropbox/local callers.
sed -n '1,140p' crates/flowstate-flow/src/persistence.rs | cat -n
printf '\n---\n'
sed -n '1,220p' crates/flowstate/src/collab/dropbox_checkpoint.rs | cat -n
printf '\n---\n'
rg -n "read_fl0|load_flow_runtime_source|decode_fl0_snapshot|sync_bound_flow_file" crates/flowstate crates/flowstate-flow -SRepository: realartists-gitmo/flowstate
Length of output: 17037
Bound the decoded output size. MAX_FL0_BYTES only caps the compressed input; zstd::stream::decode_all can still expand a small .fl0 into an arbitrarily large buffer and OOM the process. Stream the decoder and enforce a hard decoded-byte limit before accepting the snapshot.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/flowstate-flow/src/persistence.rs` around lines 31 - 63, Update
decode_fl0_snapshot to bound decompressed output, not just the input checked
against MAX_FL0_BYTES. Replace zstd::stream::decode_all with a streaming decoder
and read through a limited buffer, rejecting the payload once the decoded byte
count exceeds the hard snapshot limit before returning the collected bytes.
Comment on lines
589
to
594
| pub fn new_flow(&mut self, window: &mut Window, cx: &mut Context<Self>) { | ||
| self.add_flow_panel(flowstate_flow::FlowDocument::new(), None, window, cx); | ||
| let Ok((handle, gate)) = flowstate_collab::flow::FlowDocHandle::new_document(&flowstate_flow::FlowFormat::policy_debate()) else { | ||
| return; | ||
| }; | ||
| self.add_flow_panel(FlowRuntimeSource { handle, gate }, None, window, cx); | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
new_flow fails silently with no user feedback or logging.
If FlowDocHandle::new_document fails, this returns without any prompt or tracing::error! call — the "New Flow" action appears to do nothing, unlike add_document_panel/add_document_panel_with_title, which surface a critical prompt on runtime-start failure.
🛡️ Proposed fix: surface the failure like the sibling document-creation paths
pub fn new_flow(&mut self, window: &mut Window, cx: &mut Context<Self>) {
- let Ok((handle, gate)) = flowstate_collab::flow::FlowDocHandle::new_document(&flowstate_flow::FlowFormat::policy_debate()) else {
- return;
- };
+ let (handle, gate) = match flowstate_collab::flow::FlowDocHandle::new_document(&flowstate_flow::FlowFormat::policy_debate()) {
+ Ok(pair) => pair,
+ Err(error) => {
+ let detail = format!("The flow runtime could not be started: {error:#}");
+ tracing::error!(error = %format_args!("{error:#}"), "creating flow panel failed");
+ std::mem::drop(window.prompt(
+ PromptLevel::Critical,
+ "Flow could not be created",
+ Some(&detail),
+ &[PromptButton::ok("Ok")],
+ cx,
+ ));
+ return;
+ },
+ };
self.add_flow_panel(FlowRuntimeSource { handle, gate }, None, window, cx);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub fn new_flow(&mut self, window: &mut Window, cx: &mut Context<Self>) { | |
| self.add_flow_panel(flowstate_flow::FlowDocument::new(), None, window, cx); | |
| let Ok((handle, gate)) = flowstate_collab::flow::FlowDocHandle::new_document(&flowstate_flow::FlowFormat::policy_debate()) else { | |
| return; | |
| }; | |
| self.add_flow_panel(FlowRuntimeSource { handle, gate }, None, window, cx); | |
| } | |
| pub fn new_flow(&mut self, window: &mut Window, cx: &mut Context<Self>) { | |
| let (handle, gate) = match flowstate_collab::flow::FlowDocHandle::new_document(&flowstate_flow::FlowFormat::policy_debate()) { | |
| Ok(pair) => pair, | |
| Err(error) => { | |
| let detail = format!("The flow runtime could not be started: {error:#}"); | |
| tracing::error!(error = %format_args!("{error:#}"), "creating flow panel failed"); | |
| std::mem::drop(window.prompt( | |
| PromptLevel::Critical, | |
| "Flow could not be created", | |
| Some(&detail), | |
| &[PromptButton::ok("Ok")], | |
| cx, | |
| )); | |
| return; | |
| }, | |
| }; | |
| self.add_flow_panel(FlowRuntimeSource { handle, gate }, None, window, cx); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/flowstate/src/workspace/workspace/documents.rs` around lines 589 -
594, Update new_flow to handle FlowDocHandle::new_document failure like
add_document_panel and add_document_panel_with_title: log the runtime-start
error with tracing::error! and surface a critical user prompt before returning.
Preserve the existing add_flow_panel path for successful document creation.
Comment on lines
48
to
58
| match item { | ||
| VirtualItem::ParagraphRemainder { paragraph_ix, .. } => { | ||
| let width = self.current_layout_width(); | ||
| let start_byte = self | ||
| .valid_chunk_cache_entry(paragraph_ix, width) | ||
| .and_then(|entry| entry.chunks.last()) | ||
| .map(|chunk| chunk.end_byte) | ||
| .unwrap_or(0); | ||
| let block_ix = self.block_ix_for_paragraph(paragraph_ix)?; | ||
| let paragraph_start_item = self.item_sizes_cache.as_ref()?.block_item_ranges.get(block_ix)?.start; | ||
| let paragraph_top = self.height_prefix_index.item_top(paragraph_start_item); | ||
| Some(ScrollAnchorSnapshot::ParagraphRemainder { | ||
| paragraph_ix, | ||
| start_byte, | ||
| delta, | ||
| start_byte: 0, | ||
| delta: (content_y - paragraph_top).max(px(0.0)), | ||
| }) | ||
| }, |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the chunk-cache/item-sizes model to confirm entry.chunks spans the whole paragraph,
# and check other callers of paragraph_remainder_start_byte / ScrollAnchorSnapshot::ParagraphRemainder.
rg -n 'paragraph_remainder_start_byte|ScrollAnchorSnapshot::ParagraphRemainder|valid_chunk_cache_entry' crates/gpui-flowtext/src/rich_textRepository: realartists-gitmo/flowstate
Length of output: 5130
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== scroll_anchor outline ==\n'
ast-grep outline crates/gpui-flowtext/src/rich_text/editor/scroll_anchor.rs --view expanded
printf '\n== scroll_anchor relevant slice ==\n'
sed -n '1,280p' crates/gpui-flowtext/src/rich_text/editor/scroll_anchor.rs | nl -ba | sed -n '1,280p'
printf '\n== item_sizes relevant slice ==\n'
sed -n '1,260p' crates/gpui-flowtext/src/rich_text/editor/item_sizes.rs | nl -ba | sed -n '1,260p'
printf '\n== chunk_layout relevant slice ==\n'
sed -n '1,220p' crates/gpui-flowtext/src/rich_text/editor/chunk_layout.rs | nl -ba | sed -n '1,220p'Repository: realartists-gitmo/flowstate
Length of output: 1255
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== paragraph_remainder_start_byte callers ==\n'
rg -n 'paragraph_remainder_start_byte\(' crates/gpui-flowtext/src/rich_text
printf '\n== ParagraphRemainder construction and restore path ==\n'
sed -n '140,250p' crates/gpui-flowtext/src/rich_text/editor/scroll_anchor.rs | nl -ba | sed -n '140,250p'Repository: realartists-gitmo/flowstate
Length of output: 619
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== scroll_anchor.rs =='
sed -n '1,260p' crates/gpui-flowtext/src/rich_text/editor/scroll_anchor.rs | awk '{printf "%4d %s\n", NR, $0}' | sed -n '1,260p'
echo
echo '== item_sizes.rs =='
sed -n '1,520p' crates/gpui-flowtext/src/rich_text/editor/item_sizes.rs | awk '{printf "%4d %s\n", NR, $0}' | sed -n '1,520p'
echo
echo '== chunk_layout.rs =='
sed -n '1,340p' crates/gpui-flowtext/src/rich_text/editor/chunk_layout.rs | awk '{printf "%4d %s\n", NR, $0}' | sed -n '1,340p'
echo
echo '== chunk_materialization.rs caller =='
sed -n '1,120p' crates/gpui-flowtext/src/rich_text/editor/chunk_materialization.rs | awk '{printf "%4d %s\n", NR, $0}' | sed -n '1,120p'Repository: realartists-gitmo/flowstate
Length of output: 50383
ParagraphRemainder fallback still uses the old local-offset contract. The cache-hit path is fine because chunk layouts are stored contiguously from byte 0, but when valid_chunk_cache_entry() is missing scroll_anchor_item_and_delta() still returns (item_ix, *delta). With the new capture logic, restore_scroll_anchor() can add the exact chunk height twice and overshoot after a resize or other cache invalidation; adjust that fallback to convert delta back to the remainder item’s local offset.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/gpui-flowtext/src/rich_text/editor/scroll_anchor.rs` around lines 48 -
58, The ParagraphRemainder fallback in scroll_anchor_item_and_delta must convert
delta back to the remainder item’s local offset before returning, rather than
returning the full delta. Update the cache-miss path associated with
valid_chunk_cache_entry and preserve the cache-hit behavior, so
restore_scroll_anchor does not add the chunk height twice after cache
invalidation.
Comment on lines
+40
to
+47
| let flow_cell_surface = self.config.flow_cell_surface; | ||
| let flow_cell_height = item_sizes.iter().fold(px(1.0), |height, item| height + item.height); | ||
| div() | ||
| .size_full() | ||
| .id("rich-text-editor") | ||
| .relative() | ||
| .bg(self.document.theme.document_background_color) | ||
| .when(flow_cell_surface, |this| this.w_full().h(flow_cell_height)) | ||
| .when(!flow_cell_surface, |this| this.size_full()) | ||
| .when(!flow_cell_surface, |this| this.bg(self.document.theme.document_background_color)) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file around the reported lines.
sed -n '1,120p' crates/gpui-flowtext/src/rich_text/editor/traits.rs
printf '\n--- zoom-related files ---\n'
fd -a 'zoom.rs' crates/gpui-flowtext/src || true
fd -a 'item_sizes.rs' crates/gpui-flowtext/src || true
printf '\n--- search for flow_cell_height / zoom anchor usage ---\n'
rg -n "flow_cell_height|reset_zoom_anchor|prepare_pending_zoom_anchor|restore_preserved_zoom_anchor|flow_cell_surface|on_scroll_wheel" crates/gpui-flowtext/srcRepository: realartists-gitmo/flowstate
Length of output: 8470
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- zoom.rs ---'
sed -n '1,180p' crates/gpui-flowtext/src/rich_text/editor/zoom.rs
printf '\n%s\n' '--- item_sizes.rs ---'
sed -n '1,240p' crates/gpui-flowtext/src/rich_text/editor/item_sizes.rs
printf '\n%s\n' '--- traits.rs around scroll wheel and flow-cell child ---'
sed -n '110,290p' crates/gpui-flowtext/src/rich_text/editor/traits.rs
printf '\n%s\n' '--- search for px(1.0) in editor code ---'
rg -n "px\(1\.0\)|max\(px\(1\.0\)\)|fold\(px\(1\.0\)" crates/gpui-flowtext/src/rich_text/editorRepository: realartists-gitmo/flowstate
Length of output: 27952
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- editor config and flow_cell_surface ---'
sed -n '380,440p' crates/gpui-flowtext/src/rich_text/editor/mod.rs
printf '\n%s\n' '--- search for flow_cell_surface references ---'
rg -n "flow_cell_surface" crates/gpui-flowtext/src
printf '\n%s\n' '--- search for height-sensitive layout helpers ---'
rg -n "content_size|size_full\(\)|h\(flow_cell_height\)|track_scroll\(|scrollbar_show" crates/gpui-flowtext/src/rich_text/editorRepository: realartists-gitmo/flowstate
Length of output: 4996
Clamp flow_cell_height after summing
fold(px(1.0), ...) adds 1px to every non-empty flow-cell surface, not just the empty case. If this is only meant to avoid a zero-height surface, sum first and then clamp with .max(px(1.0)).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/gpui-flowtext/src/rich_text/editor/traits.rs` around lines 40 - 47,
Update the flow_cell_height calculation in the editor view to sum item heights
without the 1px initial offset, then clamp the resulting height to at least
px(1.0) using the existing flow_cell_height value before applying h().
Comment on lines
+65
to
+94
| pub(super) fn apply_pending_zoom_anchor(&mut self) { | ||
| if !self.zoom_anchor_apply_pending { | ||
| return; | ||
| } | ||
| let Some(anchor) = self.zoom_anchor else { | ||
| self.zoom_anchor_apply_pending = false; | ||
| return; | ||
| }; | ||
| self.restore_preserved_zoom_anchor(Some(anchor)); | ||
| } | ||
|
|
||
| pub(super) fn reset_zoom_anchor(&mut self) { | ||
| self.zoom_anchor = None; | ||
| self.zoom_anchor_apply_pending = false; | ||
| } | ||
|
|
||
| fn active_zoom_anchor(&self) -> Option<ZoomAnchorSnapshot> { | ||
| self.zoom_anchor.filter(|anchor| self.zoom_anchor_is_current(*anchor)) | ||
| } | ||
|
|
||
| fn restore_preserved_zoom_anchor(&mut self, anchor: Option<ZoomAnchorSnapshot>) { | ||
| let Some(mut anchor) = anchor else { | ||
| return; | ||
| }; | ||
| if self.restore_zoom_anchor(anchor) { | ||
| anchor.scroll_y = self.scroll_handle.offset().y; | ||
| self.zoom_anchor = Some(anchor); | ||
| self.zoom_anchor_apply_pending = false; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -e
printf 'Files:\n'
git ls-files crates/gpui-flowtext/src/rich_text/editor/zoom.rs crates/gpui-flowtext/src/rich_text/editor/mod.rs crates/flowstate/src/flow/editor/zoom.rs
printf '\nOutline zoom.rs:\n'
ast-grep outline crates/gpui-flowtext/src/rich_text/editor/zoom.rs --view expanded || true
printf '\nSearch usages of active_zoom_anchor and restore_preserved_zoom_anchor:\n'
rg -n "active_zoom_anchor|restore_preserved_zoom_anchor|zoom_anchor_apply_pending|restore_zoom_anchor" crates/gpui-flowtext/src -SRepository: realartists-gitmo/flowstate
Length of output: 4564
🏁 Script executed:
set -e
printf '\nrestore_zoom_anchor implementation:\n'
sed -n '180,240p' crates/gpui-flowtext/src/rich_text/editor/zoom.rs
printf '\nitem_sizes.rs relevant sections:\n'
sed -n '60,120p' crates/gpui-flowtext/src/rich_text/editor/item_sizes.rs
printf '\nMore item_sizes.rs relevant section:\n'
sed -n '180,220p' crates/gpui-flowtext/src/rich_text/editor/item_sizes.rsRepository: realartists-gitmo/flowstate
Length of output: 6268
🏁 Script executed:
set -e
sed -n '31,80p' crates/gpui-flowtext/src/rich_text/editor/zoom.rsRepository: realartists-gitmo/flowstate
Length of output: 1730
Clear zoom_anchor_apply_pending after each restore attempt.
If restore_zoom_anchor keeps returning false (for example, while the viewport is still too small or the anchor target can no longer be resolved), the pending flag stays set and every later layout pass redoes the same work with no progress.
Proposed fix
fn restore_preserved_zoom_anchor(&mut self, anchor: Option<ZoomAnchorSnapshot>) {
let Some(mut anchor) = anchor else {
return;
};
if self.restore_zoom_anchor(anchor) {
anchor.scroll_y = self.scroll_handle.offset().y;
self.zoom_anchor = Some(anchor);
- self.zoom_anchor_apply_pending = false;
}
+ self.zoom_anchor_apply_pending = false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub(super) fn apply_pending_zoom_anchor(&mut self) { | |
| if !self.zoom_anchor_apply_pending { | |
| return; | |
| } | |
| let Some(anchor) = self.zoom_anchor else { | |
| self.zoom_anchor_apply_pending = false; | |
| return; | |
| }; | |
| self.restore_preserved_zoom_anchor(Some(anchor)); | |
| } | |
| pub(super) fn reset_zoom_anchor(&mut self) { | |
| self.zoom_anchor = None; | |
| self.zoom_anchor_apply_pending = false; | |
| } | |
| fn active_zoom_anchor(&self) -> Option<ZoomAnchorSnapshot> { | |
| self.zoom_anchor.filter(|anchor| self.zoom_anchor_is_current(*anchor)) | |
| } | |
| fn restore_preserved_zoom_anchor(&mut self, anchor: Option<ZoomAnchorSnapshot>) { | |
| let Some(mut anchor) = anchor else { | |
| return; | |
| }; | |
| if self.restore_zoom_anchor(anchor) { | |
| anchor.scroll_y = self.scroll_handle.offset().y; | |
| self.zoom_anchor = Some(anchor); | |
| self.zoom_anchor_apply_pending = false; | |
| } | |
| } | |
| pub(super) fn apply_pending_zoom_anchor(&mut self) { | |
| if !self.zoom_anchor_apply_pending { | |
| return; | |
| } | |
| let Some(anchor) = self.zoom_anchor else { | |
| self.zoom_anchor_apply_pending = false; | |
| return; | |
| }; | |
| self.restore_preserved_zoom_anchor(Some(anchor)); | |
| } | |
| pub(super) fn reset_zoom_anchor(&mut self) { | |
| self.zoom_anchor = None; | |
| self.zoom_anchor_apply_pending = false; | |
| } | |
| fn active_zoom_anchor(&self) -> Option<ZoomAnchorSnapshot> { | |
| self.zoom_anchor.filter(|anchor| self.zoom_anchor_is_current(*anchor)) | |
| } | |
| fn restore_preserved_zoom_anchor(&mut self, anchor: Option<ZoomAnchorSnapshot>) { | |
| let Some(mut anchor) = anchor else { | |
| return; | |
| }; | |
| if self.restore_zoom_anchor(anchor) { | |
| anchor.scroll_y = self.scroll_handle.offset().y; | |
| self.zoom_anchor = Some(anchor); | |
| } | |
| self.zoom_anchor_apply_pending = false; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/gpui-flowtext/src/rich_text/editor/zoom.rs` around lines 65 - 94,
Update restore_preserved_zoom_anchor so zoom_anchor_apply_pending is cleared
after every restore_zoom_anchor attempt, including when restoration returns
false or the anchor is unavailable. Preserve the existing successful-restoration
updates to zoom_anchor and scroll_y, while preventing later layout passes from
retrying an unsuccessful restore.
Comment on lines
+1080
to
+1083
| pub(crate) fn keep_open_confirmed(&mut self, _: &mut Window, cx: &mut Context<Self>) { | ||
| cx.notify(); | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether MenuChangedEvent is subscribed to anywhere, and whether it is ever emitted.
rg -nP -C3 'MenuChangedEvent' --type=rustRepository: realartists-gitmo/flowstate
Length of output: 165
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== popup_menu references =="
rg -n "MenuChangedEvent|keep_open_confirmed|EventEmitter<MenuChangedEvent>|subscribe\\(" vendor/gpui-component/src/menu/popup_menu.rs vendor/gpui-component/src/menu -g '*.rs' -C 3 || true
echo
echo "== file outline =="
ast-grep outline vendor/gpui-component/src/menu/popup_menu.rs --view expanded || true
echo
echo "== targeted slices =="
sed -n '1,120p' vendor/gpui-component/src/menu/popup_menu.rs
echo
sed -n '1060,1100p' vendor/gpui-component/src/menu/popup_menu.rs
echo
sed -n '1340,1385p' vendor/gpui-component/src/menu/popup_menu.rsRepository: realartists-gitmo/flowstate
Length of output: 22123
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== dropdown_menu slice =="
sed -n '100,165p' vendor/gpui-component/src/menu/dropdown_menu.rs
echo
echo "== keep_open_confirmed call sites =="
rg -n -C 4 'keep_open_confirmed\(' vendor/gpui-component/src/menu/popup_menu.rs
echo
echo "== EventEmitter usage in popup_menu =="
rg -n -C 2 'cx\.emit\(|EventEmitter<' vendor/gpui-component/src/menu/popup_menu.rsRepository: realartists-gitmo/flowstate
Length of output: 4329
Emit MenuChangedEvent from keep_open_confirmed keep_open_confirmed only calls cx.notify(), so MenuChangedEvent subscribers such as dropdown_menu.rs never receive keep-open state changes.
Proposed fix
pub(crate) fn keep_open_confirmed(&mut self, _: &mut Window, cx: &mut Context<Self>) {
+ cx.emit(MenuChangedEvent);
cx.notify();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub(crate) fn keep_open_confirmed(&mut self, _: &mut Window, cx: &mut Context<Self>) { | |
| cx.notify(); | |
| } | |
| pub(crate) fn keep_open_confirmed(&mut self, _: &mut Window, cx: &mut Context<Self>) { | |
| cx.emit(MenuChangedEvent); | |
| cx.notify(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@vendor/gpui-component/src/menu/popup_menu.rs` around lines 1080 - 1083,
Update Menu's keep_open_confirmed method to emit MenuChangedEvent after changing
the keep-open state, while preserving the existing cx.notify() behavior. Use the
same event-dispatch mechanism and event construction already used by nearby menu
state-change methods so subscribers such as dropdown_menu.rs receive the update.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebuilds the Flow (.fl0) document as CRDT-first and brings it to full .db8 collaboration parity: live P2P sessions on flow tabs, real per-cell rich-text CRDT (char-level merge inside a cell), and undo through the same gated write path. Mirrors the .db8 architecture piece for piece —
flowstate-flowbecomes the flow's format crate (schema, materializer, intents),flowstate-collabgainssrc/flow/(runtime behind the same genericWriteGate, commit law, per-cellLocalWriteAuthority, I/O service), and the app's session/manager layer splits its document-facing half into two-arm enums keyed by the ticket's newDocumentKind.Built on top of
fixflow(ad54f15), whose Loro-backed flow rework stopped at "Loro as persistence" (entity state as postcard record blobs, dense-integer ordering inside them, whole-cell LWW rich text, no transport hookup); this PR includes those commits and replaces the record machinery with the CRDT-first schema.What changed
Part A — flowstate-flow v2 (format crate)
flow.meta(postcard format,schema_version=2,document_id),flow.sheet_order+ per-sheetcell_orderas MovableLists (a reorder writes only the order list — it can never clobber a concurrent text edit), per-cell LWWcolumn_id/parent_id, and each cell embedding the exact .db8 flow/text/mark/paragraph-record shape, so char-level merge, marks, cursors, and the materializer walk are all reused.FlowDefects; canonical order is also a write-path invariant, with a capped"repair"-origin canonicalization on import.FlowIntentvocabulary, pureboard_ops(drag preview == commit, zero gate traffic per frame), summary derivation on the projection, FLOWFL0 v2 container (v1 explicitly rejected — never shipped). The record-blob machinery (SheetRecord/CellRecord, entity deltas,db8_bytes) is deleted.Part B — flowstate-collab flow runtime
FlowRuntimebehindWriteGate<FlowRuntime>; one intent = one commit = one undo member; I-15 zero-mutation rejection andrevert_tocompensation under"repair"; ordered board stream (Replace-per-change) + per-cell streams.FlowCellAuthorityimplementsLocalWriteAuthorityper cell — each open cell'sRichTextEditoris driven through the identicalset_write_authorityinjection the .db8 body editor uses; anchors resolve inside the gate against the cell's text container, so foreign cursors fail safely.UndoManager(remote/repair/meta origins excluded) withFlowUndoMetacursor/focus restoration. This is the plan's one deliberate deviation: the recorded-inverse fast path was not ported — the UndoManager checkout path is the .db8 slow-path contract, a perf-only difference at flow-document scale, and a clean follow-up if profiles ever ask for it.FlowIoHandle(import coalescing with byte budget, publish pump, fork-off-gate snapshots, .fl0 save/encode) and aSyncIoHandle { RichText, Flow }enum sonet/*transport stays byte-opaque and untouched.Part C — protocol + app wiring
DocumentKindin theSessionTicketunder the HMAC tag (kind-flipping fails auth); leading-varint pre-decode gives direction-specific version-mismatch messages.CollabSessionstays one type; its document half becomesCollabEditor/JoinedAuthority/JoinedDocumentPayloadenums. Hosting on an open flow tab reuses the tab's existing authority + I/O service; joining builds them from the snapshot and hands off to a pathless panel (autosave skips it; a debounced.fl0recovery file covers crashes; leave freezes transport only — the tab stays editable, invariant 5).PresenceState.focus: PresenceFocus { Text, Flow }; flow focus carries sheet/cell/editing plus the caret as encoded Loro cursors resolved under the flow gate. Rendering: peer-color cell outline + name chip, sheet-switcher dots, and exact carets forwarded into open cell editors via the existingExternalCaretAPI. Caps and impersonation guards apply per-arm.flow.meta/document_id,flow_soaktwo-peer soak bin, hotpath measures onapply_flow_intent/materializers, workspace clippy clean.Verification
flow_convergence,flow_cell_text_fuzz, 10k+ seeded iterations + ignored long soaks): a reorder can never lose a concurrent keystroke; same-cell concurrent edits interleave char-level; N-peer boards converge byte-equal AND equal a fresh materialization.collab_gluesuite,swarm_loopback(plus a new Flow-variant direct snapshot/updates pull), allflowstate-collabintegration suites (116 lib + 21 suites), fullflowstate --lib(68 tests).cargo clippy --workspace --all-targetsclean; two-peerflow_soakconverges with all typed characters intact.flow_soak --features hotpath-cpu, 200 rounds): cell-keystroke gate hold p50 207µs / p99 401µs,apply_flow_intentmean 232µs, structural intents p50 399µs — comfortably inside the local-intent budget; drag preview stays a pure function (zero gate traffic per frame).🤖 Generated with Claude Code
https://claude.ai/code/session_01UFUYkPyp7R8gZEMA6EUjgW
Generated by Claude Code
Summary by CodeRabbit
.fl0snapshot saving, recovery, Dropbox synchronization, and direct collaboration sharing.