From 49f72718ec15ed782b82bb2877509d4e169d45b4 Mon Sep 17 00:00:00 2001 From: Stephanie Hobson Date: Mon, 9 Nov 2026 14:01:00 -0700 Subject: [PATCH 1/3] Update docs to match the current state of the repo Fixes #487 --- AGENTS.md | 6 +++--- docs/architecture/deploy.md | 17 ++++++++--------- public/contribute.json | 2 +- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 09c2ca5..2a165bf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,10 +60,10 @@ There is no `pages/`, `store/`, `hooks/`, `api/`, `locales/`, or `data/`. `index MozMEAO is taking this repo over. Before that work started there were 810 commits — 515 in 2018, 92 in 2020, **nothing in 2021–2024**, three in 2025, the last `f655559` on 2025-05-07. The toolchain has since been replaced (#441): create-react-app, Jest and Nightwatch gave way to Vite, Vitest and Playwright, and two ESLint configs became one. Code outside the toolchain has barely been exercised since 2020. - **CI and CD both run on GitHub Actions.** `.github/workflows/ci.yml` runs lint, the Vitest unit tests, a production build (`npm run build`, not browser-tested), and the Playwright end-to-end tests on every push to `main` and every pull request. The Playwright specs run twice: on recorded data in "End-to-end tests", which the `main` ruleset requires, and on live data in "Live data tests", which it doesn't. `ssdlc-actions.yml` (zizmor and Semgrep into code scanning) and `sbom.yml` (a CycloneDX SBOM per commit on `main` and per release) call Mozilla's shared reusable workflows in `mozilla/ssdlc-actions`, pinned by SHA; keep the pin in step with bedrock's. There is no `.circleci/`, `Dockerfile`, `docker-compose`, or `Jenkinsfile` — earlier automation was deliberately removed: CircleCI disabled 2018-02-22 (`433a465`), Docker/Dockerflow removed 2018-08-23 (`d3bb561`), `.github/dependabot.yml` deleted 2020-07-29 (`7c7006f`, "Disable non-security updates from Dependabot"). -- **The deploy is a static build on Google Cloud Storage** (confirmed by `x-goog-*` headers on the live site). `https://data.firefox.com/version.json` reports `1.2.1` / commit `f655559`, so **the deployed site predates the Vite toolchain on `main`**; nothing built by Vite has shipped yet. `.github/workflows/deploy.yml` deploys `main` to stage and GitHub releases to prod, via Workload Identity (no secrets). The bucket is shared with the transposer's `datasets/`, nothing in the deploy deletes, and deep links only work because `index.html` is copied to each route path. `docs/architecture/deploy.md` records the buckets, CDN, access, and open questions — read it before changing anything about hosting or the workflow. +- **The deploy is a static build on Google Cloud Storage** (confirmed by `x-goog-*` headers on the live site). `https://data.firefox.com/version.json` reports `1.2.1` / commit `f655559`, so **prod predates the Vite toolchain on `main`**; no Vite build has reached prod yet, though stage serves one from every push to `main`. `.github/workflows/deploy.yml` deploys `main` to stage and GitHub releases to prod, via Workload Identity (no secrets). The bucket is shared with the transposer's `datasets/`, nothing in the deploy deletes, and deep links only work because `index.html` is copied to each route path. `docs/architecture/deploy.md` records the buckets, CDN, access, and open questions — read it before changing anything about hosting or the workflow. - **Node 24.** `.nvmrc` holds `24` and `engines` requires `^24.15.0`. A plain `npm install` works with no flags. - **A fresh clone will not render styles until you compile Stylus.** `src/components/views/css/*` is gitignored and every component does a side-effect `import './css/Foo.css';` — run `npm run build:css` or the app, and Vitest, fail to resolve those imports. `npm start` compiles it for you. -- About 30 open issues, oldest from 2018. Notables: #313 (`MetricOverviewContainer` does not reject when metric data 404s — cited in an inline comment there), #245 (React code-splitting), #46 (localize content). #333 (replace `@babel/polyfill`) is still open, but the polyfill was removed in #441. The remote also carries about 30 stale `dependabot/npm_and_yarn/*` branches and three `%archive-*` branches. +- Open issues date back to 2018, and many predate the toolchain replacement. Check an old issue against the current code before working on it. - Upstream data is **live and current** — transposer `dates` arrays are sorted newest-first and are updated weekly. The app is unmaintained; the data is not. ## Build, Test, and Development Commands @@ -182,7 +182,7 @@ In recorded mode, `fixtureData.js`'s `test` answers every `https://data.firefox. | `build:version.json` | Needs `build/` to already exist and `git` on PATH; must follow `build:app`. | | `no-shadow` / `no-redeclare` with `builtinGlobals` | Naming a local `name`, `status`, `history`, `event` is a hard error. | -**Known rot, for recognition only — this file is not a roadmap:** `registerServiceWorker.js` exists but only `unregister()` is called, and its registration path still reads create-react-app's `process.env.PUBLIC_URL`; `dateformat@3.0.3` is several majors behind, because newer majors change its export shape (see `docs/architecture/frontend.md`). +**Known rot, for recognition only — this file is not a roadmap:** `registerServiceWorker.js` exists but only `unregister()` is called, and its registration path still reads create-react-app's `process.env.PUBLIC_URL`. ## Workflow guidelines diff --git a/docs/architecture/deploy.md b/docs/architecture/deploy.md index 46c0411..ab0ccd1 100644 --- a/docs/architecture/deploy.md +++ b/docs/architecture/deploy.md @@ -22,9 +22,9 @@ Workgroup = a managed group of people granted access to GCP resources. | Bucket | `moz-fx-data-static-websit-8565-ensemble` | `moz-fx-data-static-websit-f7e0-ensemble` | | Hostname | `data.firefox.com` | `dfc-stage.data-static-websites.nonprod.webservices.mozgcp.net` | | DNS | CNAME to `dfc-prod.data-static-websites.prod.webservices.mozgcp.net` | no vanity domain | -| Front-end content | v1.2.1, commit `f655559`, uploaded 2025-05-12 | v1.2.0, commit `f0658a8`, uploaded 2023-06-14 | +| Front-end content | v1.2.1, commit `f655559`, uploaded 2025-05-12 | tracks `main`; every push to `main` deploys | | `datasets/` content | rewritten weekly by the transposer | snapshot from 2023-06-19, never updated | -| Deep links | work (see below) | 404 until the first deploy from `main` | +| Deep links | work (see below) | work (see below) | Both buckets belong to the `data-static-websites` tenant in [`mozilla/webservices-infra`](https://github.com/mozilla/webservices-infra/tree/main/data-static-websites), @@ -36,9 +36,8 @@ administered by the `dataplatform/admins` workgroup. The tenant's `risk_level` i Ensemble is one entry, keyed `"ensemble"`, in the `cdn` map in `tf/prod/locals.tf` and `tf/stage/locals.tf`, plus a `module "ensemble_bucket"` block in each environment's `cdn.tf`. Both were added by Data Engineering in June 2023 (`d0975e2519` stage, `609c8b4fb8` prod) when the site -moved from AWS to GCP. Stage was uploaded to once, as part of that move, and never again. -`public/contribute.json` still lists the AWS-era stage host `data-ensemble.stage.mozaws.net`, which -no longer resolves. +moved from AWS to GCP. Stage was uploaded to once, as part of that move, and not again until the +deploy workflow below. ## How requests are served @@ -60,9 +59,8 @@ cannot be achieved by object metadata alone while `FORCE_CACHE_ALL` is set. **Deep links.** The URL map has no rewrite rules. Deep links work on prod because the bucket holds objects named `dashboard/hardware`, `dashboard/user-activity`, `dashboard/usage-behavior`, and `contact`, each a byte-identical copy of `index.html` (same etag). They were uploaded 40 minutes -after `index.html` on 2025-05-12, by hand. Stage has no such copies, so every stage deep link -returns 404. Any deploy process has to recreate these copies, one per route in `src/config.json` -plus `/contact`. +after `index.html` on 2025-05-12, by hand. Any deploy process has to recreate these copies, one per +route in `src/config.json` plus `/contact`. **404s.** Unknown paths and trailing-slash variants (`/dashboard/hardware/`) return GCS's XML 404 with HTTP status 404. The bucket's website configuration names `404.html`, but load-balancer backend @@ -139,7 +137,8 @@ deployed URL as a smoke test. A failed smoke test does not undo the deploy; it f - 2023-06 Data Engineering created the GCS buckets above and uploaded v1.2.0 to stage. - 2025-05-12 someone uploaded v1.2.1 to prod by hand, then the four route copies. No record of the commands used exists. -- 2026-11 the GitHub Actions workflow above replaced hand uploads (WT-1916). +- 2026-11-09 the GitHub Actions workflow above replaced hand uploads, with its first automated + deploy to stage from commit `fdeafe0` (#480, WT-1916). ## Open questions diff --git a/public/contribute.json b/public/contribute.json index 4097a95..6c25d28 100644 --- a/public/contribute.json +++ b/public/contribute.json @@ -30,7 +30,7 @@ "mentored": "https://github.com/mozilla/ensemble/labels/help%20wanted" }, "urls": { - "stage": "https://data-ensemble.stage.mozaws.net/", + "stage": "https://dfc-stage.data-static-websites.nonprod.webservices.mozgcp.net/", "prod": "https://data.firefox.com/" } } From 6fd49f7a87af54fc84f7e1f65478dd59e700378c Mon Sep 17 00:00:00 2001 From: Stephanie Hobson Date: Mon, 9 Nov 2026 14:22:53 -0700 Subject: [PATCH 2/3] Correct doc claims that no longer match the code Audit of AGENTS.md, CONTRIBUTING.md, README.md, docs/architecture/, contribute.json, and the execplans and update-deps skills against the current tree. --- .claude/skills/execplans/SKILL.md | 5 +- .claude/skills/execplans/references/PLANS.md | 22 +++++---- .claude/skills/update-deps/SKILL.md | 12 ++--- AGENTS.md | 24 +++++----- CONTRIBUTING.md | 34 +++++++------- README.md | 9 ++-- docs/architecture/deploy.md | 17 +++---- docs/architecture/frontend.md | 49 +++++++++----------- public/contribute.json | 8 ++-- 9 files changed, 89 insertions(+), 91 deletions(-) diff --git a/.claude/skills/execplans/SKILL.md b/.claude/skills/execplans/SKILL.md index 53c2e44..dc5025d 100644 --- a/.claude/skills/execplans/SKILL.md +++ b/.claude/skills/execplans/SKILL.md @@ -1,6 +1,6 @@ --- name: execplans -description: Write and maintain an ExecPlan — a self-contained, committed Markdown design document in docs/execplans/ — for work that spans more than one session or touches several files or subsystems. Use for a refactor crossing the container/view split, a tooling or dependency migration, adding CI or test coverage, #409's chart redraw path, or any change whose decisions and progress must survive a context reset. Also use when resuming or revising a plan already in docs/execplans/. Invoke with /execplans. +description: Write and maintain an ExecPlan — a self-contained, committed Markdown design document in docs/execplans/ — for work that spans more than one session or touches several files or subsystems. Use for a refactor crossing the container/view split, a tooling or dependency migration, adding CI or test coverage, or any change whose decisions and progress must survive a context reset. Also use when resuming or revising a plan already in docs/execplans/. Invoke with /execplans. --- # ExecPlans @@ -9,8 +9,7 @@ An ExecPlan is a Markdown design document that lives on disk and carries a piece design through implementation. It is the durable record: someone holding only the working tree and the plan file can pick the work up and finish it. Write one instead of relying on the built-in plan/todo tools whenever the work meets the threshold below — those tools don't survive a context -reset, and this repo's near-term work (adding CI, broadening test coverage, fixing #409's chart -redraw path) is exactly the kind that needs to. +reset. Plans in this repository go in `docs/execplans/` and are committed on the working branch. **Read `references/PLANS.md` in full before writing or revising one** — it is the specification, and its diff --git a/.claude/skills/execplans/references/PLANS.md b/.claude/skills/execplans/references/PLANS.md index 4ab1c19..14bc85e 100644 --- a/.claude/skills/execplans/references/PLANS.md +++ b/.claude/skills/execplans/references/PLANS.md @@ -39,7 +39,7 @@ Write in plain prose. Prefer sentences over lists. Avoid checklists, tables, and ## Guidelines -Self-containment and plain language are paramount. Assume the reader knows web development generally — npm scripts, React, CI, git — and define what is particular to this repository ("the container/view split", "react-refetch's `connect()`", "`flagForUpdate`", "the gitignored compiled CSS") the first time it appears, naming the files or commands where it shows up. Define each term once, in `Context and Orientation`, then use it. Do not say "according to the architecture doc"; put the needed explanation in the plan. +Self-containment and plain language are paramount. Assume the reader knows web development generally — npm scripts, React, CI, git — and define what is particular to this repository ("the container/view split", "`withFetch`'s `dataFetch` prop", "`flagForUpdate`", "the gitignored compiled CSS") the first time it appears, naming the files or commands where it shows up. Define each term once, in `Context and Orientation`, then use it. Do not say "according to the architecture doc"; put the needed explanation in the plan. Avoid common failure modes. Do not rely on undefined jargon. Do not describe "the letter of a feature" so narrowly that the resulting code compiles but does nothing meaningful. Do not outsource key decisions to the reader. When ambiguity exists, resolve it in the plan itself and explain why you chose that path. Err on the side of over-explaining user-visible effects and under-specifying incidental implementation details. @@ -171,7 +171,7 @@ branch. There is nowhere else for them here, and a plan that is not in the pull reviewable. `` is the kebab-case summary from the branch name, so the two line up: branch -`409--hardware-resize-performance`, plan `docs/execplans/2026-09-20-hardware-resize-performance.md`. +`409--hardware-resize-performance`, plan `docs/execplans/2026-10-28-hardware-resize-performance.md`. The date is the day the plan was started and does not change when the plan is revised. This specification is checked in at `.claude/skills/execplans/references/PLANS.md`. Cite that path @@ -200,17 +200,19 @@ today. These do: npm run build:css # required before Vitest on a fresh clone; the compiled CSS is gitignored npm run lint # ESLint flat config (.js and .jsx) plus stylint npm run test:jest # Vitest - npm run test:playwright # Playwright, against the Vite dev server and live data.firefox.com - npm run build # Vite production build plus version.json + npm run test:playwright # every Playwright project, against the Vite dev server + npm run test:playwright:recorded # the specs on the recorded fixtures (CI's required check) + npm run test:playwright:live # the same specs on live data.firefox.com + npm run build # Stylus compile, Vite production build, then version.json npm test # lint, then Vitest, then Playwright -The Playwright dashboard specs assert against live production data, so a failure there can come -from upstream data changing rather than from the commit. Re-run once before investigating, and say -which it was. +The recorded Playwright run is deterministic. Only `test:playwright:live` depends on upstream data, +so a failure there can come from the data changing rather than from the commit; it usually means the +recording is stale (see `AGENTS.md`). Say which it was. Run each command and report the actual result; never claim a pass you -did not observe. State the expected result, not just the command: "`npm run test:jest` — 2 files, 4 -tests passing" beats "run the tests". +did not observe. State the expected result, not just the command: "`npm run test:jest` — every file +passes, including the new `Foo.test.jsx`" beats "run the tests". ## Record what matters, not what happened @@ -264,7 +266,7 @@ neither self-contained nor, probably, correct. ## The plan and the pull request The plan file is part of the diff, so name it in the PR's **Significant changes and points to -review** as one plain bullet — "Adds `docs/execplans/2026-09-14-eslint-unification.md`" — and stop +review** as one plain bullet — "Adds `docs/execplans/2026-10-28-hardware-resize-performance.md`" — and stop there. The description keeps its four headings (One-line summary; Significant changes and points to review; Issue / Bugzilla link; Testing) and does not become a second copy of the plan. `AGENTS.md`'s ban on describing your own process still applies: the plan records how the work went, the diff --git a/.claude/skills/update-deps/SKILL.md b/.claude/skills/update-deps/SKILL.md index a0d7a4c..531ce50 100644 --- a/.claude/skills/update-deps/SKILL.md +++ b/.claude/skills/update-deps/SKILL.md @@ -2,14 +2,14 @@ name: update-deps description: Audit and update npm dependencies disable-model-invocation: true -argument-hint: "[all|sync-check|]" +argument-hint: "[all|]" allowed-tools: - Bash(npm outdated *) - Bash(npm update *) - Bash(npm install *) - Bash(npm run lint*) - Bash(npm test*) - - Bash(npm run test-build*) + - Bash(npm run build*) - Bash(npm view *) - Bash(npm audit --json*) - Bash(npm explain *) @@ -54,7 +54,7 @@ Indirect dependencies are packages in `package-lock.json` that aren't listed in - **No fix available**: report it and move on. 4. Ask the user once whether to also refresh indirect dependencies that have no security fix pending (everything `npm update` would change within the existing ranges). Default to no; this can touch hundreds of packages. -Never run `npm audit fix --force`. Its "fixes" with `isSemVerMajor: true` can be downgrades to old major versions of the parent (e.g. it suggests `@frctl/fractal@1.3.0`). +Never run `npm audit fix --force`. Its "fixes" with `isSemVerMajor: true` can be downgrades to old major versions of the parent. ### Open Dependabot PRs @@ -163,7 +163,7 @@ Record the following in `.claude/skills/update-deps/denied.md`: ## Phase 6: Execute Approved Updates -Run `npm install @` for pinned packages, or `npm update ` for range-pinned packages. Verify `package-lock.json` updated correctly. +Every direct dependency is exact-pinned, so run `npm install @`. Verify `package-lock.json` updated correctly. For indirect dependencies: @@ -180,8 +180,8 @@ After each step, check `git diff package-lock.json` for packages that changed an After executing updates, offer to run verification commands: - `npm run lint` — run JS/CSS linting -- `npm run test-build` — confirm the webpack build still succeeds -- `npm test` — run the Jasmine browser test suite +- `npm run build` — confirm the Stylus and Vite build still succeeds +- `npm test` — run lint, the Vitest unit tests, and the Playwright suite Ask the user which (if any) they want to run. Run selected checks and report results. diff --git a/AGENTS.md b/AGENTS.md index 2a165bf..58aa342 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,9 +21,9 @@ | `containers/MetricOverviewContainer.jsx` | `${dashboardSource}/${activeRegion}/${slug}/index.json` | title, description, `type: 'line'｜'table'`, axes, columns, data, annotations | | `containers/SummaryMetricContainer.jsx` | same per-metric endpoint | buckets populations under 5% into "Other" | -`src/lib/withFetch.jsx` **is the entire data layer** and is used in exactly those three files: `withFetch(props => url)(Container)` passes a `dataFetch` prop (`pending`, `rejected`, `fulfilled`, `value`, `reason`), refetches when the URL changes (back to pending first), rejects on non-2xx, and drops a response that arrives after a newer request started. Each one handles `dataFetch.pending` / `.rejected` / `.fulfilled` itself and renders `views/Spinner.jsx` or `views/Error.jsx`. All reshaping is client-side in each container's `formatData` (`ChartContainer` sorts populations by max y and drops nulls; `DataTableContainer` sorts rows by value descending). +`src/lib/withFetch.jsx` **is the entire data layer** and is used in exactly those three files: `withFetch(props => url)(Container)` passes a `dataFetch` prop (`pending`, `rejected`, `fulfilled`, `value`, `reason`), refetches when the URL changes (back to pending first), rejects on non-2xx, and drops a response that arrives after a newer request started. Each one handles `dataFetch.pending` / `.rejected` / `.fulfilled` itself: `DashboardContainer` renders `views/Spinner.jsx` while pending, the two metric containers render nothing (the chart's own spinner shows instead), and all three render `views/Error.jsx` on rejection. All reshaping is client-side, in the `formatData` of `ChartContainer`, `DataTableContainer`, and `SummaryMetricContainer` (`ChartContainer` sorts populations by max y and drops nulls; `DataTableContainer` sorts rows by value descending). -Metric `description` strings are rendered as inline Markdown through `markdown-it('zero')` with only `link`/`entity`/`emphasis`/`sup` enabled, via `dangerouslySetInnerHTML`. `src/tests/jest/MetricOverview.test.jsx` exists **specifically to guard that sanitization** — never widen the enabled-rules list without extending that test. +Metric `description` strings are rendered as inline Markdown through `markdown-it('zero')` with only `link` and `entity` enabled plus the `markdown-it-sup` plugin, via `dangerouslySetInnerHTML`. The dashboard description in `views/Dashboard.jsx` enables only `emphasis`. `src/tests/jest/MetricOverview.test.jsx` exists **specifically to guard that sanitization** — never widen the enabled-rules list without extending that test. ``` src/ @@ -47,10 +47,10 @@ src/ └── tests/ └── jest/ unit tests, run by Vitest (the directory name predates Vitest) tests/playwright/ -├── specs/ e2e specs (.spec.js, CommonJS), incl. dashboards/ and fixtures/; run on the recording and on live data +├── specs/ e2e specs (.spec.js, CommonJS), incl. dashboards/ and fixtures/; run on the recording, and all but fixtures/ and jsDisabled also on live data ├── fixtures/ recorded transposer JSON, written by `npm run record:fixtures` ├── fixtureData.js `test` that serves fixtures/ (or live data, with liveData) and fails on page/console errors; readFixture -└── utils.js shared helpers: linkWorks, linksWork, flagForUpdate, metricTitleIsCorrect +└── utils.js shared helpers: formatPercent, linkWorks, linksWork, flagForUpdate, metricTitleIsCorrect, changeRegionAndWaitForMetrics ``` There is no `pages/`, `store/`, `hooks/`, `api/`, `locales/`, or `data/`. `index.html` at the repository root is Vite's entry point (with a hardcoded `og:image` and the `