Add group blocks feature #38
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Updated the contact email for reporting security vulnerabilities.
…ction safety, implement serializer validation Co-authored-by: RETR0-OS <74290459+RETR0-OS@users.noreply.github.com>
Security hardening: Fix code injection, add transaction safety, implement serializer validation
Implement group blocks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a comprehensive group blocks feature that allows users to create reusable composite layers by grouping multiple blocks together. The implementation includes block expansion/collapse, instance-specific configuration overrides, validation, migration utilities, and full persistence support.
Key Changes
- Group block system: Create reusable blocks from selected nodes with configurable port mappings
- Expansion/collapse: Toggle between collapsed group blocks and their expanded internal structure
- Instance customization: Override internal node configurations per group block instance
- Validation & testing: Comprehensive validation for block creation with unit tests
Reviewed changes
Copilot reviewed 65 out of 79 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
project/frontend/src/lib/store.expansion.test.ts |
Tests for group block expansion/collapse functionality |
project/frontend/src/lib/projectApi.ts |
Updated API calls to handle group definitions |
project/frontend/src/lib/migrations/groupBlockMigration.ts |
Migration utility for adding isExpanded property to existing group blocks |
project/frontend/src/lib/groupBlockShapeInference.ts |
Shape propagation through group block internal structures |
project/frontend/src/lib/groupBlockHelpers.ts |
Helper functions for ID mapping and edge rewiring during expansion |
project/frontend/src/lib/exportImport.ts |
Import/export support for group definitions |
project/frontend/src/lib/blockValidation.ts |
Validation functions for block creation (connectivity, cycles, naming) |
project/frontend/src/lib/blockValidation.test.ts |
Tests for block validation functions |
project/frontend/src/lib/api.ts |
Updated error handling to pass through error data objects |
project/frontend/src/components/ValidationErrorsPanel.tsx |
UI panel displaying validation errors and warnings |
project/frontend/src/components/RenameBlockDialog.tsx |
Dialog for renaming block definitions |
project/frontend/src/components/InternalNodeConfigPanel.tsx |
Configuration panel for internal nodes within expanded groups |
project/frontend/src/components/Header.tsx |
Updated save/load/export to include group definitions |
project/frontend/src/components/GroupCreationDialog.tsx |
Two-step dialog for creating group blocks with port selection |
project/frontend/src/components/GroupCreationDialog.test.tsx |
Tests for group creation dialog |
project/frontend/src/components/GroupBlockNode.tsx |
Custom node component for rendering group blocks |
project/frontend/src/components/ExpandedGroupContainer.tsx |
Container component for expanded group blocks |
project/frontend/src/components/DeleteBlockDialog.tsx |
Dialog for deleting block definitions with cascade option |
project/frontend/src/components/ContextMenu.tsx |
Added ungroup option for group blocks |
project/frontend/src/components/ConfigPanel.tsx |
Updated to handle group block and internal node configuration |
project/frontend/src/components/Canvas.tsx |
Integrated group block creation, expansion, and validation |
project/frontend/src/components/BlockPalette.tsx |
Display custom blocks with context menu for management |
project/frontend/src/components/BlockNode.tsx |
Added visual indicator for nodes with config overrides |
project/frontend/src/components/BlockDefinitionContextMenu.tsx |
Context menu for block definition management |
project/frontend/src/App.tsx |
Load group definitions when loading projects |
project/frontend/package.json |
Added testing dependencies (vitest, testing-library) |
project/frontend/index.html |
Removed redundant DOCTYPE declaration |
project/frontend/EXPORT_FORMAT.md |
Removed outdated documentation file |
project/block_manager/views/group_views.py |
Backend views for group definition CRUD operations |
project/block_manager/views/export_views.py |
Enhanced error handling with validation error formatting |
project/block_manager/views/architecture_views.py |
Save/load group definitions with atomic transactions |
project/block_manager/urls.py |
Added group definition API endpoints |
project/block_manager/services/validation.py |
Enhanced validation with shape compatibility checks |
project/block_manager/services/tensorflow_codegen.py |
Added group block support for TensorFlow code generation |
project/block_manager/serializers.py |
Added GroupBlockDefinition serializer with validation |
project/block_manager/models.py |
Added GroupBlockDefinition model and group block fields to Block |
project/block_manager/migrations/0003_add_instance_config_overrides.py |
Migration for instance config overrides field |
project/block_manager/migrations/0002_block_is_expanded_block_repetition_metadata_and_more.py |
Migration for group block related fields |
docs/TENSORFLOW_SUMMARY.md |
Removed documentation file |
docs/TENSORFLOW_QUICK_REFERENCE.md |
Removed documentation file |
docs/TENSORFLOW_IMPLEMENTATION_COMPLETE.md |
Removed documentation file |
Files not reviewed (1)
- project/frontend/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
project/frontend/src/components/InternalNodeConfigPanel.tsx:1
- The ID parsing logic using
substringandlastIndexOfis fragile and assumes a specific format. Consider storing the original internal node ID in the node's data to avoid parsing, or extract this logic into a dedicated helper function with validation.
import { useState } from 'react'
| @@ -1,4 +1,3 @@ | |||
| <!DOCTYPE html> | |||
| <html lang="en"> | |||
There was a problem hiding this comment.
The removed DOCTYPE declaration is required for valid HTML5. Without it, browsers may render the page in quirks mode, leading to inconsistent behavior across different browsers.
| } | ||
| } else if (node.data.blockType === 'loss') { | ||
| // Loss nodes: handle multiple semantic inputs (predictions, labels, etc.) | ||
| const lossNodeDef = nodeDef as any |
There was a problem hiding this comment.
Using as any bypasses TypeScript's type safety. Consider defining a proper interface for loss node definitions with a getInputPorts method, or use a type guard to safely check for the method's existence.
|
|
||
| if (inputShapesList.length === incomingEdges.length && inputShapesList.length > 0) { | ||
| // All inputs have shapes | ||
| const nodeDefAny = nodeDef as any |
There was a problem hiding this comment.
Using as any bypasses TypeScript's type safety. Consider defining a proper interface that includes computeMultiInputShape or use a type guard to check for the method safely.
Comment on lines
+272
to
+285
| groupDefinitions: Array.from(groupDefinitions.values()).map(def => ({ | ||
| id: def.id, | ||
| name: def.name, | ||
| description: def.description, | ||
| category: def.category, | ||
| color: def.color, | ||
| internal_structure: { | ||
| nodes: def.internalNodes, | ||
| edges: def.internalEdges, | ||
| portMappings: def.portMappings | ||
| }, | ||
| createdAt: def.createdAt, | ||
| updatedAt: def.updatedAt | ||
| })) |
There was a problem hiding this comment.
The group definition transformation logic is duplicated in multiple places. Extract this mapping into a shared utility function to avoid code duplication and ensure consistent transformation.
| shape_value = config.get('shape') | ||
| try: | ||
| input_shape = ast.literal_eval(shape_value) if shape_value else None | ||
| except (ValueError, SyntaxError): |
There was a problem hiding this comment.
The except clause should also catch TypeError in case shape_value is not a string. The ast.literal_eval function can raise TypeError if passed a non-string value.
Suggested change
| except (ValueError, SyntaxError): | |
| except (ValueError, SyntaxError, TypeError): |
| architecture.blocks.all().delete() | ||
| architecture.connections.all().delete() | ||
|
|
||
| project.group_definitions.all().delete() |
There was a problem hiding this comment.
Deleting all group definitions on every save can lead to data loss if group definitions are referenced elsewhere. Consider implementing selective deletion or updating only changed definitions to preserve existing data.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a system to group blocks together to form complex layers