feat(cli,playground,docs,generators): Export LikeC4 views to Draw.io#16
feat(cli,playground,docs,generators): Export LikeC4 views to Draw.io#16
Conversation
…origin) Co-authored-by: Cursor <[email protected]>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…awio export Co-authored-by: Cursor <[email protected]>
📝 WalkthroughWalkthroughAdds a tracked CI workflow, refactors several tests, adds JSDoc and minor runtime fallbacks, introduces a workspace ID hook, adjusts MCP workspace-path parsing, enhances logging APIs/formatters, and expands language-server model/location APIs and caching behavior. Changes
Sequence Diagram(s)(Skipped — changes are documentation, small API additions, tests, and tooling; no new multi-component control flow introduced that meets the diagram criteria.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
…o empty workspace accept exit 1 or stderr; docs: clarify skip comments (drawio tgz CI, stage-exclude empty node) Co-authored-by: Cursor <[email protected]>
Co-authored-by: Cursor <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
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/protocol.ts (1)
323-339:⚠️ Potential issue | 🟡 MinorAlign
projectIdoptionality with the docstring.The comment says “(if any)” but the type is required. Either make it optional or update the comment to avoid a misleading API contract.
✏️ Suggested doc fix (if `projectId` is always present)
- /** - * Project ID this document belongs to (if any) - */ + /** + * Project ID this document belongs to + */
🤖 Fix all issues with AI agents
In `@packages/language-server/CHANGELOG.md`:
- Around line 17-29: The fenced code block showing the sample using view {
include some.element with { notes ''' ... } } lacks a language identifier and
triggers MD040; update that block by adding a language after the opening triple
backticks (e.g., "likec4") so the block becomes ```likec4 — locate the example
containing view { include some.element with { notes ''' ... } } in CHANGELOG.md
and add the language token to the opening fence.
In `@packages/language-server/src/documentation/documentation-provider.ts`:
- Around line 62-63: The call to nonexhaustive(node) is unreachable because the
earlier guard that filters to DeploymentNode, DeployedInstance, or Element and
the three case-specific returns (the handlers for those node types) always
return before that call; remove the nonexhaustive(node) invocation to eliminate
dead code (or if you intended an exhaustive-check assertion, move the
nonexhaustive(node) check into the branch where you handle all cases before
returning). Update the function that contains the
DeploymentNode/DeployedInstance/Element handlers to delete the final
nonexhaustive(node) call so control flow ends with the existing final return or
error handling.
In `@packages/language-server/src/model/last-seen-artifacts.ts`:
- Around line 12-14: The private field `#styles` is typed as c4.LikeC4Styles but
the file imports LikeC4Styles directly and the getter/getter return type uses
LikeC4Styles; change the type of the `#styles` map from c4.LikeC4Styles to
LikeC4Styles so the declaration for `#styles` = new Map<c4.ProjectId,
c4.LikeC4Styles>() becomes consistent with the imported LikeC4Styles and the
method return types (referencing `#styles` and LikeC4Styles in the class).
In `@packages/vscode/src/useWorkspaceId.ts`:
- Around line 7-21: The implementation of useWorkspaceId currently returns a new
id each session but the docstring says it is stored in workspace state; either
make the code persist the id or update the comment. To persist: restore the
commented logic in useWorkspaceId — read extensionContext.value?.workspaceState
into workspaceState, return a generated `likec4-${nanoid(4)}` only if
workspaceState.get('workspaceId') is missing, call
workspaceState.update('workspaceId', workspaceId) and then return the persisted
value; ensure nanoid is imported and handle the case where extensionContext or
workspaceState is undefined (fall back to generating a transient id).
Alternatively, change the docstring on useWorkspaceId to state it generates a
per-session id and is not stored.
🧹 Nitpick comments (8)
packages/vscode/src/node/mcp-server.ts (1)
29-42: Handle raw path values forLIKEC4_WORKSPACE.If the env var is set to a plain path (not JSON), it’s silently ignored and the server falls back to
cwd. Consider a fallback to treat the raw value as a single workspace path.♻️ Suggested fallback
try { const parsed = envSchema.parse(JSON.parse(value)) const asArray = isString(parsed) ? [parsed] : parsed if (hasAtLeast(asArray, 1)) { return map(asArray, v => URI.parse(v).fsPath) } } catch { - // ignore + // Fallback: treat the raw value as a single workspace path + return [URI.parse(value).fsPath] }packages/language-server/src/model/builder/MergedSpecification.ts (1)
6-6: Unused import:hasAtLeastThe
hasAtLeastfunction is imported but not used anywhere in this file.🧹 Remove unused import
import { - hasAtLeast, isEmpty, isEmptyish, isNonNullish, only, unique, } from 'remeda'packages/vscode/tsdown.config.ts (1)
109-124: Consider throwing an error instead ofprocess.exit(1)Using
process.exit(1)immediately terminates the process without allowing tsdown's error handling to run. Throwing an error would provide better error reporting and allow the build system to handle the failure gracefully.♻️ Proposed fix
async function copyPreview() { const vscodePreview = resolve('../vscode-preview/dist/') if (!existsSync(vscodePreview)) { - console.error(`"${vscodePreview}" not found`) - process.exit(1) + throw new Error(`vscode-preview not found at "${vscodePreview}". Ensure `@likec4/vscode-preview` is built first.`) } console.info('Copy vscode preview from %s', vscodePreview)packages/vscode/src/node/useMcpRegistration.ts (2)
28-34: Non-null assertion onextensionContext.valueUsing
extensionContext.value!assumes the extension context is always available when this composable runs. Since this is a singleton composable called during extension activation, it should be safe, but consider adding a guard or using optional chaining with a fallback for defensive coding.🛡️ Defensive check
+ const ctx = extensionContext.value + if (!ctx) { + throw new Error('useMcpRegistration must be called after extension activation') + } - const serverModule = extensionContext.value!.asAbsolutePath( + const serverModule = ctx.asAbsolutePath( join( 'dist', 'node', 'mcp-server.mjs', ), )
51-52: Tagged template literal may not format as expectedLine 52 uses tagged template syntax
logger.debug`Resolving MCP server ${server.label}`while line 54 uses a regular function call. The tagged template might not interpolate the value as expected depending on the logger implementation. Consider using consistent function call syntax.🧹 Use consistent logging syntax
resolveMcpServerDefinition: async (server) => { - logger.debug`Resolving MCP server ${server.label}` + logger.debug(`Resolving MCP server ${server.label}`) if (server.label === 'likec4' && isMcpStdioServerDefinition(server)) {packages/language-server/src/lsp/HoverProvider.ts (1)
183-200: Consider adding JSDoc for the protected hover methods.The new protected methods (
getElementHover,getElementModelHover,getDeploymentNodeHover,getDeployedInstanceHover) lack JSDoc documentation. As per coding guidelines, public/protected methods should be documented.📝 Example JSDoc for one method
+ /** + * Builds hover content for a deployed instance node. + * Displays the instance ID, FQN reference, and optionally the element title. + */ protected getDeployedInstanceHover(node: ast.DeployedInstance): MaybePromise<Hover | undefined> {packages/language-server/src/model/last-seen-artifacts.ts (1)
45-55: Consider adding JSDoc to the public accessor methods.The accessor methods (
specification,styles,model) would benefit from brief JSDoc documentation describing what they return and when they might returnundefined.📝 Example JSDoc additions
+ /** + * Returns the last seen merged specification for the given project, if available. + */ public specification(projectId: c4.ProjectId): MergedSpecification | undefined { return this.#specs.get(projectId) } + /** + * Returns the last seen styles for the given project, if available. + */ public styles(projectId: c4.ProjectId): LikeC4Styles | undefined { return this.#styles.get(projectId) } + /** + * Returns the last seen computed model for the given project, if available. + */ public model(projectId: c4.ProjectId): LikeC4Model.Computed | undefined { return this.#models.get(projectId) }packages/language-server/src/model/model-locator.ts (1)
54-91: Consider explicit type annotations for intermediate variables.The rest-args pattern works but makes type narrowing implicit. Adding explicit types to
astNodeOrFqnandprojectIdwould improve readability and make the type flow clearer for maintainers.🔧 Suggested type annotations
try { - let astNodeOrFqn - let projectId + let astNodeOrFqn: ast.Element | c4.Fqn + let projectId: c4.ProjectId if (args.length === 2) { astNodeOrFqn = args[0] projectId = args[1] } else { astNodeOrFqn = args[0] projectId = isString(astNodeOrFqn) ? this.projects.ensureProjectId() : projectIdFrom(astNodeOrFqn) }
packages/language-server/src/documentation/documentation-provider.ts
Outdated
Show resolved
Hide resolved
…able nonexhaustive, consistent LikeC4Styles, useWorkspaceId doc) Co-authored-by: Cursor <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/language-server/src/model/last-seen-artifacts.ts`:
- Around line 24-55: Add JSDoc comments to each public method
(rememberSpecification, rememberModel, specification, styles, model) describing
what the method does, its parameters and return value; for rememberSpecification
and rememberModel include `@param` with the parameter type (MergedSpecification, M
extends LikeC4Model.Computed) and note side effects (stores in internal maps),
and `@returns` with the returned type; for the accessor methods specification,
styles, and model include `@param` for projectId and `@returns` (possibly undefined)
types. Keep comments concise, consistent with project style, and placed
immediately above each method.
🧹 Nitpick comments (1)
packages/language-server/src/documentation/documentation-provider.ts (1)
8-16: Consider adding JSDoc documentation for the public class and constructor.The class and constructor lack JSDoc comments. As per coding guidelines, public classes and methods should be documented.
📝 Suggested JSDoc documentation
+/** + * Provides documentation for LikeC4 AST nodes, extending JSDoc parsing + * with specialized formatting for deployment nodes, deployed instances, and elements. + */ export class LikeC4DocumentationProvider extends JSDocDocumentationProvider { private parser: LikeC4ModelParser private locator: LikeC4ModelLocator + /** + * Creates a new documentation provider. + * `@param` services - The LikeC4 language services. + */ constructor(services: LikeC4Services) {
…own throw, useMcpRegistration guard+log, HoverProvider JSDoc, last-seen-artifacts JSDoc, model-locator types) Co-authored-by: Cursor <[email protected]>
…LastSeenArtifacts public API Co-authored-by: Cursor <[email protected]>
…erver) Co-authored-by: Cursor <[email protected]>
…-present contract Co-authored-by: Cursor <[email protected]>
…server, likec4, log, vscode) Co-authored-by: Cursor <[email protected]>
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/config/src/filenames.ts (1)
47-48:⚠️ Potential issue | 🟠 MajorType guard narrows to incorrect type when full path is passed.
The type predicate claims
filename is typeof configJsonFilenames[number](e.g.,".likec4rc"), but the function checksbasename(filename). If the caller passes"/some/path/.likec4rc", the guard returnstruewhile TypeScript incorrectly narrowsfilenameto a literal like".likec4rc".Consider returning
booleaninstead of a type predicate, or clarifying in documentation that the narrowed type only reflects the basename, not the full path.-export function isLikeC4JsonConfig(filename: string): filename is typeof configJsonFilenames[number] { +export function isLikeC4JsonConfig(filename: string): boolean { return (configJsonFilenames as readonly string[]).includes(basename(filename)) }The same issue applies to
isLikeC4NonJsonConfig(line 54) andisLikeC4Config(line 61).
🤖 Fix all issues with AI agents
In `@packages/config/src/filenames.ts`:
- Line 28: The JSDoc for the constant that lists known LikeC4 non-JSON config
filenames in packages/config/src/filenames.ts is incomplete (it mentions "JS,
MJS, TS, MTS" but the array also contains CJS and CTS); update the JSDoc comment
above that constant (the known LikeC4 non-JSON config filenames definition) to
include "CJS" and "CTS" so the comment matches the actual extensions present in
the array.
🧹 Nitpick comments (2)
packages/likec4/src/cli/index.ts (2)
28-33: Add explicit types toapplyLoggerConfig.
Align with the repo rule for explicit TypeScript types.Suggested change
-function applyLoggerConfig(isDebug = isDevelopment) { +function applyLoggerConfig(isDebug: boolean = isDevelopment): void {As per coding guidelines, "Use TypeScript with explicit types; avoid using any".
49-53: Add an explicit return type tomain.
This keeps the public signature clear and consistent with the TypeScript rule.Suggested change
-async function main() { +async function main(): Promise<void> {As per coding guidelines, "Use TypeScript with explicit types; avoid using any".
| 'likec4.config.json', | ||
| ] as const | ||
|
|
||
| /** Known LikeC4 non-JSON config filenames (JS, MJS, TS, MTS). */ |
There was a problem hiding this comment.
JSDoc comment is incomplete.
The comment lists "(JS, MJS, TS, MTS)" but the array also includes CJS and CTS extensions.
📝 Suggested fix
-/** Known LikeC4 non-JSON config filenames (JS, MJS, TS, MTS). */
+/** Known LikeC4 non-JSON config filenames (JS, CJS, MJS, TS, CTS, MTS). */📝 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.
| /** Known LikeC4 non-JSON config filenames (JS, MJS, TS, MTS). */ | |
| /** Known LikeC4 non-JSON config filenames (JS, CJS, MJS, TS, CTS, MTS). */ |
🤖 Prompt for AI Agents
In `@packages/config/src/filenames.ts` at line 28, The JSDoc for the constant that
lists known LikeC4 non-JSON config filenames in packages/config/src/filenames.ts
is incomplete (it mentions "JS, MJS, TS, MTS" but the array also contains CJS
and CTS); update the JSDoc comment above that constant (the known LikeC4
non-JSON config filenames definition) to include "CJS" and "CTS" so the comment
matches the actual extensions present in the array.
…orts Co-authored-by: Cursor <[email protected]>
1 participant
feat(cli,playground,docs,generators): Export LikeC4 views to Draw.io
Summary
This PR adds export of LikeC4 views to Draw.io (
.drawio) format from the CLI (likec4 export drawio) and the Playground (right-click on diagram → DrawIO → Export view / Export all). It does not include import; import will be proposed in a separate PR.Context for maintainers: This work was first submitted in PR #2614, which received substantial review feedback focused on clean code, structure, and maintainability. We took that feedback seriously and ran a deliberate refactor pass (Uncle Bob / Clean Code) over the DrawIO-related code. This PR re-submits the same feature set with a cleaner, refactored codebase that we believe is easier to review and maintain. We’re grateful for the earlier review and have aimed to address those concerns in this iteration.
What’s in this PR
1. Generators (
@likec4/generators)computeDiagramLayoutsplit into smaller helpers;getViewDescriptionStringextracted;buildNodeCellXml/buildEdgeCellXmland constants (SOLID/DRY/KISS); exported typeDrawioViewModelLike.parseDrawioToLikeC4Multisplit intomergeDiagramStatesIntoMaps,buildRootsFromFqnToCell,emitMultiDiagramModel(orchestrator ~50–60 lines);buildViewBlockLines/escapeLikec4Quotes; O(n²) deduplication replaced withparsedIdsSet; UserObjectfullTagusesinnerXmlso sibling<data>is preserved.generate-drawio.spec.ts,parse-drawio.spec.ts; snapshots in__snapshots__/. Decompress error assertion accepts (base64 decode|inflate|URI decode) for Node/env behavior; snapshots updated for CI.2. CLI (
@likec4/likec4)likec4 export drawiowith--outdir/-o,--all-in-one,--roundtrip,--uncompressed,--project,--use-dot. UsesDEFAULT_DRAWIO_ALL_FILENAMEfrom@likec4/generators(DRY). Phase comments and thin handler pattern aligned with other export commands.PngExportArgs,runExportPng(args, logger); PNG export supports--outdir/-o(docs updated).likec4 import drawioin this PR (nopackages/likec4/src/cli/import/).3. Playground
generateDrawio/generateDrawioMultiandparseDrawioRoundtripComments. No Import menu item or file input.4. Documentation
--outdir; no Import section.5. E2E & tests
playwright.playground.config.ts.drawio-demo-export-import.spec.ts,drawio-tutorial-export-import.spec.ts— export tests; import/round-trip tests skipped in this PR.What’s not in this PR
likec4 import drawiocommand.Refactor summary (response to PR likec4#2614 feedback)
After the initial submission (PR likec4#2614), we applied a structured clean-code pass:
DrawioViewModelLike); constants; phase comments; DRY (e.g.DEFAULT_DRAWIO_ALL_FILENAME, shared helpers).runExport*(args, logger)pattern for drawio/PNG/JSON; consistent error handling and JSDoc.e2e/helpers/.We believe this version is in better shape for review and long-term maintenance.
Correções após review (PR #11)
Incorporadas as sugestões do review anterior:
apimemoizado comuseMemo(..., [actions.openMenu])para evitar re-renders desnecessários.LayoutView.reqemsendRequest(type-safe; removido cast manual).exitCode === 1no erro rejeitado.basenameremove barras finais (/e\) e trata resultado vazio com fallback.arcSize=12(inteiro percentagem) em vez de0.12para cantos ligeiramente arredondados no Draw.io.buildCommonDiagramStateFromCells;buildSingleDiagramStateebuildDiagramStatereutilizam (DRY).useDotlido dos args e passado arunExportDrawio; graphvizbinaryvswasmconforme flag--use-dot.useDotnos args e emrunExportJson; guard quandoprojectsModels.length === 0(warn + throw).Alterações adicionais (nitpicks e robustez)
requestLayoutViewpassa a usarLayoutView.reqemsendRequest(type-safe; params e resposta inferidos; cast removido).try;finallygaranteserver.close()mesmo quandoresolveServerUrlou o export lançam (evita leak do ViteDevServer).readWorkspaceSourceContentcom proteção a ciclos de symlink:realpath+visitedDirspara não seguir o mesmo diretório duas vezes.Checklist
main(merge/rebase as appropriate).feat:,refactor:,test:).pnpm ci:test(Vitest) passes.Verification
pnpm build(filter!./apps/*),pnpm typecheck,pnpm test(orpnpm ci:test) — pass.Notes for reviewers
DrawioViewModelLikeis the public type for view models passed togenerateDrawio/generateDrawioMulti.runExport*+ thin handler).Fork / local CI changes (this session)
loggable(safe-stringify) instead ofJSON.stringifyin formatters to avoid stack overflow with circular references.runs-on: ubuntu-24.04-arm(reviewer davydkov’s suggestion)..gitignoreand removed from tracking (git rm --cached) so it is not pushed to upstream. The PR to origin does not include this file.relative="1", parse-drawioln = lines[i]?.trim()) already reflected in code; unresolved items reviewed and confirmed.Reference
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation
Chores