Skip to content

fix(gui): reduce AI settings modal full-reload usage#1134

Merged
chhoumann merged 1 commit intomasterfrom
fsm-phase2-reduce-ai-modal-reloads
Mar 5, 2026
Merged

fix(gui): reduce AI settings modal full-reload usage#1134
chhoumann merged 1 commit intomasterfrom
fsm-phase2-reduce-ai-modal-reloads

Conversation

@chhoumann
Copy link
Owner

@chhoumann chhoumann commented Mar 5, 2026

Reduce unnecessary full modal reloads in AI settings flows by rendering high-churn sections in place.

This follows the FSM migration in #1132 and queue hardening in #1133. The remaining UX churn in these modals was mostly caused by full reloads for interactions that only affect a small subsection of UI.

What changed:

  • AIAssistantCommandSettingsModal
    • show/hide advanced settings now re-renders only the advanced section container (no full modal reload)
    • name edit now updates header text directly (no full modal reload)
    • model dropdown now refreshes token count text directly (no full modal reload)
  • InfiniteAIAssistantCommandSettingsModal
    • show/hide advanced settings now re-renders only the advanced section container (no full modal reload)
    • name edit now updates header text directly (no full modal reload)
  • Updated the living exec plan with this phase-2 decision/progress/evidence

Why:

  • Advanced settings toggle is a frequent interaction and does not require rebuilding unrelated controls.
  • In-place updates reduce UI churn and make the modal feel stable even before fallback restoration logic is needed.

Validation:

  • bun run test
  • bun run build-with-lint
  • Obsidian CLI probe in dev vault:
    • open quickadd:testQuickAdd
    • force modal scrollable height
    • toggle Show advanced settings
    • observed before/after scrollTop remain 220 and active element remain TEXTAREA
    • dev:errors reported no runtime errors

Refs #1130


Open with Devin

Summary by CodeRabbit

Bug Fixes

  • AI command settings modals now update more efficiently—advanced settings, model changes, and name edits no longer trigger unnecessary full reloads, improving responsiveness.

@vercel
Copy link

vercel bot commented Mar 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
quickadd Ready Ready Preview Mar 5, 2026 6:57am

@chhoumann chhoumann marked this pull request as ready for review March 5, 2026 06:57
@coderabbitai
Copy link

coderabbitai bot commented Mar 5, 2026

📝 Walkthrough

Walkthrough

Two AI command settings modal components are refactored to eliminate full modal reloads for show advanced settings toggles, name edits, and model changes. Instead, targeted in-place UI updates are performed. A planning document is updated to reflect this phase-2 approach with supporting rationale and observations.

Changes

Cohort / File(s) Summary
Planning & Documentation
plans/issue-1130-fsm-modal-reload.md
Phase-2 plan added documenting in-place advanced settings rendering instead of full reloads; Surprises & Discoveries, Decision Log, and Outcomes sections updated with rationale and CLI evidence; Revision Note reflects phase-2 partial migration.
Modal UI Components
src/gui/MacroGUIs/AIAssistantCommandSettingsModal.ts, src/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal.ts
Two new fields added (advancedSettingsContainer, refreshTokenCountText) to manage targeted updates. Show Advanced Settings toggle now invokes renderAdvancedSettingsSection() instead of reloading. Name edits update header text in-place. Model changes trigger token-count refresh without full reload. New renderAdvancedSettingsSection() method handles advanced settings rendering directly.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 No more full reloads, just targeted care,
Advanced settings rendered right there,
Names update smoothly, models refresh with grace,
UI bounces less in this speedier space!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the primary change: reducing unnecessary full-modal reloads in AI settings modals by implementing targeted, in-place UI updates instead.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fsm-phase2-reduce-ai-modal-reloads

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link

Deploying quickadd with  Cloudflare Pages  Cloudflare Pages

Latest commit: 8cfbf43
Status: ✅  Deploy successful!
Preview URL: https://aea1e1d3.quickadd.pages.dev
Branch Preview URL: https://fsm-phase2-reduce-ai-modal-r.quickadd.pages.dev

View logs

Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 3 additional findings.

Open in Devin Review

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/gui/MacroGUIs/AIAssistantCommandSettingsModal.ts (1)

257-270: Consider extracting advanced-section rendering to a shared helper.

This block closely mirrors the implementation in src/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal.ts (Line 214-227). Centralizing it would reduce maintenance drift across the two modals.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/gui/MacroGUIs/AIAssistantCommandSettingsModal.ts` around lines 257 - 270,
The renderAdvancedSettingsSection implementations in
AIAssistantCommandSettingsModal.renderAdvancedSettingsSection and
AIAssistantInfiniteCommandSettingsModal.renderAdvancedSettingsSection are
duplicated; extract the common logic into a single shared helper (either a
utility function like renderCommonAdvancedSettings(container, settings) or a
protected method on a shared base class such as
AIAssistantBaseSettingsModal.renderAdvancedSettingsSectionCommon) that performs
the container.empty(), showAdvancedSettings check, ensures
settings.modelParameters, and calls
addTemperatureSetting/addTopPSetting/addFrequencyPenaltySetting/addPresencePenaltySetting
on the provided container; then replace both original methods to call that
shared helper to remove duplication and keep references to the existing methods
(addTemperatureSetting, addTopPSetting, addFrequencyPenaltySetting,
addPresencePenaltySetting) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/gui/MacroGUIs/AIAssistantCommandSettingsModal.ts`:
- Around line 257-270: The renderAdvancedSettingsSection implementations in
AIAssistantCommandSettingsModal.renderAdvancedSettingsSection and
AIAssistantInfiniteCommandSettingsModal.renderAdvancedSettingsSection are
duplicated; extract the common logic into a single shared helper (either a
utility function like renderCommonAdvancedSettings(container, settings) or a
protected method on a shared base class such as
AIAssistantBaseSettingsModal.renderAdvancedSettingsSectionCommon) that performs
the container.empty(), showAdvancedSettings check, ensures
settings.modelParameters, and calls
addTemperatureSetting/addTopPSetting/addFrequencyPenaltySetting/addPresencePenaltySetting
on the provided container; then replace both original methods to call that
shared helper to remove duplication and keep references to the existing methods
(addTemperatureSetting, addTopPSetting, addFrequencyPenaltySetting,
addPresencePenaltySetting) intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 07dd4105-d409-4c0f-b48d-359d7634a843

📥 Commits

Reviewing files that changed from the base of the PR and between 9373144 and 8cfbf43.

📒 Files selected for processing (3)
  • plans/issue-1130-fsm-modal-reload.md
  • src/gui/MacroGUIs/AIAssistantCommandSettingsModal.ts
  • src/gui/MacroGUIs/AIAssistantInfiniteCommandSettingsModal.ts

@chhoumann chhoumann merged commit ae0f7a1 into master Mar 5, 2026
5 checks passed
@chhoumann chhoumann deleted the fsm-phase2-reduce-ai-modal-reloads branch March 5, 2026 07:02
chhoumann added a commit that referenced this pull request Mar 5, 2026
chhoumann added a commit that referenced this pull request Mar 5, 2026
* Revert "fix(gui): reduce choice builder reload churn (#1136)"

This reverts commit 818272a.

* Revert "fix(gui): reduce macro settings modal reload churn (#1135)"

This reverts commit a1a6271.

* Revert "fix(gui): reduce ai settings modal reload churn (#1134)"

This reverts commit ae0f7a1.

* Revert "test(gui): harden modal reload queue replay (#1133)"

This reverts commit 9373144.

* Revert "fix(gui): preserve modal edit position during settings reload (#1132)"

This reverts commit 11bda19.
github-actions bot pushed a commit that referenced this pull request Mar 5, 2026
# [2.12.0](2.11.0...2.12.0) (2026-03-05)

### Bug Fixes

* **capture:** preserve canvas tab indentation on configured writes ([#1125](#1125)) ([0a1578e](0a1578e))
* disallow capture targets with .base extension ([cb39ed4](cb39ed4))
* **field-suggestions:** opt-in inline values from fenced code blocks ([#1128](#1128)) ([8597905](8597905))
* **gui:** preserve modal edit position during settings reload ([#1132](#1132)) ([11bda19](11bda19))
* **gui:** reduce ai settings modal reload churn ([#1134](#1134)) ([ae0f7a1](ae0f7a1))
* **gui:** reduce choice builder reload churn ([#1136](#1136)) ([818272a](818272a))
* **gui:** reduce macro settings modal reload churn ([#1135](#1135)) ([a1a6271](a1a6271))
* harden existing-tab matching and document issue workflow ([#1108](#1108)) ([7b12d3b](7b12d3b))
* make template path resolution deterministic ([3297d54](3297d54))
* normalize capture title for non-markdown targets ([964d672](964d672))
* preserve capture-format spacing for insert-at-end ([#1119](#1119)) ([8bb8ed4](8bb8ed4))
* preserve explicit capture target file extensions ([57e43ff](57e43ff))
* preserve insert-at-end order for non-newline captures ([#1120](#1120)) ([e7cbbf2](e7cbbf2))
* resolve template file-name paths without duplicate default folder ([7bfd41b](7bfd41b))
* resolve vault-relative template paths using root folders ([81216de](81216de))

### Features

* add AI request logging API and reduce assistant log noise ([#1110](#1110)) ([2c36800](2c36800))
* automate docs version snapshot during release ([#1111](#1111)) ([1571846](1571846))
* **capture:** fully support capture into canvas cards ([#1124](#1124)) ([a53f889](a53f889))
* **cli:** add native QuickAdd Obsidian CLI handlers ([#1129](#1129)) ([8102d47](8102d47))
* **format:** support mapped VALUE suggester display text ([#1127](#1127)) ([b8ec56c](b8ec56c))
* **macro:** add editor cursor navigation commands ([101d5f6](101d5f6))
* support .base template files for template choices ([11e6490](11e6490))

### Reverts

* **gui:** remove modal reload refactor series ([#1137](#1137)) ([3ba1a73](3ba1a73)), closes [#1136](#1136) [#1135](#1135) [#1134](#1134) [#1133](#1133) [#1132](#1132)
@github-actions
Copy link

github-actions bot commented Mar 5, 2026

🎉 This PR is included in version 2.12.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant