feat(gui): show public combo model name with one-click copy - #1164
feat(gui): show public combo model name with one-click copy#1164eachann1024 wants to merge 2 commits into
Conversation
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
github-actions
Bot
added
enhancement
|
✅ Deterministic PR hygiene checks passed. |
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe combo workspace now displays computed public model names, supports copying valid values with feedback, updates ID and alias guidance across locales, and adds responsive styling for the preview control. ChangesPublic model preview and copy flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 `@gui/src/components/combo-workspace-controls.tsx`:
- Around line 268-302: Define and export a shared ellipsis placeholder constant
in combo-workspace-controls.tsx or combo-workspace-data.ts, then replace the
hardcoded "…" checks/usages in PublicModelPreview,
combo-workspace-add-modal.tsx, and combo-workspace-detail-panel.tsx with that
constant. Update imports at both consuming sites so all placeholder comparisons
and values use one source of truth.
In `@gui/src/i18n/en.ts`:
- Around line 1763-1769: Remove the unused cws.field.idHint entry from every
locale catalog: gui/src/i18n/en.ts (1763-1769), gui/src/i18n/de.ts (1729-1735),
gui/src/i18n/ja.ts (1797-1803), gui/src/i18n/ko.ts (1756-1762),
gui/src/i18n/ru.ts (1839-1845), and gui/src/i18n/zh.ts (1749-1755). Keep
cws.field.publicModelPreview, cws.field.idInternalHint, and cws.field.idHintEdit
unchanged.
🪄 Autofix
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: ASSERTIVE
Plan: Pro Plus
Run ID: d74154df-6287-4017-9989-49a422060cdd
📒 Files selected for processing (10)
gui/src/components/combo-workspace-add-modal.tsxgui/src/components/combo-workspace-controls.tsxgui/src/components/combo-workspace-detail-panel.tsxgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/src/styles-combos-workspace.css
| export function PublicModelPreview({ model }: { model: string }) { | ||
| const t = useT(); | ||
| const { outcomeFor, copy } = useCopyFeedback<string>(); | ||
| const canCopy = model.trim().length > 0 && model !== "…"; | ||
| const outcome = outcomeFor(model); | ||
| const copyLabel = outcome === "copied" | ||
| ? t("cws.copiedPublicModel") | ||
| : outcome === "unavailable" | ||
| ? t("cws.copyUnavailable") | ||
| : t("cws.copyPublicModel"); | ||
| // Split around a sentinel so the model token stays mono in any locale word order. | ||
| const sentinel = "\u0001"; | ||
| const [before, after = ""] = t("cws.field.publicModelPreview", { model: sentinel }).split(sentinel); | ||
|
|
||
| return ( | ||
| <div className="cwi-public-model-preview"> | ||
| <p className="muted cwi-public-model-preview-text"> | ||
| {before} | ||
| <code className="mono cwi-public-model-preview-value">{model}</code> | ||
| {after} | ||
| </p> | ||
| <button | ||
| type="button" | ||
| className="btn btn-ghost btn-sm cwi-public-model-preview-copy" | ||
| disabled={!canCopy} | ||
| onClick={() => { | ||
| if (canCopy) copy(model, model); | ||
| }} | ||
| title={copyLabel} | ||
| > | ||
| <span aria-live="polite">{copyLabel}</span> | ||
| </button> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Extract the shared ellipsis placeholder into a named constant.
canCopy on Line 271 hardcodes model !== "…" to detect the placeholder value. The same "…" literal is duplicated in gui/src/components/combo-workspace-add-modal.tsx (Line 143) and gui/src/components/combo-workspace-detail-panel.tsx (Line 217). Correctness of canCopy depends on all three literals staying byte-for-byte identical (for example, a future edit that swaps "…" for "..." in one call site silently breaks the copy button in that view).
Export a shared constant from combo-workspace-controls.tsx (or combo-workspace-data.ts) and import it at all three sites, so the placeholder has one source of truth.
♻️ Proposed refactor
+export const COMBO_ID_PLACEHOLDER = "…";
+
export function PublicModelPreview({ model }: { model: string }) {
const t = useT();
const { outcomeFor, copy } = useCopyFeedback<string>();
- const canCopy = model.trim().length > 0 && model !== "…";
+ const canCopy = model.trim().length > 0 && model !== COMBO_ID_PLACEHOLDER;🤖 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 `@gui/src/components/combo-workspace-controls.tsx` around lines 268 - 302,
Define and export a shared ellipsis placeholder constant in
combo-workspace-controls.tsx or combo-workspace-data.ts, then replace the
hardcoded "…" checks/usages in PublicModelPreview,
combo-workspace-add-modal.tsx, and combo-workspace-detail-panel.tsx with that
constant. Update imports at both consuming sites so all placeholder comparisons
and values use one source of truth.
| "cws.field.idHint": "Clients request: {model}", | ||
| "cws.field.idInternalHint": "Internal id for this combo. When no public model name is set, clients use combo/<id>.", | ||
| "cws.field.idHintEdit": "Changing the id renames the combo. When no public model name is set, clients use combo/<id>.", | ||
| "cws.field.alias": "Public model name", | ||
| "cws.field.aliasPlaceholder": "deepseek-v4-flash or vendor/model", | ||
| "cws.field.aliasHint": "Optional. Use a bare name with no prefix, a custom prefix like vendor/model, or leave blank to use combo/<id>.", | ||
| "cws.field.aliasHint": "Optional override for the name clients request. Use a bare name, a custom prefix like vendor/model, or leave blank for combo/<id>.", | ||
| "cws.field.publicModelPreview": "Clients request: {model}", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm cws.field.idHint has no remaining callers in the GUI source before deleting it everywhere.
rg -n '"cws\.field\.idHint"' gui/src
rg -nP 't\(\s*"cws\.field\.idHint"' gui/srcRepository: lidge-jun/opencodex
Length of output: 587
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== i18n locale files =="
git ls-files 'gui/src/i18n/*.ts' | sort
echo
echo "== duplicate-text candidates around cws.field keys in each locale =="
python3 - <<'PY'
import os, pathlib, re
files = sorted(pathlib.Path('gui/src/i18n').glob('*.ts'))
for pat in ['en', 'de', 'ja', 'ko', 'ru', 'zh']:
for f in files:
if f.stem != pat:
continue
txt = f.read_text(encoding='utf-8')
keys = ['cws.field.idHint','cws.field.publicModelPreview','cws.field.idInternalHint','cws.field.idHintEdit','cws.field.alias','cws.field.aliasPlaceholder','cws.field.aliasHint']
print(f"\n{f}")
for key, line in [(key, txt.splitlines()[i].strip()) for i, line in enumerate(txt.splitlines(), 1) if key in line and 'cws.field.' in line][:7]:
pass
# print lines containing target keys and nearby keys for context
for i, line in enumerate(txt.splitlines(), 1):
if any(k in line for k in keys):
print(f"{i:<4} {line}")
PY
echo
echo "== call sites for cws.field.idHint via common UI i18n helpers =="
rg -n '(\buseI18n|useTranslation|i18n\.)\(([^)]*idHint|[^)]*publicModelPreview|[^)]*idInternalHint|[^)]*idHintEdit)[^)]*|t\(\s*["\047]cws\.field\.(idHint|publicModelPreview|idInternalHint|idHintEdit|alias)(["\047])' gui/src || true
echo
echo "== broader idHint/publicModelPreview text matches (excludes i18n definitions) =="
rg -n --glob '!gui/src/i18n/*.ts' 'idHint|publicModelPreview|Clients request: \{model\}|Clients 要求|クライアントのリクエスト|클라이언트 요청|Клиенты запрашивают' gui/src || trueRepository: lidge-jun/opencodex
Length of output: 582
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== i18n locale files =="
git ls-files 'gui/src/i18n/*.ts' | sort
echo
echo "== target cws.field keys in each locale =="
for f in gui/src/i18n/en.ts gui/src/i18n/de.ts gui/src/i18n/ja.ts gui/src/i18n/ko.ts gui/src/i18n/ru.ts gui/src/i18n/zh.ts; do
echo "-- $f --"
git -C "$(dirname "$f")" grep -n -e '"cws\.field\.idHint"' -e '"cws\.field\.publicModelPreview"' -e '"cws\.field\.idInternalHint"' -e '"cws\.field\.idHintEdit"' "$f" || true
done
echo
echo "== call sites for cws.field.idHint in GUI source (excluding i18n locale definitions) =="
rg -n --glob '!gui/src/i18n/*.ts' 'cws\.field\.idHint|cws\.field\.publicModelPreview|cws\.field\.idInternalHint|cws\.field\.idHintEdit|t\(' gui/src || true
echo
echo "== static text matches outside i18n locale definitions =="
rg -n --glob '!gui/src/i18n/*.ts' 'Clients request: \{model\}|Clients fordern an: \{model\}|クライアントのリクエスト: \{model\}|클라이언트 요청: \{model\}|Клиенты запрашивают: \{model\}|客户端请求：\{model\}|Clients request|publicModelPreview' gui/src || trueRepository: lidge-jun/opencodex
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== cws.field target key definitions by locale =="
for f in gui/src/i18n/en.ts gui/src/i18n/de.ts gui/src/i18n/ja.ts gui/src/i18n/ko.ts gui/src/i18n/ru.ts gui/src/i18n/zh.ts; do
echo "-- $f --"
rg -n '"cws\.field\.(idHint|publicModelPreview|idInternalHint|idHintEdit)"' "$f" || true
done
echo
echo "== GUI source references to target cws.field keys outside i18n locale files =="
rg -n --glob '!gui/src/i18n/*.ts' 'cws\.field\.(idHint|publicModelPreview|idInternalHint|idHintEdit)\b' gui/src || true
echo
echo "== GUI source direct text match for publicModelPreview equivalent outside i18n locale files =="
rg -n --glob '!gui/src/i^8n/*.ts' 'Clients request: \{model\}|Clients fordern an: \{model\}|Клиенты запрашивают: \{model\}|クライアントのリクエスト: \{model\}|클라이언트 요청: \{model\}|客户端请求：\{model\}' gui/src || trueRepository: lidge-jun/opencodex
Length of output: 3859
Remove the unused cws.field.idHint entries from all locale catalogs. The GUI now renders the request preview through cws.field.publicModelPreview (gui/src/components/combo-workspace-controls.tsx:280), while ID-related combo text uses cws.field.idInternalHint / cws.field.idHintEdit (gui/src/components/combo-workspace-add-modal.tsx:122, gui/src/components/combo-workspace-detail-panel.tsx:196). Since locale files implement the shared key type from gui/src/i18n/en.ts, delete cws.field.idHint from every locale, not just one.
📍 Affects 6 files
gui/src/i18n/en.ts#L1763-L1769(this comment)gui/src/i18n/de.ts#L1729-L1735gui/src/i18n/ja.ts#L1797-L1803gui/src/i18n/ko.ts#L1756-L1762gui/src/i18n/ru.ts#L1839-L1845gui/src/i18n/zh.ts#L1749-L1755
🤖 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 `@gui/src/i18n/en.ts` around lines 1763 - 1769, Remove the unused
cws.field.idHint entry from every locale catalog: gui/src/i18n/en.ts
(1763-1769), gui/src/i18n/de.ts (1729-1735), gui/src/i18n/ja.ts (1797-1803),
gui/src/i18n/ko.ts (1756-1762), gui/src/i18n/ru.ts (1839-1845), and
gui/src/i18n/zh.ts (1749-1755). Keep cws.field.publicModelPreview,
cws.field.idInternalHint, and cws.field.idHintEdit unchanged.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7600adb2e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "cws.field.idInternalHint": "Internal combo id. You can change it after creation.", | ||
| "cws.field.idHintEdit": "Renaming moves the combo to a new id. Clients request {model}.", | ||
| "cws.field.idHint": "Clients request: {model}", | ||
| "cws.field.idInternalHint": "Internal id for this combo. When no public model name is set, clients use combo/<id>.", |
There was a problem hiding this comment.
Changing this source string leaves the existing empty-workspace test asserting the old helper copy (Internal combo id. You can change it after creation.), so the required GUI test suite will fail in tests/combo-workspace-empty.test.tsx as soon as dependencies are installed. Update that assertion along with the locale copy (or keep the old text) so cd gui && bun test tests stays green.
AGENTS.md reference: gui/AGENTS.md:L42-L50
Useful? React with 👍 / 👎.
github-actions
Bot
removed
the
intake: hygiene-blocked
Clarify Combo ID vs client-facing model name, preview the effective public id under the alias field, and route header/preview copy through shared useCopyFeedback (including clipboard unavailable).
Extract canCopyPublicModelId into a pure module and add a focused unit test so gui/ changes satisfy missing_regression_test hygiene.
53b7e97 to
a2b6b33
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
|
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
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)
gui/src/i18n/en.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the still-unused
cws.field.idHintkey from every locale catalog. This key holds the exact same text ascws.field.publicModelPreviewin every locale (for example, ingui/src/i18n/en.ts,"cws.field.idHint": "Clients request: {model}"at line 1763 duplicates"cws.field.publicModelPreview": "Clients request: {model}"at line 1769). No component in this cohort callst("cws.field.idHint"); callers usecws.field.idInternalHint,cws.field.idHintEdit, orcws.field.publicModelPreviewinstead. This repeats a prior review comment that was not addressed in this revision.
gui/src/i18n/en.ts#L1752-1769: delete the"cws.field.idHint"entry (line 1763) from theRecord<TKey, string>object; this is theTKeysource of truth, so remove it here first.gui/src/i18n/de.ts#L1729-1735: delete the"cws.field.idHint"entry (line 1734).gui/src/i18n/ja.ts#L1797-1803: delete the"cws.field.idHint"entry (line 1797).gui/src/i18n/ko.ts#L1756-1762: delete the"cws.field.idHint"entry (line 1761).gui/src/i18n/ru.ts#L1839-1845: delete the"cws.field.idHint"entry (line 1839).gui/src/i18n/zh.ts#L1749-1755: delete the"cws.field.idHint"entry (line 1749).♻️ Proposed fix (repeat per locale file, adjust text language)
- "cws.field.idHint": "Clients request: {model}", "cws.field.idInternalHint": "Internal id for this combo. When no public model name is set, clients use combo/<id>.",🤖 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 `@gui/src/i18n/en.ts` at line 1, Remove the unused "cws.field.idHint" entry from the locale catalogs in en.ts, de.ts, ja.ts, ko.ts, ru.ts, and zh.ts. Delete it from en.ts first because its Record<TKey, string> keys define TKey, then remove the matching entries from the other catalogs while preserving all remaining translations and key parity.
🤖 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 `@gui/src/i18n/en.ts`:
- Line 1: Remove the unused "cws.field.idHint" entry from the locale catalogs in
en.ts, de.ts, ja.ts, ko.ts, ru.ts, and zh.ts. Delete it from en.ts first because
its Record<TKey, string> keys define TKey, then remove the matching entries from
the other catalogs while preserving all remaining translations and key parity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d0c0546b-3380-42b1-b628-4be7776db968
📒 Files selected for processing (12)
gui/src/combo-public-model.tsgui/src/components/combo-workspace-add-modal.tsxgui/src/components/combo-workspace-controls.tsxgui/src/components/combo-workspace-detail-panel.tsxgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/src/styles-combos-workspace.csstests/combo-public-model-preview.test.ts
1 participant
Summary
useCopyFeedback(copied + clipboard unavailable).Split from #1092 per maintainer request to keep capability/UX changes reviewable on their own.
Change graph
Screenshots
Local verification on the split branch (same public-name preview UX as #1092):
Verification
bun run typecheck— exit 0Checklist
Related: #1092
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
New Features
Documentation
Style