[PR #3463] [CLOSED] test(artifact): cover multiple inspection inventory substeps #5754

Closed
opened 2026-10-31 01:09:10 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3463
Author: @AdityaKharmaleGoat7
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/inspect-artifact-multiple-substeps


📝 Commits (5)

  • 7239128 test(artifact): cover multi-substep inspection inventory
  • eb34cca Merge origin/main into fix/inspect-artifact-multiple-substeps
  • c730c3f test: explain synthetic inspection substep id
  • c26c02c Merge upstream main into artifact inventory regression branch
  • e780368 test(artifact): refresh upstream and use Evidence-owned JSON admission

📊 Changes

1 file changed (+97 additions, -0 deletions)

View changed files

➕ tests/boundary/filesystem/artifactInspectionSubsteps.test.ts (+97 -0)

📄 Description

Regression coverage for #3459; the production fix is already on upstream main.

Summary

Adds focused regression coverage for inspect_artifact Evidence containing multiple retained inventory_artifact substeps. Current main already contains the production-side parsing and validation needed for this case, so this PR is intentionally limited to proving that all retained inventory substeps are merged rather than only the first one.

Problem and expected behavior

An inspect_artifact Evidence record can contain multiple retained inventory_artifact substeps. The reconstructed artifact inventory must include content from every retained substep while keeping the parent inspect_artifact Evidence as the comparison citation source.

The regression test constructs a two-page inventory and verifies that nodes, occurrences, and edges from both pages are reconstructed completely. On an affected pre-fix revision that consumes only the first retained substep, this test is expected to fail because the reconstructed inventory would not match the full manifest counts. Current main contains the production fix, so the regression passes there.

Change and scope

This PR adds one boundary regression test in tests/boundary/filesystem/artifactInspectionSubsteps.test.ts. The branch now includes upstream main at 1173c0a9dc1b352a60943a491462294db8f353ae, including the shared Apple metadata and Webpack fixture fixes (#6945 and #6951). The test passes payloads directly to createEvidence, which owns JSON admission, as required by current lint rules. Its synthetic substep ID comment describes schema validity without claiming uniqueness is checked.

The test:

  • builds a real two-page artifact inventory;
  • wraps both pages in one inspect_artifact Evidence record;
  • verifies the parent Evidence remains the comparison citation source;
  • verifies all nodes, occurrences, and edges from both retained substeps are assembled; and
  • verifies comparing the reconstructed parent against itself remains unchanged.

No production behavior is changed by this PR. The production-side fix is already present on current main.

Follow the contribution scope.

Contract and boundary impact

  • Semantic owner and earliest changed stage: artifact inspection/inventory Evidence parsing boundary; test coverage only.
  • Contribution scope fit and linked maintainer decision: ordinary regression coverage within existing behavior; additional maintainer scope decision not applicable.
  • CLI and MCP/tool-catalog contract: none.
  • Provider, bridge, target-format, or platform compatibility: none.
  • Evidence, artifact, provenance, or reconstruction contract: verifies that all retained inventory substeps reconstruct into one complete inventory while the parent inspect_artifact Evidence remains the cited source.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable; test uses an isolated temporary artifact fixture.
  • Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

The checked file effects are limited to the temporary test fixture created by the boundary test.

Evidence and regression coverage

  • Tests added or updated: tests/boundary/filesystem/artifactInspectionSubsteps.test.ts adds the two-substep boundary regression.
  • Base reproduction or other evidence: the test is expected to fail on an affected pre-fix revision that reads only the first retained substep, because reconstructed node/occurrence/edge counts would not match the complete manifest. Current main already contains the production fix.
  • User-visible CLI/MCP output (if applicable): not applicable.
  • Remaining proof gaps: the PR does not add a separate checkout/run against the historical affected revision; the failure mode is encoded by the manifest-count assertions in the regression.

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

On the refreshed branch:

  • npm run test:focused -- tests/boundary/filesystem/artifactInspectionSubsteps.test.ts src/native/AppleDispatchMetadata.failure.test.ts — 6 tests passed.
  • npm run typecheck — passed.
  • npm run lint — passed with existing warnings; module boundaries passed.
  • oxfmt --check tests/boundary/filesystem/artifactInspectionSubsteps.test.ts and git diff --check — passed.

The full hosted CI run is pending. A local run of the shared Webpack acceptance file on the refreshed port-test branch built successfully but failed because this environment's procps ps reports fatal library error, lookup self; no passing local CLI/MCP acceptance coverage is claimed.

Compatibility, safety, and release

  • Breaking changes or migration steps: none; test-only change.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no production execution or target interaction is added; the test operates on an isolated temporary fixture.

Author readiness

Human owner: @AdityaKharmaleGoat7

  • 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.

🔄 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/3463 **Author:** [@AdityaKharmaleGoat7](https://github.com/AdityaKharmaleGoat7) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/inspect-artifact-multiple-substeps` --- ### 📝 Commits (5) - [`7239128`](https://github.com/morluto/rea/commit/72391282785cba24fa1574530c3e6799831a6e63) test(artifact): cover multi-substep inspection inventory - [`eb34cca`](https://github.com/morluto/rea/commit/eb34cca941d448038e5e975c485a98dffecd0d9b) Merge origin/main into fix/inspect-artifact-multiple-substeps - [`c730c3f`](https://github.com/morluto/rea/commit/c730c3f7ce9d7361d7f132e08a352ee838e0f9ea) test: explain synthetic inspection substep id - [`c26c02c`](https://github.com/morluto/rea/commit/c26c02cfd5537405ff1fe47917845b901f77da93) Merge upstream main into artifact inventory regression branch - [`e780368`](https://github.com/morluto/rea/commit/e7803681b83844bde2f2eb18bcbec71c1ae0476c) test(artifact): refresh upstream and use Evidence-owned JSON admission ### 📊 Changes **1 file changed** (+97 additions, -0 deletions) <details> <summary>View changed files</summary> ➕ `tests/boundary/filesystem/artifactInspectionSubsteps.test.ts` (+97 -0) </details> ### 📄 Description Regression coverage for #3459; the production fix is already on upstream `main`. ## Summary Adds focused regression coverage for `inspect_artifact` Evidence containing multiple retained `inventory_artifact` substeps. Current `main` already contains the production-side parsing and validation needed for this case, so this PR is intentionally limited to proving that all retained inventory substeps are merged rather than only the first one. ## Problem and expected behavior An `inspect_artifact` Evidence record can contain multiple retained `inventory_artifact` substeps. The reconstructed artifact inventory must include content from every retained substep while keeping the parent `inspect_artifact` Evidence as the comparison citation source. The regression test constructs a two-page inventory and verifies that nodes, occurrences, and edges from both pages are reconstructed completely. On an affected pre-fix revision that consumes only the first retained substep, this test is expected to fail because the reconstructed inventory would not match the full manifest counts. Current `main` contains the production fix, so the regression passes there. ## Change and scope This PR adds one boundary regression test in `tests/boundary/filesystem/artifactInspectionSubsteps.test.ts`. The branch now includes upstream `main` at `1173c0a9dc1b352a60943a491462294db8f353ae`, including the shared Apple metadata and Webpack fixture fixes (#6945 and #6951). The test passes payloads directly to `createEvidence`, which owns JSON admission, as required by current lint rules. Its synthetic substep ID comment describes schema validity without claiming uniqueness is checked. The test: - builds a real two-page artifact inventory; - wraps both pages in one `inspect_artifact` Evidence record; - verifies the parent Evidence remains the comparison citation source; - verifies all nodes, occurrences, and edges from both retained substeps are assembled; and - verifies comparing the reconstructed parent against itself remains `unchanged`. No production behavior is changed by this PR. The production-side fix is already present on current `main`. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). ## Contract and boundary impact - Semantic owner and earliest changed stage: artifact inspection/inventory Evidence parsing boundary; test coverage only. - Contribution scope fit and linked maintainer decision: ordinary regression coverage within existing behavior; additional maintainer scope decision not applicable. - CLI and MCP/tool-catalog contract: none. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: verifies that all retained inventory substeps reconstruct into one complete inventory while the parent `inspect_artifact` Evidence remains the cited source. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable; test uses an isolated temporary artifact fixture. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [x] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. The checked file effects are limited to the temporary test fixture created by the boundary test. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/filesystem/artifactInspectionSubsteps.test.ts` adds the two-substep boundary regression. - Base reproduction or other evidence: the test is expected to fail on an affected pre-fix revision that reads only the first retained substep, because reconstructed node/occurrence/edge counts would not match the complete manifest. Current `main` already contains the production fix. - User-visible CLI/MCP output (if applicable): not applicable. - Remaining proof gaps: the PR does not add a separate checkout/run against the historical affected revision; the failure mode is encoded by the manifest-count assertions in the regression. 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 On the refreshed branch: - `npm run test:focused -- tests/boundary/filesystem/artifactInspectionSubsteps.test.ts src/native/AppleDispatchMetadata.failure.test.ts` — 6 tests passed. - `npm run typecheck` — passed. - `npm run lint` — passed with existing warnings; module boundaries passed. - `oxfmt --check tests/boundary/filesystem/artifactInspectionSubsteps.test.ts` and `git diff --check` — passed. The full hosted CI run is pending. A local run of the shared Webpack acceptance file on the refreshed port-test branch built successfully but failed because this environment's procps `ps` reports `fatal library error, lookup self`; no passing local CLI/MCP acceptance coverage is claimed. ## Compatibility, safety, and release - Breaking changes or migration steps: none; test-only change. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no production execution or target interaction is added; the test operates on an isolated temporary fixture. ## Author readiness Human owner: @AdityaKharmaleGoat7 - [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. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:09:10 +00:00
Sign in to join this conversation.
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#5754
No description provided.