Skip to content

delete button #112

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

Merged
MarcSkovMadsen merged 1 commit into from
Jan 20, 2026
Merged

delete button #112

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 34 additions & 2 deletions src/holoviz_mcp/display_mcp/pages/feed_page.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ def feed_page():
# Create chat feed
chat_feed = pn.Column(sizing_mode="stretch_both")

def on_delete(snippet_id):
"""Handle deletion of a visualization."""
# Delete from database
get_db().delete_snippet(snippet_id)
# Remove from cache
if snippet_id in pn.state.cache["views"]:
del pn.state.cache["views"][snippet_id]
# Refresh feed
update_chat()
Comment on lines +25 to +33

Copilot AI Jan 20, 2026

Copy link

Choose a reason for hiding this comment

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

The on_delete function does not handle potential errors from the database deletion operation. If get_db().delete_snippet() fails or raises an exception, the cache could be left in an inconsistent state (the cache entry might be deleted while the database record remains). Consider wrapping the deletion logic in a try-except block and only clearing the cache if the database deletion succeeds. The delete_snippet method returns a bool indicating success, which should be checked before modifying the cache.

Copilot uses AI. Check for mistakes.
Comment on lines +25 to +33

Copilot AI Jan 20, 2026

Copy link

Choose a reason for hiding this comment

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

There is a potential race condition between the on_delete function and the update_chat periodic callback (which runs every 1 second per line 125). When a snippet is deleted, update_chat() is called to refresh the feed. However, the periodic callback could also be executing simultaneously, potentially attempting to recreate the deleted view from the database before the deletion completes or while cache is being modified. Consider adding synchronization or checking if snippets exist in the database before caching their views.

Copilot uses AI. Check for mistakes.
Comment on lines +25 to +33

Copilot AI Jan 20, 2026

Copy link

Choose a reason for hiding this comment

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

The new delete functionality (on_delete function and delete button) lacks test coverage. While there are tests for the underlying database.delete_snippet method, there are no tests verifying the feed_page delete behavior, including cache invalidation and feed refresh logic. Consider adding tests that verify: 1) successful deletion removes items from both database and cache, 2) the feed is properly refreshed after deletion, and 3) error cases are handled gracefully.

Copilot uses AI. Check for mistakes.

def get_view(req):
"""Create view for a single visualization in the feed."""
if req.id in pn.state.cache["views"]:
Expand All @@ -35,7 +45,7 @@ def get_view(req):
title = f"""\
**{req.name or req.id}** ({created_at})\n\n{req.description}\n
"""
iframe = f"""<div style="resize: vertical; overflow: hidden; height: calc(75vh - 200px); width: 100%; max-width: 100%; border: 1px solid gray;">
iframe = f"""<div style="resize: vertical; overflow: hidden; height: calc(75vh - 300px); width: 100%; max-width: 100%; border: 1px solid gray;">
<iframe src="{url}" style="height: 100%; width: 100%; border: none;" frameborder="0"></iframe>
</div>"""
# Create copy button with JavaScript callback
Expand Down Expand Up @@ -66,9 +76,31 @@ def get_view(req):
""",
)

delete_button = pn.widgets.Button(
name="🗑️ Delete",
button_type="danger",
width=120,
description="Delete this visualization",
)
delete_button.on_click(lambda event: on_delete(req.id))

with pn.config.set(sizing_mode="stretch_width"):
message = pn.Column(
pn.pane.Markdown(title), pn.pane.Markdown(iframe), pn.Row(pn.HSpacer(), open_button, copy_button, margin=(0, 10, 0, 10), align="end")
pn.pane.Markdown(
title,
margin=(10, 10, 0, 10),
),
pn.Tabs(
pn.pane.Markdown(iframe, name="View"),
pn.widgets.CodeEditor(
value=req.app,
name="Code",
language="python",
theme="github_dark",
),
margin=(0, 10, 10, 10),
),
pn.Row(pn.HSpacer(), open_button, copy_button, delete_button, margin=(0, 10, 0, 10), align="end"),
)

pn.state.cache["views"][req.id] = message
Expand Down
Loading