Skip to content

port(upstream#1860): Phosphor icon instead of emoji in the RX coverage header - #28

Merged
dborup merged 6 commits into
masterfrom
codex/port-upstream-1860-rx-coverage-icon
Sep 19, 2026
Merged

dborup merged 6 commits into
masterfrom
codex/port-upstream-1860-rx-coverage-icon

Conversation

@adminopenclaw8-sketch

@adminopenclaw8-sketch adminopenclaw8-sketch commented Sep 13, 2026 •

Copy link
Copy Markdown
Collaborator

Update 2026-09-19: synced with master after #67


Split out of #25 (commit 37be79a2 there). This branch holds exactly one upstream change so it can be reviewed, tested and reverted on its own.

Upstream

Problem

The Mobile RX coverage page header still rendered a literal map emoji, missed by the emoji → Phosphor icon migration.

Change

Replaces it with the ph-map-trifold icon from the existing Phosphor sprite, matching the other page headers.

Adaptation to this fork

None. The cherry-pick applied without conflicts and the changed lines are identical to upstream.

Dependencies and merge order

Verification

Local run of the same commands as CI's “Go Build & Test” job (server tests with -race), on this branch and on master fda24ca5 under the same conditions (same machine, run one after another):

Check master fda24ca5 this branch verdict
xss-gate-diff PASS PASS
channel-lib-test PASS PASS
decrypt-cli-build-test PASS PASS
dockerfile-copy-invariants FAIL FAIL not runnable locally: script needs bash ≥4 (declare -A), macOS has 3.2; identical on master
staging-disk-monitor PASS PASS
css-vars-lint PASS PASS
JS unit tests from CI list + AGENTS.md set (67 files) see below 63 PASS, 4 FAIL
test-issue-1375-scope-stats-fetch.js FAIL FAIL Exactly one api('/scope-stats' call exists (the fixed loader) — found 2 baseline failure, unchanged
test-issue-1648-m4-emoji-scan.js FAIL FAIL map.js has 1 emoji/misc-icon hit(s): baseline failure, unchanged
test-a11y-axe-routes-coverage.js FAIL FAIL axe ROUTES missing analytics tabs (issue #1706): areas, foreign-traffic, wardriving baseline failure, unchanged
test-frontend-helpers.js FAIL FAIL favStar returns empty star for non-favorite: The expression evaluated to a falsy value:, favStar returns filled star for favorite: The expression evaluated to a falsy value: baseline failure, unchanged

Baseline failures (fail identically on master; not introduced or changed here): see rows marked baseline failure, unchanged.

Browser validation (local, fixture DB, no staging/production): Local Go server (built from master) on the committed E2E fixture DB (freshened, migrated and seeded exactly like CI), serving this branch's public/, compared side by side with the same server serving master's public/. Both servers started with clientRxCoverage.enabled: true (the page is gated on it). #/rx-coverage header: master renders "🗺️ Mobile RX coverage" (literal emoji). this branch renders the Phosphor ph-map-trifold sprite icon (16×16 px, no emoji) before "Mobile RX coverage". The CARTO "API KEY REQUIRED" tile watermark and "Could not load mobile observers" appear identically on master and are unrelated (CARTO key: open #13).

Not run:

  • Playwright E2E suites (no local Playwright install); CI's E2E job will also be skipped, see below.
  • eslint (not installed locally; CI installs it on the fly).
  • Go -race/tests for modules this PR does not touch (unchanged code, identical to master).
  • Browser tests against staging/production (deliberately none).

Expected GitHub CI: “Go Build & Test” is expected to fail on TestPruneOldNeighborMetrics, which already fails on master (see #25's run). Downstream jobs (Playwright, image build) are therefore skipped. “Deploy Staging” and all GHCR publish steps only run on push to master and cannot run for this PR.
Two further ingestor tests have failed intermittently in this split's CI on branches whose cmd/ingestor tree is byte-identical to master (#27, #28), so they can also appear here without being caused by this change:

GitHub CI result: run 34749200083 on 29ed4265. Go Build & Test: failure; all downstream jobs incl. Deploy Staging skipped. Failed tests:

  • TestPruneOldNeighborMetrics: fails on master, documented baseline
  • TestBackfillTxLastSeen_ResolvesFromMaxObservationTimestamp: intermittent, reproduced on unmodified master locally

🤖 Generated with Claude Code

Update — current master 693eb04 (2026-09-17)

The existing PR branch was updated without force-push from 29ed4265 to c178df5e. The merge graph preserves the original upstream-attributed commit, incorporates green master 693eb045dbb10ecdd4657444134a696f007297e1, and includes the verified local prep commit 5c2365e0.

The PR diff against current master remains exactly public/rx-coverage.js.

Fresh verification on c178df5

  • node --check public/rx-coverage.js: PASS
  • node test-rx-coverage-config-race.js: PASS
  • node test-issue-1648-m1-emoji-scan.js: PASS
  • Local headless-Chrome check of #/rx-coverage: PASS — one ph-map-trifold sprite reference and no map emoji
  • git diff --check: PASS
  • test-rx-coverage-escape.js: FAILS in its source-extraction harness because it cannot locate the row-builder end; the same failure reproduces unchanged on master 693eb045

No staging, production, deployment, workflow, or auto-merge action was performed. GitHub PR CI: run 35220046384 completed with failure. The only failing test was the known TestMQTTStallWatchdog_DisconnectedEscalationThrottled_1749 suite flake (got 2 invocations); the PR changes only public/rx-coverage.js. Playwright, release, image publication, staging deploy, and badge publication were skipped. No retry or unrelated fix was made.

Update — current master 37d82e8 (2026-09-18)

The existing PR branch was updated without force-push from c178df5e to 6e40de4c, an ordinary merge commit. The merge graph incorporates green master 37d82e8a1844234e498c0c909d0fb1155ea858d7, which now includes PR #64 (fix(nodes): separate advert timestamps from confirmed relay activity) and PR #48 (watchdog Stop-test coverage: StopJoinsLoop + StopIsIdempotent, fixing the TestMQTTStallWatchdog_DisconnectedEscalationThrottled_1749 flake this branch previously hit in CI).

The PR diff against current master remains exactly public/rx-coverage.js (1 line). git diff --check clean. .github/workflows/ unchanged by this branch.

Fresh verification on 6e40de4

  • node --check public/rx-coverage.js: PASS
  • TestMQTTStallWatchdog_DisconnectedEscalationThrottled_1749 (the previously-blocking watchdog flake), -count=5: PASS 5/5 — confirms port(upstream#2003): join the watchdog loop goroutine in tests #48's fix resolves what blocked this PR's prior CI run
  • node test-rx-coverage-config-race.js: PASS
  • node test-rx-coverage-mobile-nav-e2e.js: clean SKIP (no live server in this check — see live verification below, which covers the same ground manually)
  • node test-rx-coverage-escape.js: FAILS — could not locate row-builder end in rx-coverage.js. Reproduced identically on a plain, unmodified checkout of current master (same error, same assertion). Root cause, established (not hypothesis): the test's hardcoded end-marker string o.nodes + '</span></div>'; does not exist verbatim in public/rx-coverage.js — the actual row template has a <span class="rxb-rec">…</span> (receptions column) between o.nodes's closing </span> and the row's closing </div>, so the literal marker string was never present. This file has been essentially untouched (besides this PR's one line and the initial feature commit) since 2026-06-19; the test's marker appears to predate that structure. Unrelated to this PR's one-line header change, which is nowhere near the row-builder block.
  • Live browser verification (local server, fixture DB, clientRxCoverage.enabled: true), desktop (1280×800-class viewport) and mobile (375×812): #/rx-coverage header renders the ph-map-trifold SVG sprite (<svg class="ph-icon"><use href="/icons/phosphor-sprite.svg#ph-map-trifold">) before "Mobile RX coverage" — no emoji, sprite request 200 OK — confirmed identically at both sizes. Also confirmed "Coverage" is reachable from the mobile bottom-nav "More" sheet (the surface test-rx-coverage-mobile-nav-e2e.js checks). The CARTO "API key required" tile watermark and "Could not load mobile observers" (two unrelated 500s on /api/rx-coverage and /api/rx-leaderboard against this ad-hoc local fixture) are pre-existing environment artifacts of the throwaway local DB, not code introduced or changed by this PR, which touches no cmd/server code.

No staging, production, deployment, workflow, or auto-merge action was performed. Local test processes and temp files/DBs cleaned up after verification.

🤖 Generated with Claude Code

… rx-coverage (Kpa-clawbot#1860)

## Summary

`public/rx-coverage.js` still carried a literal `🗺️` (U+1F5FA) in the
Mobile RX coverage page header — missed by the Kpa-clawbot#1648 emoji → Phosphor
migration. Replaced with `ph-map-trifold` from the existing sprite,
matching how every other page header renders (`analytics.js`, `home.js`,
`node-analytics.js`, `customize-v2.js`).

```diff
-'<h2 style="margin:4px 0 2px;font-size:18px">🗺️ Mobile RX coverage</h2>' +
+'<h2 style="margin:4px 0 2px;font-size:18px"><svg class="ph-icon" aria-hidden="true"><use href="/icons/phosphor-sprite.svg#ph-map-trifold"/></svg> Mobile RX coverage</h2>' +
```

## How it got through

This is not a gap in the tooling — `test-issue-1648-m6-final-sweep.js`
catches it correctly. On current `master` (`a06ac8ac`):

```
✗ 1 emoji-as-icon violation(s):

  public/rx-coverage.js:29 [U+1F5FA] '<h2 style="margin:4px 0 2px;font-size:18px">🗺️ Mobile RX coverage</h2>' +
```

The gate is in `test-all.sh` but not in the CI test list in
`deploy.yml`, so it never runs. That divergence is filed separately as
Kpa-clawbot#1858 — this PR is the concrete defect it let through.

## Test plan

- [x] `node test-issue-1648-m6-final-sweep.js` — `✓ lint gate: 0
violations across public/** and cmd/**` (was 1 violation before)
- [x] `node test-issue-1648-m6-lint-self.js` — green, including the
anti-tautology probe (it requires a clean repo to run at all, so it was
failing on master purely as a cascade from the above)
- [x] `eslint public/rx-coverage.js` — 0 errors (1 pre-existing
`no-unused-vars` warning on `selectedName`, untouched)
- [x] `ph-map-trifold` confirmed present in
`public/icons/phosphor-sprite.svg`

Single-line change, no behaviour change beyond the icon glyph.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: SaarMesh-Bot <300107934+SaarMesh-Bot@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
(cherry picked from commit a3454e7)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 13, 2026
SaarMesh-Bot and others added 4 commits September 16, 2026 11:48
… rx-coverage (Kpa-clawbot#1860)

## Summary

`public/rx-coverage.js` still carried a literal `🗺️` (U+1F5FA) in the
Mobile RX coverage page header — missed by the Kpa-clawbot#1648 emoji → Phosphor
migration. Replaced with `ph-map-trifold` from the existing sprite,
matching how every other page header renders (`analytics.js`, `home.js`,
`node-analytics.js`, `customize-v2.js`).

```diff
-'<h2 style="margin:4px 0 2px;font-size:18px">🗺️ Mobile RX coverage</h2>' +
+'<h2 style="margin:4px 0 2px;font-size:18px"><svg class="ph-icon" aria-hidden="true"><use href="/icons/phosphor-sprite.svg#ph-map-trifold"/></svg> Mobile RX coverage</h2>' +
```

## How it got through

This is not a gap in the tooling — `test-issue-1648-m6-final-sweep.js`
catches it correctly. On current `master` (`a06ac8ac`):

```
✗ 1 emoji-as-icon violation(s):

  public/rx-coverage.js:29 [U+1F5FA] '<h2 style="margin:4px 0 2px;font-size:18px">🗺️ Mobile RX coverage</h2>' +
```

The gate is in `test-all.sh` but not in the CI test list in
`deploy.yml`, so it never runs. That divergence is filed separately as
Kpa-clawbot#1858 — this PR is the concrete defect it let through.

## Test plan

- [x] `node test-issue-1648-m6-final-sweep.js` — `✓ lint gate: 0
violations across public/** and cmd/**` (was 1 violation before)
- [x] `node test-issue-1648-m6-lint-self.js` — green, including the
anti-tautology probe (it requires a clean repo to run at all, so it was
failing on master purely as a cascade from the above)
- [x] `eslint public/rx-coverage.js` — 0 errors (1 pre-existing
`no-unused-vars` warning on `selectedName`, untouched)
- [x] `ph-map-trifold` confirmed present in
`public/icons/phosphor-sprite.svg`

Single-line change, no behaviour change beyond the icon glyph.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: SaarMesh-Bot <300107934+SaarMesh-Bot@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
(cherry picked from commit a3454e7)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Brings in:
- #64: fix(nodes): separate advert timestamps from confirmed relay activity
- #48: test(ingestor): reconcile watchdog Stop-test coverage (StopJoinsLoop + StopIsIdempotent), fixing the CI watchdog flake this PR previously hit

No conflicts. PR #28's own change (public/rx-coverage.js) is untouched by this merge.
dborup added a commit that referenced this pull request Sep 19, 2026
@dborup
dborup merged commit 20ab522 into master Sep 19, 2026
6 checks passed
dborup pushed a commit that referenced this pull request Sep 19, 2026
Brings the branch up to master 8b9b9d6 so CI runs on the current
master. No conflicts; no file overlaps with the Reach privacy change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

3 participants