chore(types): Type-clean library/ (142 errors) #1389
Conversation
tgasser-nv
changed the base branch from
develop
to
chore/type-clean-guardrails
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## chore/type-clean-guardrails #1389 +/- ##
===============================================================
- Coverage 71.62% 71.24% -0.38%
===============================================================
Files 171 171
Lines 17020 17161 +141
===============================================================
+ Hits 12191 12227 +36
- Misses 4829 4934 +105
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
tgasser-nv
changed the base branch from
chore/type-clean-guardrails
to
develop
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
Converting to draft while I rebase on the latest changes to develop. I'll create PRs for each sub-directory in |
tgasser-nv
force-pushed
the
chore/type-clean-library
branch
from
e1d2e32 to
1125d27
Compare
Greptile Summary
Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Comments (4)
-
nemoguardrails/library/jailbreak_detection/request.py, line 16-29 (link)style: Duplicate SPDX license header blocks should be removed
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!
-
nemoguardrails/library/jailbreak_detection/heuristics/checks.py, line 82-84 (link)logic: Division by zero risk if get_perplexity returns False (which is 0-like). When dependencies are missing, perplexity will be False, making score = len(input_string) / False cause a TypeError.
-
nemoguardrails/library/jailbreak_detection/heuristics/checks.py, line 104-105 (link)logic: Missing null checks for dependencies. If torch/transformers are unavailable, get_perplexity will return False, but this code doesn't handle that case before using the values in comparisons.
-
nemoguardrails/library/factchecking/align_score/server.py, line 36 (link)logic: NLTK download call will fail during type checking since nltk import is conditional
24 files reviewed, 8 comments
| if models_path is None: | ||
| raise ValueError("ALIGN_SCORE_PATH environment variable not set") |
There was a problem hiding this comment.
logic: Redundantnull check - models_path is already validated at lines40-43and will raise ValueError if None
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/library/factchecking/align_score/server.py
Line: 57:58
Comment:
**logic:** Redundantnull check - models_path is already validated at lines40-43and will raise ValueError if None
How can I resolve this? If you propose a fix, please make it concise.| if TYPE_CHECKING: | ||
| import nltk # type: ignore[import-not-found] | ||
| import typer # type: ignore[import-not-found] | ||
| import uvicorn # type: ignore[import-not-found] | ||
| from alignscore import AlignScore # type: ignore[import-not-found] | ||
| from fastapi import FastAPI # type: ignore[import-not-found] | ||
| else: | ||
| import nltk | ||
| import typer | ||
| import uvicorn | ||
| from alignscore import AlignScore | ||
| from fastapi import FastAPI |
There was a problem hiding this comment.
logic: TYPE_CHECKING pattern may cause runtime issues - these appear to be required dependencies, not optional ones. Are nltk, typer, uvicorn, alignscore, and fastapi actually optional dependencies that might not be installed?
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/library/factchecking/align_score/server.py
Line: 20:31
Comment:
**logic:** TYPE_CHECKING pattern may cause runtime issues - these appear to be required dependencies, not optional ones. Are nltk, typer, uvicorn, alignscore, and fastapi actually optional dependencies that might not be installed?
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| text = text or context.get("bot_message", "") | ||
| if not text: | ||
| validate_text = text if text else context.get("user_message", "") if context else "" |
There was a problem hiding this comment.
logic: Should extract 'bot_message' for output validation, not 'user_message'
| validate_text = text if text else context.get("user_message", "") if context else "" | |
| validate_text = text if text else context.get("bot_message", "") if context else "" |
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/library/guardrails_ai/actions.py
Line: 159:159
Comment:
**logic:** Should extract 'bot_message' for output validation, not 'user_message'
```suggestion
validate_text = text if text else context.get("bot_message", "") if context else ""
```
How can I resolve this? If you propose a fix, please make it concise.| llm_call_info_var.set(LLMCallInfo(task=task.value)) | ||
|
|
||
| # Use a low temperature for deterministic fact checking | ||
| temperature = config.lowest_temperature if config else RailsConfig.model_fields["lowest_temperature"].default |
There was a problem hiding this comment.
style: Accessing model_fields directly is fragile - if the field structure changes, this will break at runtime.
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!
Prompt To Fix With AI
This is a comment left during a code review.
Path: nemoguardrails/library/self_check/facts/actions.py
Line: 79:79
Comment:
**style:** Accessing `model_fields` directly is fragile - if the field structure changes, this will break at runtime.
<sub>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!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Description
I type-cleaned all the library directories which were authored by Nvidia employees in this PR. The 3rd-party integrations are listed in the
excludesection ofpyproject.toml, while everything undernemoguardrails/libraryis included by default. This means new integrations will have to be type-clean, while existing ones don't need any changes.The 3rd-party library integrations excluded are:
##Type cleaning summary
Summary
This PR addresses type errors in the
nemoguardrails/librarymodule by adding proper type annotations, null checks, and handling optional dependencies gracefully.Changes
Type Safety Improvements:
context,config, and rail-specific configuration objects across multiple action modulesOptional[dict]todictwhere context is always required (e.g.,gcp_moderate_text,llama_guard)task: str,jailbreak_result: Optional[bool])RailsConfigtype instead ofdictfor config parameters where appropriateOptional Dependency Handling:
# type: ignore[import-not-found]for optional packages:fast_langdetect,yara,aiofiles,guardrails,presidio_*,torch,transformers,numpyNoneassignmentsTYPE_CHECKINGblocks for type-only imports ininjection_detection/actions.pyandfactchecking/align_score/server.pytorch,transformers,numpy) inside class__init__methods for lazy loadingBug Fixes & Robustness:
aiohttptimeout parameter to useClientTimeout(total=30)instead of raw integeralignscore_check_factsYaraEnumMetamethods to usestr(member)instead ofmember.valuerender_task_promptreturning eitherstrorlistintopic_safetydetect_languagereturnsstr | Nonecorrectly by checkingisinstance(lang, str)Pyright Configuration:
nemoguardrails/library/**to pyright include pathsattention,autoalign,clavata,cleanlab,fiddler,patronusai,trend_microFiles Modified
content_safety/actions.pyfactchecking/align_score/actions.pyfactchecking/align_score/server.pygcp_moderate_text/actions.pyguardrails_ai/actions.py,errors.py,registry.pyhallucination/actions.pyinjection_detection/actions.py,yara_config.pyjailbreak_detection/actions.py,request.py,server.pyjailbreak_detection/heuristics/checks.pyjailbreak_detection/model_based/models.py,checks.pyllama_guard/actions.pyself_check/facts/actions.py,input_check/actions.py,output_check/actions.pysensitive_data_detection/actions.pytopic_safety/actions.pytracing/adapters/filesystem.pypyproject.tomlTest Plan
Pre-commit Type-clean summary
Unit-tests
Chat integration test