Skip to content

PR: Extend delete overlays and clean companion parts during tool deletion#247

Open
VooDisss wants to merge 4 commits into
NeuralNomadsAI:devfrom
VooDisss:Delete_step-finish_parts_remainings
Open

PR: Extend delete overlays and clean companion parts during tool deletion#247
VooDisss wants to merge 4 commits into
NeuralNomadsAI:devfrom
VooDisss:Delete_step-finish_parts_remainings

Conversation

@VooDisss

@VooDisss VooDisss commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

PR: Extend delete overlays and clean companion parts during tool deletion

Fixes: #246

Purpose

This PR addresses incomplete deletions when users bulk-delete specific tool calls from the timeline. Previously, deleting a tool part left behind orphaned reasoning logs (thinking tokens) and generated text parts, and the destructive hover overlay only highlighted the tool cards themselves.

This update ensures that deleting tool parts cleans up the entire associated response context (step-finish, reasoning, and text parts) from that message. It also extends the red delete overlay to message headers, reasoning cards, and text parts to accurately reflect the scope of deletion to the user.

Changes by File

packages/ui/src/components/message-block.tsx

  • Threaded the selectedToolPartKeys prop through MessageContentItem, MessageBlock, and ReasoningCard.
  • Updated ReasoningCard's isSelectedForDeletion logic to check both selectedMessageIds (whole message) and selectedToolPartKeys (tool-part selection via a messageId: prefix match).
  • Moved the block-level selection highlight out of isDeleteMessageHovered to be handled at the part level via data-delete-part-hover attributes avoiding double overlays.
  • Added data-delete-part-hover to the ReasoningCard's root div.

packages/ui/src/components/message-item.tsx

  • Added the selectedToolPartKeys prop to MessageItemProps.
  • Expanded isSelectedForDeletion using the same prefix-match pattern to detect tool-part selections.
  • The <header> element now includes a delete-hover-scope class and data-delete-part-hover attribute to trigger the overlay at the header level.
  • Each .message-part-shell rendering text content now includes data-part-type and data-delete-part-hover for per-part overlay highlighting.

packages/ui/src/components/message-section.tsx

  • Updated the deleteSelectedMessages bulk action. It now additionally collects and deletes step-finish, reasoning, and text parts when a message has its tool parts selected for deletion. This ensures no orphaned assistant content remains.
  • Simplified the deletion guard condition from selectedToolPartIds.size === 0 || stepFinishPartIds.length === 0 down to selectedToolPartIds.size === 0, ensuring reasoning/text parts evaluate correctly even if step-finish parts are absent.

packages/ui/src/styles/messaging/delete-overlays.css

  • Widened the part-level overlay inset from -2px to -4px for visual consistency with the message-level overlay.
  • Added a new rule for .message-reasoning-card[data-delete-part-hover] using inset: 0 for flush overlay rendering within reasoning card boundaries.

VooDisss added 3 commits March 7, 2026 11:08
Delete step-finish parts for messages when tool badges are selected so usage chips disappear with the tool deletion flow in packages/ui/src/components/message-section.tsx.
…panion parts

When users select individual tool parts for bulk deletion (rather than
entire messages), the red delete overlay now correctly highlights
all related UI elements for that message — message headers, text parts,
and reasoning cards — providing clear visual feedback of what will be
removed.

Previously, selecting tool parts for deletion only highlighted the tool
call cards themselves. The message's text content and reasoning blocks
appeared unaffected, which was misleading because deleting the tool
parts leaves an incomplete message (an assistant response with no tool
output).

Changes by file:

message-block.tsx:
  - Thread selectedToolPartKeys prop through MessageContentItem,
    MessageBlock, and ReasoningCard component trees.
  - ReasoningCard's isSelectedForDeletion now checks both
    selectedMessageIds (whole-message selection) and
    selectedToolPartKeys (tool-part selection via prefix match on
    messageId:).
  - Move the block-level selection highlight out of
    isDeleteMessageHovered — responsibility is now at the part level
    via data-delete-part-hover attributes, avoiding double overlays.
  - ReasoningCard's root div gains data-delete-part-hover to render
    the red overlay when its parent message has tool parts selected.

message-item.tsx:
  - Accept new selectedToolPartKeys prop.
  - isSelectedForDeletion uses the same prefix-match pattern to
    detect tool-part selections.
  - <header> element gains delete-hover-scope class and
    data-delete-part-hover attribute for header-level overlay.
  - Each .message-part-shell gains data-part-type and
    data-delete-part-hover for per-part overlay on text content.

message-section.tsx:
  - deleteSelectedMessages now also collects reasoning and text parts
    from messages that have tool parts selected for deletion, and
    deletes them alongside the tool parts. This prevents orphaned
    assistant text/reasoning from remaining after tool removal.
  - Guard condition simplified from
    selectedToolPartIds.size === 0 || stepFinishPartIds.length === 0
    to selectedToolPartIds.size === 0 to avoid short-circuiting when
    no step-finish parts exist but reasoning/text parts do.

delete-overlays.css:
  - Part-level overlay inset widened from -2px to -4px for consistency
    with the message-level overlay.
  - New rule for .message-reasoning-card[data-delete-part-hover] uses
    inset: 0 for flush overlay within reasoning card boundaries.
@pascalandr

Copy link
Copy Markdown
Contributor

Hi @VooDisss, is this PR still intended to address #246, or should we re-evaluate the current timeline deletion behavior from scratch?

@VooDisss

VooDisss commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

@pascalandr hi, yes

@pascalandr

Copy link
Copy Markdown
Contributor

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

Merge upstream/dev into the PR247 refresh branch in an isolated worktree and resolve the tool-deletion conflict in message timeline rendering. The resolution preserves selected tool-part overlays, companion step-finish/reasoning cleanup, and upstream post-compaction deletion restrictions/search and reasoning scroll behavior.

Validation: npm run typecheck --workspace @codenomad/ui fails on pre-existing SDK V2 type drift and implicit-any issues outside the conflict resolution after the conflict-specific CompactionCard prop issue was fixed.
@codenomadbot

codenomadbot Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Gatekeeper review for latest PR state. I interpreted the request as: review PR #247 ruthlessly for regressions, fit to #246, and implementation quality. I did not make code changes.

Verdict: request changes / do not merge yet

Findings:

  1. Blocking: compaction safety gate was removed, enabling unsafe deletion of pre-compaction history.
    packages/ui/src/components/message-section.tsx:148, :226, :473, :486

The PR deletes the prior deletableStartIndex / deletableMessageIds / isMessageDeletable guard and now treats new Set(messageIds()) as the allowed deletion set. The removed comment explicitly said deletion is only safe after the most recent compaction because earlier items do not reliably reflect what the model sees. This is a behavioral regression unrelated to #246’s companion-part cleanup. Timeline selection, range selection, select-all, direct checkbox selection, and deleteSelectedMessages() can now target messages/tool parts before compaction.

  1. Blocking: latest PR state no longer satisfies Bug / UX Issue: Incomplete deletion when removing tool calls from the timeline #246 because text parts are preserved and no longer highlighted.
    packages/ui/src/components/message-section.tsx:508-554, packages/ui/src/components/message-item.tsx:575-588

Issue #246 expected tool-part deletion to also remove corresponding step-finish, reasoning, and text parts, and expected overlays on headers, reasoning cards, and text parts. The latest commit removed textPartsToDelete and removed the delete-hover-scope / data-delete-part-hover wrapper from .message-part-shell. Result: deleting a tool call can still leave generated assistant text behind, and the UI no longer marks text as part of the destructive scope. That is the core reported bug still present for text.

  1. High: companion deletion is message-wide, not associated with the selected tool part(s).
    packages/ui/src/components/message-section.tsx:512-535

For each message with any selected tool part, the code collects every step-finish and every reasoning part in the message. In multi-step assistant messages, selecting one tool call can delete reasoning/step-finish parts that belong to other tool calls or other response steps. This is too aggressive and unpredictable for “specific tool calls from the timeline.” A safer implementation should derive the deletion set from ordered parts and delete only the companion parts associated with the selected tool segment/step, or make whole-message deletion explicit.

  1. Medium: header/reasoning checkboxes become misleading for tool-only selection.
    packages/ui/src/components/message-item.tsx:96-105, :410-422; packages/ui/src/components/message-block.tsx:1292-1300, :1454-1466

isSelectedForDeletion() returns true when any selected tool key has the message prefix, so the message header and reasoning card checkboxes render checked even when the message itself is not selected. Unchecking them calls onToggleSelectedMessage(messageId, false), which only removes from selectedForDeletion; it does not clear the selected tool timeline IDs. The checkbox therefore stays checked and cannot be used to deselect the active tool-part deletion. The visual overlay may need the prefix match, but the checkbox selected state should not be conflated with whole-message selection.

  1. Medium: deletion count/token estimate excludes auto-deleted companion parts.
    packages/ui/src/components/message-section.tsx:373-423

selectedDeleteCount and selectedTokenTotal count selected messages and selected tool parts only. The actual delete operation also deletes reasoning and step-finish parts. The toolbar can under-report what will be removed, which undermines the UX goal of accurately communicating destructive scope.

Implementation notes:

Verification:

  • Reviewed full diff against origin/dev and the current HEAD commit 6630a189 fix(ui): preserve assistant text when deleting tool parts.
  • Attempted npm run typecheck --workspace @codenomad/ui, but this checkout is missing dependencies/types (solid-js, @opencode-ai/sdk, ansi-sequence-parser, etc.), so typecheck fails before this PR can be isolated.

File size note:

  • Touched oversized files: packages/ui/src/components/message-section.tsx is ~1256 lines, packages/ui/src/components/message-block.tsx is ~1587 lines, and packages/ui/src/components/message-item.tsx is ~680 lines. I am not recommending drive-by refactors here, but future changes in this area should strongly consider extracting the deletion-selection model into a focused helper/module with tests.

--
Yours,
CodeNomadBot

@pascalandr pascalandr added the needs-work PR needs more work label Jun 7, 2026
@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/27095018604

Artifacts expire in 7 days.
Artifacts:

  • pr-247-86b7ddf24d4dd579b60afde1847cea29d8485b0c-tauri-linux
  • pr-247-86b7ddf24d4dd579b60afde1847cea29d8485b0c-tauri-macos
  • pr-247-86b7ddf24d4dd579b60afde1847cea29d8485b0c-tauri-windows
  • pr-247-86b7ddf24d4dd579b60afde1847cea29d8485b0c-electron-macos
  • pr-247-86b7ddf24d4dd579b60afde1847cea29d8485b0c-tauri-macos-arm64
  • pr-247-86b7ddf24d4dd579b60afde1847cea29d8485b0c-electron-windows
  • pr-247-86b7ddf24d4dd579b60afde1847cea29d8485b0c-electron-linux

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-work PR needs more work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug / UX Issue: Incomplete deletion when removing tool calls from the timeline

2 participants