Jorgedanisc#152
Conversation
…lationships, and improve bounds utility robustness.
…data fields into DucBlock across all language bindings.
…collections across all language bindings.
…lock instance creation and testing.
…or SST scripts with a new `sst:wrap` utility.
There was a problem hiding this comment.
Pull request overview
This pull request implements a comprehensive blocks and collections feature across the DUC file format and all language implementations (Rust, Python, TypeScript). The changes refactor the block system to separate block definitions from instances, add metadata and thumbnails to blocks, and introduce a new block collections feature for organizing blocks hierarchically.
Key Changes:
- Enhanced schema with
DucBlockMetadata,DucBlockCollection, and improved block/instance separation - Added
instance_idfield to element base for proper block instance tracking - Implemented serialization/parsing across all languages (Rust, Python, TypeScript)
- Refactored Python tests to use new block instance model
Reviewed changes
Copilot reviewed 46 out of 47 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| schema/duc.fbs | Added DucBlockMetadata, DucBlockCollection types; improved documentation for block/instance relationships |
| packages/ducrs/src/types.rs | Added Rust struct definitions for new types |
| packages/ducrs/src/serialize.rs | Implemented serialization for new block features |
| packages/ducrs/src/parse.rs | Implemented parsing for new block features |
| packages/ducrs/src/flatbuffers/duc_generated.rs | Auto-generated FlatBuffers code for Rust |
| packages/ducpy/src/ducpy/classes/ElementsClass.py | Added Python dataclasses for new types |
| packages/ducpy/src/ducpy/serialize.py | Implemented Python serialization logic |
| packages/ducpy/src/ducpy/parse.py | Implemented Python parsing logic |
| packages/ducpy/src/ducpy/builders/*.py | Added block instance builder and utilities |
| packages/ducpy/src/tests/src/*.py | Updated tests for new block model |
| packages/ducjs/src/types/elements/index.ts | Added TypeScript type definitions |
| packages/ducjs/src/serialize.ts | Implemented TypeScript serialization |
| packages/ducjs/src/parse.ts | Implemented TypeScript parsing |
| packages/ducjs/src/restore/restoreDataState.ts | Added restore logic for new types |
| packages/ducsvg/vite.config.mts | Added rollup externalization for WASM dependencies |
| packages/ducsvg/package.json | Reorganized exports configuration |
| packages/ducpdf/src/duc2pdf/Cargo.toml | Updated dependency configuration and build profile |
| package.json | Updated root dependencies and added SST helper scripts |
| apps/web/content/docs/roadmap/*.mdx | Removed specific roadmap items |
| apps/web/app/layout.tsx | Conditionally load analytics only in production |
| .vscode/extensions.json | Added recommended VS Code extensions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Set required list fields with defaults | ||
| base_params.setdefault('group_ids', []) | ||
| base_params.setdefault('block_ids', []) | ||
| base_params.setdefault('block_ids', []) |
There was a problem hiding this comment.
The field block_ids is duplicated in the base_params.setdefault() calls on lines 90 and 91. This will result in the second call overwriting the first, though since they both set the same value it has no effect. However, this is redundant and should be cleaned up.
| base_params.setdefault('block_ids', []) |
| # Remove arrow instance 2 | ||
| # 1. Remove metadata | ||
| parsed_instances = [i for i in parsed_instances if i.id != "arrow_inst_2"] | ||
| assert len(title_inst_2_elems) == 2 |
There was a problem hiding this comment.
The assertion on line 264 checks that len(title_inst_2_elems) == 2, but this variable was defined for a different instance (title_inst_2_elems). Based on the context, this should be checking len(parsed_instances) to verify that one arrow instance was deleted, reducing the count from 4 to 3.
| "ms-toolsai.jupyter", | ||
| "ms-python.python", | ||
| "ms-python.vscode-pylance", | ||
| "jock.svg", |
There was a problem hiding this comment.
There's a trailing comma after "jock.svg" on line 13 within the JSON array. While this is technically valid in JSON5 and some parsers, it's not valid in strict JSON and may cause issues with VS Code's extension recommendations file.
| "jock.svg", | |
| "jock.svg" |
| def val_get_base(element): | ||
| if hasattr(element, "linear_base"): | ||
| return element.linear_base.base | ||
| return element.base |
There was a problem hiding this comment.
The function helper val_get_base is defined at line 16-19 but a similar helper function _get_base is also defined in the imported block_utils.py at lines 8-11. These functions serve the same purpose but use different naming conventions. Consider consolidating to use the imported helper for consistency and to avoid code duplication.
| parsed_instances = [i for i in parsed_instances if i.id != "arrow_inst_2"] | ||
| assert len(title_inst_2_elems) == 2 | ||
|
|
||
| arrow_inst_1_elems = [el for el in parsed_elements if val_get_base(el.element).instance_id == "arrow_inst_1"] |
There was a problem hiding this comment.
Variable arrow_inst_1_elems is not used.
| @@ -0,0 +1,39 @@ | |||
| from typing import List, Optional | |||
| from ducpy.classes.ElementsClass import DucBlockInstance, DucBlockDuplicationArray, StringValueEntry | |||
| from ducpy.utils.rand_utils import random_versioning | |||
There was a problem hiding this comment.
Import of 'random_versioning' is not used.
| from ducpy.utils.rand_utils import random_versioning |
| ) | ||
| from .style_builders import create_simple_styles, create_text_style, create_paragraph_formatting, create_stack_format_properties, create_stack_format, create_doc_style, create_text_column, create_column_layout | ||
| from ducpy.utils import generate_random_id, DEFAULT_SCOPE, DEFAULT_STROKE_COLOR, DEFAULT_FILL_COLOR | ||
| from ducpy.utils import generate_random_id, DEFAULT_SCOPE, DEFAULT_STROKE_COLOR, DEFAULT_FILL_COLOR, DEFAULT_STROKE_WIDTH |
There was a problem hiding this comment.
Import of 'generate_random_id' is not used.
Import of 'DEFAULT_STROKE_COLOR' is not used.
Import of 'DEFAULT_FILL_COLOR' is not used.
| from ducpy.utils import generate_random_id, DEFAULT_SCOPE, DEFAULT_STROKE_COLOR, DEFAULT_FILL_COLOR, DEFAULT_STROKE_WIDTH | |
| from ducpy.utils import DEFAULT_SCOPE, DEFAULT_STROKE_WIDTH |
| from ducpy.Duc.IMAGE_STATUS import IMAGE_STATUS | ||
| from ducpy.Duc.STROKE_PREFERENCE import STROKE_PREFERENCE | ||
| from ducpy.Duc.STROKE_PLACEMENT import STROKE_PLACEMENT | ||
| from ducpy.Duc.STROKE_CAP import STROKE_CAP |
There was a problem hiding this comment.
Import of 'STROKE_CAP' is not used.
| from ducpy.Duc.STROKE_CAP import STROKE_CAP |
| from ducpy.Duc.STROKE_PREFERENCE import STROKE_PREFERENCE | ||
| from ducpy.Duc.STROKE_PLACEMENT import STROKE_PLACEMENT | ||
| from ducpy.Duc.STROKE_CAP import STROKE_CAP | ||
| from ducpy.Duc.STROKE_JOIN import STROKE_JOIN |
There was a problem hiding this comment.
Import of 'STROKE_JOIN' is not used.
| from ducpy.Duc.STROKE_JOIN import STROKE_JOIN |
|
|
||
| import ducpy as duc | ||
| from ducpy.classes.ElementsClass import DucBlockInstanceElement, StringValueEntry | ||
| from ducpy.classes.ElementsClass import DucBlockInstance, StringValueEntry, DucBlockDuplicationArray |
There was a problem hiding this comment.
Import of 'DucBlockDuplicationArray' is not used.
Import of 'StringValueEntry' is not used.
Import of 'DucBlockInstance' is not used.
|
🎉 This PR is included in version 2.1.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.2.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.1.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
No description provided.