[Fix] Metadata in agents is unset before setting the field - #630
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change centralizes managed component metadata keys and resets them to Merge Risk: 🟡 Moderate · up to The PR clears metadata before assigning agent-specific values, but existing custom values can still overwrite the reset and current metadata, allowing stale TTS/STT information to remain visible. This correctness gap should be fixed or explicitly accepted before merge. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
agents-core/vision_agents/core/agents/agents.py (1)
859-863: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPrevent custom fields from overriding managed metadata.
**self.agent_user.customis applied last. If it contains a key from_COMPONENT_METADATA_KEYS, it overwrites the reset and the currentself.components_metadata. A reused agent user can still publish stale component metadata.Merge existing custom fields first, then apply the reset and current metadata. Set
is_agentlast. Add a regression test with a pre-populated managed key.This is required by the PR objective: clear stale component metadata before applying current metadata.
Proposed fix
self.agent_user.custom = { + **self.agent_user.custom, **dict.fromkeys(_COMPONENT_METADATA_KEYS), **self.components_metadata, **{"is_agent": True}, - **self.agent_user.custom, }
🧹 Nitpick comments (2)
plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py (2)
362-363: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd explicit annotations for the new locals.
Declare
customasdict[str, object]andunsetaslist[str].Proposed annotation
- custom = {key: value for key, value in user.custom.items() if value is not None} - unset = [key for key, value in user.custom.items() if value is None] + custom: dict[str, object] = { + key: value for key, value in user.custom.items() if value is not None + } + unset: list[str] = [ + key for key, value in user.custom.items() if value is None + ]As per coding guidelines, “Use type annotations everywhere.”
Source: Coding guidelines
358-379: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression tests for both
Nonecustom-field paths.Test removal of an existing custom field while preserving unrelated fields. Also test 404 fallback creation to confirm that null-valued fields are omitted.
The supplied test at
plugins/getstream/tests/test_stream_edge_transport.py:126-148covers only preservation of a non-null field. It would not detect regressions in either new path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7254e1e0-6411-466d-ac5c-ceb5229ecd4b
📒 Files selected for processing (2)
agents-core/vision_agents/core/agents/agents.pyplugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/test_agents/test_agents.py (1)
453-453: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a return annotation to the new test.
Declare
test_authenticate_clears_stale_managed_customwith-> None.
As per coding guidelines,**/*.pyfiles must use type annotations everywhere.Proposed change
- async def test_authenticate_clears_stale_managed_custom(self): + async def test_authenticate_clears_stale_managed_custom(self) -> None:Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0638bdf9-bc1b-4fa4-afed-7b59fc328f89
📒 Files selected for processing (4)
agents-core/vision_agents/core/agents/agents.pyplugins/getstream/tests/test_stream_edge_transport.pyplugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.pytests/test_agents/test_agents.py
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/getstream/vision_agents/plugins/getstream/stream_edge_transport.py
- agents-core/vision_agents/core/agents/agents.py
Why
realtimemodel the TTS and STT fields were partially setChanges