You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "mikemikimike (via GitHub)" <gi...@apache.org> on 2026/08/22 15:52:08 UTC
[PR] fix(runtime-host): reconcile retired fallback models [maka]
mikemikimike opened a new pull request, #3517:
URL: https://github.com/apache/maka/pull/3517
(no comment)
--
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(runtime-host): reconcile retired fallback models [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #3517:
URL: https://github.com/apache/maka/pull/3517#discussion_r3836658180
##########
packages/runtime-host/src/server/bootstrap-runtime-policy.ts:
##########
@@ -196,6 +197,11 @@ const LEGACY_OPENCODE_FREE_SEEDS: readonly (readonly string[])[] = [
['nemotron-3-ultra-free', 'mimo-v2.5-free', 'deepseek-v4-flash-free'],
];
+function retiredOpencodeFreeModelIds(): readonly string[] {
Review Comment:
For the record, since my earlier comment cast doubt on this function: it behaves correctly.
Computing retired ids as `(union of legacy seeds) − (current defaults)` means a model that is still active, free and tool-capable can never be classified as retired, however many past seeds mentioned it. `big-pickle` is exactly that case, which is why it survives — and why it should.
The failing assertion expects it removed, so that's where the fix belongs.
--
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(runtime-host): reconcile retired fallback models [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on PR #3517:
URL: https://github.com/apache/maka/pull/3517#issuecomment-5384242486
**CI has now run on this head and confirms the [P1] independently.** Run [32612843956](https://github.com/apache/maka/actions/runs/32612843956), `Run Runtime Host tests`:
```
✖ reconciles retired OpenCode Free models without removing user models (24.842023ms)
✖ failing tests:
code: 'invalid_document',
```
That is the same test, the same failure, and the same `invalid_document` error the review reproduced locally before CI had ever executed.
**Worth separating two facts that looked identical from the outside:** this head previously showed no red mark, because the only workflow was parked at `action_required` with zero jobs — **the tests had never started.** After approving the run, the red appeared immediately. The absence of a failure signal was an absence of *any* signal.
No further action needed from the review side; the fix is described in the inline comments.
--
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(runtime-host): reconcile retired fallback models [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #3517:
URL: https://github.com/apache/maka/pull/3517
--
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(runtime-host): reconcile retired fallback models [maka]
Posted by "mikemikimike (via GitHub)" <gi...@apache.org>.
mikemikimike commented on code in PR #3517:
URL: https://github.com/apache/maka/pull/3517#discussion_r3837768518
##########
packages/storage/src/runtime-policy/connection-catalog-document.ts:
##########
@@ -352,37 +352,48 @@ export class ConnectionCatalogDocumentOwner {
(item) => item.slug === input.slug && item.providerType === input.providerType,
);
const previous = current.connections[index];
+ const retired = new Set(input.retiredModelIds);
const sameIds = (left: readonly string[], right: readonly string[]) =>
left.length === right.length && left.every((id, position) => id === right[position]);
- if (
- !previous ||
- sameIds(previous.enabledModelIds, input.enabledModelIds) ||
- !input.legacyEnabledModelIds.some((seed) => sameIds(previous.enabledModelIds, seed))
- ) {
+ const isLegacySeed = previous
+ ? input.legacyEnabledModelIds.some((seed) => sameIds(previous.enabledModelIds, seed))
+ : false;
+ const hasRetiredModels = previous
+ ? previous.enabledModelIds.some((id) => retired.has(id)) ||
+ previous.models.some((model) => retired.has(model.id))
+ : false;
+ if (!previous || (!isLegacySeed && !hasRetiredModels)) {
return committed(current);
}
- const fallbackModels = fallbackInventory(previous.providerType);
+ const fallbackModels = isLegacySeed
+ ? fallbackInventory(previous.providerType)
+ : previous.models.filter((model) => !retired.has(model.id));
const {
lastTest: _lastTest,
modelSource: _modelSource,
modelsFetchedAt: _modelsFetchedAt,
...retained
} = previous;
+ const migratedEnabledModelIds = isLegacySeed
+ ? [...input.enabledModelIds]
+ : previous.enabledModelIds.filter((id) => !retired.has(id));
const connections = [...current.connections];
connections[index] = {
...retained,
revision: nextRevision(previous.revision),
- enabledModelIds: [...input.enabledModelIds],
+ enabledModelIds: migratedEnabledModelIds,
models: fallbackModels,
- ...(fallbackModels.length > 0
+ ...(isLegacySeed && fallbackModels.length > 0
? { modelSource: 'fallback' as const, modelsFetchedAt: 0 }
- : {}),
+ : previous.modelSource === undefined
+ ? {}
+ : { modelSource: previous.modelSource }),
Review Comment:
Fixed in c5ba13e. The non-legacy migration now restores modelSource and modelsFetchedAt together, preserving the decoder invariant. The focused bootstrap regression passes 9/9.
##########
packages/storage/src/runtime-policy/connection-catalog-document.ts:
##########
@@ -352,37 +352,48 @@ export class ConnectionCatalogDocumentOwner {
(item) => item.slug === input.slug && item.providerType === input.providerType,
);
const previous = current.connections[index];
+ const retired = new Set(input.retiredModelIds);
const sameIds = (left: readonly string[], right: readonly string[]) =>
left.length === right.length && left.every((id, position) => id === right[position]);
- if (
- !previous ||
- sameIds(previous.enabledModelIds, input.enabledModelIds) ||
- !input.legacyEnabledModelIds.some((seed) => sameIds(previous.enabledModelIds, seed))
- ) {
+ const isLegacySeed = previous
+ ? input.legacyEnabledModelIds.some((seed) => sameIds(previous.enabledModelIds, seed))
+ : false;
+ const hasRetiredModels = previous
+ ? previous.enabledModelIds.some((id) => retired.has(id)) ||
+ previous.models.some((model) => retired.has(model.id))
+ : false;
+ if (!previous || (!isLegacySeed && !hasRetiredModels)) {
return committed(current);
}
- const fallbackModels = fallbackInventory(previous.providerType);
+ const fallbackModels = isLegacySeed
+ ? fallbackInventory(previous.providerType)
+ : previous.models.filter((model) => !retired.has(model.id));
const {
lastTest: _lastTest,
modelSource: _modelSource,
modelsFetchedAt: _modelsFetchedAt,
...retained
} = previous;
+ const migratedEnabledModelIds = isLegacySeed
+ ? [...input.enabledModelIds]
+ : previous.enabledModelIds.filter((id) => !retired.has(id));
const connections = [...current.connections];
connections[index] = {
...retained,
Review Comment:
Fixed in c5ba13e. The non-legacy migration now prunes relayModelProfiles against the migrated enabledModelIds using the existing canonical helper. The focused bootstrap regression passes 9/9.
--
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(runtime-host): reconcile retired fallback models [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on PR #3517:
URL: https://github.com/apache/maka/pull/3517#issuecomment-5384228715
Independent review of `c6536797668c07ce906291da293059341f9d9924`. **One [P1] and one [P2], both inline, both in the new non-legacy migration branch.**
- https://github.com/apache/maka/pull/3517#discussion_r3837704856 — `modelSource` is restored without `modelsFetchedAt`, producing a document that fails `getSnapshot()` on the next read. **This PR's own new test already fails on it** (8 pass / 1 fail at exact head).
- https://github.com/apache/maka/pull/3517#discussion_r3837704872 — `relayModelProfiles` is carried through unpruned while `enabledModelIds` is filtered, breaking the containment invariant independently of the first.
**On CI:** this head had **zero check runs** — one workflow sitting at `action_required`, which renders like "pending" but means it never started. Approved as a maintainer so it can actually report. **The absence of a red mark here was not evidence of green.**
**The direction is right and worth saying so.** Deriving the retired set by subtracting current defaults from historical seeds is a principled way to bound fallback drift, and generalizing from "replace the exact legacy seed wholesale" to "incrementally filter user-modified rows against a tombstone set" is the correct move. `big-pickle` correctly stays active because it is still a current default, and the revised historical test using it instead is a genuine improvement over the previous fixture.
**Both findings share one cause**, which is why they are worth fixing together rather than patching individually: the new branch hand-assembles the persisted row and, in doing so, steps around two canonicalization invariants that every neighbouring path in the same file honours. The write side never sees the decoder, so both violations surface on the *next* startup instead of the one that created them — which is the worst place for a migration bug to land.
Credit where it is due: **the new regression test is what exposed the P1**, reproducibly and without extra setup. A test that catches the change's own defect before review does is doing its job.
*Reviewed at 2026-08-23 12:58 UTC. Blind line — the provisional judgment was sealed before any existing review on this PR was read.*
--
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(runtime-host): reconcile retired fallback models [maka]
Posted by "mikemikimike (via GitHub)" <gi...@apache.org>.
mikemikimike commented on PR #3517:
URL: https://github.com/apache/maka/pull/3517#issuecomment-5383792064
Verified the review correction against model-metadata.generated.ts: big-pickle is active, while deepseek-v4-flash-free is deprecated. Updated the regression to use the deprecated model and retained big-pickle in the user-customization coverage. Pushed c653679; waiting for CI.
--
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(runtime-host): reconcile retired fallback models [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #3517:
URL: https://github.com/apache/maka/pull/3517#discussion_r3837704856
##########
packages/storage/src/runtime-policy/connection-catalog-document.ts:
##########
@@ -352,37 +352,48 @@ export class ConnectionCatalogDocumentOwner {
(item) => item.slug === input.slug && item.providerType === input.providerType,
);
const previous = current.connections[index];
+ const retired = new Set(input.retiredModelIds);
const sameIds = (left: readonly string[], right: readonly string[]) =>
left.length === right.length && left.every((id, position) => id === right[position]);
- if (
- !previous ||
- sameIds(previous.enabledModelIds, input.enabledModelIds) ||
- !input.legacyEnabledModelIds.some((seed) => sameIds(previous.enabledModelIds, seed))
- ) {
+ const isLegacySeed = previous
+ ? input.legacyEnabledModelIds.some((seed) => sameIds(previous.enabledModelIds, seed))
+ : false;
+ const hasRetiredModels = previous
+ ? previous.enabledModelIds.some((id) => retired.has(id)) ||
+ previous.models.some((model) => retired.has(model.id))
+ : false;
+ if (!previous || (!isLegacySeed && !hasRetiredModels)) {
return committed(current);
}
- const fallbackModels = fallbackInventory(previous.providerType);
+ const fallbackModels = isLegacySeed
+ ? fallbackInventory(previous.providerType)
+ : previous.models.filter((model) => !retired.has(model.id));
const {
lastTest: _lastTest,
modelSource: _modelSource,
modelsFetchedAt: _modelsFetchedAt,
...retained
} = previous;
+ const migratedEnabledModelIds = isLegacySeed
+ ? [...input.enabledModelIds]
+ : previous.enabledModelIds.filter((id) => !retired.has(id));
const connections = [...current.connections];
connections[index] = {
...retained,
revision: nextRevision(previous.revision),
- enabledModelIds: [...input.enabledModelIds],
+ enabledModelIds: migratedEnabledModelIds,
models: fallbackModels,
- ...(fallbackModels.length > 0
+ ...(isLegacySeed && fallbackModels.length > 0
? { modelSource: 'fallback' as const, modelsFetchedAt: 0 }
- : {}),
+ : previous.modelSource === undefined
+ ? {}
+ : { modelSource: previous.modelSource }),
Review Comment:
**[P1] The non-legacy branch restores `modelSource` without `modelsFetchedAt`, writing a document that fails its own decoder on the next read.**
```ts
const { lastTest: _lastTest, modelSource: _modelSource, modelsFetchedAt: _modelsFetchedAt, ...retained } = previous;
...
...(isLegacySeed && fallbackModels.length > 0
? { modelSource: 'fallback' as const, modelsFetchedAt: 0 }
: previous.modelSource === undefined
? {}
: { modelSource: previous.modelSource }),
```
Both fields are destructured out of `retained`, but the non-legacy path puts **only `modelSource`** back. The canonical decoder requires them to travel together:
```ts
if ((item.modelSource === undefined) !== (item.modelsFetchedAt === undefined)) {
throw domainError('connection model source and fetched time must occur together');
}
```
— `packages/core/src/runtime-policy/connection-catalog-codec.ts:418-420`
An ordinary OpenCode Free catalog row carries `modelSource: 'fallback', modelsFetchedAt: 0`, so as soon as this migration touches such a row it persists an invalid document. **The next `getSnapshot()` throws `invalid_document`, and Host/catalog operations from that point on are unavailable.** The user does not hit a degraded state; they hit a hard read failure on a document the upgrade itself produced.
**This is not hypothetical — this PR's own new test already fails on it.** Building the exact head and running:
```
node --test packages/runtime-host/dist/__tests__/bootstrap-runtime-policy.test.js
```
gives 8 pass / 1 fail, and the failure is `reconciles retired OpenCode Free models without removing user models`, throwing at the first `getSnapshot()` after bootstrap.
**Why the existing guards did not stop this:** `migrateSystemSeed()` assembles and writes the entry directly, bypassing the canonical decoder on the write side. The invariant is only enforced on read — which is exactly why the failure lands on the *next* startup rather than on the one that caused it.
Minimal fix: restore or reconstruct `modelsFetchedAt` alongside `modelSource` on this branch, the way the legacy path already does with its `{ modelSource: 'fallback', modelsFetchedAt: 0 }` pair.
*Independent blind line, provisional judgment sealed before any existing review was read. Verified at the gate 2026-08-23 12:58 UTC.*
##########
packages/storage/src/runtime-policy/connection-catalog-document.ts:
##########
@@ -352,37 +352,48 @@ export class ConnectionCatalogDocumentOwner {
(item) => item.slug === input.slug && item.providerType === input.providerType,
);
const previous = current.connections[index];
+ const retired = new Set(input.retiredModelIds);
const sameIds = (left: readonly string[], right: readonly string[]) =>
left.length === right.length && left.every((id, position) => id === right[position]);
- if (
- !previous ||
- sameIds(previous.enabledModelIds, input.enabledModelIds) ||
- !input.legacyEnabledModelIds.some((seed) => sameIds(previous.enabledModelIds, seed))
- ) {
+ const isLegacySeed = previous
+ ? input.legacyEnabledModelIds.some((seed) => sameIds(previous.enabledModelIds, seed))
+ : false;
+ const hasRetiredModels = previous
+ ? previous.enabledModelIds.some((id) => retired.has(id)) ||
+ previous.models.some((model) => retired.has(model.id))
+ : false;
+ if (!previous || (!isLegacySeed && !hasRetiredModels)) {
return committed(current);
}
- const fallbackModels = fallbackInventory(previous.providerType);
+ const fallbackModels = isLegacySeed
+ ? fallbackInventory(previous.providerType)
+ : previous.models.filter((model) => !retired.has(model.id));
const {
lastTest: _lastTest,
modelSource: _modelSource,
modelsFetchedAt: _modelsFetchedAt,
...retained
} = previous;
+ const migratedEnabledModelIds = isLegacySeed
+ ? [...input.enabledModelIds]
+ : previous.enabledModelIds.filter((id) => !retired.has(id));
const connections = [...current.connections];
connections[index] = {
...retained,
Review Comment:
**[P2] The same branch filters `enabledModelIds` but carries `relayModelProfiles` through untouched, breaking the second catalog invariant.**
`relayModelProfiles` arrives via `...retained` exactly as it was, while `enabledModelIds` is filtered down by `!retired.has(id)` on the line below. If the user had declared a relay profile for a model that is now retired, the persisted profile key is no longer in the enabled set — and the decoder requires containment:
```ts
if (!enabledModelIds.includes(modelId)) {
throw domainError(`relay model profile for ${modelId} is not an enabled model`);
}
```
— `packages/core/src/runtime-policy/connection-catalog-codec.ts:418` region, profile decode at `:245-250`
**This is independent of the P1 above: fixing `modelsFetchedAt` still leaves this one, and it fails the next read the same way.**
**What makes it clearly an omission rather than a design choice:** every other selection-changing path in this same file already prunes — `:283`, `:472`, `:566` all call `pruneRelayModelProfiles(...)` when the enabled set moves. The new migration path is the only place that narrows `enabledModelIds` without it.
Fix: run `pruneRelayModelProfiles(previous.relayModelProfiles, migratedEnabledModelIds)` on this branch, matching what the neighbouring paths do.
---
**Direction and scope, since they are worth saying separately from the findings.** Subtracting current defaults from historical seeds to derive the retired set, and passing it in at bootstrap, is a sound way to bound historical fallback drift — and `big-pickle` correctly stays active because it is still a current default. Widening from "replace the exact legacy seed wholesale" to "incrementally filter user-modified rows against a tombstone set" is the right generalization.
But both findings live in that new hand-assembled row, and both come from the same cause: **it rebuilds the document by hand and bypasses two canonicalization invariants that every neighbouring path respects.** On net, until they are addressed, this adds entropy to the persistence path rather than removing it — which is the opposite of the change's own goal.
Two smaller notes, neither a blocker: the legacy exact-seed atomic upgrade and the null-default handling are both still correct, and a user-modified row whose retired default is not in the retained selection fails closed to `null` as it should. The comment claiming *"user selection is never touched"* is now stale — documentation debt, not a defect.
*Independent blind line, same seal. Verified at the gate 2026-08-23 12:58 UTC. Exact head had no check runs at all when reviewed; the stalled workflow has now been approved so CI can actually report.*
--
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