| | @@ -0,0 +1,145 @@ |
| 1 | +# Handover — `blame` (branch `feat/blame`) |
| 2 | + |
| 3 | +For the orchestrator to fold into `progress.md`, `current.md` and `ui.md`. Written |
| 4 | +instead of editing those files directly, because five agents appending to them at once |
| 5 | +would conflict every time. |
| 6 | + |
| 7 | +## What shipped |
| 8 | + |
| 9 | +`GET /{handle}/repos/{name}/blame/{rev}/-/{*path}` — the same file as the blob page, |
| 10 | +read by who last changed each line. `Code · Blame · Raw` in the file header of **both** |
| 11 | +views, so the blob page gained an entry point rather than blame being reachable only by |
| 12 | +typing a URL. That is the `ui.md` "Where the next features go" row for Blame, filled in |
| 13 | +as written: *a toggle in the blob's own header, beside Raw*. |
| 14 | + |
| 15 | +Files: `src/application/blame.rs` (new), `src/infrastructure/web/blame.rs` (new), |
| 16 | +`GitQuery::blame` on `application/port.rs`, its two implementations, and three |
| 17 | +visibility 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 | + |
| 106 | +The Blame row in "Where the next features go" is done; it can move out of that table |
| 107 | +into 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. |