steid

@jamesgill /

steid/plans/handover-blame.md
9.7 KBCode·Blame·Raw
e141fa6fix: the commit column takes half an 800px window, so it narrows there16h
1# Handover — `blame` (branch `feat/blame`)
2
3For the orchestrator to fold into `progress.md`, `current.md` and `ui.md`. Written
4instead of editing those files directly, because five agents appending to them at once
5would conflict every time.
6
7## What shipped
8
9`GET /{handle}/repos/{name}/blame/{rev}/-/{*path}` — the same file as the blob page,
10read by who last changed each line. `Code · Blame · Raw` in the file header of **both**
11views, so the blob page gained an entry point rather than blame being reachable only by
12typing a URL. That is the `ui.md` "Where the next features go" row for Blame, filled in
13as written: *a toggle in the blob's own header, beside Raw*.
14
15Files: `src/application/blame.rs` (new), `src/infrastructure/web/blame.rs` (new),
16`GitQuery::blame` on `application/port.rs`, its two implementations, and three
17visibility widenings plus a header swap in `web/browse.rs`.
18
19## For `progress.md` — decisions worth not rediscovering
20
21- **The porcelain parser is pure and lives in the application layer.** All the fiddly
22 knowledge about `git blame --porcelain` is about the *format*, not about the binary:
23 the commit header block is written **only the first time a commit appears**, so every
24 later run of the same commit is a bare sha line and its details have to be looked up
25 from an accumulated map. A line of code can look exactly like a header line — a file
26 full of shas would reparse itself into nonsense — and **the leading tab is the only
27 thing that tells them apart**. Both are pinned by tests.
28- **`parse_blame` never fails.** A record git spells in a shape it does not recognise is
29 skipped. Blame renders fine missing one line's attribution; refusing the page would
30 turn a future git version into an outage.
31- **`filename` is per commit, not per hunk.** git emits it inside the first header block
32 for a commit and not again, so it is carried on `BlameCommit`. That is what makes the
33 "moved from" marker work at all — measured against real output, not assumed.
34- **The use case reads the blob before blaming.** Binary, too-large and not-there are
35 then decided by exactly the rules the blob page uses, from the same `MAX_BLOB_BYTES`,
36 rather than by a second set free to disagree. It costs one `cat-file`. The alternative
37 — deriving those states from blame's own output — **cannot work**: the porcelain
38 stream carries the bytes without saying what they are, so a binary file blames into
39 garbage rather than reporting itself.
40- **A timeout is a page, and it does not matter which read timed out.** The page catches
41 `Error::GitQuery(e) if e.is_timeout()` around the whole use case, so a timeout in the
42 blob read renders the same "Blame took too long for this file" as one in blame itself.
43 Verified by temporarily setting `GIT_TIMEOUT` to 1ns (screenshotted, then restored).
44- **The commit cell is one line tall, on purpose.** Two lines and the blame and the blob
45 disagree about where line 400 is, which is the one thing that would stop them reading
46 as two views of one file. Row height was measured at 19.5px in both, matching.
47- **The age tint is linear in time, not in rank.** `AGE_STEPS = 5`, scaled between the
48 file's own oldest and newest commit. Consequence worth knowing: **a file whose history
49 is bunched shows only the end steps.** `src/infrastructure/git_query.rs` on this branch
50 has four commits at two moments a week apart, so it renders as steps 0 and 4 only,
51 and looks like a two-value marker rather than five. `plans/progress.md`, whose history
52 is spread over five weeks, uses the middle steps and is the file to look at to judge
53 the effect. Rank-based quintiles would always use all five and always look richer —
54 and would lie about the gaps. Linear was kept; if it is ever changed, that is the
55 trade being made.
56- **A file written in one commit takes the *top* step**, not the bottom. There is
57 nothing older to contrast it with, and rendering it as uniformly ancient would be a
58 claim the data does not support.
59- **The tint is plain CSS in `styles.css`**, five `.blame-age-N` rules built from
60 `color-mix(in oklab, var(--primary) N%, transparent)`. Not a Tailwind utility, because
61 the colour is a token at a fraction of an alpha; `color-mix` keeps it a token, so it
62 follows a palette change and both colour schemes without a `.dark` variant. It rides
63 on the leftmost `<td>` of **every** row in a run, so consecutive rows draw one
64 unbroken edge under `border-collapse`.
65- **Chrome headless writes screenshots to files; the puppeteer MCP tool does not.**
66 `"/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" --headless
67 --hide-scrollbars --window-size=W,H --screenshot=path URL` is how the PNGs in this
68 handover were made, with the MCP browser used for measuring computed styles.
69- **`topcoat dev` does not always pick up a rebuild**, and its symptom is silent: it
70 keeps printing `no changes; application up to date` while serving the previous binary.
71 A `pkill -f "topcoat dev"`, wait for the port to free, then restart is the fix. Two
72 restarts in a row failed with `AddrInUse` because the old process had not yet exited.
73
74## For `current.md` — holes opened, shortcuts taken
75
76- **`/{handle}/repos/{name}/commits/{sha}` is linked and does not exist on this branch.**
77 Every sha in the blame's commit column points at it. This is the URL contract with the
78 `commit-page` agent; if that branch does not land, every blame row has a dead link and
79 the fix is one function (`commit_url` in `web/blame.rs`).
80- **Line-number links point at `blob#L<n>`.** The anchors are another agent's work. Until
81 that lands the link navigates to the blob and does not scroll.
82- **The blame page has no revision switcher**, deliberately: it would cost a whole `git`
83 process (`list_refs`, ~14ms) and would need a `Switch::Blame` variant in
84 `web/browse.rs` — a shared file five agents were editing. Reconsider once the branches
85 are merged; a `Switch::Blame(&path)` arm is about four lines.
86- **`BlameCommit::boundary` is parsed and tested but only reaches the tooltip.** A
87 boundary commit is where git stopped walking, so its lines may be older than it is.
88 There is nowhere to say that on a row that must stay one line tall. If it matters
89 later, the place for it is a marker beside the sha.
90- **`BlameCommit::author_name` is shown only in the tooltip** for the same reason. The
91 row is sha, summary, date; the author did not fit, and on a personal instance every
92 commit has the same author anyway. On a multi-user instance this is the first thing to
93 revisit.
94- **The blame page makes 4 `git` processes** for a text file: `cat-file --batch-check` +
95 `cat-file blob` for the blob read, then `cat-file --batch-check` + `git blame` for the
96 blame. Two of those are the same existence question asked twice — once by the use case
97 through `read_blob`, once inside the adapter's `blame` because the port must be
98 self-contained. **A kept-alive `cat-file --batch` collapses both**, which is the same
99 upgrade 0006 defers for the per-file last-commit column. Until then it is the price of
100 the port not assuming its caller checked.
101- **Blame is the most expensive read in `GitQuery`** and is the read most likely to meet
102 the 20s timeout. Nothing but this page calls it.
103
104## For `ui.md`
105
106The Blame row in "Where the next features go" is done; it can move out of that table
107into the page's own description. Suggested text for **The repository page**:
108
109- **A file has two views and one header.** `Code · Blame · Raw` sits where Raw alone
110 used to, with the active view in the primary colour — the same rule the tab strip
111 follows, so the primary colour still means position and nothing else. Raw is never
112 "active": it downloads rather than displays.
113- **Blame's rows are the blob's rows.** Same type, same leading, same height, so the two
114 views of a file are two readings of one thing rather than two pages. The commit is
115 shown once per run of lines and is held to a single line to keep that true.
116- **The age tint is texture, not a heat map.** A hairline down the left of each run,
117 five steps of the primary colour at 5–30% alpha, scaled between the file's own oldest
118 and newest commit. It should tell you which end of a file is moving and nothing more
119 precise; if it ever reads as a value to look up, it is too strong.
120- The page's cost: **4 `git` processes**, up from the blob page's 4 (3 + the switcher).
121
122## Verified
123
124- `cargo test`**494 passed, 0 failed** (21 of them new). `cargo clippy --all-targets`
125 — zero warnings. `cargo fmt` clean.
126- Run on port 3105 from this worktree against a claimed instance holding this
127 repository's own history, `v0.1.0` and `v0.2.0` annotated tags, and a `fixtures`
128 branch carrying a 1.7 MB text file, a binary file and a one-commit file.
129- Looked at in a browser in **both colour modes** and at **800px**: no horizontal page
130 overflow (`document.scrollWidth == innerWidth`), the table scrolls inside its own
131 container, and the commit column narrows to `w-56` below `lg` so the code keeps the
132 room.
133- Every state screenshotted and looked at: many commits, one commit, a spread of tint
134 steps, a renamed file (the `` marker beside the sha, verified end to end against a
135 real `git mv`), an empty file, binary, too large, timeout, the blob header's new
136 toggle, and 800px.
137
138## Not verified
139
140- **The commit link's destination**, because the page does not exist on this branch.
141- **A real timeout.** The state was provoked by lowering `GIT_TIMEOUT` to 1ns, not by a
142 repository large enough to take 20 seconds.
143- **A file with a run of lines carried through a rename in *this repository's* own
144 history.** The marker was verified against a `git mv` pushed to the fixture branch, and
145 the parse against captured porcelain; no existing Steid source file has been renamed.