Skip to content

Commit 447c4f7

Browse files
authored
feat(notebook): autofocus first cell in new notebooks (#1020)
The daemon now creates notebooks with zero cells instead of one. After sync completes, the frontend creates the first code cell locally via the CRDT, giving the user an instant focused editor. This is more local-first: the cell appears immediately without waiting for a daemon round-trip. Changes: - Remove cell seeding from create_empty_notebook() in the daemon - Add autoseed effect in NotebookView that creates a code cell when cellIds is empty after sync, with a didAutoSeed ref guard - Move setIsLoading(false) after materializeCells completes in the frame pipeline (both success and error paths) - Add useEffect([autoFocus]) in CodeMirrorEditor for post-mount focus - Add sync convergence test: empty daemon doc + frontend cell creation - Update Rust and Python test assertions for 0-cell notebooks Closes #1007
1 parent d60ed50 commit 447c4f7

File tree

7 files changed

+91
-23
lines changed

7 files changed

+91
-23
lines changed

apps/notebook/src/components/NotebookView.tsx

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,25 @@ function NotebookViewContent({
489489
}
490490
}, [focusedCellId]);
491491

492+
// ── Auto-seed first cell for empty notebooks ───────────────────────
493+
// For new notebooks the daemon creates zero cells. Once sync completes
494+
// (isLoading becomes false), we create the first code cell locally via
495+
// the CRDT so the user gets an instant focused editor. The ref guard
496+
// ensures we only seed once even if the effect re-fires before React
497+
// processes the focusedCellId update from onAddCell.
498+
const didAutoSeed = useRef(false);
499+
useEffect(() => {
500+
if (isLoading || focusedCellId !== null) return;
501+
if (cellIds.length === 0) {
502+
if (!didAutoSeed.current) {
503+
didAutoSeed.current = true;
504+
onAddCell("code");
505+
}
506+
} else {
507+
onFocusCell(cellIds[0]);
508+
}
509+
}, [isLoading, cellIds, focusedCellId, onFocusCell, onAddCell]);
510+
492511
const renderCell = useCallback(
493512
(
494513
cell: NotebookCell,

apps/notebook/src/lib/frame-pipeline.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,13 +205,13 @@ export function createFramePipeline(deps: FramePipelineDeps): Subscription {
205205
// Sync delivered actual document content — clear the gate
206206
// and materialize. This is the success path.
207207
deps.setAwaitingInitialSync(false);
208-
deps.setIsLoading(false);
209208
const handle = deps.getHandle();
210209
if (handle) {
211210
return from(
212211
deps
213212
.materializeCells(handle)
214213
.then(() => {
214+
deps.setIsLoading(false);
215215
notifyMetadataChanged();
216216
deps.onSyncApplied();
217217
})
@@ -220,10 +220,12 @@ export function createFramePipeline(deps: FramePipelineDeps): Subscription {
220220
"[frame-pipeline] initial materialize failed:",
221221
err,
222222
);
223+
deps.setIsLoading(false);
223224
deps.onSyncApplied();
224225
}),
225226
);
226227
}
228+
deps.setIsLoading(false); // Fallback if no handle
227229
}
228230
// changed:false — Automerge sync protocol handshake round
229231
// (exchanging heads/bloom filters, no actual content yet).

crates/notebook-doc/src/lib.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4123,6 +4123,55 @@ mod tests {
41234123
assert_eq!(actors, vec!["human:tab-1", "runtimed"]);
41244124
}
41254125

4126+
/// Validates the local-first empty notebook flow: daemon creates a doc
4127+
/// with metadata but zero cells, frontend creates a cell locally, then
4128+
/// sync converges so both sides have the cell.
4129+
#[test]
4130+
fn test_frontend_creates_cell_syncs_to_empty_daemon() {
4131+
use automerge::sync;
4132+
4133+
// Daemon creates doc with metadata but 0 cells
4134+
let mut daemon = NotebookDoc::new_with_actor("nb", "runtimed");
4135+
assert_eq!(daemon.cell_count(), 0);
4136+
4137+
// Frontend starts empty
4138+
let mut frontend = NotebookDoc::empty_with_actor("human:tab-1");
4139+
4140+
let mut ds = sync::State::new();
4141+
let mut fs = sync::State::new();
4142+
4143+
// Initial sync: frontend gets daemon's schema/metadata
4144+
for _ in 0..10 {
4145+
if let Some(m) = daemon.generate_sync_message(&mut ds) {
4146+
frontend.receive_sync_message(&mut fs, m).unwrap();
4147+
}
4148+
if let Some(m) = frontend.generate_sync_message(&mut fs) {
4149+
daemon.receive_sync_message(&mut ds, m).unwrap();
4150+
}
4151+
}
4152+
assert_eq!(frontend.cell_count(), 0);
4153+
assert_eq!(daemon.cell_count(), 0);
4154+
4155+
// Frontend creates a cell locally (like the autoseed effect)
4156+
frontend.add_cell(0, "cell-1", "code").unwrap();
4157+
assert_eq!(frontend.cell_count(), 1);
4158+
4159+
// Sync again — frontend's cell should reach the daemon
4160+
for _ in 0..10 {
4161+
if let Some(m) = frontend.generate_sync_message(&mut fs) {
4162+
daemon.receive_sync_message(&mut ds, m).unwrap();
4163+
}
4164+
if let Some(m) = daemon.generate_sync_message(&mut ds) {
4165+
frontend.receive_sync_message(&mut fs, m).unwrap();
4166+
}
4167+
}
4168+
4169+
// Both should have the cell
4170+
assert_eq!(daemon.cell_count(), 1);
4171+
assert_eq!(frontend.cell_count(), 1);
4172+
assert_eq!(daemon.get_cells()[0].id, "cell-1");
4173+
}
4174+
41264175
#[test]
41274176
fn test_per_cell_accessors() {
41284177
let mut doc = NotebookDoc::new("nb-accessors");

crates/runtimed/src/daemon.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1284,7 +1284,7 @@ impl Daemon {
12841284

12851285
/// Handle a CreateNotebook connection.
12861286
///
1287-
/// Daemon creates empty room with one code cell, generates env_id as notebook_id.
1287+
/// Daemon creates empty room with zero cells, generates env_id as notebook_id.
12881288
/// Returns NotebookConnectionInfo, then continues as normal notebook sync.
12891289
async fn handle_create_notebook<S>(
12901290
self: Arc<Self>,

crates/runtimed/src/notebook_sync_server.rs

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5031,13 +5031,8 @@ pub fn create_empty_notebook(
50315031
let env_id = env_id
50325032
.map(|s| s.to_string())
50335033
.unwrap_or_else(|| uuid::Uuid::new_v4().to_string());
5034-
let cell_id = uuid::Uuid::new_v4().to_string();
5035-
5036-
// Add a single empty code cell
5037-
doc.add_cell(0, &cell_id, "code")
5038-
.map_err(|e| format!("Failed to add cell: {}", e))?;
5039-
5040-
// Build metadata based on runtime
5034+
// Build metadata based on runtime (no cells — the frontend creates the
5035+
// first cell locally via the CRDT so the user gets instant autofocus)
50415036
let metadata_snapshot = build_new_notebook_metadata(runtime, &env_id, default_python_env);
50425037

50435038
doc.set_metadata_snapshot(&metadata_snapshot)
@@ -7030,11 +7025,8 @@ mod tests {
70307025
let env_id = result.unwrap();
70317026
assert!(!env_id.is_empty(), "Should generate an env_id");
70327027

7033-
// Should have exactly one cell
7034-
assert_eq!(doc.cell_count(), 1);
7035-
let cells = doc.get_cells();
7036-
assert_eq!(cells[0].cell_type, "code");
7037-
assert!(cells[0].source.is_empty());
7028+
// Should have zero cells (frontend creates the first cell locally)
7029+
assert_eq!(doc.cell_count(), 0);
70387030
}
70397031

70407032
#[test]
@@ -7048,7 +7040,7 @@ mod tests {
70487040
);
70497041

70507042
assert!(result.is_ok());
7051-
assert_eq!(doc.cell_count(), 1);
7043+
assert_eq!(doc.cell_count(), 0);
70527044

70537045
// Check metadata was set correctly
70547046
let metadata = doc.get_metadata_snapshot();

python/runtimed/tests/test_daemon_integration.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2308,25 +2308,23 @@ class TestCreateNotebook:
23082308
"""Test Client.create_notebook() - daemon-owned creation."""
23092309

23102310
def test_create_python_notebook(self, client):
2311-
"""Creating Python notebook returns session with one empty cell."""
2311+
"""Creating Python notebook returns session with zero cells."""
23122312
session = client.create_notebook(runtime="python")
23132313
assert session.is_connected
23142314

23152315
# notebook_id is UUID (not a path)
23162316
assert len(session.notebook_id) == 36 # UUID format
23172317

2318-
# Has one empty code cell
2318+
# Has zero cells (frontend creates the first cell locally)
23192319
cells = session.get_cells()
2320-
assert len(cells) == 1
2321-
assert cells[0].cell_type == "code"
2322-
assert cells[0].source == ""
2320+
assert len(cells) == 0
23232321

23242322
def test_create_notebook_returns_connection_info(self, client):
23252323
"""NotebookConnectionInfo is available for created notebooks."""
23262324
session = client.create_notebook(runtime="python")
23272325
info = session.connection_info
23282326
assert info is not None
2329-
assert info.cell_count == 1
2327+
assert info.cell_count == 0
23302328
assert info.notebook_id == session.notebook_id
23312329
# New notebooks don't need trust approval
23322330
assert info.needs_trust_approval is False
@@ -2336,9 +2334,9 @@ def test_create_deno_notebook(self, client):
23362334
session = client.create_notebook(runtime="deno")
23372335
assert session.is_connected
23382336

2339-
# Has one empty code cell
2337+
# Has zero cells (frontend creates the first cell locally)
23402338
cells = session.get_cells()
2341-
assert len(cells) == 1
2339+
assert len(cells) == 0
23422340

23432341
def test_create_notebook_with_working_dir(self, client, tmp_path):
23442342
"""working_dir is used for project file detection."""

src/components/editor/codemirror-editor.tsx

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,14 @@ export const CodeMirrorEditor = forwardRef<
272272
// eslint-disable-next-line react-hooks/exhaustive-deps
273273
}, []);
274274

275+
// Focus when autoFocus becomes true after mount (e.g. notebook
276+
// auto-seeds a cell then sets focusedCellId).
277+
useEffect(() => {
278+
if (autoFocus && viewRef.current && !viewRef.current.hasFocus) {
279+
requestAnimationFrame(() => viewRef.current?.focus());
280+
}
281+
}, [autoFocus]);
282+
275283
// ── Dynamic reconfiguration via compartments ─────────────────────
276284

277285
useEffect(() => {

0 commit comments

Comments
 (0)