feat: retire Flow model; typed cross-scope Connections (#62)#68
Merged
Conversation
Re-found the connection model: a Connection is now a directed, typed Edge that may link any two Components at any scope. Retires the Flow capability model. Model - Drop Flow / FlowRoute; rename the 1:1 spec row FlowSpec -> Spec and point it at derived child Components via Node.sourceSpecId + Node.specKey. - FlowInteraction -> Interaction enum, gaining ASSOCIATION (the default). - Edge drops canvasNodeId (scope is derived from endpoint ancestry, #63) and gains interaction. De-dupe is two partial unique indexes keyed on projectId: directional (projectId, source, target, interaction) and ASSOCIATION-only unordered (projectId, LEAST, GREATEST). One hand-edited migration with the Postgres enum-swap ordering and a guarded dedup pre-flight. Service - connectNodes accepts cross-scope and lineal (ingress) endpoints; rejects only the self-link; interaction is an input; dedup findFirst branches on interaction. - moveNode drops the orphan-reject (keeps the cycle-reject). - deleteNode/restoreNode drop the Flow arms and gain a Spec sweep; deleteEdge reverts to a lone soft-delete. Delete flow.service / flow-route.service / flow-parser and the routeFlow cross-scope writer. - getCanvas reduced to { interiorNodes, interiorEdges, breadcrumbs } via a single both-endpoints relation filter (no waterfall); cross-scope rendering is #63. - prisma-errors: matcher accepts both new index names; drop dead Flow matchers. - export.service / markdown: minimal seam for the dropped canvasNodeId (subtree + boundary CTEs become endpoint-membership; golden re-baselined). #67 owns the full typed-export rewrite. MCP / API - connect_components + apply_graph drop canvasNode, gain interaction, accept cross-scope endpoints. Remove the 9 flow/flowSpec/flowRoute tRPC procedures. Client - Remove dead Flow UI (boundary-group, boundary-proxy, route-flow popover, flow palette, attach-spec, flow pill, flow: handle branch, flow-direction, flow-interaction-display, spec-kinds). Connections render as plain lines; typed arrowheads are #65. Docs (travel with the slice) - Write ADR-0027 (Connection carries its own interaction), ADR-0028 (cross-scope + lineal=ingress; same-Canvas retired), ADR-0030 (cascade/undo without FlowRoutes). Amend ADR-0005/0010/0023/0024/0026. Amend/tombstone CONTEXT.md. pnpm check green; db:check clean; service tests cover cross-scope, lineal, self-link reject, dedup (both indexes), move-no-orphan, deleteEdge lone, and the Spec cascade sweep. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughRetires Flow/FlowRoute; adds Edge.interaction and Spec model; shifts edge scope to endpoint-derived cross-scope model; updates dedup to interaction-aware partial unique indexes; removes Flow parser/service/UI; simplifies Canvas/UI to interior nodes and edges; updates migrations, services, API, tests, and docs. ChangesMigration & Schema
Server: services, API, schemas
Client UI
Flow infra removal & tests
Sequence Diagram sequenceDiagram
participant Client
participant Canvas
participant ArchitectureAPI
participant EdgeService
participant DB
Client->>Canvas: user draws connection (source,target)
Canvas->>ArchitectureAPI: connectNodes(projectId, sourceId, targetId, interaction="ASSOCIATION")
ArchitectureAPI->>EdgeService: connectNodes(input)
EdgeService->>DB: findFirst(activeDuplicateWhere(...))
DB-->>EdgeService: duplicate? / none
EdgeService->>DB: create Edge with interaction
DB-->>EdgeService: created Edge
EdgeService-->>ArchitectureAPI: created Edge
ArchitectureAPI-->>Canvas: return real edge -> reconcile optimistic
Estimated code review effort 🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related issues
Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Owner
Author
|
✅ Final clean service-test run: 183/183 passed (10/10 files). The two earlier misses were confirmed transient |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/server/architecture/__tests__/markdown-export.test.ts (1)
121-126: ⚡ Quick winAdd an
inheritedboundary regression case to this fixture.This subtree setup only exercises
origin: "direct". Since the new boundary query has a separate descendant-owned path (is_direct = false), one child→external connection here would give both the pure serializer and the service test a regression case for that branch.🤖 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/__tests__/markdown-export.test.ts` around lines 121 - 126, The fixture only exercises origin: "direct" for internal connections (see the root.edges filter for ["n-api","n-auth","n-users"]); add a regression edge that models a descendant-owned/inherited boundary path by inserting an edge whose source is one of those child nodes (e.g., "n-api") and whose target is external (not in ["n-api","n-auth","n-users"]) with the boundary fields set to indicate a non-direct/inherited path (set is_direct: false or origin: "inherited" and any descendant-owned flag your model uses). This ensures both the pure serializer and service tests hit the descendant-owned (inherited) branch.src/server/architecture/__tests__/edge.service.test.ts (1)
211-229: 💤 Low valueConsider clarifying the test name.
The test creates REQUEST A→B, PUSH A→B, REQUEST B→A, and ASSOCIATION A→B (4 total edges), but the name "lets the four directional interactions and an association coexist" might suggest 4 distinct directional types plus 1 association = 5 edges. The test logic is correct; the name could be more precise (e.g., "lets directional interactions on both ordered pairs plus association coexist").
🤖 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/__tests__/edge.service.test.ts` around lines 211 - 229, Rename the test description string in the it(...) block to clearly reflect that there are directional interactions on both ordered pairs plus one association (e.g., "lets directional interactions on both ordered pairs plus association coexist") so it doesn't imply five edges; update the string literal in the test that uses seedTwoRootNodes(), draw (the helper that calls connectNodes), and the final expect on testDb.edge.count to match the clearer wording while leaving the test logic (the four connectNodes calls and the expect toBe(4)) unchanged.
🤖 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 `@prisma/schema.prisma`:
- Around line 307-323: Remove the unconditional uniqueness on Spec.ownerNodeId
in the Prisma model and enforce uniqueness only for non-deleted (live) rows:
delete the `@unique` attribute on ownerNodeId in model Spec (keeping deletedAt for
soft-delete) and add a migration that drops the existing unique constraint/index
created for ownerNodeId and replaces it with a partial/filtered unique index
that applies only when deletedAt IS NULL (i.e., CREATE UNIQUE INDEX ... ON
"Spec" ("ownerNodeId") WHERE "deletedAt" IS NULL). Ensure the migration updates
any DB-level constraint names created earlier so new Specs can be created for a
node while a tombstoned Spec row remains.
In `@src/server/architecture/export.service.ts`:
- Around line 187-199: The recursive CTE "subtree" in export.service.ts needs
the same recursion guard used elsewhere: introduce a depth column and enforce
SUBTREE_DEPTH_CAP when walking Node rows for canvasNodeId and projectId; e.g.,
seed the CTE with depth = 0 and in the recursive branch only select children
when depth < SUBTREE_DEPTH_CAP, incrementing depth on each recursion so the CTE
stops at SUBTREE_DEPTH_CAP and prevents unbounded recursion caused by cycles or
bad data.
---
Nitpick comments:
In `@src/server/architecture/__tests__/edge.service.test.ts`:
- Around line 211-229: Rename the test description string in the it(...) block
to clearly reflect that there are directional interactions on both ordered pairs
plus one association (e.g., "lets directional interactions on both ordered pairs
plus association coexist") so it doesn't imply five edges; update the string
literal in the test that uses seedTwoRootNodes(), draw (the helper that calls
connectNodes), and the final expect on testDb.edge.count to match the clearer
wording while leaving the test logic (the four connectNodes calls and the expect
toBe(4)) unchanged.
In `@src/server/architecture/__tests__/markdown-export.test.ts`:
- Around line 121-126: The fixture only exercises origin: "direct" for internal
connections (see the root.edges filter for ["n-api","n-auth","n-users"]); add a
regression edge that models a descendant-owned/inherited boundary path by
inserting an edge whose source is one of those child nodes (e.g., "n-api") and
whose target is external (not in ["n-api","n-auth","n-users"]) with the boundary
fields set to indicate a non-direct/inherited path (set is_direct: false or
origin: "inherited" and any descendant-owned flag your model uses). This ensures
both the pure serializer and service tests hit the descendant-owned (inherited)
branch.
🪄 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: 3d4b159d-1869-41f2-8be4-682ee86b7171
📒 Files selected for processing (49)
CONTEXT.mddocs/adr/0005-edge-scope-and-service-enforced-invariants.mddocs/adr/0010-edge-dedup-partial-unique-index.mddocs/adr/0023-connection-direction-derived-from-flows.mddocs/adr/0027-connection-carries-its-own-interaction.mddocs/adr/0028-cross-scope-connections-lineal-ingress.mddocs/adr/0030-cascade-undo-without-flowroutes.mdprisma/migrations/20260601120000_retire_flow_model/migration.sqlprisma/schema.prismasrc/app/p/[slug]/_canvas/boundary-group-node.tsxsrc/app/p/[slug]/_canvas/boundary-proxy-node.tsxsrc/app/p/[slug]/_canvas/canvas.tsxsrc/app/p/[slug]/_canvas/component-detail-panel.tsxsrc/app/p/[slug]/_canvas/component-node.tsxsrc/app/p/[slug]/_canvas/connection-edge.tsxsrc/app/p/[slug]/_canvas/route-flow-popover.tsxsrc/lib/flow-direction.tssrc/lib/flow-interaction-display.tssrc/lib/schemas.tssrc/lib/spec-kinds.tssrc/lib/types.tssrc/server/api/routers/architecture.tssrc/server/architecture/__tests__/apply-graph.service.test.tssrc/server/architecture/__tests__/edge.service.test.tssrc/server/architecture/__tests__/fixtures/export-project-full.mdsrc/server/architecture/__tests__/fixtures/export-subtree-full.mdsrc/server/architecture/__tests__/flow-parser.test.tssrc/server/architecture/__tests__/flow-route.service.test.tssrc/server/architecture/__tests__/flow.service.test.tssrc/server/architecture/__tests__/helpers/test-db.tssrc/server/architecture/__tests__/markdown-export.test.tssrc/server/architecture/__tests__/node.service.test.tssrc/server/architecture/apply-graph.service.tssrc/server/architecture/edge.service.tssrc/server/architecture/errors.tssrc/server/architecture/export.service.tssrc/server/architecture/flow-parser/index.tssrc/server/architecture/flow-parser/parsers/asyncapi.tssrc/server/architecture/flow-parser/parsers/graphql.tssrc/server/architecture/flow-parser/parsers/openapi.tssrc/server/architecture/flow-parser/parsers/sql-ddl.tssrc/server/architecture/flow-parser/parsers/ts-signature.tssrc/server/architecture/flow-parser/shared.tssrc/server/architecture/flow-route.service.tssrc/server/architecture/flow.service.tssrc/server/architecture/markdown.tssrc/server/architecture/node.service.tssrc/server/architecture/prisma-errors.tssrc/server/mcp/tool-catalog.ts
💤 Files with no reviewable changes (19)
- src/server/architecture/flow-route.service.ts
- src/lib/flow-direction.ts
- src/lib/flow-interaction-display.ts
- src/app/p/[slug]/_canvas/boundary-group-node.tsx
- src/server/architecture/flow-parser/parsers/openapi.ts
- src/server/architecture/tests/flow-parser.test.ts
- src/lib/spec-kinds.ts
- src/app/p/[slug]/_canvas/route-flow-popover.tsx
- src/server/architecture/flow-parser/parsers/graphql.ts
- src/server/architecture/flow-parser/index.ts
- src/app/p/[slug]/_canvas/boundary-proxy-node.tsx
- src/server/architecture/tests/flow-route.service.test.ts
- src/app/p/[slug]/_canvas/component-node.tsx
- src/server/architecture/flow-parser/parsers/ts-signature.ts
- src/server/architecture/flow.service.ts
- src/server/architecture/tests/flow.service.test.ts
- src/server/architecture/flow-parser/parsers/asyncapi.ts
- src/server/architecture/flow-parser/shared.ts
- src/server/architecture/flow-parser/parsers/sql-ddl.ts
…y unique Consolidated findings from a backend-architect review, a senior-engineer review, and CodeRabbit on PR #68. Addresses correctness, prod-safety, and stale agent/client-facing docs orphaned by the Flow retirement. Correctness - export.service: add the missing SUBTREE_DEPTH_CAP guard to the boundary CTE; its two sibling CTEs already had it, so a parent cycle (bad data or a reparent regression) could have hung the export while the others failed loud. Refresh the cap comment now that moveNode exists. - Spec ownership: drop the all-rows `ownerNodeId @unique` and replace with a live-only partial unique index `idx_spec_owner_live` (WHERE deletedAt IS NULL), mirroring how Edge de-dupe lives in partial indexes. The old constraint reserved the owner across tombstones, blocking re-attach after undo and making restoreNode's conflictingSpecIds path unreachable. The Prisma model relation becomes a list because @unique is also the arity declaration; the singular accessor `ownedSpec` was never traversed. New migration 20260601233356_spec_owner_live_partial_unique authored via pnpm db:author + hand-edited partial index (ADR-0010 pattern). Robustness / prod-safety - retire_flow migration: IF EXISTS on the DROPs as best-effort re-run defense for hand-touched prod DBs. Document that this is NOT full idempotency (FK drops target tables this migration also drops) and that idx_edge_dedup is intentionally redefined under the same name. - restoreNode: document the MUST-run-inside-$transaction contract — the Edge revival can throw after the Node revival commits its statement, so atomicity depends on the caller's transaction rolling everything back. Agent / client doc-drift (orphaned by the model change, outside the diff) - llms.txt: drop the retired same-Canvas invariant from the MCP tools blurb; rename Flow titles -> Spec source in the trust-boundary list. - connection-rules: rewrite header + canConnect docstrings to ADR-0027/0028 and the ASSOCIATION (unordered) rule this helper actually mirrors. Add a #65 forward-note that directional interactions de-dupe on the ordered triple, so this helper's existence-only check must grow an interaction arm then. P3 cleanups - Stale comments: updateNodeKind ("no Edge, Flow, or FlowRoute" -> "no Edge or Spec"); component-node DescendComponentContext doc disambiguates "the flow's double-click" -> "React Flow's double-click". - Test renames: edge.service "four directional interactions" -> "directional interactions on both ordered pairs plus an association coexist"; connection-rules.test "undirected; ADR-0023" -> "unordered pair; ADR-0027". - Inherited-boundary regression case (CodeRabbit): added n-analytics + n-users->n-analytics edge to the export fixture so subtree exports surface Analytics API as an "inherited" boundary proxy (is_direct = false), covering the previously-untested descendant-owned branch in both the pure serializer and the real-DB service test. Goldens regenerated. Validation - pnpm check (eslint + tsc): green - pnpm db:check (drift gate): green (partial index invisible to Prisma, by design — same posture as the Edge dedup indexes) - pnpm test: 183/183, 10/10 files Deferred (per the approved plan; tracked for follow-up) - P2 perf: connectNodes/apply_graph read waterfalls, updatePositions N-UPDATEs. Pre-existing; ties to ADR-0026 *_unauthorized extraction. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced Jun 2, 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 #62. The foundational re-founding slice of the #62–#67 epic: a Connection becomes a directed, typed
Edgethat may link any two Components at any scope. Retires the Flow capability model.After this slice an agent can create a typed cross-scope connection through the tRPC/MCP path; the web client compiles; same-Canvas connections still render — as plain lines (typed arrowheads → #65, cross-scope rendering → #63).
Model
Flow/FlowRoute; rename the 1:1 spec rowFlowSpec→Spec, pointing at derived child Components viaNode.sourceSpecId+Node.specKey.FlowInteraction→Interactionenum, gainingASSOCIATION(the default).EdgedropscanvasNodeId(scope is derived from endpoint ancestry — Cross-scope connections: derived rendering in getCanvas #63) and gainsinteraction. De-dupe is two partial unique indexes keyed onprojectId: directional(projectId, source, target, interaction)andASSOCIATION-only unordered(projectId, LEAST, GREATEST).Interactiontype (enum-swap footgun), a guarded dedup pre-flight, correct FK/table/type drop order.pnpm db:checkis clean.Service
connectNodesaccepts cross-scope and lineal (ingress) endpoints; rejects only the self-link;interactionis an input; the dedupfindFirstbranches on interaction (unordered for ASSOCIATION, exact-ordered for directional).moveNodedrops the orphan-reject (keeps the cycle-reject).deleteNode/restoreNodedrop the Flow arms, gain aSpecsweep;deleteEdgereverts to a lone soft-delete. Deletedflow.service/flow-route.service/flow-parserand therouteFlowcross-scope writer.getCanvasreduced to{ interiorNodes, interiorEdges, breadcrumbs }via a single both-endpoints relation filter (single round trip, no waterfall).Beyond the issue text (caught in review, included here)
prisma-errors.tsmatcher now accepts both new index names; dead Flow matchers removed (silent P2002 mis-handling otherwise).export.service/markdown.tsreferenced the droppedcanvasNodeIdin raw CTEs (a runtime crash, not a compile error). Switched subtree + boundary derivation to endpoint-membership; golden re-baselined. (Markdown export + MCP: typed cross-scope connections, generated components #67 owns the full typed-export rewrite.)MCP / API / Client
connect_components+apply_graphdropcanvasNode, gaininteraction, accept cross-scope endpoints. Removed the 9 flow/flowSpec/flowRoute tRPC procedures.flow:handle branch,flow-direction,flow-interaction-display,spec-kinds).Docs (travel with the slice)
Scope boundary
Stops at the seams: getCanvas ancestry rendering + far-end proxies → #63; spec→Component generation → #64; typed arrowhead rendering + interaction picker → #65; "Connect to…" gesture → #66; export rewrite + MCP spec-attach tool → #67.
Validation
pnpm check(eslint + tsc) — greenpnpm db:check(drift gate) — cleanREQUEST A→B/PUSH A→B/REQUEST B→Acoexistence), move-no-orphan,deleteEdgelone, and theSpeccascade sweep. The only 2 misses wereresetDb()TRUNCATE hook timeouts (transient Neon latency — one intoken.service.test.ts, untouched by this PR).🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
Removals
Improvements