Repository navigation
fix(ai): truthful agent-selection errors + model-path factory resolution - #167
Merged
Merged
Conversation
…resolution The 'Selected agent X was not found among [X, ...]' lie was a two-layer mask: (1) SelectAgent clobbered the truthful matched-but-unbuildable error with 'not found'; (2) beneath it, path-form model ids (Provider/OpenAICompatible/qwen-small) missed the factory's Id-only matching, and AddOpenAICompatible's config seed never reached the factory's Supports/credentials — any cold catalog window failed EVERY agent build. Resolver canonicalizes node-path→wire id; the OpenAI factory reads its own config sections as a snapshot-independent source of truth; resume-recovery normalizes like the entry boundary. 15+5+2 tests green; solution Release -warnaserror clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses several AI agent selection and model/factory resolution failure modes that produced misleading “agent not found” errors and prevented OpenAICompatible config-seeded models from resolving reliably during cold-start windows. It improves error truthfulness/actionability, normalizes model identifiers across path/id forms, and adds regression tests to pin the observed e2e portal failure.
Changes:
- Prevents “agent not found” error clobbering when the selected agent is actually matched but cannot be built (missing factory / factory throws), surfacing the real failure instead.
- Extends credential/model resolution to accept full LanguageModel node paths, adding resolver-backed path→id normalization.
- Adds OpenAICompatible config-section fallback in
Supports()/CreateChatClient()plus tests covering cold-catalog scenarios and path-form selections.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.AI.Test/OpenAIFactoryConfigSeededModelTest.cs | New regression coverage for OpenAICompatible config-seeded models resolving even with a cold catalog snapshot. |
| test/MeshWeaver.AI.Test/ChatClientCredentialResolverTest.cs | Adds coverage ensuring resolver handles full node-path model selections (provider stamp, credentials, wire id). |
| test/MeshWeaver.AI.Test/AgentSelectionPathResolutionTest.cs | Adds coverage for “matched-but-unbuildable” vs “unmatched” selection error messaging behavior. |
| src/MeshWeaver.AI/ThreadExecution.cs | Normalizes persisted model selection when recovering from the response cell. |
| src/MeshWeaver.AI/ChatClientCredentialResolver.cs | Adds resolver-backed canonicalization for full model node paths and applies it to provider/credential resolution. |
| src/MeshWeaver.AI/AgentChatClient.cs | Avoids overwriting truthful build failures with “agent not found”; adds actionable “no factory resolves model” messaging. |
| src/MeshWeaver.AI.OpenAI/OpenAIChatClientAgentFactory.cs | Adds config-section lookup fallback for config-seeded OpenAICompatible models (Supports/CreateChatClient). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1096
to
1101
| // The persisted cell carries the COMPOSER form of the model — the full | ||
| // node path ("Provider/OpenAICompatible/qwen-small"). Normalize exactly | ||
| // like the entry boundary above (and like Harness on the next line): | ||
| // factories/credential resolution key on the bare model id. | ||
| ModelName = SelectionId.IdOf(request.ModelName ?? cell!.ModelName), | ||
| Harness = SelectionId.IdOf(request.Harness ?? cell!.Harness) |
Contributor
Test Results 53 files ± 0 53 suites ±0 18m 58s ⏱️ - 1m 13s Results for commit 7205c3e. ± Comparison against base commit e770d68. This pull request removes 182 and adds 129 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Contributor
Test Results (shard 1)1 078 tests - 182 1 077 ✅ - 182 3m 35s ⏱️ - 1m 34s Results for commit 7205c3e. ± Comparison against base commit e770d68. This pull request removes 182 tests. |
Contributor
Contributor
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.
Root-causes the e2e 'agent not found among [the very list containing it]' failure: error clobbering in SelectAgent, path-form model ids missing Id-only factory matching, and the OpenAICompatible config seed never reaching the factory (cold-catalog window failed every agent build). Truthful, actionable errors per failure mode; canonical path→id resolution; config-section fallback in Supports/CreateChatClient. 22 tests green across the selection/resolver/factory surfaces.
🤖 Generated with Claude Code