feat: add Nexus Nemotron claw lane and sync P7 docs - #1104
Conversation
📝 WalkthroughWalkthroughA new Model Nexus provider parity registry establishes a contract layer for model routing and provider compatibility. The PR adds configuration files for Nemotron Claw GPU node integration, updates P7/TTS test documentation, implements Python utilities for model routing validation, and includes supporting agent profile and TAC tree updates. Changes
Sequence DiagramsequenceDiagram
participant Agent as Agent Code
participant Nexus as Model Nexus<br/>(Config Loader)
participant Router as Provider Router
participant Primary as Primary Provider<br/>(TensorZero/OpenAI/NIM)
participant Fallback as Fallback<br/>(TensorZero)
Agent->>Nexus: Load model_nexus.yaml
Nexus-->>Agent: Config (providers, lanes, rules)
Agent->>Router: Route request for lane
Router->>Nexus: Check lane default_provider
Nexus-->>Router: Default provider name
Router->>Nexus: Validate provider allowed for lane
Nexus-->>Router: Allowed (default or override)
Router->>Nexus: Check if provider requires adapter
alt Provider requires Nexus adapter
Nexus-->>Router: true
Router->>Primary: Call via adapter (with parity checks)
else Provider is direct (e.g., OpenAI)
Nexus-->>Router: false
Router->>Primary: Direct SDK call
end
alt Response/parity preserves requirements
Primary-->>Router: Valid response
Router-->>Agent: Route to primary result
else Parity cannot be maintained
Router->>Fallback: Fallback to TensorZero
Fallback-->>Router: TensorZero result
Router-->>Agent: Route via fallback
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72852c9f3a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """ | ||
|
|
||
| from .telemetry import * # noqa: F401,F403 | ||
| from .model_nexus import ( # noqa: F401 |
There was a problem hiding this comment.
Importing services.common now unconditionally imports model_nexus, which imports yaml at module load time. Many runtime services import services.common.* during startup, but several service requirement sets do not declare PyYAML, so this change can raise ModuleNotFoundError: No module named 'yaml' before the service boots. This should be made optional/lazy (like other optional exports) or accompanied by dependency updates for every affected service environment.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
pmoves/services/common/tests/test_model_nexus.py (1)
7-11: Consider using pytest'sconftest.pyfor import path setup.The
sys.pathandsys.modulesmanipulation works but creates global side-effects. Moving this setup to aconftest.pyfile in the tests directory would centralize the import configuration and make it reusable across test modules.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pmoves/services/common/tests/test_model_nexus.py` around lines 7 - 11, Move the test import path setup out of the test file and into a tests conftest: instead of manipulating repo_root, sys.path.append and sys.modules.setdefault directly in test_model_nexus.py, create a conftest.py that performs the same setup (e.g. in a pytest_configure or a session-scoped fixture) — compute repo_root the same way, append it to sys.path if missing, and call importlib.import_module("pmoves.services") (or set sys.modules.setdefault("services", ...)) there so all tests reuse the centralized import-path configuration without per-test global side-effects.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pmoves/configs/agent-profiles/nemotron_claw.yaml`:
- Around line 31-32: Remove the embedded credentials from the NATS URL by
replacing the hard-coded value "nats://nats:pmoves@nats:4222" with a
credential-free endpoint (e.g., "nats://nats:4222") in the nats.url setting, and
change any consumer of that profile to obtain username/password from environment
variables or a secret file instead (wire auth into the NATS client
initialization code that reads this profile); ensure the subscribe section and
any code that builds the connection (lookup code using nats.url) uses the
externalized credentials rather than relying on the profile to contain secrets.
In `@pmoves/configs/tac_trees/pinokio-p7.tac.yaml`:
- Line 72: Update the expectation string that currently reads "Claude Code
'speak hello' routes through Pinokio to TTS at :7860 or a fixed HTTPS domain" to
also include the current deployment port :7861 (and keep the optional HTTPS
domain reference) so checks match pmoves defaults; locate the YAML key/value
where expect is set (the expect string on the tac tree for Pinokio, e.g., the
expect entry containing "Claude Code 'speak hello'") and modify that text to
mention ":7861" (for example, "at :7861 or a fixed HTTPS domain" or include both
:7860 and :7861 if you prefer backward compatibility).
In `@pmoves/services/common/model_nexus.py`:
- Around line 49-50: The code uses "data = config or load_model_nexus()" which
treats an explicitly provided empty dict as falsy and reloads disk config;
change this to only call load_model_nexus() when config is None (e.g., use "data
= config if config is not None else load_model_nexus()") so explicit empty
configs are honored. Apply the same change for the similar pattern around lines
referencing the same assignment (the other occurrence at lines 59-60) and keep
references to the variable name "data" and function "load_model_nexus" to locate
and update the logic.
- Around line 24-25: Cached loader functions (_load_model_nexus_cached and the
public load_model_nexus) currently return the same mutable dict instance,
letting callers mutate shared state; update the cache-return path to return a
defensive copy (e.g., use copy.deepcopy on the cached dict) before returning so
callers receive an independent instance, and apply the same change to the other
cached loaders referenced around the other locations (lines 32-33 and 43) to
avoid leaking shared mutable state.
---
Nitpick comments:
In `@pmoves/services/common/tests/test_model_nexus.py`:
- Around line 7-11: Move the test import path setup out of the test file and
into a tests conftest: instead of manipulating repo_root, sys.path.append and
sys.modules.setdefault directly in test_model_nexus.py, create a conftest.py
that performs the same setup (e.g. in a pytest_configure or a session-scoped
fixture) — compute repo_root the same way, append it to sys.path if missing, and
call importlib.import_module("pmoves.services") (or set
sys.modules.setdefault("services", ...)) there so all tests reuse the
centralized import-path configuration without per-test global side-effects.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0e26671d-8c6a-4026-b9db-94552adab8db
📒 Files selected for processing (12)
pmoves/config/model_nexus.yamlpmoves/configs/agent-profiles/nemotron_claw.yamlpmoves/configs/agent-teams.yamlpmoves/configs/claws/opencode-nemotron-claw.jsonpmoves/configs/claws/scopes/nemotron-claw.jsonpmoves/configs/tac_trees/pinokio-p7.tac.yamlpmoves/docs/AGENTS/AGNOTE_P7_PLAYGROUND.mdpmoves/docs/MODEL_FABRIC_CONTRACT.mdpmoves/docs/NEXUS_PROVIDER_PARITY.mdpmoves/services/common/__init__.pypmoves/services/common/model_nexus.pypmoves/services/common/tests/test_model_nexus.py
| url: nats://nats:pmoves@nats:4222 | ||
| subscribe: |
There was a problem hiding this comment.
Remove embedded NATS credentials from profile config
Line 31 hard-codes Basic Auth in nats.url (nats://nats:pmoves@nats:4222). This leaks credential material into config history and makes rotation harder. Keep URL credential-free and source auth from env/secret-file wiring instead.
🧰 Tools
🪛 Checkov (3.2.508)
[medium] 31-32: Basic Auth Credentials
(CKV_SECRET_4)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pmoves/configs/agent-profiles/nemotron_claw.yaml` around lines 31 - 32,
Remove the embedded credentials from the NATS URL by replacing the hard-coded
value "nats://nats:pmoves@nats:4222" with a credential-free endpoint (e.g.,
"nats://nats:4222") in the nats.url setting, and change any consumer of that
profile to obtain username/password from environment variables or a secret file
instead (wire auth into the NATS client initialization code that reads this
profile); ensure the subscribe section and any code that builds the connection
(lookup code using nats.url) uses the externalized credentials rather than
relying on the profile to contain secrets.
| action: | ||
| type: manual | ||
| expect: "Claude Code 'speak hello' routes through Pinokio to TTS at :7861" | ||
| expect: "Claude Code 'speak hello' routes through Pinokio to TTS at :7860 or a fixed HTTPS domain" |
There was a problem hiding this comment.
Update TTS port expectation to match current deployment default
Line 72 expects :7860, but current PMOVES deployment references host/service access on :7861 (see pmoves/docker-compose.yml and TTS_BASE_URL default). This can cause false negatives during manual checks; include :7861 (and optional HTTPS domain) in the expectation text.
Suggested wording update
- expect: "Claude Code 'speak hello' routes through Pinokio to TTS at :7860 or a fixed HTTPS domain"
+ expect: "Claude Code 'speak hello' routes through Pinokio to TTS at :7861 (deployment default) or a fixed HTTPS domain"📝 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.
| expect: "Claude Code 'speak hello' routes through Pinokio to TTS at :7860 or a fixed HTTPS domain" | |
| expect: "Claude Code 'speak hello' routes through Pinokio to TTS at :7861 (deployment default) or a fixed HTTPS domain" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pmoves/configs/tac_trees/pinokio-p7.tac.yaml` at line 72, Update the
expectation string that currently reads "Claude Code 'speak hello' routes
through Pinokio to TTS at :7860 or a fixed HTTPS domain" to also include the
current deployment port :7861 (and keep the optional HTTPS domain reference) so
checks match pmoves defaults; locate the YAML key/value where expect is set (the
expect string on the tac tree for Pinokio, e.g., the expect entry containing
"Claude Code 'speak hello'") and modify that text to mention ":7861" (for
example, "at :7861 or a fixed HTTPS domain" or include both :7860 and :7861 if
you prefer backward compatibility).
| def _load_model_nexus_cached(resolved_path: str) -> dict[str, Any]: | ||
| path = Path(resolved_path) |
There was a problem hiding this comment.
Prevent shared mutable state from leaking out of cache
_load_model_nexus_cached returns a mutable dict, and load_model_nexus returns that same cached object. Any caller mutation will contaminate future reads across the process.
Suggested fix
+from copy import deepcopy
...
def load_model_nexus(path: str | Path | None = None) -> dict[str, Any]:
@@
target = Path(path) if path is not None else model_nexus_path()
- return _load_model_nexus_cached(str(target.resolve()))
+ return deepcopy(_load_model_nexus_cached(str(target.resolve())))Also applies to: 32-33, 43-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pmoves/services/common/model_nexus.py` around lines 24 - 25, Cached loader
functions (_load_model_nexus_cached and the public load_model_nexus) currently
return the same mutable dict instance, letting callers mutate shared state;
update the cache-return path to return a defensive copy (e.g., use copy.deepcopy
on the cached dict) before returning so callers receive an independent instance,
and apply the same change to the other cached loaders referenced around the
other locations (lines 32-33 and 43) to avoid leaking shared mutable state.
| data = config or load_model_nexus() | ||
| providers = data.get("providers", {}) |
There was a problem hiding this comment.
Don’t ignore explicitly provided empty configs
Using config or load_model_nexus() treats {} as falsy and silently reloads disk config. That breaks explicit test/injected configs and can mask caller intent.
Suggested fix
- data = config or load_model_nexus()
+ data = config if config is not None else load_model_nexus()
...
- data = config or load_model_nexus()
+ data = config if config is not None else load_model_nexus()Also applies to: 59-60
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pmoves/services/common/model_nexus.py` around lines 49 - 50, The code uses
"data = config or load_model_nexus()" which treats an explicitly provided empty
dict as falsy and reloads disk config; change this to only call
load_model_nexus() when config is None (e.g., use "data = config if config is
not None else load_model_nexus()") so explicit empty configs are honored. Apply
the same change for the similar pattern around lines referencing the same
assignment (the other occurrence at lines 59-60) and keep references to the
variable name "data" and function "load_model_nexus" to locate and update the
logic.
2 participants
feat: add Nexus Nemotron claw lane and sync P7 docs
Summary
Validation
Notes
Summary by CodeRabbit
Release Notes
New Features
Documentation