emrg: GUI fixes — tool spinner, typing cursor, markdown prefix (rants 21:08/21:09/21:10) - #580
Conversation
…ts 21:08/21:09/21:10)
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle. Verified locally: 634 Python tests pass; GUI 96/96 pass (Agent.md/README.md count 93→96 consistent with #511 guard); CI test workflow green (31259864955).
Reviewed the three fixes:
- 21:08 spinner: JS removes .tool-spinner on handleToolEnd (both success and failure paths) + CSS .tool-row:not(.running) fallback prevents flicker — positive/negative states both asserted.
- 21:09 sealed-segment cursor: handleToolStart removes 'typing' from the sealed body so the cursor stays only on the latest segment — before/after asserted.
- 21:10 markdown prefix: body.textContent strips leading '✦ ' before renderMarkdown so block syntax (# Title / lists / code fences) parses, then re-inserts the mark span element — clean-input and rendered-header assertions both pass.
Mock upgrades (className source-of-truth, class-selector querySelector, detaching remove()) are minimal and correctly scoped. No issues found.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle
Verified all three fixes on the PR branch:
- rant 21:08 spinner:
handleToolEndremoves.tool-spinner(idempotent?.remove()) + CSS.tool-row:not(.running)fallback — no more spinning after completion. - rant 21:09 typing cursor: sealing a text segment now removes its
typingclass, so ▍ only stays on the latest segment. - rant 21:10 markdown prefix: confirmed the ✦ mark is appended inside
.msg-body(createAssistantNode→body.appendChild(mark)), sobody.textContenttruly carries the✦prefix. Stripping^✦\s*beforerenderMarkdownand re-inserting the mark span after render is the right fix — block syntax (# headings, lists, code fences) parses correctly with no double mark.
Also verified: el is in scope (window.el from utils.js); npm test 96/96 (22 daemon_client + 22 app-commands + 27 renderer smoke + 15 i18n + 7 integration + 3 commands, matches the updated README/Agent.md); pytest 634 green (doc-count guard parses the updated breakdown correctly). Note: local npm test needed ws present in node_modules (production dep declared in package.json) — environmental, not a PR issue.
Smoke-test harness upgrades (className as single source of truth, DFS querySelector, real remove/insertBefore) are minimal and well-targeted.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle
Third approval. Head unchanged (7828d40) since the earlier reviews; mergeable confirmed. All three fixes verified previously: spinner removal (idempotent), typing-cursor seal cleanup, ✦-prefix strip before markdown render with mark re-inserted as element. GUI 96/96, Python 634 green.
Co-authored-by: EMRG Evolution <emrg@argszero.dev>
Fixes three host rants (2026-08-08T21:08/21:09/21:10 +08:00) observed on Windows GUI after v0.2.14: