[PR #2488] [MERGED] fix(process): accept a trace identifier the comparison result can represent #5268

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2488
Author: @XiaoBinGan
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/process-trace-identifier-bound


📝 Commits (1)

  • b3ddc2d fix(process): accept a trace identifier the comparison result can represent

📊 Changes

3 files changed (+55 additions, -17 deletions)

View changed files

📝 src/domain/process/processTraceComparison.test.ts (+28 -0)
📝 src/domain/process/processTraceEvaluation.ts (+4 -5)
📝 src/domain/process/processTraceSpecification.ts (+23 -12)

📄 Description

The specification owns the declared trace, event and variant identifier vocabulary, and the comparison result echoes those identifiers verbatim into raw_trace[].event_id, matched_variant and diagnostic event_ids. Both modules must therefore accept the same values.

Summary

processTraceSpecification accepts a trace identifier with the shared IDENTIFIER_PATTERN, which has no length bound. processTraceEvaluation restated that pattern by hand as /^[A-Za-z][A-Za-z0-9._\x2d]{0,63}$/u, silently adding a 64-character cap. A specification whose identifier is longer than 64 characters therefore validates, and then the result of comparing it fails its own schema, so compareProcessTraces throws a raw ZodError at the caller instead of returning a comparison or a modeled failure.

IDENTIFIER_PATTERN is the repository's identifier owner and is used unbounded by every other consumer (evidenceCompletionLedger, completionLedgerGeneration, reconstructionVerificationSchemas, browserScenarioValues, conformancePackage, adbDeviceAnalysis). The capped copy in the evaluation module is the outlier, not the contract.

Problem and expected behavior

With compareProcessTraces from the built tree:

ZodError: [
  { "path": ["left", "raw_trace", 0, "event_id"],
    "message": "Invalid string: must match pattern /^[A-Za-z][A-Za-z0-9._\\x2d]{0,63}$/u" },
  { "path": ["right", "raw_trace", 0, "event_id"], "message": "…same…" }
]

Expected: a specification that the advertised trace_spec contract accepts produces a comparison result, never an escaping schema error.

Change and scope

  • processTraceSpecification exports its identifier schema as processTraceIdentifierSchema, documented as the owner of the declared identifier vocabulary. The module-local name is the exported one; no alias is kept.
  • processTraceEvaluation imports that schema and deletes the hand-written capped pattern. Its three uses (event_id, matched_variant, event_ids) now reference the single owner, so the specification and its result cannot drift apart again.

Direction of the fix: the specification is the input contract advertised to callers, and identifiers are caller vocabulary that the result merely echoes, so the bound is removed rather than added. Adding a 64-character limit to the specification instead would reject input the contract accepted today and would be a breaking change to an advertised tool input. No separate representation, alias or wrapper remains.

Contract and boundary impact

  • Semantic owner and earliest changed stage: the declared trace identifier in the process trace specification. Cancellation, capture parsing, matching, cardinality and the comparison verdict are untouched.
  • CLI and MCP/tool-catalog contract: no tool name, command, input field, output schema or error code changes. trace_spec gains no new accepted shape; the only difference is that an identifier longer than 64 characters now produces a result instead of an escaping ZodError. Inputs of 64 characters or fewer behave exactly as before, so no advertised schema narrows.
  • Provider, target-format or platform compatibility: none — platform-independent domain logic.
  • Evidence, artifact, provenance or reconstruction contract: no Evidence, artifact byte, digest or provenance value changes. The echoed identifier is the caller's declared value in both modules.
  • Process execution, authorization, cleanup or containment impact: none.
  • Generated metadata, package or installation impact: none.

Evidence and regression coverage

A new case in processTraceComparison.test.ts, "compares a declared identifier the comparison result can represent", declares a 65-character event id, asserts the specification schema accepts it, and asserts the comparison returns verdict: "equivalent" with matched_variant: "only" and raw_trace: [{ event_id: <the 65-character id> }].

Before the change the same test failed with the ZodError quoted above; the existing six cases in that file passed throughout, which is why the case was uncovered. Verification on macOS 26.5.1 arm64 with Node.js 24.18.0 and npm 11.16.0, based on bb54082:

  • npm run test:focused over the comparison, specification, adapters, capture-comparison, capture-validation and scenario suites: 48 passed, 0 failed.
  • npm run test:focused over the three boundary suites that drive traces through the CLI/MCP adapters (processTraceAdaptersParity, processComparisonDiagnosticsMcp, processTruncationAdapters): 24 passed, 0 failed.
  • npm run check:fast: passed.
  • npm run verify:test-discovery: all 1015 test files discovered exactly once.

Unavailable coverage: no Windows or Linux run. The change does not touch platform-conditional 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/2488 **Author:** [@XiaoBinGan](https://github.com/XiaoBinGan) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/process-trace-identifier-bound` --- ### 📝 Commits (1) - [`b3ddc2d`](https://github.com/morluto/rea/commit/b3ddc2d12a79de2b6672da3d62bfdc1ab0901a72) fix(process): accept a trace identifier the comparison result can represent ### 📊 Changes **3 files changed** (+55 additions, -17 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/process/processTraceComparison.test.ts` (+28 -0) 📝 `src/domain/process/processTraceEvaluation.ts` (+4 -5) 📝 `src/domain/process/processTraceSpecification.ts` (+23 -12) </details> ### 📄 Description The specification owns the declared trace, event and variant identifier vocabulary, and the comparison result echoes those identifiers verbatim into `raw_trace[].event_id`, `matched_variant` and diagnostic `event_ids`. Both modules must therefore accept the same values. ## Summary [`processTraceSpecification`](https://github.com/morluto/rea/blob/bb54082/src/domain/process/processTraceSpecification.ts) accepts a trace identifier with the shared `IDENTIFIER_PATTERN`, which has no length bound. [`processTraceEvaluation`](https://github.com/morluto/rea/blob/bb54082/src/domain/process/processTraceEvaluation.ts) restated that pattern by hand as `/^[A-Za-z][A-Za-z0-9._\x2d]{0,63}$/u`, silently adding a 64-character cap. A specification whose identifier is longer than 64 characters therefore validates, and then the *result* of comparing it fails its own schema, so `compareProcessTraces` throws a raw `ZodError` at the caller instead of returning a comparison or a modeled failure. `IDENTIFIER_PATTERN` is the repository's identifier owner and is used unbounded by every other consumer (`evidenceCompletionLedger`, `completionLedgerGeneration`, `reconstructionVerificationSchemas`, `browserScenarioValues`, `conformancePackage`, `adbDeviceAnalysis`). The capped copy in the evaluation module is the outlier, not the contract. ## Problem and expected behavior With `compareProcessTraces` from the built tree: ``` ZodError: [ { "path": ["left", "raw_trace", 0, "event_id"], "message": "Invalid string: must match pattern /^[A-Za-z][A-Za-z0-9._\\x2d]{0,63}$/u" }, { "path": ["right", "raw_trace", 0, "event_id"], "message": "…same…" } ] ``` Expected: a specification that the advertised `trace_spec` contract accepts produces a comparison result, never an escaping schema error. ## Change and scope - [`processTraceSpecification`](https://github.com/XiaoBinGan/rea/blob/HEAD/src/domain/process/processTraceSpecification.ts) exports its identifier schema as `processTraceIdentifierSchema`, documented as the owner of the declared identifier vocabulary. The module-local name is the exported one; no alias is kept. - [`processTraceEvaluation`](https://github.com/XiaoBinGan/rea/blob/HEAD/src/domain/process/processTraceEvaluation.ts) imports that schema and deletes the hand-written capped pattern. Its three uses (`event_id`, `matched_variant`, `event_ids`) now reference the single owner, so the specification and its result cannot drift apart again. Direction of the fix: the specification is the input contract advertised to callers, and identifiers are caller vocabulary that the result merely echoes, so the bound is removed rather than added. Adding a 64-character limit to the specification instead would reject input the contract accepted today and would be a breaking change to an advertised tool input. No separate representation, alias or wrapper remains. ## Contract and boundary impact - **Semantic owner and earliest changed stage:** the declared trace identifier in the process trace specification. Cancellation, capture parsing, matching, cardinality and the comparison verdict are untouched. - **CLI and MCP/tool-catalog contract:** no tool name, command, input field, output schema or error code changes. `trace_spec` gains no new accepted shape; the only difference is that an identifier longer than 64 characters now produces a result instead of an escaping `ZodError`. Inputs of 64 characters or fewer behave exactly as before, so no advertised schema narrows. - **Provider, target-format or platform compatibility:** none — platform-independent domain logic. - **Evidence, artifact, provenance or reconstruction contract:** no Evidence, artifact byte, digest or provenance value changes. The echoed identifier is the caller's declared value in both modules. - **Process execution, authorization, cleanup or containment impact:** none. - **Generated metadata, package or installation impact:** none. ## Evidence and regression coverage A new case in [`processTraceComparison.test.ts`](https://github.com/XiaoBinGan/rea/blob/HEAD/src/domain/process/processTraceComparison.test.ts), "compares a declared identifier the comparison result can represent", declares a 65-character event id, asserts the specification schema accepts it, and asserts the comparison returns `verdict: "equivalent"` with `matched_variant: "only"` and `raw_trace: [{ event_id: <the 65-character id> }]`. Before the change the same test failed with the `ZodError` quoted above; the existing six cases in that file passed throughout, which is why the case was uncovered. Verification on macOS 26.5.1 arm64 with Node.js 24.18.0 and npm 11.16.0, based on `bb54082`: - `npm run test:focused` over the comparison, specification, adapters, capture-comparison, capture-validation and scenario suites: 48 passed, 0 failed. - `npm run test:focused` over the three boundary suites that drive traces through the CLI/MCP adapters (`processTraceAdaptersParity`, `processComparisonDiagnosticsMcp`, `processTruncationAdapters`): 24 passed, 0 failed. - `npm run check:fast`: passed. - `npm run verify:test-discovery`: all 1015 test files discovered exactly once. Unavailable coverage: no Windows or Linux run. The change does not touch platform-conditional code. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:05:18 +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#5268
No description provided.