morluto/rea

Fork 0

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

[PR #4014] [CLOSED] fix(browser): compare retained script source identities #5990

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/3879-script-content-comparison

📝 Commits (9)

81c14ac fix(browser): compare retained script source identities
d15d62e test(browser): place script comparison CLI regression in acceptance
e9f7c93 fix(browser): preserve unknown duplicate identities across omissions
7bf34bc fix(browser): keep bare script identities unknown after omissions
e819785 fix(browser): match duplicate script sources before residual changes
90eec6a fix(browser): require retained script coverage before source matching
5d78d6f fix(browser): preserve uncertainty after dropped script events
10533f5 chore: merge upstream main into script comparison fix
9756a51 fix(browser): preserve provable partial script comparisons

📊 Changes

4 files changed (+845 additions, -28 deletions)

View changed files

📝 src/domain/webCaptureDiff.ts (+25 -28)
➕ src/domain/webCaptureScriptComparison.ts (+170 -0)
➕ tests/acceptance/tools/scriptCaptureComparison.test.ts (+644 -0)
📝 tests/boundary/browser/webCaptureDiff.test.ts (+6 -0)

📄 Description

Summary

Fixes #3879. Compare retained script content without using capture-local duplicate ordinals as cross-capture identity.

complete script inventory = clear flags + zero drops + matching row counts + no nonzero/unknown script exclusions
match equal known content counts -> allow unknown source slots to explain residuals -> report only remaining changes
incomplete inventory -> preserve metadata/producer-hash comparison and unknown identity

Problem and expected behavior

Different retained sources with identical metadata and a null producer hash could report unchanged. Duplicate keys can be renumbered when scripts are omitted, so ordinal differences alone do not prove replacement. Conversely, missing source text must not hide a provable difference: [A,A] versus [B,?] cannot describe identical content. Explicit script exclusions must limit completeness even when redundant section flags are absent.

Change and scope

The shared domain comparator cancels equal known fingerprints with multiplicity, then pairs unknown sources with compatible residual metadata before comparing remaining occurrences. Original capture keys are preserved. Script inventory completeness includes nonzero or unknown script exclusions; zero counts and unrelated exclusions do not disable script comparison. Incomplete inventories retain the metadata/producer-hash path.

The existing boundary fixture helper now removes exclusion rows when it explicitly marks their sections complete. Assertions and unrelated omission evidence are preserved.

Contract and boundary impact

Semantic owner: shared offline domain comparison used by CLI/MCP.
Contribution scope: correctness fix within an existing workflow; advance maintainer scope decision not applicable.
CLI/MCP schemas, catalog, provider bridges, generated metadata and installation: unchanged.
Evidence: caller inputs, digests, multiplicity, original keys and explicit unknowns remain preserved.
Authority: compare normalized capture files; no target acquisition or mutation.

Expected effects:

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.

Evidence

Before: on published head 10533f576b, the new actual-CLI partial-source regression failed: [A,A] versus [B,?] returned unknown / 0 rather than one provable replacement; the previous 16 CLI regressions passed.
Before: after the partial-source fix, the exclusion-only regression independently failed: schema-valid script exclusions with clear section flags returned changed / 1 rather than unknown / 0.
After: both directions preserve forced replacement counts, including a matching non-null producer hash. Unknown slots that can explain all differences remain unknown. Script exclusion counts 1 and null prevent source matching; zero counts and unrelated exclusions retain known changes.
Synthetic normalized captures exercise the real compiled CLI. They do not establish live browser collection behavior.

Validation performed

Fresh on 2026-10-20, Windows x64, pinned Node 24.18.0 / npm 11.16.0:

npm run test:focused -- tests/acceptance/tools/scriptCaptureComparison.test.ts tests/boundary/browser/webCaptureDiff.test.ts — 51 passed: 22 actual-CLI tests and 29 boundary tests, zero skipped. Required runtime artifacts prepared by the repository runner; final preparation reused its valid cache, tests executed freshly.
npm run test:local -- src/domain/webCaptureDiff.network.test.ts — 45 passed, zero skipped.
npm run check — passed: typecheck, lint (0 errors, 1740 repository warnings), format, knip, metadata, module boundaries and architecture guards. Two of six Turbo tasks reused valid caches.
git diff --check — passed.
No test files added or moved; discovery not rerun. No schema or authored documentation changes; generated-document checks not rerun for this follow-up.

Compatibility, safety, and release

No migration, schema or dependency change. Full suite, live MCP transport, live Chromium and other operating systems were not newly verified. MCP shares the comparator; these tests establish CLI and boundary behavior. No performance improvement claimed.

Published-head CI 39251655449 remains failed; the local candidate has not been published and has no new CI result. Upstream cleanup repair #5249 has merged, but this candidate does not include a new upstream merge or claim that its former CI failures are resolved.

Merge Danger

Door: two-way — revert comparator changes and their regressions together.

Blast Radius: script comparison.

Author readiness

Human owner: @z2Ace0107

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/4014 **Author:** [@z2Ace0107](https://github.com/z2Ace0107) **Created:** 10/15/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/3879-script-content-comparison` --- ### 📝 Commits (9) - [`81c14ac`](https://github.com/morluto/rea/commit/81c14ac8d462a11560294af39b63ef799bc63283) fix(browser): compare retained script source identities - [`d15d62e`](https://github.com/morluto/rea/commit/d15d62edc80490d0bcefdcbe62ac1b13300548b7) test(browser): place script comparison CLI regression in acceptance - [`e9f7c93`](https://github.com/morluto/rea/commit/e9f7c934603bae6e152d82c2635aec09e868da83) fix(browser): preserve unknown duplicate identities across omissions - [`7bf34bc`](https://github.com/morluto/rea/commit/7bf34bc7584e4e11f421f12add1b57e88e242250) fix(browser): keep bare script identities unknown after omissions - [`e819785`](https://github.com/morluto/rea/commit/e819785e38cd1ced6d3f9cb2391057ebcde61982) fix(browser): match duplicate script sources before residual changes - [`90eec6a`](https://github.com/morluto/rea/commit/90eec6a7cf9691d053d252445143ec3ebf9a30da) fix(browser): require retained script coverage before source matching - [`5d78d6f`](https://github.com/morluto/rea/commit/5d78d6f5b50c18d06c64f409fa119566b08c5bf8) fix(browser): preserve uncertainty after dropped script events - [`10533f5`](https://github.com/morluto/rea/commit/10533f576b5045edccb8a9759b8a392464dbe22b) chore: merge upstream main into script comparison fix - [`9756a51`](https://github.com/morluto/rea/commit/9756a5103f5cad362985abf2fd317b04efb5d49c) fix(browser): preserve provable partial script comparisons ### 📊 Changes **4 files changed** (+845 additions, -28 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/webCaptureDiff.ts` (+25 -28) ➕ `src/domain/webCaptureScriptComparison.ts` (+170 -0) ➕ `tests/acceptance/tools/scriptCaptureComparison.test.ts` (+644 -0) 📝 `tests/boundary/browser/webCaptureDiff.test.ts` (+6 -0) </details> ### 📄 Description ## Summary Fixes #3879. Compare retained script content without using capture-local duplicate ordinals as cross-capture identity. ```text complete script inventory = clear flags + zero drops + matching row counts + no nonzero/unknown script exclusions match equal known content counts -> allow unknown source slots to explain residuals -> report only remaining changes incomplete inventory -> preserve metadata/producer-hash comparison and unknown identity ``` ## Problem and expected behavior Different retained sources with identical metadata and a null producer hash could report unchanged. Duplicate keys can be renumbered when scripts are omitted, so ordinal differences alone do not prove replacement. Conversely, missing source text must not hide a provable difference: `[A,A]` versus `[B,?]` cannot describe identical content. Explicit script exclusions must limit completeness even when redundant section flags are absent. ## Change and scope The shared domain comparator cancels equal known fingerprints with multiplicity, then pairs unknown sources with compatible residual metadata before comparing remaining occurrences. Original capture keys are preserved. Script inventory completeness includes nonzero or unknown script exclusions; zero counts and unrelated exclusions do not disable script comparison. Incomplete inventories retain the metadata/producer-hash path. The existing boundary fixture helper now removes exclusion rows when it explicitly marks their sections complete. Assertions and unrelated omission evidence are preserved. ## Contract and boundary impact - Semantic owner: shared offline domain comparison used by CLI/MCP. - Contribution scope: correctness fix within an existing workflow; advance maintainer scope decision not applicable. - CLI/MCP schemas, catalog, provider bridges, generated metadata and installation: unchanged. - Evidence: caller inputs, digests, multiplicity, original keys and explicit unknowns remain preserved. - Authority: compare normalized capture files; no target acquisition or mutation. Expected effects: - [x] 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. ## Evidence - **Before:** on published head `10533f576b`, the new actual-CLI partial-source regression failed: `[A,A]` versus `[B,?]` returned `unknown / 0` rather than one provable replacement; the previous 16 CLI regressions passed. - **Before:** after the partial-source fix, the exclusion-only regression independently failed: schema-valid script exclusions with clear section flags returned `changed / 1` rather than `unknown / 0`. - **After:** both directions preserve forced replacement counts, including a matching non-null producer hash. Unknown slots that can explain all differences remain unknown. Script exclusion counts `1` and `null` prevent source matching; zero counts and unrelated exclusions retain known changes. - Synthetic normalized captures exercise the real compiled CLI. They do not establish live browser collection behavior. ## Validation performed Fresh on 2026-10-20, Windows x64, pinned Node 24.18.0 / npm 11.16.0: - `npm run test:focused -- tests/acceptance/tools/scriptCaptureComparison.test.ts tests/boundary/browser/webCaptureDiff.test.ts` — 51 passed: 22 actual-CLI tests and 29 boundary tests, zero skipped. Required runtime artifacts prepared by the repository runner; final preparation reused its valid cache, tests executed freshly. - `npm run test:local -- src/domain/webCaptureDiff.network.test.ts` — 45 passed, zero skipped. - `npm run check` — passed: typecheck, lint (0 errors, 1740 repository warnings), format, knip, metadata, module boundaries and architecture guards. Two of six Turbo tasks reused valid caches. - `git diff --check` — passed. - No test files added or moved; discovery not rerun. No schema or authored documentation changes; generated-document checks not rerun for this follow-up. ## Compatibility, safety, and release No migration, schema or dependency change. Full suite, live MCP transport, live Chromium and other operating systems were not newly verified. MCP shares the comparator; these tests establish CLI and boundary behavior. No performance improvement claimed. Published-head CI `39251655449` remains failed; the local candidate has not been published and has no new CI result. Upstream cleanup repair #5249 has merged, but this candidate does not include a new upstream merge or claim that its former CI failures are resolved. ## Merge Danger **Door:** two-way — revert comparator changes and their regressions together. **Blast Radius:** script comparison. ## Author readiness Human owner: @z2Ace0107 - [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 - [ ] 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. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns