Repository navigation
agent_ui: Scroll to AskUserQuestion Other answers - #64007
Conversation
|
Thanks for working on this! Our contribution guidelines ask for screenshots or recordings for changes visible in the UI. Can you add them? |
|
@smitbarmase absolutely. will update these accordingly for you. thanks for flagging. |
smitbarmase
left a comment
There was a problem hiding this comment.
Thanks for fixing this.
-
Have you tested it with Zed’s built-in agent too? I don’t think scrolling to an
ask_useranswer works there yet. The tool card uses a scoped ID like1:call_1, but the question request sendscall_1, so the new lookup won’t match them. I think we need to figure out where the id mismatch is happening and also cover it in test. -
Also, I think accepting a link to continue in the browser should not make its tool a user-message scroll target. URL elicitations also become
Accepted, but that does not mean the user submitted an answer. Lets handle that case too?
Thanks again.
|
Hey @smitbarmase thanks for your guidance on these. See below for a rundown of the issues and the fixes based on the suggestions here. Built-in agent: The mismatch was in ToolCallEventStream::request_elicitation in crates/agent/src/thread.rs. It built the elicitation's tool call ID from the raw tool_use_id (call_1), but the tool card uses the scoped tool_call_id (1:call_1). It now passes the scoped ID, so the two match. I added an end-to-end test in crates/agent/src/tests/mod.rs that runs ask_user through a native thread with a fake model and checks that the elicitation targets the card's ID. That test fails without the fix. I also extended the ask_user_tool test to assert the ID. I checked this in a real Zed build with the built-in agent as well: answering with Other and then clicking "Scroll to User Message" lands on the answer. URL elicitations: tool_call_has_accepted_user_answer now only counts accepted form-mode elicitations. Accepting a URL elicitation no longer makes its tool a scroll target. There's a new test for this in conversation_view.rs. Let me know if anything else is off here. Happy to run through it again. |
ae6030b to
7aab9b1
Compare
smitbarmase
left a comment
There was a problem hiding this comment.
Thanks! I made rebase since it had conflicts.
Scroll-to-user currently targets only UserMessage entries, so a typed Other reply stored on the ask_user tool call is skipped.
The Other reply is stored on a tool call linked to an accepted elicitation, not as a UserMessage. Use one matcher for turn-end scroll, fallback, and prev/next navigation.
The production fix landed separately in zed-industries#64086; this keeps the end-to-end coverage that an answered ask_user tool call is a scroll target.
Accepting a URL elicitation only consents to opening a link in the browser; the user never types an answer. Only accepted form elicitations now make their tool call a Scroll to User Message target.
7aab9b1 to
e0eccb7
Compare
|
@smitbarmase , thanks as well for your guidance on PRs! Would you have a moment to review mine? |
Objective
ask_userwith a typed Other reply, "Scroll to User Message" jumped to the original prompt instead of the steering text.Solution
ToolCall, not asUserMessage.AcpThread::is_user_authored_scroll_targetincrates/acp_thread/src/acp_thread.rs. It matches aUserMessage, or aToolCallwhose id is referenced by an accepted session elicitation (scope.tool_call_id).crates/agent_ui/src/conversation_view/thread_view.rsshare that matcher.Sibling of #63901. That PR skips
<task-notification>UserMessageentries. This PR counts accepted elicitationToolCalls. They touch the samerposition/scroll_to_user_message_indexsites. A merge should keep both.Testing
test_scroll_to_user_message_lands_on_ask_user_elicitation_answerand ran:Result: 3 passed (including the new test). The synthesized thread is
[UserMessage, ToolCall, Elicitation, AssistantMessage]. Scroll lands onitem_ix == 1(the tool call), not0.cargo testcommand, or follow the steps in "Scroll to user message" ignores user message in response to AskUserQuestion #63942.Screen.Recording.2026-09-12.at.9.07.08.AM.mov
Self-Review Checklist:
Release Notes: