Backend node refactoring#39
Conversation
Adds a suite of new nodes for working with dates and times: - data_datetime_literal: Creates a DateTime object from a string. - time_now: Gets the current date and time. - time_format: Formats a DateTime object into a string. - time_add: Adds a duration to a DateTime object. - time_diff: Calculates the difference between two DateTime objects. - time_compare: Compares two DateTime objects. Also adds the 'date-fns' dependency to handle date logic. All new nodes are registered in the NodeRegistry. This commit also addresses feedback from the code review, adding validation for `Invalid Date` objects and error handling to improve robustness.
Adds a suite of new nodes for working with dates and times: - data_datetime_literal: Creates a DateTime object from a string. - time_now: Gets the current date and time. - time_format: Formats a DateTime object into a string. - time_add: Adds a duration to a DateTime object. - time_diff: Calculates the difference between two DateTime objects. - time_compare: Compares two DateTime objects. Also adds the 'date-fns' dependency to handle date logic. All new nodes are registered in the NodeRegistry. This commit also addresses feedback from code reviews by: - Adding validation for `Invalid Date` objects across all relevant nodes. - Improving error handling to be more explicit, throwing errors or returning descriptive messages instead of default values. - Fixing the `time:compare` node by adding a dedicated input pin for the comparison operation and updating its implementation.
Adds a suite of new nodes for working with dates and times: - data_datetime_literal: Creates a DateTime object from a string. - time_now: Gets the current date and time. - time_format: Formats a DateTime object into a string. - time_add: Adds a duration to a DateTime object. - time_diff: Calculates the difference between two DateTime objects. - time_compare: Compares two DateTime objects. Also adds the 'date-fns' dependency to handle date logic. All new nodes are registered in the NodeRegistry. This commit also addresses feedback from code reviews by: - Adding validation for `Invalid Date` objects across all relevant nodes. - Improving error handling to be more explicit, throwing errors or returning descriptive messages instead of default values. - Fixing the `time:compare` node by adding a dedicated input pin for the comparison operation and updating its implementation. - Unifying error handling in `time_format.js` to throw exceptions for consistency.
|
Caution Review failedThe pull request is closed. 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. 📝 WalkthroughWalkthroughДобавлен синглтон PrismaService; введён ValidationService с Zod‑схемами; добавлены safeJsonParse и парсер переменных; регистрация узлов переведена на модульные node‑registries с множеством новых регистраций и реализаций нод; добавлены time‑ноды, debounce и автосохранение на фронтенде; обновлены зависимости и .gitignore. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Области для повышенного внимания:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
Comment |
Руководство для ревьюераЭтот PR рефакторит ядро бэкенда для улучшения модульности, централизованного доступа к базе данных и надёжной валидации/парсинга, одновременно улучшая визуальный редактор фронтенда с автосохранением на основе debounce и расширяя общее покрытие тестами и зависимости. Диаграмма последовательности для автосохранения на основе debounce в визуальном редакторе фронтендаsequenceDiagram
participant User as actor User
participant VisualEditorStore
participant Debounce
User->>VisualEditorStore: makes changes to graph
VisualEditorStore->>Debounce: autoSaveGraph()
Debounce-->>VisualEditorStore: schedules saveGraph() after delay
User->>VisualEditorStore: leaves editor page
VisualEditorStore->>Debounce: flushSaveGraph()
Debounce-->>VisualEditorStore: immediately calls saveGraph()
Диаграмма последовательности для безопасного выполнения графа с валидацией и парсингомsequenceDiagram
participant GraphExecutionEngine
participant ValidationService
participant VariableParser
GraphExecutionEngine->>ValidationService: parseGraph(graph)
ValidationService-->>GraphExecutionEngine: parsedGraph
GraphExecutionEngine->>ValidationService: validateGraphStructure(parsedGraph)
ValidationService-->>GraphExecutionEngine: validation result
alt validation passes
GraphExecutionEngine->>VariableParser: parseVariables(graph.variables)
VariableParser-->>GraphExecutionEngine: variables object
end
Диаграмма классов для нового синглтона PrismaService и его использованияclassDiagram
class PrismaService {
- prisma: PrismaClient
+ getClient()
+ disconnect()
}
PrismaService <|.. prismaService: singleton
class GraphExecutionEngine {
- prisma: PrismaClient
}
class EventGraphManager {
- prisma: PrismaClient
}
PrismaService <.. GraphExecutionEngine : uses getClient()
PrismaService <.. EventGraphManager : uses getClient()
Диаграмма классов для ValidationService и схем валидацииclassDiagram
class ValidationService {
- validationEnabled: boolean
- strictMode: boolean
+ parseGraph(graph, contextName)
+ validateGraphStructure(graph, contextName)
+ validateNode(nodeConfig, contextName)
+ hasValidBasicStructure(graph)
+ isValidationEnabled()
+ isStrictMode()
}
ValidationService <|.. validationServiceInstance: singleton
class PinSchema
class NodeConfigSchema
class NodeDataSchema
class ConnectionSchema
class GraphSchema
ValidationService ..> NodeConfigSchema : uses
ValidationService ..> GraphSchema : uses
Диаграмма классов для новых утилит парсинга переменныхclassDiagram
class variableParser_js {
+parseVariableValue(variable, contextName)
+parseVariables(variables, contextName)
+getDefaultValueForType(type)
}
class jsonParser_js {
+safeJsonParse(jsonString, defaultValue, context)
}
variableParser_js ..> jsonParser_js : uses safeJsonParse
Диаграмма классов для автосохранения на основе debounce в visualEditorStoreclassDiagram
class debounce {
+debounce(func, wait)
+flush()
}
class visualEditorStore {
+autoSaveGraph()
+flushSaveGraph()
-debouncedSaveGraph
}
visualEditorStore ..> debounce : uses
Изменения на уровне файлов
Советы и командыВзаимодействие с Sourcery
Настройка вашего опытаПолучите доступ к вашей панели управления, чтобы:
Получение помощи
Original review guide in EnglishReviewer's GuideThis PR refactors the backend core for improved modularity, centralized database access, and robust validation/parsing, while enhancing the frontend visual editor with debounce-based auto-save and expanding overall test coverage and dependencies. Sequence diagram for debounced auto-save in frontend visual editorsequenceDiagram
participant User as actor User
participant VisualEditorStore
participant Debounce
User->>VisualEditorStore: makes changes to graph
VisualEditorStore->>Debounce: autoSaveGraph()
Debounce-->>VisualEditorStore: schedules saveGraph() after delay
User->>VisualEditorStore: leaves editor page
VisualEditorStore->>Debounce: flushSaveGraph()
Debounce-->>VisualEditorStore: immediately calls saveGraph()
Sequence diagram for safe graph execution with validation and parsingsequenceDiagram
participant GraphExecutionEngine
participant ValidationService
participant VariableParser
GraphExecutionEngine->>ValidationService: parseGraph(graph)
ValidationService-->>GraphExecutionEngine: parsedGraph
GraphExecutionEngine->>ValidationService: validateGraphStructure(parsedGraph)
ValidationService-->>GraphExecutionEngine: validation result
alt validation passes
GraphExecutionEngine->>VariableParser: parseVariables(graph.variables)
VariableParser-->>GraphExecutionEngine: variables object
end
Class diagram for new PrismaService singleton and its usageclassDiagram
class PrismaService {
- prisma: PrismaClient
+ getClient()
+ disconnect()
}
PrismaService <|.. prismaService: singleton
class GraphExecutionEngine {
- prisma: PrismaClient
}
class EventGraphManager {
- prisma: PrismaClient
}
PrismaService <.. GraphExecutionEngine : uses getClient()
PrismaService <.. EventGraphManager : uses getClient()
Class diagram for ValidationService and validation schemasclassDiagram
class ValidationService {
- validationEnabled: boolean
- strictMode: boolean
+ parseGraph(graph, contextName)
+ validateGraphStructure(graph, contextName)
+ validateNode(nodeConfig, contextName)
+ hasValidBasicStructure(graph)
+ isValidationEnabled()
+ isStrictMode()
}
ValidationService <|.. validationServiceInstance: singleton
class PinSchema
class NodeConfigSchema
class NodeDataSchema
class ConnectionSchema
class GraphSchema
ValidationService ..> NodeConfigSchema : uses
ValidationService ..> GraphSchema : uses
Class diagram for new variable parsing utilitiesclassDiagram
class variableParser_js {
+parseVariableValue(variable, contextName)
+parseVariables(variables, contextName)
+getDefaultValueForType(type)
}
class jsonParser_js {
+safeJsonParse(jsonString, defaultValue, context)
}
variableParser_js ..> jsonParser_js : uses safeJsonParse
Class diagram for debounce-based auto-save in visualEditorStoreclassDiagram
class debounce {
+debounce(func, wait)
+flush()
}
class visualEditorStore {
+autoSaveGraph()
+flushSaveGraph()
-debouncedSaveGraph
}
visualEditorStore ..> debounce : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Привет! Я просмотрел ваши изменения — вот несколько замечаний:
- Ваша Zod-схема для
nodeConfigне включаетpins,graphTypeилиdynamicPins, поэтомуvalidateNodeConfigбудет отклонять действительные конфигурации — обновите схему, чтобы она соответствовала фактической форме конфигурации. - Флаг
validationEnabled(process.env.NODE_ENV !== 'production') дублируется в нескольких классах — рассмотрите возможность извлечения его в общий конфиг/утилиту для устранения дублирования кода. - Магическое значение
MAX_DEPTH = 100вisNodeVolatileжестко закодировано — сделайте это ограничение настраиваемым или задокументируйте, почему было выбрано 100, для лучшей поддерживаемости.
Подсказка для AI-агентов
Пожалуйста, учтите комментарии из этого обзора кода:
## Общие комментарии
- Ваша Zod-схема для `nodeConfig` не включает `pins`, `graphType` или `dynamicPins`, поэтому `validateNodeConfig` будет отклонять действительные конфигурации — обновите схему, чтобы она соответствовала фактической форме конфигурации.
- Флаг `validationEnabled` (`process.env.NODE_ENV !== 'production'`) дублируется в нескольких классах — рассмотрите возможность извлечения его в общий конфиг/утилиту для устранения дублирования кода.
- Магическое значение `MAX_DEPTH = 100` в `isNodeVolatile` жестко закодировано — сделайте это ограничение настраиваемым или задокументируйте, почему было выбрано 100, для лучшей поддерживаемости.
## Индивидуальные комментарии
### Комментарий 1
<location> `backend/src/core/GraphExecutionEngine.js:65-66` </location>
<code_context>
break;
case 'array':
- value = JSON.parse(value);
+ value = safeJsonParse(value, [], `variable ${variable.name}`);
+ if (!Array.isArray(value)) value = [];
break;
}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Использование пустого массива по умолчанию при ошибке парсинга может маскировать проблемы с данными.
Рассмотрите возможность добавления предупреждения или записи в журнал ошибок при возврате к пустому массиву, чтобы помочь выявить проблемы с исходными данными.
```suggestion
value = safeJsonParse(value, [], `variable ${variable.name}`);
if (!Array.isArray(value)) {
console.warn(`[GraphExecutionEngine] Failed to parse array for variable "${variable.name}". Falling back to empty array. Raw value:`, value);
value = [];
}
```
</issue_to_address>
### Комментарий 2
<location> `backend/src/core/EventGraphManager.js:47-50` </location>
<code_context>
- default: val = v.value;
- }
- initialState[v.name] = val;
+ const parsedVars = safeJsonParse(graph.variables, [], `EventGraph ID ${graph.id} variables`);
+ for (const v of parsedVars) {
+ let val;
+ switch (v.type) {
+ case 'number': val = Number(v.value) || 0; break;
+ case 'boolean': val = v.value === 'true'; break;
+ case 'array':
+ val = safeJsonParse(v.value, [], `variable ${v.name}`);
+ if (!Array.isArray(val)) val = [];
+ break;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Тихое возвращение к пустому массиву при ошибке парсинга переменной может скрывать проблемы.
Запись предупреждения, когда переменная не может быть разобрана как массив, облегчит выявление и устранение проблем с данными.
```suggestion
case 'array':
val = safeJsonParse(v.value, [], `variable ${v.name}`);
if (!Array.isArray(val)) {
console.warn(
`[EventGraphManager] Failed to parse variable "${v.name}" as array. Value:`,
v.value
);
val = [];
}
break;
```
</issue_to_address>
### Комментарий 3
<location> `frontend/src/stores/visualEditorStore.js:16-17` </location>
<code_context>
export const useVisualEditorStore = create(
- immer((set, get) => ({
+ immer((set, get) => {
+ const debouncedSaveGraph = debounce(() => {
+ get().saveGraph();
+ }, 1000);
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Реализация Debounce может привести к пропущенным сохранениям, если хранилище размонтировано или заменено.
Если хранилище размонтировано или заменено до завершения таймера debounce, ожидающие сохранения могут быть потеряны. Реализуйте логику очистки или сброса, чтобы гарантировать обработку всех сохранений.
Предлагаемая реализация:
```javascript
export const useVisualEditorStore = create(
immer((set, get) => {
// Track the debounced function so we can flush it on cleanup
const debouncedSaveGraph = debounce(() => {
get().saveGraph();
}, 1000);
// Cleanup/flush logic to ensure pending saves are processed
const flushPendingSaves = () => {
if (debouncedSaveGraph.flush) {
debouncedSaveGraph.flush();
} else {
// If debounce implementation does not have flush, call immediately
get().saveGraph();
}
};
// Zustand supports returning a cleanup function from the store initializer
// This will be called when the store is unmounted/replaced
// See: https://docs.pmnd.rs/zustand/integrations/cleanup
return {
nodes: [],
edges: [],
debouncedSaveGraph,
flushPendingSaves,
// Zustand will call this when the store is disposed
// If your Zustand version does not support this, you may need to handle cleanup elsewhere
// __unstable_onDestroy: flushPendingSaves,
};
```
1. Если ваша реализация debounce (в '@/lib/debounce') не поддерживает метод `.flush()`, вам может потребоваться улучшить ее, чтобы предоставить эту функциональность.
2. Если вы используете `onDestroy` Zustand или аналогичные хуки очистки, убедитесь, что вы правильно зарегистрировали `flushPendingSaves`. В противном случае вам может потребоваться вручную вызывать `flushPendingSaves`, когда вы знаете, что хранилище заменяется/размонтируется.
3. Если вы хотите гарантировать очистку, рассмотрите возможность экспорта и вызова `flushPendingSaves` из логики размонтирования вашего приложения.
</issue_to_address>
### Комментарий 4
<location> `backend/src/core/validation/nodeSchemas.js:53-65` </location>
<code_context>
+
+function validateNodeConfig(nodeConfig) {
+ try {
+ return {
+ success: true,
+ data: NodeConfigSchema.parse(nodeConfig),
+ };
+ } catch (error) {
+ return {
+ success: false,
+ error: error.issues || error.errors || [{ message: error.message }],
+ };
+ }
</code_context>
<issue_to_address>
**suggestion:** Отчетность об ошибках в функциях валидации может быть непоследовательной.
Нормализуйте структуру объекта ошибки, чтобы обеспечить последовательную последующую обработку.
```suggestion
function validateNodeConfig(nodeConfig) {
try {
return {
success: true,
data: NodeConfigSchema.parse(nodeConfig),
};
} catch (error) {
let normalizedErrors = [];
if (Array.isArray(error.issues)) {
normalizedErrors = error.issues.map(issue => ({
message: issue.message,
path: issue.path,
code: issue.code,
}));
} else if (Array.isArray(error.errors)) {
normalizedErrors = error.errors.map(err => ({
message: err.message,
path: err.path,
code: err.code,
}));
} else {
normalizedErrors = [{
message: error.message || 'Unknown error',
}];
}
return {
success: false,
error: normalizedErrors,
};
}
}
```
</issue_to_address>
### Комментарий 5
<location> `backend/src/core/nodes/logic/__tests__/compare.test.js:66-73` </location>
<code_context>
+ expect(result).toBe(true);
+ });
+
+ it('should handle string comparison', async () => {
+ const node = { id: 'test', data: { operation: '<' } };
+ const helpers = mockHelpers('apple', 'banana');
+
+ const result = await logicCompare.evaluate(node, 'result', {}, helpers);
+
+ expect(result).toBe(true);
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Рассмотрите возможность добавления тестов для граничных случаев, таких как `null` или `undefined` значения.
Пожалуйста, включите тесты с `null` и `undefined` входными данными для проверки надежной обработки ошибок.
```suggestion
it('should handle string comparison', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers('apple', 'banana');
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
expect(result).toBe(true);
});
it('should handle null values', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers(null, 10);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
expect(result).toBe(false);
});
it('should handle undefined values', async () => {
const node = { id: 'test', data: { operation: '>' } };
const helpers = mockHelpers(undefined, 5);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
expect(result).toBe(false);
});
it('should handle both values as null', async () => {
const node = { id: 'test', data: { operation: '==' } };
const helpers = mockHelpers(null, null);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
expect(result).toBe(true);
});
it('should handle both values as undefined', async () => {
const node = { id: 'test', data: { operation: '==' } };
const helpers = mockHelpers(undefined, undefined);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
expect(result).toBe(true);
});
it('should handle one value null and one undefined', async () => {
const node = { id: 'test', data: { operation: '==' } };
const helpers = mockHelpers(null, undefined);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
expect(result).toBe(false);
});
```
</issue_to_address>
### Комментарий 6
<location> `backend/src/core/nodes/math/__tests__/operation.test.js:48-55` </location>
<code_context>
+ expect(result).toBe(5);
+ });
+
+ it('should handle division by zero', async () => {
+ const node = { id: 'test', data: { operation: '/' } };
+ const helpers = mockHelpers(10, 0);
+
+ const result = await mathOperation.evaluate(node, 'result', {}, helpers);
+
+ expect(result).toBe(0);
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Отсутствует тест для нечисловых входных значений.
Добавьте тесты со строковыми, `null` или `undefined` входными данными для проверки корректной обработки ошибок типов или приведения типов.
```suggestion
it('should handle division by zero', async () => {
const node = { id: 'test', data: { operation: '/' } };
const helpers = mockHelpers(10, 0);
const result = await mathOperation.evaluate(node, 'result', {}, helpers);
expect(result).toBe(0);
});
it('should handle string input values', async () => {
const node = { id: 'test', data: { operation: '+' } };
const helpers = mockHelpers("foo", 2);
const result = await mathOperation.evaluate(node, 'result', {}, helpers);
expect(result).toBeNaN();
});
it('should handle null input values', async () => {
const node = { id: 'test', data: { operation: '*' } };
const helpers = mockHelpers(null, 3);
const result = await mathOperation.evaluate(node, 'result', {}, helpers);
expect(result).toBeNaN();
});
it('should handle undefined input values', async () => {
const node = { id: 'test', data: { operation: '-' } };
const helpers = mockHelpers(undefined, 5);
const result = await mathOperation.evaluate(node, 'result', {}, helpers);
expect(result).toBeNaN();
});
```
</issue_to_address>
### Комментарий 7
<location> `backend/src/core/nodes/strings/__tests__/concat.test.js:24-31` </location>
<code_context>
+ expect(result).toBe('ABCD');
+ });
+
+ it('should handle empty strings', async () => {
+ const node = { id: 'test', data: { pinCount: 3 } };
+ const helpers = mockHelpers('Hello', '', 'World');
+
+ const result = await stringConcat.evaluate(node, 'result', {}, helpers);
+
+ expect(result).toBe('HelloWorld');
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Рассмотрите возможность добавления тестов для нестроковых входных значений.
Пожалуйста, включите тестовые случаи с нестроковыми входными данными, такими как числа, `null` или `undefined`, чтобы обеспечить корректную обработку.
```suggestion
it('should concatenate multiple strings', async () => {
const node = { id: 'test', data: { pinCount: 4 } };
const helpers = mockHelpers('A', 'B', 'C', 'D');
const result = await stringConcat.evaluate(node, 'result', {}, helpers);
expect(result).toBe('ABCD');
});
it('should handle non-string inputs: numbers', async () => {
const node = { id: 'test', data: { pinCount: 3 } };
const helpers = mockHelpers(1, 2, 3);
const result = await stringConcat.evaluate(node, 'result', {}, helpers);
expect(result).toBe('123');
});
it('should handle non-string inputs: null values', async () => {
const node = { id: 'test', data: { pinCount: 3 } };
const helpers = mockHelpers('A', null, 'B');
const result = await stringConcat.evaluate(node, 'result', {}, helpers);
expect(result).toBe('AnullB');
});
it('should handle non-string inputs: undefined values', async () => {
const node = { id: 'test', data: { pinCount: 3 } };
const helpers = mockHelpers('A', undefined, 'B');
const result = await stringConcat.evaluate(node, 'result', {}, helpers);
expect(result).toBe('AundefinedB');
});
```
</issue_to_address>
### Комментарий 8
<location> `backend/src/core/__tests__/PrismaService.test.js:11-16` </location>
<code_context>
+ expect(service1).toBe(service2);
+ });
+
+ it('should provide getClient method', () => {
+ const client = prismaService.getClient();
+
+ expect(client).toBeDefined();
+ expect(typeof client.$connect).toBe('function');
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Отсутствует тест для метода отключения.
Пожалуйста, добавьте тест, проверяющий, что отключение корректно закрывает соединение Prisma.
```suggestion
it('should provide getClient method', () => {
const client = prismaService.getClient();
expect(client).toBeDefined();
expect(typeof client.$connect).toBe('function');
});
it('should disconnect the Prisma client', async () => {
const client = prismaService.getClient();
const disconnectSpy = jest.spyOn(client, '$disconnect').mockResolvedValue();
await prismaService.disconnect();
expect(disconnectSpy).toHaveBeenCalled();
disconnectSpy.mockRestore();
});
```
</issue_to_address>Sourcery бесплатен для проектов с открытым исходным кодом — если вам нравятся наши обзоры, пожалуйста, поделитесь ими ✨
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- Your Zod schema for nodeConfig doesn’t include
pins,graphType, ordynamicPins, sovalidateNodeConfigwill reject valid configs—update the schema to match the actual config shape. - The
validationEnabledflag (process.env.NODE_ENV !== 'production') is duplicated in multiple classes—consider extracting it into a shared config/util to DRY up the code. - The magic
MAX_DEPTH = 100inisNodeVolatileis hardcoded—make that limit configurable or document why 100 was chosen for better maintainability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Your Zod schema for nodeConfig doesn’t include `pins`, `graphType`, or `dynamicPins`, so `validateNodeConfig` will reject valid configs—update the schema to match the actual config shape.
- The `validationEnabled` flag (`process.env.NODE_ENV !== 'production'`) is duplicated in multiple classes—consider extracting it into a shared config/util to DRY up the code.
- The magic `MAX_DEPTH = 100` in `isNodeVolatile` is hardcoded—make that limit configurable or document why 100 was chosen for better maintainability.
## Individual Comments
### Comment 1
<location> `backend/src/core/GraphExecutionEngine.js:65-66` </location>
<code_context>
break;
case 'array':
- value = JSON.parse(value);
+ value = safeJsonParse(value, [], `variable ${variable.name}`);
+ if (!Array.isArray(value)) value = [];
break;
}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Defaulting to empty array on parse failure may mask data issues.
Consider adding a warning or error log when falling back to an empty array to help identify upstream data issues.
```suggestion
value = safeJsonParse(value, [], `variable ${variable.name}`);
if (!Array.isArray(value)) {
console.warn(`[GraphExecutionEngine] Failed to parse array for variable "${variable.name}". Falling back to empty array. Raw value:`, value);
value = [];
}
```
</issue_to_address>
### Comment 2
<location> `backend/src/core/EventGraphManager.js:47-50` </location>
<code_context>
- default: val = v.value;
- }
- initialState[v.name] = val;
+ const parsedVars = safeJsonParse(graph.variables, [], `EventGraph ID ${graph.id} variables`);
+ for (const v of parsedVars) {
+ let val;
+ switch (v.type) {
+ case 'number': val = Number(v.value) || 0; break;
+ case 'boolean': val = v.value === 'true'; break;
+ case 'array':
+ val = safeJsonParse(v.value, [], `variable ${v.name}`);
+ if (!Array.isArray(val)) val = [];
+ break;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Silent fallback to empty array on variable parse failure may hide issues.
Logging a warning when a variable fails to parse as an array would make it easier to identify and address data issues.
```suggestion
case 'array':
val = safeJsonParse(v.value, [], `variable ${v.name}`);
if (!Array.isArray(val)) {
console.warn(
`[EventGraphManager] Failed to parse variable "${v.name}" as array. Value:`,
v.value
);
val = [];
}
break;
```
</issue_to_address>
### Comment 3
<location> `frontend/src/stores/visualEditorStore.js:16-17` </location>
<code_context>
export const useVisualEditorStore = create(
- immer((set, get) => ({
+ immer((set, get) => {
+ const debouncedSaveGraph = debounce(() => {
+ get().saveGraph();
+ }, 1000);
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Debounce implementation may cause missed saves if the store is unmounted or replaced.
If the store is unmounted or replaced before the debounce timer completes, pending saves may be lost. Implement cleanup or flush logic to ensure all saves are processed.
Suggested implementation:
```javascript
export const useVisualEditorStore = create(
immer((set, get) => {
// Track the debounced function so we can flush it on cleanup
const debouncedSaveGraph = debounce(() => {
get().saveGraph();
}, 1000);
// Cleanup/flush logic to ensure pending saves are processed
const flushPendingSaves = () => {
if (debouncedSaveGraph.flush) {
debouncedSaveGraph.flush();
} else {
// If debounce implementation does not have flush, call immediately
get().saveGraph();
}
};
// Zustand supports returning a cleanup function from the store initializer
// This will be called when the store is unmounted/replaced
// See: https://docs.pmnd.rs/zustand/integrations/cleanup
return {
nodes: [],
edges: [],
debouncedSaveGraph,
flushPendingSaves,
// Zustand will call this when the store is disposed
// If your Zustand version does not support this, you may need to handle cleanup elsewhere
// __unstable_onDestroy: flushPendingSaves,
};
```
1. If your debounce implementation (in '@/lib/debounce') does not support a `.flush()` method, you may need to enhance it to expose this functionality.
2. If you use Zustand's `onDestroy` or similar cleanup hooks, ensure you register `flushPendingSaves` appropriately. If not, you may need to manually call `flushPendingSaves` when you know the store is being replaced/unmounted.
3. If you want to guarantee cleanup, consider exporting and calling `flushPendingSaves` from your app's unmount logic.
</issue_to_address>
### Comment 4
<location> `backend/src/core/validation/nodeSchemas.js:53-65` </location>
<code_context>
+
+function validateNodeConfig(nodeConfig) {
+ try {
+ return {
+ success: true,
+ data: NodeConfigSchema.parse(nodeConfig),
+ };
+ } catch (error) {
+ return {
+ success: false,
+ error: error.issues || error.errors || [{ message: error.message }],
+ };
+ }
</code_context>
<issue_to_address>
**suggestion:** Error reporting in validation functions may be inconsistent.
Normalize the error object structure to ensure consistent downstream processing.
```suggestion
function validateNodeConfig(nodeConfig) {
try {
return {
success: true,
data: NodeConfigSchema.parse(nodeConfig),
};
} catch (error) {
let normalizedErrors = [];
if (Array.isArray(error.issues)) {
normalizedErrors = error.issues.map(issue => ({
message: issue.message,
path: issue.path,
code: issue.code,
}));
} else if (Array.isArray(error.errors)) {
normalizedErrors = error.errors.map(err => ({
message: err.message,
path: err.path,
code: err.code,
}));
} else {
normalizedErrors = [{
message: error.message || 'Unknown error',
}];
}
return {
success: false,
error: normalizedErrors,
};
}
}
```
</issue_to_address>
### Comment 5
<location> `backend/src/core/nodes/logic/__tests__/compare.test.js:66-73` </location>
<code_context>
+ expect(result).toBe(true);
+ });
+
+ it('should handle string comparison', async () => {
+ const node = { id: 'test', data: { operation: '<' } };
+ const helpers = mockHelpers('apple', 'banana');
+
+ const result = await logicCompare.evaluate(node, 'result', {}, helpers);
+
+ expect(result).toBe(true);
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests for edge cases such as null or undefined values.
Please include tests with null and undefined inputs to verify robust error handling.
```suggestion
it('should handle string comparison', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers('apple', 'banana');
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
expect(result).toBe(true);
});
it('should handle null values', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers(null, 10);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
expect(result).toBe(false);
});
it('should handle undefined values', async () => {
const node = { id: 'test', data: { operation: '>' } };
const helpers = mockHelpers(undefined, 5);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
expect(result).toBe(false);
});
it('should handle both values as null', async () => {
const node = { id: 'test', data: { operation: '==' } };
const helpers = mockHelpers(null, null);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
expect(result).toBe(true);
});
it('should handle both values as undefined', async () => {
const node = { id: 'test', data: { operation: '==' } };
const helpers = mockHelpers(undefined, undefined);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
expect(result).toBe(true);
});
it('should handle one value null and one undefined', async () => {
const node = { id: 'test', data: { operation: '==' } };
const helpers = mockHelpers(null, undefined);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
expect(result).toBe(false);
});
```
</issue_to_address>
### Comment 6
<location> `backend/src/core/nodes/math/__tests__/operation.test.js:48-55` </location>
<code_context>
+ expect(result).toBe(5);
+ });
+
+ it('should handle division by zero', async () => {
+ const node = { id: 'test', data: { operation: '/' } };
+ const helpers = mockHelpers(10, 0);
+
+ const result = await mathOperation.evaluate(node, 'result', {}, helpers);
+
+ expect(result).toBe(0);
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Missing test for non-numeric input values.
Add tests with string, null, or undefined inputs to verify correct handling of type errors or coercion.
```suggestion
it('should handle division by zero', async () => {
const node = { id: 'test', data: { operation: '/' } };
const helpers = mockHelpers(10, 0);
const result = await mathOperation.evaluate(node, 'result', {}, helpers);
expect(result).toBe(0);
});
it('should handle string input values', async () => {
const node = { id: 'test', data: { operation: '+' } };
const helpers = mockHelpers("foo", 2);
const result = await mathOperation.evaluate(node, 'result', {}, helpers);
expect(result).toBeNaN();
});
it('should handle null input values', async () => {
const node = { id: 'test', data: { operation: '*' } };
const helpers = mockHelpers(null, 3);
const result = await mathOperation.evaluate(node, 'result', {}, helpers);
expect(result).toBeNaN();
});
it('should handle undefined input values', async () => {
const node = { id: 'test', data: { operation: '-' } };
const helpers = mockHelpers(undefined, 5);
const result = await mathOperation.evaluate(node, 'result', {}, helpers);
expect(result).toBeNaN();
});
```
</issue_to_address>
### Comment 7
<location> `backend/src/core/nodes/strings/__tests__/concat.test.js:24-31` </location>
<code_context>
+ expect(result).toBe('ABCD');
+ });
+
+ it('should handle empty strings', async () => {
+ const node = { id: 'test', data: { pinCount: 3 } };
+ const helpers = mockHelpers('Hello', '', 'World');
+
+ const result = await stringConcat.evaluate(node, 'result', {}, helpers);
+
+ expect(result).toBe('HelloWorld');
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests for non-string input values.
Please include test cases with non-string inputs such as numbers, null, or undefined to ensure correct handling.
```suggestion
it('should concatenate multiple strings', async () => {
const node = { id: 'test', data: { pinCount: 4 } };
const helpers = mockHelpers('A', 'B', 'C', 'D');
const result = await stringConcat.evaluate(node, 'result', {}, helpers);
expect(result).toBe('ABCD');
});
it('should handle non-string inputs: numbers', async () => {
const node = { id: 'test', data: { pinCount: 3 } };
const helpers = mockHelpers(1, 2, 3);
const result = await stringConcat.evaluate(node, 'result', {}, helpers);
expect(result).toBe('123');
});
it('should handle non-string inputs: null values', async () => {
const node = { id: 'test', data: { pinCount: 3 } };
const helpers = mockHelpers('A', null, 'B');
const result = await stringConcat.evaluate(node, 'result', {}, helpers);
expect(result).toBe('AnullB');
});
it('should handle non-string inputs: undefined values', async () => {
const node = { id: 'test', data: { pinCount: 3 } };
const helpers = mockHelpers('A', undefined, 'B');
const result = await stringConcat.evaluate(node, 'result', {}, helpers);
expect(result).toBe('AundefinedB');
});
```
</issue_to_address>
### Comment 8
<location> `backend/src/core/__tests__/PrismaService.test.js:11-16` </location>
<code_context>
+ expect(service1).toBe(service2);
+ });
+
+ it('should provide getClient method', () => {
+ const client = prismaService.getClient();
+
+ expect(client).toBeDefined();
+ expect(typeof client.$connect).toBe('function');
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Missing test for disconnect method.
Please add a test verifying that disconnect correctly closes the Prisma connection.
```suggestion
it('should provide getClient method', () => {
const client = prismaService.getClient();
expect(client).toBeDefined();
expect(typeof client.$connect).toBe('function');
});
it('should disconnect the Prisma client', async () => {
const client = prismaService.getClient();
const disconnectSpy = jest.spyOn(client, '$disconnect').mockResolvedValue();
await prismaService.disconnect();
expect(disconnectSpy).toHaveBeenCalled();
disconnectSpy.mockRestore();
});
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| function validateNodeConfig(nodeConfig) { | ||
| try { | ||
| return { | ||
| success: true, | ||
| data: NodeConfigSchema.parse(nodeConfig), | ||
| }; | ||
| } catch (error) { | ||
| return { | ||
| success: false, | ||
| error: error.issues || error.errors || [{ message: error.message }], | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
suggestion: Отчетность об ошибках в функциях валидации может быть непоследовательной.
Нормализуйте структуру объекта ошибки, чтобы обеспечить последовательную последующую обработку.
| function validateNodeConfig(nodeConfig) { | |
| try { | |
| return { | |
| success: true, | |
| data: NodeConfigSchema.parse(nodeConfig), | |
| }; | |
| } catch (error) { | |
| return { | |
| success: false, | |
| error: error.issues || error.errors || [{ message: error.message }], | |
| }; | |
| } | |
| } | |
| function validateNodeConfig(nodeConfig) { | |
| try { | |
| return { | |
| success: true, | |
| data: NodeConfigSchema.parse(nodeConfig), | |
| }; | |
| } catch (error) { | |
| let normalizedErrors = []; | |
| if (Array.isArray(error.issues)) { | |
| normalizedErrors = error.issues.map(issue => ({ | |
| message: issue.message, | |
| path: issue.path, | |
| code: issue.code, | |
| })); | |
| } else if (Array.isArray(error.errors)) { | |
| normalizedErrors = error.errors.map(err => ({ | |
| message: err.message, | |
| path: err.path, | |
| code: err.code, | |
| })); | |
| } else { | |
| normalizedErrors = [{ | |
| message: error.message || 'Unknown error', | |
| }]; | |
| } | |
| return { | |
| success: false, | |
| error: normalizedErrors, | |
| }; | |
| } | |
| } |
Original comment in English
suggestion: Error reporting in validation functions may be inconsistent.
Normalize the error object structure to ensure consistent downstream processing.
| function validateNodeConfig(nodeConfig) { | |
| try { | |
| return { | |
| success: true, | |
| data: NodeConfigSchema.parse(nodeConfig), | |
| }; | |
| } catch (error) { | |
| return { | |
| success: false, | |
| error: error.issues || error.errors || [{ message: error.message }], | |
| }; | |
| } | |
| } | |
| function validateNodeConfig(nodeConfig) { | |
| try { | |
| return { | |
| success: true, | |
| data: NodeConfigSchema.parse(nodeConfig), | |
| }; | |
| } catch (error) { | |
| let normalizedErrors = []; | |
| if (Array.isArray(error.issues)) { | |
| normalizedErrors = error.issues.map(issue => ({ | |
| message: issue.message, | |
| path: issue.path, | |
| code: issue.code, | |
| })); | |
| } else if (Array.isArray(error.errors)) { | |
| normalizedErrors = error.errors.map(err => ({ | |
| message: err.message, | |
| path: err.path, | |
| code: err.code, | |
| })); | |
| } else { | |
| normalizedErrors = [{ | |
| message: error.message || 'Unknown error', | |
| }]; | |
| } | |
| return { | |
| success: false, | |
| error: normalizedErrors, | |
| }; | |
| } | |
| } |
| it('should handle string comparison', async () => { | ||
| const node = { id: 'test', data: { operation: '<' } }; | ||
| const helpers = mockHelpers('apple', 'banana'); | ||
|
|
||
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | ||
|
|
||
| expect(result).toBe(true); | ||
| }); |
There was a problem hiding this comment.
suggestion (testing): Рассмотрите возможность добавления тестов для граничных случаев, таких как null или undefined значения.
Пожалуйста, включите тесты с null и undefined входными данными для проверки надежной обработки ошибок.
| it('should handle string comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers('apple', 'banana'); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle string comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers('apple', 'banana'); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle null values', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers(null, 10); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(false); | |
| }); | |
| it('should handle undefined values', async () => { | |
| const node = { id: 'test', data: { operation: '>' } }; | |
| const helpers = mockHelpers(undefined, 5); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(false); | |
| }); | |
| it('should handle both values as null', async () => { | |
| const node = { id: 'test', data: { operation: '==' } }; | |
| const helpers = mockHelpers(null, null); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle both values as undefined', async () => { | |
| const node = { id: 'test', data: { operation: '==' } }; | |
| const helpers = mockHelpers(undefined, undefined); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle one value null and one undefined', async () => { | |
| const node = { id: 'test', data: { operation: '==' } }; | |
| const helpers = mockHelpers(null, undefined); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(false); | |
| }); |
Original comment in English
suggestion (testing): Consider adding tests for edge cases such as null or undefined values.
Please include tests with null and undefined inputs to verify robust error handling.
| it('should handle string comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers('apple', 'banana'); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle string comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers('apple', 'banana'); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle null values', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers(null, 10); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(false); | |
| }); | |
| it('should handle undefined values', async () => { | |
| const node = { id: 'test', data: { operation: '>' } }; | |
| const helpers = mockHelpers(undefined, 5); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(false); | |
| }); | |
| it('should handle both values as null', async () => { | |
| const node = { id: 'test', data: { operation: '==' } }; | |
| const helpers = mockHelpers(null, null); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle both values as undefined', async () => { | |
| const node = { id: 'test', data: { operation: '==' } }; | |
| const helpers = mockHelpers(undefined, undefined); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle one value null and one undefined', async () => { | |
| const node = { id: 'test', data: { operation: '==' } }; | |
| const helpers = mockHelpers(null, undefined); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(false); | |
| }); |
| it('should handle division by zero', async () => { | ||
| const node = { id: 'test', data: { operation: '/' } }; | ||
| const helpers = mockHelpers(10, 0); | ||
|
|
||
| const result = await mathOperation.evaluate(node, 'result', {}, helpers); | ||
|
|
||
| expect(result).toBe(0); | ||
| }); |
There was a problem hiding this comment.
suggestion (testing): Отсутствует тест для нечисловых входных значений.
Добавьте тесты со строковыми, null или undefined входными данными для проверки корректной обработки ошибок типов или приведения типов.
| it('should handle division by zero', async () => { | |
| const node = { id: 'test', data: { operation: '/' } }; | |
| const helpers = mockHelpers(10, 0); | |
| const result = await mathOperation.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(0); | |
| }); | |
| it('should handle division by zero', async () => { | |
| const node = { id: 'test', data: { operation: '/' } }; | |
| const helpers = mockHelpers(10, 0); | |
| const result = await mathOperation.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(0); | |
| }); | |
| it('should handle string input values', async () => { | |
| const node = { id: 'test', data: { operation: '+' } }; | |
| const helpers = mockHelpers("foo", 2); | |
| const result = await mathOperation.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBeNaN(); | |
| }); | |
| it('should handle null input values', async () => { | |
| const node = { id: 'test', data: { operation: '*' } }; | |
| const helpers = mockHelpers(null, 3); | |
| const result = await mathOperation.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBeNaN(); | |
| }); | |
| it('should handle undefined input values', async () => { | |
| const node = { id: 'test', data: { operation: '-' } }; | |
| const helpers = mockHelpers(undefined, 5); | |
| const result = await mathOperation.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBeNaN(); | |
| }); |
Original comment in English
suggestion (testing): Missing test for non-numeric input values.
Add tests with string, null, or undefined inputs to verify correct handling of type errors or coercion.
| it('should handle division by zero', async () => { | |
| const node = { id: 'test', data: { operation: '/' } }; | |
| const helpers = mockHelpers(10, 0); | |
| const result = await mathOperation.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(0); | |
| }); | |
| it('should handle division by zero', async () => { | |
| const node = { id: 'test', data: { operation: '/' } }; | |
| const helpers = mockHelpers(10, 0); | |
| const result = await mathOperation.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(0); | |
| }); | |
| it('should handle string input values', async () => { | |
| const node = { id: 'test', data: { operation: '+' } }; | |
| const helpers = mockHelpers("foo", 2); | |
| const result = await mathOperation.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBeNaN(); | |
| }); | |
| it('should handle null input values', async () => { | |
| const node = { id: 'test', data: { operation: '*' } }; | |
| const helpers = mockHelpers(null, 3); | |
| const result = await mathOperation.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBeNaN(); | |
| }); | |
| it('should handle undefined input values', async () => { | |
| const node = { id: 'test', data: { operation: '-' } }; | |
| const helpers = mockHelpers(undefined, 5); | |
| const result = await mathOperation.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBeNaN(); | |
| }); |
| it('should concatenate multiple strings', async () => { | ||
| const node = { id: 'test', data: { pinCount: 4 } }; | ||
| const helpers = mockHelpers('A', 'B', 'C', 'D'); | ||
|
|
||
| const result = await stringConcat.evaluate(node, 'result', {}, helpers); | ||
|
|
||
| expect(result).toBe('ABCD'); | ||
| }); |
There was a problem hiding this comment.
suggestion (testing): Рассмотрите возможность добавления тестов для нестроковых входных значений.
Пожалуйста, включите тестовые случаи с нестроковыми входными данными, такими как числа, null или undefined, чтобы обеспечить корректную обработку.
| it('should concatenate multiple strings', async () => { | |
| const node = { id: 'test', data: { pinCount: 4 } }; | |
| const helpers = mockHelpers('A', 'B', 'C', 'D'); | |
| const result = await stringConcat.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe('ABCD'); | |
| }); | |
| it('should concatenate multiple strings', async () => { | |
| const node = { id: 'test', data: { pinCount: 4 } }; | |
| const helpers = mockHelpers('A', 'B', 'C', 'D'); | |
| const result = await stringConcat.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe('ABCD'); | |
| }); | |
| it('should handle non-string inputs: numbers', async () => { | |
| const node = { id: 'test', data: { pinCount: 3 } }; | |
| const helpers = mockHelpers(1, 2, 3); | |
| const result = await stringConcat.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe('123'); | |
| }); | |
| it('should handle non-string inputs: null values', async () => { | |
| const node = { id: 'test', data: { pinCount: 3 } }; | |
| const helpers = mockHelpers('A', null, 'B'); | |
| const result = await stringConcat.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe('AnullB'); | |
| }); | |
| it('should handle non-string inputs: undefined values', async () => { | |
| const node = { id: 'test', data: { pinCount: 3 } }; | |
| const helpers = mockHelpers('A', undefined, 'B'); | |
| const result = await stringConcat.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe('AundefinedB'); | |
| }); |
Original comment in English
suggestion (testing): Consider adding tests for non-string input values.
Please include test cases with non-string inputs such as numbers, null, or undefined to ensure correct handling.
| it('should concatenate multiple strings', async () => { | |
| const node = { id: 'test', data: { pinCount: 4 } }; | |
| const helpers = mockHelpers('A', 'B', 'C', 'D'); | |
| const result = await stringConcat.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe('ABCD'); | |
| }); | |
| it('should concatenate multiple strings', async () => { | |
| const node = { id: 'test', data: { pinCount: 4 } }; | |
| const helpers = mockHelpers('A', 'B', 'C', 'D'); | |
| const result = await stringConcat.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe('ABCD'); | |
| }); | |
| it('should handle non-string inputs: numbers', async () => { | |
| const node = { id: 'test', data: { pinCount: 3 } }; | |
| const helpers = mockHelpers(1, 2, 3); | |
| const result = await stringConcat.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe('123'); | |
| }); | |
| it('should handle non-string inputs: null values', async () => { | |
| const node = { id: 'test', data: { pinCount: 3 } }; | |
| const helpers = mockHelpers('A', null, 'B'); | |
| const result = await stringConcat.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe('AnullB'); | |
| }); | |
| it('should handle non-string inputs: undefined values', async () => { | |
| const node = { id: 'test', data: { pinCount: 3 } }; | |
| const helpers = mockHelpers('A', undefined, 'B'); | |
| const result = await stringConcat.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe('AundefinedB'); | |
| }); |
| it('should provide getClient method', () => { | ||
| const client = prismaService.getClient(); | ||
|
|
||
| expect(client).toBeDefined(); | ||
| expect(typeof client.$connect).toBe('function'); | ||
| }); |
There was a problem hiding this comment.
suggestion (testing): Отсутствует тест для метода отключения.
Пожалуйста, добавьте тест, проверяющий, что отключение корректно закрывает соединение Prisma.
| it('should provide getClient method', () => { | |
| const client = prismaService.getClient(); | |
| expect(client).toBeDefined(); | |
| expect(typeof client.$connect).toBe('function'); | |
| }); | |
| it('should provide getClient method', () => { | |
| const client = prismaService.getClient(); | |
| expect(client).toBeDefined(); | |
| expect(typeof client.$connect).toBe('function'); | |
| }); | |
| it('should disconnect the Prisma client', async () => { | |
| const client = prismaService.getClient(); | |
| const disconnectSpy = jest.spyOn(client, '$disconnect').mockResolvedValue(); | |
| await prismaService.disconnect(); | |
| expect(disconnectSpy).toHaveBeenCalled(); | |
| disconnectSpy.mockRestore(); | |
| }); |
Original comment in English
suggestion (testing): Missing test for disconnect method.
Please add a test verifying that disconnect correctly closes the Prisma connection.
| it('should provide getClient method', () => { | |
| const client = prismaService.getClient(); | |
| expect(client).toBeDefined(); | |
| expect(typeof client.$connect).toBe('function'); | |
| }); | |
| it('should provide getClient method', () => { | |
| const client = prismaService.getClient(); | |
| expect(client).toBeDefined(); | |
| expect(typeof client.$connect).toBe('function'); | |
| }); | |
| it('should disconnect the Prisma client', async () => { | |
| const client = prismaService.getClient(); | |
| const disconnectSpy = jest.spyOn(client, '$disconnect').mockResolvedValue(); | |
| await prismaService.disconnect(); | |
| expect(disconnectSpy).toHaveBeenCalled(); | |
| disconnectSpy.mockRestore(); | |
| }); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (4)
backend/src/core/node-registries/strings.js (1)
5-5: Извлеките дублированную константу в общий модуль.Та же проблема дублирования
const all = 'all';что и в других файлах реестров. См. комментарий кbackend/src/core/node-registries/math.jsстрока 5.backend/src/core/node-registries/events.js (1)
5-7: Извлеките дублированную константу в общий модуль.Константа
allдублируется во всех реестрах. См. комментарий кbackend/src/core/node-registries/math.jsстрока 5. Константыcommandиeventиспользуются только в этом модуле, поэтому их можно оставить локальными.backend/src/core/node-registries/flow.js (1)
5-5: Извлеките дублированную константу в общий модуль.Та же проблема дублирования
const all = 'all';. См. комментарий кbackend/src/core/node-registries/math.jsстрока 5.backend/src/core/node-registries/arrays.js (1)
5-5: Извлеките дублированную константу в общий модуль.Та же проблема дублирования
const all = 'all';. См. комментарий кbackend/src/core/node-registries/math.jsстрока 5.
🧹 Nitpick comments (15)
frontend/src/lib/debounce.js (2)
4-7: Удалите избыточныйclearTimeoutв колбэкеlater.Вызов
clearTimeout(timeout)на строке 5 не нужен, так как таймаут уже сработал к моменту выполненияlater.Примените этот diff:
const later = () => { - clearTimeout(timeout); + timeout = null; func.apply(context, args); };
1-11: Рассмотрите добавление методовcancelиflush.Стандартные реализации debounce часто предоставляют методы для отмены ожидающих вызовов (
cancel) или немедленного выполнения (flush). Это повышает гибкость утилиты.Пример расширенной реализации:
export function debounce(func, wait) { let timeout; const debouncedFunc = function(...args) { const context = this; const later = () => { timeout = null; func.apply(context, args); }; clearTimeout(timeout); timeout = setTimeout(later, wait); }; debouncedFunc.cancel = () => { clearTimeout(timeout); timeout = null; }; debouncedFunc.flush = function(...args) { if (timeout) { clearTimeout(timeout); timeout = null; func.apply(this, args); } }; return debouncedFunc; }backend/src/core/nodes/arrays/get_next.js (1)
34-35: Лишние пустые строки в конце файлаДобавленные пустые строки не меняют поведение, но создают лишний шум в diff; при желании их можно удалить, если в проекте принято избегать таких хвостов.
backend/src/core/nodes/time/add.js (1)
14-15: Уточните назначение значения по умолчанию для duration.Значение по умолчанию
{}для параметраdurationозначает, что при отсутствии входного значения функцияaddвернёт исходную дату без изменений. Если это ожидаемое поведение, рассмотрите возможность добавления комментария для ясности.if (pinId === 'result') { const date = await resolvePinValue(node, 'date', new Date()); + // Пустой объект duration не изменит дату const duration = await resolvePinValue(node, 'duration', {});backend/src/core/nodes/time/compare.js (1)
18-27: Рассмотрите добавление логирования для невалидных входных данных.Функция молча возвращает
falseпри невалидных датах или неизвестной операции. Для облегчения отладки рассмотрите возможность добавления предупреждений.if (dateLeft instanceof Date && !isNaN(dateLeft.getTime()) && dateRight instanceof Date && !isNaN(dateRight.getTime())) { switch (op) { case 'after': return isAfter(dateLeft, dateRight); case 'before': return isBefore(dateLeft, dateRight); case 'equal': return isEqual(dateLeft, dateRight); - default: return false; + default: + console.warn(`[time:compare] Неизвестная операция: ${op}`); + return false; } } + console.warn('[time:compare] Невалидные входные даты'); return false;backend/src/core/nodes/time/__tests__/now.test.js (1)
1-24: Базовое покрытие тестами выглядит корректно.Тесты проверяют основную функциональность узла time:now, но можно рассмотреть расширение покрытия:
- Проверка формата возвращаемой даты (валидность timestamp)
- Тестирование поведения при различных значениях context
- Проверка работы с часовыми поясами (если применимо)
backend/src/core/nodes/math/__tests__/operation.test.js (1)
1-65: Хорошее покрытие основных математических операций.Тесты охватывают все базовые операции (+, -, *, /, деление на ноль, операция по умолчанию). Реализация корректна.
Можно рассмотреть добавление тестов для граничных случаев:
- Операции с NaN
- Операции с Infinity
- Операции с очень большими числами (overflow)
- Обработка нечисловых входных данных
backend/src/core/nodes/logic/__tests__/compare.test.js (1)
1-83: Отличное покрытие операций сравнения.Тесты охватывают все базовые операторы сравнения (>, <, ==, !=, >=, <=) и включают тест для строковых сравнений, что демонстрирует универсальность узла.
Можно рассмотреть добавление тестов для граничных случаев:
- Сравнение null и undefined
- Сравнение NaN
- Сравнение значений разных типов (например, число и строка)
- Сравнение объектов
backend/src/core/EventGraphManager.js (1)
36-53: safeJsonParse для graphJson/variables улучшает надёжность, можно чуть усилить проверку массиваПереход с прямого
JSON.parseнаsafeJsonParseдляgraph.graphJsonиgraph.variablesзаметно снижает риск падений при битом JSON; маппинг типов переменных (number/boolean/array) тоже выглядит аккуратно.Единственный момент: в зависимости от реализации
safeJsonParse, он теоретически может вернуть не‑массив (например, объект) при успешно распарсенном, но неожиданном JSON, и тогдаfor (const v of parsedVars)выбросит исключение. Можно чуть усилить защиту:- if (graph.variables) { - const parsedVars = safeJsonParse(graph.variables, [], `EventGraph ID ${graph.id} variables`); - for (const v of parsedVars) { + if (graph.variables) { + const parsedVars = safeJsonParse(graph.variables, [], `EventGraph ID ${graph.id} variables`); + if (!Array.isArray(parsedVars)) { + console.warn(`[EventGraphs] Ожидался массив variables для графа ID ${graph.id}`); + continue; + } + for (const v of parsedVars) { let val; switch (v.type) { case 'number': val = Number(v.value) || 0; break; case 'boolean': val = v.value === 'true'; break; case 'array': val = safeJsonParse(v.value, [], `variable ${v.name}`); if (!Array.isArray(val)) val = []; break; default: val = v.value; } initialState[v.name] = val; } }Это сделает поведение более предсказуемым даже при некорректно сохранённых данных.
backend/src/core/nodes/time/diff.js (1)
13-22: Рекомендуется использовать одну переменную для дефолтного значения времениПри отсутствии обоих входов код создаёт два разных объекта
new Date(), что приводит к ненулевому результату по умолчанию. Для детерминированного поведения следует использовать одно значение:- const dateLeft = await resolvePinValue(node, 'date_left', new Date()); - const dateRight = await resolvePinValue(node, 'date_right', new Date()); + const now = new Date(); + const dateLeft = await resolvePinValue(node, 'date_left', now); + const dateRight = await resolvePinValue(node, 'date_right', now);Дополнительно: функция
differenceInMillisecondsиз date-fns поддерживает числовые таймстампы (черезtoDate), поэтому текущая проверкаinstanceof Dateможет быть избыточно ограничивающей. Если граф потенциально передаёт числовые значения в эти пины, стоит расширить валидацию или явно документировать, что узел принимает только объектыDate.backend/src/core/node-registries/math.js (2)
5-5: Извлеките дублированную константу в общий модуль.Константа
const all = 'all';дублируется во всех файлах реестров узлов (actions.js, arrays.js, bot.js, data.js, debug.js, events.js, flow.js, logic.js, math.js, objects.js, strings.js, time.js, users.js). Это нарушает принцип DRY.Создайте общий модуль констант, например
backend/src/core/node-registries/constants.js:module.exports = { GRAPH_TYPE_ALL: 'all', GRAPH_TYPE_COMMAND: 'command', GRAPH_TYPE_EVENT: 'event' };Затем импортируйте в каждом реестре:
+const { GRAPH_TYPE_ALL } = require('./constants'); + function registerNodes(registry) { - const all = 'all'; + const all = GRAPH_TYPE_ALL;
29-29: Используйте переменнуюallвместо строкового литерала.На строке 12 используется переменная
all, а здесь — строковый литерал'all'. Это непоследовательно и снижает поддерживаемость.- graphType: 'all', + graphType: all,backend/src/core/node-registries/events.js (1)
14-14: Используйте переменнуюallвместо строкового литерала.На строке 14 используется строковый литерал
'all', в то время как переменнаяallопределена на строке 5 и используется в других местах (например, строки 34, 53, 69).- graphType: 'all', + graphType: all,backend/src/core/node-registries/arrays.js (1)
11-11: Используйте переменнуюallвместо строкового литерала.На строке 11 используется строковый литерал
'all', в то время как переменнаяallопределена на строке 5 и последовательно используется в остальных регистрациях узлов (строки 30, 49, 67, 87, 105, 123).- graphType: 'all', + graphType: all,backend/src/core/node-registries/actions.js (1)
5-6: Константаeventиспользуется только один раз.Константа
eventопределена в строке 6, но используется только в строке 52 для узлаaction:send_websocket_response. Остальные узлы используютall. Рассмотрите возможность либо удалить константу и использовать строковый литерал напрямую, либо убедиться, что это соответствует вашему дизайну.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
backend/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (52)
.gitignore(1 hunks)backend/jest.config.js(1 hunks)backend/package.json(1 hunks)backend/src/core/EventGraphManager.js(3 hunks)backend/src/core/GraphExecutionEngine.js(4 hunks)backend/src/core/NodeRegistry.js(4 hunks)backend/src/core/PrismaService.js(1 hunks)backend/src/core/__tests__/PrismaService.test.js(1 hunks)backend/src/core/node-registries/actions.js(1 hunks)backend/src/core/node-registries/arrays.js(1 hunks)backend/src/core/node-registries/bot.js(1 hunks)backend/src/core/node-registries/data.js(1 hunks)backend/src/core/node-registries/debug.js(1 hunks)backend/src/core/node-registries/events.js(1 hunks)backend/src/core/node-registries/flow.js(1 hunks)backend/src/core/node-registries/logic.js(1 hunks)backend/src/core/node-registries/math.js(1 hunks)backend/src/core/node-registries/objects.js(1 hunks)backend/src/core/node-registries/strings.js(1 hunks)backend/src/core/node-registries/time.js(1 hunks)backend/src/core/node-registries/users.js(1 hunks)backend/src/core/nodes/actions/bot_look_at.js(2 hunks)backend/src/core/nodes/actions/bot_set_variable.js(2 hunks)backend/src/core/nodes/actions/send_log.js(2 hunks)backend/src/core/nodes/arrays/get_next.js(1 hunks)backend/src/core/nodes/data/datetime_literal.js(1 hunks)backend/src/core/nodes/data/get_user_field.js(1 hunks)backend/src/core/nodes/flow/break.js(1 hunks)backend/src/core/nodes/flow/delay.js(1 hunks)backend/src/core/nodes/flow/for_each.js(1 hunks)backend/src/core/nodes/flow/switch.js(2 hunks)backend/src/core/nodes/flow/while.js(1 hunks)backend/src/core/nodes/logic/__tests__/compare.test.js(1 hunks)backend/src/core/nodes/math/__tests__/operation.test.js(1 hunks)backend/src/core/nodes/strings/__tests__/concat.test.js(1 hunks)backend/src/core/nodes/time/__tests__/now.test.js(1 hunks)backend/src/core/nodes/time/add.js(1 hunks)backend/src/core/nodes/time/compare.js(1 hunks)backend/src/core/nodes/time/diff.js(1 hunks)backend/src/core/nodes/time/format.js(1 hunks)backend/src/core/nodes/time/now.js(1 hunks)backend/src/core/nodes/users/check_blacklist.js(1 hunks)backend/src/core/nodes/users/get_groups.js(1 hunks)backend/src/core/nodes/users/get_permissions.js(1 hunks)backend/src/core/nodes/users/set_blacklist.js(1 hunks)backend/src/core/utils/__tests__/jsonParser.test.js(1 hunks)backend/src/core/utils/jsonParser.js(1 hunks)backend/src/core/validation/__tests__/nodeSchemas.test.js(1 hunks)backend/src/core/validation/nodeSchemas.js(1 hunks)backend/src/lib/prisma.js(1 hunks)frontend/src/lib/debounce.js(1 hunks)frontend/src/stores/visualEditorStore.js(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (34)
backend/src/core/nodes/logic/__tests__/compare.test.js (2)
backend/src/core/nodes/math/__tests__/operation.test.js (1)
mockHelpers(4-10)backend/src/core/nodes/strings/__tests__/concat.test.js (1)
mockHelpers(4-13)
backend/src/core/utils/jsonParser.js (2)
frontend/src/components/visual-editor/SettingsPanel.jsx (1)
safeJsonParse(26-35)backend/src/api/routes/bots.js (1)
JSON(2028-2028)
backend/src/core/nodes/math/__tests__/operation.test.js (2)
backend/src/core/nodes/logic/__tests__/compare.test.js (1)
mockHelpers(4-10)backend/src/core/nodes/strings/__tests__/concat.test.js (1)
mockHelpers(4-13)
backend/src/core/validation/__tests__/nodeSchemas.test.js (3)
backend/src/core/GraphExecutionEngine.js (2)
require(2-2)require(3-3)backend/src/core/NodeRegistry.js (1)
require(1-1)backend/src/core/validation/nodeSchemas.js (1)
require(1-1)
backend/src/core/nodes/users/get_groups.js (4)
backend/src/core/nodes/data/get_user_field.js (1)
User(1-1)backend/src/core/nodes/users/check_blacklist.js (1)
User(1-1)backend/src/core/nodes/users/get_permissions.js (1)
User(1-1)backend/src/core/nodes/users/set_blacklist.js (1)
User(1-1)
backend/src/core/nodes/users/get_permissions.js (4)
backend/src/core/nodes/data/get_user_field.js (1)
User(1-1)backend/src/core/nodes/users/check_blacklist.js (1)
User(1-1)backend/src/core/nodes/users/get_groups.js (1)
User(1-1)backend/src/core/nodes/users/set_blacklist.js (1)
User(1-1)
backend/src/core/node-registries/arrays.js (12)
backend/src/core/node-registries/actions.js (1)
all(5-5)backend/src/core/node-registries/bot.js (1)
all(5-5)backend/src/core/node-registries/data.js (1)
all(5-5)backend/src/core/node-registries/debug.js (1)
all(5-5)backend/src/core/node-registries/events.js (1)
all(5-5)backend/src/core/node-registries/flow.js (1)
all(5-5)backend/src/core/node-registries/logic.js (1)
all(5-5)backend/src/core/node-registries/math.js (1)
all(5-5)backend/src/core/node-registries/objects.js (1)
all(5-5)backend/src/core/node-registries/strings.js (1)
all(5-5)backend/src/core/node-registries/time.js (1)
all(5-5)backend/src/core/node-registries/users.js (1)
all(5-5)
backend/src/core/node-registries/strings.js (4)
backend/src/core/node-registries/actions.js (1)
all(5-5)backend/src/core/node-registries/arrays.js (1)
all(5-5)backend/src/core/node-registries/flow.js (1)
all(5-5)backend/src/core/node-registries/math.js (1)
all(5-5)
backend/src/core/nodes/data/get_user_field.js (4)
backend/src/core/nodes/users/check_blacklist.js (1)
User(1-1)backend/src/core/nodes/users/get_groups.js (1)
User(1-1)backend/src/core/nodes/users/get_permissions.js (1)
User(1-1)backend/src/core/nodes/users/set_blacklist.js (1)
User(1-1)
backend/src/core/EventGraphManager.js (5)
backend/src/core/GraphExecutionEngine.js (4)
prismaService(1-1)require(2-2)require(3-3)prisma(4-4)backend/src/core/PrismaService.js (1)
prismaService(36-36)backend/src/core/nodes/users/set_blacklist.js (2)
prismaService(2-2)prisma(3-3)backend/src/lib/prisma.js (1)
prismaService(1-1)frontend/src/components/visual-editor/SettingsPanel.jsx (1)
safeJsonParse(26-35)
backend/src/core/nodes/time/compare.js (3)
backend/src/core/nodes/time/add.js (1)
require(1-1)backend/src/core/nodes/time/diff.js (1)
require(1-1)backend/src/core/nodes/time/format.js (1)
require(1-1)
backend/src/core/validation/nodeSchemas.js (3)
backend/src/core/GraphExecutionEngine.js (2)
require(2-2)require(3-3)backend/src/core/NodeRegistry.js (1)
require(1-1)backend/src/core/validation/__tests__/nodeSchemas.test.js (1)
require(1-5)
frontend/src/stores/visualEditorStore.js (2)
frontend/src/pages/Bot/EventGraphsPage.jsx (1)
useVisualEditorStore(240-240)frontend/src/lib/debounce.js (1)
debounce(1-11)
backend/src/core/nodes/data/datetime_literal.js (2)
backend/src/core/nodes/time/add.js (1)
date(14-14)backend/src/core/nodes/time/format.js (1)
date(14-14)
backend/src/core/nodes/flow/break.js (3)
backend/src/core/GraphExecutionEngine.js (3)
BreakLoopSignal(6-6)require(2-2)require(3-3)backend/src/core/nodes/flow/for_each.js (1)
BreakLoopSignal(1-1)backend/src/core/nodes/flow/while.js (1)
BreakLoopSignal(1-1)
backend/src/core/nodes/users/check_blacklist.js (4)
backend/src/core/nodes/data/get_user_field.js (1)
User(1-1)backend/src/core/nodes/users/get_groups.js (1)
User(1-1)backend/src/core/nodes/users/get_permissions.js (1)
User(1-1)backend/src/core/nodes/users/set_blacklist.js (1)
User(1-1)
backend/src/core/nodes/time/add.js (3)
backend/src/core/nodes/time/compare.js (1)
require(1-1)backend/src/core/nodes/time/diff.js (1)
require(1-1)backend/src/core/nodes/time/format.js (2)
require(1-1)date(14-14)
backend/src/core/utils/__tests__/jsonParser.test.js (3)
backend/src/core/EventGraphManager.js (1)
require(4-4)backend/src/core/GraphExecutionEngine.js (2)
require(2-2)require(3-3)frontend/src/components/visual-editor/SettingsPanel.jsx (1)
safeJsonParse(26-35)
backend/src/core/node-registries/flow.js (2)
backend/src/core/node-registries/actions.js (1)
all(5-5)backend/src/core/NodeRegistry.js (1)
require(1-1)
backend/src/core/nodes/flow/while.js (3)
backend/src/core/GraphExecutionEngine.js (3)
BreakLoopSignal(6-6)require(2-2)require(3-3)backend/src/core/nodes/flow/break.js (1)
BreakLoopSignal(1-1)backend/src/core/nodes/flow/for_each.js (1)
BreakLoopSignal(1-1)
backend/src/core/nodes/users/set_blacklist.js (8)
backend/src/core/nodes/data/get_user_field.js (1)
User(1-1)backend/src/core/nodes/users/check_blacklist.js (1)
User(1-1)backend/src/core/nodes/users/get_groups.js (1)
User(1-1)backend/src/core/nodes/users/get_permissions.js (1)
User(1-1)backend/src/core/EventGraphManager.js (3)
require(4-4)prismaService(3-3)prisma(6-6)backend/src/core/GraphExecutionEngine.js (4)
require(2-2)require(3-3)prismaService(1-1)prisma(4-4)backend/src/core/PrismaService.js (2)
require(1-1)prismaService(36-36)backend/src/lib/prisma.js (1)
prismaService(1-1)
backend/src/core/node-registries/time.js (16)
backend/src/core/node-registries/actions.js (1)
all(5-5)backend/src/core/node-registries/arrays.js (1)
all(5-5)backend/src/core/node-registries/bot.js (1)
all(5-5)backend/src/core/node-registries/data.js (1)
all(5-5)backend/src/core/node-registries/debug.js (1)
all(5-5)backend/src/core/node-registries/events.js (1)
all(5-5)backend/src/core/node-registries/flow.js (1)
all(5-5)backend/src/core/node-registries/logic.js (1)
all(5-5)backend/src/core/node-registries/math.js (1)
all(5-5)backend/src/core/node-registries/objects.js (1)
all(5-5)backend/src/core/node-registries/strings.js (1)
all(5-5)backend/src/core/node-registries/users.js (1)
all(5-5)backend/src/core/nodes/time/add.js (1)
require(1-1)backend/src/core/nodes/time/compare.js (1)
require(1-1)backend/src/core/nodes/time/diff.js (1)
require(1-1)backend/src/core/nodes/time/format.js (1)
require(1-1)
backend/src/core/node-registries/events.js (2)
backend/src/core/node-registries/actions.js (2)
all(5-5)event(6-6)backend/src/core/node-registries/data.js (2)
all(5-5)command(6-6)
backend/src/core/nodes/time/format.js (2)
backend/src/core/nodes/time/add.js (2)
require(1-1)date(14-14)backend/src/core/nodes/data/datetime_literal.js (2)
helpers(9-9)date(14-14)
backend/src/core/node-registries/debug.js (1)
backend/src/core/node-registries/actions.js (1)
all(5-5)
backend/src/core/nodes/actions/send_log.js (1)
backend/src/core/nodes/actions/send_message.js (1)
message(11-11)
backend/src/core/__tests__/PrismaService.test.js (1)
backend/src/core/PrismaService.js (2)
prismaService(36-36)require(1-1)
backend/src/core/nodes/strings/__tests__/concat.test.js (2)
backend/src/core/nodes/logic/__tests__/compare.test.js (1)
mockHelpers(4-10)backend/src/core/nodes/math/__tests__/operation.test.js (1)
mockHelpers(4-10)
backend/src/core/PrismaService.js (5)
backend/src/core/EventGraphManager.js (2)
require(4-4)prismaService(3-3)backend/src/core/GraphExecutionEngine.js (3)
require(2-2)require(3-3)prismaService(1-1)backend/src/core/__tests__/PrismaService.test.js (1)
prismaService(1-1)backend/src/core/nodes/users/set_blacklist.js (1)
prismaService(2-2)backend/src/lib/prisma.js (1)
prismaService(1-1)
backend/src/lib/prisma.js (3)
backend/src/core/PrismaService.js (1)
prismaService(36-36)backend/src/core/__tests__/PrismaService.test.js (1)
prismaService(1-1)backend/src/core/nodes/users/set_blacklist.js (1)
prismaService(2-2)
backend/src/core/node-registries/math.js (2)
backend/src/core/node-registries/actions.js (1)
all(5-5)backend/src/core/node-registries/arrays.js (1)
all(5-5)
backend/src/core/nodes/flow/for_each.js (3)
backend/src/core/GraphExecutionEngine.js (3)
BreakLoopSignal(6-6)require(2-2)require(3-3)backend/src/core/nodes/flow/break.js (1)
BreakLoopSignal(1-1)backend/src/core/nodes/flow/while.js (1)
BreakLoopSignal(1-1)
backend/src/core/nodes/time/diff.js (3)
backend/src/core/nodes/time/add.js (1)
require(1-1)backend/src/core/nodes/time/compare.js (3)
require(1-1)dateLeft(14-14)dateRight(15-15)backend/src/core/nodes/time/format.js (1)
require(1-1)
backend/src/core/NodeRegistry.js (1)
backend/src/core/GraphExecutionEngine.js (2)
require(2-2)require(3-3)
🪛 Biome (2.1.2)
backend/src/core/PrismaService.js
[error] 10-10: The constructor should not return a value.
The constructor is here:
Returning a value from a constructor may confuse users of the class.
(lint/correctness/noConstructorReturn)
⏰ 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). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (46)
frontend/src/stores/visualEditorStore.js (1)
15-18: Корректная реализация debounced save.Создание
debouncedSaveGraphвнутри замыкания с использованиемget().saveGraph()правильно интегрируется с Zustand. Задержка в 1 секунду является разумным выбором для функции автосохранения.backend/package.json (1)
31-37: Новые зависимости: date-fns, express-rate-limit, zodДобавленные версии выглядят консистентно с остальным стеком; проверьте, что они совместимы с целевой версией Node, lock-файл обновлён и тесты/линтеры проходят на чистой установке.
backend/src/core/nodes/flow/delay.js (1)
41-43: Файл без функциональных измененийИзменение сводится к форматированию export-блока/точке с запятой; логика узла delay остаётся той же, всё выглядит корректно.
.gitignore (1)
67-69: Игнорирование каталога .claude/Добавление .claude/ и правила для /packages выглядит безопасно и помогает не тащить локальные/служебные артефакты в репозиторий.
backend/src/core/nodes/users/check_blacklist.js (1)
1-1: Обновлён путь к UserServiceПереход на
../../UserServiceделает импорт консистентным с другими нодами (data/users) и корректно поднимает его до уровня core; при наличииbackend/src/core/UserService.jsэто изменение выглядит полностью ожидаемым.backend/src/core/nodes/actions/send_log.js (1)
10-10: Изменения выглядят хорошо.Форматирование улучшено без изменения логики.
Also applies to: 15-15, 28-28
backend/src/core/nodes/time/add.js (2)
17-26: Логика валидации корректна.Проверка валидности даты и обработка ошибок реализованы правильно. Локализованные сообщения об ошибках соответствуют стилю проекта.
1-1: Сигнатура функцииaddподтверждена и использование корректно.Функция
addв date-fns 4.1.0 имеет сигнатуруadd<DateType extends Date, ResultDate extends Date = DateType>(date: DateArg<DateType>, duration: Duration, options?: AddOptions<ResultDate> | undefined) => ResultDate. Код правильно использует эту функцию:
- Параметр
dateкорректно проверяется как валидный Date объект- Параметр
durationпередаётся как объект- Опциональный параметр
optionsне передаётся (допустимо)Импорт и использование соответствуют API date-fns 4.1.0.
backend/src/core/nodes/flow/while.js (1)
1-1: Изменение пути импорта корректно.Обновление пути импорта
BreakLoopSignalс../BreakLoopSignalна../../BreakLoopSignalсогласуется с аналогичными изменениями в других файлах потока управления (break.js, for_each.js) и соответствует новой структуре проекта.backend/src/core/nodes/actions/bot_set_variable.js (1)
14-14: Форматирование улучшено.Изменения пробельных символов не влияют на функциональность.
Also applies to: 26-26
backend/src/core/nodes/flow/for_each.js (1)
1-1: Путь импорта обновлён корректно.Изменение пути импорта согласуется с реструктуризацией проекта и аналогичными изменениями в других узлах потока управления.
backend/src/core/nodes/flow/switch.js (1)
14-14: Форматирование корректно.Изменения улучшают читаемость кода без изменения логики.
Also applies to: 19-19, 31-31, 39-39
backend/src/core/nodes/users/get_permissions.js (1)
1-1: Путь импорта обновлён правильно.Изменение пути импорта
UserServiceс../UserServiceна../../UserServiceсогласуется с аналогичными изменениями в других узлах, связанных с пользователями (check_blacklist.js, get_groups.js, set_blacklist.js), и соответствует централизованному разрешению UserService.backend/src/core/nodes/users/get_groups.js (1)
1-1: LGTM! Путь импорта обновлен корректно.Изменение пути импорта согласуется с централизацией UserService и соответствует изменениям в других файлах PR.
backend/src/core/nodes/actions/bot_look_at.js (1)
19-30: LGTM! Форматирование кода улучшено.Изменения касаются только форматирования кода (расположение фигурных скобок и пустых строк) без изменения функциональности.
backend/src/core/nodes/data/get_user_field.js (1)
1-1: Путь импорта корректен и проверен.Изменение импорта с
../UserServiceна../../UserServiceуспешно проверено — файлUserService.jsсуществует по адресуbackend/src/core/UserService.jsи доступен по указанному относительному пути изbackend/src/core/nodes/data/get_user_field.js. Изменение соответствует архитектурным изменениям, примененным в других файлах проекта.backend/src/core/nodes/flow/break.js (1)
1-1: Импорт BreakLoopSignal установлен корректно.Скрипт подтвердил, что
BreakLoopSignal.jsсуществует по путиbackend/src/core/. Относительный путь../../BreakLoopSignalиз файлаbackend/src/core/nodes/flow/break.jsправильно разрешается на целевой файл. Изменение архитектуры реализовано верно.backend/src/core/node-registries/objects.js (1)
1-98: Все модули evaluator успешно найдены. Код готов к использованию.Проверка подтвердила наличие всех пяти требуемых модулей evaluator в директории
backend/src/core/nodes/objects/. Структура регистрации узлов корректна, все пути к модулям валидны, и нет никаких проблем или конфликтов зависимостей.backend/src/core/utils/__tests__/jsonParser.test.js (1)
1-44: Отличное покрытие тестами!Тесты охватывают все основные сценарии: валидный JSON, некорректный JSON, null/undefined, не-строковые значения, массивы и проверку логирования ошибок. Реализация чистая и следует лучшим практикам.
backend/src/core/__tests__/PrismaService.test.js (1)
1-24: Корректная проверка поведения синглтона!Тесты правильно проверяют паттерн синглтона для PrismaService, включая повторное использование экземпляра клиента и наличие методов Prisma Client.
backend/jest.config.js (1)
41-41: Проверьте намерение включения узлов в покрытие.Строка 41 включает
src/core/nodes/**/*.jsв покрытие кода. Однако согласно AI-сводке, эти файлы должны быть исключены из покрытия. Убедитесь, что это соответствует вашему намерению. Если узлы должны быть исключены, добавьте префикс!:- 'src/core/nodes/**/*.js', + '!src/core/nodes/**/*.js',backend/src/core/nodes/time/now.js (1)
1-18: Простая и корректная реализация.Функция правильно возвращает текущую дату для pin 'now' и null для остальных случаев. Сигнатура
asyncсогласуется с другими node evaluators в системе.backend/src/core/node-registries/bot.js (1)
1-23: Корректная регистрация узла бота.Модуль правильно регистрирует узел
bot:get_positionсо всеми необходимыми метаданными и соответствует шаблону других registry-модулей в проекте.backend/src/core/nodes/users/set_blacklist.js (1)
1-3: Хороший рефакторинг для централизации клиента БД.Переход от прямого использования PrismaClient к PrismaService обеспечивает единую точку управления соединениями с базой данных. Импорты обновлены корректно.
backend/src/core/utils/jsonParser.js (1)
1-18: Надежная реализация парсинга JSON.Функция правильно обрабатывает все граничные случаи с хорошим логированием ошибок. Обратите внимание, что похожая реализация существует во фронтенде (
frontend/src/components/visual-editor/SettingsPanel.jsx, строки 25-34). Хотя прямое переиспользование между фронтендом и бэкендом может быть затруднено, стоит учитывать эту дублированную логику при будущих изменениях.backend/src/core/node-registries/debug.js (1)
1-26: Корректная регистрация отладочного узла.Модуль правильно определяет узел
debug:logс необходимыми пинами. Использование типаWildcardдля входного значения уместно для отладочного вывода, а флагrequired: trueгарантирует, что значение будет предоставлено.backend/src/core/nodes/data/datetime_literal.js (1)
11-19: Логика обработки даты выглядит корректнойПри
pinId === 'value'вы аккуратно пытаетесь получить и распарсить входdate, при невалидном/пустом значении возвращаете текущий момент; для остальных пинов возвращаетсяnull. Для этого узла поведение выглядит предсказуемым и безопасным.backend/src/core/node-registries/logic.js (1)
7-42: Регистрация логических нод выглядит согласованнойОпределения
logic:operationиlogic:compare(типы пинов,graphType,dynamicPins, привязкаevaluator) хорошо вписываются в существующую архитектуру реестров нод. Явных логических или структурных проблем не видно.backend/src/core/EventGraphManager.js (2)
3-7: Интеграция с PrismaService выполнена корректноПолучение клиента через
PrismaServiceна уровне модуля (const prisma = prismaService.getClient();) хорошо централизует доступ к БД и убирает необходимость создаватьPrismaClientв нескольких местах.
155-158: Унификация обработки массивов переменных при выполнении графаВ блоке подготовки
savedVariablesвы так же используетеsafeJsonParseи проверкуArray.isArrayдля переменных типаarray, что синхронизирует логику с загрузкой графов и предотвращает падения на битом JSON. Это хорошее выравнивание поведения.backend/src/core/nodes/strings/__tests__/concat.test.js (1)
1-50: Отличная тестовая покрытие!Тесты хорошо структурированы и охватывают основные сценарии: конкатенацию двух строк, множественных строк, обработку пустых строк и значение по умолчанию для
pinCount. РеализацияmockHelpersкорректно эмулирует разрешение пинов.backend/src/lib/prisma.js (1)
1-3: Рефакторинг на использование PrismaService выглядит корректно.Централизация управления Prisma клиентом через singleton PrismaService улучшает поддерживаемость и обеспечивает единственный экземпляр клиента по всему приложению.
backend/src/core/validation/__tests__/nodeSchemas.test.js (1)
14-18: Тестовая структура соответствует определениюNodeConfigSchema— review некорректен.Проверка показывает, что ваша озабоченность основана на неправильном предположении:
NodeConfigSchemaявно определяетinputsиoutputsна верхнем уровне объекта (не вложены вpins)- Тестовый файл правильно использует эту структуру при валидации конфигурации
- Функция
validateNodeConfig()напрямую применяет эту схемуСтруктуры в реестрах (math.js, strings.js и т.д.) с
pins: { inputs: [...], outputs: [...] }— это отдельная структура данных, которая не валидируется схемойNodeConfigSchema. Реестры и схема валидации служат разным целям и имеют независимые структуры.Тест актуален и не требует изменений.
Likely an incorrect or invalid review comment.
backend/src/core/node-registries/actions.js (2)
8-27: Убедитесь, что флагdynamicPinsкорректно обрабатывается.Узел
action:send_messageимеет флагdynamicPins: true(строка 14), что предполагает динамическое изменение пинов во время выполнения. Убедитесь, что исполнитель этого узла и система выполнения графов корректно обрабатывают динамические пины.
104-130: LGTM: Хорошая реализация узла HTTP-запроса.Узел
action:http_requestимеет хорошо продуманную структуру с разделением успешного выполнения и ошибок. Параметрtimeout(строка 118) опциональный — убедитесь, что исполнитель применяет разумное значение по умолчанию для предотвращения бесконечного ожидания.backend/src/core/nodes/time/format.js (1)
10-28: LGTM: Корректная реализация форматирования даты.Реализация узла
time:formatвыглядит правильно:
- Использование
date-fnsдля форматирования (строка 1)- Корректная валидация объекта Date (строка 17)
- Разумные значения по умолчанию (строки 14-15)
- Обработка ошибок с понятными сообщениями (строки 21, 24)
backend/src/core/NodeRegistry.js (2)
28-28: LGTM: Добавлена валидация конфигурации узлов.Введение флага
validationEnabled(строка 28) и проверки конфигурации узлов (строки 41-47) улучшает надёжность системы. Отключение валидации в production разумно с точки зрения производительности.Also applies to: 41-47
106-135: LGTM: Модульная архитектура регистрации узлов.Замена встроенных регистраций узлов на модульную систему (строки 106-135) значительно улучшает maintainability кода. Каждая категория узлов теперь находится в отдельном файле, что упрощает навигацию и поддержку.
backend/src/core/validation/nodeSchemas.js (1)
53-93: LGTM: Корректная реализация валидаторов.Функции валидации (строки 53-93) правильно обрабатывают ошибки Zod и возвращают унифицированный формат результата
{ success, data/error }. Обработка ошибок с фоллбэками (строки 62, 76, 90) обеспечивает надёжность.backend/src/core/node-registries/time.js (2)
7-22: Проверьте расположение evaluator дляtime:datetime_literal.Узел
time:datetime_literalиспользует evaluator из../nodes/data/datetime_literal(строка 13), в то время как все остальные узлы времени используют evaluator из../nodes/time/*. Это может вызвать путаницу при поддержке кода.Убедитесь, что это намеренное решение. Если
datetime_literalлогически относится к категории "Данные", возможно, стоит переместить его регистрацию вbackend/src/core/node-registries/data.js.
39-55: LGTM: Узел форматирования даты.Регистрация узла
time:formatкорректна и хорошо согласуется с реализацией вbackend/src/core/nodes/time/format.js. Опциональный параметр формата с разумным значением по умолчанию упрощает использование.backend/src/core/node-registries/data.js (1)
257-288: Примечание: категория "Массив" для операций с массивами.Узлы
data:length(строка 260) иdata:array_literal(строка 277) используют категорию"Массив"вместо"Данные", что отличается от других узлов в этом файле. Это может быть намеренным решением для лучшей организации UI, но убедитесь, что это согласуется с вашей архитектурой категорий узлов.backend/src/core/GraphExecutionEngine.js (4)
1-4: LGTM: Интеграция PrismaService и валидации.Введение
PrismaService(строки 1, 4), утилит безопасного парсинга JSON (строка 2) и валидации графов (строка 3) улучшает архитектуру приложения. ФлагvalidationEnabled(строка 18) позволяет отключить дорогостоящую валидацию в production.Also applies to: 18-18
34-40: LGTM: Валидация структуры графа перед выполнением.Добавление проверки валидности графа перед выполнением (строки 34-40) предотвращает выполнение некорректных графов и обеспечивает раннее обнаружение ошибок. Логирование деталей ошибки (строка 37) поможет при отладке.
64-66: LGTM: Безопасный парсинг переменных-массивов.Использование
safeJsonParseдля переменных типаarray(строка 65) с фоллбэком на пустой массив и дополнительной проверкой типа (строка 66) обеспечивает надёжность инициализации переменных и предотвращает runtime ошибки.
318-345: LGTM: Защита от бесконечной рекурсии вisNodeVolatile.Добавление параметров
visitedиdepth(строка 318) с проверками:
- Ограничение глубины рекурсии
MAX_DEPTH = 100(строки 321-325)- Обнаружение циклов через
visitedSet (строки 327-330)- Корректная передача параметров в рекурсивных вызовах (строка 339)
Эти изменения предотвращают переполнение стека и зависание при обработке графов с циклами.
| const dateLeft = await resolvePinValue(node, 'date_left', new Date()); | ||
| const dateRight = await resolvePinValue(node, 'date_right', new Date()); |
There was a problem hiding this comment.
Пересмотрите значения по умолчанию для дат.
Использование new Date() в качестве значения по умолчанию для обеих дат приведёт к тому, что при отсутствии входных значений обе даты будут практически идентичными. Это может вызвать неожиданное поведение:
- Операция
'after'вернётfalse - Операция
'before'вернётfalse - Операция
'equal'может вернутьtrue
Рассмотрите возможность использования null или undefined в качестве значений по умолчанию и явной обработки случаев отсутствия входных данных.
if (pinId === 'result') {
- const dateLeft = await resolvePinValue(node, 'date_left', new Date());
- const dateRight = await resolvePinValue(node, 'date_right', new Date());
+ const dateLeft = await resolvePinValue(node, 'date_left', null);
+ const dateRight = await resolvePinValue(node, 'date_right', null);
const op = await resolvePinValue(node, 'operation', 'after');
- if (dateLeft instanceof Date && !isNaN(dateLeft.getTime()) &&
- dateRight instanceof Date && !isNaN(dateRight.getTime())) {
+ if (dateLeft && dateRight &&
+ dateLeft instanceof Date && !isNaN(dateLeft.getTime()) &&
+ dateRight instanceof Date && !isNaN(dateRight.getTime())) {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
backend/src/core/nodes/time/compare.js around lines 14-15: current code uses new
Date() as the default for both date_left and date_right which makes absent
inputs effectively equal; change the defaults to null/undefined when calling
resolvePinValue (e.g., no default or explicit null) and add explicit handling
after resolution: if either value is missing, either return a defined result
(e.g., false for 'after'/'before' and false for 'equal') or throw a clear
validation error depending on desired behavior; only coerce values to Date when
present and validate they are valid dates before performing comparisons, logging
or returning a meaningful error if validation fails.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
frontend/src/lib/debounce.js (1)
6-9: Избыточная очистка таймера.Вызов
clearTimeout(timeout)в строке 7 избыточен, так как к моменту выполненияlater()таймер уже сработал. Это не ошибка, но можно убрать для чистоты кода.Примените этот diff, чтобы убрать избыточную операцию:
const later = () => { - clearTimeout(timeout); + timeout = null; func.apply(context, args); };backend/src/core/validation/nodeSchemas.js (2)
11-28: Избыточная структура для inputs/outputs может вызвать путаницу.В схеме
NodeConfigSchemaопределены входы и выходы на двух уровнях:
- Напрямую:
inputsиoutputs(строки 20-21)- Вложенные:
pins.inputsиpins.outputs(строки 22-25)Эта двойственность может привести к несогласованности данных и затруднить понимание того, какая структура является каноничной.
Рассмотрите одно из решений:
Вариант 1: Оставить только верхнеуровневые поля (рекомендуется):
const NodeConfigSchema = z.object({ type: z.string().min(1), label: z.string().min(1), category: z.string().min(1), description: z.string().optional(), name: z.string().optional(), graphType: z.enum(['all', 'command', 'event']).optional(), isEvent: z.boolean().optional(), dynamicPins: z.boolean().optional(), inputs: z.array(PinSchema).optional().default([]), outputs: z.array(PinSchema).optional().default([]), - pins: z.object({ - inputs: z.array(PinSchema).optional().default([]), - outputs: z.array(PinSchema).optional().default([]), - }).optional(), executor: z.function().optional(), evaluator: z.function().optional(), }).passthrough();Вариант 2: Если обе структуры необходимы, добавьте нормализацию:
Добавьте метод
.transform()для объединения обеих структур в единую каноническую форму.
60-72: Структура ошибок соответствует Zod 4, но можно улучшить.Текущая обработка ошибок
error.issues || error.errors || [{ message: error.message }]покрывает различные случаи, однако в Zod 4 стандартная структура — этоerror.issuesс типизированными объектами ошибок.Для более предсказуемой обработки ошибок можно явно проверить тип ошибки:
function validateNodeConfig(nodeConfig) { try { return { success: true, data: NodeConfigSchema.parse(nodeConfig), }; } catch (error) { + // В Zod 4 все ошибки валидации содержат issues + const issues = error.issues || [{ message: error.message || 'Unknown validation error' }]; return { success: false, - error: error.issues || error.errors || [{ message: error.message }], + error: issues, }; } }Примените аналогичное изменение к
validateNodeData(строки 74-86) иvalidateGraph(строки 88-100).backend/src/core/EventGraphManager.js (1)
47-53: Дублирование логики парсинга массивов.Логика парсинга переменных типа
arrayс использованиемsafeJsonParseи обработкой ошибок дублируется в двух местах:
- При загрузке графов (строки 47-53)
- При выполнении графов (строки 158-161)
Извлеките логику парсинга в вспомогательную функцию:
/** * Парсит значение переменной на основе её типа * @param {Object} variable - Объект переменной с полями name, type, value * @returns {*} Распарсенное значение */ function parseVariableValue(variable) { let val; switch (variable.type) { case 'number': val = Number(variable.value) || 0; break; case 'boolean': val = variable.value === 'true'; break; case 'array': val = safeJsonParse(variable.value, [], `variable ${variable.name}`); if (!Array.isArray(val)) { console.warn( `[EventGraphManager] Failed to parse variable "${variable.name}" as array. Falling back to empty array. Value:`, variable.value ); val = []; } break; default: val = variable.value; } return val; }Затем используйте эту функцию в обоих местах:
for (const v of parsedVars) { - let val; - switch (v.type) { - case 'number': val = Number(v.value) || 0; break; - case 'boolean': val = v.value === 'true'; break; - case 'array': - val = safeJsonParse(v.value, [], `variable ${v.name}`); - if (!Array.isArray(val)) { - console.warn(`[EventGraphManager] Failed to parse variable "${v.name}" as array. Falling back to empty array. Value:`, v.value); - val = []; - } - break; - default: val = v.value; - } + const val = parseVariableValue(v); initialState[v.name] = val; }Also applies to: 158-161
backend/src/core/GraphExecutionEngine.js (1)
66-70: Дублирование логики парсинга массивов.Логика парсинга переменных типа
arrayдублируется здесь и вEventGraphManager.js(строки 47-53, 158-161).Создайте общую утилиту для парсинга переменных (см. комментарий к файлу
EventGraphManager.js) и переиспользуйте её в обоих модулях. Это можно разместить, например, вbackend/src/core/utils/variableParser.js.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/src/core/EventGraphManager.js(3 hunks)backend/src/core/GraphExecutionEngine.js(4 hunks)backend/src/core/NodeRegistry.js(4 hunks)backend/src/core/PrismaService.js(1 hunks)backend/src/core/config/validation.js(1 hunks)backend/src/core/node-registries/users.js(1 hunks)backend/src/core/validation/nodeSchemas.js(1 hunks)frontend/src/lib/debounce.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- backend/src/core/PrismaService.js
- backend/src/core/NodeRegistry.js
- backend/src/core/node-registries/users.js
🧰 Additional context used
🧬 Code graph analysis (3)
backend/src/core/EventGraphManager.js (3)
backend/src/core/GraphExecutionEngine.js (5)
prismaService(1-1)require(2-2)require(3-3)require(4-4)prisma(5-5)backend/src/core/PrismaService.js (1)
prismaService(31-31)frontend/src/components/visual-editor/SettingsPanel.jsx (1)
safeJsonParse(26-35)
backend/src/core/validation/nodeSchemas.js (3)
backend/src/core/GraphExecutionEngine.js (3)
require(2-2)require(3-3)require(4-4)backend/src/core/NodeRegistry.js (2)
require(1-1)require(2-2)backend/src/core/validation/__tests__/nodeSchemas.test.js (1)
require(1-5)
backend/src/core/GraphExecutionEngine.js (3)
backend/src/core/PrismaService.js (1)
prismaService(31-31)backend/src/core/config/validation.js (2)
VALIDATION_ENABLED(6-6)MAX_RECURSION_DEPTH(13-13)frontend/src/components/visual-editor/SettingsPanel.jsx (1)
safeJsonParse(26-35)
⏰ 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). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (5)
frontend/src/lib/debounce.js (2)
1-22: Проблема с контекстомthisиз предыдущего ревью успешно исправлена.Реализация корректно захватывает контекст через
const context = this(строка 5) и применяет его при вызове черезfunc.apply(context, args)(строка 8). Логика debounce работает правильно: каждый вызов сбрасывает предыдущий таймер и устанавливает новый. Методflush()позволяет отменить отложенный вызов.
14-19: Методflush()реализован корректно.Проверка наличия таймера (строка 15), его очистка (строка 16) и обнуление переменной (строка 17) обеспечивают корректную отмену отложенного вызова без утечек памяти.
backend/src/core/config/validation.js (1)
1-18: LGTM!Конфигурация валидации хорошо продумана:
VALIDATION_ENABLEDотключена в продакшене для производительности, что разумноMAX_RECURSION_DEPTH = 100— адекватный лимит для предотвращения бесконечных циклов в графахbackend/src/core/EventGraphManager.js (1)
3-6: Корректная интеграция PrismaService и safeJsonParse.Переход на централизованный
PrismaServiceи использованиеsafeJsonParseулучшают модульность и надёжность парсинга JSON.backend/src/core/GraphExecutionEngine.js (1)
322-348: LGTM! Отличное улучшение isNodeVolatile.Добавление защиты от циклов через
visitedSet и ограничение глубины рекурсии черезdepthиMAX_RECURSION_DEPTHпредотвращает зависания и переполнение стека. Рекурсивные вызовы корректно передают оба параметра.
|
@sourcery-ai review |
There was a problem hiding this comment.
Привет! Я просмотрел ваши изменения, и они выглядят отлично!
Подсказка для агентов ИИ
Пожалуйста, учтите комментарии из этого обзора кода:
## Отдельные комментарии
### Комментарий 1
<location> `backend/src/core/EventGraphManager.js:41` </location>
<code_context>
+ const parsedVars = safeJsonParse(graph.variables, [], `EventGraph ID ${graph.id} variables`);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Fallback to empty array on variable parse failure may hide configuration errors.
Logging the raw value and context when parsing fails, or surfacing these issues to maintainers, would help prevent silent misconfigurations.
```suggestion
let parsedVars;
try {
parsedVars = safeJsonParse(graph.variables, undefined, `EventGraph ID ${graph.id} variables`);
if (!Array.isArray(parsedVars)) {
throw new Error(`[EventGraphManager] Parsed variables are not an array. Raw value: ${graph.variables}`);
}
} catch (err) {
console.error(`[EventGraphManager] Failed to parse EventGraph variables for graph ID ${graph.id}. Raw value:`, graph.variables,
<details>
<summary>Original comment in English</summary>
Hey there - I've reviewed your changes and they look great!
<details>
<summary>Prompt for AI Agents</summary>
~~~markdown
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `backend/src/core/EventGraphManager.js:41` </location>
<code_context>
+ const parsedVars = safeJsonParse(graph.variables, [], `EventGraph ID ${graph.id} variables`);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Fallback to empty array on variable parse failure may hide configuration errors.
Logging the raw value and context when parsing fails, or surfacing these issues to maintainers, would help prevent silent misconfigurations.
```suggestion
let parsedVars;
try {
parsedVars = safeJsonParse(graph.variables, undefined, `EventGraph ID ${graph.id} variables`);
if (!Array.isArray(parsedVars)) {
throw new Error(`[EventGraphManager] Parsed variables are not an array. Raw value: ${graph.variables}`);
}
} catch (err) {
console.error(`[EventGraphManager] Failed to parse EventGraph variables for graph ID ${graph.id}. Raw value:`, graph.variables, 'Error:', err);
// Optionally, you can throw here to surface the error to maintainers
throw err;
}
```
</issue_to_address>
### Comment 2
<location> `frontend/src/stores/visualEditorStore.js:12` </location>
<code_context>
import { toast } from "@/hooks/use-toast";
import { randomUUID } from '@/lib/uuid';
import { getConversionChain, createConverterNode } from '@/lib/typeConversionHelper';
+import { debounce } from '@/lib/debounce';
export const useVisualEditorStore = create(
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Debounced auto-save is introduced, but flush logic is not exposed.
Unsaved changes could be lost if the debounce timer hasn't completed before the user leaves. Exposing a flush method would allow forced saves during unload or navigation.
Suggested implementation:
```javascript
const debouncedSaveGraph = debounce(() => {
get().saveGraph();
}, 1000);
// Expose flush method for debounced save
const flushSaveGraph = () => {
if (typeof debouncedSaveGraph.flush === "function") {
debouncedSaveGraph.flush();
} else {
// fallback: immediately call saveGraph if flush not available
get().saveGraph();
}
};
return {
nodes: [],
edges: [],
command: null,
debouncedSaveGraph,
flushSaveGraph,
```
1. Ensure your debounce implementation in `@/lib/debounce` supports a `.flush()` method. If not, you will need to update it to expose this functionality.
2. Wherever you handle unload/navigation (e.g., in a React effect or router event), call `useVisualEditorStore.getState().flushSaveGraph()` to force-save any pending changes.
</issue_to_address>
### Comment 3
<location> `backend/src/core/PrismaService.js:10-12` </location>
<code_context>
+ */
+class PrismaService {
+ constructor() {
+ this.prisma = new PrismaClient({
+ log: ['error', 'warn'],
+ });
+ }
+
</code_context>
<issue_to_address>
**suggestion:** PrismaClient is instantiated with error and warn logging only.
Evaluate adding 'info' or 'query' logging for improved debugging and monitoring in development environments.
</issue_to_address>
### Comment 4
<location> `backend/src/core/nodes/strings/__tests__/concat.test.js:33-13` </location>
<code_context>
+ expect(result).toBe('ABCD');
+ });
+
+ it('should handle empty strings', async () => {
+ const node = { id: 'test', data: { pinCount: 3 } };
+ const helpers = mockHelpers('Hello', '', 'World');
+
+ const result = await stringConcat.evaluate(node, 'result', {}, helpers);
+
+ expect(result).toBe('HelloWorld');
+ });
+
+ it('should use default pinCount of 2', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests for non-string inputs and very large input arrays.
Testing with non-string inputs and large arrays will help verify robustness and performance.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| default: val = v.value; | ||
| } | ||
| initialState[v.name] = val; | ||
| const parsedVars = safeJsonParse(graph.variables, [], `EventGraph ID ${graph.id} variables`); |
There was a problem hiding this comment.
suggestion (bug_risk): Возврат к пустому массиву при сбое разбора переменной может скрывать ошибки конфигурации.
Ведение журнала необработанного значения и контекста при сбое разбора, или доведение этих проблем до сведения сопровождающих, поможет предотвратить скрытые неверные конфигурации.
| const parsedVars = safeJsonParse(graph.variables, [], `EventGraph ID ${graph.id} variables`); | |
| let parsedVars; | |
| try { | |
| parsedVars = safeJsonParse(graph.variables, undefined, `EventGraph ID ${graph.id} variables`); | |
| if (!Array.isArray(parsedVars)) { | |
| throw new Error(`[EventGraphManager] Parsed variables are not an array. Raw value: ${graph.variables}`); | |
| } | |
| } catch (err) { | |
| console.error(`[EventGraphManager] Failed to parse EventGraph variables for graph ID ${graph.id}. Raw value:`, graph.variables, | |
| <details> | |
| <summary>Original comment in English</summary> | |
| **suggestion (bug_risk):** Fallback to empty array on variable parse failure may hide configuration errors. | |
| Logging the raw value and context when parsing fails, or surfacing these issues to maintainers, would help prevent silent misconfigurations. | |
| ```suggestion | |
| let parsedVars; | |
| try { | |
| parsedVars = safeJsonParse(graph.variables, undefined, `EventGraph ID ${graph.id} variables`); | |
| if (!Array.isArray(parsedVars)) { | |
| throw new Error(`[EventGraphManager] Parsed variables are not an array. Raw value: ${graph.variables}`); | |
| } | |
| } catch (err) { | |
| console.error(`[EventGraphManager] Failed to parse EventGraph variables for graph ID ${graph.id}. Raw value:`, graph.variables, 'Error:', err); | |
| // Optionally, you can throw here to surface the error to maintainers | |
| throw err; | |
| } |
| this.prisma = new PrismaClient({ | ||
| log: ['error', 'warn'], | ||
| }); |
There was a problem hiding this comment.
suggestion: PrismaClient инициализируется только с логированием ошибок и предупреждений.
Рассмотрите возможность добавления логирования "info" или "query" для улучшенной отладки и мониторинга в средах разработки.
Original comment in English
suggestion: PrismaClient is instantiated with error and warn logging only.
Evaluate adding 'info' or 'query' logging for improved debugging and monitoring in development environments.
| } | ||
| return defaultValue; | ||
| }), | ||
| }); |
There was a problem hiding this comment.
suggestion (testing): Рассмотрите возможность добавления тестов для нестроковых входных данных и очень больших входных массивов.
Тестирование с нестроковыми входными данными и большими массивами поможет проверить надежность и производительность.
Original comment in English
suggestion (testing): Consider adding tests for non-string inputs and very large input arrays.
Testing with non-string inputs and large arrays will help verify robustness and performance.
|
@sourcery-ai review |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/src/core/GraphExecutionEngine.js (1)
35-46: Критические проблемы с валидацией графа в production.Обнаружены серьёзные проблемы:
Большое сообщение об ошибке (строка 38):
JSON.stringify(validation.error)создаёт нечитаемые ошибки. Используйте краткое описание.Выполнение невалидных графов в production (строки 41-45): Продолжение выполнения с невалидной структурой графа может привести к runtime-ошибкам, повреждению данных или непредсказуемому поведению. Невалидные графы не должны выполняться даже в production.
Примените этот diff:
if (this.validationEnabled) { const validation = validateGraph(parsedGraph); if (!validation.success) { - const errorMsg = `Invalid graph structure: ${JSON.stringify(validation.error)}`; - console.error('[GraphExecutionEngine] Graph validation failed:', validation.error); + console.error('[GraphExecutionEngine] Graph validation failed:', { + errors: validation.error + }); if (VALIDATION_STRICT_MODE) { - throw new Error(errorMsg); + throw new Error('Invalid graph structure - validation failed'); + } else { + // В production не выполняем невалидные графы + console.error('[GraphExecutionEngine] Aborting execution of invalid graph in production mode'); + return context; } - // В production логируем, но продолжаем выполнение } }
🧹 Nitpick comments (2)
backend/src/core/nodes/strings/__tests__/concat.test.js (1)
51-88: Граничные случаи обработаны, предыдущие замечания учтены.Тесты для нестроковых входных данных (числа, булевы значения,
null,undefined), а также для большого количества входов (100) успешно покрывают замечания из предыдущих ревью. Обработка объектов также протестирована.Опционально можно добавить тесты для дополнительных граничных случаев:
pinCount = 0(как ведёт себя узел без входов?)- Отрицательное значение
pinCount- Входные данные типа массив (проверка сериализации)
Пример теста для массива:
+ it('should handle array inputs by stringifying', async () => { + const node = { id: 'test', data: { pinCount: 2 } }; + const helpers = mockHelpers('Array: ', [1, 2, 3]); + + const result = await stringConcat.evaluate(node, 'result', {}, helpers); + + expect(result).toBe('Array: 1,2,3'); + });frontend/src/pages/Bot/VisualEditorPage.jsx (1)
75-80: Хорошее решение для сохранения изменений при размонтировании, но можно упростить массив зависимостей.Добавление вызова
flushSaveGraph()в cleanup-функцию useEffect обеспечивает сохранение несохранённых изменений при размонтировании компонента — это правильное решение для улучшения UX.Однако функция
flushSaveGraphявляется действием из zustand-стора и представляет собой стабильную ссылку, которая не изменяется между рендерами. Согласно распространённым паттернам использования zustand, функции-действия из стора обычно не включаются в массив зависимостей useEffect, так как они стабильны и их включение может привести к ненужным повторным выполнениям эффекта.Рассмотрите возможность удаления
flushSaveGraphиз массива зависимостей:- }, [botId, entityId, editorType, init, flushSaveGraph]); + }, [botId, entityId, editorType, init]);Примечание: если ESLint правило
exhaustive-depsтребует включения, можно добавить комментарий-исключение:// eslint-disable-next-line react-hooks/exhaustive-deps }, [botId, entityId, editorType, init]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/src/core/GraphExecutionEngine.js(4 hunks)backend/src/core/NodeRegistry.js(4 hunks)backend/src/core/config/validation.js(1 hunks)backend/src/core/nodes/strings/__tests__/concat.test.js(1 hunks)frontend/src/pages/Bot/VisualEditorPage.jsx(2 hunks)frontend/src/stores/visualEditorStore.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/stores/visualEditorStore.js
🧰 Additional context used
🧬 Code graph analysis (3)
backend/src/core/GraphExecutionEngine.js (5)
backend/src/core/PrismaService.js (1)
prismaService(31-31)backend/src/core/EventGraphManager.js (3)
prismaService(3-3)require(4-4)prisma(6-6)backend/src/core/nodes/users/set_blacklist.js (2)
prismaService(2-2)prisma(3-3)backend/src/core/NodeRegistry.js (2)
require(1-1)require(2-2)backend/src/core/config/validation.js (3)
VALIDATION_ENABLED(8-8)VALIDATION_STRICT_MODE(14-14)MAX_RECURSION_DEPTH(21-21)
frontend/src/pages/Bot/VisualEditorPage.jsx (1)
frontend/src/stores/visualEditorStore.js (2)
useVisualEditorStore(14-682)useVisualEditorStore(14-682)
backend/src/core/NodeRegistry.js (2)
backend/src/core/GraphExecutionEngine.js (3)
require(2-2)require(3-3)require(4-4)backend/src/core/config/validation.js (2)
VALIDATION_ENABLED(8-8)VALIDATION_STRICT_MODE(14-14)
⏰ 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). (2)
- GitHub Check: Sourcery review
- GitHub Check: Sourcery review
🔇 Additional comments (7)
backend/src/core/nodes/strings/__tests__/concat.test.js (2)
1-13: Хорошая структура тестов и качественная настройка мока.Функция
mockHelpersкорректно извлекает индексы пинов и предоставляет значения по умолчанию. Структура тестов понятна и читаема.
15-49: Базовые сценарии покрыты полностью.Тесты охватывают основные случаи: конкатенацию двух строк, нескольких строк, пустых строк и поведение по умолчанию для
pinCount. Реализация корректна.frontend/src/pages/Bot/VisualEditorPage.jsx (1)
43-43: Корректное извлечение функции из стора.Селектор
flushSaveGraphизвлекается правильно и будет использоваться для сохранения несохранённых изменений при размонтировании компонента.backend/src/core/config/validation.js (1)
1-21: Хорошая централизованная конфигурация валидации.Подход с постоянно включённой валидацией (
VALIDATION_ENABLED = true) и режимом строгости, зависящим от окружения, правильный. Это позволяет обнаруживать проблемы в production через логи, не прерывая выполнение. ЗначениеMAX_RECURSION_DEPTH = 100адекватно для типичной глубины графов.backend/src/core/GraphExecutionEngine.js (3)
27-30: Правильное использование безопасного парсинга.Замена
JSON.parseнаsafeJsonParseс проверкой результата предотвращает падение при невалидном JSON и обеспечивает graceful degradation.
71-75: Отличная обработка парсинга массивов с предупреждением.Использование
safeJsonParseс дополнительной проверкой типа и логированием предупреждения обеспечивает надёжность и упрощает отладку. Учтена обратная связь из предыдущих ревью.
327-353: Отличная защита от бесконечной рекурсии.Добавление параметров
visitedиdepthс проверками предотвращает бесконечные циклы в графах с циклическими зависимостями и защищает от переполнения стека на глубоких графах. Реализация корректна и включает информативное логирование.
| // Импортируем модули регистрации нод | ||
| const eventsRegistry = require('./node-registries/events'); | ||
| const flowRegistry = require('./node-registries/flow'); | ||
| const actionsRegistry = require('./node-registries/actions'); | ||
| const dataRegistry = require('./node-registries/data'); | ||
| const timeRegistry = require('./node-registries/time'); | ||
| const stringsRegistry = require('./node-registries/strings'); | ||
| const arraysRegistry = require('./node-registries/arrays'); | ||
| const mathRegistry = require('./node-registries/math'); | ||
| const logicRegistry = require('./node-registries/logic'); | ||
| const objectsRegistry = require('./node-registries/objects'); | ||
| const usersRegistry = require('./node-registries/users'); | ||
| const botRegistry = require('./node-registries/bot'); | ||
| const debugRegistry = require('./node-registries/debug'); | ||
|
|
||
| // Регистрируем все ноды | ||
| eventsRegistry.registerNodes(this); | ||
| flowRegistry.registerNodes(this); | ||
| actionsRegistry.registerNodes(this); | ||
| dataRegistry.registerNodes(this); | ||
| timeRegistry.registerNodes(this); | ||
| stringsRegistry.registerNodes(this); | ||
| arraysRegistry.registerNodes(this); | ||
| mathRegistry.registerNodes(this); | ||
| logicRegistry.registerNodes(this); | ||
| objectsRegistry.registerNodes(this); | ||
| usersRegistry.registerNodes(this); | ||
| botRegistry.registerNodes(this); | ||
| debugRegistry.registerNodes(this); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Отличный рефакторинг на модульную архитектуру.
Вынос регистраций узлов в отдельные модули по категориям значительно улучшает структуру кода, тестируемость и поддерживаемость. Подход с вызовом registerNodes(this) для каждого модуля обеспечивает чистую интеграцию.
🤖 Prompt for AI Agents
In backend/src/core/NodeRegistry.js around lines 112 to 141: reviewer approved
the modular refactor and there are no requested changes; no code modifications
are necessary—keep the current imports and registerNodes(this) calls for each
registry as-is.
There was a problem hiding this comment.
Привет! Я просмотрел ваши изменения — вот несколько замечаний:
- В GraphExecutionEngine и EventGraphManager много дублирующейся логики safeJsonParse + fallback — рассмотрите возможность извлечения многоразового вспомогательного метода, чтобы избежать дублирования кода для обработки ошибок и запасных вариантов для конкретных типов.
- Ручной импорт каждого подмодуля реестра узлов может стать причиной ошибок при добавлении категорий — рассмотрите возможность динамического обнаружения и загрузки реестров из файловой системы, чтобы уменьшить накладные расходы на обслуживание.
- Многие вызовы console.log/console.error по всему ядру можно заменить вашим логгером Pino для согласованного, структурированного логирования и лучшего контроля над уровнями логирования.
Подсказка для AI-агентов
Пожалуйста, учтите комментарии из этого обзора кода:
## Общие комментарии
- В GraphExecutionEngine и EventGraphManager много дублирующейся логики safeJsonParse + fallback — рассмотрите возможность извлечения многоразового вспомогательного метода, чтобы избежать дублирования кода для обработки ошибок и запасных вариантов для конкретных типов.
- Ручной импорт каждого подмодуля реестра узлов может стать причиной ошибок при добавлении категорий — рассмотрите возможность динамического обнаружения и загрузки реестров из файловой системы, чтобы уменьшить накладные расходы на обслуживание.
- Многие вызовы console.log/console.error по всему ядру можно заменить вашим логгером Pino для согласованного, структурированного логирования и лучшего контроля над уровнями логирования.
## Индивидуальные комментарии
### Комментарий 1
<location> `backend/src/core/__tests__/PrismaService.test.js:3-9` </location>
<code_context>
+const prismaService = require('../PrismaService');
+
+describe('PrismaService', () => {
+ it('should return singleton instance', () => {
+ const service1 = require('../PrismaService');
+ const service2 = require('../PrismaService');
+
+ expect(service1).toBe(service2);
+ });
+
+ it('should provide getClient method', () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Одиночный экземпляр и повторное использование клиента для PrismaService хорошо протестированы.
Пожалуйста, добавьте тест для метода disconnect, чтобы убедиться, что ресурсы правильно освобождаются.
</issue_to_address>
### Комментарий 2
<location> `backend/src/core/nodes/logic/__tests__/compare.test.js:66-73` </location>
<code_context>
+ expect(result).toBe(true);
+ });
+
+ it('should handle string comparison', async () => {
+ const node = { id: 'test', data: { operation: '<' } };
+ const helpers = mockHelpers('apple', 'banana');
+
+ const result = await logicCompare.evaluate(node, 'result', {}, helpers);
+
+ expect(result).toBe(true);
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Добавьте тесты для граничных случаев при приведении типов и смешанных типах.
Добавьте тесты с 'a' и 'b' в качестве разных типов (например, число против строки, булево значение против строки), чтобы проверить правильную обработку приведения типов или ожидаемые ошибки.
```suggestion
it('should handle string comparison', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers('apple', 'banana');
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
expect(result).toBe(true);
});
it('should handle number vs string comparison (number < string)', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers(5, '10');
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
// JS coerces '10' to number, so 5 < 10 is true
expect(result).toBe(true);
});
it('should handle string vs number comparison (string < number)', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers('5', 10);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
// JS coerces '5' to number, so 5 < 10 is true
expect(result).toBe(true);
});
it('should handle boolean vs string comparison', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers(true, 'false');
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
// true coerces to 1, 'false' coerces to NaN, 1 < NaN is false
expect(result).toBe(false);
});
it('should handle boolean vs number comparison', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers(false, 1);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
// false coerces to 0, 0 < 1 is true
expect(result).toBe(true);
});
it('should handle string vs boolean comparison', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers('1', false);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
// '1' coerces to 1, false to 0, 1 < 0 is false
expect(result).toBe(false);
});
it('should handle null vs number comparison', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers(null, 1);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
// null coerces to 0, 0 < 1 is true
expect(result).toBe(true);
});
it('should handle undefined vs number comparison', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers(undefined, 1);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
// undefined coerces to NaN, NaN < 1 is false
expect(result).toBe(false);
});
```
</issue_to_address>
### Комментарий 3
<location> `backend/src/core/nodes/math/__tests__/operation.test.js:1-10` </location>
<code_context>
+describe('math:operation node', () => {
+ const mathOperation = require('../operation');
+
+ const mockHelpers = (aValue, bValue) => ({
+ resolvePinValue: jest.fn(async (node, pinId, defaultValue) => {
+ if (pinId === 'a') return aValue;
+ if (pinId === 'b') return bValue;
+ return defaultValue;
+ }),
+ });
+
+ it('should add two numbers', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Хорошее покрытие для узла math:operation.
Пожалуйста, добавьте тесты для нечисловых входных данных и экстремальных значений, чтобы еще больше повысить надежность.
</issue_to_address>Sourcery бесплатен для открытого исходного кода — если вам нравятся наши обзоры, пожалуйста, рассмотрите возможность поделиться ими ✨
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- There’s a lot of duplicated safeJsonParse + fallback logic in GraphExecutionEngine and EventGraphManager—consider extracting a reusable helper to DRY up error handling and type‐specific fallbacks.
- Manually importing each node-registry submodule can become error-prone as you add categories—consider dynamically discovering and loading registries from the file system to reduce maintenance overhead.
- Many of the console.log/console.error calls throughout the core could be replaced with your Pino logger for consistent, structured logging and better control over log levels.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s a lot of duplicated safeJsonParse + fallback logic in GraphExecutionEngine and EventGraphManager—consider extracting a reusable helper to DRY up error handling and type‐specific fallbacks.
- Manually importing each node-registry submodule can become error-prone as you add categories—consider dynamically discovering and loading registries from the file system to reduce maintenance overhead.
- Many of the console.log/console.error calls throughout the core could be replaced with your Pino logger for consistent, structured logging and better control over log levels.
## Individual Comments
### Comment 1
<location> `backend/src/core/__tests__/PrismaService.test.js:3-9` </location>
<code_context>
+const prismaService = require('../PrismaService');
+
+describe('PrismaService', () => {
+ it('should return singleton instance', () => {
+ const service1 = require('../PrismaService');
+ const service2 = require('../PrismaService');
+
+ expect(service1).toBe(service2);
+ });
+
+ it('should provide getClient method', () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Singleton and client reuse for PrismaService are well tested.
Please add a test for the disconnect method to verify that resources are properly released.
</issue_to_address>
### Comment 2
<location> `backend/src/core/nodes/logic/__tests__/compare.test.js:66-73` </location>
<code_context>
+ expect(result).toBe(true);
+ });
+
+ it('should handle string comparison', async () => {
+ const node = { id: 'test', data: { operation: '<' } };
+ const helpers = mockHelpers('apple', 'banana');
+
+ const result = await logicCompare.evaluate(node, 'result', {}, helpers);
+
+ expect(result).toBe(true);
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for edge cases in type coercion and mixed types.
Add tests with 'a' and 'b' as different types (e.g., number vs string, boolean vs string) to verify correct handling of type coercion or expected errors.
```suggestion
it('should handle string comparison', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers('apple', 'banana');
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
expect(result).toBe(true);
});
it('should handle number vs string comparison (number < string)', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers(5, '10');
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
// JS coerces '10' to number, so 5 < 10 is true
expect(result).toBe(true);
});
it('should handle string vs number comparison (string < number)', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers('5', 10);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
// JS coerces '5' to number, so 5 < 10 is true
expect(result).toBe(true);
});
it('should handle boolean vs string comparison', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers(true, 'false');
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
// true coerces to 1, 'false' coerces to NaN, 1 < NaN is false
expect(result).toBe(false);
});
it('should handle boolean vs number comparison', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers(false, 1);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
// false coerces to 0, 0 < 1 is true
expect(result).toBe(true);
});
it('should handle string vs boolean comparison', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers('1', false);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
// '1' coerces to 1, false to 0, 1 < 0 is false
expect(result).toBe(false);
});
it('should handle null vs number comparison', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers(null, 1);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
// null coerces to 0, 0 < 1 is true
expect(result).toBe(true);
});
it('should handle undefined vs number comparison', async () => {
const node = { id: 'test', data: { operation: '<' } };
const helpers = mockHelpers(undefined, 1);
const result = await logicCompare.evaluate(node, 'result', {}, helpers);
// undefined coerces to NaN, NaN < 1 is false
expect(result).toBe(false);
});
```
</issue_to_address>
### Comment 3
<location> `backend/src/core/nodes/math/__tests__/operation.test.js:1-10` </location>
<code_context>
+describe('math:operation node', () => {
+ const mathOperation = require('../operation');
+
+ const mockHelpers = (aValue, bValue) => ({
+ resolvePinValue: jest.fn(async (node, pinId, defaultValue) => {
+ if (pinId === 'a') return aValue;
+ if (pinId === 'b') return bValue;
+ return defaultValue;
+ }),
+ });
+
+ it('should add two numbers', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Good coverage for math:operation node.
Please add tests for non-numeric inputs and extreme values to further improve robustness.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| describe('PrismaService', () => { | ||
| it('should return singleton instance', () => { | ||
| const service1 = require('../PrismaService'); | ||
| const service2 = require('../PrismaService'); | ||
|
|
||
| expect(service1).toBe(service2); | ||
| }); |
There was a problem hiding this comment.
suggestion (testing): Одиночный экземпляр и повторное использование клиента для PrismaService хорошо протестированы.
Пожалуйста, добавьте тест для метода disconnect, чтобы убедиться, что ресурсы правильно освобождаются.
Original comment in English
suggestion (testing): Singleton and client reuse for PrismaService are well tested.
Please add a test for the disconnect method to verify that resources are properly released.
| it('should handle string comparison', async () => { | ||
| const node = { id: 'test', data: { operation: '<' } }; | ||
| const helpers = mockHelpers('apple', 'banana'); | ||
|
|
||
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | ||
|
|
||
| expect(result).toBe(true); | ||
| }); |
There was a problem hiding this comment.
suggestion (testing): Добавьте тесты для граничных случаев при приведении типов и смешанных типах.
Добавьте тесты с 'a' и 'b' в качестве разных типов (например, число против строки, булево значение против строки), чтобы проверить правильную обработку приведения типов или ожидаемые ошибки.
| it('should handle string comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers('apple', 'banana'); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle string comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers('apple', 'banana'); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle number vs string comparison (number < string)', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers(5, '10'); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| // JS coerces '10' to number, so 5 < 10 is true | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle string vs number comparison (string < number)', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers('5', 10); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| // JS coerces '5' to number, so 5 < 10 is true | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle boolean vs string comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers(true, 'false'); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| // true coerces to 1, 'false' coerces to NaN, 1 < NaN is false | |
| expect(result).toBe(false); | |
| }); | |
| it('should handle boolean vs number comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers(false, 1); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| // false coerces to 0, 0 < 1 is true | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle string vs boolean comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers('1', false); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| // '1' coerces to 1, false to 0, 1 < 0 is false | |
| expect(result).toBe(false); | |
| }); | |
| it('should handle null vs number comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers(null, 1); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| // null coerces to 0, 0 < 1 is true | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle undefined vs number comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers(undefined, 1); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| // undefined coerces to NaN, NaN < 1 is false | |
| expect(result).toBe(false); | |
| }); |
Original comment in English
suggestion (testing): Add tests for edge cases in type coercion and mixed types.
Add tests with 'a' and 'b' as different types (e.g., number vs string, boolean vs string) to verify correct handling of type coercion or expected errors.
| it('should handle string comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers('apple', 'banana'); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle string comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers('apple', 'banana'); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle number vs string comparison (number < string)', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers(5, '10'); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| // JS coerces '10' to number, so 5 < 10 is true | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle string vs number comparison (string < number)', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers('5', 10); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| // JS coerces '5' to number, so 5 < 10 is true | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle boolean vs string comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers(true, 'false'); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| // true coerces to 1, 'false' coerces to NaN, 1 < NaN is false | |
| expect(result).toBe(false); | |
| }); | |
| it('should handle boolean vs number comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers(false, 1); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| // false coerces to 0, 0 < 1 is true | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle string vs boolean comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers('1', false); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| // '1' coerces to 1, false to 0, 1 < 0 is false | |
| expect(result).toBe(false); | |
| }); | |
| it('should handle null vs number comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers(null, 1); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| // null coerces to 0, 0 < 1 is true | |
| expect(result).toBe(true); | |
| }); | |
| it('should handle undefined vs number comparison', async () => { | |
| const node = { id: 'test', data: { operation: '<' } }; | |
| const helpers = mockHelpers(undefined, 1); | |
| const result = await logicCompare.evaluate(node, 'result', {}, helpers); | |
| // undefined coerces to NaN, NaN < 1 is false | |
| expect(result).toBe(false); | |
| }); |
| describe('math:operation node', () => { | ||
| const mathOperation = require('../operation'); | ||
|
|
||
| const mockHelpers = (aValue, bValue) => ({ | ||
| resolvePinValue: jest.fn(async (node, pinId, defaultValue) => { | ||
| if (pinId === 'a') return aValue; | ||
| if (pinId === 'b') return bValue; | ||
| return defaultValue; | ||
| }), | ||
| }); |
There was a problem hiding this comment.
suggestion (testing): Хорошее покрытие для узла math:operation.
Пожалуйста, добавьте тесты для нечисловых входных данных и экстремальных значений, чтобы еще больше повысить надежность.
Original comment in English
suggestion (testing): Good coverage for math:operation node.
Please add tests for non-numeric inputs and extreme values to further improve robustness.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- There’s duplicated safeJsonParse + validation logic in both GraphExecutionEngine and EventGraphManager—extract that into a shared preprocessing helper to reduce repetition and improve maintainability.
- Requiring a dozen individual registry modules in NodeRegistry may slow startup—consider aggregating or lazy-loading these registry imports to minimize initial overhead.
- The frontend’s debounced autoSaveGraph is only called on unmount—integrate it into the store’s change handlers so you don’t rely solely on flushSaveGraph in component cleanup and risk missing saves.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s duplicated safeJsonParse + validation logic in both GraphExecutionEngine and EventGraphManager—extract that into a shared preprocessing helper to reduce repetition and improve maintainability.
- Requiring a dozen individual registry modules in NodeRegistry may slow startup—consider aggregating or lazy-loading these registry imports to minimize initial overhead.
- The frontend’s debounced autoSaveGraph is only called on unmount—integrate it into the store’s change handlers so you don’t rely solely on flushSaveGraph in component cleanup and risk missing saves.
## Individual Comments
### Comment 1
<location> `backend/src/core/validation/__tests__/nodeSchemas.test.js:37-49` </location>
<code_context>
+ expect(result.error).toBeDefined();
+ });
+
+ it('should fail validation for invalid pin structure', () => {
+ const invalidConfig = {
+ type: 'test:node',
+ label: 'Test Node',
+ category: 'Test',
+ inputs: [
+ { id: '', name: 'Input 1', type: 'String' }
+ ],
+ };
+
+ const result = validateNodeConfig(invalidConfig);
+ expect(result.success).toBe(false);
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding tests for edge cases in pin validation.
Please include tests for pins with missing or extra fields, and for unexpected types such as non-string id or name, to ensure schema violations are detected.
```suggestion
it('should fail validation for invalid pin structure', () => {
const invalidConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ id: '', name: 'Input 1', type: 'String' }
],
};
const result = validateNodeConfig(invalidConfig);
expect(result.success).toBe(false);
});
it('should fail validation for pin missing required fields', () => {
const missingFieldsConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ name: 'Input 1', type: 'String' } // missing id
],
};
const result = validateNodeConfig(missingFieldsConfig);
expect(result.success).toBe(false);
});
it('should fail validation for pin with extra fields', () => {
const extraFieldsConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ id: 'input1', name: 'Input 1', type: 'String', extra: 'unexpected' }
],
};
const result = validateNodeConfig(extraFieldsConfig);
expect(result.success).toBe(false);
});
it('should fail validation for pin with non-string id', () => {
const nonStringIdConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ id: 123, name: 'Input 1', type: 'String' }
],
};
const result = validateNodeConfig(nonStringIdConfig);
expect(result.success).toBe(false);
});
it('should fail validation for pin with non-string name', () => {
const nonStringNameConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ id: 'input1', name: {}, type: 'String' }
],
};
const result = validateNodeConfig(nonStringNameConfig);
expect(result.success).toBe(false);
});
```
</issue_to_address>
### Comment 2
<location> `backend/src/core/__tests__/PrismaService.test.js:3-9` </location>
<code_context>
+const prismaService = require('../PrismaService');
+
+describe('PrismaService', () => {
+ it('should return singleton instance', () => {
+ const service1 = require('../PrismaService');
+ const service2 = require('../PrismaService');
+
+ expect(service1).toBe(service2);
+ });
+
+ it('should provide getClient method', () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Singleton and client reuse for PrismaService are well tested.
Please add a test for the disconnect method to verify its correct behavior.
</issue_to_address>
### Comment 3
<location> `backend/src/core/nodes/logic/__tests__/compare.test.js:1-10` </location>
<code_context>
+describe('logic:compare node', () => {
+ const logicCompare = require('../compare');
+
+ const mockHelpers = (aValue, bValue) => ({
+ resolvePinValue: jest.fn(async (node, pinId) => {
+ if (pinId === 'a') return aValue;
+ if (pinId === 'b') return bValue;
+ return null;
+ }),
+ });
+
+ it('should compare greater than', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Logic compare node tests cover all standard operations and types.
Please add tests for edge cases like comparing null, undefined, or objects to verify correct handling.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| it('should fail validation for invalid pin structure', () => { | ||
| const invalidConfig = { | ||
| type: 'test:node', | ||
| label: 'Test Node', | ||
| category: 'Test', | ||
| inputs: [ | ||
| { id: '', name: 'Input 1', type: 'String' } | ||
| ], | ||
| }; | ||
|
|
||
| const result = validateNodeConfig(invalidConfig); | ||
| expect(result.success).toBe(false); | ||
| }); |
There was a problem hiding this comment.
suggestion (testing): Consider adding tests for edge cases in pin validation.
Please include tests for pins with missing or extra fields, and for unexpected types such as non-string id or name, to ensure schema violations are detected.
| it('should fail validation for invalid pin structure', () => { | |
| const invalidConfig = { | |
| type: 'test:node', | |
| label: 'Test Node', | |
| category: 'Test', | |
| inputs: [ | |
| { id: '', name: 'Input 1', type: 'String' } | |
| ], | |
| }; | |
| const result = validateNodeConfig(invalidConfig); | |
| expect(result.success).toBe(false); | |
| }); | |
| it('should fail validation for invalid pin structure', () => { | |
| const invalidConfig = { | |
| type: 'test:node', | |
| label: 'Test Node', | |
| category: 'Test', | |
| inputs: [ | |
| { id: '', name: 'Input 1', type: 'String' } | |
| ], | |
| }; | |
| const result = validateNodeConfig(invalidConfig); | |
| expect(result.success).toBe(false); | |
| }); | |
| it('should fail validation for pin missing required fields', () => { | |
| const missingFieldsConfig = { | |
| type: 'test:node', | |
| label: 'Test Node', | |
| category: 'Test', | |
| inputs: [ | |
| { name: 'Input 1', type: 'String' } // missing id | |
| ], | |
| }; | |
| const result = validateNodeConfig(missingFieldsConfig); | |
| expect(result.success).toBe(false); | |
| }); | |
| it('should fail validation for pin with extra fields', () => { | |
| const extraFieldsConfig = { | |
| type: 'test:node', | |
| label: 'Test Node', | |
| category: 'Test', | |
| inputs: [ | |
| { id: 'input1', name: 'Input 1', type: 'String', extra: 'unexpected' } | |
| ], | |
| }; | |
| const result = validateNodeConfig(extraFieldsConfig); | |
| expect(result.success).toBe(false); | |
| }); | |
| it('should fail validation for pin with non-string id', () => { | |
| const nonStringIdConfig = { | |
| type: 'test:node', | |
| label: 'Test Node', | |
| category: 'Test', | |
| inputs: [ | |
| { id: 123, name: 'Input 1', type: 'String' } | |
| ], | |
| }; | |
| const result = validateNodeConfig(nonStringIdConfig); | |
| expect(result.success).toBe(false); | |
| }); | |
| it('should fail validation for pin with non-string name', () => { | |
| const nonStringNameConfig = { | |
| type: 'test:node', | |
| label: 'Test Node', | |
| category: 'Test', | |
| inputs: [ | |
| { id: 'input1', name: {}, type: 'String' } | |
| ], | |
| }; | |
| const result = validateNodeConfig(nonStringNameConfig); | |
| expect(result.success).toBe(false); | |
| }); |
| describe('logic:compare node', () => { | ||
| const logicCompare = require('../compare'); | ||
|
|
||
| const mockHelpers = (aValue, bValue) => ({ | ||
| resolvePinValue: jest.fn(async (node, pinId) => { | ||
| if (pinId === 'a') return aValue; | ||
| if (pinId === 'b') return bValue; | ||
| return null; | ||
| }), | ||
| }); |
There was a problem hiding this comment.
suggestion (testing): Logic compare node tests cover all standard operations and types.
Please add tests for edge cases like comparing null, undefined, or objects to verify correct handling.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
backend/src/core/GraphExecutionEngine.js (5)
1-5: Неиспользуемыйprismaи возможная лишняя инициализация клиентаВ этом модуле переменная
prisma, полученная черезprismaService.getClient(), нигде далее не используется. Это выглядит как мёртвый код и усложняет чтение.Если ранняя инициализация клиента через
PrismaServiceне задумана как побочный эффект (например, прогрев соединения), имеет смысл удалить и импортPrismaService, и созданиеprisma. Если же инициализация действительно нужна, стоит либо сохранить клиента в поле экземпляра (this.prisma) и явно прокидывать его в executor'ы, либо добавить комментарий, чтобы было понятно, что это осознанный side effect, а не забытый код.
25-49: Парсинг графа и валидация выглядят корректно, но флагVALIDATION_ENABLEDможет спрятать ошибкиПлюсы текущей реализации:
- Использование
safeJsonParse(graph, null, 'GraphExecutionEngine.execute')даёт более безопасный парсинг и контекст в логах.- При ошибке парсинга (
parsedGraphложный) выполнение явно пропускается и возвращается исходныйcontext.- Валидация через
validateGraph(parsedGraph)обёрнута в флагthis.validationEnabled, а строгий режим контролируетсяVALIDATION_STRICT_MODE:
- при строгом режиме выбрасывается ошибка
"Invalid graph structure",- в противном случае логируется предупреждение и выполнение графа пропускается.
Небольшие замечания/предложения:
- Если в конфиге кто‑то выставит
VALIDATION_ENABLED = false, валидация полностью отключится без явного сигнала — это может позволить невалидным графам бесшумно уходить в прод. Имеет смысл либо логировать значение флага при старте, либо явно задокументировать, что отключать его в продакшене не рекомендуется.- Сообщение
// В production...фактически завязано на значенииVALIDATION_STRICT_MODE; если конфиг будут менять, комментарий может стать неточным — стоит либо сослаться на флаг, либо уточнить формулировку.
25-31: Поведение при ошибке парсинга графа может быть трудно отлаживатьСейчас при некорректном JSON
safeJsonParseвозвращаетnull(по умолчанию), после чегоexecuteпросто возвращает исходныйcontext, не пытаясь выполнять граф.Если
safeJsonParseуже достаточно информативно логирует ошибки, этого, возможно, достаточно. Но если логов на уровне util мало, то такой "тихий" early-return усложнит диагностику проблем с графом (особенно в проде).Подумайте, не стоит ли:
- либо логировать отдельное предупреждение прямо здесь (с указанием, что граф не будет выполнен из-за ошибки парсинга),
- либо хотя бы в dev/test окружении выбрасывать исключение, а в продакшене продолжать использовать текущий мягкий сценарий.
60-79: Обработка переменных типаarrayвыглядит устойчивойБлок инициализации
this.context.variablesи особенно веткаcase 'array':реализованы аккуратно:
- Используется
safeJsonParse(value, [], \variable ${variable.name}`)` с безопасным дефолтом.- Дополнительная проверка
Array.isArray(value)и предупреждение в логе защищают от случаев, когда парсер вернул не массив.- Фоллбек на
[]предотвращает падение графа из‑за некорректного значения по умолчанию.С функциональной точки зрения этот фрагмент выглядит надёжным. Единственное, что можно рассмотреть в будущем — вынести преобразование типов (
number/boolean/array) в отдельный helper, чтобы избежать дублирования логики, если подобное преобразование понадобится ещё где‑то.
330-356: Лимит глубины вisNodeVolatileможет приводить к излишнему кешированию в очень глубоких графахНовая версия
isNodeVolatile(node, visited = new Set(), depth = 0)с:
- контролем глубины через
MAX_RECURSION_DEPTH,- трекингом
visitedдля разрыва циклов,хорошо защищает от бесконечной рекурсии и переполнения стека.
Однако при срабатывании лимита сейчас выполняется:
if (depth > MAX_RECURSION_DEPTH) { console.warn('[GraphExecutionEngine] isNodeVolatile достиг максимальной глубины рекурсии (...)'); return false; }То есть узел, для которого не удалось "досмотреть" цепочку зависимостей до конца, маркируется как неволатильный, и его результат может быть закеширован, даже если выше по цепочке есть
data:get_variable.С точки зрения корректности безопаснее вести себя пессимистично и считать такой узел волатильным (не кешировать), чтобы не получить потенциально устаревшие значения в очень глубоких/сложных графах:
- if (depth > MAX_RECURSION_DEPTH) { - console.warn(`[GraphExecutionEngine] isNodeVolatile достиг максимальной глубины рекурсии (${MAX_RECURSION_DEPTH})`); - return false; - } + if (depth > MAX_RECURSION_DEPTH) { + console.warn(`[GraphExecutionEngine] isNodeVolatile достиг максимальной глубины рекурсии (${MAX_RECURSION_DEPTH}), считаем узел волатильным`); + return true; + }Так вы сохраните защиту от runaway‑рекурсии, но избежите рискованного кеширования в пограничных случаях.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/src/core/GraphExecutionEngine.js(4 hunks)backend/src/core/NodeRegistry.js(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
backend/src/core/NodeRegistry.js (2)
backend/src/core/GraphExecutionEngine.js (3)
require(2-2)require(3-3)require(4-4)backend/src/core/config/validation.js (2)
VALIDATION_ENABLED(8-8)VALIDATION_STRICT_MODE(14-14)
backend/src/core/GraphExecutionEngine.js (3)
backend/src/core/PrismaService.js (1)
prismaService(31-31)backend/src/core/config/validation.js (3)
VALIDATION_ENABLED(8-8)VALIDATION_STRICT_MODE(14-14)MAX_RECURSION_DEPTH(21-21)frontend/src/components/visual-editor/SettingsPanel.jsx (1)
safeJsonParse(26-35)
⏰ 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). (2)
- GitHub Check: Sourcery review
- GitHub Check: Sourcery review
🔇 Additional comments (4)
backend/src/core/NodeRegistry.js (4)
1-3: Корректные импорты для валидации.Импорты поддерживают новую функциональность валидации с использованием Zod-схем и переключения строгого/production режимов.
27-31: Правильная инициализация флага валидации.Поле
validationEnabledпозволяет управлять валидацией через конфигурацию.
42-57: Предыдущие замечания успешно устранены.Валидационная логика теперь реализована корректно:
- Используется структурированное логирование вместо
JSON.stringify- В production-режиме невалидные ноды пропускаются (не регистрируются), а не вызывают сбой приложения
- Сообщения об ошибках стали лаконичными
Поведение соответствует best practices: строгая валидация в dev/test окружениях (fail fast) и graceful degradation в production.
116-147: Отличная модульная архитектура, предыдущее одобрение сохраняется.Разделение регистраций нод по категориям значительно улучшает поддерживаемость и тестируемость кода. Использование
require()внутри метода оправдано необходимостью избежать циклических зависимостей, поскольку модули регистрации вызываютregisterNodeType.Отсутствие обработки ошибок вокруг
require()иregisterNodes()вызовов является правильным решением — если базовые регистры не загружаются, приложение не должно запускаться.
|
@sourcery-ai review |
There was a problem hiding this comment.
Привет - я просмотрел ваши изменения - вот несколько замечаний:
- Возможно, стоит выделить флаги и логику валидации и JSON-парсинга в общий ValidationService (или аналогичный сервис), чтобы избежать дублирования кода в GraphExecutionEngine и EventGraphManager и улучшить тестируемость.
- Рассмотрите возможность улучшения вашей утилиты debounce, чтобы ее метод flush не только отменял ожидающий таймаут, но и немедленно вызывал ожидающую функцию, гарантируя, что ни один вызов автосохранения не будет потерян.
- Поскольку NodeRegistry разделен на несколько реестров, добавьте простой дымовой тест или проверку работоспособности, чтобы убедиться, что каждый ожидаемый тип узла по-прежнему зарегистрирован после рефакторинга и что порядок загрузки модулей остается детерминированным.
Подсказка для агентов ИИ
Пожалуйста, рассмотрите комментарии из этого обзора кода:
## Общие комментарии
- Возможно, стоит выделить флаги и логику валидации и JSON-парсинга в общий ValidationService (или аналогичный сервис), чтобы избежать дублирования кода в GraphExecutionEngine и EventGraphManager и улучшить тестируемость.
- Рассмотрите возможность улучшения вашей утилиты debounce, чтобы ее метод flush не только отменял ожидающий таймаут, но и немедленно вызывал ожидающую функцию, гарантируя, что ни один вызов автосохранения не будет потерян.
- Поскольку NodeRegistry разделен на несколько реестров, добавьте простой дымовой тест или проверку работоспособности, чтобы убедиться, что каждый ожидаемый тип узла по-прежнему зарегистрирован после рефакторинга и что порядок загрузки модулей остается детерминированным.
## Индивидуальные комментарии
### Comment 1
<location> `backend/src/core/__tests__/PrismaService.test.js:3-9` </location>
<code_context>
+const prismaService = require('../PrismaService');
+
+describe('PrismaService', () => {
+ it('should return singleton instance', () => {
+ const service1 = require('../PrismaService');
+ const service2 = require('../PrismaService');
+
+ expect(service1).toBe(service2);
+ });
+
+ it('should provide getClient method', () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Модульные тесты для синглтона PrismaService и повторного использования клиента.
Добавление теста для логики отключения улучшит покрытие и поможет обеспечить правильную очистку ресурсов.
</issue_to_address>
### Comment 2
<location> `backend/src/core/nodes/logic/__tests__/compare.test.js:1-10` </location>
<code_context>
+describe('logic:compare node', () => {
+ const logicCompare = require('../compare');
+
+ const mockHelpers = (aValue, bValue) => ({
+ resolvePinValue: jest.fn(async (node, pinId) => {
+ if (pinId === 'a') return aValue;
+ if (pinId === 'b') return bValue;
+ return null;
+ }),
+ });
+
+ it('should compare greater than', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Тесты узла сравнения логики охватывают все операции сравнения и типы.
Рассмотрите возможность добавления тестов для граничных случаев, таких как сравнения null/undefined и приведение типов, чтобы повысить надежность.
</issue_to_address>
### Comment 3
<location> `backend/src/core/nodes/math/__tests__/operation.test.js:1-10` </location>
<code_context>
+describe('math:operation node', () => {
+ const mathOperation = require('../operation');
+
+ const mockHelpers = (aValue, bValue) => ({
+ resolvePinValue: jest.fn(async (node, pinId, defaultValue) => {
+ if (pinId === 'a') return aValue;
+ if (pinId === 'b') return bValue;
+ return defaultValue;
+ }),
+ });
+
+ it('should add two numbers', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Тесты узла математических операций охватывают все основные операции и деление на ноль.
Рассмотрите возможность добавления тестов для нечисловых входных данных и точности чисел с плавающей запятой, чтобы обеспечить надежность.
</issue_to_address>
### Comment 4
<location> `backend/src/core/validation/__tests__/nodeSchemas.test.js:37-49` </location>
<code_context>
+ expect(result.error).toBeDefined();
+ });
+
+ it('should fail validation for invalid pin structure', () => {
+ const invalidConfig = {
+ type: 'test:node',
+ label: 'Test Node',
+ category: 'Test',
+ inputs: [
+ { id: '', name: 'Input 1', type: 'String' }
+ ],
+ };
+
+ const result = validateNodeConfig(invalidConfig);
+ expect(result.success).toBe(false);
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Тест на неверную структуру пина присутствует, но может быть расширен.
Добавьте тестовые случаи для пинов, у которых отсутствуют обязательные поля или которые имеют неверные типы данных, чтобы улучшить покрытие валидации схемы.
```suggestion
it('should fail validation for invalid pin structure', () => {
const invalidConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ id: '', name: 'Input 1', type: 'String' }
],
};
const result = validateNodeConfig(invalidConfig);
expect(result.success).toBe(false);
});
it('should fail validation when pin is missing required fields', () => {
const missingIdConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ name: 'Input 1', type: 'String' } // missing 'id'
],
};
const missingNameConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ id: 'input1', type: 'String' } // missing 'name'
],
};
const missingTypeConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ id: 'input1', name: 'Input 1' } // missing 'type'
],
};
expect(validateNodeConfig(missingIdConfig).success).toBe(false);
expect(validateNodeConfig(missingNameConfig).success).toBe(false);
expect(validateNodeConfig(missingTypeConfig).success).toBe(false);
});
it('should fail validation when pin fields have incorrect data types', () => {
const idAsNumberConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ id: 123, name: 'Input 1', type: 'String' } // id should be string
],
};
const nameAsObjectConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ id: 'input1', name: { text: 'Input 1' }, type: 'String' } // name should be string
],
};
const typeAsArrayConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ id: 'input1', name: 'Input 1', type: ['String'] } // type should be string
],
};
expect(validateNodeConfig(idAsNumberConfig).success).toBe(false);
expect(validateNodeConfig(nameAsObjectConfig).success).toBe(false);
expect(validateNodeConfig(typeAsArrayConfig).success).toBe(false);
});
```
</issue_to_address>Sourcery бесплатен для открытого исходного кода — если вам нравятся наши обзоры, пожалуйста, рассмотрите возможность поделиться ими ✨
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- It may be worth extracting the validation and JSON‐parsing flags and logic into a shared ValidationService (or similar) to avoid duplicating code in GraphExecutionEngine and EventGraphManager and to improve testability.
- Consider enhancing your debounce util so that its flush method not only cancels the pending timeout but also invokes the pending function immediately, ensuring that no autosave call is ever lost.
- With NodeRegistry split into multiple registries, add a simple smoke test or health check to verify that every expected node type is still registered after refactoring and that module load order remains deterministic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- It may be worth extracting the validation and JSON‐parsing flags and logic into a shared ValidationService (or similar) to avoid duplicating code in GraphExecutionEngine and EventGraphManager and to improve testability.
- Consider enhancing your debounce util so that its flush method not only cancels the pending timeout but also invokes the pending function immediately, ensuring that no autosave call is ever lost.
- With NodeRegistry split into multiple registries, add a simple smoke test or health check to verify that every expected node type is still registered after refactoring and that module load order remains deterministic.
## Individual Comments
### Comment 1
<location> `backend/src/core/__tests__/PrismaService.test.js:3-9` </location>
<code_context>
+const prismaService = require('../PrismaService');
+
+describe('PrismaService', () => {
+ it('should return singleton instance', () => {
+ const service1 = require('../PrismaService');
+ const service2 = require('../PrismaService');
+
+ expect(service1).toBe(service2);
+ });
+
+ it('should provide getClient method', () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Unit tests for PrismaService singleton and client reuse.
Adding a test for disconnect logic would improve coverage and help ensure proper resource cleanup.
</issue_to_address>
### Comment 2
<location> `backend/src/core/nodes/logic/__tests__/compare.test.js:1-10` </location>
<code_context>
+describe('logic:compare node', () => {
+ const logicCompare = require('../compare');
+
+ const mockHelpers = (aValue, bValue) => ({
+ resolvePinValue: jest.fn(async (node, pinId) => {
+ if (pinId === 'a') return aValue;
+ if (pinId === 'b') return bValue;
+ return null;
+ }),
+ });
+
+ it('should compare greater than', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Logic compare node tests cover all comparison operations and types.
Consider adding tests for edge cases like null/undefined comparisons and type coercion to enhance robustness.
</issue_to_address>
### Comment 3
<location> `backend/src/core/nodes/math/__tests__/operation.test.js:1-10` </location>
<code_context>
+describe('math:operation node', () => {
+ const mathOperation = require('../operation');
+
+ const mockHelpers = (aValue, bValue) => ({
+ resolvePinValue: jest.fn(async (node, pinId, defaultValue) => {
+ if (pinId === 'a') return aValue;
+ if (pinId === 'b') return bValue;
+ return defaultValue;
+ }),
+ });
+
+ it('should add two numbers', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Math operation node tests cover all basic operations and division by zero.
Consider adding tests for non-numeric inputs and floating-point precision to ensure robustness.
</issue_to_address>
### Comment 4
<location> `backend/src/core/validation/__tests__/nodeSchemas.test.js:37-49` </location>
<code_context>
+ expect(result.error).toBeDefined();
+ });
+
+ it('should fail validation for invalid pin structure', () => {
+ const invalidConfig = {
+ type: 'test:node',
+ label: 'Test Node',
+ category: 'Test',
+ inputs: [
+ { id: '', name: 'Input 1', type: 'String' }
+ ],
+ };
+
+ const result = validateNodeConfig(invalidConfig);
+ expect(result.success).toBe(false);
+ });
+
</code_context>
<issue_to_address>
**suggestion (testing):** Test for invalid pin structure is present but could be expanded.
Add test cases for pins missing required fields or with incorrect data types to improve schema validation coverage.
```suggestion
it('should fail validation for invalid pin structure', () => {
const invalidConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ id: '', name: 'Input 1', type: 'String' }
],
};
const result = validateNodeConfig(invalidConfig);
expect(result.success).toBe(false);
});
it('should fail validation when pin is missing required fields', () => {
const missingIdConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ name: 'Input 1', type: 'String' } // missing 'id'
],
};
const missingNameConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ id: 'input1', type: 'String' } // missing 'name'
],
};
const missingTypeConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ id: 'input1', name: 'Input 1' } // missing 'type'
],
};
expect(validateNodeConfig(missingIdConfig).success).toBe(false);
expect(validateNodeConfig(missingNameConfig).success).toBe(false);
expect(validateNodeConfig(missingTypeConfig).success).toBe(false);
});
it('should fail validation when pin fields have incorrect data types', () => {
const idAsNumberConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ id: 123, name: 'Input 1', type: 'String' } // id should be string
],
};
const nameAsObjectConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ id: 'input1', name: { text: 'Input 1' }, type: 'String' } // name should be string
],
};
const typeAsArrayConfig = {
type: 'test:node',
label: 'Test Node',
category: 'Test',
inputs: [
{ id: 'input1', name: 'Input 1', type: ['String'] } // type should be string
],
};
expect(validateNodeConfig(idAsNumberConfig).success).toBe(false);
expect(validateNodeConfig(nameAsObjectConfig).success).toBe(false);
expect(validateNodeConfig(typeAsArrayConfig).success).toBe(false);
});
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
backend/src/core/utils/variableParser.js (1)
20-21: Парсинг булевых значений может быть слишком строгим.Текущая логика
value === 'true'обрабатывает только точное совпадение строки. Рассмотрите поддержку других распространённых представлений:
'false'→false'1'/'0'→ булевы значения- Числовые значения
Если текущее поведение намеренное, можно оставить как есть. Однако для большей гибкости рассмотрите:
case 'boolean': - return value === 'true'; + if (typeof value === 'boolean') return value; + if (typeof value === 'string') { + const lower = value.toLowerCase(); + return lower === 'true' || lower === '1' || lower === 'yes'; + } + return Boolean(value);backend/src/core/GraphExecutionEngine.js (1)
36-50: Проверьте стратегию обработки невалидных графов в production.Текущая логика в production пропускает выполнение невалидных графов с предупреждением (строки 47-48), что лучше безмолвного сбоя, но может скрывать критические проблемы конфигурации от разработчиков.
Рассмотрите:
- Метрики/алертинг: добавьте счётчик неудачных валидаций для мониторинга в production
- Структурированное логирование: используйте logger вместо
console.warnдля интеграции с системами наблюдения- Уведомления: критичные графы с ошибками валидации могут требовать немедленного уведомления команды
Пример с метриками:
if (VALIDATION_STRICT_MODE) { throw new Error('Invalid graph structure'); } // В production логируем и возвращаем контекст без выполнения console.warn('[GraphExecutionEngine] Skipping graph execution due to validation errors in production mode'); + // Increment metrics counter for monitoring + if (typeof metrics !== 'undefined') { + metrics.increment('graph.validation.failed'); + } return context;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/src/core/EventGraphManager.js(3 hunks)backend/src/core/GraphExecutionEngine.js(4 hunks)backend/src/core/utils/variableParser.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
backend/src/core/utils/variableParser.js (2)
backend/src/core/EventGraphManager.js (2)
require(4-4)require(5-5)backend/src/core/GraphExecutionEngine.js (4)
require(2-2)require(3-3)require(4-4)require(5-5)
backend/src/core/EventGraphManager.js (3)
backend/src/core/GraphExecutionEngine.js (6)
prismaService(1-1)require(2-2)require(3-3)require(4-4)require(5-5)prisma(6-6)backend/src/core/PrismaService.js (1)
prismaService(31-31)backend/src/core/nodes/users/set_blacklist.js (2)
prismaService(2-2)prisma(3-3)
backend/src/core/GraphExecutionEngine.js (4)
backend/src/core/EventGraphManager.js (4)
prismaService(3-3)require(4-4)require(5-5)prisma(7-7)backend/src/core/PrismaService.js (1)
prismaService(31-31)backend/src/core/nodes/users/set_blacklist.js (2)
prismaService(2-2)prisma(3-3)backend/src/core/config/validation.js (3)
VALIDATION_ENABLED(8-8)VALIDATION_STRICT_MODE(14-14)MAX_RECURSION_DEPTH(21-21)
⏰ 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). (2)
- GitHub Check: Sourcery review
- GitHub Check: Sourcery review
🔇 Additional comments (10)
backend/src/core/EventGraphManager.js (4)
3-7: Отличная централизация зависимостей!Переход на
PrismaService.getClient()и использование утилит безопасного парсинга улучшают модульность и надёжность кода.
37-38: Хорошая защита от невалидного JSON.Использование
safeJsonParseс проверкой наличияnodesпредотвращает выполнение графов с невалидной структурой.
42-43: Правильное использование централизованных утилит парсинга.Двухэтапный подход (JSON-парсинг → типизированный парсинг переменных) обеспечивает надёжную обработку данных графа.
137-143: Единообразная обработка переменных.Использование
parseVariableValueдля инициализации переменных обеспечивает консистентную типизацию во всём графе.backend/src/core/utils/variableParser.js (2)
50-61: Чистая реализация значений по умолчанию.Функция корректно возвращает типобезопасные значения по умолчанию для всех поддерживаемых типов.
69-83: Надёжная пакетная обработка переменных.Проверка типов входных данных и предупреждения о переменных без имени помогают выявить проблемы конфигурации на ранних этапах.
backend/src/core/GraphExecutionEngine.js (4)
1-21: Хорошая интеграция новых зависимостей.Импорты организованы логично, а использование
VALIDATION_ENABLEDиз конфигурации обеспечивает единую точку управления валидацией.
28-31: Безопасный парсинг графов.Использование
safeJsonParseс проверкой результата предотвращает выполнение при невалидном JSON.
62-66: Консистентная инициализация переменных контекста.Использование
parseVariablesгарантирует единообразную обработку переменных графа независимо от точки входа.
309-335: Отличная защита от бесконечной рекурсии!Добавление отслеживания посещённых узлов и ограничения глубины рекурсии предотвращает зависание на циклических графах. Предупреждение на строке 313 помогает идентифицировать аномально глубокие структуры.
|
@sourcery-ai review |
There was a problem hiding this comment.
Привет! Я просмотрел ваши изменения — вот несколько замечаний:
- Вместо того чтобы жестко кодировать каждый модуль реестра в NodeRegistry._registerBaseNodes, рассмотрите возможность автоматического обнаружения и подключения всех файлов в каталоге node-registries, чтобы уменьшить избыточность кода и избежать забывания добавлять новые реестры.
- GraphExecutionEngine вызывает как validateGraphStructure, так и hasValidBasicStructure — рассмотрите возможность объединения проверок базовой структуры в validateGraphStructure, чтобы централизовать логику валидации и упростить поток выполнения.
- Вы используете строковые литералы для graphType ('all', 'command', 'event') по всему коду; извлечение их в общие константы или перечисление поможет предотвратить опечатки и сохранить значения согласованными.
Подсказка для агентов ИИ
Пожалуйста, учтите комментарии из этого обзора кода:
## Общие комментарии
- Вместо того чтобы жестко кодировать каждый модуль реестра в NodeRegistry._registerBaseNodes, рассмотрите возможность автоматического обнаружения и подключения всех файлов в каталоге node-registries, чтобы уменьшить избыточность кода и избежать забывания добавлять новые реестры.
- GraphExecutionEngine вызывает как validateGraphStructure, так и hasValidBasicStructure — рассмотрите возможность объединения проверок базовой структуры в validateGraphStructure, чтобы централизовать логику валидации и упростить поток выполнения.
- Вы используете строковые литералы для graphType ('all', 'command', 'event') по всему коду; извлечение их в общие константы или перечисление поможет предотвратить опечатки и сохранить значения согласованными.
## Индивидуальные комментарии
### Комментарий 1
<location> `backend/src/core/services/__tests__/ValidationService.test.js:3-10` </location>
<code_context>
+const validationService = require('../ValidationService');
+
+describe('ValidationService', () => {
+ describe('parseGraph', () => {
+ it('should parse JSON string graph', () => {
+ const graphJson = JSON.stringify({ nodes: [], connections: [] });
+ const result = validationService.parseGraph(graphJson, 'test');
+
+ expect(result).toEqual({ nodes: [], connections: [] });
+ });
+
+ it('should return graph object as-is', () => {
</code_context>
<issue_to_address>
**предложение (тестирование):** Тесты ValidationService охватывают строгий и нестрогий режимы, но им не хватает больше граничных случаев.
Рассмотрите возможность добавления тестов для изменений VALIDATION_ENABLED и VALIDATION_STRICT_MODE во время выполнения, а также для случаев с недопустимыми типами графов или узлов.
</issue_to_address>
### Комментарий 2
<location> `backend/src/core/__tests__/PrismaService.test.js:3-9` </location>
<code_context>
+const prismaService = require('../PrismaService');
+
+describe('PrismaService', () => {
+ it('should return singleton instance', () => {
+ const service1 = require('../PrismaService');
+ const service2 = require('../PrismaService');
+
+ expect(service1).toBe(service2);
+ });
+
+ it('should provide getClient method', () => {
</code_context>
<issue_to_address>
**предложение (тестирование):** Синглтон PrismaService и повторное использование клиента хорошо протестированы.
Рассмотрите возможность добавления теста для метода disconnect, чтобы убедиться, что он корректно закрывает соединение и обрабатывает ошибки.
</issue_to_address>
### Комментарий 3
<location> `backend/src/core/nodes/logic/__tests__/compare.test.js:1-10` </location>
<code_context>
+describe('logic:compare node', () => {
+ const logicCompare = require('../compare');
+
+ const mockHelpers = (aValue, bValue) => ({
+ resolvePinValue: jest.fn(async (node, pinId) => {
+ if (pinId === 'a') return aValue;
+ if (pinId === 'b') return bValue;
+ return null;
+ }),
+ });
+
+ it('should compare greater than', async () => {
</code_context>
<issue_to_address>
**предложение (тестирование):** Тесты узла сравнения логики охватывают стандартные операции и сравнение строк.
Пожалуйста, добавьте тесты для граничных случаев, таких как сравнения null/undefined и приведение типов между числами и строками.
</issue_to_address>
### Комментарий 4
<location> `backend/src/core/nodes/math/__tests__/operation.test.js:1-10` </location>
<code_context>
+describe('math:operation node', () => {
+ const mathOperation = require('../operation');
+
+ const mockHelpers = (aValue, bValue) => ({
+ resolvePinValue: jest.fn(async (node, pinId, defaultValue) => {
+ if (pinId === 'a') return aValue;
+ if (pinId === 'b') return bValue;
+ return defaultValue;
+ }),
+ });
+
+ it('should add two numbers', async () => {
</code_context>
<issue_to_address>
**предложение (тестирование):** Тесты узла математических операций охватывают базовую арифметику и деление на ноль.
Рассмотрите возможность добавления тестов для нечисловых входных данных и экстремальных числовых значений для охвата дополнительных граничных случаев.
</issue_to_address>
### Комментарий 5
<location> `backend/src/core/nodes/time/__tests__/now.test.js:1-10` </location>
<code_context>
+describe('time:now node', () => {
+ const timeNow = require('../now');
+
+ it('should return current date from now pin', async () => {
+ const node = { id: 'test', type: 'time:now' };
+ const context = {};
+ const helpers = {};
+
+ const result = await timeNow.evaluate(node, 'now', context, helpers);
+
+ expect(result).toBeInstanceOf(Date);
+ expect(result.getTime()).toBeLessThanOrEqual(Date.now());
+ });
+
+ it('should return null for unknown pin', async () => {
</code_context>
<issue_to_address>
**мелкое замечание (тестирование):** Тесты узла Time:now лаконичны и эффективны.
Добавление теста на дрейф времени или мокирование объекта Date поможет обеспечить надежность тестов в средах CI.
</issue_to_address>Sourcery бесплатен для открытого исходного кода — если вам нравятся наши обзоры, пожалуйста, поделитесь ими ✨
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- Instead of hard-coding each registry module in NodeRegistry._registerBaseNodes, consider auto-discovering and requiring all files in the node-registries directory to reduce boilerplate and avoid forgetting to add new registries.
- GraphExecutionEngine is calling both validateGraphStructure and hasValidBasicStructure—consider merging basic structure checks into validateGraphStructure to centralize validation logic and simplify the execution flow.
- You’re using literal strings for graphType ('all', 'command', 'event') throughout the code; extracting them into shared constants or an enum would help prevent typos and keep values consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Instead of hard-coding each registry module in NodeRegistry._registerBaseNodes, consider auto-discovering and requiring all files in the node-registries directory to reduce boilerplate and avoid forgetting to add new registries.
- GraphExecutionEngine is calling both validateGraphStructure and hasValidBasicStructure—consider merging basic structure checks into validateGraphStructure to centralize validation logic and simplify the execution flow.
- You’re using literal strings for graphType ('all', 'command', 'event') throughout the code; extracting them into shared constants or an enum would help prevent typos and keep values consistent.
## Individual Comments
### Comment 1
<location> `backend/src/core/services/__tests__/ValidationService.test.js:3-10` </location>
<code_context>
+const validationService = require('../ValidationService');
+
+describe('ValidationService', () => {
+ describe('parseGraph', () => {
+ it('should parse JSON string graph', () => {
+ const graphJson = JSON.stringify({ nodes: [], connections: [] });
+ const result = validationService.parseGraph(graphJson, 'test');
+
+ expect(result).toEqual({ nodes: [], connections: [] });
+ });
+
+ it('should return graph object as-is', () => {
</code_context>
<issue_to_address>
**suggestion (testing):** ValidationService tests cover strict and non-strict modes, but could use more edge cases.
Consider adding tests for runtime changes to VALIDATION_ENABLED and VALIDATION_STRICT_MODE, as well as cases with invalid graph or node types.
</issue_to_address>
### Comment 2
<location> `backend/src/core/__tests__/PrismaService.test.js:3-9` </location>
<code_context>
+const prismaService = require('../PrismaService');
+
+describe('PrismaService', () => {
+ it('should return singleton instance', () => {
+ const service1 = require('../PrismaService');
+ const service2 = require('../PrismaService');
+
+ expect(service1).toBe(service2);
+ });
+
+ it('should provide getClient method', () => {
</code_context>
<issue_to_address>
**suggestion (testing):** PrismaService singleton and client reuse are well tested.
Consider adding a test for the disconnect method to verify it closes the connection and handles errors correctly.
</issue_to_address>
### Comment 3
<location> `backend/src/core/nodes/logic/__tests__/compare.test.js:1-10` </location>
<code_context>
+describe('logic:compare node', () => {
+ const logicCompare = require('../compare');
+
+ const mockHelpers = (aValue, bValue) => ({
+ resolvePinValue: jest.fn(async (node, pinId) => {
+ if (pinId === 'a') return aValue;
+ if (pinId === 'b') return bValue;
+ return null;
+ }),
+ });
+
+ it('should compare greater than', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Logic compare node tests cover standard operations and string comparison.
Please add tests for edge cases like null/undefined comparisons and type coercion between numbers and strings.
</issue_to_address>
### Comment 4
<location> `backend/src/core/nodes/math/__tests__/operation.test.js:1-10` </location>
<code_context>
+describe('math:operation node', () => {
+ const mathOperation = require('../operation');
+
+ const mockHelpers = (aValue, bValue) => ({
+ resolvePinValue: jest.fn(async (node, pinId, defaultValue) => {
+ if (pinId === 'a') return aValue;
+ if (pinId === 'b') return bValue;
+ return defaultValue;
+ }),
+ });
+
+ it('should add two numbers', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Math operation node tests cover basic arithmetic and division by zero.
Consider adding tests for non-numeric inputs and extreme numeric values to cover additional edge cases.
</issue_to_address>
### Comment 5
<location> `backend/src/core/nodes/time/__tests__/now.test.js:1-10` </location>
<code_context>
+describe('time:now node', () => {
+ const timeNow = require('../now');
+
+ it('should return current date from now pin', async () => {
+ const node = { id: 'test', type: 'time:now' };
+ const context = {};
+ const helpers = {};
+
+ const result = await timeNow.evaluate(node, 'now', context, helpers);
+
+ expect(result).toBeInstanceOf(Date);
+ expect(result.getTime()).toBeLessThanOrEqual(Date.now());
+ });
+
+ it('should return null for unknown pin', async () => {
</code_context>
<issue_to_address>
**nitpick (testing):** Time:now node tests are concise and effective.
Adding a test for time drift or mocking the Date object would help ensure test reliability in CI environments.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
frontend/src/stores/visualEditorStore.js (1)
666-668: Неиспользуемый методautoSaveGraph— требуется удаление.Метод
autoSaveGraphне используется нигде в кодовой базе. Проверка показала, что компоненты извлекают толькоflushSaveGraph, но неautoSaveGraph. Рекомендуется удалить неиспользуемый код для поддержания чистоты API хранилища.Примените этот diff для удаления неиспользуемого метода:
- autoSaveGraph: () => { - debouncedSaveGraph(); - }, - /** * Принудительное сохранение графа с очисткой отложенного таймера
🧹 Nitpick comments (5)
frontend/src/lib/__tests__/debounce.test.js (1)
59-107: Поведение flush() протестировано хорошо.Покрыты основные сценарии: немедленный вызов, использование последних аргументов, отмена таймера и случай отсутствия ожидающих вызовов.
Рассмотрите добавление тестов для граничных случаев:
it('flush() can be called multiple times safely', () => { const mockFn = jest.fn(); const debouncedFn = debounce(mockFn, 100); debouncedFn('test'); debouncedFn.flush(); debouncedFn.flush(); // второй flush expect(mockFn).toHaveBeenCalledTimes(1); }); it('flush() after natural timer expiration should not call again', () => { const mockFn = jest.fn(); const debouncedFn = debounce(mockFn, 100); debouncedFn('test'); jest.advanceTimersByTime(100); // естественное истечение expect(mockFn).toHaveBeenCalledTimes(1); debouncedFn.flush(); // flush после истечения expect(mockFn).toHaveBeenCalledTimes(1); // не должно быть повторного вызова });Also applies to: 134-142
backend/src/core/services/__tests__/ValidationService.test.js (1)
75-94: Сделать тесты независимыми от текущего NODE_ENVСейчас ожидания для невалидного графа и ноды, а также для
isStrictMode, зависят от фактическогоprocess.env.NODE_ENVво время прогона тестов. Это усложняет воспроизведение поведения production/非‑production и даёт покрытие только одной ветки за запуск. Рассмотрите вариант в самих тестах временно устанавливать нужныйNODE_ENVи пересоздавать инстанс сервиса черезjest.resetModules()/повторныйrequire, чтобы явно и детерминированно проверять обе ветви (strict и non‑strict).Also applies to: 115-134, 142-145
backend/src/core/NodeRegistry.js (1)
101-130: Динамические require в _registerBaseNodes можно вынести наверх файла (по желанию)Сейчас все registry‑модули подключаются через
requireвнутри_registerBaseNodes. Это нормально при singleton‑использовании, но для ускорения загрузки и лучшей статической анализируемости можно со временем перенести этиrequireна верхний уровень и вызывать толькоregisterNodes(this)здесь. Изменение не критично и может быть отложено.backend/src/core/GraphExecutionEngine.js (1)
43-48: Проверить ожидаемую семантику инициализации context.variables
parseVariablesвызывается только еслиthis.context.variablesотсутствует, то есть текущая реализация не дополняет и не переписывает уже существующие переменные в контексте значениями из графа. Если предполагаются сценарии, где граф определяет дефолтные переменные поверх существующего контекста, возможно, стоит явно описать приоритеты (например, merge/override) или добавить тесты на это поведение.backend/src/core/services/ValidationService.js (1)
11-15: Флаги strictMode/validationEnabled фиксируются на момент импорта модуляПоскольку
validationEnabledиstrictModeинициализируются в конструкторе один раз из значений config, последующие измененияprocess.env(например, в тестах при симуляции разных окружений) не будут отражаться в уже созданном singleton‑инстансе. Это, скорее всего, ожидаемо, но если когда‑нибудь понадобится динамически переключать режимы, можно либо читать флаги напрямую из config внутри методов, либо экспортировать фабрику/класс и создавать отдельные инстансы под разные окружения.Also applies to: 104-114
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/src/core/EventGraphManager.js(3 hunks)backend/src/core/GraphExecutionEngine.js(4 hunks)backend/src/core/NodeRegistry.js(3 hunks)backend/src/core/services/ValidationService.js(1 hunks)backend/src/core/services/__tests__/ValidationService.test.js(1 hunks)frontend/src/lib/__tests__/debounce.test.js(1 hunks)frontend/src/lib/debounce.js(1 hunks)frontend/src/stores/visualEditorStore.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/src/core/EventGraphManager.js
- frontend/src/lib/debounce.js
🧰 Additional context used
🧬 Code graph analysis (6)
backend/src/core/services/__tests__/ValidationService.test.js (1)
backend/src/core/services/ValidationService.js (3)
require(1-1)require(2-2)require(3-3)
frontend/src/lib/__tests__/debounce.test.js (1)
frontend/src/lib/debounce.js (2)
debouncedFn(6-17)debounce(1-34)
backend/src/core/services/ValidationService.js (3)
backend/src/core/EventGraphManager.js (2)
require(4-4)require(5-5)backend/src/core/config/validation.js (2)
VALIDATION_ENABLED(8-8)VALIDATION_STRICT_MODE(14-14)frontend/src/components/visual-editor/SettingsPanel.jsx (1)
safeJsonParse(26-35)
frontend/src/stores/visualEditorStore.js (3)
frontend/src/pages/Bot/EventGraphsPage.jsx (1)
useVisualEditorStore(240-240)frontend/src/components/visual-editor/NodePanel.jsx (1)
useVisualEditorStore(6-6)frontend/src/lib/debounce.js (1)
debounce(1-34)
backend/src/core/GraphExecutionEngine.js (6)
backend/src/core/EventGraphManager.js (5)
prismaService(3-3)require(4-4)require(5-5)validationService(6-6)prisma(8-8)backend/src/core/PrismaService.js (1)
prismaService(31-31)backend/src/core/__tests__/PrismaService.test.js (1)
prismaService(1-1)backend/src/core/nodes/users/set_blacklist.js (2)
prismaService(2-2)prisma(3-3)backend/src/core/services/__tests__/ValidationService.test.js (1)
validationService(1-1)backend/src/core/config/validation.js (1)
MAX_RECURSION_DEPTH(21-21)
backend/src/core/NodeRegistry.js (2)
backend/src/core/EventGraphManager.js (3)
validationService(6-6)require(4-4)require(5-5)backend/src/core/GraphExecutionEngine.js (3)
validationService(3-3)require(2-2)require(4-4)
⏰ 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). (2)
- GitHub Check: Sourcery review
- GitHub Check: Sourcery review
🔇 Additional comments (11)
frontend/src/stores/visualEditorStore.js (2)
12-20: LGTM! Корректная инициализация debounced‑сохранения.Импорт и создание
debouncedSaveGraphвнутри инициализатора хранилища реализовано правильно. Debounce‑функция создаётся один раз при инициализации и переиспользуется, аget().saveGraph()внутри замыкания обеспечивает доступ к актуальному состоянию хранилища. Интервал в 1000 мс является разумным выбором для автосохранения.
670-681: LGTM! МетодflushSaveGraphкорректно реализован.Реализация принудительного сохранения выглядит правильно:
- Проверка наличия метода
flush()делает код защищённым- JSDoc чётко описывает назначение метода
- Метод вызывается в cleanup‑эффекте
VisualEditorPage.jsxдля предотвращения потери несохранённых изменений при размонтированииЭто решает проблему потенциальной потери данных, упомянутую в предыдущих комментариях к ревью.
frontend/src/lib/__tests__/debounce.test.js (4)
1-8: Правильная настройка тестового окружения.Использование fake timers и очистка в
afterEachобеспечивают изолированность и предсказуемость тестов.
10-42: Базовые сценарии debounce покрыты корректно.Тесты правильно проверяют отложенное выполнение и использование последних аргументов.
44-57: Корректная проверка сохранения контекста.Тесты правильно используют
function()синтаксис для проверки привязкиthisкак при естественном истечении таймера, так и при вызовеflush().Also applies to: 109-121
144-158: Важный тест переиспользования после flush().Правильно проверяет, что debounced функция остается работоспособной после вызова
flush().backend/src/core/services/__tests__/ValidationService.test.js (1)
1-147: Хорошее, целостное покрытие поведения ValidationServiceТесты последовательно проверяют парсинг графа, базовую структурную проверку, валидацию графа и нод, а также утилитарные методы и различия поведения между strict/non‑strict режимами — контракт
{ success, shouldSkip, error }отражён корректно.backend/src/core/NodeRegistry.js (1)
1-1: Корректная интеграция ValidationService в процесс регистрации нодВызов
validationService.validateNodeперед добавлением вthis.nodesи раннийreturnприshouldSkipгарантируют, что в non‑strict/production режиме невалидные ноды не регистрируются, а в strict режиме по‑прежнему будет выброшено исключение — логика хорошо согласована с контрактом ValidationService.Also applies to: 40-43
backend/src/core/GraphExecutionEngine.js (2)
24-37: Хорошая централизованная обработка парсинга и валидации графаСвязка
parseGraph→validateGraphStructure→hasValidBasicStructureперед запуском графа аккуратно отсекает невалидные/нечитаемые графы, при этом в случае ошибки выполнение мягко возвращает исходныйcontext, что соответствует ожидаемому поведению без жёсткого падения в продакшене.
291-317: Надёжная защита от циклов и чрезмерной глубины в isNodeVolatileДобавление
visitedи проверкиdepth > MAX_RECURSION_DEPTHустраняет риск бесконечной рекурсии на циклических графах; предупреждение в логах при достижении лимита поможет диагностике. ВызовыisNodeVolatile(node)с параметрами по умолчанию выглядят корректно и не меняют внешний контракт метода.backend/src/core/services/ValidationService.js (1)
1-15: Хорошо спроектированный централизованный ValidationServiceСервис аккуратно инкапсулирует zod‑валидацию графа и нод, возвращая единый контракт
{ success, shouldSkip, error }и разводя поведение dev/prod черезstrictMode. Структурированное логирование (объект сerrors) вместоJSON.stringifyделает логи читаемыми, аhasValidBasicStructureдаёт дешёвую дополнительную защиту для потребителей.Also applies to: 36-58, 66-89, 96-98
|
@sourcery-ai review |
There was a problem hiding this comment.
Привет! Я просмотрел твои изменения — вот несколько замечаний:
- В NodeRegistry._registerBaseNodes оберните синхронный вызов fs.readdirSync проверкой существования каталога (или переключитесь на асинхронные API), чтобы избежать сбоев при запуске, если папка node-registries отсутствует или пуста.
- PrismaService создает экземпляр PrismaClient при require, что может открывать соединения с БД в тестовых или сценарных запусках — рассмотрите возможность ленивой инициализации или разрешения внедрения мок-клиента для предотвращения нежелательных соединений.
- GraphExecutionEngine выполняет полную валидацию Zod при каждом выполнении; рассмотрите возможность кэширования или мемоизации результатов validationService.validateGraphStructure для повторных выполнений графа, чтобы уменьшить накладные расходы.
Подсказка для AI-агентов
Пожалуйста, учтите комментарии из этого обзора кода:
## Общие комментарии
- В NodeRegistry._registerBaseNodes оберните синхронный вызов fs.readdirSync проверкой существования каталога (или переключитесь на асинхронные API), чтобы избежать сбоев при запуске, если папка node-registries отсутствует или пуста.
- PrismaService создает экземпляр PrismaClient при require, что может открывать соединения с БД в тестовых или сценарных запусках — рассмотрите возможность ленивой инициализации или разрешения внедрения мок-клиента для предотвращения нежелательных соединений.
- GraphExecutionEngine выполняет полную валидацию Zod при каждом выполнении; рассмотрите возможность кэширования или мемоизации результатов validationService.validateGraphStructure для повторных выполнений графа, чтобы уменьшить накладные расходы.
## Индивидуальные комментарии
### Комментарий 1
<location> `backend/src/core/EventGraphManager.js:44` </location>
<code_context>
- console.error(`[EventGraphs] Ошибка парсинга переменных графа ID ${graph.id}:`, e);
- }
+ const parsedVars = safeJsonParse(graph.variables, [], `EventGraph ID ${graph.id} variables`);
+ Object.assign(initialState, parseVariables(parsedVars, `EventGraph ID ${graph.id}`));
}
this.graphStates.set(`${botId}-${graph.id}`, initialState);
</code_context>
<issue_to_address>
**предложение:** parseVariables пропускает переменные без имени; рассмотрите возможность логирования пропущенных переменных.
Логирование количества пропущенных переменных поможет выявить потенциальные проблемы с данными и упростит устранение неполадок.
Suggested implementation:
```javascript
const { parsed, skippedCount } = parseVariables(parsedVars, `EventGraph ID ${graph.id}`);
Object.assign(initialState, parsed);
if (skippedCount > 0) {
console.warn(`[EventGraphs] ${skippedCount} variable(s) without a name were skipped in EventGraph ID ${graph.id}`);
}
```
```javascript
const parseVariables = (variables, context) => {
const parsed = {};
let skippedCount = 0;
for (const v of variables) {
if (v && typeof v === 'object' && 'name' in v && v.name) {
parsed[v.name] = v.value;
} else {
skippedCount++;
}
}
return { parsed, skippedCount };
};
```
</issue_to_address>
### Комментарий 2
<location> `frontend/src/stores/visualEditorStore.js:677-681` </location>
<code_context>
+ * Метод flush() теперь автоматически вызывает отложенную функцию,
+ * поэтому больше не нужно дополнительно вызывать saveGraph()
+ */
+ flushSaveGraph: () => {
+ if (debouncedSaveGraph.flush) {
+ debouncedSaveGraph.flush();
+ }
+ },
+ };
</code_context>
<issue_to_address>
**предложение (риск бага):** flushSaveGraph полагается на debouncedSaveGraph.flush; убедитесь, что flush всегда присутствует.
Если flush будет удален или изменен в реализации debounce, flushSaveGraph может завершиться сбоем. Добавьте запасной вариант или обработку ошибок для случаев, когда flush не определен.
```suggestion
flushSaveGraph: () => {
if (typeof debouncedSaveGraph.flush === 'function') {
debouncedSaveGraph.flush();
} else {
console.warn('flushSaveGraph: debouncedSaveGraph.flush is not available. Graph changes may not be saved immediately.');
}
},
```
</issue_to_address>Sourcery бесплатен для открытого исходного кода — если вам нравятся наши обзоры, пожалуйста, поделитесь ими ✨
Original comment in English
Hey there - I've reviewed your changes - here's some feedback:
- In NodeRegistry._registerBaseNodes, wrap the synchronous fs.readdirSync call with a directory existence check (or switch to async APIs) to avoid startup failures when the node-registries folder is missing or empty.
- PrismaService instantiates PrismaClient on require, which can open DB connections in test or script runners—consider lazy initialization or allowing injection of a mock client to prevent unwanted connections.
- The GraphExecutionEngine runs full Zod validation on each execute; consider caching or memoizing validationService.validateGraphStructure results for repeated graph executions to reduce overhead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In NodeRegistry._registerBaseNodes, wrap the synchronous fs.readdirSync call with a directory existence check (or switch to async APIs) to avoid startup failures when the node-registries folder is missing or empty.
- PrismaService instantiates PrismaClient on require, which can open DB connections in test or script runners—consider lazy initialization or allowing injection of a mock client to prevent unwanted connections.
- The GraphExecutionEngine runs full Zod validation on each execute; consider caching or memoizing validationService.validateGraphStructure results for repeated graph executions to reduce overhead.
## Individual Comments
### Comment 1
<location> `backend/src/core/EventGraphManager.js:44` </location>
<code_context>
- console.error(`[EventGraphs] Ошибка парсинга переменных графа ID ${graph.id}:`, e);
- }
+ const parsedVars = safeJsonParse(graph.variables, [], `EventGraph ID ${graph.id} variables`);
+ Object.assign(initialState, parseVariables(parsedVars, `EventGraph ID ${graph.id}`));
}
this.graphStates.set(`${botId}-${graph.id}`, initialState);
</code_context>
<issue_to_address>
**suggestion:** parseVariables skips variables without a name; consider logging skipped variables.
Logging the count of skipped variables will help identify potential data issues and simplify troubleshooting.
Suggested implementation:
```javascript
const { parsed, skippedCount } = parseVariables(parsedVars, `EventGraph ID ${graph.id}`);
Object.assign(initialState, parsed);
if (skippedCount > 0) {
console.warn(`[EventGraphs] ${skippedCount} variable(s) without a name were skipped in EventGraph ID ${graph.id}`);
}
```
```javascript
const parseVariables = (variables, context) => {
const parsed = {};
let skippedCount = 0;
for (const v of variables) {
if (v && typeof v === 'object' && 'name' in v && v.name) {
parsed[v.name] = v.value;
} else {
skippedCount++;
}
}
return { parsed, skippedCount };
};
```
</issue_to_address>
### Comment 2
<location> `frontend/src/stores/visualEditorStore.js:677-681` </location>
<code_context>
+ * Метод flush() теперь автоматически вызывает отложенную функцию,
+ * поэтому больше не нужно дополнительно вызывать saveGraph()
+ */
+ flushSaveGraph: () => {
+ if (debouncedSaveGraph.flush) {
+ debouncedSaveGraph.flush();
+ }
+ },
+ };
</code_context>
<issue_to_address>
**suggestion (bug_risk):** flushSaveGraph relies on debouncedSaveGraph.flush; ensure flush is always present.
If flush is removed or altered in the debounce implementation, flushSaveGraph could fail. Add a fallback or error handling for cases where flush is undefined.
```suggestion
flushSaveGraph: () => {
if (typeof debouncedSaveGraph.flush === 'function') {
debouncedSaveGraph.flush();
} else {
console.warn('flushSaveGraph: debouncedSaveGraph.flush is not available. Graph changes may not be saved immediately.');
}
},
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| flushSaveGraph: () => { | ||
| if (debouncedSaveGraph.flush) { | ||
| debouncedSaveGraph.flush(); | ||
| } | ||
| }, |
There was a problem hiding this comment.
предложение (риск бага): flushSaveGraph полагается на debouncedSaveGraph.flush; убедитесь, что flush всегда присутствует.
Если flush будет удален или изменен в реализации debounce, flushSaveGraph может завершиться сбоем. Добавьте запасной вариант или обработку ошибок для случаев, когда flush не определен.
| flushSaveGraph: () => { | |
| if (debouncedSaveGraph.flush) { | |
| debouncedSaveGraph.flush(); | |
| } | |
| }, | |
| flushSaveGraph: () => { | |
| if (typeof debouncedSaveGraph.flush === 'function') { | |
| debouncedSaveGraph.flush(); | |
| } else { | |
| console.warn('flushSaveGraph: debouncedSaveGraph.flush is not available. Graph changes may not be saved immediately.'); | |
| } | |
| }, |
Original comment in English
suggestion (bug_risk): flushSaveGraph relies on debouncedSaveGraph.flush; ensure flush is always present.
If flush is removed or altered in the debounce implementation, flushSaveGraph could fail. Add a fallback or error handling for cases where flush is undefined.
| flushSaveGraph: () => { | |
| if (debouncedSaveGraph.flush) { | |
| debouncedSaveGraph.flush(); | |
| } | |
| }, | |
| flushSaveGraph: () => { | |
| if (typeof debouncedSaveGraph.flush === 'function') { | |
| debouncedSaveGraph.flush(); | |
| } else { | |
| console.warn('flushSaveGraph: debouncedSaveGraph.flush is not available. Graph changes may not be saved immediately.'); | |
| } | |
| }, |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
backend/src/core/validation/nodeSchemas.js (1)
61-101: Нормализуйте структуру ошибок в валидаторах для единообразной обработкиВо всех трёх валидаторах (
validateNodeConfig,validateNodeData,validateGraph) теперь возвращается:error: error.issues || error.errors || [{ message: error.message }]В итоге downstream‑код должен учитывать несколько форматов ошибки (массив issues/errors разных форматов или просто объект с message). Чтобы упростить дальнейшую обработку, имеет смысл нормализовать ошибки в единый формат (например, всегда массив объектов
{ message, path?, code? }), а внутри валидатора уже разруливатьissues/errors/message.Пример направления для рефакторинга:
- } catch (error) { - return { - success: false, - error: error.issues || error.errors || [{ message: error.message }], - }; - } + } catch (error) { + const rawIssues = Array.isArray(error.issues) + ? error.issues + : Array.isArray(error.errors) + ? error.errors + : [{ message: error.message }]; + + const normalizedErrors = rawIssues.map(issue => ({ + message: issue.message, + path: issue.path, + code: issue.code, + })); + + return { success: false, error: normalizedErrors }; + }И аналогично — в
validateNodeDataиvalidateGraph. Это сделает формат ошибок предсказуемым.
🧹 Nitpick comments (12)
frontend/src/lib/debounce.js (3)
1-5: LGTM: сигнатура и состояние корректны.Объявление функции и переменных состояния выглядит правильно. Замыкание корректно сохраняет
timeout,lastArgsиlastContextмежду вызовами.Опционально можно добавить валидацию параметров для повышения надежности:
export function debounce(func, wait) { + if (typeof func !== 'function') { + throw new TypeError('Ожидается функция в качестве первого аргумента'); + } + if (typeof wait !== 'number' || wait < 0) { + throw new TypeError('Ожидается неотрицательное число в качестве второго аргумента'); + } let timeout; let lastArgs; let lastContext;
6-19: LGTM: debounced функция реализована корректно.Логика debounce выглядит правильно:
- Контекст
thisсохраняется корректно (исправление из прошлого review применено)- Аргументы захватываются при каждом вызове
- Таймер отменяется перед установкой нового
- Состояние очищается после выполнения
Опционально можно добавить обработку исключений, чтобы гарантировать очистку состояния:
const later = () => { timeout = null; - func.apply(lastContext, lastArgs); - lastArgs = null; - lastContext = null; + try { + func.apply(lastContext, lastArgs); + } finally { + lastArgs = null; + lastContext = null; + } };
21-33: LGTM: метод flush реализован корректно.Метод
flushправильно отменяет pending вызов и немедленно выполняет функцию с последними сохраненными аргументами.Опционально можно:
- Добавить обработку исключений (аналогично основной функции):
if (lastArgs) { - func.apply(lastContext, lastArgs); - lastArgs = null; - lastContext = null; + try { + func.apply(lastContext, lastArgs); + } finally { + lastArgs = null; + lastContext = null; + } }
- Убрать избыточный комментарий на строке 26, так как код самодокументирован.
backend/src/core/node-registries/data.js (1)
1-288: Единственное несоответствие: дваgraphType: 'all'вместоGRAPH_TYPES.ALLВ целом реестр нод категории «Данные» выглядит аккуратно: пины, описания и
evaluator‑ы/dynamicPinsзаданы последовательно, аGRAPH_TYPESв основном используется корректно.Обратите внимание только на два места, где остались строковые литералы:
- Нода
data:get_server_players(строка 169):graphType: 'all'- Нода
data:length(строка 260):graphType: 'all'Для единообразия и чтобы не разъезжались значения при возможном изменении констант, имеет смысл привести их к использованию
GRAPH_TYPES.ALL, как в остальных нодах:- graphType: 'all', + graphType: GRAPH_TYPES.ALL,Такая мелкая унификация улучшит читаемость и снизит риск расхождений в будущем.
backend/src/core/GraphExecutionEngine.js (2)
24-32: Ранний выход при ошибке парсинга/валидации графаХорошо, что парсинг и структурная валидация вынесены в
ValidationServiceи даётся раннийreturn contextпри проблемах с графом — это упрощает контроль потока. Единственный момент: чтобы не получать «тихие» пропуски выполнения графа, имеет смысл убедиться, что внутриparseGraph/validateGraphStructureвсегда есть понятное логирование причины (особенно когдаvalidation.shouldSkip === true).
286-307: ПоведениеisNodeVolatileпри циклах/глубокой рекурсии может приводить к излишнему кешированиюСейчас при достижении
MAX_RECURSION_DEPTHи при повторном посещении ноды функция возвращаетfalse, тем самым помечая ноду как «неволатильную» и позволяя кешировать её результат. С точки зрения безопасности корректности это довольно агрессивный fallback: в ситуациях, когда анализ зависимостей оборван защитой (глубина, цикл), безопаснее считать ноду волатильной и не кешировать её значение.Я бы предложил в защитных ветках возвращать
true(и сохранять лог), чтобы в сомнительных случаях жертвовать производительностью, а не корректностью:isNodeVolatile(node, visited = new Set(), depth = 0) { if (!node) return false; - if (depth > MAX_RECURSION_DEPTH) { - console.warn(`[GraphExecutionEngine] isNodeVolatile достиг максимальной глубины рекурсии (${MAX_RECURSION_DEPTH})`); - return false; - } + if (depth > MAX_RECURSION_DEPTH) { + console.warn(`[GraphExecutionEngine] isNodeVolatile достиг максимальной глубины рекурсии (${MAX_RECURSION_DEPTH})`); + return true; + } - if (visited.has(node.id)) { - return false; - } + if (visited.has(node.id)) { + return true; + } visited.add(node.id);Так вы сохраните защиту от бесконечной рекурсии/глубоких графов, но уберёте риск некорректного кеширования для сложных топологий.
backend/src/core/node-registries/events.js (2)
8-24:event:command: стоит уточнитьgraphTypeСейчас стартовая нода команды описана как
graphType: GRAPH_TYPES.ALL, хотя по описанию это именно входная точка «графа команды». Чтобы не появлялась в палитре графов событий и не путала пользователей, логичнее ограничить её типом команд:- graphType: GRAPH_TYPES.ALL, + graphType: GRAPH_TYPES.COMMAND,Если по задумке ноду действительно нужно использовать и в других типах графов, имеет смысл это явно зафиксировать в описании.
26-185: Единообразие признакаisEventдля событийных нодДля части нод (
event:chat,event:raw_message) выставлен флагisEvent: true, тогда как другие события (playerJoined,playerLeft,entitySpawnи т.п.) его не содержат. Если UI/валидатор полагается наisEventдля группировки/фильтрации стартовых событий, имеет смысл либо проставить его для всех событийных нод, либо документировать, что он нужен только для некоторых специальных кейсов.backend/src/core/node-registries/strings.js (1)
1-153: Набор строковых нод оформлен последовательно и покрывает основные операцииРегистрация
string:contains/matches/equals/starts_with/ends_with/length/split/concatс чётким описанием входов/выходов и вынесенными evaluator’ами выглядит аккуратно и расширяемо;dynamicPins: trueдляstring:concatудачно решает кейс произвольного числа аргументов. В целом модуль хорошо вписывается в новую систему реестров.backend/src/core/NodeRegistry.js (2)
41-43: Рассмотрите возможность использовать нормализованный nodeConfig из валидацииСейчас
registerNodeTypeопирается только на флагshouldSkipи регистрирует исходныйnodeConfig. Это ок с текущей реализациейValidationService, но в будущем вы можете захотеть, чтобы схема не только валидировала, но и нормализовала конфиг (дефолты,.transform(), очистка лишних полей).Имеет смысл подумать о расширении контракта
validateNode, чтобы приsuccess === trueвозвращать нормализованный объект и регистрировать именно его — так вы лучше используете возможности схем.
100-132: Автообнаружение node-registries удачное, можно улучшить логирование ошибокДинамическая регистрация через обход директории
node-registriesи вызовregisterNodes(this)делает архитектуру расширяемой без правок этого файла — это плюс.Опционально можно усилить диагностику в
catch‑блоках: сейчас выводится толькоerror.message, что усложняет разбор сложных ошибокrequire. Либо логируйте полный объект/error.stack, либо в дев‑окружении дополнительно выводите стек, чтобы упростить поиск проблемных реестров.backend/src/core/services/ValidationService.js (1)
36-71: Можно переиспользовать hasValidBasicStructure и (опционально) возвращать нормализованный графЛогика
validateGraphStructureв целом выглядит корректно: проверка минимальной структуры, затем полная схема‑валидация, с различным поведением в строгом и production‑режимах.Есть пара возможных улучшений:
Избавиться от дублирования проверки структуры.
Вместо ручной проверкиgraph && Array.isArray(graph.nodes) && ...можно использовать уже определённыйhasValidBasicStructure(graph), чтобы не расходились реализации при будущих изменениях.Опционально пробрасывать нормализованный граф.
Сейчас результатvalidateGraph(graph)используется только для проверкиsuccess, а нормализованный объект с дефолтами (validation.data) выбрасывается. Если в будущем вы захотите полагаться на дефолты схемы (например,variables: []), имеет смысл расширить контрактvalidateGraphStructure, чтобы при успехе возвращать либоvalidation.data, либо хотя бы дать доступ к нему вызывающему коду.Оба пункта не блокирующие, но улучшат переиспользование и предсказуемость поведения сервиса.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
backend/src/core/GraphExecutionEngine.js(4 hunks)backend/src/core/NodeRegistry.js(4 hunks)backend/src/core/constants/graphTypes.js(1 hunks)backend/src/core/node-registries/actions.js(1 hunks)backend/src/core/node-registries/arrays.js(1 hunks)backend/src/core/node-registries/bot.js(1 hunks)backend/src/core/node-registries/data.js(1 hunks)backend/src/core/node-registries/debug.js(1 hunks)backend/src/core/node-registries/events.js(1 hunks)backend/src/core/node-registries/flow.js(1 hunks)backend/src/core/node-registries/logic.js(1 hunks)backend/src/core/node-registries/math.js(1 hunks)backend/src/core/node-registries/objects.js(1 hunks)backend/src/core/node-registries/strings.js(1 hunks)backend/src/core/node-registries/time.js(1 hunks)backend/src/core/node-registries/users.js(1 hunks)backend/src/core/services/ValidationService.js(1 hunks)backend/src/core/services/__tests__/ValidationService.test.js(1 hunks)backend/src/core/validation/nodeSchemas.js(1 hunks)frontend/src/lib/debounce.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- backend/src/core/node-registries/objects.js
- backend/src/core/services/tests/ValidationService.test.js
- backend/src/core/node-registries/users.js
- backend/src/core/node-registries/arrays.js
- backend/src/core/node-registries/math.js
- backend/src/core/node-registries/time.js
🧰 Additional context used
🧬 Code graph analysis (12)
backend/src/core/node-registries/bot.js (1)
backend/src/core/constants/graphTypes.js (1)
GRAPH_TYPES(10-17)
backend/src/core/node-registries/flow.js (1)
backend/src/core/constants/graphTypes.js (1)
GRAPH_TYPES(10-17)
backend/src/core/node-registries/data.js (2)
backend/src/core/NodeRegistry.js (1)
require(2-2)backend/src/core/constants/graphTypes.js (1)
GRAPH_TYPES(10-17)
backend/src/core/node-registries/debug.js (2)
backend/src/core/NodeRegistry.js (1)
require(2-2)backend/src/core/constants/graphTypes.js (1)
GRAPH_TYPES(10-17)
backend/src/core/validation/nodeSchemas.js (1)
backend/src/core/constants/graphTypes.js (1)
GRAPH_TYPES(10-17)
backend/src/core/services/ValidationService.js (2)
backend/src/core/EventGraphManager.js (2)
require(4-4)require(5-5)backend/src/core/config/validation.js (2)
VALIDATION_ENABLED(8-8)VALIDATION_STRICT_MODE(14-14)
backend/src/core/GraphExecutionEngine.js (3)
backend/src/core/EventGraphManager.js (5)
prismaService(3-3)require(4-4)require(5-5)validationService(6-6)prisma(8-8)backend/src/core/PrismaService.js (1)
prismaService(31-31)backend/src/core/config/validation.js (1)
MAX_RECURSION_DEPTH(21-21)
backend/src/core/node-registries/strings.js (1)
backend/src/core/constants/graphTypes.js (1)
GRAPH_TYPES(10-17)
backend/src/core/node-registries/actions.js (1)
backend/src/core/constants/graphTypes.js (1)
GRAPH_TYPES(10-17)
backend/src/core/NodeRegistry.js (3)
backend/src/core/GraphExecutionEngine.js (3)
validationService(3-3)require(2-2)require(4-4)backend/src/core/EventGraphManager.js (3)
validationService(6-6)require(4-4)require(5-5)backend/src/core/constants/graphTypes.js (1)
GRAPH_TYPES(10-17)
backend/src/core/node-registries/events.js (1)
backend/src/core/constants/graphTypes.js (1)
GRAPH_TYPES(10-17)
backend/src/core/node-registries/logic.js (2)
backend/src/core/NodeRegistry.js (1)
require(2-2)backend/src/core/constants/graphTypes.js (1)
GRAPH_TYPES(10-17)
⏰ 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). (2)
- GitHub Check: Sourcery review
- GitHub Check: Sourcery review
🔇 Additional comments (13)
backend/src/core/constants/graphTypes.js (1)
1-21: Централизованные константы типов графов определены корректноJSDoc-тип и
GRAPH_TYPESвыглядят согласованно, экспорт минимальный и понятный; это хорошая база для дальнейшей унификации использования типов графов.backend/src/core/node-registries/logic.js (1)
1-45: Регистрация логических нод выглядит согласованнойОбе ноды аккуратно описаны, используют
GRAPH_TYPES.ALL, пины и required‑флаги совпадают с описанием; корректно прокинутыevaluator‑ы иdynamicPinsтолько там, где это действительно нужно.backend/src/core/node-registries/actions.js (1)
1-132: Конфигурации action‑нод проработаны и единообразныГраф‑типы заданы через
GRAPH_TYPES, пины дляExec/данных/ошибок выглядят полно и согласованно, разделение по категориям (Действия,WebSocket API) и ограничениеsend_websocket_responseтолько дляEVENT‑графов выглядят логично.backend/src/core/node-registries/flow.js (1)
1-139: Ноды управления потоком описаны полно и последовательноВсе flow‑ноды имеют ожидаемые Exec‑входы/выходы, описания соответствуют структуре пинов,
dynamicPinsуflow:switchпроставлен корректно, использованиеGRAPH_TYPES.ALLединообразно.backend/src/core/GraphExecutionEngine.js (2)
1-5: Централизация доступа к Prisma через PrismaServiceПереход на
PrismaServiceвместо прямогоPrismaClientвыглядит корректным шагом к единообразному управлению подключением к БД и упрощает дальнейший рефакторинг, здесь всё выглядит согласованно.
38-43: Инициализацияcontext.variablesчерезparseVariablesИнициализация
this.context.variablesчерезparseVariables(this.activeGraph.variables, 'GraphExecutionEngine')выглядит правильной унификацией с остальным кодом. Важно только, чтобыparseVariablesбыл устойчив к случаям, когдаthis.activeGraph.variablesотсутствует или не является массивом (старые графы, частично заполненные данные), иначе здесь возможенTypeErrorна реальных графах.backend/src/core/node-registries/bot.js (1)
1-23: Регистрация нодыbot:get_positionвыглядит согласованнойТип, категория,
graphType: GRAPH_TYPES.ALLи описание хорошо вписываются в существующую систему; выходной пинpositionс типомObjectлогичен для позиции бота. Реестр читается прозрачно и не вызывает вопросов.backend/src/core/node-registries/debug.js (1)
1-26: Нодаdebug:logуниверсальна и хорошо описанаИспользование типа
Wildcardдля входаvalueиgraphType: GRAPH_TYPES.ALLделает отладочную ноду гибкой для любых графов; структура пинов (Exec in/out) и ссылка наexecutorвыглядят консистентно с остальными нодами.backend/src/core/NodeRegistry.js (2)
1-2: Централизация валидации и констант графов выглядит удачноПодключение
ValidationServiceи импортGRAPH_TYPESиз единого модуля делают поведение реестра нод согласованным с остальным ядром (GraphExecutionEngine/EventGraphManager). На этом уровне изменений возражений нет.
79-79: Использование GRAPH_TYPES.ALL вместо «магической строки» корректноСмена литерала
'all'наGRAPH_TYPES.ALLубирает «магическую строку» и синхронизирует фильтрацию поgraphTypeс центральным модулем констант. Логика отбора нод остаётся прежней и выглядит корректной.backend/src/core/validation/nodeSchemas.js (1)
12-29: NodeConfigSchema хорошо покрывает конфигурацию нод и допускает эволюцию схемыСхема аккуратно описывает основные поля ноды,
graphTypeзавязан наGRAPH_TYPES, а наличие и корневыхinputs/outputs, и вложенногоpinsдаёт гибкость для разных регистраций. Добавленное.passthrough()позволяет безопасно добавлять новые поля без немедленных правок схемы.В целом эта часть выглядит согласованной с остальным кодом ядра.
backend/src/core/services/ValidationService.js (2)
23-28: parseGraph корректно инкапсулирует безопасный парсинг графаМетод аккуратно различает строку и уже готовый объект и делегирует парсинг в
safeJsonParseс контекстом. Это упрощает вызывающий код (GraphExecutionEngine/EventGraphManager) и централизует обработку ошибок парсинга.
79-102: Централизация валидации нод и пропуска невалидных конфигураций в productionМетод
validateNodeпереносит вValidationServiceту логику, которая ранее была вNodeRegistry: полноценная валидация, подробный лог ошибки, throw в строгом режиме иshouldSkipдля пропуска регистрации невалидных нод в production.Такой подход делает поведение валидации единообразным по всем потребителям сервиса и устраняет дублирование в ядре.
1 participant
Сводка от Sourcery
Переработка ядра бэкенда для улучшения модульности, валидации и надежности: разделение регистраций узлов на отдельные модули реестра, внедрение валидации на основе Zod и безопасного парсинга JSON повсеместно, централизация PrismaClient в синглтон-сервисе и улучшение логики выполнения графов. Расширение хранилища фронтенда с помощью отложенного автосохранения и расширение тестового покрытия для новой функциональности.
Новые возможности:
safeJsonParseдля надежного парсинга JSON с запасным значением по умолчаниюPrismaServiceдля централизации использованияPrismaClientautoSaveGraphв хранилище визуального редактора фронтендаУлучшения:
NodeRegistryпутем разделения регистраций узлов на категорийные модули вcore/node-registriesNodeRegistryиGraphExecutionEngineс переключателями, управляемыми переменными окруженияGraphExecutionEngineиEventGraphManagerдля использованияsafeJsonParseи унифицированной логики парсингаisNodeVolatileс глубиной рекурсии и отслеживанием посещенных узлов для предотвращения бесконечных цикловСборка:
date-fns,express-rate-limitиzodв бэкендТесты:
safeJsonParse,PrismaServiceи различных модулей оценщика узловOriginal summary in English
Сводка от Sourcery
Рефакторинг ядра бэкенда для улучшения модульности, валидации и надежности путем централизации доступа к базе данных, валидации входных данных по схеме и реорганизации регистраций узлов, одновременно с расширением хранилища фронтенда функцией автосохранения и увеличением тестового покрытия.
Новые функции:
safeJsonParseдля надежного парсинга JSON с резервными значениями по умолчаниюPrismaServiceдля централизованного управленияPrismaClientautoSaveGraphна основеdebounceв хранилище визуального редактора фронтендаУлучшения:
NodeRegistryпутем перемещения регистраций узлов в модули, специфичные для категорийGraphExecutionEngineиEventGraphManagerisNodeVolatileс ограничениями глубины рекурсии и обнаружением цикловJestв соответствии с новой структурой проектаСборка:
date-fns,express-rate-limitиzodв зависимости бэкендаТесты:
PrismaServiceи различных модулей узловOriginal summary in English
Сводка от Sourcery
Рефакторинг ядра бэкенда для улучшения модульности, валидации и надежности путем централизации доступа к базе данных, валидации ввода на основе схем и реорганизации регистрации узлов; улучшение хранилища фронтенда с помощью отложенного автосохранения; и расширение тестового покрытия для новой функциональности.
Новые возможности:
Улучшения:
Сборка:
Тесты:
Original summary in English
Краткое описание от Sourcery
Рефакторинг ядра бэкенда для улучшения модульности, валидации входных данных и надежности за счет централизации доступа к базе данных, использования валидации на основе схем с Zod и введения безопасного парсинга JSON. Модуляризация регистрации узлов в отдельные модули реестра, улучшение логики выполнения графа и предотвращение проблем с рекурсией. Улучшение визуального редактора фронтенда с помощью отложенного автосохранения и сохранения при размонтировании. Расширение тестового покрытия для новой логики валидации, утилит и оценки узлов.
Новые возможности:
Улучшения:
Сборка:
CI:
Тесты:
Original summary in English
Сводка от Sourcery
Рефакторинг ядра бэкенда для улучшения модульности, валидации и надежности за счет централизации доступа к базе данных, валидации ввода на основе схем и безопасного парсинга JSON, а также реструктуризация регистрации узлов в модули, основанные на категориях. Расширение хранилища визуального редактора фронтенда функцией автоматического сохранения графа с использованием debounce и приведение тестового покрытия в соответствие с новой архитектурой.
Новые возможности:
Улучшения:
Сборка:
Тесты:
Original summary in English
Summary by Sourcery
Refactor the backend core for improved modularity, validation, and reliability by centralizing database access, schema-based input validation, and safe JSON parsing, while restructuring node registrations into category-based modules. Extend the frontend visual editor store with debounce-based automatic graph saving and align test coverage to the new architecture.
New Features:
Enhancements:
Build:
Tests:
Original summary in English
Сводка от Sourcery
Рефакторинг ядра бэкенда для улучшения модульности, валидации и надежности за счет централизации доступа к базе данных, валидации ввода на основе схем и безопасного парсинга JSON, а также реструктуризация регистрации узлов в модули, основанные на категориях. Расширение хранилища визуального редактора фронтенда функцией автоматического сохранения графа с использованием debounce и приведение тестового покрытия в соответствие с новой архитектурой.
Новые возможности:
Улучшения:
Сборка:
Тесты:
Original summary in English
Summary by Sourcery
Refactor the backend core for improved modularity, validation, and reliability by centralizing database access, schema-based input validation, and safe JSON parsing, while restructuring node registrations into category-based modules. Extend the frontend visual editor store with debounce-based automatic graph saving and align test coverage to the new architecture.
New Features:
Enhancements:
Build:
Tests:
Original summary in English
Сводка от Sourcery
Рефакторинг ядра бэкенда для улучшения модульности, валидации и надежности за счет централизации доступа к базе данных, валидации ввода на основе схем и безопасного парсинга JSON, а также реструктуризация регистрации узлов в модули, основанные на категориях. Расширение хранилища визуального редактора фронтенда функцией автоматического сохранения графа с использованием debounce и приведение тестового покрытия в соответствие с новой архитектурой.
Новые возможности:
Улучшения:
Сборка:
Тесты:
Original summary in English
Summary by Sourcery
Refactor the backend core for improved modularity, validation, and reliability by centralizing database access, schema-based input validation, and safe JSON parsing, while restructuring node registrations into category-based modules. Extend the frontend visual editor store with debounce-based automatic graph saving and align test coverage to the new architecture.
New Features:
Enhancements:
Build:
Tests:
Original summary in English
Сводка от Sourcery
Рефакторинг ядра бэкенда для улучшения модульности, валидации и надежности за счет централизации доступа к базе данных, валидации ввода на основе схем и безопасного парсинга JSON, а также реструктуризация регистрации узлов в модули, основанные на категориях. Расширение хранилища визуального редактора фронтенда функцией автоматического сохранения графа с использованием debounce и приведение тестового покрытия в соответствие с новой архитектурой.
Новые возможности:
Улучшения:
Сборка:
Тесты:
Original summary in English
Summary by Sourcery
Refactor the backend core for improved modularity, validation, and reliability by centralizing database access, schema-based input validation, and safe JSON parsing, while restructuring node registrations into category-based modules. Extend the frontend visual editor store with debounce-based automatic graph saving and align test coverage to the new architecture.
New Features:
Enhancements:
Build:
Tests:
Summary by CodeRabbit
Новые функции
Улучшения
Тесты
Прочее