Skip to content

Conversation

tgasser-nv
Copy link
Collaborator

@tgasser-nv tgasser-nv commented Sep 15, 2025

Description

Cleaned the logging/ directory with help from Cursor/Claude 4 Sonnet to get the low-hanging items.

Type Error Fixes Summary Report

This report analyzes the type error fixes implemented in commit 033dc0fab7a8b1b05e0d7f01ece49a73c8a2ae83 for the logging directory. The fixes have been categorized by risk level based on the potential for functionality disruption.

High Risk Fixes

1. ContextVar Type Annotation Fixes (context.py)

Files: nemoguardrails/context.py (lines 26-28, 31-33, 39-41)
Original Errors:

  • Argument of type "LLMCallInfo" cannot be assigned to parameter "value" of type "None" in function "set"
  • Argument of type "LLMStats" cannot be assigned to parameter "value" of type "None" in function "set"

Fixed Lines:

explain_info_var: contextvars.ContextVar[Optional["ExplainInfo"]] = contextvars.ContextVar("explain_info", default=None)
llm_call_info_var: contextvars.ContextVar[Optional["LLMCallInfo"]] = contextvars.ContextVar("llm_call_info", default=None)
llm_stats_var: contextvars.ContextVar[Optional["LLMStats"]] = contextvars.ContextVar("llm_stats", default=None)

How it was fixed: Added explicit type annotations to ContextVar declarations with proper generic typing using Optional[T] and forward references with quotes. Added TYPE_CHECKING imports to avoid circular dependencies.

Assumptions: The context variables can legitimately hold None values as defaults, which matches the runtime behavior.

Alternative fixes: Could have used Union[T, None] instead of Optional[T], but Optional is more idiomatic. Could have avoided forward references by restructuring imports, but that would be more invasive.

2. Callback Handler Inheritance Fix (callbacks.py)

File: nemoguardrails/logging/callbacks.py (line 40)
Original Error: Multiple method override incompatibilities with base classes

Fixed Lines:

class LoggingCallbackHandler(AsyncCallbackHandler):

How it was fixed: Removed StdOutCallbackHandler from the inheritance chain, keeping only AsyncCallbackHandler. This eliminates conflicts between synchronous and asynchronous callback method signatures.

Assumptions: The functionality from StdOutCallbackHandler wasn't essential for the logging behavior, or the async methods provide sufficient functionality.

Alternative fixes: Could have created a proper multiple inheritance hierarchy with method resolution, but removing the conflicting inheritance is cleaner and less error-prone.

Medium Risk Fixes

3. Exception Type Parameter Fixes (callbacks.py)

File: nemoguardrails/logging/callbacks.py (lines 273, 304, 335)
Original Errors: Parameter type mismatch for error handlers - base expects BaseException, override used Union[Exception, KeyboardInterrupt]

Fixed Lines:

async def on_llm_error(self, error: BaseException, *, ...):
async def on_chain_error(self, error: BaseException, *, ...):
async def on_tool_error(self, error: BaseException, *, ...):

How it was fixed: Changed parameter type from Union[Exception, KeyboardInterrupt] to BaseException to match the base class signature.

Assumptions: The error handling logic can work with any BaseException subclass, not just Exception and KeyboardInterrupt.

Alternative fixes: Could have used method overloading or runtime type checking, but matching the base class signature is the correct approach.

4. List Type Casting for Callback Managers (callbacks.py)

File: nemoguardrails/logging/callbacks.py (lines 376-377, 382-383)
Original Errors: list[LoggingCallbackHandler] not assignable to list[BaseCallbackHandler] due to invariance

Fixed Lines:

handlers=cast(List[BaseCallbackHandler], handlers),
inheritable_handlers=cast(List[BaseCallbackHandler], handlers),

How it was fixed: Used typing.cast() to explicitly cast the list type from List[LoggingCallbackHandler] to List[BaseCallbackHandler].

Assumptions: LoggingCallbackHandler is indeed a subclass of BaseCallbackHandler, making the cast safe. The lists won't be modified in ways that would violate type safety.

Alternative fixes: Could have declared handlers with the base type initially, or used Sequence which is covariant, but casting is the most minimal change.

Low Risk Fixes

5. Null-Safe Arithmetic Operations (callbacks.py, processing_log.py)

Files: nemoguardrails/logging/callbacks.py (lines 191-200), nemoguardrails/logging/processing_log.py (multiple locations)
Original Errors: Arithmetic operations with potentially None values

Fixed Lines:

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
    # ... use took
else:
    log.warning("LLM call timing information incomplete")
    llm_call_info.duration = 0.0

How it was fixed: Added null checks before arithmetic operations and provided fallback values when timing information is incomplete.

Assumptions: Setting duration to 0.0 when timing is incomplete is acceptable behavior. Logging a warning is sufficient notification.

Alternative fixes: Could have raised exceptions, used default values, or computed approximate durations, but graceful degradation with logging is most robust.

6. Null-Safe Member Access (processing_log.py)

File: nemoguardrails/logging/processing_log.py (lines 156, 163-167, 171-175, 184, 191-201)
Original Errors: Optional member access on potentially None objects

Fixed Lines:

if activated_rail is not None:
    activated_rail.executed_actions.append(executed_action)

if executed_action is not None:
    executed_action.finished_at = event["timestamp"]
    # ... other operations

How it was fixed: Added null checks before accessing members of potentially None objects.

Assumptions: It's valid for activated_rail and executed_action to be None in certain execution paths, and skipping operations is the correct behavior.

Alternative fixes: Could have ensured objects are never None through architectural changes, but defensive programming is safer.

7. Null-Safe Accumulation Operations (processing_log.py)

File: nemoguardrails/logging/processing_log.py (lines 282-299)
Original Errors: Operator "+=" not supported for types "int | None" and "Literal[1]"

Fixed Lines:

generation_log.stats.llm_calls_count = (generation_log.stats.llm_calls_count or 0) + 1
generation_log.stats.llm_calls_duration = (generation_log.stats.llm_calls_duration or 0) + (llm_call.duration or 0)

How it was fixed: Used explicit null-coalescing pattern (value or 0) before arithmetic operations to ensure non-null operands.

Assumptions: Zero is an appropriate default value for all statistical counters when they haven't been initialized.

Alternative fixes: Could have pre-initialized all stats fields to avoid null checks, but this approach is more defensive and handles partial initialization.

8. Safe Attribute Access with getattr (verbose.py)

File: nemoguardrails/logging/verbose.py (lines 57, 72-73)
Original Errors: Cannot access attribute "id"/"task" for class "LogRecord" - attributes not known to exist

Fixed Lines:

record_id = getattr(record, "id", "unknown")
record_task = getattr(record, "task", "unknown")
console.print(f"[cyan]LLM {title} ({record_id[:5] if record_id != 'unknown' else record_id}..)[/]")

How it was fixed: Replaced direct attribute access with getattr() calls providing default values, and added conditional logic for string slicing.

Assumptions: "unknown" is an acceptable default value for missing record attributes. The logging system may or may not add these custom attributes to LogRecord instances.

Alternative fixes: Could have subclassed LogRecord to ensure attributes exist, or used try/except blocks, but getattr with defaults is cleaner and more pythonic.

Summary

The fixes demonstrate a comprehensive approach to type safety while maintaining backward compatibility. The high-risk changes address fundamental type system issues that could affect core functionality, while medium and low-risk changes focus on defensive programming and graceful error handling. All fixes follow Python best practices and maintain the original functionality while satisfying the type checker requirements.


Test Plan

Type-checking

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

Unit-tests

$   poetry run pytest -n4 tests
================================================== test session starts ==================================================
platform darwin -- Python 3.13.2, pytest-8.3.4, pluggy-1.5.0
rootdir: /Users/tgasser/projects/nemo_guardrails
configfile: pytest.ini
plugins: cov-6.0.0, xdist-3.8.0, httpx-0.35.0, asyncio-0.25.3, anyio-4.8.0, profiling-1.8.1, langsmith-0.3.6
asyncio: mode=Mode.STRICT, asyncio_default_fixture_loop_scope=function
4 workers [1440 items]
s.ss.ss..s..........ss............................................................................s.............. [  7%]
.................................................................................ss.............................. [ 15%]
.........................................sss...........ss........................................................ [ 23%]
.......ss........s................ssssssss.ssssssssss.........sssss.............................................. [ 31%]
.s.............s................ssss..............ssssssss..............sss...............ss...ss................ [ 39%]
.s............................................................................................................... [ 47%]
................................s....................................sssssssss................................... [ 54%]
..........................ss..................................................................................... [ 62%]
..................ssssssss.............................sssssssssssss............................................. [ 70%]
....s............................................................................................................ [ 78%]
................................................................................................................. [ 86%]
................................................................................................................. [ 94%]
........s....................................................s......................                              [100%]
========================================== 1340 passed, 100 skipped in 54.46s ===========================================

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. My name is Assistrix, by the way - I'm an AI
assistant designed to provide you with helpful and informative responses to any questions or topics you'd like to
discuss. I'm all ears, or rather, all text, and I'm excited to chat with you about anything that's on your mind.

Before we get started, I just want to let you know that I'm capable of generating human-like text based on the input I
receive, so please feel free to ask me anything - whether it's a simple question, a complex topic, or even just a fun
conversation starter. I'll do my best to provide you with detailed and engaging responses that are both helpful and
entertaining.

So, what's been going on in your world lately? Is there something specific you'd like to talk about, or are you just
looking for some friendly conversation? I'm here to listen and help in any way I can!

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

Related Issue(s)

Top-level PR to merge into before develop-branch merge: #1367

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant