diff --git a/ArtifactoryKit/Sources/Artifactory.swift b/ArtifactoryKit/Sources/Artifactory.swift index 1a8eccac..8a0a718c 100644 --- a/ArtifactoryKit/Sources/Artifactory.swift +++ b/ArtifactoryKit/Sources/Artifactory.swift @@ -55,6 +55,26 @@ public struct ArtifactoryPost: Codable, Equatable, Identifiable, Sendable { /// own quota, where everything on the board was a note. public let kind: Kind + /// Whether a loop *wrote* this, or the board is only noting that a delivery happened. + /// + /// A separate axis from `kind` on purpose, and the two were conflated once. `kind` + /// answers "whose budget prunes this" — mirrored traffic must have its own quota or a + /// talkative graph evicts every note. It was also read as "is there anything in here + /// to read", which it never was: `node send` mirrors as a `.record` because that is + /// the budget it belongs to, and the whole text a loop typed rode along inside it. + /// Every surface then folded it away, so two loops correcting each other's diagnosis + /// held the conversation somewhere no supervisor ever looked (#273). + /// + /// A hand-off nudge carrying no payload really is a receipt. The text of a `node send` + /// is not, and this is the bit that says so. + /// + /// Absent from boards saved before the split, where a record was only ever a receipt — + /// which is also why it rides beside `kind` rather than becoming a third case of it: + /// an older build decoding a newer board must not meet a raw value it has never heard + /// of, and losing a project's whole graph to a rolled-back beta is a steep price for a + /// tidier enum. + public let wasWritten: Bool + /// Cached formatter for CLI rendering — one `DateFormatter` per process rather than /// per post, and a fixed `dateFormat` with a pinned locale rather than /// `Date.formatted` or named `DateFormatter.Style` cases, neither of which is @@ -66,9 +86,11 @@ public struct ArtifactoryPost: Codable, Equatable, Identifiable, Sendable { return formatter }() + /// `wasWritten` defaults to what the kind implies: a note is always somebody speaking, + /// and a record is a receipt unless its caller says otherwise. public init( id: Int, at: Date, authorID: UUID?, author: String, topic: String?, body: String, - kind: Kind = .note + kind: Kind = .note, wasWritten: Bool? = nil ) { self.id = id self.at = at @@ -77,10 +99,11 @@ public struct ArtifactoryPost: Codable, Equatable, Identifiable, Sendable { self.topic = topic self.body = body self.kind = kind + self.wasWritten = wasWritten ?? (kind == .note) } private enum CodingKeys: String, CodingKey { - case id, at, authorID, author, topic, body, kind + case id, at, authorID, author, topic, body, kind, wasWritten } /// Hand-written for the reason `LoopNode`'s is: a board saved before `kind` existed @@ -93,8 +116,12 @@ public struct ArtifactoryPost: Codable, Equatable, Identifiable, Sendable { authorID = try container.decodeIfPresent(UUID.self, forKey: .authorID) author = try container.decode(String.self, forKey: .author) topic = try container.decodeIfPresent(String.self, forKey: .topic) - body = try container.decode(String.self, forKey: .body) kind = try container.decodeIfPresent(Kind.self, forKey: .kind) ?? .note + let body = try container.decode(String.self, forKey: .body) + self.body = body + wasWritten = + try container.decodeIfPresent(Bool.self, forKey: .wasWritten) + ?? (kind == .note || !Artifactory.readsAsADeliveryReceipt(body)) } /// The same post with its author's handle gone — what deleting a loop leaves behind. @@ -106,7 +133,7 @@ public struct ArtifactoryPost: Codable, Equatable, Identifiable, Sendable { public func withAuthorDeleted() -> ArtifactoryPost { ArtifactoryPost( id: id, at: at, authorID: nil, author: "\(author) (deleted)", topic: topic, - body: body, kind: kind) + body: body, kind: kind, wasWritten: wasWritten) } /// The bound that keeps "check the board" cheap. A note that cannot fit in a @@ -135,11 +162,57 @@ public enum Artifactory { /// loops that cannot read that log. public static let maxNotes = 200 - /// How many mirrored records a board keeps, pruned entirely separately from the - /// notes. Smaller because a record is a receipt for something already delivered: - /// enough that a loop joining mid-flight can see what was recently said, not so - /// many that the graph's chatter becomes the board. - public static let maxRecords = 50 + /// How many mirrored *messages* a board keeps — a `node send`, or an edge that fired + /// carrying a payload — pruned entirely separately from the notes. Smaller because a + /// message was already delivered elsewhere: enough that a loop joining mid-flight can + /// see what was recently said, not so many that the graph's chatter becomes the board. + public static let maxMessages = 50 + + /// How many delivery receipts a board keeps, on their own budget beneath the messages. + /// + /// Sharing one quota was survivable while both halves were hidden. Once the written + /// half takes rows, it is not: a `.none`-transform edge on a cycle mirrors a fresh + /// "@X: Y finished." every pass (`reenterCycle` resets `fireCount`), so fifty passes + /// would evict the conversation this section exists to show and leave "50 delivery + /// receipts" in its place. Bookkeeping cannot be allowed to price out the thing it is + /// bookkeeping for, which is the same argument that split notes from records to begin + /// with, one level down. + /// + /// Small on purpose: a receipt says only that an edge fired, and the rollup shows + /// eight before it offers the rest. + public static let maxReceipts = 20 + + /// What the mirror appends to a source loop's title when an edge fires carrying + /// nothing — the whole of such a post's body, and the reason it is a receipt. + /// + /// Here rather than at the two places in `GraphcodeKit` that write it, because + /// `readsAsADeliveryReceipt` has to recognise the same words. Duplicated across the + /// module boundary they would drift silently: reword either one and every legacy + /// receipt reclassifies as written, with no compile error and no failing test. + public static let firedWithNothingToSay = " finished." + + /// The same, for a hand-off that carried no payload. A hand-off *with* one appends it + /// after this, which is why the test that uses it is a suffix test. + public static let handedOffWithNothingToSay = " finished and handed its work off to you." + + /// Whether a record saved before `wasWritten` existed is one of the two lines the + /// daemon generates itself, rather than something a loop said. + /// + /// Boards written before the split carry no flag, and a default of "receipt" would + /// leave every conversation already on them exactly as buried as #273 found it — a + /// fix that only helps graphs created after it shipped. There is no other signal left + /// on those posts, so this reads the two shapes the mirror produces when an edge fires + /// with nothing in it (`MessageBus.messageText`'s `.none`, and a hand-off with no + /// payload). Everything else on those topics is a `node send` or a payload, which is + /// somebody talking. + /// + /// Deliberately narrow. A written message that happens to end "… finished." is read as + /// a receipt and stays folded, which is where it already was; the opposite mistake + /// would put the daemon's own bookkeeping in front of a reader as though a loop had + /// said it. Only posts decoded without the flag are ever asked. + public static func readsAsADeliveryReceipt(_ body: String) -> Bool { + body.hasSuffix(firedWithNothingToSay) || body.hasSuffix(handedOffWithNothingToSay) + } /// The id the next post gets. Maximum-plus-one, never count-plus-one: pruning /// removes the oldest posts, and reusing their ids would make unread cursors @@ -174,14 +247,19 @@ public enum Artifactory { return posts.filter { $0.id > lastRead } } - /// A board pruned to both budgets, oldest of each kind gone first and the survivors - /// back in one sequence. Applied by the store on every write so no caller can forget. + /// A board pruned to all three budgets, oldest of each pool gone first and the + /// survivors back in one sequence. Applied by the store on every write so no caller + /// can forget. public static func pruned(_ posts: [ArtifactoryPost]) -> [ArtifactoryPost] { let notes = posts.filter { $0.kind == .note } - let records = posts.filter { $0.kind == .record } - guard notes.count > maxNotes || records.count > maxRecords else { return posts } + let messages = posts.filter { $0.kind == .record && $0.wasWritten } + let receipts = posts.filter { $0.kind == .record && !$0.wasWritten } + guard + notes.count > maxNotes || messages.count > maxMessages || receipts.count > maxReceipts + else { return posts } let kept = Set( - (notes.suffix(maxNotes) + records.suffix(maxRecords)).map(\.id)) + (notes.suffix(maxNotes) + messages.suffix(maxMessages) + receipts.suffix(maxReceipts)) + .map(\.id)) return posts.filter { kept.contains($0.id) } } } diff --git a/GraphcodeKit/Sources/Domain/LoopGraph.swift b/GraphcodeKit/Sources/Domain/LoopGraph.swift index 0a5ed908..e4dabbcd 100644 --- a/GraphcodeKit/Sources/Domain/LoopGraph.swift +++ b/GraphcodeKit/Sources/Domain/LoopGraph.swift @@ -22,7 +22,7 @@ public struct LoopGraph: Identifiable, Codable, Equatable, Sendable { public var edges: IdentifiedArrayOf /// The project's Artifactory — every post any loop has dropped onto the shared board, /// oldest first, notes and mirrored records each capped on their own budget - /// (`Artifactory.maxNotes`, `Artifactory.maxRecords`). Kept on the graph rather than in a + /// (`Artifactory.maxNotes`, `maxMessages`, `maxReceipts`). Kept on the graph rather than in a /// side store so it inherits for free everything graph state already has: one /// writer (the daemon), atomic persistence beside the graph file, a snapshot in /// every `.graphChanged` (which is how the CLI reads it — no second read path), and diff --git a/GraphcodeKit/Sources/GraphStore.swift b/GraphcodeKit/Sources/GraphStore.swift index 565cb015..04d3c7b0 100644 --- a/GraphcodeKit/Sources/GraphStore.swift +++ b/GraphcodeKit/Sources/GraphStore.swift @@ -1660,15 +1660,21 @@ public actor GraphStore { } /// Writes a shared communication onto the artifactory — the durable record the - /// board keeps of everything the graph's loops said to each other. Record-only by + /// board keeps of everything the graph's loops said to each other. Silent by /// design: the communication already reached its target (or is waiting in staged /// memory to), so mirroring must not ring the watchers, or a busy graph would have /// every direct message waking every listener on top of its real delivery. /// Gated like every board write; body carries the target so a reader can tell a - /// note to the room from a note to a peer. Written as `.record`, which is what keeps + /// note to the room from a note to a peer. Always a `.record`, which is what keeps /// a talkative graph inside its own budget instead of evicting the notes. + /// + /// `wasWritten` is the other half, and the caller is the only one who knows it: the + /// text of a `node send` is a loop talking, while "Author finished." is the board + /// noticing that an edge fired. Both belong in the record's budget; only the first + /// belongs in front of a reader. private func recordArtifactoryCommunication( - from senderID: UUID?, to target: LoopNode, text: String, topic: String + from senderID: UUID?, to target: LoopNode, text: String, topic: String, + wasWritten: Bool ) { guard onArtifactoryEnabled?() == true else { return } let sender = senderID.flatMap { graph.nodes[id: $0]?.title } ?? "a human" @@ -1681,7 +1687,7 @@ public actor GraphStore { } let post = ArtifactoryPost( id: Artifactory.nextID(after: graph.artifactory), at: Date(), authorID: senderID, - author: sender, topic: topic, body: body, kind: .record) + author: sender, topic: topic, body: body, kind: .record, wasWritten: wasWritten) graph.artifactory = Artifactory.pruned(graph.artifactory + [post]) } @@ -2359,7 +2365,12 @@ public actor GraphStore { if record.hasPrefix("\(source.title): ") { record.removeFirst("\(source.title): ".count) } - recordArtifactoryCommunication(from: source.id, to: target, text: record, topic: "direct") + recordArtifactoryCommunication( + from: source.id, to: target, text: record, topic: "direct", + // An edge with no transform says only that the upstream finished — the board's + // own observation, not the loop's. A template or a captured script is content + // somebody put there. + wasWritten: edge.payloadTransform != .none) graph.edges[id: edgeID]?.fireCount += 1 } } @@ -2454,7 +2465,7 @@ public actor GraphStore { "Cycle re-entry \(edge.fireCount)\(bound) — the stop condition is not yet met. " + "Continue toward your goal.") } else { - parts.append("\(source.title) finished and handed its work off to you.") + parts.append("\(source.title)\(Artifactory.handedOffWithNothingToSay)") // The handoff itself is shared communication and gets its record — with its // payload, which is the part a later reader actually needs. Cycle re-entries // are the daemon's own metronome, not a loop saying anything, so they stay @@ -2462,7 +2473,10 @@ public actor GraphStore { var record = parts.joined(separator: " ") if let payload { record += " " + payload } recordArtifactoryCommunication( - from: source.id, to: target, text: record, topic: "handoff") + from: source.id, to: target, text: record, topic: "handoff", + // The payload is the part a later reader needs; without one the line is the + // boilerplate above and nothing else, which is a receipt. + wasWritten: payload != nil) } if let payload { parts.append(payload) @@ -2541,7 +2555,7 @@ public actor GraphStore { // that already exists, and recording it would have the board record itself. if mirror { recordArtifactoryCommunication( - from: senderID, to: target, text: trimmed, topic: "direct") + from: senderID, to: target, text: trimmed, topic: "direct", wasWritten: true) } // Attributed when the sender is a loop in this graph, the way a message edge names // its source — the target should know who's talking without guessing. diff --git a/GraphcodeKit/Sources/Sessions/MessageBus.swift b/GraphcodeKit/Sources/Sessions/MessageBus.swift index 281ec967..21675ce7 100644 --- a/GraphcodeKit/Sources/Sessions/MessageBus.swift +++ b/GraphcodeKit/Sources/Sessions/MessageBus.swift @@ -1,3 +1,4 @@ +import ArtifactoryKit import Foundation /// Delivery for `.message`-kind edges — docs/02-graph-of-loops.md#inter-loop-messaging-in-practice @@ -130,7 +131,9 @@ public enum MessageBus { switch edge.payloadTransform { case .none: // Still worth sending: the fact that the upstream finished is itself the message. - return "[graphcode] \(source.title) finished." + // The suffix is `ArtifactoryKit`'s so the board can recognise its own bookkeeping + // on a post saved before it was flagged as such. + return "[graphcode] \(source.title)\(Artifactory.firedWithNothingToSay)" case .template(let text): return text.isEmpty ? nil : "[graphcode] \(source.title): \(text)" case .script(let command): diff --git a/graphcode/Sources/Features/LoopWorkspace/ArtifactorySection.swift b/graphcode/Sources/Features/LoopWorkspace/ArtifactorySection.swift index 9d1032fa..e827bd5d 100644 --- a/graphcode/Sources/Features/LoopWorkspace/ArtifactorySection.swift +++ b/graphcode/Sources/Features/LoopWorkspace/ArtifactorySection.swift @@ -14,25 +14,33 @@ enum ArtifactoryPresentation { enabled && !graph.artifactory.isEmpty } - /// The posts somebody wrote on purpose, newest last — the direction the summary and - /// the terminal beside it already run. - static func notes(in graph: LoopGraph) -> [ArtifactoryPost] { - graph.artifactory.filter { $0.kind == .note } + /// Everything somebody wrote, newest last — the direction the summary and the + /// terminal beside it already run. Notes to the room and the loop-to-loop messages + /// mirrored from `node send` and payload-carrying edges, which are the same act: + /// a loop chose those words for somebody to read. + /// + /// The split used to be `kind`, and `kind` is a budget. Mirrored traffic prunes on + /// its own quota, so every direct message was a `.record` and every `.record` was + /// hidden — which put a root-cause correction between two loops in the one place on + /// the board nobody reads (#273). + static func posts(in graph: LoopGraph) -> [ArtifactoryPost] { + graph.artifactory.filter(\.wasWritten) } - /// The mirrored direct messages and handoffs. Kept apart from the notes because they - /// are receipts for deliveries that already happened, not something written to be - /// read here. - static func records(in graph: LoopGraph) -> [ArtifactoryPost] { - graph.artifactory.filter { $0.kind == .record } + /// The bookkeeping half of the mirrored traffic: a hand-off that carried no payload, + /// an edge whose whole message is that the upstream finished. That two loops spoke is + /// all a reader needs from these, so they roll up rather than take a row each. + static func receipts(in graph: LoopGraph) -> [ArtifactoryPost] { + graph.artifactory.filter { !$0.wasWritten } } - /// How many notes have landed since the human last looked — `seenPostID` is - /// `LoopWorkspaceFeature.seenArtifactoryPostID`, not the loop's sync cursor. Records - /// are excluded: they are folded away by default, and a badge counting mail nobody is - /// being shown is a badge that cannot be cleared. - static func unreadNoteCount(graph: LoopGraph, seenPostID: Int?) -> Int { - Artifactory.unread(in: notes(in: graph), since: seenPostID).count + /// How many written posts have landed since the human last looked — `seenPostID` is + /// `LoopWorkspaceFeature.seenArtifactoryPostID`, not the loop's sync cursor. Receipts + /// are excluded, and the rule is the one that has always governed this badge: it + /// counts what the section shows, because a badge counting mail nobody is being shown + /// is a badge that cannot be cleared. + static func unreadCount(graph: LoopGraph, seenPostID: Int?) -> Int { + Artifactory.unread(in: posts(in: graph), since: seenPostID).count } } @@ -59,32 +67,46 @@ struct ArtifactorySection: View { /// which is exactly what a person talking to the whole graph is. let onPost: (String, String?) -> Void - /// Whether the mirrored records are unfolded. Local and unpersisted, unlike the - /// section's own fold: opening the receipts is a thing you do once to answer a - /// question, not a way you prefer to read the board. /// The most the board's scroll box will ever be, on any window: about ten posts at /// the rail's default width — enough to read a conversation, not so many that the /// rail is nothing but the board. The rail hands down a smaller cap on a short /// window (`LoopWorkspaceRail.artifactoryHeightCap`); this is the ceiling on that. static let maxScrollHeight: CGFloat = 600 - @State private var showsRecords = false + /// Whether the receipts are unfolded — persisted beside the section's own fold. It + /// was local `@State` on the reasoning that opening the receipts is a thing you do + /// once to answer a question; in practice the rollup forgot it had been opened every + /// time you changed loops, which is not a preference the app gets to keep re-asking. + @AppStorage(LoopWorkspaceRail.artifactoryReceiptsShownDefaultsKey) + private var showsReceipts = false + /// Whether the rollup shows every receipt or stops at the newest few. Deliberately + /// not persisted, unlike the line above: this is a drill-down inside something you + /// already opened, and the default it returns to is the short one — which is a claim + /// `collapseReceipts` and the "show fewer" line have to keep true, since nothing else + /// in a workspace's lifetime would. + @State private var showsAllReceipts = false + /// What the rollup scrolls to when it opens. The board is bottom-anchored and the + /// rollup sits at its top, so on any board tall enough to scroll, growing the rollup + /// moves content *above* the viewport: without this, tapping "show all" reveals + /// nothing and moves nothing, which is the same inert affordance the tap was added to + /// remove. + private static let receiptsAnchor = "artifactory-receipts" @State private var isComposing = false @State private var draft = "" @State private var draftTopic = "" @FocusState private var draftFocused: Bool - private var notes: [ArtifactoryPost] { ArtifactoryPresentation.notes(in: graph) } - private var records: [ArtifactoryPost] { ArtifactoryPresentation.records(in: graph) } + private var posts: [ArtifactoryPost] { ArtifactoryPresentation.posts(in: graph) } + private var receipts: [ArtifactoryPost] { ArtifactoryPresentation.receipts(in: graph) } private var unread: Int { - ArtifactoryPresentation.unreadNoteCount(graph: graph, seenPostID: seenPostID) + ArtifactoryPresentation.unreadCount(graph: graph, seenPostID: seenPostID) } - /// The id the unread rule is drawn above — the first note that landed after the human + /// The id the unread rule is drawn above — the first post that landed after the human /// last looked. `nil` when everything is read, which is when nothing should be drawn. private var firstUnreadID: Int? { guard unread > 0 else { return nil } - return notes.suffix(unread).first?.id + return posts.suffix(unread).first?.id } var body: some View { @@ -93,30 +115,32 @@ struct ArtifactorySection: View { if isFolded { foldedLine } else { - ScrollView(.vertical) { - VStack(alignment: .leading, spacing: 11) { - recordsRollup - ForEach(notes) { post in - if post.id == firstUnreadID { sinceYouLooked } - postRow(post) + ScrollViewReader { scroll in + ScrollView(.vertical) { + VStack(alignment: .leading, spacing: 11) { + receiptsRollup(scroll) + ForEach(posts) { post in + if post.id == firstUnreadID { sinceYouLooked } + postRow(post) + } } + .frame(maxWidth: .infinity, alignment: .leading) } - .frame(maxWidth: .infinity, alignment: .leading) + .scrollBounceBehavior(.basedOnSize) + .defaultScrollAnchor(.bottom) + // Hug the posts, and only then scroll. A `ScrollView` is greedy — offered the + // rail's slack it takes it, and `defaultScrollAnchor(.bottom)` then pins the + // posts to the foot of that box with a gap between them and the header. Two + // attempts measured the content through a preference and sized the box to it; + // both mis-sized (a zero start that never laid out, then a stale reading that + // left the gap). This is the idiom that needs no measuring: `fixedSize` + // (vertical) asks the scroll view for its *ideal* height, which is its + // content's, and `frame(maxHeight:)` under it clamps that. The box is exactly + // as tall as the posts until the cap, and scrolls after — no state, nothing to + // go stale, nothing to fire late. + .frame(maxHeight: maxHeight) + .fixedSize(horizontal: false, vertical: true) } - .scrollBounceBehavior(.basedOnSize) - .defaultScrollAnchor(.bottom) - // Hug the posts, and only then scroll. A `ScrollView` is greedy — offered the - // rail's slack it takes it, and `defaultScrollAnchor(.bottom)` then pins the - // posts to the foot of that box with a gap between them and the header. Two - // attempts measured the content through a preference and sized the box to it; - // both mis-sized (a zero start that never laid out, then a stale reading that - // left the gap). This is the idiom that needs no measuring: `fixedSize` - // (vertical) asks the scroll view for its *ideal* height, which is its - // content's, and `frame(maxHeight:)` under it clamps that. The box is exactly - // as tall as the posts until the cap, and scrolls after — no state, nothing to - // go stale, nothing to fire late. - .frame(maxHeight: maxHeight) - .fixedSize(horizontal: false, vertical: true) // Outside the scroll view on purpose. Inside it the composer was one more row // in a list that can be taller than the rail — it could open scrolled out of // sight, and it moved under the pointer as posts arrived. Pinned here it is @@ -139,7 +163,7 @@ struct ArtifactorySection: View { } private var unreadIDs: Set { - Set(notes.suffix(unread).map(\.id)) + Set(posts.suffix(unread).map(\.id)) } // MARK: - Header @@ -173,14 +197,14 @@ struct ArtifactorySection: View { .help(isFolded ? "Show the board" : "Collapse to one line") } - /// Folded keeps the newest note, for the reason the summary's fold keeps its beat: a + /// Folded keeps the newest post, for the reason the summary's fold keeps its beat: a /// folded section that shows nothing is a section you forget exists. private var foldedLine: some View { HStack(spacing: 6) { Circle() - .fill(accent(for: notes.last)) + .fill(accent(for: posts.last)) .frame(width: 6, height: 6) - Text(notes.last?.body ?? "no notes yet") + Text(posts.last?.body ?? "no notes yet") .font(.system(size: 11.5)) .foregroundStyle(.white.opacity(0.75)) .lineLimit(1) @@ -230,53 +254,6 @@ struct ArtifactorySection: View { } } - // MARK: - Records - - /// The mirrored traffic, one line each and never a body: a record says that two loops - /// spoke, which is all a reader of the board needs from it. - @ViewBuilder - private var recordsRollup: some View { - if !records.isEmpty { - VStack(alignment: .leading, spacing: 7) { - HStack(spacing: 6) { - Image(systemName: showsRecords ? "chevron.down" : "chevron.right") - .font(.system(size: 8, weight: .semibold)) - .foregroundStyle(.white.opacity(0.38)) - Text( - records.count == 1 ? "1 message record" : "\(records.count) message records" - ) - .font(.system(size: 10.5)) - .foregroundStyle(.white.opacity(0.5)) - Spacer(minLength: 0) - } - .contentShape(Rectangle()) - .onTapGesture { showsRecords.toggle() } - if showsRecords { - VStack(alignment: .leading, spacing: 7) { - ForEach(records.suffix(8)) { record in - HStack(alignment: .firstTextBaseline, spacing: 6) { - Text(ArtifactoryPost.stampFormat.string(from: record.at)) - .font(.system(size: 10.5, design: .monospaced)) - .foregroundStyle(.white.opacity(0.32)) - Text(record.body) - .font(.system(size: 11)) - .foregroundStyle(.white.opacity(0.5)) - .lineLimit(1) - .truncationMode(.tail) - } - } - if records.count > 8 { - Text("\(records.count - 8) earlier") - .font(.system(size: 10.5)) - .foregroundStyle(.white.opacity(0.4)) - } - } - .padding(.leading, 14) - } - } - } - } - // MARK: - Posts private func postRow(_ post: ArtifactoryPost) -> some View { @@ -329,6 +306,112 @@ struct ArtifactorySection: View { } return author.loopType.accent } +} + +/// Kept out of the struct's own body, which sits over swiftlint's length bound: the +/// receipts rollup and the composer are each a self-contained thing nothing above them +/// needs to see into. +extension ArtifactorySection { + // MARK: - Receipts + + /// How many receipts the rollup shows before it offers the rest — enough to see what + /// the graph has been doing lately without the bookkeeping outgrowing the board it + /// sits above. + private static let receiptsShownAtFirst = 8 + + private var visibleReceipts: [ArtifactoryPost] { + showsAllReceipts ? receipts : Array(receipts.suffix(Self.receiptsShownAtFirst)) + } + + /// The bookkeeping, one line each and never a body: a receipt says that two loops + /// spoke, which is all a reader of the board needs from it. + /// + /// Takes the scroll proxy because every affordance in here grows the rollup upward, + /// against a bottom-anchored box — see `receiptsAnchor`. + @ViewBuilder + private func receiptsRollup(_ scroll: ScrollViewProxy) -> some View { + if !receipts.isEmpty { + VStack(alignment: .leading, spacing: 7) { + HStack(spacing: 6) { + Image(systemName: showsReceipts ? "chevron.down" : "chevron.right") + .font(.system(size: 8, weight: .semibold)) + .foregroundStyle(.white.opacity(0.38)) + Text( + receipts.count == 1 + ? "1 delivery receipt" : "\(receipts.count) delivery receipts" + ) + .font(.system(size: 10.5)) + .foregroundStyle(.white.opacity(0.5)) + Spacer(minLength: 0) + } + .contentShape(Rectangle()) + .onTapGesture { toggleReceipts(scroll) } + if showsReceipts { + VStack(alignment: .leading, spacing: 7) { + ForEach(visibleReceipts) { receipt in + HStack(alignment: .firstTextBaseline, spacing: 6) { + Text(ArtifactoryPost.stampFormat.string(from: receipt.at)) + .font(.system(size: 10.5, design: .monospaced)) + .foregroundStyle(.white.opacity(0.32)) + Text(receipt.body) + .font(.system(size: 11)) + .foregroundStyle(.white.opacity(0.5)) + .lineLimit(1) + .truncationMode(.tail) + } + } + receiptsCutoff(scroll) + } + .padding(.leading, 14) + } + } + .id(Self.receiptsAnchor) + } + } + + /// The older receipts were counted and then left with no way to reach them — a line + /// that names something and does nothing reads as a bug in the panel. Both directions + /// are offered, because an expansion nothing can undo is the same complaint again. + @ViewBuilder + private func receiptsCutoff(_ scroll: ScrollViewProxy) -> some View { + if receipts.count > visibleReceipts.count { + cutoffLine("\(receipts.count - visibleReceipts.count) earlier — show all") { + showsAllReceipts = true + revealRollup(scroll) + } + .help("Show every delivery receipt on this board") + } else if showsAllReceipts, receipts.count > Self.receiptsShownAtFirst { + cutoffLine("show fewer") { + showsAllReceipts = false + revealRollup(scroll) + } + .help("Show only the most recent delivery receipts") + } + } + + private func cutoffLine(_ title: String, action: @escaping () -> Void) -> some View { + Text(title) + .font(.system(size: 10.5)) + .foregroundStyle(.white.opacity(0.4)) + .contentShape(Rectangle()) + .onTapGesture(perform: action) + } + + private func toggleReceipts(_ scroll: ScrollViewProxy) { + showsReceipts.toggle() + // Closing it puts the drill-down back where its doc comment says it starts. + if !showsReceipts { showsAllReceipts = false } + revealRollup(scroll) + } + + /// Bring the rollup back under the eye after it changes height. The board anchors to + /// its bottom and the rollup sits at the top, so without this every one of these taps + /// pushes its own result off the top of the box and looks like it did nothing. + private func revealRollup(_ scroll: ScrollViewProxy) { + withAnimation(.easeOut(duration: 0.18)) { + scroll.scrollTo(Self.receiptsAnchor, anchor: .top) + } + } // MARK: - Composing diff --git a/graphcode/Sources/Features/LoopWorkspace/LoopWorkspaceFeature.swift b/graphcode/Sources/Features/LoopWorkspace/LoopWorkspaceFeature.swift index 6ae3fd9d..f7258ef9 100644 --- a/graphcode/Sources/Features/LoopWorkspace/LoopWorkspaceFeature.swift +++ b/graphcode/Sources/Features/LoopWorkspace/LoopWorkspaceFeature.swift @@ -322,11 +322,12 @@ struct LoopWorkspaceFeature { case .workspaceLeft: state.seenBeatID = state.node.summary?.current?.id - // Only what was actually on screen counts as looked at: a hidden rail or a - // folded section showed no posts, and marking them seen would clear a badge - // the human never had a chance to read. + // Only what was actually on screen counts as looked at. A hidden rail drew + // nothing; a folded section drew its newest post and no other, so advancing + // over the rest would clear the badge rather than let it be read — the badge + // stays, and the one gesture it is asking for is the one that clears it. if state.isRailVisible, !state.isArtifactoryFolded, - let newest = ArtifactoryPresentation.notes(in: state.graph).last?.id + let newest = ArtifactoryPresentation.posts(in: state.graph).last?.id { state.seenArtifactoryPostID = newest LoopWorkspaceRail.saveSeenArtifactoryPost(newest, forProjectPath: state.projectPath) diff --git a/graphcode/Sources/Features/LoopWorkspace/LoopWorkspaceRail.swift b/graphcode/Sources/Features/LoopWorkspace/LoopWorkspaceRail.swift index 8b55b459..89ff1a52 100644 --- a/graphcode/Sources/Features/LoopWorkspace/LoopWorkspaceRail.swift +++ b/graphcode/Sources/Features/LoopWorkspace/LoopWorkspaceRail.swift @@ -104,6 +104,12 @@ struct LoopWorkspaceRail: View { UserDefaults.standard.set(folded, forKey: artifactoryFoldedDefaultsKey) } + /// Whether the board's delivery-receipt rollup is open. Read through `@AppStorage` + /// rather than the reducer, because unlike the section's fold nothing outside the + /// section ever needs to know — but persisted all the same: a rollup that forgets it + /// was opened every time you change loops is one you stop opening. + static let artifactoryReceiptsShownDefaultsKey = "loopArtifactoryReceiptsShown" + static let boardFoldedDefaultsKey = "loopBoardSectionFolded" static func loadBoardFolded() -> Bool { diff --git a/graphcode/Tests/ArtifactoryBudgetTests.swift b/graphcode/Tests/ArtifactoryBudgetTests.swift index e2f6caac..801afce3 100644 --- a/graphcode/Tests/ArtifactoryBudgetTests.swift +++ b/graphcode/Tests/ArtifactoryBudgetTests.swift @@ -44,14 +44,14 @@ struct ArtifactoryBudgetTests { await store.handle( .artifactoryPost(text: "DEAD END: approach X fails", topic: "findings", from: ids[0])) - for index in 0..<(Artifactory.maxRecords * 4) { + for index in 0..<(Artifactory.maxMessages * 4) { await store.handle( .messageNode(ids[1], text: "ping \(index)", from: ids[0], followUp: true)) } let board = await store.graph.artifactory #expect(board.contains { $0.body.contains("DEAD END") }) - #expect(board.filter { $0.kind == .record }.count == Artifactory.maxRecords) + #expect(board.filter { $0.kind == .record }.count == Artifactory.maxMessages) #expect(board.filter { $0.kind == .note }.count == 1) } @@ -78,7 +78,7 @@ struct ArtifactoryBudgetTests { func pruningKeepsTheBoardInOneSequence() { let base = Date() var posts: [ArtifactoryPost] = [] - for index in 1...(Artifactory.maxRecords + 4) { + for index in 1...(Artifactory.maxReceipts + 4) { posts.append( ArtifactoryPost( id: index, at: base, authorID: nil, author: "a human", topic: nil, @@ -90,7 +90,7 @@ struct ArtifactoryBudgetTests { } let pruned = Artifactory.pruned(posts.sorted { $0.id < $1.id }) #expect(pruned == pruned.sorted { $0.id < $1.id }) - #expect(pruned.filter { $0.kind == .record }.count == Artifactory.maxRecords) + #expect(pruned.filter { $0.kind == .record }.count == Artifactory.maxReceipts) } /// A board written before records had a kind decodes as all notes — everything on it diff --git a/graphcode/Tests/ArtifactorySinceYouLookedTests.swift b/graphcode/Tests/ArtifactorySinceYouLookedTests.swift index 428e544a..b2f125b6 100644 --- a/graphcode/Tests/ArtifactorySinceYouLookedTests.swift +++ b/graphcode/Tests/ArtifactorySinceYouLookedTests.swift @@ -47,9 +47,9 @@ struct ArtifactorySinceYouLookedTests { @Test func unreadIsTheHumansNotTheLoops() { let graph = makeGraph(noteIDs: [1, 2, 3], loopCursor: 3) - #expect(ArtifactoryPresentation.unreadNoteCount(graph: graph, seenPostID: nil) == 3) - #expect(ArtifactoryPresentation.unreadNoteCount(graph: graph, seenPostID: 2) == 1) - #expect(ArtifactoryPresentation.unreadNoteCount(graph: graph, seenPostID: 3) == 0) + #expect(ArtifactoryPresentation.unreadCount(graph: graph, seenPostID: nil) == 3) + #expect(ArtifactoryPresentation.unreadCount(graph: graph, seenPostID: 2) == 1) + #expect(ArtifactoryPresentation.unreadCount(graph: graph, seenPostID: 3) == 0) } @Test @@ -87,20 +87,62 @@ struct ArtifactorySinceYouLookedTests { #expect(store.state.seenArtifactoryPostID == nil) } - /// A record (mirrored `node send`) is not a note: it is folded away, so it must not - /// be what "looked" advances to, or a badge could count something never drawn. + /// A delivery receipt is folded away, so it must not be what "looked" advances to, or + /// a badge could count something never drawn. @Test @MainActor - func lookedAdvancesToTheNewestNoteNotTheNewestRecord() async { + func lookedAdvancesToTheNewestPostNotTheNewestReceipt() async { + var graph = makeGraph(noteIDs: [1, 2], loopCursor: nil) + graph.artifactory.append(receipt(id: 3)) + let store = makeStore(graph: graph, railVisible: true) + + await store.send(.workspaceLeft) + + #expect(store.state.seenArtifactoryPostID == 2) + } + + /// The other half of #273: a mirrored `node send` carries the whole text a loop typed, + /// so it is shown, counted, and cleared like any other post. It prunes on the record + /// budget all the same — that is a quota, not a verdict on whether anyone should read + /// it. + @Test + func aWrittenMessageIsShownAndCounted() { var graph = makeGraph(noteIDs: [1, 2], loopCursor: nil) graph.artifactory.append( ArtifactoryPost( - id: 3, at: Date(), authorID: nil, author: "a human", topic: "direct", - body: "@Worker: hi", kind: .record)) + id: 3, at: Date(), authorID: nil, author: "Peer", topic: "direct", + body: "@Worker: truncation is not the mechanism", kind: .record, wasWritten: true)) + graph.artifactory.append(receipt(id: 4)) + + #expect(ArtifactoryPresentation.posts(in: graph).map(\.id) == [1, 2, 3]) + #expect(ArtifactoryPresentation.receipts(in: graph).map(\.id) == [4]) + #expect(ArtifactoryPresentation.unreadCount(graph: graph, seenPostID: 2) == 1) + } + + /// The badge must stay clearable: everything it counts has to be something leaving the + /// workspace marks as looked at. + @Test + @MainActor + func aWrittenMessageClearsTheBadge() async { + var graph = makeGraph(noteIDs: [1, 2], loopCursor: nil) + graph.artifactory.append( + ArtifactoryPost( + id: 3, at: Date(), authorID: nil, author: "Peer", topic: "direct", + body: "@Worker: correction accepted", kind: .record, wasWritten: true)) + graph.artifactory.append(receipt(id: 4)) let store = makeStore(graph: graph, railVisible: true) await store.send(.workspaceLeft) - #expect(store.state.seenArtifactoryPostID == 2) + #expect(store.state.seenArtifactoryPostID == 3) + #expect( + ArtifactoryPresentation.unreadCount( + graph: graph, seenPostID: store.state.seenArtifactoryPostID) == 0) + } + + private func receipt(id: Int) -> ArtifactoryPost { + ArtifactoryPost( + id: id, at: Date(), authorID: nil, author: "a human", topic: "handoff", + body: "@Worker: Peer finished and handed its work off to you.", kind: .record) } } diff --git a/graphcode/Tests/ArtifactoryTests.swift b/graphcode/Tests/ArtifactoryTests.swift index dfe79514..038125e1 100644 --- a/graphcode/Tests/ArtifactoryTests.swift +++ b/graphcode/Tests/ArtifactoryTests.swift @@ -242,6 +242,7 @@ extension ArtifactoryTests { // MARK: - Shared-communication mirroring #expect(record.topic == "direct") #expect(record.author == "Author") #expect(record.body == "@Reader: the API changed under you") + #expect(record.wasWritten) } @Test @@ -277,6 +278,7 @@ extension ArtifactoryTests { // MARK: - Shared-communication mirroring #expect(records.count == 1) #expect(records[0].author == "Author") #expect(records[0].body == "@Reader: specs moved to docs/api.md") + #expect(records[0].wasWritten) } @Test @@ -297,8 +299,171 @@ extension ArtifactoryTests { // MARK: - Shared-communication mirroring #expect( records[0].body == "@Reader: Author finished and handed its work off to you. branch: fix/auth") + #expect(records[0].wasWritten) } +} + +/// #273: which mirrored posts a reader is shown. `kind` is a budget — mirrored traffic +/// prunes on its own quota so a talkative graph cannot evict a note — and it was doing +/// double duty as "is there anything in here to read". A `node send` carries every word +/// its sender typed; an edge that fired with no payload carries none. +extension ArtifactoryTests { // MARK: - Written messages and delivery receipts + @Test + func aPayloadlessEdgeMirrorsAsAReceipt() async { + let delivered = LockIsolated<[(UUID, String)]>([]) + let store = await makeStore(delivered: delivered) + let ids = nodeIDs(await store.graph) + + await store.handle( + .createEdge(from: ids[0], to: ids[1], spec: EdgeSpec(kind: .message))) + await store.handle(.nodeCheckApproved(ids[0])) + + let graph = await store.graph + let records = graph.artifactory.filter { $0.topic == "direct" } + #expect(records.count == 1) + #expect(records[0].body == "@Reader: Author finished.") + #expect(!records[0].wasWritten) + } + + @Test + func aPayloadlessHandoffMirrorsAsAReceipt() async { + let store = await makeStore() + let ids = nodeIDs(await store.graph) + + await store.handle( + .createEdge(from: ids[0], to: ids[1], spec: EdgeSpec(kind: .handoff))) + await store.handle(.nodeCheckApproved(ids[0])) + + let graph = await store.graph + let records = graph.artifactory.filter { $0.topic == "handoff" } + #expect(records.count == 1) + #expect(!records[0].wasWritten) + } + + /// Mirrored traffic still prunes apart from the notes, which is the invariant the + /// split was shaped to leave alone: a graph that merely talks must not cost a note. + @Test + func writtenMessagesStillPruneApartFromTheNotes() async { + let store = await makeStore() + let ids = nodeIDs(await store.graph) + + await store.handle(.artifactoryPost(text: "DEAD END: approach X", topic: nil, from: ids[0])) + for index in 0..<(Artifactory.maxMessages * 2) { + await store.handle( + .messageNode(ids[1], text: "ping \(index)", from: ids[0], followUp: nil)) + } + + let board = await store.graph.artifactory + #expect(board.filter { $0.kind == .note }.count == 1) + #expect(board.filter { $0.kind == .record }.count == Artifactory.maxMessages) + #expect(board.filter(\.wasWritten).count == Artifactory.maxMessages + 1) + } + + /// The eviction this PR would otherwise have introduced into its own feature. A + /// `.none`-transform edge on a cycle mirrors a fresh receipt every pass, and while + /// receipts and messages shared one quota those passes would delete the conversation + /// the board exists to show — invisible before, because both halves were hidden. + @Test + func receiptsCannotEvictTheWrittenConversation() { + let base = Date() + var posts = [ + ArtifactoryPost( + id: 1, at: base, authorID: nil, author: "Author", topic: "direct", + body: "@Reader: the truncation is a decoy", kind: .record, wasWritten: true) + ] + for index in 2...(Artifactory.maxReceipts * 4) { + posts.append( + ArtifactoryPost( + id: index, at: base, authorID: nil, author: "Author", topic: "direct", + body: "@Reader: Author\(Artifactory.firedWithNothingToSay)", kind: .record)) + } + + let pruned = Artifactory.pruned(posts) + + #expect(pruned.contains { $0.body.contains("the truncation is a decoy") }) + #expect(pruned.filter { !$0.wasWritten }.count == Artifactory.maxReceipts) + #expect(pruned == pruned.sorted { $0.id < $1.id }) + } + + /// Ties the two modules together. `readsAsADeliveryReceipt` recognises a legacy post by + /// words `GraphcodeKit` writes, and reading either literal out of the other module + /// would let them drift with no compile error — so the shape the mirror actually + /// produces is asserted against the reader, not against a copy of the string. + @Test + func whatTheMirrorGeneratesIsWhatTheReaderRecognises() async { + let store = await makeStore() + let ids = nodeIDs(await store.graph) + + await store.handle( + .createEdge(from: ids[0], to: ids[1], spec: EdgeSpec(kind: .message))) + await store.handle( + .createEdge(from: ids[0], to: ids[1], spec: EdgeSpec(kind: .handoff))) + await store.handle(.nodeCheckApproved(ids[0])) + + let board = await store.graph.artifactory + #expect(board.count == 2) + for post in board { + #expect(Artifactory.readsAsADeliveryReceipt(post.body)) + } + } + + /// A board saved before the split carries no flag, and defaulting it to "receipt" + /// would leave every conversation already on one as buried as #273 found it. The two + /// generated shapes are read back out; a `node send` is not one of them. + @Test + func recordsSavedBeforeTheSplitAreReadBackFromTheirBodies() throws { + let written = try decodeLegacyRecord(body: "@Reader: truncation is not the mechanism") + #expect(written.kind == .record) + #expect(written.wasWritten) + + let firedEdge = try decodeLegacyRecord(body: "@Reader: Author finished.") + #expect(!firedEdge.wasWritten) + let handoff = try decodeLegacyRecord( + body: "@Reader: Author finished and handed its work off to you.") + #expect(!handoff.wasWritten) + } + + /// A hand-off that carried a payload ends with the payload, not the boilerplate — the + /// part a later reader actually needs, and the reason the suffix test is anchored. + @Test + func aLegacyHandoffCarryingAPayloadReadsAsWritten() throws { + let post = try decodeLegacyRecord( + body: "@Reader: Author finished and handed its work off to you. branch: fix/auth") + #expect(post.wasWritten) + } + + /// An explicit flag always wins: the body is only consulted when there is none. + @Test + func anExplicitFlagIsNeverSecondGuessedByTheBody() throws { + #expect(try decodeRecord(body: "@Reader: Author finished.", wasWritten: true).wasWritten) + } + + /// A board with no `wasWritten` key at all, which the current encoder can no longer + /// produce — so the JSON is assembled rather than round-tripped. + private func decodeLegacyRecord(body: String) throws -> ArtifactoryPost { + try decodeRecord(body: body, wasWritten: nil) + } + + private func decodeRecord(body: String, wasWritten: Bool?) throws -> ArtifactoryPost { + var fields: [String: Any] = [ + "id": 9, "at": 747_000_000, "author": "Author", "topic": "direct", "body": body, + "kind": "record", + ] + if let wasWritten { fields["wasWritten"] = wasWritten } + return try JSONDecoder().decode( + ArtifactoryPost.self, from: JSONSerialization.data(withJSONObject: fields)) + } + + @Test + func aNoteIsWrittenWithoutBeingToldSo() { + let note = ArtifactoryPost( + id: 1, at: Date(), authorID: nil, author: "a human", topic: nil, body: "hello") + #expect(note.wasWritten) + } +} + +extension ArtifactoryTests { // MARK: - Deletion /// Deleting a loop takes the handle to its posts, never the posts. A note is