Skip to content

fix(models): persist discovery membership without deleting bindings - #1901

Merged
vastsa merged 2 commits into
vastsa:mainfrom
imsinolam:fix/stale-model-cache-1892
Oct 26, 2026
Merged

vastsa merged 2 commits into
vastsa:mainfrom
imsinolam:fix/stale-model-cache-1892

Conversation

@imsinolam

Copy link
Copy Markdown

Summary

Fixes #1892.

Separate saved model bindings from current endpoint publication state. Previously discovered configured models omitted from a complete response no longer return to ordinary/recent Composer choices through binding merging, offline fallback, or restart hydration. This is provider-independent; there is no gateway-specific logic or external-config synchronization.

  • Persist optional published / unlisted discovery state in Host-owned SQLite (schema v25, backed-up v24 migration).
  • Reconcile only explicitly complete, nonempty valid responses. Empty/failed/known-partial responses cannot invalidate missing rows; omitted RPC complete defaults to additive-only.
  • Preserve bindings, user-owned rows, credentials, defaults and history. Never-discovered manual IDs stay selectable; reappearance restores choices; endpoint edits clear old membership evidence.
  • Carry state through live/cache/catalog/fallback IPC paths and filter Composer candidates. Settings retains editable bindings.
  • Add migration/restart/consumer regressions and an isolated real Host → IPC → Composer E2E, plus synchronized specs.

unlisted is not proof of failed inference. Current/default models are not automatically switched or runtime-blocked. Undisclosed server truncation cannot be reliably detected. OAuth list results without a completeness contract remain non-reconciling.

Review notes

The earlier unpushed delete-only candidate was rejected because IPC and Composer re-added configured IDs. This commit replaces that candidate, preserves the original binding-retention invariant, and fixes the consumer boundary. The issue body was corrected to withdraw unsupported 404, binding-provenance and context-window claims.

The schema change is additive and backed up. Older apps refuse schema v25; downgrade requires the pre-migration backup. No installed app or live database was touched during development.

Validation

Candidate: efaa782f63cfd0b21cf73bdde314c69068f2ef32
Base main: 541471d55648df021b232b5d01f0e1dd14cbb033

Passed:

  • 309 model-related Node tests (apps/desktop/test/*model*.test.mjs plus endpoint-probe tests).
  • 64 provider and 40 database Rust tests.
  • node scripts/e2e-model-discovery-state.mjs against the exact committed candidate: real isolated Host/SQLite and production IPC/Composer, mocked HTTP only; disappearance, offline, restart, recovery, case changes, partial/malformed/empty lists, manual IDs, legacy default, endpoint isolation.
  • Shared/agent-runtime builds, desktop TypeScript, desktop production build, Host build, Rust formatting, clippy, Biome, style-token and agent-policy checks.
  • PR-base ancestry check after fetching origin/main.

Known limitations, not hidden as green:

  • Latest full Host suite: 1029 passed, 1 failed, 1 ignored. Remaining failure is data_relocation::tests::relocated_project_memory_remains_readable_through_the_repository (empty content versus Keep project memory), previously reproduced on the unmodified base. A worktree setup test failed once in a concurrent run and passed in isolated and subsequent full runs. A serial full-suite attempt exceeded 240 seconds and was stopped.
  • Reused existing local dependencies rather than installing a second environment. Local Pi dependency is 1.1.0, while this branch requests 1.3.0; clean-lockfile CI remains required despite successful local typecheck/build/tests.
  • pnpm automatic switching to 12.8.1 fails locally (ENOEXEC). PR-base used the installed pnpm with auto-switch disabled; the exact lint constituent commands ran directly and passed.
  • Clippy reports one pre-existing filter_next warning in transcripts.rs. Production build reports chunk-size warnings.
  • No live app visual verification, installation, or restart was performed. This PR is not a merge approval; upstream integration checks remain necessary.

sinolin and others added 2 commits October 26, 2026 20:28
Separate saved model configuration from endpoint publication state so a
previously discovered model cannot silently return to Composer through
configured-binding merging or restart hydration.

Reconcile only complete nonempty answers, retain manual ids and history,
and cover the real Host-to-IPC-to-Composer path with isolated storage.
Schema v25 adds a backed-up migration for durable publication state.

fixes vastsa#1892

vastsa commented Oct 26, 2026

Copy link
Copy Markdown
Owner

Independent review confirms the old real Host → IPC → Composer path still re-adds beta after a complete list drops it; the same regression now passes through removal, offline fallback, restart, reappearance, malformed/partial/empty lists, manual bindings and endpoint changes. Verified 309 model/endpoint tests, 64 Rust provider tests, 40 DB tests (including backed-up v24→v25 migration/reopen), shared/runtime builds, Desktop typecheck/production build, format, all-target Clippy (one unchanged warning), Biome/style/architecture/policy checks. Existing bindings/defaults remain intact and inference is not blocked by unlisted. The PR branch now contains current main and exactly matches the tested integration tree. Proceeding to merge while ignoring CI status as requested. Compatibility note: schema v25 is additive but older app versions require the pre-migration backup to downgrade; no live user database was used.

@vastsa
vastsa merged commit 90226c5 into vastsa:main Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Previously discovered configured models remain normal choices after leaving the provider list

2 participants