fix: coerce UUID session_id to string in ChatOutputResponse#11958
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe changes introduce UUID-to-string coercion for session_id fields across the backend and lfx codebases. A shared utility function Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. ❌ Your project status has failed because the head coverage (42.37%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #11958 +/- ##
==========================================
+ Coverage 37.31% 37.36% +0.04%
==========================================
Files 1592 1592
Lines 78280 78304 +24
Branches 11824 11826 +2
==========================================
+ Hits 29212 29256 +44
+ Misses 47448 47426 -22
- Partials 1620 1622 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
the helper expects str | UUID | None, but the call site's value for session is not necessarily of those types
🧹 Nitpick comments (1)
src/lfx/src/lfx/graph/vertex/vertex_types.py (1)
170-173: The isinstance check is redundant.The
coerce_to_str_if_uuidfunction already handles all input types safely (UUID → string, str → passthrough, None → passthrough, other → passthrough). The conditional adds complexity without changing behavior.Consider simplifying to match
base.py(line 449):♻️ Simplify by removing redundant type check
- raw_session_id = message_dict.get("session_id") - session_id = ( - coerce_to_str_if_uuid(raw_session_id) if isinstance(raw_session_id, (UUID, str)) else raw_session_id - ) + session_id = coerce_to_str_if_uuid(message_dict.get("session_id"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lfx/src/lfx/graph/vertex/vertex_types.py` around lines 170 - 173, The isinstance guard around coerce_to_str_if_uuid is redundant; change the session_id assignment to call coerce_to_str_if_uuid(raw_session_id) unconditionally (replace the conditional expression that references raw_session_id, session_id and coerce_to_str_if_uuid with a direct call) so behavior matches base.py usage and simplifies the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lfx/src/lfx/graph/vertex/vertex_types.py`:
- Around line 170-173: The isinstance guard around coerce_to_str_if_uuid is
redundant; change the session_id assignment to call
coerce_to_str_if_uuid(raw_session_id) unconditionally (replace the conditional
expression that references raw_session_id, session_id and coerce_to_str_if_uuid
with a direct call) so behavior matches base.py usage and simplifies the code.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/backend/base/langflow/schema/validators.pysrc/backend/base/langflow/utils/schemas.pysrc/backend/tests/unit/utils/test_schemas.pysrc/lfx/src/lfx/graph/vertex/base.pysrc/lfx/src/lfx/graph/vertex/vertex_types.pysrc/lfx/src/lfx/schema/validators.pysrc/lfx/src/lfx/utils/schemas.pysrc/lfx/tests/unit/graph/vertex/test_vertex_base.pysrc/lfx/tests/unit/utils/test_schemas.py
|
Went for a middle ground approach, by validating session id to be str | uuid | None inside the ‘before’ validator with a more descriptive error message instead of silently coercing to str and then waiting for the ‘after’ check to fail with a generic message |
* fix: coerce UUID session_id to string in ChatOutputResponse and lfx callsite * simplify patching in tests * validate and raise early * valueerror instead of typerror
Explicitly coerces
session_idinChatOutputResponseschema to be a string if a UUID is provided to prevent UUID inputs (common for session ids) from causing validation errors. Added tests covering model-level coercion.Reproduction (before fix):
lfxenvironment, run:lfx run agent.json "hi", where agent.json is the simple agent starter flow.{"success": false, "type": "error", "exception_type": "ValidationError", "exception_message": "1 validation error for ChatOutputResponse\nsession_id\n Input should be a valid string [type=string_type, input_value=UUID('...'), input_type=UUID]\n For further information visit https://errors.pydantic.dev/2.12/v/string_type"}Result (after fix):
ChatOutputResponse.session_idUUID validation.This failure occurs in LFX because the graph's session id is not pre-populated as it is in Langflow. See Line 255 in
src/lfx/src/lfx/base/agents/agent.py.Summary by CodeRabbit
New Features
Tests