Fix weather tooltip, silent drop failures, and retry prompt divergence in Fishtank/Dispatch - #5
Conversation
… in start scripts Agent-Logs-Url: https://github.com/potemkin666/merman/sessions/ee368e01-7a56-4aa1-a339-370c3ce9e126 Co-authored-by: potemkin666 <183807833+potemkin666@users.noreply.github.com>
…d seed, fish icon in bat title Agent-Logs-Url: https://github.com/potemkin666/merman/sessions/99ab56f2-f3c3-48bb-b7e3-94ec9529b68b Co-authored-by: potemkin666 <183807833+potemkin666@users.noreply.github.com>
Agent-Logs-Url: https://github.com/potemkin666/merman/sessions/406d3a74-e7fa-4a58-be3c-8ac9dc4229a1 Co-authored-by: potemkin666 <183807833+potemkin666@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes several UX issues across the Fishtank and Dispatch screens, and adjusts native-module rebuild behavior/messaging in launch scripts.
Changes:
- Add weather-state tooltips and non-file drag-and-drop feedback in
Fishtank.tsx, plus randomize seabed seed per session. - Ensure Retry reuses the last dispatched prompt in
Dispatch.tsx(with updated tooltip copy). - Update native rebuild messaging in
start.sh/start.batand remove thepostinstallrebuild hook inpackage.json(plus tweak habit suggestion copy).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| start.sh | Improves rebuild step messaging when native rebuild fails. |
| start.bat | Updates window title and rebuild messaging on Windows. |
| src/renderer/src/screens/Fishtank.tsx | Adds tooltips for weather indicator; adds explicit feedback for non-file drops; changes seabed seed to be randomized per session. |
| src/renderer/src/screens/Dispatch.tsx | Captures last dispatched prompt and uses it for Retry; updates Retry tooltip text. |
| src/main/services/habitService.ts | Updates suggestion text copy. |
| package.json | Removes postinstall electron-rebuild hook. |
Comments suppressed due to low confidence (3)
src/renderer/src/screens/Dispatch.tsx:97
- Retrying via
handleDispatch(lastDispatchedPrompt)breaks in Advanced mode:handleDispatchwill early-return after opening the confirm dialog, losing the override prompt. When the user confirms,AdvancedConfirmDialogcallsonConfirm()with no args, so dispatch uses the current textarea prompt (and the dialog displaysprompt.trim()), not the original failed prompt. Persist the override prompt through the confirmation flow (e.g., store a pending prompt in state and pass it to the dialog / onConfirm).
const handleDispatch = async (overridePrompt?: string) => {
const promptToUse = overridePrompt !== undefined ? overridePrompt : prompt.trim()
if (!promptToUse) return
if (cooldown > 0) return
// If the selected mode is "advanced" (Advanced Custom preset), require confirmation first
const isAdvancedMode = mode === 'advanced'
if (isAdvancedMode && !showAdvancedConfirm) {
setShowAdvancedConfirm(true)
return
}
src/renderer/src/screens/Dispatch.tsx:276
- The retry tooltip says it will re-send the original task “exactly as it was dispatched”, but retry currently reuses only the prompt; it still uses the current
mode(and other mutable inputs) when callingDISPATCH_TASK. Either capture and reuse the original mode alongside the prompt, or adjust the tooltip copy to match the actual behavior.
<Tooltip text={cooldown > 0 ? `Cooling down — ready in ${cooldown}s.` : "Re-send the original task exactly as it was dispatched — even if you have edited the text above."}>
<button
onClick={handleRetry}
disabled={dispatching || cooldown > 0}
aria-label={cooldown > 0 ? `Retry available in ${cooldown}s` : "Retry the failed task"}
className="btn btn--warning"
style={{ marginTop: 12, fontSize: 13 }}
>
{cooldown > 0 ? `⏳ Retry in ${cooldown}s` : '🔄 Retry'}
package.json:16
- PR description focuses on Fishtank/Dispatch UX bugs, but this PR also removes the
postinstallrebuild hook (and updates launcher rebuild messaging, plus modifies Habit suggestion copy). Consider updating the PR description to mention these additional changes or splitting them into a separate PR to keep scope clear.
"scripts": {
"dev": "electron-vite dev",
"build": "electron-vite build",
"preview": "electron-vite preview",
"start": "electron .",
"pack": "electron-vite build && electron-builder --dir",
"dist": "electron-vite build && electron-builder",
"dist:mac": "electron-vite build && electron-builder --mac",
"dist:win": "electron-vite build && electron-builder --win",
"dist:linux": "electron-vite build && electron-builder --linux",
"rebuild": "electron-rebuild",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| call npm run rebuild 2>nul | ||
| if %ERRORLEVEL% neq 0 ( | ||
| echo ⚠️ Native rebuild had warnings ^(non-fatal — the app will still work^). | ||
| echo ⚠️ Native module build skipped — Visual Studio not found. | ||
| echo The terminal will use a basic shell instead. | ||
| echo Install Visual Studio Build Tools to enable full PTY support. | ||
| echo See: https://aka.ms/vs/buildtools | ||
| ) else ( | ||
| echo ✅ Native modules built successfully! |
There was a problem hiding this comment.
%ERRORLEVEL% is evaluated when the surrounding parenthesized block is parsed, so this check can be stale and may not reflect the exit code from npm run rebuild. Use if errorlevel 1 (...) or delayed expansion (!ERRORLEVEL!) for a reliable rebuild-success/failure branch.
| if %ERRORLEVEL% neq 0 ( | ||
| echo ⚠️ Native rebuild had warnings ^(non-fatal — the app will still work^). | ||
| echo ⚠️ Native module build skipped — Visual Studio not found. | ||
| echo The terminal will use a basic shell instead. | ||
| echo Install Visual Studio Build Tools to enable full PTY support. | ||
| echo See: https://aka.ms/vs/buildtools | ||
| ) else ( |
There was a problem hiding this comment.
The fallback message assumes the rebuild failed because Visual Studio is missing, but npm run rebuild can fail for other reasons (e.g., corrupt node_modules, incompatible Electron headers). Consider making the message generic (“native module rebuild failed”) and either surface stderr or point users to rerun npm run rebuild without redirecting output for diagnostics.
| if npm run rebuild 2>/dev/null; then | ||
| echo " ✅ Native modules built successfully!" | ||
| else | ||
| echo " ⚠️ Native module build skipped — compiler toolchain not found." | ||
| echo " The terminal will use a basic shell instead." | ||
| echo " Install build-essential (Linux) or Xcode CLT (macOS) for full PTY support." |
There was a problem hiding this comment.
The else branch assumes the rebuild was skipped specifically due to a missing compiler toolchain, but npm run rebuild can fail for many other reasons. Consider using a more general message (rebuild failed) and/or logging stderr somewhere (or instructing rerunning npm run rebuild without suppressing output) so users can diagnose non-toolchain failures.
| if npm run rebuild 2>/dev/null; then | |
| echo " ✅ Native modules built successfully!" | |
| else | |
| echo " ⚠️ Native module build skipped — compiler toolchain not found." | |
| echo " The terminal will use a basic shell instead." | |
| echo " Install build-essential (Linux) or Xcode CLT (macOS) for full PTY support." | |
| REBUILD_LOG="rebuild-error.log" | |
| if npm run rebuild 2>"$REBUILD_LOG"; then | |
| echo " ✅ Native modules built successfully!" | |
| rm -f "$REBUILD_LOG" | |
| else | |
| echo " ⚠️ Native module rebuild failed." | |
| echo " The terminal may use a basic shell instead." | |
| echo " See $REBUILD_LOG for details, or rerun 'npm run rebuild' manually to view the full error output." | |
| echo " If the error mentions missing build tools, install build-essential (Linux) or Xcode CLT (macOS)." |
| position="top" | ||
| maxWidth={320} | ||
| > | ||
| <span style={{ cursor: 'help' }}> |
There was a problem hiding this comment.
This tooltip trigger is a non-focusable <span>, so keyboard users can’t focus it to reveal the tooltip (Tooltip shows onFocus/onBlur only when something inside can receive focus). Consider making the trigger focusable (e.g., tabIndex={0}) and ensuring it has an accessible name so the weather explanations are available without a mouse.
| <span style={{ cursor: 'help' }}> | |
| <span | |
| tabIndex={0} | |
| aria-label={`Weather status: ${weather === 'calm' ? 'clear' : weather === 'working' ? 'active' : weather === 'stormy' ? 'stormy' : weather === 'golden' ? 'golden' : 'storm'}. Focus for explanation.`} | |
| style={{ cursor: 'help' }} | |
| > |
Three independent UX bugs: the Fishtank weather indicator was completely unexplained, dropping non-file content silently did nothing, and the Retry button could silently send a different prompt than the one that originally failed.
Weather tooltip (
Fishtank.tsx)fishtank-weather-labelin the existingTooltipcomponent; each of the five weather states now has a hover explanation of what triggered it and when it clearscursor: helpto signal interactivityDrag-and-drop non-file feedback (
Fishtank.tsx)Two previously silent early-returns now surface
🚫 Drop files or folders — not links or text.via the existing catch-toast + emissary speech:files.length === 0butdataTransfer.itemsexist → URL / text / browser-link drag.path→ sandboxed drag edge caseRetry uses original prompt (
Dispatch.tsx)handleDispatchnow accepts an optionaloverridePrompt; the sent prompt is captured in state before the async call; a dedicatedhandleRetrypasses it back so textarea edits between failure and retry are ignored:Retry button tooltip updated to make this behaviour explicit to the user.