Skip to content

feat: update @svta/cml-cmcd to 2.4.0 - [Claude Code]#5

Open
littlespex wants to merge 16 commits into
developmentfrom
claude/infallible-mendeleev-493f12
Open

feat: update @svta/cml-cmcd to 2.4.0 - [Claude Code]#5
littlespex wants to merge 16 commits into
developmentfrom
claude/infallible-mendeleev-493f12

Conversation

@littlespex
Copy link
Copy Markdown
Member

Summary

Upgrades @svta/cml-cmcd from 2.3.2 to ^2.4.0 and adapts dash.js to the new CmcdReporter semantics. The headline change in 2.4.0: update() auto-fires the corresponding state-change event when a tracked field changes, and a follow-up recordEvent() for the same event is dedup-suppressed — silently dropping its data argument.

The existing dash.js pattern hit this footgun in _onPlaybackStateChange: update({sta}) then recordEvent('ps', cmcdData) was silently losing bl, mtp, pt, ltc, msd enrichment under 2.4.0. This PR consolidates that into a single update({ ...continuousMetrics, sta }) call so the auto-fire emits the full payload. It also splits state-change vs non-state event paths cleanly, adds throttled pt tracking via PLAYBACK_TIME_UPDATED, and replaces the local CmcdRequestCollector functional-test helper with the library's new CmcdReportRecorder.

What changed

  • package.json — bump @svta/cml-cmcd 2.3.2 → ^2.4.0 and @svta/cml-request 1.0.12 → 1.0.13 (peer-dep alignment).
  • src/streaming/controllers/CmcdController.js:
    • Add _gatherContinuousMetrics() helper as the single source of truth for fields pulled from CmcdModel.
    • Refactor _onPlaybackStateChange to a single consolidated update({ ..._gatherContinuousMetrics(), sta: state }) call. This is the dedup-footgun fix.
    • Rename triggerCmcdEventMode_recordNonStateEvent. Continuous metrics flow through update(); only ephemeral per-event payload (e.g., ec) rides recordEvent()'s data arg.
    • _onPlayerError now passes { ec: [String(code)] } (array form per CTA-5004-B / the 2.4.0 Cmcd.ec type).
    • _handleResponseReceived now also uses _gatherContinuousMetrics() — continuous metrics persist into the store via update(), only ephemeral CMSD-derived data rides recordResponseReceived()'s data arg.
    • Add throttled _onPlaybackTimeUpdated listener (4 Hz; validates e.time is a finite non-negative number) that writes pt directly to the reporter's persistent store. Subscribed in _initializeEventBus, unsubscribed in reset, state reset in _resetInitialSettings.
  • test/unit/test/streaming/streaming.controllers.CmcdController.js — new tests: pt lands in event payload after listener fires; throttle suppresses second fire within window; invalid e.time values are ignored; PLAY_STATE event includes ltc+pt enrichment (regression test for the footgun); ERROR event carries ec as string[].
  • test/functional/test/feature-support/cmcd-v2.js — migrated from local CmcdRequestCollector to CmcdReportRecorder from @svta/cml-cmcd. Single attach-level waitTimeout replaces per-call timeouts. Added end-to-end assertion that PLAY_STATE event reports carry ≥2 continuous-metric enrichment fields (pt/bl/mtp).
  • test/functional/helpers/CmcdRequestCollector.js — deleted (superseded).
  • githook.cjs — fixed the prepare script's hook installer to resolve the actual git hooks directory when .git is a worktree file (was failing with ENOTDIR in worktrees).

Design and plan

  • Design spec: docs/superpowers/specs/2026-05-27-cmcd-v2.4-update-design.md
  • Implementation plan: docs/superpowers/plans/2026-05-27-cmcd-v2.4-update.md

Behavior deltas to be aware of

  1. PLAYBACK_RATE auto-fire is new. Under 2.4.0, update(prData) in _onPlaybackRateChanged now auto-fires a PLAYBACK_RATE event-mode report when an event target subscribes to the pr event. This is correct per CTA-5004-B but is a visible behavior change for event-target consumers that weren't previously receiving PLAYBACK_RATE events.
  2. ec shape change. ERROR event payloads now carry ec as string[] (was a scalar). Required by 2.4.0 typings and CTA-5004-B's list notation. Downstream CMCD event validators parsing error payloads should accept array form.
  3. pt is now emitted in event-mode reports. Previously not reported by dash.js at all; now flows into the persistent store at 4 Hz from PLAYBACK_TIME_UPDATED.
  4. Stubbed event-target POST status code shifts 200 → 204. Transparent to consumers because HTTPLoader already treats any 2xx as success.

Test Plan

  • npm run lint — clean
  • npm test — 3670 passing, 0 failing (+12 new unit tests in this branch)
  • npm run test-functional — exit 0 (CMCD-specific: Query 7/8 with pre-existing unrelated flake, Header 6/6, Event 10/10, Key Filtering 6/6, Version 6/6)
  • npm run build — modern + legacy both clean
  • Spot-check samples/cmcd/cmcd-v2.html in a browser: PLAY_STATE event report shows sta, pt, bl, mtp; playback rate change triggers a pr event report
  • Confirm manifest/segment requests still carry monotonically-increasing sn (now reporter-internal)

🤖 Generated with Claude Code

littlespex and others added 12 commits May 27, 2026 15:56
Captures the design for adopting CmcdReporter's new auto-firing
state-change semantics, replacing CmcdRequestCollector with the
library's CmcdReportRecorder, and adding throttled `pt` tracking.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Eight-task TDD plan derived from the design at
docs/superpowers/specs/2026-05-27-cmcd-v2.4-update-design.md.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2.4.0 adds CmcdReporter.update() auto-firing of state-change events and
ships CmcdReportRecorder as an official test helper. Also bumps
@svta/cml-request to 1.0.13 (its peer-dep range required 2.3.2 exactly).

Fixes the `prepare` githook installer to resolve the actual git hooks
directory when `.git` is a worktree file (otherwise `npm install` fails
in a worktree with ENOTDIR).

Behavior adjustments in this codebase follow in subsequent commits.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Subscribes CmcdController to PLAYBACK_TIME_UPDATED and writes pt
(playhead time in ms) directly to the reporter's persistent store,
throttled to 4 Hz. The listener also validates that e.time is a
finite, non-negative number before forwarding — CMCD spec requires
pt to be a non-negative integer.

Both auto-fired state-change events and TIME_INTERVAL reports now
see fresh pt without the controller plumbing it at every event site.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Under @svta/cml-cmcd 2.4.0, CmcdReporter.update({sta}) auto-fires the
PLAY_STATE event, and a follow-up recordEvent('ps', ...) is dedup-
suppressed, silently dropping its data argument. This change folds the
prior two-call pattern into a single update({ ...continuousMetrics, sta })
call so enrichment fields (ltc, bl, mtp, pt, msd, ...) ride the auto-fire.

Adds _gatherContinuousMetrics() helper as the single source of truth for
fields pulled from CmcdModel at each event site.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Renames triggerCmcdEventMode -> _recordNonStateEvent to match the
@svta/cml-cmcd 2.4.0 canonical pattern: continuous metrics flow
through update() into the persistent data store, and recordEvent()
carries only ephemeral per-event payload. Updates _onPlayerError to
pass ec as a string array per CTA-5004-B / the 2.4.0 Cmcd.ec type.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sn is now managed internally by CmcdReporter; dash.js no longer
tracks or sets it anywhere.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the locally maintained XHR-patching helper with the
CmcdReportRecorder shipped from @svta/cml-cmcd 2.4.0. Single attach-
level waitTimeout replaces per-call timeouts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cosmetic cleanup from code review.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
End-to-end witness of the v2.4.0 update + recordEvent consolidation:
verifies that pt / ltc / bl / mtp show up in real PLAY_STATE event
reports rather than being silently dropped by the reporter's dedup.

Also adds 'pt' to the play-state target's enabledKeys so the new
assertion has at least one continuous-metric signal to find.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Brings the response-received path in line with the rest of the
2.4.0-aligned pattern: continuous metrics flow through update() into
the persistent data store, and only ephemeral per-response payload
(CMSD headers) rides the recordResponseReceived() data argument.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add bl and mtp to the PLAY_STATE event target's enabledKeys and
require >=2 distinct enrichment fields in the emitted report. The
earlier >=1 check passed on pt alone, leaving the bl/mtp portion of
the regression intent unverified. ltc is excluded because
PlaybackController.getCurrentLiveLatency() returns NaN for VOD streams.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@littlespex littlespex changed the title Upgrade @svta/cml-cmcd to 2.4.0, fix update/recordEvent dedup-footgun [Claude Code] @svta/cml-cmcd to 2.4.0 May 28, 2026
@littlespex littlespex changed the title [Claude Code] @svta/cml-cmcd to 2.4.0 feat: update @svta/cml-cmcd to 2.4.0 - [Claude Code] May 28, 2026
@littlespex littlespex requested a review from Copilot May 28, 2026 17:32
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR upgrades CMCD reporting to @svta/cml-cmcd 2.4.0 and adapts dash.js event-mode reporting to the library’s new update() auto-fire semantics.

Changes:

  • Refactors CmcdController to consolidate PLAY_STATE updates, add throttled pt tracking, and separate state-change from non-state event reporting.
  • Migrates CMCD v2 functional tests from the local request collector to CmcdReportRecorder and adds regression coverage for enriched event payloads.
  • Bumps CMCD-related dependencies and adjusts the git hook installer for worktree .git files.

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/streaming/controllers/CmcdController.js Updates CMCD event/reporting logic for 2.4.0 semantics and adds pt listener support.
test/unit/test/streaming/streaming.controllers.CmcdController.js Adds unit coverage for pt, PLAY_STATE enrichment, throttling, invalid times, and ERROR ec shape.
test/functional/test/feature-support/cmcd-v2.js Migrates functional CMCD v2 tests to CmcdReportRecorder and adds event enrichment assertions.
test/functional/helpers/CmcdRequestCollector.js Removes the now-superseded local functional-test collector helper.
package.json Updates @svta/cml-cmcd and @svta/cml-request dependency versions.
package-lock.json Locks the updated dependency versions.
githook.cjs Attempts to resolve the actual git hooks directory for worktree checkouts.
docs/superpowers/specs/2026-05-27-cmcd-v2.4-update-design.md Adds the implementation design for the CMCD 2.4.0 upgrade.
docs/superpowers/plans/2026-05-27-cmcd-v2.4-update.md Adds the task-by-task implementation plan for the CMCD 2.4.0 upgrade.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/streaming/controllers/CmcdController.js
Comment thread src/streaming/controllers/CmcdController.js
Comment thread githook.cjs Outdated
littlespex and others added 4 commits May 28, 2026 10:50
- Remove docs/superpowers/{plans,specs} — these were branch-local design
  artifacts that don't belong in the upstream PR.
- Revert githook.cjs to its pre-branch state. The worktree-aware
  resolver was only needed to make `npm install` succeed in this
  development worktree; it is not core to the CMCD upgrade.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rdering

- CmcdModel.getGenericCmcdData() now always sets bg (true or false based
  on document.hidden), so hidden->visible transitions round-trip through
  the reporter's persistent store. The previous code only set bg:true
  when hidden and left it unset when visible, which let a stale bg:true
  persist after the tab became visible again and leak into unrelated
  event-mode payloads. The library's emission filter handles wire-format
  semantics: bg:false is stripped on non-`e=b` events and emitted as `?0`
  on `e=b` per CTA-5004-B and the 2.4.0 carve-out.

- _onPlaybackTimeUpdated now calls _rebuildReporterIfNeeded() before
  writing pt to the reporter, so a time update that arrives after
  MANIFEST_LOADED but before the next state change lands on the new
  reporter (the one that will actually emit) rather than on the old
  reporter that's about to be discarded by the rebuild.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Match the version pinning style of the other @svta/cml-* packages in
package.json (none of which use carets). The earlier ^2.4.0 would
silently float onto 2.x minor updates and bypass review for
behavior-shifting library changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The lockfile had accumulated ~5500 lines of unrelated transitive-dep
drift from earlier installs in this branch. Restored to the base
lockfile and applied only the four changes the CML bumps actually
require:

- top-level deps: @svta/cml-cmcd 2.3.2 -> 2.4.0
- top-level deps: @svta/cml-request 1.0.12 -> 1.0.13
- node_modules/@svta/cml-cmcd: version/resolved/integrity to 2.4.0
- node_modules/@svta/cml-request: version/resolved/integrity to
  1.0.13 and its peer-dep pin on @svta/cml-cmcd updated to 2.4.0

Net diff vs base: 9 insertions / 9 deletions.

Co-Authored-By: Claude Opus 4.7 (1M context) <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.

2 participants