From dfcf8cf078401bc66b857584f9069f6c2580a838 Mon Sep 17 00:00:00 2001 From: gilpanal Date: Fri, 12 Jun 2026 16:47:14 +0200 Subject: [PATCH] docs: fix demo instructions in install.md, resolve stale-text review findings - install.md "Running the demo locally" told readers to run npm run dev, which serves the src/ dev pages, not the demo. Correct flow: build:component:legacy + npm run demo, plus a short note separating the three local servers (demo / dev / docs:dev). - build-output.md: note that dist/ is gitignored but is what npm ships (files whitelist + prepublishOnly rebuild). - claude_codex_workflow_context.md: dated historical banner (references the merged webcomponent branch and a nonexistent npm run build script). - Delete .github/review-prompt.md: one-shot review prompt frozen at an old webcomponent HEAD, no operational value, unreferenced; recoverable from git history. Co-Authored-By: Claude Fable 5 --- .github/review-prompt.md | 103 ------------------------ agents/claude_codex_workflow_context.md | 2 + docs/build-output.md | 2 + docs/install.md | 12 ++- 4 files changed, 14 insertions(+), 105 deletions(-) delete mode 100644 .github/review-prompt.md diff --git a/.github/review-prompt.md b/.github/review-prompt.md deleted file mode 100644 index 5ee1f74..0000000 --- a/.github/review-prompt.md +++ /dev/null @@ -1,103 +0,0 @@ -# Review Prompt — latency-test Web Component - -## Task - -Write a report covering the following 4 areas. **Do NOT modify any files.** Return findings as a prioritised list (P0 = critical, P1 = important, P2 = nice-to-have). - -**Current HEAD:** `f1ca6a6` — "chore: merge main — update repo references to latency-test, sync README and MLS diagram" (webcomponent branch) - ---- - -## 1. Changes Since Last Review - -### P1 fixes -- **premature error resolve** (`demo/common.js`): `startTest()` Promise resolves immediately if `latency-error` fires before `latency-start` (mic denied). Stale `prematureErrorHandler` listeners are cleaned up on retry to prevent leaks. -- **Mode Toggle compare button** (`demo/mode-toggle.js`): `compareBtn` stays disabled until both MR + AW tests complete (`checkBothDone()`) or an error occurs. -- **Config snippet fixes**: `latency-complete` → `latency-result` in Minimal and Mode Toggle snippets. Top-level `await` wrapped in async IIFE for Context Share and Mode Toggle snippets. -- **Docs update** (`docs/api.md`): `latency-start` description corrected — fires on EVERY `start()` call (not just first). - -### P2 fixes -- **Activity counter clamped** (`demo/common.js`): `Math.max(0, ...)` on decrement to prevent negative values. -- **Start buttons disabled on click** (Minimal, Multi-Run, Lifecycle JS files) — prevents double-submit. -- **Trailing whitespace removed**: `src/scripts/test.js` (15 lines), `docs/examples/vanilla-js.md`. - -### Housekeeping -- `skills-lock.json` removed from `.gitignore` and committed for pinned modern-web-guidance skill version. -- Attempted codec warmup patch was reverted — user disagreed since the original `main` branch worked without it. - -### Build tooling & housekeeping -- **`--dev` flag for build script** (`scripts/build-component.mjs`): `node scripts/build-component.mjs` builds minified (production); `node scripts/build-component.mjs --dev` builds unminified (debugging). Both ESM and IIFE are now minified in production mode (previously only IIFE was). Stale Parcel leftovers auto-cleaned from `dist/` before each build. -- **Two npm scripts**: `build:component` (production) and `build:component:dev` (development). -- **`docs/build-output.md`**: new documentation page explaining all 4 dist/ files (ESM, IIFE, source maps) and when to use each. -- **Demo JS files moved**: `demo/common.js`, `demo/minimal.js`, `demo/multi-run.js`, `demo/context-share.js`, `demo/mode-toggle.js`, `demo/lifecycle.js` → `demo/js/`. Script paths in `demo/index.html` updated accordingly. -- `.nvmrc` unchanged (still `18.12.1`, matching current runtime). - -### No changes to -- `src/scripts/latency-test-element.js` — unchanged. -- `src/scripts/test.js` — unchanged. -- `src/scripts/worker.js`, `src/scripts/mls.js` — untouched. - ---- - -## 2. Open Concern: Single-Run Regression - -**Observation:** The very first test after warmup produces inflated latency (~141ms vs ~37ms expected) in both Firefox and Chrome. Multi-run sequence works correctly — only the first of N results is bad; subsequent results cluster around ~37ms. The original `main` branch (pre-refactor) did not exhibit this, suggesting it's a regression in our code. - -**What's been tried unsuccessfully:** -- Cwilso silence keepalive in `prepareAudioToPlayAndRecord()` (`test.js`): 2s silence buffer started simultaneously with every test run to keep the audio thread active. -- Element-level `#startSilence()` (`latency-test-element.js`): 2s warmup silence on first user gesture (Chrome cold-start fix). Currently starts in response to the first `start()` call, before the test begins. -- `startTest()` warmup detection (`demo/common.js`): Detects warmup (listens for `latency-start`, checks if `latency-recording` follows within 300ms), retries immediately if no recording seen, so the test starts while warmup silence is still playing. - -**Current theory:** There's a timing gap between warmup silence commencement and actual test initiation. The original `main` branch started silence + test simultaneously (cwilso pattern) with a single user click. Our code splits this: first `start()` call starts warmup silence and returns early; second `start()` call (~300ms later) starts the test. If getUserMedia takes > ~1.7s (2s buffer minus 300ms detection window), the warmup silence may have finished before the test starts, potentially allowing the audio pipeline to relax. - -**What hasn't been tried:** -- Direct timing comparison against `main` branch (not yet tested in current session). -- Removing the warmup gap entirely — i.e., having the first click start both silence AND test simultaneously (matching original cwilso pattern). -- Instrumenting with console timestamps to measure the exact gap between `noiseSource.start()` and `mediaRecorder.start()` on first vs subsequent runs. - ---- - -## 3. Proposed New Feature: Debug Logging Mode - -### Requirement -Add a `debug` boolean attribute/property to the `` element. When enabled, key methods and functions log timestamped messages to `console.log`/`console.warn` for tracing execution flow. This helps users (and us) diagnose timing issues like the single-run regression above without modifying source code. - -### Questions for reviewers - -1. **Attribute vs property design:** - - **Option A:** `` — boolean HTML attribute, reflected to `tester.debug` JS property. `debug` is `observedAttributes`, so element can react to attribute changes at any time. - - **Option B:** `tester.debug = true` — JS-only property, no reflected attribute. Simpler, but not configurable from HTML alone. - - Which is more correct for Web Components conventions? - -2. **Logging approach:** - - **Option A:** Internal `#log(...)` method on the element class that conditionally calls `console.log` with a `[latency-test]` prefix. Pass `debug` flag to the controller (`LatencyTestController`) for its own `#log(...)`. - - **Option B:** Single static `log()` utility function imported by both files, controlled by a shared flag. - - **Option C:** Use DOM custom events (`latency-debug`) instead of `console.log` — emits a `CustomEvent` with `bubbles: true` `composed: true` containing the message and data. Users subscribe via `addEventListener('latency-debug', ...)`. - - Which approach is most appropriate for a headless Web Component? What are the tradeoffs for end users vs developers? - -3. **What to instrument:** - - Element: `start()`, `stop()`, `#acquireMic()`, `#runNextTest()`, `#createController()`, `#startSilence()`, `disconnectedCallback()`, `attributeChangedCallback()`. - - Controller: `constructor`, `initialize()`, `prepareAudioToPlayAndRecord()`, `startMediaRecorderCapture()`, `startWorkletCapture()`, `finishTest()`, `workerMessageHandler()`, `stop()`. - - Is this too much? Should certain internal methods (e.g. `workerMessageHandler`) be omitted? Should there be log levels (debug / info / warn)? - -4. **Performance concerns:** - - String concatenation + `console.log` on the hot path of audio processing callbacks (e.g., `workerMessageHandler` is called ~15 times per correlation). Are there any performance pitfalls? - - Should hot-path logs be guarded by a separate flag (e.g., `debugWorker`) vs general `debug`? - -5. **Security/privacy:** - - Debug logs may contain audio buffer metadata (sample rates, durations, array sizes). Could also include timestamps from `AudioContext.currentTime`, `performance.now()`, or `Date.now()`. Any concerns about exposing this in console output? - -6. **Framework alternatives:** - - Are there established debugging patterns for Web Components we should follow instead of rolling our own? (e.g., `loglevel` library, built-in browser devtools for Custom Elements, `performance.mark()`/`performance.measure()` for profiling.) - ---- - -## 4. Review Focus Areas - -Please evaluate: - -1. **Demo page completeness** — any tab broken, missing UX, or confusing element? -2. **Debug mode proposal** — which logging approach is correct for this headless Web Component? Any missing considerations? -3. **Single-run regression** — what could cause the inflated first result? What debugging approach would you recommend (instrumentation strategy, targeted experiment)? -4. **Architecture / packaging / API** — anything blocking Phase 7 (npm publish)? -5. **Code quality** — any regressions or issues in the current HEAD? diff --git a/agents/claude_codex_workflow_context.md b/agents/claude_codex_workflow_context.md index 0391759..ce4cba1 100644 --- a/agents/claude_codex_workflow_context.md +++ b/agents/claude_codex_workflow_context.md @@ -1,5 +1,7 @@ # Claude Code + Codex Workflow +> **Historical document (banner added 2026-06-12).** These are the original workflow design notes, preserved as a record. Some sections reference repository state that no longer exists — the `webcomponent` working branch (fully merged via PRs #14–24) and a root `npm run build` script (the build commands are `npm run build:component*` and `npm run docs:build`). For the current workflow see `AGENTS.md`; for current status see `agents/SESSION_HANDOFF.md`. + Date: 2026-05-04 Repo: https://github.com/idsinge/latency-test Working branch: `webcomponent` diff --git a/docs/build-output.md b/docs/build-output.md index f3340af..ef4acee 100644 --- a/docs/build-output.md +++ b/docs/build-output.md @@ -1,5 +1,7 @@ # Build Output +`dist/` is generated output: it is gitignored and never committed, but it is exactly what npm ships — the package's `files` whitelist contains `dist/`, so `npm pack` and the published tarball include it (rebuilt automatically by `prepublishOnly`). + `npm run build:component` produces the modern build in `dist/`: | File | Purpose | diff --git a/docs/install.md b/docs/install.md index f7253df..95b3a6b 100644 --- a/docs/install.md +++ b/docs/install.md @@ -136,10 +136,18 @@ There is no reliable cross-browser API to force both devices to share the same s ## Running the demo locally +The demo page loads the built legacy IIFE bundle, so build it first: + ```bash git clone https://github.com/idsinge/latency-test.git cd latency-test npm install -npm run dev -# open http://localhost:3000 +npm run build:component:legacy +npm run demo +# open http://localhost:3000/demo/ ``` + +Other local servers, for reference: + +- `npm run dev` — internal dev/test pages, served natively from `src/` (no build step needed) +- `npm run docs:dev` — this documentation site