test(ui): make ui-visual-update honour its file filter #206

Merged
logan merged 1 commits from fix/204-ui-visual-update-filter into main 2026-08-25 16:37:53 +00:00
Collaborator

The issue. make ui-visual-update UI_ARGS=<path> re-recorded
every screenshot baseline in the repo rather than the one file
named. vitest takes the positional after a bare --update as the
flag's value, so the path was swallowed and the run had no filter at
all; two paths were worse — the first was eaten and only the second
ran. That is #196's own hazard living in the tool meant to resolve it:
the rule is "refresh the reference your change moved, and never one you
did not cause", and the documented way to refresh one blessed all ten.

The change. --update=true in the ui-visual-update recipe, with
the reason in a comment above it because =true reads like something
to tidy away, and UI_ARGS=<path> to filter in the help text.

I also ran the issue's closing sweep — every other place a variable is
interpolated after a flag (--seed, --fresh, --label, --range,
E2E_ARGS, and UI_ARGS's other two call sites). The trap is specific
to this one recipe: the rest are either value flags that always carry a
value, or hand-rolled case parsers in shell scripts. make ui-visual
and make ui-test are unaffected — their $(UI_ARGS) follows run,
with no flag to swallow it.

Verification. Mostly against vitest's collector rather than by
re-recording, since the point is which files are selected.

tier result
npx vitest list --filesOnly (honours the same filter) bare --update <path> lists 99 files; --update=true <path> lists 1. --update a b lists only b; --update=true a b lists both. The issue's measurement reproduced, then inverted.
make ui-visual-update UI_ARGS='test/components/transport.test.ts' 1 file, 40 tests, git status clean afterwards. On the pre-fix recipe that command is the whole suite.
that the flag still updates checked rather than assumed: with a deliberately wrong image in seek-bar's baseline slot the same command passes and rewrites it, where make ui-visual fails on it. The committed baseline was restored byte-for-byte from a copy taken first; the tree is clean.
make skill-check 47 documented targets, all present.

Not run, and why: no Go, no shipped frontend code and no .sql/
.templ changed, so make lint, make test, make generate and
make bindings have nothing to say here; make e2e needs a running
app and this recipe is not in that path; make ui-test exercises the
suite, not the recipe that invokes it.

Deliberately not done. No documentation edit. #196's branch (open,
unmerged) rewrites .pi/skills/yellowjacket-dev/references/ui-tier.md
and its new text carries "Until #204 lands, record one file with
--update=true" — editing the same paragraph from here would be a
conflict for the sake of one sentence. Whichever of the two merges
second should trim that bullet to the rule without the workaround.

One observation, filed nowhere because it is the property the tier is
already opt-in for: a fresh capture of seek-bar on this machine
differs byte-wise from the committed baseline while comparing green.

Closes #204

**The issue.** `make ui-visual-update UI_ARGS=<path>` re-recorded **every** screenshot baseline in the repo rather than the one file named. vitest takes the positional after a bare `--update` as the flag's *value*, so the path was swallowed and the run had no filter at all; two paths were worse — the first was eaten and only the second ran. That is #196's own hazard living in the tool meant to resolve it: the rule is "refresh the reference your change moved, and never one you did not cause", and the documented way to refresh one blessed all ten. **The change.** `--update=true` in the `ui-visual-update` recipe, with the reason in a comment above it because `=true` reads like something to tidy away, and `UI_ARGS=<path> to filter` in the help text. I also ran the issue's closing sweep — every other place a variable is interpolated after a flag (`--seed`, `--fresh`, `--label`, `--range`, `E2E_ARGS`, and `UI_ARGS`'s other two call sites). The trap is specific to this one recipe: the rest are either value flags that always carry a value, or hand-rolled `case` parsers in shell scripts. `make ui-visual` and `make ui-test` are unaffected — their `$(UI_ARGS)` follows `run`, with no flag to swallow it. **Verification.** Mostly against vitest's collector rather than by re-recording, since the point is which files are selected. | tier | result | |---|---| | `npx vitest list --filesOnly` (honours the same filter) | bare `--update <path>` lists **99 files**; `--update=true <path>` lists **1**. `--update a b` lists only `b`; `--update=true a b` lists both. The issue's measurement reproduced, then inverted. | | `make ui-visual-update UI_ARGS='test/components/transport.test.ts'` | 1 file, 40 tests, `git status` clean afterwards. On the pre-fix recipe that command is the whole suite. | | that the flag still *updates* | checked rather than assumed: with a deliberately wrong image in `seek-bar`'s baseline slot the same command **passes** and rewrites it, where `make ui-visual` fails on it. The committed baseline was restored byte-for-byte from a copy taken first; the tree is clean. | | `make skill-check` | 47 documented targets, all present. | Not run, and why: no Go, no shipped frontend code and no `.sql`/ `.templ` changed, so `make lint`, `make test`, `make generate` and `make bindings` have nothing to say here; `make e2e` needs a running app and this recipe is not in that path; `make ui-test` exercises the suite, not the recipe that invokes it. **Deliberately not done.** No documentation edit. #196's branch (open, unmerged) rewrites `.pi/skills/yellowjacket-dev/references/ui-tier.md` and its new text carries "**Until #204 lands**, record one file with `--update=true`" — editing the same paragraph from here would be a conflict for the sake of one sentence. **Whichever of the two merges second should trim that bullet to the rule without the workaround.** One observation, filed nowhere because it is the property the tier is already opt-in for: a fresh capture of `seek-bar` on this machine differs byte-wise from the committed baseline while comparing green. Closes #204
logan added 1 commit 2026-08-23 08:37:18 +00:00
test(ui): make ui-visual-update honour its file filter
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m27s
CI / e2e (pull_request) Successful in 9m27s
e5d0f2714b
vitest parses a bare `--update` as taking the next positional as its
value, so `make ui-visual-update UI_ARGS=<path>` handed the path to the
flag and ran with no filter at all: 99 files, every baseline in the repo
re-recorded, any stale one blessed in silence. Two paths were worse
still — the first was eaten and only the second ran.

That is #196's own hazard living in the tool meant to resolve it: the
rule is "refresh the reference your change moved and never one you did
not cause", and the documented way to refresh one refreshed the set.

`--update=true` is the whole fix, with the reason beside it because
`=true` reads like something to tidy away. `make ui-visual` and
`make ui-test` are unaffected — their `$(UI_ARGS)` follows `run`, with
no flag to swallow it — and no other target interpolates a variable
after a boolean flag.

Closes #204
Author
Collaborator

CI green on the first run: check (task 21828) and e2e (task 21829)
both success, over both Playwright projects.

Not merging — leaving it for review. #204 keeps Status/In Progress.

CI green on the first run: `check` (task 21828) and `e2e` (task 21829) both success, over both Playwright projects. Not merging — leaving it for review. #204 keeps `Status/In Progress`.
logan merged commit 871a3b7aac into main 2026-08-25 16:37:53 +00:00
Sign in to join this conversation.