Asset-Frameworker/Documentation/gui_file_table_refactor_plan.md

8.9 KiB

Code Review & Refactoring Plan: GUI File Table

Objective: To identify the root causes of file list discrepancies and persistent empty asset rows in the GUI file table, and to propose refactoring solutions for improved robustness and maintainability.

Phase 1: In-Depth Code Review

This phase will focus on understanding the current implementation and data flow within the relevant GUI modules.

  1. Identify Key Modules & Classes:

    • gui/unified_view_model.py (UnifiedViewModel): This is the primary focus. We need to analyze:
      • How it loads and represents the hierarchical data (SourceRule -> AssetRule -> FileRule).
      • Methods responsible for updating the model with new data (e.g., from predictions or user edits).
      • Logic for adding, removing, and modifying rows, especially AssetRule and FileRule items.
      • How it handles data consistency when underlying data changes (e.g., after LLM processing or renaming operations).
      • Signal/slot connections related to data changes.
    • gui/asset_restructure_handler.py (AssetRestructureHandler):
      • How it listens to changes in AssetRule names or FileRule target asset overrides.
      • The logic for moving FileRule items between AssetRule items.
      • The conditions under which it creates new AssetRule items or removes empty ones. This is critical for the "persistent empty asset rows" issue.
    • gui/llm_prediction_handler.py (LLMPredictionHandler):
      • How it parses the LLM response.
      • How it translates the LLM's (potentially hallucinated) file list into the SourceRule structure.
      • How this new SourceRule data is passed to and integrated by the UnifiedViewModel. This is key for the "file list discrepancies" issue.
    • gui/prediction_handler.py (RuleBasedPredictionHandler):
      • Similar to the LLM handler, how it generates SourceRule data from presets.
      • How its output is integrated into the UnifiedViewModel, especially when "reinterpreting with a systematic approach" restores correct files.
    • gui/main_window.py (MainWindow) & gui/main_panel_widget.py (MainPanelWidget):
      • How these components instantiate and connect the UnifiedViewModel, AssetRestructureHandler, and prediction handlers.
      • Event handling related to loading data, triggering predictions, and user interactions that modify the table data.
    • rule_structure.py:
      • Review the definitions of SourceRule, AssetRule, and FileRule to ensure a clear understanding of the data being managed.
  2. Trace Data Flow & State Management:

    • Initial Load: How is the initial list of files/assets loaded and represented in the UnifiedViewModel?
    • LLM Processing:
      • Trace the data flow from the LLM response -> LLMPredictionHandler -> UnifiedViewModel.
      • How does the UnifiedViewModel reconcile the LLM's version of the file list with any existing state? Is there a clear "source of truth" for the file list before and after LLM processing?
    • Preset-Based Processing:
      • Trace data flow from preset selection -> RuleBasedPredictionHandler -> UnifiedViewModel.
      • How does this "systematic approach" correct discrepancies? Does it fully replace the model's data or merge it?
    • Renaming/Restructuring:
      • Trace the events and actions from a user renaming an asset -> AssetRestructureHandler -> UnifiedViewModel.
      • How are AssetRule items checked for emptiness and subsequently removed (or not removed)?
  3. Analyze Event Handling and Signal/Slot Connections:

    • Map out the key signals and slots between the UnifiedViewModel, AssetRestructureHandler, prediction handlers, and the main UI components.
    • Ensure that signals are emitted and slots are connected correctly to trigger necessary updates and prevent race conditions or missed updates.

Phase 2: Identify Issues & Propose Refactoring Strategies

Based on the review, we will pinpoint specific areas contributing to the reported problems and suggest improvements.

  1. For File List Discrepancies (especially post-LLM):

    • Potential Issue: The UnifiedViewModel might be directly replacing its internal data with the LLM's output without proper validation or merging against the original input file list.
    • Proposed Strategy:
      • Establish a clear "source of truth" for the actual input files that remains independent of the LLM's interpretation.
      • When the LLM provides its categorized list, the LLMPredictionHandler or UnifiedViewModel should map the LLM's findings onto the existing source files rather than creating a new list from scratch based on LLM hallucinations.
      • If the LLM identifies files not in the original input, these should be flagged or handled as discrepancies, not added as if they were real.
      • If the LLM misses files from the original input, these should remain visible, perhaps marked as "uncategorized by LLM."
  2. For Persistent Empty Asset Rows:

    • Potential Issue: The AssetRestructureHandler's logic for detecting and removing empty AssetRule items might be flawed or not consistently triggered. It might not correctly count child FileRule items after a move, or the signal to check for emptiness might not always fire.
    • Proposed Strategy:
      • Review and strengthen the logic within AssetRestructureHandler that checks if an AssetRule is empty after its FileRule children are moved or its name changes.
      • Ensure that this check is reliably performed after all relevant model updates have completed.
      • Consider adding explicit methods to UnifiedViewModel or AssetRule to query if an asset group is truly empty (has no associated FileRule items).
      • Ensure that the UnifiedViewModel correctly emits signals that the AssetRestructureHandler can use to trigger cleanup of empty asset rows.
  3. General Robustness & Maintainability:

    • State Management: Clarify state management within UnifiedViewModel. Ensure data consistency and minimize side effects.
    • Modularity: Ensure clear separation of concerns between the UnifiedViewModel (data representation), prediction handlers (data generation), and AssetRestructureHandler (data manipulation).
    • Logging & Error Handling: Improve logging in these critical sections to make troubleshooting easier. Add robust error handling for unexpected data states.
    • Unit Tests: Identify areas where unit tests could be added or improved to cover the scenarios causing these bugs, especially around model updates and restructuring.

Phase 3: Documentation & Handoff

  1. Document the findings of the code review.
  2. Detail the agreed-upon refactoring plan.
  3. Prepare for handoff to a developer (e.g., by switching to "Code" mode).

Visual Plan (Mermaid Diagram):

graph TD
    subgraph GUI Interaction
        UserAction[User Action (Load Files, Rename Asset, Trigger LLM)]
    end

    subgraph Prediction Layer
        LLM_Handler([`gui.llm_prediction_handler.LLMPredictionHandler`])
        Preset_Handler([`gui.prediction_handler.RuleBasedPredictionHandler`])
    end

    subgraph Core GUI Logic
        MainWindow([`gui.main_window.MainWindow`])
        MainPanel([`gui.main_panel_widget.MainPanelWidget`])
        ViewModel([`gui.unified_view_model.UnifiedViewModel`])
        RestructureHandler([`gui.asset_restructure_handler.AssetRestructureHandler`])
    end

    subgraph Data Structures
        RuleStruct([`rule_structure.py` <br> SourceRule, AssetRule, FileRule])
    end

    UserAction --> MainWindow
    MainWindow --> MainPanel
    MainPanel -- Triggers Predictions --> LLM_Handler
    MainPanel -- Triggers Predictions --> Preset_Handler
    MainPanel -- Displays Data From --> ViewModel

    LLM_Handler -- Provides SourceRule Data --> ViewModel
    Preset_Handler -- Provides SourceRule Data --> ViewModel

    ViewModel -- Manages --> RuleStruct
    ViewModel -- Signals Changes --> RestructureHandler
    ViewModel -- Signals Changes --> MainPanel


    RestructureHandler -- Modifies --> ViewModel

    %% Issues
    style LLM_Handler fill:#f9d,stroke:#333,stroke-width:2px %% Highlight LLM Handler for file list issue
    style ViewModel fill:#f9d,stroke:#333,stroke-width:2px %% Highlight ViewModel for both issues
    style RestructureHandler fill:#f9d,stroke:#333,stroke-width:2px %% Highlight Restructure Handler for empty row issue

    note right of LLM_Handler: Potential source of file list discrepancies
    note right of RestructureHandler: Potential source of persistent empty asset rows