[PR #4020] [CLOSED] fix(javascript): preserve structural fingerprint semantics #5994

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4020
Author: @ShitK
Created: 10/15/2026
Status: ❌ Closed

Base: main ← Head: fix/javascript-structural-fingerprint

📝 Commits (2)

61c826f fix(javascript): retain literal and assignment fingerprint semantics
df093a7 fix(javascript): retain class member names in structural fingerprints

📊 Changes

8 files changed (+314 additions, -46 deletions)

View changed files

📝 docs/javascript-artifact-reconstruction.md (+20 -6)
📝 src/domain/javascript/javascriptApplicationVersionMatching.test.ts (+28 -2)
📝 src/domain/javascript/javascriptAstFingerprint.test.ts (+70 -1)
📝 src/domain/javascript/javascriptAstFingerprint.ts (+20 -9)
📝 src/domain/javascript/javascriptStaticAnalysisTypes.ts (+1 -1)
📝 src/javascript/analysis/JavaScriptAnalysisWorkerSchemas.test.ts (+31 -26)
📝 tests/acceptance/applications/applicationVersionMatchingCli.test.ts (+143 -0)
📝 tests/boundary/filesystem/javascript-applications/javascriptArtifactReconstruction.test.ts (+1 -1)

📄 Description

Summary

Fixes #3952.

Bundle factories could keep the same structural fingerprint after bigint values or keys, directive text, or assignment operators changed. Public class method and field names had the same omission. Exact source digests differed, but version comparison could still pair these modules through structural equality.

Change and scope

Retain bigint/directive values, assignment operators, and fixed public class method/field names at the AST fingerprint producer. Reuse the existing exact property-key reader for fixed object and class keys, including bigint and empty keys.
Preserve ordinary identifier, computed-key binding, and private member rename compatibility. Cover public instance/static methods and fields, getters/setters, computed literal keys, and fixed-versus-computed keys.
Emit babel-ast-v3. The comparison key includes the algorithm: retained v1↔v1 and v2↔v2 Evidence can still match structurally, while v1/v2 cannot structurally match v3. Other matching tiers remain available across versions. Re-analyze both snapshots to use the new rules.
Verify comparison outcomes through the compiled CLI. With complete input coverage and no alternative match or ambiguous candidate, the changed modules produce separate removed/added items; their containing files still report changed. All three local-rename controls retain structural matching. Document these conditions rather than treating every semantic change as removal/addition.

The class-key correction is at the same producer and remains part of this PR's unmerged v3 change. A fresh check of main 47b4628ba and 348 open PR file lists (including large-PR pagination) found no duplicate class-key fix. Same-file #3552 changes export alias handling, not fingerprint tokens.

This stays within existing static analysis. Parser support, dependencies, CLI/MCP inputs, exact-source evidence, and process ownership are unchanged. No target code is executed and no generated artifacts are committed.

Evidence and validation

Executed on macOS arm64 with pinned Node 24.18.0 / npm 11.16.0, based on 6fcfc5b5e0c063300cd5278dd8bad5b1ab1374f6:

Before the initial fix, six new AST cases and the compiled CLI regression failed; the other 13 tests in those two files passed.
Before the class-key follow-up, eight class AST regressions and the expanded CLI regression failed; the other 28 tests in those three files passed. The CLI failure specifically observed equal class-method fingerprints.
After the complete fix, 68/68 tests across six files passed, including actual CLI analysis of both application versions followed by comparison, filesystem artifact reconstruction, matching/comparison, AST fingerprints, and worker schema admission.
npm run check passed (zero lint errors; warning output remains).
npm run docs:check passed, including 5 README consistency tests.
npm run verify:test-discovery passed: all 1,321 files discovered exactly once.
git diff --check passed.

Focused command:

npm run test:focused -- src/domain/javascript/javascriptAstFingerprint.test.ts src/domain/javascript/javascriptApplicationVersionMatching.test.ts src/domain/javascript/javascriptApplicationVersionComparison.test.ts src/javascript/analysis/JavaScriptAnalysisWorkerSchemas.test.ts tests/boundary/filesystem/javascript-applications/javascriptArtifactReconstruction.test.ts tests/acceptance/applications/applicationVersionMatchingCli.test.ts

CLI tests used normal host access for the macOS process-ownership helper's Clang cache; no process checks or assertions were weakened. Separate MCP transport, Windows/Linux execution, and the full suite were not run locally. On the updated commit df093a70e, static checks, package build, docs generation, and real JavaScript recovery have passed. Source tests, test shards, platform lanes, and automated review are still running or queued at the latest check; no failures are reported yet. Current CI run.

Author readiness

Human owner: @ShitK

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.

AI assistance was used for investigation, implementation and validation. The existing initial-readiness declarations are preserved; this PR remains ready for review.

_{🔄 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/4020 **Author:** [@ShitK](https://github.com/ShitK) **Created:** 10/15/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/javascript-structural-fingerprint` --- ### 📝 Commits (2) - [`61c826f`](https://github.com/morluto/rea/commit/61c826f882c535778cc2c44cfa1ae125891ca59d) fix(javascript): retain literal and assignment fingerprint semantics - [`df093a7`](https://github.com/morluto/rea/commit/df093a70e7fbe235c6a9b997ae52d342119c8e50) fix(javascript): retain class member names in structural fingerprints ### 📊 Changes **8 files changed** (+314 additions, -46 deletions) <details> <summary>View changed files</summary> 📝 `docs/javascript-artifact-reconstruction.md` (+20 -6) 📝 `src/domain/javascript/javascriptApplicationVersionMatching.test.ts` (+28 -2) 📝 `src/domain/javascript/javascriptAstFingerprint.test.ts` (+70 -1) 📝 `src/domain/javascript/javascriptAstFingerprint.ts` (+20 -9) 📝 `src/domain/javascript/javascriptStaticAnalysisTypes.ts` (+1 -1) 📝 `src/javascript/analysis/JavaScriptAnalysisWorkerSchemas.test.ts` (+31 -26) 📝 `tests/acceptance/applications/applicationVersionMatchingCli.test.ts` (+143 -0) 📝 `tests/boundary/filesystem/javascript-applications/javascriptArtifactReconstruction.test.ts` (+1 -1) </details> ### 📄 Description ## Summary Fixes #3952. Bundle factories could keep the same structural fingerprint after bigint values or keys, directive text, or assignment operators changed. Public class method and field names had the same omission. Exact source digests differed, but version comparison could still pair these modules through structural equality. ## Change and scope - Retain bigint/directive values, assignment operators, and fixed public class method/field names at the AST fingerprint producer. Reuse the existing exact property-key reader for fixed object and class keys, including bigint and empty keys. - Preserve ordinary identifier, computed-key binding, and private member rename compatibility. Cover public instance/static methods and fields, getters/setters, computed literal keys, and fixed-versus-computed keys. - Emit `babel-ast-v3`. The comparison key includes the algorithm: retained v1↔v1 and v2↔v2 Evidence can still match structurally, while v1/v2 cannot structurally match v3. Other matching tiers remain available across versions. Re-analyze both snapshots to use the new rules. - Verify comparison outcomes through the compiled CLI. With complete input coverage and no alternative match or ambiguous candidate, the changed modules produce separate `removed`/`added` items; their containing files still report `changed`. All three local-rename controls retain structural matching. Document these conditions rather than treating every semantic change as removal/addition. The class-key correction is at the same producer and remains part of this PR's unmerged v3 change. A fresh check of main `47b4628ba` and 348 open PR file lists (including large-PR pagination) found no duplicate class-key fix. Same-file #3552 changes export alias handling, not fingerprint tokens. This stays within existing static analysis. Parser support, dependencies, CLI/MCP inputs, exact-source evidence, and process ownership are unchanged. No target code is executed and no generated artifacts are committed. ## Evidence and validation Executed on macOS arm64 with pinned Node 24.18.0 / npm 11.16.0, based on `6fcfc5b5e0c063300cd5278dd8bad5b1ab1374f6`: - Before the initial fix, six new AST cases and the compiled CLI regression failed; the other 13 tests in those two files passed. - Before the class-key follow-up, eight class AST regressions and the expanded CLI regression failed; the other 28 tests in those three files passed. The CLI failure specifically observed equal class-method fingerprints. - After the complete fix, **68/68 tests across six files passed**, including actual CLI analysis of both application versions followed by comparison, filesystem artifact reconstruction, matching/comparison, AST fingerprints, and worker schema admission. - `npm run check` passed (zero lint errors; warning output remains). - `npm run docs:check` passed, including 5 README consistency tests. - `npm run verify:test-discovery` passed: all 1,321 files discovered exactly once. - `git diff --check` passed. Focused command: ```sh npm run test:focused -- src/domain/javascript/javascriptAstFingerprint.test.ts src/domain/javascript/javascriptApplicationVersionMatching.test.ts src/domain/javascript/javascriptApplicationVersionComparison.test.ts src/javascript/analysis/JavaScriptAnalysisWorkerSchemas.test.ts tests/boundary/filesystem/javascript-applications/javascriptArtifactReconstruction.test.ts tests/acceptance/applications/applicationVersionMatchingCli.test.ts ``` CLI tests used normal host access for the macOS process-ownership helper's Clang cache; no process checks or assertions were weakened. Separate MCP transport, Windows/Linux execution, and the full suite were not run locally. On the updated commit `df093a70e`, static checks, package build, docs generation, and real JavaScript recovery have passed. Source tests, test shards, platform lanes, and automated review are still running or queued at the latest check; no failures are reported yet. [Current CI run](https://github.com/morluto/rea/actions/runs/38714950664). ## Author readiness Human owner: @ShitK - [x] 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. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. AI assistance was used for investigation, implementation and validation. The existing initial-readiness declarations are preserved; this PR remains ready for review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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