feat(prompts): add reusable prompt presets#213
Conversation
📝 WalkthroughWalkthroughAdded a shared Web UI prompt preset pool with persistence, CRUD workflows, localized UI, tests, responsive styling, and documentation. The OpenAI bridge now converts Responses requests through chat completions without probing upstream Responses endpoints. ChangesPrompt preset pool
OpenAI Responses routing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant PromptPresetMethods
participant PromptEditor
participant WebUiPreferences
User->>PromptPresetMethods: Save or apply a prompt preset
PromptPresetMethods->>PromptEditor: Update editor content
PromptPresetMethods->>WebUiPreferences: Persist promptPresets
WebUiPreferences-->>PromptPresetMethods: Return normalized preset state
PromptPresetMethods-->>User: Show operation status
sequenceDiagram
participant Client
participant OpenAIBridge
participant ChatCompletions
Client->>OpenAIBridge: Send Responses request
OpenAIBridge->>ChatCompletions: Send converted chat request
ChatCompletions-->>OpenAIBridge: Return chat response or stream
OpenAIBridge-->>Client: Return Responses-compatible JSON or SSE
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.44.1)web-ui/res/web-ui-render.precompiled.jsast-grep timed out on this file 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. Comment |
awsl233777
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web-ui/modules/app.methods.agents.mjs (1)
871-891: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSwitching away from the presets tab can silently discard an unsaved draft.
switchPromptsSubTab('codex' | 'claude-project')unconditionally reassignspromptsSubTab, which triggers thepromptsSubTabwatcher'sloadPromptsContent()call (seeweb-ui/app.jsLine 752), overwritingagentsContentwith the freshly-fetched file — with no confirmation. Since the whole point of the newpresetstab is to let a user pause mid-edit, save the draft as a preset, and come back, a user who navigates topresets(without saving) and then clicks back tocodex/claude-projectloses their in-progress edit with no warning.applyPromptPresetToEditor(Lines 806-815) already guards this exact scenario viahasAgentsContentChanged()+requestConfirmDialog; the same guard is missing here for plain tab navigation.🛡️ Suggested fix
- switchPromptsSubTab(subTab) { + async switchPromptsSubTab(subTab) { const normalized = subTab === 'claude-project' || subTab === 'presets' ? subTab : 'codex'; if (normalized === 'presets') { this.promptsSubTab = normalized; this.$nextTick(() => { document.querySelector('.main-panel')?.scrollTo({ top: 0, left: 0, behavior: 'auto' }); }); return; } + if (this.promptsSubTab === 'presets' && normalized !== this.promptsSubTab && this.hasAgentsContentChanged()) { + const confirmed = await this.requestConfirmDialog({ + title: this.t('prompts.presets.confirm.discardTitle'), + message: this.t('prompts.presets.confirm.discardMessage'), + confirmText: this.t('prompts.presets.confirm.discardConfirm'), + cancelText: this.t('common.cancel'), + danger: true + }); + if (!confirmed) return; + } if (normalized === 'claude-project' && !this.projectPathOptions.length && !this.projectPathOptionsLoading) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/modules/app.methods.agents.mjs` around lines 871 - 891, Update switchPromptsSubTab to guard navigation from the presets tab to codex or claude-project with hasAgentsContentChanged() and requestConfirmDialog, matching applyPromptPresetToEditor. Only change promptsSubTab and continue loading or scrolling after confirmation; preserve existing behavior when there are no unsaved changes or when entering presets.
🧹 Nitpick comments (4)
tests/unit/prompt-presets.test.mjs (2)
91-92: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert persisted state, not only write counts.
The tests do not verify that the final persisted snapshot contains the overwritten/deleted presets and cleared selection. Compare the serialized snapshot with the expected state to catch stale or partial persistence.
Also applies to: 145-145
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/prompt-presets.test.mjs` around lines 91 - 92, Update the persistence assertions in the affected tests around the existing persisted.length checks to verify the serialized persisted snapshot, not just the number of writes. Compare the final snapshot against the expected state, including overwritten and deleted presets and the cleared selection, in both locations.
54-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake confirmation responses controllable in the test VM.
requestConfirmDialogalways returnstrue, so overwrite, apply, and delete cancellation paths are untested. Add a response queue or per-test override and assert that cancellation leaves state unchanged.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/prompt-presets.test.mjs` around lines 54 - 57, Update the test VM’s requestConfirmDialog stub to support queued or per-test confirmation responses instead of always returning true. Use false responses to cover overwrite, apply, and delete cancellation paths, and assert each cancellation leaves the relevant state unchanged while preserving existing confirmation behavior.web-ui/styles/modals-core.css (1)
779-791: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeprecated
word-break: break-word(flagged by stylelint).Per CSS Text spec and MDN,
word-break: break-wordis deprecated; it's equivalent tooverflow-wrap: anywhere+word-break: normal. Browsers still honor it for now, but prefer the modern properties.🎨 Suggested fix
.prompt-preset-preview { max-height: 180px; margin: 8px 0 0; padding: 10px; overflow: auto; white-space: pre-wrap; - word-break: break-word; + overflow-wrap: anywhere; + word-break: normal; border-radius: var(--radius-sm);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/styles/modals-core.css` around lines 779 - 791, Update the .prompt-preset-preview rule to remove the deprecated word-break: break-word declaration and use the modern equivalent properties: overflow-wrap: anywhere and word-break: normal.Source: Linters/SAST tools
web-ui/partials/index/panel-prompts.html (1)
60-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
selectedPromptPresetIdis tracked but never surfaced in the UI.
web-ui/modules/app.methods.agents.mjssets/clearsselectedPromptPresetIdon save, apply, and delete (andweb-ui-preferences.mjsclears it defensively on preference sync), but no preset card here binds a class topreset.id === selectedPromptPresetId, so the tracked "currently selected/applied" preset is never visually indicated. Either surface it (e.g., highlight the active card) or, if unused, this is dead state that adds upkeep cost without benefit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/partials/index/panel-prompts.html` around lines 60 - 78, Surface the tracked selectedPromptPresetId in the prompt preset card UI by conditionally applying the existing active/selected card styling when preset.id matches selectedPromptPresetId. Update the article element in the promptPresets loop, preserving the current card rendering and actions for unselected presets.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@web-ui/modules/app.methods.agents.mjs`:
- Around line 871-891: Update switchPromptsSubTab to guard navigation from the
presets tab to codex or claude-project with hasAgentsContentChanged() and
requestConfirmDialog, matching applyPromptPresetToEditor. Only change
promptsSubTab and continue loading or scrolling after confirmation; preserve
existing behavior when there are no unsaved changes or when entering presets.
---
Nitpick comments:
In `@tests/unit/prompt-presets.test.mjs`:
- Around line 91-92: Update the persistence assertions in the affected tests
around the existing persisted.length checks to verify the serialized persisted
snapshot, not just the number of writes. Compare the final snapshot against the
expected state, including overwritten and deleted presets and the cleared
selection, in both locations.
- Around line 54-57: Update the test VM’s requestConfirmDialog stub to support
queued or per-test confirmation responses instead of always returning true. Use
false responses to cover overwrite, apply, and delete cancellation paths, and
assert each cancellation leaves the relevant state unchanged while preserving
existing confirmation behavior.
In `@web-ui/partials/index/panel-prompts.html`:
- Around line 60-78: Surface the tracked selectedPromptPresetId in the prompt
preset card UI by conditionally applying the existing active/selected card
styling when preset.id matches selectedPromptPresetId. Update the article
element in the promptPresets loop, preserving the current card rendering and
actions for unselected presets.
In `@web-ui/styles/modals-core.css`:
- Around line 779-791: Update the .prompt-preset-preview rule to remove the
deprecated word-break: break-word declaration and use the modern equivalent
properties: overflow-wrap: anywhere and word-break: normal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b693f286-cfc8-4fc9-95cb-a1870f27fc15
📒 Files selected for processing (17)
README.mdREADME.vi.mdREADME.zh.mdtests/unit/prompt-presets.test.mjstests/unit/run.mjstests/unit/web-ui-behavior-parity.test.mjsweb-ui/app.jsweb-ui/modules/app.methods.agents.mjsweb-ui/modules/app.methods.web-ui-preferences.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/panel-prompts.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/modals-core.css
✅ Files skipped from review due to trivial changes (5)
- README.vi.md
- tests/unit/run.mjs
- README.md
- README.zh.md
- web-ui/modules/i18n/locales/zh.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- web-ui/modules/i18n/locales/ja.mjs
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: CodeRabbit
🧰 Additional context used
🪛 Stylelint (17.14.0)
web-ui/styles/modals-core.css
[error] 785-785: Deprecated keyword "break-word" for property "word-break" (declaration-property-value-keyword-no-deprecated)
(declaration-property-value-keyword-no-deprecated)
🔇 Additional comments (15)
web-ui/modules/i18n/locales/en.mjs (1)
170-176: LGTM!Also applies to: 226-251
web-ui/modules/i18n/locales/vi.mjs (1)
130-137: LGTM!Also applies to: 241-266
web-ui/modules/i18n/locales/zh-tw.mjs (1)
170-176: LGTM!Also applies to: 226-251
tests/unit/prompt-presets.test.mjs (1)
1-53: LGTM!Also applies to: 58-90, 93-144, 146-147
tests/unit/web-ui-behavior-parity.test.mjs (1)
378-383: LGTM!Also applies to: 430-435, 658-668
web-ui/app.js (2)
77-82: LGTM!
739-755: 🎯 Functional CorrectnessWatcher/skip-flag logic verified consistent.
The
__skipNextPromptsSubTabLoadguard is set before the reactivepromptsSubTabassignment inapplyPromptPresetToEditor(agents.mjs), so the watcher correctly no-ops for that one transition, andpersistWebUiPreferencesstill fires regardless of the skip flag. No double-load or missed-load found. Note: the unconditionalloadPromptsContent()call for non-skipped, non-presetstransitions can discard unsaved editor drafts when returning from thepresetstab — see the corresponding comment onswitchPromptsSubTabinweb-ui/modules/app.methods.agents.mjs.web-ui/modules/app.methods.web-ui-preferences.mjs (2)
69-86: 📐 Maintainability & Code Quality | 💤 Low valueSolid normalization/dedup logic.
De-dup via the
seenSet with comma-operator side effect, ID fallback generation, and field coercion all look correct. One minor note: the epoch fallback for missingupdatedAt(new Date(0).toISOString()) means callers relying on "empty string means unknown" (seeformatPromptPresetTimeinweb-ui/modules/app.methods.agents.mjs) will never actually hit that fallback path, since this always returns a non-empty string.
336-336: LGTM!Also applies to: 389-394
web-ui/modules/app.methods.agents.mjs (3)
712-754: LGTM!
755-824: LGTM!
825-870: LGTM!web-ui/partials/index/panel-prompts.html (1)
11-11: LGTM!Also applies to: 115-115
web-ui/res/web-ui-render.precompiled.js (1)
6677-6684: LGTM!Also applies to: 6722-6819, 6828-7038
web-ui/styles/modals-core.css (1)
729-777: LGTM!Also applies to: 793-802
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cli/openai-bridge.js (1)
501-512: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnreachable SSE branch — dead code.
Reaching Line 501 implies the earlier
if (streamRequested && wantsSse)guard (Line 435) was false and returned otherwise. Sinceconverted.streamRequestedis derived from the samestream === trueasstreamRequested(seeconvertResponsesRequestToChatCompletions), the conditionconverted.streamRequested && wantsSseis always false at this point, so this block never executes. Astream: truerequest without antext/event-streamAccept correctly falls through to the JSON response at Line 514.♻️ Remove the dead SSE branch
- if (converted.streamRequested && wantsSse) { - res.writeHead(200, { - 'Content-Type': 'text/event-stream; charset=utf-8', - 'Cache-Control': 'no-cache', - 'Connection': 'keep-alive', - 'X-Accel-Buffering': 'no' - }); - if (typeof res.flushHeaders === 'function') res.flushHeaders(); - sendResponsesSse(res, responsesPayload); - res.end(); - return; - } - res.writeHead(200, { 'Content-Type': 'application/json; charset=utf-8' });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/openai-bridge.js` around lines 501 - 512, Remove the unreachable SSE response branch guarded by converted.streamRequested && wantsSse, including its header setup, sendResponsesSse call, and early return. Preserve the earlier streaming guard and allow non-SSE requests to continue to the existing JSON response path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cli/openai-bridge.js`:
- Around line 501-512: Remove the unreachable SSE response branch guarded by
converted.streamRequested && wantsSse, including its header setup,
sendResponsesSse call, and early return. Preserve the earlier streaming guard
and allow non-SSE requests to continue to the existing JSON response path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 37c4dc47-b6d8-4c9d-8e04-ada087ab380c
📒 Files selected for processing (6)
cli/openai-bridge.jstests/unit/config-tabs-ui.test.mjstests/unit/openai-bridge-upstream-responses.test.mjsweb-ui/partials/index/panel-prompts.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/modals-core.css
✅ Files skipped from review due to trivial changes (1)
- web-ui/styles/modals-core.css
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/unit/config-tabs-ui.test.mjs
- web-ui/partials/index/panel-prompts.html
- web-ui/res/web-ui-render.precompiled.js
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: CodeRabbit
🔇 Additional comments (3)
cli/openai-bridge.js (2)
428-435: LGTM!
435-464: LGTM!tests/unit/openai-bridge-upstream-responses.test.mjs (1)
143-227: LGTM!Also applies to: 229-294, 296-352, 535-541, 598-657, 667-732, 1215-1273, 1553-1606
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/config-tabs-ui.test.mjs (1)
86-92: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding parity assertions for the prompts side tab.
Other side tabs verify
@pointerdown,data-main-tab,:aria-current, andisMainTabNavActive(e.g., lines 46–48 for sessions, 371–374 for config/docs, 421–422 for settings/market). The prompts tab assertions don't cover these attributes. Adding them would keep the test suite consistent and catch regressions if the tab's active-state or pointer binding is accidentally removed.💡 Optional parity assertions
assert.match(html, /id="side-tab-prompts"/); assert.doesNotMatch(html, /id="side-tab-prompts-agents"/); assert.doesNotMatch(html, /id="side-tab-prompts-project"/); assert.doesNotMatch(html, /id="side-tab-prompts-presets"/); assert.match(html, /@click="onMainTabClick\('prompts', \$event\)"/); + assert.match(html, /@pointerdown="onMainTabPointerDown\('prompts', \$event\)"/); + assert.match(html, /data-main-tab="prompts"/); + assert.match(html, /:aria-current="mainTab === 'prompts' \? 'page' : null"/); + assert.match(html, /isMainTabNavActive\('prompts'\)/); assert.match(html, /t\('side\.prompts'\)/); assert.match(html, /t\('side\.prompts\.meta'\)/);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/config-tabs-ui.test.mjs` around lines 86 - 92, Extend the prompts side-tab assertions around the existing prompts checks to cover parity with other main tabs: verify its `@pointerdown` binding, data-main-tab value, :aria-current binding, and isMainTabNavActive usage. Keep the existing prompts content and click-handler assertions unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/unit/config-tabs-ui.test.mjs`:
- Around line 86-92: Extend the prompts side-tab assertions around the existing
prompts checks to cover parity with other main tabs: verify its `@pointerdown`
binding, data-main-tab value, :aria-current binding, and isMainTabNavActive
usage. Keep the existing prompts content and click-handler assertions unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3184b58e-789f-4fa7-8516-9279ef3d7caa
📒 Files selected for processing (9)
tests/unit/config-tabs-ui.test.mjstests/unit/i18n-locales.test.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/layout-header.htmlweb-ui/res/web-ui-render.precompiled.js
✅ Files skipped from review due to trivial changes (4)
- web-ui/modules/i18n/locales/en.mjs
- web-ui/modules/i18n/locales/zh.mjs
- web-ui/modules/i18n/locales/ja.mjs
- web-ui/modules/i18n/locales/vi.mjs
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/unit/i18n-locales.test.mjs
- web-ui/modules/i18n/locales/zh-tw.mjs
- web-ui/res/web-ui-render.precompiled.js
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: CodeRabbit
🔇 Additional comments (2)
web-ui/partials/index/layout-header.html (1)
258-270: LGTM!tests/unit/config-tabs-ui.test.mjs (1)
86-97: LGTM!
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web-ui/modules/app.methods.agents.mjs (1)
834-840: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winApplying a preset silently discards unsaved editor edits.
applyPromptPresetToEditoroverwritesthis.agentsContentunconditionally, with no check againsthasAgentsContentChanged(). If the user has unsaved edits in the AGENTS.md/CLAUDE.md editor, clicking "paste" on a preset discards them with no confirmation — confirmed intentional by the test attests/unit/prompt-presets.test.mjs:146-163(agentsContent: 'dirty'vsagentsOriginalContent: 'original', assertingconfirms.length === 0). Given other destructive actions in this file (e.g.closeAgentsModal) do guard viahasPendingAgentsDraft()/requestConfirmDialog, consider adding an analogous guard here to avoid silent loss of in-progress edits, even though the current behavior matches the documented PR intent.♻️ Optional guard for unsaved edits
async applyPromptPresetToEditor(preset) { if (!preset || typeof preset.content !== 'string' || !preset.content) return; + if (this.hasAgentsContentChanged()) { + const confirmed = await this.requestConfirmDialog({ + title: this.t('prompts.presets.confirm.overwriteEditorTitle'), + message: this.t('prompts.presets.confirm.overwriteEditorMessage'), + confirmText: this.t('common.confirm'), + cancelText: this.t('common.cancel'), + danger: true + }); + if (!confirmed) return; + } this.agentsContent = preset.content; this.onAgentsContentInput(); this.selectedPromptPresetId = preset.id; this.showMessage(this.t('prompts.presets.toast.pasted'), 'success'); },Note: adopting this would require updating the existing test expectations (
confirms.lengthassertions) accordingly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/modules/app.methods.agents.mjs` around lines 834 - 840, Update applyPromptPresetToEditor to check for unsaved editor content via hasAgentsContentChanged() before overwriting agentsContent; when edits exist, request confirmation through the established requestConfirmDialog flow and abort unless confirmed. Preserve the existing preset application and success message behavior when there are no pending edits or the user confirms, and update the related test expectations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@web-ui/modules/app.methods.agents.mjs`:
- Around line 834-840: Update applyPromptPresetToEditor to check for unsaved
editor content via hasAgentsContentChanged() before overwriting agentsContent;
when edits exist, request confirmation through the established
requestConfirmDialog flow and abort unless confirmed. Preserve the existing
preset application and success message behavior when there are no pending edits
or the user confirms, and update the related test expectations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c938c425-bbc3-40ce-bb89-9c4a2335f7be
📒 Files selected for processing (11)
tests/unit/config-tabs-ui.test.mjstests/unit/i18n-locales.test.mjstests/unit/prompt-presets.test.mjsweb-ui/modules/app.methods.agents.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/panel-prompts.htmlweb-ui/res/web-ui-render.precompiled.js
✅ Files skipped from review due to trivial changes (4)
- web-ui/modules/i18n/locales/zh-tw.mjs
- web-ui/modules/i18n/locales/zh.mjs
- web-ui/modules/i18n/locales/vi.mjs
- web-ui/modules/i18n/locales/en.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/unit/i18n-locales.test.mjs
- web-ui/res/web-ui-render.precompiled.js
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: CodeRabbit
🔇 Additional comments (5)
web-ui/modules/app.methods.agents.mjs (1)
711-754: LGTM!Also applies to: 786-833, 841-907
web-ui/partials/index/panel-prompts.html (1)
11-11: LGTM!Also applies to: 36-85, 118-118
web-ui/modules/i18n/locales/ja.mjs (1)
147-147: LGTM!Also applies to: 172-186, 230-254
tests/unit/config-tabs-ui.test.mjs (1)
86-103: LGTM!tests/unit/prompt-presets.test.mjs (1)
146-163: LGTM!
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web-ui/modules/app.methods.agents.mjs (1)
834-839: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winGuard preset application while the editor is not writable.
The preset-list “Paste” action remains available during loading, saving, and diff preview. This method unconditionally replaces
agentsContent, thenonAgentsContentInput()clears the active diff. A click can therefore discard an unsaved draft or have its content overwritten by an in-flight file load. Guard this method withagentsLoading || agentsSaving || agentsDiffVisible, and mirror that guard in the row action buttons.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/modules/app.methods.agents.mjs` around lines 834 - 839, Update applyPromptPresetToEditor to return immediately when agentsLoading, agentsSaving, or agentsDiffVisible is true, before modifying agentsContent or clearing the diff. Apply the same disabled/guard condition to the preset-list row action buttons so Paste cannot be triggered while the editor is loading, saving, or showing a diff.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@web-ui/modules/app.methods.agents.mjs`:
- Around line 834-839: Update applyPromptPresetToEditor to return immediately
when agentsLoading, agentsSaving, or agentsDiffVisible is true, before modifying
agentsContent or clearing the diff. Apply the same disabled/guard condition to
the preset-list row action buttons so Paste cannot be triggered while the editor
is loading, saving, or showing a diff.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5bc2d1ba-b615-4e33-bae9-c57ad42a4829
📒 Files selected for processing (15)
tests/unit/config-tabs-ui.test.mjstests/unit/i18n-locales.test.mjstests/unit/prompt-presets.test.mjstests/unit/web-ui-behavior-parity.test.mjsweb-ui/app.jsweb-ui/modules/app.methods.agents.mjsweb-ui/modules/app.methods.web-ui-preferences.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/panel-prompts.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/modals-core.css
✅ Files skipped from review due to trivial changes (5)
- web-ui/styles/modals-core.css
- tests/unit/i18n-locales.test.mjs
- web-ui/modules/i18n/locales/en.mjs
- web-ui/modules/i18n/locales/zh.mjs
- web-ui/modules/i18n/locales/vi.mjs
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/unit/config-tabs-ui.test.mjs
- web-ui/modules/i18n/locales/zh-tw.mjs
- tests/unit/web-ui-behavior-parity.test.mjs
- tests/unit/prompt-presets.test.mjs
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: CodeRabbit
🔇 Additional comments (8)
web-ui/app.js (2)
77-82: LGTM!Also applies to: 735-745
746-751: 🎯 Functional CorrectnessDrop this concern:
__skipNextPromptsSubTabLoadis not armed inapplyWebUiPreferences, so a no-oppromptsSubTabrestore cannot leave the watcher stuck.> Likely an incorrect or invalid review comment.web-ui/modules/app.methods.web-ui-preferences.mjs (1)
63-85: LGTM!Also applies to: 336-336, 389-394
web-ui/modules/app.methods.agents.mjs (2)
711-749: LGTM!Also applies to: 895-896
750-752: 🗄️ Data Integrity & IntegrationPreset persistence is best-effort, not transactional.
persistWebUiPreferences()only queues a preference flush and swallows write errors, so these save/rename/delete flows can’t observe a rejection or roll back in-memory changes.> Likely an incorrect or invalid review comment.web-ui/partials/index/panel-prompts.html (1)
35-35: LGTM!Also applies to: 69-76, 95-132
web-ui/res/web-ui-render.precompiled.js (1)
365-380: LGTM!Also applies to: 6763-6776
web-ui/modules/i18n/locales/ja.mjs (1)
147-147: LGTM!Also applies to: 172-186, 230-255
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web-ui/styles/responsive.css (1)
623-638: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe 540px preset rules are identical to the 720px rules and thus redundant.
Since
max-width: 540pxis a subset ofmax-width: 720px, the rules at lines 623–638 are already in effect at 540px. This duplication is a maintainability risk—if the 720px rules change, the 540px copy may be forgotten and diverge silently.Note: the existing
.prompts-editor-toolbar/.prompts-editor-actionsrules follow the same pattern, so this may be an intentional convention. If so, consider adding a comment noting the duplication is deliberate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/styles/responsive.css` around lines 623 - 638, Remove the redundant 540px declarations for .prompt-presets-inline-row, .prompt-presets-inline-group, .prompt-presets-inline-group--save, .prompt-presets-select, and .prompt-presets-name-input because the max-width: 720px rules already apply; if retaining the duplication to match the existing responsive convention, add a concise comment documenting that it is intentional.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@web-ui/styles/responsive.css`:
- Around line 623-638: Remove the redundant 540px declarations for
.prompt-presets-inline-row, .prompt-presets-inline-group,
.prompt-presets-inline-group--save, .prompt-presets-select, and
.prompt-presets-name-input because the max-width: 720px rules already apply; if
retaining the duplication to match the existing responsive convention, add a
concise comment documenting that it is intentional.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 545c01c9-9dd9-41a0-8c4f-d484b6e86992
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
package.jsonweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/panel-prompts.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/modals-core.cssweb-ui/styles/responsive.css
✅ Files skipped from review due to trivial changes (3)
- package.json
- web-ui/modules/i18n/locales/vi.mjs
- web-ui/res/web-ui-render.precompiled.js
🚧 Files skipped from review as they are similar to previous changes (5)
- web-ui/styles/modals-core.css
- web-ui/partials/index/panel-prompts.html
- web-ui/modules/i18n/locales/zh-tw.mjs
- web-ui/modules/i18n/locales/en.mjs
- web-ui/modules/i18n/locales/ja.mjs
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: CodeRabbit
🔇 Additional comments (1)
web-ui/modules/i18n/locales/zh.mjs (1)
146-146: LGTM!Also applies to: 184-185, 229-234, 245-252
2 participants
Summary
/chat/completionsinstead of probing upstream/responsesfirst.Behavior notes
codexmate_bridge = "openai"is enabled, the bridge now treats builtin conversion as forced conversion: upstream/responsesis not called for Codex Responses requests; converted requests go to/chat/completions.Validation
PR status
Summary by CodeRabbit