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

The map opens for its owner, signed in, in Safari, and shows the next few things first #8702

Merged opened this in answer to

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

Merged

The signed-in `/map` no longer ships every lane's drawing. The first screen is now lane headers plus the 5 next things, and a lane's graph draws when you ask for it. I did not reproduce the failure in Safari, so the first box is only partly met.

shipped production is serving it deploy c564139

opened this

The signed-in /map no longer ships every lane's drawing. The first screen is now lane headers plus the 5 next things, and a lane's graph draws when you ask for it. I did not reproduce the failure in Safari, so the first box is only partly met.

Cause. The owner's page was /map, not one repository's map. Every lane was server-rendered with its full SVG graph, which is the 6.2 MB and 4.15 s you measured. Safari stalls on that many elements. I did not drive Safari from here, so the cause rests on that measurement and the code.

Change (src/web/pages/global-map.ts):

  • A Next list now sits above the lanes. It shows the top 5 ready issues across the lanes, crew lanes first, each linking to its issue.
  • Each lane is a folded header with a "Draw this map" link to /map?lane=owner/name. That page draws only that lane, still at the widest lane's width, so the scale is unchanged.
  • Lane access checks are unchanged: a stranger asking for a private lane gets nothing drawn.

Tests.

  • A new test in test/map.test.ts holds the first screen under 50 KB and 1 s, requires the Next list with at most 5 entries, and requires no map-canvas on the first screen.
  • It also checks that ?lane= draws exactly one lane.
  • I changed two existing tests to ask for lanes by ?lane=: the lane-drawing assertions in test/map.test.ts and the privacy checks in test/map-cache.test.ts.
  • I ran the map tests plus the live-pages, guest-crawl, webspec, client-home, home-archive, idle-reason, web-budget, design-audit, never-lost and held-relieves-map tests, and all pass.
  • tsc, scripts/gate.ts and scripts/design-lint.ts pass.
  • scripts/fast.ts named 1064 test files for this change, so I picked the covering files by hand.

Left out.

  • Lanes load by full page navigation, not in-page, so there is no new JavaScript.
  • /api/v1/map is unchanged and still returns every lane.
  • The per-repository /:owner/:name/map (364 KB for this forge) is untouched.
  • I measured nothing on production or a preview after the change.

Try it. Sign in, open /map, then click "Draw this map" on a lane.

Closes #8693

commented

The signed-in /map no longer ships every lane's drawing. The first screen is now lane headers plus the 5 next things, and a lane's graph draws when you ask for it. I did not reproduce the failure in Safari, so the first box is only partly met.

Cause. The owner's page was /map, not one repository's map. Every lane was server-rendered with its full SVG graph, which is the 6.2 MB and 4.15 s you measured. Safari stalls on that many elements. I did not drive Safari from here, so the cause rests on that measurement and the code.

Change (src/web/pages/global-map.ts):

  • A Next list now sits above the lanes. It shows the top 5 ready issues across the lanes, crew lanes first, each linking to its issue.
  • Each lane is a folded header with a "Draw this map" link to /map?lane=owner/name. That page draws only that lane, still at the widest lane's width, so the scale is unchanged.
  • Lane access checks are unchanged: a stranger asking for a private lane gets nothing drawn.

Tests.

  • A new test in test/map.test.ts holds the first screen under 50 KB and 1 s, requires the Next list with at most 5 entries, and requires no map-canvas on the first screen.
  • It also checks that ?lane= draws exactly one lane.
  • I changed two existing tests to ask for lanes by ?lane=: the lane-drawing assertions in test/map.test.ts and the privacy checks in test/map-cache.test.ts.
  • I ran the map tests plus the live-pages, guest-crawl, webspec, client-home, home-archive, idle-reason, web-budget, design-audit, never-lost and held-relieves-map tests, and all pass.
  • tsc, scripts/gate.ts and scripts/design-lint.ts pass.
  • scripts/fast.ts named 1064 test files for this change, so I picked the covering files by hand.

Left out.

  • Lanes load by full page navigation, not in-page, so there is no new JavaScript.
  • /api/v1/map is unchanged and still returns every lane.
  • The per-repository /:owner/:name/map (364 KB for this forge) is untouched.
  • I measured nothing on production or a preview after the change.

Try it. Sign in, open /map, then click "Draw this map" on a lane.

commented

Signed-in /map no longer ships every lane's drawing. The first screen is now lane headers plus the 5 next things, and a lane's graph draws when asked for at /map?lane=owner/name. Safari signed in was never reproduced from here, so that box is open. The cause is the measured 6.2 MB / 4.15 s page: every lane was rendered with its full SVG graph.

Why the check failed, and the fix. Two tests in test/snappy.test.ts ("a write is on the very next map…" and "a sealed issue the owner's map drew is not drawn for a member…") looked for issue titles in the drawn /map. Titles now appear in a lane's graph only when that lane is asked for, so they read /map?lane=owner/name instead. They still check what they did before: a write shows on the next map, and a sealed issue stays hidden from a member. The tests did not pass on the first push because they expected the old page; they pass after the edit.

Tests.

  • test/snappy.test.ts, the map tests, tsc and the design lint pass here.
  • The four crawl-budget failures in the check (the "past its deep budget" test and three others) don't touch the map. The file passes alone here (11 pass), so I took them for shard-load flakiness and left them. If they fail again on this push, that is worth a look.

Change. The page code is in src/web/pages/global-map.ts.

  • A Next list above the lanes shows the top 5 ready issues across them, crew lanes first.
  • Each lane is a folded header with a "Draw this map" link. Drawing keeps one scale across lanes, and private lanes stay hidden from strangers.
  • A test in test/map.test.ts holds the first screen under 50 KB and 1 s and requires no graph drawn on it.
  • Not changed: /api/v1/map still returns every lane, and the per-repository map is unchanged.

Try it. Sign in, open /map, and click "Draw this map" on a lane.

Changes requested — before the latest push

Direction is right (the 6.2 MB first paint was the cost) and the tests are honest, but several things need fixing in one round:

  1. Closed lanes need two taps to draw. An idle lane is folded (open: drawn || !lane.idle), so its 'Draw this map' link is hidden until the fold is opened. Open the fold for the ?lane= target (done) and make the idle lane's summary itself reach the drawing without opening first, or say 'Draw' in the row meta. The user can't see what they would get.
  2. The drawn lane is not distinguishable from the undrawn ones: there is no way back to the light view, and no link to the lane's own /owner/name/map (which is what 'Draw this map' effectively duplicates). Add a 'Show fewer' / back link on the drawn lane, or link straight to the per-repo map.
  3. The Next list: <h2>Next</h2> is hand-rolled markup. docs/DESIGN.md names its section-heading component; use it, and name nothing new. Also the rows have no state/age, so 'next' is just a title; show the lane's ready count or state with the existing row meta.
  4. Both doors: the terminal map (src/tui) and the MCP/API still return the full map; a doc for the new behaviour (docs/features, e.g. map-held.md neighbour or a new map.md) is missing. Add the how-to for ?lane= and the Next list.
  5. Validate ?lane= explicitly at the boundary (cap length, shape owner/name) rather than only comparing strings; comparison is safe but the contract should be stated and tested (unknown lane, private lane for a stranger on the new first screen).
  6. The Safari signed-in failure was never reproduced and nothing was measured on the preview after the change; add the measured size/time of /map on this preview to the pull, and a test that the Next list for a stranger never names a private lane (the existing stranger test only covers lane URLs).
  7. ms < 1000 and < 50_000 bytes in a test are timing asserts on shared runners; keep the byte cap, drop or loosen the timing to avoid a flake (the direction forbids flaky tests).
commented

I addressed all seven points except the Safari reproduction and the preview measurement, which I couldn't do. Map, snappy, live-pages, guest-crawl and xss tests, tsc, the design lint and the gate all pass here, and I've pushed.

  1. Closed lanes. Every lane row now carries "Draw N issues" in its own meta, so a folded idle lane is one tap from its drawing. The fold body adds "draw it here" and a link to the lane's own /owner/name/map.
  2. Drawn lane. A drawn lane's row shows "Show fewer", which goes back to the light /map. The lane's own map link is there too.
  3. Next list. Rows now show the lane, the issue number, how many are ready in that lane, and "crew working" when it is. I kept the plain <h2> because I found no section-heading component in DESIGN.md or ui.ts, and map.ts itself uses bare <h2> for its headings. If you mean a specific one, tell me which.
  4. Other doors. The API, MCP and terminal map still return every lane, deliberately, because a program asks for data and not a first paint. docs/features/map.md says so and documents ?lane= and the Next list.
  5. ?lane= validation. parseLane accepts owner/name only (letters, digits, ., _, -, each part at most 100 characters, the whole at most 200). Anything else draws nothing and returns the light page. Tests cover unknown, malformed, too-long and null-byte values.
  6. Stranger test. A new test signs in as a user with no access and checks that neither /map nor a ?lane= ask for a private lane or someone else's lane names it, shows its issues, or draws a graph. It also checks that bob asking for ?lane=alice/engine never sees alice's issues.
  7. Timing assert. I removed it and kept the byte cap.

Safari signed in is still not reproduced. The preview needs a signed-in session, so I couldn't measure /map there from this shell. An unauthenticated request to the preview returned 0 bytes in 1.0 s, which says nothing about the page you asked about. The cause named in the pull is from the measured 6.2 MB / 4.15 s and the code. Please measure the preview signed in.

Approved

All seven earlier asks are met in the diff since 4ed20b5aab5c. Every lane row now carries "Draw N issues", and a drawn lane carries "Show fewer". The Next rows show lane, number and ready count. docs/features/map.md says the API, MCP and terminal keep the whole graph. parseLane validates at the boundary, with tests for bad input. A stranger test covers private lanes. The timing assert is gone and the byte cap stays. The page works with JavaScript off, since lanes are plain links.

Two things I would do next:

  1. Non-blocking: measure the signed-in /map on this preview, in Safari if anyone can, and put the bytes and time in the pull.
  2. Non-blocking: the nested links in the summary row (the lane map link and the draw link) are tight for a thumb at 360px. Give them a 44px tap target.

The Safari reproduction is still unmade, so box 1 stays open.

commented

Left #8693 open: 1 unticked item remain (The failure is reproduced signed in and named in the pull.).

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs