[PR #7439] fix(browser): keep duplicated scenario step ids from being classified as absent #7840

Open
opened 2026-11-05 01:48:02 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7439
Author: @kittimzhe
Created: 10/27/2026
Status: 🔄 Open

Base: main ← Head: fix/compare-captures-duplicate-absence


📝 Commits (1)

  • f08494e fix(browser): keep duplicated scenario step ids from being classified as absent

📊 Changes

2 files changed (+60 additions, -2 deletions)

View changed files

📝 src/domain/browserScenarioDiff.test.ts (+55 -0)
📝 src/domain/browserScenarioDiff.ts (+5 -2)

📄 Description

Summary

compare-web-captures classified a step_id duplicated in one capture as also missing from that capture: alignment keeps only unique step ids per side, so the same id arriving uniquely on the other side landed in before_only/after_only and produced a missing_before_step/missing_after_step failure beside the correct duplicate failure — a definite absence claim for a step the capture retains. Alignment now keeps a duplicated id out of the absence lists on both sides; the id stays unaligned with its duplicate failure, and genuinely missing ids keep their missing_* classification. Fixes #7415.

Problem and expected behavior

Smallest trigger: compare a before capture retaining save twice against an after capture retaining it once (the reduced CLI fixtures from #7415). The alignment reported duplicate_before_step_id and missing_before_step for the same id, plus after_only: ["save"]. Reversing the captures produced the symmetric missing_after_step.

Violated invariant: docs/browser-scenario-contract.md — duplicate ids make alignment fail — and AGENTS.md: "Unresolved identity or applicability must remain unknown rather than be treated as absent". The duplicated identity is ambiguous, which must keep it unaligned, not turn its presence into an absence claim.

Expected: the duplicate failure alone; no missing_* and no absence-list entry for the duplicated id on either side; a genuinely absent id keeps its missing_* classification and absence entry.

Change and scope

  • src/domain/browserScenarioDiff.ts (alignScenarioSteps): beforeOnly/afterOnly additionally exclude ids duplicate-classified on the other side. Duplicate failures, aligned-step ordering, and true-absence classification are unchanged.
  • src/domain/browserScenarioDiff.test.ts: two regressions covering both duplicate directions, next to the existing missing-step coverage.
  • Not included: no change to how duplicates are detected or reported, and no change to overall_status.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contract and boundary impact

  • Semantic owner and earliest changed stage: the scenario comparison alignment (alignScenarioSteps), the single producer of before_only/after_only and alignment failures; no consumer reinterprets them. No superseded path.
  • Contribution scope fit and linked maintainer decision: ordinary fix within the existing comparison boundary — not applicable.
  • CLI and MCP/tool-catalog contract: alignment.before_only/after_only no longer contain ids duplicated on the other side, and alignment.failures no longer carries a missing_* entry for them. duplicate_*_step_id failures, alignment.status and overall_status are unchanged for these inputs (partial/unknown). No schema or tool-catalog change.
  • Provider, bridge, target-format, or platform compatibility: none.
  • Evidence, artifact, provenance, or reconstruction contract: retained captures are read unchanged; only the derived alignment projection changes.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
  • 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.

Evidence and regression coverage

  • Tests added or updated: two cases in src/domain/browserScenarioDiff.test.ts (duplicate in the before capture, duplicate in the after capture), each asserting empty absence lists and the duplicate failure alone.
  • Base reproduction or other evidence: executed. #7415's reduced CLI fixtures on a freshly built base reproduced the false missing_before_step with after_only: ["save"]. After the change, the same fixtures were run through the real CLI in five comparisons: duplicate-before, duplicate-after, unique self-comparison (unchanged), genuinely missing (both missing_* retained), and duplicate on both sides (duplicate failures only).
  • User-visible CLI/MCP output (if applicable): duplicate-before after the change — before_only: [], after_only: [], failures [duplicate_before_step_id], overall_status: unknown.
  • Remaining proof gaps: no real browser capture; the fixtures are synthetic retained captures, and comparison never accesses the recorded origin.

Boundary regressions covered: duplicate-identity and missing-identity alignment for the comparison projection. Lifecycle regressions: not applicable (no resources).

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

  • npm run test:local -- src/domain/browserScenarioDiff.test.ts — 34 passed, including the two new regressions.
  • npm run test:focused -- tests/boundary/cli/browserCompareJsonFiles.test.ts tests/boundary/mcp/browserComparisonInput.test.ts — 5 passed.
  • npm run check:fast — passed.

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: none — synthetic retained captures only.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: none.

Author readiness

Human owner: @kittimzhe

  • 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/7439 **Author:** [@kittimzhe](https://github.com/kittimzhe) **Created:** 10/27/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/compare-captures-duplicate-absence` --- ### 📝 Commits (1) - [`f08494e`](https://github.com/morluto/rea/commit/f08494e8c35c4a7e66380f7c151ae6938649d2ec) fix(browser): keep duplicated scenario step ids from being classified as absent ### 📊 Changes **2 files changed** (+60 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/browserScenarioDiff.test.ts` (+55 -0) 📝 `src/domain/browserScenarioDiff.ts` (+5 -2) </details> ### 📄 Description ## Summary `compare-web-captures` classified a step_id duplicated in one capture as also missing from that capture: alignment keeps only unique step ids per side, so the same id arriving uniquely on the other side landed in `before_only`/`after_only` and produced a `missing_before_step`/`missing_after_step` failure beside the correct duplicate failure — a definite absence claim for a step the capture retains. Alignment now keeps a duplicated id out of the absence lists on both sides; the id stays unaligned with its duplicate failure, and genuinely missing ids keep their `missing_*` classification. Fixes #7415. ## Problem and expected behavior Smallest trigger: compare a before capture retaining `save` twice against an after capture retaining it once (the reduced CLI fixtures from #7415). The alignment reported `duplicate_before_step_id` **and** `missing_before_step` for the same id, plus `after_only: ["save"]`. Reversing the captures produced the symmetric `missing_after_step`. Violated invariant: `docs/browser-scenario-contract.md` — duplicate ids make alignment fail — and `AGENTS.md`: "Unresolved identity or applicability must remain unknown rather than be treated as absent". The duplicated identity is ambiguous, which must keep it unaligned, not turn its presence into an absence claim. Expected: the duplicate failure alone; no `missing_*` and no absence-list entry for the duplicated id on either side; a genuinely absent id keeps its `missing_*` classification and absence entry. ## Change and scope - `src/domain/browserScenarioDiff.ts` (`alignScenarioSteps`): `beforeOnly`/`afterOnly` additionally exclude ids duplicate-classified on the **other** side. Duplicate failures, aligned-step ordering, and true-absence classification are unchanged. - `src/domain/browserScenarioDiff.test.ts`: two regressions covering both duplicate directions, next to the existing missing-step coverage. - Not included: no change to how duplicates are detected or reported, and no change to `overall_status`. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. ## Contract and boundary impact - Semantic owner and earliest changed stage: the scenario comparison alignment (`alignScenarioSteps`), the single producer of `before_only`/`after_only` and alignment failures; no consumer reinterprets them. No superseded path. - Contribution scope fit and linked maintainer decision: ordinary fix within the existing comparison boundary — not applicable. - CLI and MCP/tool-catalog contract: `alignment.before_only`/`after_only` no longer contain ids duplicated on the other side, and `alignment.failures` no longer carries a `missing_*` entry for them. `duplicate_*_step_id` failures, `alignment.status` and `overall_status` are unchanged for these inputs (`partial`/`unknown`). No schema or tool-catalog change. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: retained captures are read unchanged; only the derived alignment projection changes. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - 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. - [ ] 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 and regression coverage - Tests added or updated: two cases in `src/domain/browserScenarioDiff.test.ts` (duplicate in the before capture, duplicate in the after capture), each asserting empty absence lists and the duplicate failure alone. - Base reproduction or other evidence: executed. #7415's reduced CLI fixtures on a freshly built base reproduced the false `missing_before_step` with `after_only: ["save"]`. After the change, the same fixtures were run through the real CLI in five comparisons: duplicate-before, duplicate-after, unique self-comparison (`unchanged`), genuinely missing (both `missing_*` retained), and duplicate on both sides (duplicate failures only). - User-visible CLI/MCP output (if applicable): duplicate-before after the change — `before_only: []`, `after_only: []`, failures `[duplicate_before_step_id]`, `overall_status: unknown`. - Remaining proof gaps: no real browser capture; the fixtures are synthetic retained captures, and comparison never accesses the recorded origin. Boundary regressions covered: duplicate-identity and missing-identity alignment for the comparison projection. Lifecycle regressions: not applicable (no resources). 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 - `npm run test:local -- src/domain/browserScenarioDiff.test.ts` — 34 passed, including the two new regressions. - `npm run test:focused -- tests/boundary/cli/browserCompareJsonFiles.test.ts tests/boundary/mcp/browserComparisonInput.test.ts` — 5 passed. - `npm run check:fast` — passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: none — synthetic retained captures only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none. ## Author readiness Human owner: @kittimzhe - [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>
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#7840
No description provided.