Skip to content

Conversation

@tgasser-nv
Copy link
Collaborator

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

Description

Summary

Type-cleaning nemoguardrails/colang directory.

Test Plan

Pre-commit (includes type-checking)

$ poetry run pre-commit run --all-files
check yaml...............................................................Passed
fix end of files.........................................................Passed
trim trailing whitespace.................................................Passed
ruff (legacy alias)......................................................Passed
ruff format..............................................................Passed
Insert license in comments...............................................Passed
pyright..................................................................Passed

Unit-tests

$ poetry run pytest -q
........................................................................................... [  3%]
........................................................................................... [  7%]
...................................ssss.................................................... [ 11%]
.....................................s..................................................... [ 15%]
................................................................sssssss............s.s..... [ 19%]
.ss........................................................................................ [ 23%]
........................................................................................... [ 27%]
..s.......s................................................................................ [ 31%]
.............................ss........ss...ss................................ss........... [ 35%]
.....s...................................................s............s.................... [ 39%]
........................................................................................... [ 43%]
........................................................................................... [ 47%]
.....................................sssss......ssssssssssssssssss.........sssss........... [ 51%]
......................................................................s...........ss....... [ 55%]
...................ssssssss.ssssssssss..................................................... [ 59%]
............................s....s.....................................ssssssss............ [ 63%]
..sss...ss...ss.....ssssssssssssss............................................/Users/tgasser/Library/Caches/pypoetry/virtualenvs/nemoguardrails-Btg1Laya-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
............. [ 66%]
...s....................................................................................... [ 70%]
.......................sssssssss.........ss................................................ [ 74%]
...........................................................sssssss......................... [ 78%]
.......................................................................s................... [ 82%]
.....................................................................................ss.... [ 86%]
........................................................................................... [ 90%]
........................................................................................... [ 94%]
........................................................................................... [ 98%]
............s.....................                                                          [100%]
2177 passed, 132 skipped in 127.14s (0:02:07)

Interactive chat (Colang 1)

$ poetry run nemoguardrails chat --config examples/configs/nemoguards
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 Assistron,
by the way - I'm an AI designed to provide helpful and informative responses to your questions and
engage in conversation. 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 ask me,
and I'll do my best to provide a detailed and interesting response. If I don't know the answer to
something, I'll be sure to let you know - I'm all about transparency and honesty. So, what's on
your mind today? Would you like to chat about something in particular, or just see where the
conversation takes us?

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

Interactive chat (Colang 2)

$  poetry run nemoguardrails chat --config exampl
es/configs/nemoguards_v2
Starting the chat (Press Ctrl + C twice to quit) ...

> Hello!

Hello again! It's nice to see you're back. Is there something I can help you with or would you like
to continue our previous conversation about NVIDIA?


> 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 tgasser-nv self-assigned this Sep 9, 2025
@tgasser-nv
Copy link
Collaborator Author

Tagging @cparisien, @Pouyanpi , @trebedea for review

@tgasser-nv tgasser-nv changed the title chore(types): Type-clean colang/ chore(types): Type-clean colang/ (111 errors) Sep 10, 2025
@tgasser-nv tgasser-nv changed the base branch from chore/type-clean-guardrails to develop September 22, 2025 16:28
@github-actions
Copy link
Contributor

Documentation preview

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

@tgasser-nv tgasser-nv changed the base branch from develop to chore/type-clean-guardrails September 22, 2025 16:35
@tgasser-nv tgasser-nv changed the base branch from chore/type-clean-guardrails to develop September 22, 2025 16:35
@tgasser-nv tgasser-nv changed the base branch from develop to chore/type-clean-guardrails September 22, 2025 18:14
@tgasser-nv tgasser-nv changed the base branch from chore/type-clean-guardrails to develop September 22, 2025 18:15
@tgasser-nv tgasser-nv changed the base branch from develop to chore/type-clean-guardrails September 22, 2025 18:15
@tgasser-nv tgasser-nv changed the base branch from chore/type-clean-guardrails to develop September 22, 2025 18:16
@tgasser-nv tgasser-nv changed the base branch from develop to chore/type-clean-guardrails September 22, 2025 18:19
@tgasser-nv tgasser-nv changed the base branch from chore/type-clean-guardrails to develop September 22, 2025 18:19
@tgasser-nv tgasser-nv force-pushed the chore/type-clean-colang branch from f5e281f to 489b2a3 Compare September 22, 2025 18:24
@tgasser-nv tgasser-nv changed the base branch from develop to chore/type-clean-guardrails September 22, 2025 18:45
@tgasser-nv tgasser-nv changed the base branch from chore/type-clean-guardrails to develop September 22, 2025 18:46
@tgasser-nv tgasser-nv force-pushed the chore/type-clean-colang branch from b4a4f85 to f507ea0 Compare September 26, 2025 21:57
@tgasser-nv tgasser-nv marked this pull request as draft October 13, 2025 13:55
@tgasser-nv
Copy link
Collaborator Author

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

@tgasser-nv tgasser-nv force-pushed the chore/type-clean-colang branch from f507ea0 to 76caade Compare October 13, 2025 16:20
@tgasser-nv tgasser-nv marked this pull request as ready for review October 13, 2025 16:53
@tgasser-nv
Copy link
Collaborator Author

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

@Pouyanpi
Copy link
Collaborator

@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?

@tgasser-nv tgasser-nv force-pushed the chore/type-clean-colang branch from 647cc32 to dbeba8b Compare October 24, 2025 16:06
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

This PR systematically eliminates 111 Pyright type-checking errors across the nemoguardrails/colang/ directory by adding defensive null-checks, type guards, and explicit type annotations. The changes prevent potential runtime TypeError, AttributeError, and KeyError exceptions by validating that variables are not None before dereferencing, providing default values for optional parameters, and wrapping non-dict action results into dictionaries where expected. The modifications span both Colang v1.0 and v2.x parsers, runtimes, and AST processors, following a defensive programming pattern that silently skips operations when values are missing rather than failing fast. This aligns with the broader Guardrails architecture where robustness in parsing and flow execution is prioritized over strict validation.


Important Files Changed

Filename Score Overview
pyproject.toml 5/5 Adds nemoguardrails/colang/** to Pyright include list to enable type-checking for the module
nemoguardrails/logging/processing_log.py 5/5 Adds type annotation to processing_log_var context variable for improved static analysis
nemoguardrails/actions/llm/generation.py 5/5 Removes explicit -> None return type hint to fix type-checker complaints about implicit returns
nemoguardrails/colang/v2_x/lang/colang_ast.py 5/5 Adds __hash__ method to Element class to satisfy Python's hashable protocol
nemoguardrails/colang/v2_x/runtime/serialization.py 4/5 Adds type guard after deserialization to ensure State instance before accessing attributes
nemoguardrails/colang/v1_0/runtime/sliding.py 5/5 Wraps imports in TYPE_CHECKING block to resolve circular dependencies
nemoguardrails/colang/v2_x/runtime/flows.py 5/5 Uses TYPE_CHECKING guard for RailsConfig import to prevent circular dependency
nemoguardrails/colang/v1_0/lang/comd_parser.py 5/5 Adds null-check for sym variable before processing symbol mappings
nemoguardrails/colang/runtime.py 4/5 Adds null-check for config.imported_paths and uses getattr() for conditional method registration
nemoguardrails/colang/v2_x/lang/parser.py 4/5 Corrects return type annotation and adds null-check for import_el.package
nemoguardrails/colang/v2_x/lang/transformer.py 5/5 Renames variable to avoid shadowing and broadens return type to Any
nemoguardrails/colang/v2_x/runtime/eval.py 5/5 Fixes lambda closure bugs and adds type-cast for SimplifyFormatter().format()
nemoguardrails/colang/v1_0/lang/colang_parser.py 3/5 Adds extensive null-checks and type annotations throughout parser logic
nemoguardrails/colang/v1_0/lang/utils.py 3/5 Adds defensive null-checks for string variables and introduces unused multiline_indentation
nemoguardrails/colang/v1_0/runtime/flows.py 3.5/5 Adds null-checks for slide() return values but has return-type inconsistency in _slide_with_subflows()
nemoguardrails/colang/v1_0/runtime/runtime.py 4/5 Adds guards for None values and wraps non-dict action results silently
nemoguardrails/colang/v2_x/lang/expansion.py 4/5 Adds compound type-safety guards for element._source.line and spec element validation
nemoguardrails/colang/v2_x/lang/utils.py 5/5 Adds critical type guard to prevent TypeError when calling asdict() on dataclass classes
nemoguardrails/colang/v2_x/runtime/statemachine.py 4/5 Comprehensive null-checks across state machine runtime with repeated defensive patterns
nemoguardrails/colang/v1_0/lang/coyml_parser.py 4/5 Adds defensive checks for dictionary keys and converts jump offsets to strings
nemoguardrails/colang/v2_x/runtime/runtime.py 4/5 Adds conditional checks for langchain attributes and filters Event objects from history

Confidence score: 3.5/5

  • This PR improves robustness but introduces several silent failure modes where exceptions or missing values are ignored rather than raising errors, which could mask bugs
  • Score reflects that while the type-safety improvements reduce immediate crash risk, several changes have medium risk: _slide_with_subflows() always returns None despite declaring Optional[int] return type, unused multiline_indentation variable in utils.py, duplicate RailsConfig import in flows.py, silent wrapping of action results without logging, and repeated defensive patterns that could benefit from refactoring
  • Pay close attention to nemoguardrails/colang/v1_0/runtime/flows.py (return type mismatch), nemoguardrails/colang/v1_0/lang/utils.py (dead code), and nemoguardrails/colang/v1_0/runtime/runtime.py (silent error suppression)

Additional Comments (1)

  1. nemoguardrails/colang/v1_0/lang/coyml_parser.py, line 437-450 (link)

    style: Converting _next_else from int to str and then back to int (line 450) is unnecessary. Keep it as int throughout or document why string representation is required by the runtime.

21 files reviewed, 19 comments

Edit Code Review Agent Settings | Greptile

Comment on lines 228 to 229
if not isinstance(state, State):
raise ValueError("Decoded object is not a State instance")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Check that decode_from_dict returns a dict with expected structure before casting to State, since decode_from_dict can return primitives or other types. Should we verify the decoded data has the expected structure (e.g., flow_states attribute) before the isinstance check?

Comment on lines 1452 to 1453
stripped_tokens = get_stripped_tokens(split_max(flow_name, "=", 1))
return_vars, flow_name = stripped_tokens[0], stripped_tokens[1]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: When split_max returns only 1 token, stripped_tokens[1] will cause IndexError

current_comment = None
multiline_string = False
current_string = None
multiline_indentation = 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: multiline_indentation is initialized but never written to after this point. The value set on line 120 is never used because it's only set inside the multiline_string block that never reads this variable.

Comment on lines +443 to +425
# Ensure element is valid for SpecOp
if isinstance(element, (dict, Spec)):
spec_element: Union[dict, Spec] = element
else:
spec_element = {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Wrapping with an empty dict {} as fallback could hide programming errors. If element is not a valid dict or Spec, the error might surface later in unexpected ways rather than failing fast here. Consider logging a warning or using a more defensive default. Should invalid elements be logged or raise an error instead of silently defaulting to {}?

greptile-apps[bot]

This comment was marked as outdated.

@tgasser-nv tgasser-nv requested a review from Pouyanpi October 24, 2025 16:57
greptile-apps[bot]

This comment was marked as outdated.

@tgasser-nv tgasser-nv force-pushed the chore/type-clean-colang branch from f43e73e to 84484f5 Compare December 12, 2025 10:58
@tgasser-nv tgasser-nv marked this pull request as draft December 12, 2025 13:17
@tgasser-nv
Copy link
Collaborator Author

@greptileai

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 15, 2025

Greptile Overview

Greptile Summary

This PR comprehensively type-cleans the nemoguardrails/colang directory, addressing 111 static type checking errors by adding proper type annotations, implementing defensive programming patterns, and resolving circular import issues. The changes span both Colang v1.0 and v2.x components including parsers, runtimes, AST transformers, and utilities. Key improvements include adding TYPE_CHECKING import guards to prevent circular dependencies, implementing null safety checks throughout parsing and runtime operations, adding explicit type casts and return type annotations, and enhancing error handling with proper exception validation. The changes also enable the colang directory in the Pyright configuration in pyproject.toml, making it part of the static analysis pipeline. All modifications preserve backward compatibility and existing functionality while significantly improving code robustness and maintainability.

Important Files Changed

Filename Score Overview
pyproject.toml 5/5 Adds colang directory to Pyright type-checking configuration
nemoguardrails/colang/runtime.py 5/5 Adds null safety checks and improves attribute access patterns
nemoguardrails/actions/llm/generation.py 5/5 Removes return type annotation to resolve type checker conflicts
nemoguardrails/colang/v2_x/lang/colang_ast.py 5/5 Implements proper __hash__ method for Element class consistency
nemoguardrails/colang/v2_x/runtime/flows.py 5/5 Adds TYPE_CHECKING imports to resolve circular import issues
nemoguardrails/colang/v1_0/runtime/sliding.py 5/5 Adds TYPE_CHECKING guards and proper type annotations
nemoguardrails/colang/v1_0/lang/comd_parser.py 4/5 Adds critical symbol validation to prevent parsing errors
nemoguardrails/logging/processing_log.py 5/5 Adds explicit type annotations to ContextVar
nemoguardrails/colang/v2_x/lang/utils.py 5/5 Adds type guard to prevent dataclass serialization errors
nemoguardrails/colang/v2_x/runtime/serialization.py 4/5 Adds runtime validation for State object deserialization
nemoguardrails/colang/v1_0/runtime/flows.py 4/5 Comprehensive type improvements with null safety and error handling
nemoguardrails/colang/v1_0/runtime/runtime.py 5/5 Adds defensive programming patterns for parallel rail execution
nemoguardrails/colang/v2_x/lang/parser.py 4/5 Fixes return type annotations and adds null safety checks
nemoguardrails/colang/v1_0/lang/coyml_parser.py 4/5 Adds type safety for YAML parsing with defensive checks
nemoguardrails/colang/v2_x/lang/transformer.py 5/5 Fixes variable naming conflicts and updates return type annotations
nemoguardrails/colang/v2_x/lang/expansion.py 5/5 Enhances error handling and type validation for AST processing
nemoguardrails/colang/v2_x/runtime/eval.py 5/5 Simplifies lambda functions and adds explicit type casts
nemoguardrails/colang/v2_x/runtime/statemachine.py 4/5 Comprehensive type safety improvements with null checks
nemoguardrails/colang/v2_x/runtime/runtime.py 4/5 Major type safety improvements for the runtime system
nemoguardrails/colang/v1_0/lang/colang_parser.py 4/5 Extensive defensive programming patterns for parsing operations
nemoguardrails/colang/v1_0/lang/utils.py 4/5 Type annotations and null safety for utility functions

Confidence score: 4/5

  • This PR implements comprehensive type safety improvements across the entire colang directory with minimal risk of functional breakage
  • Score reflects the complexity of the changes across multiple critical runtime components, though all changes preserve backward compatibility and add defensive programming patterns
  • Pay close attention to runtime files (nemoguardrails/colang/v*/runtime/*.py) and parser files (nemoguardrails/colang/v*/lang/*parser.py) for proper testing

Sequence Diagram

sequenceDiagram
    participant User
    participant Parser as ColangParser
    participant Transformer as ColangTransformer  
    participant AST as ColangAST
    participant Runtime as RuntimeV1_0/V2_x
    participant Actions as ActionDispatcher
    participant LLM as LLMTaskManager

    User->>Parser: parse_colang_file(filename, content, version)
    
    alt Colang 1.0
        Parser->>Parser: ColangParser(filename, content)
        Parser->>Parser: parse() 
        Parser->>Parser: _normalize_line_text()
        Parser->>Parser: _process_define()
        Parser->>Parser: _extract_markdown()
        Parser->>AST: create flow configs
        AST-->>Parser: flow definitions
    else Colang 2.0  
        Parser->>Parser: _apply_pre_parsing_expansions(content)
        Parser->>Parser: get_parsing_tree(content)
        Parser->>Transformer: ColangTransformer(source)
        Transformer->>Transformer: transform(tree)
        Transformer->>AST: create Flow/SpecOp/Assignment elements
        AST-->>Transformer: typed AST elements
        Transformer-->>Parser: parsed flows
    end

    Parser-->>User: {"flows": [...], "import_paths": [...]}
    
    User->>Runtime: RuntimeV1_0/V2_x(config)
    Runtime->>Runtime: _init_flow_configs()
    Runtime->>Runtime: register actions/parameters
    
    alt V2.x Runtime
        Runtime->>AST: expand_elements(flow.elements, flow_configs) 
        AST->>AST: _expand_send_element()
        AST->>AST: _expand_match_element()
        AST->>AST: _expand_await_element()
        AST-->>Runtime: expanded elements
    end

    User->>Runtime: generate_events(events) / process_events(events, state)
    
    alt V1.0 Flow Processing
        Runtime->>Runtime: compute_next_steps(events, flow_configs)
        Runtime->>Runtime: slide(state, flow_config, head)
        Runtime->>Runtime: eval_expression(expr, context)
    else V2.x Flow Processing  
        Runtime->>Runtime: run_to_completion(state, event)
        Runtime->>Runtime: _advance_head_front(state, heads)
        Runtime->>Runtime: eval_expression(expr, context)
    end

    Runtime->>Actions: execute_action(action_name, kwargs)
    Actions->>LLM: llm_call(prompt, params)
    LLM-->>Actions: response
    Actions-->>Runtime: ActionResult
    
    Runtime-->>User: output events and updated state
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

21 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@tgasser-nv tgasser-nv marked this pull request as ready for review December 15, 2025 22:42
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

21 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

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.

4 participants