Skip to content

[bug-fix] Fix non-latin-feature-names: preserve Unicode feature names - #4780

Open
github-actions[bot] wants to merge 7 commits into
mainfrom
fix/4574-non-latin-feature-names-e406e85dcd5b1c74
Open

github-actions[bot] wants to merge 7 commits into
mainfrom
fix/4574-non-latin-feature-names-e406e85dcd5b1c74

Conversation

@github-actions

@github-actions github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fixes #4574 using the maintainer's UTF-8 naming policy. Feature descriptions and explicit short names retain Unicode letters and decimal digits in Bash, PowerShell, and Python: 添加用户 now generates 001-添加用户 instead of 001-. Punctuation-only names still warn, and names are truncated on character boundaries to GitHub's 244-byte branch limit.

Bash delegates non-ASCII classification to Python 3 so behavior does not depend on platform-specific character classes; ASCII input, including tabs and newlines, remains shell-only. Explicit LC_ALL is honored: when it cannot handle Unicode names the script reports a clear error rather than switching locales, while ASCII names continue to work. Follow-up fixes preserve configured Python paths and py -3 arguments, match normalized stop words without Unicode case folding, and use a bounded search for UTF-8 truncation. The Python helper writes UTF-8 on stdout and stderr, including on Windows. docs/reference/core.md describes the behavior. The optional extensions/git scripts are outside this change.

Testing

  • Tested locally with uv run specify --help
  • Ran existing tests with uv sync && uv run pytest
  • Tested with a sample project (if applicable)

The exact checklist commands and a manual initialized-project walkthrough were not run. The script tests create temporary project fixtures. On the PR-head checkout, PYTHONPATH="$PWD/src:$PWD" ../.venv/bin/python -m pytest tests/test_create_new_feature_python_parity.py tests/test_timestamp_branches.py -q passed: 225 passed, 5 skipped. bash -n scripts/bash/create-new-feature.sh, git diff --check, and ruff check --ignore PLW1510 tests/test_create_new_feature_python_parity.py passed locally (the ignored lint code is a pre-existing warning; CI's unmodified ruff command passed).

Before the fix, the original Chinese-name case produced 001- in all three variants; it now produces 001-添加用户. Review-round regressions reproduced an ASCII-tab locale error, a newline left in a branch name, and 184 byte-count calls for a long four-byte name before the latest fix. The new tests pass afterward; the long-name case produces a valid 244-byte branch with no more than 16 byte-count calls (about 1.50 s before versus 0.17 s after in local single runs). Existing negative tests continue to cover unavailable UTF-8 locales, explicitly non-UTF-8 LC_ALL, and unavailable Python 3 for Unicode names. The earlier bug-test report identified pre-fix test expectations that were updated alongside the implementation.

At head 7e46f9f37d73e94b2a13b46f17e6c44c39142f5b, CI passed the full pytest matrix on Ubuntu, macOS, and Windows with Python 3.13 and 3.14 (using uv sync --extra test and uv run pytest), plus ruff, shellcheck, markdownlint, CodeQL, and the other reported checks. The Windows 3.13 job explicitly reports test_windows_powershell_51_preserves_unicode_and_utf8_limit PASSED. Human review remains pending; reviewer conversations were left unresolved.

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (fill in the disclosure below)

AI disclosure: The initial proposal was generated autonomously by the GitHub Copilot bug-fix workflow (gpt52codex). On behalf of @mnriem, GitHub Copilot (GPT-6 Sol, autonomous mode; reasoning effort not separately recorded) authored subsequent code, tests, documentation, commits, and this updated PR body. Validation was automated; no human line-by-line review is claimed. Existing descriptions with non-ASCII letters intentionally generate different names.

Generated by 🛠️ Fix Bug from Labeled Issue for #4574 · copilot · gpt52codex · 3.45 AIC · ⌖ 7.44 AIC · ⊞ 17K · ◷

Apply the remediation from the bug assessment on issue #4574.

Refs #4574

Assisted-by: GitHub Copilot (model: gpt-5.2-codex, autonomous)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions Bot added automated bug-fix Trigger the bug-fix agentic workflow labels Sep 28, 2026
@mnriem mnriem added the triage-can-wait Verdict: valid and in-scope but deprioritized; held behind the evidence gate label Sep 29, 2026
Replace the initial proposed sanitizer with Unicode-aware name generation across Bash, PowerShell, and Python. Keep UTF-8 branch names within GitHub byte limits, preserve existing punctuation-only warnings, and update parity tests and documentation. Refs #4574.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@mnriem
mnriem requested a balanced review from Copilot September 29, 2026 12:58
@mnriem
mnriem marked this pull request as ready for review September 29, 2026 12:58
@mnriem
mnriem self-requested a review as a code owner September 29, 2026 12:58

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

Windows Unicode output currently fails, and the three backends disagree for some Unicode number categories.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity · 1 Low severity

Open (3)
What changed in this PR

Preserves Unicode feature names consistently across the core creation scripts and documents the updated naming policy.

Changes:

  • Retains Unicode letters and digits across Bash, PowerShell, and Python.
  • Enforces the 244-byte branch limit on UTF-8 boundaries.
  • Adds cross-backend regression and parity coverage.
File Description
scripts/​bash/​create-new-feature.sh Adds locale-aware Unicode naming and byte truncation.
scripts/​powershell/​create-new-feature.ps1 Preserves Unicode categories and safely truncates UTF-8.
scripts/​python/​create_new_feature.py Retains Unicode names and counts encoded bytes.
tests/​test_create_new_feature_python_parity.py Expands Unicode, locale, warning, and truncation tests.
docs/​reference/​core.md Documents Unicode naming and Bash locale requirements.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/python/create_new_feature.py Outdated
Comment thread scripts/bash/create-new-feature.sh Outdated
Comment thread scripts/bash/create-new-feature.sh
mnriem and others added 2 commits September 29, 2026 08:27
Use Unicode letters and decimal digits consistently for feature names. Emit Python output as UTF-8 on Windows, decode parity subprocesses as UTF-8, and test the no-locale error and non-decimal number cases. Refs #4574.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use Python 3 for non-ASCII character classification where POSIX locale classes vary by platform. Preserve the shell-only ASCII path, report a clear error when a Unicode name lacks Python, and cover both paths. Refs #4574.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@mnriem

mnriem commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator

Addressed this review in 461d1583 and f2c0f938 (current head). Python now writes UTF-8 in JSON and text modes, and the parity harness decodes UTF-8 explicitly. All three backends use Unicode letters and decimal digits; Bash uses Python 3 to classify non-ASCII characters because POSIX locale classes differ across platforms, while ASCII names still work without Python. Added regression coverage for CP1252 output, number categories, unavailable UTF-8 locales, and missing Python for a Unicode Bash name.

Validation: the naming/timestamp suites on the PR checkout passed (212 passed, 4 skipped), and the full pytest matrix now passes on Ubuntu, macOS, and Windows for Python 3.13 and 3.14. Ruff, shellcheck, markdownlint, and the other reported checks are green. The existing review request remains pending; I have not resolved the review threads.

AI disclosure: On behalf of @mnriem, GitHub Copilot (GPT-6 Sol; autonomous code authoring in an interactive session) authored the follow-up code, tests, documentation, PR-body update, and this comment. Local and CI validations were automated; no human line-by-line review is claimed.

Copilot AI 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.

Copilot review overview

🟡 Changes recommended

The Bash helper can reject a usable Python 3 interpreter or invoke a valid launcher incorrectly for Unicode names.

Review effort: Balanced
Findings: 2 Medium severity · 1 Low severity

Open (3)
Resolved since last review (2)

Comment thread scripts/bash/create-new-feature.sh Outdated
Comment thread scripts/bash/create-new-feature.sh Outdated
Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@mnriem

mnriem commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

Addressed review 5366281044 in f23ccf4: the Bash Unicode path accepts a configured Python 3 without PyYAML, and its fallback preserves newline-delimited command arguments, including py -3 and executable paths with spaces. Both added regressions failed before the fix and passed afterward. Local naming/timestamp tests: 214 passed, 4 skipped. Hosted pytest passed on Ubuntu, macOS, and Windows with Python 3.13 and 3.14; shellcheck, ruff, and remaining checks passed. Reviewer threads remain open for confirmation.

Posted on behalf of @mnriem. GitHub Copilot (model: GPT-6 Sol, autonomous mode) authored the fix, tests, commit, and this comment; validation was executed automatically, without line-by-line human supervision.

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Cross-platform Unicode handling has an unresolved naming mismatch and warrants human review.

Review effort: Balanced
Findings: None

Resolved since last review (3)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Use C-locale case-sensitive stop-word matching for Unicode parity

scripts/​bash/​create-new-feature.sh:282

The newly retained Unicode words still pass through the case-insensitive stop-word check above, which inherits LC_ALL=C.UTF-8. Here grep -iE treats ſet as set, so a description like ſet account becomes 001-account in Bash, while the Python variant produces 001-ſet-account. The words already have ASCII capitals lowered; compare them case-sensitively in the C locale instead, and add this input to the Bash/Python parity tests.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@mnriem

mnriem commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

Addressed review 5367045587 in 68d800b: Bash now matches already-normalized stop words case-sensitively under LC_ALL=C, preserving ſet rather than treating it as set. A regression simulating Unicode case-folding in grep failed before the fix and passes afterward; ASCII Set remains filtered. Per the requested locale behavior, an explicitly non-empty LC_ALL now takes precedence over automatic UTF-8 selection. Unicode names fail with a clear error under LC_ALL=C/POSIX, while ASCII names still work; absent or empty LC_ALL retains automatic UTF-8 selection. Updated the naming reference and the existing locale parity coverage. Local naming/timestamp tests: 219 passed, 4 skipped. The full Ubuntu/macOS/Windows pytest matrix on Python 3.13/3.14 and all other PR checks passed. Reviewer threads remain open for confirmation.

Posted on behalf of @mnriem. GitHub Copilot (model: GPT-6 Sol, autonomous mode) authored this fix, tests, documentation, commit, and comment; validation was executed automatically without line-by-line human supervision.

Copilot AI 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.

Copilot review overview

🔵 Needs a closer look

Cross-platform Unicode and locale behavior still needs human review, and the Bash issues identified in the review remain unresolved.

Review effort: Balanced
Findings: None

Previously missed (2)

In code that hasn't changed since last review

Medium severity ASCII controls incorrectly trigger Unicode locale handling

scripts/​bash/​create-new-feature.sh:181

Both the locale guard here and unicode_words at line 181 use [^ -~] to detect Unicode, but that also matches ASCII controls. With LC_ALL=C, --short-name $'foo\tbar' fails with a UTF-8-locale error; with no Python 3 available, the same ASCII name instead fails with a Python-required error. The shell sanitizer could produce foo-bar in both cases. Detect non-ASCII bytes in both checks (for example, under LC_ALL=C, use [^[:print:][:cntrl:]]), and cover an ASCII tab in the locale and missing-Python tests.

Medium severity Use binary search to avoid excessive subprocesses during truncation

scripts/​bash/​create-new-feature.sh:237

When a UTF-8 suffix has 240 four-byte letters, truncating it to 240 characters still leaves 960 bytes. This loop then removes 180 characters, calling branch_byte_count (which launches wc and tr) each time: about 360 subprocesses for one name. Use a binary search for the longest character prefix that fits the byte budget, reducing the count to about eight checks while keeping the boundary intact.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@mnriem

mnriem commented Sep 30, 2026

Copy link
Copy Markdown
Collaborator

Addressed the two Bash concerns in review 5368868990 with commit 7e46f9f. ASCII tabs and newlines now sanitize to separators without requiring UTF-8 or Python 3; non-ASCII input still honors an explicitly set LC_ALL and gets the same Unicode classification. UTF-8 truncation now finds the longest fitting character prefix with bounded byte checks. The tab, newline, and subprocess-count regressions failed before the fix and pass afterward; a 240-character four-byte suffix dropped from 184 byte-count calls to at most 16 and produced the correct 244-byte branch. I also added explicit Windows PowerShell 5.1 Unicode/boundary coverage and refreshed the PR body to the repository template with current evidence.

Local naming/timestamp suites: 225 passed, 5 skipped. Hosted pytest on Ubuntu, macOS, and Windows with Python 3.13 and 3.14 passed, including the Windows PowerShell 5.1 test; ruff, shellcheck, markdownlint, CodeQL, and other checks passed. Reviewer conversations remain open for confirmation.

Posted on behalf of @mnriem. GitHub Copilot (model: GPT-6 Sol, autonomous mode) authored the fix, tests, documentation, commit, PR-body update, and this comment. Validation was automated without human line-by-line supervision.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated bug-fix Trigger the bug-fix agentic workflow triage-can-wait Verdict: valid and in-scope but deprioritized; held behind the evidence gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Non-Latin feature descriptions produce a nameless branch/directory (001-, 004-, ...)

2 participants