-
Notifications
You must be signed in to change notification settings - Fork 2
Fix saving amd opening #83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
kleer001
merged 7 commits into
main
from
claude/review-code-organization-01JbJSt3sQCGzxs5GbnT3tkj
Nov 23, 2025
Merged
Fix saving amd opening #83
kleer001
merged 7 commits into
main
from
claude/review-code-organization-01JbJSt3sQCGzxs5GbnT3tkj
Nov 23, 2025
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
This commit reverts the NODE_ATTRIBUTES list in flowstate_manager.py back to non-underscored names (as it was before commit 5f16e80), treating flowstate_manager.py as a sacred file that should not be modified to match internal implementation details. Instead of modifying NODE_ATTRIBUTES directly, we introduce a new helper module (attribute_mapper.py) that handles the transformation between: - Public attribute names (non-underscored, e.g., 'is_time_dependent') - Internal attribute names (underscored, e.g., '_is_time_dependent') Changes: 1. Reverted NODE_ATTRIBUTES in flowstate_manager.py to use non-underscored names ('name', 'is_time_dependent', etc.) 2. Created new module src/core/attribute_mapper.py with helper functions: - get_node_attribute(): Get node attributes using public names - set_node_attribute(): Set node attributes using public names - has_node_attribute(): Check if node has attribute using public name - to_internal_name() / to_public_name(): Name conversion utilities 3. Updated flowstate_manager.py to use attribute_mapper functions in: - _apply_node_data() - _serialize_node() - _deserialize_node() This preserves the original design of flowstate_manager.py while ensuring compatibility with the Node class's internal attribute naming convention. The helper module acts as a translation layer, keeping the sacred file untouched while enabling the GUI to function properly.
This commit reverts the previous approach of modifying flowstate_manager.py (the sacred file) and instead implements a GUI-specific fix in the API layer. Problem: - flowstate_manager.py uses non-underscored NODE_ATTRIBUTES for TUI/REPL/batch compatibility (working perfectly before commit 5f16e80) - When loading, flowstate_manager does setattr(node, 'is_time_dependent', value) which creates a NEW non-underscored attribute instead of updating the existing node._is_time_dependent (underscored) - This caused GUI imports to fail with ValidationError because node_to_response couldn't find the correctly set attributes Solution: 1. Reverted flowstate_manager.py to its pre-5f16e80 state (sacred file restored) - NODE_ATTRIBUTES back to non-underscored names - No attribute_mapper imports or usage - TUI/REPL/batch continue working as before 2. Removed attribute_mapper.py (no longer needed) 3. Added GUI-specific fix in workspace.py: - New function _fix_loaded_node_attributes_for_gui() - Called after load_flowstate() in import_workspace endpoint - Migrates any incorrectly created non-underscored attributes to their correct underscored locations (e.g., node.is_time_dependent → node._is_time_dependent) - Cleans up the incorrectly created attributes This approach: - Keeps flowstate_manager.py sacred and unchanged - Only affects GUI import path (workspace.py) - TUI/REPL/batch mode unaffected - Other fixes from commit 5f16e80 remain (undo_manager defaults, api/models None check)
Applied SOLID, DRY, KISS principles: - Removed verbose docstring and inline comments (self-documenting code) - Moved imports to module level - Shortened function name: _migrate_node_attributes - Combined duplicate hasattr checks - Added node_type enum fix from original commit 5f16e80 - Reduced from 34 lines to 14 lines No functional changes, purely refactoring for code quality.
Bug: _migrate_node_attributes was treating methods like name() as instance attributes, attempting to migrate and delete them. This caused 500 errors during workspace import. Fix: Check if attribute is callable before migration. Skip methods and only migrate actual instance attributes.
Removed automatic browser opening and 'Frontend not ready' messages. Now simply displays URLs for manual access. Cleanup: - Removed webbrowser import - Removed ServerProbe class (no longer needed) - Simplified _open_browser_if_requested to just show URLs
Browser now opens silently when configured (default: enabled). No more noisy 'Frontend not ready' or 'Opening browser' messages. Usage: - Default: browser opens automatically - python text_loom.py --gui --no-browser: disable auto-open Changes: - Restored webbrowser import - Restored ServerProbe class - Silent browser launch on startup if open_browser=True - Always show clean URLs regardless of browser setting
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.
No description provided.