mirror of
https://github.com/garrytan/gbrain.git
synced 2026-07-30 03:12:32 +00:00
slugifySegment stripped every character outside [a-z0-9._-] + four CJK
ranges, so filenames in Hebrew, Arabic, Cyrillic, Greek, Thai (and every
other script) collapsed to empty segments. Distinct files then mapped to
the SAME slug (their shared directory prefix) and silently overwrote each
other, last-writer-wins, with import reporting 0 errors.
All three slug grammars move together so sync never emits a slug that
put_page rejects:
- sync.ts SLUGIFY_KEEP_RE + SLUG_SEGMENT_PATTERN — now keep
\p{Ll}\p{Lm}\p{Lo}\p{M}\p{N} (new single-source SLUG_WORD_CHARS in
cjk.ts), with the u flag
- cjk.ts PAGE_SLUG_SEG — rebuilt on SLUG_WORD_CHARS; consumers
(validatePageSlug, SlugRegistry SLUG_RE, dream-cycle SUMMARY_SLUG_RE,
takes-fence HOLDER_REGEX) all gain the required u flag
CJK_SLUG_CHARS is untouched — it also drives the countCJKAwareWords
chunking density heuristic and must not move with slug grammar.
Kebab-casing, lowercasing, Latin accent-strip (cafe), dots/underscores,
path handling, and the NFC re-normalize (NFD macOS filenames converge
with NFC git filenames) are all unchanged and regression-pinned in
test/slug-unicode-scripts.test.ts. No data migration: the collapse was
N-to-1, so only one row per group ever persisted; a normal gbrain sync
recreates the lost pages under their real slugs.
Co-authored-by: Garry Tan <garrytan@gmail.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Garry Tan
Claude Opus 5
parent
d58bb2b0bb
commit
784358f5fd
+28
-5
@@ -21,12 +21,35 @@ export const CJK_SLUG_CHARS = '一-鿿-ゟ゠-ヿ가-';
|
||||
export const CJK_RANGES_REGEX = new RegExp(`[${CJK_SLUG_CHARS}]`);
|
||||
|
||||
/**
|
||||
* Page-slug segment grammar (no anchors): alnum-or-CJK lead char, then
|
||||
* alnum/CJK/hyphen continuation. Single source for validatePageSlug
|
||||
* (operations.ts), SlugRegistry's SLUG_RE, and the dream-cycle
|
||||
* SUMMARY_SLUG_RE so every slug validator shares one grammar (#738).
|
||||
* Slug "word" character class (#3417): every script's letters, not just
|
||||
* Latin + CJK. Unicode property escapes — REQUIRES the `u` flag on any
|
||||
* regex composed from this string (without `u`, `\p{Ll}` silently matches
|
||||
* the literal chars `p`, `L`, `l`, `{`, `}`).
|
||||
*
|
||||
* \p{Ll} lowercase letters (a-z, Cyrillic/Greek lowercase, đ, …)
|
||||
* \p{Lm} modifier letters
|
||||
* \p{Lo} caseless-script letters (Hebrew, Arabic, Thai, CJK, Devanagari, …)
|
||||
* \p{M} combining marks that survive the Latin accent-strip pass
|
||||
* (Hebrew niqqud, Arabic harakat, Thai/Devanagari vowel signs)
|
||||
* \p{N} numbers (0-9, Arabic-Indic digits, …)
|
||||
*
|
||||
* Uppercase (\p{Lu}/\p{Lt}) is deliberately excluded: slugifySegment()
|
||||
* lowercases before filtering, so validators stay lowercase-canonical.
|
||||
*
|
||||
* Distinct from CJK_SLUG_CHARS above, which also drives the
|
||||
* countCJKAwareWords density heuristic — do NOT merge the two, or slug
|
||||
* grammar changes silently change chunking behavior.
|
||||
*/
|
||||
export const PAGE_SLUG_SEG = `[a-z0-9${CJK_SLUG_CHARS}][a-z0-9${CJK_SLUG_CHARS}\\-]*`;
|
||||
export const SLUG_WORD_CHARS = '\\p{Ll}\\p{Lm}\\p{Lo}\\p{M}\\p{N}';
|
||||
|
||||
/**
|
||||
* Page-slug segment grammar (no anchors): word-char lead, then word-char or
|
||||
* hyphen continuation. Single source for validatePageSlug (operations.ts),
|
||||
* SlugRegistry's SLUG_RE, and the dream-cycle SUMMARY_SLUG_RE so every slug
|
||||
* validator shares one grammar (#738). Compose with the `u` flag — see
|
||||
* SLUG_WORD_CHARS.
|
||||
*/
|
||||
export const PAGE_SLUG_SEG = `[${SLUG_WORD_CHARS}][${SLUG_WORD_CHARS}\\-]*`;
|
||||
|
||||
export const CJK_SENTENCE_DELIMITERS = ['。', '!', '?']; // 。!?
|
||||
export const CJK_CLAUSE_DELIMITERS = [';', ':', ',', '、']; // ;:,、
|
||||
|
||||
@@ -48,8 +48,9 @@ import { safeSplitIndex } from '../text-safe.ts';
|
||||
import { PAGE_SLUG_SEG } from '../cjk.ts';
|
||||
|
||||
// Slug grammar from validatePageSlug — shared via PAGE_SLUG_SEG (#738).
|
||||
// Used for the orchestrator-written summary index slug.
|
||||
const SUMMARY_SLUG_RE = new RegExp(`^${PAGE_SLUG_SEG}(\\/${PAGE_SLUG_SEG})*$`);
|
||||
// Used for the orchestrator-written summary index slug. `u` flag required
|
||||
// by PAGE_SLUG_SEG's \p{...} classes (#3417).
|
||||
const SUMMARY_SLUG_RE = new RegExp(`^${PAGE_SLUG_SEG}(\\/${PAGE_SLUG_SEG})*$`, 'u');
|
||||
|
||||
// ── Model context budget (D1, D5, D7, D9) ─────────────────────────────
|
||||
|
||||
|
||||
@@ -1092,8 +1092,8 @@ export async function importFromFile(
|
||||
chunks: 0,
|
||||
error:
|
||||
`Filename "${relativePath}" produces no usable slug. ` +
|
||||
`Add a "slug:" to the frontmatter, or rename the file to use ` +
|
||||
`ASCII / Chinese / Japanese / Korean characters.`,
|
||||
`Add a "slug:" to the frontmatter, or rename the file to include ` +
|
||||
`at least one letter or number (any script).`,
|
||||
};
|
||||
}
|
||||
} else if (parsed.slug !== expectedSlug) {
|
||||
|
||||
@@ -162,10 +162,11 @@ export function validatePageSlug(slug: string): void {
|
||||
if (slug.length > 255) {
|
||||
throw new OperationError('invalid_params', 'page_slug exceeds 255 characters');
|
||||
}
|
||||
// v0.32.7: CJK ranges (Han / Hiragana / Katakana / Hangul Syllables) allowed
|
||||
// in segments. ASCII shape rules (lead char, hyphen continuation) preserved.
|
||||
if (!new RegExp(`^${PAGE_SLUG_SEG}(\\/${PAGE_SLUG_SEG})*$`, 'i').test(slug)) {
|
||||
throw new OperationError('invalid_params', `Invalid page_slug: ${slug} (allowed: alphanumeric, CJK, hyphens, forward-slash separated segments)`);
|
||||
// #3417: letters/numbers from any script allowed in segments (u flag required
|
||||
// for the \p{...} classes in PAGE_SLUG_SEG). Shape rules (lead char, hyphen
|
||||
// continuation) preserved.
|
||||
if (!new RegExp(`^${PAGE_SLUG_SEG}(\\/${PAGE_SLUG_SEG})*$`, 'iu').test(slug)) {
|
||||
throw new OperationError('invalid_params', `Invalid page_slug: ${slug} (allowed: letters/numbers in any script, hyphens, forward-slash separated segments)`);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -72,9 +72,10 @@ export class SlugRegistryError extends Error {
|
||||
// SlugRegistry
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
// Shares the page-slug segment grammar (incl. CJK ranges, #738) with
|
||||
// Shares the page-slug segment grammar (all scripts, #738/#3417) with
|
||||
// validatePageSlug; keeps this site's dir/name shape (>= 2 segments).
|
||||
const SLUG_RE = new RegExp(`^${PAGE_SLUG_SEG}(\\/${PAGE_SLUG_SEG})+$`);
|
||||
// `u` flag required by PAGE_SLUG_SEG's \p{...} classes.
|
||||
const SLUG_RE = new RegExp(`^${PAGE_SLUG_SEG}(\\/${PAGE_SLUG_SEG})+$`, 'u');
|
||||
|
||||
export class SlugRegistry {
|
||||
constructor(private engine: BrainEngine) {}
|
||||
|
||||
+13
-8
@@ -11,7 +11,7 @@
|
||||
* pathToSlug() → convert file paths to page slugs
|
||||
*/
|
||||
|
||||
import { CJK_SLUG_CHARS } from './cjk.ts';
|
||||
import { SLUG_WORD_CHARS } from './cjk.ts';
|
||||
// v0.37.7.0 #1169 submodule-detection helpers. Bottom-of-file already
|
||||
// aliases existsSync as `_existsSync` for other purposes; the top-of-file
|
||||
// import keeps the pruneDir helper's deps near its callsite.
|
||||
@@ -396,8 +396,10 @@ export function unsyncableReason(path: string, opts: SyncableOptions = {}): Sync
|
||||
|
||||
/**
|
||||
* Character class for the lowercase-canonical form of a slug segment after
|
||||
* slugifySegment() has run. Lowercase letters, digits, dots, underscores,
|
||||
* hyphens. Exposed so adjacent code (e.g. takes-fence holder validation,
|
||||
* slugifySegment() has run. Letters/numbers in any script (lowercase where
|
||||
* the script has case — #3417), dots, underscores, hyphens. Uses \p{...}
|
||||
* classes, so composed regexes need the `u` flag (this one carries it).
|
||||
* Exposed so adjacent code (e.g. takes-fence holder validation,
|
||||
* v0.32 EXP-4) can reuse the actual repo slug grammar instead of inventing
|
||||
* a stricter parallel one and emitting false-positive warnings on legitimate
|
||||
* `companies/acme.io` / `people/foo_bar` slugs (codex review #3).
|
||||
@@ -405,15 +407,18 @@ export function unsyncableReason(path: string, opts: SyncableOptions = {}): Sync
|
||||
* Pattern is the inner character class only (no anchors); callers wrap it
|
||||
* in `^...$` or compose it with prefixes like `(?:people|companies)/...`.
|
||||
*/
|
||||
export const SLUG_SEGMENT_PATTERN = new RegExp(`[a-z0-9._\\-${CJK_SLUG_CHARS}]+`);
|
||||
export const SLUG_SEGMENT_PATTERN = new RegExp(`[${SLUG_WORD_CHARS}._\\-]+`, 'u');
|
||||
|
||||
/**
|
||||
* Slugify a single path segment: lowercase, strip special chars, spaces → hyphens.
|
||||
* CJK ranges (Han / Hiragana / Katakana / Hangul Syllables) are preserved (v0.32.7).
|
||||
* NFC re-normalize after the NFD-strip-accents pass so Hangul Jamo recomposes back
|
||||
* into precomposed syllables that fall inside the whitelist.
|
||||
* Letters and numbers from EVERY script are preserved (#3417): previously only
|
||||
* Latin + CJK survived, so Hebrew/Arabic/Cyrillic/Greek/Thai/... filenames
|
||||
* collapsed to empty segments and distinct files silently merged onto one slug.
|
||||
* NFC re-normalize after the NFD-strip-accents pass so Hangul Jamo recomposes
|
||||
* back into precomposed syllables, and so NFD filenames (macOS) and NFC
|
||||
* filenames (Linux/git) of the same name produce the SAME slug.
|
||||
*/
|
||||
const SLUGIFY_KEEP_RE = new RegExp(`[^a-z0-9.\\s_\\-${CJK_SLUG_CHARS}]`, 'g');
|
||||
const SLUGIFY_KEEP_RE = new RegExp(`[^${SLUG_WORD_CHARS}.\\s_\\-]`, 'gu');
|
||||
|
||||
export function slugifySegment(segment: string): string {
|
||||
return segment
|
||||
|
||||
@@ -134,6 +134,7 @@ export const TAKES_FENCE_END = '<!--- gbrain:takes:end -->';
|
||||
import { SLUG_SEGMENT_PATTERN } from './sync.ts';
|
||||
export const HOLDER_REGEX = new RegExp(
|
||||
`^(?:world|brain|(?:people|companies)/${SLUG_SEGMENT_PATTERN.source}|${SLUG_SEGMENT_PATTERN.source})$`,
|
||||
'u', // required by SLUG_SEGMENT_PATTERN's \p{...} classes (#3417)
|
||||
);
|
||||
|
||||
/**
|
||||
|
||||
@@ -522,7 +522,7 @@ just content.
|
||||
const result = await importFile(engine, filePath, '🌟🚀.md', { noEmbed: true });
|
||||
expect(result.status).toBe('skipped');
|
||||
expect(result.error).toContain('no usable slug');
|
||||
expect(result.error).toContain('ASCII / Chinese / Japanese / Korean');
|
||||
expect(result.error).toContain('at least one letter or number (any script)');
|
||||
expect((engine as any)._calls.length).toBe(0);
|
||||
});
|
||||
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
import { describe, test, expect } from 'bun:test';
|
||||
import { slugifySegment, slugifyPath } from '../src/core/sync.ts';
|
||||
import { validatePageSlug } from '../src/core/operations.ts';
|
||||
import { isValidHolder } from '../src/core/takes-fence.ts';
|
||||
|
||||
/**
|
||||
* #3417 — silent data loss for non-Latin, non-CJK scripts.
|
||||
*
|
||||
* Pre-fix, slugifySegment stripped every character outside [a-z0-9._-] + CJK,
|
||||
* so whole filenames in Hebrew / Arabic / Cyrillic / Greek / Thai collapsed to
|
||||
* empty segments. Distinct files then mapped to the SAME slug (their shared
|
||||
* directory prefix) and last-writer-wins overwrote each other with `import`
|
||||
* reporting 0 errors.
|
||||
*
|
||||
* Every assertion here is behavioral (input → output), so this file FAILS on
|
||||
* pre-fix master and passes with the Unicode-property-escape grammar.
|
||||
*/
|
||||
|
||||
describe('#3417: non-Latin scripts survive slugification', () => {
|
||||
// The six script families from the issue, before/after.
|
||||
const cases: Array<[string, string, string]> = [
|
||||
['Hebrew', 'notes/רשימת קניות.md', 'notes/רשימת-קניות'],
|
||||
['Arabic', 'notes/قائمة المهام.md', 'notes/قائمة-المهام'],
|
||||
['Cyrillic', 'notes/Список задач.md', 'notes/список-задач'],
|
||||
// Greek: tonos marks decompose to U+0301 under NFD and are stripped by the
|
||||
// same combining-accent pass that turns café → cafe. Consistent, stable.
|
||||
['Greek', 'notes/Λίστα εργασιών.md', 'notes/λιστα-εργασιων'],
|
||||
['Thai', 'notes/รายการซื้อของ.md', 'notes/รายการซื้อของ'],
|
||||
['Hebrew + digits', 'notes/תוכנית עבודה 2026.md', 'notes/תוכנית-עבודה-2026'],
|
||||
];
|
||||
|
||||
for (const [name, input, expected] of cases) {
|
||||
test(`${name}: ${input} → ${expected}`, () => {
|
||||
expect(slugifyPath(input)).toBe(expected);
|
||||
});
|
||||
}
|
||||
|
||||
test('distinct same-directory files no longer collapse onto one slug', () => {
|
||||
// Pre-fix ALL of these slugified to "notes" — one page, last writer wins.
|
||||
const slugs = [
|
||||
slugifyPath('notes/רשימת קניות.md'),
|
||||
slugifyPath('notes/قائمة المهام.md'),
|
||||
slugifyPath('notes/Список задач.md'),
|
||||
slugifyPath('notes/Λίστα εργασιών.md'),
|
||||
slugifyPath('notes/รายการซื้อของ.md'),
|
||||
];
|
||||
expect(new Set(slugs).size).toBe(slugs.length);
|
||||
for (const s of slugs) expect(s).not.toBe('notes');
|
||||
});
|
||||
|
||||
test('emitted slugs are ACCEPTED by validatePageSlug (three-grammar coherence)', () => {
|
||||
// The trap: fixing only sync.ts makes sync emit slugs put_page rejects.
|
||||
for (const [, input] of cases) {
|
||||
const slug = slugifyPath(input);
|
||||
expect(() => validatePageSlug(slug)).not.toThrow();
|
||||
}
|
||||
});
|
||||
|
||||
test('takes-fence holder grammar accepts non-Latin slugs', () => {
|
||||
expect(isValidHolder('people/גארי-כהן')).toBe(true);
|
||||
expect(isValidHolder('companies/شركة-مثال')).toBe(true);
|
||||
// Uppercase still rejected (lowercase-canonical contract preserved).
|
||||
expect(isValidHolder('people/Garry-Tan')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('#3417: normalization — NFD (macOS) and NFC (git/Linux) converge', () => {
|
||||
test('Hebrew NFD filename produces the same slug as NFC', () => {
|
||||
const nfc = 'notes/רשימת קניות.md'.normalize('NFC');
|
||||
const nfd = 'notes/רשימת קניות.md'.normalize('NFD');
|
||||
expect(slugifyPath(nfd)).toBe(slugifyPath(nfc));
|
||||
});
|
||||
|
||||
test('Vietnamese NFD filename produces the same slug as NFC', () => {
|
||||
const nfc = 'notes/người dùng.md'.normalize('NFC');
|
||||
const nfd = 'notes/người dùng.md'.normalize('NFD');
|
||||
expect(slugifyPath(nfd)).toBe(slugifyPath(nfc));
|
||||
});
|
||||
});
|
||||
|
||||
describe('#3417: regressions — existing behavior unchanged', () => {
|
||||
test('ASCII kebab-casing, lowercasing, dots, underscores', () => {
|
||||
expect(slugifyPath('notes/Shopping List.md')).toBe('notes/shopping-list');
|
||||
expect(slugifyPath('notes/v1.0.0.md')).toBe('notes/v1.0.0');
|
||||
expect(slugifySegment('my_file_name')).toBe('my_file_name');
|
||||
expect(slugifySegment('notes (march 2024)')).toBe('notes-march-2024');
|
||||
});
|
||||
|
||||
test('Latin accents still strip (café → cafe)', () => {
|
||||
expect(slugifySegment('café résumé')).toBe('cafe-resume');
|
||||
});
|
||||
|
||||
test('CJK still preserved', () => {
|
||||
expect(slugifyPath('notes/购物清单.md')).toBe('notes/购物清单');
|
||||
expect(slugifyPath('inbox/品牌圣经.md')).toBe('inbox/品牌圣经');
|
||||
expect(slugifySegment('한글테스트'.normalize('NFD'))).toBe('한글테스트');
|
||||
});
|
||||
|
||||
test('all-symbol input still collapses to empty (frontmatter-fallback path intact)', () => {
|
||||
expect(slugifySegment('!!!')).toBe('');
|
||||
expect(slugifySegment('🎉🎉')).toBe('');
|
||||
});
|
||||
|
||||
test('control chars, RTL override, punctuation still stripped', () => {
|
||||
expect(slugifySegment('evilgnp')).toBe('evilgnp');
|
||||
expect(slugifySegment('a\u0000b')).toBe('ab');
|
||||
});
|
||||
|
||||
test('validatePageSlug still rejects traversal, backslash, RTL override, uppercase-only weirdness', () => {
|
||||
expect(() => validatePageSlug('../etc/passwd')).toThrow();
|
||||
expect(() => validatePageSlug('notes\\file')).toThrow();
|
||||
expect(() => validatePageSlug('notes/evil')).toThrow();
|
||||
expect(() => validatePageSlug('notes/a\u0007b')).toThrow();
|
||||
});
|
||||
});
|
||||
@@ -259,10 +259,10 @@ describe('SLUG_SEGMENT_PATTERN (v0.32.7)', () => {
|
||||
expect(SLUG_SEGMENT_PATTERN.test('icp-理想客户画像')).toBe(true);
|
||||
});
|
||||
|
||||
test('REGRESSION: rejects non-CJK Unicode (Vietnamese)', () => {
|
||||
// Scope is CJK only; Vietnamese with combining diacritics stays rejected
|
||||
// until we widen to Unicode property escapes in v0.33+.
|
||||
const result = 'người-dùng'.match(new RegExp(`^${SLUG_SEGMENT_PATTERN.source}$`));
|
||||
expect(result).toBeNull();
|
||||
test('accepts non-CJK Unicode (Vietnamese) since the #3417 all-script widening', () => {
|
||||
// Pre-#3417 this was rejected (scope was CJK only). The grammar now uses
|
||||
// Unicode property escapes, so đ/ư/etc. are valid slug characters.
|
||||
const result = 'người-dùng'.match(new RegExp(`^${SLUG_SEGMENT_PATTERN.source}$`, 'u'));
|
||||
expect(result).not.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user