Skip to content

fix(parsers): resolve media source writes in their lexical scopes - #5255

Merged
miguel-heygen merged 1 commit into
heygen-com:mainfrom
user-github-me:fix/parser-media-mutation-scopes
Oct 26, 2026
Merged

miguel-heygen merged 1 commit into
heygen-com:mainfrom
user-github-me:fix/parser-media-mutation-scopes

Conversation

@user-github-me

Copy link
Copy Markdown
Contributor

The source-mutation scanner stored DOM bindings in one map for an entire script. A block or function that reused media could replace another scope’s target: a later video source write was attributed to an image, suppressing the lint warning and the browser probe that reconciles render sources. Parameters and unknown local declarations could also inherit an unrelated outer target.

Resolve bindings through their lexical scopes, including block and function declarations, var, parameters, catch bindings, loops, and named classes. Unknown local bindings shadow outer ones, and alias resolution keeps its declaration scope with a cycle guard. Use Acorn’s node types throughout the scanner. This remains a static heuristic; runtime reassignment and control flow are outside its existing contract.

Validation: 1,249 parser tests pass (4 existing skips, 3 existing todos), all 837 lint tests pass, and 44 producer probe-stage tests pass. Eleven parser regressions, three lint cases, and the source-reconciliation regression fail on current main. Parser build, parser/lint/producer typechecks, repository lint, formatting, and test reachability pass. A Chrome 152 probe compares actual DOM src mutations with scanner output in seven scope cases; every target sequence matches.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Confirmed: the single binding map on main sends a video source write to an image when the same name is reused in another scope, which suppresses both the lint warning and the probe-stage reconciliation. Scoped resolution with lazy alias lookup fixes this. The regression tests cover blocks, sibling functions, params, loops, catch and var hoisting, and they fail on main and pass here.

— Rames

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Checked at 3562929. I ran mediaSrcMutation.test.ts (17) and lint's media.test.ts (97): all pass. With mediaSrcMutation.ts put back to the merge base, 11 scope cases fail (block, sibling function, parameter, catch, loop and class scopes), so the tests guard the change.

Verdict: APPROVE
Reasoning: resolves media source writes in their lexical scope at the one parser lint and probe share; tests fail without it; CI green.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 26, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 26, 2026
@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 26, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 26, 2026
@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 26, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 26, 2026
@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 26, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 26, 2026
@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 26, 2026
Merged via the queue into heygen-com:main with commit 5279b6f Oct 26, 2026
88 checks passed
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