fix: handle non-serializable objects in agent canvas SSE and state se…#14210
Open
RazmikGevorgyan wants to merge 1 commit intoinfiniflow:mainfrom
Open
fix: handle non-serializable objects in agent canvas SSE and state se…#14210RazmikGevorgyan wants to merge 1 commit intoinfiniflow:mainfrom
RazmikGevorgyan wants to merge 1 commit intoinfiniflow:mainfrom
Conversation
dosubot
Bot
added
the
size:S
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR makes JSON serialization more resilient by adding custom Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
agent/canvas.py (1)
138-138: Prefer deterministic fallback values over rawstr(obj)in persisted canvas snapshots.At Line 138,
default=strmay store unstable repr strings (e.g., memory-address-bearing object representations) into committed DSL state. Consider a controlled serializer (e.g.,Noneor typed placeholder) for unsupported runtime-only objects.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agent/canvas.py` at line 138, Replace the unstable default=str fallback used in the json.dumps call (json.dumps(dsl, ensure_ascii=False, default=str)) with a deterministic serializer function that returns controlled placeholders for unsupported runtime-only objects (for example None or a dict like {"__type__": type(obj).__name__} or {"__placeholder__": "<type>"}); implement a small helper (e.g., fallback_serializer(obj)) and pass it as default=fallback_serializer so persisted canvas snapshots never contain unpredictable reprs from objects in dsl.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@agent/canvas.py`:
- Around line 122-125: Add logging inside the deepcopy fallback branches so
failures are not silently swallowed: before assigning the original reference in
the except blocks, call the module/class logger (e.g., logger.exception or
logger.warning) to record the key k, a representation and type of self.dsl[k]
(e.g., repr(self.dsl[k]) and type(self.dsl[k]).__name__) and the caught
exception; apply the same change to both occurrences around the deepcopy of
self.dsl[k] (the try/except that assigns dsl[k]) so the component/key/type and
stacktrace are logged prior to falling back to the original reference.
In `@api/apps/canvas_app.py`:
- Line 238: The SSE payload currently uses json.dumps(..., default=str) which
coerces arbitrary objects to opaque strings and breaks client expectations;
replace that by ensuring the emitted ans is JSON-serializable without
default=str: implement a sanitizer/encoder (e.g., sanitize_for_json or
_sse_json_default) and use it when producing the SSE payload where yield "data:"
+ json.dumps(ans, ensure_ascii=False, ...) is called (the generator that yields
SSE lines in canvas_app.py); the sanitizer should recursively convert known
non-serializable types to safe representations (e.g., datetimes -> ISO strings,
UUIDs -> str, bytes -> base64) and raise or omit unsupported types so callers
can notice errors, then call json.dumps(ans_sanitized, ensure_ascii=False) (or
json.dumps(ans, default=_sse_json_default) if your default only handles specific
types and raises TypeError for others) instead of using default=str.
---
Nitpick comments:
In `@agent/canvas.py`:
- Line 138: Replace the unstable default=str fallback used in the json.dumps
call (json.dumps(dsl, ensure_ascii=False, default=str)) with a deterministic
serializer function that returns controlled placeholders for unsupported
runtime-only objects (for example None or a dict like {"__type__":
type(obj).__name__} or {"__placeholder__": "<type>"}); implement a small helper
(e.g., fallback_serializer(obj)) and pass it as default=fallback_serializer so
persisted canvas snapshots never contain unpredictable reprs from objects in
dsl.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 569bf9e2-debd-4366-bfb0-a50f06de42db
📒 Files selected for processing (3)
agent/canvas.pyagent/component/base.pyapi/apps/canvas_app.py
RazmikGevorgyan
force-pushed
the
fix/agent-canvas-serialization
branch
from
907efe0 to
8faee2d
Compare
dosubot
Bot
added
size:M
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@agent/canvas.py`:
- Around line 140-144: The current _serialize_default used in Graph.__str__
raises TypeError for non-callable non-JSON-serializable objects, which still
lets json.dumps crash; change _serialize_default (the function used as the
default= handler in the json.dumps(...) call) to return a safe string
representation (e.g., str(obj) or repr(obj)) instead of raising, while still
returning None for callables if you want them as null; this ensures json.dumps
will not raise and Graph.__str__ will always produce a serializable output.
In `@api/apps/canvas_app.py`:
- Around line 24-35: Update _canvas_json_default to log a warning before
returning None for callables: import or obtain a module logger (e.g.
logging.getLogger(__name__)) and call logger.warning with context including the
object's type and a short repr (e.g. f"Coercing callable to None in canvas SSE
payload: type=%s repr=%s", type(obj).__name__, repr(obj)) right before the
"return None" line in _canvas_json_default so that callable-to-None coercions
are recorded without changing behavior for non-callables.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 94bcec32-4ce8-4f14-90da-23ff8bd92d8e
📒 Files selected for processing (3)
agent/canvas.pyagent/component/base.pyapi/apps/canvas_app.py
🚧 Files skipped from review as they are similar to previous changes (1)
- agent/component/base.py
Comment on lines
+24
to
+35
| def _canvas_json_default(obj): | ||
| """Fallback serializer for canvas SSE events. | ||
|
|
||
| Agent components store functools.partial objects as deferred streaming | ||
| handles (see llm.py, agent_with_tools.py, message.py). These leak into | ||
| SSE event dicts via component input/output propagation and are not | ||
| JSON-serializable. This handler converts them to None so that downstream | ||
| consumers never receive opaque ``str(partial(...))`` representations. | ||
| """ | ||
| if callable(obj): | ||
| return None | ||
| raise TypeError(f"Object of type {type(obj).__name__} is not JSON serializable") |
Contributor
There was a problem hiding this comment.
Log callable-to-None coercions in _canvas_json_default.
This new fallback path currently drops data silently. Add a warning before coercion so production traceability is preserved when payload fields disappear.
🔧 Proposed fix
def _canvas_json_default(obj):
@@
if callable(obj):
+ logging.warning(
+ "canvas_app: SSE JSON fallback coerced callable type=%s to None",
+ type(obj).__name__,
+ )
return None
raise TypeError(f"Object of type {type(obj).__name__} is not JSON serializable")As per coding guidelines, **/*.py: Add logging for new flows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/apps/canvas_app.py` around lines 24 - 35, Update _canvas_json_default to
log a warning before returning None for callables: import or obtain a module
logger (e.g. logging.getLogger(__name__)) and call logger.warning with context
including the object's type and a short repr (e.g. f"Coercing callable to None
in canvas SSE payload: type=%s repr=%s", type(obj).__name__, repr(obj)) right
before the "return None" line in _canvas_json_default so that callable-to-None
coercions are recorded without changing behavior for non-callables.
…rialization Agent components (llm.py, agent_with_tools.py, message.py) store functools.partial objects as deferred streaming handles in their output slots. When the canvas state gets serialized for SSE events, Redis commits, or logging, these partials — plus non-copyable objects like Langfuse clients — crash json.dumps and deepcopy. Reproduced with Azure OpenAI (gpt-5.4-nano via LiteLLM) and Langfuse tracing enabled on agent workflows. Changes: - canvas_app.py: add _canvas_json_default() that converts callables to None (not str) to preserve client JSON contracts, re-raises for all other non-serializable types - canvas.py: wrap deepcopy calls in try/except with logging.warning so failures are diagnosable, add callable→None serializer for final json.dumps - base.py: add callable→None serializer to ComponentParamBase.__str__ Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
RazmikGevorgyan
force-pushed
the
fix/agent-canvas-serialization
branch
from
8faee2d to
734efd5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…rialization
Agent components (llm.py, agent_with_tools.py, message.py) store functools.partial objects as deferred streaming handles in their output slots. When the canvas state gets serialized for SSE events, Redis commits, or logging, these partials — plus non-copyable objects like Langfuse clients — crash json.dumps and deepcopy.
Changes:
Closes #14229
What problem does this PR solve?
Briefly describe what this PR aims to solve. Include background context that will help reviewers understand the purpose of the PR.
Type of change