Add OCI LangChain support for hosted Nemotron workflows#1804
Open
fede-kamel wants to merge 1 commit intoNVIDIA:developfrom
Open
Add OCI LangChain support for hosted Nemotron workflows#1804fede-kamel wants to merge 1 commit intoNVIDIA:developfrom
fede-kamel wants to merge 1 commit intoNVIDIA:developfrom
Conversation
WalkthroughAdds OCI Generative AI support: new OCIModelConfig and async oci_llm provider, a LangChain ChatOCIGenAI wrapper, docs and redirects, dependency updates, unit/integration tests, and a pytest fixture for OCI Nemotron environment variables. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(100,149,237,0.5)
participant Client
participant WorkflowBuilder
participant Registry
participant ProviderFactory
participant LangChainWrapper
participant OCI_GenAI
end
Client->>WorkflowBuilder: request LLM build with `oci_llm` config
WorkflowBuilder->>Registry: resolve provider for OCIModelConfig
Registry-->>WorkflowBuilder: provider factory (oci_llm)
WorkflowBuilder->>ProviderFactory: invoke async oci_llm(config)
ProviderFactory-->>WorkflowBuilder: yield LLMProviderInfo
WorkflowBuilder->>LangChainWrapper: wrap provider (oci_langchain) if requested
LangChainWrapper->>OCI_GenAI: call ChatOCIGenAI (service_endpoint, model_id, model_kwargs)
OCI_GenAI-->>LangChainWrapper: streaming/response
LangChainWrapper-->>Client: return wrapped model interface / response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
fede-kamel
force-pushed
the
fk/oci-langchain-nemotron
branch
3 times, most recently
from
6b1ec5b to
3daa5eb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/nvidia_nat_core/src/nat/llm/oci_llm.py (1)
75-78: Type and document the new public provider factory.
oci_llmis part of the core public surface, but it has neither a return annotation nor a docstring. Please add theAsyncIterator[LLMProviderInfo]return type and a short Google-style docstring while this API is new.✍️ Proposed cleanup
+from collections.abc import AsyncIterator + from pydantic import AliasChoices from pydantic import ConfigDict from pydantic import Field @@ `@register_llm_provider`(config_type=OCIModelConfig) -async def oci_llm(config: OCIModelConfig, _builder: Builder): +async def oci_llm(config: OCIModelConfig, _builder: Builder) -> AsyncIterator[LLMProviderInfo]: + """Yield provider metadata for an OCI Generative AI model. + + Args: + config: OCI model configuration. + _builder: Builder instance. + + Yields: + `LLMProviderInfo` describing the configured OCI model. + """ yield LLMProviderInfo(config=config, description="An OCI Generative AI model for use with an LLM client.")As per coding guidelines, "All public APIs require Python 3.11+ type hints on parameters and return values" and "Provide Google-style docstrings for every public module, class, function and CLI command".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nvidia_nat_core/src/nat/llm/oci_llm.py` around lines 75 - 78, The public factory function oci_llm lacks a return type and docstring; update its signature to return AsyncIterator[LLMProviderInfo] and add a short Google-style docstring describing that it yields an LLMProviderInfo for an OCI Generative AI model, its parameters (config: OCIModelConfig, _builder: Builder), and its behavior; also import AsyncIterator from typing (or typing_extensions if required) so the annotation resolves. Ensure references to LLMProviderInfo, OCIModelConfig, and Builder remain accurate in the docstring and signature.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/source/build-workflows/llms/index.md`:
- Around line 31-32: The docs table and sample config for the `oci` provider
describe an OpenAI-compatible API shape (api_key, OCI_GENAI_* env vars) but the
implementation uses the SDK-backed OCIModelConfig with fields like
`compartment_id`, `auth_type`, `auth_profile`, `auth_file_location` and optional
`provider`; update the `oci` entry and all sample snippets (the table row and
the sample config blocks referenced) to reflect the actual `OCIModelConfig`
fields and example values, remove or replace references to
`api_key`/OpenAI-compatible env vars, and ensure examples include
`compartment_id`, `auth_type`, `auth_profile`/`auth_file_location` and the
`provider` field where appropriate so the docs match the `OCIModelConfig`
implementation.
In `@packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.py`:
- Around line 63-69: The test currently seeds registry._llm_provider_map
directly which masks registration bugs; instead patch
nat.cli.type_registry.GlobalTypeRegistry and let importing nat.llm.oci_llm
invoke the `@register_llm_provider` decorator, then inspect the mocked registry to
assert that OCIModelConfig is registered to the oci_llm provider (use the same
identifiers: test_oci_provider_registration, GlobalTypeRegistry, OCIModelConfig,
oci_llm, and the register_llm_provider decorator) rather than writing to
_llm_provider_map yourself.
In `@packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py`:
- Around line 239-259: The OCIModelConfig's max_retries and request_timeout are
never applied—update the oci_langchain() wrapper to preconfigure an OCI SDK
client with the desired retry and timeout settings (based on
OCIModelConfig.max_retries and request_timeout) and pass that SDK client into
ChatOCIGenAI via its client= parameter (instead of expecting ChatOCIGenAI to
accept those kwargs), referencing the ChatOCIGenAI(...) instantiation and
OCIModelConfig fields; alternatively, if you choose not to support custom
retry/timeout, remove max_retries and request_timeout from OCIModelConfig and
any docs/usages in oci_langchain() so the public config surface no longer
exposes unsupported fields.
In `@packages/nvidia_nat_langchain/tests/test_langchain_agents.py`:
- Around line 145-150: The test sets up OCIModelConfig in llm_config using
os.environ.get("OCI_GENAI_AUTH_PROFILE", "API_KEY_AUTH") which uses an auth type
as the fallback; update the fallback to the OCI profile default by using
"DEFAULT" instead of "API_KEY_AUTH" so OCIModelConfig receives a valid profile
name when OCI_GENAI_AUTH_PROFILE is unset.
In `@packages/nvidia_nat_test/src/nat/test/plugin.py`:
- Line 155: Add explicit return type annotations to the new pytest fixture
functions; for example change def oci_nemotron_endpoint_fixture(fail_missing:
bool): to def oci_nemotron_endpoint_fixture(fail_missing: bool) ->
typing.Generator[Any, None, None]: (and do the same for the other new fixture
defined nearby) and ensure typing.Generator and Any are imported from typing at
the top of the module if not already present.
---
Nitpick comments:
In `@packages/nvidia_nat_core/src/nat/llm/oci_llm.py`:
- Around line 75-78: The public factory function oci_llm lacks a return type and
docstring; update its signature to return AsyncIterator[LLMProviderInfo] and add
a short Google-style docstring describing that it yields an LLMProviderInfo for
an OCI Generative AI model, its parameters (config: OCIModelConfig, _builder:
Builder), and its behavior; also import AsyncIterator from typing (or
typing_extensions if required) so the annotation resolves. Ensure references to
LLMProviderInfo, OCIModelConfig, and Builder remain accurate in the docstring
and signature.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a1bc7293-0222-46e6-b9ee-400069afb2ae
⛔ Files ignored due to path filters (1)
packages/nvidia_nat_langchain/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
docs/source/build-workflows/llms/index.mddocs/source/components/integrations/index.mddocs/source/components/integrations/integrating-oci-generative-ai-models.mddocs/source/conf.pydocs/source/get-started/installation.mdexamples/frameworks/agno_personal_finance/pyproject.tomlexamples/frameworks/multi_frameworks/pyproject.tomlpackages/nvidia_nat_agno/pyproject.tomlpackages/nvidia_nat_core/src/nat/llm/oci_llm.pypackages/nvidia_nat_core/src/nat/llm/register.pypackages/nvidia_nat_core/tests/nat/llm/test_oci_llm.pypackages/nvidia_nat_langchain/pyproject.tomlpackages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.pypackages/nvidia_nat_langchain/tests/test_langchain_agents.pypackages/nvidia_nat_langchain/tests/test_llm_langchain.pypackages/nvidia_nat_test/src/nat/test/plugin.py
Comment on lines
+145
to
+150
| llm_config = OCIModelConfig( | ||
| model_name=os.environ["OCI_GENAI_MODEL"], | ||
| endpoint=os.environ["OCI_GENAI_SERVICE_ENDPOINT"], | ||
| compartment_id=os.environ["OCI_COMPARTMENT_ID"], | ||
| auth_profile=os.environ.get("OCI_GENAI_AUTH_PROFILE", "API_KEY_AUTH"), | ||
| temperature=0.0, |
There was a problem hiding this comment.
Use the OCI profile default expected by OCIModelConfig.
At Line 149, the fallback "API_KEY_AUTH" appears to be an auth type, not a profile name. This can break integration runs when OCI_GENAI_AUTH_PROFILE is unset. Using "DEFAULT" aligns with the config model default.
Suggested patch
- auth_profile=os.environ.get("OCI_GENAI_AUTH_PROFILE", "API_KEY_AUTH"),
+ auth_profile=os.environ.get("OCI_GENAI_AUTH_PROFILE", "DEFAULT"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nvidia_nat_langchain/tests/test_langchain_agents.py` around lines
145 - 150, The test sets up OCIModelConfig in llm_config using
os.environ.get("OCI_GENAI_AUTH_PROFILE", "API_KEY_AUTH") which uses an auth type
as the fallback; update the fallback to the OCI profile default by using
"DEFAULT" instead of "API_KEY_AUTH" so OCIModelConfig receives a valid profile
name when OCI_GENAI_AUTH_PROFILE is unset.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
docs/source/build-workflows/llms/index.md (2)
126-148:⚠️ Potential issue | 🟠 MajorDocumentation describes OpenAI-compatible fields but
OCIModelConfiguses OCI SDK auth.The section lists
api_keyandOCI_GENAI_API_KEY/OCI_GENAI_BASE_URLenvironment variables, butOCIModelConfiginoci_llm.pyuses OCI SDK authentication with fields likecompartment_id,auth_type,auth_profile, andauth_file_location. There is noapi_keyfield inOCIModelConfig.Update this section to match the actual implementation fields:
compartment_id,auth_type,auth_profile,auth_file_location, andprovider.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/source/build-workflows/llms/index.md` around lines 126 - 148, The docs currently list API key fields but the implementation uses OCI SDK auth; update the OCI Hosted OpenAI-Compatible section to reflect the actual OCIModelConfig fields used in oci_llm.py (class OCIModelConfig) by replacing `api_key`/OCI_GENAI_* entries with the SDK auth fields: `compartment_id`, `auth_type`, `auth_profile`, `auth_file_location`, and `provider`; also note that this provider targets OCI-hosted OpenAI-compatible chat-completions endpoints (Responses API not enabled) and remove or correct any references to `OCI_GENAI_API_KEY`/`api_key` to avoid mismatch with OCIModelConfig.
31-32:⚠️ Potential issue | 🟠 MajorTable and YAML example need OCI SDK auth fields.
The table description and YAML example should reflect that this uses OCI SDK authentication (
compartment_id,auth_type,auth_profile), not OpenAI-styleapi_key. Update the example to include the required OCI fields.Also applies to: 56-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/source/build-workflows/llms/index.md` around lines 31 - 32, Update the OCI entry and its YAML example so they use OCI SDK auth fields instead of OpenAI-style api_key: replace or augment the `oci` provider example to include `compartment_id`, `auth_type`, and `auth_profile` in the sample configuration and update the table description for "OCI Hosted OpenAI-Compatible" to state it uses OCI SDK authentication; ensure any example YAML referencing `api_key` for `oci` is changed to demonstrate `compartment_id`, `auth_type`, and `auth_profile` usage.packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.py (1)
63-69:⚠️ Potential issue | 🟡 MinorRegistration test is self-seeded and doesn't validate the decorator.
The test writes
{OCIModelConfig: oci_llm}intoregistry._llm_provider_mapthen asserts the same entry. This would pass even if@register_llm_providerwas broken. Test the actual registration by importing the module and inspecting the real registry.Suggested approach
def test_oci_provider_registration(): """Verify OCIModelConfig is registered via the decorator.""" from nat.cli.type_registry import GlobalTypeRegistry # Import triggers registration via `@register_llm_provider` decorator from nat.llm.oci_llm import OCIModelConfig, oci_llm registry = GlobalTypeRegistry.get() assert registry._llm_provider_map.get(OCIModelConfig) is oci_llm🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.py` around lines 63 - 69, The test currently seeds registry._llm_provider_map directly and doesn't exercise the `@register_llm_provider` decorator; change test_oci_provider_registration to import the module that defines OCIModelConfig and oci_llm (so the decorator runs), obtain the real registry via GlobalTypeRegistry.get() (or GlobalTypeRegistry.get.return_value if still patching the class but better: remove the patch), and assert registry._llm_provider_map.get(OCIModelConfig) is oci_llm; reference the symbols GlobalTypeRegistry, OCIModelConfig, oci_llm and the test function test_oci_provider_registration when making this change.packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py (1)
232-261:⚠️ Potential issue | 🟠 Major
max_retriesandrequest_timeoutare exposed in config but not applied.
OCIModelConfigexposesmax_retriesandrequest_timeoutas public fields, but they are not passed toChatOCIGenAI. According tolangchain-ociv0.2.4, these must be configured on the underlying OCI SDK client and passed via theclient=parameter. Either implement proper support or remove these fields fromOCIModelConfigand documentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py` around lines 232 - 261, The OCIModelConfig fields max_retries and request_timeout are not being applied in oci_langchain; update oci_langchain to build/configure the underlying OCI SDK client with those settings and pass it into ChatOCIGenAI via the client= parameter (instead of only using service_endpoint/compartment_id/auth_*), ensuring the SDK client's retry and timeout are set from llm_config.max_retries and llm_config.request_timeout before calling _patch_llm_based_on_config on the returned client.
🧹 Nitpick comments (4)
docs/source/build-workflows/llms/index.md (1)
126-126: Minor: Add hyphen for compound modifier.Per static analysis hint, "OCI Hosted" should be hyphenated when used as a compound modifier.
Suggested fix
-### OCI Hosted OpenAI-Compatible +### OCI-Hosted OpenAI-Compatible🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/source/build-workflows/llms/index.md` at line 126, The heading "OCI Hosted OpenAI-Compatible" should use a hyphenated compound modifier; update the markdown heading text "OCI Hosted OpenAI-Compatible" to "OCI-Hosted OpenAI-Compatible" so the compound modifier is correct (locate the heading string in the file and replace it accordingly).packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py (1)
257-257: Simplifyis_streamassignment.
OCIModelConfigdoes not define astreamattribute, sogetattr(llm_config, "stream", False)always returnsFalse. Consider usingis_stream=Falsedirectly for clarity.Suggested fix
- is_stream=getattr(llm_config, "stream", False), + is_stream=False,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py` at line 257, The is_stream argument is being set via getattr(llm_config, "stream", False) even though OCIModelConfig has no stream attribute; replace that expression with the literal is_stream=False to make intent explicit and remove the unnecessary getattr usage (update the call site where is_stream=getattr(llm_config, "stream", False) and use is_stream=False, referencing the llm_config variable and the is_stream parameter).packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.py (1)
42-53: Test uses auth type instead of profile name.Line 47 uses
auth_profile="API_KEY_AUTH"butauth_profileshould be an OCI config profile name (e.g.,"DEFAULT"or"API_KEY_AUTH"as a profile section name). IfAPI_KEY_AUTHis intended as a profile name, this is fine; otherwise, consider using"DEFAULT"for consistency with the model defaults.Suggested fix for consistency
- auth_profile="API_KEY_AUTH", + auth_profile="DEFAULT",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.py` around lines 42 - 53, The test test_oci_llm_provider_yields_provider_info sets OCIModelConfig.auth_profile to "API_KEY_AUTH" which should be an OCI config profile name; update the test to use a canonical profile name like "DEFAULT" (or replace with the actual profile section name you expect) so OCIModelConfig in the test matches the intended OCI config profile; modify the OCIModelConfig instantiation in test_oci_llm_provider_yields_provider_info (used by oci_llm) to set auth_profile="DEFAULT" (or the real profile name) to keep the test consistent with model defaults and the oci_llm provider expectations.packages/nvidia_nat_core/src/nat/llm/oci_llm.py (1)
60-60: Potential confusion betweenmax_retriesandRetryMixin.num_retries.
OCIModelConfiginherits fromRetryMixin(which providesnum_retriesfor the retry patching mechanism) and also defines its ownmax_retriesfield. Sincemax_retriesisn't currently wired toChatOCIGenAI, consider whether both fields are needed or if they should be consolidated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nvidia_nat_core/src/nat/llm/oci_llm.py` at line 60, OCIModelConfig defines max_retries while RetryMixin already provides num_retries, causing confusion; consolidate them by removing the standalone max_retries field and using the RetryMixin.num_retries value throughout, or (to preserve backwards compatibility) sync them by setting RetryMixin.num_retries = max_retries during OCIModelConfig initialization/validation; also update ChatOCIGenAI to read the retry count from config.num_retries (not config.max_retries) so the retry patching mechanism uses the single authoritative value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/source/components/integrations/integrating-oci-generative-ai-models.md`:
- Around line 50-52: The example config uses auth_profile incorrectly—replace
the auth_profile value so it contains an OCI config profile name (e.g.,
"DEFAULT") rather than an auth-type string; update the snippet that currently
sets auth_type: API_KEY and auth_profile: API_KEY_AUTH to use the proper OCI
profile name (per OCIModelConfig) and ensure auth_type remains the appropriate
enum if needed, so consumers know auth_profile refers to a ~/.oci/config profile
section like "DEFAULT".
- Around line 90-92: Update the Usage example so the auth_profile value matches
the earlier example by replacing the auth_profile entry from "API_KEY_AUTH" to
"DEFAULT"; locate the auth_profile setting in the usage block (near
compartment_id and other OCI config entries) and change its value to "DEFAULT"
to ensure consistency across examples.
---
Duplicate comments:
In `@docs/source/build-workflows/llms/index.md`:
- Around line 126-148: The docs currently list API key fields but the
implementation uses OCI SDK auth; update the OCI Hosted OpenAI-Compatible
section to reflect the actual OCIModelConfig fields used in oci_llm.py (class
OCIModelConfig) by replacing `api_key`/OCI_GENAI_* entries with the SDK auth
fields: `compartment_id`, `auth_type`, `auth_profile`, `auth_file_location`, and
`provider`; also note that this provider targets OCI-hosted OpenAI-compatible
chat-completions endpoints (Responses API not enabled) and remove or correct any
references to `OCI_GENAI_API_KEY`/`api_key` to avoid mismatch with
OCIModelConfig.
- Around line 31-32: Update the OCI entry and its YAML example so they use OCI
SDK auth fields instead of OpenAI-style api_key: replace or augment the `oci`
provider example to include `compartment_id`, `auth_type`, and `auth_profile` in
the sample configuration and update the table description for "OCI Hosted
OpenAI-Compatible" to state it uses OCI SDK authentication; ensure any example
YAML referencing `api_key` for `oci` is changed to demonstrate `compartment_id`,
`auth_type`, and `auth_profile` usage.
In `@packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.py`:
- Around line 63-69: The test currently seeds registry._llm_provider_map
directly and doesn't exercise the `@register_llm_provider` decorator; change
test_oci_provider_registration to import the module that defines OCIModelConfig
and oci_llm (so the decorator runs), obtain the real registry via
GlobalTypeRegistry.get() (or GlobalTypeRegistry.get.return_value if still
patching the class but better: remove the patch), and assert
registry._llm_provider_map.get(OCIModelConfig) is oci_llm; reference the symbols
GlobalTypeRegistry, OCIModelConfig, oci_llm and the test function
test_oci_provider_registration when making this change.
In `@packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py`:
- Around line 232-261: The OCIModelConfig fields max_retries and request_timeout
are not being applied in oci_langchain; update oci_langchain to build/configure
the underlying OCI SDK client with those settings and pass it into ChatOCIGenAI
via the client= parameter (instead of only using
service_endpoint/compartment_id/auth_*), ensuring the SDK client's retry and
timeout are set from llm_config.max_retries and llm_config.request_timeout
before calling _patch_llm_based_on_config on the returned client.
---
Nitpick comments:
In `@docs/source/build-workflows/llms/index.md`:
- Line 126: The heading "OCI Hosted OpenAI-Compatible" should use a hyphenated
compound modifier; update the markdown heading text "OCI Hosted
OpenAI-Compatible" to "OCI-Hosted OpenAI-Compatible" so the compound modifier is
correct (locate the heading string in the file and replace it accordingly).
In `@packages/nvidia_nat_core/src/nat/llm/oci_llm.py`:
- Line 60: OCIModelConfig defines max_retries while RetryMixin already provides
num_retries, causing confusion; consolidate them by removing the standalone
max_retries field and using the RetryMixin.num_retries value throughout, or (to
preserve backwards compatibility) sync them by setting RetryMixin.num_retries =
max_retries during OCIModelConfig initialization/validation; also update
ChatOCIGenAI to read the retry count from config.num_retries (not
config.max_retries) so the retry patching mechanism uses the single
authoritative value.
In `@packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.py`:
- Around line 42-53: The test test_oci_llm_provider_yields_provider_info sets
OCIModelConfig.auth_profile to "API_KEY_AUTH" which should be an OCI config
profile name; update the test to use a canonical profile name like "DEFAULT" (or
replace with the actual profile section name you expect) so OCIModelConfig in
the test matches the intended OCI config profile; modify the OCIModelConfig
instantiation in test_oci_llm_provider_yields_provider_info (used by oci_llm) to
set auth_profile="DEFAULT" (or the real profile name) to keep the test
consistent with model defaults and the oci_llm provider expectations.
In `@packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py`:
- Line 257: The is_stream argument is being set via getattr(llm_config,
"stream", False) even though OCIModelConfig has no stream attribute; replace
that expression with the literal is_stream=False to make intent explicit and
remove the unnecessary getattr usage (update the call site where
is_stream=getattr(llm_config, "stream", False) and use is_stream=False,
referencing the llm_config variable and the is_stream parameter).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ce069a94-587f-4879-bad4-fca9f7ec4ba0
⛔ Files ignored due to path filters (2)
packages/nvidia_nat_langchain/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
docs/source/build-workflows/llms/index.mddocs/source/components/integrations/index.mddocs/source/components/integrations/integrating-oci-generative-ai-models.mddocs/source/conf.pydocs/source/get-started/installation.mdexamples/frameworks/agno_personal_finance/pyproject.tomlexamples/frameworks/multi_frameworks/pyproject.tomlpackages/nvidia_nat_agno/pyproject.tomlpackages/nvidia_nat_core/src/nat/llm/oci_llm.pypackages/nvidia_nat_core/src/nat/llm/register.pypackages/nvidia_nat_core/tests/nat/llm/test_oci_llm.pypackages/nvidia_nat_langchain/pyproject.tomlpackages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.pypackages/nvidia_nat_langchain/tests/test_langchain_agents.pypackages/nvidia_nat_langchain/tests/test_llm_langchain.pypackages/nvidia_nat_test/src/nat/test/plugin.pypyproject.toml
🚧 Files skipped from review as they are similar to previous changes (7)
- examples/frameworks/agno_personal_finance/pyproject.toml
- packages/nvidia_nat_core/src/nat/llm/register.py
- docs/source/get-started/installation.md
- packages/nvidia_nat_langchain/pyproject.toml
- examples/frameworks/multi_frameworks/pyproject.toml
- packages/nvidia_nat_langchain/tests/test_llm_langchain.py
- packages/nvidia_nat_test/src/nat/test/plugin.py
docs/source/components/integrations/integrating-oci-generative-ai-models.md
Show resolved
Hide resolved
docs/source/components/integrations/integrating-oci-generative-ai-models.md
Show resolved
Hide resolved
fede-kamel
force-pushed
the
fk/oci-langchain-nemotron
branch
from
3daa5eb to
5f441da
Compare
Signed-off-by: Federico Kamelhar <federico.kamelhar@oracle.com>
fede-kamel
force-pushed
the
fk/oci-langchain-nemotron
branch
from
5f441da to
50b208c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/nvidia_nat_core/src/nat/llm/oci_llm.py (1)
62-62: Consider documenting the distinction betweenmax_retriesandnum_retries.
OCIModelConfiginheritsRetryMixinwhich providesnum_retries(used by NAT's_patch_llm_based_on_configfor application-level retries), whilemax_retriesis defined here for OCI SDK-level retries. This dual-retry mechanism is intentional but could be confusing. Consider adding a brief note in the field description clarifying this is for OCI SDK retries specifically.📝 Suggested clarification
- max_retries: int = Field(default=10, description="The max number of retries for the request.") + max_retries: int = Field(default=10, description="The max number of OCI SDK-level retries for the request.")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nvidia_nat_core/src/nat/llm/oci_llm.py` at line 62, The max_retries field in OCIModelConfig currently reads generically; clarify that it controls OCI SDK-level retries (not NAT's application-level retry loop) by updating the Field description to state it is for OCI SDK/client retries, while num_retries (from RetryMixin) is used by NAT's application-level retry logic in _patch_llm_based_on_config. Reference OCIModelConfig.max_retries, RetryMixin.num_retries and the _patch_llm_based_on_config usage to ensure the distinction is clear in the docstring.packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.py (1)
65-76: LGTM - Registration test properly validates decorator behavior.The test now correctly patches
GlobalTypeRegistry, reimports the module to trigger decorator registration, and asserts the registration call was made with the correct config type and build function. This addresses the previous review comment about self-seeding.Note: The file is missing a trailing newline at line 76.
📝 Add trailing newline
assert info.config_type is module.OCIModelConfig assert info.build_fn is module.oci_llm +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.py` around lines 65 - 76, Add a missing trailing newline at EOF of the test file containing the test_oci_provider_registration function so the file ends with a newline character; simply edit the file to insert a single newline after the last line (after the end of the test function) to satisfy POSIX/formatter expectations.packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py (3)
232-277: Add a docstring for the wrapper function.Per coding guidelines, public APIs require Google-style docstrings. This wrapper follows the same pattern as other LangChain wrappers (e.g.,
aws_bedrock_langchain,nim_langchain) which also lack docstrings, but adding one would improve documentation consistency.📝 Suggested docstring
`@register_llm_client`(config_type=OCIModelConfig, wrapper_type=LLMFrameworkEnum.LANGCHAIN) async def oci_langchain(llm_config: OCIModelConfig, _builder: Builder): + """Create a LangChain ChatOCIGenAI client for OCI Generative AI. + + Args: + llm_config: OCI model configuration. + _builder: Builder instance. + + Yields: + A ChatOCIGenAI client configured for the specified OCI model. + """ import oci🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py` around lines 232 - 277, Add a Google-style docstring to the public wrapper function oci_langchain describing its purpose, parameters, and yield value consistent with other LangChain wrappers; mention that it registers an OCI Generative AI client, explain the llm_config (OCIModelConfig) and _builder parameters, note returned/yielded value is a configured LangChain-compatible chat model (result of _patch_llm_based_on_config), and include summary, Args, and Yields sections to match project docstring conventions.
58-61: Consider adding a return type hint.For consistency with coding guidelines requiring type hints on all public APIs, and for better IDE support, consider adding a return type annotation.
♻️ Suggested improvement
-def _get_langchain_oci_chat_model(): +def _get_langchain_oci_chat_model() -> type: from langchain_oci import ChatOCIGenAI return ChatOCIGenAI🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py` around lines 58 - 61, The helper _get_langchain_oci_chat_model should have an explicit return type; update its signature to include a type hint returning the class object (e.g., -> Type[ChatOCIGenAI]) and ensure typing.Type is imported (either at module top or inside the function) so the signature becomes _get_langchain_oci_chat_model() -> Type[ChatOCIGenAI]; keep the local import of ChatOCIGenAI and only adjust the function signature and imports to satisfy the public API typing guideline.
273-273: Consider adding astreamfield toOCIModelConfigif streaming is intended.
OCIModelConfigdoes not define astreamfield. The expressiongetattr(llm_config, "stream", False)will always returnFalsefor OCI configurations. If streaming support is intended for OCI, add astreamfield toOCIModelConfig. Otherwise, simplify this line tois_stream=Falsefor clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py` at line 273, The call to set is_stream uses getattr(llm_config, "stream", False) but OCIModelConfig does not define a stream field, so streaming will never be enabled for OCI; either add a boolean stream attribute to OCIModelConfig (e.g., stream: bool = False) and ensure code that constructs OCIModelConfig can set it, or change the caller to set is_stream=False explicitly; update references around the is_stream assignment and the OCIModelConfig definition to keep them consistent (look for OCIModelConfig and the line using getattr(llm_config, "stream", False) to apply the fix).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/nvidia_nat_core/src/nat/llm/oci_llm.py`:
- Line 33: OCIModelConfig inherits SSLVerificationMixin exposing verify_ssl but
that value is never passed to the OCI client; either wire verify_ssl into the
OCI client construction or remove the mixin. Locate the OCI client creation in
oci_llm.py (the code that constructs the oci_langchain/OCI client inside the
OCIModel/OCI wrapper) and if the OCI SDK or oci_langchain wrapper supports
disabling SSL verification, pass OCIModelConfig.verify_ssl into the
client/configuration call (or into any HttpClient/requests adapter used);
otherwise remove SSLVerificationMixin from the OCIModelConfig class definition
so the unused verify_ssl field is not exposed. Ensure you reference
OCIModelConfig and the client construction code when making the change.
In `@packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.py`:
- Around line 44-55: The test test_oci_llm_provider_yields_provider_info
references a missing pytest fixture mock_builder causing "fixture not found"
failures; add a fixture named mock_builder (returning a MagicMock) either by
creating a conftest.py in the same llm test directory with a pytest.fixture
named "mock_builder" that returns MagicMock(), or define the same fixture at the
top of this test file before test_oci_llm_provider_yields_provider_info; ensure
you import pytest and MagicMock and keep the fixture name exactly "mock_builder"
so the async test using oci_llm can use it.
---
Nitpick comments:
In `@packages/nvidia_nat_core/src/nat/llm/oci_llm.py`:
- Line 62: The max_retries field in OCIModelConfig currently reads generically;
clarify that it controls OCI SDK-level retries (not NAT's application-level
retry loop) by updating the Field description to state it is for OCI SDK/client
retries, while num_retries (from RetryMixin) is used by NAT's application-level
retry logic in _patch_llm_based_on_config. Reference OCIModelConfig.max_retries,
RetryMixin.num_retries and the _patch_llm_based_on_config usage to ensure the
distinction is clear in the docstring.
In `@packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.py`:
- Around line 65-76: Add a missing trailing newline at EOF of the test file
containing the test_oci_provider_registration function so the file ends with a
newline character; simply edit the file to insert a single newline after the
last line (after the end of the test function) to satisfy POSIX/formatter
expectations.
In `@packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py`:
- Around line 232-277: Add a Google-style docstring to the public wrapper
function oci_langchain describing its purpose, parameters, and yield value
consistent with other LangChain wrappers; mention that it registers an OCI
Generative AI client, explain the llm_config (OCIModelConfig) and _builder
parameters, note returned/yielded value is a configured LangChain-compatible
chat model (result of _patch_llm_based_on_config), and include summary, Args,
and Yields sections to match project docstring conventions.
- Around line 58-61: The helper _get_langchain_oci_chat_model should have an
explicit return type; update its signature to include a type hint returning the
class object (e.g., -> Type[ChatOCIGenAI]) and ensure typing.Type is imported
(either at module top or inside the function) so the signature becomes
_get_langchain_oci_chat_model() -> Type[ChatOCIGenAI]; keep the local import of
ChatOCIGenAI and only adjust the function signature and imports to satisfy the
public API typing guideline.
- Line 273: The call to set is_stream uses getattr(llm_config, "stream", False)
but OCIModelConfig does not define a stream field, so streaming will never be
enabled for OCI; either add a boolean stream attribute to OCIModelConfig (e.g.,
stream: bool = False) and ensure code that constructs OCIModelConfig can set it,
or change the caller to set is_stream=False explicitly; update references around
the is_stream assignment and the OCIModelConfig definition to keep them
consistent (look for OCIModelConfig and the line using getattr(llm_config,
"stream", False) to apply the fix).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 40b7b228-d85f-4048-930c-fa04f1f216bb
⛔ Files ignored due to path filters (2)
packages/nvidia_nat_langchain/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
docs/source/build-workflows/llms/index.mddocs/source/components/integrations/index.mddocs/source/components/integrations/integrating-oci-generative-ai-models.mddocs/source/conf.pydocs/source/get-started/installation.mdexamples/frameworks/agno_personal_finance/pyproject.tomlexamples/frameworks/multi_frameworks/pyproject.tomlpackages/nvidia_nat_agno/pyproject.tomlpackages/nvidia_nat_core/src/nat/llm/oci_llm.pypackages/nvidia_nat_core/src/nat/llm/register.pypackages/nvidia_nat_core/tests/nat/llm/test_oci_llm.pypackages/nvidia_nat_langchain/pyproject.tomlpackages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.pypackages/nvidia_nat_langchain/tests/test_langchain_agents.pypackages/nvidia_nat_langchain/tests/test_llm_langchain.pypackages/nvidia_nat_test/src/nat/test/plugin.pypyproject.toml
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/nvidia_nat_langchain/tests/test_langchain_agents.py
- docs/source/build-workflows/llms/index.md
- packages/nvidia_nat_langchain/pyproject.toml
- docs/source/components/integrations/integrating-oci-generative-ai-models.md
- packages/nvidia_nat_agno/pyproject.toml
- docs/source/conf.py
- examples/frameworks/multi_frameworks/pyproject.toml
- pyproject.toml
- packages/nvidia_nat_test/src/nat/test/plugin.py
| from nat.data_models.ssl_verification_mixin import SSLVerificationMixin | ||
| from nat.data_models.thinking_mixin import ThinkingMixin | ||
|
|
||
| class OCIModelConfig(LLMBaseConfig, RetryMixin, OptimizableMixin, ThinkingMixin, SSLVerificationMixin, name="oci"): |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if verify_ssl is used in the OCI langchain wrapper
rg -n "verify_ssl" packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py
# Check OCI SDK documentation for SSL verification options
rg -n "verify" packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.pyRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 840
🏁 Script executed:
#!/bin/bash
# Find the OCI wrapper implementation in llm.py
rg -n "class.*OCI" packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py
# Search for where OCIModelConfig is used
rg -n "OCIModelConfig" packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py
# Look for the OCI client initialization (likely ChatOCI or similar)
rg -n -A 10 "def.*oci" packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py -iRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 1156
🏁 Script executed:
#!/bin/bash
# Read the complete oci_langchain function
sed -n '233,280p' packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.pyRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 1978
🏁 Script executed:
#!/bin/bash
# Check how verify_ssl is handled in other wrapper functions
rg -n -B 5 -A 15 "_handle_litellm_verify_ssl" packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py
# Check other wrapper implementations to see how they use verify_ssl
rg -n -B 2 -A 10 "verify_ssl" packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py | head -100Repository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 3574
🏁 Script executed:
#!/bin/bash
# Check what parameters create_oci_client_kwargs accepts
rg -n "def create_oci_client_kwargs" --type python
# Look for any mention of verify or ssl in relation to OCI
rg -n "verify|ssl" packages/nvidia_nat_langchain/src/nat/plugins/langchain/llm.py | grep -i "oci\|line 2[3-7][0-9]"
# Check if there's documentation or type hints for create_oci_client_kwargs
find . -name "*.py" -exec grep -l "create_oci_client_kwargs" {} \;Repository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 241
🏁 Script executed:
#!/bin/bash
# Check the test file for OCI configuration
rg -n "oci\|OCI\|verify_ssl" packages/nvidia_nat_langchain/tests/test_llm_langchain.py -iRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Check if test file exists and what's in it
head -50 packages/nvidia_nat_langchain/tests/test_llm_langchain.py
# Search for OCI tests across all test files
find . -path ./node_modules -prune -o -name "*.py" -type f -exec grep -l "OCIModelConfig\|oci_langchain" {} \;Repository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 2332
🏁 Script executed:
#!/bin/bash
# Check OCI-specific tests
rg -n "verify_ssl\|SSLVerificationMixin" packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.py
# Check OCI wrapper tests
rg -n -A 20 "class.*OCI.*Test\|def.*oci.*test" packages/nvidia_nat_langchain/tests/test_llm_langchain.py -iRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Read the OCI test file
cat packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.py
# Find OCI-related tests in langchain test file
rg -n "oci\|OCI" packages/nvidia_nat_langchain/tests/test_llm_langchain.py -iRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 2906
Wire verify_ssl into the OCI client or remove SSLVerificationMixin from OCIModelConfig.
OCIModelConfig inherits SSLVerificationMixin providing a verify_ssl field, but the oci_langchain wrapper doesn't use this field when constructing the OCI client. Unlike other LLM wrappers that either pass verify_ssl through (NVIDIA) or explicitly handle it (LiteLLM), the OCI wrapper ignores it entirely. Either pass verify_ssl to the OCI client configuration or remove the mixin if SSL verification customization isn't supported by the OCI SDK.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nvidia_nat_core/src/nat/llm/oci_llm.py` at line 33, OCIModelConfig
inherits SSLVerificationMixin exposing verify_ssl but that value is never passed
to the OCI client; either wire verify_ssl into the OCI client construction or
remove the mixin. Locate the OCI client creation in oci_llm.py (the code that
constructs the oci_langchain/OCI client inside the OCIModel/OCI wrapper) and if
the OCI SDK or oci_langchain wrapper supports disabling SSL verification, pass
OCIModelConfig.verify_ssl into the client/configuration call (or into any
HttpClient/requests adapter used); otherwise remove SSLVerificationMixin from
the OCIModelConfig class definition so the unused verify_ssl field is not
exposed. Ensure you reference OCIModelConfig and the client construction code
when making the change.
Comment on lines
+44
to
+55
| async def test_oci_llm_provider_yields_provider_info(mock_builder): | ||
| config = OCIModelConfig( | ||
| model_name="nvidia/Llama-3.1-Nemotron-Nano-8B-v1", | ||
| endpoint="https://inference.generativeai.us-chicago-1.oci.oraclecloud.com", | ||
| compartment_id="ocid1.compartment.oc1..example", | ||
| auth_profile="API_KEY_AUTH", | ||
| ) | ||
|
|
||
| async with oci_llm(config, mock_builder) as provider: | ||
| assert isinstance(provider, LLMProviderInfo) | ||
| assert provider.config is config | ||
| assert "OCI" in provider.description |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify mock_builder fixture is defined in conftest.py
rg -n "mock_builder" packages/nvidia_nat_core/tests/conftest.pyRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Find all conftest.py files in the test directory
find packages/nvidia_nat_core/tests -name "conftest.py" 2>/dev/nullRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 134
🏁 Script executed:
#!/bin/bash
# Search for mock_builder fixture definition in the entire test directory
rg -r "mock_builder" packages/nvidia_nat_core/tests --type pyRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Verify the test file exists
ls -la packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.pyRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 172
🏁 Script executed:
#!/bin/bash
# Read the actual test file to see mock_builder usage
cat -n packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.pyRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 3438
🏁 Script executed:
#!/bin/bash
# Search for mock_builder definition across entire repository
rg "mock_builder" packages/nvidia_nat_core --type py -A 2Repository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 50382
🏁 Script executed:
#!/bin/bash
# Check the conftest.py that exists to see what fixtures are defined there
cat -n packages/nvidia_nat_core/tests/nat/front_ends/fastapi/conftest.pyRepository: NVIDIA/NeMo-Agent-Toolkit
Length of output: 1834
Missing fixture definition prevents test from running.
The test uses mock_builder as a parameter, but no fixture with that name is defined for this test file. There is no conftest.py in packages/nvidia_nat_core/tests/nat/llm/ or parent directories that provides this fixture. Pytest will fail with "fixture 'mock_builder' not found" at runtime.
Add a fixture definition. Following the pattern used in other test files (e.g., test_defense_middleware.py), create a conftest.py in the llm/ test directory with:
`@pytest.fixture`(name="mock_builder")
def fixture_mock_builder():
"""Create a mock builder."""
return MagicMock()Or define the fixture directly in the test file if it's only used here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.py` around lines 44 - 55,
The test test_oci_llm_provider_yields_provider_info references a missing pytest
fixture mock_builder causing "fixture not found" failures; add a fixture named
mock_builder (returning a MagicMock) either by creating a conftest.py in the
same llm test directory with a pytest.fixture named "mock_builder" that returns
MagicMock(), or define the same fixture at the top of this test file before
test_oci_llm_provider_yields_provider_info; ensure you import pytest and
MagicMock and keep the fixture name exactly "mock_builder" so the async test
using oci_llm can use it.
Contributor
|
Hello @fede-kamel, Is there an issue or some other thread where this feature was requested and discussed with the maintainers? |
Author
|
@AnuradhaKaruppiah thanks for raising this. There was not a linked tracking issue for this work before the PR, so I created #1809 and linked the request/discussion there. The issue captures the main context from this PR: OCI would like to create an official integration path in NAT similar to the existing cloud-provider integrations, rather than keeping OCI support as an ad hoc path. It also calls out the intended scope around a first-class We can use #1809 as the maintainer discussion thread going forward. |
willkill07
added
feature request
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
3 participants
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.
Summary
langchain-oci, matching the workflow-layer integration shape used by AWS Bedrock in this repouvextra conflicts so the workspace remains solvable whenlangchain-ociintroducesopenai>=2alongside existingstrandsandvannasurfacesWhat Was Tested
PYTHONPATH=$(pwd)/packages/nvidia_nat_core/src:$(pwd)/packages/nvidia_nat_langchain/src:$(pwd)/packages/nvidia_nat_test/src .venv/bin/pytest packages/nvidia_nat_core/tests/nat/llm/test_oci_llm.py packages/nvidia_nat_langchain/tests/test_llm_langchain.py -q -k 'OCI or oci'OCI_NEMOTRON_BASE_URL=http://127.0.0.1:8080/v1 OCI_NEMOTRON_MODEL=nvidia/Llama-3.1-Nemotron-Nano-8B-v1 PYTHONPATH=$(pwd)/packages/nvidia_nat_core/src:$(pwd)/packages/nvidia_nat_langchain/src:$(pwd)/packages/nvidia_nat_test/src .venv/bin/pytest packages/nvidia_nat_langchain/tests/test_langchain_agents.py -q --run_integration -k oci_hosted_nemotron_openai_compatible_agentuv lockNotes
nvidia/Llama-3.1-Nemotron-Nano-8B-v1nvidia_nat_langchainSummary by CodeRabbit
New Features
Documentation
Tests
Chores