feat: semantic workflow base sheets #53
Conversation
FramePerfection
force-pushed
the
feat/semantic-workflow-base-sheets
branch
from
953b450 to
9b659b2
Compare
There was a problem hiding this comment.
Pull request overview
This pull request adds a base sheet chaining feature to the Semantic Workflow system, allowing sheets to be based on other sheets rather than requiring independent savestates for each sheet. This significantly reduces storage requirements and improves workflow efficiency.
Changes:
- Added
bsbutton in the Project tab UI to select and manage base sheet relationships between sheets - Implemented cycle prevention logic to avoid circular base sheet dependencies
- Introduced an in-memory caching system that stores the end state of base sheets to avoid re-running entire chains
- Modified save/load logic to skip saving savestates for sheets that have a base sheet, reducing project folder size
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/views/SemanticWorkflow/Version.lua | Bumped version from 1.0.0 to 1.1.0 to reflect the new base sheet feature |
| src/views/SemanticWorkflow/Implementations/Sheet.lua | Core logic for base sheet chaining, including recursive running, caching, and invalidation tracking |
| src/views/SemanticWorkflow/Implementations/ProjectTab.lua | UI implementation for selecting base sheets with cycle detection and visual feedback |
| src/views/SemanticWorkflow/Implementations/Project.lua | Updated project loading to establish base sheet relationships and conditionally load savestates |
| src/views/SemanticWorkflow/Definitions/Sheet.lua | Updated type definitions and documentation for new base sheet methods and fields |
| src/res/lang/en_US.lua | Added localization strings for base sheet UI elements and tooltips |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
|
|
||
| if ugui.toggle_button({ | ||
| if selecting_sheet_base_for ~= nil then |
There was a problem hiding this comment.
The variable selecting_sheet_base_for is used without being declared. Following the pattern established for SemanticWorkflowDialog in Main.lua line 42, this should be declared as a global variable (e.g., in Main.lua or at the module level). Undeclared variables in Lua default to global scope, but this should be made explicit for clarity and maintainability.
| local has_state = sheet_meta.base_sheet == nil | ||
| new_sheet:load(project_folder .. sheet_meta.name .. '.sws', has_state) | ||
| if not has_state then | ||
| self.all[sheet_meta.name]:set_base_sheet(self.all[sheet_meta.base_sheet]) |
There was a problem hiding this comment.
No validation is performed to ensure that sheet_meta.base_sheet refers to a valid sheet that exists in self.all. If the project file is corrupted or manually edited with an invalid base sheet name, line 123 would attempt to pass nil to set_base_sheet, which could cause issues. Consider adding validation to check that self.all[sheet_meta.base_sheet] is not nil before calling set_base_sheet, and handle the error case appropriately (e.g., log a warning and clear the invalid base sheet reference).
| self.all[sheet_meta.name]:set_base_sheet(self.all[sheet_meta.base_sheet]) | |
| local base_sheet = self.all[sheet_meta.base_sheet] | |
| if base_sheet ~= nil then | |
| self.all[sheet_meta.name]:set_base_sheet(base_sheet) | |
| else | |
| -- Invalid base_sheet reference in project metadata; clear it to avoid errors. | |
| print("SemanticWorkflow Project: warning - invalid base_sheet reference '" .. | |
| tostring(sheet_meta.base_sheet) .. "' for sheet '" .. tostring(sheet_meta.name) .. "'.") | |
| sheet_meta.base_sheet = nil | |
| end |
| return self._invalidated or (self._base_sheet ~= nil and self._base_sheet:invalidated()) | ||
| end | ||
|
|
||
| function __impl:run_to_preview(from_base) |
There was a problem hiding this comment.
The parameter name from_base is inconsistent with the documentation (line 41 in Definitions/Sheet.lua) and call sites (e.g., Project.lua line 96) which use load_state. Additionally, the name from_base is misleading as the parameter controls whether to load any savestate at all, not specifically whether to run from a base sheet. Consider renaming this parameter to load_state to match the documentation and improve clarity.
This adds a new button
bs(for base sheet) next to the.st-Button for each sheet in the project tab.This affects several points in #26.
bs-Button is clicked, the user may choose another sheet to base the selected one on, effectively chaining the two together, or click the samebs-Button again to abort the action.