[PR #4407] [CLOSED] fix(javascript): exclude parameter-property declarations from reads #6218

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:12:46 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4407
Author: @SII-YDD
Created: 10/16/2026
Status: ❌ Closed

Base: main ← Head: fix/javascript-parameter-property-references

📝 Commits (1)

62a336a fix(javascript): keep parameter properties out of lexical reads

📊 Changes

3 files changed (+243 additions, -0 deletions)

View changed files

➕ src/domain/javascript/javascriptSemanticAnalysis.parameter_properties.test.ts (+86 -0)
📝 src/domain/javascript/javascriptSemanticProjection.ts (+3 -0)
➕ tests/acceptance/applications/javascriptParameterProperties.test.ts (+154 -0)

📄 Description

Summary

Fixes #4371. TypeScript constructor parameter-property declarations were emitted as lexical reads, producing resolved influence edges at declaration tokens. Keep them as definitions while preserving actual initializer, closure and body uses.

Problem and expected behavior

class C { constructor(public x: number) {} } has a parameter definition and no authored read of x. Both this form and public x = 1 incorrectly gained x:read. An ordinary constructor parameter already worked.

A real CLI fixture produced five resolved reads edges: three false declaration reads and two real default/body uses. The fix removes the three false edges and preserves both genuine uses, parameter nodes, binding identity and source provenance. Results remain static observations/candidates, not runtime execution claims.

Change and scope

javascriptSemanticProjection.ts owns reference classification. Recognize TSParameterProperty as a direct declaration parent and as the declaration owner of its default's bound pattern. Traverse the initializer normally; do not filter graph output or skip the entire subtree. Existing parameter-binding collection already unwraps this node.

This adds constant kind/slot checks to the existing traversal. No measured token, speed, memory or real-application accuracy benefit is claimed.

Contract and boundary impact

Ordinary REA-owned static-analysis fix; no new provider/capability requiring a scope decision.
CLI/MCP commands, schemas, package/setup and generated metadata unchanged.
Preserve artifact identity/provenance, canonical CLI/MCP Evidence parity, source bytes/ranges, real references and static-analysis limitations.
Existing artifact reads and owned REA analysis workers remain; no target execution, attachment, mutation or target network access. Existing CLI process, temporary-directory and stdio harness cleanup is reused.

Evidence and regression coverage

On main 2c7583599b9ac1c847bd77b1940930fd8d4594c7, the final regressions failed12/passed2 because of false references and the actual CLI graph assertion. With the fix: 14passed. Controls cover typed/default properties, accessibility/read-only modifiers, ordinary parameters, real default reads, closure captures, body read-modify-write and shadow binding identity.

Compiled CLI and real stdio MCP produce matching canonical application Evidence. Exact-node traces retain zero reads for the unused parameter and one each for the genuine body/default uses. After an invalid missing-target request, the same connection pings and successfully traces retained Evidence. Binary session stays unopened; quoted Unicode paths and source bytes are preserved. The inert fixture throws if executed.

Validation performed

Pinned Node24.18.0/npm11.16.0; project tests received no GitHub credentials.

npm run test:focused -- src/domain/javascript/javascriptSemanticAnalysis.parameter_properties.test.ts tests/acceptance/applications/javascriptParameterProperties.test.ts — 14passed/2files, Linux arm64, actual CLI/stdio MCP.
New domain + existing structure, function_names, logical_assignment suites — 110passed/4files, macOS arm64; overlaps the13 domain cases above.
npm run check:fast, npm run check, npm run check:ci, npm run docs:check — passed.
npm run verify:test-discovery — 1416 files exactly once; git diff --check passed.
Independent AI review: no blockers; reviewer independently ran166 source tests and additional reference-role probes, and inspected the Linux logs. This is separate reviewer evidence, not an independent Linux/full-suite rerun.

Compatibility, safety, and release

Previously false declaration reads disappear; no invocation migration or schema change. Public workflows ran on Linux because this host's macOS Swift ownership-helper compilation is independently broken. Other OS public workflows, native engines and real-application prevalence were not verified. Locked Linux dependencies were installed in isolation; target source was never executed.

Current head 62a336a70ef12e8eecf3ad885b3f287b1ea2dbd1: CI38861870463 SUCCESS —15 successful jobs,16 conditional skips,0 failed/incomplete. All five built shards, source/static/docs checks, Linux package baseline, portable report, CI required and real JavaScript recovery passed. Portable aggregate: 16,337passed/144skipped, including the new acceptance regression. Actual tested merge 7fa62e63c99ea7cbdb61ff2990c08f747cc612e8 into base 2c758359. This is fast-tier complete portable verification; coverage/full qualification were not executed. Verified2026-10-16T06:45:04Z. Full qualification is not yet claimed: fork dispatch currently returns HTTP422 Actions-disabled, and upstream dispatch permission is unavailable. No checks, thresholds, exclusions or assertions were weakened. AI-assisted work.

Author readiness

Human owner: @SII-YDD

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.

Draft under the project's human-ownership rule; AI review and CI do not attest to human review.

Before merge

Concrete problem, focused outcome, linked issue, no authority expansion.
Observable behavior and preservation controls verified.
Current-head complete required verification and full qualification confirmed.
Human owner completed the readiness declarations.

_{🔄 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/4407 **Author:** [@SII-YDD](https://github.com/SII-YDD) **Created:** 10/16/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/javascript-parameter-property-references` --- ### 📝 Commits (1) - [`62a336a`](https://github.com/morluto/rea/commit/62a336a70ef12e8eecf3ad885b3f287b1ea2dbd1) fix(javascript): keep parameter properties out of lexical reads ### 📊 Changes **3 files changed** (+243 additions, -0 deletions) <details> <summary>View changed files</summary> ➕ `src/domain/javascript/javascriptSemanticAnalysis.parameter_properties.test.ts` (+86 -0) 📝 `src/domain/javascript/javascriptSemanticProjection.ts` (+3 -0) ➕ `tests/acceptance/applications/javascriptParameterProperties.test.ts` (+154 -0) </details> ### 📄 Description ## Summary Fixes #4371. TypeScript constructor parameter-property declarations were emitted as lexical reads, producing resolved influence edges at declaration tokens. Keep them as definitions while preserving actual initializer, closure and body uses. ## Problem and expected behavior `class C { constructor(public x: number) {} }` has a parameter definition and no authored read of `x`. Both this form and `public x = 1` incorrectly gained `x:read`. An ordinary constructor parameter already worked. A real CLI fixture produced five resolved `reads` edges: three false declaration reads and two real default/body uses. The fix removes the three false edges and preserves both genuine uses, parameter nodes, binding identity and source provenance. Results remain static observations/candidates, not runtime execution claims. ## Change and scope `javascriptSemanticProjection.ts` owns reference classification. Recognize `TSParameterProperty` as a direct declaration parent and as the declaration owner of its default's bound pattern. Traverse the initializer normally; do not filter graph output or skip the entire subtree. Existing parameter-binding collection already unwraps this node. This adds constant kind/slot checks to the existing traversal. No measured token, speed, memory or real-application accuracy benefit is claimed. ## Contract and boundary impact - Ordinary REA-owned static-analysis fix; no new provider/capability requiring a scope decision. - CLI/MCP commands, schemas, package/setup and generated metadata unchanged. - Preserve artifact identity/provenance, canonical CLI/MCP Evidence parity, source bytes/ranges, real references and static-analysis limitations. - Existing artifact reads and owned REA analysis workers remain; no target execution, attachment, mutation or target network access. Existing CLI process, temporary-directory and stdio harness cleanup is reused. ## Evidence and regression coverage On main `2c7583599b9ac1c847bd77b1940930fd8d4594c7`, the final regressions **failed12/passed2** because of false references and the actual CLI graph assertion. With the fix: **14passed**. Controls cover typed/default properties, accessibility/read-only modifiers, ordinary parameters, real default reads, closure captures, body read-modify-write and shadow binding identity. Compiled CLI and real stdio MCP produce matching canonical application Evidence. Exact-node traces retain zero reads for the unused parameter and one each for the genuine body/default uses. After an invalid missing-target request, the same connection pings and successfully traces retained Evidence. Binary session stays unopened; quoted Unicode paths and source bytes are preserved. The inert fixture throws if executed. ## Validation performed Pinned Node24.18.0/npm11.16.0; project tests received no GitHub credentials. - `npm run test:focused -- src/domain/javascript/javascriptSemanticAnalysis.parameter_properties.test.ts tests/acceptance/applications/javascriptParameterProperties.test.ts` — **14passed/2files**, Linux arm64, actual CLI/stdio MCP. - New domain + existing `structure`, `function_names`, `logical_assignment` suites — **110passed/4files**, macOS arm64; overlaps the13 domain cases above. - `npm run check:fast`, `npm run check`, `npm run check:ci`, `npm run docs:check` — passed. - `npm run verify:test-discovery` — **1416 files exactly once**; `git diff --check` passed. - Independent AI review: no blockers; reviewer independently ran166 source tests and additional reference-role probes, and inspected the Linux logs. This is separate reviewer evidence, not an independent Linux/full-suite rerun. ## Compatibility, safety, and release Previously false declaration reads disappear; no invocation migration or schema change. Public workflows ran on Linux because this host's macOS Swift ownership-helper compilation is independently broken. Other OS public workflows, native engines and real-application prevalence were not verified. Locked Linux dependencies were installed in isolation; target source was never executed. Current head `62a336a70ef12e8eecf3ad885b3f287b1ea2dbd1`: [CI38861870463](https://github.com/morluto/rea/actions/runs/38861870463) **SUCCESS** —15 successful jobs,16 conditional skips,0 failed/incomplete. All five built shards, source/static/docs checks, Linux package baseline, portable report, CI required and real JavaScript recovery passed. Portable aggregate: **16,337passed/144skipped**, including the new acceptance regression. Actual tested merge `7fa62e63c99ea7cbdb61ff2990c08f747cc612e8` into base `2c758359`. This is **fast-tier** complete portable verification; coverage/full qualification were not executed. Verified2026-10-16T06:45:04Z. Full qualification is not yet claimed: fork dispatch currently returns HTTP422 Actions-disabled, and upstream dispatch permission is unavailable. No checks, thresholds, exclusions or assertions were weakened. AI-assisted work. ## Author readiness Human owner: @SII-YDD - [ ] 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. Draft under the project's human-ownership rule; AI review and CI do not attest to human review. ## Before merge - [x] Concrete problem, focused outcome, linked issue, no authority expansion. - [x] Observable behavior and preservation controls verified. - [ ] Current-head complete required verification and full qualification confirmed. - [ ] Human owner completed the readiness declarations. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:12:46 +00:00