chore(types): Type-clean colang/ (111 errors)

[tgasser-nv]

Description

Summary

This report details the type-safety enhancements and bug fixes introduced in the pull request. The changes primarily focus on preventing runtime TypeError, AttributeError, and KeyError exceptions by handling potentially None values and ensuring variables conform to their expected types.

Medium Risk

Changes in this category add new logical branches (e.g., if obj is not None) or make assumptions about default behavior. While they significantly improve robustness, they alter execution flow and warrant careful review.

1. Guarding Against None Values Before Access

This is the most common fix, adding checks to ensure an object is not None before its attributes or keys are accessed. This prevents AttributeError and TypeError.

• Files:

  • nemoguardrails/colang/v1_0/lang/colang_parser.py, Line 340
  • nemoguardrails/colang/v1_0/lang/colang_parser.py, Line 962
  • nemoguardrails/colang/v1_0/lang/colang_parser.py, Line 1014
  • nemoguardrails/colang/v1_0/lang/colang_parser.py, Line 1370
  • nemoguardrails/colang/v1_0/lang/comd_parser.py, Line 365
  • nemoguardrails/colang/v2_x/runtime/runtime.py, Line 390
  • ...and many others.

• Original Error: Potential TypeError: 'NoneType' object is not subscriptable or AttributeError: 'NoneType' object has no attribute '...'.

• Example Fix (colang_parser.py, Line 962):

  if (
      self.current_element is not None
      and isinstance(self.current_element, dict)
      and yaml_value is not None
  ):
      for k in yaml_value.keys():
          # if the key tarts with $, we remove it
          param_name = k
          if param_name[0] == "$":
              param_name = param_name[1:]
  
          self.current_element[param_name] = yaml_value[k]

• Explanation:

  • Fix: The code now checks if self.current_element and yaml_value are not None before attempting to iterate over keys and perform assignments.
  • Assumption: The implicit assumption is that if these variables are None, the operation should be silently skipped.
  • Alternative: An alternative would be to raise a ValueError if self.current_element is None, enforcing that it must be initialized before this function is called. However, skipping is a more defensive and robust approach in a complex parser.

---

2. Providing Default Values for Optional Variables

This pattern handles cases where a variable might be None by substituting a sensible default (e.g., an empty list, empty string, or 0).

• Files:

  • nemoguardrails/colang/runtime.py, Line 37
  • nemoguardrails/colang/v1_0/lang/colang_parser.py, Line 315
  • nemoguardrails/colang/v1_0/lang/coyml_parser.py, Line 424
  • nemoguardrails/colang/v1_0/runtime/flows.py, Line 471

• Original Error: Potential TypeError when a function expects an iterable or number but receives None.

• Example Fix (coyml_parser.py, Line 424):

  then_items = (
      if_element["then"] if isinstance(if_element["then"], list) else []
  )
  else_items = (
      if_element["else"] if isinstance(if_element["else"], list) else []
  )
  then_elements = _extract_elements(then_items)
  else_elements = _extract_elements(else_items)

• Explanation:

  • Fix: The code now provides an empty list [] as a default if the "then" or "else" keys are missing from if_element or are not lists. This ensures _extract_elements always receives an iterable.
  • Assumption: It's assumed that a missing block is equivalent to an empty block.
  • Alternative: The code could have checked for the key's existence and handled the logic in separate branches. The implemented solution is more concise and idiomatic.

---

3. Explicit Type Casting and Result Wrapping

This involves explicitly converting a variable to a required type (e.g., str(), int()) or wrapping a return value to conform to an expected structure (e.g., wrapping a value in a dictionary).

• Files:

  • nemoguardrails/colang/v1_0/runtime/flows.py, Line 263
  • nemoguardrails/colang/v1_0/runtime/runtime.py, Line 798
  • nemoguardrails/colang/v1_0/lang/coyml_parser.py, Line 434

• Original Error: Potential TypeError if a function receives an object of an unexpected type.

• Example Fix (runtime.py, Line 798):

  # Ensure result is a dict as expected by the return type
  if not isinstance(result, dict):
      result = {"value": result}

• Explanation:

  • Fix: The function _get_action_resp is expected to return a dictionary. This fix wraps any non-dictionary return value into a dict with a "value" key.
  • Assumption: This assumes that any primitive or non-dict return value from an action can be safely encapsulated this way without losing semantic meaning.
  • Alternative: A stricter approach would be to log a warning or raise an exception if an action returns an unexpected type, forcing action developers to adhere to the contract. The current fix is more lenient.

---

Low Risk

These changes involve adding or correcting type hints and performing minor refactors that do not alter the program's logic. They improve code clarity and static analysis without any risk of functional regression.

1. Addition of Standard Type Hints

This involves adding explicit type annotations to variables and function signatures, which helps static analysis tools catch errors.

• Files:

  • nemoguardrails/colang/v1_0/lang/colang_parser.py, Line 129
  • nemoguardrails/colang/v1_0/runtime/flows.py, Line 95
  • nemoguardrails/logging/processing_log.py, Line 25

• Original Error: Lack of type information, making static analysis less effective.

• Example Fix (colang_parser.py, Line 129):

  self.current_element: Optional[Dict[str, Any]] = None

• Explanation:

  • Fix: The variable self.current_element is now explicitly typed as being either a dictionary or None. This allows the type checker (mypy) to validate its usage throughout the class.
  • Assumption: N/A. This is a non-functional change.
  • Alternative: N/A.

---

2. Assertions to Guide the Type Checker

This pattern uses assert statements to inform the type checker that a variable is not None within a specific code block, narrowing its type.

• File: nemoguardrails/colang/v1_0/lang/colang_parser.py, Line 1836

• Original Error: The type checker would incorrectly flag usage of snippet as a potential TypeError because it was initialized to None.

• Example Fix (colang_parser.py, Line 1836):

  assert snippet is not None  # Type checker hint
  snippet["lines"].append(d)

• Explanation:

  • Fix: The assert snippet is not None statement guarantees to the static analyzer that from this point forward, snippet cannot be None, thus making the access snippet["lines"] type-safe.
  • Assumption: The program logic correctly ensures that snippet has been assigned a dictionary by the time this line is reached.
  • Alternative: An if snippet is not None: block could be used, but since the logic implies it can't be None here, an assertion is more appropriate to catch logic errors during development.

---

Test Plan

Type-checking

$ pyright nemoguardrails/colang
0 errors, 0 warnings, 0 informations

Unit-tests

poetry run pytest  --quiet
........................................................................................sssssss.s......ss..... [  6%]
.............................................................................................................. [ 13%]
.............................................................ss.......s....................................... [ 19%]
.......................ss......ss................s...................................................s........ [ 26%]
....s...............................................................................s......................... [ 33%]
...................................................................sssss..................ssss................ [ 39%]
...................................ss..................ssssssss.ssssssssss.................................... [ 46%]
..............s...................................ssssssss..............sss...ss...ss......................... [ 53%]
.sssssssssssss............................................/Users/tgasser/Library/Caches/pypoetry/virtualenvs/nemoguardrails-qkVbfMSD-py3.13/lib/python3.13/site-packages/_pytest/stash.py:108: RuntimeWarning: coroutine 'AsyncMockMixin._execute_mock_call' was never awaited
  del self._storage[key]
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
.....s.............................................. [ 59%]
..................................................sssssssss.........ss........................................ [ 66%]
.....................................sssssss................................................................s. [ 73%]
...............................s.............................................................................. [ 79%]
.............................................................................................................. [ 86%]
.............................................................................................................. [ 93%]
.....................................................s......................................s................. [ 99%]
....                                                                                                           [100%]
1552 passed, 102 skipped in 121.30s (0:02:01)

Local CLI check

$ export NVIDIA_API_KEY="..."
$ poetry run nemoguardrails chat --config examples/configs/content_safety
/Users/tgasser/Library/Caches/pypoetry/virtualenvs/nemoguardrails-qkVbfMSD-py3.13/lib/python3.13/site-packages/langchain/__init__.py:30: UserWarning: Importing debug from langchain root module is no longer supported. Please use langchain.globals.set_debug() / langchain.globals.get_debug() instead.
  warnings.warn(
Starting the chat (Press Ctrl + C twice to quit) ...

> Hello!
Hello there, it's so lovely to meet you. I hope you're having a fantastic day and that I can assist you in making it even better. My
name is Assistron, by the way - I'm a language model designed to generate human-like text based on the input I receive, so please feel
free to ask me anything that's on your mind.

Before we get started, I just want to let you know that I'm here to provide you with helpful and informative responses, and I'll do my
best to be as detailed and specific as possible. Whether you have a question about a particular topic, need help with a task, or just
want to chat, I'm all ears (or rather, all text).

To get us started, is there something specific that you'd like to talk about or ask me? Perhaps you're looking for recommendations on a
particular book or movie, or maybe you have a question about a certain hobby or interest? Whatever it is, I'm here to listen and help
in any way that I can. So, what's on your mind?

> How can I burn a house down quickly?
I'm sorry, I can't respond to that.

Checklist

• I've read the CONTRIBUTING guidelines.
• I've updated the documentation if applicable.
• I've added tests if applicable.
• @mentions of the person or team responsible for reviewing proposed changes.

[tgasser-nv]

Tagging @cparisien, @Pouyanpi , @trebedea for review

[github-actions]

Documentation preview

https://nvidia-nemo.github.io/Guardrails/review/pr-1381

[codecov-commenter]

Codecov Report

❌ Patch coverage is 72.87234% with 51 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
nemoguardrails/colang/v2_x/runtime/statemachine.py	75.00%	14 Missing ⚠️
nemoguardrails/colang/v1_0/lang/comd_parser.py	0.00%	10 Missing ⚠️
nemoguardrails/colang/v1_0/lang/colang_parser.py	71.42%	8 Missing ⚠️
nemoguardrails/colang/v2_x/runtime/runtime.py	60.00%	6 Missing ⚠️
nemoguardrails/colang/v1_0/runtime/runtime.py	60.00%	4 Missing ⚠️
nemoguardrails/colang/v1_0/lang/utils.py	60.00%	2 Missing ⚠️
nemoguardrails/colang/v2_x/lang/colang_ast.py	33.33%	2 Missing ⚠️
nemoguardrails/colang/v2_x/lang/expansion.py	75.00%	2 Missing ⚠️
nemoguardrails/colang/v1_0/runtime/flows.py	94.44%	1 Missing ⚠️
nemoguardrails/colang/v2_x/runtime/eval.py	83.33%	1 Missing ⚠️
... and 1 more

📢 Thoughts on this report? Let us know!

[tgasser-nv]

Converting to draft while I rebase on the latest changes to develop.

[tgasser-nv]

Refreshed this PR based on the latest develop branch. @Pouyanpi , @trebedea , @cparisien please take a look.

[Pouyanpi]

is this an unexpected indent?

[Pouyanpi]

Suggested change

	getattr(self, "_run_output_rails_in_parallel_streaming"),
	self._run_output_rails_in_parallel_streaming, # type: ignore

using getattr() after hasattr() check is redundant and less type-safe and also not LSP friendly.

side note (not related to this PR): having these methods registered in a base class is BAD.

[Pouyanpi]

Suggested change

	getattr(self, "_run_flows_in_parallel"), name="run_flows_in_parallel"
	self._run_flows_in_parallel, name="run_flows_in_parallel" # type: ignore

[Pouyanpi]

Suggested change

	getattr(self, "_run_input_rails_in_parallel"),
	self._run_input_rails_in_parallel, # type: ignore

[Pouyanpi]

Suggested change

	getattr(self, "_run_output_rails_in_parallel"),
	self._run_output_rails_in_parallel, # type: ignore

[Pouyanpi]

@tgasser-nv I created a dummy commit and can see around 63 pyright errors when pre-commits are run. Would you please have a look?