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

70-73: Nit: ...config spreads sinks and loggers that are immediately overwritten.

On line 72, ...config may include sinks and loggers from the caller, but both are unconditionally overridden on lines 73–74. This is harmless but slightly misleading to readers. Consider destructuring to exclude them:

Proposed refactor

   const sinks = config?.sinks ?? {}
+  const { sinks: _sinks, loggers: _loggers, ...restConfig } = config ?? {}
   const sinksWithConsole: Record<TSinkId | 'console', Sink> = {
     console: getConsoleSink(),
     ...sinks,
   } as Record<TSinkId | 'console', Sink>
   configureLogtape<TSinkId | 'console', TFilterId>({
     reset: true,
-    ...config,
+    ...restConfig,
     sinks: sinksWithConsole,

apps/playground/src/components/drawio/DrawioContextMenuProvider.tsx (2)

88-93: Ref assignment during render — consider moving to useEffect for concurrent-safety.

Writing filesRef.current = files during render can be problematic if React discards a render (Suspense, transitions in concurrent mode). Since the ref is only read from event handlers (not render), the practical risk is low, but moving the assignment into a useEffect is the React-recommended pattern.

Proposed fix

  const filesRef = useRef(files)
-  filesRef.current = files
+  useEffect(() => {
+    filesRef.current = files
+  }, [files])
   const getSourceContent = useCallback(() => {
     const contents = Object.values(filesRef.current).filter(Boolean)
     return contents.length > 0 ? contents.join('\n\n') : undefined
   }, [])

---

95-105: Conditional spread is fine but slightly obscure — consider explicit assignment for readability.

The ...(condition && { ... }) pattern works but can be non-obvious at a glance. This is purely a readability nit; no functional issue.

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

258-265: Add an explicit type annotation to exportParams for compile-time safety.

If a field is later added to ExportDrawioParams, this untyped object literal won't produce a compile error. Adding satisfies ExportDrawioParams (or a : ExportDrawioParams annotation) catches drift early.

Suggested change

-  const exportParams = {
+  const exportParams: ExportDrawioParams = {
     viewmodels,
     outdir: args.outdir,
     workspacePath: args.path,
     roundtrip: args.roundtrip,
     uncompressed: args.uncompressed,
     logger,
   }

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

107-107: Remove commented-out code.

Line 107 (// const format = options?.format) is a leftover artifact that adds noise.

Proposed fix

-  // const format = options?.format

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

236-239: Minor: pngHandler creates a logger with prefix 'export' while JSON uses 'c4:export'.

The logger prefix inconsistency ('export' here vs 'c4:export' in the JSON handler at line 122 of json/handler.ts) produces different log output formatting. Consider aligning them if a consistent prefix is desired.

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

983-989: Empty elements still emit { } block — is this intentional?

When hasBody is false, the code emits an element header followed by an empty { } block. In LikeC4, a simple declaration without braces (myElem = container 'Title') is valid and more concise. If the empty block is required by the parser, this is fine — otherwise it adds visual noise to generated output.

---

1019-1025: Operator precedence in titlePart ternary chain is hard to read.

const titlePart = title ? ` '${title}'` : desc || tech ? ` ''` : ''

The || binds tighter than ?:, so this works correctly as title ? ... : (desc || tech) ? ... : ''. However, adding parentheses would improve clarity.

♻️ Suggested clarification

-    const titlePart = title ? ` '${title}'` : desc || tech ? ` ''` : ''
+    const titlePart = title ? ` '${title}'` : (desc || tech) ? ` ''` : ''

---

1438-1456: DiagramState is declared after its first usage.

CommonDiagramStateFromCells at Line 1314 references DiagramState, which is defined 120+ lines later at Line 1438. TypeScript hoists type declarations so this compiles, but it hurts readability. Consider moving the DiagramState and SingleDiagramState type declarations before their first use (before buildCommonDiagramStateFromCells).

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

1293-1314: buildDrawioExportOptionsFromSource parses round-trip once and distributes — clean design.

One minor observation: buildDrawioExportOptionsForViews (Line 1324) calls buildDrawioExportOptionsFromSource per view, which re-parses the entire .c4 source each time. For projects with many views, parsing once and distributing would be more efficient.

♻️ Suggested optimization

 export function buildDrawioExportOptionsForViews(
   viewIds: string[],
   sourceContent: string | undefined,
   overrides?: Partial<GenerateDrawioOptions>,
 ): Record<string, GenerateDrawioOptions> {
+  if (!sourceContent) {
+    return Object.fromEntries(viewIds.map(id => [id, { compressed: false, ...overrides }]))
+  }
+  const roundtrip = parseDrawioRoundtripComments(sourceContent)
   return Object.fromEntries(
     viewIds.map(viewId => [
       viewId,
-      buildDrawioExportOptionsFromSource(viewId, sourceContent, overrides),
+      buildOptionsFromRoundtrip(viewId, roundtrip, overrides),
     ]),
   )
 }

Extract a helper that takes the already-parsed DrawioRoundtripData | null so parseDrawioRoundtripComments runs once.

apps/playground/src/components/drawio/DrawioContextMenuProvider.tsx (1)

75-75: usePlayground() is called but its return value is never used.

playground is assigned on Line 75 but never referenced in the component body. This is dead code that also triggers hook execution unnecessarily.

Proposed fix

-  const playground = usePlayground()

And remove usePlayground from the import on Line 1:

-import { usePlayground, usePlaygroundSnapshot } from '$/hooks/usePlayground'
+import { usePlaygroundSnapshot } from '$/hooks/usePlayground'

#!/bin/bash
# Verify that `playground` is not referenced elsewhere in this file
rg -n '\bplayground\b' -- "apps/playground/src/components/drawio/DrawioContextMenuProvider.tsx"

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

86-107: Symlinked source directories are silently skipped during round-trip reads.

e.isDirectory() returns false for symlinks pointing to directories, so walk never descends into them. If a workspace uses symlinked source folders (e.g., monorepo setups), their .c4/.likec4 files won't be included in the round-trip content.

If this is intentional for safety, a brief comment explaining the choice would help. Otherwise, consider adding an e.isSymbolicLink() check that resolves the target and recurses if it's a directory (the visitedDirs/realpath cycle guard already handles loops).

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

334-356: Minor: inconsistent access style for max-attempts / maxAttempts.

Line 336 accesses the yargs arg as args['max-attempts'], while line 346 uses the camelCase form args.maxAttempts. Both work due to yargs' automatic camelCase aliasing, but mixing styles in the same handler is a readability nit.

Suggested consistency fix

-      invariant(args['max-attempts'] >= 1, 'max-attempts must be >= 1')
+      invariant(args.maxAttempts >= 1, 'max-attempts must be >= 1')

---

18-40: Consider documenting all public fields on PngExportArgs.

Several fields (project, theme, useDotBin, timeoutMs, maxAttempts, filter) lack JSDoc while others have it. For a publicly exported type this is a minor inconsistency. As per coding guidelines, "Use JSDoc to document public classes and methods".

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

1447-1466: DiagramState type is defined after its first usage via CommonDiagramStateFromCells.

CommonDiagramStateFromCells at line 1324 is DiagramState & { byId: ... }, but DiagramState is declared at line 1448. TypeScript hoists type declarations so this compiles fine, but it hurts readability — the reader encounters the alias before the definition.

---

646-689: computeContainerTitles uses .find() — only the first matching text shape near a container is considered.

If multiple text-shape cells overlap the title area of a container, only the first one found (in array order) becomes the title cell. Remaining text shapes near the container's top would not be filtered as title cells and could become spurious elements. This is fine for LikeC4-generated drawio files but may produce unexpected results for hand-crafted diagrams with multiple overlapping labels.

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

86-96: Accessing optional DSL fields via as casts — pragmatic but fragile.

The ExtendedNode type and nodeOptionalFields / edgeOptionalFields accessors cast Node to a wider type to access fields (notes, summary, tags, navigateTo, etc.) that may or may not exist on the runtime object. This works today but is invisible to the type system — if upstream types change, these casts won't produce compile errors.

Consider adding a runtime guard (e.g., 'notes' in node) or using a shared type from @likec4/core if one becomes available.

Also applies to: 104-133