harden(distributed): defensive companion-artifact option handling (follow-ups to #11075)#11094
Closed
localai-bot wants to merge 2 commits into
Closed
harden(distributed): defensive companion-artifact option handling (follow-ups to #11075)#11094localai-bot wants to merge 2 commits into
localai-bot wants to merge 2 commits into
Conversation
…esolved Defensive follow-up to #11075. It does NOT fix the longcat-video remote-load failure observed earlier — that turned out to be a stale worker backend (predating #10949, so it lacked _resolve_option_path) and was fixed by upgrading the backend. This only hardens the option-synthesis path. withCompanionArtifactOptions previously dropped a declared companion artifact whenever it had no resolved snapshot, emitting nothing. A dropped companion is invisible to the backend, which then falls back to its own hardcoded default and fails far from the cause. Instead, fall back to the companion's DECLARED source repository id so the backend fetches the artifact the config actually asked for; the resolved snapshot path (the staged, no-download fast path) is still preferred whenever the companion is resolved, so the healthy path is unchanged. The fallback logs a warning so the missing controller-side resolution stays diagnosable. The synthesis is factored into an exported ManagedCompanionOptions / CompanionArtifactOptions so the distributed reconciler can re-derive the same options when it replays a stored proto blob (next commit). Assisted-by: Claude Code:claude-opus-4-8[1m] [Read] [Edit] [Bash] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
…replays a stored blob Defensive follow-up to #11075; not the fix for the earlier longcat-video remote failure (that was a stale worker backend, fixed by upgrading it). The reconciler's scale-up path (ScheduleAndLoadModel) replays a ModelOptions proto blob captured at first load and never runs grpcModelOpts. So a companion resolved AFTER that blob was captured (e.g. the blob predates the companion's resolution) would never reach a scaled-up replica, which would then load without the companion option. applyCompanionOptions re-derives the model's managed companion options from its CURRENT config (via the new backend.CompanionArtifactOptions, wired through SmartRouterOptions.CompanionOptionsFor in distributed startup) and merges in only those the blob is missing — an option the blob already carries, including an author pin, always wins. nil-safe: single node and any caller that does not wire the resolver are an unchanged no-op. Also adds a regression guard for the controller->worker handoff: FileStagingClient (the client wrapping every distributed LoadModel) must forward ModelOptions.Options — the channel a managed companion option rides on — and ModelPath to the backend unchanged. The test pins that so a future refactor of the wrapper cannot silently drop options. Assisted-by: Claude Code:claude-opus-4-8[1m] [Read] [Edit] [Bash] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this is (and is not)
Two small defensive-hardening follow-ups to #11075 for managed companion artifacts (a model that pulls a second HuggingFace repo, e.g.
longcat-video-avatar-1.5'sbase_model).This is NOT a fix for a live bug. The remote-load failure that motivated the investigation (
base_model must point to a LongCat-Video checkpoint, backend re-downloading the base repo) turned out to be a stalelongcat-videoworker backend that predated #10949 and lacked_resolve_option_path; upgrading the backend fixed it, verified end-to-end with a real video on a clean cluster (released controller image, no workarounds). These changes just make two adjacent code paths fail-safe instead of silently dropping a companion.1. Unresolved companion → declared source repo (not silence)
withCompanionArtifactOptionsused to emit nothing for a declared companion that had no resolved snapshot. A dropped companion is invisible to the backend, which then loads its own hardcoded default and fails far from the cause. Now it falls back to the companion's declared source repository id, so the backend fetches the artifact the config actually asked for. The resolved snapshot path (the staged, no-download fast path) is still preferred whenever the companion is resolved — the healthy path is unchanged — and the fallback logs a warning so a missing controller-side resolution stays diagnosable.2. Reconciler replay re-derives companion options
The reconciler's scale-up path (
ScheduleAndLoadModel) replays aModelOptionsproto blob captured at first load and never runsgrpcModelOpts. A companion resolved after that blob was captured would never reach a scaled-up replica.applyCompanionOptionsre-derives the model's managed companion options from its current config (new exportedbackend.CompanionArtifactOptions, wired viaSmartRouterOptions.CompanionOptionsForin distributed startup) and merges in only those the blob is missing — an option the blob already carries (including an author pin) always wins.nil-safe: single-node and any caller that doesn't wire the resolver are an unchanged no-op.Also adds a regression guard:
FileStagingClient(the client wrapping every distributedLoadModel) must forwardModelOptions.OptionsandModelPathto the backend unchanged, so a future refactor can't silently drop options.Tests (Ginkgo/Gomega, TDD)
core/backend/companion_options_test.go— unresolved companion names the source repo; resolved companion still prefers the snapshot path.core/services/nodes/router_companion_replay_test.go— reconciler injects a companion missing from the stored blob; never overrides one already present; no-op without a resolver. (Red-before / green-after confirmed by stubbingapplyCompanionOptions.)core/services/nodes/file_staging_loadmodel_test.go— the handoff forwards.Options/ModelPathunchanged.Verification (exit codes read separately)
go build ./core/... ./pkg/...→ 0go test ./core/backend/→ 0go test ./core/config/→ 0go test ./core/application/→ 0go test ./core/services/nodes/→ 0 (~200s)golangci-lint run --new-from-merge-base=origin/master(core/backend, core/config, core/services/nodes, core/application) → 0 issuestests/e2egrep for the touched symbols → no references.No diagnostic tracing and no backend changes ship in this PR.
🤖 Generated with Claude Code
https://claude.ai/code/session_0156CbKpDh8qbAYzJZJDZ2yk