morluto/rea

Fork 0

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

[PR #569] [MERGED] fix(artifacts): exclude removed paths from unchanged counts #2070

New issue

Closed

opened 2026-10-16 21:13:56 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-16 21:13:56 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/569
Author: @kaoru0822-kitauji
Created: 10/5/2026
Status: ✅ Merged
Merged: 10/5/2026
Merged by: @morluto

Base: main ← Head: fix/artifact-comparison-unchanged-count

📝 Commits (1)

01accbc fix(artifacts): exclude removed paths from unchanged counts

📊 Changes

2 files changed (+90 additions, -1 deletions)

View changed files

📝 src/domain/artifactComparison.ts (+2 -1)
➕ tests/boundary/filesystem/artifactComparisonSummary.test.ts (+88 -0)

📄 Description

Summary

Correct compare_artifacts so simultaneous additions and removals are not counted as unchanged paths. This was found during a repository audit; no existing issue describes this summary error.

Problem and expected behavior

The summary used the smaller inventory length, then subtracted changed, contradictory, and unknown paths. When one version removes a file and adds another, both inventories can have the same size even though the shared path set is smaller. For two preserved files and one replaced filename, the result incorrectly reported three unchanged paths instead of two.

The unchanged count must include only paths present in both inventories that have no change classification.

Change and scope

Start with the left inventory count and subtract removals as well as changed, contradictory, and unknown shared paths.
Add six real-filesystem inventory regressions for balanced and unbalanced additions/removals, additions only, removals only, and identical inputs.
Include two preserved paths with identical bytes to verify that the summary counts occurrences rather than unique content digests.

Contract and boundary impact

Semantic owner: artifact-comparison summary in the domain layer.
CLI/MCP schemas and tool inventory: unchanged; the existing summary.unchanged result is corrected.
Provider, bridge, format, or platform compatibility: unchanged.
Evidence/provenance: existing citations and individual change classifications are preserved. Corrected summaries naturally produce different derived Evidence IDs where the old count was wrong.
Execution, authorization, cleanup, and containment: unchanged.
Generated metadata and packaging: no changes required.

Evidence and regression coverage

On base 92617f965720aaa74ec20ad67291adc5e83677a1, the three simultaneous-add/remove cases fail with unchanged: 3 instead of 2; the three control cases pass. With this fix, all six cases and the five existing artifact-comparison tests pass.

Validation performed

Rebased without conflicts onto current main c6d3bdf597d9cd75a6f7396236e1a47f5ee4288d. The 11 focused tests and npm run check:fast were rerun successfully after rebasing. The broader verification below ran on the original base 92617f9; GitHub CI is expected to verify the final rebased commit.

npx --no-install vitest run --coverage.enabled=false tests/boundary/filesystem/artifactComparisonSummary.test.ts tests/composition/analysis-sessions/artifactComparison.test.ts: 11 tests passed.
npm run check:ci: passed all seven static tasks, including typecheck, lint, formatting, dead-code checks, metadata, duplication, and TODO scan.
npm run docs:check: passed; TypeDoc reported 228 existing warnings and no errors.
npm run verify:package: passed the Linux package/install, CLI, and MCP verifier (122 tools).
npm pack --dry-run --json: passed.
git diff --check: passed.
npm run check:pr: static stages passed; complete deterministic suite reported 1,941 passed, 40 failed, and 2 skipped. All 40 failures in the same seven Hopper/Ghidra bridge suites reproduce on the unchanged base in a detached worktree. This execution environment rejects Unix-domain socket listeners with EPERM, leading to bridge-startup failures/timeouts. No tests or permission checks were relaxed.

Compatibility, safety, and release

No breaking schema or migration changes. This change does not alter real Hopper/Ghidra behavior; no real Hopper/Ghidra verification was performed. Package verification establishes its documented narrower package/integration claims, not real native-provider coverage.

Review checklist

One focused outcome and Conventional Commit title.
Observable regression reproduced on the base revision.
Tests cover both churn directions and unaffected controls.
Independent code review found no correctness issues.
No generated metadata changes needed.
Diff checked for unrelated changes, secrets, 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/569 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/5/2026 **Status:** ✅ Merged **Merged:** 10/5/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/artifact-comparison-unchanged-count` --- ### 📝 Commits (1) - [`01accbc`](https://github.com/morluto/rea/commit/01accbc4989106ef1f3233d529b9f20d07ee54cf) fix(artifacts): exclude removed paths from unchanged counts ### 📊 Changes **2 files changed** (+90 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/artifactComparison.ts` (+2 -1) ➕ `tests/boundary/filesystem/artifactComparisonSummary.test.ts` (+88 -0) </details> ### 📄 Description ## Summary Correct `compare_artifacts` so simultaneous additions and removals are not counted as unchanged paths. This was found during a repository audit; no existing issue describes this summary error. ## Problem and expected behavior The summary used the smaller inventory length, then subtracted changed, contradictory, and unknown paths. When one version removes a file and adds another, both inventories can have the same size even though the shared path set is smaller. For two preserved files and one replaced filename, the result incorrectly reported three unchanged paths instead of two. The unchanged count must include only paths present in both inventories that have no change classification. ## Change and scope - Start with the left inventory count and subtract removals as well as changed, contradictory, and unknown shared paths. - Add six real-filesystem inventory regressions for balanced and unbalanced additions/removals, additions only, removals only, and identical inputs. - Include two preserved paths with identical bytes to verify that the summary counts occurrences rather than unique content digests. ## Contract and boundary impact - Semantic owner: artifact-comparison summary in the domain layer. - CLI/MCP schemas and tool inventory: unchanged; the existing `summary.unchanged` result is corrected. - Provider, bridge, format, or platform compatibility: unchanged. - Evidence/provenance: existing citations and individual change classifications are preserved. Corrected summaries naturally produce different derived Evidence IDs where the old count was wrong. - Execution, authorization, cleanup, and containment: unchanged. - Generated metadata and packaging: no changes required. ## Evidence and regression coverage On base `92617f965720aaa74ec20ad67291adc5e83677a1`, the three simultaneous-add/remove cases fail with `unchanged: 3` instead of `2`; the three control cases pass. With this fix, all six cases and the five existing artifact-comparison tests pass. ## Validation performed Rebased without conflicts onto current main `c6d3bdf597d9cd75a6f7396236e1a47f5ee4288d`. The 11 focused tests and `npm run check:fast` were rerun successfully after rebasing. The broader verification below ran on the original base `92617f9`; GitHub CI is expected to verify the final rebased commit. - `npx --no-install vitest run --coverage.enabled=false tests/boundary/filesystem/artifactComparisonSummary.test.ts tests/composition/analysis-sessions/artifactComparison.test.ts`: 11 tests passed. - `npm run check:ci`: passed all seven static tasks, including typecheck, lint, formatting, dead-code checks, metadata, duplication, and TODO scan. - `npm run docs:check`: passed; TypeDoc reported 228 existing warnings and no errors. - `npm run verify:package`: passed the Linux package/install, CLI, and MCP verifier (122 tools). - `npm pack --dry-run --json`: passed. - `git diff --check`: passed. - `npm run check:pr`: static stages passed; complete deterministic suite reported 1,941 passed, 40 failed, and 2 skipped. All 40 failures in the same seven Hopper/Ghidra bridge suites reproduce on the unchanged base in a detached worktree. This execution environment rejects Unix-domain socket listeners with `EPERM`, leading to bridge-startup failures/timeouts. No tests or permission checks were relaxed. ## Compatibility, safety, and release No breaking schema or migration changes. This change does not alter real Hopper/Ghidra behavior; no real Hopper/Ghidra verification was performed. Package verification establishes its documented narrower package/integration claims, not real native-provider coverage. ## Review checklist - [x] One focused outcome and Conventional Commit title. - [x] Observable regression reproduced on the base revision. - [x] Tests cover both churn directions and unaffected controls. - [x] Independent code review found no correctness issues. - [x] No generated metadata changes needed. - [x] Diff checked for unrelated changes, secrets, 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-16 21:13:56 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-10-31 00:57:54 +00:00

[PR #2070] [MERGED] test(apple): skip ibtool-compiled NIB tests when Xcode ibtool is unavailable #5004

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#2070

No description provided.

Rows
Columns