morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #3857] [CLOSED] fix(javascript): scope this emitters to the receiver that binds them #5953

New issue

Closed

opened 2026-10-31 01:10:42 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:10:42 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3857
Author: @kaoru0822-kitauji
Created: 10/15/2026
Status: ❌ Closed

Base: main ← Head: fix/3360-scoped-this-emitters

📝 Commits (1)

418db11 fix(javascript): scope this emitters to the receiver that binds them

📊 Changes

3 files changed (+277 additions, -10 deletions)

View changed files

➕ src/domain/javascript/javascriptSemanticAnalysis.this_emitters.test.ts (+93 -0)
📝 src/domain/javascript/javascriptSemanticAsyncEffects.ts (+51 -10)
➕ src/domain/javascript/javascriptSemanticThisReceivers.ts (+133 -0)

📄 Description

Summary

Fixes #3360. Every this.on/emit/off receiver got the same complete emitter key, "this". As a result, same-named events in unrelated classes joined one resolved event node, and a listener in one class looked like it was dispatched from another.

Problem and expected behavior

Smallest trigger: class Producer { run() { this.on("ready", f) } } and class Unrelated { run() { this.emit("ready") } } produce one resolved ready event that links both. Event identity is documented as one event per exact emitter/name pair. Two unrelated classes' this are not one emitter.

Change and scope

A new javascriptSemanticThisReceivers.ts owns this receiver scoping, using the existing parentsByNode and semanticClassCandidates facts. The scope depends on where this appears:

Where `this` appears	Emitter key scope	Resolution
Class member: method, constructor, accessor, field initializer, static block	The class's local hierarchy root, instance or static side	complete for a lone class; partial when several local classes share the root or the base is ambiguous
Object-literal method or `function` property value	That object literal	complete
Any other non-arrow function	That function	complete
Top level	The program	complete

Arrow functions and computed member keys keep the enclosing receiver.
Keying class members by the hierarchy root keeps the common links: the constructor registers and a method emits, or a base method registers and a subclass emits. Several local classes in one hierarchy have distinct instances, so their joins are candidates. The existing graph projection then marks them as candidate relations with an event unknown.
Hierarchy sizes are computed lazily, only when a file has a class receiver.

Not included: .call/.apply/.bind receiver rebinding. That is already unmodelled for function receivers, and it stays at the same level of precision as binding-scoped emitter keys.

Contract and boundary impact

Semantic owner and earliest changed stage: JavaScript semantic IR eventOperations[].emitterKey and resolution (javascriptSemanticThisReceivers.ts).
Contribution scope fit and linked maintainer decision: an ordinary fix within existing boundaries, so not applicable.
CLI and MCP/tool-catalog contract: no schema change. emitter_key values for this receivers change from "this" to this:class:<range>:instance|static, this:object:<range>, this:function:<range> or this:program, the same range form as expression: keys. Event node IDs for this emitters change in new analyses.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: unrelated receivers no longer share a resolved event. Joins within a multi-class hierarchy are candidates with explicit event unknowns.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.

Expected effects:

Read existing files or captured data.

Evidence and regression coverage

Tests added: src/domain/javascript/javascriptSemanticAnalysis.this_emitters.test.ts covers:
- unrelated classes plus the stable binding control (the issue fixture);
- constructor/method/field sharing in one class;
- a local base/subclass hierarchy as partial;
- function, arrow, object-method and program receivers;
- static vs instance sides.
4 of the 5 fail on main. The single-class case already passed and guards against over-splitting.
Base reproduction, executed with the issue's CLI fixture via scripts/rea.mjs analyze-javascript-application, main 034342ba2 vs. this branch:

main:   event {'emitter_key': 'this', 'event_name': 'ready'}      <- Producer listener and Unrelated dispatch, both resolved
        event {'emitter_key': 'binding:…:stable', 'event_name': 'ready'}
branch: event {'emitter_key': 'this:class:44:123:instance', …}    <- Producer listener only
        event {'emitter_key': 'this:class:124:196:instance', …}   <- Unrelated dispatch only
        event {'emitter_key': 'binding:…:stable', …}              <- control unchanged

Real-app semantic IR comparison, executed with analyzeParsedJavaScriptSemantics on every JS file of six corpora, main 034342ba2 vs. this branch:
- Files: 3,300 files in total. 140 differ, all of them files containing this event operations.
- Changed operations: only eventOperations[].emitterKey and resolution changed. No other IR field or operation changed.
- Re-keyed operations: 1,604 to this:class, 206 to this:function, 8 to this:object.
- Split joins: 102 of the 606 former this event nodes joined more than one receiver. Those are false joins of the kind in the issue. The 606 become 797 receiver-scoped nodes.
- Partial: 457 operations in multi-class local hierarchies (for example minipass/tar subclass chains) moved from complete to partial.
- Exception: one Claude chunk throws RangeError on both main and the branch. That is the pre-existing #3839.
Analysis time, semantic ms main → branch, single warm runs:

Corpus	Main (ms)	Branch (ms)
Discord	39	35
Notion	8,133	8,067
Signal	19,942	19,648
Claude	36,035	35,736
Tabby	10,576	10,374
VS Code `out/`	16,753	16,556

The VS Code figure was re-measured after making hierarchy sizing lazy.

CLI Evidence size and time, best of 3 analyze-javascript-application runs, main → branch:

Package (Tabby `node_modules`)	Evidence bytes	Time	Event nodes	Event unknowns
`tar`	121,550,189 → 121,661,334 (+0.09%)	11.42 → 11.53 s	227 → 242	76 → 212
`undici`	132,966,987 → 132,968,385	12.95 → 13.07 s	216 → 216	95 → 95

The extra tar unknowns explicitly mark minipass hierarchy joins as candidates.

Remaining proof gaps:
- No MCP-adapter run. The CLI and MCP share the semantic graph producer.
- Timings come from one machine.
- Receiver rebinding through .call/.apply/.bind remains unmodelled.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npm run test:local -- src/domain/javascript/javascriptSemanticAnalysis.this_emitters.test.ts: 5 passed. On main, 4 failed.
161 JavaScript/domain test files via npm run test:local: 2,987 passed.
npm run check:fast: typecheck and lint pass. The pre-push hook also passed.
The CLI reproduction, the corpus IR comparison and the CLI size/time measurements above.

Compatibility, safety, and release

Breaking changes or migration steps: none at the schema level. Event IDs and emitter_key values of this emitters differ in new analyses. The docs already state that new analyses may change event IDs.
Real Hopper/Ghidra, browser, or OS coverage: not applicable (static JavaScript analysis).
Package or release metadata impact: none.
Security, privacy, process, or containment review: no target interaction. Private app bundles are referenced only by name and path.

Author readiness

Human owner:

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/3857 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/15/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/3360-scoped-this-emitters` --- ### 📝 Commits (1) - [`418db11`](https://github.com/morluto/rea/commit/418db11e6d7a179bca5d9d45aae7465a7f07d19a) fix(javascript): scope this emitters to the receiver that binds them ### 📊 Changes **3 files changed** (+277 additions, -10 deletions) <details> <summary>View changed files</summary> ➕ `src/domain/javascript/javascriptSemanticAnalysis.this_emitters.test.ts` (+93 -0) 📝 `src/domain/javascript/javascriptSemanticAsyncEffects.ts` (+51 -10) ➕ `src/domain/javascript/javascriptSemanticThisReceivers.ts` (+133 -0) </details> ### 📄 Description ## Summary Fixes #3360. Every `this.on/emit/off` receiver got the same complete emitter key, `"this"`. As a result, same-named events in unrelated classes joined one resolved event node, and a listener in one class looked like it was dispatched from another. ## Problem and expected behavior Smallest trigger: `class Producer { run() { this.on("ready", f) } }` and `class Unrelated { run() { this.emit("ready") } }` produce one resolved `ready` event that links both. Event identity is documented as one event per exact emitter/name pair. Two unrelated classes' `this` are not one emitter. ## Change and scope A new `javascriptSemanticThisReceivers.ts` owns `this` receiver scoping, using the existing `parentsByNode` and `semanticClassCandidates` facts. The scope depends on where `this` appears: | Where `this` appears | Emitter key scope | Resolution | |---|---|---| | Class member: method, constructor, accessor, field initializer, static block | The class's local hierarchy root, instance or static side | complete for a lone class; partial when several local classes share the root or the base is ambiguous | | Object-literal method or `function` property value | That object literal | complete | | Any other non-arrow function | That function | complete | | Top level | The program | complete | - Arrow functions and computed member keys keep the enclosing receiver. - Keying class members by the hierarchy root keeps the common links: the constructor registers and a method emits, or a base method registers and a subclass emits. Several local classes in one hierarchy have distinct instances, so their joins are candidates. The existing graph projection then marks them as candidate relations with an event unknown. - Hierarchy sizes are computed lazily, only when a file has a class receiver. Not included: `.call`/`.apply`/`.bind` receiver rebinding. That is already unmodelled for function receivers, and it stays at the same level of precision as binding-scoped emitter keys. ## Contract and boundary impact - Semantic owner and earliest changed stage: JavaScript semantic IR `eventOperations[].emitterKey` and `resolution` (`javascriptSemanticThisReceivers.ts`). - Contribution scope fit and linked maintainer decision: an ordinary fix within existing boundaries, so not applicable. - CLI and MCP/tool-catalog contract: no schema change. `emitter_key` values for `this` receivers change from `"this"` to `this:class:<range>:instance|static`, `this:object:<range>`, `this:function:<range>` or `this:program`, the same range form as `expression:` keys. Event node IDs for `this` emitters change in new analyses. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: unrelated receivers no longer share a resolved event. Joins within a multi-class hierarchy are candidates with explicit event unknowns. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added: `src/domain/javascript/javascriptSemanticAnalysis.this_emitters.test.ts` covers: - unrelated classes plus the stable binding control (the issue fixture); - constructor/method/field sharing in one class; - a local base/subclass hierarchy as partial; - function, arrow, object-method and program receivers; - static vs instance sides. 4 of the 5 fail on main. The single-class case already passed and guards against over-splitting. - Base reproduction, executed with the issue's CLI fixture via `scripts/rea.mjs analyze-javascript-application`, main 034342ba2 vs. this branch: ```text main: event {'emitter_key': 'this', 'event_name': 'ready'} <- Producer listener and Unrelated dispatch, both resolved event {'emitter_key': 'binding:…:stable', 'event_name': 'ready'} branch: event {'emitter_key': 'this:class:44:123:instance', …} <- Producer listener only event {'emitter_key': 'this:class:124:196:instance', …} <- Unrelated dispatch only event {'emitter_key': 'binding:…:stable', …} <- control unchanged ``` - Real-app semantic IR comparison, executed with `analyzeParsedJavaScriptSemantics` on every JS file of six corpora, main 034342ba2 vs. this branch: - **Files:** 3,300 files in total. 140 differ, all of them files containing `this` event operations. - **Changed operations:** only `eventOperations[].emitterKey` and `resolution` changed. No other IR field or operation changed. - **Re-keyed operations:** 1,604 to `this:class`, 206 to `this:function`, 8 to `this:object`. - **Split joins:** 102 of the 606 former `this` event nodes joined more than one receiver. Those are false joins of the kind in the issue. The 606 become 797 receiver-scoped nodes. - **Partial:** 457 operations in multi-class local hierarchies (for example minipass/tar subclass chains) moved from `complete` to `partial`. - **Exception:** one Claude chunk throws `RangeError` on both main and the branch. That is the pre-existing #3839. - Analysis time, semantic ms main → branch, single warm runs: | Corpus | Main (ms) | Branch (ms) | |---|---:|---:| | Discord | 39 | 35 | | Notion | 8,133 | 8,067 | | Signal | 19,942 | 19,648 | | Claude | 36,035 | 35,736 | | Tabby | 10,576 | 10,374 | | VS Code `out/` | 16,753 | 16,556 | The VS Code figure was re-measured after making hierarchy sizing lazy. - CLI Evidence size and time, best of 3 `analyze-javascript-application` runs, main → branch: | Package (Tabby `node_modules`) | Evidence bytes | Time | Event nodes | Event unknowns | |---|---|---|---|---| | `tar` | 121,550,189 → 121,661,334 (+0.09%) | 11.42 → 11.53 s | 227 → 242 | 76 → 212 | | `undici` | 132,966,987 → 132,968,385 | 12.95 → 13.07 s | 216 → 216 | 95 → 95 | The extra `tar` unknowns explicitly mark minipass hierarchy joins as candidates. - Remaining proof gaps: - No MCP-adapter run. The CLI and MCP share the semantic graph producer. - Timings come from one machine. - Receiver rebinding through `.call`/`.apply`/`.bind` remains unmodelled. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:local -- src/domain/javascript/javascriptSemanticAnalysis.this_emitters.test.ts`: 5 passed. On main, 4 failed. - 161 JavaScript/domain test files via `npm run test:local`: 2,987 passed. - `npm run check:fast`: typecheck and lint pass. The pre-push hook also passed. - The CLI reproduction, the corpus IR comparison and the CLI size/time measurements above. ## Compatibility, safety, and release - Breaking changes or migration steps: none at the schema level. Event IDs and `emitter_key` values of `this` emitters differ in new analyses. The docs already state that new analyses may change event IDs. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (static JavaScript analysis). - Package or release metadata impact: none. - Security, privacy, process, or containment review: no target interaction. Private app bundles are referenced only by name and path. ## Author readiness Human owner:  - [ ] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [ ] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [ ] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:10:42 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:45:58 +00:00

[PR #6175] [MERGED] fix(model): keep host I/O failures distinct from unavailable model targets #7184

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#5953

No description provided.

Rows
Columns