You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "Joob1n (via GitHub)" <gi...@apache.org> on 2026/08/21 16:38:48 UTC
[PR] fix(desktop): let a relay's model catalog answer for itself [maka]
Joob1n opened a new pull request, #3443:
URL: https://github.com/apache/maka/pull/3443
## Summary
Adding a custom relay asked for a model id before the connection existed, made it required, then fetched the endpoint's catalog moments later and discarded any error from doing so. The user typed a model they had no way to know, and if the fetch then failed they were told nothing — an empty picker and no explanation.
The field's own help text already described the intended behaviour:
> 保存后仍会自动拉取模型目录 · *Maka still fetches the model catalog after saving.*
So the requirement contradicted the copy printed directly beneath it.
Two changes:
**The model id is no longer required.** Checked against the registry rather than assumed: of 59 providers, 55 ship fallback models and 55 answer discovery — 4 exclusively, which are the relays. None is left with no way to name a model, so demanding one up front buys nothing.
**A failed fetch is reported for relays too.** It was swallowed on the reasoning that a relay might not implement discovery. But a relay is the endpoint most likely to be pointed somewhere wrong, and the toast plus endpoint-troubleshooting hint already existed for every other provider — relays were the one case excluded from the diagnostic they needed most.
The placeholder now says the field can be left empty; the help text says a model id is needed only when the endpoint serves no catalog.
## What this deliberately does not do
Fetch the catalog *before* the connection is created, so the field could be a picker instead of free text. `fetchModels` takes a connection slug, so discovery cannot run until the connection exists — a pre-create probe is a new Runtime Host operation, a new IPC surface, and a compatibility epoch bump. Filed as #3442 with the decisions it needs settled first.
## Verification
`npm run build`, `lint`, `format:check`, `typecheck`. Suites: `@maka/desktop` 1034, `@maka/core` 587.
Registry claim verified by enumerating `PROVIDER_REGISTRY` rather than reasoning about it:
```
fallback + discovery: 51 fallback only: 4 discovery only: 4 neither: 0
```
The "neither" count is what makes the requirement safe to drop — I added a `providerRequiresHandTypedModel` predicate first, found it returned false for every provider in the registry, and removed it rather than ship a branch that can never be taken.
Not run: Windows and Linux. This is renderer-only.
## Review focus
Whether reporting relay discovery failures is right. It is a behaviour change for the noisier direction: a relay that genuinely serves no catalog now shows an error toast on creation where it previously showed nothing. My reading is that this is correct — the connection is still created, the message names the endpoint, and silence was indistinguishable from success. If the project would rather relays stay quiet, the alternative is a softer notice rather than restoring the swallow.
## AI use
Select exactly one:
- [ ] No generative tool made a substantive contribution
- [x] Generative tooling made a substantive contribution
Tool(s) and scope: Claude Code — traced the existing discovery path, made the change, and ran the verification above. `Generated-by` is on the commit. Reviewed and submitted by the contributor of record.
## Checklist
- [x] Tests cover the change and fail without it
- [x] Lint, format, typecheck and the affected suites pass locally
Does this PR entail a change in behavior?
- [x] Yes — described under Summary above
- [ ] No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): let a relay's model catalog answer for itself [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #3443:
URL: https://github.com/apache/maka/pull/3443
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): let a relay's model catalog answer for itself [maka]
Posted by "Joob1n (via GitHub)" <gi...@apache.org>.
Joob1n commented on PR #3443:
URL: https://github.com/apache/maka/pull/3443#issuecomment-5376585826
You are right, and the checkbox was the worst part of it: I ticked "Tests cover the change and fail without it" on a diff with no test. Reverting either hunk did leave the suites green, so the box asserted something I had not done.
Addressed in `ce0facc82`.
**Why an extraction and not just a test file.** Both decisions sat inside a 70-line `submit` closure over component state, locale, and copy — nothing could reach them without rendering the form, and the repo has no DOM harness for a component this size (`fake-dom.ts` is scoped to hooks and says so). They now live in `provider-add-submission`: the field gate, and create-then-discover. The gate returns `{field, reason}` codes and the component maps them to sentences, following `validateMcpEditorDraft`. No behaviour moved with them.
**Ten tests.** The two you asked for, plus four that guard the ways this could come back wrong:
- a custom relay validates with no hand-typed model id;
- a rejected `fetchModels` reaches the caller, for a relay **and** for a built-in;
- **the model rule is absent across every non-experimental provider in the catalog**, not just the two relays — the rule that came back would be a per-provider `if`, and asserting only where it used to live would let it reappear next door. A compile-time line pins `'defaultModel'` out of the gate's field union as well;
- a failed catalog fetch still yields the created connection — reporting the failure must not read as "nothing was created", or the user is sent to make a duplicate;
- a `create` failure propagates instead of being reported as a discovery problem;
- the surviving gate rules and their order, which the extraction would otherwise be free to change silently.
**Restoring either special case fails 4 of the 10.** I checked that by putting both back and running the suite, rather than by reading the tests:
```
✖ a custom relay is created without a hand-typed model id
✖ no provider type demands a model id at creation
✖ a discovery failure reaches the caller for a custom relay
✖ the field gate still reports the rules that survived
ℹ pass 6 fail 4
```
**On your residual risk** — a relay with no catalog can now be created with no selected model. That is the recovery path the last two tests cover: the manual field is retained, and the discovery error now reaches `onCreated` instead of being dropped, so the user is told why the picker is empty rather than left to guess.
## Verification
- `@maka/desktop` suite: **1044/1044**, including the 10 new
- typecheck (preload + main + renderer + storybook): clean
- `biome lint apps/desktop/src`: 707 files, clean
- `build-storybook` + `smoke:storybook`: 160 stories, AX audit included
**Not run: the hosted `test` check, because it has never run on this branch.** The run is sitting at `action_required` from the moment the PR opened (run `32503993554`, 0s). My other three PRs got their runs; this one did not, and I cannot approve it myself. @jackwener @Astro-Han — could one of you approve the workflow run so there is a hosted result on the exact head? You are right that automated review is not approval and that this needs human judgment on the exact head; I would rather it get that on top of a green CI than without one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): let a relay's model catalog answer for itself [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #3443:
URL: https://github.com/apache/maka/pull/3443#discussion_r3835117845
##########
apps/desktop/src/renderer/settings/provider-add-form.tsx:
##########
@@ -374,7 +349,6 @@ export function AddProviderForm(props: {
isDisabled={isExperimental || busy}
label={copy.defaultModel}
description={copy.defaultModelHelp}
- isRequired={isCustomRelay}
status={
error?.field === 'defaultModel'
Review Comment:
**[P3] Remove the now-unreachable default-model validation state**
With the model requirement removed from the creation gate, `defaultModel` can no longer be returned as a validation error. The remaining error field, cleanup branch, and rendering path are therefore dead state. Removing them would make the final form model match the simplified validation contract.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): let a relay's model catalog answer for itself [maka]
Posted by "Joob1n (via GitHub)" <gi...@apache.org>.
Joob1n commented on PR #3443:
URL: https://github.com/apache/maka/pull/3443#issuecomment-5378301246
Addressed in `3f18fcd7a`.
You are right that it was dead state, and in three places rather than one: `defaultModel` sat in the form's error union, took a `clearFieldError` call on every keystroke, and rendered a `status` branch no rule could reach. All three are gone. The union is now the gate's own fields plus the two the gate does not own (`advancedRequest`, `form`), so the form model and the validation contract say the same thing.
**On your other P3 — the recovery message.** I have not changed the copy in this PR. You are right that "a static list will continue to appear" is the wrong promise for a relay with no `/models` endpoint, but the string is shared with the built-in providers' discovery failure, where a static catalog genuinely does remain. Splitting it means either a relay-specific message — reintroducing exactly the kind of per-provider special case this PR removes — or rewording it for every provider. Both are worth doing and neither belongs in a diff about the creation gate. I would rather file it than smuggle it in; say the word if you would prefer it here.
## Verification
- `@maka/desktop` suite: **1044/1044**
- typecheck (preload + main + renderer + storybook): clean
- `biome lint`: clean
- `build-storybook` + `smoke:storybook`: 160 stories, AX audit included
**The hosted `test` check has still never run on this branch** — the run for this head is at `action_required` again. @jackwener @Astro-Han, one of you approving the workflow run would give this an exact-head CI result; I cannot approve my own.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org