Migrate DuckDB to ClickHouse#18
Conversation
Resolves the single-node limit: heavy analytical requests across many FastAPI pods now share one synchronized OLAP backend instead of forking into split local gridify.duckdb files. - BaseOLAPEngine interface so routers/agents only depend on one contract - DuckDBOLAPEngine (local-first), ClickHouseOLAPEngine (HTTP), MotherDuckOLAPEngine (shared cloud store) - OLAP_BACKEND config + factory singleton - /api/olap/engine and read-only /api/olap/query endpoints - Unit tests covering binding, literals, and backend selection
…pt context Feeds a structured abstraction (not raw schema) into Gemini to cut token usage and enforce role-based data security. - SemanticModel with logical measures/dimensions and per-role grants - authorize() enforces RBAC before any SQL is compiled - compile() emits engine-specific SQL (DuckDB / ClickHouse dialects) - build_prompt_context() emits a token-efficient description for the LLM - /api/semantic/context and /api/semantic/compile endpoints - Unit tests for RBAC denial, dialect quoting, and context scoping
Cuts initialization cost ('Time to First Query') for ONNX/vector-index
delivery on mobile and slow networks.
- quantization.ts: float32 -> int8 scalar quantize/dequantize + int8 cosine
- modelStreamCache.ts: chunked Cache Storage API delivery with resume +
in-memory fallback for SSR/tests
- ONNX worker returns quantized embeddings to shrink worker->main payload
- RAG client caches/restores a quantized index so refreshes skip re-embed
- Vitest coverage for quantization and chunked cache
GenAI drafts and Framer-Motion drags now stream to every active session instead of living only in ephemeral Zustand state. - realtimeSync.ts: provider abstraction (BroadcastChannel default, Yjs multiplayer CRDT lazy-loaded, Supabase hosted) + DashboardSync binder - LayoutRepository: in-memory default + PostgreSQL store for durable, shareable, cross-pod layout persistence - /api/layouts CRUD (save/load/list/delete, public sharing) - REALTIME_PROVIDER + Yjs/Supabase config - Vitest + pytest coverage for sync convergence and layout storage
- Shared in-memory fallback store keyed by cacheName (modelStreamCache) - Serialize int8 codes as plain arrays for JSON round-trip - Restore Int8Array on load of quantized index - Avoid parameter shadowing the worker quantize() fn - Make ONNXEmbedder.modelUrl/dim accessible; fix dynamic-import typing - Fix InMemoryLayoutRepository.save when creating a new layout
|
Warning Review limit reached
Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (26)
✨ Finishing Touches🧪 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 |
Greptile SummaryThis PR migrates analytics and collaboration features behind new backend and frontend abstractions. The main changes are:
Confidence Score: 4/5The new backend data and SQL paths need fixes before merging. Layout ownership, raw OLAP SQL handling, and semantic SQL generation can all behave incorrectly.
backend/app/routers/layouts.py, backend/app/routers/olap.py, backend/app/services/semantic_model.py, backend/app/services/olap/motherduck_engine.py, src/lib/realtimeSync.ts
|
| Filename | Overview |
|---|---|
| backend/app/routers/layouts.py | Adds saved-layout CRUD routes, but the routes do not enforce owner checks for read, overwrite, or delete operations. |
| backend/app/routers/olap.py | Adds a raw SQL query endpoint with prefix-only read checks and unconditional limit wrapping. |
| backend/app/services/semantic_model.py | Adds semantic SQL compilation, but some generated SQL is invalid, unsafe, or not aligned with the created OLAP tables. |
| backend/app/services/olap/motherduck_engine.py | Adds a MotherDuck engine, with token handling inside a generated SQL statement. |
| src/lib/realtimeSync.ts | Adds realtime layout sync providers, but join and Yjs replacement behavior can overwrite shared state. |
| src/lib/modelStreamCache.ts | Adds chunked model caching and quantized-index serialization with mostly coherent round-trip behavior. |
| src/lib/ragClient.ts | Integrates quantized index caching into the local RAG flow. |
Prompt To Fix All With AI
Fix the following 11 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 11
backend/app/routers/layouts.py:28-31
**Client-Controlled Layout Owner**
`create_layout` trusts `owner` and `id` from the request body, then `save()` overwrites on ID conflicts. A caller can submit another user's layout ID and owner string to replace that saved dashboard's widgets or public flag.
### Issue 2 of 11
backend/app/routers/layouts.py:36-41
**Private Layouts Read By ID**
`get_layout` returns any stored layout by ID without checking ownership or `is_public`. With the new persistent repository, anyone who obtains a layout ID can read private dashboard widget data through this route.
### Issue 3 of 11
backend/app/routers/layouts.py:56-61
**Layouts Deleted By ID**
`delete_layout` deletes by layout ID alone, and the repository delete call has no owner predicate. Any caller with a saved layout ID can delete another user's dashboard layout.
### Issue 4 of 11
backend/app/routers/olap.py:35-40
**Prefix Check Allows Writes**
The read-only check only verifies that the SQL starts with `select`, `show`, or `with`, then forwards the full string to the engine. A request such as `select 1; drop table telemetry` passes the guard and can execute the later mutating statement before the appended limit helps.
### Issue 5 of 11
backend/app/routers/olap.py:40
**Duplicate Limit Breaks Queries**
`olap_query` appends `LIMIT` even when the supplied SQL already has one. SQL produced by `/api/semantic/compile` with a `limit` becomes `... LIMIT n LIMIT m`, which DuckDB and ClickHouse reject and the API returns as a backend error.
### Issue 6 of 11
backend/app/services/semantic_model.py:172-176
**Where After Group By**
`compile()` appends `GROUP BY` before `WHERE`, so any semantic query with both dimensions and filters emits invalid SQL. A grouped query filtered by `metric_type`, for example, fails at execution instead of returning analytics rows.
### Issue 7 of 11
backend/app/services/semantic_model.py:248-259
**Semantic Columns Missing From Schema**
The default semantic model advertises fields backed by `score`, `uptime`, `load`, `status`, and `type`, but the new OLAP schemas only create `id`, `device_id`, `metric_type`, `value`, `timestamp`, and `metadata`. Queries for those advertised measures or dimensions compile successfully and then fail with missing-column errors.
### Issue 8 of 11
backend/app/services/semantic_model.py:197-201
**Range Values Interpolated Raw**
For `gt`, `gte`, `lt`, `lte`, and `between`, filter values are inserted directly into the generated SQL. A caller using an allowed filter field can pass non-numeric text that is returned as part of the SQL and then reaches the OLAP query path.
### Issue 9 of 11
backend/app/services/olap/motherduck_engine.py:34
**Token Interpolated Into SQL**
The MotherDuck token is placed directly inside a `CREATE SECRET` SQL string. A valid token containing a quote or SQL metacharacter can break engine startup or change the statement being executed.
### Issue 10 of 11
src/lib/realtimeSync.ts:291-294
**Join Broadcast Overwrites State**
`start()` publishes the local initial widgets as a fresh `init` revision. When a second client joins with stale or empty state, that newer init is accepted by existing clients and can replace the current shared layout.
### Issue 11 of 11
src/lib/realtimeSync.ts:182-183
**Yjs Replacement Exposes Empty State**
The Yjs provider replaces the whole array by deleting everything and then inserting the new widgets, while its observer also receives local-origin changes. Peers or local callbacks can observe the intermediate empty array and apply an empty layout, and self-reflected updates can loop if `onChange` calls back into `update()`.
Reviews (1): Last reviewed commit: "Fix type/serialization issues in realtim..." | Re-trigger Greptile
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several scalability, real-time, and AI-safety architectural improvements, including a pluggable OLAP storage tier (supporting DuckDB, ClickHouse, and MotherDuck), real-time collaborative layout sync, a formalized semantic layer with RBAC and dialect-aware compilation, and edge-RAG tiered quantization and streaming. The review feedback identifies several critical bugs and security vulnerabilities: an AttributeError when saving new layouts in PostgresLayoutRepository, a SQL injection vulnerability and incorrect operator mapping in the semantic model's filter rendering, potential query limit bypasses via SQL comments in the OLAP router, channel churn in the Supabase realtime provider, query corruption in ClickHouse parameter binding, and encoding issues with multi-byte UTF-8 characters in the model stream cache.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| existing = self.get(layout.id) | ||
| created = existing.created_at or now.isoformat() | ||
| with self._engine.begin() as conn: | ||
| conn.execute( | ||
| text( | ||
| """ | ||
| INSERT INTO dashboard_layouts | ||
| (id, name, owner, widgets, is_public, created_at, updated_at) | ||
| VALUES (:id, :name, :owner, :widgets, :public, :created, :updated) | ||
| ON CONFLICT (id) DO UPDATE SET | ||
| name = EXCLUDED.name, | ||
| owner = EXCLUDED.owner, | ||
| widgets = EXCLUDED.widgets, | ||
| is_public = EXCLUDED.is_public, | ||
| updated_at = EXCLUDED.updated_at | ||
| """ | ||
| ), | ||
| { | ||
| "id": layout.id, | ||
| "name": layout.name, | ||
| "owner": layout.owner, | ||
| "widgets": json.dumps(layout.widgets), | ||
| "public": layout.is_public, | ||
| "created": existing.created_at or now, | ||
| "updated": now, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Critical bug in PostgresLayoutRepository.save. If existing is None (which is always the case when saving a new layout), accessing existing.created_at on lines 132 and 154 will raise an AttributeError: 'NoneType' object has no attribute 'created_at'. This will completely crash the save operation for any new layout. We should safely check if existing is not None before accessing created_at.
existing = self.get(layout.id)
created_str = existing.created_at if (existing and existing.created_at) else now.isoformat()
created_dt = datetime.fromisoformat(created_str) if isinstance(created_str, str) else created_str
with self._engine.begin() as conn:
conn.execute(
text(
"""
INSERT INTO dashboard_layouts
(id, name, owner, widgets, is_public, created_at, updated_at)
VALUES (:id, :name, :owner, :widgets, :public, :created, :updated)
ON CONFLICT (id) DO UPDATE SET
name = EXCLUDED.name,
owner = EXCLUDED.owner,
widgets = EXCLUDED.widgets,
is_public = EXCLUDED.is_public,
updated_at = EXCLUDED.updated_at
"""
),
{
"id": layout.id,
"name": layout.name,
"owner": layout.owner,
"widgets": json.dumps(layout.widgets),
"public": layout.is_public,
"created": created_dt,
"updated": now,
},
)| if op in ("gt", "gte", "lt", "lte"): | ||
| return f"{field} {op} {value}" | ||
| if op == "between": | ||
| a, b = value | ||
| return f"{field} BETWEEN {a} AND {b}" |
There was a problem hiding this comment.
Critical SQL injection vulnerability and correctness bug in _render_filter. The operators gt, gte, lt, and lte are not valid SQL operators (they should be mapped to >, >=, <, <=). Furthermore, the value is directly interpolated without any escaping or quoting for these comparison operators and the between operator, allowing arbitrary SQL injection. We should map the operators to their SQL equivalents and safely escape/quote the values.
| if op in ("gt", "gte", "lt", "lte"): | |
| return f"{field} {op} {value}" | |
| if op == "between": | |
| a, b = value | |
| return f"{field} BETWEEN {a} AND {b}" | |
| OP_MAP = {"gt": ">", "gte": ">=", "lt": "<", "lte": "<="} | |
| if op in OP_MAP: | |
| val = str(value) if isinstance(value, (int, float)) else f"'{SemanticModel._esc(value)}'" | |
| return f"{field} {OP_MAP[op]} {val}" | |
| if op == "between": | |
| a, b = value | |
| val_a = str(a) if isinstance(a, (int, float)) else f"'{SemanticModel._esc(a)}'" | |
| val_b = str(b) if isinstance(b, (int, float)) else f"'{SemanticModel._esc(b)}'" | |
| return f"{field} BETWEEN {val_a} AND {val_b}" |
| broadcast(message: Omit<SyncMessage, "room" | "sender">): void { | ||
| if (!this.client) return; | ||
| this.client.channel(`layout:${this.room}`).send({ | ||
| type: "broadcast", | ||
| event: "sync", | ||
| payload: { ...message, room: this.room, sender: this.sender }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
In SupabaseProvider.broadcast, calling this.client.channel(...) on every single broadcast creates a new channel instance each time. During active dragging or typing, this will cause massive channel churn, leading to memory leaks, connection issues, or rate limits. The channel should be created once during connect, stored as a private property (e.g., this.channel), and reused here.
| safe_sql = sql.strip().rstrip(";") | ||
| if not safe_sql.lower().startswith(("select", "show", "with")): | ||
| raise HTTPException( | ||
| status_code=400, detail="Only read-only statements are permitted" | ||
| ) | ||
| rows = engine.query(f"{safe_sql} LIMIT {limit}") |
There was a problem hiding this comment.
Security and correctness issue in olap_query. Appending LIMIT {limit} directly to safe_sql can be bypassed if the client query ends with a SQL comment (e.g., -- or /*), which comments out the limit clause and can lead to Denial of Service (DoS) or out-of-memory errors by fetching the entire dataset. Consider stripping trailing comments or wrapping the query as a subquery to guarantee the limit is enforced.
| function encodeJSON(value: unknown): ArrayBuffer { | ||
| const text = JSON.stringify(value); | ||
| const buf = new Uint8Array(text.length); | ||
| for (let i = 0; i < text.length; i++) buf[i] = text.charCodeAt(i); | ||
| return buf.buffer; | ||
| } | ||
|
|
||
| function decodeJSON<T>(buf: ArrayBuffer): T { | ||
| const bytes = new Uint8Array(buf); | ||
| let text = ""; | ||
| for (let i = 0; i < bytes.length; i++) text += String.fromCharCode(bytes[i]); | ||
| return JSON.parse(text) as T; | ||
| } |
There was a problem hiding this comment.
The encodeJSON and decodeJSON functions use manual loops with charCodeAt and String.fromCharCode. This does not correctly handle multi-byte UTF-8 characters (e.g., non-ASCII characters in layout names or widget titles), which can lead to data corruption. Use the standard TextEncoder and TextDecoder APIs instead, which are robust, native, and much more efficient.
function encodeJSON(value: unknown): ArrayBuffer {
const text = JSON.stringify(value);
return new TextEncoder().encode(text).buffer;
}
function decodeJSON<T>(buf: ArrayBuffer): T {
const text = new TextDecoder().decode(new Uint8Array(buf));
return JSON.parse(text) as T;
}| def _bind(sql: str, params: List[Any]) -> str: | ||
| """Positional ``?`` binding for simple literals (ClickHouse HTTP). | ||
|
|
||
| Values are escaped and wrapped as SQL literals. This is sufficient for | ||
| telemetry-style parameters; complex payloads should use the native | ||
| client with parameterized statements. | ||
| """ | ||
| out: List[str] = [] | ||
| idx = 0 | ||
| for ch in sql: | ||
| if ch == "?" and idx < len(params): | ||
| out.append(ClickHouseOLAPEngine._literal(params[idx])) | ||
| idx += 1 | ||
| else: | ||
| out.append(ch) | ||
| return "".join(out) |
There was a problem hiding this comment.
Naive positional ? binding replaces any ? character in the SQL string, even if it is inside a string literal or comment (e.g., WHERE device_id = 'dev?1'). This can corrupt queries or cause parameter mismatch errors. Consider using ClickHouse's native HTTP parameterization support (e.g., passing param_val=... in query parameters and using {val:String} in the SQL query) for a robust and safe implementation.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| owner=payload.get("owner", "anonymous"), | ||
| widgets=payload.get("widgets", []), | ||
| is_public=bool(payload.get("is_public", False)), | ||
| ) |
There was a problem hiding this comment.
create_layout trusts owner and id from the request body, then save() overwrites on ID conflicts. A caller can submit another user's layout ID and owner string to replace that saved dashboard's widgets or public flag.
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/app/routers/layouts.py
Line: 28-31
Comment:
**Client-Controlled Layout Owner**
`create_layout` trusts `owner` and `id` from the request body, then `save()` overwrites on ID conflicts. A caller can submit another user's layout ID and owner string to replace that saved dashboard's widgets or public flag.
How can I resolve this? If you propose a fix, please make it concise.| @router.get("/layouts/{layout_id}") | ||
| async def get_layout(layout_id: str) -> Dict[str, Any]: | ||
| repo = get_layout_repository() | ||
| layout = repo.get(layout_id) | ||
| if not layout: | ||
| raise HTTPException(status_code=404, detail="Layout not found") |
There was a problem hiding this comment.
get_layout returns any stored layout by ID without checking ownership or is_public. With the new persistent repository, anyone who obtains a layout ID can read private dashboard widget data through this route.
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/app/routers/layouts.py
Line: 36-41
Comment:
**Private Layouts Read By ID**
`get_layout` returns any stored layout by ID without checking ownership or `is_public`. With the new persistent repository, anyone who obtains a layout ID can read private dashboard widget data through this route.
How can I resolve this? If you propose a fix, please make it concise.| @router.delete("/layouts/{layout_id}") | ||
| async def delete_layout(layout_id: str) -> Dict[str, Any]: | ||
| repo = get_layout_repository() | ||
| ok = repo.delete(layout_id) | ||
| if not ok: | ||
| raise HTTPException(status_code=404, detail="Layout not found") |
There was a problem hiding this comment.
delete_layout deletes by layout ID alone, and the repository delete call has no owner predicate. Any caller with a saved layout ID can delete another user's dashboard layout.
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/app/routers/layouts.py
Line: 56-61
Comment:
**Layouts Deleted By ID**
`delete_layout` deletes by layout ID alone, and the repository delete call has no owner predicate. Any caller with a saved layout ID can delete another user's dashboard layout.
How can I resolve this? If you propose a fix, please make it concise.| safe_sql = sql.strip().rstrip(";") | ||
| if not safe_sql.lower().startswith(("select", "show", "with")): | ||
| raise HTTPException( | ||
| status_code=400, detail="Only read-only statements are permitted" | ||
| ) | ||
| rows = engine.query(f"{safe_sql} LIMIT {limit}") |
There was a problem hiding this comment.
The read-only check only verifies that the SQL starts with select, show, or with, then forwards the full string to the engine. A request such as select 1; drop table telemetry passes the guard and can execute the later mutating statement before the appended limit helps.
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/app/routers/olap.py
Line: 35-40
Comment:
**Prefix Check Allows Writes**
The read-only check only verifies that the SQL starts with `select`, `show`, or `with`, then forwards the full string to the engine. A request such as `select 1; drop table telemetry` passes the guard and can execute the later mutating statement before the appended limit helps.
How can I resolve this? If you propose a fix, please make it concise.| raise HTTPException( | ||
| status_code=400, detail="Only read-only statements are permitted" | ||
| ) | ||
| rows = engine.query(f"{safe_sql} LIMIT {limit}") |
There was a problem hiding this comment.
olap_query appends LIMIT even when the supplied SQL already has one. SQL produced by /api/semantic/compile with a limit becomes ... LIMIT n LIMIT m, which DuckDB and ClickHouse reject and the API returns as a backend error.
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/app/routers/olap.py
Line: 40
Comment:
**Duplicate Limit Breaks Queries**
`olap_query` appends `LIMIT` even when the supplied SQL already has one. SQL produced by `/api/semantic/compile` with a `limit` becomes `... LIMIT n LIMIT m`, which DuckDB and ClickHouse reject and the API returns as a backend error.
How can I resolve this? If you propose a fix, please make it concise.| def build_default_model() -> SemanticModel: | ||
| model = SemanticModel() | ||
| model.add_measure(LogicalMeasure("score", "score", "AVG", "device score")) | ||
| model.add_measure(LogicalMeasure("uptime", "uptime", "AVG", "uptime minutes")) | ||
| model.add_measure(LogicalMeasure("load_count", "load", "COUNT", "load events")) | ||
| model.add_measure(LogicalMeasure("temperature", "value", "AVG", "sensor temp")) | ||
| model.add_measure(LogicalMeasure("humidity", "value", "AVG", "sensor humidity")) | ||
|
|
||
| model.add_dimension(LogicalDimension("device_id", "device_id", "device identifier")) | ||
| model.add_dimension(LogicalDimension("metric_type", "metric_type", "metric kind")) | ||
| model.add_dimension(LogicalDimension("status", "status", "device status")) | ||
| model.add_dimension(LogicalDimension("type", "type", "device type")) |
There was a problem hiding this comment.
The default semantic model advertises fields backed by score, uptime, load, status, and type, but the new OLAP schemas only create id, device_id, metric_type, value, timestamp, and metadata. Queries for those advertised measures or dimensions compile successfully and then fail with missing-column errors.
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/app/services/semantic_model.py
Line: 248-259
Comment:
**Semantic Columns Missing From Schema**
The default semantic model advertises fields backed by `score`, `uptime`, `load`, `status`, and `type`, but the new OLAP schemas only create `id`, `device_id`, `metric_type`, `value`, `timestamp`, and `metadata`. Queries for those advertised measures or dimensions compile successfully and then fail with missing-column errors.
How can I resolve this? If you propose a fix, please make it concise.| if op in ("gt", "gte", "lt", "lte"): | ||
| return f"{field} {op} {value}" | ||
| if op == "between": | ||
| a, b = value | ||
| return f"{field} BETWEEN {a} AND {b}" |
There was a problem hiding this comment.
For gt, gte, lt, lte, and between, filter values are inserted directly into the generated SQL. A caller using an allowed filter field can pass non-numeric text that is returned as part of the SQL and then reaches the OLAP query path.
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/app/services/semantic_model.py
Line: 197-201
Comment:
**Range Values Interpolated Raw**
For `gt`, `gte`, `lt`, `lte`, and `between`, filter values are inserted directly into the generated SQL. A caller using an allowed filter field can pass non-numeric text that is returned as part of the SQL and then reaches the OLAP query path.
How can I resolve this? If you propose a fix, please make it concise.| "MOTHERDUCK_TOKEN is required for the MotherDuck OLAP backend" | ||
| ) | ||
| # DuckDB attaches to MotherDuck over the wire using the ``md:`` URL and | ||
| # an auth token kept in a local config file (never logged). |
There was a problem hiding this comment.
The MotherDuck token is placed directly inside a CREATE SECRET SQL string. A valid token containing a quote or SQL metacharacter can break engine startup or change the statement being executed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/app/services/olap/motherduck_engine.py
Line: 34
Comment:
**Token Interpolated Into SQL**
The MotherDuck token is placed directly inside a `CREATE SECRET` SQL string. A valid token containing a quote or SQL metacharacter can break engine startup or change the statement being executed.
How can I resolve this? If you propose a fix, please make it concise.| revision: Date.now(), | ||
| }); | ||
| this.connected = true; | ||
| } |
There was a problem hiding this comment.
start() publishes the local initial widgets as a fresh init revision. When a second client joins with stale or empty state, that newer init is accepted by existing clients and can replace the current shared layout.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/realtimeSync.ts
Line: 291-294
Comment:
**Join Broadcast Overwrites State**
`start()` publishes the local initial widgets as a fresh `init` revision. When a second client joins with stale or empty state, that newer init is accepted by existing clients and can replace the current shared layout.
How can I resolve this? If you propose a fix, please make it concise.| this.array.delete(0, this.array.length); | ||
| this.array.insert(0, message.widgets); |
There was a problem hiding this comment.
The Yjs provider replaces the whole array by deleting everything and then inserting the new widgets, while its observer also receives local-origin changes. Peers or local callbacks can observe the intermediate empty array and apply an empty layout, and self-reflected updates can loop if onChange calls back into update().
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/realtimeSync.ts
Line: 182-183
Comment:
**Yjs Replacement Exposes Empty State**
The Yjs provider replaces the whole array by deleting everything and then inserting the new widgets, while its observer also receives local-origin changes. Peers or local callbacks can observe the intermediate empty array and apply an empty layout, and self-reflected updates can loop if `onChange` calls back into `update()`.
How can I resolve this? If you propose a fix, please make it concise.1 participant
No description provided.