Skip to content

feat(gui): add custom models from provider workspace - #449

Closed
apple-ouyang wants to merge 1 commit into
lidge-jun:devfrom
apple-ouyang:codex/provider-custom-model-input
Closed

feat(gui): add custom models from provider workspace#449
apple-ouyang wants to merge 1 commit into
lidge-jun:devfrom
apple-ouyang:codex/provider-custom-model-input

Conversation

@apple-ouyang

@apple-ouyang apple-ouyang commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a provider-scoped custom model input to the workspace Models tab
  • validate empty, namespaced, duplicate, loading, and failed-lookup states before submission
  • show successful additions immediately while preserving configured fallback models for custom-only catalogs
  • reuse the existing custom-model API and existing localized strings; no API or schema changes

Fixes #448

Testing

  • bun test ./gui/tests/provider-model-custom-add.test.tsx tests/provider-workspace-state.test.ts — 17 passed
  • bun run typecheck
  • bun run lint:gui
  • bun run build:gui
  • bun run privacy:scan
  • desktop and 375px browser QA with no console errors

Local full-suite note

bun run test completed 4,074 tests successfully and reported 25 environment-specific failures. In this local network, the affected test hostnames resolve to Clash-style 198.18.0.x fake IPs, so the destination-policy guard blocks mocked provider discovery before the tests reach their fetch stubs. The changed focused suites pass; CI remains the authoritative clean-environment full-suite result.

Summary by CodeRabbit

  • New Features

    • Added support for adding custom provider models directly from the provider workspace.
    • Custom models are loaded, displayed, deduplicated, and available immediately after being added.
    • Added validation and inline feedback for duplicate, invalid, failed, or successful additions.
    • Preserved configured fallback models when catalog results contain only custom models.
  • Bug Fixes

    • Improved model filtering and fallback behavior for custom-only provider catalogs.

Reason:
- Provider workspace users could not add a model when live discovery was incomplete.

Changes:
- Add provider-scoped custom model entry with validation, failure handling, and immediate list updates.
- Preserve configured fallback models and cover duplicate, loading, refresh, and network error paths.
@github-actions github-actions Bot added the enhancement New feature or request label Jul 25, 2026
@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

The provider workspace now supports adding provider-scoped custom model IDs, loading existing custom models, merging them into filtered results, and retaining configured fallback models for custom-only catalogs. Validation covers successful, duplicate, server-error, network-error, and unavailable-refresh flows.

Provider custom model support

Layer / File(s) Summary
Custom model filtering
gui/src/provider-workspace/report.ts, tests/provider-workspace-state.test.ts
filterModels merges custom IDs with live or fallback models, deduplicates results, and tests custom-only catalog behavior.
Provider model add flow
gui/src/components/provider-workspace/ProviderDetails.tsx, gui/src/components/provider-workspace/ProviderModels.tsx, gui/src/styles/provider-workspace-shell.css
ProviderDetails passes apiBase; ProviderModels loads and saves custom IDs, updates local results, validates submissions, and renders the new form and feedback styles.
Custom model flow validation
gui/tests/provider-model-custom-add.test.tsx
Tests cover trimmed submissions, duplicate and namespaced IDs, server and network failures, immediate display, fallback visibility, and lookup-failure gating.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ProviderDetails
  participant ProviderModels
  participant CustomModelsAPI
  ProviderDetails->>ProviderModels: Pass apiBase and provider name
  ProviderModels->>CustomModelsAPI: GET /api/custom-models
  CustomModelsAPI-->>ProviderModels: Return custom model IDs
  ProviderModels->>CustomModelsAPI: POST provider and modelId
  CustomModelsAPI-->>ProviderModels: Return save result
  ProviderModels->>ProviderModels: Update local IDs and invoke onRetryModels
Loading

Suggested reviewers: ingwannu, lidge-jun

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding custom models from the provider workspace.
Linked Issues check ✅ Passed The GUI changes implement #448’s provider-scoped custom-model flow, validation, immediate updates, and fallback handling.
Out of Scope Changes check ✅ Passed All changed files support the custom-model feature or its tests/styles, with no unrelated scope apparent.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/provider-workspace/ProviderModels.tsx`:
- Around line 164-173: Update the custom model input’s onChange handler in
ProviderModels so changing the draft value also clears customSuccess and
customError, while preserving the existing customModelId update and save
behavior.
- Around line 58-81: Extract the custom-model fetch logic from the useEffect
into a reusable loadCustomModels function, preserving its success, validation,
cleanup, and error behavior. Expose that function to the custom-model error
state and render a Retry button alongside customError, following the existing
modelsLoadFailed/onRetryModels pattern; the button must invoke
loadCustomModels() without requiring the component to remount.
🪄 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: ASSERTIVE

Plan: Pro Plus

Run ID: 451f57ff-b1da-4b66-9574-1680f23bf31f

📥 Commits

Reviewing files that changed from the base of the PR and between 7346510 and eeb61e9.

📒 Files selected for processing (6)
  • gui/src/components/provider-workspace/ProviderDetails.tsx
  • gui/src/components/provider-workspace/ProviderModels.tsx
  • gui/src/provider-workspace/report.ts
  • gui/src/styles/provider-workspace-shell.css
  • gui/tests/provider-model-custom-add.test.tsx
  • tests/provider-workspace-state.test.ts

Comment thread gui/src/components/provider-workspace/ProviderModels.tsx
Comment thread gui/src/components/provider-workspace/ProviderModels.tsx
@lidge-jun

Copy link
Copy Markdown
Owner

🔒 Maintainer integration in progress — please hold off on merging

@lidge-jun (maintainer) is actively integrating this PR into dev as part of a coordinated
integration pass. Please do not merge, rebase, force-push, or close this PR until this
marker is removed.

What is happening

This PR is being integrated on the maintainer branch codex/260725-pr-rework, which is
based on dev. The pass applies the contributed change, adds any missing regression
coverage, and repairs review-identified defects before a single verified merge into dev.
Every integrated change goes through bun run typecheck, the full bun run test suite,
bun run privacy:scan, and bun run lint:gui, followed by exact-SHA hosted
Cross-platform CI and Service lifecycle runs.

What this means for you

  • Your authorship and commits are preserved. Nothing is being rewritten under your name.
  • No action is needed from you right now. If a defect repair changes your intended
    behavior, it will be described explicitly in a follow-up comment before the merge.
  • If you have work in flight on this branch, please comment here instead of pushing, so we
    do not race each other.

This PR will be closed with a merge receipt (integration commit SHA, verification output,
and hosted CI links) once the pass lands on dev. Thanks for the contribution.

Integration tracker: devlog/_plan/260725_pr_issue_rework · marker posted by the maintainer integration pass

lidge-jun added a commit that referenced this pull request Jul 25, 2026
PR #449 (head eeb61e9, author apple-ouyang)를 통합하고 결함 2건을 함께 고친다.

provider workspace Models 탭에서 provider 범위 custom 모델을 추가할 수 있다.
빈 ID, 네임스페이스 포함 ID, 중복은 클라이언트와 서버 양쪽에서 거부한다.

결함 1 — live catalog provenance:
custom ID 차집합으로 live 여부를 추론하면, 기존 custom ID가 나중에 live
discovery에도 나타날 때 live catalog를 custom-only로 오분류해 configured
fallback이 잘못 authoritative로 남았다. 이제 discovery 성공 시 실제 반환된 행
수를 기록하고 /api/selected-models가 그 값을 전달한다. count는 configured alias
증강 전에 capture한다. markProviderDiscoveryOk의 count와 GUI의 hasLiveModels를
required로 두어 배선 누락이 typecheck에서 잡히게 했다.

결함 2 — custom-model 조회 실패 후 영구 비활성:
GET이 한 번 실패하면 customModelsReady가 false로 남고 effect 재실행 트리거가
없어 remount 전까지 Add가 막혔다. 실패를 별도 상태로 두고 Retry 버튼과
epoch 기반 재조회를 추가했다.

테스트:
- overlap된 custom/live ID에서 live catalog가 authoritative로 유지되고, 실제로
  discovery가 비었을 때만 fallback이 돌아옴
- discovery 3시나리오(non-empty, 성공한 empty, 실패 후 stale 보존)를 실제
  discovery 경로로 통과시켜 count를 관찰. 직접 marker 호출로는 production
  branch의 누락을 잡지 못한다
- 같은 mount에서 GET 실패 -> Retry -> 성공 -> Add 활성화 -> POST 정확히 1회

Co-authored-by: apple-ouyang <apple-ouyang@users.noreply.github.com>
@lidge-jun

Copy link
Copy Markdown
Owner

✅ Integrated into dev

Thank you for this feature. It has been integrated into dev as part of a verified integration pass.

Integration commit: 121a3512feat(gui): add custom models from provider workspace (#448)
Merged into dev at: ebc62d1f
Your authorship is preserved via Co-authored-by.

What was changed on top

The add flow itself went in as written — provider scoping, the empty/namespaced/duplicate guards, and the immediate local update all hold up, and the server-side re-validation covers the duplicate race.

Two defects were repaired in the same commit:

  1. Live-catalog provenance. Inferring "does this provider have a live catalog?" by subtracting custom ids from the catalog fails exactly when a custom id later also appears in live discovery — the subtraction leaves nothing, the live catalog reads as custom-only, and the configured fallback wrongly stays authoritative. The count is now recorded at the source (captured before configured-alias augmentation, otherwise configured rows pollute it), carried on /api/selected-models, and read by the GUI. The count argument and the hasLiveModels prop are both required, so a missed wiring is a compile error rather than a silent zero — which is what caught my own omission in ProviderModels's destructuring.

  2. Recovery after a failed lookup. A single failed /api/custom-models GET left customModelsReady false with no remaining effect trigger, so Add stayed disabled until the panel remounted. There is now a Retry control backed by an epoch refetch.

Verification

  • Discovery provenance runs through the real path — non-empty, successful-empty, and failure-preserves-last-count. Removing the count recording fails two of the three, so the tests genuinely pin the production branch
  • The recovery regression drives GET fail → Retry → success → Add enabled → POST exactly once in one mount
  • GUI suite: 148 pass / 0 fail; build clean. Full suite at merge: 4222 pass / 0 fail
  • Rendered check at 1280×720: with an overlapping custom/live id, the configured fallback correctly stays hidden; console clean
  • Cross-platform CI on ebc62d1f: green on ubuntu, macOS, Windows

Issue #448 is closed by this change.

@lidge-jun

Copy link
Copy Markdown
Owner

Closing: integrated into dev at ebc62d1f with authorship preserved. See the receipt comment above for the integration commit, the changes made on top, and the verification evidence.

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.

2 participants