Skip to content

chore: remove the next-agent shortcut and the tool detail copy buttons - #1572

Merged
devswha merged 1 commit into
mainfrom
chore/remove-next-agent-and-tool-copy
Oct 21, 2026
Merged

devswha merged 1 commit into
mainfrom
chore/remove-next-agent-and-tool-copy

Conversation

@devswha

@devswha devswha commented Oct 21, 2026

Copy link
Copy Markdown
Owner

Change

Remove two features added after v0.4.12 because the maintainer judged them unnecessary
during manual QA of main:

Deliberately keep the agentRoster.ts extraction and App-owned sidebar activity state
because the sidebar still uses them. Preserve the roster regression test as
agentRoster.test.ts. Keep the existing Output/Error headings, typed tool input,
whole-output loading, and message/Markdown copying.

Stored overrides for the removed shortcut are ignored by the existing shortcut-ID
allowlist. Existing settings tests already cover unknown stored IDs, so no duplicate
test or storage migration is needed.

Neither git tag --contains 626204fa nor git tag --contains 0cd899f7 lists a tag
(both exit 0). Delete both unreleased changelog fragments rather than adding a
Removed entry. No new UI design is introduced; the tool detail returns to its existing
pre-#1565 design.

Validation

  • bun run check fast: exit 0; 5,810 passed, 0 failed, 8 skipped.
    Generated types/themes, workflow syntax, typechecks, build, and unit tests passed.
  • Affected unit files: exit 0.
    bun test src/lib/shortcuts.test.ts src/lib/settings.test.ts src/lib/agentRoster.test.ts src/lib/sidebarOrder.test.ts src/lib/sidebarActivity.test.ts scripts/ci-browser.test.ts scripts/pages-gate.test.ts
  • Browser-lane bookkeeping tests after final cleanup: exit 0.
    bun test scripts/ci-browser.test.ts scripts/pages-gate.test.ts
  • bun scripts/sidebar-activity-demo-regression.ts: exit 0.
  • bun scripts/shortcut-exact-modifiers-demo-regression.ts: exit 0.
    Both used a fresh shared demo build and isolated fictional fixtures.
  • Manual browser QA of the actual built client at desktop and phone sizes, light
    and dark: no tool-detail copy controls; Output/Error and whole-output loading remain;
    removed palette action and shortcut setting are absent; old saved/default chords do
    not navigate; the existing palette and Settings shortcuts still work.
  • git diff --check: exit 0.
  • Independent Codex final review: APPROVE, no concrete blockers. The reviewer
    inspected all 12 desktop/phone, light/dark screenshots. Its only initial finding
    was a dead reference to the removed browser script in the parallel runner; that
    reference is removed.
    The Omo reviewer could not start because this session reached its nesting limit.

All checks used isolated fixtures, not the maintainer's live herdr session.

@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 364a4ff8-7813-4939-9fc6-138deb9c48a8
📥 Commits

Reviewing files that changed from the base of the PR and between ee2daec and e5a9792.

📒 Files selected for processing: 27
  • DESIGN.md
  • changelog.d/next-attention-agent.md
  • changelog.d/tool-detail-copy.md
  • docs/development.md
  • scripts/attention-jump-demo-regression.ts
  • scripts/ci-browser-parallel.ts
  • scripts/ci-browser.sh
  • scripts/ci-browser.test.ts
  • scripts/tool-copy-demo-regression.ts
  • src/App.tsx
  • src/components/AgentSidebar.tsx
  • src/components/ChatView.css
  • src/components/ChatView.tsx
  • src/components/CommandPalette.tsx
  • src/components/MachineSidebar.tsx
  • src/lib/actions.ts
  • src/lib/agentRoster.test.ts
  • src/lib/agentRoster.ts
  • src/lib/attentionJump.test.ts
  • src/lib/attentionJump.ts
  • src/lib/i18n.ja.ts
  • src/lib/i18n.ko.ts
  • src/lib/i18n.zh.ts
  • src/lib/paletteSettings.ts
  • src/lib/shortcutBindings.ts
  • src/lib/shortcuts.test.ts
  • src/lib/shortcuts.ts
💤 Files with no reviewable changes: 13
  • src/lib/attentionJump.test.ts
  • changelog.d/next-attention-agent.md
  • docs/development.md
  • changelog.d/tool-detail-copy.md
  • scripts/attention-jump-demo-regression.ts
  • src/lib/actions.ts
  • src/lib/shortcuts.test.ts
  • src/lib/i18n.ja.ts
  • src/lib/i18n.ko.ts
  • src/lib/paletteSettings.ts
  • src/lib/i18n.zh.ts
  • src/lib/attentionJump.ts
  • scripts/tool-copy-demo-regression.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

📝 Summary

Summary by CodeRabbit

  • Removed Features

    • Removed the “Next agent needing attention” command, including its command-palette entry and keyboard shortcut.
    • Removed automatic navigation between agents awaiting attention.
    • Removed chat tool input/output copying controls and related copy-status behavior.
  • Bug Fixes

    • Tool output remains available directly in chat, including loading and retry states.
  • Tests

    • Updated browser checks and roster-ordering coverage to reflect the streamlined functionality.

The pull request removes next-agent attention navigation and Chat tool input/output copy controls. It also removes their shortcut and palette entries, implementation code, localizations, documentation, and browser regression coverage.

Changes: Next-Agent Attention Navigation

Layer / File(s) Summary
Remove attention navigation action
src/lib/actions.ts, src/lib/attentionJump.ts, src/App.tsx, src/lib/shortcuts.ts, src/components/CommandPalette.tsx, src/lib/*, src/components/*
The app action, attention-target selection, shortcut, and palette action are removed. Related localization entries and attention-navigation tests are removed. Agent roster ordering tests are added.
Remove attention-navigation support
DESIGN.md, changelog.d/*, docs/development.md, scripts/attention-jump-demo-regression.ts, scripts/ci-browser*
The attention-navigation documentation and browser regression script are removed. CI no longer runs the removed regression scripts, and browser test expectations are updated.

Chat Tool Input and Output Copy

Layer / File(s) Summary
Remove tool copy controls and coverage
src/components/ChatView.*, src/lib/i18n.*, scripts/tool-copy-demo-regression.ts, scripts/ci-browser.sh, DESIGN.md, changelog.d/tool-detail-copy.md
The work detail view removes input and output copy controls and their styling. Related translations and browser regression coverage are removed. Existing output rendering, loading, and retry behavior remains.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Other

Possibly related issues

Suggested reviewers: kratam, radicor

Merge Risk: ⚪ Minimal · up to e5a97

No actionable issue is established for the feature removal. The PR appears mergeable after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 12 files. (2 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies both primary changes: removal of the next-agent shortcut and tool-detail copy buttons.
Description check Passed The description follows the required Change and Validation sections. It explains the removed features, retained behavior, changelog handling, and provides detailed test and manual QA results.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 12 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devswha
devswha merged commit 90b66e8 into main Oct 21, 2026
9 checks passed
@devswha
devswha deleted the chore/remove-next-agent-and-tool-copy branch October 21, 2026 22:48
devswha added a commit that referenced this pull request Oct 23, 2026
## Change

`src/lib/agentRoster.test.ts`'s fixture still set a per-pane `finish_id`
and a `finish_epoch` left over from the attention tests #1572 removed;
nothing reads them. They are gone, and the test stays, since it is the
only coverage of the kept roster order.

The issue's second point (the sidebar activity state in `App.tsx`) is
only worth doing if profiling shows a cost, and this sweep leaves
`App.tsx` alone, so it stays open.

Refs #1575

## Validation

- `bun test src/lib/agentRoster.test.ts`: pass.
- `bun run check fast`: pass.
devswha pushed a commit that referenced this pull request Oct 25, 2026
Users never got #1845 (no tag contains 314200a), so, as in #1572, its Added fragment is deleted and no Removed entry is added.
devswha added a commit that referenced this pull request Oct 25, 2026
## Change

Selecting text in one of the agent's chat replies drew a **Quote in
message** row over the message box (#1845). That row joined the composer
stack, so the chat moved under the reader's selection. The maintainer
asked for the feature to be removed. Measured on the demo, on main at
a1e4f8b:

| width | chat scroller height | selected reply's position |
| --- | --- | --- |
| 1440 | −44 px | unchanged |
| 390 (phone) | −84 px | moved 52 px (scrollTop +52) |

This PR removes the whole chat-reply feature:

- `.composer-quote-row` / `.chat-quote` (PaneTerminal.css) and the row,
its press hold and its pointerup listener (PaneTerminal.tsx)
- the reply selection tracking and the `.chat-reply` wrapper in
ChatView. The page-key line step goes back to `.chat-turn-agent >
.markdown`, in ChatView and in `pane-paging-demo-regression.ts`.
- ChatTimeline's `holdSelection` row pinning
- `formatReplyQuote`, `REPLY_QUOTE_LABEL` and
`ComposerDraftStore.appendQuote`, with the tests only they needed
- `scripts/reply-quote-demo-regression.ts` and its browser-lane line,
the DESIGN.md entry, and the Added fragment

The terminal lens keeps its own **Quote in message** (#951) as it was
before #1845. #1845 changed only one thing on that path: it moved the
fence into a shared `fenceFor` helper, which is now back inline in
`formatTerminalQuote`. The terminal still uses its three strings (Quote
in message, Added to the message draft, Too long for the message draft),
so they stay translated. Files that no later commit touched are restored
to their pre-#1845 state; ChatView, PaneTerminal, DESIGN.md and
ci-browser.sh were edited by hand, so later work in them is kept.

No new design. Chat selection goes back to the plain browser selection
and copy that DESIGN.md already describes (the Claude/ChatGPT
distinction between selecting text and copying an answer).

Changelog: users never got #1845 (`git tag --contains 314200a` is
empty). As in #1572, its unreleased Added fragment is deleted and no
Removed entry is added.

## Validation

- `bun run check fast` (through `dori heavy`): workflow syntax,
generated types and themes, typecheck, build, and unit tests (6714 pass)
all ok.
- No-movement QA on the demo build at 1440 (a real mouse drag) and at
390 phone emulation (a selection range, as a long press leaves it),
sampling 40 animation frames after the selection. On this branch the
reply's position, the scroller's height and scrollTop, and the message
box's position all stay put (deltas 0), no quote row is drawn, and the
selection stays. The same script on main shows the movement in the table
above.
- Terminal lens: `checkTerminalCopyOff`
(terminal-copy-off-regression.ts, which drags in the terminal and waits
for its Quote in message button) passes on an isolated test herdr.
`terminalQuote.test.ts` passes.
- Adjacent regressions: `pane-paging-demo-regression.ts`,
`chat-selection-copy-regression.ts` and `chat-virtual-browser-qa.ts`
pass. `scripts/ci-browser.test.ts` passes.
- Reviewed by Codex (gpt-6.1-sol, xhigh): no findings, APPROVE.

---------

Co-authored-by: mp <mp@local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant