Feat/sheet live presence

[SamTV12345]

No description provided.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Add sheet live presence and live calculation overlays
✨ Enhancement 🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Description

• Relay ephemeral SHEET_PRESENCE frames for remote cursors and in-progress edits.
• Overlay remote live raws into formula evaluation to recompute dependents before commit.
• Render remote cursor borders/name tags and add unit tests plus design/plan docs.

Diagram

graph TD
  A["UI: sheetEditor"] --> B["Emit SHEET_PRESENCE"] --> C["Server: ws client routing"] --> D["HandlePresence relay"] --> E["Other room clients"] --> F["UI: SheetPresence reducer"] --> G["Overlay into FormulaEngine + render view"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Serialize presence via per-pad sheet op goroutine

• ➕ Provides a single total order with other sheet events
• ➕ Potentially simplifies race reasoning between ops and presence
• ➖ Adds load/latency to an intentionally ephemeral signal
• ➖ Blurs separation between persisted OT ops and best-effort presence

2. Add server-side presence cache + join snapshot + TTL/heartbeat

• ➕ New joiners immediately see existing cursors/live edits
• ➕ Mitigates half-open connections leaving stale presence
• ➖ Introduces new server state to maintain and test
• ➖ Requires heartbeat/expiry tuning and more protocol surface

3. Replace strings.Contains routing with explicit message decoding

• ➕ More robust dispatch (avoids accidental substring matches)
• ➕ Centralizes message parsing/validation
• ➖ Larger refactor of websocket readPump and message model
• ➖ Out of scope for a minimal v1 presence feature

Recommendation: The PR’s approach (ephemeral relay + client-side reducer/overlay, no server cache) is the right v1 tradeoff: it adds visible collaboration value without entangling OT persistence or introducing server presence state. If this ships successfully, the next incremental improvement worth considering is a small server snapshot/TTL to reduce "no cursor until first move" and mitigate half-open sockets.

Files changed (10) +1782 / -20

Enhancement (6) +332 / -16

sheetMessages.goDefine SHEET_PRESENCE websocket message types +40/-0

Define SHEET_PRESENCE websocket message types

• Adds SheetPresenceIncoming (client→server) and SheetPresence/SheetPresenceData (server→clients) structs. Establishes the JSON shape and marks the feature as ephemeral and server-stamped for identity.

lib/models/ws/sheetMessages.go

SheetHandler.goRelay presence frames with server-stamped identity and read-only stripping +54/-0

Relay presence frames with server-stamped identity and read-only stripping

• Implements PadMessageHandler.HandlePresence to broadcast SHEET_PRESENCE to all other room sockets. Stamps userId/name/color from session.Author via authorManager, strips editing/raw for read-only sessions, and logs a warning on author lookup failure while continuing best-effort relay.

lib/ws/SheetHandler.go

client.goRoute incoming SHEET_PRESENCE frames to HandlePresence +7/-0

Route incoming SHEET_PRESENCE frames to HandlePresence

• Extends the websocket readPump dispatch to detect and unmarshal SHEET_PRESENCE messages. Calls the new handler directly (bypassing the per-document sheet op queue) to keep presence ephemeral.

lib/ws/client.go

sheetPresence.tsIntroduce SheetPresence reducer and effectiveCells overlay helper +86/-0

Introduce SheetPresence reducer and effectiveCells overlay helper

• Adds a client-side presence store tracking remote cursors and live edits keyed by userId, ignoring self frames. Provides effectiveCells() to overlay remote in-progress raws on the committed grid to drive live recomputation.

ui/src/js/sheet/sheetPresence.ts

sheetView.tsAdd selection/live-input callbacks and remote cursor/live-edit decorations +79/-12

Add selection/live-input callbacks and remote cursor/live-edit decorations

• Extends DomSheetView with onSelect/onLiveEdit/onEditEnd callbacks and Escape handling. Adds rendering for remote cursor borders and name tags and shows remote live raw text in-cell while not interrupting local editing.

ui/src/js/sheet/sheetView.ts

sheetEditor.tsWire presence transport, throttle/debounce, and live overlay recompute +66/-4

Wire presence transport, throttle/debounce, and live overlay recompute

• Adds a presence client alongside the existing SheetCollabClient, emitting SHEET_PRESENCE with throttled live edits and debounced selection changes. Overlays remote live raws into the FormulaEngine grid, updates view decorations, clears overlays on NEW_SHEET_OP.author, and drops presence on USER_LEAVE.

ui/src/js/sheet/sheetEditor.ts

Tests (2) +213 / -4

sheet_handler_test.goAdd tests for presence relay, identity stamping, and read-only behavior +150/-4

Add tests for presence relay, identity stamping, and read-only behavior

• Enhances the test harness to include an authorManager and adds helpers to build/decode presence frames. Adds coverage for stamped identity, anonymous relay when author lookup fails, sender-not-echoed behavior, and read-only live edit stripping.

lib/ws/sheet_handler_test.go

sheetPresence.test.tsUnit test presence reducer and live overlay recomputation +63/-0

Unit test presence reducer and live overlay recomputation

• Adds vitest coverage for cursor/live-edit state transitions (applyPresence/drop/clearLiveEdit) and sheet filtering. Verifies effectiveCells drives HyperFormula recompute of dependent cells from a remote live formula.

ui/src/js/sheet/sheetPresence.test.ts

Documentation (2) +1237 / -0

2026-06-25-sheet-live-presence.mdAdd step-by-step implementation plan for sheet live presence +1059/-0

Add step-by-step implementation plan for sheet live presence

• Introduces a detailed task plan covering server relay, frontend reducer/overlay, view/editor wiring, and test strategy. Documents constraints (ephemeral frames, server-stamped identity, read-only stripping) and upgrade paths.

docs/superpowers/plans/2026-06-25-sheet-live-presence.md

2026-06-25-sheet-live-presence-design.mdAdd design spec for live presence and live calculation +178/-0

Add design spec for live presence and live calculation

• Defines the wire protocol, server responsibilities, client reducer and rendering behavior, and lifecycle edge-cases. Explicitly scopes out server caching/heartbeats in v1 and documents future upgrades.

docs/superpowers/specs/2026-06-25-sheet-live-presence-design.md

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (5) 📘 Rule violations (0) 📜 Skill insights (0)

1. Presence rate-limit drops 🐞 Bug ☼ Reliability

Description

SHEET_PRESENCE messages are sent on a ~60ms cadence but the server applies the global
CommitRateLimiting check to every inbound websocket message, so presence frames will be dropped
under default limits and can starve other message types from the same IP.

Code

lib/ws/client.go[R184-190]

+		} else if strings.Contains(decodedMessage, "SHEET_PRESENCE") {
+			var presence ws.SheetPresenceIncoming
+			if err := json.Unmarshal(message, &presence); err != nil {
+				logger.Error("Error unmarshalling SHEET_PRESENCE: ", err)
+				continue
+			}
+			c.Handler.HandlePresence(c, presence)

Evidence

The server rate-limits every inbound message before checking its type, while the client explicitly
emits live presence on a 60ms timer; the default limiter allows only 10 events per second, so
presence exceeds it under normal typing.

lib/ws/client.go[132-190]
lib/settings/configRegistry.go[241-250]
ui/src/js/sheet/sheetEditor.ts[42-73]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The websocket read loop applies `CheckRateLimit()` before routing any inbound message. With live-presence emitting ~16 msgs/sec (60ms throttle), the default commit rate limit (10 events/sec) will drop presence frames and can also prevent other message processing.

### Issue Context
Presence is ephemeral and high-frequency, unlike commits; it should not share the same tight limiter used to protect write/commit paths.

### Fix Focus Areas
- lib/ws/client.go[132-191]
- lib/settings/configRegistry.go[241-250]
- ui/src/js/sheet/sheetEditor.ts[42-73]

### Suggested fix
Route messages first (or minimally parse `event` + `data.data.type`) and apply rate limiting only to commit-like operations (e.g., `USER_CHANGES`, `SHEET_OP`, etc.). Alternatively, introduce a separate, higher-threshold limiter specifically for `SHEET_PRESENCE` (and possibly cursor-only frames).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Live-edit coordinates unbounded 🐞 Bug ⛨ Security

Description

Relayed SHEET_PRESENCE row/col values are trusted and fed into FormulaEngine.setGrid(), which
allocates and indexes a grid by row/col; negative indices can throw at grid[c.row][c.col] and huge
indices can trigger massive allocations, crashing viewers.

Code

lib/ws/SheetHandler.go[R177-188]

+	out := ws.SheetPresence{Type: "COLLABROOM"}
+	out.Data = ws.SheetPresenceData{
+		Type:    "SHEET_PRESENCE",
+		UserId:  session.Author,
+		Name:    name,
+		Color:   color,
+		Sheet:   msg.Data.Data.Sheet,
+		Row:     msg.Data.Data.Row,
+		Col:     msg.Data.Data.Col,
+		Editing: editing,
+		Raw:     raw,
+	}

Evidence

The server forwards row/col as-is; the client reducer stores them in liveEdits; the editor overlays
liveEdits into the engine grid; and setGrid allocates/indexes using those coordinates with no
guards.

lib/ws/SheetHandler.go[148-199]
ui/src/js/sheet/sheetPresence.ts[39-50]
ui/src/js/sheet/sheetEditor.ts[96-99]
ui/src/js/sheet/formulaEngine.ts[50-64]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The server relays `row`/`col` from the client without validation, and the frontend stores them as live edits and recomputes via `engine.setGrid(...)`. `FormulaEngine.setGrid()` builds a rectangular `grid` up to `maxRow/maxCol` and then writes `grid[c.row][c.col] = ...`; this will throw for negative indices and can OOM for very large indices.

### Issue Context
SHEET_PRESENCE is an unauthenticated (within the pad) ephemeral channel; a malicious collaborator can send a single crafted frame to DoS other clients.

### Fix Focus Areas
- lib/ws/SheetHandler.go[153-199]
- ui/src/js/sheet/sheetPresence.ts[39-50]
- ui/src/js/sheet/sheetEditor.ts[96-99]
- ui/src/js/sheet/formulaEngine.ts[50-64]

### Suggested fix
Add strict bounds checks for `row`/`col`:
- Server-side: drop (or clamp) presence frames with `row < 0`, `col < 0`, or above a reasonable maximum (ideally tied to supported sheet dimensions for v1).
- Client-side defense-in-depth: ignore presence frames with invalid coordinates before inserting into `liveEdits`, and/or cap what is passed into `effectiveCells`.
- Harden `FormulaEngine.setGrid()` to skip any cell with out-of-range coordinates instead of indexing directly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Substring routing collision 🐞 Bug ≡ Correctness

Description

readPump routes by strings.Contains() on the entire JSON payload; because the SHEET_PRESENCE branch
is before CHAT_MESSAGE, any chat text containing "SHEET_PRESENCE" can be misrouted and skipped by
the chat handler.

Code

lib/ws/client.go[R184-190]

+		} else if strings.Contains(decodedMessage, "SHEET_PRESENCE") {
+			var presence ws.SheetPresenceIncoming
+			if err := json.Unmarshal(message, &presence); err != nil {
+				logger.Error("Error unmarshalling SHEET_PRESENCE: ", err)
+				continue
+			}
+			c.Handler.HandlePresence(c, presence)

Evidence

Routing is substring-based and ordered; chat messages carry free-form text fields that can contain
the sentinel string, triggering the earlier SHEET_PRESENCE branch.

lib/ws/client.go[137-210]
lib/models/ws/ChatMessage.go[3-37]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`readPump` routes based on `strings.Contains(decodedMessage, ...)`, which matches arbitrary substrings anywhere in the JSON, including user-controlled text fields. Adding a `SHEET_PRESENCE` branch increases collision risk (notably with chat message text) and can cause the wrong handler path to run.

### Issue Context
Chat messages include free-form `text`, so collisions are realistic even if infrequent.

### Fix Focus Areas
- lib/ws/client.go[137-238]
- lib/models/ws/ChatMessage.go[3-37]

### Suggested fix
Parse a small routing envelope first (e.g., unmarshal into a struct that captures `event`, `data.type`, and `data.data.type`) and switch on those exact fields. As a minimal mitigation, check for a stronger sentinel like `"\"type\":\"SHEET_PRESENCE\""` and verify `presence.Data.Data.Type == "SHEET_PRESENCE"` before calling `HandlePresence`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. Presence author lookup hot-path 🐞 Bug ➹ Performance

Description

HandlePresence calls authorManager.GetAuthor() for every presence frame, which becomes per-keystroke
datastore reads under live typing and can become a scalability bottleneck.

Code

lib/ws/SheetHandler.go[R166-175]

+	var name, color string
+	if a, err := p.authorManager.GetAuthor(session.Author); err == nil && a != nil {
+		if a.Name != nil {
+			name = *a.Name
+		}
+		color = a.ColorId
+	} else if err != nil {
+		p.Logger.Warn("SHEET_PRESENCE: author lookup failed for ", session.Author, ": ", err)
+		// relay continues with empty name/color — presence is best-effort
+	}

Evidence

The presence relay does a GetAuthor() lookup, and GetAuthor() directly calls the datastore; with the
client emitting presence frequently, this becomes a hot-path DB operation.

lib/ws/SheetHandler.go[166-175]
lib/author/authorManager.go[161-170]
ui/src/js/sheet/sheetEditor.ts[49-59]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`HandlePresence` performs `authorManager.GetAuthor(session.Author)` on every presence relay. With live typing, this can be 10+ lookups/sec/user, causing unnecessary datastore load.

### Issue Context
Author name/color are relatively static per session; they can be cached and reused.

### Fix Focus Areas
- lib/ws/SheetHandler.go[153-188]
- lib/author/authorManager.go[161-170]

### Suggested fix
Cache `{name,color}` per `session.Author` (or per session) in memory (e.g., sync.Map with TTL) and refresh only on connect / USERINFO_UPDATE / explicit changes. Use cached values in `HandlePresence` and fall back to datastore lookup only on cache miss.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Readonly not enforced client 🐞 Bug ≡ Correctness

Description

The sheet editor receives the readonly flag from SHEET_VARS but still wires onEdit/onLiveEdit to
send ops and live edits, causing confusing local behavior and avoidable traffic (server later
rejects/strips).

Code

ui/src/js/sheet/sheetEditor.ts[R129-136]

+      onSelect: (r, c) => sendSelect(r, c),
+      onLiveEdit: (r, c, raw) => sendLiveEdit(r, c, raw),
+      onEditEnd: (r, c, committed) => {
+        cancelPendingLive();
+        // Commit path: the setCell op clears the overlay on receivers via
+        // NEW_SHEET_OP.author — sending editing:false here would flicker.
+        if (!committed) sendPresence(r, c, false);
+      },

Evidence

Server includes readonly in SHEET_VARS, but the editor unconditionally wires edit and live-edit
callbacks that emit operations/presence regardless of the flag.

ui/src/js/sheet/sheetEditor.ts[10-16]
ui/src/js/sheet/sheetEditor.ts[114-136]
lib/ws/SheetHandler.go[231-254]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The client has `readonly` in `SheetVarsData` and the server populates it, but `initSheet()` does not use it to disable editing or suppress live-edit presence emits. Readonly users can type locally and emit traffic that will never commit.

### Issue Context
Server-side enforcement exists (`session.ReadOnly` strips presence live edits and rejects sheet ops), but the client should prevent sending and editing for UX and load.

### Fix Focus Areas
- ui/src/js/sheet/sheetEditor.ts[10-16]
- ui/src/js/sheet/sheetEditor.ts[114-136]
- lib/ws/SheetHandler.go[231-254]

### Suggested fix
When `data.readonly` is true:
- do not call `collab.applyLocal(...)` in `onEdit`
- do not call `sendLiveEdit` (and/or `sendPresence(..., true, raw)`)
- optionally disable `contentEditable` in `DomSheetView` (requires a small API change), while still allowing `onSelect` cursor updates.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-free-for-open-source-projects]

1. Presence rate-limit drops 🐞 Bug ☼ Reliability

SHEET_PRESENCE messages are sent on a ~60ms cadence but the server applies the global
CommitRateLimiting check to every inbound websocket message, so presence frames will be dropped
under default limits and can starve other message types from the same IP.

Agent Prompt

### Issue description
The websocket read loop applies `CheckRateLimit()` before routing any inbound message. With live-presence emitting ~16 msgs/sec (60ms throttle), the default commit rate limit (10 events/sec) will drop presence frames and can also prevent other message processing.

### Issue Context
Presence is ephemeral and high-frequency, unlike commits; it should not share the same tight limiter used to protect write/commit paths.

### Fix Focus Areas
- lib/ws/client.go[132-191]
- lib/settings/configRegistry.go[241-250]
- ui/src/js/sheet/sheetEditor.ts[42-73]

### Suggested fix
Route messages first (or minimally parse `event` + `data.data.type`) and apply rate limiting only to commit-like operations (e.g., `USER_CHANGES`, `SHEET_OP`, etc.). Alternatively, introduce a separate, higher-threshold limiter specifically for `SHEET_PRESENCE` (and possibly cursor-only frames).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[qodo-free-for-open-source-projects]

2. Live-edit coordinates unbounded 🐞 Bug ⛨ Security

Relayed SHEET_PRESENCE row/col values are trusted and fed into FormulaEngine.setGrid(), which
allocates and indexes a grid by row/col; negative indices can throw at grid[c.row][c.col] and huge
indices can trigger massive allocations, crashing viewers.

Agent Prompt

### Issue description
The server relays `row`/`col` from the client without validation, and the frontend stores them as live edits and recomputes via `engine.setGrid(...)`. `FormulaEngine.setGrid()` builds a rectangular `grid` up to `maxRow/maxCol` and then writes `grid[c.row][c.col] = ...`; this will throw for negative indices and can OOM for very large indices.

### Issue Context
SHEET_PRESENCE is an unauthenticated (within the pad) ephemeral channel; a malicious collaborator can send a single crafted frame to DoS other clients.

### Fix Focus Areas
- lib/ws/SheetHandler.go[153-199]
- ui/src/js/sheet/sheetPresence.ts[39-50]
- ui/src/js/sheet/sheetEditor.ts[96-99]
- ui/src/js/sheet/formulaEngine.ts[50-64]

### Suggested fix
Add strict bounds checks for `row`/`col`:
- Server-side: drop (or clamp) presence frames with `row < 0`, `col < 0`, or above a reasonable maximum (ideally tied to supported sheet dimensions for v1).
- Client-side defense-in-depth: ignore presence frames with invalid coordinates before inserting into `liveEdits`, and/or cap what is passed into `effectiveCells`.
- Harden `FormulaEngine.setGrid()` to skip any cell with out-of-range coordinates instead of indexing directly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools