Skip to content

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

Open
grimicorn-agent wants to merge 2 commits into
mainfrom
agent/nonlatin-slug
Open

Fix non-Latin record titles collapsing to identical untitled slug#235
grimicorn-agent wants to merge 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.

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