The commit page shows a diff instead of printing one

57d5fd933cfd · AtlantisPleb · · parent 819a54005e33

The commit page shows a diff instead of printing one

First item of the Pierre port. The commit page rendered the whole unified diff
as one `<pre>`: no per-file split, no hunk headers, no add and remove colour,
and no line numbers at all.

`OpenAgents.Diff` parses that text into files, hunks, and lines, each line
carrying its number on **both** sides. That pairing is the whole reason to
parse rather than print. The two sides stop agreeing at the first change --
a deletion advances only the old side, an insertion only the new -- and a
`<pre>` can never say where a line went.

`UI.diff_file/1` renders one file, adapted from Pierre's `FileDiff`
(@pierre/diffs, Apache 2.0). Unified rather than split: a split view needs
about twice the width to say the same thing and collapses on a narrow screen,
and the two gutters carry what it is for. Every new-side line number is a link
to itself, scoped by path since one page holds many files, so a reader can
point at a line rather than describe where it is. The marker column says `+`
and `-`, so nothing depends on colour alone.

The parser is tolerant on purpose. A diff describes somebody else's repository
and reaches us already truncated by our own cap, so unrecognised lines become
`:meta` and render as themselves rather than raising. If parsing yields
nothing at all, the page falls back to the raw text: a shape this code has not
seen should still reach the reader.

Checked against `git diff-tree --numstat` on real commits -- file counts and
both line totals match exactly -- plus sixteen unit tests for renames,
binaries, mid-hunk truncation, absent hunk counts and quoted paths. Two bugs
came out of that: a newline-terminated diff split to a trailing empty element
that became a phantom context line on every file's last hunk, and line numbers
were assigned by re-counting the hunk per line, which is quadratic in hunk
length. Numbering is now one pass at the end.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0149rBWy7br1Z7bbz9NrQhEr
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>

Deploy story

What this commit did to the running system — joined from the forge receipt chain, the part a commit page elsewhere cannot show.

Not deployed through the forge lane

No push, promotion, build, or deploy receipt references this commit (receipts are scanned over a bounded recent window). Changes shipped by full node replacement carry their proof in the release gate receipt instead.

Changed files

  • modified assets/css/openagents.css
  • modified docs/2026-08-20-pierre-code-surfaces-port.md
  • added lib/openagents/diff.ex
  • modified lib/openagents_web/component_catalog.ex
  • modified lib/openagents_web/components/ui.ex
  • modified lib/openagents_web/live/code_commit_live.ex
  • modified lib/openagents_web/live/components_live.ex
  • added test/openagents/diff_test.exs
  • modified test/openagents_web/live/code_live_test.exs

Diff

9 files changed, +990 -14

assets/css/openagents.css modified +251

@@ -4595,3 +4595,254 @@

4595 4595
    font-variant-numeric: tabular-nums;
4596 4596
  }
4597 4597
}
4598
4599
/* ── Diff ─────────────────────────────────────────────────────────────────── */
4600
4601
/* One file's diff. Adapted from Pierre's `FileDiff` (@pierre/diffs, Apache
4602
 * 2.0, pierrecomputer/pierre); the layout decisions carried over, the values
4603
 * are ours. See docs/2026-08-20-pierre-code-surfaces-port.md.
4604
 *
4605
 * A table, not a stack of flex rows: the gutters must align down the whole
4606
 * file regardless of how many digits a line number has, and that is what a
4607
 * table does without measuring anything. */
4608
4609
@layer components {
4610
  .diff-file {
4611
    overflow: hidden;
4612
    border: 1px solid var(--line);
4613
    border-radius: var(--radius-lg);
4614
    background: var(--ink-surface);
4615
  }
4616
4617
  .diff-file + .diff-file {
4618
    margin-block-start: 12px;
4619
  }
4620
4621
  .diff-file__header {
4622
    display: flex;
4623
    align-items: center;
4624
    gap: 8px;
4625
    cursor: pointer;
4626
    list-style: none;
4627
    padding: 10px 12px;
4628
    background: var(--ink-raised);
4629
    font-size: 0.8125rem;
4630
  }
4631
4632
  .diff-file__header::-webkit-details-marker {
4633
    display: none;
4634
  }
4635
4636
  .diff-file__header:focus-visible {
4637
    outline: 2px solid var(--ring);
4638
    outline-offset: -2px;
4639
  }
4640
4641
  .diff-file__caret {
4642
    width: 14px;
4643
    height: 14px;
4644
    flex: none;
4645
    color: var(--icon-tertiary);
4646
    font-size: 14px;
4647
    transition: transform var(--motion-fast, 150ms) var(--ease);
4648
  }
4649
4650
  .diff-file[open] .diff-file__caret {
4651
    transform: rotate(90deg);
4652
  }
4653
4654
  .diff-file__path {
4655
    min-width: 0;
4656
    flex: 1;
4657
    overflow: hidden;
4658
    color: var(--text-primary);
4659
    font-family: var(--font-mono, ui-monospace, monospace);
4660
    text-overflow: ellipsis;
4661
    white-space: nowrap;
4662
  }
4663
4664
  .diff-file__from {
4665
    color: var(--text-dim);
4666
  }
4667
4668
  /* The status is a word, not a colour: "renamed" and "modified" are not
4669
     distinguishable as tints, and a reader should not have to learn a legend. */
4670
  .diff-file__status {
4671
    flex: none;
4672
    border: 1px solid var(--line);
4673
    border-radius: 999px;
4674
    padding: 1px 8px;
4675
    color: var(--text-dim);
4676
    font-size: 0.6875rem;
4677
    text-transform: lowercase;
4678
  }
4679
4680
  .diff-file__count {
4681
    flex: none;
4682
    font-family: var(--font-mono, ui-monospace, monospace);
4683
    font-size: 0.75rem;
4684
    font-variant-numeric: tabular-nums;
4685
  }
4686
4687
  .diff-file__count[data-kind="insert"] { color: var(--success); }
4688
  .diff-file__count[data-kind="delete"] { color: var(--danger); }
4689
4690
  .diff-file__note {
4691
    padding: 12px;
4692
    color: var(--text-dim);
4693
    font-size: 0.8125rem;
4694
  }
4695
4696
  /* ── Hunks ──────────────────────────────────────────────────────────────── */
4697
4698
  .diff-hunk + .diff-hunk {
4699
    border-top: 1px solid var(--line);
4700
  }
4701
4702
  .diff-hunk__header {
4703
    display: flex;
4704
    align-items: baseline;
4705
    gap: 12px;
4706
    border-block: 1px solid var(--line-faint);
4707
    background: var(--wash-hover);
4708
    padding: 4px 12px;
4709
    font-size: 0.75rem;
4710
  }
4711
4712
  .diff-hunk__range {
4713
    flex: none;
4714
    color: var(--text-dim);
4715
    font-family: var(--font-mono, ui-monospace, monospace);
4716
  }
4717
4718
  /* Git puts the enclosing function here, which is the cheapest orientation a
4719
     reader gets in a long file. */
4720
  .diff-hunk__heading {
4721
    min-width: 0;
4722
    overflow: hidden;
4723
    color: var(--text-muted);
4724
    font-family: var(--font-mono, ui-monospace, monospace);
4725
    text-overflow: ellipsis;
4726
    white-space: nowrap;
4727
  }
4728
4729
  /* ── Lines ──────────────────────────────────────────────────────────────── */
4730
4731
  .diff-lines {
4732
    width: 100%;
4733
    border-collapse: collapse;
4734
    font-family: var(--font-mono, ui-monospace, monospace);
4735
    font-size: 0.8125rem;
4736
    line-height: 1.5;
4737
  }
4738
4739
  .diff-line td {
4740
    padding: 0;
4741
    vertical-align: top;
4742
  }
4743
4744
  /* Tints are held well back. A diff is read for its text, and a saturated row
4745
     fights the code sitting on it; the marker column carries the same
4746
     information without colour. */
4747
  .diff-line[data-kind="insert"] {
4748
    background: color-mix(in oklab, var(--success) 12%, transparent);
4749
  }
4750
4751
  .diff-line[data-kind="delete"] {
4752
    background: color-mix(in oklab, var(--danger) 12%, transparent);
4753
  }
4754
4755
  .diff-line[data-kind="meta"] {
4756
    background: var(--wash-hover);
4757
    color: var(--text-dim);
4758
  }
4759
4760
  /* The addressed line, after following an anchor. */
4761
  .diff-line:target {
4762
    background: color-mix(in oklab, var(--info) 22%, transparent);
4763
  }
4764
4765
  .diff-line__number {
4766
    width: 1%;
4767
    min-width: 44px;
4768
    padding-inline: 8px !important;
4769
    border-right: 1px solid var(--line-faint);
4770
    color: var(--text-dim);
4771
    font-size: 0.75rem;
4772
    font-variant-numeric: tabular-nums;
4773
    text-align: end;
4774
    user-select: none;
4775
    white-space: nowrap;
4776
  }
4777
4778
  .diff-line__number a {
4779
    color: inherit;
4780
    text-decoration: none;
4781
  }
4782
4783
  @media (hover: hover) {
4784
    .diff-line__number a:hover {
4785
      color: var(--text-primary);
4786
      text-decoration: underline;
4787
    }
4788
  }
4789
4790
  .diff-line__marker {
4791
    width: 1%;
4792
    padding-inline: 6px !important;
4793
    color: var(--text-dim);
4794
    text-align: center;
4795
    user-select: none;
4796
  }
4797
4798
  .diff-line[data-kind="insert"] .diff-line__marker { color: var(--success); }
4799
  .diff-line[data-kind="delete"] .diff-line__marker { color: var(--danger); }
4800
4801
  /* The code scrolls inside the cell rather than widening the page: a long
4802
     line is normal in a diff, and a horizontal scrollbar on the document is
4803
     not. `pre` keeps the leading whitespace that indentation depends on. */
4804
  .diff-line__text {
4805
    width: 100%;
4806
    overflow-x: auto;
4807
    padding-inline: 10px !important;
4808
  }
4809
4810
  .diff-line__text pre {
4811
    margin: 0;
4812
    color: var(--text-body);
4813
    font: inherit;
4814
    white-space: pre;
4815
  }
4816
4817
  @media (prefers-reduced-motion: reduce) {
4818
    .diff-file__caret {
4819
      transition: none;
4820
    }
4821
  }
4822
}
4823
4824
@layer components {
4825
  /* The count under a diff describes the diff above it, including when the
4826
     input was truncated -- it is derived from the parsed lines, not from git's
4827
     own summary of the whole commit. */
4828
  .diff-totals {
4829
    display: flex;
4830
    align-items: baseline;
4831
    gap: 8px;
4832
    padding-block: 8px;
4833
    color: var(--text-muted);
4834
    font-size: 0.8125rem;
4835
  }
4836
4837
  .diff-totals [data-kind="insert"] {
4838
    color: var(--success);
4839
    font-family: var(--font-mono, ui-monospace, monospace);
4840
    font-variant-numeric: tabular-nums;
4841
  }
4842
4843
  .diff-totals [data-kind="delete"] {
4844
    color: var(--danger);
4845
    font-family: var(--font-mono, ui-monospace, monospace);
4846
    font-variant-numeric: tabular-nums;
4847
  }
4848
}
docs/2026-08-20-pierre-code-surfaces-port.md modified +35 -11

@@ -99,17 +99,41 @@ highlighting to a correct structure than the reverse.

99 99
100 100
Status is one of **done**, **next**, or **planned**.
101 101
102
### 1. `diff_file/1` — a single file's diff — **next**
103
104
Adapted from `FileDiff`. Decomposes a unified diff into files, hunks, and lines
105
carrying old and new numbers, and renders it with a file header, hunk headers,
106
add/remove tinting, and per-line anchors. Collapsible per file through native
107
`<details>`.
108
109
Lands as a parser plus a component, catalogued and demoed, and replaces the
110
raw `<pre>` on the commit page.
111
112
### 2. `code_file/1` — one blob, numbered and addressable — **planned**
102
### 1. `diff_file/1` — a single file's diff — **done**
103
104
Adapted from `FileDiff`. Landed as two pieces:
105
106
- `OpenAgents.Diff` parses a unified diff into files, hunks, and lines, each
107
  line carrying its number on **both** sides. That pairing is the whole reason
108
  to parse rather than print: the two sides stop agreeing at the first change,
109
  and a `<pre>` can never say where a line went.
110
- `OpenAgentsWeb.UI.diff_file/1` renders one file: header with status and
111
  counts, hunk headers including git's enclosing-function hint, held-back
112
  add/remove tints, and a link on every new-side line number. Catalogued at
113
  `/components/openagents-diff-file`.
114
115
The commit page uses it in place of the raw `<pre>`, with a totals line
116
counted from the parsed lines so it describes the diff actually shown even
117
when the input was truncated. If the parser returns nothing — a shape it has
118
not seen — the page falls back to the raw text rather than showing a reader
119
nothing.
120
121
Decisions worth keeping:
122
123
- **Unified, not split.** A split view needs roughly twice the width to say
124
  the same thing and collapses badly on a narrow screen. The two number
125
  gutters carry what split is for.
126
- **Colour is never the only signal.** The marker column says `+` and `-`, so
127
  the diff survives greyscale and a reader who cannot separate the tints.
128
- **Tolerant parsing.** A diff describes somebody else's repository and
129
  arrives truncated by our own cap. Unrecognised lines become `:meta` and
130
  render as themselves; nothing raises.
131
132
Validated against `git diff-tree --numstat` on real commits — file counts and
133
both line totals match exactly — plus sixteen unit tests covering renames,
134
binaries, mid-hunk truncation, missing hunk counts, and quoted paths.
135
136
### 2. `code_file/1` — one blob, numbered and addressable — **next**
113 137
114 138
Adapted from `File`. Line numbers, line anchors and ranges (`#L12`,
115 139
`#L12-L20`), a sticky filename header, copy and raw actions. Replaces the
lib/openagents/diff.ex added +311

@@ -0,0 +1,311 @@

1
defmodule OpenAgents.Diff do
2
  @moduledoc """
3
  A unified diff, decomposed into files, hunks, and lines.
4
5
  The model is adapted from Pierre's `@pierre/diffs` (Apache 2.0,
6
  `pierrecomputer/pierre`), which is the part of that library worth taking:
7
  what a diff *is* once you stop treating it as text. See
8
  `docs/2026-08-20-pierre-code-surfaces-port.md`.
9
10
  A diff arrives from `git diff-tree -p -M` as one string, and rendering it as
11
  one string is what the commit page used to do. The whole value of parsing it
12
  is that each line then carries **both** line numbers -- where it sits in the
13
  old file and in the new one -- which is what lets a reader point at a line,
14
  and what a `<pre>` blob can never provide.
15
16
  The parser is deliberately tolerant. A diff is a report about somebody else's
17
  repository: it can be truncated mid-hunk by an upstream cap, describe a
18
  binary file, record a rename with no content change, or use headers this code
19
  has not seen. None of that should raise on a page whose job is to show what
20
  happened. Anything unrecognised inside a file becomes a `:meta` line, which
21
  renders as plain text and is honest about being unparsed.
22
  """
23
24
  defmodule Line do
25
    @moduledoc "One line of a hunk, carrying its position in both files."
26
27
    @type kind :: :context | :insert | :delete | :meta
28
29
    @type t :: %__MODULE__{
30
            kind: kind(),
31
            text: String.t(),
32
            old_number: pos_integer() | nil,
33
            new_number: pos_integer() | nil
34
          }
35
36
    @enforce_keys [:kind, :text]
37
    defstruct [:kind, :text, :old_number, :new_number]
38
  end
39
40
  defmodule Hunk do
41
    @moduledoc """
42
    One contiguous run of changes.
43
44
    `heading` is the text git puts after the `@@` marker -- usually the
45
    enclosing function -- which is the cheapest orientation a reader gets and
46
    the reason the header is worth rendering rather than discarding.
47
    """
48
49
    @type t :: %__MODULE__{
50
            old_start: non_neg_integer(),
51
            old_count: non_neg_integer(),
52
            new_start: non_neg_integer(),
53
            new_count: non_neg_integer(),
54
            heading: String.t() | nil,
55
            lines: [Line.t()]
56
          }
57
58
    @enforce_keys [:old_start, :old_count, :new_start, :new_count]
59
    defstruct [:old_start, :old_count, :new_start, :new_count, :heading, lines: []]
60
  end
61
62
  defmodule File do
63
    @moduledoc """
64
    One file's worth of a diff.
65
66
    `status` distinguishes the cases a header can describe: `:added`,
67
    `:deleted`, `:renamed`, or `:modified`. `binary?` is its own flag rather
68
    than an absence of hunks, because "no textual change to show" and "this
69
    file cannot be shown as text" are different statements and a reader should
70
    be told which one they are looking at.
71
    """
72
73
    @type status :: :added | :deleted | :renamed | :modified
74
75
    @type t :: %__MODULE__{
76
            path: String.t(),
77
            old_path: String.t() | nil,
78
            status: status(),
79
            binary?: boolean(),
80
            hunks: [Hunk.t()],
81
            insertions: non_neg_integer(),
82
            deletions: non_neg_integer()
83
          }
84
85
    @enforce_keys [:path]
86
    defstruct [
87
      :path,
88
      :old_path,
89
      status: :modified,
90
      binary?: false,
91
      hunks: [],
92
      insertions: 0,
93
      deletions: 0
94
    ]
95
  end
96
97
  @doc """
98
  Parse a unified diff into `%File{}` structs, in the order git emitted them.
99
100
  Returns `[]` for empty or unparseable input rather than raising: a commit
101
  page that shows nothing is a worse answer than one that shows the files it
102
  understood, but both are better than a crash.
103
  """
104
  @spec parse(String.t() | nil) :: [File.t()]
105
  def parse(nil), do: []
106
  def parse(""), do: []
107
108
  def parse(diff) when is_binary(diff) do
109
    diff
110
    |> lines()
111
    |> collect_files(nil, [])
112
    |> Enum.map(&finalize_file/1)
113
  end
114
115
  # A newline-terminated diff splits to a trailing empty element, which is the
116
  # terminator rather than a line. Left in, it became a phantom context line on
117
  # the last hunk of every file and shifted that hunk's line count by one. Only
118
  # the final one is dropped: an empty element anywhere else is a real line
119
  # from a generator that writes bare blank lines instead of `" "`.
120
  defp lines(diff) do
121
    case String.split(diff, "\n") do
122
      [] -> []
123
      parts -> if List.last(parts) == "", do: Enum.drop(parts, -1), else: parts
124
    end
125
  end
126
127
  @doc """
128
  Totals across a parsed diff: how many files, and how many lines each way.
129
130
  Reported from the parsed lines rather than from git's own summary, so the
131
  number under a diff always describes the diff above it -- including when the
132
  input was truncated and the tail is missing.
133
  """
134
  @spec totals([File.t()]) :: %{
135
          files: non_neg_integer(),
136
          insertions: non_neg_integer(),
137
          deletions: non_neg_integer()
138
        }
139
  def totals(files) when is_list(files) do
140
    Enum.reduce(files, %{files: 0, insertions: 0, deletions: 0}, fn file, acc ->
141
      %{
142
        files: acc.files + 1,
143
        insertions: acc.insertions + file.insertions,
144
        deletions: acc.deletions + file.deletions
145
      }
146
    end)
147
  end
148
149
  # ── file boundaries ───────────────────────────────────────────────────────
150
151
  defp collect_files([], nil, done), do: Enum.reverse(done)
152
  defp collect_files([], current, done), do: Enum.reverse([current | done])
153
154
  defp collect_files(["diff --git " <> paths | rest], current, done) do
155
    file = %File{path: path_from_header(paths)}
156
    collect_files(rest, file, if(current, do: [current | done], else: done))
157
  end
158
159
  # Lines before the first `diff --git` are the commit's own headers, not a
160
  # file's, and are dropped rather than attached to something they precede.
161
  defp collect_files([_line | rest], nil, done), do: collect_files(rest, nil, done)
162
163
  defp collect_files([line | rest], current, done) do
164
    collect_files(rest, absorb(current, line), done)
165
  end
166
167
  # `diff --git a/x b/x`, where either side may be quoted and contain spaces.
168
  # The b-side is preferred: it is the path the file has now.
169
  defp path_from_header(paths) do
170
    case Regex.run(~r|^"?a/(.*?)"? "?b/(.*?)"?$|, String.trim(paths), capture: :all_but_first) do
171
      [_old, new] -> new
172
      nil -> String.trim(paths)
173
    end
174
  end
175
176
  # ── headers and body ──────────────────────────────────────────────────────
177
178
  defp absorb(file, "new file mode" <> _rest), do: %{file | status: :added}
179
  defp absorb(file, "deleted file mode" <> _rest), do: %{file | status: :deleted}
180
181
  defp absorb(file, "rename from " <> old),
182
    do: %{file | status: :renamed, old_path: unquote_path(old)}
183
184
  defp absorb(file, "rename to " <> new), do: %{file | path: unquote_path(new)}
185
186
  defp absorb(file, "Binary files " <> _rest), do: %{file | binary?: true}
187
  defp absorb(file, "GIT binary patch" <> _rest), do: %{file | binary?: true}
188
189
  # Dropped: they restate the paths already in the `diff --git` header, and
190
  # `/dev/null` on either side restates the status.
191
  defp absorb(file, "--- " <> _rest), do: file
192
  defp absorb(file, "+++ " <> _rest), do: file
193
  defp absorb(file, "index " <> _rest), do: file
194
  defp absorb(file, "old mode " <> _rest), do: file
195
  defp absorb(file, "new mode " <> _rest), do: file
196
  defp absorb(file, "similarity index " <> _rest), do: file
197
  defp absorb(file, "dissimilarity index " <> _rest), do: file
198
199
  defp absorb(file, "@@" <> _rest = line) do
200
    case parse_hunk_header(line) do
201
      {:ok, hunk} -> %{file | hunks: [hunk | file.hunks]}
202
      :error -> push_line(file, %Line{kind: :meta, text: line})
203
    end
204
  end
205
206
  defp absorb(%File{hunks: []} = file, _line), do: file
207
208
  defp absorb(file, "+" <> text), do: push_line(file, :insert, text)
209
  defp absorb(file, "-" <> text), do: push_line(file, :delete, text)
210
  defp absorb(file, " " <> text), do: push_line(file, :context, text)
211
  defp absorb(file, ""), do: push_line(file, :context, "")
212
213
  # "\ No newline at end of file", and anything else that turns up inside a
214
  # hunk. Stated rather than swallowed.
215
  defp absorb(file, line), do: push_line(file, %Line{kind: :meta, text: line})
216
217
  defp unquote_path(path), do: path |> String.trim() |> String.trim("\"")
218
219
  # `@@ -old,count +new,count @@ optional heading`, where either count may be
220
  # omitted and means 1.
221
  defp parse_hunk_header(line) do
222
    case Regex.run(~r/^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@ ?(.*)$/, line,
223
           capture: :all_but_first
224
         ) do
225
      [old_start, old_count, new_start, new_count | heading] ->
226
        {:ok,
227
         %Hunk{
228
           old_start: String.to_integer(old_start),
229
           old_count: count(old_count),
230
           new_start: String.to_integer(new_start),
231
           new_count: count(new_count),
232
           heading: heading |> List.first() |> presence()
233
         }}
234
235
      nil ->
236
        :error
237
    end
238
  end
239
240
  defp count(""), do: 1
241
  defp count(value), do: String.to_integer(value)
242
243
  defp presence(nil), do: nil
244
  defp presence(""), do: nil
245
  defp presence(value), do: value
246
247
  # ── line numbering ────────────────────────────────────────────────────────
248
249
  # Lines are appended unnumbered and numbered once, in `finalize_file/1`.
250
  # Numbering on the way in means re-counting the hunk for every line, which is
251
  # quadratic in the hunk's length -- fine for the four-line hunk in a test and
252
  # not fine for the thousand-line hunk in a real reformatting commit.
253
  defp push_line(file, kind, text) do
254
    [hunk | rest] = file.hunks
255
    line = %Line{kind: kind, text: text}
256
257
    %{
258
      file
259
      | hunks: [%{hunk | lines: [line | hunk.lines]} | rest],
260
        insertions: file.insertions + if(kind == :insert, do: 1, else: 0),
261
        deletions: file.deletions + if(kind == :delete, do: 1, else: 0)
262
    }
263
  end
264
265
  defp push_line(%File{hunks: []} = file, %Line{} = line) do
266
    # A meta line before any hunk has nowhere to sit; the file header already
267
    # said everything it could say.
268
    _ = line
269
    file
270
  end
271
272
  defp push_line(file, %Line{} = line) do
273
    [hunk | rest] = file.hunks
274
    %{file | hunks: [%{hunk | lines: [line | hunk.lines]} | rest]}
275
  end
276
277
  defp finalize_file(file) do
278
    hunks =
279
      file.hunks
280
      |> Enum.reverse()
281
      |> Enum.map(&number_hunk/1)
282
283
    %{file | hunks: hunks}
284
  end
285
286
  # One pass, carrying the next number on each side. A deletion advances only
287
  # the old side and an insertion only the new one, which is the whole reason a
288
  # line needs two numbers: they stop agreeing at the first change.
289
  defp number_hunk(hunk) do
290
    {numbered, _old, _new} =
291
      hunk.lines
292
      |> Enum.reverse()
293
      |> Enum.reduce({[], hunk.old_start, hunk.new_start}, fn line, {acc, old, new} ->
294
        case line.kind do
295
          :context ->
296
            {[%{line | old_number: old, new_number: new} | acc], old + 1, new + 1}
297
298
          :delete ->
299
            {[%{line | old_number: old} | acc], old + 1, new}
300
301
          :insert ->
302
            {[%{line | new_number: new} | acc], old, new + 1}
303
304
          :meta ->
305
            {[line | acc], old, new}
306
        end
307
      end)
308
309
    %{hunk | lines: Enum.reverse(numbered)}
310
  end
311
end
lib/openagents_web/component_catalog.ex modified +7

@@ -65,6 +65,13 @@ defmodule OpenAgentsWeb.ComponentCatalog do

65 65
          source: "OpenAgentsWeb.UI.breadcrumb/1",
66 66
          summary: "Ancestor trail ending in the current page, which is not a link."
67 67
        },
68
        %{
69
          slug: "openagents-diff-file",
70
          title: "Diff",
71
          icon: "code",
72
          source: "OpenAgentsWeb.UI.diff_file/1",
73
          summary: "One file's diff: hunks, both line numbers, addressable lines."
74
        },
68 75
        %{
69 76
          slug: "openagents-github-login",
70 77
          title: "GitHub login",
lib/openagents_web/components/ui.ex modified +106

@@ -818,6 +818,112 @@ defmodule OpenAgentsWeb.UI do

818 818
    """
819 819
  end
820 820
821
  @doc """
822
  One file's diff: a header, its hunks, and every line numbered on both sides.
823
824
  Adapted from Pierre's `FileDiff` (`@pierre/diffs`, Apache 2.0,
825
  `pierrecomputer/pierre`). What carried over is the model rather than the
826
  code -- see `docs/2026-08-20-pierre-code-surfaces-port.md`. Takes an
827
  `OpenAgents.Diff.File`, which `OpenAgents.Diff.parse/1` produces from the
828
  output of `git diff-tree -p -M`.
829
830
  Unified rather than split. A split view needs roughly twice the width to say
831
  the same thing, and on a narrow screen it either scrolls sideways or squeezes
832
  both sides into columns too thin to read. The two line-number gutters carry
833
  what the split layout is for: which line this was, and which line it is now.
834
835
  Every line is addressable. A line's new-side number is a link to itself, so a
836
  reader can point someone at a line rather than describing where it is. The
837
  anchor is scoped by path, since one page holds many files.
838
839
  Colour is not the only carrier of meaning: an inserted line is marked `+` and
840
  a deleted one `-` in the gutter, so the diff survives greyscale and a reader
841
  who cannot separate the two tints.
842
843
  Collapsible through native `<details>`, open by default. A reviewer opening a
844
  commit wants to see it, and a large file is the one they most want to fold
845
  away -- so the control is there without costing a click on arrival.
846
  """
847
  attr :file, :map, required: true, doc: "an `OpenAgents.Diff.File`"
848
  attr :open, :boolean, default: true
849
  attr :class, :any, default: nil
850
  attr :rest, :global
851
852
  def diff_file(assigns) do
853
    assigns = assign(assigns, :slug, diff_slug(assigns.file.path))
854
855
    ~H"""
856
    <details id={"diff-#{@slug}"} class={["diff-file", @class]} open={@open} {@rest}>
857
      <summary class="diff-file__header">
858
        <.icon name="chevron-right" class="diff-file__caret" />
859
        <span class="diff-file__path">
860
          <span :if={@file.old_path} class="diff-file__from">{@file.old_path} →</span>
861
          {@file.path}
862
        </span>
863
        <span class="diff-file__status" data-status={@file.status}>{@file.status}</span>
864
        <span :if={@file.insertions > 0} class="diff-file__count" data-kind="insert">
865
          +{@file.insertions}
866
        </span>
867
        <span :if={@file.deletions > 0} class="diff-file__count" data-kind="delete">
868
          -{@file.deletions}
869
        </span>
870
      </summary>
871
872
      <p :if={@file.binary?} class="diff-file__note">
873
        Binary file. Nothing to show as text.
874
      </p>
875
876
      <p :if={not @file.binary? and @file.hunks == []} class="diff-file__note">
877
        No content change.
878
      </p>
879
880
      <div :for={hunk <- @file.hunks} class="diff-hunk">
881
        <p class="diff-hunk__header">
882
          <span class="diff-hunk__range">
883
            @@ -{hunk.old_start},{hunk.old_count} +{hunk.new_start},{hunk.new_count} @@
884
          </span>
885
          <span :if={hunk.heading} class="diff-hunk__heading">{hunk.heading}</span>
886
        </p>
887
888
        <table class="diff-lines">
889
          <tbody>
890
            <tr
891
              :for={line <- hunk.lines}
892
              id={line_id(@slug, line)}
893
              class="diff-line"
894
              data-kind={line.kind}
895
            >
896
              <td class="diff-line__number diff-line__number--old">{line.old_number}</td>
897
              <td class="diff-line__number diff-line__number--new">
898
                <a :if={line.new_number} href={"##{line_id(@slug, line)}"}>{line.new_number}</a>
899
                <span :if={is_nil(line.new_number)}>{nil}</span>
900
              </td>
901
              <td class="diff-line__marker" aria-hidden="true">{marker(line.kind)}</td>
902
              <td class="diff-line__text">
903
                <pre><code>{line.text}</code></pre>
904
              </td>
905
            </tr>
906
          </tbody>
907
        </table>
908
      </div>
909
    </details>
910
    """
911
  end
912
913
  # A path is not a DOM id: slashes and dots make `#a/b.ex` an invalid
914
  # fragment, so the anchor uses a flattened form. Path-scoped rather than
915
  # global, because one page holds many files and `#L12` alone would be
916
  # ambiguous across them.
917
  defp diff_slug(path), do: String.replace(path, ~r/[^A-Za-z0-9]+/, "-")
918
919
  defp line_id(slug, %{new_number: number}) when is_integer(number), do: "#{slug}-L#{number}"
920
  defp line_id(slug, %{old_number: number}) when is_integer(number), do: "#{slug}-R#{number}"
921
  defp line_id(_slug, _line), do: nil
922
923
  defp marker(:insert), do: "+"
924
  defp marker(:delete), do: "-"
925
  defp marker(_kind), do: " "
926
821 927
  @doc """
822 928
  The GitHub sign-in control.
823 929
lib/openagents_web/live/code_commit_live.ex modified +18 -1

@@ -47,6 +47,8 @@ defmodule OpenAgentsWeb.CodeCommitLive do

47 47
        {nil, false}
48 48
      end
49 49
50
    diff_files = OpenAgents.Diff.parse(diff)
51
50 52
    if connected?(socket) do
51 53
      Enum.each(
52 54
        ["forge:target", "forge:deploys"],

@@ -64,6 +66,8 @@ defmodule OpenAgentsWeb.CodeCommitLive do

64 66
     |> assign(:files, files)
65 67
     |> assign(:diff, diff)
66 68
     |> assign(:diff_truncated, diff_truncated)
69
     |> assign(:diff_files, diff_files)
70
     |> assign(:diff_totals, OpenAgents.Diff.totals(diff_files))
67 71
     |> assign(:receipts, Forge.receipts_for(repo, commit.sha))}
68 72
  end
69 73

@@ -224,7 +228,20 @@ defmodule OpenAgentsWeb.CodeCommitLive do

224 228
            <.alert :if={@diff_truncated} id="commit-diff-truncated" variant={:warning}>
225 229
              The diff is larger than the display bound; the tail is cut.
226 230
            </.alert>
227
            <pre class="code-source code-diff"><code>{@diff}</code></pre>
231
232
            <p :if={@diff_files != []} id="commit-diff-totals" class="diff-totals">
233
              {@diff_totals.files} {ngettext("file", "files", @diff_totals.files)} changed,
234
              <span data-kind="insert">+{@diff_totals.insertions}</span>
235
              <span data-kind="delete">-{@diff_totals.deletions}</span>
236
            </p>
237
238
            <.diff_file :for={file <- @diff_files} file={file} />
239
240
            <%!-- The parser returns [] for input it cannot read. Showing the
241
            raw text then is better than showing nothing: the diff is a report
242
            about a repository, and a shape this code has not seen should still
243
            reach the reader. --%>
244
            <pre :if={@diff_files == []} class="code-source code-diff"><code>{@diff}</code></pre>
228 245
          </.card>
229 246
230 247
          <footer class="code-footer">
lib/openagents_web/live/components_live.ex modified +52

@@ -958,6 +958,58 @@ defmodule OpenAgentsWeb.ComponentsLive do

958 958
    """
959 959
  end
960 960
961
  defp component_demo(%{item: %{slug: "openagents-diff-file"}} = assigns) do
962
    assigns =
963
      assign(
964
        assigns,
965
        :files,
966
        OpenAgents.Diff.parse(~S"""
967
        diff --git a/lib/openagents/greeter.ex b/lib/openagents/greeter.ex
968
        index 1111111..2222222 100644
969
        --- a/lib/openagents/greeter.ex
970
        +++ b/lib/openagents/greeter.ex
971
        @@ -1,8 +1,9 @@ defmodule OpenAgents.Greeter do
972
         defmodule OpenAgents.Greeter do
973
        -  def hello(name) do
974
        -    "Hello, " <> name
975
        +  def hello(name) when is_binary(name) do
976
        +    "Hello, " <> name <> "!"
977
           end
978
        +
979
        +  def hello(_other), do: {:error, :not_a_name}
980
         end
981
        diff --git a/priv/static/logo.png b/priv/static/logo.png
982
        index 4415be7..d9aaa46 100644
983
        Binary files a/priv/static/logo.png and b/priv/static/logo.png differ
984
        diff --git a/lib/old/name.ex b/lib/new/name.ex
985
        similarity index 100%
986
        rename from lib/old/name.ex
987
        rename to lib/new/name.ex
988
        """)
989
      )
990
991
    ~H"""
992
    <div class="space-y-3">
993
      <p class="text-sm text-base-content/60">
994
        Takes an <code>OpenAgents.Diff.File</code>, which the parser produces from <code>git diff-tree -p -M</code>. Unified rather than split: a split view needs
995
        about twice the width to say the same thing, and the two number gutters already
996
        carry what it is for — which line this was, and which line it is now.
997
      </p>
998
      <p class="text-sm text-base-content/60">
999
        Every line with a new-side number is a link to itself, scoped by path, so a
1000
        reader can point at a line instead of describing where it is. Click one and the
1001
        row highlights. Colour is never the only signal: the marker column says <code>+</code>
1002
        and <code>-</code>, so the diff survives greyscale.
1003
      </p>
1004
      <UI.diff_file :for={file <- @files} file={file} />
1005
      <p class="text-sm text-base-content/60">
1006
        The last two show the cases that are not code: a binary file, which says so
1007
        rather than looking unchanged, and a rename with no content change.
1008
      </p>
1009
    </div>
1010
    """
1011
  end
1012
961 1013
  defp component_demo(%{item: %{slug: "openagents-github-login"}} = assigns) do
962 1014
    ~H"""
963 1015
    <div class="space-y-3">
test/openagents/diff_test.exs added +204

@@ -0,0 +1,204 @@

1
defmodule OpenAgents.DiffTest do
2
  @moduledoc """
3
  The diff parser reads output from `git diff-tree -p -M`, which means its
4
  input is generated by something we do not control and describes a repository
5
  we do not control. So these hold two things: that the ordinary shapes parse
6
  exactly, and that the awkward ones -- truncation, binary files, headers this
7
  code has never seen -- degrade instead of raising.
8
9
  Line numbering gets the most attention. It is the whole reason for parsing a
10
  diff rather than printing it, and it is the part with an off-by-one in every
11
  direction available: the two sides stop agreeing at the first change, and
12
  each kind of line advances one side, the other, or both.
13
  """
14
15
  use ExUnit.Case, async: true
16
17
  alias OpenAgents.Diff
18
19
  describe "line numbering" do
20
    test "the two sides diverge at the first change and stay diverged" do
21
      [file] =
22
        Diff.parse("""
23
        diff --git a/a.ex b/a.ex
24
        index 1111111..2222222 100644
25
        --- a/a.ex
26
        +++ b/a.ex
27
        @@ -10,6 +10,7 @@ def run do
28
         context one
29
        -removed line
30
        +added one
31
        +added two
32
         context two
33
        """)
34
35
      [hunk] = file.hunks
36
37
      assert Enum.map(hunk.lines, &{&1.kind, &1.old_number, &1.new_number}) == [
38
               {:context, 10, 10},
39
               # A deletion exists only in the old file.
40
               {:delete, 11, nil},
41
               # Insertions exist only in the new one, and both take new numbers
42
               # from where the deletion left the new side untouched.
43
               {:insert, nil, 11},
44
               {:insert, nil, 12},
45
               # Context resumes on both, now two apart.
46
               {:context, 12, 13}
47
             ]
48
    end
49
50
    test "a hunk header without counts means one line" do
51
      [file] = Diff.parse("diff --git a/a b/a\n@@ -3 +3 @@\n-old\n+new\n")
52
      [hunk] = file.hunks
53
54
      assert {hunk.old_start, hunk.old_count} == {3, 1}
55
      assert {hunk.new_start, hunk.new_count} == {3, 1}
56
    end
57
58
    test "each hunk numbers from its own start" do
59
      [file] =
60
        Diff.parse("""
61
        diff --git a/a b/a
62
        @@ -1,2 +1,2 @@
63
        -a
64
        +b
65
        @@ -100,2 +100,2 @@ inside something
66
        -c
67
        +d
68
        """)
69
70
      assert [first, second] = file.hunks
71
      assert Enum.map(first.lines, & &1.old_number) == [1, nil]
72
      assert Enum.map(second.lines, & &1.old_number) == [100, nil]
73
      assert second.heading == "inside something"
74
      assert first.heading == nil
75
    end
76
  end
77
78
  describe "file status" do
79
    test "an added file" do
80
      [file] = Diff.parse("diff --git a/n b/n\nnew file mode 100644\n@@ -0,0 +1 @@\n+hello\n")
81
82
      assert file.status == :added
83
      assert file.insertions == 1
84
      assert file.deletions == 0
85
    end
86
87
    test "a deleted file" do
88
      [file] = Diff.parse("diff --git a/g b/g\ndeleted file mode 100644\n@@ -1 +0,0 @@\n-bye\n")
89
90
      assert file.status == :deleted
91
      assert file.deletions == 1
92
    end
93
94
    test "a rename carries both paths" do
95
      [file] =
96
        Diff.parse("""
97
        diff --git a/old/name.ex b/new/name.ex
98
        similarity index 96%
99
        rename from old/name.ex
100
        rename to new/name.ex
101
        """)
102
103
      assert file.status == :renamed
104
      assert file.old_path == "old/name.ex"
105
      assert file.path == "new/name.ex"
106
    end
107
108
    test "a binary file is flagged rather than left looking unchanged" do
109
      [file] =
110
        Diff.parse("""
111
        diff --git a/i.png b/i.png
112
        index 4415be7..d9aaa46 100644
113
        Binary files a/i.png and b/i.png differ
114
        """)
115
116
      assert file.binary?
117
      assert file.hunks == []
118
    end
119
120
    test "a modification is the default, not a special case" do
121
      [file] = Diff.parse("diff --git a/a b/a\n@@ -1 +1 @@\n-a\n+b\n")
122
      assert file.status == :modified
123
    end
124
  end
125
126
  describe "input we do not control" do
127
    test "a diff truncated mid-hunk keeps the lines it did receive" do
128
      # The upstream caps `Browse.diff/2`, so this is the normal state of a
129
      # large commit rather than a corrupt one.
130
      [file] =
131
        Diff.parse("""
132
        diff --git a/a b/a
133
        @@ -1,900 +1,900 @@
134
         one
135
         two
136
        """)
137
138
      assert length(hd(file.hunks).lines) == 2
139
      assert Enum.map(hd(file.hunks).lines, & &1.old_number) == [1, 2]
140
    end
141
142
    test "a line inside a hunk that is not a diff line is kept as meta" do
143
      [file] =
144
        Diff.parse("diff --git a/a b/a\n@@ -1 +1 @@\n-a\n\\ No newline at end of file\n+b\n")
145
146
      kinds = hd(file.hunks).lines |> Enum.map(& &1.kind)
147
      assert :meta in kinds
148
149
      # Meta advances neither side: the numbering of the lines around it is
150
      # unaffected by its presence.
151
      assert Enum.map(hd(file.hunks).lines, &{&1.kind, &1.old_number, &1.new_number}) == [
152
               {:delete, 1, nil},
153
               {:meta, nil, nil},
154
               {:insert, nil, 1}
155
             ]
156
    end
157
158
    test "an unrecognised @@ line does not raise" do
159
      [file] = Diff.parse("diff --git a/a b/a\n@@ this is not a hunk header\n")
160
      assert file.hunks == []
161
    end
162
163
    test "content before the first file header is not attributed to a file" do
164
      files =
165
        Diff.parse("commit abc123\nAuthor: someone\n\ndiff --git a/a b/a\n@@ -1 +1 @@\n+x\n")
166
167
      assert [%{path: "a"}] = files
168
    end
169
170
    test "empty and nil input parse to nothing" do
171
      assert Diff.parse("") == []
172
      assert Diff.parse(nil) == []
173
      assert Diff.parse("not a diff at all\njust some text\n") == []
174
    end
175
176
    test "a quoted path containing a space" do
177
      [file] = Diff.parse(~s|diff --git "a/with space.ex" "b/with space.ex"\n|)
178
      assert file.path == "with space.ex"
179
    end
180
  end
181
182
  describe "totals" do
183
    test "count the parsed lines, so they describe the diff actually shown" do
184
      files =
185
        Diff.parse("""
186
        diff --git a/a b/a
187
        @@ -1,2 +1,2 @@
188
        -one
189
        +uno
190
        diff --git a/b b/b
191
        new file mode 100644
192
        @@ -0,0 +1,2 @@
193
        +x
194
        +y
195
        """)
196
197
      assert Diff.totals(files) == %{files: 2, insertions: 3, deletions: 1}
198
    end
199
200
    test "an empty diff totals to nothing" do
201
      assert Diff.totals([]) == %{files: 0, insertions: 0, deletions: 0}
202
    end
203
  end
204
end
test/openagents_web/live/code_live_test.exs modified +6 -2

@@ -287,7 +287,10 @@ defmodule OpenAgentsWeb.CodeLiveTest do

287 287
      assert html =~ "docs/audit.md"
288 288
289 289
      # The ledger level publishes that a file changed, never its contents.
290
      refute html =~ "diff --git"
290
      # The diff is parsed now, so the absence to assert is the rendered
291
      # component rather than git's header line, which no longer reaches the
292
      # page in either case.
293
      refute html =~ "diff-file"
291 294
      refute html =~ @audit_heading
292 295
    end
293 296

@@ -295,7 +298,8 @@ defmodule OpenAgentsWeb.CodeLiveTest do

295 298
      browsable()
296 299
      {:ok, _view, html} = live(conn, "/OpenAgentsInc/openagents.com/commit/#{short}")
297 300
298
      assert html =~ "diff --git"
301
      assert html =~ "diff-file"
302
      assert html =~ "diff-line"
299 303
      assert html =~ @audit_heading
300 304
    end
301 305

This page updates live while a promote is in flight · changelog