Skip to content

fix: keep identifier casing locale-independent (Turkish) - #34709

Merged
OrcaWin merged 2 commits into
mainfrom
OrcaWin/i18n-tr-casing
Oct 28, 2026
Merged

OrcaWin merged 2 commits into
mainfrom
OrcaWin/i18n-tr-casing

Conversation

@OrcaWin

@OrcaWin OrcaWin commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator
Files Added Deleted Net
Test 0 0 0 0
Prod 25 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​57 $\color{#cf222e}{\Huge{\mathbf{−}}}$​60 $\color{#cf222e}{\Huge{\mathbf{−}}}$​3

Prerequisite for adding a Turkish UI. If the system or UI locale is tr/az, toLocaleLowerCase()/toLocaleUpperCase() called with no locale turn I into dotless ı and i into İ. Default-locale collation also stops treating I and i as base-equal. Identifier matching then breaks (we hit this class of bug in #31159).

Audit

I grepped src/ and mobile/ for toLocaleLowerCase(), toLocaleUpperCase(), localeCompare( and Intl.Collator, excluding tests, and found 278 hits. Ten parallel Codex reviewers classified slices of them, and I checked each one by hand.

Fixed

Casing on identifiers changed to toLowerCase()/toUpperCase():

  • Linear CLI/runtime selectors: priority flag, team id/key, label id/name, state id/name/type, assignee id/name/email, issue identifiers in retry. Example: INFRA no longer fails to match infra.
  • Linear error classification: RATE LIMIT turned into rate lımıt and was missed.
  • Searches over ASCII ids: Jira project keys, plugin keys and publishers, hidden-worktree names and paths, address-picker typeahead, slash command and skill names.

Collation pinned to 'en' where ordering or equality has to give the same result on every host:

  • Daemon build fingerprint, GitHub read-scope hash, agent env cache key, skill repo digest, stableJson.
  • Semver prerelease order, primary git remote choice, SSH Include glob order.
  • WSL distro equality, codex subagent snapshot order, VM runtime feature identity order.

'en' gives the same ordering en-US hosts produce today, so existing hashes and keys do not change.

Left locale-aware on purpose

  • Prose casing and display-list sorting.
  • Markdown and DOM find, which already handles İ expansion on purpose.
  • Tie-breaks that only order lists on one host.
  • 'me' token checks, which contain no I/i.

Validation

  • Typecheck: tc:web, node and cli projects all pass.
  • lint:ast-grep and oxlint pass. verify:localization-extraction passes.
  • 41 related test files pass: 548 tests. No new test was added, because a test would have to stub the ICU default locale and would only check implementation details.

Under a tr/az default locale, toLocaleLowerCase()/toLocaleUpperCase() map
I to dotless ı and İ, and default-locale collation stops treating I/i as
base-equal. Switch identifier matching (Linear CLI selectors, priorities,
error classification, plugin/Jira/worktree/address/slash-command search) to
toLowerCase()/toUpperCase(), and pin collation to 'en' where ordering feeds
hashes, canonical JSON, semver precedence, remote selection, SSH Include
order, or WSL distro equality. User-visible prose casing and display-list
sorting stay locale-aware.
@coderabbitai

coderabbitai Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Walkthrough

The changes replace locale-sensitive case conversion with locale-independent case conversion in CLI, Linear, renderer, and native chat matching. They also specify English-locale comparisons for sorting and fingerprinting, and code-point ordering for SSH Include glob matches. The summaries report that matching criteria, error handling, and existing sorting behavior remain otherwise unchanged.

Priority: Low

Merge Risk: Low · up to 99a74

Turkish and Azerbaijani users may miss dashboard search results, and a narrow prerelease-version case may be ordered incorrectly during update decisions. These are bounded issues, so merge is possible with owner awareness and follow-up.

Pre-merge checks: Passed 3 · Failed 2

Failed Failed checks: 2 warnings

Check name Status Explanation Resolution
Description check Warning The description explains the change, motivation, audit scope, and validation results, but it omits most required template sections, including an actual linked issue, ELI5, Visual Proof, Testing checkb… Add the required template sections and complete them. Include an actual issue link after “Fixes #”, write an ELI5 summary, state the user-visible change and mechanism, add N/A with a reason for Visual Proof if applicable, document testing…
Docstring Coverage Warning Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly summarizes the main change: making identifier casing locale-independent for Turkish-related issues.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the change, motivation, audit scope, and validation results, but it omits most required template sections, including an actual linked issue, ELI5, Visual Proof, Testing checkboxes, AI Disclosure, Review, boundary confirmation, Notes, and Checklist.

Resolution

Add the required template sections and complete them. Include an actual issue link after “Fixes #”, write an ELI5 summary, state the user-visible change and mechanism, add N/A with a reason for Visual Proof if applicable, document testing and platforms, complete the AI Disclosure and Agent skill upstream boundary sections, address the Notes items, and check or explain each Checklist item.

Full details: Docstring Coverage

Explanation

Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 17 functions across 25 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

Minor · Pin provider-root key ordering to 'en'. · skill-discovery-target.ts:129-131

src/main/skills/skill-discovery-target.ts:129-131
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Pin provider-root key ordering to 'en'.

scanKey serializes providerRoots after sorting them. SkillProviderRootOverrides accepts the full provider-ID union, and lowercase ASCII IDs can still sort differently by locale. Use an explicit locale.

Suggested fix
-    left.localeCompare(right)
+    left.localeCompare(right, 'en')

Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: da4f9b50-c09f-458a-aae1-e6ec0e8603a1
Commits

Reviewing files that changed from the base of the PR and between a012e45 and a08e24b.

Files selected for processing: 23
  • src/cli/linear-request-builders.ts
  • src/main/daemon/daemon-host-build-inventory.ts
  • src/main/github/github-read-execution-scope.ts
  • src/main/ipc/dropped-path-resolution.ts
  • src/main/linear/linear-issue-listing.ts
  • src/main/native-chat/agent-model-catalog/agent-model-catalog-account-expiry.ts
  • src/main/runtime/runtime-linear-label-write-commands.ts
  • src/main/runtime/runtime-linear-retry-commands.ts
  • src/main/runtime/runtime-linear-task-fields-commands.ts
  • src/main/runtime/runtime-linear-team-write-commands.ts
  • src/main/skills/skill-discovery-target.ts
  • src/main/ssh/config/ssh-config-include-expander.ts
  • src/renderer/src/components/jira-project-picker-filter.ts
  • src/renderer/src/components/network/AddressPicker.tsx
  • src/renderer/src/components/settings/PluginMarketplaceBrowser.tsx
  • src/renderer/src/components/settings/PluginSettingsOverview.tsx
  • src/renderer/src/components/sidebar/HiddenWorktreeRecoveryList.tsx
  • src/shared/app-version.ts
  • src/shared/codex-subagent-roster.ts
  • src/shared/ephemeral-vm-runtime-feature-store.ts
  • src/shared/git-remote-identity.ts
  • src/shared/native-chat/input/native-chat-picker-items.ts
  • src/shared/workspace-layout/workspace-layout-load-report.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

Comment thread src/shared/app-version.ts
return 1
}
return left.localeCompare(right)
return left.localeCompare(right, 'en')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor · Functional Correctness · Quick win

Supported by static analysis

Script executed:

git show ea6c69be9a193f1a5cf8e5a5fda28ea6c4253f46:src/shared/app-version.ts | sed -n '1,110p'
sed -n '1,110p' src/shared/app-version.ts
node -e "for(const l of ['en','tr','az','de']) console.log(l, 'Beta'.localeCompare('alpha',l), 'Beta'.localeCompare('alpha'))"

Repository: stablyai/orca

Length of output: 6551


Script executed:

for locale in C C.UTF-8 en_US.UTF-8 tr_TR.UTF-8 az_AZ.UTF-8 de_DE.UTF-8; do
  echo "LANG=$locale"
  LANG="$locale" LC_ALL="$locale" node - <<'JS'
const resolved = new Intl.Collator().resolvedOptions().locale
const pairs = [['Beta', 'alpha'], ['I', 'i'], ['-a', '.a']]
console.log('default locale:', resolved)
for (const [left, right] of pairs) {
  console.log(`${left}/${right}:`, left.localeCompare(right), 'en:', left.localeCompare(right, 'en'))
}
JS
done

Repository: stablyai/orca

Length of output: 673


Use ASCII ordering for SemVer identifiers.

The Beta/alpha failure predates this PR: both the base default-locale comparator and the head’s English comparator return the wrong order. However, this change can worsen the same SemVer violation for other valid identifiers. Under Turkish or Azerbaijani default locales, the base orders I before i, while the explicit English comparator reverses that order.

Use code-unit ordering for non-numeric identifiers.

Proposed fix
--- "a/src/shared/app-version.ts"
+++ "b/src/shared/app-version.ts"
@@ -53,7 +53,7 @@
   if (rightNumeric) {
     return 1
   }
-  return left.localeCompare(right, 'en')
+  return left < right ? -1 : left > right ? 1 : 0
 }
 
 /** Returns negative if left < right, 0 if equal, positive if left > right. */
Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return left.localeCompare(right, 'en')
return left < right ? -1 : left > right ? 1 : 0

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1c75336d-e327-4dde-97f5-c01b38f74e09
Commits

Reviewing files that changed from the base of the PR and between a08e24b and 99a7442.

Files selected for processing: 3
  • src/main/ssh/config/ssh-config-include-expander.ts
  • src/renderer/src/components/automations/automation-runs-dashboard-model.ts
  • src/renderer/src/components/dashboard-popout/agent-board-filtering.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

key: `${row.key}:${run.id}`,
hostKey: getAutomationRunsHostKey(row),
searchText: [row.automation.name, run.title, row.hostLabel].join('\n').toLocaleLowerCase(),
searchText: [row.automation.name, run.title, row.hostLabel].join('\n').toLowerCase(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor · Functional Correctness · Quick win

Preserve locale-aware matching for prose fields.

automation.name, run.title, and hostLabel are display text, not only identifiers. The companion filter also searches task and message text. (github.com)

With a Turkish or Azerbaijani runtime, İstanbul.toLowerCase() becomes i\u0307stanbul, while the query istanbul becomes istanbul. The includes() check then hides a matching dashboard item. ECMAScript defines toLowerCase() as locale-insensitive, and Unicode assigns this default mapping to U+0130. (tc39.es)

Keep prose matching locale-aware. Apply the invariant transform only to fields with an identifier contract. Apply the same split to agent-board-filtering.ts.

Also applies to: 113-113

@OrcaWin
OrcaWin merged commit 0a5f9e9 into main Oct 28, 2026
78 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant