Skip to content

spec: editable prompt suggestions (#9842)#10232

Open
lonexreb wants to merge 5 commits intowarpdotdev:masterfrom
lonexreb:spec/9842-editable-prompt-suggestions
Open

spec: editable prompt suggestions (#9842)#10232
lonexreb wants to merge 5 commits intowarpdotdev:masterfrom
lonexreb:spec/9842-editable-prompt-suggestions

Conversation

@lonexreb
Copy link
Copy Markdown
Contributor

@lonexreb lonexreb commented May 6, 2026

Spec for #9842. New insert-as-draft affordance (edit icon + modifier-click) on each prompt suggestion. Inserts the text into the agent input without sending; user edits and submits manually. Existing send-on-click behavior preserved.

Closes (spec-only) #9842.

@cla-bot cla-bot Bot added the cla-signed label May 6, 2026
@github-actions github-actions Bot added the external-contributor Indicates that a PR has been opened by someone outside the Warp team. label May 6, 2026
@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 6, 2026

@lonexreb

I'm starting a first review of this spec-only pull request.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview

This spec adds an insert-as-draft path for prompt suggestions while preserving the existing send-on-click behavior.

Concerns

  • The affordance contract does not unambiguously require both the edit icon and modifier-click path, even though the PR body describes both.
  • Existing draft handling is left for a future TECH decision, so implementation and acceptance tests can pick different behaviors.
  • The telemetry category mapping needs a named source in the existing model to keep event payloads consistent and private.

Verdict

Found: 0 critical, 2 important, 1 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Comment thread specs/GH9842/SPEC.md Outdated
Comment on lines +22 to +25
- **New: insert-as-draft.** Discoverable via either:
- A small "edit" icon next to the suggestion (preferred, low
friction).
- Modifier-click on the suggestion (Cmd-click / Ctrl-click).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [IMPORTANT] State whether both the edit icon and modifier-click are required; this currently permits implementing only one path while the PR body promises both.

Comment thread specs/GH9842/SPEC.md Outdated
Comment on lines +26 to +30
- B2. Insert-as-draft replaces the current agent-input contents
with the suggestion text. If the input already has user text,
show a one-time confirmation tooltip ("Replace existing
draft?") OR insert at the current caret position — TECH spec
picks one.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [IMPORTANT] Choose the existing-draft behavior in this spec instead of deferring it to TECH so implementation and acceptance tests have one deterministic contract.

Comment thread specs/GH9842/SPEC.md Outdated
Comment on lines +37 to +40
- B6. Telemetry: emit a single `prompt_suggestion_inserted_as_draft`
event when the user takes the new affordance. No payload beyond
the suggestion category (e.g., "zero_state", "follow_up") —
never the suggestion text or the user's edited result.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 [SUGGESTION] Define how suggestion category maps to existing PromptSuggestion fields so implementers do not invent inconsistent categories or use text-derived values.

Co-Authored-By: Warp <agent@warp.dev>
@lonexreb
Copy link
Copy Markdown
Contributor Author

lonexreb commented May 7, 2026

/oz-review

@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 7, 2026

@lonexreb

I'm re-reviewing this spec-only pull request in response to a review request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview

This spec defines an insert-as-draft affordance for prompt suggestions, with behavior, telemetry, acceptance criteria, implementation pointers, and tests.

Concerns

  • The spec conflicts on whether sequential suggestion composition is supported or out of scope, and does not say whether the suggestion banner is dismissed after insert-as-draft.
  • A couple of UI details should be clarified so implementers preserve behavior rather than impossible pixel parity and expose the icon affordance accessibly.

Verdict

Found: 0 critical, 1 important, 2 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Comment thread specs/GH9842/SPEC.md Outdated

## Out of scope

- Multi-suggestion compose (insert two suggestions in sequence
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [IMPORTANT] This conflicts with B2: preserving existing draft text and inserting at the caret permits sequential inserts unless insert-as-draft dismisses the suggestion; specify whether the banner is cleared after insertion or repeated inserts are allowed.

Comment thread specs/GH9842/SPEC.md Outdated

- B1. Each prompt-suggestion entry exposes two affordances:
- **Primary click (existing):** sends the suggestion as-is.
Pixel-equivalent to today.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 [SUGGESTION] Pixel-equivalent conflicts with adding an adjacent edit icon; scope this to send-on-click behavior instead.

Suggested change
Pixel-equivalent to today.
Behavior-equivalent to today.

Comment thread specs/GH9842/SPEC.md Outdated
Pixel-equivalent to today.
- **New: insert-as-draft.** Both entry points are required and
dispatch the same action:
- A small edit icon next to the suggestion.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 [SUGGESTION] Specify the edit icon's accessible label, tooltip, and focus/click target so the non-send path is discoverable beyond modifier-click users.

Round-1 fixes for oz-for-oss bot review:
- Affordance contract now explicitly requires BOTH edit icon AND
  Alt/Ctrl-click insert path with separate AC items
- Define V1 existing-draft behavior as replace-with-confirm; choice
  is not persisted across inserts
- Telemetry uses existing prompt_suggestion.action event with new
  action_type=insert_as_draft alongside send; no new fields, reuses
  existing category and source
- Sequential composition is SUPPORTED in V1 with banner persistence
  until send/close/context-loss
- Add UI/accessibility details: 16x16 pencil glyph, tabindex roving
  focus, aria-label, modifier-click tooltip; no pixel-parity claim
@lonexreb
Copy link
Copy Markdown
Contributor Author

lonexreb commented May 8, 2026

Round-1 fixes for oz-for-oss review: (1) affordance contract now explicitly requires BOTH edit icon AND Alt/Ctrl-click insert path with separate AC items; (2) defined V1 existing-draft behavior as replace-with-confirm (no persistence of choice); (3) telemetry uses existing prompt_suggestion.action event with new action_type="insert_as_draft" — no new fields; (4) sequential composition is SUPPORTED with banner persistence until send/close; (5) added UI/accessibility details: 16×16 pencil glyph, tabindex roving focus, aria-label, modifier-click tooltip; pixel parity not required.

@lonexreb
Copy link
Copy Markdown
Contributor Author

lonexreb commented May 8, 2026

/oz-review

@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 8, 2026

@lonexreb

I'm re-reviewing this spec-only pull request in response to a review request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview

This spec defines an insert-as-draft path for prompt suggestions while preserving existing send-on-click behavior.

Concerns

  • The telemetry contract references an existing prompt_suggestion.action event with category, source, and action_type, but the current client code appears to expose separate accepted events without that payload shape.
  • The implementation pointer says to target an insert-at-caret path, which conflicts with the V1 replace-with-confirm behavior for existing drafts.

Verdict

Found: 0 critical, 2 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Comment thread specs/GH9842/SPEC.md Outdated
- B5. The existing inline-banner prompt-suggestion model
(`app/src/terminal/view/inline_banner/prompt_suggestions.rs`)
is the data source; no schema change.
- B6. Telemetry. Insert-as-draft reuses the existing
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [IMPORTANT] The current client telemetry appears to use PromptSuggestionAccepted / StaticPromptSuggestionAccepted with id, view, and interaction_source, not an existing prompt_suggestion.action event with category, source, and action_type; clarify whether implementation should add a new unified event or extend the existing events so the telemetry contract is implementable.

Comment thread specs/GH9842/SPEC.md Outdated
- `TerminalAction::ResolvePromptSuggestion(...)` (search:
`app/src/terminal/view/init.rs`) is today's "send-it-now" path.
Add a sibling `InsertPromptSuggestionAsDraft(PromptSuggestion)`
action that targets the agent input editor's insert-at-caret
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [IMPORTANT] insert-at-caret conflicts with B2/A5, which require replacing the whole existing draft after confirmation; update the implementation pointer to target a replace-input-and-place-caret-at-end path so implementers do not ship caret insertion for V1.

@lonexreb
Copy link
Copy Markdown
Contributor Author

lonexreb commented May 8, 2026

Round-2 fixes: (1) telemetry — verified actual event names in codebase (PromptSuggestion::Sent and similar event-per-action pattern); replaced fictional unified prompt_suggestion.action with new V1 event PromptSuggestion::InsertedAsDraft reusing existing payload shape; (2) implementation pointer aligned with V1 replace-with-confirm — removed conflicting 'insert-at-caret' wording; V1 sets input value entirely on insert (or via Replace confirm if non-empty); insert-at-caret deferred to V1.5.

@lonexreb
Copy link
Copy Markdown
Contributor Author

lonexreb commented May 8, 2026

/oz-review

@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 8, 2026

@lonexreb

I'm re-reviewing this spec-only pull request in response to a review request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overview

This spec defines an insert-as-draft path for prompt suggestions, including edit-icon and modifier-click affordances, replace-confirm behavior for existing drafts, telemetry, and test coverage.

Concerns

  • The accessibility contract uses DOM-specific tabindex/aria-label requirements for a native Warp UI surface; it needs to name the equivalent focus/accessibility behavior and validation target before implementation.

Verdict

Found: 0 critical, 1 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Comment thread specs/GH9842/SPEC.md Outdated
Comment on lines +93 to +95
- Keyboard-focusable; participates in the suggestion list's
existing roving-focus / `tabindex` model.
- `aria-label="Edit suggestion before sending"`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [IMPORTANT] tabindex and aria-label are DOM-specific, but this prompt-suggestion surface is implemented in native Warp UI; specify the equivalent native focus/accessibility API and test assertion instead of requiring nonexistent attributes.

@lonexreb
Copy link
Copy Markdown
Contributor Author

lonexreb commented May 8, 2026

Round-3 review fixes (commit 9a2a04b):

  • Native a11y contract (B7 + new 'Accessibility contract — native Warp UI' subsection): removed the browser-DOM-only tabindex / aria-label references. Warp UI is GPUI-rendered (no DOM). The spec now names the equivalent native primitives, verified against the codebase:
    • Label: accessibility_label() trait method already used by Warp search items and pickers (app/src/search/data_source.rs, app/src/terminal/input/{prompts,profiles,slash_commands,...}/search_item.rs::accessibility_label). Required value: literal "Edit suggestion before sending".
    • Focus / tab order: Warp's native FocusHandle / PaneFocusHandle system; the icon registers a focus handle and is woven into the suggestion list's existing roving-focus order suggestion-card → edit-icon → next-card. No tabindex is set (Warp has no DOM).
    • Activation: Enter via existing KeyboardAction::Confirm, Space via the same handler.
  • Validation target named explicitly: native AT bridge (macOS Accessibility Inspector + VoiceOver, Windows Inspect / Accessibility Insights for UIA Name, Linux Accerciser / dogtail for AT-SPI accessible-name). Automated coverage uses Warp's existing accessibility test harness (app/src/search/search_bar.rs, app/src/search/command_search/searcher_test.rs::accessibility_label). Browser-DOM tools (Lighthouse, axe-core, Cypress a11y) are explicitly disqualified.
  • A2 and T3 updated to drop DOM language and assert against the native primitives.

@lonexreb
Copy link
Copy Markdown
Contributor Author

lonexreb commented May 8, 2026

/oz-review

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

Labels

cla-signed external-contributor Indicates that a PR has been opened by someone outside the Warp team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant