Skip to content

feat(gui): edit discovered model display names with recovery - #3824

Merged
lidge-jun merged 5 commits into
devfrom
codex/axis5-model-name-editor
Sep 7, 2026
Merged

feat(gui): edit discovered model display names with recovery#3824
lidge-jun merged 5 commits into
devfrom
codex/axis5-model-name-editor

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • Carry feat: add discovered model display name editor #2716: add the discovered-model Name editor, keeping exact model selectors and routing aliases separate. Save/reset, validation, localization and focus behavior use the existing API.
  • Preserve confirmed writes across failed refreshes and retries, reconcile unknown outcomes through reads, and bound stalled operations without claiming rollback. The dialog retains its mounted focus state and avoids effect-driven draft synchronization.

Manual chain: #3820 native labels → #3821 provider JSONL → #3824 discovered-name editor. No native stack is registered.

Co-authored-by: Zig Zag shafishahin786@proton.me

Verification

  • Final feature candidate f51ec2421c49df0fd4eac8a9a56a6283b426387d: Cross-platform CI, all 25 jobs successful. Only the unsuccessful Mac jobs were replayed; passing jobs were reused. Initial timeout/timing failures remain recorded, not claimed fixed.
  • Late platform changes on dev 7fdb0e977 have zero file overlap with this axis and passed 26-job platform CI. Independent compatibility audit passed.
  • Prospective merged tree 85c9b25818a93859a6d6fc824e2ed0678da46c8f: 370 focused tests passed on isolated Linux with project Bun 1.4.0 (344 catalog/CLI + 26 editor). This is focused merged-tree evidence, distinct from full CI on the feature head.
  • No local test suites/typecheck/build ran, per owner instruction. Pushes used --no-verify. Documentation was source-reviewed; no docs build is claimed by this axis.
  • CI built artifact source and tree were checked. The compiled application passed Korean desktop/mobile save/reset, invalid-input rejection, selector-preservation and focus-return smoke with disposable API fixtures. Its 1,737 dashboard tests passed.

Desktop editor

Mobile editor

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs were updated where needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults; authentication and routing are unchanged.

Maintainer integration

The owner explicitly authorized admin integration into dev. Lower-layer independent CI was deliberately deferred in favor of the tested combined head; no missing or cancelled lower check is presented as green. Independent source reviews and late-base compatibility review passed. This is maintainer integration, not self-approval.

Carry the net diff from PR #2716 at 93ed440.
Reconcile confirmed saved/reset receipts, including saved:true errors, with the
editor draft, current label and reset availability. Preserve reset intent on
retry and retry only the list read after a successful mutation and failed read.

Reuse createBoundedFetch for a single 60-second write-and-refresh budget.
Timeouts retain the draft, release the modal lock and leave persistence unknown;
retry reads current state before another mutation. Keep global fetch unchanged.

Add focused regression coverage, nine-locale recovery copy and workflow docs.
Local tests, typecheck, build and browser smoke NOT RUN by owner mandate.
Static diff inspection only; parent owns remote CI and browser verification.

Co-authored-by: Zig Zag <shafishahin786@proton.me>
@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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

❤️ Share

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

@github-actions github-actions Bot added the enhancement New feature or request label Sep 6, 2026
Keep an earlier confirmed save/reset receipt when retrying the same value and
convergence fails with an ordinary HTTP error. Do not replace the saved snapshot
from an unconfirmed response; retain the pending reset intent for another retry.

Treat transport and response-body failures without a usable receipt as unknown
outcomes, hide the stale current name, and make Retry/Enter read-only. Keep
known unpersisted HTTP failures on the existing editable-draft path.

Add regressions for reset -> saved:true -> HTTP failure -> success and persisted
save/reset followed by rejected transport or response-body reads without abort.
Local tests/typecheck/build NOT RUN by owner mandate; static diff check only.

Co-authored-by: Zig Zag <shafishahin786@proton.me>
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

Browser review found an isolated Korean ending at 390px. Balance the short helper sentence without fixed line breaks or changing the existing layout.

Co-authored-by: Zig Zag <shafishahin786@proton.me>
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 58 / 80

이 PR은 Axis5 체인 3층(GUI)이다. 베이스 codex/axis5-provider-jsonl, 헤드 codex/axis5-model-name-editor. 지금 dev HEAD(137d6a727)를 직접 보면 gui/src/pages/Models.tsxgui/src/pages/models-shared.ts는 이미 있고, PUT /api/providers/:provider/model-display-namesdisplayNameOverride/displayNameSource 필드도 src/server/management/model-routes.ts·model-rows.ts에 있다. 반대로 gui/src/components/ModelDisplayNameDialog.tsx는 HEAD에 없다. 즉 서버 계약(#3212)은 이미 있는데, Models 페이지에 Name 버튼·다이얼로그·저장 실패 복구 UI만 없던 상태다. 이 PR이 그 UI를 #2716에서 가져와 현재 계약에 맞게 붙인다.

새 다이얼로그는 정확한 provider/model 셀렉터를 code로 항상 보여 주고, 별칭(pencil) 동작과 분리한다. 검증은 models-shared.tsmodelDisplayNameValidationKey가 서버 규칙을 미러한다(빈 값, 128자, 슬래시, 제어문자). 저장/리셋은 기존 API만 쓰고 새 엔드포인트를 만들지 않는다. 중요한 복구 경로가 있다. 라우트가 값을 저장한 뒤 카탈로그 수렴 실패로 503 + saved:true를 줄 수 있다는 점을 인정하고, 그 영수증을 버린 채 실패로만 처리하지 않는다. 리스트 새로고침만 실패한 경우에는 다이얼로그를 연 채로 재시도하고, 리셋 intent도 유지한다. 요청은 기존 createBoundedFetch(60_000)로 감싸서 멈춘 요청이 편집기를 영원히 잠그지 않게 했다. HEAD의 Models.tsx가 이미 같은 헬퍼를 쓰는 것과 결이 맞다.

버튼 노출 조건은 !m.native && !m.custom다. 네이티브 Codex 행 표시 이름은 1층 #3820의 설정/동기화 계약이고, 이 GUI는 디스커버드(HTTP) 모델 라벨만 다룬다. 커스텀 행도 제외한다. 아홉 로케일 키, 스타일, 729줄 GUI 테스트, 영문 providers 문서의 Name 절이 같이 들어왔다. 작성자 브라우저 스모크(한국어 저장/리셋, 슬래시 거절, 셀렉터 보존, 포커스 복귀)도 적어 두었다. 원본 기여는 Zig Zag(#2716)다.

주의할 UX 변화가 하나 있다. 모델 행에서 비네이티브 라벨을 formatNamespacedModelId(m.namespaced, t) 대신 생 m.namespaced로 찍고, 친화 이름이 있으면 옆에 작은 글씨로 붙인다. Name 기능을 드러내려면 필요할 수 있지만, 기존 로케일 포맷 헬퍼를 빼는 부수 효과라서 시각 회귀 가능성이 있다. 또한 네이티브 행은 Name 버튼이 없으므로, Astra 같은 네이티브 오버레이는 여전히 설정/API/#3820 경로로만 간다. 체인 설명과 UI 범위가 일치하는지는 문서에 한 줄 더 있으면 사용자 혼란이 줄어든다.

라인 Models.tsx 모델 identity - formatNamespacedModelId 제거가 Name PR의 필수인지 재확인이 필요하다. 포맷 헬퍼를 유지한 채 친화 이름 칩만 추가하는 편이 회귀가 작을 수 있다.
경로 ModelDisplayNameDialog.tsx - 접근성(dialog, aria-labelledby, focus, saving 중 close 차단)은 괜찮다. maxLength 129와 검증 128자 trim 조합은 서버와 맞는지 테스트가 잠그는지 보면 좋다.
경로 Models.tsx saveDisplayName - saved:true·unknown outcome·refresh-only 재시도 분기가 복잡하다. 단위/컴포넌트 테스트 729줄이 그 분기를 실제로 밟는다면 merge 설득력이 크다. CI GUI job이 최종 HEAD에서 초록이어야 한다.
경로 createBoundedFetch(60_000) - 같은 파일의 다른 호출은 15s/30s다. 카탈로그 강제 로드를 포함해서 60s로 올린 이유는 이해되지만, 실패 시 "저장됨/모름" 카피가 사용자에게 과하게 자주 보이진 않는지 스모크만 한 번 더 보면 좋다.
경로 HEAD 대조 - gui/src/components/ModelDisplayNameDialog.tsx는 신규가 맞고, Models/models-shared/i18n/API는 HEAD에 존재한다. 경로 날조 없음.

메인테이너의 판단이 필요한 지점

  • 비네이티브 행에서 formatNamespacedModelId를 빼도 되는지(시각 회귀 vs Name UX).
  • 네이티브 행 Name UI를 의도적으로 영구 제외할지, 나중에 feat(catalog): preserve native display-name overrides #3820 오버레이용 편집을 붙일지.
  • 최종 Cross-platform CI(작성자 기록 run)가 이 헤드 SHA에서 초록일 때만 admin merge할지.

너의 추천
3층 최종 통합 후보로 본다. 서버 계약을 재발명하지 않고 UI+복구만 추가했다. merge 전에 (1) identity 포맷 헬퍼 유지 여부 결정, (2) 결합 HEAD GUI/CI 초록 확인, (3) #3820#3821→이 PR 순 bottom-up. 통과하면 Axis5 닫고 원본 #3627/#3780/#2716 leftover는 landed 처리하면 된다.

이 댓글은 grok-bot이 작성했습니다

t and others added 2 commits September 7, 2026 08:06
Replace effect-driven draft synchronization with a guarded render-state
adjustment when the parent supplies a new confirmed model snapshot. Keep the
dialog mounted, retaining focus refs, pending state and request errors. Ordinary
typing and catalog polling do not replace the editor snapshot.

Local tests, lint, typecheck and build NOT RUN by owner mandate.
Static diff inspection only; final CI and independent review remain parent-owned.

Co-authored-by: Zig Zag <shafishahin786@proton.me>
Co-authored-by: Zig Zag <shafishahin786@proton.me>
Base automatically changed from codex/axis5-provider-jsonl to dev September 7, 2026 01:01
@lidge-jun
lidge-jun marked this pull request as ready for review September 7, 2026 01:01
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 7, 2026 01:01
@lidge-jun
lidge-jun merged commit 44c69fd into dev Sep 7, 2026
67 of 88 checks passed
@lidge-jun
lidge-jun deleted the codex/axis5-model-name-editor branch September 7, 2026 01:01
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 7, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-07T01:03:22.860886Z f51ec24 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

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

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant