feat(studio): make source configuration usable and deduplicated - #51
Merged
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
✅ Health: 8.9 📋 At a glance 🚨 Change risk: 9.3/10 (high)
📊 Full report · ⭐ Star Repowise · 📥 Install bot · Last updated 2026-07-28 03:41 UTC |
📝 WalkthroughSummary by CodeRabbit
WalkthroughOpenCLI now supports site-directory navigation, selected-site command browsing, grouped source presets, undoable removal, expanded adapter catalogs, positional-argument-aware source identity, scoped market updates, stricter catalog decoding, and updated workflow and regression coverage. ChangesOpenCLI catalog navigation
OpenCLI source management
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CommandPalette
participant OpenCLICatalog
User->>CommandPalette: Open tools view
CommandPalette->>OpenCLICatalog: Group sites and categories
OpenCLICatalog-->>CommandPalette: Return directory entries
User->>CommandPalette: Select a site
CommandPalette->>OpenCLICatalog: Read site commands
OpenCLICatalog-->>CommandPalette: Return grouped commands
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@frontend/components/flow/inspector.tsx`:
- Around line 128-133: Update sourceCardLabel so English output preserves
customized source.label values for built-in sources; derive the ID-based label
only when source.label still matches that source’s preset default. Keep the
existing Chinese behavior and non-built-in label handling unchanged, using the
established built-in source defaults for comparison.
- Around line 1959-1985: Update addContentSources and restoreSource to compare
sources by immutable source.id rather than sourceSlotKey(), so preset additions
and undo cannot introduce duplicate source IDs after mutable args change. Keep
the existing ordering, removal, and restoration behavior unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 87d725c1-fb8a-4f10-8b86-78632808249e
📒 Files selected for processing (6)
frontend/components/flow/command-palette.tsxfrontend/components/flow/inspector.tsxfrontend/lib/workflow/opencli-business-workflows.tsfrontend/lib/workflow/source-business-config.tsfrontend/lib/workflow/studio-templates.tsfrontend/scripts/check-node-capability-catalog-regressions.mjs
Comment on lines
+128
to
+133
| function sourceCardLabel(source: OpenCLISourceSlot, language: WorkflowLanguage): string { | ||
| if (language === "zh-CN" || !BUILT_IN_SOURCE_IDS.has(source.id)) return source.label | ||
| return source.id | ||
| .split("-") | ||
| .map((part) => SOURCE_ID_ACRONYMS[part] ?? `${part.charAt(0).toUpperCase()}${part.slice(1)}`) | ||
| .join(" ") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve customized built-in source names in English.
The advanced editor permits changing source.label, but this function always replaces built-in labels with an ID-derived value in English. Only derive an English default when the label still matches the preset default.
🤖 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 `@frontend/components/flow/inspector.tsx` around lines 128 - 133, Update
sourceCardLabel so English output preserves customized source.label values for
built-in sources; derive the ID-based label only when source.label still matches
that source’s preset default. Keep the existing Chinese behavior and
non-built-in label handling unchanged, using the established built-in source
defaults for comparison.
Comment on lines
+1959
to
+1985
| const addContentSources = (contentType: string | null) => { | ||
| if (!contentType) return | ||
| const presets = contentType === "video" | ||
| ? OPENCLI_SITUATION_SOURCES.filter((source) => source.sourceGroup?.startsWith("video-")) | ||
| : ASHARE_OPENCLI_SOURCES.filter((source) => source.sourceGroup === contentType) | ||
| const selectedKeys = new Set(sources.map(sourceSlotKey)) | ||
| const additions = presets.filter((source) => !selectedKeys.has(sourceSlotKey(source))) | ||
| if (additions.length > 0) onChange([...sources, ...additions]) | ||
| } | ||
|
|
||
| const removeSource = (index: number) => { | ||
| setRemovedSource({ source: sources[index], index }) | ||
| onChange(sources.filter((_, sourceIndex) => sourceIndex !== index)) | ||
| } | ||
|
|
||
| const restoreSource = () => { | ||
| if (!removedSource) return | ||
| const removedKey = sourceSlotKey(removedSource.source) | ||
| if (sources.some((source) => sourceSlotKey(source) === removedKey)) { | ||
| setRemovedSource(null) | ||
| return | ||
| } | ||
| const restored = [...sources] | ||
| restored.splice(Math.min(removedSource.index, restored.length), 0, removedSource.source) | ||
| onChange(restored) | ||
| setRemovedSource(null) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Deduplicate preset additions and undo by immutable source ID.
sourceSlotKey() includes mutable args. After changing a preset’s market, re-adding its group treats the original preset as new; undo can likewise restore a second slot with the same id. This duplicates collection and produces duplicate React card keys. Compare source.id for these preset/undo operations.
Proposed fix
- const selectedKeys = new Set(sources.map(sourceSlotKey))
- const additions = presets.filter((source) => !selectedKeys.has(sourceSlotKey(source)))
+ const selectedIds = new Set(sources.map((source) => source.id))
+ const additions = presets.filter((source) => !selectedIds.has(source.id))
@@
- const removedKey = sourceSlotKey(removedSource.source)
- if (sources.some((source) => sourceSlotKey(source) === removedKey)) {
+ if (sources.some((source) => source.id === removedSource.source.id)) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const addContentSources = (contentType: string | null) => { | |
| if (!contentType) return | |
| const presets = contentType === "video" | |
| ? OPENCLI_SITUATION_SOURCES.filter((source) => source.sourceGroup?.startsWith("video-")) | |
| : ASHARE_OPENCLI_SOURCES.filter((source) => source.sourceGroup === contentType) | |
| const selectedKeys = new Set(sources.map(sourceSlotKey)) | |
| const additions = presets.filter((source) => !selectedKeys.has(sourceSlotKey(source))) | |
| if (additions.length > 0) onChange([...sources, ...additions]) | |
| } | |
| const removeSource = (index: number) => { | |
| setRemovedSource({ source: sources[index], index }) | |
| onChange(sources.filter((_, sourceIndex) => sourceIndex !== index)) | |
| } | |
| const restoreSource = () => { | |
| if (!removedSource) return | |
| const removedKey = sourceSlotKey(removedSource.source) | |
| if (sources.some((source) => sourceSlotKey(source) === removedKey)) { | |
| setRemovedSource(null) | |
| return | |
| } | |
| const restored = [...sources] | |
| restored.splice(Math.min(removedSource.index, restored.length), 0, removedSource.source) | |
| onChange(restored) | |
| setRemovedSource(null) | |
| } | |
| const addContentSources = (contentType: string | null) => { | |
| if (!contentType) return | |
| const presets = contentType === "video" | |
| ? OPENCLI_SITUATION_SOURCES.filter((source) => source.sourceGroup?.startsWith("video-")) | |
| : ASHARE_OPENCLI_SOURCES.filter((source) => source.sourceGroup === contentType) | |
| const selectedIds = new Set(sources.map((source) => source.id)) | |
| const additions = presets.filter((source) => !selectedIds.has(source.id)) | |
| if (additions.length > 0) onChange([...sources, ...additions]) | |
| } | |
| const removeSource = (index: number) => { | |
| setRemovedSource({ source: sources[index], index }) | |
| onChange(sources.filter((_, sourceIndex) => sourceIndex !== index)) | |
| } | |
| const restoreSource = () => { | |
| if (!removedSource) return | |
| if (sources.some((source) => source.id === removedSource.source.id)) { | |
| setRemovedSource(null) | |
| return | |
| } | |
| const restored = [...sources] | |
| restored.splice(Math.min(removedSource.index, restored.length), 0, removedSource.source) | |
| onChange(restored) | |
| setRemovedSource(null) | |
| } |
🤖 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 `@frontend/components/flow/inspector.tsx` around lines 1959 - 1985, Update
addContentSources and restoreSource to compare sources by immutable source.id
rather than sourceSlotKey(), so preset additions and undo cannot introduce
duplicate source IDs after mutable args change. Keep the existing ordering,
removal, and restoration behavior unchanged.
2233admin
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/components/flow/command-palette.tsx (1)
1040-1134: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftThree near-identical group-rendering blocks (featured / selected-site / more-presets).
Each of these sections repeats the same
<section><SectionLabel count={...}>…</SectionLabel><div className="grid gap-2 lg:grid-cols-2">{items.map(item => <OpenCLIPickerRow .../>)}</div></section>shape, differing only in header/label logic. Extracting a small local helper (e.g.renderOpenCLIGroup(key, label, items)) would remove the duplication and reduce the risk of the three blocks drifting apart on future edits.♻️ Sketch of a shared helper
function renderOpenCLIGroup( key: string, label: string, items: WorkflowOpenCLIAdapterNode[], language: WorkflowLanguage, onSelect: (item: WorkflowOpenCLIAdapterNode) => void, ) { return ( <section key={key}> <SectionLabel count={items.length}>{label}</SectionLabel> <div className="grid gap-2 lg:grid-cols-2"> {items.map((item) => ( <OpenCLIPickerRow key={item.id} item={item} language={language} onClick={() => onSelect(item)} /> ))} </div> </section> ) }🤖 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 `@frontend/components/flow/command-palette.tsx` around lines 1040 - 1134, Extract the repeated OpenCLI group markup into a shared local helper near the command-palette rendering logic, such as renderOpenCLIGroup, accepting the group key, label, items, language, and selection callback. Replace the featured, selected-site, and more-presets group-rendering blocks with this helper while preserving each block’s existing label logic, keys, item ordering, and addOpenCLIAdapter behavior.
🤖 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 `@frontend/lib/plugins/opencli-adapter-catalog.ts`:
- Around line 475-483: Update openCLIKeyboardCandidates to match the
featured-first ordering used by matchingOpenCLINodes: when no selectedSite is
provided, derive candidates from featuredOpenCLIAdapterNodes(matchingNodes),
then append remaining matching nodes while deduplicating them. Preserve the
selectedSite?.commands result and the empty-query/no-selection behavior.
In `@tests/integration/test_workflow_capabilities_api.py`:
- Around line 661-663: Update the test after refresh_opencli_adapter_catalog()
to call the catalog-listing operation again and assert that it still returns an
empty response for the invalid catalog. Ensure the second refresh result is
observed rather than ending immediately after cache refresh.
---
Nitpick comments:
In `@frontend/components/flow/command-palette.tsx`:
- Around line 1040-1134: Extract the repeated OpenCLI group markup into a shared
local helper near the command-palette rendering logic, such as
renderOpenCLIGroup, accepting the group key, label, items, language, and
selection callback. Replace the featured, selected-site, and more-presets
group-rendering blocks with this helper while preserving each block’s existing
label logic, keys, item ordering, and addOpenCLIAdapter behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ff3429d5-2ce4-4c23-9727-d39f96b4db41
📒 Files selected for processing (9)
backend/workflow/opencli_adapter_nodes.pyfrontend/components/flow/command-palette.tsxfrontend/lib/plugins/opencli-adapter-catalog.tsfrontend/lib/workflow/backend-opencli-adapter-nodes.tsfrontend/lib/workflow/opencli-business-workflows.tsfrontend/lib/workflow/source-business-config.tsfrontend/lib/workflow/studio-templates.tsfrontend/scripts/check-node-capability-catalog-regressions.mjstests/integration/test_workflow_capabilities_api.py
💤 Files with no reviewable changes (1)
- backend/workflow/opencli_adapter_nodes.py
Comment on lines
+475
to
+483
| export function openCLIKeyboardCandidates( | ||
| queryText: string, | ||
| selectedSite: OpenCLIAdapterPlugin | null, | ||
| matchingNodes: WorkflowOpenCLIAdapterNode[], | ||
| ): WorkflowOpenCLIAdapterNode[] { | ||
| if (!queryText && !selectedSite) return [] | ||
| return selectedSite?.commands ?? matchingNodes | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect featuredOpenCLIAdapterGroups to check whether matchingNodes order
# already matches the featured-first rendering order used by command-palette.tsx.
rg -nP -C8 'function featuredOpenCLIAdapterGroups' frontend/lib/plugins/opencli-adapter-catalog.tsRepository: 2233admin/opencli-admin
Length of output: 161
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== matching files =="
fd -a 'command-palette\.tsx$|opencli-adapter-catalog\.ts$' .
echo "== symbol searches =="
rg -n "openCLIKeyboardCandidates|featuredOpenCLIAdapterGroups|matchingNodes|OpenCLIKeyboardCandidates" frontend || true
echo "== catalog outline around relevant area =="
ast-grep outline frontend/lib/plugins/opencli-adapter-catalog.ts --view expanded 2>/dev/null | sed -n '1,220p' || true
echo "== catalog lines 430-510 =="
sed -n '430,510p' frontend/lib/plugins/opencli-adapter-catalog.ts
echo "== palette candidates references =="
file="$(fd 'command-palette\.tsx$' frontend | head -n1 || true)"
if [ -n "$file" ]; then
echo "$file"
sed -n '1,260p' "$file"
fiRepository: 2233admin/opencli-admin
Length of output: 15842
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== featuredOpenCLIAdapterGroups definition =="
sed -n '240,320p' frontend/lib/workflow/backend-opencli-adapter-nodes.ts
echo "== command palette relevant lines =="
sed -n '700,790p' frontend/components/flow/command-palette.tsx
echo "== workflow catalog opencli adapter candidates definitions =="
rg -n -C6 "openCLIKeyboardCandidates|featuredOpenCLIAdapterGroups|searchText|sortOpenCLIAdapterNodes|findMatching" frontend/lib/workflow
echo "== focused search for matching/flattening in backend catalog =="
rg -n -C8 "function .*matching|matchingOpenCLINodes|OPENCLI_SEARCH_RESULT_LIMIT|selectedSite|firstOpenCLI" frontend/components/flow/command-palette.tsx backend-opencli-adapter-nodes.ts workflow/use-opencli-adapter-catalog.tsRepository: 2233admin/opencli-admin
Length of output: 28529
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== opencliNodes source definitions in palette =="
rg -n -C5 "opencliNodes|useOpenCLI|fetchWorkflowOpenCLIAdapterNodes|openCLIAdapterNodeSort|sort" frontend/components/flow/command-palette.tsx
echo "== use opencli catalog =="
sed -n '1,220p' frontend/lib/workflow/use-opencli-adapter-catalog.ts
echo "== relevant palette around state/top =="
sed -n '1,180p' frontend/components/flow/command-palette.tsx
echo "== deterministic ordering probe: compare featured-first array vs input array for same query =="
python3 - <<'PY'
from pathlib import Path
src = Path('frontend/components/flow/command-palette.tsx').read_text()
print(src[src.find('opencliNodes'):src.find('opencliNodes')+1200])
PY
echo "== catalog tests around openCLIKeyboardCandidates =="
sed -n '530,600p' frontend/scripts/check-node-capability-catalog-regressions.mjsRepository: 2233admin/opencli-admin
Length of output: 15244
Align keyboard candidates with the featured-first OpenCLI results.
When searching without a selected site, matchingOpenCLINodes are rendered as featured groups first (commonOpenCLINodes), but openCLIKeyboardCandidates() still returns the unfiltered matchingNodes order, so Enter can select the first matching source from the raw catalog order rather than the first featured matching group. Dedupe and order candidates the same way, for example by starting from featuredOpenCLIAdapterNodes(matchingNodes) and appending the remaining matching nodes.
🤖 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 `@frontend/lib/plugins/opencli-adapter-catalog.ts` around lines 475 - 483,
Update openCLIKeyboardCandidates to match the featured-first ordering used by
matchingOpenCLINodes: when no selectedSite is provided, derive candidates from
featuredOpenCLIAdapterNodes(matchingNodes), then append remaining matching nodes
while deduplicating them. Preserve the selectedSite?.commands result and the
empty-query/no-selection behavior.
Comment on lines
+661
to
+663
| opencli_adapter_nodes.refresh_opencli_adapter_catalog() | ||
|
|
||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the second refresh result.
Line 661 only clears the cache; the test ends before observing another invalid-catalog load. Re-list and reassert the empty response after it.
Proposed fix
assert response.total == 0
assert response.nodes == []
opencli_adapter_nodes.refresh_opencli_adapter_catalog()
+ response = list_opencli_adapter_nodes(refresh=True)
+ assert response.total == 0
+ assert response.nodes == []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| opencli_adapter_nodes.refresh_opencli_adapter_catalog() | |
| opencli_adapter_nodes.refresh_opencli_adapter_catalog() | |
| response = list_opencli_adapter_nodes(refresh=True) | |
| assert response.total == 0 | |
| assert response.nodes == [] |
🤖 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/integration/test_workflow_capabilities_api.py` around lines 661 - 663,
Update the test after refresh_opencli_adapter_catalog() to call the
catalog-listing operation again and assert that it still returns an empty
response for the invalid catalog. Ensure the second refresh result is observed
rather than ending immediately after cache refresh.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Outcome
Duplicate audit
Verification
No backend contract changes.