Skip to content

Night Shift: Unicode display names for i18n users #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
EtanHey wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Night Shift: Unicode display names for i18n users #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 58 additions & 24 deletions convex/leaderboard.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ function getDateDaysAgo(days: number): string {
}

// Calculate current streak for a user
async function calculateCurrentStreak(ctx: any, userId: string): Promise<number> {
async function calculateCurrentStreak(
ctx: any,
userId: string,
): Promise<number> {
const practiceLogs = await ctx.db
.query("userPracticeLog")
.withIndex("by_user", (q: any) => q.eq("userId", userId))
Expand Down Expand Up @@ -51,7 +54,7 @@ async function calculateCurrentStreak(ctx: any, userId: string): Promise<number>
const startDate = hasPracticedToday ? today : yesterday;
let currentStreak = 0;
let date = startDate;

while (practiceMap.has(date)) {
currentStreak++;
const dateObj = new Date(date);
Expand All @@ -66,7 +69,7 @@ async function calculateCurrentStreak(ctx: any, userId: string): Promise<number>
// resolves songs with error resilience, computes weighted score.
async function calculateProgressScore(
ctx: any,
userId: string
userId: string,
): Promise<{ score: number; topLanguage: string }> {
const wordProgress = await ctx.db
.query("wordProgress")
Expand All @@ -82,17 +85,25 @@ async function calculateProgressScore(

const uniqueSongIds = [...new Set(lineProgress.map((l: any) => l.songId))];
const songSettled = await Promise.allSettled(
uniqueSongIds.map((id: any) => ctx.db.get(id))
uniqueSongIds.map((id: any) => ctx.db.get(id)),
);
const songMap = new Map(
uniqueSongIds.map((id: any, i: number) => [
id,
songSettled[i].status === 'fulfilled' ? (songSettled[i] as PromiseFulfilledResult<any>).value : null,
])
songSettled[i].status === "fulfilled"
? (songSettled[i] as PromiseFulfilledResult<any>).value
: null,
]),
);

const languageScores = new Map<string, { wordsLearned: number; linesCompleted: number }>();
languageScores.set("mixed", { wordsLearned: wordProgress.length, linesCompleted: 0 });
const languageScores = new Map<
string,
{ wordsLearned: number; linesCompleted: number }
>();
languageScores.set("mixed", {
wordsLearned: wordProgress.length,
linesCompleted: 0,
});

for (const line of lineProgress) {
const song = songMap.get(line.songId);
Expand All @@ -111,7 +122,9 @@ async function calculateProgressScore(

for (const [language, scores] of languageScores.entries()) {
const multiplier = getLanguageMultiplier(language);
const languageScore = (scores.wordsLearned * multiplier) + (scores.linesCompleted * multiplier * 0.5);
const languageScore =
scores.wordsLearned * multiplier +
scores.linesCompleted * multiplier * 0.5;
totalScore += languageScore;
if (languageScore > maxLanguageScore) {
maxLanguageScore = languageScore;
Expand Down Expand Up @@ -144,7 +157,9 @@ export const getUserInfo = query({
// Get streak leaderboard
export const getStreakLeaderboard = query({
args: {
period: v.optional(v.union(v.literal("weekly"), v.literal("monthly"), v.literal("all-time"))),
period: v.optional(
v.union(v.literal("weekly"), v.literal("monthly"), v.literal("all-time")),
),
limit: v.optional(v.number()),
offset: v.optional(v.number()),
},
Expand All @@ -157,7 +172,7 @@ export const getStreakLeaderboard = query({

// Calculate streak for each user and collect results
const userStreaks = [];

for (const user of users) {
// Use user's ID for practice data lookup
const streak = await calculateCurrentStreak(ctx, user._id);
Expand All @@ -172,21 +187,24 @@ export const getStreakLeaderboard = query({
// Batch-fetch songs with deduplication + error resilience
const uniqueSongIds = [...new Set(lineProgress.map((l) => l.songId))];
const songSettled = await Promise.allSettled(
uniqueSongIds.map((id) => ctx.db.get(id))
uniqueSongIds.map((id) => ctx.db.get(id)),
);
const songMap = new Map(
uniqueSongIds.map((id, i) => [
id,
songSettled[i].status === 'fulfilled' ? songSettled[i].value : null,
])
songSettled[i].status === "fulfilled" ? songSettled[i].value : null,
]),
);

// Count lines per language
const languageCounts = new Map<string, number>();
for (const line of lineProgress) {
const song = songMap.get(line.songId);
if (song?.sourceLanguage) {
languageCounts.set(song.sourceLanguage, (languageCounts.get(song.sourceLanguage) || 0) + 1);
languageCounts.set(
song.sourceLanguage,
(languageCounts.get(song.sourceLanguage) || 0) + 1,
);
}
}

Expand Down Expand Up @@ -227,7 +245,9 @@ export const getStreakLeaderboard = query({
// Get progress leaderboard with difficulty multipliers
export const getProgressLeaderboard = query({
args: {
period: v.optional(v.union(v.literal("weekly"), v.literal("monthly"), v.literal("all-time"))),
period: v.optional(
v.union(v.literal("weekly"), v.literal("monthly"), v.literal("all-time")),
),
limit: v.optional(v.number()),
offset: v.optional(v.number()),
},
Expand All @@ -240,9 +260,12 @@ export const getProgressLeaderboard = query({

// Calculate progress score for each user
const userScores = [];

for (const user of users) {
const { score, topLanguage } = await calculateProgressScore(ctx, user._id);
const { score, topLanguage } = await calculateProgressScore(
ctx,
user._id,
);

userScores.push({
displayName: user.displayName!,
Expand Down Expand Up @@ -272,7 +295,9 @@ export const getProgressLeaderboard = query({
export const getUserRank = query({
args: {
type: v.union(v.literal("streak"), v.literal("progress")),
period: v.optional(v.union(v.literal("weekly"), v.literal("monthly"), v.literal("all-time"))),
period: v.optional(
v.union(v.literal("weekly"), v.literal("monthly"), v.literal("all-time")),
),
},
handler: async (ctx, { type, period: _period = "all-time" }) => {
const userId = await getAuthUserId(ctx);
Expand Down Expand Up @@ -314,7 +339,10 @@ export const getUserRank = query({

let betterCount = 0;
for (const otherUser of allUsers) {
const { score: otherScore } = await calculateProgressScore(ctx, otherUser._id);
const { score: otherScore } = await calculateProgressScore(
ctx,
otherUser._id,
);
if (otherScore > userScore) {
betterCount++;
}
Expand Down Expand Up @@ -342,10 +370,13 @@ export const setDisplayName = mutation({
return { success: false, error: "Display name must be 3-20 characters" };
}

// Validate display name contains valid characters (alphanumeric + spaces)
const validPattern = /^[a-zA-Z0-9\s]+$/;
// Validate display name contains valid characters (Unicode letters, numbers + spaces)
const validPattern = /^[\p{L}\p{N}\s]+$/u;
Copy link

@cursor cursor bot Feb 26, 2026

Choose a reason for hiding this comment

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

Unicode names reject combining marks

Medium Severity

setDisplayName validates with ^[\p{L}\p{N}\s]+$, which excludes Unicode mark characters. Many valid names require combining marks, so users in several scripts can still be blocked despite the Unicode rollout. This makes the new i18n display-name support incomplete in convex/leaderboard.ts.

Fix in Cursor Fix in Web

if (!validPattern.test(displayName)) {
return { success: false, error: "Display name can only contain letters, numbers, and spaces" };
return {
success: false,
error: "Display name can only contain letters, numbers, and spaces",
};
Comment on lines +373 to +379
Copy link

@coderabbitai coderabbitai bot Feb 26, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

JavaScript Unicode regex \s whitespace characters tabs newlines

💡 Result:

In JavaScript (ECMAScript), regex \s matches all ECMAScript “WhiteSpace” code points plus all “LineTerminator” code points (so it includes tabs and newlines). [2][3]

Practically, it’s equivalent to this set (as listed by MDN):
[\f\n\r\t\v\u0020\u00a0\u1680\u2000-\u200a\u2028\u2029\u202f\u205f\u3000\ufeff] [1]

So yes, \s matches:

  • Tabs: \t (U+0009) and vertical tab \v (U+000B) [1][2]
  • Newlines / line breaks: \n (LF, U+000A), \r (CR, U+000D), plus Unicode line/paragraph separators U+2028, U+2029 [1][2]
  • Other whitespace: space U+0020, NBSP U+00A0, many Unicode “space separator” chars (e.g. U+2000–U+200A), etc. [1][2]

Sources:
[1] MDN – Character classes (\s equivalence list) (developer.mozilla.org)
[2] ECMAScript spec – WhiteSpace & LineTerminator code points (tc39.es)
[3] ECMAScript spec – CharacterClassEscape :: s definition (tc39.es)

Citations:

🌐 Web query:

Unicode combining marks \p{M} Hindi Arabic script names

💡 Result:

  • \p{M} in Unicode-aware regex engines means General_Category = Mark, i.e. the union Mn (Nonspacing_Mark) + Mc (Spacing_Mark) + Me (Enclosing_Mark). In other words, “combining marks” in the broad Unicode sense. [1]

Devanagari (Hindi) — examples of \p{M} character names

Common combining marks used in Hindi/Devanagari include (not exhaustive):

  • U+094D DEVANAGARI SIGN VIRAMA (Hindi name: halant) [2]
  • U+0901 DEVANAGARI SIGN CANDRABINDU [3]
  • U+0902 DEVANAGARI SIGN ANUSVARA [3]
  • U+093C DEVANAGARI SIGN NUKTA [3]
  • Many dependent vowel signs are also marks (Unicode includes both spacing and nonspacing dependent vowel signs in Devanagari). [4]

Arabic — examples of \p{M} character names

Arabic vowel/phonetic combining marks (“tashkil”) include:

  • U+064B ARABIC FATHATAN
  • U+064C ARABIC DAMMATAN
  • U+064D ARABIC KASRATAN
  • U+064E ARABIC FATHA
  • U+064F ARABIC DAMMA
  • U+0650 ARABIC KASRA
  • U+0651 ARABIC SHADDA
  • U+0652 ARABIC SUKUN [5]
    Unicode also encodes additional Arabic combining marks such as ARABIC HAMZA ABOVE (U+0654) and ARABIC HAMZA BELOW (U+0655) (among others). [6]

Practical regex filters (Unicode-property regex)

  • “Any combining mark”: \p{M} [1]
  • “Devanagari combining marks”: \p{Script=Devanagari}&&\p{M}
  • “Arabic combining marks”: \p{Script=Arabic}&&\p{M}

Sources: [1] UAX #44 General_Category (Mark = Mn/Mc/Me); [2] Unicode Devanagari names list (Virama/halant); [3] Unicode Standard Ch.12 (Devanagari combining marks: candrabindu, anusvara, nukta); [4] Unicode Standard Ch.12 (dependent vowel signs as marks); [5] Unicode Arabic names list (tashkil U+064B..U+0652); [6] UTR #53 list of Arabic modifier combining marks (includes hamza above/below, etc.).

🏁 Script executed:

# First, let's look at the context of this code in the leaderboard.ts file
cd convex && head -390 leaderboard.ts | tail -25

Repository: EtanHey/songscript

Length of output: 990

Display name validation regex needs Unicode combining marks and must reject line terminators.

The current pattern /^[\p{L}\p{N}\s]+$/u has two problems:

  1. \s is over-permissive: It allows tabs, newlines, and Unicode line/paragraph separators (U+2028, U+2029) — not just spaces — which could introduce formatting control characters into display names.

  2. Missing \p{M}: Omitting combining marks rejects valid names in scripts like Hindi (Devanagari) and Arabic, where combining marks (e.g., Devanagari VIRAMA, Arabic FATHA) are essential to proper character representation.

Update to:

-    const validPattern = /^[\p{L}\p{N}\s]+$/u;
+    // Letters + combining marks + numbers + space separators (no tabs/newlines)
+    const validPattern = /^[\p{L}\p{M}\p{N}\p{Zs}]+$/u;

This allows only regular space characters (not tabs/newlines) and includes combining marks needed for i18n support.

📝 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
// Validate display name contains valid characters (Unicode letters, numbers + spaces)
const validPattern = /^[\p{L}\p{N}\s]+$/u;
if (!validPattern.test(displayName)) {
return { success: false, error: "Display name can only contain letters, numbers, and spaces" };
return {
success: false,
error: "Display name can only contain letters, numbers, and spaces",
};
// Validate display name contains valid characters (Unicode letters, numbers + spaces)
// Letters + combining marks + numbers + space separators (no tabs/newlines)
const validPattern = /^[\p{L}\p{M}\p{N}\p{Zs}]+$/u;
if (!validPattern.test(displayName)) {
return {
success: false,
error: "Display name can only contain letters, numbers, and spaces",
};
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@convex/leaderboard.ts` around lines 373 - 379, The display name validation
regex (validPattern) is too permissive and misses Unicode combining marks;
update the pattern used where displayName is tested so it accepts Unicode
letters, numbers and combining marks but only literal space characters (not
tabs/newlines/other whitespace). Specifically, replace the character class
[\p{L}\p{N}\s] with one that adds \p{M} for combining marks and uses a literal
space instead of \s, and keep the start/end anchors and /u flag as used in
validPattern to ensure line terminators and control whitespace are rejected
while preserving i18n support.

}

// Reject names with only spaces
Expand All @@ -358,7 +389,10 @@ export const setDisplayName = mutation({
const lowerDisplayName = displayName.toLowerCase();
for (const word of offensiveWords) {
if (lowerDisplayName.includes(word)) {
return { success: false, error: "Display name contains inappropriate content" };
return {
success: false,
error: "Display name contains inappropriate content",
};
}
}

Expand Down
8 changes: 5 additions & 3 deletions src/routes/_authed/welcome.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@ function WelcomePage() {
const [error, setError] = useState<string | null>(null);
const [loading, setLoading] = useState(false);

const setDisplayNameMutation = useConvexMutation(api.leaderboard.setDisplayName);
const setDisplayNameMutation = useConvexMutation(
api.leaderboard.setDisplayName,
);

// Check if user already has a display name
const { data: userInfo } = useQuery(
convexQuery(api.leaderboard.getUserInfo, {})
convexQuery(api.leaderboard.getUserInfo, {}),
);

// If user already has displayName, redirect to dashboard
Expand Down Expand Up @@ -95,7 +97,7 @@ function WelcomePage() {
maxLength={20}
/>
<p className="text-xs text-gray-500 mt-1">
3-20 characters, letters and spaces only
3-20 characters, letters, numbers, and spaces
Copy link

@coderabbitai coderabbitai bot Feb 26, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# First, let's find if WelcomePage.test.tsx exists anywhere
echo "=== Searching for WelcomePage.test.tsx ==="
fd 'WelcomePage\.test\.tsx$' 2>/dev/null || echo "No test file found"

echo ""
echo "=== Searching for test files in welcome route directory ==="
fd '\.test\.tsx$' src/routes/_authed/ 2>/dev/null || echo "No test files found in that directory"

echo ""
echo "=== Locating welcome.tsx component ==="
fd 'welcome\.tsx$' src/routes/_authed/ 2>/dev/null

echo ""
echo "=== Checking component structure at src/routes/_authed/welcome.tsx ==="
wc -l src/routes/_authed/welcome.tsx 2>/dev/null && echo "File exists, reading..."

Repository: EtanHey/songscript

Length of output: 367

🏁 Script executed:

# Read the welcome.tsx file to see the component definition and line 100
cat -n src/routes/_authed/welcome.tsx | head -120

Repository: EtanHey/songscript

Length of output: 4765

🏁 Script executed:

# Check for any test files in the project that might test this component
echo "=== Searching for test files that reference WelcomePage ==="
rg -l 'WelcomePage' --type=tsx --type=ts src/ 2>/dev/null | grep -i test

Repository: EtanHey/songscript

Length of output: 121

Create WelcomePage.test.tsx to cover this component's logic and validation flow.

The WelcomePage component contains state management, side effects, mutations, and form validation (including the UX text at line 100). A corresponding test file WelcomePage.test.tsx is required in the same directory per coding guidelines.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/routes/_authed/welcome.tsx` at line 100, Add a new test file
WelcomePage.test.tsx next to welcome.tsx that exercises the WelcomePage
component’s state, side effects, mutation calls and form validation (including
the UX text "3-20 characters, letters, numbers, and spaces"); mount WelcomePage
with React Testing Library, mock the mutation/network layer used by the
component (e.g., the create/update hook or API client), simulate user input and
submit flows to assert success and failure branches, assert the validation
message at the relevant input shows for invalid values and disappears for valid
values, and assert any side-effect behavior (redirects, toasts, or mutation
calls) by spying/mocking the functions referenced in the component (e.g., the
submit handler and mutation hook used inside WelcomePage).

</p>
</div>

Expand Down