Skip to content

test(packets): make the Details advert-link E2E independent of fixture age (#244) - #252

Merged
dborup merged 1 commit into
masterfrom
codex/issue-244-details-click-e2e
Oct 5, 2026
Merged

dborup merged 1 commit into
masterfrom
codex/issue-244-details-click-e2e

Conversation

@dborup-agent

Copy link
Copy Markdown
Collaborator

Relates to #244

Problem

The Kpa-clawbot#1122 Playwright step advert links in Details stay visible and clickable failed at 1200 and 900 px whenever it ran about 13 min or more after tools/freshen-fixture.sh:

advert link in Details is not hit-testable: {"found":true,"hitIsLink":false,"text":"R5-D4 300D Rak"}

Cause

Two things combined:

  1. Time-dependent layout. makeColumnsResizable() (public/app.js) sizes the packets columns once, from the rows of the first render. Later time-window changes keep those widths. The test opened #/packets with the default 15-min window. That window empties as the fixture ages: in CI it held only a few packets about 13 min after freshen, and none after 15 min. The one-shot measurement also credits the full-width colspan row (the vscroll spacer, or "No packets found") to the first column. So once the window is (nearly) empty, the expand column gets about 43 % of the table and Details about 5 % (64 px at 1200 px). Advert names then wrap onto the 2nd line, which the fix(packets): keep the Details summary on one line so rows stay bounded #67 clamp hides.
  2. Wrong probe point. The test hit-tested the centre of the link's bounding box. For a wrapped inline link, that box is the union of all its line fragments, so the centre lies on a hidden line. At 1200 px the hit was the next row's td.col-details; at 900 px it was the .col-details-clip span.

The failing CI attempt reached the test 13m45s after freshen; the passing retry reached it after 12m03s.

Fix (test only)

  • Open #/packets?timeWindow=1440 (180 at ≤1024 px, where longer windows are disabled). The one-shot column measurement then sees the same newest rows for the whole e2e job (150-min timeout).
  • Check every on-screen advert link, not just the first. The visible part of its first line fragment, inside the clip, must be ≥ 12 px wide and must hit-test to the link.
  • Pin a fixture row with a long name (e8b09a35ac87fa5c, "KN6PLV-BrkOxfLA-Yebes", 21 chars). The test requires that this row is on screen and is cut by the clip, so it cannot pass without exercising a long advert name.
  • Failure messages now name the element that was hit, the link's fragments and the clip box.

There is no product change in this PR. The underlying column-measurement behaviour of makeColumnsResizable() is a real UX issue: with a quiet default window, advert names in Details can be hidden entirely. It needs a JS fix and is left for a follow-up; details are in the PR report.

Evidence (local, CI-like fixture: freshen, inline seed, corescope-migrate, seed-2073, seed-199)

Fixture age Frontend Result
0 min public/ 20/20
20 min public/ 20/20
120 min public/ 20/20
30 min public-instrumented/ 20/20

Master's version of the test fails at 1 h, 2 h and 3 h, and on the instrumented 30-min fixture, with the exact CI message.

Mutants (all red):

  • Without the window pin, at fixture age 20 min: 3/5 advert links show < 12px of their name ("CLTR Repeater" and "KN6PLV-…" sit entirely on the hidden line).
  • With the fix(packets): keep the Details summary on one line so rows stay bounded #67 clamp CSS removed: the one-line, < 60 px and long-name steps fail.
  • With an overlay pseudo-element over .col-details-clip: 5/5 advert links ... are not hit-testable.

sh test-all.sh: 217/217 passed. scripts/check-xss-sinks.sh --diff: no public/ changes.

🤖 Generated with Claude Code

…e age (#244)

The Kpa-clawbot#1122 step "advert links in Details stay visible and clickable" failed
in CI whenever it ran ~13+ min after freshen-fixture.sh.

Cause: makeColumnsResizable() (app.js) sizes the packets columns once,
from the rows of the first render. The test opened /packets with the
default 15-min window, which empties as the fixture ages; the measured
layout then gave the expand column ~43% and Details ~5% (64px at 1200px).
Advert names wrapped onto the clamped-away 2nd line, and the test probed
the centre of the link's bounding box, which is on that hidden line (the
hit was the next row's Details cell, or the clip itself at 900px).

- Open /packets with the widest selectable window (24 h, 3 h at
  <=1024px), so the one-shot measurement sees the same rows for the
  whole e2e job.
- Check every on-screen advert link: the visible part of its first line
  fragment must be >= 12px and hit-test to the link. Pin a fixture row
  with a 21-char name and require that it is cut by the clip.
- Failure messages name the element that was hit and the fragments.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dborup-agent

Copy link
Copy Markdown
Collaborator Author

Rapport — CS-pve-agent2 PR#252 #244 — head 0e8f656

Status: Test-only fix ready (draft). Local evidence is complete; CI Go Build & Test passed, and the Playwright E2E job was still running when this report was written.

Evidence tags: [T] tested or run here, [A] assumption or approximation, [K] known from CI logs or code.

Cause

  • [K] The failing CI attempt (run 37274272380, attempt 1) reached the test 13m45s after freshen; the passing retry reached it after 12m03s.
  • [T] Reproduced locally: master's test fails deterministically at fixture age 1 h, 2 h and 3 h, and at 30 min with the instrumented frontend. It passes when fresh.
  • [T] Element actually hit: at 1200 px, the next row's TD.col-details ("1f → 8b"); at 900 px, the SPAN.col-details-clip itself. The link "R5-D4 300D Rak" had a bounding box 51 px tall in a 29 px row. It had wrapped into 3–5 line fragments, and the test probed the centre of that box, which lies on a clamped, hidden line.
  • [T]/[K] Why the layout depends on age: makeColumnsResizable() (app.js) sizes the columns once, from the first render under the default 15-min window. The window empties as the fixture ages (a handful of rows at about 13 min, none after 15 min). The one-shot measurement also credits the full-width colspan row to column 0. Inline th widths: fresh, expand 15 % / Details 15 %; aged, expand 43.5 % / Details 5.4 % (64 px at 1200 px).
  • [T] This is a real UX issue, not only a test issue. In the aged layout, "CLTR Repeater" and "KN6PLV-BrkOxfLA-Yebes" sit entirely on the hidden 2nd line, so only the icon shows. It cannot be fixed in CSS: width: 32px !important on th.col-expand had no effect, because Chrome's auto layout still gives the leftover width to that column. A JS-side fix is out of scope for this PR.

Fix (test only, test-issue-1122-details-row-clamp-e2e.js)

  • Open #/packets?timeWindow=1440 (180 at ≤1024 px). The one-shot measurement then sees the same rows for the whole job.
  • Check every on-screen advert link: the visible part of its first fragment inside the clip must be ≥ 12 px and must hit-test to the link.
  • Pin the long-name row e8b09a35ac87fa5c (21 chars). It must be on screen and cut by the clip.
  • Failure messages name the hit element, the fragments and the clip box.

20/20 runs (CI-like prep: freshen, inline seed, migrate, seed-2073, seed-199) [T]

Age Frontend Result
0 min public 20/20
20 min public 20/20
120 min public 20/20
30 min public-instrumented 20/20

[A] Aging is simulated by shifting all now-derived timestamps back after the prep.

Mutants [T]

Other checks [T]

  • sh test-all.sh: 217/217 passed.
  • Full Playwright step from deploy.yml locally (instrumented, fresh fixture): every file reported 0 failed.
  • scripts/check-xss-sinks.sh --diff: clean (no public/ changes).
  • Fork guards unchanged: 9 and 1.

CI per job [K]

  • Go Build & Test: pass
  • Release Artifacts: skipped
  • Playwright E2E Tests: pending when this report was written; needs a re-check.

Remaining

  • The E2E CI result still needs confirming.
  • Follow-up suggested: fix makeColumnsResizable() so it ignores colspan rows and does not size columns from an empty first window.

@dborup-agent

Copy link
Copy Markdown
Collaborator Author

Rapport — CS-pve-agent2 PR#252 #244 — head 0e8f656

Status: CI green on head 0e8f6569 (third E2E attempt). The Kpa-clawbot#1122 test passed 18/18 in all three attempts; the two red attempts failed only on an unrelated #226 flake, root cause below.

This follows up the earlier report, whose CI section said E2E was pending.

Evidence tags: [T] tested here, [K] known from CI logs or code.

CI per job [K] (run 37293748200)

Attempt Freshen → Kpa-clawbot#1122 test Kpa-clawbot#1122 Details clamp Job result
1 13m39s 18/18 passed fail: test-issue-226-hash-stats-sort-e2e.js, step "a hostile ?mbsort= keeps the server order" (not the server order)
2 13m47s 18/18 passed fail: same #226 test, steps "a plain visit keeps the server order" and "a hostile ?mbsort="
3 11m39s 18/18 passed pass (whole E2E step)
  • Go Build & Test: pass. Build & Publish Docker Image: pass. Release Artifacts, Deploy Staging and Publish Badges: skipped.
  • [K] Before this change, master's version of the test failed at 13m45s (run 37274272380, attempt 1). Here it passed at 13m39s and 13m47s.

Unrelated flake: #226 Hash Stats adopters sort E2E [K]

Remaining

@dborup-agent

Copy link
Copy Markdown
Collaborator Author

Review — CS-pve-agent1 PR#252 details-e2e — head 0e8f656

Dom: APPROVE med nits

Read-only review of head 0e8f65696b57, using the head tree and the merged tree (git merge-tree --write-tree origin/master with master d05b0db5), each extracted with git archive. Nothing was pushed or changed.

Evidence tags: [T] tested or run by me, [A] assumption or approximation, [K] known from code, CI logs or GitHub metadata.

Findings

# Severity Finding Evidence
1 Nit (heads-up for the follow-up) The guard pinned.truncated depends on today's column widths, which come from the miscredited colspan row. When makeColumnsResizable() skips colspan rows, the long name fits: Details grows from 178 to 301 px at 1200. The step then fails at 1200 and 900 with pinned advert name fits its Details clip. The follow-up fix has to update this guard, for example by checking truncation only at 375 px, or by picking a row whose name overflows as measured. Not a blocker: today the guard is correct and it prevents a vacuous pass. [T]
2 Nit PINNED_ADVERT_ROW hard-codes a hash from the fixture. If the fixture is recaptured, the test fails, but with a clear message (pinned advert row … is not on screen). Acceptable. [K]
3 Info The 3 h pin at ≤ 1024 px keeps a full first render (29 data rows, same widths) until the fixture is about 165 min old. At 175 min it renders 13 data rows with the same widths. At 190 min the window is empty and Details is back to 104 px. The e2e job timeout is 150 min, and the test runs about 12–14 min after freshen, so the margin is fine. The comment in the test is accurate. [T]
4 Info (UX write-up) The colspan miscredit also hurts with a full window, not only with an empty one. See "UX remainder" below. [T]

No blocking issues.

1. Is the cause analysis right? Yes [T]

  • I reproduced it with master's test against a CI-like fixture aged by shifting timestamps back [A]:

    • aged 20 min: 2/2 red;
    • aged 120 min: 2/2 red;
    • instrumented frontend, aged 20 min: 2/2 red.

    Each run fails at 1200 and 900 px with the exact CI message: advert link in Details is not hit-testable: {"found":true,"hitIsLink":false,"text":"R5-D4 300D Rak"}. Fresh (age 0): 2/2 green.

  • I captured the column widths at the moment makeColumnsResizable() runs (1200 px, default 15-min window):

    Fixture age First render Expand Details Details px
    0 min 29 data rows + 1 spacer 14.97 % 15.14 % 178
    13 min 3 data rows + 2 colspan 18.88 % 11.48 % 135
    20 min only No packets found 43.54 % 5.44 % 64

    These match the report.

  • The element actually hit at 20 min / 1200 px: the probe at the box centre lands on the next row's TD.col-details. "R5-D4 300D Rak" has 5 line fragments and a 51 px box. At 900 px it lands on SPAN.col-details-clip. This matches the report.

  • Also [T]: even when fresh, the box centre of the pinned KN6PLV-BrkOxfLA-Yebes link (3 fragments) hits SPAN.col-details-clip. The old test passed only because it returned after the first on-screen link. The new rule (check every link at the visible part of its first fragment) is the right fix.

  • CI [K]: in run 37293748200, attempts 1 and 2 reached the bug(packets): filter UX disaster — help panel overlaps table, toolbar chaotic, path chips spill rows Kpa-clawbot/CoreScope#1122 test 13m39s and 13m47s after freshen with no ✗ in it. Their only failures were the fix(analytics): Hash Stats multi-byte adopters column sort reads the wrong column and has no state #226 steps (not the server order). Attempt 3 is green.

2. Does the fix weaken the test? No [T]

All mutants ran against the PR test with a fresh fixture, 2 runs each, all red:

Mutant Result
#67 clamp CSS removed (.col-details-clip -webkit-line-clamp block) red at 1200/900: Details summaries wrap, rows >= 60px, and the advert step
Overlay ::after (inset 0) over .col-details-clip red at all 3 viewports: 5/5 advert links … are not hit-testable
pointer-events: none on the advert links red at all 3 viewports: 5/5 … not hit-testable
Details squeezed (th.col-details { width: 3% !important }) red at 1200: 3/5 advert links show < 12px
Test without the window pin, aged 20 min and 120 min red at 1200: 3/5 … < 12px ("CLTR Repeater" etc.)
Same, fresh (control) green 3/3

So the new test catches what the old one did (overlay, hit-test), and also the real "name hidden" case that the old one missed.

3. Stable at different ages? Yes [T]

I ran the merged-tree test against a fresh CI-like prep each time: freshen, inline seed SQL from deploy.yml, corescope-migrate, seed-2073 and seed-199. Aging was simulated by shifting every now-derived timestamp back [A].

Fixture age public/ public-instrumented/ (scripts/instrument-frontend.sh)
0 min 10/10 5/5
20 min 10/10 5/5
120 min 10/10 5/5

With the pin, the first render at 1200 px (24 h window) and at 900 px (3 h window) has identical widths at 0, 120 and 150 min [T].

4. UX remainder: described correctly? Mostly. It is worth an issue.

  • [T] The one-shot measurement and the "an empty first window squeezes Details" part are correct. So is the claim that CSS cannot fix it: th/td.col-expand { width: 32px !important } left expand at 515 px.
  • [T] One addition: the colspan miscredit also applies with a full window. The vscroll spacer row is credited to column 0, so when fresh, expand gets 176 px (14.97 %) for a chevron. With colspan rows skipped in the measurement, expand is 36 px and Details 301 px instead of 178 px (1200 px, fresh), and the long name fits on one line.
  • [T] Skipping colspan rows alone does not fix the empty-window case: aged 20 min, Details stays 64 px and Path takes the leftover (36 %). The fix needs both parts.
  • [K] Measured widths are not saved on their own. But the resize handler's onUp saves all current widths to meshcore-pkt-col-widths. A user who drags one column after a bad first render therefore keeps the squeezed Details permanently.
  • Suggested issue (not filed): "packets: column widths are measured once from the first render — colspan rows inflate Expand, and an empty first window squeezes Details to ~5 %."
    • Fix: skip colspan/spacer rows in makeColumnsResizable(). When the first render has no data rows (and nothing is saved), defer the measurement until the first non-empty render, or fall back to CSS widths.
    • Tests: E2E with the default 15-min window on a fixture aged ≥ 20 min, then switch to 24 h. Details must have a minimum width and advert names must be visible. Add an assertion that Expand stays ≤ ~40 px when fresh. Update the pinned.truncated guard in this test (finding 1).

Other checks

  • [K] Only test-issue-1122-details-row-clamp-e2e.js changed (+71/−14). No production code. The merged tree differs from master in that file only, and it is the same file as on the head.
  • [K] Fork guards (github.repository == 'Kpa-clawbot/CoreScope'): 9 in deploy.yml and 1 in release-fast-path.yml, the same on head, merged tree and master.
  • [K] No closing keywords in the title, body or commit (closingIssuesReferences is empty; the body says "Relates to test: Details-clamp E2E 'advert links in Details stay visible and clickable' is time-dependent and flaky #244").
  • [K] Commit author and committer: dborup <kontakt@meshview.dk>. One commit on top of the merge base.

Not verified

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants