[PR #3803] [MERGED] fix(javascript): preserve case-sensitive Unicode seed identity #5909

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

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3803
Author: @AdityaKharmaleGoat7
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/17/2026
Merged by: @N0zoM1z0

Base: main ← Head: fix/case-sensitive-feature-seed-unicode


📝 Commits (8)

  • 90b8d2d fix(javascript): preserve case-sensitive Unicode seed identity
  • c56e26e Merge remote-tracking branch 'origin/main' into cifix/3803
  • a7fa460 test(javascript): use the contains seed match in the Unicode test
  • e30e68d Merge branch 'main' into fix/case-sensitive-feature-seed-unicode
  • ab56363 Merge branch 'main' into fix/case-sensitive-feature-seed-unicode
  • 500fd8d Merge branch 'main' into fix/case-sensitive-feature-seed-unicode
  • 67af4d7 Merge branch 'main' into fix/case-sensitive-feature-seed-unicode
  • 5177d13 Merge branch 'main' into fix/case-sensitive-feature-seed-unicode

📊 Changes

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

View changed files

📝 src/domain/javascript/javascriptFeatureSeed.ts (+1 -1)
➕ src/domain/javascript/javascriptFeatureSeed.unicode.test.ts (+58 -0)

📄 Description

Fixes #3314

Summary

Preserve compatibility-distinct strings during case-sensitive application feature-seed matching while retaining the existing compatibility fold for explicit case-insensitive matching.

Problem and expected behavior

An exact case-sensitive channel seed for ff currently also selects the distinct ff channel because both query and candidate are normalized with NFKC. Issue #3314 has been independently reproduced on current main, and no existing PR claims it.

Case-sensitive matching should preserve the distinction between compatibility-equivalent strings. Case-insensitive matching should retain the existing NFKC-based query behavior.

Change and scope

  • use NFC rather than NFKC when case_sensitive is true
  • retain NFKC plus lower-casing for explicit case-insensitive matching
  • add focused tests covering exact matching, substring matching, and the existing case-insensitive compatibility fold

Contribution scope fit and linked maintainer decision: ordinary correctness fix inside the existing static feature-seed matcher; no new authority or provider capability.

Contract and boundary impact

  • Semantic owner and earliest changed stage: JavaScript application feature-seed selection.
  • CLI and MCP/tool-catalog contract: unchanged schemas; corrected matching semantics for case-sensitive literal seeds.
  • Provider, bridge, target-format, or platform compatibility: unchanged.
  • Evidence, artifact, provenance, or reconstruction contract: prevents unrelated compatibility-equivalent nodes from entering a selected trace.
  • Target selection, authority, upstream behavior, and cleanup details: no target interaction or execution change.
  • Generated metadata, package, or installation impact: none.

Evidence and regression coverage

  • Tests added: focused unit coverage for ff versus ff under exact case-sensitive, substring case-sensitive, and explicit case-insensitive matching.
  • Base reproduction: issue #3314 was independently reproduced on recent main; current main still uses NFKC for case-sensitive matches.
  • Remaining proof gaps: local repository test execution was unavailable in this environment; CI is the authoritative validation.

Validation performed

  • branch is based directly on upstream main at af6a9980669129aae882c9216766fc8d9f544079
  • branch comparison is 0 commits behind that base
  • diff is limited to one matcher line and focused regression coverage

Compatibility, safety, and release

  • Breaking changes or migration steps: none; this narrows case-sensitive matching to the documented literal identity instead of compatibility-folded identity.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable; pure static domain logic.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no new process, filesystem, network, or target authority.

Maintainer review update — Linux (17 October 2026)

Reviewed head: ab56363a9230919cff07b4535a2038f027a3d8c9, merged with main 150e996a86f2bec4982ba87f37bf502a8d5e8214. Its whole tree matches the independently computed integration tree; its stable patch ID matches the previously reviewed change, and it does not modify workflows.

52 feature-seed tests pass. Compiled CLI/MCP workflows pass: case-sensitive ff and ff select distinct source identities, substring matching preserves that distinction, insensitive matching selects their union, and absent queries stay empty. Full Evidence agrees between adapters; the source fixture is unchanged and is not executed as Electron.

Historical baseline: the former firmware and three async BinarySessionStatus failures were independently reproduced earlier; both are now repaired upstream. The current-head baseline is recorded below.

Local jobs ran serially on two CPU cores with a 3 GiB Node heap; maximum sampled job-family RSS across this retry was 3.28 GiB, with no resource stop. This is not a measurement of the assistant/shared host total. Model token use, controlled cold/warm comparison and remote resource costs are unmeasured.

Baseline follow-up (2026-10-17)

Current head 5177d13f4824ef0aada5f14560edc209d43df888 incorporates main 5b243993872c2bcc70cb00060ffb79b347321370, including the normally merged #4691/#4703 fixture repairs. Exact merge-tree verification and stable patch IDs confirm that this PR's patch is unchanged by the update. Current-head CI: All five built-dependent shards and all required gates passed on unchanged attempt 3. The first worker OOM remains documented below.

Fresh exact-head production build and public static JavaScript workflow pass through 6 CLI and 5 MCP calls: exact and substring case-sensitive identities remain distinct, insensitive matching selects their union, absent selection remains empty, and complete Evidence agrees between adapters. Input source remains unchanged. Completed task: 15.03 s wall, 19.18 s reaped CPU, 1.10 GiB sampled process-family RSS. Hosted shard 5 failed because its existing 384 MiB cli-boundary worker exhausted its heap while running cliIntegerArguments.test.ts; no feature assertion failed. Failure logs are retained. The unchanged rerun passed that shard and the complete required CI gates with the same 384 MiB worker bound. The feature patch does not touch this test or runner configuration; the cause remains unconfirmed and is tracked separately in #4713. No heap/timeout increase, test skip or feature-code change was used.

Local heavy jobs ran serially on two CPU cores with a 3 GiB Node heap. RSS is sampled process-family memory, may double-count shared pages and excludes assistant/shared services; CPU covers reaped descendants. OS/dependency caches were not controlled. Model token use, controlled cold/warm costs and remote resource costs remain unmeasured; no token or speedup claim is inferred. Prior observations above remain tied to their stated revisions. Initial human-owner readiness remains valid and was not supplied on the author's behalf. Available automated reviews have no unresolved findings; Codex explicitly reports repository review-credit exhaustion, so no fresh Codex review is claimed.

Author readiness

Human owner: @AdityaKharmaleGoat7

  • 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.
  • 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/3803 **Author:** [@AdityaKharmaleGoat7](https://github.com/AdityaKharmaleGoat7) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/17/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/case-sensitive-feature-seed-unicode` --- ### 📝 Commits (8) - [`90b8d2d`](https://github.com/morluto/rea/commit/90b8d2ddffd5b1326c706925c5518394c0f23126) fix(javascript): preserve case-sensitive Unicode seed identity - [`c56e26e`](https://github.com/morluto/rea/commit/c56e26e227214891f538371410ed033d9fd8a55f) Merge remote-tracking branch 'origin/main' into cifix/3803 - [`a7fa460`](https://github.com/morluto/rea/commit/a7fa46037a671443eb81af1aeb6bb642ea9164c9) test(javascript): use the contains seed match in the Unicode test - [`e30e68d`](https://github.com/morluto/rea/commit/e30e68d1f5a505130708ed2c22193d96abf639db) Merge branch 'main' into fix/case-sensitive-feature-seed-unicode - [`ab56363`](https://github.com/morluto/rea/commit/ab56363a9230919cff07b4535a2038f027a3d8c9) Merge branch 'main' into fix/case-sensitive-feature-seed-unicode - [`500fd8d`](https://github.com/morluto/rea/commit/500fd8d7654ace8547838eda637a2c6c56826a59) Merge branch 'main' into fix/case-sensitive-feature-seed-unicode - [`67af4d7`](https://github.com/morluto/rea/commit/67af4d7fb70dc5a20cda46955e3fa7dd2dc5b061) Merge branch 'main' into fix/case-sensitive-feature-seed-unicode - [`5177d13`](https://github.com/morluto/rea/commit/5177d13f4824ef0aada5f14560edc209d43df888) Merge branch 'main' into fix/case-sensitive-feature-seed-unicode ### 📊 Changes **2 files changed** (+59 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptFeatureSeed.ts` (+1 -1) ➕ `src/domain/javascript/javascriptFeatureSeed.unicode.test.ts` (+58 -0) </details> ### 📄 Description Fixes #3314 ## Summary Preserve compatibility-distinct strings during case-sensitive application feature-seed matching while retaining the existing compatibility fold for explicit case-insensitive matching. ## Problem and expected behavior An exact case-sensitive channel seed for `ff` currently also selects the distinct `ff` channel because both query and candidate are normalized with NFKC. Issue #3314 has been independently reproduced on current main, and no existing PR claims it. Case-sensitive matching should preserve the distinction between compatibility-equivalent strings. Case-insensitive matching should retain the existing NFKC-based query behavior. ## Change and scope - use NFC rather than NFKC when `case_sensitive` is true - retain NFKC plus lower-casing for explicit case-insensitive matching - add focused tests covering exact matching, substring matching, and the existing case-insensitive compatibility fold Contribution scope fit and linked maintainer decision: ordinary correctness fix inside the existing static feature-seed matcher; no new authority or provider capability. ## Contract and boundary impact - Semantic owner and earliest changed stage: JavaScript application feature-seed selection. - CLI and MCP/tool-catalog contract: unchanged schemas; corrected matching semantics for case-sensitive literal seeds. - Provider, bridge, target-format, or platform compatibility: unchanged. - Evidence, artifact, provenance, or reconstruction contract: prevents unrelated compatibility-equivalent nodes from entering a selected trace. - Target selection, authority, upstream behavior, and cleanup details: no target interaction or execution change. - Generated metadata, package, or installation impact: none. ## Evidence and regression coverage - Tests added: focused unit coverage for `ff` versus `ff` under exact case-sensitive, substring case-sensitive, and explicit case-insensitive matching. - Base reproduction: issue #3314 was independently reproduced on recent main; current main still uses NFKC for case-sensitive matches. - Remaining proof gaps: local repository test execution was unavailable in this environment; CI is the authoritative validation. ## Validation performed - branch is based directly on upstream `main` at `af6a9980669129aae882c9216766fc8d9f544079` - branch comparison is 0 commits behind that base - diff is limited to one matcher line and focused regression coverage ## Compatibility, safety, and release - Breaking changes or migration steps: none; this narrows case-sensitive matching to the documented literal identity instead of compatibility-folded identity. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; pure static domain logic. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new process, filesystem, network, or target authority. ### Maintainer review update — Linux (17 October 2026) Reviewed head: `ab56363a9230919cff07b4535a2038f027a3d8c9`, merged with main `150e996a86f2bec4982ba87f37bf502a8d5e8214`. Its whole tree matches the independently computed integration tree; its stable patch ID matches the previously reviewed change, and it does not modify workflows. 52 feature-seed tests pass. Compiled CLI/MCP workflows pass: case-sensitive `ff` and `ff` select distinct source identities, substring matching preserves that distinction, insensitive matching selects their union, and absent queries stay empty. Full Evidence agrees between adapters; the source fixture is unchanged and is not executed as Electron. Historical baseline: the former firmware and three async BinarySessionStatus failures were independently reproduced earlier; both are now repaired upstream. The current-head baseline is recorded below. Local jobs ran serially on two CPU cores with a 3 GiB Node heap; maximum sampled job-family RSS across this retry was 3.28 GiB, with no resource stop. This is not a measurement of the assistant/shared host total. Model token use, controlled cold/warm comparison and remote resource costs are unmeasured. ## Baseline follow-up (2026-10-17) Current head `5177d13f4824ef0aada5f14560edc209d43df888` incorporates main `5b243993872c2bcc70cb00060ffb79b347321370`, including the normally merged #4691/#4703 fixture repairs. Exact merge-tree verification and stable patch IDs confirm that this PR's patch is unchanged by the update. [Current-head CI](https://github.com/morluto/rea/actions/runs/39011262277): All five built-dependent shards and all required gates passed on unchanged attempt 3. The first worker OOM remains documented below. Fresh exact-head production build and public static JavaScript workflow pass through 6 CLI and 5 MCP calls: exact and substring case-sensitive identities remain distinct, insensitive matching selects their union, absent selection remains empty, and complete Evidence agrees between adapters. Input source remains unchanged. Completed task: 15.03 s wall, 19.18 s reaped CPU, 1.10 GiB sampled process-family RSS. Hosted shard 5 failed because its existing 384 MiB cli-boundary worker exhausted its heap while running cliIntegerArguments.test.ts; no feature assertion failed. Failure logs are retained. The unchanged rerun passed that shard and the complete required CI gates with the same 384 MiB worker bound. The feature patch does not touch this test or runner configuration; the cause remains unconfirmed and is tracked separately in #4713. No heap/timeout increase, test skip or feature-code change was used. Local heavy jobs ran serially on two CPU cores with a 3 GiB Node heap. RSS is sampled process-family memory, may double-count shared pages and excludes assistant/shared services; CPU covers reaped descendants. OS/dependency caches were not controlled. Model token use, controlled cold/warm costs and remote resource costs remain unmeasured; no token or speedup claim is inferred. Prior observations above remain tied to their stated revisions. Initial human-owner readiness remains valid and was not supplied on the author's behalf. Available automated reviews have no unresolved findings; Codex explicitly reports repository review-credit exhaustion, so no fresh Codex review is claimed. ## Author readiness Human owner: @AdityaKharmaleGoat7 - [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. - [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. --- <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:21 +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#5909
No description provided.