Repository navigation
feat(server): Load and validate single-configuration at startup - #2461
tgasser-nv wants to merge 1 commit into
Conversation
github-actions
Bot
added
size: M
status: needs triage
tgasser-nv
added
status: triaged
|
Staged Fern docs preview: https://nvidia-preview-pr-2461.docs.buildwithfern.com/nemo/guardrails |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 53.85% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 3 files. (2 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: loading and validating the single configuration during server startup. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Test Results For Major Changes The PR introduces a major server behavior change and adds 177 lines across startup logic and tests. The description documents testing information, including pre-commit results, make testresults, an…
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 53.85% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 3 files. (2 skipped: 2 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
 [Medium impact] Server startup now validates single configuration at boot. The PR appears safe to merge, with a non-blocking memory cleanup suggested. Findings
Fix with agent prompt### Issue 1
nemoguardrails/server/api.py:180
`app.single_config` keeps the entire parsed configuration, but no request reads it. `_get_rails` still calls `RailsConfig.from_path` separately. This leaves an unused copy of the flows and knowledge-base documents in memory for the server's lifetime.
If startup only needs to validate the configuration, discard the return value. Otherwise, connect this object to the request path and account for auto-reload.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryThe server now parses and validates a single configuration during startup, so parsing or validation errors stop startup.
Reviews (1) · Last reviewed commit: "Validate config in server single-config ..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # Loading here rather than on the first request makes an invalid config stop | ||
| # the server from starting. Load before setting the mode, so a failed load | ||
| # leaves no partial single-config state. | ||
| app.single_config = RailsConfig.from_path(app.rails_config_path) |
There was a problem hiding this comment.
app.single_config keeps the entire parsed configuration, but no request reads it. _get_rails still calls RailsConfig.from_path separately. This leaves an unused copy of the flows and knowledge-base documents in memory for the server's lifetime.
If startup only needs to validate the configuration, discard the return value. Otherwise, connect this object to the request path and account for auto-reload.
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/server/api.py
Line: 180
Comment:
**Unused configuration consumes memory**
`app.single_config` keeps the entire parsed configuration, but no request reads it. `_get_rails` still calls `RailsConfig.from_path` separately. This leaves an unused copy of the flows and knowledge-base documents in memory for the server's lifetime.
If startup only needs to validate the configuration, discard the return value. Otherwise, connect this object to the request path and account for auto-reload.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
This branch has not been deployed
1 participant
Description
The Guardrails server can run in a single-config or multi-config mode. In single-config mode, a single RailsConfig is applied to all incoming inference/check requests, and clients don't need to specify a config in their request. The
guardrailskey can be absent. In multi-config mode, the server knows a directory under which one or more configs are stored, and will load and validate the relevant config at the first inference-time.This PR makes the server load and validate the single-config file at startup, rather than at the first inference-time. This prevents the server from running invalid configs in single-config mode.
Related Issue(s)
Verification
Pre-commit
Unit-test
Integration test with Chat (broken config)
Integration-test with working config
AI Assistance
Checklist
Summary by CodeRabbit
--prefixskips startup steps, including single-configuration detection and validation.