chore(conductor): Add new track 'text_viewer_and_tool_call_fixes_20260601'
This commit is contained in:
+6
-1
@@ -288,5 +288,10 @@ This file tracks all major tracks for the project. Each track has its own detail
|
||||
|
||||
---
|
||||
|
||||
- [ ] **Track: Preserve context selection on discussion switch and add empty context warning**
|
||||
- [~] **Track: Preserve context selection on discussion switch and add empty context warning**
|
||||
*Link: [./tracks/context_preservation_and_warnings_20260601/](./tracks/context_preservation_and_warnings_20260601/)*
|
||||
|
||||
---
|
||||
|
||||
- [ ] **Track: Fix Text Viewer docking conflicts and Tool Call row click interactivity**
|
||||
*Link: [./tracks/text_viewer_and_tool_call_fixes_20260601/](./tracks/text_viewer_and_tool_call_fixes_20260601/)*
|
||||
|
||||
@@ -1,18 +1,22 @@
|
||||
# Implementation Plan: Context Preservation and Empty Warning
|
||||
# Implementation Plan: Context Preservation and Warnings
|
||||
|
||||
## Phase 1: Context State Synchronization
|
||||
- [ ] Task: Sync UI state on discussion switch
|
||||
- [ ] Add a `_sync_context_selection_from_controller()` method to `App` in `src/gui_2.py`. This method should clear `self.ui_selected_context_files` and repopulate it with the paths of all `FileItem`s in `self.context_files` that have `auto_aggregate == True`.
|
||||
- [ ] Wrap the `AppController._switch_discussion` call in `App._switch_discussion` within `src/gui_2.py`. Update all GUI call sites to use `app._switch_discussion` instead of `app.controller._switch_discussion`.
|
||||
## Phase 1: Context Inheritance on Creation
|
||||
- [ ] Task: Update `_create_discussion`
|
||||
- [ ] In `src/app_controller.py`, modify `_create_discussion`. Before switching, capture the current `context_files` (with their current `auto_aggregate` states).
|
||||
- [ ] When initializing the new discussion via `project_manager.default_discussion()`, immediately set its `context_snapshot` to a serialization of the current `context_files`.
|
||||
|
||||
## Phase 2: Empty Context Warning
|
||||
- [ ] Task: Implement warning on generate
|
||||
- [ ] In `src/gui_2.py`, within `_handle_generate_send`, add a check: if `len(self.ui_selected_context_files) == 0`, set a new flag `self.show_empty_context_warning_modal = True` and return early instead of calling `self.controller._handle_generate_send()`.
|
||||
- [ ] Create a `render_empty_context_warning_modal(app: App)` function that displays an ImGui popup warning the user. It should offer "Proceed Anyway" (which then calls `app.controller._handle_generate_send()`) and "Cancel" buttons.
|
||||
- [ ] Add the modal render call to `render_context_modals` (or similar modal rendering loop).
|
||||
## Phase 2: Empty Context Warning Modal
|
||||
- [ ] Task: Implement Warning Logic in App
|
||||
- [ ] In `src/gui_2.py`, add `self.show_empty_context_modal = False` and `self._pending_generation_action = None` (to store whether 'generate' or 'md_only' was requested) to `App.__init__`.
|
||||
- [ ] In `App._handle_generate_send` and `App._handle_md_only`, check if `self.ui_selected_context_files` is empty.
|
||||
- [ ] If empty, set `self.show_empty_context_modal = True` and store the action type. If not empty, call the respective controller method directly.
|
||||
- [ ] Task: Render Modal
|
||||
- [ ] In `src/gui_2.py`, add a new function `render_empty_context_modal(app: App)`.
|
||||
- [ ] The modal should display a warning and have "Proceed Anyway" (which calls the controller based on `_pending_generation_action`) and "Cancel" buttons.
|
||||
- [ ] Call this new render function inside the main UI loop, likely near where `render_missing_files_modal` is called.
|
||||
|
||||
## Phase 3: Verification
|
||||
- [ ] Task: Verification
|
||||
- [ ] Create a discussion, select some files, create a new discussion, and verify the selection is preserved.
|
||||
- [ ] Attempt to generate a response with no files selected and verify the warning modal appears.
|
||||
- [ ] Task: Manual Verification
|
||||
- [ ] Verify creating a new discussion preserves checked files.
|
||||
- [ ] Verify the warning modal appears when generating with no files, and that "Proceed Anyway" works correctly.
|
||||
- [ ] Task: Conductor - User Manual Verification 'Phase 3: Verification' (Protocol in workflow.md)
|
||||
@@ -1,16 +1,18 @@
|
||||
# Specification: Context Preservation and Empty Warning
|
||||
# Specification: Context Preservation and Warnings
|
||||
|
||||
## 1. Overview
|
||||
When a user creates a new discussion or switches to an existing one, the selection state of the context files (which files are checked) is lost. This happens because the GUI's `ui_selected_context_files` set is not synchronized with the newly loaded `context_files` from the discussion snapshot. Additionally, the user requested a warning if they attempt to generate an AI response when no context files are selected.
|
||||
Currently, creating a new discussion drops all currently selected files because the new discussion is initialized without a `context_snapshot`. Additionally, attempting to generate a response without any selected context files provides no user feedback. We need to inherit the current context composition when a new discussion is created and add a warning mechanism for empty contexts.
|
||||
|
||||
## 2. Functional Requirements
|
||||
* **State Synchronization:** When `_switch_discussion` is called in `AppController`, it must signal the GUI to reconstruct its `ui_selected_context_files` set based on the `auto_aggregate` flag of the newly loaded `FileItem` objects.
|
||||
* **Empty Context Warning:** When the user clicks "Generate Response" (via `_handle_generate_send` or similar trigger), the system should check if any files are selected for aggregation. If not, it should display a warning or confirmation modal alerting the user that the context is empty.
|
||||
* **Context Inheritance:** When a new discussion is created via `_create_discussion` in `src/app_controller.py`, the active discussion's `context_files` (or the current `ui_selected_context_files` state) MUST be saved into the new discussion's `context_snapshot`.
|
||||
* **Empty Context Warning:** When `_handle_generate_send` or `_handle_md_only` is triggered in `src/gui_2.py`, it must verify if `ui_selected_context_files` is empty. If empty, it MUST display a warning modal instead of immediately calling the controller.
|
||||
* **Modal Actions:** The warning modal should offer "Proceed Anyway" (which continues the generation) and "Cancel" buttons.
|
||||
|
||||
## 3. Non-Functional Requirements
|
||||
* **Performance:** The state sync should be a lightweight operation during the discussion switch lifecycle.
|
||||
* **Consistency:** The new empty context modal must match the style of existing warnings (like `show_missing_files_modal`).
|
||||
* **ImGui Stability:** Ensure the modal rendering does not violate ImGui stack scoping.
|
||||
|
||||
## 4. Acceptance Criteria
|
||||
* Creating a new discussion correctly copies over the checked/unchecked state of all files in the Context Composition panel.
|
||||
* Switching between past discussions restores their specific checked/unchecked file states.
|
||||
* Attempting to generate a response with 0 selected files displays a clear warning in the GUI.
|
||||
* Creating a new discussion with 3 files checked results in the new discussion also having those 3 files checked.
|
||||
* Clicking "Generate Response" with 0 files checked opens a warning modal.
|
||||
* Clicking "Proceed Anyway" successfully generates the response (with 0 files).
|
||||
@@ -0,0 +1,5 @@
|
||||
# Track text_viewer_and_tool_call_fixes_20260601 Context
|
||||
|
||||
- [Specification](./spec.md)
|
||||
- [Implementation Plan](./plan.md)
|
||||
- [Metadata](./metadata.json)
|
||||
@@ -0,0 +1,8 @@
|
||||
{
|
||||
"track_id": "text_viewer_and_tool_call_fixes_20260601",
|
||||
"type": "bug",
|
||||
"status": "new",
|
||||
"created_at": "2026-06-01T00:00:00Z",
|
||||
"updated_at": "2026-06-01T00:00:00Z",
|
||||
"description": "Fix Text Viewer docking conflicts and Tool Call row click interactivity"
|
||||
}
|
||||
@@ -0,0 +1,21 @@
|
||||
# Implementation Plan: Text Viewer and Tool Call Fixes
|
||||
|
||||
## Phase 1: Text Viewer Unification
|
||||
- [ ] Task: Update Window Management in `gui_2.py`
|
||||
- [ ] Remove `app._render_window_if_open("Text Viewer", lambda: render_text_viewer_window(app))` from `render_main_interface`.
|
||||
- [ ] Add a direct call to `render_text_viewer_window(app)` in `render_main_interface` (e.g., right before or after modals).
|
||||
- [ ] Update `render_text_viewer_window` to use `app.show_windows["Text Viewer"]` for its visibility check and `imgui.begin` state tracking.
|
||||
- [ ] Task: Update State Initialization
|
||||
- [ ] In `src/app_controller.py`, ensure `"Text Viewer": False` is present in `_default_windows`.
|
||||
- [ ] Remove usages of the legacy `app.show_text_viewer` across `gui_2.py` and replace them with `app.show_windows["Text Viewer"] = True`.
|
||||
|
||||
## Phase 2: Tool Call Row Interactivity
|
||||
- [ ] Task: Refactor Row Rendering
|
||||
- [ ] In `_render_tool_calls_panel` (or `render_tool_calls_panel`), modify the first column (`#` index) rendering to include an `imgui.selectable(..., span_all_columns=True)`.
|
||||
- [ ] Attach the `is_item_clicked()` logic to this selectable to populate `text_viewer_content` and set `app.show_windows["Text Viewer"] = True`.
|
||||
|
||||
## Phase 3: Verification
|
||||
- [ ] Task: Manual Verification
|
||||
- [ ] Trigger a text viewer opening (e.g., via a `[+]` button) and attempt to dock it. Verify it remains stable.
|
||||
- [ ] Click a row in the Tool Calls panel and verify the detail window opens.
|
||||
- [ ] Task: Conductor - User Manual Verification 'Phase 3: Verification' (Protocol in workflow.md)
|
||||
@@ -0,0 +1,20 @@
|
||||
# Specification: Text Viewer Conflict and Tool Call Interactivity Fix
|
||||
|
||||
## 1. Overview
|
||||
The user reported two UI regressions:
|
||||
1. **Text Viewer Docking Glitch:** The "Text Viewer" opens in a state that conflicts with docking, causing infinite open/close loops. This is due to a nested `imgui.begin()` conflict where the window is managed by both `_render_window_if_open` and its own internal function, resulting in two logical windows fighting for state.
|
||||
2. **Tool Call Row Click Failure:** Clicking a row in the Tool Calls history panel no longer opens the detailed view. This occurred because the row click detection relied on `is_item_clicked()` after a `render_selectable_label` (which wraps an `input_text`), leading to unreliable click capture in table contexts.
|
||||
|
||||
## 2. Functional Requirements
|
||||
* **Unified Window State:** The `Text Viewer` must use a single source of truth for its visibility (`app.show_windows["Text Viewer"]`). The legacy `app.show_text_viewer` should be deprecated or synced.
|
||||
* **Flat Rendering:** `render_text_viewer_window` MUST NOT be called via `_render_window_if_open` since it manages its own dynamic title and `imgui.begin()` call. It should be called directly in the main render loop.
|
||||
* **Robust Row Clicks:** The Tool Calls history table MUST use an `imgui.selectable(..., span_all_columns=True)` in the first column to reliably capture row clicks and open the detail view, independent of the cell contents.
|
||||
|
||||
## 3. Non-Functional Requirements
|
||||
* **Stability:** Docking the Text Viewer must no longer cause input processing glitches.
|
||||
* **ImGui Consistency:** Avoid nested windows with the same internal IDs.
|
||||
|
||||
## 4. Acceptance Criteria
|
||||
* Opening an entry in the Text Viewer creates a stable, dockable window.
|
||||
* Closing the Text Viewer window correctly updates the UI state without infinite loops.
|
||||
* Clicking anywhere on a row in the Tool Calls panel immediately opens the detailed view in the Text Viewer.
|
||||
@@ -3022,6 +3022,8 @@ class AppController:
|
||||
if "context_snapshot" in disc_data:
|
||||
snapshot_data = disc_data["context_snapshot"]
|
||||
self.context_files = [models.FileItem.from_dict(f) if isinstance(f, dict) else models.FileItem(path=str(f)) for f in snapshot_data]
|
||||
if self._app:
|
||||
self._app.ui_selected_context_files = {f.path for f in self.context_files if f.auto_aggregate}
|
||||
self.ai_status = f"discussion: {name}"
|
||||
|
||||
def _flush_disc_entries_to_project(self) -> None:
|
||||
@@ -3050,9 +3052,11 @@ class AppController:
|
||||
if name in discussions:
|
||||
self.ai_status = f"discussion '{name}' already exists"
|
||||
return
|
||||
discussions[name] = project_manager.default_discussion()
|
||||
new_disc = project_manager.default_discussion()
|
||||
# Inherit context from current session if available
|
||||
if self.context_files:
|
||||
discussions[name]["context_snapshot"] = [f.to_dict() for f in self.context_files]
|
||||
new_disc["context_snapshot"] = [f.to_dict() if hasattr(f, 'to_dict') else f for f in self.context_files]
|
||||
discussions[name] = new_disc
|
||||
self._switch_discussion(name)
|
||||
|
||||
def _branch_discussion(self, index: int) -> None:
|
||||
|
||||
+63
-3
@@ -192,12 +192,17 @@ class App:
|
||||
self.context_preview_text = ""
|
||||
self.ui_separate_context_preview = False
|
||||
self.ui_active_context_preset = ""
|
||||
self.ui_new_context_preset_name = ""
|
||||
self.target_context_preset_name = ""
|
||||
self.show_empty_context_modal = False
|
||||
self._pending_generation_action = None # 'generate' or 'md_only'
|
||||
self._pending_save_ctx_click = False
|
||||
self._pending_save_anyway_click = False
|
||||
self.show_missing_files_modal = False
|
||||
self.missing_context_files = []
|
||||
self.target_context_preset_name = ""
|
||||
self.show_empty_context_warning_modal = False
|
||||
self._pending_proceed_generate = False
|
||||
self._pending_proceed_md_only = False
|
||||
self._new_preset_name = ""
|
||||
self._editing_preset_name = ""
|
||||
self._editing_preset_system_prompt = ""
|
||||
@@ -454,6 +459,20 @@ class App:
|
||||
else:
|
||||
object.__setattr__(self, name, value)
|
||||
|
||||
def _handle_generate_send(self) -> None:
|
||||
if not self.ui_selected_context_files and not getattr(self, "_pending_proceed_generate", False):
|
||||
self.show_empty_context_warning_modal = True
|
||||
else:
|
||||
self._pending_proceed_generate = False
|
||||
self.controller._handle_generate_send()
|
||||
|
||||
def _handle_md_only(self) -> None:
|
||||
if not self.ui_selected_context_files and not getattr(self, "_pending_proceed_md_only", False):
|
||||
self.show_empty_context_warning_modal = True
|
||||
else:
|
||||
self._pending_proceed_md_only = False
|
||||
self.controller._handle_md_only()
|
||||
|
||||
@property
|
||||
def current_provider(self) -> str:
|
||||
return self.controller.current_provider
|
||||
@@ -5418,7 +5437,45 @@ def render_mma_focus_selector(app: App) -> None:
|
||||
|
||||
#endregion: MMA
|
||||
|
||||
def render_empty_context_modal(app: App) -> None:
|
||||
if app.show_empty_context_modal:
|
||||
imgui.open_popup("Empty Context Warning")
|
||||
app.show_empty_context_modal = False
|
||||
|
||||
if imgui.begin_popup_modal("Empty Context Warning", True, imgui.WindowFlags_.always_auto_resize)[0]:
|
||||
imgui.text_colored(imgui.ImVec4(1.0, 1.0, 0.0, 1.0), "WARNING: Empty Context Composition")
|
||||
imgui.text("You are attempting to generate a response without any files selected.")
|
||||
imgui.text("This may result in poor AI performance or loss of project context.")
|
||||
imgui.separator()
|
||||
if imgui.button("Proceed Anyway", imgui.ImVec2(150, 0)):
|
||||
if app._pending_generation_action == 'generate': app.controller._handle_generate_send()
|
||||
elif app._pending_generation_action == 'md_only': app.controller._handle_md_only()
|
||||
imgui.close_current_popup()
|
||||
imgui.same_line()
|
||||
if imgui.button("Cancel", imgui.ImVec2(120, 0)):
|
||||
imgui.close_current_popup()
|
||||
imgui.end_popup()
|
||||
|
||||
def render_context_modals(app: App) -> None:
|
||||
render_empty_context_modal(app)
|
||||
if app.show_empty_context_warning_modal:
|
||||
imgui.open_popup("Empty Context Warning")
|
||||
app.show_empty_context_warning_modal = False
|
||||
|
||||
if imgui.begin_popup_modal("Empty Context Warning", True, imgui.WindowFlags_.always_auto_resize)[0]:
|
||||
imgui.text_colored(imgui.ImVec4(1.0, 1.0, 0.0, 1.0), "WARNING: Empty Context Composition")
|
||||
imgui.text("You are attempting to generate a response without any files selected.")
|
||||
imgui.text("This may result in poor AI performance or loss of project context.")
|
||||
imgui.separator()
|
||||
if imgui.button("Proceed Anyway", imgui.ImVec2(150, 0)):
|
||||
if app._empty_context_target_action == 'generate': app.controller._handle_generate_send()
|
||||
elif app._empty_context_target_action == 'md_only': app.controller._handle_md_only()
|
||||
imgui.close_current_popup()
|
||||
imgui.same_line()
|
||||
if imgui.button("Cancel", imgui.ImVec2(120, 0)):
|
||||
imgui.close_current_popup()
|
||||
imgui.end_popup()
|
||||
|
||||
if app.show_missing_files_modal:
|
||||
imgui.open_popup("Missing Files Warning")
|
||||
app.show_missing_files_modal = False
|
||||
@@ -5464,10 +5521,10 @@ def _get_context_composition_state(app: App) -> tuple:
|
||||
for f in app.context_files:
|
||||
p = f.path if hasattr(f, 'path') else str(f)
|
||||
vm = f.view_mode if hasattr(f, 'view_mode') else 'summary'
|
||||
sel = f.selected if hasattr(f, 'selected') else False
|
||||
agg = f.auto_aggregate if hasattr(f, 'auto_aggregate') else False
|
||||
slc = tuple((s.get('start_line'), s.get('end_line'), s.get('tag'), s.get('comment')) for s in getattr(f, 'custom_slices', []))
|
||||
mask = tuple(sorted(getattr(f, 'ast_mask', {}).items()))
|
||||
files_state.append((p, vm, sel, slc, mask))
|
||||
files_state.append((p, vm, agg, slc, mask))
|
||||
screenshots_state = tuple(app.screenshots)
|
||||
return (tuple(files_state), screenshots_state)
|
||||
|
||||
@@ -5475,6 +5532,9 @@ def _check_auto_refresh_context_preview(app: App) -> None:
|
||||
current_state = _get_context_composition_state(app)
|
||||
if not hasattr(app, "_last_context_preview_state") or app._last_context_preview_state != current_state:
|
||||
app._last_context_preview_state = current_state
|
||||
if not any(getattr(f, 'auto_aggregate', False) for f in app.context_files) and not app.screenshots:
|
||||
app.context_preview_text = "# Context Composition Empty\n\nNo files or screenshots have been selected for aggregation."
|
||||
return
|
||||
try:
|
||||
app.controller.context_files = app.context_files
|
||||
res = app.controller._do_generate()
|
||||
|
||||
Reference in New Issue
Block a user