Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,6 @@ jobs:
# people edit is in web/, the copy is committed so a pip install needs no
# build, and a stale copy is a red build rather than two visual identities.
- run: python tools/sync_webui_assets.py --check
# The comparison page. Generated from tests/benchmark.json so the page
# cannot claim one thing while the measurement says another.
- run: python tools/gen_compare_page.py --check

lint:
runs-on: ubuntu-latest
Expand Down
97 changes: 64 additions & 33 deletions GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,10 @@ are not; the browser engine has had it since the WASM codec tier landed.

Which candidates a run is allowed to use comes from `destinations.py`, not from
here. **That is the single place the format policy lives**, and it is shared with
`web/worker.js`, `web/app.js` and the desktop UI — the same five entries with the
same numbers in all four.
`web/worker.js`, the browser UI's `web/js/` modules and the desktop UI — the same
five entries with the same numbers in all four. The browser's copy is
`web/destinations.js`, generated by `tools/gen_destinations.py` and committed; CI
regenerates it and fails on a diff.

`JpegEncoder` is hardcoded to 4:4:4 chroma. That's deliberate: on saturated
content, matching 4:4:4's quality-76 score with 4:2:0 required quality 97 and
Expand Down Expand Up @@ -259,34 +261,57 @@ is how a "no dependencies to compile" promise quietly breaks.
# The web version (`web/`)

A static port of the same engine that runs entirely in the browser, deployed at
[imgcompress-app.vercel.app](https://imgcompress-app.vercel.app). Five files:
`index.html` (landing + app shell), `app.css`, `app.js` (UI, worker pool, zip
download), `ss2.js` (the metric), `worker.js` (the engine: ladder bisection,
the bake-off, dual-backdrop transparency scoring, the never-bigger rule — a
port of `quality.py` + `core.py` + `encoders.py`).
[imgcompress-app.vercel.app](https://imgcompress-app.vercel.app).

**The engine**, unchanged and independent of any interface: `worker.js` (ladder
bisection, the bake-off, dual-backdrop transparency scoring, the never-bigger
rule — a port of `quality.py` + `core.py` + `encoders.py`), `ss2.js` (the metric)
and `destinations.js` (generated from `destinations.py`).

**The interface**, one page and nothing else. `index.html` is the dashboard;
`web/css/` holds six stylesheets, one per concern, with every colour and space
defined once in `base.css`; `web/js/` holds thirteen ES modules with a strict
dependency direction — `format` and `dom` depend on nothing, `state` holds the
store, `engine` owns the worker pool and the message contract, `queue`/`compare`/
`facts` only render, `render` schedules them, and `main` is the only module that
binds an event listener.

The interface deliberately does **not** read the `--oz-*` token layer. That layer
now serves the desktop app alone; the browser app was reset to a baseline of
system faces and a six-name palette of its own. See *One design system* below for
what is still shared and what is not.

There is one page. The marketing sections, the `/compare` and `/download` pages,
the theme switch, the synthetic demo, the lifetime savings counter and the
CSV/JSON report export were removed: none of them was part of compressing an
image.

### The first five seconds after a drop

There are two pages, not three: landing → studio. A drop starts the work with
nothing to press. The sequence that makes that acceptable is an ordering, and
the ordering is load-bearing — `probe_flow.mjs` asserts each step, because
every one of them is a thing someone will later be tempted to collapse.

1. **The untouched original is painted first.** `addFiles()` calls
`renderNow()` — synchronously, not `scheduleRender()` — so the studio and
the original's `src` are in the document immediately, and then holds
`dispatch()` until the *next* animation frame so the browser has actually
painted before an encoder is asked for anything. It costs a frame. Do not
"optimise" it away: the difference between *here is your image, now watch*
and *something happened to my file* is entirely in that ordering.
2. **A sentence, not a spinner.** `WORKING_LINE` in `app.js` is the landing
page's promise in the present tense. It is copy that ships verbatim — the
E2E compares it character for character — because its whole job is to be
recognisably the same claim that got the person to click *Choose images*.
3. **The result appears beside the original.** `mode` starts at `"split"` and
the `.wipe` animation sweeps the compressed half in over the original.
The original never leaves the stage.
4. **The result state is the control surface.** See below.
There is one page, and a drop starts the work with nothing to press. The sequence
that makes that acceptable is an ordering, and the ordering is load-bearing —
`probe_flow.mjs` and `e2e.mjs` assert each step, because every one of them is a
thing someone will later be tempted to collapse.

1. **The untouched original is painted first.** `addFiles()` in `js/intake.js`
calls the three renderers synchronously — not `scheduleRender()` — so the
original's `src` is in the document immediately, and then holds `dispatch()`
until the *next* animation frame so the browser has actually painted before an
encoder is asked for anything. It costs a frame. Do not "optimise" it away:
the difference between *here is your image, now watch* and *something happened
to my file* is entirely in that ordering.
The harness observes this frame through `imgc.holdWork(true)`, which is why
that seam exists: `dispatch` is a module binding, so there is no global to
stub.
2. **What is being tried is named, not spun.** `#stage-work` reports the format
being measured right now. Never a bare spinner: the wait should be legible
rather than merely long.
3. **The result appears beside the original.** `mode` starts at `"split"`, and
both layers live in one `#frame` at natural size so a single transform moves
them together. The original never leaves the stage.
4. **The evidence is on screen, not behind a disclosure.** The chips, the
measurements and the per-image override are three blocks in `#facts`, always
present. There is no drawer to find.

### Candidates: the chips are the format control

Expand Down Expand Up @@ -419,8 +444,12 @@ edit it — it is generated by that repo's `node build/build.mjs`, and a change
here is silently overwritten on the next sync. To change a value, change it
upstream, rebuild, and re-copy.

`web/app.css` consumes those tokens and hand-types nothing: no hex, no
`rgb()`, no `cubic-bezier`. Four of the system's rules are load-bearing here:
`imgcompress/webui/app.html` consumes those tokens and hand-types nothing: no
hex, no `rgb()`, no `cubic-bezier`. **The browser app no longer consumes them at
all** — it was reset to its own baseline in `web/css/base.css`, where the same
guarantee (values defined once, consumed by name everywhere else) is enforced by
`TheBrowserAppHasOnePlaceForValues` in `tests/test_design_system.py`. Four of the
system's rules are load-bearing for the desktop app:

* **Adjacent regions never share a surface rung.** Separation is a surface step
or space, never a border — `background` → `surface-primary` (toolbar, queue,
Expand Down Expand Up @@ -534,10 +563,12 @@ identical runs, so timings are reported as min-of-N.

Both live in `tests/web/`:

* `verify_tokens.mjs` — fails on a `var(--oz-*)` the token layer does not
define, any colour literal in `app.css`, a leftover pre-migration variable, a
weight above 600 reaching the app layer, and a declared face missing from
disk.
* `verify_tokens.mjs` — the desktop app only, since it is the only consumer of
the token layer now. Fails on a `var(--oz-*)` the layer does not define, any
colour literal in `webui/app.html`, a leftover pre-migration variable, a weight
above 600 reaching the app layer, and a declared face missing from disk. The
browser app's equivalent rules run in `tests/test_design_system.py`, without
Chrome and without Node.
* `verify_fonts.mjs` — loads the real page in Chrome and asserts the six faces
register and parse, that Bricolage and Geist are what actually paint, that
**no rendered element** computes above 600 (checked twice: empty state, then
Expand Down
3 changes: 2 additions & 1 deletion imgcompress/destinations.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
and was never told why. One list, named after destinations, is the fix.

This table is the single source of truth for the Python side. `web/worker.js`,
`web/app.js` and `imgcompress/webui/app.html` carry the same entries with the
`web/destinations.js` (generated from this file) and
`imgcompress/webui/app.html` carry the same entries with the
same numbers; if you change one, change all four. `tests/test_compress.py` has
a test per destination so the Python side cannot drift on its own.
"""
Expand Down
2 changes: 1 addition & 1 deletion tests/test_compress.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def test_every_named_format_has_an_encoder(self):
class DestinationTests(unittest.TestCase):
"""The table is a promise about where an image is going. Pin all of it.

These same five entries are duplicated in `web/worker.js`, `web/app.js` and
These same five entries are duplicated in `web/worker.js`, `web/destinations.js` and
the desktop UI, which cannot be checked from here - but the Python side is
the reference, so at least it cannot drift on its own.
"""
Expand Down
98 changes: 86 additions & 12 deletions tests/test_design_system.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,24 +204,96 @@ def test_every_token_it_references_is_defined(self):
self.assertEqual(sorted(used - defined), [])


WEB_CSS_DIR = WEB / "css"

# The browser app's stylesheets, in load order. base.css is where every colour
# and space is *defined*; the others may only consume them by name.
WEB_SHEETS = ("base.css", "layout.css", "controls.css",
"queue.css", "compare.css", "facts.css")


def _web_css(name: str) -> str:
return re.sub(r"/\*.*?\*/", "", _read(WEB_CSS_DIR / name), flags=re.S)


class TheBrowserAppHasOnePlaceForValues(unittest.TestCase):
"""The browser app was reset to a baseline of its own: system faces, a six
name palette, no brand layer. That is a deliberate break from the token
layer, which now serves only the desktop app.

A baseline still needs the property the token layer was bought for - values
defined once - so it is enforced here directly. base.css defines; every other
sheet consumes. Without this the reset would drift back into scattered
literals within a few edits, which is the state it was reset out of.
"""

def test_every_sheet_exists(self):
for name in WEB_SHEETS:
with self.subTest(sheet=name):
self.assertTrue((WEB_CSS_DIR / name).is_file(),
f"web/css/{name} is missing")

def test_index_links_them_all_with_base_first(self):
html = _read(WEB / "index.html")
seen = [html.find(f"/css/{name}") for name in WEB_SHEETS]
for name, at in zip(WEB_SHEETS, seen):
with self.subTest(sheet=name):
self.assertNotEqual(at, -1, f"index.html does not link {name}")
self.assertEqual(seen, sorted(seen),
"base.css must load before the sheets that consume it")

def test_only_base_defines_colour_literals(self):
"""A hex outside base.css is a value with no name, and a value with no
name is one nobody can change in both themes at once."""
for name in WEB_SHEETS[1:]:
with self.subTest(sheet=name):
found = re.findall(r"#[0-9a-fA-F]{3,8}\b", _web_css(name))
# compare.css paints the transparency checkerboard and the
# caliper, both of which sit on top of a photograph and must not
# follow the page theme. They are the documented exception.
if name == "compare.css":
continue
self.assertEqual(found, [], f"{name} hand-types {found}")

def test_every_token_used_is_defined_in_base(self):
defined = set(re.findall(r"^\s*(--[a-z0-9-]+)\s*:", _web_css("base.css"), re.M))
for name in WEB_SHEETS[1:]:
used = set(re.findall(r"var\((--[a-z0-9-]+)", _web_css(name)))
# Locally-set custom properties, written by JS or by a sibling rule.
used -= {"--clip", "--bar-h"}
with self.subTest(sheet=name):
self.assertEqual(sorted(used - defined), [],
f"{name} uses tokens base.css does not define")

def test_the_page_carries_no_inline_script(self):
"""The CSP forbids inline script outright rather than allow-listing a
hash. An inline <script> here fails only in production, and only after a
deploy, which is the worst way to find out."""
html = _read(WEB / "index.html")
self.assertEqual(re.findall(r"<script(?![^>]*\bsrc=)[^>]*>", html), [])


class MotionIsTokenised(unittest.TestCase):
"""The rules the brief asked for, enforced on both app layers. The full
check with its per-declaration reporting is in verify_tokens.mjs; these are
the two that must never regress.

The brief proposed a new --oz-motion-* set and a second --oz-ease-exit.
Deliberately not done: the token layer already ships --oz-duration-*,
--oz-ease-* and the --oz-spring-* pairs, so a parallel set would be the
duplication this work removes, and --oz-ease-exit already exists with a
different curve - redefining it would silently change every exit animation.
"""Two rules on both interfaces, and one that only the desktop app can meet.

The layout and `all` rules are about performance and hold anywhere: a
transition on a layout property forces reflow on every frame, and `all`
animates properties nobody chose. Those apply to the browser app's baseline
exactly as they did to its predecessor.

The literal-duration rule is different. It exists to keep one motion
vocabulary - --oz-duration-*, --oz-ease-* - and only the desktop app reads
that layer now. Holding the browser app to it would mean inventing a parallel
motion token set for a baseline whose entire point is not having one, so it is
scoped to the layer where it means something.
"""

LAYOUT = ("width", "height", "top", "right", "bottom", "left",
"margin", "padding", "inset")

def layers(self):
return [("app.css", re.sub(r"/\*.*?\*/", "", _read(WEB / "app.css"), flags=re.S)),
("webui/app.html", _desktop_css())]
return [(f"css/{name}", _web_css(name)) for name in WEB_SHEETS] + [
("webui/app.html", _desktop_css())]

def test_no_transition_touches_a_layout_property(self):
for name, css in self.layers():
Expand All @@ -241,7 +313,9 @@ def test_no_transition_uses_all(self):
f"{name}: name the properties")

def test_no_literal_durations_or_curves(self):
for name, css in self.layers():
"""Desktop only - see the class docstring. The browser app's baseline has
no motion vocabulary to be consistent with, by design."""
for name, css in [("webui/app.html", _desktop_css())]:
for kind, value in re.findall(r"(transition|animation)\s*:\s*([^;{}]+);", css):
bare = re.sub(r"var\(\s*--[a-z0-9-]+\s*(,[^)]*)?\)", " ", value)
with self.subTest(file=name, kind=kind):
Expand Down
49 changes: 36 additions & 13 deletions tests/test_destination_parity.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,13 @@
from tools import gen_destinations # noqa: E402

WORKER_JS = ROOT / "web" / "worker.js"
APP_JS = ROOT / "web" / "app.js"
# The browser UI is a module graph now, not one script. Every one of its modules
# is a consumer of the generated table and is held to the same rule, so the checks
# below scan the whole directory rather than a single file - a restated table in
# any of them is the thing being guarded against.
APP_JS_DIR = ROOT / "web" / "js"
APP_JS_FILES = sorted(APP_JS_DIR.glob("*.js"))
SETTINGS_JS = APP_JS_DIR / "settings.js"
INDEX_HTML = ROOT / "web" / "index.html"
GENERATED_JS = ROOT / "web" / "destinations.js"
DESKTOP_HTML = ROOT / "imgcompress" / "webui" / "app.html"
Expand Down Expand Up @@ -103,16 +109,18 @@ def test_the_worker_pulls_in_the_generated_file(self):
self.assertIn('importScripts("destinations.js")', _read(WORKER_JS))

def test_the_ui_declares_no_table_of_its_own(self):
source = self._source(APP_JS)
for banned in ("const DESTINATIONS", "const OLD_TARGET_NAMES"):
with self.subTest(declaration=banned):
self.assertNotIn(banned, source)
for path in APP_JS_FILES:
source = self._source(path)
for banned in ("const DESTINATIONS", "const OLD_TARGET_NAMES",
"const DESTINATION_NUMBERS", "const DESTINATION_FORMATS"):
with self.subTest(file=path.name, declaration=banned):
self.assertNotIn(banned, source)

def test_no_consumer_hardcodes_a_frame_size(self):
"""2560, 1920, 512 and 4096 are the reference's to state."""
sizes = {str(d.max_dimension) for d in dest.visible() if d.max_dimension}
sizes.add(str(dest.get("documents").hard_cap))
for path in (WORKER_JS, APP_JS):
for path in [WORKER_JS, *APP_JS_FILES]:
source = self._source(path)
# A destination name and one of its numbers on the same line is the
# signature of a restated table; either alone is innocent.
Expand Down Expand Up @@ -156,18 +164,33 @@ def test_the_format_control_types_no_destination_names(self):
self.assertNotIn(name, values)

def test_the_ui_renders_the_options(self):
self.assertIn("function renderDestinationOptions", _read(APP_JS))
self.assertIn("DESTINATION_ORDER", _read(APP_JS))
"""settings.js owns the plan's controls, so it is where the destination
list is built from the generated table."""
source = _read(SETTINGS_JS)
self.assertIn("function renderDestinationOptions", source)
self.assertIn("DESTINATION_ORDER", source)

def test_the_page_loads_the_generated_file_before_the_app(self):
"""Three scripts, in one order that matters.

destinations.js is a classic script - it has to be, because worker.js pulls
in the same file with importScripts and that cannot take a module. The bridge
hands its top-level bindings to the module graph explicitly, and main.js
reads them through window.DESTINATIONS at start-up. Any other order leaves
the app reading an undefined table.
"""
text = _read(INDEX_HTML)
gen = text.find('src="/destinations.js"')
app = text.find('src="/app.js"')
bridge = text.find('src="/js/destinations-bridge.js"')
app = text.find('src="/js/main.js"')
self.assertNotEqual(gen, -1, "index.html does not load destinations.js")
self.assertNotEqual(app, -1, "index.html does not load app.js")
self.assertLess(gen, app,
"destinations.js must load before app.js, which reads its "
"bindings at start-up")
self.assertNotEqual(bridge, -1, "index.html does not load the bridge")
self.assertNotEqual(app, -1, "index.html does not load js/main.js")
self.assertLess(gen, bridge,
"destinations.js must load before the bridge that reads it")
self.assertLess(bridge, app,
"the bridge must run before main.js, which reads "
"window.DESTINATIONS at start-up")

def test_the_desktop_control_is_generated_not_typed(self):
"""The desktop page renders the server's table instead. Different
Expand Down
Loading
Loading