You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "liuxiaocs7 (via GitHub)" <gi...@apache.org> on 2026/09/01 18:18:35 UTC
[PR] fix(desktop): read model metadata coverage from the Host catalog entry [maka]
liuxiaocs7 opened a new pull request, #4500:
URL: https://github.com/apache/maka/pull/4500
## What
Settings → a provider connection showed a spurious "declare this model's
capabilities by hand" row for models the Runtime Host already describes — any
model models.dev started listing since this Maka build was cut.
The renderer decided whether a model needs a hand-written declaration by asking
`hasModelMetadata()`, a question about its **own bundled** models.dev snapshot.
Since #4411 clients read Host-resolved `catalogEntries` instead of resolving a
catalog themselves, and since #4467 the Host refreshes that catalog at startup.
So the Host can describe a newly-listed model while this one renderer — a
different process, often on a different machine, whose snapshot never refreshes
— cannot, and it fell back to asking the user.
## Fix
The Host owns the answer, so the entry now states it:
- `ModelCatalogEntry` carries `describedByMetadata`, set by `makeEntry` from the
same metadata lookup `hasModelMetadata` reads.
- `provider-connection-detail.tsx` reads that field off the `modelChoices` it
already has, instead of consulting its bundled table.
- The field is **required** on the wire and decoded in
`model-catalog-entry-codec`, so `RUNTIME_HOST_COMPATIBILITY_EPOCH` moves
`89 → 90`; a Host and client that disagree about its presence are refused at
the handshake rather than dropping the field silently.
Inferring coverage from whether some optional field (e.g. `contextWindow`)
happens to be present would have put a second, weaker copy of the rule in the
renderer — exactly the split this line of work exists to close — so the entry
answers directly.
The `#1584` case (a user-typed id no inventory describes) still reports
`false`, so such a model keeps its declaration row. `provider-endpoint-presentation.ts`
reads the bundled table too, but only for build-time
`generatedModelProviderOverrides` a Host refresh never installs, so it stays
consistent and is left untouched (as the issue asks).
## Impact
Cosmetic and recoverable: it removed one spurious form row on a normal user
path. No persisted state was affected.
## Testing
- `packages/core` — new test in `model-catalog.test.ts`: a known model reports
`describedByMetadata: true`, a bare id reports `false` (matching
`hasModelMetadata`), the field survives a wire round-trip, and an entry
lacking it is rejected by the decoder. `node --test`: 20/20.
- Codec (13/13) and runtime-policy-coordinator (17/17) suites pass.
- `npm run build` (all packages incl. desktop renderer) ✓; `biome format`/`lint`
on changed files ✓.
- Regenerated `apps/desktop/renderer-architecture.json` (`--write`); the "Check
renderer architecture" snapshot passes.
Fixes #4496
Generated-by: Claude Code
--
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): read model metadata coverage from the Host catalog entry [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #4500:
URL: https://github.com/apache/maka/pull/4500
--
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): read model metadata coverage from the Host catalog entry [maka]
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on PR #4500:
URL: https://github.com/apache/maka/pull/4500#issuecomment-5510832020
Thanks — the pre-merge fix is in (`da1d8f9d`).
**Absence vs. `described: false`.** Fixed in `provider-connection-detail.tsx`. The capability-row filter now reads:
```ts
const entry = entryById.get(modelId);
return entry !== undefined && !entry.describedByMetadata;
```
A missing entry — a quarantined id the provider registry drops from the catalog (`OPENCODE_FREE_BROKEN_MODEL_IDS`, the Codex normaliser) but `enabledModelIds` still carries so the user can untick it — no longer counts as "no metadata," so it stops growing a declaration row `main` never showed. A present-but-uncovered entry still gets its row, and since `savedModelIds` always yields an entry for a hand-typed id, #1584 is unaffected. Good catch — same class of bug the PR set out to close; I reproduced your `opencode-free` case before and after.
**Sync point.** Added a comment at the `resolveDraftConnectionModelCatalog` overlay noting `describedByMetadata` now has two producers (`makeEntry` and the `hostCoverage` patch), so a future Host-decided field must be patched back there too.
**On the two smaller items — happy to fold either in this round, tell me which you'd prefer:**
- `hasModelMetadata` now has no production caller (only the codec/entry tests assert against it). Its body is exactly `Object.keys(lookupModelMetadata(...)).length > 0`, which `makeEntry` re-implements by hand, so I can route `makeEntry` through it or delete it and let the tests assert the entry — I left it as-is to keep the pre-merge diff to the behaviour fix.
- A dedicated test for the changed expression: the page has no test harness today, so a clean seam means extracting the predicate, which adds a renderer dependency edge and churns the architecture snapshot. I kept it out of the pre-merge change for that reason, but can add it as a small extraction if you'd like the coverage.
**Epoch / merge order.** Noted — this head is 95 now (main reached 94 since your review). Whichever 95-claimant lands first keeps it; I'll re-resolve to `main + 1` when that happens.
--
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): read model metadata coverage from the Host catalog entry [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4500:
URL: https://github.com/apache/maka/pull/4500#discussion_r3914819898
##########
apps/desktop/src/renderer/settings/provider-connection-detail.tsx:
##########
@@ -219,7 +220,7 @@ function ConnectionDetailInner(props: ConnectionDetailProps) {
(modelId) =>
isRelay ||
relayProfileDraft[modelId] !== undefined ||
- !hasModelMetadata(connection.providerType, modelId),
+ entryById.get(modelId)?.describedByMetadata !== true,
Review Comment:
A missing entry now reads as "not described". Quarantined ids are omitted from the catalog on purpose but stay in `enabledModelIds` and in the enabled-model manager, so they grow a declaration row here (reproduced with `opencode-free` / `x-preview-f-free`). Use `entry !== undefined && !entry.describedByMetadata` so absence and "described: false" stay distinct.
--
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): read model metadata coverage from the Host catalog entry [maka]
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on PR #4500:
URL: https://github.com/apache/maka/pull/4500#issuecomment-5507356554
Thanks for the careful review — both points are addressed in the current head (`72ac2dcb`).
**[P2] Preserve the Host's metadata-coverage decision when the draft diverges.** Fixed. `resolveDraftConnectionModelCatalog` still resolves locally once the draft diverges from storage, but it no longer lets the renderer's bundled table overturn the Host's answer: it now keeps the Host-projected `describedByMetadata` for every id already present in `connection.catalogEntries`, and falls back to local resolution only for an id the Host has never described. So ticking a model id or accepting a fetched list can no longer flip a refreshed-only model back to `false` and bring the spurious capability-declaration row back.
Added a regression test in `packages/core/src/__tests__/model-catalog.test.ts` — *"a divergent draft keeps the Host metadata-coverage decision for ids it already described"* — that reproduces your scenario: a synthetic future model marked `describedByMetadata: true` on the Host entry but absent from the bundled table (asserted via `hasModelMetadata(...) === false`). The unedited draft retains `true`; after adding another enabled id, the edited draft still reports `true` for that id, while a brand-new id the Host never described takes the local answer (`false`).
**[P2] Compatibility section.** Fixed in the PR body to state the transition this head actually makes, with both ledger entries present. It now reads `94 → 95` rather than `93 → 94`: `main` has kept advancing and further epoch bumps have landed since the review, so the branch re-numbers to `main`'s epoch + 1 on each rebase.
--
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