Skip to content

Fix non-Latin record titles collapsing to identical untitled slug - #235

Merged
grimicorn merged 2 commits into
mainfrom
agent/nonlatin-slug
Aug 29, 2026
Merged

Fix non-Latin record titles collapsing to identical untitled slug#235
grimicorn merged 2 commits into
mainfrom
agent/nonlatin-slug

Conversation

@grimicorn-agent

Copy link
Copy Markdown
Collaborator

What & why

titleToSlug in server/utils/markdown.ts ran .toLowerCase().replace(/[^a-z0-9\s-]/g, ""), which deleted every non-ASCII letter. Any title written in a non-Latin script (Cyrillic, CJK, Arabic) therefore produced an identical empty base and collapsed onto untitled, forcing untitled-1/untitled-2 disambiguation, and accented Latin titles were mangled (Cafécaf).

This normalizes the title with Unicode NFKD and drops combining marks before slugifying:

  • Accented Latin folds to its ASCII base: Café Meetingcafe-meeting, Zürich Naïvezurich-naive.
  • Non-Latin scripts survive NFKD with no ASCII, so they strip cleanly to the FALLBACK_SLUG untitled — the fallback is applied deliberately and downstream resolveUniqueFilePath handles uniqueness (per the issue's acceptance criteria).
  • Non-ASCII whitespace (NBSP U+00A0, ideographic space U+3000) NFKD-normalizes to a plain space, so it still acts as a word separator instead of fusing words.

Decisions

  • NFKD normalization over transliteration. The acceptance criteria call for non-Latin titles to yield FALLBACK_SLUG, not for transliterated slugs. NFKD achieves that while also improving the accented-Latin case, with no new dependency.
  • Operation order: normalize → strip marks → lowercase → strip. Lowercasing first would delete ASCII emitted by compatibility decomposition of uncased symbols (Noo). Normalizing first preserves it (№5 Meetingno5-meeting).
  • Named regex constants (COMBINING_MARKS_PATTERN, NON_SLUG_CHAR_PATTERN) replace the inline literal for clarity.

Tests

Added titleToSlug cases in tests/server/utils/markdown.test.ts: non-Latin-script titles → untitled, accented-Latin folding, compatibility-decomposition ASCII survival, and non-ASCII whitespace as a separator. Full suite: 1753 passing. lint:fix, lint:ci clean.

Viewable

Server-side slug utility; exercised on ingest via parseWebhookPayload / parseEmailPayload (webhook + email routes). Behavior is covered by tests/server/utils/markdown.test.ts.

Closes #229

Follow-up suggestions

  • Fold stroke/ligature Latin letters — letters with no NFKD decomposition (ß, ø, ł, æ, þ, đ) are stripped rather than folded, so Straßestrae; an explicit fold map would produce readable slugs (suggested: P3, effort: S, evidence: server/utils/markdown.ts titleToSlug)
  • Bound title length before NFKD normalization — NFKD can expand a string up to ~18x (e.g. U+FDFA), and titleToSlug normalizes the full untruncated title; pre-slicing would cap the transient allocation (suggested: P4, effort: S, evidence: server/utils/markdown.ts titleToSlug)

Normalize titles with NFKD and drop combining marks before slugifying so
accented Latin folds to ASCII (café -> cafe) and non-Latin scripts strip
cleanly to the FALLBACK_SLUG. Operation order fixed so compatibility
decompositions that emit ASCII (№ -> No) survive.

Closes #229
@grimicorn-agent

Copy link
Copy Markdown
Collaborator Author

Independent code review trail

Reviewer: fresh Claude (Opus), did not write the code. 3 rounds.

Round 1 — flagged:

  • Explicit non-ASCII strip was a functional no-op vs. the existing whitelist, AND a regression: it deleted non-ASCII whitespace (NBSP, ideographic space) that previously became hyphens. Fixed — replaced the strip with NFKD normalization + combining-mark drop, which folds accented Latin (cafécafe), normalizes non-ASCII whitespace to a plain space, and still strips non-Latin scripts to untitled.
  • Tests didn't discriminate the change / comment overclaimed. Fixed — added cafécafe-meeting (fails on the old regex, verified) and reworded the comment.

Round 2 — flagged:

  • toLowerCase() ran before normalize("NFKD"), so ASCII emitted by compatibility decomposition of uncased symbols was deleted (Noo). Fixed — reordered to normalize → strip marks → lowercase → strip; added №5 Meetingno5-meeting test.
  • titleToSlug("Привет") === titleToSlug("こんにちは") asserts nothing (passes on a constant return) and is redundant. Fixed — removed; the assertion above already pins each non-Latin title to untitled with concrete values.

Round 3 — flagged, all declined with reasons:

  • Add a fold map for stroke/ligature Latin (ß, ø, ł…). Skipped — pre-existing (the old code mangled these identically; not a regression), and the acceptance chose strip/normalize over transliteration. Recorded as a follow-up suggestion.
  • Replace stripped chars with a space instead of deleting. Skipped — changes existing ASCII behavior (don'tdon-t instead of dont), out of scope.
  • Hash-suffix the untitled fallback so distinct non-Latin titles get distinct filenames. Skipped — directly contradicts the acceptance ("yields the FALLBACK_SLUG 'untitled'"); downstream resolveUniqueFilePath already disambiguates collisions.
  • Bound title length before NFKD. Skipped — low severity, bounded by request body limits; a correct bound needs a non-obvious multiplier. Recorded as a follow-up suggestion.

Unresolved after 3 rounds: none blocking. Two declined items captured as follow-up suggestions in the PR body.

@grimicorn-agent grimicorn-agent added the has-suggestions PR carries follow-up suggestions for the improvement digest label Aug 27, 2026
@grimicorn-agent

Copy link
Copy Markdown
Collaborator Author

Merge maintenance + review trail

Merged origin/main into agent/nonlatin-slug (was BEHIND). No conflicts — main's changes (new server routes, filenameTemplate.ts, vault utils, etc.) don't overlap this PR's server/utils/markdown.ts change. After merge: npm run lint:fix clean, npm run lint:ci clean, npm run test green (1842 passed / 119 files).

Independent review (Opus)

Round 1 flagged code-standards clean (small function, full names, no nesting). Behavioral findings, all skipped with reasons:

  • Non-Latin titles collapse to untitled — this is the PR's deliberate, code-commented design (transliteration folds accented Latin; non-Latin scripts fall back to FALLBACK_SLUG). Collision risk is mitigated upstream: buildFilename composes the slug with {{date}} and {{source}} template tokens (documented in shared/utils/filenameTemplate.ts as the collision guard). Reviewer confirmed it is not introduced by this diff. Redesigning the fallback (e.g. appending an id) is out of scope for a behind-main merge and titleToSlug has no id in scope. Skipped.
  • Adjacent non-Latin/ASCII merges words (Deploy部署v2deployv2) — edge case of the intended strip-to-empty behavior; changing it to strip-to-space is a design choice belonging to the PR author, not a merge task. Skipped.

No correctness/security/standards issues to fix in the diff, so no code changes were made and no further rounds were needed.

@grimicorn
grimicorn merged commit 14bf86d into main Aug 29, 2026
15 checks passed
@grimicorn
grimicorn deleted the agent/nonlatin-slug branch August 29, 2026 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

has-suggestions PR carries follow-up suggestions for the improvement digest

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Non-Latin record titles all collapse to the same 'untitled' filename slug

2 participants