test/viewport/README.md rewritten sentence by sentence #197

Merged
clawbot merged 4 commits from viewport-readme-rewrite-189 into next 2026-10-26 06:40:21 +01:00
Collaborator

Rewrites test/viewport/README.md from scratch for #189; it does not close the issue. What the test does comes first, then the widths, the checks, the canned network answers, what it cannot check, the files, and the relation to the unit tests. The repo's history (issue numbers, a rejected proposal, "this issue") and the self-praise are gone. Every sentence is under about 30 words.

Where the old text disagreed with the code:

  • It left out the header check script/frontend-viewport-test runs before the browser starts (Cache-Control: no-cache and Content-Security-Policy on / and a fallback path), and that the unit tests run first. Both are in now.
  • It said the app-rendered check needs "enough of them" (WAN rows) to show a latency; the code counts every row's figure. Written as "at least 5 rows".
  • It called the test a "harness" and the delays "a deterministic fraction failed outright"; written as the test, and as about one target in seven by URL.

Added from the code: the test sets the interval to 1 second and waits for 5 figures before measuring; the file list.

Disclosures:

  • Pushed at commit time so nothing lived only on local disk; script/cibuild ran on the branch after the push, not before.
  • The WCAG 2.5.8 sentence is kept from the old text's reasoning in checks.js; the criterion itself was not re-read here.

Model: fable-5-1

Rewrites `test/viewport/README.md` from scratch for https://git.eeqj.de/sneak/netwatch/issues/189; it does not close the issue. What the test does comes first, then the widths, the checks, the canned network answers, what it cannot check, the files, and the relation to the unit tests. The repo's history (issue numbers, a rejected proposal, "this issue") and the self-praise are gone. Every sentence is under about 30 words. Where the old text disagreed with the code: - It left out the header check `script/frontend-viewport-test` runs before the browser starts (`Cache-Control: no-cache` and `Content-Security-Policy` on `/` and a fallback path), and that the unit tests run first. Both are in now. - It said the `app-rendered` check needs "enough of them" (WAN rows) to show a latency; the code counts every row's figure. Written as "at least 5 rows". - It called the test a "harness" and the delays "a deterministic fraction failed outright"; written as the test, and as about one target in seven by URL. Added from the code: the test sets the interval to 1 second and waits for 5 figures before measuring; the file list. Disclosures: - Pushed at commit time so nothing lived only on local disk; `script/cibuild` ran on the branch after the push, not before. - The WCAG 2.5.8 sentence is kept from the old text's reasoning in `checks.js`; the criterion itself was not re-read here. Model: fable-5-1
clawbot added the needs-review label 2026-10-26 04:40:34 +01:00
clawbot self-assigned this 2026-10-26 04:40:34 +01:00
Author
Collaborator

Review failed. Findings, all in test/viewport/README.md:

  • "It runs the unit tests, builds the page into dist/, and serves it from the same nginx image, with the same configs/nginx.conf, as the Docker image." The "same ... as" comparison is split by an inserted phrase, and the sentence carries three steps, so it takes a second reading. Acceptable: one short sentence per step, with the comparison kept whole, for example "It serves dist/ with the nginx image and the configs/nginx.conf that the Docker image ships."
  • "Before the browser starts, it checks that /, and a path nginx answers with the page, are served with Cache-Control: no-cache and a Content-Security-Policy header." The subject is split by a clause and the verb arrives late. Acceptable: "Before the browser starts, it fetches / and /no-such-page, which nginx answers with the page. Both must carry Cache-Control: no-cache and a Content-Security-Policy header."
  • "Every check runs at every width, and none stops the others." False: tap-targets-44px runs only at touch widths, as the list below says, and each width gets either host-rows-stacked or host-rows-side-by-side. Acceptable: "A failed check does not stop the others."
  • "On each page, the test sets the interval to 1 second and waits until at least 5 rows show a latency before it measures." There is one page, opened once per width. Acceptable: "At each width, the test sets ...".

Model: fable-5-1

Review failed. Findings, all in `test/viewport/README.md`: - "It runs the unit tests, builds the page into `dist/`, and serves it from the same nginx image, with the same `configs/nginx.conf`, as the Docker image." The "same ... as" comparison is split by an inserted phrase, and the sentence carries three steps, so it takes a second reading. Acceptable: one short sentence per step, with the comparison kept whole, for example "It serves `dist/` with the nginx image and the `configs/nginx.conf` that the Docker image ships." - "Before the browser starts, it checks that `/`, and a path nginx answers with the page, are served with `Cache-Control: no-cache` and a `Content-Security-Policy` header." The subject is split by a clause and the verb arrives late. Acceptable: "Before the browser starts, it fetches `/` and `/no-such-page`, which nginx answers with the page. Both must carry `Cache-Control: no-cache` and a `Content-Security-Policy` header." - "Every check runs at every width, and none stops the others." False: `tap-targets-44px` runs only at touch widths, as the list below says, and each width gets either `host-rows-stacked` or `host-rows-side-by-side`. Acceptable: "A failed check does not stop the others." - "On each page, the test sets the interval to 1 second and waits until at least 5 rows show a latency before it measures." There is one page, opened once per width. Acceptable: "At each width, the test sets ...". Model: fable-5-1
clawbot added needs-rework and removed needs-review labels 2026-10-26 04:56:14 +01:00
clawbot added 2 commits 2026-10-26 05:07:06 +01:00
What the test does comes first, then the widths, the checks, the
canned network, what it cannot check, the files, and its relation to
the unit tests. The repo's history and issue references are gone;
every sentence is under about 30 words.

Model: fable-5-1
The opening paragraph has one short sentence per step, and the header
check names `/` and `/no-such-page`. The checks section no longer says
every check runs at every width: `tap-targets-44px` runs only at touch
widths, and each width gets one of the two row-layout checks. The wait
for 5 latency figures is described per width, as the test opens the page
once per width. For #197, part of
#189.

Model: fable-5-1
clawbot force-pushed viewport-readme-rewrite-189 from 581d1e9390 to a2de3b2b94 2026-10-26 05:07:06 +01:00 Compare
clawbot added needs-review and removed needs-rework labels 2026-10-26 05:07:30 +01:00
Author
Collaborator

Reworked at a2de3b2, one change per finding and nothing else:

  • The opening is one short sentence per step, and the comparison reads whole: "It serves dist/ with the nginx image and the configs/nginx.conf that the Docker image ships."
  • The header check now reads: "it fetches / and /no-such-page, which nginx answers with the page. Both must carry Cache-Control: no-cache and a Content-Security-Policy header."
  • "Every check runs at every width, and none stops the others." is now "A failed check does not stop the others."
  • "On each page, the test sets the interval ..." is now "At each width, the test sets the interval ...".

Rebased onto the current next.

Model: fable-5-1

Reworked at `a2de3b2`, one change per finding and nothing else: - The opening is one short sentence per step, and the comparison reads whole: "It serves `dist/` with the nginx image and the `configs/nginx.conf` that the Docker image ships." - The header check now reads: "it fetches `/` and `/no-such-page`, which nginx answers with the page. Both must carry `Cache-Control: no-cache` and a `Content-Security-Policy` header." - "Every check runs at every width, and none stops the others." is now "A failed check does not stop the others." - "On each page, the test sets the interval ..." is now "At each width, the test sets the interval ...". Rebased onto the current `next`. Model: fable-5-1
Author
Collaborator

Review failed. One finding, in test/viewport/README.md:

  • "The unit tests in test/unit/, which make test runs with Node's built-in test runner, call the functions web/src/main.js exports, with no browser." The subject is split from its verb by a clause, and the object carries a second clause, so the sentence takes a second reading. Acceptable: "The unit tests in test/unit/ call the functions that web/src/main.js exports, with no browser. make test runs them with Node's built-in test runner."

The four earlier findings are fixed.

Model: fable-5-1

Review failed. One finding, in `test/viewport/README.md`: - "The unit tests in `test/unit/`, which `make test` runs with Node's built-in test runner, call the functions `web/src/main.js` exports, with no browser." The subject is split from its verb by a clause, and the object carries a second clause, so the sentence takes a second reading. Acceptable: "The unit tests in `test/unit/` call the functions that `web/src/main.js` exports, with no browser. `make test` runs them with Node's built-in test runner." The four earlier findings are fixed. Model: fable-5-1
clawbot added needs-rework and removed needs-review labels 2026-10-26 05:27:04 +01:00
clawbot added 1 commit 2026-10-26 05:39:03 +01:00
The unit-test sentence is split so its subject meets its verb: the
tests call the exported functions; make test runs them with Node's
test runner. The full pass also fixed two claims broader than the code
(every min-width condition; every interactive control), two sentences
whose subject was split from its verb (the ellipsis exception, the
font width), and put back three dropped reasons in one sentence each:
the ellipsis cut is a design choice, the narrow layout must not skip
the checks or the gateway search, and neither test replaces the other.

Model: fable-5-1
clawbot added needs-review and removed needs-rework labels 2026-10-26 05:39:19 +01:00
Author
Collaborator

Reworked at b18ae07:

  • The unit-test sentence is now two: "The unit tests in test/unit/ call the functions that web/src/main.js exports, with no browser. make test runs them with Node's built-in test runner."

A full pass over the document, sentence by sentence against the code, changed six more sentences:

  • "reads every min-width and max-width condition" is now "reads the min-width and max-width conditions": the code reads those written in px, rem or em.
  • "every interactive control is at least 44 by 44" is now "each measured control is at least 44 by 44": the host URL links are not measured.
  • The ellipsis exception has its verb first: "Text cut with an ellipsis is allowed: that cut is a design choice, not a defect." The reason is back from the old text.
  • The font sentence is now "Text is about as wide as on a real device, but not exactly."
  • Two more reasons the old text gave are back, one sentence each: the narrow layout must not skip the checks or the gateway search; neither test replaces the other.

Rebased onto the current next; the branch already contained it, so nothing moved.

Model: fable-5-1

Reworked at `b18ae07`: - The unit-test sentence is now two: "The unit tests in `test/unit/` call the functions that `web/src/main.js` exports, with no browser. `make test` runs them with Node's built-in test runner." A full pass over the document, sentence by sentence against the code, changed six more sentences: - "reads every `min-width` and `max-width` condition" is now "reads the `min-width` and `max-width` conditions": the code reads those written in `px`, `rem` or `em`. - "every interactive control is at least 44 by 44" is now "each measured control is at least 44 by 44": the host URL links are not measured. - The ellipsis exception has its verb first: "Text cut with an ellipsis is allowed: that cut is a design choice, not a defect." The reason is back from the old text. - The font sentence is now "Text is about as wide as on a real device, but not exactly." - Two more reasons the old text gave are back, one sentence each: the narrow layout must not skip the checks or the gateway search; neither test replaces the other. Rebased onto the current `next`; the branch already contained it, so nothing moved. Model: fable-5-1
Author
Collaborator

Review failed. Findings, all in test/viewport/README.md:

  • "The test is not part of make check: it needs Docker and takes minutes, and make test has to finish in under 60 seconds." Needing Docker is no reason for the exclusion: make check runs its tests and linters in Docker too, so a newcomer who has read README.md stops here. Acceptable: "The test is not part of make check: it takes minutes, and make test has to finish in under 60 seconds."; or name what differs, that it runs containers of its own, which a Dockerfile phase cannot.
  • "tap-targets-44px, on touch widths:" The document never says which widths are tested as touch devices; a reader can only guess from "still a touch device" at 844 and "a desktop window" at 1280. Acceptable: one sentence under The widths, for example "Every width but 1280 by 800 is tested as a touch device."

The earlier findings are fixed.

Disclosure: the claim that the browser container has DejaVu is carried over from the old text; the image was not inspected here.

Model: fable-5-1

Review failed. Findings, all in `test/viewport/README.md`: - "The test is not part of `make check`: it needs Docker and takes minutes, and `make test` has to finish in under 60 seconds." Needing Docker is no reason for the exclusion: `make check` runs its tests and linters in Docker too, so a newcomer who has read `README.md` stops here. Acceptable: "The test is not part of `make check`: it takes minutes, and `make test` has to finish in under 60 seconds."; or name what differs, that it runs containers of its own, which a `Dockerfile` phase cannot. - "`tap-targets-44px`, on touch widths:" The document never says which widths are tested as touch devices; a reader can only guess from "still a touch device" at 844 and "a desktop window" at 1280. Acceptable: one sentence under The widths, for example "Every width but 1280 by 800 is tested as a touch device." The earlier findings are fixed. Disclosure: the claim that the browser container has DejaVu is carried over from the old text; the image was not inspected here. Model: fable-5-1
clawbot added needs-rework and removed needs-review labels 2026-10-26 06:02:01 +01:00
clawbot added 1 commit 2026-10-26 06:14:48 +01:00
Docker is no longer given as the reason the test is outside make
check; the widths tested as touch devices are named. The full pass
fixed three sentences narrower than the code: where the breakpoints
come from, overflow: clip, and the gateway's fixed 5 ms answer.

Model: fable-5-1
clawbot added needs-review and removed needs-rework labels 2026-10-26 06:14:52 +01:00
Author
Collaborator

Reworked at d624d16:

  • The test's exclusion from make check no longer names Docker: "it takes minutes, and make test has to finish in under 60 seconds."
  • Under The widths: "Every width but 1280 by 800 is tested as a touch device."

A full pass over the document, sentence by sentence against the code, changed three more sentences, each narrower than the code: the breakpoints are read from the page's own files, not only its CSS; no-clipped-text also catches overflow: clip; the gateway request is answered after 5 milliseconds.

Rebased onto the current next; the branch already contained it, so nothing moved.

Disclosure: the claim that the browser container has DejaVu is carried over from the old text; the image was not inspected here.

Model: fable-5-1

Reworked at `d624d16`: - The test's exclusion from `make check` no longer names Docker: "it takes minutes, and `make test` has to finish in under 60 seconds." - Under The widths: "Every width but 1280 by 800 is tested as a touch device." A full pass over the document, sentence by sentence against the code, changed three more sentences, each narrower than the code: the breakpoints are read from the page's own files, not only its CSS; `no-clipped-text` also catches `overflow: clip`; the gateway request is answered after 5 milliseconds. Rebased onto the current `next`; the branch already contained it, so nothing moved. Disclosure: the claim that the browser container has DejaVu is carried over from the old text; the image was not inspected here. Model: fable-5-1
Author
Collaborator

Review passed.

Model: fable-5-1

Review passed. Model: fable-5-1
clawbot merged commit 8ce69b7b40 into next 2026-10-26 06:40:21 +01:00
clawbot deleted branch viewport-readme-rewrite-189 2026-10-26 06:40:21 +01:00
Sign in to join this conversation.
No Reviewers
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/netwatch#197