fix(validation): load lazy types before cross-model expression checks

[keeb]

Summary

validateModelPathReference called modelRegistry.get() without first awaiting ensureTypeLoaded(). For a type that is catalog-known but not yet imported (lazy-registered), has(type) returns true while get(type) returns undefined, so cross-model CEL expressions like ${{ model.llm.definition.globalArguments.ollamaUrl }} failed validation with a misleading Unknown model type "@keeb/ollama" error — even though the type was registered and worked at execution time (because the execution path already awaits ensureTypeLoaded).

This is a missed call site from #1063 (lazy per-bundle loading). The execution path was wired up; the validation path was not. The fix is the one-liner suggested in the issue — an await modelRegistry.ensureTypeLoaded(result.type) before the existing get() call. ensureTypeLoaded is a no-op on already-loaded types, so the perf win from #1063 is fully preserved.

cc @Paulstack — this is the missed call site from #1063 (lazy per-bundle loading).

Closes swamp-club #89.

Test plan

• New regression test validateModel loads lazy types before resolving cross-model references in validation_service_test.ts. Registers a unique type via registerLazy, wires a setTypeLoader that promoteFromLazys when invoked, and asserts both that the Expression paths check passes and that the type loader was actually called (pinning the behavior to "ensureTypeLoaded was awaited" rather than "the type happened to already be loaded").
• Verified the new test fails when the fix is reverted (git stash the fix → test fails with the exact "Unknown model type" error path; restore → passes).
• deno check — clean
• deno lint — clean
• deno fmt — clean
• deno run test — 4297 passed, 0 failed
• deno run compile — binary rebuilt

🤖 Generated with Claude Code

[github-actions]

Code Review

Clean, well-scoped fix for a missed ensureTypeLoaded call site from lazy per-bundle loading (#1063). The one-liner production change is correct — ensureTypeLoaded is already the established pattern (used in method_execution_service.ts:646) and is a no-op for already-loaded types, so there's no performance regression.

Blocking Issues

None.

Suggestions

None — this is a model fix. The regression test is thorough (asserts both the result and the mechanism, uses a unique type to avoid global singleton collision, and cleans up the type loader in a finally block). Production change is minimal and correctly placed. LGTM.

[github-actions]

Adversarial Review

Critical / High

None.

Medium

1. Test singleton pollution (validation_service_test.ts:1022-1074): The test calls modelRegistry.registerLazy() and then promoteFromLazy() on the global singleton, permanently adding @test/issue-89-lazy-regression to modelRegistry.models. The finally block restores the type loader but cannot clean up the registered model (no unregister method exists). If this test were re-run in the same process (e.g., --watch mode), the second run would fail: ensureTypeLoaded would see the type already in this.models, return immediately without calling the loader, and the loaderCalled assertion at line 1068 would fail. In normal CI this is a non-issue since tests run once, but it's worth noting as a fragility if the registry ever gains a reset() for test isolation.

Low

1. Unhandled loader errors propagate as crashes (validation_service.ts:706): If ensureTypeLoaded throws (e.g., corrupt bundle file, transient I/O error), the error propagates through Promise.all at line 566 and causes validateModel to throw instead of returning a structured validation result. However, this is consistent with the execution path (method_execution_service.ts:646) which also lets loader errors propagate, and transient I/O errors are a reasonable thing to surface as exceptions rather than validation failures. Not a regression introduced by this PR.

Verdict

PASS — The fix is a correct one-liner (await modelRegistry.ensureTypeLoaded(result.type)) placed immediately before the existing modelRegistry.get() call in validateModelPathReference. It follows the exact same pattern already established in the execution path (method_execution_service.ts:646). ensureTypeLoaded is a no-op for already-loaded types, so there is no regression for non-lazy types and the perf win from #1063 is preserved. The regression test is well-designed: it simulates the lazy state by using registerLazy directly (rather than defineModel which eagerly registers), asserts both that validation passes AND that the loader was actually invoked (pinning the fix to ensureTypeLoaded being called, not to the type happening to be loaded already).