From b25c85ec47ff57d68b0696e788a6021e9cb115ff Mon Sep 17 00:00:00 2001 From: Koji Ishimoto Date: Wed, 15 Apr 2026 18:53:05 +0900 Subject: [PATCH 1/3] =?UTF-8?q?chore:=20update=20loom=20submodule=20?= =?UTF-8?q?=E2=80=94=20Layer=205=20ReactiveParser=20migration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Picks up loom PRs #79 and #80: - Migrate ReactiveParser to use get_result()/Signal::get() for dual-context reads - Fix dependency tracking regression (rt.read() dropped tracking in derived memos) - Add regression test for derived memo invalidation through term() Co-Authored-By: Claude Opus 4.6 (1M context) --- loom | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/loom b/loom index fae30bae..157e3fa8 160000 --- a/loom +++ b/loom @@ -1 +1 @@ -Subproject commit fae30bae7e7b373be0892174b99a78cb68ec293c +Subproject commit 157e3fa8d212355e4afd1178daf7b332eba9128b From ce7e08db19f6ab24ed322fcad8cd750f7df2c65a Mon Sep 17 00:00:00 2001 From: Koji Ishimoto Date: Wed, 15 Apr 2026 18:57:09 +0900 Subject: [PATCH 2/3] feat(editor): semantic Before/After drop with placeholder, fix Memo::get outside tracked context Before/After drop was using raw text splice (delete source + insert unparse), which left invalid syntax (e.g. `let id = ` with empty RHS). Now mirrors Exchange: extracts expression text from source map, replaces source with placeholder via Renderable::placeholder(), preserving valid syntax. Also fixes pre-existing Memo::get() abort when called outside tracked context (broke editor unit tests and ideal editor web app). All projection accessors now use rt.read(memo) which is safe in both contexts. Adds Playwright E2E tests for drag-drop Before/After/Inside/self-drop. Co-Authored-By: Claude Opus 4.6 (1M context) --- editor/projection_memo.mbt | 15 +- editor/sync_editor_tree_edit.mbt | 56 ++++--- examples/ideal/web/e2e/drag-drop.spec.ts | 197 +++++++++++++++++++++++ lang/lambda/companion/lambda_editor.mbt | 27 +++- 4 files changed, 258 insertions(+), 37 deletions(-) create mode 100644 examples/ideal/web/e2e/drag-drop.spec.ts diff --git a/editor/projection_memo.mbt b/editor/projection_memo.mbt index a401fe9b..25b0f163 100644 --- a/editor/projection_memo.mbt +++ b/editor/projection_memo.mbt @@ -2,21 +2,24 @@ ///| /// Returns the cached ProjNode (converted from FlatProj once per invalidation). +/// Safe to call both inside and outside tracked context. pub fn[T] SyncEditor::get_proj_node(self : SyncEditor[T]) -> @core.ProjNode[T]? { - self.cached_proj_node.get() + self.parser_rt.read(self.cached_proj_node) } ///| +/// Safe to call both inside and outside tracked context. pub fn[T] SyncEditor::get_source_map(self : SyncEditor[T]) -> @core.SourceMap { - self.source_map_memo.get() + self.parser_rt.read(self.source_map_memo) } ///| /// Returns the full node registry (NodeId → ProjNode). +/// Safe to call both inside and outside tracked context. pub fn[T] SyncEditor::get_registry( self : SyncEditor[T], ) -> Map[@core.NodeId, @core.ProjNode[T]] { - self.registry_memo.get() + self.parser_rt.read(self.registry_memo) } ///| @@ -30,7 +33,7 @@ pub fn[T] SyncEditor::get_node( self : SyncEditor[T], id : @core.NodeId, ) -> @core.ProjNode[T]? { - self.registry_memo.get().get(id) + self.parser_rt.read(self.registry_memo).get(id) } ///| @@ -38,7 +41,7 @@ pub fn[T] SyncEditor::node_at_position( self : SyncEditor[T], position : Int, ) -> @core.NodeId? { - self.source_map_memo.get().innermost_node_at(position) + self.parser_rt.read(self.source_map_memo).innermost_node_at(position) } ///| @@ -46,5 +49,5 @@ pub fn[T] SyncEditor::get_node_range( self : SyncEditor[T], id : @core.NodeId, ) -> @loom_core.Range? { - self.source_map_memo.get().get_range(id) + self.parser_rt.read(self.source_map_memo).get_range(id) } diff --git a/editor/sync_editor_tree_edit.mbt b/editor/sync_editor_tree_edit.mbt index 7421f853..8b327975 100644 --- a/editor/sync_editor_tree_edit.mbt +++ b/editor/sync_editor_tree_edit.mbt @@ -18,7 +18,7 @@ pub fn[T] SyncEditor::is_dirty(self : SyncEditor[T]) -> Bool { /// inside a getter. pub fn[T] SyncEditor::refresh(self : SyncEditor[T]) -> Unit { self.projection_dirty = false - let _ = self.cached_proj_node.get() + let _ = self.parser_rt.read(self.cached_proj_node) } ///| @@ -132,14 +132,18 @@ pub fn[T] SyncEditor::apply_text_transform( } ///| -/// Move a node to a new position in the tree by: -/// 1. Unparsing the source node via T::unparse -/// 2. Deleting the source span (replace with placeholder) -/// 3. Inserting the unparsed text adjacent to the target span +/// Move a node to a new position in the tree. +/// +/// Inside (exchange): swaps the expression content of source and target, +/// preserving leading whitespace/separators. +/// +/// Before/After: replaces the source expression with its placeholder +/// (keeping the syntax valid) and inserts the source text at the target. /// /// CAUTION: Insert position must be adjusted when source precedes target /// in the document (deletion shifts subsequent offsets backward by -/// `source_len - placeholder_len`). This method handles the adjustment. +/// `source_len - placeholder_len`). apply_span_edits handles this via +/// reverse-order application. pub fn[T : @loom_core.Renderable] SyncEditor::move_node( self : SyncEditor[T], source_id : @core.NodeId, @@ -190,6 +194,18 @@ pub fn[T : @loom_core.Renderable] SyncEditor::move_node( ) } let doc_text = self.get_text() + // Strip leading whitespace from a span to find the expression-only start. + // Source map ranges include leading separators; we keep those in place. + fn skip_ws(text : String, start : Int, end : Int) -> Int { + let mut i = start + while i < end { + match text[i] { + ' ' | '\t' | '\n' | '\r' => i = i + 1 + _ => break + } + } + i + } let edits : Array[@core.SpanEdit] = match position { // Inside = exchange: swap the expression content, preserving leading whitespace Inside => { @@ -202,18 +218,6 @@ pub fn[T : @loom_core.Renderable] SyncEditor::move_node( ), ) } - // Strip leading whitespace from each span to find the expression-only range. - // Source map ranges include leading separators; we keep those in place. - fn skip_ws(text : String, start : Int, end : Int) -> Int { - let mut i = start - while i < end { - match text[i] { - ' ' | '\t' | '\n' | '\r' => i = i + 1 - _ => break - } - } - i - } let src_expr_start = skip_ws(doc_text, src_range.start, src_range.end) let tgt_expr_start = skip_ws(doc_text, tgt_range.start, tgt_range.end) let src_expr = doc_text[src_expr_start:src_range.end].to_string() @@ -231,20 +235,24 @@ pub fn[T : @loom_core.Renderable] SyncEditor::move_node( }, ] } - // Before/After = move: remove source and insert at target position + // Before/After = move: replace source with placeholder, insert at target Before | After => { - let src_text = @loom_core.Renderable::unparse(src_node.kind) + // Extract expression text from source map (whitespace-aware, like Exchange) + let src_expr_start = skip_ws(doc_text, src_range.start, src_range.end) + let src_expr = doc_text[src_expr_start:src_range.end].to_string() + // Replace source expression with placeholder (keeps leading whitespace) + let placeholder = @loom_core.Renderable::placeholder(src_node.kind) let tgt_insert = match position { Before => tgt_range.start _ => tgt_range.end } [ { - start: src_range.start, - delete_len: src_range.end - src_range.start, - inserted: "", + start: src_expr_start, + delete_len: src_range.end - src_expr_start, + inserted: placeholder, }, - { start: tgt_insert, delete_len: 0, inserted: src_text + " " }, + { start: tgt_insert, delete_len: 0, inserted: src_expr + " " }, ] } } diff --git a/examples/ideal/web/e2e/drag-drop.spec.ts b/examples/ideal/web/e2e/drag-drop.spec.ts new file mode 100644 index 00000000..78b11c69 --- /dev/null +++ b/examples/ideal/web/e2e/drag-drop.spec.ts @@ -0,0 +1,197 @@ +// Drag-and-drop E2E tests for Before/After/Inside (exchange) moves. +// Validates that AST-level operations produce valid syntax (no empty RHS, +// no orphaned separators) by checking the resulting editor text. + +import { test, expect, type Page } from '@playwright/test'; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +/** Enter Structure mode and wait for structure blocks to render. */ +async function setupStructureMode(page: Page) { + await page.goto('/'); + await page.getByRole('button', { name: 'Structure' }).click(); + await expect(page.getByLabel('Code editor')).toBeVisible(); + await page.waitForFunction(() => { + const ce = document.querySelector('canopy-editor'); + return ce?.shadowRoot?.querySelector('.structure-block') !== null; + }, { timeout: 10_000 }); +} + +/** Read the current editor text via the CRDT global. */ +async function getEditorText(page: Page): Promise { + return await page.evaluate(() => { + const g = globalThis as any; + if (g.__canopy_crdt && g.__canopy_crdt_handle != null) { + return g.__canopy_crdt.get_text(g.__canopy_crdt_handle) as string; + } + return ''; + }); +} + +/** Count structure blocks of a given type inside the shadow DOM. */ +async function countNodes(page: Page, type: string): Promise { + return await page.evaluate( + (type) => { + const ce = document.querySelector('canopy-editor'); + if (!ce?.shadowRoot) return 0; + return ce.shadowRoot.querySelectorAll(`.structure-${type}`).length; + }, + type, + ); +} + +/** + * Perform a drag-drop between two structure nodes using dispatched events. + * HTML5 drag-and-drop requires DataTransfer which page.mouse can't provide, + * so we synthesize the full dragstart/dragover/drop/dragend sequence. + */ +async function dragDrop( + page: Page, + srcSelector: string, + srcNth: number, + tgtSelector: string, + tgtNth: number, + position: 'Before' | 'After' | 'Inside', +) { + await page.evaluate( + ({ srcSelector, srcNth, tgtSelector, tgtNth, position }) => { + const ce = document.querySelector('canopy-editor'); + if (!ce?.shadowRoot) throw new Error('canopy-editor not found'); + + const srcBlocks = ce.shadowRoot.querySelectorAll(srcSelector); + const tgtBlocks = ce.shadowRoot.querySelectorAll(tgtSelector); + const src = srcBlocks[srcNth] as HTMLElement; + const tgt = tgtBlocks[tgtNth] as HTMLElement; + if (!src || !tgt) throw new Error(`Node not found: src=${srcSelector}[${srcNth}], tgt=${tgtSelector}[${tgtNth}]`); + + // Enable draggable (normally done by grip mousedown) + src.draggable = true; + + const tgtRect = tgt.getBoundingClientRect(); + let clientY: number; + switch (position) { + case 'Before': clientY = tgtRect.top + tgtRect.height * 0.1; break; + case 'After': clientY = tgtRect.top + tgtRect.height * 0.9; break; + case 'Inside': clientY = tgtRect.top + tgtRect.height * 0.5; break; + } + const clientX = tgtRect.left + tgtRect.width / 2; + + // Get source nodeId + const srcNodeId = (src as any).__pmNode?.attrs?.nodeId + ?? src.getAttribute('data-node-id') + ?? '0'; + + // Create DataTransfer with source node ID + const dt = new DataTransfer(); + dt.setData('application/x-canopy-node', String(srcNodeId)); + dt.effectAllowed = 'move'; + + // Dispatch drag sequence + src.dispatchEvent(new DragEvent('dragstart', { + bubbles: true, composed: true, dataTransfer: dt, + })); + + tgt.dispatchEvent(new DragEvent('dragover', { + bubbles: true, composed: true, dataTransfer: dt, + clientX, clientY, cancelable: true, + })); + + tgt.dispatchEvent(new DragEvent('drop', { + bubbles: true, composed: true, dataTransfer: dt, + clientX, clientY, cancelable: true, + })); + + src.dispatchEvent(new DragEvent('dragend', { + bubbles: true, composed: true, dataTransfer: dt, + })); + }, + { srcSelector, srcNth, tgtSelector, tgtNth, position }, + ); + + // Wait for reparse after edit + await page.waitForTimeout(500); +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +test.describe('Drag-Drop — Before/After/Inside', () => { + + test.beforeEach(async ({ page }) => { + await setupStructureMode(page); + // Load the "Basics" example: "let id = \x. { x }\nlet apply = ..." + await page.getByRole('button', { name: 'Basics' }).click(); + await page.waitForTimeout(500); + }); + + test('structure blocks render for default example', async ({ page }) => { + const letDefs = await countNodes(page, 'let_def'); + expect(letDefs).toBeGreaterThanOrEqual(2); + }); + + test('grip element is visible on let_def blocks', async ({ page }) => { + const gripVisible = await page.evaluate(() => { + const ce = document.querySelector('canopy-editor'); + if (!ce?.shadowRoot) return false; + const grip = ce.shadowRoot.querySelector('.structure-let_def .structure-grip'); + if (!grip) return false; + const rect = grip.getBoundingClientRect(); + return rect.width > 0 && rect.height > 0; + }); + expect(gripVisible).toBe(true); + }); + + test('CRDT text is accessible', async ({ page }) => { + const text = await getEditorText(page); + expect(text).toContain('let'); + expect(text.length).toBeGreaterThan(10); + }); + + test('exchange (Inside) swaps two let-definitions', async ({ page }) => { + const textBefore = await getEditorText(page); + + await dragDrop(page, '.structure-let_def', 0, '.structure-let_def', 1, 'Inside'); + + const textAfter = await getEditorText(page); + expect(textAfter).not.toEqual(textBefore); + const letCount = (textAfter.match(/let /g) || []).length; + expect(letCount).toBeGreaterThanOrEqual(2); + }); + + test('Before drop produces valid syntax with placeholder', async ({ page }) => { + const textBefore = await getEditorText(page); + + await dragDrop(page, '.structure-let_def', 1, '.structure-let_def', 0, 'Before'); + + const textAfter = await getEditorText(page); + expect(textAfter).not.toEqual(textBefore); + // Critical: no "let x = " with empty RHS — placeholder should fill it + expect(textAfter).not.toMatch(/let \w+ = \s*\n/); + const letCount = (textAfter.match(/let /g) || []).length; + expect(letCount).toBeGreaterThanOrEqual(2); + }); + + test('After drop produces valid syntax with placeholder', async ({ page }) => { + const textBefore = await getEditorText(page); + + await dragDrop(page, '.structure-let_def', 0, '.structure-let_def', 1, 'After'); + + const textAfter = await getEditorText(page); + expect(textAfter).not.toEqual(textBefore); + expect(textAfter).not.toMatch(/let \w+ = \s*\n/); + const letCount = (textAfter.match(/let /g) || []).length; + expect(letCount).toBeGreaterThanOrEqual(2); + }); + + test('self-drop is rejected (no change)', async ({ page }) => { + const textBefore = await getEditorText(page); + + await dragDrop(page, '.structure-let_def', 0, '.structure-let_def', 0, 'Inside'); + + const textAfter = await getEditorText(page); + expect(textAfter).toEqual(textBefore); + }); +}); diff --git a/lang/lambda/companion/lambda_editor.mbt b/lang/lambda/companion/lambda_editor.mbt index 978124b9..ed587150 100644 --- a/lang/lambda/companion/lambda_editor.mbt +++ b/lang/lambda/companion/lambda_editor.mbt @@ -6,6 +6,7 @@ /// Holds lambda-specific memos alongside a generic SyncEditor. /// The FFI layer stores this alongside the editor. pub struct LambdaCompanion { + priv rt : @incr.Runtime priv proj_memo : @incr.Memo[@lambda_flat.VersionedFlatProj] // Escalation memo merges Tier 1 (direct) + Tier 2 (egglog) results. // Falls back to Tier 1 results when no definitions are Suppressed. @@ -16,7 +17,7 @@ pub struct LambdaCompanion { pub fn LambdaCompanion::get_flat_proj( self : LambdaCompanion, ) -> @lambda_proj.FlatProj? { - self.proj_memo.get().proj + self.rt.read(self.proj_memo).proj } ///| @@ -24,7 +25,7 @@ pub fn LambdaCompanion::get_eval_results( self : LambdaCompanion, ) -> Array[@lambda_eval.EvalResult] { // Return escalated results (Tier 1 + Tier 2 merge) - self.escalation_memo.get() + self.rt.read(self.escalation_memo) } ///| @@ -90,8 +91,9 @@ pub fn new_lambda_editor( agent_id : String, capture_timeout_ms? : Int = 500, ) -> (@editor.SyncEditor[@ast.Term], LambdaCompanion) { - // Ref side-channels to capture memos from build_memos callback. + // Ref side-channels to capture memos and runtime from build_memos callback. // Safe: new_generic calls build_memos during construction, before returning. + let rt_ref : Ref[@incr.Runtime?] = Ref::new(None) let proj_memo_ref : Ref[@incr.Memo[@lambda_flat.VersionedFlatProj]?] = Ref::new( None, ) @@ -108,6 +110,7 @@ pub fn new_lambda_editor( agent_id, fn(s) { @loom.new_imperative_parser(s, @parser.lambda_grammar) }, fn(rt, source_text, syntax_tree, parser) { + rt_ref.val = Some(rt) let (proj_memo, cached_proj_node, registry_memo, source_map_memo) = @lambda_flat.build_lambda_projection_memos( rt, source_text, syntax_tree, parser, ) @@ -124,10 +127,11 @@ pub fn new_lambda_editor( }, capture_timeout_ms~, capabilities=build_lambda_capabilities( - cached_proj_node_ref, escalation_memo_ref, + rt_ref, cached_proj_node_ref, escalation_memo_ref, ), ) let companion : LambdaCompanion = { + rt: rt_ref.val.unwrap(), proj_memo: proj_memo_ref.val.unwrap(), escalation_memo: escalation_memo_ref.val.unwrap(), } @@ -137,24 +141,33 @@ pub fn new_lambda_editor( ///| /// Build capabilities using the escalation memo (Tier 1 + Tier 2 merged results). fn build_lambda_capabilities( + rt_ref : Ref[@incr.Runtime?], cached_proj_node_ref : Ref[@incr.Memo[@core.ProjNode[@ast.Term]?]?], escalation_memo_ref : Ref[@incr.Memo[Array[@lambda_eval.EvalResult]]?], ) -> @editor.LanguageCapabilities[@ast.Term] { @editor.LanguageCapabilities::new( get_annotations=Some(fn() { + let rt = match rt_ref.val { + Some(rt) => rt + None => return {} + } let eval_results = match escalation_memo_ref.val { - Some(memo) => memo.get() + Some(memo) => rt.read(memo) None => return {} } let proj_node = match cached_proj_node_ref.val { - Some(memo) => memo.get() + Some(memo) => rt.read(memo) None => None } build_eval_annotations(eval_results, proj_node) }), pretty_post_process=Some(fn(layout) { + let rt = match rt_ref.val { + Some(rt) => rt + None => return layout + } let eval_results = match escalation_memo_ref.val { - Some(memo) => memo.get() + Some(memo) => rt.read(memo) None => return layout } @lambda_eval.inject_eval_annotations(layout, eval_results) From 90502ae1487e0731ae875e82f56e3bcc78c09e12 Mon Sep 17 00:00:00 2001 From: Koji Ishimoto Date: Wed, 15 Apr 2026 19:01:04 +0900 Subject: [PATCH 3/3] docs(TODO): mark semantic-aware drop as done (PR #179) Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/TODO.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/TODO.md b/docs/TODO.md index 10173541..15101695 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -328,9 +328,8 @@ From SuperOOP analysis and handler chain refactor (PR #54): Why: v1 hardcodes `"After"`. Users need to control where the node lands. Exit: mouse Y within target bounding box determines Before/After/Inside; visual indicator shows landing position. -- [ ] **Semantic-aware drop** - Why: text-based splice creates parse errors (e.g. `let id =` with empty RHS after moving the lambda out). Drop should operate at AST level. - Exit: dropping a node produces a syntactically valid result or rejects with an error. +- [x] **Semantic-aware drop** (PR #179) + Done: Before/After replaces source with placeholder via `Renderable::placeholder()`, inserts source text from source map. Exchange unchanged. Playwright E2E tests cover all positions. - [x] **Unit tests for drag-drop JSON parser and legality** Why: no test coverage for `StartDrag`/`DragOver`/`Drop` parsing or self-drop/descendant-drop guards.