refactor(models): unify connection-scoped model configuration

[Astro-Han]

Summary

Model configuration had two user declaration sources, and the renderer could rebuild a Host catalog from older local metadata. Disabling a model also removed its configuration, while one context-window field meant both model capacity and the user's compaction target.

I consolidated manual model registration and parameter edits into modelOverrides, keyed by exact model ID inside each connection. The existing connection catalog owns the records and enabled selection in one atomic write. Discovery only replaces provider inventory; disabling a model or clearing its last parameter preserves its record. The Host resolves effective execution models and catalog entries. The renderer consumes that catalog and holds only the current edit draft. Single-model saves merge against the latest connection and reject a stale draft for the same model.

The add/edit form now shares display name, image input, total context capacity, input limit, compaction threshold, maximum output, and supported thinking/Fast controls. Capacity is optional; an absent compaction threshold disables proactive compaction. Image settings offer automatic, supported and unsupported choices, with the automatic result visible when known. Chinese labels use concise settings terms; tooltips explain the model-information fallback and available thinking levels. Every model has a ghost configuration icon with a tooltip beside its enable switch. Adding and editing share a 440px Astryx form dialog with vertically stacked fields and field help in tooltips. Clicking the configuration icon keeps model rows in place; closing the dialog returns focus to that icon. Add is the primary action. Storybook discovery now persists its refreshed inventory and rebuilds the Host projection; relay examples use connection-local model slugs without OpenRouter routing prefixes. I removed bulk thinking edits, client catalog rebuilding, whole-table draft synchronization, metadata-coverage/provenance fields, and the old model-facts projection/fingerprint paths and their redundant tests.

Total context capacity and maximum input tokens are independent facts. Both can be overridden or cleared in the shared form; each independently falls back to connection inventory and then model metadata. Core resolves these facts once for the Host catalog and Runtime. Only Runtime derives the usable input bound from the resolved limits. An input limit above the total context window is shown as a form error and rejected before the atomic catalog write; Runtime also rejects conflicting resolved limits. Changing total capacity never creates or updates an input-limit override.

Explicit output limits are per-request budgets, including thinking tokens, scoped to the exact model in one connection. Runtime honors them across adapters and bounds them by known model capacity. Budgets no longer overwrite model capacity in projections. Without an explicit budget, the existing provider-specific catalog defaults remain unchanged; Anthropic fixed thinking is reserved inside the wire limit.

Migration and compatibility

• connection-catalog.json schema 2 is the single writable declaration document. Keeping records and selection in this existing document preserves the current atomic publication and recovery boundary.
• Schema 1 reads convert relayModelProfiles and matching legacy model-facts.json values into connection-scoped records. Old declared context windows seed capacity and compaction policy, but never synthesize an input limit. Only explicitly declared input limits migrate. No repair heuristic guesses whether an already-written schema 2 input limit was derived; it is now visible and independently clearable. The next successful catalog write persists schema 2 atomically; later reads ignore the legacy file. Malformed legacy input fails closed without overwriting it.
• This is a Nightly breaking change. Older builds cannot read schema 2, and Client/Host compatibility epoch is 149; update both together.

Verification

Final head: 41f1e0f26, rebased onto main d2e1be5db. The full required CI passed on this head. Merged as de134f9b1 on September 12, 2026 at 18:13 UTC.

• CI regression fixed: the Codex history-compaction fixture reduced total capacity to 32K but inherited a larger metadata input limit under the new per-field resolver. It now explicitly declares both test capacities. All 37 execution-model-composition tests pass, including native rejection followed by a text checkpoint. Production conflict validation and the existing fallback assertions remain intact. Format and lint pass on the current head.
• Persistence and limits: 21 Core catalog/codec, 4 production storage save/restart, 10 Runtime capacity/policy and 30 Host catalog/coordinator tests passed. Coverage includes independent overrides and clearing, migration without a fabricated input limit, disabled-model retention, connection isolation, stale saves and byte-for-byte unchanged storage after a conflicting save. Restoring the old derived-input projection makes the independent-limit regression fail.
• Save chain and UI: 17 IPC tests pass with the production Renderer service, IPC handlers and disk-backed store; dropping the forwarded patch fails the canonical snapshot assertion. The existing ModelParameterSave Storybook interaction passes in Chinese and English: image support and both limits survive save/reopen, disabled models remain disabled, conflicting limits are rejected, cancel discards changes and focus returns to the configuration icon. Reversing the image-support mapping makes the interaction fail.
• Output budgets: 288 affected Runtime, 30 Core and 77 storage tests passed during implementation. Eight adapter paths cover catalog save/reopen through ModelAdapter to HTTP request bodies, including clearing and capacity bounds. Restoring the old protocol gate makes five regressions fail. Responses are controlled fixtures, without live provider generation.
• Integration: Host, UI and Desktop main builds, Desktop typechecks, 35 credential-render/save-chain tests, Storybook build, locale hygiene, format/lint, renderer architecture and E2E budget checks passed after the rebase. Light/dark dialogs were checked at matching comparison dimensions; English also passed at 480px without horizontal overflow or console errors.

The two model-settings Electron journeys and their exclusive fixture were removed because neither asserted an Electron-owned boundary. Contract coverage stays in unit/integration tests and dialog interaction in Storybook. The Electron budget is 35 tests in 21 files, one fewer than the rebased main. No retries or timeouts were added. The full repository suite was not run locally.

Comparisons use the original main 41da161d6 BEFORE baseline and 283fd6aa4 AFTER capture, at matching light/dark 1280 × 1600 editor and 1280 × 1000 add-model viewports. These captures already show the independent input limit, final concise labels and connection-local model slugs. Subsequent changes do not alter the captured form layout or visible labels; the output-limit tooltip wording changed from “per request” to “per reply” and is not expanded in these images.

AI use

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Maka authored the initial vision hint and test. Codex reviewed and revised persistence, migration, Host projections, runtime policy, parameter editing, tests, Storybook, and comparison images.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[ARE404]

Standards

No documented standards violation or actionable correctness issue was found on this axis. Two non-blocking naming debts remain: model-thinking.ts now owns general model metadata and capacity projection despite its thinking-specific name, and connection-catalog-codec.ts still calls generalized modelOverrides relay profiles in helper names and diagnostics. Commit attribution, formatting, lint, and git diff --check are clean.

Spec

One actionable P1 finding is attached inline: the new universal Maximum output setting is stored and projected correctly but is omitted from requests for most provider wires. No other missing requirement or scope creep was established in the reviewed Renderer → IPC → Host → Runtime path. I also checked disable/clear retention, discovery retention, stale same-model draft rejection, capacity/compaction semantics, image/thinking/Fast propagation, and schema-1-only legacy model-facts.json migration.

This review is pinned to bb610b1b465b8f61191e4728a0005f601ee95c2d. The hosted test check was still running when this review was submitted; I did not make a live provider call, so the output-limit finding is established from deterministic request lowering and the existing Mistral request-level unit test.

Summary: Standards — 0 actionable findings; Spec — 1 P1 finding.

---

Automated review notice: This comment was posted by an automated review agent operated by ARE404. It is not an independent human review and does not replace one.

[jackwener]

Scoped review at bb610b1b465b8f61191e4728a0005f601ee95c2d. I took the storage, migration and concurrency surface; a second reviewer under a different owner is covering the renderer-to-host data chain separately. This is a partial review by design, and I say at the end what I did not cover.

I found nothing that blocks on this surface. The claims I was asked to verify from source rather than from the description hold.

The legacy store is genuinely demoted, not merely described as demoted. The old facts module now exports a single reader class whose only method reads with diagnostics; no write, save or delete path remains, and nothing outside tests references it. The type name states the role. This is the kind of claim that is easy to assert in a description and easy to leave half-done in code, so it is worth recording that the code matches.

Publication is atomic in the way this kind of document needs. The write goes to a uniquely named temporary file opened exclusively with restrictive permissions, is flushed to disk, then renamed into place, and the containing directory is synchronized afterwards. A temporary file left by a failure before publication is removed. Most usefully, the failure path distinguishes three outcomes rather than two: success, a failure before publication, and a commit whose outcome is genuinely unknown because the rename succeeded but a later step did not — that last case tells the caller to reload before retrying instead of reporting a clean failure it cannot substantiate.

Concurrent mutation of the same connection is serialized, and I checked the mechanism rather than the name. Every catalog mutation runs through a lane whose run captures the previous tail, installs a new one, and awaits the previous completion before executing. A read-modify-write therefore cannot interleave with another, so two saves landing together cannot both compute the same next revision and have one silently overwrite the other. Discovery additionally carries a per-connection revision and is rejected outright if that revision has moved, so a stale discovery result cannot be admitted even from outside the lane.

Discovery does not overwrite user declarations. The reconciled entry carries the previous overrides forward rather than rebuilding them, and the reconciler is told whether an inventory already exists so that an empty selection against a known inventory is treated as the user's answer rather than as missing data. Where discovery would produce a default outside the selection it just computed, the write fails closed with an attributable error instead of publishing an inconsistent document.

What I did not cover. I did not review the renderer-to-host data chain, model resolution, or how image, thinking and output parameters reach an actual request; I did not review the capacity and compaction-threshold default and clearing semantics; and I did not review the shared add-and-edit dialog's save, cancel, close and focus behaviour, or whether a second authority or duplicate path survives on the interaction side. Those are the other two risk surfaces and are not mine in this pass. Within my own surface I did not exercise restart or crash recovery, and I did not test failure and retry behaviour against a real filesystem — my reading of the publication path is from source, not from an induced failure.

Verification limits and state. I read code only; I ran no tests, build or application. This pull request is a draft and its hosted check was still running when I posted, so I am not making any statement about check results. The head has moved several times today; this review is bound to the commit named above and I re-read it immediately before posting.

---

Automated review notice. This review was produced by an automated reviewer (agent seat kabi-opus, model Claude Opus) acting on its own review identity. It is posted from a shared machine account, so the seat is named here because the account alone does not identify the reviewer. It is not an independent human review and does not substitute for one.

[jackwener]

Supplementary pass at bb610b1b465b8f61191e4728a0005f601ee95c2d, covering the interaction surface my earlier review on this commit left out. To be clear about what this is and is not: this is a second pass from the same reviewer and the same owner as that earlier review, not an additional independent seat. It adds coverage, not independence.

The shared add-and-edit dialog's lifecycle is sound where I checked it. Closing resets every field the dialog owns — the identifier, the parameter draft, the numeric inputs, the context-window text and the submit-attempted flag — before it reports the close upward, so a discarded draft cannot reappear on the next open. The close path is additionally gated so it cannot fire while a save is in flight.

The one place the dialog deliberately does not reset is after a failed submit, and the reasoning stated beside it is sound: an exact model identifier is not something a user can retype from memory, the failure surfaces through the caller's notification, and what the dialog owes the user at that moment is the text they typed, still there to retry from. That is a considered trade, not an oversight.

A path I examined and am explicitly not reporting as a defect. The dialog is rendered unconditionally and shown through an isOpen prop rather than being mounted on demand, and the row it belongs to is tracked in a single-slot state shared with the other editable rows. In principle, moving that state to a different row while this dialog is open would hide it without running its reset, leaving a stale draft for the next open. I could not establish that this is reachable: the other rows' handlers only run when those rows are themselves in edit mode, and reaching them would require interacting with the surface underneath an open dialog. I did not verify the dialog's modality, so I am recording this as examined-and-unresolved rather than presenting a path I have not shown to exist.

On the second-authority question from the other direction. My earlier pass established from source that the legacy facts module retains only a reader and has no write path or non-test caller, and the independent reviewer covering the data chain reached the same conclusion from the projection side. Two passes entering from storage and from the renderer respectively agreeing that there is one write authority is worth more than either statement alone.

Coverage and limits. This pass covered the shared dialog's save, cancel, close and focus-relevant state lifecycle, and the duplicate-authority question as it appears from the storage side. I did not review model resolution, the request parameters, or the capacity and compaction-threshold semantics — those belong to the data-chain pass, which is published separately. I read code only and ran nothing; the pull request is a draft and I make no statement about its checks.

---

Automated review notice. This review was produced by an automated reviewer (agent seat kabi-opus, model Claude Opus) acting on its own review identity. It is posted from a shared machine account, so the seat is named here because the account alone does not identify the reviewer. It is not an independent human review and does not substitute for one.

[ARE404]

Final repair re-review

The repair closes the previously reported output-budget P1. I found no new actionable issue in bb610b1b..ae186f0a: the exact connection/model override now reaches the shared streamText seam for every adapter, is capped by known model capacity, keeps catalog capacity separate from the request budget, and reserves fixed Anthropic thinking inside the total wire limit. The persisted-save/reopen-to-HTTP conformance coverage exercises the representative request shapes and preserves provider-default behavior when the override is cleared.

The exact head is nevertheless not merge-ready. One P1 finding is attached inline: a PR-introduced extensionless local import prevents the compiled Desktop Node ESM test graph from loading. Required test run 34698409184 is terminal red, with seven Desktop test files failing from the same ERR_MODULE_NOT_FOUND.

This review is pinned to ae186f0a0e6b0c0cccc44ecd3ec67ac347cb222e. Summary: repair delta — 0 new P0-P3 and prior P1 closed; exact head — 1 P1 and required CI red.

---

Automated review notice: This comment was posted by an automated review agent operated by ARE404. It is not an independent human review and does not replace one.

[jackwener]

Cross-layer review at d37494aecc22c50e142fdfc3b44be1326d932c8b, and a formal answer on the output-budget finding I was asked to close.

The output-budget issue is closed, and here is my own basis rather than a restatement of anyone else's. The projection helper now destructures the output budget out of the override before spreading the remainder, so a user's budget no longer overwrites the model's capacity fact — the two are separate values rather than one shadowing the other. On the request side, the selection reads the user's budget from the connection's per-model override and the capacity from the projected model or the metadata table, then takes the smaller of the two. Where no budget is set, the previous behaviour is preserved exactly: adapters that never carried a wire limit still return none, so the earlier regression this guarded against is not reintroduced. Where a budget is set, every adapter now honours it. I read this at this commit; I did not run it.

On whether another entry bypasses the budget — the question I was asked. For the reply path, no. The turn reads the limit through a single accessor on the adapter, which is the only place the selection above is computed, so there is one funnel rather than several.

There are, however, separate request paths that set their own output limit and never consult the user's budget: history compaction and its summarizer, the goal evaluator with a fixed small limit, and the connection test with a token-sized one. I do not consider these a bypass, because the stated product boundary separates context capacity and the user's own compaction threshold, and applying a per-reply limit to an internal summarization or a liveness probe would be wrong rather than right.

I would raise one wording consequence of that, non-blocking. The tooltip describes the setting as applying to a single request and as including thinking. A user who sets a small budget to bound cost may read "a single request" as "every request the app makes on my behalf", which is not what it governs — compaction in particular will still use its own, potentially larger, limit. The implementation matches the stated boundary; it is the description that does not distinguish the reply path from the internal ones.

On the Electron tests removed in this round. The replacement story is stronger than what it replaces on the part that genuinely needs a browser. The deleted journey worked around the save button's blur-driven enabled state by clicking raw coordinates to avoid an actionability wait; the story instead asserts that the field still holds focus and that save is already enabled at that moment, then that focus returns to the trigger after both save and cancel, that an invalid abbreviation leaves save disabled, that the abbreviated value is stored expanded, and that a disabled model stays disabled through the edit. Those are Chromium-owned behaviours — focus and submit ordering, native dialog focus restoration — and the story says so in its own comment. Lowering that tier is consistent with this repository's admission rules.

What I want to flag is narrower. The budget entry removed alongside the test recorded its own reason for requiring Electron:

the saved window is read back from the Host's connection snapshot, not from renderer state

That is the one protection the replacement does not carry: the story exercises production UI without a real Host, so "the value the UI saved is what the Host hands back on reopen" is now covered by composition — the UI side in the story, the storage side in the lower-layer suites — rather than by one integrated pass. This repository's admission rules name cross-process persistence as a legitimate reason to keep a test in Electron, and this entry claimed exactly that. I am not calling it a defect, because the composed coverage may well be sufficient and I have not established that it is not; I am saying the stated justification for the removed test is not answered by the stated replacement, and that gap deserves an explicit answer rather than an implicit one.

Coverage and limits. This pass covered the projection helper, the request-side selection and its callers, the other request paths that set an output limit, and the tier change for the removed Electron journey. I read code only and ran nothing; the author's local runs are not part of my evidence. I did not re-verify the adapter-by-adapter wire behaviour — that is another reviewer's surface and they have published separately. The hosted check was still running when I posted, so I make no statement about it.

---

Automated review notice. This review was produced by an automated reviewer (agent seat kabi-opus, model Claude Opus) acting on its own review identity. It is posted from a shared machine account, so the seat is named here because the account alone does not identify the reviewer. It is not an independent human review and does not substitute for one.

[ARE404]

Standards

The .js import repair is correct, the three delta commits are conventionally named with Generated-by: Codex, git diff --check passes, and the E2E budget is internally consistent at 34 tests in 20 files. The migration does not fully meet the repository's test-tier rules, though: two independent protections were deleted rather than transferred, and the new Product story is missing the required literal // Real path: annotation. Details are attached inline as two P2 findings and one P3.

Spec

The new ModelParameterSave story is a stronger browser-level oracle for focused-input save readiness, invalid-input disabling, focus restoration, reopen display, and cancel discard. Parser grammar, direct Host merge/stale-write handling, catalog persistence/restart, disabled-record retention, clearing, connection isolation, projection, and request lowering also retain focused lower-tier coverage.

Two protections remain partial: no test now composes the production Renderer service with the model-parameter Host write/snapshot, and no browser/component test touches the Image selector at all. The former leaves a service-forwarding regression invisible; the latter leaves the exact Supported/Automatic UI-to-declaration mapping invisible even though downstream tests accept prebuilt vision values.

This review is pinned to d37494aecc22c50e142fdfc3b44be1326d932c8b. Required test run 34702309196 was still in progress at submission. Summary: Standards — 3 findings (worst P2); Spec — 2 missing protections (worst P2).

---

Automated review notice: This comment was posted by an automated review agent operated by ARE404. It is not an independent human review and does not replace one.

[jackwener]

Re-review at 6645997822b362c82237176ab67c0e40788c9db9. Two things I raised are resolved, one of them more precisely than I asked for, and I have one new observation about a constraint that left with the code it guarded.

The build failure is genuinely fixed, and the diagnosis matches what I found. I had reported a type error naming a property that does not exist on the catalog entry, and noted the oddity that the symbol was absent from the branch while the error pointed at a line beyond the branch file's length. The cause is that checks compile the merge result rather than the branch: the property lived at the merge base — in the entry type, its persistence codec, a production consumer and a dedicated test — and the merge combined this change's removal with a fixture from main that still referenced it. The current head removes the property from every one of those places rather than only from the fixture, so the removal is complete rather than a patch over the symptom.

The wording issue I raised is fixed, and fixed better than I suggested. I had said the help text describing the budget as applying to "a single request" invites a user to read it as every request the application makes, while compaction and internal calls keep their own limits. The output budget's text now says "a single reply". What I want to note is that the input limit's text still says "a single request" — and that is correct, because an input limit genuinely does bound every request including the internal ones. A blanket rename would have made the second string wrong; distinguishing the two is the accurate fix.

A new observation, non-blocking. Removing the metadata-coverage property also removed the filter it fed, and with it a constraint the code stated explicitly. At the merge base, the set of models offered a hand editor was computed as those present in the catalog but not covered by metadata, with a comment recording why: treating absence as "no metadata" would grow an editor the mainline never showed, and only a present-but-uncovered entry needed it, referencing the issue that produced that rule. That filter, the set derived from it, and the bulk control gated on its size are all gone at this head.

Part of that is clearly intended: removing the bulk thinking-level control is a stated boundary for this change, so it needs no defence. The editor-visibility rule is different. This change makes both capacity limits independently editable and clearable for every model, which supersedes a filter whose purpose was to decide who gets an editor at all — so I read the removal as deliberate rather than accidental. But the constraint was written down with an issue reference precisely so a later change would not reintroduce what it prevented, and it has now left without a replacement statement. I am not asking for the filter back; I am asking that the change say it supersedes that rule, so the next reader finds an answer rather than an absence. The same issue number still appears elsewhere in the file for an unrelated reason, which makes a silent removal easier to miss.

Coverage and limits. This pass covered the merge-result build failure and its removal across type, codec, consumer and tests; the help-text change; and the consequences of removing the coverage property in the settings surface. I read code only and ran nothing, and the author's local results are not part of my evidence. The hosted check was still running when I posted, so I make no statement about it. The output-budget contract I reviewed earlier is unchanged by this increment. Whether the two contracts an independent reviewer found unmigrated are now answered is their call on their own evidence, not mine to assert.

---

Automated review notice. This review was produced by an automated reviewer (agent seat kabi-opus, model Claude Opus) acting on its own review identity. It is posted from a shared machine account, so the seat is named here because the account alone does not identify the reviewer. It is not an independent human review and does not substitute for one.

[jackwener]

Approve on code review at 6645997822b362c82237176ab67c0e40788c9db9. This approval speaks to the code I read. It is not a statement that checks pass, and it is not a merge recommendation — the hosted check is failing on this commit and I say below what it is and what I did not establish about it.

The two contract gaps another reviewer raised are resolved, and I verified each myself rather than accepting that they were addressed.

The missing oracle for the settings-to-catalog round trip. The new integration test is a real round trip, not a restatement: it creates a temporary storage root, acquires the interactive owner lease, opens the real policy stores for write, builds the production renderer-side connection settings services, registers the actual IPC handlers and forwards through them. Its assertions then read from the store rather than from the service's own state — the saved overrides are compared against a store snapshot, a reopen is compared again, and a rejected update is followed by asserting the store snapshot is byte-for-byte what it was before. That last one is more than was asked for: it pins that a refused write leaves nothing behind.

The image declaration mapping with no remaining browser coverage. The story now selects the explicit "allow images" option, saves, reopens and asserts the control still reads that value; then switches to the metadata-derived option, saves, reopens and asserts it reads that one. Both pass through save and reopen rather than checking the control in place, and both run across the locales the story covers, while the same steps keep asserting the capacity value, the input limit and that a disabled model stays disabled. The new independent input-limit rule is pinned alongside it: an input limit above the total capacity leaves save disabled, and a valid one enables it.

The wording point I raised is resolved precisely. The output budget's help text now says a single reply, while the input limit's text still says a single request — which is correct, because an input limit does bound every request including internal ones. A blanket rename would have made the second string wrong.

The editor-visibility question I raised is answered. Making every model's parameters editable and clearable, regardless of whether it is enabled or covered by catalog metadata, is a deliberate product decision that supersedes the older rule limiting the hand editor to uncovered entries, with the bulk thinking control removed in the same move. My ask was that the change state the supersession rather than let a documented constraint disappear silently; it has been stated.

The check on this commit is failing, and here is what I know and do not know about it. The build error I reported earlier is gone. What fails now is the Runtime Host test step, where Codex OAuth history compaction falls back to a text checkpoint after native rejection is reported as failing. I did not establish whether this change causes it. What I can say is that the file containing that test is not touched here, while this change does modify compaction-budget production code — so a connection is plausible and unproven, and that is where I would look first rather than assuming either way. I am not treating this as resolved, and I am not treating it as unrelated.

What this approval covers, and what it does not. I reviewed: the storage and migration surface, including that the legacy facts module retains only a reader with no write path or non-test caller; the write-authority question, including that the adapter-level fix reads the override and capacity without introducing a second writer; the cross-layer budget contract, including that the reply path funnels through one accessor while compaction, the goal evaluator and the connection probe keep their own limits by design; the shared dialog's save, cancel, close and focus lifecycle; the tier change for the removed Electron journeys and where their protection now lives; and the two resolutions above.

I ran nothing. No test, build, Storybook run or application session of any kind was executed by me, so no statement here rests on my own execution. The author's local results are theirs and are not part of my evidence. The adapter-by-adapter wire behaviour was reviewed independently by a reviewer under a different owner and is their conclusion, not mine. A third independent owner was sought for this change and none was available, so the coverage here is two owners rather than three.

---

Automated review notice. This review was produced by an automated reviewer (agent seat kabi-opus, model Claude Opus) acting on its own review identity. It is posted from a shared machine account, so the seat is named here because the account alone does not identify the reviewer. It is not an independent human review and does not substitute for one. No merge was performed.

[github-actions]

No description provided.