fix: Message ID and Output inconsistencies #11061
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 WalkthroughA refactoring to standardize message ID access patterns across the codebase. New public methods ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touchesImportant Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 3 warnings)
✅ Passed checks (3 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/lfx/src/lfx/base/agents/agent.py(1 hunks)src/lfx/src/lfx/base/agents/altk_base_agent.py(1 hunks)src/lfx/src/lfx/base/agents/events.py(1 hunks)src/lfx/src/lfx/components/datastax/astradb_assistant_manager.py(1 hunks)src/lfx/src/lfx/custom/custom_component/component.py(6 hunks)src/lfx/src/lfx/schema/message.py(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-24T19:47:28.997Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-24T19:47:28.997Z
Learning: Applies to src/backend/tests/**/*.py : Test Langflow's `Message` objects using `langflow.schema.message.Message` and validate text, sender, sender_name, session_id, files, and properties attributes
Applied to files:
src/lfx/src/lfx/schema/message.py
🧬 Code graph analysis (6)
src/lfx/src/lfx/base/agents/altk_base_agent.py (2)
src/lfx/src/lfx/custom/custom_component/component.py (1)
get_id(206-207)src/lfx/src/lfx/schema/message.py (1)
get_id(335-345)
src/lfx/src/lfx/base/agents/agent.py (2)
src/lfx/src/lfx/custom/custom_component/component.py (1)
get_id(206-207)src/lfx/src/lfx/schema/message.py (1)
get_id(335-345)
src/lfx/src/lfx/custom/custom_component/component.py (1)
src/lfx/src/lfx/schema/message.py (3)
has_id(347-358)Message(34-377)get_id(335-345)
src/lfx/src/lfx/schema/message.py (1)
src/lfx/src/lfx/custom/custom_component/component.py (1)
get_id(206-207)
src/lfx/src/lfx/base/agents/events.py (2)
src/lfx/src/lfx/custom/custom_component/component.py (1)
get_id(206-207)src/lfx/src/lfx/schema/message.py (1)
get_id(335-345)
src/lfx/src/lfx/components/datastax/astradb_assistant_manager.py (2)
src/lfx/src/lfx/custom/custom_component/component.py (1)
get_id(206-207)src/lfx/src/lfx/schema/message.py (1)
get_id(335-345)
🪛 GitHub Actions: Ruff Style Check
src/lfx/src/lfx/custom/custom_component/component.py
[error] 1569-1569: W293 Blank line contains whitespace. Ruff check failed during 'uv run --only-dev ruff check --output-format=github .'.
🪛 GitHub Check: Ruff Style Check (3.13)
src/lfx/src/lfx/custom/custom_component/component.py
[failure] 1582-1582: Ruff (W293)
src/lfx/src/lfx/custom/custom_component/component.py:1582:1: W293 Blank line contains whitespace
[failure] 1579-1579: Ruff (W293)
src/lfx/src/lfx/custom/custom_component/component.py:1579:1: W293 Blank line contains whitespace
[failure] 1574-1574: Ruff (W293)
src/lfx/src/lfx/custom/custom_component/component.py:1574:1: W293 Blank line contains whitespace
[failure] 1569-1569: Ruff (W293)
src/lfx/src/lfx/custom/custom_component/component.py:1569:1: W293 Blank line contains whitespace
src/lfx/src/lfx/schema/message.py
[failure] 47-47: Ruff (W293)
src/lfx/src/lfx/schema/message.py:47:1: W293 Blank line contains whitespace
[failure] 42-42: Ruff (W293)
src/lfx/src/lfx/schema/message.py:42:1: W293 Blank line contains whitespace
[failure] 36-36: Ruff (W293)
src/lfx/src/lfx/schema/message.py:36:1: W293 Blank line contains whitespace
[failure] 346-346: Ruff (W293)
src/lfx/src/lfx/schema/message.py:346:1: W293 Blank line contains whitespace
[failure] 340-340: Ruff (W293)
src/lfx/src/lfx/schema/message.py:340:1: W293 Blank line contains whitespace
[failure] 337-337: Ruff (W293)
src/lfx/src/lfx/schema/message.py:337:1: W293 Blank line contains whitespace
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run Ruff Check and Format
- GitHub Check: Update Component Index
- GitHub Check: Update Starter Projects
🔇 Additional comments (9)
src/lfx/src/lfx/schema/message.py (1)
335-378: Well-designed ID accessor methods with minor whitespace issues.The implementation provides a clean, safe API for ID access:
get_id()for optional accesshas_id()for existence checksrequire_id()for mandatory access with clear error messagesFix the trailing whitespace on blank lines (337, 340, 346) to resolve pipeline failures.
def get_id(self) -> str | UUID | None: """Safely get the message ID. - + Returns: The message ID if it exists, None otherwise. - + Note: A message only has an ID if it has been stored in the database. Messages that are skipped (via _should_skip_message) will not have an ID. """ return getattr(self, "id", None) - + def has_id(self) -> bool:src/lfx/src/lfx/base/agents/events.py (1)
391-391: LGTM!The change to use
agent_message.get_id()aligns with the standardized ID access pattern introduced in theMessageclass. This safely returnsNoneif the message hasn't been stored yet (e.g., when the Agent is not connected to a Chat Output).src/lfx/src/lfx/components/datastax/astradb_assistant_manager.py (1)
302-306: LGTM!The exception handling now safely:
- Retrieves the message ID via
get_id()instead of direct attribute access- Guards the
delete_messagecall to only execute when an ID existsThis prevents errors when handling exceptions for messages that were never stored in the database.
src/lfx/src/lfx/base/agents/agent.py (1)
275-282: LGTM with a minor observation.The exception handling now safely guards the
delete_messagecall. However, thehasattr(e, "agent_message")check on line 276 may be redundant sinceExceptionWithMessageError(defined inevents.py) always initializesagent_messagein its constructor.That said, the defensive check doesn't hurt and provides safety if exception handling were to change in the future.
src/lfx/src/lfx/base/agents/altk_base_agent.py (1)
381-388: LGTM!The exception handling mirrors the pattern in
agent.py, ensuring consistent ID-safe deletion behavior across all agent implementations.src/lfx/src/lfx/custom/custom_component/component.py (4)
1568-1591: Helpful documentation with whitespace issues.The expanded docstring clearly explains when messages are skipped and the implications for ID availability. Fix the trailing whitespace on blank lines (1569, 1574, 1579, 1582) to resolve pipeline failures.
def _should_skip_message(self, message: Message) -> bool: """Check if the message should be skipped based on vertex configuration and message type. - + When a message is skipped: - It is NOT stored in the database - It will NOT have an ID (message.get_id() will return None) - It is still returned to the caller, but no events are sent to the frontend - + Messages are skipped when: - The component is not an input or output vertex - The component is not connected to a Chat Output - The message is not an ErrorMessage - + This prevents intermediate components from cluttering the database with messages that aren't meant to be displayed in the chat UI. - + Returns: bool: True if the message should be skipped, False otherwise """
1660-1681: LGTM!The
send_messagechanges are well-designed:
Validation for
skip_db_update=True(lines 1662-1668): Correctly enforces that messages must already have an ID when skipping DB updates, since this path is for updating existing messages.Safe ID access (lines 1671, 1681): Uses
get_id()for assigning_stored_message_id, consistent with the new safe access patterns.The error message clearly explains the requirement, helping developers understand why their code might fail.
1747-1753: LGTM!Using
stored_message.has_id()is the correct pattern for the boolean check in the streaming eligibility condition.
1773-1798: LGTM!The ID validation in
_stream_messageis appropriate:
- Streaming requires a message ID to associate token events with the correct message
- The validation fails early with a clear error message
- The
message_idvariable is consistently passed to helper methods
| """Message schema for Langflow. | ||
| Message ID Semantics: | ||
| - Messages only have an ID after being stored in the database | ||
| - Messages that are skipped (via Component._should_skip_message) will NOT have an ID | ||
| - Always use get_id(), has_id(), or require_id() methods to safely access the ID | ||
| - Never access message.id directly without checking if it exists first | ||
| Safe ID Access Patterns: | ||
| - Use get_id() when ID may or may not exist (returns None if missing) | ||
| - Use has_id() to check if ID exists before operations that require it | ||
| - Use require_id() when ID is required (raises ValueError if missing) | ||
| Example: | ||
| message_id = message.get_id() # Safe: returns None if no ID | ||
| if message.has_id(): | ||
| # Safe to use message_id | ||
| do_something_with_id(message_id) | ||
| """ |
There was a problem hiding this comment.
Fix trailing whitespace in docstring.
The docstring is comprehensive and clearly documents the ID semantics. However, the static analysis tool flagged trailing whitespace on blank lines (lines 36, 42, 47) which is causing the pipeline to fail.
Apply this diff to fix the whitespace issues:
"""Message schema for Langflow.
-
+
Message ID Semantics:
- Messages only have an ID after being stored in the database
- Messages that are skipped (via Component._should_skip_message) will NOT have an ID
- Always use get_id(), has_id(), or require_id() methods to safely access the ID
- Never access message.id directly without checking if it exists first
-
+
Safe ID Access Patterns:
- Use get_id() when ID may or may not exist (returns None if missing)
- Use has_id() to check if ID exists before operations that require it
- Use require_id() when ID is required (raises ValueError if missing)
-
+
Example:
message_id = message.get_id() # Safe: returns None if no ID
if message.has_id():
# Safe to use message_id
do_something_with_id(message_id)
"""🧰 Tools
🪛 GitHub Check: Ruff Style Check (3.13)
[failure] 47-47: Ruff (W293)
src/lfx/src/lfx/schema/message.py:47:1: W293 Blank line contains whitespace
[failure] 42-42: Ruff (W293)
src/lfx/src/lfx/schema/message.py:42:1: W293 Blank line contains whitespace
[failure] 36-36: Ruff (W293)
src/lfx/src/lfx/schema/message.py:36:1: W293 Blank line contains whitespace
🤖 Prompt for AI Agents
In src/lfx/src/lfx/schema/message.py around lines 35 to 53, there are trailing
spaces on blank lines in the module docstring (notably lines 36, 42, 47) causing
static analysis failures; remove the trailing whitespace on those blank lines so
they are truly empty (no spaces or tabs), leaving the rest of the docstring
content unchanged.
github-actions
bot
added
bug
github-actions
bot
added
bug
github-actions
bot
added
bug
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
github-actions
bot
added
bug
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (26.47%) is below the target coverage (40.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #11061 +/- ##
==========================================
- Coverage 33.23% 33.22% -0.01%
==========================================
Files 1391 1391
Lines 65848 65870 +22
Branches 9745 9751 +6
==========================================
+ Hits 21884 21888 +4
- Misses 42842 42859 +17
- Partials 1122 1123 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
ogabrielluiz
left a comment
ogabrielluiz
left a comment
There was a problem hiding this comment.
This is a nice improvement.
LGTM
github-actions
bot
added
bug
Summary by CodeRabbit
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.