fix: remove uninitialized details_placeholder reference in app_stream…#232
fix: remove uninitialized details_placeholder reference in app_stream…#232madhununna wants to merge 5 commits intostrands-agents:mainfrom
Conversation
…ing.py In app_streaming.py, details_placeholder was checked in session state but never initialized, causing AttributeError on newer Streamlit versions. Unlike app.py which properly initializes it, app_streaming.py has no use for it. Removed the dead code block. Fixes strands-agents#140
|
Latest scan for commit: ✅ Security Scan Report (PR Files Only)Scanned Files
Security Scan Results
Threshold: High No security issues detected in your changes. Great job! This scan only covers files changed in this PR. |
JiwaniZakir
left a comment
There was a problem hiding this comment.
Removing the details_placeholder.empty() call in app_streaming.py fixes the uninitialized reference crash, but it also silently drops the cleanup behavior that was there intentionally. If details_placeholder is still written to during streaming (e.g., in the tool-use callback), stale tool details from a previous turn will now persist in the UI when a new prompt is submitted, since nothing clears them. A safer fix would be to initialize details_placeholder to a known state at app startup (e.g., st.session_state.setdefault("details_placeholder", st.empty())) and keep the .empty() call, rather than removing the cleanup entirely. It's worth checking whether the placeholder is still populated downstream in the streaming handler — if it is, users will see tool details from message N bleeding into message N+1.
Initialize details_placeholder in session state at app startup to prevent uninitialized reference errors while preserving the cleanup behavior that prevents stale tool details from persisting across prompts. This addresses feedback on PR-232 by using the safer approach of initializing the placeholder rather than removing the cleanup call.
Add back the cleanup call that clears details_placeholder between prompts. The previous commit only added initialization but missed restoring the cleanup behavior that prevents stale tool details from persisting. Now the fix properly: - Initializes details_placeholder at startup (prevents crash) - Clears it before each new prompt (prevents stale content)
|
Thanks for the detailed feedback! You're absolutely right about the safety concern. I've updated the PR with the safer approach you suggested. The fix now:
This addresses the crash while preserving the intentional cleanup behavior that prevents tool details from |
…ing.py
Issue #, if available:140
Description of changes:In app_streaming.py, details_placeholder was checked in session state but never initialized, causing AttributeError on newer Streamlit versions. Unlike app.py which properly initializes it, app_streaming.py has no use for it. Removed the dead code block.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.