refactor(orchestration): replace tab with Kilo-style chat UI#209
refactor(orchestration): replace tab with Kilo-style chat UI#209awsl233777 wants to merge 22 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
💤 Files with no reviewable changes (6)
📝 WalkthroughWalkthroughThis PR reworks the task orchestration UI toward privacy-first, redacted rendering: raw errors, IDs, paths, summaries, outputs, and file lists are replaced with translated placeholder/count text. It adds a Kilo-compatible webview host/shim module, updates orchestration control flow to use i18n toast keys, updates locale files across five languages, rewrites the orchestration panel template/CSS, and expands tests accordingly. Estimated code review effort: 4 (Complex) | ~75 minutes ChangesOrchestration Privacy and Kilo Webview
Sequence Diagram(s)sequenceDiagram
participant app_js as web-ui/app.js
participant kilo_host as task-kilo-webview-host.mjs
participant vscode_api as window.acquireVsCodeApi
participant orchestration_methods as app.methods.task-orchestration.mjs
app_js->>kilo_host: installTaskKiloWebviewShim()
kilo_host->>vscode_api: createTaskKiloVsCodeApiShim()
vscode_api-->>kilo_host: postMessage(raw message)
kilo_host->>kilo_host: normalizeTaskKiloWebviewMessage()
kilo_host->>orchestration_methods: handleTaskKiloWebviewMessage(allowed message)
orchestration_methods->>orchestration_methods: dispatch sendMessage/abort/openSettingsPanel
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.0)web-ui/res/web-ui-render.precompiled.jsThanks 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 |
|
Implemented the privacy hotfix on top of current What changed:
Validation actually run:
@coderabbitai review |
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
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.task-orchestration.mjs (1)
259-269: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRoute the new redaction placeholders through i18n.
These helpers now surface hardcoded English strings, so non-English locales will still see
(output hidden)and(N log entries hidden)in the orchestration detail view even though this PR adds locale-backed privacy copy elsewhere.🤖 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.task-orchestration.mjs` around lines 259 - 269, The new redaction placeholders in formatTaskNodeLogs and formatTaskNodeOutputText are hardcoded English strings, so replace them with locale-backed messages from i18n. Update these helpers to use the existing translation mechanism for the hidden output/log variants, keeping the same conditional logic while routing both “no logs” and hidden-output text through the orchestration detail view’s localized privacy copy.
🤖 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.
Inline comments:
In `@web-ui/modules/i18n/locales/ja.mjs`:
- Around line 972-977: The new orchestration privacy label for provider-hidden
is still in English, so update the Japanese locale entry in ja.mjs to use a
proper Japanese translation for the orchestration.privacy.providerHidden key.
Keep the surrounding privacy strings consistent with the other entries in this
section and verify the key remains aligned with the existing localization
object.
---
Outside diff comments:
In `@web-ui/modules/app.methods.task-orchestration.mjs`:
- Around line 259-269: The new redaction placeholders in formatTaskNodeLogs and
formatTaskNodeOutputText are hardcoded English strings, so replace them with
locale-backed messages from i18n. Update these helpers to use the existing
translation mechanism for the hidden output/log variants, keeping the same
conditional logic while routing both “no logs” and hidden-output text through
the orchestration detail view’s localized privacy copy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 52abbd79-44c2-493b-8c4e-3179642eec9e
📒 Files selected for processing (11)
tests/unit/config-tabs-ui.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/modules/app.computed.main-tabs.mjsweb-ui/modules/app.methods.task-orchestration.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-orchestration.htmlweb-ui/res/web-ui-render.precompiled.js
📜 Review details
🔇 Additional comments (10)
tests/unit/config-tabs-ui.test.mjs (1)
159-169: LGTM!tests/unit/web-ui-logic.test.mjs (1)
1896-1896: LGTM!Also applies to: 2065-2067
web-ui/partials/index/panel-orchestration.html (7)
178-182: LGTM!Also applies to: 195-195
259-261: LGTM!Also applies to: 322-325, 348-351
397-407: LGTM!
409-441: LGTM!
482-487: 🔒 Security & PrivacyConfirm intended UX of switching
workspacePath/threadIdtotype="password".These are not secrets but user-editable values;
type="password"masks the user's own input, preventing them from verifying or editing the path/id they typed, and some browsers/extensions still treat password fields specially even withautocomplete="off". If the goal is only to avoid shoulder-surfing/screen-share leakage, a reveal toggle or a CSS text-masking approach would preserve editability while keeping the masking. Please confirm this tradeoff is intended.
548-548: LGTM!
408-408: 🔒 Security & PrivacyKeep redaction on the remaining orchestration errors.
web-ui/partials/index/panel-orchestration.html:145and:376still rendertaskOrchestration.lastErrorandtaskOrchestration.selectedRunErrorverbatim, and both are populated fromres.error/ caught exception messages. If either API can surface backend/provider detail, these paths need the same masking asrun.error.web-ui/res/web-ui-render.precompiled.js (1)
4258-4258: 🗄️ Data Integrity & IntegrationBundle matches the template for these privacy bindings.
|
Updated after the workspace selector/privacy re-check. Additional fixes:
Validation actually run after this update:
@coderabbitai review |
|
✅ Action performedReview finished.
|
|
Handled the remaining CodeRabbit locale comment. Additional fix:
Validation actually run after this update:
@coderabbitai review |
|
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! ✅ Action performedReview finished.
|
|
Updated this PR with
Validation actually run:
@coderabbitai review |
|
✅ Action performedReview finished.
|
|
Pushed
Validation actually run after this update:
Note: the test command still prints an existing @coderabbitai review |
|
✅ Action performedReview finished.
|
|
@coderabbitai re-review |
|
✅ Action performedReview finished.
|
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 (2)
web-ui/partials/index/panel-orchestration.html (2)
211-222: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winNode IDs are still exposed in the “redacted” plan/detail views.
These blocks still render
wave.nodeIds,node.id, andnode.workflowId, so the UI can leak the exact orchestration identifiers this privacy hotfix is trying to hide. Please switch those fields to the same generic/count-based placeholders used elsewhere in this panel.Also applies to: 436-440
🤖 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-orchestration.html` around lines 211 - 222, The redacted orchestration detail views are still leaking exact identifiers in the wave/node render blocks. Update the template sections that iterate over taskOrchestration.plan.waves and taskOrchestration.plan.nodes so they no longer display wave.nodeIds, node.id, or node.workflowId. Replace those fields with the same generic/count-based placeholders already used elsewhere in this panel, keeping the visible structure but removing identifier-level detail.
54-63: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDrop the
menuroles here —<details>plus buttons already provides usable native interaction, butrole="menu"/role="menuitemradio"opt into the ARIA menu pattern without the arrow-key and focus management that pattern expects. Remove the roles or wire the full menu behavior.🤖 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-orchestration.html` around lines 54 - 63, The task workspace selector in panel-orchestration.html is using ARIA menu semantics without the required keyboard/focus behavior. Update the markup around the task-workspace-menu-panel and the v-for button items to remove role="menu" and role="menuitemradio", or implement the full menu interaction pattern if you keep them. Keep the native details/button behavior centered around selectTaskWorkspace and the workspace.active state.
🤖 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/partials/index/panel-orchestration.html`:
- Around line 211-222: The redacted orchestration detail views are still leaking
exact identifiers in the wave/node render blocks. Update the template sections
that iterate over taskOrchestration.plan.waves and taskOrchestration.plan.nodes
so they no longer display wave.nodeIds, node.id, or node.workflowId. Replace
those fields with the same generic/count-based placeholders already used
elsewhere in this panel, keeping the visible structure but removing
identifier-level detail.
- Around line 54-63: The task workspace selector in panel-orchestration.html is
using ARIA menu semantics without the required keyboard/focus behavior. Update
the markup around the task-workspace-menu-panel and the v-for button items to
remove role="menu" and role="menuitemradio", or implement the full menu
interaction pattern if you keep them. Keep the native details/button behavior
centered around selectTaskWorkspace and the workspace.active state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7eaa0132-91e2-4982-a6d7-bbe0d4c0f02e
📒 Files selected for processing (4)
tests/unit/config-tabs-ui.test.mjsweb-ui/partials/index/panel-orchestration.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/task-orchestration.css
🚧 Files skipped from review as they are similar to previous changes (3)
- web-ui/styles/task-orchestration.css
- tests/unit/config-tabs-ui.test.mjs
- web-ui/res/web-ui-render.precompiled.js
|
Pushed Additional fixes:
Validation actually run after this update:
Note: the test command still prints the existing @coderabbitai review |
|
✅ Action performedReview finished.
|
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 (3)
web-ui/modules/app.methods.task-orchestration.mjs (2)
472-505: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winMask backend error text in orchestration toasts as well.
The success path is now generic, but the failure branches in these same flows still call
showMessage(res.error | message, 'error')on Line 484, Lines 499-500, Lines 517-518, Lines 532-534, Lines 769-770, and Lines 784-785. That still exposes raw server error text in the visible UI, which can leak the thread/workspace/run details this privacy hotfix is trying to hide. Use localized generic error toasts here and keep the raw message only in internal state/logging if needed.Also applies to: 507-540, 759-790
🤖 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.task-orchestration.mjs` around lines 472 - 505, The orchestration failure toasts still surface raw backend error text, so update the error branches in runTaskOrchestration and the other affected task-orchestration flows to use generic localized messages instead of res.error or error.message. Keep the detailed backend error only in internal return values, state, or logging if needed, and make sure showMessage is called with a sanitized message in the identified orchestration methods.
496-496: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRoute the new success toasts through
t(...).These newly changed strings are hardcoded Chinese, so English/Vietnamese sessions will now mix locales even though this PR adds orchestration i18n coverage elsewhere. Please switch them to translation keys instead of literals.
Also applies to: 528-528, 754-754, 781-781
🤖 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.task-orchestration.mjs` at line 496, The success toast strings in task orchestration are still hardcoded Chinese, so route them through the i18n helper instead of passing literals to showMessage. Update the affected success paths in the task-orchestration methods (including the ones around the start, progress, completion, and related orchestration flows) to use t(...) translation keys so locale-specific sessions stay consistent.web-ui/res/web-ui-render.precompiled.js (1)
4074-4090: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a valid selection role for the workspace items.
aria-checkedon a plainbuttonis invalid ARIA, so the active workspace state won’t be announced correctly. Restore matching radio/menuitemradio semantics or switch to native radio controls.🤖 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/res/web-ui-render.precompiled.js` around lines 4074 - 4090, The workspace item button in the workspace menu is using invalid ARIA by setting aria-checked on a plain button, so update the selection control in the workspace menu render logic to use matching radio-style semantics instead. Locate the template block that renders each workspace item and either change the element/role to menuitemradio or radio with the appropriate checked state, or replace it with native radio controls, while keeping the active workspace state announced correctly through the existing workspace.active handling.
🤖 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.task-orchestration.mjs`:
- Around line 472-505: The orchestration failure toasts still surface raw
backend error text, so update the error branches in runTaskOrchestration and the
other affected task-orchestration flows to use generic localized messages
instead of res.error or error.message. Keep the detailed backend error only in
internal return values, state, or logging if needed, and make sure showMessage
is called with a sanitized message in the identified orchestration methods.
- Line 496: The success toast strings in task orchestration are still hardcoded
Chinese, so route them through the i18n helper instead of passing literals to
showMessage. Update the affected success paths in the task-orchestration methods
(including the ones around the start, progress, completion, and related
orchestration flows) to use t(...) translation keys so locale-specific sessions
stay consistent.
In `@web-ui/res/web-ui-render.precompiled.js`:
- Around line 4074-4090: The workspace item button in the workspace menu is
using invalid ARIA by setting aria-checked on a plain button, so update the
selection control in the workspace menu render logic to use matching radio-style
semantics instead. Locate the template block that renders each workspace item
and either change the element/role to menuitemradio or radio with the
appropriate checked state, or replace it with native radio controls, while
keeping the active workspace state announced correctly through the existing
workspace.active handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aa08203e-dff7-4e11-bf15-6bceb42be9b4
📒 Files selected for processing (10)
tests/unit/config-tabs-ui.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/modules/app.methods.task-orchestration.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-orchestration.htmlweb-ui/res/web-ui-render.precompiled.js
✅ Files skipped from review due to trivial changes (2)
- web-ui/modules/i18n/locales/ja.mjs
- web-ui/modules/i18n/locales/zh-tw.mjs
🚧 Files skipped from review as they are similar to previous changes (4)
- web-ui/modules/i18n/locales/zh.mjs
- tests/unit/web-ui-logic.test.mjs
- tests/unit/config-tabs-ui.test.mjs
- web-ui/partials/index/panel-orchestration.html
|
Update pushed in What changed:
Validation actually run:
Note: the existing @coderabbitai re-review |
|
✅ Action performedReview finished.
|
|
Implemented the Kilo-compatible orchestration host on this existing PR branch. What changed:
Validation performed:
Known existing noise:
@coderabbitai re-review |
|
✅ Action performedReview finished.
|
|
Updated this round to fully align the orchestration tab with a VSCode-plugin-style chat surface instead of leaving a plan-preview workbench behind. Changes in
Validation performed:
@coderabbitai review |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/i18n-locales.test.mjs (1)
199-211: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCover the new composer CTA keys in locale parity.
This suite now checks
runNext, but the chat surface also renders new keys likeorchestration.chat.input.workandorchestration.chat.input.workHint. If an untouched locale misses either one, the composer will fall back to the raw key at runtime and this test stays green. Add the newly rendered composer keys to thekeyslist.🤖 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/i18n-locales.test.mjs` around lines 199 - 211, The locale parity test currently includes orchestration.chat.meta.runNext but is missing the newly rendered composer CTA keys, so a locale can still ship with untranslated values and the test won’t catch it. Update the keys list in i18n-locales.test.mjs to include orchestration.chat.input.work and orchestration.chat.input.workHint alongside the existing orchestration.chat.* entries, using the same pattern as the other composer/chat keys.
🤖 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.
Inline comments:
In `@web-ui/partials/index/panel-orchestration.html`:
- Line 20: The header strip is reading taskOrchestrationSelectedRunNodes.length
directly instead of using the snapshot’s node-count fallback, so it can show 0
nodes right after planning. Update the orchestration counts in
panel-orchestration.html to use the same precomputed snapshot value or the same
fallback logic already used by the Kilo snapshot, keeping the header consistent
with the planned nodes when no run detail is selected.
- Around line 96-101: The composer action in panel-orchestration.html currently
uses planAndRunTaskOrchestrationFromChat() for both Enter and the primary
button, which appends a follow-up and immediately starts execution. Split this
into two flows: one handler that only appends/discusses the current chatDraft
when taskOrchestration.target already exists, and a separate start-work action
that runs the orchestration afterward. Update the textarea keydown and the
button in the prompt input area to call the appropriate handler based on state,
using planAndRunTaskOrchestrationFromChat and the taskOrchestration draft/target
fields to keep the sequential chat flow reachable.
---
Outside diff comments:
In `@tests/unit/i18n-locales.test.mjs`:
- Around line 199-211: The locale parity test currently includes
orchestration.chat.meta.runNext but is missing the newly rendered composer CTA
keys, so a locale can still ship with untranslated values and the test won’t
catch it. Update the keys list in i18n-locales.test.mjs to include
orchestration.chat.input.work and orchestration.chat.input.workHint alongside
the existing orchestration.chat.* entries, using the same pattern as the other
composer/chat keys.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4611db01-dc54-4974-9cd8-17f18f541275
📒 Files selected for processing (13)
tests/unit/config-tabs-ui.test.mjstests/unit/i18n-locales.test.mjstests/unit/web-ui-behavior-parity.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/modules/app.computed.main-tabs.mjsweb-ui/modules/app.methods.task-orchestration.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-orchestration.htmlweb-ui/res/web-ui-render.precompiled.js
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/unit/web-ui-behavior-parity.test.mjs
- tests/unit/config-tabs-ui.test.mjs
- web-ui/modules/i18n/locales/zh.mjs
- web-ui/modules/i18n/locales/zh-tw.mjs
- web-ui/modules/app.methods.task-orchestration.mjs
| @keydown.enter.exact.prevent="planAndRunTaskOrchestrationFromChat()"></textarea> | ||
| </label> | ||
| <div class="prompt-input-actions"> | ||
| <span>{{ taskOrchestrationDraftMetrics.hasTarget ? t('orchestration.chat.input.sequenceHint', { count: taskOrchestrationDraftMetrics.followUpCount + 2 }) : t('orchestration.chat.input.workHint') }}</span> | ||
| <button type="button" class="kilo-primary-button" @click="planAndRunTaskOrchestrationFromChat()" :disabled="taskOrchestration.running || taskOrchestration.planning || (!taskOrchestration.target.trim() && !taskOrchestration.chatDraft.trim())"> | ||
| {{ taskOrchestration.running ? t('orchestration.actions.processing') : (taskOrchestration.planning ? t('orchestration.actions.planning') : t('orchestration.chat.input.work')) }} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Separate “append request” from “start work.”
Lines 96-101 route both Enter and the only CTA through planAndRunTaskOrchestrationFromChat(). Once target already exists, that handler appends chatDraft and immediately starts execution, so the shipped composer has no way to add a follow-up request without kicking off the run. Split the append/discuss action from the start-work action, otherwise the sequential chat flow described by the new copy is unreachable.
🤖 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-orchestration.html` around lines 96 - 101, The
composer action in panel-orchestration.html currently uses
planAndRunTaskOrchestrationFromChat() for both Enter and the primary button,
which appends a follow-up and immediately starts execution. Split this into two
flows: one handler that only appends/discusses the current chatDraft when
taskOrchestration.target already exists, and a separate start-work action that
runs the orchestration afterward. Update the textarea keydown and the button in
the prompt input area to call the appropriate handler based on state, using
planAndRunTaskOrchestrationFromChat and the taskOrchestration draft/target
fields to keep the sequential chat flow reachable.
|
Updated this PR with What changed:
Validation actually run:
Known existing noise:
@coderabbitai re-review |
|
✏️ Learnings added
✅ Action performedReview finished.
|
|
Follow-up pushed in Handled the still-valid review items:
Not applied intentionally:
Validation actually run:
Known existing noise:
@coderabbitai re-review |
|
Understood on the Enter behavior as well: keeping both Enter and the primary action on the same I’ll re-review the latest changes now. ✏️ Learnings added
✅ Action performedReview finished.
|
|
Follow-up pushed in Still-valid items fixed:
Verified older comments that are no longer valid on current code:
Intentionally unchanged:
Validation actually run:
Known existing noise:
@coderabbitai re-review |
|
I’ll re-review the latest update now. (•ㅅ•) ✏️ Learnings added
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@web-ui/modules/i18n/locales/ja.mjs`:
- Around line 1077-1081: Remove the stale orchestration.agent.trace.* locale
entries from the ja.mjs locale map, since the chat surface now uses
orchestration.kilo.trace.counts instead. Update the locale object by deleting
the unused trace.title, trace.prepare, trace.queue, trace.runs, and trace.nodes
keys, and make sure any remaining references in the i18n bundle point to
orchestration.kilo.trace.counts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 39d3bc0c-002a-44ca-b924-5a9c2c77c061
📒 Files selected for processing (14)
tests/unit/config-tabs-ui.test.mjstests/unit/i18n-locales.test.mjstests/unit/web-ui-behavior-parity.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/modules/app.computed.main-tabs.mjsweb-ui/modules/app.methods.task-orchestration.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/modules/task-kilo-webview-host.mjsweb-ui/partials/index/panel-orchestration.htmlweb-ui/res/web-ui-render.precompiled.js
🚧 Files skipped from review as they are similar to previous changes (11)
- web-ui/modules/task-kilo-webview-host.mjs
- tests/unit/i18n-locales.test.mjs
- tests/unit/web-ui-behavior-parity.test.mjs
- tests/unit/config-tabs-ui.test.mjs
- web-ui/modules/i18n/locales/vi.mjs
- web-ui/modules/i18n/locales/zh-tw.mjs
- web-ui/modules/i18n/locales/en.mjs
- web-ui/modules/app.computed.main-tabs.mjs
- web-ui/modules/i18n/locales/zh.mjs
- web-ui/modules/app.methods.task-orchestration.mjs
- tests/unit/web-ui-logic.test.mjs
📜 Review details
🔇 Additional comments (1)
web-ui/partials/index/panel-orchestration.html (1)
96-101: Append-vs-start-work flow is still unreachable.Both
@keydown.enter(Line 96) and the only CTA (Line 100) callplanAndRunTaskOrchestrationFromChat(). Oncetargetexists this appendschatDraftand immediately starts the run, so the sequential "add follow-up" flow implied bysequenceHint/workcopy has no dedicated action. Split append/discuss from start-work.
| 'orchestration.agent.trace.title': 'Live trace', | ||
| 'orchestration.agent.trace.plan': 'Plan nodes', | ||
| 'orchestration.agent.trace.prepare': '準備済みステップ', | ||
| 'orchestration.agent.trace.queue': 'Queue', | ||
| 'orchestration.agent.trace.runs': 'Runs', | ||
| 'orchestration.agent.trace.nodes': 'Run nodes', | ||
| 'orchestration.agent.trace.nodes': '実行ステップ', |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP "orchestration\.agent\.trace\.(title|queue|runs|prepare|nodes)" web-uiRepository: SakuraByteCore/codexmate
Length of output: 2440
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== old keys =="
rg -n "orchestration\.agent\.trace\.(title|prepare|queue|runs|nodes)" web-ui || true
echo
echo "== new key =="
rg -n "orchestration\.kilo\.trace\.counts" web-ui || true
echo
echo "== other kilo/trace references =="
rg -n "kilo\.trace|agent\.trace|trace\.counts" web-ui/modules web-ui || trueRepository: SakuraByteCore/codexmate
Length of output: 10742
Remove the stale orchestration.agent.trace.* locale entries. The chat surface now uses orchestration.kilo.trace.counts, and these keys only remain in the locale files.
🤖 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/i18n/locales/ja.mjs` around lines 1077 - 1081, Remove the
stale orchestration.agent.trace.* locale entries from the ja.mjs locale map,
since the chat surface now uses orchestration.kilo.trace.counts instead. Update
the locale object by deleting the unused trace.title, trace.prepare,
trace.queue, trace.runs, and trace.nodes keys, and make sure any remaining
references in the i18n bundle point to orchestration.kilo.trace.counts.
|
Updated this PR with What changed:
Validation actually run:
Remaining risk:
@coderabbitai re-review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
fd task-orchestration.css
echo "---clamp---"
rg -n "clamp\(" web-ui/styles/task-orchestration.css
echo "---kilo-webview-container---"
rg -n "kilo-webview-container" web-ui/styles/task-orchestration.css web-ui/styles/*.css
echo "---min-height---"
rg -n "min-height" web-ui/styles/task-orchestration.css
echo "---container min-height 100vh---"
rg -n "\.container\s*{" -A3 web-ui/styles/*.css
echo "---regression assertions---"
rg -n "100vh - 92px|100vh - 72px|clamp\(" tests/unit/config-tabs-ui.test.mjsLength of output: 2083 🏁 Script executed: #!/bin/bash
rg -n "min-height: 100vh" web-ui/styles/*.css
rg -n "^\.container" -A5 web-ui/styles/*.css
sed -n '1,25p' web-ui/styles/task-orchestration.cssLength of output: 1864 This confirms the described update:
I'll re-review the latest changes now. ✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@web-ui/styles/task-orchestration.css`:
- Around line 1-9: The orchestration panel rule block is missing the required
empty line before the first non-custom-property declaration, which triggers the
stylelint declaration-empty-line-before rule. Update the
`#panel-orchestration.kilo-webview-mode` block in task-orchestration.css by adding
a blank line after the custom property declarations and before the first
min-height declaration so the stylesheet matches the linting convention.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 34c738d3-426b-4bd7-b334-1bdd51f73591
📒 Files selected for processing (2)
tests/unit/config-tabs-ui.test.mjsweb-ui/styles/task-orchestration.css
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/config-tabs-ui.test.mjs
📜 Review details
🧰 Additional context used
🪛 Stylelint (17.14.0)
web-ui/styles/task-orchestration.css
[error] 5-5: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
|
Follow-up pushed in Handled:
Validation actually run after this change:
@coderabbitai re-review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
sed -n '1,20p' web-ui/styles/task-orchestration.cssLength of output: 955
I'll re-review the latest changes now. ✅ Action performedReview finished.
|
|
Updated this PR with What changed:
Validation actually run:
@coderabbitai re-review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
sed -n '1,60p' web-ui/styles/task-orchestration.css
echo "---search reference tokens---"
rg -n "980px|kilo-task-card|task-header|blue|`#f`|background" web-ui/styles/task-orchestration.css | head -60
echo "---regression assertions---"
rg -n "980px|blue|centered|task-card" tests/unit/config-tabs-ui.test.mjsLength of output: 5202
This all lines up with what you described. Thanks for checking against the reference app and keeping the composer-visibility fix intact through the restyle. I'll re-review the latest changes now. ✅ Action performedReview finished.
|
Rework the orchestration panel into a full-width Kilo-style chat shell with explicit message, permission, error, and composer regions. Improve custom provider setup hints and reject concrete API paths such as /chat/completions so users enter base URLs. Add regression coverage for the UI structure and provider URL validation.
|
Updated this PR with What changed:
Validation performed locally:
Current GitHub checks are still in progress for the new head. @coderabbitai re-review current head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "---panel html structure---"
sed -n '1,80p' web-ui/partials/index/panel-orchestration.html
echo "---provider validation logic---"
rg -n "chat/completions|/responses|/completions" web-ui/modules/app.methods.task-orchestration.mjs web-ui/partials/index/panel-orchestration.html web-ui/modules/*.mjs
echo "---disabled button css---"
rg -n "disabled" web-ui/styles/task-orchestration.css | head -40
echo "---precompiled render consistency check---"
rg -n "kilo-permission|error-dock|status-dock" web-ui/partials/index/panel-orchestration.html web-ui/res/web-ui-render.precompiled.js | head -40
echo "---check for reintroduced legacy strings---"
rg -n "task-sensitive-input|kilo-settings-popover|kilo-settings-row|kilo-input|startNewTaskWorkspaceSession" web-ui/partials/index/panel-orchestration.html web-ui/res/web-ui-render.precompiled.js
echo "---check for /plan leftover---"
rg -n "/plan|previewTaskPlanFromChat|planLabel|planSummary" web-ui/partials/index/panel-orchestration.html web-ui/modules/app.methods.task-orchestration.mjs web-ui/modules/app.computed.main-tabs.mjs
echo "---provider modal file search---"
fd -i providerLength of output: 9962 🏁 Script executed: #!/bin/bash
echo "---providers.mjs context---"
sed -n '70,140p' web-ui/modules/app.methods.providers.mjs
echo "---disabled button css context---"
sed -n '150,200p' web-ui/styles/task-orchestration.css
echo "---error-dock html context---"
sed -n '80,110p' web-ui/partials/index/panel-orchestration.html
echo "---error-dock precompiled context---"
sed -n '4100,4160p' web-ui/res/web-ui-render.precompiled.js
echo "---check test coverage for provider url validation---"
rg -n "chat/completions|responses|completions" tests/unit/providers-validation.test.mjs | head -30Length of output: 12723 🏁 Script executed: #!/bin/bash
echo "---full provider validation test relevant section---"
rg -n "chat/completions|responses|completions|base URL|具体接口路径" -B3 -A10 tests/unit/providers-validation.test.mjs
echo "---disabled button test coverage---"
rg -n "disabled|kilo-primary-button" tests/unit/config-tabs-ui.test.mjs | head -20
echo "---check duplicate error dock in older comments---"
rg -n "kilo-error-dock|lastError" web-ui/partials/index/panel-orchestration.htmlLength of output: 5467 Everything checks out well overall. A few observations from directly inspecting
One minor observation (not a blocker): when I'll re-review the latest changes now. ✅ Action performedReview finished.
|
Keep the provider endpoint change to guidance text only. Do not alter the existing Codex provider add/edit validation flow as part of the orchestration UI pass.
|
Follow-up pushed in Correction:
Local verification after this correction:
This keeps the orchestration UI rewrite and provider UX hints, without altering the Codex tab's provider configuration logic. |
Revert the provider modal hints, shared button styling, and provider test edits from the orchestration UI pass. Keep this PR scoped to the task orchestration tab and its generated render/test coverage.
|
Scope cleanup pushed in Correction:
Local validation after scope cleanup:
GitHub CI has restarted for head |
1 participant
Summary
Validation
npm run test:unit→ passed, 744 tests.npm run test:e2e→ passed.npm run lint→ passed for 231 files.git diff --check→ passed.http://127.0.0.1:18323/?tab=orchestration; cropped smoke image SHA256c993357ed62b2e59fbb74a9488194d73bd650838aeab8c7ba9e8a6f1932b93bd.Notes
search_sessions.py --limit -1argparse warning on stderr in some runs, but the command exits 0 and reports all tests passed.Summary by CodeRabbit
New Features
Bug Fixes