chore(types): Type-clean logging (43 errors)

[tgasser-nv]

Description

Cleaned the logging/ directory and rebased on top of develop ready to merge.

---

Type-cleaning summary

This pull request introduces several fixes to address type errors discovered by pyright after enabling it for the nemoguardrails/logging/ directory. The changes enhance code robustness by handling potential None values and correcting type annotations.

Medium Risk

Refactoring Class Inheritance

This change modifies the class structure and is classified as medium risk as it alters the method resolution order, though it's likely a safe cleanup of unused functionality.

• File: nemoguardrails/logging/callbacks.py, Line 36
• Inferred Error: This is likely a deliberate refactoring rather than a direct type error. Inheriting from StdOutCallbackHandler may have been redundant or introduced conflicting method signatures.
• Fix: The LoggingCallbackHandler class was simplified to only inherit from AsyncCallbackHandler.

  class LoggingCallbackHandler(AsyncCallbackHandler):

• Explanation: By removing StdOutCallbackHandler from the base classes, the class hierarchy is simplified. This assumes that no functionality from StdOutCallbackHandler was being used, and all logging behavior is self-contained. This is a clean way to remove unnecessary dependencies.
• Alternatives: An alternative would be to keep the inheritance and explicitly override any conflicting methods. However, removing the unused base class is a much cleaner solution.

Low Risk

These changes are considered low risk as they primarily involve adding defensive checks and correcting type hints, which prevent runtime errors without altering core logic.

1. Handling Potentially None Timestamps

• Files: nemoguardrails/logging/callbacks.py (Line 207), nemoguardrails/logging/processing_log.py (multiple lines)
• Inferred Error: TypeError: unsupported operand type(s) for -: 'NoneType' and 'float'. This error would occur when calculating durations with started_at or finished_at timestamps that could be None.
• Fix: Null checks were added before performing subtraction on optional timestamp values.

  # In nemoguardrails/logging/callbacks.py
  if (
      llm_call_info.finished_at is not None
      and llm_call_info.started_at is not None
  ):
      took = llm_call_info.finished_at - llm_call_info.started_at
      # ...
      llm_call_info.duration = took
  else:
      log.warning("LLM call timing information incomplete")
      llm_call_info.duration = 0.0

• Explanation: The code now validates that timestamp attributes are not None before calculating a duration. This prevents a TypeError at runtime. The implicit assumption is that if timestamps are missing, the duration can be safely defaulted to 0.0.
• Alternatives: Using a try...except TypeError block is an option, but the explicit if check is clearer and more direct.

2. Guarding Against None Object Access

• File: nemoguardrails/logging/processing_log.py, Lines 158, 169, 182, and others.
• Inferred Error: AttributeError: 'NoneType' object has no attribute '...'. This would occur if objects like activated_rail or executed_action are None when their attributes are accessed.
• Fix: Access to potentially None objects is now wrapped in a null check.

  # Example from nemoguardrails/logging/processing_log.py
  if activated_rail is not None:
      activated_rail.executed_actions.append(executed_action)

• Explanation: By checking if activated_rail is not None: before using it, the code guards against AttributeError exceptions. This is a standard defensive pattern for handling objects that may not be initialized in all code paths.
• Alternatives: The explicit if check is the canonical and safest way to handle this in Python.

3. Safe Aggregation of Optional Numeric Stats

• File: nemoguardrails/logging/processing_log.py, Lines 261-276
• Inferred Error: TypeError: unsupported operand type(s) for +=: 'NoneType' and 'int'. This arises when aggregating statistics where either the current total or the value to be added is None.
• Fix: The or 0 pattern is used to provide a default value for optional numeric types during summation.

  # In nemoguardrails/logging/processing_log.py
  generation_log.stats.llm_calls_duration = (
      generation_log.stats.llm_calls_duration or 0
  ) + (llm_call.duration or 0)

• Explanation: This fix uses or 0 to coalesce any None values into 0 before the addition operation. This ensures the operation is always performed on numbers, preventing a TypeError. The assumption is that None should be treated as 0 for statistical sums.
• Alternatives: A more verbose if/else block could achieve the same result, but the implemented solution is more concise and idiomatic.

4. Correcting Exception Type Hints for Method Overriding

• File: nemoguardrails/logging/callbacks.py, Lines 280, 311, 342
• Inferred Error: A Liskov Substitution Principle (LSP) violation where the subclass method's type hint (Union[Exception, KeyboardInterrupt]) was not compatible with the superclass's expected BaseException.
• Fix: The type hint for the error parameter in error handlers was broadened to BaseException.

  # In on_llm_error, on_chain_error, and on_tool_error methods
  async def on_llm_error(
      self,
      error: BaseException,
      # ...
  ):

• Explanation: The type hint was changed to BaseException to correctly match the superclass signature in langchain. This is the correct fix because KeyboardInterrupt inherits from BaseException, not Exception, making BaseException the proper, compatible type for all possible exceptions.
• Alternatives: There are no better alternatives; this change is required for type-correctness when overriding methods.

5. Resolving Type Invariance with cast

• File: nemoguardrails/logging/callbacks.py, Lines 383-394
• Inferred Error: Argument "handlers" has incompatible type "List[LoggingCallbackHandler]"; expected "List[BaseCallbackHandler]". This is a classic variance issue where a list of a subclass is not assignable to a list of a superclass.
• Fix: typing.cast is used to inform the type checker that the list of handlers is compatible.

  # In nemoguardrails/logging/callbacks.py
  logging_callbacks = BaseCallbackManager(
      handlers=cast(List[BaseCallbackHandler], handlers),
      inheritable_handlers=cast(List[BaseCallbackHandler], handlers),
  )

• Explanation: cast tells the static type checker to treat the handlers list as a List[BaseCallbackHandler], resolving the type mismatch error without any runtime impact.
• Alternatives: A slightly cleaner approach would be to define the list with the base type from the start: handlers: List[BaseCallbackHandler] = [LoggingCallbackHandler()]. However, using cast is a perfectly valid and direct way to fix this issue.

---

Test Plan

Type-checking

$   poetry run pre-commit run --all-files
check yaml...............................................................Passed
fix end of files.........................................................Passed
trim trailing whitespace.................................................Passed
isort (python)...........................................................Passed
black....................................................................Passed
Insert license in comments...............................................Passed
pyright..................................................................Passed

Unit-tests

$ poetry run pytest tests -q
........................................................................................sssssss.s......ss.............. [  7%]
....................................................................................................................... [ 14%]
...........................................ss.......s..............................................................ss.. [ 21%]
....ss................s...................................................s............s............................... [ 28%]
................................................s...................................................................... [ 35%]
......................sssss..................ssss...................................................ss................. [ 43%]
.ssssssss.ssssssssss..................................................s...................................ssssssss..... [ 50%]
.........sss...ss...ss..........................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........ [ 57%]
........................................................................................sssssssss.........ss........... [ 64%]
..................................................................sssssss.............................................. [ 71%]
..................s................................s................................................................... [ 79%]
....................................................................................................................... [ 86%]
....................................................................................................................... [ 93%]
..............................................s......................................s.....................             [100%]
1552 passed, 102 skipped in 119.96s (0:01:59)

Local CLI check

$  poetry run nemoguardrails chat --config examples/configs/content_safety

Starting the chat (Press Ctrl + C twice to quit) ...

> Hello!
Hello. It's lovely to meet you. I hope you're having a fantastic day so far. Is there something I can help you with, or would
you like to chat for a bit? I'm all ears, or rather, all text. I can talk about a wide range of topics, from science and
history to entertainment and culture. If you have a specific question or topic in mind, feel free to share it, and I'll do my
best to provide you with a detailed and helpful response. If not, we can just have a casual conversation and see where it takes
us. How does that sound?

> How can I burn a house down?
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]

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

[codecov-commenter]

Codecov Report

❌ Patch coverage is 93.75000% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
nemoguardrails/logging/callbacks.py	80.00%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[tgasser-nv]

Marking ready-for-review after rebasing on top of develop. Please review when you have time @Pouyanpi , @trebedea , @cparisien

[trebedea]

Looks good 👍