From f7a1750d04db49d250cabbe14f2dee447904f313 Mon Sep 17 00:00:00 2001 From: Cao Hanzhe Date: Thu, 7 May 2026 22:05:56 +0800 Subject: [PATCH 1/3] fix: rename shadowed 'format' builtin in _render_graphviz In burr/core/graph.py, the function _render_graphviz() used 'format' as a local variable name, which shadows Python's built-in format() function. Renamed it to 'fmt' to avoid the shadowing while preserving all existing behavior. The variable is used only within the function scope and the rename is consistent and self-documenting. --- burr/core/graph.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/burr/core/graph.py b/burr/core/graph.py index 871135962..b31e1cab6 100644 --- a/burr/core/graph.py +++ b/burr/core/graph.py @@ -95,20 +95,20 @@ def _render_graphviz( if output_file_path.suffix != "": # infer format from path; i.e., extract `svg` from the `.svg` suffix - format = output_file_path.suffix.partition(".")[-1] + fmt = output_file_path.suffix.partition(".")[-1] else: - format = "png" + fmt = "png" path_without_suffix = pathlib.Path(output_file_path.parent, output_file_path.stem) if write_dot or view: # `.render()` appends the `format` kwarg to the filename # i.e., we need to pass `/my/filepath` to generate `/my/filepath.png` # otherwise, passing `/my/filepath.png` will generate `/my/filepath.png.png` - graphviz_obj.render(path_without_suffix, format=format, view=view) + graphviz_obj.render(path_without_suffix, format=fmt, view=view) else: # `.pipe()` doesn't append the format to the filename, so we do it explicitly - pathlib.Path(f"{path_without_suffix}.{format}").write_bytes( - graphviz_obj.pipe(format=format) + pathlib.Path(f"{path_without_suffix}.{fmt}").write_bytes( + graphviz_obj.pipe(format=fmt) ) From 0e883bbb091250b441539a2413f6c186eed51c1a Mon Sep 17 00:00:00 2001 From: Cao Hanzhe Date: Thu, 7 May 2026 22:07:02 +0800 Subject: [PATCH 2/3] fix: correct validate_inputs error message when both missing and additional inputs exist In burr/core/action.py, the validate_inputs() method had a bug in its error message construction due to operator precedence in a nested ternary expression. The original code: raise ValueError( f"Inputs to function {self} are invalid. " + f"Missing the following inputs: ..." if missing_inputs else (...) ) Due to Python operator precedence, the ternary 'if' binds lower than '+', so the whole expression is evaluated as: (str1 + str2) if missing_inputs else (str3 if additional_inputs else "") This means when BOTH missing_inputs and additional_inputs are non-empty, the error message only reports the missing inputs and silently drops info about the additional inputs. Similarly, when only additional_inputs exist, the 'Inputs to function ... are invalid' prefix is omitted. Fixed by building error message parts independently: - Always include the 'Inputs to function ... are invalid' prefix - Conditionally append missing inputs info - Conditionally append additional inputs info - Join all parts with spaces This ensures all relevant validation errors are reported regardless of which combination of issues exist. --- burr/core/action.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/burr/core/action.py b/burr/core/action.py index eb6fceff5..5a228e0ad 100644 --- a/burr/core/action.py +++ b/burr/core/action.py @@ -243,16 +243,12 @@ def validate_inputs(self, inputs: Optional[Dict[str, Any]]) -> None: missing_inputs = required_inputs - given_inputs additional_inputs = given_inputs - required_inputs - optional_inputs if missing_inputs or additional_inputs: - raise ValueError( - f"Inputs to function {self} are invalid. " - + f"Missing the following inputs: {', '.join(missing_inputs)}." - if missing_inputs - else ( - "" f"Additional inputs: {','.join(additional_inputs)}." - if additional_inputs - else "" - ) - ) + parts = [f"Inputs to function {self} are invalid."] + if missing_inputs: + parts.append(f"Missing the following inputs: {', '.join(missing_inputs)}.") + if additional_inputs: + parts.append(f"Additional inputs: {', '.join(additional_inputs)}.") + raise ValueError(" ".join(parts)) def is_async(self) -> bool: """Convenience method to check if the function is async or not. From 7b2049209cbc81ec2ba764e5b2aae9f21e0bd7f9 Mon Sep 17 00:00:00 2001 From: Cao Hanzhe Date: Thu, 7 May 2026 22:04:59 +0800 Subject: [PATCH 3/3] fix: use module logger instead of root logger in b_aiosqlite.py All other persister modules in burr/integrations/persisters/ use logging.getLogger(__name__) to create a properly scoped logger. b_aiosqlite.py was using logging.getLogger() (no arguments), which returns the root logger. Using the root logger is bad practice because: - It causes log messages to bypass the module's namespace, making it harder to filter or configure logging for this specific module. - It can lead to unexpected log output appearing in the root logger's handlers even when the module's logging is intended to be suppressed. This one-line fix changes it to logging.getLogger(__name__) for consistency with all other persister implementations. --- burr/integrations/persisters/b_aiosqlite.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/burr/integrations/persisters/b_aiosqlite.py b/burr/integrations/persisters/b_aiosqlite.py index a75eb682c..8439b965b 100644 --- a/burr/integrations/persisters/b_aiosqlite.py +++ b/burr/integrations/persisters/b_aiosqlite.py @@ -26,7 +26,7 @@ from burr.core import State from burr.core.persistence import AsyncBaseStatePersister, PersistedStateData -logger = logging.getLogger() +logger = logging.getLogger(__name__) try: from typing import Self