diff --git a/docs/port/DECISIONS.md b/docs/port/DECISIONS.md index 56bd351b..58e38e3c 100644 --- a/docs/port/DECISIONS.md +++ b/docs/port/DECISIONS.md @@ -2238,3 +2238,130 @@ and taking it here would be inconsistent, and it would trade an inaudible **A peak reading is not a clipping measurement.** One sample at 0 dBFS and two seconds of square wave give the same number, and I reported the first as though it were the second — twice, in red, in two milestones' write-ups. + +## The P1 regression harness had been broken since the monorepo merge, 2026-08-29 + +`tools/port/verify-screen` is the P1 gate's regression detector: Godot's drawing +of a screen against `sylpheed-cli screen render` of the same build. It had not +been run since P1, across four milestones that changed the renderer — rotation, +the focus record, the spinning ring, two builds composited at once. + +It could not have been run. **It resolves its reference binary to a path that +`build-reference-cli` stopped being able to produce.** That script greps +`crates/sylpheed-export/Cargo.toml` for + +``` +sylpheed-formats = { git = "…Syplheed-Reborn.git", rev = "…" } +``` + +and the monorepo merge (`65cefa7`) replaced that line with +`{ path = "../sylpheed-formats" }`. The grep returns nothing, the script exits 1, +and the binary left at `reference-cli/sylpheed-cli` is whatever predated the +merge — here, **three hours older than the sources** and built from a revision +nothing in the tree points at any more. + +Running the diff against it would have compared the port to a decoder from +another era and called the result a regression check. `DECISIONS.md` already +carries *"The reference renderer was stale for three diff runs"* from P2. This +would have been the fourth, and the mechanism was different: not a forgotten +rebuild, but a **build step that could no longer succeed and a consumer that +only checked whether the file existed**. + +### The fix is a deletion, not a repair + +The revision-keying solved a two-repo problem: `/reborn`'s `target/` was a live +mount of the other agent's checkout and moved mid-run, so a pixel disagreement +against it had a free variable in it. **The monorepo removed that problem by +construction** — the exporter, the reference and the port now read one decoder, +the working tree's. So `verify-screen` builds `sylpheed-cli` from the workspace. +`SYLPHEED_CLI` still overrides for anyone who wants to pin one deliberately. + +### The baseline, all 16 screens + +``` +build_00/01 max 3 over3 0 OK +press_start(_jp) max 1 over3 0 OK +title max 6 over3 790 DIFFERS +main_menu(_jp) max 4 over3 0 DIFFERS +extras(_jp) max 3 over3 0 OK +publisher_logo(_r) max 1-2 over3 0 OK +developer_logos(_r) max 2 over3 0 OK +title_jp max 155 over3 20498 DIFFERS +build_12/15 max 0 over3 0 OK +``` + +**No new drift.** Four milestones of renderer change and the only screen with a +substantial disagreement is `title_jp` — which is the *same* one P1 recorded and +left open: `ptlogo_eff2` is the single drawn element in the whole export at a +scale that is not a whole multiple of 100 % (125 %), and the two renderers pick +different source texels there. `ui_layout::blit` samples at the destination +pixel's top-left corner, a GPU at its centre. **The port has still not changed to +match**, because matching would mean reproducing a half-pixel bias on purpose to +make a number smaller. Only an oracle capture settles it. + +`title`'s 790 pixels at ≤ 6/255 are the same class, one texel wide, on the logo's +scaled edges. `main_menu` and `main_menu_jp` say DIFFERS on a max of 4 with +**zero** pixels over the bar — a couple of pixels differing in a single channel. + +### `max` alone could not tell 2 pixels from 25 000 + +The script reported only the largest difference anywhere in the frame, so +`main_menu` (two pixels) and `title_jp` (2.8 % of the frame) produced the same +verdict. It now also reports how many pixels are over the bar. + +**The bar itself is not raised.** Tuning a threshold until things match is the +failure the script's own header warns about; adding a second number is +information, not a loosened bound. ⚠️ The count is thresholded on **greyscale +luma** while `max` is a per-channel maximum, so they are not two views of one +measurement — a per-channel check counts 957 on `title` where the luma count +says 790. + +### What this harness cannot see, stated because the OK rows look reassuring + +It renders `--pose=rest`. That is deliberate — it holds both renderers to the +same declared pose so the test is *port vs reference* and not *rest vs timeline* +— but it means **none of this iteration's or the last four's visible work is +under test**: not the spinning focus ring, not the plate composited over the +title, not any timeline behaviour, not audio. Sixteen OK rows are a statement +about the resting composite and nothing else. + +And it remains what its header says: a consistency check between two renderers +that share their assumptions. Both have been wrong together three times — +`pteff05`, scale-0, `rest()` — and each time only a capture caught it. + +## Refutation — "builds 0/1 and 10/11 are the loading screen" is false in the index space this export uses + +**The claim**, from the RE agent 2026-08-29, answering the port's ask to name +`GP_TITLE`'s unnamed bundles: *"builds 0/1 and 10/11 are the loading screen, +decoded from their own `pgloading_*` element names."* + +**In this export, entries 10 and 11 are the splash screens**, and it is not close: + +| entry | elements | +|---|---| +| 10 | `palogo_eff0`, **`palogo_sqex`**, `palogo_sqex_eff` | +| 11 | `palogo_eff0`, **`palogo_gamearts`**, `palogo_seta`, `palogo_anima` … | +| 12 / 15 | `pgloading_eff00`, `pgloading_loop1`, `pgloading_str` … | + +Entry 10 is the **SQUARE ENIX** wordmark and 11 the developer logos — which the +same agent identified, in the answer to the port's ask 1, as *"entries 10/13 are +the SQUARE ENIX publisher wordmark, the first thing the boot shows"*. + +**Verdict: the finding is almost certainly right and the index space is wrong.** +Over the twelve bundles `is_build` accepts — entries 0,1,2,3,4,5,6,7,8,9,12,15 — +ordinals 10 and 11 are entries **12 and 15**, which are exactly the two dressed +loading variants. So "0/1 and 10/11" is the `is_build` ordinal, and this export +addresses by **pak entry index**. + +**Why this is worth a section rather than a shrug.** `authored/screen_names.json` +is keyed by entry index, and the exporter's own comment says why: *"keyed by +ENTRY, not by the enumeration ordinal — widening the enumeration to reach the +splash renumbers the ordinals, and a name that moves when the rule changes is not +a name."* Someone reading that message and writing keys `"10"` and `"11"` would +**name the publisher wordmark and the developer logos as loading screens**, and +the export would validate, and the boot would still run. + +Two enumerations of the same archive differ by exactly the four bundles the port +had to add an allow-list to reach. That is the sharpest possible demonstration of +why the exporter switched, and it has now nearly caused the error it switched to +prevent. Reported; the names are still the RE agent's to give. diff --git a/tools/port/verify-screen b/tools/port/verify-screen index f1ef2ada..03be9d38 100755 --- a/tools/port/verify-screen +++ b/tools/port/verify-screen @@ -53,15 +53,39 @@ set -euo pipefail cd "${PROJECT_DIR:-/work}" -# `reference-cli/`, not `release/`: the reference binary is built per pinned -# revision so a pin change cannot silently reuse the previous revision's build. -# See docker/bin/build-reference-cli. -CLI="${SYLPHEED_CLI:-${CARGO_TARGET_DIR:-/sylph-home/port/target-container}/reference-cli/sylpheed-cli}" +# THE REFERENCE IS THE WORKSPACE'S OWN `sylpheed-cli`, and that is a change. +# +# It used to be a binary built per PINNED REVISION into `reference-cli//`, +# because `sylpheed-formats` was a git dependency and /reborn's target/ was a +# live mount of the other agent's checkout that moved mid-run. A pixel +# disagreement against a moving decoder has a free variable in it. +# +# The monorepo merge (`65cefa7`) removed that problem by construction: +# `crates/sylpheed-export/Cargo.toml` now says +# `sylpheed-formats = { path = "../sylpheed-formats" }`, so the exporter, this +# reference and the port all read ONE decoder -- the working tree's. +# +# 🔴 It also silently broke the old machinery, and this script did not notice. +# `build-reference-cli` greps Cargo.toml for `Syplheed-Reborn.git", rev = "..."`; +# that line no longer exists, so the script exits 1 and the binary at +# `reference-cli/sylpheed-cli` is whatever was last built before the merge -- +# here, three hours older than the sources and from a revision nothing points +# at any more. Running the diff against it would have compared the port to a +# decoder from another era and called the result a regression check. This +# corpus has already been bitten by a stale reference renderer three times. +# +# So: build it from the workspace. `SYLPHEED_CLI` still overrides, for anyone +# who does want to pin one deliberately. +CLI="${SYLPHEED_CLI:-}" +if [ -z "$CLI" ]; then + CLI="${CARGO_TARGET_DIR:-/sylph-home/port/target-container}/release/sylpheed-cli" + cargo build --release -p sylpheed-cli >/dev/null 2>&1 || true +fi DISC="${SYLPHEED_DISC:-/disc}" OUT="${OUT:-${TMPDIR:-/tmp}/verify-screen}" export DISPLAY="${DISPLAY:-:97}" -[ -x "$CLI" ] || { echo "no reference CLI at $CLI -- run build-reference-cli" >&2; exit 2; } +[ -x "$CLI" ] || { echo "no reference CLI at $CLI -- \`cargo build --release -p sylpheed-cli\` failed?" >&2; exit 2; } [ -f export/manifest.json ] || { echo "no export/manifest.json -- run build-export --run" >&2; exit 2; } mkdir -p "$OUT" @@ -98,12 +122,24 @@ print(json.load(open("export/"+f))["source"]["build"])' "$name") read -r max mean <<<"$(convert "$OUT/$name.godot.png" "$OUT/$name.ref.png" \ -compose difference -composite -format "%[fx:maxima*255] %[fx:mean*255]" info:)" + # HOW MANY pixels are over the bar, not just how far the worst one is. A + # single `max` cannot tell 2 pixels from 25 444, and this run produced both: + # `main_menu` trips the threshold on TWO pixels out of 921 600 while + # `title_jp` trips it on 2.8 % of the frame. Reporting only the max made those + # the same verdict, which is how a real disagreement hides behind a rounding + # one. The bar itself is NOT raised -- tuning a threshold until things match + # is the failure this script's own header warns about. + over=$(convert "$OUT/$name.godot.png" "$OUT/$name.ref.png" \ + -compose difference -composite -colorspace Gray -threshold $((3*65535/255)) \ + -format "%[fx:int(mean*w*h)]" info:) + # 3/255 is what integer-truncating compositing in the CLI and float rounding # in a GPU differ by. Anything above that is a placement, order or colour # disagreement and needs a reason, not a threshold. verdict=OK awk "BEGIN{exit !($max > 3)}" && { verdict=DIFFERS; status=1; } - printf '%-16s build %-3s max %-5s mean %-8s %s\n' "$name" "$build" "$max" "${mean:0:6}" "$verdict" + printf '%-17s build %-3s max %-5s mean %-8s over3 %-7s %s\n' \ + "$name" "$build" "$max" "${mean:0:6}" "$over" "$verdict" done echo "artifacts in $OUT" exit $status