packages/vscode/src/utils.ts (1)

3-38: Add explicit return types to exported functions.

Guidelines require explicit types; these exports currently rely on inference. Please annotate return types (and consider a named type for performanceMark for clarity).

✏️ Proposed fix

-export function now() {
+export function now(): number {
   try {
     return performance.now()
   } catch {
     return Date.now()
   }
 }

-export function performanceMark() {
+export type PerformanceMark = {
+  readonly ms: number
+  readonly pretty: string
+}
+
+export function performanceMark(): PerformanceMark {
   const t0 = now()
   return {
     get ms(): number {
       return now() - t0
     },
     get pretty(): string {
       return prettyMs(now() - t0)
     },
   }
 }

-export function isLikeC4Source(path: string) {
+export function isLikeC4Source(path: string): boolean {
   const p = path.toLowerCase()
   return p.endsWith('.c4') || p.endsWith('.likec4') || p.endsWith('.like-c4')
 }

.changeset/drawio-cli-roundtrip-e2e.md (1)

8-8: Minor clarity improvement: Add "files" after file extensions.

For better readability, consider adding the word "files" after the file extensions.

📝 Suggested improvement

-- **CLI:** `likec4 export drawio --roundtrip` reads all `.c4`/`.likec4` in the workspace, parses round-trip comment blocks (layout, stroke colors/widths, edge waypoints), and applies them when generating each view's `.drawio` file.
+- **CLI:** `likec4 export drawio --roundtrip` reads all `.c4`/`.likec4` files in the workspace, parses round-trip comment blocks (layout, stroke colors/widths, edge waypoints), and applies them when generating each view's `.drawio` file.

apps/playground/scripts/generate.mts (1)

5-13: Consider storing platform() result to avoid redundant calls.

The Windows handling logic is correct. Minor nit: platform() is called twice—once on line 6 and again on line 12. While platform() is cheap (returns a cached value), storing the result improves readability.

♻️ Optional: Store platform result in a variable

+const isWindows = platform() === 'win32'
+
 // On Windows, zx needs PowerShell (no bash by default in v8+)
-if (platform() === 'win32') {
+if (isWindows) {
   usePowerShell()
 }
 const $ = _$({
   stdio: 'inherit',
   preferLocal: true,
-  ...(platform() === 'win32' ? { quote: quotePowerShell } : {}),
+  ...(isWindows ? { quote: quotePowerShell } : {}),
 })

packages/vscode/src/node/mcp-server.ts (1)

29-34: JSDoc should clarify expected input format.

The JSDoc mentions "JSON array of paths" but doesn't specify whether these should be URI-formatted strings (file:///...) or plain filesystem paths. Given the inconsistency noted above, clarifying the expected format would help users provide correct input.

📝 Suggested JSDoc improvement

 /**
- * Read LIKEC4_WORKSPACE env: JSON array of paths or single path string.
+ * Read LIKEC4_WORKSPACE env: JSON array of file URIs (e.g., "file:///path") or single path string.
  * Falls back to treating raw value as one workspace path if not valid JSON.
  * `@param` name - Environment variable name (e.g. 'LIKEC4_WORKSPACE').
- * `@returns` Array of absolute fs paths, or undefined if unset/invalid.
+ * `@returns` Non-empty array of absolute fs paths, or undefined if unset/empty.
  */

packages/log/src/index.ts (1)

65-70: Simplify redundant sinks extraction.

Line 65 extracts sinks as _sinks (unused), then line 66 re-extracts it via config?.sinks. Use default value in destructuring instead.

♻️ Suggested simplification

-    const { sinks: _sinks, loggers: _loggers, ...restConfig } = config ?? {}
-    const sinks = config?.sinks ?? {}
+    const { sinks = {}, loggers: _loggers, ...restConfig } = config ?? {}

packages/core/src/compute-view/deployment-view/stages/stage-exclude.spec.ts (1)

60-62: Consider tracking this limitation more formally.

The added comments clearly document why this test is skipped and the underlying limitation. However, since the comment asks to "Decide: fix stage-exclude or adjust expectation," this appears to be an unresolved design question rather than a temporary skip.

Consider:

• Using it.todo instead of it.skip to make this more visible in test reports as pending work
• Creating a tracking issue to document the decision needed and ensure it's not forgotten

Alternative: Use it.todo for better visibility

-    // Skip: excluding both ui and api leaves their parent deployment node (e.g. zone1) in final; implementation
-    // does not remove "empty" deployment nodes when all children are excluded. Decide: fix stage-exclude or adjust expectation.
-    it.skip('should remove element and all its connections', () => {
+    // Excluding both ui and api leaves their parent deployment node (e.g. zone1) in final; implementation
+    // does not remove "empty" deployment nodes when all children are excluded. Decide: fix stage-exclude or adjust expectation.
+    it.todo('should remove element and all its connections', () => {

This makes the pending decision more visible in test output.

packages/config/src/filenames.ts (1)

47-62: Type predicate narrows to basename type, not the full path.

The type predicates claim filename is typeof configJsonFilenames[number], but the check is performed on basename(filename). This means if you pass "/path/to/likec4.config.json", the function returns true and TypeScript narrows the type to '.likec4rc' | '.likec4.config.json' | 'likec4.config.json', even though the actual value is a full path.

This is a common pattern and works for most use cases where you just need to know "is this a config file". However, if callers rely on the narrowed type expecting an exact filename, they may be surprised.

Consider whether a simple boolean return is more accurate, or document this behavior:

/** Returns true if the **basename** of the given path matches a known config filename. */
export function isLikeC4JsonConfig(filename: string): boolean {
  return (configJsonFilenames as readonly string[]).includes(basename(filename))
}

packages/language-server/src/lsp/HoverProvider.ts (4)

83-85: Good addition of documentation.

The JSDoc comment accurately describes the method's purpose. Consider enhancing it with @param and @returns tags for more comprehensive documentation:

/**
 * Builds hover content for an element node (title, summary, model details and view links).
 * `@param` node - The element AST node to generate hover content for
 * `@returns` Hover information with markdown content, or undefined if element not found
 */

This would provide more context for developers working with this class.

---

115-117: Documentation added effectively.

The comment clearly explains the method's functionality. For completeness, consider adding parameter and return type documentation:

/**
 * Builds markdown hover content for an element model (relationship counts and links to views).
 * `@param` model - The element model containing relationship and view information
 * `@param` projectId - The project identifier for constructing view links
 * `@returns` Markdown string with model details, or undefined if no details available
 */

---

167-169: Documentation improves code clarity.

The JSDoc comment appropriately describes the method. As with the other methods, consider using structured JSDoc tags:

/**
 * Builds hover content for a deployment node (id, title, kind, summary).
 * `@param` node - The deployment node AST node to generate hover content for
 * `@returns` Hover information with markdown content
 */

---

192-194: Documentation enhances maintainability.

The comment accurately captures the method's behavior. For consistency with JSDoc best practices, consider the structured format:

/**
 * Builds hover content for a deployed instance (instance id, element ref, title and kind).
 * `@param` node - The deployed instance AST node to generate hover content for
 * `@returns` Hover information with markdown content showing instance details
 */

packages/likec4/src/cli/export/json/handler.ts (1)

71-72: Extension check is case-sensitive.

The check extname(outfile) !== '.json' won't match uppercase variants like .JSON. This is a minor edge case but could cause files like output.JSON.json.

♻️ Proposed fix for case-insensitive check

- if (extname(outfile) !== '.json') outfile = outfile + '.json'
+ if (extname(outfile).toLowerCase() !== '.json') outfile = outfile + '.json'

packages/likec4/src/cli/export/png/handler.ts (1)

346-354: Inconsistent property access for yargs args could cause undefined values.

The handler mixes kebab-case and camelCase property access:

• Line 336: args['max-attempts'] (kebab-case)
• Line 346: args.maxAttempts (camelCase)
• Line 349: args.serverUrl (camelCase) for option 'server-url'

While yargs performs camelCase conversion by default, this inconsistency makes the code harder to maintain and could break if yargs configuration changes. Consider using consistent kebab-case access throughout to match the option definitions.

♻️ Proposed fix for consistent property access

       await pngHandler(
         {
           path: args.path,
           useDotBin: args['use-dot'],
           output: args.outdir,
           project: args.project,
           timeoutMs: args.timeout * 1000,
-          maxAttempts: args.maxAttempts,
+          maxAttempts: args['max-attempts'],
           ignore: args.ignore === true,
           outputType: args.flat ? 'flat' : 'relative',
-          serverUrl: args.serverUrl,
+          serverUrl: args['server-url'],
           theme,
           filter: args.filter,
           sequence: args.seq,
           chromiumSandbox: args['chromium-sandbox'],
         } satisfies PngExportArgs,
       )

packages/language-server/src/documentation/documentation-provider.ts (1)

71-72: Consider refactoring the if-chain to switch(true).
This aligns the block with the preferred control-flow style.

♻️ Suggested refactor

-      if (ast.isDeploymentNode(node)) {
-        const el = parser.parseDeploymentNode(node)
-        return `**${el.title}**`
-      }
-
-      if (ast.isDeployedInstance(node)) {
-        const instance = parser.parseDeployedInstance(node)
-        const [projectId, fqn] = FqnRef.isImportRef(instance.element)
-          ? [instance.element.project as ProjectId, instance.element.model as Fqn]
-          : [doc.likec4ProjectId, instance.element.model as Fqn]
-        const found = projectId ? this.locator.getParsedElement(fqn, projectId) : this.locator.getParsedElement(fqn)
-        const lines = [
-          `_instance of_ \`${fqn}\``,
-        ]
-        if (found) {
-          lines.push(`**${found.element.title}**`)
-        }
-        return lines.join('  \n')
-      }
-
-      if (ast.isElement(node)) {
-        const el = parser.parseElement(node)
-        if (!el) {
-          return
-        }
-        const lines = [
-          `**${el.title}**`,
-          `<small>kind: \`${el.kind}\`</small>`,
-        ]
-        return lines.join('  \n')
-      }
-
-      // Exhaustiveness check — errors at compile time if guard admits a new node type without a handler
-      const _exhaustive: never = node
+      switch (true) {
+        case ast.isDeploymentNode(node): {
+          const el = parser.parseDeploymentNode(node)
+          return `**${el.title}**`
+        }
+        case ast.isDeployedInstance(node): {
+          const instance = parser.parseDeployedInstance(node)
+          const [projectId, fqn] = FqnRef.isImportRef(instance.element)
+            ? [instance.element.project as ProjectId, instance.element.model as Fqn]
+            : [doc.likec4ProjectId, instance.element.model as Fqn]
+          const found = projectId ? this.locator.getParsedElement(fqn, projectId) : this.locator.getParsedElement(fqn)
+          const lines = [
+            `_instance of_ \`${fqn}\``,
+          ]
+          if (found) {
+            lines.push(`**${found.element.title}**`)
+          }
+          return lines.join('  \n')
+        }
+        case ast.isElement(node): {
+          const el = parser.parseElement(node)
+          if (!el) {
+            return
+          }
+          const lines = [
+            `**${el.title}**`,
+            `<small>kind: \`${el.kind}\`</small>`,
+          ]
+          return lines.join('  \n')
+        }
+        default: {
+          // Exhaustiveness check — errors at compile time if guard admits a new node type without a handler
+          const _exhaustive: never = node
+          return _exhaustive
+        }
+      }

As per coding guidelines, "Favor switch(true) over if-else chains".

packages/likec4/src/cli/export/png/takeScreenshot.ts (1)

23-27: Consider refining the JSDoc for accuracy.

The added JSDoc follows the coding guideline requirement but could be more precise:

1. The @param params documents a parameter that doesn't exist in the signature (the function uses destructuring).
2. The @returns description mentions "or failed after maxAttempts" but the function always resolves—it returns only the successful captures as an array of { view, path } objects.

Based on learnings, use JSDoc to document public classes and methods.

📝 Suggested refinement

 /**
  * Capture each view as PNG under the output directory using the provided browser context.
- * `@param` params - Browser context, views, output path, logger, timeout, and options (outputType, dynamicVariant, theme).
- * `@returns` Promise resolving when all views are captured (or failed after maxAttempts).
+ * `@param` params - Configuration object containing browser context, views, output path, logger, timeout, and rendering options.
+ * `@returns` Promise resolving to an array of successfully captured views with their file paths.
  */

Alternatively, document each destructured property individually:

 /**
  * Capture each view as PNG under the output directory using the provided browser context.
- * `@param` params - Browser context, views, output path, logger, timeout, and options (outputType, dynamicVariant, theme).
- * `@returns` Promise resolving when all views are captured (or failed after maxAttempts).
+ * `@param` params.browserContext - The Playwright browser context to use for rendering
+ * `@param` params.views - Non-empty array of diagram views to capture
+ * `@param` params.output - Output directory path for PNG files
+ * `@param` params.logger - Logger instance for progress and error reporting
+ * `@param` params.timeout - Timeout in milliseconds for page operations
+ * `@param` params.maxAttempts - Maximum number of retry attempts per view
+ * `@param` params.dynamicVariant - Optional variant for dynamic views (e.g., 'sequence')
+ * `@param` params.outputType - Output path strategy: 'relative' preserves source structure, 'flat' outputs all to root
+ * `@param` params.theme - Theme to use for rendering ('light' or 'dark')
+ * `@returns` Promise resolving to an array of successfully captured views with their output file paths
  */

packages/likec4/src/cli/index.ts (1)

111-115: Minor redundancy in exit handling.

Setting process.exitCode = 1 before exit(1) is redundant since exit(1) explicitly uses the provided code. However, this is harmless and may be intentional for defensive coding.

♻️ Optional simplification

 function exitWithFailure(err: unknown, prefix?: string): never {
-  process.exitCode = 1
   console.error(prefix != null ? `${prefix} ${loggable(err)}` : loggable(err))
   exit(1)
 }

packages/likec4/src/cli/export/drawio/handler.ts (1)

136-144: Consider type-safe view ID extraction.

The cast vm.$view.id as string bypasses type safety. If the view ID type is already string, the cast is redundant; if it's a branded type (e.g., ViewId), the cast could hide type mismatches downstream.

Suggested approach

If the type system guarantees vm.$view.id is string-compatible, consider using String(vm.$view.id) for explicit conversion, or verify the underlying type to remove the cast entirely.

e2e/E2E-COVERAGE.md (1)

1-32: Consider translating documentation to English for consistency.

The E2E coverage documentation is written in Portuguese while the rest of the codebase (code comments, other docs, commit messages) appears to use English. For consistency and broader team accessibility, consider translating this to English.

e2e/src/likec4-cli-export-drawio.spec.ts (1)

23-37: Document the skip reason in the test description or add a tracking issue.

The test is skipped with a detailed comment explaining the CI issue, but this information is only visible when reading the source. Consider:

1. Including a link to a tracking issue if one exists
2. Adding the skip reason to the test name for visibility in test reports

Example

-test.skip(
-  'LikeC4 CLI - export drawio produces .drawio file with mxfile',
+test.skip(
+  'LikeC4 CLI - export drawio produces .drawio file with mxfile (SKIPPED: CI tgz path resolution issue)',

Or create a GitHub issue and reference it:

// TODO(`#123`): Re-enable when CI tgz path resolution is fixed
test.skip(

packages/vscode/src/utils.ts (1)

15-33: Consider sharing the performance mark helper to avoid drift.
There’s a very similar performanceMark() in packages/language-server/src/utils/index.ts; if package boundaries allow, centralizing the logic could prevent subtle divergence.

packages/diagram/vite.config.ts (1)

164-165: Consider adding the same explanatory comment for consistency.

The bundleConfig uses the same extensions array as defaultConfig, but lacks the comment explaining why. Adding a similar comment (or extracting the array to a shared constant) would improve maintainability.

♻️ Option 1: Add comment

   resolve: {
+    // Prefer .ts/.tsx over .js so build uses source (avoid CJS .js with JSX in src/)
     extensions: ['.ts', '.tsx', '.mts', '.mjs', '.js', '.jsx', '.json'],

♻️ Option 2: Extract to shared constant

+// Prefer .ts/.tsx over .js so build uses source (avoid CJS .js with JSX in src/)
+const resolveExtensions = ['.ts', '.tsx', '.mts', '.mjs', '.js', '.jsx', '.json']
+
 const defaultConfig = defineConfig({
   // ...
   resolve: {
     conditions: ['sources'],
-    // Prefer .ts/.tsx over .js so build uses source (avoid CJS .js with JSX in src/)
-    extensions: ['.ts', '.tsx', '.mts', '.mjs', '.js', '.jsx', '.json'],
+    extensions: resolveExtensions,
     // ...
   },
   // ...
 })

 const bundleConfig = defineConfig({
   // ...
   resolve: {
-    extensions: ['.ts', '.tsx', '.mts', '.mjs', '.js', '.jsx', '.json'],
+    extensions: resolveExtensions,
     // ...
   },
   // ...
 })

packages/likec4/src/logger.ts (1)

66-70: hasErrorLogged should also be omitted for the optional override to take effect.

The JSDoc states the intent is Vite compatibility. Currently, only 'debug' is omitted before re-adding both debug and hasErrorLogged as optional. However, hasErrorLogged from ReturnType (which is required) intersected with the optional version still results in a required property due to TypeScript intersection semantics.

If the intent is for both to be optional:

♻️ Proposed fix

-export type ViteLogger = Omit<ReturnType<typeof createLikeC4Logger>, 'debug'> & {
+export type ViteLogger = Omit<ReturnType<typeof createLikeC4Logger>, 'debug' | 'hasErrorLogged'> & {
   debug?: (msg: string, ...args: unknown[]) => void
   hasErrorLogged?: (error: Error | RollupError) => boolean
 }

apps/playground/src/monaco/LanguageClientSync.tsx (1)

112-144: Consider adding minimal error logging for debugging.

The silent catch blocks at lines 123-124 and 136-137 make debugging difficult if issues arise. While the errors are non-critical for the Draw.io export feature, adding a debug-level log would help during development.

✨ Optional: Add debug logging

           try {
             const { model } = await c.sendRequest(FetchLayoutedModel.req, {})
             return model ?? null
           } catch {
+            if (import.meta.env.DEV) logger.debug`getLayoutedModel request failed`
             return null
           }

             try {
               const res = await requestLayoutView(c, viewId as ViewId)
               if (res.result?.diagram) out[viewId] = res.result.diagram
             } catch {
+              if (import.meta.env.DEV) logger.debug`layoutViews failed for ${viewId}`
               // skip failed view
             }

scripts/ci-validate-in-docker.sh (1)

16-18: Prefer a frozen lockfile for CI reproducibility.

For CI-like validation, consider --frozen-lockfile to avoid drift and ensure deterministic installs.

♻️ Proposed change

-echo "==> Installing dependencies..."
-pnpm install --prefer-offline
+echo "==> Installing dependencies..."
+pnpm install --prefer-offline --frozen-lockfile

scripts/ci-validate-in-docker.ps1 (1)

9-9: Use Write-Output instead of Write-Host for compatibility.

PSScriptAnalyzer flags Write-Host as non-redirectable; Write-Output is safer across hosts.

🔧 Proposed change

-Write-Host "==> Starting Linux container (node:22-bookworm) with workspace: $workspace"
+Write-Output "==> Starting Linux container (node:22-bookworm) with workspace: $workspace"

e2e/tests/drawio-playground.spec.ts (1)

45-50: Consider logging or commenting the intentional catch.

The empty .catch(() => {}) on line 48 silently swallows networkidle timeout errors. While this is a common pattern to avoid flaky tests when network-idle isn't strictly necessary, adding a brief comment would clarify the intent for future maintainers.

💡 Suggested improvement

     await page.goto(TUTORIAL_PATH)
     await page.waitForURL(/\/w\/tutorial\//)
-    await page.waitForLoadState('networkidle').catch(() => {})
+    // networkidle is best-effort; tests can proceed once selector is ready
+    await page.waitForLoadState('networkidle').catch(() => {})
     await page.waitForSelector(CANVAS_SELECTOR, { timeout: TIMEOUT_DIAGRAM })

packages/generators/src/drawio/parse-drawio.ts (1)

671-677: Consider extracting a typed helper for filtered container cells.

The non-null assertions (cont.x!, cont.y!, etc.) are safe because the filter on lines 663-670 guarantees these properties exist. However, TypeScript's type narrowing doesn't propagate through filter(). A minor refinement would be to use a type guard or map to a narrower type.

Optional: Type-safe container cell extraction

+type ContainerCellWithGeometry = DrawioCell & {
+  x: number
+  y: number
+  width: number
+  height: number
+}
+
+function isContainerWithGeometry(v: DrawioCell): v is ContainerCellWithGeometry {
+  return (
+    v.style?.toLowerCase().includes('container=1') === true &&
+    v.x != null &&
+    v.y != null &&
+    v.width != null &&
+    v.height != null
+  )
+}

-  const containerCells = vertices.filter(
-    v =>
-      v.style?.toLowerCase().includes('container=1') &&
-      v.x != null &&
-      v.y != null &&
-      v.width != null &&
-      v.height != null,
-  )
+  const containerCells = vertices.filter(isContainerWithGeometry)
   for (const cont of containerCells) {
-    const cx = cont.x!,
-      cy = cont.y!,
-      cw = cont.width!,
-      ch = cont.height!
+    const cx = cont.x,
+      cy = cont.y,
+      cw = cont.width,
+      ch = cont.height