Skip to content

feat(core): add post-patch formatting via LikeC4 SDK#76

Merged
parse merged 2 commits intomainfrom
feat/likec4-post-patch-formatting
Mar 23, 2026
Merged

feat(core): add post-patch formatting via LikeC4 SDK#76
parse merged 2 commits intomainfrom
feat/likec4-post-patch-formatting

Conversation

@parse
Copy link
Copy Markdown
Contributor

@parse parse commented Mar 22, 2026

Summary

  • After the patcher generates DSL and validation passes, format the patched content using the LikeC4 SDK's format() API
  • Produces cleaner model update PRs that match the project's existing .c4 style
  • Gracefully degrades when SDK format() is unavailable (older LikeC4 versions)
  • New formatAfterPatch config option (default: true), configurable via .eroderc.json or ERODE_LIKEC4_FORMAT_AFTER_PATCH env var

Verbose output

With ERODE_VERBOSE=true, the patcher logs formatting status:

... Generating model patch
[LikeC4Patcher] Input relationships [...]
[LikeC4Patcher] Attempting LLM-based patching
[LikeC4Patcher] LLM patch accepted {"valid":true}
[LikeC4Patcher] Post-patch formatting applied
✓ Patch: 1 component(s), 3 line(s)
ℹ Patched model formatted with LikeC4 formatter
✓ Model patched: likec4/model.c4

When formatting is unavailable:

[LikeC4Patcher] Post-patch formatting skipped (SDK unavailable)

Test plan

  • npm run check:ci passes (lint, typecheck, format, schema)
  • npm run test passes (819 tests including 7 new formatter/patcher tests)
  • npm run knip reports no unused exports
  • Manual: run erode analyze with --patch-local against a LikeC4 workspace, verify formatted output

Summary by CodeRabbit

  • New Features

    • Automatic LikeC4 DSL formatting after patching (enabled by default).
    • New config option to control formatting: adapter.likec4.formatAfterPatch (env: ERODE_LIKEC4_FORMAT_AFTER_PATCH).
    • Pipeline now reports when a patched model was formatted.
  • Tests

    • Added tests covering LikeC4 formatter and post-patch formatting behavior.
  • Chores

    • Updated LikeC4 dependency to v1.53.0.

After the patcher generates DSL and validation passes, format the
patched content using the LikeC4 SDK's format() API. This produces
cleaner model update PRs that match the project's existing .c4 style.

- Add dsl-formatter.ts using SDK format() via temp workspace copy
- Add postFormat hook to BasePatcher, override in LikeC4Patcher
- Add formatAfterPatch config option (default: true)
- Add formatted field to PatchResult for user-facing output
- Bump likec4 dependency to ^1.53.0
- Gracefully degrade when SDK format() is unavailable
@parse parse requested a review from a team as a code owner March 22, 2026 20:47
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 22, 2026

📝 Walkthrough

Walkthrough

Added a post-patch formatting hook and LikeC4-based formatter, integrated formatting into the patch workflow behind a new adapter.likec4.formatAfterPatch config flag, and updated dependency and tests to support the feature.

Changes

Cohort / File(s) Summary
Dependency & Schemas
packages/core/package.json, packages/core/schemas/eroderc.schema.json, packages/web/public/schemas/v0/eroderc.schema.json
Bumped likec4 dependency to ^1.53.0; added adapter.likec4.formatAfterPatch boolean (default: true) to schema files.
Base patcher & result type
packages/core/src/adapters/base-patcher.ts, packages/core/src/adapters/model-patcher.ts
Added protected postFormat(content, modelPath, targetFile) hook to BasePatcher; PatchResult gains optional formatted?: boolean.
LikeC4 formatter implementation
packages/core/src/adapters/likec4/dsl-formatter.ts, packages/core/src/adapters/likec4/patcher.ts
Added formatLikeC4Dsl(...) to run LikeC4 SDK in a temp workspace and return structured results; LikeC4Patcher overrides postFormat() to call the formatter conditionally and handle outcomes (formatted, skipped, error).
Configuration
packages/core/src/utils/config.ts
Added adapter.likec4.formatAfterPatch to runtime config schema and env var mapping (ERODE_LIKEC4_FORMAT_AFTER_PATCH) with boolean coercion.
Pipeline reporting
packages/core/src/pipelines/analyze.ts
Pipeline now emits an informational message when a patch was formatted (checks ctx.patchResult.formatted).
Tests
packages/core/src/adapters/likec4/__tests__/dsl-formatter.test.ts, packages/core/src/adapters/likec4/__tests__/patcher-format.test.ts, packages/core/src/adapters/likec4/__tests__/patcher.test.ts
Added tests for formatLikeC4Dsl covering success, missing target, SDK absence/errors; added patcher tests verifying post-format behavior and formatted flag handling.

Sequence Diagram(s)

sequenceDiagram
    participant Patcher as BasePatcher
    participant LikeC4P as LikeC4Patcher
    participant Formatter as formatLikeC4Dsl
    participant SDK as LikeC4 SDK
    participant FS as Filesystem

    Patcher->>Patcher: patch() -> produce patchedContent
    Patcher->>LikeC4P: postFormat(patchedContent, modelPath, targetFile)
    LikeC4P->>LikeC4P: check CONFIG.adapter.likec4.formatAfterPatch
    alt enabled
        LikeC4P->>Formatter: formatLikeC4Dsl(workspace, targetFile, patchedContent)
        Formatter->>FS: withTempWorkspaceCopy (create temp, copy files)
        Formatter->>SDK: LikeC4.fromWorkspace(tempDir)
        alt SDK.format available
            Formatter->>SDK: format({ documentUris: [targetUri] })
            SDK-->>Formatter: Map of formatted contents
            alt target found
                Formatter-->>LikeC4P: { formatted: true, content }
            else target missing
                Formatter-->>LikeC4P: { formatted: false, error }
            end
        else SDK missing/unsupported
            Formatter-->>LikeC4P: { formatted: false, skipped: true }
        end
        Formatter->>FS: cleanup temp workspace
        LikeC4P-->>Patcher: return formattedContent or original
    else disabled
        LikeC4P-->>Patcher: return original content
    end
    Patcher->>Patcher: set PatchResult.formatted based on content change
    Patcher-->>Caller: return PatchResult
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 I hopped in with a patch, then tipped my hat,
I called LikeC4 to tidy the flat,
Temp copies, format maps, a tidy tune,
The model gleams beneath the silvery moon,
Hop, patch, format — the rabbit's happy chat.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the primary change: adding post-patch formatting via the LikeC4 SDK, which is the main feature introduced across the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/likec4-post-patch-formatting

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
packages/core/src/utils/config.ts (1)

124-124: Keep exported env-name constants in sync with ENV_MAP.

ERODE_LIKEC4_FORMAT_AFTER_PATCH is mapped and coerced, but not exposed in ENV_VAR_NAMES. Adding it keeps a single discoverable source for env knobs.

♻️ Proposed sync update
 export const ENV_VAR_NAMES = {
   aiProvider: 'ERODE_AI_PROVIDER',
   geminiApiKey: 'ERODE_GEMINI_API_KEY',
   anthropicApiKey: 'ERODE_ANTHROPIC_API_KEY',
   openaiApiKey: 'ERODE_OPENAI_API_KEY',
   githubToken: 'ERODE_GITHUB_TOKEN',
   gitlabToken: 'ERODE_GITLAB_TOKEN',
   bitbucketToken: 'ERODE_BITBUCKET_TOKEN',
   structurizrCliPath: 'ERODE_STRUCTURIZR_CLI_PATH',
   modelRepoPrToken: 'ERODE_MODEL_REPO_PR_TOKEN',
   modelPath: 'ERODE_MODEL_PATH',
   modelRepo: 'ERODE_MODEL_REPO',
   modelRef: 'ERODE_MODEL_REF',
+  likec4FormatAfterPatch: 'ERODE_LIKEC4_FORMAT_AFTER_PATCH',
 } as const;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/utils/config.ts` at line 124, ENV_VAR_NAMES is missing the
exported constant for the mapped/coerced key ERODE_LIKEC4_FORMAT_AFTER_PATCH;
add ERODE_LIKEC4_FORMAT_AFTER_PATCH to the ENV_VAR_NAMES export so the env-name
constants stay in sync with ENV_MAP. Locate the constants/exports that define
ENV_VAR_NAMES and insert the symbol ERODE_LIKEC4_FORMAT_AFTER_PATCH (matching
the existing key 'adapter.likec4.formatAfterPatch') into that collection so
consumers can import the same canonical env-name used by ENV_MAP and the
coercion logic.
packages/core/src/adapters/likec4/dsl-formatter.ts (1)

40-42: Silently swallowing errors is intentional but limits debuggability.

The empty catch clause discards error details, which aligns with the graceful degradation design. However, consider logging the error when verbose mode is enabled to aid troubleshooting formatter issues.

💡 Optional: Add verbose logging
-  } catch {
+  } catch (err) {
+    if (process.env.ERODE_VERBOSE === 'true') {
+      console.error('[formatLikeC4Dsl] Formatting skipped due to error:', err);
+    }
     return { formatted: false, skipped: true };
   }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/adapters/likec4/dsl-formatter.ts` around lines 40 - 42, The
catch block in packages/core/src/adapters/likec4/dsl-formatter.ts currently
swallows errors (catch { return { formatted: false, skipped: true }; }); change
it to capture the error (catch (err)) and emit the error to a verbose logger
when verbose mode is enabled—e.g., if an options.verbose flag or a logger is
available, call logger.debug or console.debug with the error and context
(preserve the existing return { formatted: false, skipped: true } behavior when
not verbose). Ensure you reference the same catch block and keep the
graceful-degradation return unchanged.
🤖 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/core/package.json`:
- Line 63: The code relies on an undocumented programmatic API likec4.format
obtained from LikeC4.fromWorkspace() (see dsl-formatter.ts and the typeof
likec4.format !== 'function' guard); this version bump won't add a supported
.format() method—either remove/replace calls to likec4.format and instead use
documented APIs (likec4.diagrams, likec4.model, likec4.computedModel) to perform
formatting logic, or explicitly detect absence of likec4.format and fall back to
invoking the new CLI (e.g., spawn the `likec4 format` command) with clear
comments documenting the experimental dependency; update dsl-formatter.ts to
implement the chosen approach and remove reliance on the undocumented .format()
method.

In `@packages/core/src/adapters/base-patcher.ts`:
- Around line 145-149: postFormat can throw and currently aborts the whole patch
flow; wrap the await this.postFormat(finalContent, modelPath, targetFile) call
in a try/catch so optional post-processing "fails open": if postFormat throws,
catch the error, log or record it, keep finalContent as preFormatContent and set
formatted to false (do not rethrow). Apply the same guarded pattern to the other
post-format invocation noted (the call around line 170) so any optional
formatting/processing never interrupts a successful patch.

In `@packages/core/src/adapters/likec4/__tests__/patcher.test.ts`:
- Around line 480-530: The test file patcher.test.ts exceeds the 500-line limit
due to added formatter-focused cases; extract the two tests that reference
mockFormatLikeC4Dsl (the "should apply post-patch formatting when formatter
returns content" and "should set formatted to undefined when formatting is
skipped") into a new colocated test file (e.g., post-format.test.ts). In the new
file, import the same helpers and fixtures used in patcher.test.ts (patcher
instance, SAMPLE_C4, makeIndex, makeProvider, and the mocked
readdirSync/readFileSync/formatLikeC4Dsl utilities), copy the two test cases
there, and remove them from the original patcher.test.ts so the original stays
under 500 lines; ensure mocks/setup/teardown are either duplicated or shared via
a small test-utils import so both files run identically.

In `@packages/core/src/adapters/likec4/dsl-formatter.ts`:
- Line 28: The current construction of targetUri using the template
`file://${tmpTarget}` yields invalid file URIs on Windows; replace this with
Node's pathToFileURL to convert the filesystem path correctly (import
pathToFileURL from 'url') and set targetUri to the resulting URL's href so it
produces platform-correct file:///... URIs; update usage in the dsl-formatter
where targetUri is declared (refer to the targetUri variable and tmpTarget
value).

---

Nitpick comments:
In `@packages/core/src/adapters/likec4/dsl-formatter.ts`:
- Around line 40-42: The catch block in
packages/core/src/adapters/likec4/dsl-formatter.ts currently swallows errors
(catch { return { formatted: false, skipped: true }; }); change it to capture
the error (catch (err)) and emit the error to a verbose logger when verbose mode
is enabled—e.g., if an options.verbose flag or a logger is available, call
logger.debug or console.debug with the error and context (preserve the existing
return { formatted: false, skipped: true } behavior when not verbose). Ensure
you reference the same catch block and keep the graceful-degradation return
unchanged.

In `@packages/core/src/utils/config.ts`:
- Line 124: ENV_VAR_NAMES is missing the exported constant for the
mapped/coerced key ERODE_LIKEC4_FORMAT_AFTER_PATCH; add
ERODE_LIKEC4_FORMAT_AFTER_PATCH to the ENV_VAR_NAMES export so the env-name
constants stay in sync with ENV_MAP. Locate the constants/exports that define
ENV_VAR_NAMES and insert the symbol ERODE_LIKEC4_FORMAT_AFTER_PATCH (matching
the existing key 'adapter.likec4.formatAfterPatch') into that collection so
consumers can import the same canonical env-name used by ENV_MAP and the
coercion logic.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3a196280-9776-470a-bb7e-153f935b0b04

📥 Commits

Reviewing files that changed from the base of the PR and between e949b60 and add798d.

📒 Files selected for processing (11)
  • packages/core/package.json
  • packages/core/schemas/eroderc.schema.json
  • packages/core/src/adapters/base-patcher.ts
  • packages/core/src/adapters/likec4/__tests__/dsl-formatter.test.ts
  • packages/core/src/adapters/likec4/__tests__/patcher.test.ts
  • packages/core/src/adapters/likec4/dsl-formatter.ts
  • packages/core/src/adapters/likec4/patcher.ts
  • packages/core/src/adapters/model-patcher.ts
  • packages/core/src/pipelines/analyze.ts
  • packages/core/src/utils/config.ts
  • packages/web/public/schemas/v0/eroderc.schema.json

"@octokit/rest": "^22.0.1",
"dotenv": "^17.2.3",
"likec4": "^1.52.0",
"likec4": "^1.53.0",
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read-only verification: confirm lockfile resolution and current usage of LikeC4 format API.

echo "== Lockfiles present =="
fd '^(pnpm-lock\\.yaml|package-lock\\.json|yarn\\.lock)$'

echo
echo "== likec4 entries in lockfiles =="
rg -n "likec4|@?likec4" pnpm-lock.yaml package-lock.json yarn.lock 2>/dev/null || true

echo
echo "== LikeC4 API usage in source =="
rg -n -C2 "LikeC4\\.fromWorkspace|\\.format\\s*\\(" packages/core/src/adapters/likec4

Repository: erode-app/erode

Length of output: 8088


🌐 Web query:

In the official LikeC4 changelog/docs, which version first guarantees support for LikeC4.fromWorkspace(...).format(...)? Please provide the exact link and version note.

💡 Result:

No version in the official LikeC4 changelog or docs explicitly guarantees support for LikeC4.fromWorkspace(...).format(...). The LikeC4.fromWorkspace method has been available since at least v1.25.0 (2025-03-07), as documented in the CHANGELOG.md under "LikeC4 Model API". Current docs at https://likec4.dev/tooling/model-api show examples of LikeC4.fromWorkspace but do not mention a .format method on the returned object. The API provides methods like likec4.diagrams, likec4.model, and likec4.computedModel, but no .format. A new likec4 format CLI command was added in v1.53.0 (2026-03-20) for formatting .c4 source files, but this is unrelated to the programmatic API chain.

Citations:


The version bump to 1.53.0 does not address the underlying API compatibility issue.

The code in dsl-formatter.ts relies on likec4.format() as a programmatic method, but the official LikeC4 API documentation does not list .format() as a public method on the object returned by LikeC4.fromWorkspace(). The v1.53.0 release added a new CLI command likec4 format for file formatting, not the programmatic API that the code depends on. The codebase has defensive checks (typeof likec4.format !== 'function') acknowledging this uncertainty, which suggests the method is undocumented or unstable. Upgrading the version will not resolve this API mismatch. Consider either:

  • Using only documented LikeC4 API methods (diagrams, model, computedModel), or
  • If .format() is an internal/experimental API, document the dependency and compatibility clearly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/package.json` at line 63, The code relies on an undocumented
programmatic API likec4.format obtained from LikeC4.fromWorkspace() (see
dsl-formatter.ts and the typeof likec4.format !== 'function' guard); this
version bump won't add a supported .format() method—either remove/replace calls
to likec4.format and instead use documented APIs (likec4.diagrams, likec4.model,
likec4.computedModel) to perform formatting logic, or explicitly detect absence
of likec4.format and fall back to invoking the new CLI (e.g., spawn the `likec4
format` command) with clear comments documenting the experimental dependency;
update dsl-formatter.ts to implement the chosen approach and remove reliance on
the undocumented .format() method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The format() method is present in the SDK's TypeScript type definitions (node_modules/likec4/dist/index.d.mts, line 904) and we verified it works at runtime on v1.52.0 before implementing this. The type definitions ARE the API contract for TypeScript consumers. The typeof guard handles graceful degradation for any version where it might not exist.

The web search result is inaccurate — the docs site doesn't cover all SDK methods. The method signature is:

format(options?: LikeC4FormatOptions): Promise<Map<string, string>>;

Dismissing this as a false positive.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@parse, thank you for the clarification — you're absolutely right. The TypeScript type definitions in node_modules/likec4/dist/index.d.mts are the authoritative API contract for SDK consumers, and the presence of format(options?: LikeC4FormatOptions): Promise<Map<string, string>> there confirms this is a supported programmatic method. The typeof guard is a sound defensive pattern for cross-version compatibility, not an indicator of an undocumented/unstable API. My original comment was a false positive driven by an inaccurate web search result. I apologize for the noise.


✏️ Learnings added
Learnt from: parse
Repo: erode-app/erode PR: 76
File: packages/core/package.json:63-63
Timestamp: 2026-03-23T05:49:49.004Z
Learning: In the erode-app/erode repository, the `format()` method on the object returned by `LikeC4.fromWorkspace()` (from the `likec4` npm package) IS a supported programmatic API. Its full signature is `format(options?: LikeC4FormatOptions): Promise<Map<string, string>>` and it is present in the TypeScript type definitions at `node_modules/likec4/dist/index.d.mts` (line 904 as of v1.52.0+). The `typeof likec4.format !== 'function'` guard in `dsl-formatter.ts` is an intentional graceful-degradation pattern for older versions, not evidence of an undocumented API. Do not flag this as a missing/undocumented method in future reviews.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Comment on lines +145 to +149
// 7. Optional post-processing (e.g., formatting)
const preFormatContent = finalContent;
finalContent = await this.postFormat(finalContent, modelPath, targetFile);
const formatted = finalContent !== preFormatContent;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Guard optional post-formatting so patching cannot fail hard.

At Line 147, an exception in postFormat aborts the whole patch flow, even when finalContent is already valid. Optional post-processing should fail open.

🛠️ Suggested reliability fix
-    const preFormatContent = finalContent;
-    finalContent = await this.postFormat(finalContent, modelPath, targetFile);
-    const formatted = finalContent !== preFormatContent;
+    const preFormatContent = finalContent;
+    let formatted = false;
+    try {
+      const postFormatted = await this.postFormat(finalContent, modelPath, targetFile);
+      formatted = postFormatted !== preFormatContent;
+      finalContent = postFormatted;
+    } catch (error) {
+      this.debugLog(
+        'Post-processing failed; proceeding with unformatted patch',
+        error instanceof Error ? error.message : String(error)
+      );
+    }

Also applies to: 170-170

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/adapters/base-patcher.ts` around lines 145 - 149,
postFormat can throw and currently aborts the whole patch flow; wrap the await
this.postFormat(finalContent, modelPath, targetFile) call in a try/catch so
optional post-processing "fails open": if postFormat throws, catch the error,
log or record it, keep finalContent as preFormatContent and set formatted to
false (do not rethrow). Apply the same guarded pattern to the other post-format
invocation noted (the call around line 170) so any optional
formatting/processing never interrupts a successful patch.

Comment on lines +480 to +530
it('should apply post-patch formatting when formatter returns content', async () => {
mockReaddirSync.mockReturnValue(['model.c4'] as unknown as ReturnType<typeof readdirSync>);
mockReadFileSync.mockReturnValue(SAMPLE_C4);

const formattedContent = SAMPLE_C4 + "\n customer -> backend 'Formatted'\n";
mockFormatLikeC4Dsl.mockResolvedValue({
formatted: true,
content: formattedContent,
});

const rels: StructuredRelationship[] = [
{ source: 'customer', target: 'backend', description: 'New dep' },
];

const result = await patcher.patch({
modelPath: '/model',
relationships: rels,
existingRelationships: [],
componentIndex: makeIndex(['customer', 'backend']),
provider: makeProvider(),
});

expect(result).not.toBeNull();
if (!result) return;
expect(result.content).toBe(formattedContent);
expect(result.formatted).toBe(true);
expect(mockFormatLikeC4Dsl).toHaveBeenCalled();
});

it('should set formatted to undefined when formatting is skipped', async () => {
mockReaddirSync.mockReturnValue(['model.c4'] as unknown as ReturnType<typeof readdirSync>);
mockReadFileSync.mockReturnValue(SAMPLE_C4);
mockFormatLikeC4Dsl.mockResolvedValue({ formatted: false, skipped: true });

const rels: StructuredRelationship[] = [
{ source: 'customer', target: 'backend', description: 'New dep' },
];

const result = await patcher.patch({
modelPath: '/model',
relationships: rels,
existingRelationships: [],
componentIndex: makeIndex(['customer', 'backend']),
provider: makeProvider(),
});

expect(result).not.toBeNull();
if (!result) return;
expect(result.formatted).toBeUndefined();
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Split this test file to satisfy the 500-line cap.

These added formatter cases are valuable, but this file now exceeds the repository max-length rule. Please move formatter-focused cases into a separate colocated test file (for example, post-format.test.ts) to keep maintenance manageable.

As per coding guidelines: packages/**/*.ts: Keep files under 500 lines maximum length.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/adapters/likec4/__tests__/patcher.test.ts` around lines 480
- 530, The test file patcher.test.ts exceeds the 500-line limit due to added
formatter-focused cases; extract the two tests that reference
mockFormatLikeC4Dsl (the "should apply post-patch formatting when formatter
returns content" and "should set formatted to undefined when formatting is
skipped") into a new colocated test file (e.g., post-format.test.ts). In the new
file, import the same helpers and fixtures used in patcher.test.ts (patcher
instance, SAMPLE_C4, makeIndex, makeProvider, and the mocked
readdirSync/readFileSync/formatLikeC4Dsl utilities), copy the two test cases
there, and remove them from the original patcher.test.ts so the original stays
under 500 lines; ensure mocks/setup/teardown are either duplicated or shared via
a small test-utils import so both files run identically.

- Wrap postFormat in try/catch so formatting errors fail open
- Use pathToFileURL for cross-platform file URI construction
- Add verbose logging to formatter catch block
- Add ERODE_LIKEC4_FORMAT_AFTER_PATCH to ENV_VAR_NAMES
- Extract formatter tests into patcher-format.test.ts (500-line cap)
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
packages/core/src/adapters/likec4/__tests__/patcher-format.test.ts (2)

101-121: Consider adding assertion that formatter was called.

For consistency with the first test (line 98), this test could assert expect(mockFormatLikeC4Dsl).toHaveBeenCalled() to verify the formatter was invoked before returning the skipped result.

💡 Suggested addition
     expect(result).not.toBeNull();
     if (!result) return;
     expect(result.formatted).toBeUndefined();
+    expect(mockFormatLikeC4Dsl).toHaveBeenCalled();
   });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/adapters/likec4/__tests__/patcher-format.test.ts` around
lines 101 - 121, Add an assertion to the test "should set formatted to undefined
when formatting is skipped" to verify the formatter was invoked by calling
expect(mockFormatLikeC4Dsl).toHaveBeenCalled(); place it after the patcher.patch
invocation and before the formatted undefined assertion so you confirm
mockFormatLikeC4Dsl was called when patcher.patch (the patcher.patch call in
this test) returned the skipped formatted result.

63-121: Consider adding a test for the error handling path.

The tests cover successful formatting and skipped formatting, but there's no test verifying the try-catch behavior in base-patcher.ts when postFormat throws an error. Adding this test would ensure the fail-open behavior works as intended.

💡 Suggested additional test case
it('should proceed with unformatted patch when formatting throws', async () => {
  mockReaddirSync.mockReturnValue(['model.c4'] as unknown as ReturnType<typeof readdirSync>);
  mockReadFileSync.mockReturnValue(SAMPLE_C4);
  mockFormatLikeC4Dsl.mockRejectedValue(new Error('Format failed'));

  const rels: StructuredRelationship[] = [
    { source: 'customer', target: 'backend', description: 'New dep' },
  ];

  const result = await patcher.patch({
    modelPath: '/model',
    relationships: rels,
    existingRelationships: [],
    componentIndex: makeIndex(['customer', 'backend']),
    provider: makeProvider(),
  });

  expect(result).not.toBeNull();
  if (!result) return;
  expect(result.formatted).toBeUndefined();
  // Content should still contain the patched DSL (unformatted)
  expect(result.content).toContain("customer -> backend");
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/adapters/likec4/__tests__/patcher-format.test.ts` around
lines 63 - 121, Add a test in the existing LikeC4Patcher post-format spec to
cover the error path where the formatter throws: in the test file using
LikeC4Patcher and patch(), mockFormatLikeC4Dsl should be set to reject (e.g.,
mockRejectedValue(new Error('Format failed'))), keep
mockReaddirSync/readFileSync returning the model, call patcher.patch(...) with
the same rels and provider, then assert the result is not null, result.formatted
is undefined, and result.content still contains the unformatted relationship
(e.g., "customer -> backend"); this verifies the fail-open behavior in the
try/catch around postFormat in base-patcher.ts.
🤖 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/core/src/adapters/likec4/__tests__/patcher-format.test.ts`:
- Around line 101-121: Add an assertion to the test "should set formatted to
undefined when formatting is skipped" to verify the formatter was invoked by
calling expect(mockFormatLikeC4Dsl).toHaveBeenCalled(); place it after the
patcher.patch invocation and before the formatted undefined assertion so you
confirm mockFormatLikeC4Dsl was called when patcher.patch (the patcher.patch
call in this test) returned the skipped formatted result.
- Around line 63-121: Add a test in the existing LikeC4Patcher post-format spec
to cover the error path where the formatter throws: in the test file using
LikeC4Patcher and patch(), mockFormatLikeC4Dsl should be set to reject (e.g.,
mockRejectedValue(new Error('Format failed'))), keep
mockReaddirSync/readFileSync returning the model, call patcher.patch(...) with
the same rels and provider, then assert the result is not null, result.formatted
is undefined, and result.content still contains the unformatted relationship
(e.g., "customer -> backend"); this verifies the fail-open behavior in the
try/catch around postFormat in base-patcher.ts.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d29a2c87-e3af-4e68-bd96-1747909fc449

📥 Commits

Reviewing files that changed from the base of the PR and between add798d and 6c14558.

📒 Files selected for processing (5)
  • packages/core/src/adapters/base-patcher.ts
  • packages/core/src/adapters/likec4/__tests__/patcher-format.test.ts
  • packages/core/src/adapters/likec4/__tests__/patcher.test.ts
  • packages/core/src/adapters/likec4/dsl-formatter.ts
  • packages/core/src/utils/config.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/core/src/utils/config.ts
  • packages/core/src/adapters/likec4/dsl-formatter.ts
  • packages/core/src/adapters/likec4/tests/patcher.test.ts

@parse parse merged commit 1ba3c49 into main Mar 23, 2026
9 checks passed
@parse parse deleted the feat/likec4-post-patch-formatting branch March 23, 2026 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant