refactor: remove deprecated ManualLayoutV1 and related dependencies#2713
Merged
refactor: remove deprecated ManualLayoutV1 and related dependencies#2713
Conversation
Remove all V1 manual layout references including the ViewManualLayout type, serialization/deserialization logic, migration command, and V1-specific layout application. Also removes @msgpack/msgpack and @smithy/util-base64 dependencies that were only used for V1 encoding. Co-Authored-By: Claude Opus 4.6 <[email protected]>
🦋 Changeset detectedLatest commit: 932fed6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRemoves the deprecated ManualLayoutV1 surface: types, parsing, serialization, validation, migration command, runtime application, tests, and associated utilities across core, language-server, layouts, and VSCode packages. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(220,220,255,0.5)
participant VSCode
participant Extension
participant LS as LanguageServer
participant Workspace
end
Note over VSCode,Extension: OLD FLOW (before this PR)
VSCode->>Extension: User triggers migrate-manual-layouts
Extension->>LS: request computed models / views
LS->>Workspace: read files / AST / comments
LS->>Workspace: detect & remove ManualLayoutV1 (workspace edit)
Workspace-->>LS: edit applied
LS-->>Extension: results
Extension-->>VSCode: show completion
Note over VSCode,Extension: NEW FLOW (after this PR)
VSCode->>Extension: (command removed) — no trigger
Extension--xLS: no migration flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/language-server/src/view-utils/manual-layout.ts (1)
45-56: Minor: Consolidate edge property updates to avoid intermediate objects.When both
colorandlinediffer, this creates two intermediate objects. Consider combining the checks:♻️ Proposed consolidation
- if (latestEdge.color !== e.color) { - e = { - ...e, - color: latestEdge.color, - } - } - if (latestEdge.line !== e.line) { - e = { - ...e, - line: latestEdge.line, - } - } + if (latestEdge.color !== e.color || latestEdge.line !== e.line) { + e = { + ...e, + color: latestEdge.color, + line: latestEdge.line, + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/language-server/src/view-utils/manual-layout.ts` around lines 45 - 56, Consolidate the two separate updates that create intermediate objects by merging the property checks for latestEdge.color and latestEdge.line into a single conditional that builds one updated edge object; locate the code where variable e is compared to latestEdge (variables named e and latestEdge in manual-layout.ts) and replace the two sequential assignments with one update that only sets changed properties (color and/or line) on e and assigns the new object once.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/language-server/src/view-utils/manual-layout.ts`:
- Around line 45-56: Consolidate the two separate updates that create
intermediate objects by merging the property checks for latestEdge.color and
latestEdge.line into a single conditional that builds one updated edge object;
locate the code where variable e is compared to latestEdge (variables named e
and latestEdge in manual-layout.ts) and replace the two sequential assignments
with one update that only sets changed properties (color and/or line) on e and
assigns the new object once.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 76a81531-cd06-4a19-a386-5b3386831dae
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
.changeset/remove-manual-layout-v1.mdpackages/core/src/types/view-common.tspackages/core/src/types/view-computed.tspackages/language-server/package.jsonpackages/language-server/src/ast.tspackages/language-server/src/lsp/CodeActionProvider.tspackages/language-server/src/model-change/ModelChanges.tspackages/language-server/src/model-change/removeManualLayoutV1.tspackages/language-server/src/model/__tests__/model-builder.spec.tspackages/language-server/src/model/parser/DeploymentViewParser.tspackages/language-server/src/model/parser/ViewsParser.tspackages/language-server/src/validation/view.tspackages/language-server/src/view-utils/manual-layout.spec.tspackages/language-server/src/view-utils/manual-layout.tspackages/layouts/src/graphviz/GraphvizLayoter.tspackages/layouts/src/graphviz/GraphvizParser.tspackages/layouts/src/manual/applyManualLayout.tspackages/vscode/package.jsonpackages/vscode/package.nls.jsonpackages/vscode/src/commands/index.tspackages/vscode/src/commands/migrateManualLayouts.ts
💤 Files with no reviewable changes (17)
- packages/language-server/src/ast.ts
- packages/layouts/src/graphviz/GraphvizLayoter.ts
- packages/vscode/package.nls.json
- packages/vscode/package.json
- packages/language-server/src/model/parser/DeploymentViewParser.ts
- packages/language-server/src/model/tests/model-builder.spec.ts
- packages/language-server/package.json
- packages/language-server/src/model/parser/ViewsParser.ts
- packages/vscode/src/commands/index.ts
- packages/language-server/src/model-change/ModelChanges.ts
- packages/vscode/src/commands/migrateManualLayouts.ts
- packages/language-server/src/view-utils/manual-layout.spec.ts
- packages/layouts/src/manual/applyManualLayout.ts
- packages/core/src/types/view-common.ts
- packages/language-server/src/model-change/removeManualLayoutV1.ts
- packages/core/src/types/view-computed.ts
- packages/layouts/src/graphviz/GraphvizParser.ts
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/language-server/src/validation/view.ts (1)
7-31:⚠️ Potential issue | 🟠 MajorKeep a diagnostic for legacy manual-layout blocks.
After this change, V1 manual-layout comments stop being applied and stop producing any validation message here, so existing workspaces can lose layout behavior with no clue why. Even if migration support is intentionally gone, this validator should still surface an error or warning like “ManualLayoutV1 is no longer supported; remove this block” when legacy comment blocks are present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a6ab4922-59c4-4926-acbc-8bbc4f7f73e3
📒 Files selected for processing (1)
packages/language-server/src/validation/view.ts
- Add warning diagnostic for legacy @likec4-generated blocks so users know V1 is no longer supported and should remove the block - Consolidate edge property updates to avoid intermediate objects Co-Authored-By: Claude Opus 4.6 <[email protected]>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/language-server/src/view-utils/manual-layout.ts (1)
11-63:⚠️ Potential issue | 🟠 MajorWire
applyStylesToManualLayoutinto the active manual-layout flow and complete edge style synchronization.This helper is currently unused. The layout pipeline in
LikeC4Views.tscallsapplyManualLayout(layouted, snapshot)forlayoutType === 'manual', bypassing these style updates entirely.Additionally, edge style synchronization is incomplete.
ComputedEdgeincludesheadandtailfields, but the edge sync logic only copiescolorandline, missing arrow style drifts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/language-server/src/view-utils/manual-layout.ts` around lines 11 - 63, The manual-layout style updates are never applied and edge arrows aren't synced: wire applyStylesToManualLayout into the manual layout pipeline by replacing the current call that uses applyManualLayout(...) when layoutType === 'manual' (in LikeC4Views.ts) so the returned layouted view passes through applyStylesToManualLayout(manualLayouted, snapshot/latest) before returning; and update applyStylesToManualLayout's edge-sync logic to copy latestEdge.head and latestEdge.tail (in addition to color and line) into the edge object so ComputedEdge arrow styles are preserved (copy head/tail when they differ, similar to how color/line are handled).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/language-server/src/view-utils/manual-layout.ts`:
- Around line 45-50: The merge only updates color and line on variable e when
latestEdge differs, but ComputedEdge also carries head and tail styling so
arrowheads can drift; update the merge in manual-layout logic (where latestEdge
and e are compared) to also copy latestEdge.head and latestEdge.tail into e (or
include head/tail in the equality check and assign) so head and tail styling are
synced along with color and line.
---
Outside diff comments:
In `@packages/language-server/src/view-utils/manual-layout.ts`:
- Around line 11-63: The manual-layout style updates are never applied and edge
arrows aren't synced: wire applyStylesToManualLayout into the manual layout
pipeline by replacing the current call that uses applyManualLayout(...) when
layoutType === 'manual' (in LikeC4Views.ts) so the returned layouted view passes
through applyStylesToManualLayout(manualLayouted, snapshot/latest) before
returning; and update applyStylesToManualLayout's edge-sync logic to copy
latestEdge.head and latestEdge.tail (in addition to color and line) into the
edge object so ComputedEdge arrow styles are preserved (copy head/tail when they
differ, similar to how color/line are handled).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a91275ed-2da5-4359-8997-cf3464dc1d31
📒 Files selected for processing (2)
packages/language-server/src/validation/view.tspackages/language-server/src/view-utils/manual-layout.ts
Comment on lines
+45
to
50
| if (latestEdge.color !== e.color || latestEdge.line !== e.line) { | ||
| e = { | ||
| ...e, | ||
| color: latestEdge.color, | ||
| } | ||
| } | ||
| if (latestEdge.line !== e.line) { | ||
| e = { | ||
| ...e, | ||
| line: latestEdge.line, | ||
| } |
Contributor
There was a problem hiding this comment.
Sync head and tail edge styling too.
ComputedEdge styling also includes head and tail, so arrow changes in the latest layout will still drift after this merge.
Proposed fix
- if (latestEdge.color !== e.color || latestEdge.line !== e.line) {
+ if (
+ latestEdge.color !== e.color ||
+ latestEdge.line !== e.line ||
+ latestEdge.head !== e.head ||
+ latestEdge.tail !== e.tail
+ ) {
e = {
...e,
color: latestEdge.color,
line: latestEdge.line,
+ head: latestEdge.head,
+ tail: latestEdge.tail,
}
}📝 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
| if (latestEdge.color !== e.color || latestEdge.line !== e.line) { | |
| e = { | |
| ...e, | |
| color: latestEdge.color, | |
| } | |
| } | |
| if (latestEdge.line !== e.line) { | |
| e = { | |
| ...e, | |
| line: latestEdge.line, | |
| } | |
| if ( | |
| latestEdge.color !== e.color || | |
| latestEdge.line !== e.line || | |
| latestEdge.head !== e.head || | |
| latestEdge.tail !== e.tail | |
| ) { | |
| e = { | |
| ...e, | |
| color: latestEdge.color, | |
| line: latestEdge.line, | |
| head: latestEdge.head, | |
| tail: latestEdge.tail, | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/language-server/src/view-utils/manual-layout.ts` around lines 45 -
50, The merge only updates color and line on variable e when latestEdge differs,
but ComputedEdge also carries head and tail styling so arrowheads can drift;
update the merge in manual-layout logic (where latestEdge and e are compared) to
also copy latestEdge.head and latestEdge.tail into e (or include head/tail in
the equality check and assign) so head and tail styling are synced along with
color and line.
Member
Author
There was a problem hiding this comment.
Will address in another PR
Contributor
There was a problem hiding this comment.
@davydkov, understood! Would you like me to open a follow-up GitHub issue to track syncing head and tail edge styling in applyStylesToManualLayout?
🧠 Learnings used
Learnt from: CR
Repo: likec4/likec4 PR: 0
File: packages/core/AGENTS.md:0-0
Timestamp: 2026-02-11T10:07:53.330Z
Learning: Applies to packages/core/src/manual-layout/**/*.{ts,tsx,js,jsx} : When changes relate to views (types or models), update View-drifts detection/auto-applying in `src/manual-layout` accordingly. If unsure what leads to layout drifts or can be auto-applied, ask for confirmation.
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.
Summary
ViewManualLayouttype, serialization/deserialization logic, and V1-specific layout application@msgpack/msgpackand@smithy/util-base64dependencies that were only used for V1 encodingTest plan
pnpm typecheckpassespnpm testpasses🤖 Generated with Claude Code
Summary by CodeRabbit