Skip to content

fix(context): retain Unicode concepts and preserve upgrade notes - #552

Open
xiehuanyi wants to merge 1 commit into
trailhq:mainfrom
xiehuanyi:fix/concept-slug-collisions
Open

xiehuanyi wants to merge 1 commit into
trailhq:mainfrom
xiehuanyi:fix/concept-slug-collisions

Conversation

@xiehuanyi

Copy link
Copy Markdown

Concept slugs retain Unicode letters, digits and combining marks, with NFC normalization. Cyrillic, CJK, Arabic and Greek concepts keep their separate files and links; mixed-script names retain the words around URL instead of collapsing into one URL node.

Fixes #543 for the reported non-Latin and mixed-script name loss.

Upgrading the slug policy preserves human notes: an unambiguous old name transfers its notes to a new file, and every changed note file is backed up byte-for-byte under .cache/slug-upgrades/ before pruning. Formerly merged or already-existing targets retain a backup for manual reconciliation; reusing the old ASCII slug cannot attach a merged node's notes to a different concept.

Validation: both Unicode regressions fail before the slug fix. Three real legacy-upgrade cases cover unique notes, formerly merged names, ASCII slug reuse, exact backups and repeated rebuilds; all 38 context tests pass. Build and diff checks pass. The complete suite with test concurrency limited to four reports 1,361 passed / 1 existing skip; the first broad run hit an unrelated signup loopback-fetch failure, whose 16 tests pass in isolation. Independent review caught and verified the note migration fix.

This changes Unicode slug construction; the general ASCII punctuation-collision policy is outside this fix.

AI-assisted implementation and wording.

Copilot AI balanced review requested due to automatic review settings October 6, 2026 08:04

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@trailhq-graft

trailhq-graft Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

🌱 graft blast radius

1 area changed → 2 areas can be affected. 2 dependent symbols, depth 2.
Tests: 1 area updated its tests.
Tag: @anirudhkumar-nanonets — 3 of 3 areas · @Frankie-Xu — Build Context · @shhdwi — Workspace CLI

flowchart TB
  A0(("Workspace CLI<br/>1 symbol"))
  A1(("Engine Initialization<br/>1 symbol"))
  classDef reached fill:#D9EDF3,stroke:#3AA7C9,stroke-width:1.5px,color:#0E313C;
  class A0,A1 reached;
Loading
Can be affected Symbols Nearest hop Reached from
Workspace CLI 1 src/graph/workspace-cli.ts:L49-L70 buildChild — calls, depth 2 Build Context
Engine Initialization 1 src/engine.ts:L63-L73 init — calls, depth 1 Build Context
Who knows this code — 3 people across 3 areas
Area Who knows it
Build Context · changed @anirudhkumar-nanonets — 10 commits, last 18d ago · @Frankie-Xu — 5 commits, last 1mo ago
Workspace CLI · affected @shhdwi — 3 commits, last 2mo ago · @anirudhkumar-nanonets — 2 commits, last 2mo ago
Engine Initialization · affected @anirudhkumar-nanonets — 16 commits, last 2mo ago

Ownership is git history over each area's own files, weighted towards recent work (120-day half-life). Merge commits and bots are dropped, and you are dropped from your own PR. A name with no @ has no GitHub handle in its commit email — tag them by hand, or add a .mailmap entry. A suggestion from history, not a CODEOWNERS rule.

All 2 dependent symbols, grouped by area

Workspace CLI — 1 symbol in 1 file

  • src/graph/workspace-cli.ts:L49-L70 — buildChild (calls, depth 2)

Engine Initialization — 1 symbol in 1 file

  • src/engine.ts:L63-L73 — init (calls, depth 1)
    64: return buildContext(dir, {
Test signal per changed area — 1 ✓

Reached = a node under a test path has a resolved edge into the changed symbol. It undercounts anything called indirectly — through a CLI, a spawned process or a dynamic import — so read a low ratio as “look here”, never as a coverage gate.

  • ✓ Build Context — 2 of 4 reached · 1 test file changed here: test/context.test.ts
    • not reached: key, legacy
4 test suites also reference this code

4 symbols, kept out of the diagram and the table so they cannot crowd out the areas a reviewer has to look at.

  • test/context-checkpoint.test.ts
  • test/context-only-dir.test.ts
  • test/covers.test.ts
  • test/utf16-source.test.ts

graft blast · origin/main...HEAD · depth 2 · 3 changed files

Open the interactive graph → — click an area to see its dependent symbols at file:line.

github-actions Bot added a commit that referenced this pull request Oct 6, 2026

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Concept synthesis silently merges all non-Latin-named concepts into one node (slugify drops non-ASCII, Phase 3 merges by slug)

2 participants