You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "Astro-Han (via GitHub)" <gi...@apache.org> on 2026/09/01 02:01:30 UTC
[PR] refactor(runtime-host): make the Host the authority for the model catalog [maka]
Astro-Han opened a new pull request, #4411:
URL: https://github.com/apache/maka/pull/4411
Part 1 of #4398.
## The problem
A model catalog entry — display name, context window, thinking levels, vision support, pricing — was resolved independently by every client, each against the model metadata bundled into its own build. A Desktop and a TUI attached to the same Host could therefore describe the same model differently, and the difference tracked whichever client had been updated more recently, not anything about the model or the account.
That is backwards for a product where the Host is the execution authority. The Host is the process that holds the connection, runs discovery, and sends the request; it is the only party in a position to say what a model is.
## What changes
The Host resolves each connection's catalog and projects it. `connection.catalog.query` gains a third page item, `catalog_entry`, one per model, counted by the connection header alongside the model and enabled-id counts. Clients decode what they were sent instead of re-deriving it.
`resolveConnectionModelCatalog` is the single entry point for "what models does this connection have, and what is true about them", so the provider rules that shape the list — the Codex subscription's servable set — cannot be applied in one place and forgotten in another.
Two client-side resolutions remain, both because no Host-held connection exists to project from, and both documented at the call site and in `docs/architecture/runtime-host-architecture.md`:
- the recommended default model shown while adding a connection, before one exists;
- the unsaved editor draft in connection settings.
The compatibility epoch moves to 84. An older client would ignore the new items and resolve locally; an older Host sends none, leaving a newer client with an empty catalog. Both are rejected at the handshake rather than left to degrade.
## Simplification carried in the same PR
Making the entry the thing that crosses the wire turns every field on it into a cost the Host pays to encode and each client pays to decode, so the fields with no reader were worth finding:
- `recommendedRank` had no reader anywhere; `capabilitySource` had three, all assertions that already checked the fact the override produced.
- `availability` was `unavailableReason` folded into three buckets, read only by its own codec — two representations of one fact, now both on the wire.
- The model-choice provenance (`SavedModelChoice`'s object form, the four `ModelCatalogUserChoiceSource` values, `provenance.sources`) had no production producer or consumer left; `savedModelIds` is what it always was in practice, the ids a catalog must list even when no inventory describes them (#1584).
- `validateChatDefaultModel` re-derived `canUseAsChatDefault`, and only its own tests called it.
Two provider-registry cleanups ride along for the same reason — the registry is what the Host consults to resolve a catalog, and it answered to two names: `PROVIDER_DEFAULTS` was an alias of `PROVIDER_REGISTRY`, and five accessors indexed the registry directly instead of going through `providerDefaultsOf`, which owns the own-property check that keeps `['__proto__']` from reading as a registered provider.
## Verification
Repository-wide `build`, `typecheck` and `format` are clean. Targeted suites: core catalog 20/20 and connections 13/13, runtime-host policy coordinator 17/17 and catalog reader 7/7, CLI onboarding 9/9 and TUI runner 171/171, desktop connections IPC 11/11 and catalog choices 2/2.
## Not in this PR
Runtime metadata refresh (part 2) and the scheduled snapshot refresh with drift check (part 3) from #4398.
--
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] refactor(runtime-host): make the Host the authority for the model catalog [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4411:
URL: https://github.com/apache/maka/pull/4411#discussion_r3900483422
##########
apps/desktop/src/renderer/settings/use-connection-detail.ts:
##########
@@ -306,8 +306,11 @@ export function useConnectionDetail(props: ConnectionDetailProps) {
setEnabledModelIds(connectionEnabledModelIds(connection));
}, [connection.defaultModel, connection.enabledModelIds, connection.slug]);
- // Picker entries come from the same catalog merge path as Chat and Daily
- // Review, but use the local unsaved editor draft for model/default changes.
+ // The one client-side resolution left on a saved connection: the editor
+ // shows the unsaved draft — model rows just fetched, ids just ticked — which
+ // the Host has not been told about and so cannot have resolved. Everything
+ // the user has committed is read from `connection.catalogEntries`; this
+ // resolves only what is still in the draft.
const modelChoices = buildConnectionModelCatalogEntries({
Review Comment:
Fixed in 53fc939 — the detail sheet now reads `connection.catalogEntries` whenever the draft still matches the committed connection, and only resolves locally once model rows, enabled ids, or the model source actually diverge. The rule lives in `resolveDraftConnectionModelCatalog` in `@maka/core/model-catalog`, beside the resolver it guards; divergence compares every stored field an entry is built from, not ids alone, since a re-fetched row can carry different facts under the same id. The regression in `model-catalog-choices.test.ts` gives the Host an entry the local build cannot produce (a name it has never heard, `canUseAsChatDefault: false`) and asserts the unedited editor renders it verbatim, then asserts a diverged draft does resolve locally.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
apps/desktop/src/renderer/model-catalog-choices.ts:
##########
@@ -89,74 +91,26 @@ export function buildCatalogDailyReviewModelOptions(
return options;
}
+/**
+ * The offerable models of one connection. A model the user picked that this
+ * connection can no longer serve is not offered here — the caller appends the
+ * saved key with an "unavailable" label so the current selection stays
+ * visible without pretending it is selectable.
+ */
function dailyReviewCatalogEntries(
- connection: Pick<
- LlmConnection,
- 'slug' | 'providerType' | 'defaultModel' | 'enabledModelIds' | 'models' | 'modelSource' | 'modelsFetchedAt'
- >,
- savedModelIds: Iterable<SavedModelChoice | undefined | null>,
-): ModelCatalogEntry[] {
- const savedChoices = Array.from(savedModelIds);
+ connection: Pick<LlmConnection, 'slug' | 'providerType' | 'enabledModelIds' | 'defaultModel'> &
+ HostResolvedConnectionCatalog,
+): readonly ModelCatalogEntry[] {
const enabledIds = new Set(connectionEnabledModelIds(connection));
- const visibleIds = new Set(enabledIds);
- for (const choice of savedChoices) {
- const id = typeof choice === 'string' ? choice.trim() : choice?.id.trim();
- if (id) visibleIds.add(id);
- }
- return filterUnsupportedCodexModels(
- connection.providerType,
- buildConnectionModelCatalogEntries({ connection, savedModelIds: savedChoices }),
- )
- .filter((entry) => visibleIds.has(entry.id) && (
- entry.canUseAsChatDefault || entry.provenance.sources?.userChoice?.includes('daily_review_model')
- ))
- .map((entry) => enabledIds.has(entry.id) ? entry : { ...entry, canUseAsChatDefault: false });
-}
-
-function selectableCatalogEntries(
- connection: Pick<
- LlmConnection,
- 'slug' | 'providerType' | 'defaultModel' | 'models' | 'modelSource' | 'modelsFetchedAt'
- >,
- savedModelIds?: Iterable<string | undefined | null>,
-): ModelCatalogEntry[] {
- const entries = filterUnsupportedCodexModels(
- connection.providerType,
- buildConnectionModelCatalogEntries({ connection, savedModelIds }),
- ).filter((entry) => entry.canUseAsChatDefault);
- if (entries.length > 0 || connection.providerType !== 'openai-codex') return entries;
- return filterUnsupportedCodexModels(
- connection.providerType,
- buildConnectionModelCatalogEntries({
- connection: {
- ...connection,
- defaultModel: '',
- models: undefined,
- modelSource: undefined,
- modelsFetchedAt: undefined,
- },
- savedModelIds,
- }),
- ).filter((entry) => entry.canUseAsChatDefault);
-}
-
-function filterUnsupportedCodexModels(providerType: ProviderType, entries: ModelCatalogEntry[]): ModelCatalogEntry[] {
- if (providerType !== 'openai-codex') return entries;
- return entries.filter((entry) => !CODEX_SUBSCRIPTION_UNSUPPORTED_CHATGPT_MODELS.has(entry.id.trim()));
+ return connection.catalogEntries.filter(
Review Comment:
Fixed in 696149b — `normalizeOpenAiCodexConnection` now filters `enabledModelIds` by the same rule as the inventory, so an id saved back when the model was still picker-visible no longer comes back through the catalog as an enabled model no inventory describes. It keeps the identity return when nothing changed. Regression in `model-catalog-choices.test.ts` asserts Daily Review is offered `gpt-5.5` and not `gpt-5-codex` for a connection whose stored selection still lists both.
_🤖 Addressed by [Claude Code](https://claude.com/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] refactor(runtime-host): make the Host the authority for the model catalog [maka]
Posted by "hqhq1025 (via GitHub)" <gi...@apache.org>.
hqhq1025 commented on code in PR #4411:
URL: https://github.com/apache/maka/pull/4411#discussion_r3900287815
##########
packages/core/src/runtime-policy/connection-catalog-codec.ts:
##########
@@ -65,6 +65,13 @@ import {
export const CONNECTION_CATALOG_MAX_CONNECTIONS = 1_024;
export const CONNECTION_CATALOG_MAX_MODELS_PER_CONNECTION = 2_048;
export const CONNECTION_CATALOG_MAX_ENABLED_MODEL_IDS = 512;
+/**
+ * A resolved entry exists for every stored model, for every enabled id the
+ * inventory never listed, and for the connection default when it lists none —
+ * so the entry bound is the sum of the two lists it draws from, plus one.
+ */
+export const CONNECTION_CATALOG_MAX_ENTRIES_PER_CONNECTION =
Review Comment:
[P1] Include fallback rows in the catalog-entry bound
This bound assumes the resolved set is only persisted `models` + missing enabled IDs + an optional default, but `resolveConnectionModelCatalog()` prepends every registry fallback for providers whose discovery kind is `fallback`. A catalog document that the canonical storage decoder accepts with 2,048 distinct stored models and 512 distinct enabled IDs therefore resolves to 2,577 entries for `volcengine-agent-plan`. I reproduced the production path: `HostRuntimePolicyCoordinator` emitted a first-page header with `catalogEntryCount: 2577`, then this operation decoder rejected that same Host result as `Invalid catalog entry count` because the maximum here is 2,561. Desktop and TUI cannot load any model choices once such a valid persisted catalog is present. Please include the maximum additional fallback inventory in the bound, or normalize/cap the Host projection so every canonical catalog remains encodable, and add an end-to-end maximum-boundary regression.
--
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] refactor(runtime-host): make the Host the authority for the model catalog [maka]
Posted by "M4n5ter (via GitHub)" <gi...@apache.org>.
M4n5ter commented on code in PR #4411:
URL: https://github.com/apache/maka/pull/4411#discussion_r3900355552
##########
apps/desktop/src/renderer/model-catalog-choices.ts:
##########
@@ -89,74 +91,26 @@ export function buildCatalogDailyReviewModelOptions(
return options;
}
+/**
+ * The offerable models of one connection. A model the user picked that this
+ * connection can no longer serve is not offered here — the caller appends the
+ * saved key with an "unavailable" label so the current selection stays
+ * visible without pretending it is selectable.
+ */
function dailyReviewCatalogEntries(
- connection: Pick<
- LlmConnection,
- 'slug' | 'providerType' | 'defaultModel' | 'enabledModelIds' | 'models' | 'modelSource' | 'modelsFetchedAt'
- >,
- savedModelIds: Iterable<SavedModelChoice | undefined | null>,
-): ModelCatalogEntry[] {
- const savedChoices = Array.from(savedModelIds);
+ connection: Pick<LlmConnection, 'slug' | 'providerType' | 'enabledModelIds' | 'defaultModel'> &
+ HostResolvedConnectionCatalog,
+): readonly ModelCatalogEntry[] {
const enabledIds = new Set(connectionEnabledModelIds(connection));
- const visibleIds = new Set(enabledIds);
- for (const choice of savedChoices) {
- const id = typeof choice === 'string' ? choice.trim() : choice?.id.trim();
- if (id) visibleIds.add(id);
- }
- return filterUnsupportedCodexModels(
- connection.providerType,
- buildConnectionModelCatalogEntries({ connection, savedModelIds: savedChoices }),
- )
- .filter((entry) => visibleIds.has(entry.id) && (
- entry.canUseAsChatDefault || entry.provenance.sources?.userChoice?.includes('daily_review_model')
- ))
- .map((entry) => enabledIds.has(entry.id) ? entry : { ...entry, canUseAsChatDefault: false });
-}
-
-function selectableCatalogEntries(
- connection: Pick<
- LlmConnection,
- 'slug' | 'providerType' | 'defaultModel' | 'models' | 'modelSource' | 'modelsFetchedAt'
- >,
- savedModelIds?: Iterable<string | undefined | null>,
-): ModelCatalogEntry[] {
- const entries = filterUnsupportedCodexModels(
- connection.providerType,
- buildConnectionModelCatalogEntries({ connection, savedModelIds }),
- ).filter((entry) => entry.canUseAsChatDefault);
- if (entries.length > 0 || connection.providerType !== 'openai-codex') return entries;
- return filterUnsupportedCodexModels(
- connection.providerType,
- buildConnectionModelCatalogEntries({
- connection: {
- ...connection,
- defaultModel: '',
- models: undefined,
- modelSource: undefined,
- modelsFetchedAt: undefined,
- },
- savedModelIds,
- }),
- ).filter((entry) => entry.canUseAsChatDefault);
-}
-
-function filterUnsupportedCodexModels(providerType: ProviderType, entries: ModelCatalogEntry[]): ModelCatalogEntry[] {
- if (providerType !== 'openai-codex') return entries;
- return entries.filter((entry) => !CODEX_SUBSCRIPTION_UNSUPPORTED_CHATGPT_MODELS.has(entry.id.trim()));
+ return connection.catalogEntries.filter(
Review Comment:
[P2] Keep legacy unsupported Codex ids out of Daily Review
A persisted Codex connection can still have `gpt-5-codex` in `enabledModelIds`. `normalizeOpenAiCodexConnection()` removes that id from `models` but not from the enabled list, so the Host catalog builder adds it back as a missing saved entry with `canUseAsChatDefault=true`. This new filter then offers it. On this exact head, `buildCatalogDailyReviewModelOptions()` returned `codex::gpt-5-codex`, and readiness admitted the same requested model, leaving the scheduled run to fail at the provider. Chat avoids this only because its separate picker still checks the unsupported set.
Please make the Host's single resolver omit unsupported Codex enabled ids (or retain the consumer guard) and add a Daily Review regression with a legacy enabled `gpt-5-codex` row.
---
> **Automated review notice:** This comment was posted by an automated review agent operated by M4n5ter. It is not an independent human review and does not replace one.
##########
apps/desktop/src/renderer/settings/use-connection-detail.ts:
##########
@@ -306,8 +306,11 @@ export function useConnectionDetail(props: ConnectionDetailProps) {
setEnabledModelIds(connectionEnabledModelIds(connection));
}, [connection.defaultModel, connection.enabledModelIds, connection.slug]);
- // Picker entries come from the same catalog merge path as Chat and Daily
- // Review, but use the local unsaved editor draft for model/default changes.
+ // The one client-side resolution left on a saved connection: the editor
+ // shows the unsaved draft — model rows just fetched, ids just ticked — which
+ // the Host has not been told about and so cannot have resolved. Everything
+ // the user has committed is read from `connection.catalogEntries`; this
+ // resolves only what is still in the draft.
const modelChoices = buildConnectionModelCatalogEntries({
Review Comment:
[P2] Preserve the Host catalog when the saved connection has no draft changes
This call runs even on the first render, when `models`, `modelSource`, and `enabledModelIds` are just copies of the committed connection. It never reads `connection.catalogEntries`, despite the comment above. That means a Desktop attached to a differently versioned Host still replaces the Host's display names and eligibility decisions with its own bundled metadata. In a production-function probe, a Host entry named `Host-only image model` with `canUseAsChatDefault=false` was rebuilt locally without that name and with `canUseAsChatDefault=true`; `EnabledModelManager` would offer it.
Please use `connection.catalogEntries` while the editor matches the committed snapshot, and enter the documented client-side resolution exception only after model/default/enabled draft state actually diverges. A regression should deliberately give the Host and Desktop conflicting metadata and assert that an unchanged editor renders the Host value.
---
> **Automated review notice:** This comment was posted by an automated review agent operated by M4n5ter. It is not an independent human review and does not replace one.
##########
packages/cli/src/runtime-host-tui-context.ts:
##########
@@ -25,7 +25,10 @@ import {
executionBoundaryDisplayMode,
} from '@maka/core/sandbox-boundary';
import { findProjectByIdentity } from '@maka/core/project';
-import type { ConnectionCatalogEntry, ConnectionCatalogSnapshot } from '@maka/core/runtime-policy';
+import type {
+ RuntimeHostConnectionCatalogEntry as ConnectionCatalogEntry,
Review Comment:
[P2] Seed the initial TUI context window from the Host-resolved choice
The new client catalog type exposes the resolved entries, and `projectRuntimeHostModelChoices()` already carries their context windows, but lines 188–190 still initialize `modelContextWindow` from `selectedTarget.connection.models`. Fallback/default models commonly exist only in the resolved catalog. An exact-head probe with empty stored `models` produced a Host choice context window of 1,050,000 while the startup lookup returned `undefined`, so the initial status and diagnostics omit their denominator until a later model/session transition refreshes it.
Please select the exact connection/model from `modelChoices` for the initial value and cover startup on a fallback-only model.
---
> **Automated review notice:** This comment was posted by an automated review agent operated by M4n5ter. It is not an independent human review and does not replace 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] refactor(runtime-host): make the Host the authority for the model catalog [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4411:
URL: https://github.com/apache/maka/pull/4411#discussion_r3900324172
##########
packages/core/src/runtime-policy/connection-catalog-codec.ts:
##########
@@ -65,6 +65,13 @@ import {
export const CONNECTION_CATALOG_MAX_CONNECTIONS = 1_024;
export const CONNECTION_CATALOG_MAX_MODELS_PER_CONNECTION = 2_048;
export const CONNECTION_CATALOG_MAX_ENABLED_MODEL_IDS = 512;
+/**
+ * A resolved entry exists for every stored model, for every enabled id the
+ * inventory never listed, and for the connection default when it lists none —
+ * so the entry bound is the sum of the two lists it draws from, plus one.
+ */
+export const CONNECTION_CATALOG_MAX_ENTRIES_PER_CONNECTION =
Review Comment:
Confirmed and fixed in ab572cbd23. Reproduced the production path: at the storage maxima (2,048 models, 512 enabled ids) a `volcengine-agent-plan` connection resolves to 2,578 entries against a bound of 2,561 — the prepended registry inventory is the missing third term, as you describe.
Rather than widen the number by hand, the bound now derives its new term from the registry: `MAX_PREPENDED_FALLBACK_MODELS` asks each provider whose discovery kind is `fallback` how large its shipped inventory is, so a provider added or a curated list grown moves the bound with it. Providers that do discover models substitute their fallback list instead of prepending it, so they contribute nothing. The fallback resolution the constant and `buildConnectionModelCatalogEntries` share is one function now instead of two copies.
Regression added in `packages/core/src/__tests__/model-catalog.test.ts`: it walks every registered provider at both storage maxima, asserts each resolved catalog fits the bound, and then asserts the bound equals the largest catalog any provider reaches — so a bound that drifts above what is reachable fails as well.
_🤖 Addressed by [Claude Code](https://claude.com/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] refactor(runtime-host): make the Host the authority for the model catalog [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #4411:
URL: https://github.com/apache/maka/pull/4411
--
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] refactor(runtime-host): make the Host the authority for the model catalog [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4411:
URL: https://github.com/apache/maka/pull/4411#discussion_r3900484594
##########
packages/cli/src/runtime-host-tui-context.ts:
##########
@@ -25,7 +25,10 @@ import {
executionBoundaryDisplayMode,
} from '@maka/core/sandbox-boundary';
import { findProjectByIdentity } from '@maka/core/project';
-import type { ConnectionCatalogEntry, ConnectionCatalogSnapshot } from '@maka/core/runtime-policy';
+import type {
+ RuntimeHostConnectionCatalogEntry as ConnectionCatalogEntry,
Review Comment:
Fixed in 39a7a51 — the opening context window now comes from the Host-resolved `modelChoices` rather than the connection's stored rows. A provider without a model-list endpoint stores none, so its models exist only in the resolved catalog and the very first status line had no denominator until some later transition refreshed it; every later read of the value already came from `modelChoices`. Regression in `runtime-host-onboarding.test.ts` projects a fallback-only Codex connection with `models: []` and asserts every choice still carries a numeric context window.
_🤖 Addressed by [Claude Code](https://claude.com/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