Lazy-load Monaco Editor

[dimaMachina]

No description provided.

[changeset-bot]

🦋 Changeset detected

Latest commit: 4525c03

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages

Name	Type
@inkeep/agents-manage-ui	Patch
@inkeep/agents-manage-api	Patch
@inkeep/agents-run-api	Patch
@inkeep/agents-cli	Patch
@inkeep/agents-core	Patch
@inkeep/agents-manage-mcp	Patch
@inkeep/agents-sdk	Patch
@inkeep/ai-sdk-provider	Patch
@inkeep/create-agents	Patch

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

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
agents-manage-api	Ready	Preview, Comment	Dec 17, 2025 11:07am
agents-manage-ui	Ready	Preview, Comment	Dec 17, 2025 11:07am
agents-run-api	Ready	Preview, Comment	Dec 17, 2025 11:07am

1 Skipped Deployment

Project	Deployment	Review	Updated (UTC)
agents-docs	Skipped		Dec 17, 2025 11:07am

[claude]

Claude finished @dimaMachina's task —— View job

---

Code Review: Lazy-load Monaco Editor

I've completed a thorough review of the Monaco Editor lazy-loading implementation. This PR successfully moves Monaco Editor from being eagerly loaded at the app level to being lazy-loaded on demand. Here's my comprehensive analysis:

Todo List:

• Analyze the core lazy-loading implementation
• Review state management changes in monaco-store
• Examine component-level changes
• Check for potential race conditions and error handling
• Evaluate consistency with existing patterns
• Provide comprehensive feedback and recommendations

---

Summary

Positive Changes:

• Bundle Size Optimization: Removes Monaco Editor from the main bundle, significantly reducing initial bundle size
• SSR Compatibility: Properly handles Monaco's dependency on window object
• Clean Architecture: Well-structured lazy-loading with proper state management
• Theme Integration: Maintains theme synchronization between app and Monaco

Areas for Improvement:

🚨 Critical Issues

1. Race Condition in Monaco Initialization

Severity: High

The wasInitialized flag creates a potential race condition:

// use-monaco-store.ts:28, 42-45
let wasInitialized = false;

async setMonaco() {
  if (wasInitialized) {
    return;
  }
  wasInitialized = true;
  // ... async operations
}

Issue: Multiple components calling setMonaco() simultaneously could result in multiple initialization attempts before the first one sets wasInitialized = true.

Recommendation: Use a Promise-based approach:

let monacoPromise: Promise<void> | null = null;

async setMonaco() {
  if (monacoPromise) {
    return monacoPromise;
  }
  
  monacoPromise = this.initializeMonaco();
  return monacoPromise;
}

private async initializeMonaco() {
  // existing initialization logic
}

2. Missing Error Handling in Async Initialization

Severity: High

The setMonaco() function has no error handling for the dynamic imports:

// use-monaco-store.ts:46-58
const [monaco, { createHighlighter }, { shikiToMonaco }, monacoCompatibleSchema] = await Promise.all([
  import('monaco-editor'),
  import('shiki'),
  import('@shikijs/monaco'),
  import('@/lib/monaco-editor/dynamic-ref-compatible-json-schema.json'),
  import('@/lib/monaco-editor/setup-monaco-workers'),
]);

Issue: If any import fails, the Monaco store state becomes inconsistent, and components will be stuck showing skeleton loaders indefinitely.

Recommendation: Add comprehensive error handling:

async setMonaco() {
  try {
    // ... existing logic
  } catch (error) {
    console.error('Failed to initialize Monaco Editor:', error);
    wasInitialized = false; // Reset for retry
    // Consider setting an error state in the store
  }
}

3. Component Lifecycle Issues

Severity: Medium

Multiple components (MonacoEditor, CodeDiff, PromptEditor) all trigger Monaco loading independently:

// Each component does this:
useEffect(() => {
  setMonaco();
}, []);

Issue: This creates unnecessary duplicate calls and makes the loading behavior unpredictable.

Recommendation: Consider implementing a more centralized initialization strategy or ensure the Promise-based approach from issue #1 is implemented.

⚠️ Design Considerations

4. Module-Level State vs Store State

Severity: Medium

The use of module-level wasInitialized flag outside the Zustand store creates two sources of truth:

let wasInitialized = false; // Module level
// vs
monaco: null // Store level

Recommendation: Move the initialization state into the store for better consistency and debuggability:

interface MonacoStateData {
  monaco: typeof Monaco | null;
  isInitializing: boolean;
  initializationError: Error | null;
}

5. Theme Synchronization Complexity

Severity: Low

The theme-toggle component directly accesses Monaco store, creating tight coupling:

// theme-toggle.tsx:38-42
useEffect(() => {
  const isDark = resolvedTheme === 'dark';
  const monacoTheme = isDark ? MONACO_THEME_NAME.dark : MONACO_THEME_NAME.light;
  monaco?.editor.setTheme(monacoTheme);
}, [resolvedTheme, monaco]);

Recommendation: Consider moving this logic to the Monaco store's setMonacoTheme action for better encapsulation.

🔧 Performance & User Experience

6. Loading State Management

Severity: Low

Components show skeleton loaders while Monaco loads, but there's no loading timeout or retry mechanism.

Recommendation: Add timeout handling and retry logic for better resilience.

7. Bundle Splitting Optimization

Severity: Low

The current implementation loads all Monaco dependencies simultaneously. Consider if some can be loaded on-demand (e.g., Shiki only when syntax highlighting is needed).

📋 Code Quality

8. TypeScript Safety

Severity: Low

The assignment window.monaco = monaco (line 157) should be properly typed:

declare global {
  interface Window {
    monaco: typeof Monaco;
  }
}

📚 Architecture Praise

Excellent Patterns:

• ✅ Proper separation of concerns with dedicated Monaco store
• ✅ SSR-safe dynamic imports
• ✅ Clean component composition with higher-order patterns
• ✅ Consistent use of Zustand patterns with actions/state separation
• ✅ Good TypeScript usage throughout
• ✅ Proper cleanup with disposables pattern

🚀 Recommendations Summary

1. Fix the race condition with Promise-based initialization (Critical)
2. Add error handling to the async initialization (Critical)
3. Move initialization state into the store for consistency (Medium)
4. Add retry/timeout logic for better resilience (Low)
5. Consider creating a changeset as noted by the bot

This is a well-architected lazy-loading implementation that will significantly improve the initial bundle size. The critical issues around race conditions and error handling should be addressed before merging to ensure production stability.

[Copilot]

Pull request overview

This PR implements lazy-loading for Monaco Editor to improve initial page load performance and reduce bundle size. The changes consolidate Monaco initialization into a single import function, update related packages, and refactor state management to separate concerns.

Key changes:

• Monaco Editor and Shiki packages upgraded to latest versions (monaco-editor@0.55.1, shiki@3.20.0, @shikijs/monaco@3.20.0)
• Lazy-loading implementation via importMonaco() function that dynamically imports Monaco and dependencies only when needed
• State management refactored to move variableSuggestions from monaco-store to agent-store for better separation of concerns

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
pnpm-lock.yaml	Updates Monaco Editor (0.54.0 → 0.55.1), Shiki (3.13.0 → 3.20.0), and @shikijs/monaco dependencies with their transitive dependencies
agents-manage-ui/package.json	Updates package dependencies to match lock file versions
agents-manage-ui/src/features/agent/state/use-monaco-store.ts	Refactors Monaco initialization into single importMonaco() function that lazy-loads all dependencies, consolidates theme setup, and removes separate highlighter/theme methods
agents-manage-ui/src/features/agent/state/use-agent-store.ts	Adds variableSuggestions state and setVariableSuggestions action moved from monaco-store
agents-manage-ui/src/components/editors/monaco-editor.tsx	Adds importMonaco() call on mount, removes theme handling logic (now in monaco-store), and simplifies component styling
agents-manage-ui/src/components/agent/copilot/components/code-diff.tsx	Adds importMonaco() call on mount and removes theme handling logic
agents-manage-ui/src/components/editors/prompt-editor.tsx	Updates to use agentStore for variableSuggestions instead of monacoStore
agents-manage-ui/src/components/theme-toggle.tsx	Adds theme switching logic to update Monaco theme when system theme changes
agents-manage-ui/src/lib/monaco-editor/monaco-utils.ts	Updates token type matching from specific JSON types to generic types (constant, string)
agents-manage-ui/src/lib/tests/monaco-utils.test.tsx	Updates test to initialize Monaco before describe block and uses file snapshot for tokenization test
agents-manage-ui/src/lib/tests/markers-tokenize.snapshot	New snapshot file for tokenization output with updated token types
agents-manage-ui/src/constants/theme.ts	Renames Monaco theme constants from 'github-light-default'/'github-dark-default' to 'inkeep-light'/'inkeep-dark'
agents-manage-ui/src/app/[tenantId]/layout.tsx	Removes Monaco initialization and theme setup (now handled in components), simplifies to just layout wrapper
agents-manage-ui/src/app/[tenantId]/projects/[projectId]/agents/[agentId]/layout.tsx	Moves context suggestions logic from parent layout, adds setVariableSuggestions call
agents-manage-ui/src/components/agent/copilot/components/diff-viewer.tsx	Removes commented-out import
agents-manage-ui/cypress/support/commands.ts	Updates assertMonacoContent to use Monaco API directly instead of parsing DOM content
.changeset/nine-zoos-sneeze.md	Changeset documenting the fixes and lazy-loading feature

Files not reviewed (1)

• pnpm-lock.yaml: Language not supported

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The import assertion syntax using with may not be supported in all environments. The JSON import with { with: { type: 'json' } } is a Stage 3 proposal that requires appropriate TypeScript and bundler configuration. Verify that your build tooling (Next.js/Webpack/etc.) supports this syntax, or consider using a traditional import or require statement for the JSON file to ensure compatibility.

[Copilot]

The variableSuggestions array is duplicated in the state initialization. It's defined in initialAgentState at line 150 and then again in the state creator at line 158. This duplication could lead to confusion and maintenance issues. Since initialAgentState is spread into the state creator, the duplicate assignment is redundant.

[Copilot]

The module-level wasInitialized flag creates a potential race condition when multiple components call importMonaco() simultaneously. If two components call this method before the first one completes, both will pass the if (wasInitialized) check and proceed to import, causing duplicate initialization.

Consider using a Promise-based approach or a more robust synchronization mechanism to ensure only one initialization occurs, similar to how the old code used highlighterPromise.

[Copilot]

The useEffect will try to set the Monaco theme even before Monaco is loaded (when monaco is null). While the optional chaining protects against errors, this creates unnecessary re-renders and effect executions. Consider adding an early return if monaco is null, or only setting up the effect once Monaco is available.

[Copilot]

The describe callback should not be async. According to Jest/Vitest best practices, test suite callbacks (describe, beforeEach, afterEach) should be synchronous. The async initialization of Monaco should be moved into a beforeAll hook instead.

This will prevent potential race conditions and ensure proper test suite initialization.

[amikofalvy]

@dimaMachina Is this related to lazy load, is it required for the package upgrades or is it auxiliary?

It's fine to keep this in this PR in any case, but I would like to know the reason to make sure this is desired behavior.

[dimaMachina]

it's for unit test of copy field value feature in readonly json editor

tests pass, it's safe

agents/agents-manage-ui/src/lib/__tests__/monaco-utils.test.tsx

Lines 115 to 132 in 4525c03

	const expectedContentWithDecorations = `{
	"null": null,❌
	"number": 1,❌
	"boolean": false,❌
	"array": [
	true,❌
	{
	"foo": "bar"❌
	},
	[
	2,❌
	"baz"❌
	]
	],
	"string": "hello",❌
	"emptyString": "",❌
	"multipleQuotes": "[\\"/Users/Inkeep/.fnm/node-versions/v22.20.0/installation/bin/node\\",\\"/Users/Inkeep/Desktop/agents/agents-run-api/node_modules/vite/bin/vite.js\\"]"❌
	}`;