Skip to content

UI Cleanup: Visible Keys and Objective Removal #25

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Marcus-Mok-GH wants to merge 2 commits into
base: main
Choose a base branch
from
Open

UI Cleanup: Visible Keys and Objective Removal #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4da1527
Clean up UI: show API keys, remove objective, and finalize logging re…
Marcus-Mok-GH May 12, 2026
ccb3aa4
📝 CodeRabbit Chat: Add generated unit tests
coderabbitai[bot] May 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 29 additions & 47 deletions app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ def initialize_session_state():
st.session_state.current_chat_id = None
if 'local_storage' not in st.session_state:
st.session_state.local_storage = LocalStorage()
if 'config_minimized' not in st.session_state:
st.session_state.config_minimized = False
if 'usage_data' not in st.session_state:
st.session_state.usage_data = {
'count': 0,
Expand All @@ -79,8 +77,8 @@ def delete_chat_screenshots(chat_id):
if msg.get('type') == 'image' and os.path.exists(msg.get('content')):
try:
os.remove(msg.get('content'))
except Exception as e:
print(f"Error deleting screenshot {msg.get('content')}: {e}")
except Exception:
pass

def save_chats_to_local():
"""Save all chats and usage data to localStorage"""
Expand All @@ -97,8 +95,8 @@ def save_chats_to_local():

# Save usage data
st.session_state.local_storage.setItem("mbu_usage", json.dumps(st.session_state.usage_data, default=str))
except Exception as e:
print(f"Error saving to localStorage: {e}")
except Exception:
pass
Comment on lines +98 to +99

@coderabbitai coderabbitai Bot May 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Silent localStorage failures could result in data loss.

Both save_chats_to_local() and load_chats_from_local() now suppress all exceptions. Users will not know if their chat history failed to persist due to quota limits, browser privacy settings, or other localStorage errors. Consider at minimum showing a warning indicator in the UI when persistence fails, even if detailed errors aren't logged.

Also applies to: 120-121

🧰 Tools
🪛 Ruff (0.15.12)

[error] 98-99: try-except-pass detected, consider logging the exception

(S110)

[warning] 98-98: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app.py` around lines 98 - 99, The current broad "except Exception: pass" in
save_chats_to_local and load_chats_from_local silently swallows storage errors
and can cause data loss; change these handlers to catch specific storage-related
exceptions (e.g., DOMException/StorageError or more specific exceptions raised
by your environment) instead of a bare Exception, log the error details to your
logger, and surface a UI-facing warning/state (e.g., set a persistenceError flag
or call showPersistenceWarning()) so the app can display a warning to the user;
ensure the functions return a success boolean or propagate the error state so
callers can react appropriately.


def load_chats_from_local():
"""Load all chats and usage data from localStorage"""
Expand All @@ -119,15 +117,13 @@ def load_chats_from_local():
if stored_chats:
st.session_state.chats = json.loads(stored_chats)
return True
except Exception as e:
print(f"Error loading from localStorage: {e}")
except Exception:
pass
return False

def setup_chat_menu():
"""Setup left sidebar for chat management"""
st.sidebar.title("💬 Chats")

if st.sidebar.button("➕ New Chat", use_container_width=True):
def setup_chat_menu(container):
"""Setup chat management menu in a container"""
if container.button("➕ New Chat", use_container_width=True):
new_id = str(uuid.uuid4())
st.session_state.chats[new_id] = {
'title': 'New Chat',
Expand All @@ -137,12 +133,13 @@ def setup_chat_menu():
st.session_state.current_chat_id = new_id
st.session_state.messages = []
save_chats_to_local()
st.rerun()

st.sidebar.divider()
container.divider()

# List existing chats
for cid, chat in list(st.session_state.chats.items()):
col_chat, col_del = st.sidebar.columns([0.8, 0.2])
col_chat, col_del = container.columns([0.8, 0.2])

# Highlight current chat
is_current = (cid == st.session_state.current_chat_id)
Expand All @@ -154,6 +151,7 @@ def setup_chat_menu():
st.session_state.current_chat_id = cid
st.session_state.messages = chat.get('messages', [])
st.session_state.todos = chat.get('todos', [])
st.rerun()

if col_del.button("🗑️", key=f"del_{cid}"):
delete_chat_screenshots(cid)
Expand All @@ -163,11 +161,10 @@ def setup_chat_menu():
st.session_state.messages = []
st.session_state.todos = []
save_chats_to_local()
st.rerun()

def setup_configuration_panel(container):
"""Setup right configuration panel with improved spacing and organization"""
# Title is now handled in the main layout for better control with the minimize button

"""Setup configuration panel with improved spacing and organization"""
cookie_manager = stx.CookieManager()

cookies = cookie_manager.get_all()
Expand Down Expand Up @@ -211,7 +208,6 @@ def setup_configuration_panel(container):
mistral_api_key = st.text_input(
"Mistral API Key",
value=st.session_state.mistral_api_key,
type="password",
help="Enter your Mistral AI API key",
key="mistral_input"
)
Comment on lines 208 to 213

@qodo-code-review qodo-code-review Bot May 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Api keys unmasked 🐞 Bug ⛨ Security

The configuration panel now renders stored Mistral/Firecrawl API keys as plain text by removing
password masking, making accidental disclosure likely (screen share, screenshots, shoulder-surfing).
Because the values are also loaded from and persisted to cookies, the raw keys will be displayed
whenever present.
Agent Prompt 
## Issue description
API keys are rendered in clear text because `st.text_input(..., type="password")` was removed.

## Issue Context
Keys are loaded from cookies into session state and then passed as `value=` to the inputs, so any existing key immediately appears on-screen.

## Fix Focus Areas
- app.py[175-259]

### Implementation sketch
- Re-introduce `type="password"` for both key inputs.
- If visibility is desired, add a `st.checkbox("Show API keys")` and conditionally set `type=None` only when explicitly enabled (default to masked).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Expand Down Expand Up @@ -245,7 +241,6 @@ def setup_configuration_panel(container):
firecrawl_api_key = st.text_input(
"Firecrawl API Key",
value=st.session_state.firecrawl_api_key,
type="password",
help="Enter your Firecrawl API key",
key="firecrawl_input"
)
Expand Down Expand Up @@ -345,11 +340,10 @@ def take_screenshot_and_analyze():

# Take screenshot
screenshot_path = st.session_state.browser.take_screenshot()
add_message("assistant", screenshot_path, "image", "Current page screenshot")

# Detect and highlight elements
annotated_image_path = st.session_state.element_detector.detect_and_annotate_elements(screenshot_path, st.session_state.browser)
add_message("assistant", annotated_image_path, "image", "Elements detected and indexed")
add_message("assistant", annotated_image_path, "image")

@chatgpt-codex-connector chatgpt-codex-connector Bot May 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve raw screenshot path for later deletion

take_screenshot_and_analyze now stores only the annotated image message, but detect_and_annotate_elements writes that file as a copy and leaves the original screenshot on disk. Because delete_chat_screenshots deletes only image paths present in chat messages, every successful automation step leaves an untracked raw screenshot in screenshots/, causing disk growth and stale sensitive captures over time; either persist screenshot_path for cleanup or delete it immediately after annotation succeeds.

Useful? React with 👍 / 👎.


return annotated_image_path

Expand Down Expand Up @@ -474,7 +468,7 @@ def execute_automation_step(user_objective):
return True

except Exception as e:
error_msg = f"Automation step failed: {str(e)}\n{traceback.format_exc()}"
error_msg = f"Automation step failed: {str(e)}"
add_message("assistant", error_msg, "error")
st.session_state.automation_active = False
return False
Expand All @@ -500,14 +494,19 @@ def main():
st.session_state.messages = current_chat.get('messages', [])
st.session_state.todos = current_chat.get('todos', [])

setup_chat_menu()
# Sidebar with tabs for Chats and Configuration
with st.sidebar:
st.title("🛠️ Control Center")
tab_chats, tab_config = st.tabs(["💬 Chats", "⚙️ Config"])

# Layout: adjusted ratios for a wider config panel when expanded
if st.session_state.config_minimized:
col_main, col_config = st.columns([10, 1])
else:
# Wider configuration panel (e.g., [2, 1] instead of [3, 1])
col_main, col_config = st.columns([2, 1])
with tab_chats:
setup_chat_menu(st.container())

with tab_config:
setup_configuration_panel(st.container())

# Main layout
col_main = st.container()

with col_main:
st.title("🤖 Web Automation Assistant")
Expand All @@ -517,7 +516,6 @@ def main():
st.info("👈 Please start a new chat or select an existing one from the menu.")
else:
# Main chat interface
st.write(f"Objective: **{st.session_state.chats[st.session_state.current_chat_id].get('title')}**")

# Display Todo List if it exists
if st.session_state.todos:
Expand Down Expand Up @@ -555,21 +553,6 @@ def main():
elif promo_code:
st.error("❌ Invalid code")

with col_config:
if st.session_state.config_minimized:
# Expand button when minimized
st.button("<", help="Expand Configuration", key="expand_btn", on_click=lambda: st.session_state.update({"config_minimized": False}))
else:
# Layout for Minimize button and Title
# Adjusted ratio to give the button more space and prevent title wrapping
top_col1, top_col2 = st.columns([0.25, 0.75])
with top_col1:
st.button(">", help="Minimize Configuration", key="minimize_btn", on_click=lambda: st.session_state.update({"config_minimized": True}))
with top_col2:
# Removed redundant icon to save horizontal space
st.markdown("### Configuration")

setup_configuration_panel(st.container())

if user_input:
add_message("user", user_input)
Expand Down Expand Up @@ -615,7 +598,6 @@ def main():

while st.session_state.automation_active and step_count < max_steps:
step_count += 1
add_message("assistant", f"--- Step {step_count} ---")

success = execute_automation_step(user_input)
if not success:
Expand Down
15 changes: 2 additions & 13 deletions browser_automation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,12 @@ def start_browser(self):
error_msg = getattr(response, 'error', 'Unknown error') if hasattr(response, 'error') else 'Failed to get session ID'
raise Exception(f"Failed to start Firecrawl browser: {error_msg}")

print(f"Firecrawl browser session started: {self.session_id}")

# Navigate to a default page
self.navigate_to('https://www.google.com')

return True

except Exception as e:
print(f"Failed to start Firecrawl browser: {str(e)}")
print(traceback.format_exc())
raise e

def take_screenshot(self):
Expand Down Expand Up @@ -106,9 +102,7 @@ def take_screenshot(self):

# Detect image format
try:
# Log first 16 bytes for debugging
header_hex = image_bytes[:16].hex()
print(f"Image header (hex): {header_hex}")

# Manual check for common formats if PIL fails or to be sure
if header_hex.startswith('89504e470d0a1a0a'):
Expand All @@ -122,9 +116,7 @@ def take_screenshot(self):
img = Image.open(io.BytesIO(image_bytes))
extension = img.format.lower()

print(f"Detected image format: {extension}")
except Exception as e:
print(f"Warning: Could not detect image format, defaulting to png: {e}")
except Exception:
extension = "png"

timestamp = datetime.now().strftime("%Y%m%d_%H%M%S")
Expand All @@ -138,7 +130,6 @@ def take_screenshot(self):
return filepath

except Exception as e:
print(f"Error taking screenshot: {str(e)}")
raise e

def get_interactable_elements(self):
Expand Down Expand Up @@ -214,8 +205,7 @@ def get_interactable_elements(self):

return self.element_map

except Exception as e:
print(f"Error getting elements: {str(e)}")
except Exception:
return {}

def click_element_by_index(self, index):
Expand Down Expand Up @@ -342,4 +332,3 @@ def close(self):
pass
self.session_id = None
self.element_map = {}
print("Firecrawl browser session closed")
13 changes: 4 additions & 9 deletions element_detector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,7 @@ def detect_and_annotate_elements(self, screenshot_path, browser_automation=None)

return annotated_path

except Exception as e:
print(f"Error in element detection: {str(e)}")
except Exception:
return screenshot_path # Return original if annotation fails
Comment on lines +78 to 79

@coderabbitai coderabbitai Bot May 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoff

Silent failure eliminates all diagnostic information.

The exception handler now suppresses all errors without providing any user feedback. Users cannot distinguish between successful annotation with no elements versus annotation failures due to file corruption, PIL errors, or font loading issues. In a browser automation context, this observability gap makes debugging significantly harder.

Consider at minimum returning a tuple (path, success_flag) or logging to a debug channel that developers can enable.

🧰 Tools
🪛 Ruff (0.15.12)

[warning] 78-78: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@element_detector.py` around lines 78 - 79, The current bare except that
returns screenshot_path swallows errors; change the except block that catches
Exception (the one returning screenshot_path) to "except Exception as e:" and
log the full error/stack trace via the module logger (e.g., logger.exception or
logging.exception) so diagnostics are preserved, then return a tuple
(screenshot_path, False) instead of the raw path so callers can distinguish
failure vs success; update any callers of the function to handle the (path,
success_flag) return or alternatively keep a backward-compatible code path that
returns just the path while also emitting the logged exception.


def annotate_elements_with_positions(self, screenshot_path, element_positions):
Expand Down Expand Up @@ -139,8 +138,7 @@ def annotate_elements_with_positions(self, screenshot_path, element_positions):

return annotated_path

except Exception as e:
print(f"Error in element annotation: {str(e)}")
except Exception:
return screenshot_path

def get_element_positions_from_browser(self, browser_automation):
Expand All @@ -166,8 +164,7 @@ def get_element_positions_from_browser(self, browser_automation):

return positions

except Exception as e:
print(f"Error getting element positions: {str(e)}")
except Exception:
return {}

def create_annotated_screenshot(self, browser_automation):
Expand All @@ -183,14 +180,12 @@ def create_annotated_screenshot(self, browser_automation):
positions = self.get_element_positions_from_browser(browser_automation)

if not positions:
print("No elements detected for annotation")
return screenshot_path

# Annotate with positions
annotated_path = self.annotate_elements_with_positions(screenshot_path, positions)

return annotated_path

except Exception as e:
print(f"Error creating annotated screenshot: {str(e)}")
except Exception:
return None
Loading