fix(llmrails): load library files deterministically (2/5)#1975
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
6fec9aa to
9cad57c
Compare
59c19ea to
c0ff0c7
Compare
Greptile SummaryThis PR makes library and prompt file loading order deterministic by sorting directory and file lists during
|
| Filename | Overview |
|---|---|
| nemoguardrails/llm/prompts.py | Adds dirs.sort() and files.sort() inside the os.walk loop so prompt YAML files are loaded in a stable, platform-independent order. |
| nemoguardrails/rails/llm/llmrails.py | Adds dirs.sort() and switches to for file in sorted(files): during library .co file traversal, making bot_message insertion order deterministic across platforms. |
| tests/test_prompt_override.py | Adds a new test that verifies _load_prompts() returns files in sorted order by passing an out-of-order list through a monkeypatched os.walk stub. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[LLMRails.__init__] --> B[os.walk library_path]
B --> C[dirs.sort in-place for os.walk recursion order]
C --> D[for file in sorted files]
D --> E{file.endswith .co?}
E -- Yes --> F[parse_colang_file]
F --> G[extend config.flows]
F --> H[insert bot_messages if not already set]
E -- No --> D
I[_load_prompts] --> J[os.walk prompts_dir]
J --> K[dirs.sort + files.sort]
K --> L[for filename in files]
L --> M{.yml or .yaml?}
M -- Yes --> N[yaml.safe_load]
N --> O[extend prompts list]
M -- No --> L
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[LLMRails.__init__] --> B[os.walk library_path]
B --> C[dirs.sort in-place for os.walk recursion order]
C --> D[for file in sorted files]
D --> E{file.endswith .co?}
E -- Yes --> F[parse_colang_file]
F --> G[extend config.flows]
F --> H[insert bot_messages if not already set]
E -- No --> D
I[_load_prompts] --> J[os.walk prompts_dir]
J --> K[dirs.sort + files.sort]
K --> L[for filename in files]
L --> M{.yml or .yaml?}
M -- Yes --> N[yaml.safe_load]
N --> O[extend prompts list]
M -- No --> L
Reviews (10): Last reviewed commit: "test(prompts): isolate prompt loading or..." | Re-trigger Greptile
c0ff0c7 to
b3c164e
Compare
7310fa7 to
6b783f2
Compare
b3c164e to
f2aa414
Compare
3d885f4 to
7000fcc
Compare
d44488d to
68db0d2
Compare
05def3f to
f912ae5
Compare
ab362c6 to
4aa1590
Compare
f912ae5 to
e5a52c6
Compare
6bf95dc to
b6d7a5b
Compare
e5a52c6 to
14ddbc7
Compare
tgasser-nv
left a comment
There was a problem hiding this comment.
Looks good, need to add unit-tests for the library traversal as well as prompts before merging.
Foundation for converging the recorded suite's cross-surface drift, consumed by the public_api and library layers above: - rails/helpers.py: shared build_rails() construction helper + async_chunks() (replaces the LLMRails(load_config(...)) boilerplate inlined per test, D11/F). - assertions.py: assert_blocked_generation() asserts refusal + rail stop semantics, not just non-empty text (D6).
Replay under --block-network must not depend on ambient proxy env: a SOCKS proxy makes httpx raise ImportError (missing socksio) on a cassette hit, turning a deterministic replay into a shell-dependent error. Add an autouse fixture that strips proxy vars during replay (record_mode == none) while leaving them intact for recording. Also fix the README 'Adding a test' snippet to include the imports it relies on (LLMRails, load_config, suite-local snapshot, OPENAI_BASELINE_CONFIG) so a new contributor can copy-paste it and land on the intended snapshot re-export.
e06a4fe to
e522751
Compare
14ddbc7 to
82429ff
Compare
|
Staged Fern docs preview: https://nvidia-preview-pr-1975.docs.buildwithfern.com/nemo/guardrails |
Adds the library-traversal sibling of test_load_prompts_sorts_files_for_deterministic_overrides, addressing the stack-2 review ask. Mocks os.walk to yield two library .co files defining the same bot message in non-sorted order and asserts the alphabetically-first file wins the collision, pinning the dirs.sort()/sorted(files) fix in LLMRails.__init__ so library load order stays filesystem-independent.
Summary
Sorts library traversal during LLMRails initialization so recorded rail tests do not depend on filesystem walk order.
Why
Recorded tests expose that library bot message insertion order can vary across platforms when files are loaded in filesystem order.
What Changed
Review Notes
This is the only runtime change in the stack.
Stack Position
Part 2 of 5.
Stack Context
This stack decomposes recorded end-to-end replay coverage into reviewable slices. The PRs should be reviewed against their parent branch in the stack.
Please review each PR against its parent branch, not directly against the root base branch, except for part 1.
stack/recorded-tests-01-harnessdevelopstack/recorded-tests-02-deterministic-library-loadstack/recorded-tests-01-harnessstack/recorded-tests-03-clientsstack/recorded-tests-02-deterministic-library-loadstack/recorded-tests-04-public-apistack/recorded-tests-03-clientsstack/recorded-tests-05-library-railsstack/recorded-tests-04-public-apiValidation