Skip to content

Add editable flowchart sections - #47

Merged
donovan-yohan merged 1 commit into
mainfrom
codex/edit-subgraphs
Aug 10, 2026
Merged

donovan-yohan merged 1 commit into
mainfrom
codex/edit-subgraphs

Conversation

@donovan-yohan

@donovan-yohan donovan-yohan commented Aug 10, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • render Mermaid flowchart subgraphs as selectable section bounds and headers
  • allow explicit-id section labels to be renamed with a narrow declaration rewrite that preserves nested source byte-for-byte
  • drag a section by moving its recursive member nodes through the existing durable per-node layout path
  • support keyboard selection/rename, touch group dragging, history-preview Fit, and normal Space/middle camera panning
  • add a dedicated section browser gate to CI

Ownership and compatibility

  • no section position metadata is introduced; section geometry is derived from current member-node positions
  • local section selection/editor state remains browser-local
  • only unique explicit subgraph id[title] declarations expose Rename; implicit-title sections remain selectable and draggable without a doomed mutation action
  • Mermaid AST parse validates structure, but no AST render/round-trip is used, preserving nested sections and authored source

Exact-head evidence

  • shared build passed
  • root lint and typecheck passed
  • root tests: shared 7/7, server 78/78, web 185/185
  • root production build passed
  • test:e2e-workspace-ux: 27 passed, 0 skipped
  • test:e2e-subgraphs: passed, including nested mouse/touch group drag, durable A/B/C-only layout, reload Fit, and immutable read-only history Fit
  • e2e-validate.ts: all checks passed
  • test:e2e-sequence: passed
  • visual inspection: nested section selection/rename and centered moved revision artifact
  • git diff --check passed

Review closure

Adversarial review found and closed stale Fit bounds, keyboard selection, unsupported Rename exposure, touch/camera gesture conflicts, and read-only history Fit. Rebase integration additionally preserved PR #46 presence, undo/redo, cursor, pan, and control behavior. Focused re-reviews found no remaining P0/P1/P2 issues. The final hit-target helper change only makes rect/center/elementFromPoint sampling atomic; it retains the same real-obstruction assertion.

Summary by CodeRabbit

  • New Features

    • Added interactive Mermaid subgraphs with selectable boundaries, grouped dragging, and nested subgraph support.
    • Added the ability to rename eligible subgraph labels directly in the diagram.
    • Preserved nested structure, node content, and diagram source when editing labels.
  • Bug Fixes

    • Hidden stale diagram geometry to prevent visual artifacts after layout updates.
  • Tests

    • Added end-to-end coverage for subgraph selection, editing, dragging, persistence, reload, and keyboard interactions.
    • Expanded automated validation for nested subgraph behavior.

@vercel

vercel Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
arielcharts Ready Ready Preview Aug 10, 2026 8:45am

@coderabbitai

coderabbitai Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@donovan-yohan, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 5 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 78f6ce23-4fc8-4717-a1a5-aaa2fef59297

📥 Commits

Reviewing files that changed from the base of the PR and between 1c030ec and 986cfb8.

📒 Files selected for processing (8)
  • apps/web/src/app/globals.css
  • apps/web/src/components/diagram-canvas.test.ts
  • apps/web/src/components/diagram-canvas.tsx
  • apps/web/src/components/workspace-settings.tsx
  • e2e-subgraph-validate.ts
  • e2e-workspace-ux-validate.ts
  • e2e/support/canvas-camera.test.ts
  • e2e/support/canvas-camera.ts
📝 Walkthrough

Walkthrough

The PR adds interactive nested Mermaid subgraphs. It supports selection, grouped dragging, explicit label renaming, live bounds, mutation persistence, history previews, and focused end-to-end validation.

Changes

Interactive Mermaid subgraph editing

Layer / File(s) Summary
Subgraph source and geometry model
apps/web/src/lib/diagram-subgraphs.ts, apps/web/src/lib/diagram-mutations.ts, apps/web/src/lib/*test.ts
Adds declaration validation and renaming, nested member resolution, interactive bounds translation, and mutation-queue support.
Canvas subgraph interactions
apps/web/src/components/diagram-canvas.tsx, apps/web/src/app/globals.css, apps/web/src/app/globals.test.ts
Adds subgraph overlays, selection, label editing, grouped dragging, live bounds, state cleanup, and hidden React Flow cluster geometry.
Workspace mutation wiring
apps/web/src/components/session-workspace.tsx, docs/context-map.md
Passes Mermaid source to DiagramCanvas and routes label edits through MutationQueue with activity tracking.
Subgraph end-to-end verification
e2e-subgraph-validate.ts, e2e/support/interactions.ts, package.json, .github/workflows/ci.yml
Adds the focused validator, browser hit-target calculation, package script, and CI execution step.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Browser
  participant DiagramCanvas
  participant MutationQueue
  participant YjsHistory
  Browser->>DiagramCanvas: Select and edit a subgraph label
  DiagramCanvas->>MutationQueue: Submit subgraph label mutation
  MutationQueue->>MutationQueue: Rewrite declaration and parse snapshot
  MutationQueue->>YjsHistory: Persist diagram mutation
  YjsHistory-->>Browser: Update source and activity history
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding editable flowchart sections.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/edit-subgraphs

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (4)
apps/web/src/components/diagram-canvas.tsx (1)

701-710: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Memoize the rename-capability check.

canRenameFlowchartSubgraphDeclaration splits and rewrites the whole Mermaid source. Line 707 calls it in the render body. While a section is selected, every pointer move and every camera update re-renders this component, so the whole source is re-scanned on each frame. Wrap the call in useMemo.

⚡ Proposed fix
-  const selectedSubgraphCanRename = Boolean(
-    selectedSubgraphId && canRenameFlowchartSubgraphDeclaration(mermaidSource, selectedSubgraphId),
-  );
+  const selectedSubgraphCanRename = useMemo(() => Boolean(
+    selectedSubgraphId && canRenameFlowchartSubgraphDeclaration(mermaidSource, selectedSubgraphId),
+  ), [mermaidSource, selectedSubgraphId]);
🤖 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 `@apps/web/src/components/diagram-canvas.tsx` around lines 701 - 710, Memoize
the selected-subgraph rename check currently computed in the render body. Wrap
the canRenameFlowchartSubgraphDeclaration call in useMemo, with mermaidSource
and selectedSubgraphId as dependencies, while preserving the existing false
result when no subgraph is selected.
apps/web/src/app/globals.test.ts (1)

46-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Make the assertion independent of selector order.

The regex requires .diagram-canvas-svg--reactflow svg g.cluster to be the first selector in the rule. A harmless reorder of the selector list breaks the test. Anchor on the selector and the declaration instead.

♻️ Proposed change
-    expect(css).toMatch(/\.diagram-canvas-svg--reactflow svg g\.cluster,[^}]*\{\s*opacity:\s*0;/u);
+    expect(css).toMatch(/\.diagram-canvas-svg--reactflow svg g\.cluster[^{}]*\{[^}]*opacity:\s*0;/u);
🤖 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 `@apps/web/src/app/globals.test.ts` around lines 46 - 50, Update the assertion
in the “editable section presentation” test to match the
`.diagram-canvas-svg--reactflow svg g.cluster` selector and its `opacity: 0`
declaration regardless of selector-list order; remove the assumption that this
selector appears first in the CSS rule.
apps/web/src/lib/diagram-subgraphs.test.ts (1)

10-32: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add a test for label encoding.

The tests cover declaration matching but not encodeQuotedMermaidLabel. Add a case with a label that contains & and ". The test then locks in the escape order, so " is not double-encoded.

💚 Proposed test
   it('rejects implicit-title sections rather than changing their identity', () => {
+    // ...
+  });
+
+  it('encodes ampersands and quotes exactly once', () => {
+    expect(renameFlowchartSubgraphDeclaration('flowchart TD\n  subgraph s[Old]\n  end\n', 's', 'A & "B"'))
+      .toBe('flowchart TD\n  subgraph s["A & "B""]\n  end\n');
+  });
🤖 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 `@apps/web/src/lib/diagram-subgraphs.test.ts` around lines 10 - 32, Add a test
in the existing flowchart subgraph source edits suite that renames an explicit
subgraph using a label containing both ampersand and double-quote characters,
then assert the generated Mermaid label encodes them correctly without
double-encoding the ampersand in entities such as ". Exercise
renameFlowchartSubgraphDeclaration and verify the complete resulting declaration
source.
apps/web/src/lib/diagram-mutations.test.ts (1)

85-97: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Assert the Yjs document content after the mutation.

The test checks result.nextText only. The durable contract is the Y.Text content written by applyDiff. Add an assertion on yText.toString(). That covers the queue apply path, not only the pure transform.

💚 Proposed addition
     expect(result.nextText).toBe(source.replace('subgraph inner[Inner]', 'subgraph inner["Renamed"]'));
+    expect(yText.toString()).toBe(result.nextText);

As per coding guidelines: "Flowchart canvas mutations must serialize through MutationQueue and use minimal Y.Text diffs".

🤖 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 `@apps/web/src/lib/diagram-mutations.test.ts` around lines 85 - 97, Extend the
test for MutationQueue.editSubgraphLabel to assert that yText.toString() matches
the expected renamed source after the mutation is applied. Keep the existing
result.nextText and snapshot assertions, ensuring the assertion verifies the
durable Y.Text content through the queue’s apply path.

Source: Coding guidelines

🤖 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 `@apps/web/src/components/diagram-canvas.tsx`:
- Around line 1689-1690: Prevent blur-triggered rename commits whenever the
subgraph editor is closed externally by setting cancelSubgraphEditRef.current
before clearing editingSubgraphId. Apply this in handleCanvasClick at
apps/web/src/components/diagram-canvas.tsx#L1689-L1690 and also reset
editingSubgraphLabel there; apply the same cancellation guard in
handleNodeActivation at `#L1702-L1703`, handleFlowSelectionChange at `#L1884-L1887`,
the ReactFlow onEdgeClick handler at `#L2275-L2276`, and onEdgeDoubleClick handler
at `#L2283-L2284`.
- Around line 1817-1831: Update handleSubgraphPointerEnd to clear
controlled-node runtime state for all dragged nodeIds after committing the final
positions, using setFlowNodeRuntime with releaseControlledNodeRuntime(current,
nodeIds).
- Around line 1757-1770: Update commitSubgraphEdit to compute the trimmed edited
label and the current subgraph label, then return without calling
onEditSubgraphLabel when they are equal. Preserve the existing fallback for
empty labels and continue clearing the editing state after actual changes.

In `@apps/web/src/lib/diagram-mutations.ts`:
- Around line 252-261: Update editSubgraphLabel to handle
parseFlowchartSnapshot(nextText) failures from
renameFlowchartSubgraphDeclaration: catch parsing errors and return a failed
MutationResult with the error, rather than enqueuing a mutation whose source
error is undefined. Preserve the existing successful snapshot behavior for valid
renamed text.

---

Nitpick comments:
In `@apps/web/src/app/globals.test.ts`:
- Around line 46-50: Update the assertion in the “editable section presentation”
test to match the `.diagram-canvas-svg--reactflow svg g.cluster` selector and
its `opacity: 0` declaration regardless of selector-list order; remove the
assumption that this selector appears first in the CSS rule.

In `@apps/web/src/components/diagram-canvas.tsx`:
- Around line 701-710: Memoize the selected-subgraph rename check currently
computed in the render body. Wrap the canRenameFlowchartSubgraphDeclaration call
in useMemo, with mermaidSource and selectedSubgraphId as dependencies, while
preserving the existing false result when no subgraph is selected.

In `@apps/web/src/lib/diagram-mutations.test.ts`:
- Around line 85-97: Extend the test for MutationQueue.editSubgraphLabel to
assert that yText.toString() matches the expected renamed source after the
mutation is applied. Keep the existing result.nextText and snapshot assertions,
ensuring the assertion verifies the durable Y.Text content through the queue’s
apply path.

In `@apps/web/src/lib/diagram-subgraphs.test.ts`:
- Around line 10-32: Add a test in the existing flowchart subgraph source edits
suite that renames an explicit subgraph using a label containing both ampersand
and double-quote characters, then assert the generated Mermaid label encodes
them correctly without double-encoding the ampersand in entities such as ".
Exercise renameFlowchartSubgraphDeclaration and verify the complete resulting
declaration source.
🪄 Autofix

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 Plus

Run ID: 37250443-45d9-4773-aa02-29db9cbfca73

📥 Commits

Reviewing files that changed from the base of the PR and between 04ddb22 and 1c030ec.

📒 Files selected for processing (13)
  • .github/workflows/ci.yml
  • apps/web/src/app/globals.css
  • apps/web/src/app/globals.test.ts
  • apps/web/src/components/diagram-canvas.tsx
  • apps/web/src/components/session-workspace.tsx
  • apps/web/src/lib/diagram-mutations.test.ts
  • apps/web/src/lib/diagram-mutations.ts
  • apps/web/src/lib/diagram-subgraphs.test.ts
  • apps/web/src/lib/diagram-subgraphs.ts
  • docs/context-map.md
  • e2e-subgraph-validate.ts
  • e2e/support/interactions.ts
  • package.json

Comment on lines +1689 to +1690
setSelectedSubgraphId(null);
setEditingSubgraphId(null);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Unintended rename commit when editingSubgraphId is cleared outside the editor. The label input commits on blur through commitSubgraphEdit. Every site that sets editingSubgraphId to null unmounts that input, the browser fires focusout, and the previous-render blur handler still closes over the old editingSubgraphId. The result is a rename write to MutationQueue.editSubgraphLabel plus an activity record that the user did not request. The Escape handler at line 2827 already guards this with cancelSubgraphEditRef.current = true; the other sites do not. Set that flag before clearing the id at each site, or extract a single closeSubgraphEditor(cancel: boolean) helper and call it everywhere.

  • apps/web/src/components/diagram-canvas.tsx#L1689-L1690: in handleCanvasClick, set cancelSubgraphEditRef.current = true before setEditingSubgraphId(null) and also reset editingSubgraphLabel.
  • apps/web/src/components/diagram-canvas.tsx#L1702-L1703: in handleNodeActivation, apply the same guard before clearing the editing id.
  • apps/web/src/components/diagram-canvas.tsx#L1884-L1887: in handleFlowSelectionChange, apply the same guard before clearing the editing id.
  • apps/web/src/components/diagram-canvas.tsx#L2275-L2276: in the ReactFlow onEdgeClick handler, apply the same guard before clearing the editing id.
  • apps/web/src/components/diagram-canvas.tsx#L2283-L2284: in the ReactFlow onEdgeDoubleClick handler, apply the same guard before clearing the editing id.
📍 Affects 1 file
  • apps/web/src/components/diagram-canvas.tsx#L1689-L1690 (this comment)
  • apps/web/src/components/diagram-canvas.tsx#L1702-L1703
  • apps/web/src/components/diagram-canvas.tsx#L1884-L1887
  • apps/web/src/components/diagram-canvas.tsx#L2275-L2276
  • apps/web/src/components/diagram-canvas.tsx#L2283-L2284
🤖 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 `@apps/web/src/components/diagram-canvas.tsx` around lines 1689 - 1690, Prevent
blur-triggered rename commits whenever the subgraph editor is closed externally
by setting cancelSubgraphEditRef.current before clearing editingSubgraphId.
Apply this in handleCanvasClick at
apps/web/src/components/diagram-canvas.tsx#L1689-L1690 and also reset
editingSubgraphLabel there; apply the same cancellation guard in
handleNodeActivation at `#L1702-L1703`, handleFlowSelectionChange at `#L1884-L1887`,
the ReactFlow onEdgeClick handler at `#L2275-L2276`, and onEdgeDoubleClick handler
at `#L2283-L2284`.

Comment on lines +1757 to +1770
const commitSubgraphEdit = useCallback(() => {
if (cancelSubgraphEditRef.current) {
cancelSubgraphEditRef.current = false;
return;
}
if (!canEditStructure || !editingSubgraphId) return;
const current = subgraphById.get(editingSubgraphId);
onEditSubgraphLabel?.(
editingSubgraphId,
editingSubgraphLabel.trim() || (current ? getSubgraphLabel(current) : editingSubgraphId),
);
setEditingSubgraphId(null);
setEditingSubgraphLabel('');
}, [canEditStructure, editingSubgraphId, editingSubgraphLabel, onEditSubgraphLabel, subgraphById]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Skip the mutation when the label did not change.

commitSubgraphEdit always calls onEditSubgraphLabel. If the user opens the editor and clicks away without typing, the rename still runs. renameFlowchartSubgraphDeclaration rewrites subgraph inner[Inner] to subgraph inner["Inner"], so the source text changes, a Yjs write occurs, an undo entry is pushed, and an activity record "Renamed a diagram section" appears. Compare the trimmed value with the current label first.

🐛 Proposed fix
     if (!canEditStructure || !editingSubgraphId) return;
     const current = subgraphById.get(editingSubgraphId);
-    onEditSubgraphLabel?.(
-      editingSubgraphId,
-      editingSubgraphLabel.trim() || (current ? getSubgraphLabel(current) : editingSubgraphId),
-    );
+    const currentLabel = current ? getSubgraphLabel(current) : editingSubgraphId;
+    const nextLabel = editingSubgraphLabel.trim() || currentLabel;
+    if (nextLabel !== currentLabel) {
+      onEditSubgraphLabel?.(editingSubgraphId, nextLabel);
+    }
     setEditingSubgraphId(null);
     setEditingSubgraphLabel('');
📝 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
const commitSubgraphEdit = useCallback(() => {
if (cancelSubgraphEditRef.current) {
cancelSubgraphEditRef.current = false;
return;
}
if (!canEditStructure || !editingSubgraphId) return;
const current = subgraphById.get(editingSubgraphId);
onEditSubgraphLabel?.(
editingSubgraphId,
editingSubgraphLabel.trim() || (current ? getSubgraphLabel(current) : editingSubgraphId),
);
setEditingSubgraphId(null);
setEditingSubgraphLabel('');
}, [canEditStructure, editingSubgraphId, editingSubgraphLabel, onEditSubgraphLabel, subgraphById]);
const commitSubgraphEdit = useCallback(() => {
if (cancelSubgraphEditRef.current) {
cancelSubgraphEditRef.current = false;
return;
}
if (!canEditStructure || !editingSubgraphId) return;
const current = subgraphById.get(editingSubgraphId);
const currentLabel = current ? getSubgraphLabel(current) : editingSubgraphId;
const nextLabel = editingSubgraphLabel.trim() || currentLabel;
if (nextLabel !== currentLabel) {
onEditSubgraphLabel?.(editingSubgraphId, nextLabel);
}
setEditingSubgraphId(null);
setEditingSubgraphLabel('');
}, [canEditStructure, editingSubgraphId, editingSubgraphLabel, onEditSubgraphLabel, subgraphById]);
🤖 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 `@apps/web/src/components/diagram-canvas.tsx` around lines 1757 - 1770, Update
commitSubgraphEdit to compute the trimmed edited label and the current subgraph
label, then return without calling onEditSubgraphLabel when they are equal.
Preserve the existing fallback for empty labels and continue clearing the
editing state after actual changes.

Comment on lines +1817 to +1831
const handleSubgraphPointerEnd = useCallback((event: ReactPointerEvent<HTMLElement>) => {
const drag = subgraphDragRef.current;
if (!drag || drag.pointerId !== event.pointerId) return;
subgraphDragRef.current = null;
if (!drag.begun) return;
const nodeIds = Object.keys(drag.latestPositions);
nodeIds.forEach((nodeId) => activeDragNodeIdsRef.current.delete(nodeId));
if (onNodeDragStop) onNodeDragStop(drag.latestPositions);
else setNodePositions((current) => ({ ...current, ...drag.latestPositions }), 'merge', drag.latestPositions);
setLiveNodePositions((current) => {
const next = { ...current };
nodeIds.forEach((nodeId) => delete next[nodeId]);
return next;
});
}, [onNodeDragStop, setNodePositions]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect the controlled-node runtime contract and its release path.
rg -nP -C6 'releaseControlledNodeRuntime|applyControlledNodeChanges|reconcileControlledNodeRuntime' --type=ts --type=tsx apps/web/src 2>/dev/null \
  || rg -nP -C6 'releaseControlledNodeRuntime|applyControlledNodeChanges|reconcileControlledNodeRuntime' -g '*.ts' -g '*.tsx' apps/web/src

Repository: donovan-yohan/arielcharts

Length of output: 21774


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== diagram-canvas relevant sections =="
sed -n '1140,1190p' apps/web/src/components/diagram-canvas.tsx | cat -n -v
sed -n '1760,1970p' apps/web/src/components/diagram-canvas.tsx | cat -n -v
sed -n '2280,2355p' apps/web/src/components/diagram-canvas.tsx | cat -n -v

echo "== adapter release/reconcile =="
sed -n '1,210p' apps/web/src/lib/reactflow-controlled-node-adapter.ts | cat -n -v

Repository: donovan-yohan/arielcharts

Length of output: 24451


Release subgraph drag runtime on normal stop.

handleSubgraphPointerEnd already commits the position flush, but it does not clear the controlled-node runtime for the dragged ids. If any subgraph member experienced positional React Flow changes during the drag, those runtime positions can remain active. Release them with setFlowNodeRuntime((current) => releaseControlledNodeRuntime(current, nodeIds)).

🤖 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 `@apps/web/src/components/diagram-canvas.tsx` around lines 1817 - 1831, Update
handleSubgraphPointerEnd to clear controlled-node runtime state for all dragged
nodeIds after committing the final positions, using setFlowNodeRuntime with
releaseControlledNodeRuntime(current, nodeIds).

Source: Coding guidelines

Comment on lines +252 to +261
async editSubgraphLabel(subgraphId: string, newLabel: string): Promise<MutationResult> {
return this.enqueueResult((currentText) => {
const nextText = renameFlowchartSubgraphDeclaration(currentText, subgraphId, newLabel);
return {
nextText,
previousText: currentText,
snapshot: parseFlowchartSnapshot(nextText),
};
});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Locate parseFlowchartSnapshot definition and all call sites.
fd -t f 'diagram-mutations.ts' apps/web/src/lib --exec ast-grep outline {} --items all
rg -nP -C4 '\bparseFlowchartSnapshot\b' --type=ts apps/web/src

Repository: donovan-yohan/arielcharts

Length of output: 13443


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the mutation enqueue/error path and the exact parseFlowchartSnapshot implementation.
sed -n '220,270p' apps/web/src/lib/diagram-mutations.ts
printf '\n--- parseFlowchartSnapshot implementation ---\n'
sed -n '120,136p' apps/web/src/lib/diagram-mutations.ts

# Probe the deterministic behavior of Flowchart.parse on edge inputs and inspect the relevant mutation result handling.
rg -n -C3 'function isSameDiagramEdge|async flush|reject\(this.onAfterApplyError|runVisualSourceMutation|editSubgraphLabel\(subgraphId|snapshot' apps/web/src --type=ts --type=tsx

Repository: donovan-yohan/arielcharts

Length of output: 2535


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect flowchart mutation helpers and the mutation error handling around edits/snapshots.
rg -n -C3 'function isSameDiagramEdge|async flush|onAfterApplyError|editSubgraphLabel|runVisualSourceMutation|snapshot' apps/web/src --glob '*.ts' --glob '*.tsx'
printf '\n--- relevant flowchart mutation methods ---\n'
sed -n '520,575p' apps/web/src/lib/diagram-mutations.ts
printf '\n--- relevant error handling ---\n'
sed -n '185,222p' apps/web/src/lib/diagram-mutations.ts

Repository: donovan-yohan/arielcharts

Length of output: 30703


Avoid using parseFlowchartSnapshot(nextText) for non-empty but unparsable subgraph label edits.

parseFlowchartSnapshot delegates to Flowchart.parse(nextText) and can throw. In this edit path, an unparsable nextText means the rename produced invalid source, so the mutation should report that failure instead of leaving a mutation with source error undefined.

🤖 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 `@apps/web/src/lib/diagram-mutations.ts` around lines 252 - 261, Update
editSubgraphLabel to handle parseFlowchartSnapshot(nextText) failures from
renameFlowchartSubgraphDeclaration: catch parsing errors and return a failed
MutationResult with the error, rather than enqueuing a mutation whose source
error is undefined. Preserve the existing successful snapshot behavior for valid
renamed text.

This branch was successfully deployed

1 active deployment
Preview — 986cfb88 Deployed Aug 10, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant