Repository navigation
feat: resolve model bindings during planning - #81
Closed
AjayThorve wants to merge 3 commits into
Closed
AjayThorve wants to merge 3 commits into
AjayThorve wants to merge 3 commits into
Conversation
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Contributor
WalkthroughFabric adds planning-time model compatibility resolution and propagates secret-free model bindings through run plans, diagnostics, adapter invocations, Claude/Codex adapters, Harbor integration, schemas, typed SDK contracts, tests, and documentation. ChangesModel binding planning
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant UserConfig
participant FabricPlanner
participant AdapterDescriptor
participant RunPlan
participant Adapter
UserConfig->>FabricPlanner: configure provider, protocol, model, endpoint, credentials
FabricPlanner->>AdapterDescriptor: validate compatibility
AdapterDescriptor-->>FabricPlanner: supported roles and protocols
FabricPlanner->>RunPlan: create secret-free model_binding
RunPlan->>Adapter: pass model_binding in invocation
Adapter->>Adapter: resolve credential environment and endpoint routing
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
📖 Fern docs preview: https://nvidia-preview-pull-request-81.docs.buildwithfern.com/nemo/fabric |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@adapters/claude/README.md`:
- Around line 77-80: Update the provider selection documentation near
models.default to state that provider="anthropic" uses Anthropic’s native
endpoint only when base_url is omitted; clarify that configuring base_url
directs requests to the specified custom endpoint.
In `@adapters/claude/src/nemo_fabric_adapters/claude/adapter.py`:
- Around line 760-765: Update the credential-mapping block in the adapter to
clear the original credential_env entry in values after resolving the credential
and mapping it to ANTHROPIC_API_KEY. Preserve the existing clearing of
ANTHROPIC_AUTH_ENV_NAMES and ensure the source secret is removed consistently
when it came from configured environment values.
In `@adapters/codex/README.md`:
- Around line 79-81: Update the Codex adapter README description to state that
models.default selects the Codex model, rather than referring broadly to the
models map; preserve the existing requirements for the built-in openai provider
and openai-responses protocol.
In `@adapters/codex/src/nemo_fabric_adapters/codex/adapter.py`:
- Around line 335-341: Update the credential-mapping logic in the adapter flow
around _model_credential_env and configured.get so that, after reading the
selected source credential and mapping it to OPENAI_API_KEY, the source
environment variable is cleared from values when it originated from
harness.settings.env. Preserve the existing fallback behavior and only clear the
selected credential name.
In `@crates/fabric-core/src/config.rs`:
- Around line 1677-1679: Update the model compatibility logic around the
empty-model guard so an aware descriptor with no configured models returns the
ModelCompatibility error instead of Ok(None), requiring the adapter’s default
binding. Add a regression test covering an aware descriptor with empty
config.models and verify the expected ModelCompatibility result, while
preserving the existing behavior for unrelated compatibility cases.
- Around line 1871-1892: Update validate_model_base_url to parse the complete
trimmed endpoint with the project’s URL parser before accepting it, rejecting
malformed ports and unmatched bracketed hosts. Validate the parsed URL’s scheme
is http or https, require a host, and preserve rejection of credentials, query
strings, and fragments while returning the trimmed value on success.
In
`@docs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-modelcredentialref.mdx`:
- Line 5: Update the navigation position for the ModelCredentialRef
documentation or the conflicting RelayAtOfEndpointConfig documentation so the
two entries use distinct position values, preserving the intended sidebar
ordering.
In
`@docs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-resolvedmodelbinding.mdx`:
- Around line 2-14: Update the ResolvedModelBinding API documentation
description to remove the unsupported “Immutable” claim, while retaining the
accurate secret-free model-selection guarantee. Do not change the generated Rust
signature or make the type opaque.
In `@python/src/nemo_fabric/types.py`:
- Around line 1041-1051: Update the capabilities normalization in the
model-binding parsing flow to match Rust’s BTreeSet semantics: validate each
entry with _required_text, remove duplicates, and produce a deterministic sorted
collection before assigning data["capabilities"]. Preserve the existing
invalid-input error behavior.
- Around line 1122-1125: Update RunPlan.to_mapping() to omit the model_binding
key when RunPlan._normalize() has set it to None, matching Rust’s Option::None
serialization behavior; retain serialization through
ResolvedModelBinding.from_mapping() when a binding is present, and add a parity
test covering the absent-binding case.
In `@tests/adapters/test_claude_adapter.py`:
- Around line 331-378: Update
test_custom_provider_maps_endpoint_and_credential_without_anthropic_fallback and
test_explicit_credential_reference_overrides_native_anthropic_auth to use direct
os.environ assignments instead of monkeypatch.setenv for NVIDIA_API_KEY,
ANTHROPIC_AUTH_TOKEN, FABRIC_CLAUDE_KEY, and ANTHROPIC_PROFILE. Preserve the
existing values and assertions, relying on the repository’s autouse fixture for
environment restoration.
In `@tests/adapters/test_codex_adapter.py`:
- Around line 650-658: Update
test_environment_maps_resolved_credential_to_codex_api_key to assign the
FABRIC_OPENAI_KEY and OPENAI_API_KEY values through os.environ instead of
monkeypatch.setenv, relying on the repository’s autouse environment-restoration
fixture and preserving the existing credential values.
In `@tests/python/test_sdk_contract.py`:
- Around line 493-500: Remove the -> None return annotation from the
test_model_binding_types_reject_unknown_secret_fields test function, leaving its
parameters and test body unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 64d4ed4e-20c6-4247-9841-2ee28e27e066
📒 Files selected for processing (119)
adapters/claude/README.mdadapters/claude/fabric-adapter.jsonadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/codex/README.mdadapters/codex/fabric-adapter.jsonadapters/codex/src/nemo_fabric_adapters/codex/adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.pycrates/fabric-cli/src/main.rscrates/fabric-core/src/config.rscrates/fabric-core/src/doctor.rscrates/fabric-core/src/error.rscrates/fabric-core/src/lib.rscrates/fabric-core/src/runtime.rscrates/fabric-python/src/lib.rsdocs/integrations/claude.mdxdocs/integrations/codex.mdxdocs/reference/api/python-library-reference/index.mddocs/reference/api/python-library-reference/nemo_fabric.types.mddocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-capabilitykind.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-capabilitytarget.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-controllocation.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-environmentownership.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-fabricdocument.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-mcpexposure.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-modelcredentialref.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-modelendpointref.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatifstorageconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofendpointfieldnamepolicy.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofendpointtransport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofmode.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayotlptransport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayunsupportedbehavior.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-resolutionstrategy.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-telemetryprovider.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-load-adapter-descriptor.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-load-fabric-document.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-resolve-effective-config-from-config.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-resolve-effective-config-with-profiles.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-resolve-effective-config.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-resolve-run-plan-from-config.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-resolve-run-plan-from-effective-config.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-resolve-run-plan-with-profiles.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-resolve-run-plan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-validate-agent-directory.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterdescriptor.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adaptermodelprotocolsupport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adaptermodelsupport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterrequirements.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adaptertelemetryprovidersupport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adaptertelemetrysupport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-capabilityplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-capabilityroute.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-capabilitytargetplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-effectiveconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-environmentconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-environmentplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-fabricconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-harnessconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-mcpconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-mcpserverconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-mcpserverplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-metadataconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-modelconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-profileconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-profileregistryconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayatifconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayatofconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayatofendpointconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relaycomponentconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayconfigpolicy.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayobservabilityconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayotlpconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-resolvecontext.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-resolvedadapterdescriptor.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-resolvedmodelbinding.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-runplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-runtimecapabilities.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-runtimeconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-skillconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-telemetryconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-telemetryplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-telemetryproviderconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-toolsconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-toolsplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/enum-doctorstatus.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/fn-doctor-effective-config.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/fn-doctor-plan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/struct-doctorcheck.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/struct-doctorreport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/enum-fabricerror.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/type-result.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/fn-version.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/runtime/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/runtime/struct-adapterinvocation.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/schema/index.mdxdocs/sdk/python.mdxexamples/harbor/README.mdexamples/harbor/calculator/README.mdexamples/harbor/swebench/README.mdexamples/harbor/swebench/adapters/claude/fabric-adapter.jsonpython/src/nemo_fabric/__init__.pypython/src/nemo_fabric/integrations/harbor/fabric_agent.pypython/src/nemo_fabric/models.pypython/src/nemo_fabric/types.pyschemas/adapter-descriptor.schema.jsonschemas/adapter-invocation.schema.jsonschemas/agent.schema.jsonschemas/effective-config.schema.jsonschemas/run-plan.schema.jsontests/adapters/test_adapaters_common_utils.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.pytests/integrations/test_harbor_runner.pytests/python/test_sdk_contract.py
Comment on lines
+77
to
+80
| - `models.default` selects the Claude model. `provider="anthropic"` uses | ||
| Claude's native endpoint. Any provider name can target an explicit | ||
| Anthropic Messages-compatible `base_url` by setting | ||
| `protocol="anthropic-messages"`. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Should fix: Qualify when Anthropic uses the native endpoint.
provider="anthropic" with a configured base_url resolves to that custom endpoint. State that the native endpoint is selected only when base_url is omitted.
Proposed rewrite
-- `models.default` selects the Claude model. `provider="anthropic"` uses
- Claude's native endpoint. Any provider name can target an explicit
+- `models.default` selects the Claude model. `provider="anthropic"` uses
+ Anthropic's native endpoint when `base_url` is omitted. Any provider can target an explicitAs per path instructions, review documentation for technical accuracy against the current API.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - `models.default` selects the Claude model. `provider="anthropic"` uses | |
| Claude's native endpoint. Any provider name can target an explicit | |
| Anthropic Messages-compatible `base_url` by setting | |
| `protocol="anthropic-messages"`. | |
| - `models.default` selects the Claude model. `provider="anthropic"` uses | |
| Anthropic's native endpoint when `base_url` is omitted. Any provider can target an explicit | |
| Anthropic Messages-compatible `base_url` by setting | |
| `protocol="anthropic-messages"`. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@adapters/claude/README.md` around lines 77 - 80, Update the provider
selection documentation near models.default to state that provider="anthropic"
uses Anthropic’s native endpoint only when base_url is omitted; clarify that
configuring base_url directs requests to the specified custom endpoint.
Sources: Coding guidelines, Path instructions
Comment on lines
760
to
+765
| values.update(configured) | ||
| if credential_env is not None: | ||
| credential = configured.get(credential_env, os.environ.get(credential_env)) | ||
| for name in ANTHROPIC_AUTH_ENV_NAMES: | ||
| values[name] = "" | ||
| values["ANTHROPIC_API_KEY"] = credential or "" |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Clear the source credential variable after mapping it.
When credential_env comes from harness.settings.env, values.update(configured) leaves that secret under its original name as well as ANTHROPIC_API_KEY. Clear the source entry consistently.
Proposed fix
if credential_env is not None:
credential = configured.get(credential_env, os.environ.get(credential_env))
for name in ANTHROPIC_AUTH_ENV_NAMES:
values[name] = ""
+ values[credential_env] = ""
values["ANTHROPIC_API_KEY"] = credential or ""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| values.update(configured) | |
| if credential_env is not None: | |
| credential = configured.get(credential_env, os.environ.get(credential_env)) | |
| for name in ANTHROPIC_AUTH_ENV_NAMES: | |
| values[name] = "" | |
| values["ANTHROPIC_API_KEY"] = credential or "" | |
| values.update(configured) | |
| if credential_env is not None: | |
| credential = configured.get(credential_env, os.environ.get(credential_env)) | |
| for name in ANTHROPIC_AUTH_ENV_NAMES: | |
| values[name] = "" | |
| values[credential_env] = "" | |
| values["ANTHROPIC_API_KEY"] = credential or "" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@adapters/claude/src/nemo_fabric_adapters/claude/adapter.py` around lines 760
- 765, Update the credential-mapping block in the adapter to clear the original
credential_env entry in values after resolving the credential and mapping it to
ANTHROPIC_API_KEY. Preserve the existing clearing of ANTHROPIC_AUTH_ENV_NAMES
and ensure the source secret is removed consistently when it came from
configured environment values.
Comment on lines
+79
to
+81
| - `models` selects the Codex model. The adapter requires the built-in `openai` | ||
| provider over the `openai-responses` wire protocol. Planning rejects custom | ||
| providers, protocols, and model endpoint URLs before the SDK starts. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Should fix: Name the required models.default role.
The descriptor accepts only default; referring to the whole models map suggests that another alias can be selected.
Proposed rewrite
-- `models` selects the Codex model. The adapter requires the built-in `openai`
+- `models.default` selects the Codex model. The adapter requires the built-in `openai`As per path instructions, review documentation for technical accuracy against the current API.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - `models` selects the Codex model. The adapter requires the built-in `openai` | |
| provider over the `openai-responses` wire protocol. Planning rejects custom | |
| providers, protocols, and model endpoint URLs before the SDK starts. | |
| - `models.default` selects the Codex model. The adapter requires the built-in `openai` | |
| provider over the `openai-responses` wire protocol. Planning rejects custom | |
| providers, protocols, and model endpoint URLs before the SDK starts. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@adapters/codex/README.md` around lines 79 - 81, Update the Codex adapter
README description to state that models.default selects the Codex model, rather
than referring broadly to the models map; preserve the existing requirements for
the built-in openai provider and openai-responses protocol.
Sources: Coding guidelines, Path instructions
Comment on lines
335
to
+341
| values.update(configured) | ||
| credential_env = _model_credential_env(_model_binding(payload)) | ||
| if credential_env is not None: | ||
| values["OPENAI_API_KEY"] = "" | ||
| credential = configured.get(credential_env, os.environ.get(credential_env)) | ||
| if credential is not None: | ||
| values["OPENAI_API_KEY"] = credential |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Clear the source credential variable after mapping it.
If the referenced credential is supplied through harness.settings.env, the child receives both FABRIC_OPENAI_KEY and OPENAI_API_KEY. Clear the selected source name after reading it.
Proposed fix
if credential_env is not None:
values["OPENAI_API_KEY"] = ""
credential = configured.get(credential_env, os.environ.get(credential_env))
+ values[credential_env] = ""
if credential is not None:
values["OPENAI_API_KEY"] = credential📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| values.update(configured) | |
| credential_env = _model_credential_env(_model_binding(payload)) | |
| if credential_env is not None: | |
| values["OPENAI_API_KEY"] = "" | |
| credential = configured.get(credential_env, os.environ.get(credential_env)) | |
| if credential is not None: | |
| values["OPENAI_API_KEY"] = credential | |
| values.update(configured) | |
| credential_env = _model_credential_env(_model_binding(payload)) | |
| if credential_env is not None: | |
| values["OPENAI_API_KEY"] = "" | |
| credential = configured.get(credential_env, os.environ.get(credential_env)) | |
| values[credential_env] = "" | |
| if credential is not None: | |
| values["OPENAI_API_KEY"] = credential |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@adapters/codex/src/nemo_fabric_adapters/codex/adapter.py` around lines 335 -
341, Update the credential-mapping logic in the adapter flow around
_model_credential_env and configured.get so that, after reading the selected
source credential and mapping it to OPENAI_API_KEY, the source environment
variable is cleared from values when it originated from harness.settings.env.
Preserve the existing fallback behavior and only clear the selected credential
name.
Comment on lines
+1677
to
+1679
| if descriptor.models.is_empty() || config.models.is_empty() { | ||
| return Ok(None); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require a binding when a model-aware adapter is selected.
Line 1677 returns None when config.models is empty. Claude or Codex can therefore pass planning and doctor without their required default role, deferring failure until adapter execution.
Proposed fix
- if descriptor.models.is_empty() || config.models.is_empty() {
+ if descriptor.models.is_empty() {
return Ok(None);
}Add a regression test asserting that an aware descriptor with no configured models returns ModelCompatibility.
As per path instructions, review the Rust core for runtime lifecycle correctness and ensure public API changes match tests and documentation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if descriptor.models.is_empty() || config.models.is_empty() { | |
| return Ok(None); | |
| } | |
| if descriptor.models.is_empty() { | |
| return Ok(None); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/fabric-core/src/config.rs` around lines 1677 - 1679, Update the model
compatibility logic around the empty-model guard so an aware descriptor with no
configured models returns the ModelCompatibility error instead of Ok(None),
requiring the adapter’s default binding. Add a regression test covering an aware
descriptor with empty config.models and verify the expected ModelCompatibility
result, while preserving the existing behavior for unrelated compatibility
cases.
Source: Path instructions
Comment on lines
+1041
to
+1051
| capabilities = data.get("capabilities", []) | ||
| if isinstance(capabilities, (str, bytes)) or not isinstance( | ||
| capabilities, Sequence | ||
| ): | ||
| raise FabricConfigError( | ||
| "model binding capabilities must be a sequence of strings" | ||
| ) | ||
| data["capabilities"] = tuple( | ||
| _required_text(capability, "model binding capability") | ||
| for capability in capabilities | ||
| ) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Normalize capabilities with Rust set semantics.
Rust uses BTreeSet<String>, but this tuple preserves duplicates and input order. Python can therefore emit bindings that differ from Rust serialization.
Proposed fix
- data["capabilities"] = tuple(
+ data["capabilities"] = tuple(sorted({
_required_text(capability, "model binding capability")
for capability in capabilities
- )
+ }))As per coding guidelines, Python SDK contracts must remain in parity with Rust public surfaces.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| capabilities = data.get("capabilities", []) | |
| if isinstance(capabilities, (str, bytes)) or not isinstance( | |
| capabilities, Sequence | |
| ): | |
| raise FabricConfigError( | |
| "model binding capabilities must be a sequence of strings" | |
| ) | |
| data["capabilities"] = tuple( | |
| _required_text(capability, "model binding capability") | |
| for capability in capabilities | |
| ) | |
| capabilities = data.get("capabilities", []) | |
| if isinstance(capabilities, (str, bytes)) or not isinstance( | |
| capabilities, Sequence | |
| ): | |
| raise FabricConfigError( | |
| "model binding capabilities must be a sequence of strings" | |
| ) | |
| data["capabilities"] = tuple(sorted({ | |
| _required_text(capability, "model binding capability") | |
| for capability in capabilities | |
| })) |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 1045-1047: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/src/nemo_fabric/types.py` around lines 1041 - 1051, Update the
capabilities normalization in the model-binding parsing flow to match Rust’s
BTreeSet semantics: validate each entry with _required_text, remove duplicates,
and produce a deterministic sorted collection before assigning
data["capabilities"]. Preserve the existing invalid-input error behavior.
Source: Coding guidelines
Comment on lines
+1122
to
+1125
| binding = data.get("model_binding") | ||
| data["model_binding"] = ( | ||
| None if binding is None else ResolvedModelBinding.from_mapping(binding) | ||
| ) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline python/src/nemo_fabric/types.py \
--items all --match 'FabricMapping|RunPlan' --view expanded
rg -n -C4 '_omit_if_none|omit.*None|def to_mapping|class FabricMapping' \
python/src/nemo_fabric/types.py tests/pythonRepository: NVIDIA/NeMo-Fabric
Length of output: 8647
🏁 Script executed:
sed -n '120,170p' python/src/nemo_fabric/types.py
printf '\n---\n'
sed -n '1080,1135p' python/src/nemo_fabric/types.py
printf '\n---\n'
fd -a . rust src python | sed -n '1,120p'Repository: NVIDIA/NeMo-Fabric
Length of output: 5405
🏁 Script executed:
sed -n '130,155p' python/src/nemo_fabric/types.py
printf '\n---\n'
sed -n '1088,1129p' python/src/nemo_fabric/types.py
printf '\n---\n'
rg -n -C2 'model_binding|_omit_if_empty|to_mapping\(' python/src/nemo_fabric/types.py tests/python rustRepository: NVIDIA/NeMo-Fabric
Length of output: 26515
🏁 Script executed:
sed -n '135,150p' python/src/nemo_fabric/types.py
printf '\n---\n'
sed -n '1112,1128p' python/src/nemo_fabric/types.pyRepository: NVIDIA/NeMo-Fabric
Length of output: 1415
🏁 Script executed:
git ls-files | rg '(^|/)(types|plan|run).*\\.(rs|py|pyi|toml|json|yaml|yml)$|model_binding|RunPlan|fabric'
printf '\n---\n'
rg -n -C3 'model_binding|RunPlan|to_mapping|serialize|skip_serializing_if|omit' . -g '*.rs' -g '*.py' -g '*.pyi'
printf '\n---\n'
rg -n -C3 'test_run_plan|model_binding' tests/pythonRepository: NVIDIA/NeMo-Fabric
Length of output: 50375
🏁 Script executed:
git ls-files | sed -n '1,220p' | rg '(^|/)(src|crates)/.*\\.rs$|Cargo.toml$|model_binding|RunPlan'
printf '\n---\n'
rg -n -C3 'model_binding|RunPlan' . -g '*.rs'
printf '\n---\n'
rg -n -C3 'model_binding|RunPlan' tests -g '*.rs' -g '*.py'Repository: NVIDIA/NeMo-Fabric
Length of output: 50374
Omit absent model_binding from RunPlan.to_mapping()
RunPlan._normalize() sets model_binding to None, but Rust’s RunPlan skips serializing Option::None. Add omission for model_binding and a parity test for the absent-binding case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@python/src/nemo_fabric/types.py` around lines 1122 - 1125, Update
RunPlan.to_mapping() to omit the model_binding key when RunPlan._normalize() has
set it to None, matching Rust’s Option::None serialization behavior; retain
serialization through ResolvedModelBinding.from_mapping() when a binding is
present, and add a parity test covering the absent-binding case.
Sources: Coding guidelines, Learnings
Comment on lines
+331
to
+378
| def test_custom_provider_maps_endpoint_and_credential_without_anthropic_fallback( | ||
| claude_payload, monkeypatch | ||
| ): | ||
| claude_payload["effective_config"]["config"]["harness"]["settings"]["env"] = {} | ||
| claude_payload["model_binding"].update( | ||
| { | ||
| "provider": "nvidia", | ||
| "endpoint_ref": { | ||
| "kind": "configured", | ||
| "url": "https://inference.example.test/anthropic", | ||
| }, | ||
| "credential_ref": { | ||
| "kind": "environment", | ||
| "name": "NVIDIA_API_KEY", | ||
| }, | ||
| } | ||
| ) | ||
| monkeypatch.setenv("NVIDIA_API_KEY", "nvidia-secret") | ||
| monkeypatch.setenv("ANTHROPIC_AUTH_TOKEN", "must-not-leak") | ||
|
|
||
| options = adapter.build_options(claude_payload, resume=None) | ||
|
|
||
| assert options.env["ANTHROPIC_BASE_URL"] == ( | ||
| "https://inference.example.test/anthropic" | ||
| ) | ||
| assert options.env["ANTHROPIC_API_KEY"] == "nvidia-secret" | ||
| assert options.env["ANTHROPIC_AUTH_TOKEN"] == "" | ||
| assert options.env["NVIDIA_API_KEY"] == "" | ||
|
|
||
|
|
||
| def test_explicit_credential_reference_overrides_native_anthropic_auth( | ||
| claude_payload, monkeypatch | ||
| ): | ||
| settings = claude_payload["effective_config"]["config"]["harness"]["settings"] | ||
| settings["env"] = {"ANTHROPIC_AUTH_TOKEN": "configured-token"} | ||
| claude_payload["model_binding"]["credential_ref"] = { | ||
| "kind": "environment", | ||
| "name": "FABRIC_CLAUDE_KEY", | ||
| } | ||
| monkeypatch.setenv("FABRIC_CLAUDE_KEY", "selected-secret") | ||
| monkeypatch.setenv("ANTHROPIC_PROFILE", "inherited-profile") | ||
|
|
||
| options = adapter.build_options(claude_payload, resume=None) | ||
|
|
||
| assert options.env["ANTHROPIC_API_KEY"] == "selected-secret" | ||
| assert options.env["ANTHROPIC_AUTH_TOKEN"] == "" | ||
| assert options.env["ANTHROPIC_PROFILE"] == "" | ||
| assert options.env["FABRIC_CLAUDE_KEY"] == "" |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use os.environ for test environment changes.
These tests violate the repository’s environment-restoration convention. Replace monkeypatch.setenv with direct assignments.
Proposed fix
- monkeypatch.setenv("NVIDIA_API_KEY", "nvidia-secret")
- monkeypatch.setenv("ANTHROPIC_AUTH_TOKEN", "must-not-leak")
+ os.environ["NVIDIA_API_KEY"] = "nvidia-secret"
+ os.environ["ANTHROPIC_AUTH_TOKEN"] = "must-not-leak"
...
- monkeypatch.setenv("FABRIC_CLAUDE_KEY", "selected-secret")
- monkeypatch.setenv("ANTHROPIC_PROFILE", "inherited-profile")
+ os.environ["FABRIC_CLAUDE_KEY"] = "selected-secret"
+ os.environ["ANTHROPIC_PROFILE"] = "inherited-profile"As per coding guidelines, use os.environ in tests because the autouse fixture restores it after each test.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def test_custom_provider_maps_endpoint_and_credential_without_anthropic_fallback( | |
| claude_payload, monkeypatch | |
| ): | |
| claude_payload["effective_config"]["config"]["harness"]["settings"]["env"] = {} | |
| claude_payload["model_binding"].update( | |
| { | |
| "provider": "nvidia", | |
| "endpoint_ref": { | |
| "kind": "configured", | |
| "url": "https://inference.example.test/anthropic", | |
| }, | |
| "credential_ref": { | |
| "kind": "environment", | |
| "name": "NVIDIA_API_KEY", | |
| }, | |
| } | |
| ) | |
| monkeypatch.setenv("NVIDIA_API_KEY", "nvidia-secret") | |
| monkeypatch.setenv("ANTHROPIC_AUTH_TOKEN", "must-not-leak") | |
| options = adapter.build_options(claude_payload, resume=None) | |
| assert options.env["ANTHROPIC_BASE_URL"] == ( | |
| "https://inference.example.test/anthropic" | |
| ) | |
| assert options.env["ANTHROPIC_API_KEY"] == "nvidia-secret" | |
| assert options.env["ANTHROPIC_AUTH_TOKEN"] == "" | |
| assert options.env["NVIDIA_API_KEY"] == "" | |
| def test_explicit_credential_reference_overrides_native_anthropic_auth( | |
| claude_payload, monkeypatch | |
| ): | |
| settings = claude_payload["effective_config"]["config"]["harness"]["settings"] | |
| settings["env"] = {"ANTHROPIC_AUTH_TOKEN": "configured-token"} | |
| claude_payload["model_binding"]["credential_ref"] = { | |
| "kind": "environment", | |
| "name": "FABRIC_CLAUDE_KEY", | |
| } | |
| monkeypatch.setenv("FABRIC_CLAUDE_KEY", "selected-secret") | |
| monkeypatch.setenv("ANTHROPIC_PROFILE", "inherited-profile") | |
| options = adapter.build_options(claude_payload, resume=None) | |
| assert options.env["ANTHROPIC_API_KEY"] == "selected-secret" | |
| assert options.env["ANTHROPIC_AUTH_TOKEN"] == "" | |
| assert options.env["ANTHROPIC_PROFILE"] == "" | |
| assert options.env["FABRIC_CLAUDE_KEY"] == "" | |
| def test_custom_provider_maps_endpoint_and_credential_without_anthropic_fallback( | |
| claude_payload, monkeypatch | |
| ): | |
| claude_payload["effective_config"]["config"]["harness"]["settings"]["env"] = {} | |
| claude_payload["model_binding"].update( | |
| { | |
| "provider": "nvidia", | |
| "endpoint_ref": { | |
| "kind": "configured", | |
| "url": "https://inference.example.test/anthropic", | |
| }, | |
| "credential_ref": { | |
| "kind": "environment", | |
| "name": "NVIDIA_API_KEY", | |
| }, | |
| } | |
| ) | |
| os.environ["NVIDIA_API_KEY"] = "nvidia-secret" | |
| os.environ["ANTHROPIC_AUTH_TOKEN"] = "must-not-leak" | |
| options = adapter.build_options(claude_payload, resume=None) | |
| assert options.env["ANTHROPIC_BASE_URL"] == ( | |
| "https://inference.example.test/anthropic" | |
| ) | |
| assert options.env["ANTHROPIC_API_KEY"] == "nvidia-secret" | |
| assert options.env["ANTHROPIC_AUTH_TOKEN"] == "" | |
| assert options.env["NVIDIA_API_KEY"] == "" | |
| def test_explicit_credential_reference_overrides_native_anthropic_auth( | |
| claude_payload, monkeypatch | |
| ): | |
| settings = claude_payload["effective_config"]["config"]["harness"]["settings"] | |
| settings["env"] = {"ANTHROPIC_AUTH_TOKEN": "configured-token"} | |
| claude_payload["model_binding"]["credential_ref"] = { | |
| "kind": "environment", | |
| "name": "FABRIC_CLAUDE_KEY", | |
| } | |
| os.environ["FABRIC_CLAUDE_KEY"] = "selected-secret" | |
| os.environ["ANTHROPIC_PROFILE"] = "inherited-profile" | |
| options = adapter.build_options(claude_payload, resume=None) | |
| assert options.env["ANTHROPIC_API_KEY"] == "selected-secret" | |
| assert options.env["ANTHROPIC_AUTH_TOKEN"] == "" | |
| assert options.env["ANTHROPIC_PROFILE"] == "" | |
| assert options.env["FABRIC_CLAUDE_KEY"] == "" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/adapters/test_claude_adapter.py` around lines 331 - 378, Update
test_custom_provider_maps_endpoint_and_credential_without_anthropic_fallback and
test_explicit_credential_reference_overrides_native_anthropic_auth to use direct
os.environ assignments instead of monkeypatch.setenv for NVIDIA_API_KEY,
ANTHROPIC_AUTH_TOKEN, FABRIC_CLAUDE_KEY, and ANTHROPIC_PROFILE. Preserve the
existing values and assertions, relying on the repository’s autouse fixture for
environment restoration.
Source: Coding guidelines
Comment on lines
+650
to
+658
| def test_environment_maps_resolved_credential_to_codex_api_key( | ||
| codex_payload, monkeypatch | ||
| ): | ||
| codex_payload["model_binding"]["credential_ref"] = { | ||
| "kind": "environment", | ||
| "name": "FABRIC_OPENAI_KEY", | ||
| } | ||
| monkeypatch.setenv("FABRIC_OPENAI_KEY", "resolved-secret") | ||
| monkeypatch.setenv("OPENAI_API_KEY", "unselected-secret") |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use os.environ for credential setup.
Replace monkeypatch.setenv so this test uses the repository’s autouse environment-restoration fixture.
Proposed fix
- monkeypatch.setenv("FABRIC_OPENAI_KEY", "resolved-secret")
- monkeypatch.setenv("OPENAI_API_KEY", "unselected-secret")
+ os.environ["FABRIC_OPENAI_KEY"] = "resolved-secret"
+ os.environ["OPENAI_API_KEY"] = "unselected-secret"As per coding guidelines, tests must use os.environ instead of monkeypatch.setenv.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def test_environment_maps_resolved_credential_to_codex_api_key( | |
| codex_payload, monkeypatch | |
| ): | |
| codex_payload["model_binding"]["credential_ref"] = { | |
| "kind": "environment", | |
| "name": "FABRIC_OPENAI_KEY", | |
| } | |
| monkeypatch.setenv("FABRIC_OPENAI_KEY", "resolved-secret") | |
| monkeypatch.setenv("OPENAI_API_KEY", "unselected-secret") | |
| def test_environment_maps_resolved_credential_to_codex_api_key( | |
| codex_payload, monkeypatch | |
| ): | |
| codex_payload["model_binding"]["credential_ref"] = { | |
| "kind": "environment", | |
| "name": "FABRIC_OPENAI_KEY", | |
| } | |
| os.environ["FABRIC_OPENAI_KEY"] = "resolved-secret" | |
| os.environ["OPENAI_API_KEY"] = "unselected-secret" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/adapters/test_codex_adapter.py` around lines 650 - 658, Update
test_environment_maps_resolved_credential_to_codex_api_key to assign the
FABRIC_OPENAI_KEY and OPENAI_API_KEY values through os.environ instead of
monkeypatch.setenv, relying on the repository’s autouse environment-restoration
fixture and preserving the existing credential values.
Source: Coding guidelines
Comment on lines
+493
to
+500
| def test_model_binding_types_reject_unknown_secret_fields( | ||
| model: type[ModelCredentialRef] | ||
| | type[ModelEndpointRef] | ||
| | type[ResolvedModelBinding], | ||
| payload: dict[str, Any], | ||
| ) -> None: | ||
| with pytest.raises(FabricConfigError, match="unknown fields"): | ||
| model.from_mapping(payload) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the test return annotation.
tests/**/*.py forbids -> None on test functions.
Proposed fix
def test_model_binding_types_reject_unknown_secret_fields(
model: type[ModelCredentialRef]
| type[ModelEndpointRef]
| type[ResolvedModelBinding],
payload: dict[str, Any],
-) -> None:
+):As per coding guidelines, “Do not add -> None return annotations to test functions.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def test_model_binding_types_reject_unknown_secret_fields( | |
| model: type[ModelCredentialRef] | |
| | type[ModelEndpointRef] | |
| | type[ResolvedModelBinding], | |
| payload: dict[str, Any], | |
| ) -> None: | |
| with pytest.raises(FabricConfigError, match="unknown fields"): | |
| model.from_mapping(payload) | |
| def test_model_binding_types_reject_unknown_secret_fields( | |
| model: type[ModelCredentialRef] | |
| | type[ModelEndpointRef] | |
| | type[ResolvedModelBinding], | |
| payload: dict[str, Any], | |
| ): | |
| with pytest.raises(FabricConfigError, match="unknown fields"): | |
| model.from_mapping(payload) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/python/test_sdk_contract.py` around lines 493 - 500, Remove the -> None
return annotation from the test_model_binding_types_reject_unknown_secret_fields
test function, leaving its parameters and test body unchanged.
Source: Coding guidelines
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Collaborator
Author
|
Closing by design: this model-resolution contract is broader than we want to land independently. FABRIC-64 will be deferred as a sub-issue of FABRIC-84 and revisited with the adapter settings/schema work. Current adapter-level provider validation remains the supported behavior. |
This was referenced Jul 20, 2026
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.
Overview
Define one planning-time model compatibility contract shared by plan, doctor, runtime invocation, Claude, and Codex. Fabric resolves consumer model intent into an immutable, secret-free
ResolvedModelBinding; adapters perform only the final harness-native translation.The concrete hosted-Claude example uses provider
nvidiawith an explicitly configured Anthropic Messages-compatible URL and credential reference. Fabric does not hardcode an endpoint or default a credential variable. This PR intentionally excludes Harbor-level NVIDIA credential defaults (FABRIC-93) and genericharness.settingsschema or introspection work (FABRIC-84).No dependency, lockfile, or attribution changes are included.
Details
ResolvedModelBindingduring planning and carry it through Rust, CLI, Python SDK, schemas, and adapter invocation.doctor.Validation
just test-rust— 71 passed.just --set no_uv true test-python— 381 passed, 11 skipped.cargo fmt --all -- --check— passed.cargo check -p fabric-python --locked— passed.just docs— passed; Fern reported 0 errors.git diff --check— passed.Where should the reviewer start?
Start with
resolve_model_bindingincrates/fabric-core/src/config.rs. The key invariant is that Fabric owns semantic compatibility and emits only endpoint and credential references; adapters own final harness-native translation. Then review the Claude and Codexmodel_bindingconsumers and the shared doctor diagnostic.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Resolves FABRIC-64
I confirm this contribution is my own work, or I have the right to submit it under this project's license.
I searched existing issues and open pull requests, and this does not duplicate existing work.