krz/gitbay-ios

a native ios client for gitbay client swift https://gitbay.org

docs/superpowers/plans/2026-09-06-mr05-compare-refs.md

v1.2.0
main All refs →
gitbay-ios/docs/superpowers/plans/2026-09-06-mr05-compare-refs.md rendered · source · history · blame · raw

300 lines · 13882 bytes

MR 5: Compare two refs Implementation Plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: Close the compare two refs parity row — pick two refs, see the diff between them.

Architecture: Purely additive. A view model reads repo diff <owner/name> <base> <head>, parses its patch with the existing UnifiedDiff.parse, and renders it with the existing DiffFileSection. No refactor.

Spec: docs/superpowers/specs/2026-09-06-ios-parity-design.md

Correction to the spec

The spec says this merge request must split DiffView, calls it "the only existing code the plan modifies rather than extends", and flags it as the one to expect a review round on. That is wrong, and this plan supersedes it.

The seam already exists and already has a second caller:

  • gitbay/Views/MRs/DiffView.swift — struct DiffFileSection takes var model: MRDetailViewModel? = nil, commented "Review threads belong to merge requests; a commit diff has none."
  • gitbay/Views/Repos/CommitView.swift:71 — already calls DiffFileSection(file: file) with no model.

So compare reuses DiffFileSection(file:) exactly as the commit screen does. Do not refactor DiffView, DiffFileSection, HunkView or LineView. Touching them is out of scope and risks the merge request review screen, which is the app's most-used surface.

Global Constraints

  • Swift 6 language mode, default MainActor isolation. Wire models are nonisolated structs.
  • Swift Testing only — never XCTest.
  • gitbayTests is hermetic and offline; network goes through StubProtocol.
  • New files under gitbay/ and gitbayTests/ need no project.pbxproj edit.
  • The label model is IssueLabel, never Label.
  • Never mention Claude, LLMs or AI in commits, comments, or the merge request. No Co-Authored-By trailer.
  • Never commit to main.

The command, verbatim from the registry:

repo diff <owner/name> <base> <head>

Both refs are positional, base first. It returns:

{"base":"f1cc68c…","head":"0f4f9b4…","merge_base":"0f4f9b4…","patch":"","truncated":false}

patch is unified-diff text — the same shape UnifiedDiff.parse already handles for mr diff and repo show's commit patch. truncated says the server cut the patch short.

Counting tests reliably — the naive grep undercounts because xcodebuild interleaves output:

grep -cE "^Test case '[^']*' passed" /tmp/out.txt
grep -cE "^Test case '[^']*' failed" /tmp/out.txt
grep -rhoE "@Test(\([^)]*\))? func" gitbayTests/*.swift | wc -l   # declared

Declared minus skipped should equal passed. One test is always skipped (LiveInstanceTests, conditionally enabled on a token).

File Structure

File Responsibility
gitbay/Repos/CompareViewModel.swift (create) RepoDiff wire model + the view model over repo diff
gitbayTests/CompareTests.swift (create) Every test in this plan
gitbay/Views/Repos/CompareView.swift (create) Two ref pickers and the rendered patch
gitbay/Views/Repos/RepoRoute.swift (modify) compare(repo:) case
gitbay/ContentView.swift (modify) Route to CompareView
gitbay/Views/Repos/RefsView.swift (modify) Entry point

Task 1: The compare view model

Files: Create gitbay/Repos/CompareViewModel.swift; test in gitbayTests/CompareTests.swift

Interfaces produced:

  • nonisolated struct RepoDiff: Decodable, Sendable, Hashable — base: String, head: String, mergeBase: String, patch: String, truncated: Bool
  • @Observable @MainActor final class CompareViewModel — base: String, head: String, state: LoadState<UnifiedDiff>, truncated: Bool, mergeBase: String?, refs: RepoRefs?, func loadRefs() async, func compare() async

Behaviour, each pinned by a test:

  1. compare() sends exactly ["repo", "diff", repoPath, base, head] — both positional, base first. Swapping them silently returns the opposite diff, which is why the test asserts the full array.
  2. Comparing a ref with itself yields an empty patch, which is an .empty state, not .failed.
  3. truncated is decoded and exposed. A partial patch rendered as though it were whole is a bug the user cannot see.
  4. mergeBase is decoded and exposed, so the screen can say what the comparison is against.
  5. Either ref being blank issues no request — there is nothing to compare yet.
  6. refs comes from repo refs, reusing the existing RepoRefs/RepoRef models; a failure there is not fatal to the screen but must be distinguishable from "this repository has no branches" (the same three-state lesson MR 4 learned on its retarget picker: expose the refs as an optional plus an error string, not an empty array).
  • Step 1: Write the failing tests

Create gitbayTests/CompareTests.swift:

import Foundation
import Testing
@testable import gitbay

private func makeClient() throws -> (GitbayClient, StubProtocol.Box) {
    let box = StubProtocol.box()
    let client = GitbayClient(
        instance: try GitbayInstance(url: "https://gitbay.org"),
        token: "test-token",
        session: box.session()
    )
    return (client, box)
}

private func argvFrom(_ url: URL) -> [String] {
    URLComponents(url: url, resolvingAgainstBaseURL: false)?
        .queryItems?.filter { $0.name == "argv" }.compactMap(\.value) ?? []
}

private let patchJSON = """
    {"protocol_version":1,"data":{"base":"aaa111","head":"bbb222","merge_base":"ccc333",\
    "patch":"diff --git a/x.txt b/x.txt\\n--- a/x.txt\\n+++ b/x.txt\\n@@ -1 +1 @@\\n-old\\n+new\\n",\
    "truncated":false},"exit_code":0}
    """

struct RepoDiffDecodingTests {

    @Test func decodesEveryField() throws {
        let decoder = JSONDecoder()
        let json = """
            {"base":"aaa111","head":"bbb222","merge_base":"ccc333","patch":"x","truncated":true}
            """
        let diff = try decoder.decode(RepoDiff.self, from: Data(json.utf8))
        #expect(diff.base == "aaa111")
        #expect(diff.head == "bbb222")
        #expect(diff.mergeBase == "ccc333")
        #expect(diff.truncated)
    }
}

@MainActor
struct CompareViewModelTests {

    @Test func bothRefsArePositionalAndBaseComesFirst() async throws {
        let (client, stub) = try makeClient()
        stub.enqueue(.init(status: 200, json: patchJSON))
        let model = CompareViewModel(client: client, repoPath: "krz/gitbay")
        model.base = "main"
        model.head = "feature"
        await model.compare()

        #expect(argvFrom(try #require(stub.seen.last).url)
            == ["repo", "diff", "krz/gitbay", "main", "feature"])
    }

    @Test func theParsedPatchReachesTheScreen() async throws {
        let (client, stub) = try makeClient()
        stub.enqueue(.init(status: 200, json: patchJSON))
        let model = CompareViewModel(client: client, repoPath: "krz/gitbay")
        model.base = "main"
        model.head = "feature"
        await model.compare()

        #expect(model.state.value?.files.count == 1)
        #expect(model.mergeBase == "ccc333")
        #expect(model.truncated == false)
    }

    /// A partial patch rendered as though whole is a bug the user cannot
    /// see, so the flag must survive to the screen.
    @Test func truncationIsCarriedThrough() async throws {
        let (client, stub) = try makeClient()
        stub.enqueue(.init(status: 200, json: """
            {"protocol_version":1,"data":{"base":"a","head":"b","merge_base":"c",\
            "patch":"diff --git a/x b/x\\n--- a/x\\n+++ b/x\\n@@ -1 +1 @@\\n-a\\n+b\\n",\
            "truncated":true},"exit_code":0}
            """))
        let model = CompareViewModel(client: client, repoPath: "krz/gitbay")
        model.base = "main"
        model.head = "feature"
        await model.compare()

        #expect(model.truncated)
    }

    @Test func comparingARefWithItselfIsEmptyNotFailed() async throws {
        let (client, stub) = try makeClient()
        stub.enqueue(.init(status: 200, json: """
            {"protocol_version":1,"data":{"base":"a","head":"a","merge_base":"a",\
            "patch":"","truncated":false},"exit_code":0}
            """))
        let model = CompareViewModel(client: client, repoPath: "krz/gitbay")
        model.base = "main"
        model.head = "main"
        await model.compare()

        guard case .empty = model.state else {
            Testing.Issue.record("expected empty, got \(model.state)")
            return
        }
    }

    @Test func aBlankRefComparesNothing() async throws {
        let (client, stub) = try makeClient()
        let model = CompareViewModel(client: client, repoPath: "krz/gitbay")
        model.base = "main"
        model.head = "   "
        await model.compare()

        #expect(stub.seen.isEmpty)
    }

    @Test func refsComeFromRepoRefs() async throws {
        let (client, stub) = try makeClient()
        stub.enqueue(.init(status: 200, json: """
            {"protocol_version":1,"data":{"branches":[{"name":"main","sha":"a"},\
            {"name":"feature","sha":"b"}],"tags":[{"name":"v1","sha":"c"}]},"exit_code":0}
            """))
        let model = CompareViewModel(client: client, repoPath: "krz/gitbay")
        await model.loadRefs()

        #expect(model.refs?.branches.map(\.name) == ["main", "feature"])
        #expect(model.refs?.tags.map(\.name) == ["v1"])
        #expect(argvFrom(try #require(stub.seen.last).url)
            == ["repo", "refs", "krz/gitbay"])
    }

    /// The lesson from the retarget picker: a failed ref load must not
    /// look like a repository with no branches.
    @Test func aFailedRefLoadIsDistinguishableFromNoBranches() async throws {
        let (client, stub) = try makeClient()
        stub.enqueue(.init(status: 200, json: """
            {"protocol_version":1,"error":"denied","exit_code":4}
            """))
        let model = CompareViewModel(client: client, repoPath: "krz/gitbay")
        await model.loadRefs()

        #expect(model.refs == nil)
        #expect(model.refsError?.isEmpty == false)
    }

    @Test func aFailedCompareIsTheScreensState() async throws {
        let (client, stub) = try makeClient()
        stub.enqueue(.init(status: 200, json: """
            {"protocol_version":1,"error":"unknown revision","exit_code":1}
            """))
        let model = CompareViewModel(client: client, repoPath: "krz/gitbay")
        model.base = "main"
        model.head = "nope"
        await model.compare()

        guard case .failed = model.state else {
            Testing.Issue.record("expected failed, got \(model.state)")
            return
        }
    }
}
  • Step 2: Run to verify failure.
  • Step 3: Implement. RepoDiff with CodingKeys mapping merge_base. CompareViewModel holding client, repoPath, the two ref strings, state, truncated, mergeBase, refs: RepoRefs?, refsError: String?. compare() guards on both refs being non-blank, reads, parses with UnifiedDiff.parse(diff.patch), and sets .empty when the parsed diff has no files.
  • Step 4: Run the tests.
  • Step 5: Commit — git commit -m "Compare two refs through repo diff"

Task 2: The compare screen

Files: Create gitbay/Views/Repos/CompareView.swift; modify RepoRoute.swift, ContentView.swift, RefsView.swift

No unit tests — UI.

Reuse DiffFileSection(file:) with no model, exactly as CommitView.swift:71 does. Do not modify it.

The screen: two ref pickers (base and head) fed by model.refs, a Compare action, then the rendered patch. Follow RefsView's idiom for presenting branches and tags together.

Three things the screen must say:

  1. When truncated — a visible notice that the patch is incomplete. GBNotice exists for this. Rendering a cut-short diff silently is the one way this screen misleads.
  2. The merge base, so the user knows what the comparison is against — repo diff reports it and a three-dot comparison against an unstated base is ambiguous.
  3. A failed ref load is not "no branches" — render refsError distinctly from an empty ref list, the same way MR 4's retarget picker does.

Add RepoRoute.compare(repo:), wire it in ContentView's RepoRoute switch, and add the entry point to RefsView (which is the branches-and-tags screen and the natural home).

  • Step 1: Route case + ContentView destination
  • Step 2: CompareView with the two pickers and the patch
  • Step 3: Entry point in RefsView
  • Step 4: Build and run the full suite. No drop.
  • Step 5: Commit — git commit -m "Compare screen with two ref pickers"

Task 3: Flip the parity row and open the merge request

  • Step 1: In the Repositories table, compare two refs goes to yes for iOS. Touch no other row.

Land it via a worktree off origin/main in krz/gitbay, merged --strategy ff. That repo requires signed commits, so squash and rebase merges are refused, and its working tree usually holds unrelated work — never switch its branch.

  • Step 2: Run the full suite; record the real number.
  • Step 3: Open the merge request.

Notes for whoever executes this

Do not refactor the diff views. DiffFileSection is already reusable and already reused. The spec's claim that this merge request must split DiffView is wrong and has been superseded by this plan.

Base comes first. repo diff <owner/name> <base> <head> — swapping them returns the opposite diff with no error at all.

truncated must reach the screen. A partial patch that looks whole is worse than an error.