[PR #6006] [MERGED] fix(process): reject finite trace variants that only reorder unordered members #7116

Closed
opened 2026-11-01 21:45:30 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

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

Base: main ← Head: fix/5916-finite-trace-unordered-duplicates


📝 Commits (1)

  • aeb302d fix(process): reject finite trace variants that only reorder unordered members

📊 Changes

3 files changed (+119 additions, -3 deletions)

View changed files

📝 src/domain/process/processTraceSpecification.test.ts (+46 -0)
📝 src/domain/process/processTraceSpecification.ts (+19 -3)
📝 tests/boundary/mcp/processTraceAdaptersParity.test.ts (+54 -0)

📄 Description

What broke: Finite-trace validation accepted two variants that differ only in the order of members inside an unordered token, although matching treats those members as a multiset and the later variant can never be matched_variant.
Fix: Key the duplicate-trace check by the observations a trace accepts: sort unordered members, and expand a token that repeats one event into that event's sequence. Token positions and multiplicities stay significant.
Risk: Low. The change only affects finite_traces duplicate validation. Specifications that previously had such redundant variants are now rejected with the existing invalid_input diagnostic.

Summary

Fixes #5916. compare_process_captures (CLI and MCP) accepted a trace_spec whose finite_traces variants differed only by unordered-member order, then returned comparison Evidence matching only the first variant. Such a spec is now rejected with the existing finite trace variants must not duplicate a trace issue at trace_spec.language.variants[N].trace, before any comparison Evidence is created.

Problem and expected behavior

Smallest trigger: variants [{ unordered: ["ready", "worker"] }] and [{ unordered: ["worker", "ready"] }]. Duplicate detection used canonicalTraceJson(variant.trace), which keeps array order. The evaluator (processTraceEvaluation.ts) counts each token's members as a multiset. Both variants therefore accept exactly the same traces, which breaks the "variants must not duplicate a trace" invariant.

Change and scope

processTraceSpecification.ts owns the variant-identity rule, so it now has a finiteTraceKey that follows the evaluator's matching semantics:

  • Unordered members are sorted, so order no longer matters and multiplicities are kept.
  • An unordered token whose members are all one event (for example { unordered: ["a", "a"] }) can match only a, a, so it becomes that sequence of single tokens.
  • Token boundaries and positions stay significant. For example, [{unordered:[a,b]}, a], [a, {unordered:[a,b]}] and [{unordered:[a,a,b]}] stay distinct.

With this canonical form, two keys are equal exactly when the two variants accept the same set of traces. Partial overlaps between different languages are still not rejected, as the issue asked. canonicalTraceJson is unchanged for its other callers (ignored fields are already sorted before keying, and partial-order unordered_groups duplicates are already detected order-independently through pair overlaps). Contribution scope: not applicable (validation fix inside the existing boundary).

Contract and boundary impact

  • Semantic owner and earliest changed stage: processTraceSpecificationSchema (domain), request validation.

  • Contribution scope fit and linked maintainer decision: not applicable; ordinary fix.

  • CLI and MCP/tool-catalog contract: no schema or catalog change; more specs are now rejected through the existing invalid_input outcome.

  • Provider, bridge, target-format, or platform compatibility: none.

  • Evidence, artifact, provenance, or reconstruction contract: none; rejected specs never produced a distinct matched_variant.

  • Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.

  • Generated metadata, package, or installation impact: none.

  • Read existing files or captured data.

Evidence and regression coverage

  • Tests added or updated:

    • tests/boundary/mcp/processTraceAdaptersParity.test.ts: the reordered-unordered spec is rejected through compareProcessEvidenceFiles (CLI path) and the MCP compare_process_captures tool, with the issue at ["trace_spec","language","variants",1,"trace"].
    • src/domain/process/processTraceSpecification.test.ts: the repeated-single-event token is treated as a sequence, and swapped members are rejected, while distinct positions and token boundaries are still accepted.
  • Base reproduction: both new tests failed on base 6d356b843. The boundary test showed that the CLI returned rea.process-comparison Evidence instead of invalid_input; the domain test showed success: true.

  • User-visible CLI output (issue repro run through the built node scripts/rea.mjs compare-process-captures left.json right.json spec.json --format json): exit 1, {"code":"invalid_request","category":"invalid_input",...,"details":{"operation":"compare_process_captures","issues":[{"path":["trace_spec","language","variants",1,"trace"], ... (was exit 0 with trace verdict equivalent on base).

  • Remaining proof gaps: no live process capture (not needed; this is a validation-only change). Full suite not run locally.

  • 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/process/*.test.ts (6 files): 74 passed
  • npm run test:focused -- tests/boundary/mcp/processTraceAdaptersParity.test.ts tests/boundary/mcp/processComparisonDiagnosticsMcp.test.ts tests/boundary/process/processCli.test.ts tests/boundary/process/processTruncationAdapters.test.ts tests/boundary/filesystem/processCliFileInput.test.ts: 64 passed
  • Issue repro through the built CLI subprocess: rejected as above
  • npm run check:fast: passed
  • npm run format:check: passed

Compatibility, safety, and release

  • Breaking changes or migration steps: none intended. A spec with such redundant variants is now rejected; to migrate, remove the redundant variant, which leaves the accepted language unchanged.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no new I/O or process behavior. The extra cost is one sort per unordered token during validation.

Author readiness

Human owner: @kaoru0822-kitauji

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

🤖 Generated with Claude Code


🔄 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/6006 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/21/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/5916-finite-trace-unordered-duplicates` --- ### 📝 Commits (1) - [`aeb302d`](https://github.com/morluto/rea/commit/aeb302ddaac03bdb8c2a15520067f5923143df20) fix(process): reject finite trace variants that only reorder unordered members ### 📊 Changes **3 files changed** (+119 additions, -3 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/process/processTraceSpecification.test.ts` (+46 -0) 📝 `src/domain/process/processTraceSpecification.ts` (+19 -3) 📝 `tests/boundary/mcp/processTraceAdaptersParity.test.ts` (+54 -0) </details> ### 📄 Description **What broke:** Finite-trace validation accepted two variants that differ only in the order of members inside an `unordered` token, although matching treats those members as a multiset and the later variant can never be `matched_variant`. **Fix:** Key the duplicate-trace check by the observations a trace accepts: sort unordered members, and expand a token that repeats one event into that event's sequence. Token positions and multiplicities stay significant. **Risk:** Low. The change only affects `finite_traces` duplicate validation. Specifications that previously had such redundant variants are now rejected with the existing `invalid_input` diagnostic. ## Summary Fixes #5916. `compare_process_captures` (CLI and MCP) accepted a `trace_spec` whose `finite_traces` variants differed only by unordered-member order, then returned comparison Evidence matching only the first variant. Such a spec is now rejected with the existing `finite trace variants must not duplicate a trace` issue at `trace_spec.language.variants[N].trace`, before any comparison Evidence is created. ## Problem and expected behavior Smallest trigger: variants `[{ unordered: ["ready", "worker"] }]` and `[{ unordered: ["worker", "ready"] }]`. Duplicate detection used `canonicalTraceJson(variant.trace)`, which keeps array order. The evaluator (`processTraceEvaluation.ts`) counts each token's members as a multiset. Both variants therefore accept exactly the same traces, which breaks the "variants must not duplicate a trace" invariant. ## Change and scope `processTraceSpecification.ts` owns the variant-identity rule, so it now has a `finiteTraceKey` that follows the evaluator's matching semantics: - Unordered members are sorted, so order no longer matters and multiplicities are kept. - An unordered token whose members are all one event (for example `{ unordered: ["a", "a"] }`) can match only `a, a`, so it becomes that sequence of single tokens. - Token boundaries and positions stay significant. For example, `[{unordered:[a,b]}, a]`, `[a, {unordered:[a,b]}]` and `[{unordered:[a,a,b]}]` stay distinct. With this canonical form, two keys are equal exactly when the two variants accept the same set of traces. Partial overlaps between different languages are still not rejected, as the issue asked. `canonicalTraceJson` is unchanged for its other callers (ignored fields are already sorted before keying, and partial-order `unordered_groups` duplicates are already detected order-independently through pair overlaps). Contribution scope: not applicable (validation fix inside the existing boundary). ## Contract and boundary impact - Semantic owner and earliest changed stage: `processTraceSpecificationSchema` (domain), request validation. - Contribution scope fit and linked maintainer decision: not applicable; ordinary fix. - CLI and MCP/tool-catalog contract: no schema or catalog change; more specs are now rejected through the existing `invalid_input` outcome. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none; rejected specs never produced a distinct `matched_variant`. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: - `tests/boundary/mcp/processTraceAdaptersParity.test.ts`: the reordered-unordered spec is rejected through `compareProcessEvidenceFiles` (CLI path) and the MCP `compare_process_captures` tool, with the issue at `["trace_spec","language","variants",1,"trace"]`. - `src/domain/process/processTraceSpecification.test.ts`: the repeated-single-event token is treated as a sequence, and swapped members are rejected, while distinct positions and token boundaries are still accepted. - Base reproduction: both new tests failed on base `6d356b843`. The boundary test showed that the CLI returned `rea.process-comparison` Evidence instead of `invalid_input`; the domain test showed `success: true`. - User-visible CLI output (issue repro run through the built `node scripts/rea.mjs compare-process-captures left.json right.json spec.json --format json`): exit 1, `{"code":"invalid_request","category":"invalid_input",...,"details":{"operation":"compare_process_captures","issues":[{"path":["trace_spec","language","variants",1,"trace"], ...` (was exit 0 with trace verdict `equivalent` on base). - Remaining proof gaps: no live process capture (not needed; this is a validation-only change). Full suite not run locally. - [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/process/*.test.ts` (6 files): 74 passed - `npm run test:focused -- tests/boundary/mcp/processTraceAdaptersParity.test.ts tests/boundary/mcp/processComparisonDiagnosticsMcp.test.ts tests/boundary/process/processCli.test.ts tests/boundary/process/processTruncationAdapters.test.ts tests/boundary/filesystem/processCliFileInput.test.ts`: 64 passed - Issue repro through the built CLI subprocess: rejected as above - `npm run check:fast`: passed - `npm run format:check`: passed ## Compatibility, safety, and release - Breaking changes or migration steps: none intended. A spec with such redundant variants is now rejected; to migrate, remove the redundant variant, which leaves the accepted language unchanged. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new I/O or process behavior. The extra cost is one sort per unordered token during validation. ## Author readiness Human owner: @kaoru0822-kitauji - [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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-11-01 21:45:30 +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#7116
No description provided.