From ccb08ab6ecfc63859d4f3bf0e2ab1e299c4eb20f Mon Sep 17 00:00:00 2001 From: Ian Gulliver Date: Wed, 21 Oct 2026 16:05:26 -0700 Subject: [PATCH] Let the human comment on each decision in the ready-for-merge card --- internal/agents/ready.go | 2 +- mac/Sources/Byre/Attention.swift | 9 +- mac/Sources/Byre/Chat.swift | 107 ++++++++++++++++++++--- mac/Sources/Byre/Markdown.swift | 48 +++++++++- mac/Sources/Byre/MarkdownView.swift | 10 ++- mac/Sources/Byre/Questions.swift | 29 +----- mac/Sources/Byre/TranscriptBuilder.swift | 6 +- 7 files changed, 166 insertions(+), 45 deletions(-) diff --git a/internal/agents/ready.go b/internal/agents/ready.go index 7189c38..15c97d2 100644 --- a/internal/agents/ready.go +++ b/internal/agents/ready.go @@ -50,7 +50,7 @@ func (m *Manager) readyForMergeTool(l *live) mcp.Tool { InputSchema: json.RawMessage(`{ "type": "object", "properties": { - "overview": {"type": "string", "description": "Markdown, with sections for the summary, decisions, not done, and future TODOs; leave out a section with nothing in it."}, + "overview": {"type": "string", "description": "Markdown, with sections for the summary, decisions, not done, and future TODOs; leave out a section with nothing in it. Write each decision as its own bullet starting with (?), e.g. '- (?) **Render on the server** rather than on a client: …'; byre gives a (?) bullet a Discuss link, so the human can comment on it. Use (?) on any other bullet the human may want to question."}, "merge": {"type": "string", "enum": ["complete", "continue"], "description": "The button byre highlights: complete (the default) lands the work and closes the task; continue lands the work so far and keeps the task open, for when you or the human mean to do more on it."} }, "required": ["overview"] diff --git a/mac/Sources/Byre/Attention.swift b/mac/Sources/Byre/Attention.swift index 11d9b37..379def1 100644 --- a/mac/Sources/Byre/Attention.swift +++ b/mac/Sources/Byre/Attention.swift @@ -61,7 +61,7 @@ struct AttentionReply: View { } } -private struct OtherEntry: View { +struct OtherEntry: View { let key: String let placeholder: String let busy: Bool @@ -82,6 +82,13 @@ private struct OtherEntry: View { focus: $editing, border: editing ? Palette.accent : Palette.border, send: send) .disabled(busy) } + .onAppear { + if trimmed(store.drafts?.textAnswer(key) ?? "").isEmpty && !store.images.has(key) { + DispatchQueue.main.async { + editing = true + } + } + } .onChange(of: editing) { if !editing { if trimmed(text).isEmpty && !store.images.has(key) && !busy { diff --git a/mac/Sources/Byre/Chat.swift b/mac/Sources/Byre/Chat.swift index d74d031..c7d9b11 100644 --- a/mac/Sources/Byre/Chat.swift +++ b/mac/Sources/Byre/Chat.swift @@ -1396,12 +1396,25 @@ private struct ReadyBubble: View { var body: some View { let task = taskId.flatMap { store.findTask($0) } let current = latest && task?.status == .active - AttentionCard(open: current) { + var ctx = MDContext(taskId: taskId) + if current, let task { + let busy = dismissing + let focus = $focus + let base = ctx + func discuss(_ item: MDItem) -> AnyView { + var inner = base + inner.discuss = discuss + return AnyView(DiscussItem(item: item, key: discussKey(task, item.index), busy: busy, focus: focus, ctx: inner, send: { send(task) })) + } + ctx.discuss = discuss + } + return AttentionCard(open: current) { VStack(alignment: .leading, spacing: 4) { AttentionHeader(label: discard ? "suggests discarding" : "ready for merge", when: when, open: current) - MarkdownView(blocks: blocks, ctx: MDContext(taskId: taskId)) + MarkdownView(blocks: blocks, ctx: ctx) if current, let task { let busy = store.taskBusy(task.id) + let commented = comments(task).count AttentionReply(key: draftKey(task), placeholder: "Something else to tell the agent; sending it dismisses this", busy: dismissing, focus: $focus, send: { send(task) }) { Button("Dismiss") { dismiss(task, then: nil) @@ -1410,7 +1423,13 @@ private struct ReadyBubble: View { .help(discard ? "Drop this suggestion; the task stays open" : "Drop this ready call; the task stays open") DiscardButton(task: task, busy: busy) } trailing: { - if !discard { + if commented > 0 { + Button(commented == 1 ? "Send 1 comment" : "Send \(commented) comments") { + send(task) + } + .buttonStyle(WebButtonStyle(primary: true)) + .help("Send your comments to the agent in one message; sending dismisses this") + } else if !discard { MergeButton(task: task, busy: busy, unmerged: unmerged, keepOpen: true, primary: task.readyMerge == ._continue) MergeButton(task: task, busy: busy, unmerged: unmerged, keepOpen: false, primary: task.readyMerge != ._continue) } @@ -1425,10 +1444,40 @@ private struct ReadyBubble: View { "ready:\(task.id):\(when.timeIntervalSince1970)" } + private func discussKey(_ task: ByreTask, _ i: Int) -> String { + "\(draftKey(task)):discuss:\(i)" + } + + private func keys(_ task: ByreTask) -> [String] { + discussItems(blocks).map { discussKey(task, $0.index) } + [draftKey(task)] + } + + private func comments(_ task: ByreTask) -> [(MDItem, String)] { + discussItems(blocks).compactMap { item in + let key = discussKey(task, item.index) + let text = trimmed(store.drafts?.textAnswer(key) ?? "") + if text.isEmpty && !store.images.has(key) { + return nil + } + return (item, text) + } + } + + private func message(_ task: ByreTask) -> String { + let other = trimmed(store.drafts?.textAnswer(draftKey(task)) ?? "") + let parts = comments(task).map { item, text in + item.quote.components(separatedBy: "\n").map { "> \($0)" }.joined(separator: "\n") + (text.isEmpty ? "" : "\n\n\(text)") + } + if parts.isEmpty { + return other + } + let intro = discard ? "Comments on your suggestion to discard:" : "Comments on your ready-for-merge overview:" + return ([intro] + parts + (other.isEmpty ? [] : [other])).joined(separator: "\n\n") + } + private func send(_ task: ByreTask) { - let key = draftKey(task) - let text = store.drafts?.textAnswer(key) ?? "" - if trimmed(text).isEmpty && !store.images.has(key) { + let text = message(task) + if text.isEmpty && !keys(task).contains(where: { store.images.has($0) }) { return } dismiss(task, then: text) @@ -1436,19 +1485,23 @@ private struct ReadyBubble: View { private func dismiss(_ task: ByreTask, then text: String?) { dismissing = true - let key = draftKey(task) - let imgs = store.images.take(key) + let keys = keys(task) + let imgs = keys.map { store.images.take($0) } Task { do { - let ids = text == nil ? nil : try await uploadImages(store.api, imgs) + let ids = text == nil ? nil : try await uploadImages(store.api, imgs.flatMap { $0 }) _ = try await store.api.client.dismissReady(path: .init(taskId: task.id)).ok if let text { try await store.messageTask(task.id, text, images: ids) } - store.drafts?.dropTextAnswer(key) - store.paneFocus.setDraft(key, false) + for key in keys { + store.drafts?.dropTextAnswer(key) + store.paneFocus.setDraft(key, false) + } } catch { - store.images.restore(key, imgs) + for (key, imgs) in zip(keys, imgs) { + store.images.restore(key, imgs) + } overlays.showAlert(errorText(error)) } dismissing = false @@ -1456,6 +1509,36 @@ private struct ReadyBubble: View { } } +private struct DiscussItem: View { + let item: MDItem + let key: String + let busy: Bool + var focus: FocusState.Binding + let ctx: MDContext + let send: () -> Void + @Environment(Store.self) private var store + + var body: some View { + let open = otherOpen(store, key) + VStack(alignment: .leading, spacing: 6) { + HStack(alignment: .firstTextBaseline, spacing: 8) { + ItemBody(item: item, ctx: ctx) + if !open { + Button("Discuss") { + store.drafts?.setTextAnswer(key, "") + } + .buttonStyle(SmallLinkButtonStyle()) + .help("Comment on this; your comments go to the agent together") + .disabled(busy) + } + } + if open { + OtherEntry(key: key, placeholder: "Your comment on this", busy: busy, focus: focus, send: send) + } + } + } +} + private struct ToolResult: View { let result: String? let unfinished: Bool diff --git a/mac/Sources/Byre/Markdown.swift b/mac/Sources/Byre/Markdown.swift index 267f2d5..5f97c64 100644 --- a/mac/Sources/Byre/Markdown.swift +++ b/mac/Sources/Byre/Markdown.swift @@ -27,8 +27,10 @@ private let leadToken = Re(#"^\s*(\S*)([\s\S]*)$"#) struct MDItem: Sendable { var choice: String? - var choiceIndex = 0 + var discuss = false + var index = 0 var raw = "" + var quote = "" var inline: [MDInline] = [] var blocks: [MDBlock] = [] } @@ -74,6 +76,7 @@ struct Re: Sendable { } private let choiceMarker = Re(#"^(\([ *]\)|\[[ xX]\])\s+(\S.*?)\s*$"#) +private let discussMarker = Re(#"^\(\?\)\s+(.*)$"#) private let detailsOpen = Re(#"^\s*
\s*(?:(.*?))?\s*$"#, caseless: true) private let summaryLine = Re(#"^\s*(.*?)\s*$"#, caseless: true) private let detailsClose = Re(#"^\s*
\s*$"#, caseless: true) @@ -92,6 +95,42 @@ func parseMarkdown(_ src: String, choices: Bool = false) -> [MDBlock] { return blocks(text.components(separatedBy: "\n"), choices) } +func numberMarked(_ blocks: [MDBlock], _ next: inout Int) -> [MDBlock] { + blocks.map { block in + switch block { + case .list(let ordered, let start, let items, let grid): + return .list(ordered: ordered, start: start, items: items.map { item in + var out = item + if item.choice != nil || item.discuss { + out.index = next + next += 1 + } + out.blocks = numberMarked(item.blocks, &next) + return out + }, grid: grid) + case .quote(let inner): + return .quote(numberMarked(inner, &next)) + case .details(let summary, let body): + return .details(summary: summary, body: numberMarked(body, &next)) + default: + return block + } + } +} + +func discussItems(_ blocks: [MDBlock]) -> [MDItem] { + blocks.flatMap { block -> [MDItem] in + switch block { + case .list(_, _, let items, _): + items.flatMap { ($0.discuss ? [$0] : []) + discussItems($0.blocks) } + case .quote(let inner), .details(_, let inner): + discussItems(inner) + default: + [] + } + } +} + func parseInline(_ src: String) -> [MDInline] { inline(src) } @@ -282,10 +321,15 @@ private func parseList(_ lines: [String], _ start: Int, _ choices: Bool) -> (MDB return (.list(ordered: isOrdered, start: startNumber, items: items, grid: imageChoices), i) } -private func item(_ body: [String], _ choices: Bool) -> MDItem { +private func item(_ lines: [String], _ choices: Bool) -> MDItem { + var body = lines var out = MDItem(raw: body[0]) if choices, let m = choiceMarker.exec(body[0]) { out.choice = m[2] + } else if let m = discussMarker.exec(body[0]) { + body[0] = m[1]! + out.discuss = true + out.quote = trimmed(body.joined(separator: "\n")) } let first = out.choice != nil ? [] : [body[0]] let rest = Array(body.dropFirst()) diff --git a/mac/Sources/Byre/MarkdownView.swift b/mac/Sources/Byre/MarkdownView.swift index 9574efe..4830c6a 100644 --- a/mac/Sources/Byre/MarkdownView.swift +++ b/mac/Sources/Byre/MarkdownView.swift @@ -412,6 +412,7 @@ struct MDContext { var color: Color = Palette.text var taskId: String? var choice: ((MDItem, Bool) -> AnyView)? + var discuss: ((MDItem) -> AnyView)? var gridColumns = 4 var weight: CGFloat = 400 var dir = "" @@ -470,6 +471,9 @@ struct MarkdownView: View { if ctx.choice != nil && items.contains(where: { $0.choice != nil }) { return false } + if ctx.discuss != nil && items.contains(where: { $0.discuss }) { + return false + } return items.allSatisfy { !hasImage($0.inline) && $0.blocks.allSatisfy(flows) } default: return false @@ -846,7 +850,11 @@ private struct ListBlock: View { Text(verbatim: ordered ? "\(start + n)." : "•") .font(Fonts.font(ctx.size)) .foregroundStyle(ctx.color) - ItemBody(item: item, ctx: ctx) + if item.discuss, let discuss = ctx.discuss { + discuss(item) + } else { + ItemBody(item: item, ctx: ctx) + } } } } diff --git a/mac/Sources/Byre/Questions.swift b/mac/Sources/Byre/Questions.swift index fa4d608..78e5bab 100644 --- a/mac/Sources/Byre/Questions.swift +++ b/mac/Sources/Byre/Questions.swift @@ -8,36 +8,13 @@ typealias InboxResponseCreate = Components.Schemas.InboxResponseCreate private let delegateText = "Use your judgment for this decision and continue within the existing task instructions." private let httpURL = Re(#"^https?://"#, caseless: true) -private func numberChoices(_ blocks: [MDBlock], _ next: inout Int) -> [MDBlock] { - blocks.map { block in - switch block { - case .list(let ordered, let start, let items, let grid): - return .list(ordered: ordered, start: start, items: items.map { item in - var out = item - if item.choice != nil { - out.choiceIndex = next - next += 1 - } - out.blocks = numberChoices(item.blocks, &next) - return out - }, grid: grid) - case .quote(let inner): - return .quote(numberChoices(inner, &next)) - case .details(let summary, let body): - return .details(summary: summary, body: numberChoices(body, &next)) - default: - return block - } - } -} - private func firstChoices(_ blocks: [MDBlock], _ out: inout [String: Int]) { for block in blocks { switch block { case .list(_, _, let items, _): for item in items { if let c = item.choice, out[c] == nil { - out[c] = item.choiceIndex + out[c] = item.index } firstChoices(item.blocks, &out) } @@ -68,7 +45,7 @@ private final class CardModel { self.store = store self.overlays = overlays var next = 0 - context = numberChoices(parseMarkdown(jsString(item.payload.value["context"] ?? ""), choices: true), &next) + context = numberMarked(parseMarkdown(jsString(item.payload.value["context"] ?? ""), choices: true), &next) var first: [String: Int] = [:] firstChoices(context, &first) firstChoice = first @@ -340,7 +317,7 @@ private struct ChoiceBox: View { var body: some View { let text = item.choice ?? "" - if model.options.contains(text) && model.firstChoice[text] == item.choiceIndex { + if model.options.contains(text) && model.firstChoice[text] == item.index { box(text) } else { HStack(alignment: .firstTextBaseline, spacing: 6) { diff --git a/mac/Sources/Byre/TranscriptBuilder.swift b/mac/Sources/Byre/TranscriptBuilder.swift index 6610a4b..96d7ef3 100644 --- a/mac/Sources/Byre/TranscriptBuilder.swift +++ b/mac/Sources/Byre/TranscriptBuilder.swift @@ -972,10 +972,12 @@ final class TranscriptBuilder { return note([.text("\(ev.actor) set the task summary")]) case "task.ready": - return Row(.ready(when: when, md: parseMarkdown(jsString(p["overview"])), discard: false), taskId: rowTask) + var next = 0 + return Row(.ready(when: when, md: numberMarked(parseMarkdown(jsString(p["overview"])), &next), discard: false), taskId: rowTask) case "task.discard_suggested": - return Row(.ready(when: when, md: parseMarkdown(jsString(p["reason"])), discard: true), taskId: rowTask) + var next = 0 + return Row(.ready(when: when, md: numberMarked(parseMarkdown(jsString(p["reason"])), &next), discard: true), taskId: rowTask) case "task.discard_started": return note([.text("\(ev.actor) asked to discard the task; the agent writes the task summary first")])