Merged
Conversation
Auto-generate scoped views for all elements without explicit views, enabling drill-down navigation out of the box. Implicit views are placed under an "Auto" folder in the view sidebar and can be disabled via the `implicitViews` config option (enabled by default). - Implicit views have view ID prefixed with `__` and title prefixed with `Auto /` - Navigation works automatically via existing `assignNavigateTo` system - Configurable via `implicitViews` boolean in project config (defaults to true) - Updated JSON schema for editor autocomplete - Added configuration documentation Co-Authored-By: Claude Opus 4.6 <[email protected]>
🦋 Changeset detectedLatest commit: c4e2b4f 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
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughAdds configurable implicit scoped views: a new Changes
Sequence Diagram(s)(Skipped) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/docs/src/content/docs/dsl/Config/index.mdx (1)
182-194: Consider documenting the "Auto" folder name and__view-ID prefix convention.The PR description notes that implicit views receive an
Auto /title prefix (appearing under an Auto folder in the sidebar) and that their IDs are prefixed with__. Users who title an explicit viewAuto / ...or choose an ID starting with__will silently land in the auto-generated folder / conflict with implicit IDs. A brief note in this section would prevent that confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/src/content/docs/dsl/Config/index.mdx` around lines 182 - 194, Add a short note to the "Implicit views" section documenting the naming convention so users don’t accidentally collide with auto-generated views: state that implicit views are shown under an "Auto" folder (title prefixed with "Auto /") and their view IDs are prefixed with "__", and warn authors not to create explicit views with titles starting with "Auto /" or IDs starting with "__" (or recommend choosing a different prefix) to avoid silent conflicts; update the text near the "Implicit views" heading and the example config to mention this convention.packages/language-server/src/model/builder/buildModel.ts (1)
322-361: Missing test forimplicitViews: falseconfig opt-outThe feature flag
project.config.implicitViews !== falseis exercised by the existing tests only in its enabled (default) state. There is no test that setsimplicitViews: falseon the project config and verifies that no__-prefixed views are generated. Without it, a regression that ignores the flag would go undetected. Based on learnings, "Aim to cover new features with relevant tests and keep test names descriptive."I can draft a
createTestServices-based test case that validatesimplicitViews: falsesuppresses all implicit view generation — would you like me to open an issue or add it directly?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/language-server/src/model/builder/buildModel.ts` around lines 322 - 361, Add a test that verifies the implicit view opt-out by setting project.config.implicitViews = false and asserting that no auto-generated element views (IDs prefixed with "__" created in the buildModel logic that pushes into parsedViews) are produced; specifically, use the existing test helper (createTestServices) to build a model with elements that would normally generate implicit views, run the code path that calls the buildModel logic (so parsedViews and elements are populated), and assert parsedViews contains no entries whose id equals the computed viewId pattern ('__' + fqn.replaceAll('.', '_')) or simply startsWith("__"); reference the same symbols used in the diff (project.config.implicitViews, parsedViews, elements, keys(), isGlobalFqn, viewId) so the test targets the exact feature gate and prevents regressions.
🤖 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/model/builder/buildModel.ts`:
- Around line 330-341: existingViewIds is built once from parsedViews and never
updated, so when you generate synthetic viewId (const viewId = ('__' +
fqn.replaceAll('.', '_')) as ViewId) you may allow duplicates to be added to
parsedViews and later overwritten by indexBy(prop('id')). Fix: after you push a
new view into parsedViews (the synthetic view created for FQNs not in
elementsWithExplicitViews and not isGlobalFqn), immediately add viewId to
existingViewIds (existingViewIds.add(viewId)) so subsequent iterations detect
the collision and skip or handle it; keep the existing guards
(elementsWithExplicitViews, isGlobalFqn) and the viewId normalization logic
unchanged.
---
Nitpick comments:
In `@apps/docs/src/content/docs/dsl/Config/index.mdx`:
- Around line 182-194: Add a short note to the "Implicit views" section
documenting the naming convention so users don’t accidentally collide with
auto-generated views: state that implicit views are shown under an "Auto" folder
(title prefixed with "Auto /") and their view IDs are prefixed with "__", and
warn authors not to create explicit views with titles starting with "Auto /" or
IDs starting with "__" (or recommend choosing a different prefix) to avoid
silent conflicts; update the text near the "Implicit views" heading and the
example config to mention this convention.
In `@packages/language-server/src/model/builder/buildModel.ts`:
- Around line 322-361: Add a test that verifies the implicit view opt-out by
setting project.config.implicitViews = false and asserting that no
auto-generated element views (IDs prefixed with "__" created in the buildModel
logic that pushes into parsedViews) are produced; specifically, use the existing
test helper (createTestServices) to build a model with elements that would
normally generate implicit views, run the code path that calls the buildModel
logic (so parsedViews and elements are populated), and assert parsedViews
contains no entries whose id equals the computed viewId pattern ('__' +
fqn.replaceAll('.', '_')) or simply startsWith("__"); reference the same symbols
used in the diff (project.config.implicitViews, parsedViews, elements, keys(),
isGlobalFqn, viewId) so the test targets the exact feature gate and prevents
regressions.
Update view count assertions in drawio-tutorial-export-import.spec.ts to account for auto-generated implicit scoped views. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/likec4/src/drawio-tutorial-export-import.spec.ts (1)
138-143: Consider adding at least one assertion on an implicit view to guard the new feature.The for-loop only checks structural validity (
<mxGraphModel) for all diagrams, and the two explicit views get content assertions — but none of the three new implicit views are individually verified. A minimal check like the one below would provide a regression guard for the feature being introduced:✨ Suggested addition
expect(indexDiagram!.content).toContain('Customer') expect(saasDiagram!.content).toContain('Frontend') expect(saasDiagram!.content).toContain('Backend') + + // Verify at least one implicit view is present and has content + const customerImplicitDiagram = diagrams.find(d => d.id === 'likec4-__customer') + expect(customerImplicitDiagram).toBeDefined() + expect(customerImplicitDiagram!.content).toContain('Customer')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/likec4/src/drawio-tutorial-export-import.spec.ts` around lines 138 - 143, The tests currently validate that each entry in diagrams contains '<mxGraphModel' and check indexDiagram and saasDiagram contents, but they don't assert any of the three new implicit views individually; update the spec to locate at least one implicit view from the diagrams collection (e.g., by name or id) and add an assertion that its .content contains an expected token (for example a view-specific label like 'ImplicitViewName' or another domain string), referencing the existing variables diagrams, indexDiagram, and saasDiagram to find and assert the implicit view content to guard the new feature.
🤖 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/likec4/src/drawio-tutorial-export-import.spec.ts`:
- Around line 138-143: The tests currently validate that each entry in diagrams
contains '<mxGraphModel' and check indexDiagram and saasDiagram contents, but
they don't assert any of the three new implicit views individually; update the
spec to locate at least one implicit view from the diagrams collection (e.g., by
name or id) and add an assertion that its .content contains an expected token
(for example a view-specific label like 'ImplicitViewName' or another domain
string), referencing the existing variables diagrams, indexDiagram, and
saasDiagram to find and assert the implicit view content to guard the new
feature.
When projectConfig is provided to createTestServices, implicitViews
defaults to false. View-folder tests explicitly opt out with
projectConfig: {}. Two new tests verify implicit views behavior
when explicitly enabled.
Co-Authored-By: Claude Opus 4.6 <[email protected]>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/language-server/src/model/__tests__/model-builder-view-folders.spec.ts (1)
178-202: Consider also asserting that__sys2is placed in the Auto folder.The current assertions are sufficient to verify the skip logic, but the test doesn't confirm that the generated
__sys2ends up inside theAutofolder rather than at the root. Switching tobuildLikeC4Model()and adding a folder-placement check would make this test parallel in depth to the first new test:♻️ Optional extension
- const model = await buildModel() - // sys1 has an explicit scoped view, so no implicit view for it - // sys2 gets an implicit view __sys2 - expect(keys(model.views)).toContain('__sys2') - expect(keys(model.views)).not.toContain('__sys1') + const model = await buildLikeC4Model() + // sys1 has an explicit scoped view, so no implicit view for it + // sys2 gets an implicit view __sys2 + expect(keys(model.$data.views)).toContain('__sys2') + expect(keys(model.$data.views)).not.toContain('__sys1') + // __sys2 should be placed in the Auto folder + expect([...model.viewFolder('Auto').views]).toEqual([model.view('__sys2')])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/language-server/src/model/__tests__/model-builder-view-folders.spec.ts` around lines 178 - 202, Update the test to also verify that the implicit view __sys2 is placed inside the Auto folder: instead of only calling buildModel(), call buildLikeC4Model() (or otherwise obtain foldered output) and then assert that model.views['__sys2'] (or the view entry found via keys(model.views)) has its folder set to 'Auto' (or that the Auto folder contains '__sys2'); keep the existing checks for presence/absence of __sys2 and __sys1 and add this single folder-placement assertion referencing buildLikeC4Model and model.views/__sys2.
🤖 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/model/__tests__/model-builder-view-folders.spec.ts`:
- Around line 178-202: Update the test to also verify that the implicit view
__sys2 is placed inside the Auto folder: instead of only calling buildModel(),
call buildLikeC4Model() (or otherwise obtain foldered output) and then assert
that model.views['__sys2'] (or the view entry found via keys(model.views)) has
its folder set to 'Auto' (or that the Auto folder contains '__sys2'); keep the
existing checks for presence/absence of __sys2 and __sys1 and add this single
folder-placement assertion referencing buildLikeC4Model and model.views/__sys2.
Update existingViewIds set after generating each implicit view so that FQNs producing the same viewId (e.g. a.b_c and a_b.c both mapping to __a_b_c) don't result in duplicate entries. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Add implicitViews: false to e2e project configs to prevent auto-generated views from breaking screenshot snapshots and expected view ID lists. Co-Authored-By: Claude Opus 4.6 <[email protected]>
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
Auto-generate scoped views for all elements without explicit views, enabling drill-down navigation out of the box. Implicit views are grouped under an "Auto" folder in the view sidebar and can be disabled via the
implicitViewsconfig option (enabled by default).Changes
__(e.g.,__system_backend) and titles prefixed withAuto /to organize them in the sidebarassignNavigateTosystem — clicking an element navigates to its implicit scoped viewimplicitViews: falsein project config (defaults to true)Testing
All 869 tests pass. Updated snapshots and test assertions to reflect new implicit views in models.
Co-Authored-By: Claude Opus 4.6 [email protected]
Summary by CodeRabbit
New Features
Documentation
Tests