feat: give the built-in demo real diffs, and finish the interface pass - #8
Merged
Merged
Conversation
Expanding almost any file in the demo answered:
This source did not provide a diff for this file.
That is not the same as the file being unchanged.
Measured against production: of the 122 changed-file rows across the sixteen
commits, **16 had a diff and 106 did not**. That sentence is an honest fallback
for a live repository, which cannot be asked twice. For data the application
ships itself it only ever meant "the fixture was never finished" — and the demo
is the first thing a visitor sees.
Content is now the source of truth
----------------------------------
The fixture used to carry hand-written hunks for a handful of files and
hand-written line counts for all of them. It now carries the **complete text of
every file at every revision**, and the provider derives the rest:
- the patch, by diffing the previous revision against the new one;
- `additions` and `deletions`, counted from that patch's own hunks;
- the tree entry's size, from the bytes;
- the blob id, from the bytes — content-addressed, as Git is, so a file that
returns to earlier content correctly compares as unchanged.
None of those can drift from each other, because none of them is written by
hand. Result: **121 real unified diffs, 0 dead ends.**
`src/lib/diff/unified.ts` is the diff itself — a trimmed-prefix LCS producing
`diff -U3` output, about 200 lines and no new dependency. It is covered by a
200-seed property test that applies each patch back onto its input.
Comparisons compute the difference instead of reassembling it
------------------------------------------------------------
`compareLocalHistory` gained an optional `contentAt`. When a source can produce
file content — the demo can, for every commit — the net difference between two
points is computed from the two versions rather than reused from a single
intervening hunk. So a file touched by three of fifteen commits now shows its
actual net diff, where before it showed `aggregated` and a summed line count.
All 256 ordered pairs of the demo's commits are asserted to produce a net diff
with no `aggregated` and no `not-provided` anywhere.
That also fixed a bug this introduced in passing: the direction was read from
`forward` rather than from the requested endpoints, so a reversed comparison
diffed the wrong way round. A comparison always describes head relative to base.
Files that are deliberately not shown
-------------------------------------
Two are withheld: `package-lock.json` and `public/generated/search-index.json`.
Both are machine-written, and a lockfile diff teaches a visitor nothing about
how a history grew.
They no longer dead-end:
- the row is marked `generated` *before* it is opened, from the path, using the
same classifier the file tree uses, so the two always agree;
- opening it explains that the diff is withheld on purpose, and says the line
count is the size the fixture *declares* rather than a count taken from a
diff — because there is no diff to take it from;
- `patchOmittedReason` gained `generated` and `rename-only`, so each of these is
its own sentence instead of collapsing into the generic one.
`generated/` also joins the classifier's list of machine-written directories,
which is as common a convention as `dist/` and now benefits live repositories too.
What did not change
-------------------
Sixteen commits, the same shas, subjects, bodies, dates, tags and file paths.
`dataSource: builtin`, `htmlUrl: null`, no blob URLs, and **0 requests to
github.com** — asserted by running the whole demo with `fetch` replaced by a
throw, and by walking the provider's transitive imports to prove the GitHub
layer is not even reachable from it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ts rhythm A polish pass over the structure the previous round established. Nothing was removed and no view was restructured. Palette ------- `src/styles/tokens.css` now carries the specified scale exactly — page `#F7F8F5` on `#FFFFFF` with `#EEF3EE` behind secondary regions, accent `#2F7657`, and separate `added` / `removed` / `warning` pairs — with a dark theme built as its own scale rather than an inversion. Measured across every text role on every surface it is used on, in both themes: **41 of 41 pairs pass** (4.5:1 for text, 3:1 for borders, focus rings and chart strokes). The tightest is `removed` on `removed-soft` at 4.51:1, which is the specified pair and passes with little to spare; it is left exactly as specified rather than quietly adjusted. Replay ------ - The source line has a **row of its own** under the repository's name. Sharing the heading's line, `Built-in demo · 0 GitHub requests · This is a curated, synthetic history…` made the title read as a sentence rather than a title. - The player is **one bounded region**: previous, play, next, Latest and speed in a single cluster with the track beneath them. Speed pinned to the far right left a hole across the middle of the row that read as a break in the layout. Play stays the only filled control; previous and next are the same height without the fill; speed is compact, as a third-level setting. - The commit heading has a clearer hierarchy: position in the smallest type, the subject at 22px as the visual centre, author and sha as 13px metadata with the sha in monospace so it reads as an identifier. - The history column is **296px** and gives each subject **two lines** instead of truncating after a few words. Rows stay a fixed 80px so the virtualiser's arithmetic is still exact, and the number, date, sha and marker sit on a four-column grid so they hold their positions down the list. The full subject is in the tooltip and in the row's accessible name. - Changed-file rows use fixed columns for the status word and the line counts, so a row carrying a `generated` badge no longer pushes them out of line. Side by side ------------ Above 1280px the diff can be read in two columns; below it, and on every phone, unified is the only option — two columns of code at 390px is one unreadable column twice. `toSideBySide` derives the layout from the same hunks the unified view renders, pairing a replaced line onto one row, so the two cannot disagree about what changed. Every cell is a text node, as before, and the box scrolls inside itself. Insights -------- The milestone list opens at five commits with `Show all milestones` beside the heading, so Growth and the scope statement still fit the first screen. The control is in the heading and not below the list on purpose: below it, the button moved down the page as the list grew and the browser scrolled to follow the element it had just focused — which threw the reader to the bottom of a section they were part-way through. An end-to-end test now measures that neither the first row nor the control moves when the list expands. Home ---- The two entries are equal width and end on the same line. The demo keeps the only filled button on the screen and needs no input, so it is still the quicker path; sizing it as the smaller card made it look like the lesser option instead. The privacy note is one sentence, and the message slot under the field reserves its height so an invalid entry replaces the hint in place — asserted by measuring the layout before and after a rejection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two things a visitor chose were being thrown away. Returning to the replay lost where you were ------------------------------------------- `ReplayView` unmounts when the visitor switches to Compare or Insights, and it held its own sub-view, its diff focus and the file tree's filter, expansion and selection. So the trip back reset the sub-view to Repository, emptied the path filter and closed every folder that had been opened to get somewhere. All of it now lives in `TimeMachine`, which does not unmount, and the tree panel takes it as a controlled prop. Opening a different repository clears it, because a different repository has a different tree. An expanded diff still collapses, and that is deliberate: the sub-view and the filter are where you were, an open patch is what you were reading, and a list of collapsed files is the right thing to come back to. There is a test saying so, so it reads as a decision rather than an oversight. Opening the history drawer jumped the page to the top ----------------------------------------------------- `Overlay` captured the scroll position *after* focusing its first control. That control sits in a `position: fixed` surface at the top of the viewport, so focusing it scrolled the page to 0 — and the position then captured was 0. The page behind the drawer jumped to the top, and closing it restored the jump. Now the position is read before anything inside is focused, both focus calls pass `preventScroll`, and the restore forces a layout read before scrolling: while the body is fixed the document is only as tall as the viewport, so scrolling before it has been re-measured gets clamped to whatever fits. Measured at four offsets, opened by dispatching the click so the harness is not scrolling the button into view and measuring itself: locked at exactly the offset it opened at, restored to exactly that offset, zero drift. An end-to-end test holds it there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ME claims Four small things the screenshot pass turned up. The demo card carried a title, a button and a cost line, while the card beside it carried a field and two paragraphs of help. Stretching both to the same height left a hole in the middle of the demo card. It now opens with the same sentence the demo itself uses, which fills the space with something worth reading and gives the two entries equal weight. Hover and selection were within 0.01 of each other in luminance and both green-tinted, so the pointer tinted a row as strongly as the current commit did and the history read as having two current rows. Hover is now clearly fainter and less green. Selection keeps its accent bar, so it stays the only row that looks chosen. (`--bg-hover` is ours, not one of the palette values fixed by the brief.) Insights measured 960px while Compare measured 1080px, for no reason anybody could see. They now agree. An empty repository offered only "Change repository". Somebody who arrived to look at a history should be offered one, so it now also offers the demo — the same escape hatch every other failure state gives. The contrast claim in the README was measured once, by hand, which is how such a claim quietly stops being true. tests/contrast.test.ts reads the shipped stylesheet, composites translucent values over their backdrop, and holds 54 pairs per theme to 4.5:1 for text and 3:1 for borders, rings, chart strokes and categorical dots. It also asserts that both themes declare the same token names and that the prefers-color-scheme block is value-for-value identical to the explicit dark theme, so nobody can end up with two different dark palettes. All 108 pairs pass. The tightest are removed-line red on its own tint at 4.51:1 in light, and muted text on the selected row at 4.71:1 in dark. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ages The README described a fixture that stored diffs. It now describes the one the application ships: file operations carrying full text, with the patch, the line counts, the file size and the object id all derived from that text. A table says which shown number comes from which computation, and the section says plainly what was wrong before and why deriving everything from one source fixes it. Also corrected or added: - Compare's numbers section now says the demo computes true net diffs from content at both ends, so the "changed more than once" fallback — which is still the honest answer where only diffs are available — never applies to it. - The accessibility section points at tests/contrast.test.ts and quotes the measured figures rather than an older hand count. - The commit history's virtualised row height was still documented as 64px. - The replay section says that state survives leaving the view, and that diffs read as unified by default with an optional two-column view on wide desktop. - The Playwright section lists what the suite now asserts about the route a first-time visitor takes. The screenshot script no longer reports the deliberate 429 on the rate-limit shot as a problem; a shot of a failure state has to provoke the failure. Any other console error on that page is still reported. docs/timeline.jpg and the committed review images are regenerated from the built-in demo and the synthetic fixtures only, at the current palette and layout. The set now covers a real diff, a withheld generated file, the two-column diff, a Compare net diff, both themes, 1280x720, and 390px for the replay, the drawer, Compare and a diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 165f828ddd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+183
to
+184
| additions: forward ? (touch?.additions ?? 0) : (touch?.deletions ?? 0), | ||
| deletions: forward ? (touch?.deletions ?? 0) : (touch?.additions ?? 0), |
There was a problem hiding this comment.
When a comparison spans more than one change to an opaque file, these values remain cumulative touch totals rather than a net difference. For example, comparing the demo's first commit with its last reports package-lock.json as +2,008/−28 (the +1,840 creation plus +168/−28 modification), even though the base lacks the file and the head declares a 1,980-line file, so the correct net result is +1,980/−0. This also corrupts the comparison-wide line totals while the new generated-file fallback presents the figures as endpoint-derived; use the stored endpoint line counts for additions/removals or explicitly retain the aggregated limitation.
Useful? React with 👍 / 👎.
Comment on lines
+287
to
+291
| } else { | ||
| flush(); | ||
| rows.push({ kind: 'context', oldNumber, newNumber, text: body }); | ||
| oldNumber += 1; | ||
| newNumber += 1; |
There was a problem hiding this comment.
In the side-by-side layout, a GitHub patch containing \ No newline at end of file reaches this fallback branch. The marker is then rendered as unchanged source on both sides and both line counters are incremented, making any following context or changes in the same hunk display incorrect line numbers. Recognize this backslash-prefixed diff annotation as metadata and do not advance either counter.
Useful? React with 👍 / 👎.
Comment on lines
+62
to
+64
| const [subView, setSubView] = useState<SubView>('repository'); | ||
| const [diffFocus, setDiffFocus] = useState<{ path: string } | null>(null); | ||
| const [treeView, setTreeView] = useState<TreeViewState>(EMPTY_TREE_VIEW); |
There was a problem hiding this comment.
These values now survive ReplayView unmounts, but they are reset only by openRepo; the popstate path calls applyUrl and loads another repository without clearing them. After configuring a filter or selecting Changes in repository B and using Back to return to repository A, A inherits B's subview, filter, folder overrides, and selected path, potentially showing an empty or unexpected workspace. Reset this state whenever a URL load changes the repository, not only for picker-driven loads.
Useful? React with 👍 / 👎.
This reverses a decision made earlier on this branch. Walking the brief's own path — open a diff, look at Compare, come back — the collapse that commit 36aae0f called deliberate did not read as deliberate. It read as having lost your place. The argument for collapsing was that a patch is what you were reading rather than where you were. Two things are wrong with it. Getting back to the patch costs a scroll and a click, which is exactly the friction the sub-view and the filter are preserved to avoid. And collapsing the diff reflows the list, so the scroll position that *is* preserved lands somewhere else — the two behaviours worked against each other. `FileChangeList` now takes the open diff as an optional controlled prop, and the replay path passes it down from `TimeMachine`. Compare keeps the uncontrolled default, because its result already lives above the list. The state stays keyed by commit, so stepping to another commit still collapses it: those are different files, and following the playhead onto a path that may not exist there would be worse than closing it. Two tests now, one for each half of that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The built-in demo is the first thing most visitors see, and until this branch it
could not show what the application is for. Expanding
next.config.tsorapp/layout.tsxreached:That is an honest message about a live repository that withheld a patch. In the
application's own demo it is a dead end. This branch fixes that, and then makes
a pass over the three views, the visual system and the responsive and keyboard
paths.
Why there was no diff
The fixture stored hand-written hunks next to hand-written line counts, and
for most files it stored no hunk at all — only
additions,deletionsand astatus. So a row said
+8and then had nothing to show. Two further problemsfollowed from the same arrangement: the counts and the patch could disagree with
each other, and
Comparecould not compute a true net difference between twoarbitrary commits because it had no content to diff, so most rows reported
changed more than onceand showed only summed numbers.How it generates real diffs now
The fixture stores file content, not diffs. Each commit lists operations —
add,mod,del,ren— carrying the file's full text at that commit.Nothing about a change is written down twice:
diffText(previousText, newText)+n/−non a filediffTextbetween the two commits' stored textssrc/lib/diff/unified.tsis the diff engine: a longest-common-subsequence editscript with common prefix and suffix trimmed, three lines of context, and
diff -U3hunk headers. About three hundred lines, no dependencies, and it alsoproduces the two-column view so the unified and split renderings cannot disagree
about what changed.
Result, across the sixteen commits:
package-lock.json(twice) andpublic/generated/search-index.json. Their rows are markedgeneratedbefore you expand them; expanding explains why the content is not carried
and says their line counts are declared, not measured from a diff.
not-providednever appears in the demo.aggregated, nomissing patch, and a reversed pair swaps base, head, additions and deletions.
The demo keeps
dataSource: builtin,htmlUrl: null, exactly 16 commits, andno real repository names, commit hashes or project history.
The 0-GitHub-requests proof
Three independent arguments, all in CI:
fetchis replaced by a throw.tests/builtin-demo-diffs.test.tsbuildsthe demo, serves every commit, projects every tree and runs all 256
comparisons with
globalThis.fetchset to a function that throws. Everythingpasses, so nothing in serving the demo reaches the network.
The adapter is unreachable. An import-graph walk from the demo provider
proves no path leads to
src/lib/github/. It cannot be called by accident.Measured in a browser. Against this production build, one full repository
view of the demo:
github.laiyagushi.comrepo1,commits1,commit16,tree16,tags1,probe10Insights, theme twice, back, one stepCompare(runs the proposed pair once)SwapCompare45 is the same initial figure as
main; no request was added by any of this.The requests it does make are to this application's own routes, which serve
the demo from memory.
The rest of the pass
Replay. History is 296px with a two-line subject and a stable four-column
grid for number, date, sha and marker. The commit header re-establishes its
hierarchy — the title is the visual centre, and
Built-in demo,0 GitHub requestsand the disclosure moved off the H1 line into one quietmetadata row. The player is one bounded region:
Previous/Play/Next,Latestand the speed setting sit in a single cluster instead of leaving a holeacross the middle. Diffs read unified by default, with an optional two-column
view at ≥1280px; narrow screens are always unified.
Compare is unchanged in interaction, as intended. It benefits from the
fixture work: every row now opens on a real net diff.
Insights. Milestones collapse to five with
Show all milestonesin thesection heading rather than under the list — the old placement moved as the list
grew, and the browser scrolled to follow it, throwing the reader to the bottom
of the page.
Three real bugs the audit found, each with a test:
ReplayforCompareand coming back reset the sub-view, emptied thepath filter and collapsed the open diff, because all of it was local state in a
component that unmounts. It now lives in the shell. (Stepping to another commit
still collapses the diff — those are different files.)
top behind it: the overlay captured
window.scrollYafter focusing its firstcontrol, and that focus had already scrolled the page. Now captured first, with
preventScrollon both focus calls and a layout flush before restoring. Zerodrift at scroll offsets 0, 120 and 200.
netFileskeyed itsfromshaoff the direction flag. It always diffs base → head now.
Visual system. The palette is applied exactly as specified, in all three
token blocks.
tests/contrast.test.tsnow reads the shipped stylesheet and holds54 pairs per theme — 108 in all — to 4.5:1 for text and 3:1 for borders, focus
rings, chart strokes and categorical dots. All pass; the tightest are 4.51:1 and
4.71:1. The same file asserts both themes declare the same token names and that
the
prefers-color-schemefallback is value-for-value identical to the explicitdark theme.
Verification
npm run typechecknpm run lintnpm testmain)npm run buildnpx playwright testtokenConfigured=trueis the only credential fact exposedScreenshots are in
docs/screenshots/review-*.jpg: home, Replay commit 1,Replay commit 2, a real diff, a withheld generated file, the two-column diff,
Compare, a Compare net diff, Insights, both themes, 1280×720, and 390px for the
replay, the drawer, Compare and a diff. Every one comes from the built-in demo
or the synthetic fixtures — none from a real repository.
Walking the brief's path against a production build of this branch — Explore the
demo, Commit 1, Next to Commit 2, Repository, Changes, expand
next.config.ts,expand
app/layout.tsx, Compare, Insights, back to Replay — lands on Commit 1of 16 paused, shows a real hunk for both files, reads
f195117 is 1 commit ahead of efc489e. 8 files changed, 1,908 lines added and 1 removed., finds Insights with its milestones collapsed, and comes back toCommit 2 with the
Changessub-view and the open diff intact. 0 requests togithub.laiyagushi.com, 0 console errors. (The Vercel preview for this branch is behinddeployment protection, so the walk ran against a local production build of the
same commit; I will walk public production after the merge.)
No feature was removed, nothing became a three-column dashboard, and there is no
generated summary anywhere.
🤖 Generated with Claude Code