[PR #6274] [CLOSED] fix(javascript): propagate ancestor absence in export shape comparisons #7248

New issue

Closed

opened 2026-11-01 21:46:28 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:46:28 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/javascript-export-shape-ancestor-absence

📝 Commits (2)

1baadbf fix(javascript): propagate ancestor absence in export shape comparisons
9a59548 test(javascript): align container coverage with ancestor absence

📊 Changes

4 files changed (+220 additions, -57 deletions)

View changed files

📝 src/domain/javascript/javascriptExportShapeVariants.ts (+49 -38)
📝 tests/acceptance/applications/applicationWorkflowCli.test.ts (+52 -0)
📝 tests/boundary/filesystem/javascriptExportContainerCoverage.test.ts (+10 -19)
📝 tests/boundary/filesystem/javascriptExportShapeComparison.test.ts (+109 -0)

📄 Description

Summary

Fixes #5914.

Adding nested: { total: 1 } to a tagged exported return object currently reports /nested as added but /nested/total as unknown, even when the other shape proves the whole object absent. Propagate proven ancestor absence so both paths are added, or removed in the reverse comparison, with complete comparison coverage.

Change and scope

Resolve missing-field presence by walking JSON Pointer parents in the shared export-shape comparator. Stop at retained containers, unresolved values, or uncertain presence unless absence is established. Index fields and property coverage once per paired shape; existing retained fields continue to use their explicit presence.

Semantic owner: src/domain/javascript/javascriptExportShapeVariants.ts; the producer already retains the necessary presence and coverage facts.
Contribution scope: ordinary static-analysis correctness fix; a separate maintainer scope decision is not applicable.
CLI/MCP schemas, tool catalog, providers, package metadata, and installation are unchanged. Both adapters use the corrected comparison result. Source inventories and Evidence links remain intact; derived comparison identities reflect the corrected result.
Target effects: existing inert source analysis and comparison only; this change adds no target execution, mutation, or network access.

Evidence and regression coverage

Executed on macOS arm64 with the pinned Node 24.18.0 and npm 11.16.0.

Base 2edf3b23c with the new tests: 5 failed, 88 passed across the two edited test files. Failures cover absent objects, multiple ancestors with escaped/empty names, absent arrays, explicit array holes, and the public CLI regression. Uncertainty controls already pass on the base.
After the fix: 157 tests passed across 8 related files, including the existing container-coverage regression and the new regression through the compiled CLI and stdio MCP in both directions.
The existing container-subtree fixture now expects both /options and /options/nested to be added (removed in reverse), with zero unknown changes and complete comparison coverage. It retains the value-availability, source-range, and Evidence-link assertions and explicitly checks property presence. The unchanged partial-coverage controls still require unknown descendants in both directions.
Forward public result: summary: { added: 2, removed: 0, changed: 0, unknown: 0 }; reverse: { added: 0, removed: 2, changed: 0, unknown: 0 }. Both report complete-within-inputs and correct presence for /nested and /nested/total.
Dynamic values, partial containers, unknown ancestor presence, and deleted ancestors still report unknown descendants in both directions. Existing large-inventory, schema, selection, and session tests also pass.

Executed focused test command

npm run test:focused -- \
  tests/boundary/filesystem/javascriptExportContainerCoverage.test.ts \
  tests/boundary/filesystem/javascriptExportShapeComparison.test.ts \
  tests/acceptance/applications/applicationWorkflowCli.test.ts \
  tests/boundary/mcp/applicationExportShapeMcp.test.ts \
  tests/boundary/filesystem/javascriptApplicationWorkflows.test.ts \
  src/domain/javascript/javascriptExportShapeComparisonSchemas.test.ts \
  tests/boundary/mcp/applicationMutationPresenceMcp.test.ts \
  tests/boundary/mcp/retainedApplicationEvidenceMcp.test.ts

Validation performed

Focused suite above — 8 files, 157 tests passed.
npm run check — passed, including typecheck, lint (0 errors), formatting, module boundaries, metadata, unused-code checks, and architecture guards.
npm run verify:test-discovery — all 1,993 test files discovered exactly once.
git diff --check — passed.

The full deterministic suite is left to CI. Windows/Linux and native reverse-engineering providers were not exercised; this change is in the shared static comparison logic. No performance improvement is claimed; CPU and memory were not benchmarked.

Current CI baseline comparison

PR CI 39976429109 tested merge commit 7e79fd5cd0fc3d258c5bb255194d2ec8ffa8a6d7 (base 60accad2de8f2862fe91901502fe26cc5390fb36 plus PR head 9a59548f53db21daf67d9b6292d71a80e1d10de3). The independent base CI 39975172638 tested that exact base without this PR. Both used Linux, Node 24.22.1, and npm 11.19.1.

All six observed PR test failures are also present on that base, with matching assertions/errors:

sourceToBundleComparison.test.ts: webpack:// source-root resolution returns unresolved instead of the expected suffix.
environmentBundle.test.ts: environment discovery exceeds its serialization reservation budget.
environmentReplayCapacity.test.ts: replay matching reports exact: false instead of true.
peCertificatesCli.test.ts: the TeletexString rejection case exits 0 instead of 1.
javascriptLiteralLoaderOwnership.test.ts: both the resolve-only and createRequire base MCP cases fail the same loader-ownership assertions.

These are existing failures on the executed base, so no unrelated code changes or CI retries were made for them. This comparison uses the recorded CI runs; it is not a new local reproduction. The failed source/shard jobs also make the aggregate checks fail. PR fail-fast cancellation stopped the other seven shards; shard 3 has no completion record for the updated container test. The full merged-tree suite therefore remains unverified; the local 157-test result above is the completed focused validation.

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.

_{🔄 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/6274 **Author:** [@ShitK](https://github.com/ShitK) **Created:** 10/22/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/javascript-export-shape-ancestor-absence` --- ### 📝 Commits (2) - [`1baadbf`](https://github.com/morluto/rea/commit/1baadbfe3f233d92ba6dc0970241939299db5051) fix(javascript): propagate ancestor absence in export shape comparisons - [`9a59548`](https://github.com/morluto/rea/commit/9a59548f53db21daf67d9b6292d71a80e1d10de3) test(javascript): align container coverage with ancestor absence ### 📊 Changes **4 files changed** (+220 additions, -57 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptExportShapeVariants.ts` (+49 -38) 📝 `tests/acceptance/applications/applicationWorkflowCli.test.ts` (+52 -0) 📝 `tests/boundary/filesystem/javascriptExportContainerCoverage.test.ts` (+10 -19) 📝 `tests/boundary/filesystem/javascriptExportShapeComparison.test.ts` (+109 -0) </details> ### 📄 Description ## Summary Fixes #5914. Adding `nested: { total: 1 }` to a tagged exported return object currently reports `/nested` as added but `/nested/total` as unknown, even when the other shape proves the whole object absent. Propagate proven ancestor absence so both paths are added, or removed in the reverse comparison, with complete comparison coverage. ## Change and scope Resolve missing-field presence by walking JSON Pointer parents in the shared export-shape comparator. Stop at retained containers, unresolved values, or uncertain presence unless absence is established. Index fields and property coverage once per paired shape; existing retained fields continue to use their explicit presence. - Semantic owner: `src/domain/javascript/javascriptExportShapeVariants.ts`; the producer already retains the necessary presence and coverage facts. - Contribution scope: ordinary static-analysis correctness fix; a separate maintainer scope decision is not applicable. - CLI/MCP schemas, tool catalog, providers, package metadata, and installation are unchanged. Both adapters use the corrected comparison result. Source inventories and Evidence links remain intact; derived comparison identities reflect the corrected result. - Target effects: existing inert source analysis and comparison only; this change adds no target execution, mutation, or network access. ## Evidence and regression coverage Executed on macOS arm64 with the pinned Node 24.18.0 and npm 11.16.0. - **Base `2edf3b23c` with the new tests:** 5 failed, 88 passed across the two edited test files. Failures cover absent objects, multiple ancestors with escaped/empty names, absent arrays, explicit array holes, and the public CLI regression. Uncertainty controls already pass on the base. - **After the fix:** 157 tests passed across 8 related files, including the existing container-coverage regression and the new regression through the compiled CLI and stdio MCP in both directions. - The existing container-subtree fixture now expects both `/options` and `/options/nested` to be added (removed in reverse), with zero unknown changes and complete comparison coverage. It retains the value-availability, source-range, and Evidence-link assertions and explicitly checks property presence. The unchanged partial-coverage controls still require unknown descendants in both directions. - Forward public result: `summary: { added: 2, removed: 0, changed: 0, unknown: 0 }`; reverse: `{ added: 0, removed: 2, changed: 0, unknown: 0 }`. Both report `complete-within-inputs` and correct presence for `/nested` and `/nested/total`. - Dynamic values, partial containers, unknown ancestor presence, and deleted ancestors still report unknown descendants in both directions. Existing large-inventory, schema, selection, and session tests also pass. <details> <summary>Executed focused test command</summary> ```sh npm run test:focused -- \ tests/boundary/filesystem/javascriptExportContainerCoverage.test.ts \ tests/boundary/filesystem/javascriptExportShapeComparison.test.ts \ tests/acceptance/applications/applicationWorkflowCli.test.ts \ tests/boundary/mcp/applicationExportShapeMcp.test.ts \ tests/boundary/filesystem/javascriptApplicationWorkflows.test.ts \ src/domain/javascript/javascriptExportShapeComparisonSchemas.test.ts \ tests/boundary/mcp/applicationMutationPresenceMcp.test.ts \ tests/boundary/mcp/retainedApplicationEvidenceMcp.test.ts ``` </details> ## Validation performed - Focused suite above — 8 files, 157 tests passed. - `npm run check` — passed, including typecheck, lint (0 errors), formatting, module boundaries, metadata, unused-code checks, and architecture guards. - `npm run verify:test-discovery` — all 1,993 test files discovered exactly once. - `git diff --check` — passed. The full deterministic suite is left to CI. Windows/Linux and native reverse-engineering providers were not exercised; this change is in the shared static comparison logic. No performance improvement is claimed; CPU and memory were not benchmarked. ## Current CI baseline comparison [PR CI 39976429109](https://github.com/morluto/rea/actions/runs/39976429109) tested merge commit `7e79fd5cd0fc3d258c5bb255194d2ec8ffa8a6d7` (base `60accad2de8f2862fe91901502fe26cc5390fb36` plus PR head `9a59548f53db21daf67d9b6292d71a80e1d10de3`). The independent [base CI 39975172638](https://github.com/morluto/rea/actions/runs/39975172638) tested that exact base without this PR. Both used Linux, Node 24.22.1, and npm 11.19.1. All six observed PR test failures are also present on that base, with matching assertions/errors: - `sourceToBundleComparison.test.ts`: `webpack://` source-root resolution returns unresolved instead of the expected suffix. - `environmentBundle.test.ts`: environment discovery exceeds its serialization reservation budget. - `environmentReplayCapacity.test.ts`: replay matching reports `exact: false` instead of `true`. - `peCertificatesCli.test.ts`: the TeletexString rejection case exits 0 instead of 1. - `javascriptLiteralLoaderOwnership.test.ts`: both the `resolve-only` and `createRequire base` MCP cases fail the same loader-ownership assertions. These are existing failures on the executed base, so no unrelated code changes or CI retries were made for them. This comparison uses the recorded CI runs; it is not a new local reproduction. The failed source/shard jobs also make the aggregate checks fail. PR fail-fast cancellation stopped the other seven shards; shard 3 has no completion record for the updated container test. The full merged-tree suite therefore remains unverified; the local 157-test result above is the completed focused validation. ## 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:46:28 +00:00