Files
galaxy/specs/APP-3892/TECH.md
T

6.6 KiB
Raw Blame History

Always Show Comment Buttons Regardless of AI State — Technical Spec

Problem

The code review panel gates all comment and add-as-context button visibility on AISettings::is_any_ai_enabled(). This hides the buttons entirely when AI is disabled, even though CLI agent terminals (Claude Code, Gemini) can accept review comments without Warp AI. The implementation needs to decouple button visibility from AI state, restrict non-CLI terminal availability when AI is off, and surface the correct reason when the send button is disabled.

Relevant Code

  • app/src/code_review/code_review_header.rs (127-231) — header dropdown buttons gated by is_ai_enabled
  • app/src/code_review/code_review_view.rs:5175 — per-file "add as context" button gated by is_ai_enabled
  • app/src/code/editor/element.rs (418, 516, 959, 1170, 1176, 1315, 1517)EditorWrapper field, constructor param, and 5 usage sites
  • app/src/code/editor/view.rs (628, 2216) — both EditorWrapper::new call sites pass is_ai_enabled
  • app/src/code/editor/view/actions.rs (1070, 1082)NewCommentOnLine and RequestOpenSavedComment gated by AI check
  • app/src/workspace/right_panel.rs (341, 1067, 1154, 1184, 1218)RightPanelView::new, route_review_comments, is_terminal_available_for_review, find_available_terminal_for_review, recompute_terminal_availability
  • app/src/code_review/comment_list_view.rs (864, 888)send_button_tooltip_text, render_send_button

Current State

  • is_ai_enabled is checked by AISettings::as_ref(ctx).is_any_ai_enabled(ctx), which evaluates global toggle + login state + remote session org policy.
  • AISettingsChangedEvent::IsAnyAIEnabled is auto-generated by the define_settings_group! macro and fires when AI state changes.
  • ReviewDestination enum in right_panel.rs: None, Warp, Cli(CLIAgent).
  • recompute_terminal_availability currently computes ReviewDestination purely from terminal state — returns Warp for any idle, non-executing terminal regardless of AI setting.
  • RightPanelView does not currently subscribe to AISettingsChangedEvent.

Proposed Changes

1. Remove AI gate from header and per-file buttons

Files: code_review_header.rs, code_review_view.rs

Remove is_ai_enabled from the visibility conditions in render_wide_layout (line 138), render_compact_layout (line 220), and render_file_header (line 5175). The conditions become purely feature-flag-based:

  • Header: FeatureFlag::DiffSetAsContext.is_enabled() && !has_no_changes
  • Per-file: FeatureFlag::DiffSetAsContext.is_enabled()

2. Remove is_ai_enabled from EditorWrapper

Files: code/editor/element.rs, code/editor/view.rs, code/editor/view/actions.rs

  • Delete is_ai_enabled: bool field (line 418) and constructor parameter (line 516).
  • Simplify the 5 internal usages to check only the feature flag:
    • Line 959: if FeatureFlag::InlineCodeReview.is_enabled()
    • Line 1170: self.add_hunk_as_context_button.is_some()
    • Line 1176: FeatureFlag::InlineCodeReview.is_enabled() && ...
    • Lines 1315, 1517: FeatureFlag::InlineCodeReview.is_enabled()
  • Remove the is_ai_enabled argument from both EditorWrapper::new call sites in view.rs (lines 628, 2216).
  • Remove the AISettings::as_ref(ctx).is_any_ai_enabled(ctx) guard from NewCommentOnLine (line 1070) and RequestOpenSavedComment (line 1082) in actions.rs.

3. Filter non-CLI terminals when AI is disabled

File: right_panel.rs

  • Add ai_enabled: bool parameter to is_terminal_available_for_review (line 1154). When false, reject terminals that don't have an active CLI agent.
  • Thread ai_enabled through find_available_terminal_for_review (line 1184).
  • In recompute_terminal_availability (line 1218), read AISettings::as_ref(ctx).is_any_ai_enabled(ctx) and pass it down.
  • In route_review_comments (line 1067), also read is_any_ai_enabled and pass it through so routing respects AI state at submission time.

4. Subscribe to AI settings changes

File: right_panel.rsnew() (around line 341)

Subscribe to the AISettings model, matching on AISettingsChangedEvent::IsAnyAIEnabled, and call recompute_terminal_availability(ctx). This ensures the send button's enabled state and tooltip update immediately when the user toggles AI.

Import needed: use crate::settings::ai::{AISettings, AISettingsChangedEvent}.

5. Differentiate disabled button tooltip

File: comment_list_view.rs

  • Add ai_enabled: bool parameter to send_button_tooltip_text (line 864). New priority:
    1. CLI agent destination → existing CLI tooltip
    2. !ai_enabled → "AI must be enabled to send comments to Agent"
    3. !ai_available → "Agent code review requires AI credits" (existing)
    4. ReviewDestination::None → "All terminals are busy" (clarified)
    5. !has_sendable_comments → existing
    6. Default → existing
  • In render_send_button (line 888), read AISettings::as_ref(ctx).is_any_ai_enabled(ctx) and pass it to the tooltip function.

End-to-End Flow

  1. User opens code review panel with AI disabled.
  2. Comment and add-as-context buttons are visible (changes 1+2).
  3. User creates an inline comment via gutter button.
  4. recompute_terminal_availability runs, sees AI is off, filters to CLI-only terminals (change 3).
  5. If a CLI agent terminal exists → ReviewDestination::Cli(agent) → send button enabled.
  6. If no CLI agents → ReviewDestination::None → send button disabled, tooltip says "AI must be enabled to send comments to Agent" (change 5).
  7. User toggles AI on → subscription fires (change 4) → recompute includes non-CLI terminals → button updates.

Risks and Mitigations

  • Deadlock risk: Changes 34 read AISettings in right_panel.rs. The AI settings model is separate from the terminal model, so no lock contention with existing TerminalModel locks.
  • Regression risk: Removing is_ai_enabled from EditorWrapper touches many lines. Mitigated by the field being entirely removed (compiler will catch any missed references).

Testing and Validation

  • Build verification: cargo check, cargo fmt, cargo clippy.
  • Manual testing: toggle AI on/off with and without CLI agents; verify button visibility and tooltip in each combination.
  • WASM build per repo convention.

Parallelization

Changes 1+2 (button visibility) are independent from changes 3+4+5 (terminal availability and tooltip). These can be implemented concurrently:

  • Agent A: Changes 1+2 — code_review_header.rs, code_review_view.rs, code/editor/element.rs, code/editor/view.rs, code/editor/view/actions.rs
  • Agent B: Changes 3+4+5 — right_panel.rs, comment_list_view.rs

Follow-Ups

None currently.