feat(#66): "Connect to…" cross-scope connection gesture#73
Merged
Conversation
Closes #66. The search-first gesture that resolves the original need — wiring a Component to one in another subtree without both endpoints on screen. - Two slug-readable reads, deliberately distinct from scope-keyed getCanvas (ADR-0032): listProjectComponents (project-wide flat search source) and listNodeConnections (a Component's COMPLETE incident Connections across scopes, including lineal ones that collapse off its home Canvas). - ConnectToPalette: a cmdk surface modeled on the kind palette, with client-built ancestor-path labels for disambiguation and self/already-connected exclusion. - Connections section in the Component-detail panel: list for everyone (viewer read-only), owner adds via the project-wide search (affordance omitted, not disabled, for viewers). - Optimistic cross-scope connect (commitConnect): inserts the far-end boundary proxy this frame and reconciles temp→real ids on success. The on-scope render follows the SAME rep(N,S) partition getCanvas derives (ADR-0031), computed client-side from the already-loaded listProjectComponents map — correct for the same-Canvas, altitude, off-scope-proxy, and lineal-collapse cases live (the RF store is seeded once and not re-seeded by a refetch, so reconcile is manual). - New Connections default to ASSOCIATION; interaction is set afterward via the #65 picker. Docs travel with the slice: CONTEXT.md (Connection + Component-detail panel tense flips, new Connections section term) and ADR-0032. Verified: pnpm check green; 12 new service tests (+ full node/edge suites, 140 passing) against real Postgres; dev-browser end-to-end — deep A→deep B across subtrees renders far-end proxy + list row on A's canvas, reciprocal proxy on B's, altitude edge on the common ancestor, persists across reload, and the viewer sees a read-only Connections list with no add affordance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughPR implements the cross-scope "Connect to…" gesture: a project-wide component search lets users wire a component to any other at any scope with optimistic boundary-proxy insertion and reconciliation. The component detail panel gains a Connections section listing incident edges (viewer read-only, owner editable via search). Canvas orchestration handles multi-store optimistic updates, temp-id reconciliation, and conflict rollback. ChangesCross-Scope Connection Gesture
Sequence Diagram(s)sequenceDiagram
participant User as User (Browser)
participant Panel as ComponentDetailPanel
participant Search as ConnectToPalette/Popover
participant Canvas as Canvas.commitConnect
participant Server as Server (commitConnect)
participant Stores as React Flow + Panel Cache
User->>Panel: Click "Connect to…"
Panel->>Search: Open popover (lazy listProjectComponents)
Search->>Server: listProjectComponents(slug)
Server-->>Search: Component list (byId,parentId)
User->>Search: Select target
Search-->>Panel: ConnectTarget{id,title,kind}
Panel->>Canvas: onConnect(sourceNodeId, target)
Canvas->>Canvas: repOnScope(targetId, scopeId)
Canvas->>Stores: Optimistic insert (panel list, edge, proxy)
Canvas->>Server: commitConnect {source, targetRep,...}
alt Success
Server-->>Canvas: real ids (edgeId, proxyId)
Canvas->>Stores: Reconcile temp -> real
else Conflict
Server-->>Canvas: CONFLICT
Canvas->>Stores: Rollback optimistic inserts
Canvas->>Panel: Show conflict toast
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/server/architecture/edge.service.ts (1)
205-245: ⚡ Quick winConsider filtering soft-deleted endpoints at the query level for better performance.
The current implementation loads soft-deleted endpoint nodes from the database and then filters them in application code (line 235). This approach is correct but less efficient than filtering at the query level, and it's inconsistent with the pattern used in
getCanvas(lines 373-374 filter at the SQL level withAND sn."deletedAt" IS NULL).♻️ More efficient query-level filtering
You can use Prisma relation filters within the OR clause to exclude edges with soft-deleted endpoints before they're loaded:
const edges = await db.edge.findMany({ where: { projectId: project.id, deletedAt: null, - OR: [{ sourceId: nodeId }, { targetId: nodeId }], + OR: [ + { sourceId: nodeId, target: { deletedAt: null } }, + { targetId: nodeId, source: { deletedAt: null } }, + ], }, select: { id: true, sourceId: true, interaction: true, label: true, source: { - select: { id: true, title: true, kind: true, deletedAt: true }, + select: { id: true, title: true, kind: true }, }, target: { - select: { id: true, title: true, kind: true, deletedAt: true }, + select: { id: true, title: true, kind: true }, }, }, orderBy: { createdAt: "asc" }, }); const connections: NodeConnection[] = []; for (const edge of edges) { const sourceIsSelf = edge.sourceId === nodeId; const far = sourceIsSelf ? edge.target : edge.source; - // A soft-deleted far endpoint hides the Connection — the same posture - // `getCanvas` takes when an endpoint is dead. - if (far.deletedAt !== null) continue; connections.push({ id: edge.id, interaction: edge.interaction, label: edge.label, sourceIsSelf, other: { id: far.id, title: far.title, kind: far.kind }, }); } return connections;🤖 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 `@src/server/architecture/edge.service.ts` around lines 205 - 245, The edges query loads soft-deleted endpoint nodes and then filters them in code (checking far.deletedAt), which is less efficient; update the db.edge.findMany where clause to filter out edges whose endpoint rows are soft-deleted by adding relation filters to each OR branch (e.g. replace OR: [{ sourceId: nodeId }, { targetId: nodeId }] with OR: [{ sourceId: nodeId, source: { deletedAt: null } }, { targetId: nodeId, target: { deletedAt: null } }]) and remove the runtime far.deletedAt check so only non-deleted endpoints are returned by db.edge.findMany.src/app/p/[slug]/_canvas/canvas.tsx (1)
40-40: ⚡ Quick winUse the
~/*alias for the newConnectTargetimport.This new relative import extends the local
src/*path style the repo is trying to avoid. As per coding guidelines "Use the path alias~/*to referencesrc/*throughout the codebase".🤖 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 `@src/app/p/`[slug]/_canvas/canvas.tsx at line 40, The import for ConnectTarget in canvas.tsx uses a relative path; update the import to use the project path alias by replacing the relative "./connect-to-palette" import with the aliased path "~/<path-to-connect-to-palette>" so that the ConnectTarget type is imported via the ~/ alias (ensure the symbol ConnectTarget remains unchanged in the import statement).src/app/p/[slug]/_canvas/connect-to-palette.tsx (1)
56-78: Whole-project list is rendered up front into the DOM.
ConnectToPalettemounts aCommandItemfor every project Component (cmdk renders all rows and hides non-matches via filtering). Unlike the bounded kind palette this models on, this list is project-wide and can grow without limit, so very large projects could create thousands of DOM nodes on popover open. Consider virtualizing the list (e.g. cmdk + a windowing layer) if projects are expected to scale.🤖 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 `@src/app/p/`[slug]/_canvas/connect-to-palette.tsx around lines 56 - 78, The Command list currently renders every project Component up front (see ConnectToPalette mapping candidates -> ConnectItem inside CommandList/CommandGroup, using ancestorPath and onSelect), which will create thousands of DOM nodes for large projects; change this to a windowed/virtualized list so only visible items are mounted — for example integrate a virtualization library (react-window, react-virtual, or similar) to drive the CommandList/CommandGroup rendering and render only the slice of candidates visible in the viewport, mapping those items to ConnectItem (preserving key=component.id and passing ancestorPath/component/onSelect) and keep CommandEmpty and headings outside/above the virtualized viewport as appropriate. Ensure keyboard search/filtering still works by virtualizing the filtered candidates array rather than the entire project list.
🤖 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 `@src/app/p/`[slug]/_canvas/canvas.tsx:
- Around line 853-856: The code currently computes targetRep from the cached
listProjectComponents (components, byId, repOnScope) after connectNodes
succeeds, which can keep an optimistic/incorrect on-scope representative if the
canvas changed; update the success/reconciliation path to re-read authoritative
canvas data instead of relying on listProjectComponents cache: call the
getCanvas query (or have the connectNodes mutation return the resolved
representative/proxy data) and derive targetRep from that fresh getCanvas result
before applying final reconcilation logic (replace usages of
utils.architecture.listProjectComponents.getData, components, byId and
repOnScope in this success path with data from getCanvas or the
mutation-returned resolved representative).
In `@src/app/p/`[slug]/_canvas/component-detail-panel.tsx:
- Around line 326-336: The UI currently treats a failed listNodeConnections
query the same as an empty result because it only checks isLoading and
connections; update the render logic in component-detail-panel to explicitly
handle the query error by checking isError (from the listNodeConnections
hook/query) before the empty-state branch and render an error message (include
the error.message if available) so failures are distinguishable from "No
connections yet."; locate the block using isLoading, connections, and
ConnectionRow and insert the isError branch there.
In `@src/app/p/`[slug]/_canvas/connect-to-palette.tsx:
- Around line 200-213: The UI currently treats missing components the same as
loading so a failed listProjectComponents query (isError=true) will still show
"Loading components…" forever; update the render to check isError and surface an
error state (or pass an empty array) instead of only checking isLoading ||
!components. Specifically, change the conditional around isLoading/components
to: if (isLoading) show the loading box; else if (isError) show an error box
that displays the query error and a retry action that calls the query's refetch;
otherwise render <ConnectToPalette components={components ?? []}
excludeIds={excludeIds} onSelect={...}> so ConnectToPalette receives a safe
array when data is missing. Use the existing query symbols
(listProjectComponents, isLoading, isError, error, refetch, components) and the
ConnectToPalette props to implement this.
---
Nitpick comments:
In `@src/app/p/`[slug]/_canvas/canvas.tsx:
- Line 40: The import for ConnectTarget in canvas.tsx uses a relative path;
update the import to use the project path alias by replacing the relative
"./connect-to-palette" import with the aliased path
"~/<path-to-connect-to-palette>" so that the ConnectTarget type is imported via
the ~/ alias (ensure the symbol ConnectTarget remains unchanged in the import
statement).
In `@src/app/p/`[slug]/_canvas/connect-to-palette.tsx:
- Around line 56-78: The Command list currently renders every project Component
up front (see ConnectToPalette mapping candidates -> ConnectItem inside
CommandList/CommandGroup, using ancestorPath and onSelect), which will create
thousands of DOM nodes for large projects; change this to a windowed/virtualized
list so only visible items are mounted — for example integrate a virtualization
library (react-window, react-virtual, or similar) to drive the
CommandList/CommandGroup rendering and render only the slice of candidates
visible in the viewport, mapping those items to ConnectItem (preserving
key=component.id and passing ancestorPath/component/onSelect) and keep
CommandEmpty and headings outside/above the virtualized viewport as appropriate.
Ensure keyboard search/filtering still works by virtualizing the filtered
candidates array rather than the entire project list.
In `@src/server/architecture/edge.service.ts`:
- Around line 205-245: The edges query loads soft-deleted endpoint nodes and
then filters them in code (checking far.deletedAt), which is less efficient;
update the db.edge.findMany where clause to filter out edges whose endpoint rows
are soft-deleted by adding relation filters to each OR branch (e.g. replace OR:
[{ sourceId: nodeId }, { targetId: nodeId }] with OR: [{ sourceId: nodeId,
source: { deletedAt: null } }, { targetId: nodeId, target: { deletedAt: null }
}]) and remove the runtime far.deletedAt check so only non-deleted endpoints are
returned by db.edge.findMany.
🪄 Autofix (Beta)
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
Run ID: bf8e48cf-52b1-4b49-8ec5-7ef0dee62af0
📒 Files selected for processing (12)
CONTEXT.mddocs/adr/0032-connect-to-gesture-node-keyed-reads-and-client-derived-optimism.mdsrc/app/p/[slug]/_canvas/canvas.tsxsrc/app/p/[slug]/_canvas/component-detail-panel.tsxsrc/app/p/[slug]/_canvas/connect-to-palette.tsxsrc/lib/schemas.tssrc/lib/types.tssrc/server/api/routers/architecture.tssrc/server/architecture/__tests__/edge.service.test.tssrc/server/architecture/__tests__/node.service.test.tssrc/server/architecture/edge.service.tssrc/server/architecture/node.service.ts
Three findings from the autofix run on PR #73: - canvas.tsx commitConnect — fire a background `getCanvas.invalidate(canvasInput)` and `listProjectComponents.invalidate({slug})` after the temp→real reconcile, so a target reparented in-flight (e.g. by a concurrent MCP `move_component`) self- heals on the next remount. The live `rep(N,S)` derivation stays for the common case; the invalidate is the 1% edge-case safety net. No flicker — the RF store keeps its already-correct reconciled state. (ADR-0032 amended to record this.) - component-detail-panel.tsx ConnectionsSection — destructure `isError` and add a branch so a failed `listNodeConnections` query renders "Couldn’t load connections." instead of falling through to "No connections yet." (TanStack v5 leaves `data` undefined on first-fetch error). - connect-to-palette.tsx ConnectToPopover — destructure `isError` and add a branch so a failed `listProjectComponents` query renders "Couldn’t load components." instead of spinning "Loading components…" forever after retries exhaust. pnpm check green. No service-layer change; existing tests cover the queries. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
Fixes Applied SuccessfullyAddressed 3 CodeRabbit feedback items across 4 files (including ADR-0032). Files modified:
Commit: The latest autofix changes are on the |
This was referenced Jun 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #66. The search-first gesture that resolves the original need — wiring a Component to one in another subtree without ever having both endpoints on screen — plus a Connections section in the Component-detail panel. Builds on #65's cross-scope rendering.
What changed
Two slug-readable reads (ADR-0032), deliberately distinct from scope-keyed
getCanvas.listProjectComponents— every live Component flat ({id, title, kind, parentId}); the project-wide search source. The flatparentIdlets the client rebuild ancestor paths for disambiguation with no server walk.listNodeConnections— a Component's complete incident Connections across scopes, far end resolved to its display fields. Node-keyed, so it includes lineal Connections to a Component's own descendants that collapse off its home Canvas and never appear ingetCanvas. Both single round trip; neither folded intogetCanvas's CTE.ConnectToPalette— acmdksurface modeled on the kind palette: search every Component at any depth by title/kind, muted ancestor-path label per row for disambiguation, excludes self + already-connected targets. Project-wide read is lazy (fires only when the popover opens).Connections section in the Component-detail panel — list for everyone (viewer read-only), owner adds via the search. The "+ Add connection" affordance is omitted, not disabled for viewers (ADR-0002).
Optimistic cross-scope connect (
commitConnect) — inserts the far-end boundary proxy this frame and reconcilestemp→realids on success. Where the edge lands on the current scope follows the samerep(N,S)partitiongetCanvasderives (ADR-0031), computed client-side from the already-loadedlistProjectComponentsmap — correct for the same-Canvas, altitude, off-scope-proxy, and lineal-collapse cases live. (The RF store is seeded once and not re-seeded by a refetch, so thetemp→realreconcile is manual, exactly like the same-Canvas drag path.) New Connections default toASSOCIATION; the interaction is set afterward via the #65 picker.Scope boundary
connectNodesstays the sole Edge writer (ADR-0028) — the gesture is a thin caller, no new write path or de-dupe logic.~/lib/connection-rules.ts.Docs (travel with the slice)
CONTEXT.md tense flips (Connection + Component-detail panel) and a new Connections section term; ADR-0032 records the node/project-keyed reads, the complete-across-scopes list, and the client-derived optimistic insert (refining the plan's invalidate-to-reconcile sketch once the RF-store-not-reseeded constraint surfaced).
Verification
pnpm checkgreen.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests