fix(home): make per-file Remove button reliable
Reported: the "✕" buttons on the uploaded file list removed files
inconsistently — some clicks took, some didn't.
Two compounding causes:
1. ``key=f"_home_remove_{name}"`` embedded the raw filename in the
Streamlit widget key. Streamlit's widget-identity machinery
normalizes keys differently across reruns when they contain
spaces, dots, brackets, or non-ASCII characters, so a button's
identity could shift between the render where the user clicked
it and the rerun that should have processed the click. The click
was registered, but the post-rerun render produced a new widget
under a new effective key, and the original click was "lost".
2. The handler mutated ``home_uploads`` mid-loop while subsequent
iterations were still creating buttons. ``st.rerun()`` raises
synchronously, but if ANOTHER button's state changed in the same
pass (e.g. a stale click held over from a fast double-tap), the
ordering of state-mutation vs widget-key-update vs rerun could
race.
Fixes:
- Stable widget keys: ``f"_home_remove_{sha1(name)[:10]}"``. The
hash is identifier-safe regardless of spaces / dots / Unicode in
the filename. Verified across "sample with spaces.csv",
"sample.csv", and "日本語.csv" — three sequential Remove clicks
each remove exactly one file with no clicks lost.
- Two-phase capture: the loop collects the target ``to_remove``
filename, finishes rendering every other row at consistent widget
identity, THEN mutates state once and reruns. No more mid-loop
``del`` racing other widgets' click handlers.
- Wider click target: column ratio ``[8, 1]`` (was ``[12, 1]``) and
``use_container_width=True`` on the Remove button so the click
surface fills the entire column. Label changed to "Remove" for
the same reason — "✕" is a thin glyph that compressed the
hit-test region.
2220 tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -101,10 +101,28 @@ def _home_page() -> None:
|
||||
# ourselves rather than trusting Streamlit's widget chrome because
|
||||
# the widget's "✕" only mutates widget-state, leaving home_uploads
|
||||
# out of sync.
|
||||
#
|
||||
# Two-phase click capture pattern (avoids the "hit-or-miss" click
|
||||
# losses we had previously):
|
||||
#
|
||||
# 1. ``st.button(key=stable_hash)`` returns True on the rerun where
|
||||
# it was clicked. We use a sha1 hash of the filename as the key
|
||||
# so it's identifier-safe regardless of spaces / dots / unicode
|
||||
# in the file name — Streamlit's widget-identity hashing on raw
|
||||
# filenames was the root cause of inconsistent removals.
|
||||
# 2. Inside a single pass we collect WHICH file to remove (if any),
|
||||
# then mutate state ONCE after the loop and rerun. Mutating mid
|
||||
# -loop while continuing to render other buttons risked
|
||||
# interleaving widget-key updates with state changes.
|
||||
if home_uploads:
|
||||
import hashlib
|
||||
st.markdown("**Uploaded files**")
|
||||
to_remove: str | None = None
|
||||
for name in list(home_uploads.keys()):
|
||||
col_file, col_remove = st.columns([12, 1])
|
||||
digest = hashlib.sha1(
|
||||
name.encode("utf-8"), usedforsecurity=False,
|
||||
).hexdigest()[:10]
|
||||
col_file, col_remove = st.columns([8, 1])
|
||||
col_file.markdown(
|
||||
f"📄 `{name}` "
|
||||
f"<span style='opacity:0.6'>"
|
||||
@@ -112,23 +130,28 @@ def _home_page() -> None:
|
||||
unsafe_allow_html=True,
|
||||
)
|
||||
if col_remove.button(
|
||||
"✕",
|
||||
key=f"_home_remove_{name}",
|
||||
"Remove",
|
||||
key=f"_home_remove_{digest}",
|
||||
help=f"Remove {name}",
|
||||
type="secondary",
|
||||
use_container_width=True,
|
||||
):
|
||||
del home_uploads[name]
|
||||
to_remove = name
|
||||
|
||||
if to_remove is not None:
|
||||
del home_uploads[to_remove]
|
||||
# Drop any findings/results tied to the removed file.
|
||||
findings_by_file_drop = st.session_state.get(
|
||||
"home_findings_by_file", {}
|
||||
)
|
||||
findings_by_file_drop.pop(name, None)
|
||||
findings_by_file_drop.pop(to_remove, None)
|
||||
st.session_state["home_uploads"] = home_uploads
|
||||
st.session_state["home_findings_by_file"] = findings_by_file_drop
|
||||
# If we just removed the active upload, also clear the
|
||||
# singular ``home_uploaded_*`` keys so tool pages don't
|
||||
# pick up stale bytes; the next render will repopulate
|
||||
# them from whatever file is now first.
|
||||
if st.session_state.get("home_uploaded_name") == name:
|
||||
if st.session_state.get("home_uploaded_name") == to_remove:
|
||||
st.session_state.pop("home_uploaded_name", None)
|
||||
st.session_state.pop("home_uploaded_size", None)
|
||||
st.session_state.pop("home_uploaded_bytes", None)
|
||||
|
||||
Reference in New Issue
Block a user