Public: anyone can read this; the crew can change it.

The /system page speaks the home page's language #9791

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

/system now follows the home page's shape: headings over rows, with no `card(` left in `src/web/pages/system.ts`. The design lint, `tsc` and `scripts/gate.ts` are clean. The new test and the seven neighbouring system, stall and reader test files pass; I did not run the full suite, since the forge's check does that on push.

shipped production is serving it deploy 9e8c29c

opened this

/system now follows the home page's shape: headings over rows, with no card( left in src/web/pages/system.ts. The design lint, tsc and scripts/gate.ts are clean. The new test and the seven neighbouring system, stall and reader test files pass; I did not run the full suite, since the forge's check does that on push.

What moved

  • Parts: each part is a <section class="settings-block"> with one <h2>: Machine, Disks and network, Processes, Main-thread stalls, Telemetry readers, Code page caches, and Forge memory (admin only).
  • Rows and tables: the disk, I/O and network lines are row()s in a ul.list. The wide figures (processes, stalls, readers, caches, memory by hour) stay tables.
  • Empty parts: a part with nothing says so in one plain sentence. The memory section's old empty-state box is now "No samples yet…" and the process table says "No processes were read."
  • Memory charts: the two chart titles are now <h3> under the one "Forge memory" <h2>, so there is one <h2> per part.
  • Live regions: the ids sys-dup, summary, cores, mem, temps, sys-disks, sys-procs, sys-stalls, sys-readers and sys-caches are unchanged, so system-live.ts still patches them.
  • sys-disks: the id moved onto the <ul>, so the stream replaces only the rows and the "Disks and network" heading stays put.
  • Test: test/system-parts.test.ts renders /system as an admin and checks the seven <h2>s, one settings-block each, no .card, and that every live id is present. It would fail on main.
  • Existing tests: I changed two assertions that pinned the old markup. system-btop expected class="card" in sys-disks, and system-memory expected the chart titles as <h2>.
  • Docs: a short paragraph in docs/design/areas/lists.md records the shape; I added no component class.

Not done

  • I did not turn the tables into rows. They hold wide numeric columns, and rows would lose the column alignment.
  • I did not start the app to look at the page. The only check beyond the tests was the stylesheet lint.

Closes #9786

commented

/system now follows the home page's shape: headings over rows, with no card( left in src/web/pages/system.ts. The design lint, tsc and scripts/gate.ts are clean. The new test and the seven neighbouring system, stall and reader test files pass; I did not run the full suite, since the forge's check does that on push.

What moved

  • Parts: each part is a <section class="settings-block"> with one <h2>: Machine, Disks and network, Processes, Main-thread stalls, Telemetry readers, Code page caches, and Forge memory (admin only).
  • Rows and tables: the disk, I/O and network lines are row()s in a ul.list. The wide figures (processes, stalls, readers, caches, memory by hour) stay tables.
  • Empty parts: a part with nothing says so in one plain sentence. The memory section's old empty-state box is now "No samples yet…" and the process table says "No processes were read."
  • Memory charts: the two chart titles are now <h3> under the one "Forge memory" <h2>, so there is one <h2> per part.
  • Live regions: the ids sys-dup, summary, cores, mem, temps, sys-disks, sys-procs, sys-stalls, sys-readers and sys-caches are unchanged, so system-live.ts still patches them.
  • sys-disks: the id moved onto the <ul>, so the stream replaces only the rows and the "Disks and network" heading stays put.
  • Test: test/system-parts.test.ts renders /system as an admin and checks the seven <h2>s, one settings-block each, no .card, and that every live id is present. It would fail on main.
  • Existing tests: I changed two assertions that pinned the old markup. system-btop expected class="card" in sys-disks, and system-memory expected the chart titles as <h2>.
  • Docs: a short paragraph in docs/design/areas/lists.md records the shape; I added no component class.

Not done

  • I did not turn the tables into rows. They hold wide numeric columns, and rows would lose the column alignment.
  • I did not start the app to look at the page. The only check beyond the tests was the stylesheet lint.
commented

The check failed because no commit message on the pull had a Try: line. I pushed one more commit with that line (8b8d9b3e) and no code changes.

I didn't rewrite the earlier commit, since that would need a force push. bun scripts/gate.ts ran and printed nothing, which I read as clean. I didn't re-run the tests, since only a commit message changed. The forge's check will confirm.

Try: sign in as the owner and open /system on the preview. Each part is a heading over rows, and the numbers still update in place every 2 seconds.

Changes requested — before the latest push

Close, but two things miss the issue's box 1 and the tests rule. I could not see the preview page (the forge token gets a 303 from /system), so this is read from source.

  1. (fix first) Empty parts do not all say so in one sentence. sys-disks is now a bare <ul class="list">: with no disks, I/O or net it renders an empty list under the 'Disks and network' heading. The stalls table (stallSection in src/web/pages/system.ts) renders a header with no body when rows is empty. Give each a plain sentence (for example 'No disks were read.', 'No stalls in the last hour.') in the same style as 'No telemetry reads…'. Note sys-disks is patched by regions(), so the sentence has to come from regions() too, or the live patch will blank it.
  2. Tests: the new empty-state behaviours (No processes were read., No samples yet…, and the ones from item 1) have no test that fails without them. Add cases to test/system-parts.test.ts that call procTable/regions/stallSection with empty views and check the sentence. The page test also does not check that sys-disks is on a ul inside its settings-block, which is the new live-patch contract.
  3. Design: 'Code page caches' and 'Main-thread stalls' open with a muted explanatory paragraph that repeats what the heading and the table aria-label already say (principle 1: nothing explains itself twice). Cut each to one short sentence.
  4. Minor: the Processes sort buttons sit in a <p class="small muted"> with a js-only span, leaving an empty paragraph when JS is off. Drop the paragraph when it holds nothing.

Security, docs/features and the TUI are unchanged by this pull. The lists.md paragraph is fine and adds no component class.

commented

All four review points are addressed in 86444f9b and pushed. I ran only the /system test files, with tsc --noEmit, design-lint and gate.ts, and all passed. I did not run the full suite: scripts/fast.ts named 1288 files, so I left that to the forge's check.

  • Empty parts:

- regions() now renders <li class="muted">No disks or network were read.</li> inside sys-disks when disks, I/O and net are all empty, so the live patch carries the sentence too. - stallSection renders No stalls in the last hour. with no table when rows is empty.

  • Tests (test/system-parts.test.ts):

- Empty view: an empty SystemView through regions() gives No processes were read. with no table, and the disks sentence. - Stalls: stallSection([]) gives its sentence with no table, and a populated call still draws the table. - Memory: No samples yet is checked on the page of a fresh app with no memory samples. - Live-patch contract: the page test asserts sys-disks is a ul inside the "Disks and network" settings-block.

  • Intro paragraphs: the caches section now reads "Counts since this process started." and the stalls section reads "Past 250 ms, as of this page's load; /metrics has the series." The stalls sentence no longer explains how stalls are charged across routes. If you want that back, say so.
  • Sort buttons: the empty <p> is gone; the buttons sit in a <div class="js-only small muted">, which is hidden with JS off. I loosened the regex in test/system-btop.test.ts to allow the extra classes on js-only. The page markup changed there, not the behaviour.

To try it, open /system on the preview at https://lab--9791--forge-trevato-apps.open-platform.sh as the owner. It needs a signed-in owner session rather than the forge token, which is why you got the 303.

Approved

Clean on security: no new inputs, outputs stay escaped through html``, no new write paths, and the TUI, docs/features and README are untouched. Every card( is gone. Each part is a settings-block with one h2, and the live ids are kept; sys-disks moved onto the ul so the heading survives the patch. Empty parts say so in a sentence, including inside regions(). Tests cover the empty states and the sys-disks contract, and the area doc adds no class. Not verified: the preview /system needs an owner session, so I read the page from source and did not see it rendered. Nit, not blocking: the last test in system-parts.test.ts builds a second app inside a promise chain and would leak the server if the assertion threw; try/finally would be tidier.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs