Compare commits

..
Author SHA1 Message Date
yonlu f714fe513d fix(scripts): report only what generation changed, not the worktree
CI / check (push) Skipped
CI / e2e (push) Skipped
The codegen-check hook was `go generate` followed by a bare
`git diff --name-only`, which is the whole unstaged worktree rather than
the generators' output. So a commit whose staged changes were fine failed
whenever anything unrelated sat unstaged — notes, a plan document, the
next commit's files — reporting "Generated code is out of date" and then
a diffstat of files no generator has ever written. `make generate` fixed
nothing, because nothing was stale, so the message sent you looking for a
codegen problem that did not exist. Splitting one piece of work into
several commits is exactly the shape that triggers it.

The tree is snapshotted either side of `go generate` and only what moved
across it is reported. That is deliberately a snapshot rather than the
list of generated paths the issue offers as the other option: a fourth
generator is one //go:generate line away, and a path list is a second
place to remember it.

Two things it has to get right. The comparison is a *symmetric*
difference, because generation can push a file into the unstaged set or
pull it out of one — a hand-edited generated file that the generator puts
back is stale generated code just as much as a source change that
outdates it, and comparing one direction reports it as current. And the
snapshot is content, not names, or a generated file that was already
dirty and is then rewritten further keeps its name on both sides and
slips through.

Closes #131
2026-08-19 14:08:05 -04:00
logan bb7dde1963 Merge pull request 'A CI-only change is ci:, not fix(ci):' (#112) from docs/ci-commit-type into main
CI / check (push) Successful in 2m26s
CI / e2e (push) Successful in 6m31s
2026-08-19 16:23:20 +00:00
yonlu 446380e3a9 docs: a CI-only change is ci:, not fix(ci):
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / e2e (pull_request) Successful in 6m41s
CI / check (pull_request) Successful in 2m28s
The commit-analyzer reads the type and ignores the scope, so `fix` is a
patch whatever sits in the brackets. Two commits touching nothing but
.gitea/workflows/unclaim.yml were written `fix(ci):` and cut v0.2.1 and
v0.2.2 -- real releases, published to Arch, Homebrew and the APK
registry, containing no user-facing change.

CLAUDE.md already warned that a mistyped feat ships a minor version.
That was not enough, because this was not a mistyped type: `fix` was
chosen deliberately, in the belief that the (ci) scope qualified it.

The version bump is the small half, which is why this gets a paragraph
rather than a clause. A merge to main starts two workflows; if
release.yml then pushes a tag, that tag push starts four more --
arch-package, homebrew-formula, android-apk and desktop-assets -- on a
runner with capacity 1, where the APK build alone is tens of minutes
and publishes a signed artifact to a public registry. So a mistyped
type is six workflow runs, not an odd-looking changelog.

`make release-dry` answers this before the merge instead of after, and
is cheaper than any one of those runs.

The two releases are staying: they are already published, and a version
that vanishes is worse for whoever pulled it than one that turns out to
be empty.

Closes #111
2026-08-19 16:03:38 +00:00
logan e07f248cc8 Merge pull request 'Wait for the scroll range the assertion needs' (#134) from fix/133-album-dropdown-scroll-race into main
CI / e2e (push) Successful in 6m39s
CI / check (push) Successful in 2m33s
2026-08-19 16:03:17 +00:00
logan 90ac6e0825 test(e2e): wait for the scroll range the assertion needs
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m33s
CI / e2e (pull_request) Successful in 6m22s
The guard polled for `scrollHeight > clientHeight + 40` and the next
line asserted the container could be scrolled to 80, so any range in
41-79 satisfied the precondition and could not satisfy the assertion.
The grid passes through exactly that while it settles, because it
recomputes its columns after a viewport change rather than during it,
so the test read a clamped scrollTop and reported 10 against 80.

It failed CI on a pull request that changes one paragraph of CLAUDE.md
and nothing else, while WebKit passed in the same run. Reproduced
locally: 0 failures in 6 runs before #132, 2 in 9 after, 0 in 10 with
this change.

#132 is what made it reachable rather than what broke it. The queue
panel's mode is measured rather than media-queried, so a viewport
change at this width costs one more layout pass, and cover-grid settles
after it instead of before. The settled range is 330 and stable, the
main panel is 700px, and the panel is correctly display:none while
closed — there is no user-visible defect, only a wider window for a
race the spec already had.

A threshold below the value its caller depends on is not a guard, so
the target is one constant that both the guard and the assertion read.

Closes #133
2026-08-19 11:50:46 -04:00
3 changed files with 114 additions and 22 deletions
+27 -11
View File
@@ -1,6 +1,12 @@
import { test, expect } from '../support/fixtures.js';
import type { Page } from '@playwright/test';
/**
* How far the scroll test scrolls. One constant, because the guard and
* the assertion have to agree about it — they did not, which is #133.
*/
const SCROLL_TARGET = 80;
/**
* Plan 007 phase 5: expanding an album shows its tracks.
*
@@ -104,20 +110,27 @@ test.describe('the album dropdown', () => {
await app.setViewportSize({ width: 900, height: 600 });
try {
await expect.poll(() => scrollRange(app)).toMatchObject({
scrollable: true,
overflowY: 'auto',
});
// Wait for the range the assertion below actually needs, not for
// "scrollable at all" (#133). The guard used to be
// `scrollHeight > clientHeight + 40` while the next line asks to
// reach 80, so any range in 41-79 satisfied it and could not
// satisfy the assertion — and the grid passes through exactly
// that while it settles, because it recomputes its columns after
// the resize rather than during it. The settled range here is
// 330, so this waits rather than weakening anything.
await expect
.poll(() => scrollRange(app))
.toMatchObject({ room: true, overflowY: 'auto' });
await app.evaluate(() => {
await app.evaluate((target) => {
const sc = document
.querySelector('cover-grid')
?.shadowRoot?.querySelector('.grid-scroll-container');
if (sc) sc.scrollTop = 80;
});
if (sc) sc.scrollTop = target;
}, SCROLL_TARGET);
expect(await scrollTop(app)).toBe(80);
expect(await scrollTop(app)).toBe(SCROLL_TARGET);
// And the dropdown it opens is on screen, wherever the manager
// decides that leaves the scroll. It is *not* "the position is
@@ -250,16 +263,19 @@ async function closeDropdown(app: Page): Promise<void> {
/** Whether the grid can scroll at all, which decides if a probe can move. */
async function scrollRange(app: Page) {
return app.evaluate(() => {
return app.evaluate((target) => {
const sc = document
.querySelector('cover-grid')
?.shadowRoot?.querySelector('.grid-scroll-container');
return {
scrollable: !!sc && sc.scrollHeight > sc.clientHeight + 40,
// `room` is the precondition of the assertion that follows it:
// enough range to actually reach the target. A threshold below
// what the caller depends on is not a guard.
room: !!sc && sc.scrollHeight - sc.clientHeight >= target,
overflowY: sc ? getComputedStyle(sc).overflowY : '',
};
});
}, SCROLL_TARGET);
}
async function scrollTop(app: Page): Promise<number> {
+6 -11
View File
@@ -20,19 +20,14 @@ pre-commit:
glob: "*.go"
run: go tool golangci-lint run --timeout 5m ./...
# Snapshots the tree either side of the generators and reports only
# what moved across them. This used to be `go generate` plus a bare
# `git diff --name-only`, which is the *whole unstaged worktree* — so
# any unrelated edit sitting there was reported as stale generated
# code, and `make generate` then fixed nothing. See the script.
codegen-check:
glob: "*.{go,sql,templ}"
run: |
go generate ./...
if [ -n "$(git diff --name-only)" ]; then
echo "Generated code is out of date. Run 'make generate' and stage the changes."
# --no-pager, or this blocks forever on `less` waiting for a
# keypress that a hook run without a tty will never get: the
# commit hangs at exactly the moment it is trying to tell you
# why it failed.
git --no-pager diff --stat
exit 1
fi
run: ./scripts/codegen-check.sh
# frontend/bindings is generated by `wails3`, not `go generate`, so
# the check above does not cover it. ~3.5s warm, ~20s on a cold
+81
View File
@@ -0,0 +1,81 @@
#!/usr/bin/env bash
#
# Fails when `go generate ./...` would change something that is not staged.
#
# The obvious spelling of this is `go generate && git diff --name-only`,
# which is what the hook used to be, and it answers the wrong question:
# that diff is the *whole unstaged worktree*, so any unrelated edit — a
# note, a plan document, the next commit's files sitting there while this
# one lands — was reported as
#
# Generated code is out of date. Run 'make generate' and stage the changes.
#
# Running `make generate` then does nothing, because nothing generated is
# stale, and the message sends you looking for a codegen problem that does
# not exist. Splitting one piece of work into several commits is exactly
# the shape that triggers it, so the workaround was a constraint on commit
# order for no real reason.
#
# So the tree is snapshotted either side of the generators and only what
# *moved across them* is reported. That is deliberately not a list of
# generated paths: sqlcgen, `*_templ.go` and `frontend/src/events.ts` are
# today's answer, a fourth generator is one `//go:generate` line away, and
# a path list is a second place to remember it — the same reasoning that
# keeps staleshape.go parsing sql/schemas/ rather than restating it.
#
# Content, not names: a generated file that is *already* dirty and is then
# rewritten further keeps its name in both snapshots and would otherwise
# slip through.
set -euo pipefail
cd "$(dirname "$0")/.."
# name + worktree blob hash for every file that differs from the index.
# A file listed but absent (a deletion) hashes as "gone" rather than
# aborting the pipeline.
snapshot() {
git diff --name-only | while IFS= read -r f; do
if [ -f "$f" ]; then
printf '%s %s\n' "$f" "$(git hash-object -- "$f")"
else
printf '%s gone\n' "$f"
fi
done
}
# A brand-new generated file is not in either diff, because it is not
# tracked at all — the same blind spot bindings-check.sh names. Both
# snapshots are taken before the generators run.
before="$(snapshot)"
before_untracked="$(git ls-files --others --exclude-standard)"
go generate ./...
after="$(snapshot)"
after_untracked="$(git ls-files --others --exclude-standard)"
# Symmetric difference, and the symmetry is the whole point. Generation
# can push a file *into* the unstaged set (it was current, now it is not)
# or *out* of it (someone hand-edited generated output and the generator
# put it back) — and the second is stale generated code just as much as
# the first. Comparing one direction only reports "current" for it,
# which is the failure this script was written to stop.
moved="$(comm -3 <(printf '%s\n' "$before" | sort) <(printf '%s\n' "$after" | sort) |
cut -d' ' -f1 | tr -d '\t' | sort -u | grep -v '^$' || true)"
if [ -n "$moved" ]; then
echo "codegen-check: generated code is out of date." >&2
echo "Run 'make generate' and stage:" >&2
printf ' %s\n' $moved >&2
exit 1
fi
if [ "$after_untracked" != "$before_untracked" ]; then
echo "codegen-check: generation produced new files. Stage them:" >&2
comm -13 <(printf '%s\n' "$before_untracked" | sort) \
<(printf '%s\n' "$after_untracked" | sort) >&2
exit 1
fi
echo "codegen-check: generated code is current"