feat: adding max-depth and file thresholds#2446
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThe include configuration is refactored from a string array to an object { paths, maxDepth, fileThreshold } across config schema, docs, project registration, workspace loading, filesystem traversal, tests, and JSON schema. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
💤 Files with no reviewable changes (2)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx,json,md}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧬 Code graph analysis (1)packages/diagram/src/likec4diagram/state/machine.state.navigating.ts (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (1)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/docs/src/content/docs/dsl/Config/multi-projects.mdx (1)
73-78: Inconsistentincludesyntax in documentation.This example shows the array syntax
"include": ["../shared"], while examples at lines 100-103 and 110-112 show the new object syntax"include": { "paths": [...] }.If both formats are supported (backward compatibility), please add a note clarifying this. Otherwise, update this example to match the object syntax:
- "include": ["../shared"] + "include": { + "paths": ["../shared"] + }
🧹 Nitpick comments (3)
packages/language-server/src/filesystem/LikeC4FileSystem.ts (1)
43-60: Verify behavior whenmaxDepthis 0.The implementation correctly defaults to
Infinityand applies depth limiting conditionally. However, if a user passesmaxDepth: 0, it will be applied viawithMaxDepth(0).Depending on
fdir's behavior, this could either return no results or behave unexpectedly. IfmaxDepthshould be at least 1 (as suggested by the documentation range "1-20"), consider adding validation or clamping:- const maxDepth = opts?.maxDepth ?? Infinity + const maxDepth = opts?.maxDepth ?? Infinity + const effectiveMaxDepth = typeof maxDepth === 'number' && maxDepth < 1 ? 1 : maxDepthAlternatively, validation at the schema level may already prevent this.
schemas/likec4-config.schema.json (1)
55-57: Consider using$idinstead ofidfor JSON Schema 2020-12 compliance.In JSON Schema draft 2020-12 (as referenced in line 2), the identifier keyword is
$id, notid. Theidkeyword was used in older drafts (draft-04 and earlier)."include-config": { - "id": "include-config", + "$id": "include-config", "description": "Configuration for including additional LikeC4 source files",packages/config/src/schema.include.ts (1)
38-48: Consider adding a maximum constraint to fileThreshold.The JSON schema defines
maximum: 9007199254740991for fileThreshold, but the Zod schema has no upper bound. While this is unlikely to cause issues in practice, adding.max(Number.MAX_SAFE_INTEGER)would maintain consistency between the schemas.fileThreshold: z.number() .int() .min(1) + .max(Number.MAX_SAFE_INTEGER) .default(30) .meta({
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/docs/src/content/docs/dsl/Config/multi-projects.mdx(1 hunks)packages/config/src/index.ts(1 hunks)packages/config/src/schema.include.ts(1 hunks)packages/config/src/schema.spec.ts(8 hunks)packages/language-server/src/filesystem/LikeC4FileSystem.ts(2 hunks)packages/language-server/src/filesystem/index.ts(1 hunks)packages/language-server/src/workspace/ProjectsManager.spec.ts(9 hunks)packages/language-server/src/workspace/ProjectsManager.ts(9 hunks)packages/language-server/src/workspace/WorkspaceManager.ts(1 hunks)schemas/likec4-config.schema.json(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
pnpm ci:typecheckfor TypeScript type checking across all packages except documentation
Files:
packages/language-server/src/filesystem/LikeC4FileSystem.tspackages/language-server/src/workspace/WorkspaceManager.tspackages/language-server/src/filesystem/index.tspackages/config/src/index.tspackages/language-server/src/workspace/ProjectsManager.spec.tspackages/config/src/schema.spec.tspackages/config/src/schema.include.tspackages/language-server/src/workspace/ProjectsManager.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use oxlint with type-aware rules for linting instead of eslint, and run
pnpm ci:lintto validate all packages except documentation
Files:
packages/language-server/src/filesystem/LikeC4FileSystem.tspackages/language-server/src/workspace/WorkspaceManager.tspackages/language-server/src/filesystem/index.tspackages/config/src/index.tspackages/language-server/src/workspace/ProjectsManager.spec.tspackages/config/src/schema.spec.tspackages/config/src/schema.include.tspackages/language-server/src/workspace/ProjectsManager.ts
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
pnpm fmtto format code with dprint instead of Prettier or eslint for formatting
Files:
packages/language-server/src/filesystem/LikeC4FileSystem.tspackages/language-server/src/workspace/WorkspaceManager.tspackages/language-server/src/filesystem/index.tspackages/config/src/index.tspackages/language-server/src/workspace/ProjectsManager.spec.tsschemas/likec4-config.schema.jsonpackages/config/src/schema.spec.tspackages/config/src/schema.include.tspackages/language-server/src/workspace/ProjectsManager.ts
packages/*/**/!(node_modules)/**/*.spec.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Unit tests must be located in
packages/*/src/**/*.spec.tsorpackages/*/src/**/__test__/*.spec.tsfiles
Files:
packages/language-server/src/workspace/ProjectsManager.spec.tspackages/config/src/schema.spec.ts
🧠 Learnings (3)
📚 Learning: 2025-12-05T15:36:11.968Z
Learnt from: CR
Repo: likec4/likec4 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T15:36:11.968Z
Learning: Applies to packages/*/**/!(node_modules)/**/*.spec.ts : Unit tests must be located in `packages/*/src/**/*.spec.ts` or `packages/*/src/**/__test__/*.spec.ts` files
Applied to files:
packages/language-server/src/workspace/ProjectsManager.spec.tspackages/config/src/schema.spec.ts
📚 Learning: 2025-12-05T15:36:11.968Z
Learnt from: CR
Repo: likec4/likec4 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T15:36:11.968Z
Learning: Applies to e2e/{tests,src}/**/*.spec.ts : E2E tests must be located in `e2e/tests/*.spec.ts` or `e2e/src/*.spec.ts` files
Applied to files:
packages/language-server/src/workspace/ProjectsManager.spec.ts
📚 Learning: 2025-12-05T15:36:11.968Z
Learnt from: CR
Repo: likec4/likec4 PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-05T15:36:11.968Z
Learning: Applies to {packages/language-server/src/generated/**/*,packages/vscode/src/meta.ts,packages/likec4/app/src/routeTree.gen.ts,styled-system/preset/src/generated.ts} : Generated files in `packages/language-server/src/generated/`, `packages/vscode/src/meta.ts`, `packages/likec4/app/src/routeTree.gen.ts`, and `styled-system/preset/src/generated.ts` must be regenerated via `pnpm generate` and should not be manually edited
Applied to files:
packages/language-server/src/workspace/ProjectsManager.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Agent
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: checks / 🛠️ build
- GitHub Check: checks / ⊞ windows build
🔇 Additional comments (23)
packages/config/src/index.ts (1)
12-14: LGTM! Clean public API surface expansion.The new exports follow the existing file organization pattern—types grouped separately from value exports. This properly exposes the
IncludeConfigtype and associated normalization/validation utilities for downstream consumers.Also applies to: 23-23
packages/language-server/src/workspace/WorkspaceManager.ts (2)
77-95: LGTM! Good integration of includeConfig with depth-limited traversal.The destructuring of
includeConfigand passingmaxDepthtoreadDirectorycleanly integrates the new configuration. Per-path file counting with debug logging provides good observability for troubleshooting.
101-115: Verify the minimum-threshold design choice is intentional.Using
Math.min()across all include paths means the most restrictivefileThresholdgoverns the warning. This is a safe/conservative approach, but may surprise users if one project has a low threshold while others are higher.Consider whether per-project threshold checking would provide more accurate feedback, or document this cross-project behavior.
packages/language-server/src/filesystem/index.ts (1)
27-31: LGTM! Backwards-compatible interface extension.The optional
maxDepthparameter with clear JSDoc documentation extends the interface without breaking existing consumers. The default behavior (Infinity) preserves previous functionality.apps/docs/src/content/docs/dsl/Config/multi-projects.mdx (1)
116-139: LGTM! Clear documentation for the new configuration options.The new section provides clear documentation of the object-based
includeconfiguration with sensible defaults, valid ranges, and helpful performance optimization tips. The asides provide good context for users.schemas/likec4-config.schema.json (2)
77-83: The fileThreshold maximum is set toNumber.MAX_SAFE_INTEGER.The maximum value of
9007199254740991effectively means no practical upper limit. This is reasonable for avoiding artificial constraints while still providing schema validation.
33-36: Well-structured include-config schema definition.The new object-based include configuration is well-documented with clear descriptions, sensible defaults, and appropriate constraints. The required
pathsarray withminItems: 1ensures users must provide at least one path when using this configuration.Also applies to: 55-88
packages/config/src/schema.spec.ts (4)
184-209: Tests properly validate the new object-based include configuration.The test cases comprehensively cover valid include paths using the new
{ paths: [...] }structure and correctly usetoMatchObjectfor partial matching of the result.
212-215: Good validation of empty paths rejection at schema level.This test ensures that
{ paths: [] }is rejected with the appropriate error message during schema parsing, enforcing theminItems: 1constraint.
537-598: IncludeSchema tests correctly updated for object-based configuration.The tests properly exercise the new schema shape including valid paths, empty array rejection, undefined handling, and path validation (absolute paths, URLs, relative paths).
601-657: Comprehensive validateIncludePaths tests with new object shape.The tests cover all validation scenarios including undefined handling, empty paths (which passes validation since it's post-normalization), and invalid path detection with proper error messages.
packages/language-server/src/workspace/ProjectsManager.spec.ts (4)
340-360: Include path resolution tests properly updated.The tests correctly verify that include paths are resolved relative to the project folder using the new
{ paths: [...] }configuration format.
410-422: Empty include paths validation test correctly expects rejection.This test ensures that attempting to register a project with
include: { paths: [] }throws an error, which aligns with the schema'sminItems: 1constraint.
448-478: getAllIncludePaths tests properly verify includeConfig propagation.The tests confirm that each include path entry now includes the full
includeConfigobject withpaths,maxDepth, andfileThresholdvalues. This enables consumers to access configuration details for each include path.
502-531: Project reload test validates config change handling.The test demonstrates that changing the project name during reload (
project1→project3) correctly re-registers the project and updates include paths. This is an important behavior verification for configuration hot-reloading scenarios.packages/language-server/src/workspace/ProjectsManager.ts (5)
71-75: Well-documented includeConfig field addition.The new
includeConfigfield properly documents its purpose and stores the normalized include configuration withmaxDepthandfileThresholdvalues that can be used by consumers.
87-95: DefaultProject includes sensible includeConfig defaults.The default values (
paths: [],maxDepth: 3,fileThreshold: 30) align with the schema defaults and ensure the default project has a consistent configuration structure.
345-352: Consistent normalization of include configuration during registration.The
normalizeIncludeConfigfunction is correctly called both when creating new projects and when updating existing ones, ensuring defaults are always applied.Also applies to: 378-380
392-400: Include path resolution uses normalized configuration.The path resolution logic correctly accesses
project.includeConfig.pathsand resolves each path relative to the project folder. The null check withhasAtLeastensures safe iteration.
649-671: getAllIncludePaths correctly exposes includeConfig to consumers.This enables the WorkspaceManager to access
maxDepthandfileThresholdwhen scanning include directories, which is essential for implementing the depth limiting and file threshold warning features. WorkspaceManager actively usesmaxDepth(line 82) andfileThreshold(line 104) from the returnedincludeConfigfor these features.packages/config/src/schema.include.ts (3)
16-54: Well-structured IncludeConfigSchema with clear metadata.The schema correctly defines the object structure with appropriate constraints. The
.meta()usage for descriptions aligns with Zod 4 patterns documented in the library context.
68-73: normalizeIncludeConfig correctly provides consistent defaults.This function ensures that consumers always receive a complete
IncludeConfigobject with defaults, avoiding null checks throughout the codebase. The default values match those inDefaultProject.includeConfig.
75-97: validateIncludePaths correctly handles the new object structure.The function safely handles undefined input and validates each path in the
include.pathsarray. The error message is clear and actionable.
There was a problem hiding this comment.
Pull request overview
This PR adds performance control features to the include configuration by introducing maxDepth and fileThreshold parameters. The change transitions the include field from a simple array of paths (string[]) to a configuration object ({ paths: string[], maxDepth?: number, fileThreshold?: number }), enabling users to limit directory traversal depth and receive warnings when too many files are loaded from include paths.
Key changes:
- Schema updated to use object-based include configuration with
paths,maxDepth(default: 3, range: 1-20), andfileThreshold(default: 30) - FileSystemProvider enhanced to accept
maxDepthparameter for directory scanning - WorkspaceManager now enforces maxDepth limits and warns when file threshold is exceeded
- Documentation updated with new configuration format and performance optimization guidance
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| schemas/likec4-config.schema.json | JSON schema updated to define include as object with paths, maxDepth, and fileThreshold properties |
| packages/config/src/schema.include.ts | Replaced simple array schema with IncludeConfigSchema object; added normalizeIncludeConfig helper |
| packages/config/src/index.ts | Exported new IncludeConfig type and normalizeIncludeConfig function |
| packages/config/src/schema.spec.ts | Updated all tests to use new object format; changed expectation for empty array handling |
| packages/language-server/src/filesystem/index.ts | Added maxDepth parameter to readDirectory interface |
| packages/language-server/src/filesystem/LikeC4FileSystem.ts | Implemented maxDepth support using fdir's withMaxDepth method |
| packages/language-server/src/workspace/ProjectsManager.ts | Added includeConfig field to ProjectData; normalized include config using helper function |
| packages/language-server/src/workspace/ProjectsManager.spec.ts | Updated test fixtures to use new include object format |
| packages/language-server/src/workspace/WorkspaceManager.ts | Added file counting and threshold warning logic when loading from include paths |
| apps/docs/src/content/docs/dsl/Config/multi-projects.mdx | Added documentation section explaining new include configuration options |
| // Warn if total files loaded exceeds threshold across all include paths | ||
| if (includePaths.length > 0 && totalFilesLoaded > 0) { | ||
| // Get the minimum threshold from all projects | ||
| const minThreshold = Math.min(...includePaths.map(p => p.includeConfig.fileThreshold)) | ||
|
|
||
| if (totalFilesLoaded > minThreshold) { | ||
| logger.warn( | ||
| `Loaded ${totalFilesLoaded} files from include paths (threshold: ${minThreshold}). ` + | ||
| 'Large include directories may slow workspace initialization. ' + | ||
| 'Consider adjusting "include.fileThreshold" or "include.maxDepth" in your project configuration.', | ||
| ) | ||
| } else { | ||
| logger.info`loaded ${totalFilesLoaded} total files from ${includePaths.length} include paths` | ||
| } |
There was a problem hiding this comment.
The warning logic is confusing because it compares total files loaded across all projects against the minimum threshold from any single project. This can produce misleading warnings.
For example:
- Project A (threshold: 30) loads 20 files
- Project B (threshold: 50) loads 25 files
- Total: 45 files, but the warning will use minThreshold=30, making it seem like the limit was exceeded when both projects are actually under their individual thresholds.
Consider one of these approaches:
Option 1: Track and warn per-project:
for (const { projectId, includePath, includeConfig } of includePaths) {
// ... load files ...
if (filesLoadedFromPath > includeConfig.fileThreshold) {
logger.warn(
`Project "${projectId}" loaded ${filesLoadedFromPath} files from ${includePath.fsPath} ` +
`(threshold: ${includeConfig.fileThreshold}). Consider adjusting "include.fileThreshold" or "include.maxDepth".`
)
}
}Option 2: Group by project and check totals per project:
const filesPerProject = new Map<string, number>()
// After loading all files, check each project's total
for (const [projectId, count] of filesPerProject) {
const threshold = projects.get(projectId).includeConfig.fileThreshold
if (count > threshold) { /* warn */ }
}| // Reload with different include paths | ||
| await projectsManager.registerProject({ | ||
| config: { | ||
| name: 'project1', | ||
| include: ['../new-shared'], | ||
| name: 'project3', | ||
| include: { paths: ['../new-shared'] }, | ||
| }, | ||
| folderUri, | ||
| }) | ||
|
|
||
| project = projectsManager.getProject('project1' as ProjectId) | ||
| project = projectsManager.getProject('project3' as ProjectId) |
There was a problem hiding this comment.
The project name was changed from 'project1' to 'project3' during the reload, which changes the test's purpose. The test name says "should update include paths when project is reloaded with different config", suggesting it should test updating the same project's configuration, not registering a different project at the same location.
This should remain 'project1' to accurately test the reload scenario:
// Reload with different include paths
await projectsManager.registerProject({
config: {
name: 'project1', // Keep the same name
include: { paths: ['../new-shared'] },
},
folderUri,
})
project = projectsManager.getProject('project1' as ProjectId)| it('should pass when include has empty paths array', ({ expect }) => { | ||
| expect(() => validateIncludePaths({ paths: [], maxDepth: 3, fileThreshold: 30 })).not.toThrow() | ||
| }) |
There was a problem hiding this comment.
[nitpick] This test is misleading because it tests validateIncludePaths with { paths: [] }, which would never pass Zod schema validation in the first place. The IncludeConfigSchema has .min(1) for the paths array (line 18 in schema.include.ts), so Zod rejects empty arrays before validateIncludePaths is called.
The validateIncludePaths function is designed to be called after Zod validation (see schema.ts line 253), so this test validates behavior that shouldn't occur in practice.
Consider either:
- Removing this test since empty arrays are caught by Zod validation
- Or changing the test expectation to verify it properly handles pre-validated input:
it('should skip validation when include has empty paths array (already validated by Zod)', ({ expect }) => {
// This scenario shouldn't happen in practice, but validateIncludePaths handles it gracefully
expect(() => validateIncludePaths({ paths: [], maxDepth: 3, fileThreshold: 30 })).not.toThrow()
})- Remove unused SyncLayoutActorRef import - Fix event.xynodes to eventWithXYData.xynodes reference - Remove deprecated zIndexMode prop from ReactFlow
Checklist
mainbefore creating this PR.Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.