[shaohuzhang1]

fix: patterns bug

[shaohuzhang1]

Here are the review findings:

Potential Issues

1. - list item regex is too broad

'(?<! )- .*'

This only guards against a preceding space, so it will incorrectly match hyphen-minus in contexts like:

• Negative numbers: value - 5, x - y
• YAML-like text or inline math
• A -- flag with a following word

It also lacks a lookbehind for \n or start-of-line anchor pairing, unlike the heading patterns. Consider aligning it with the heading style (match line-start or after \n).

2. Missing anchor consistency
Heading patterns use (?<=^) but ^ in a lookbehind without the multiline flag only matches string start. If this regex runs on multi-line text without m flag, (?<=^) never fires except at position 0. The (?<=\n) branch covers most cases, but this is fragile — precompiled regexes should ideally use the m flag. Verify how the backend applies these patterns.

3. Heading negative lookaheads can mismatch across branches
For ##: (?<=\n)(?<!#)## (?!#).*|(?<=^)(?<!#)## (?!#).*. The (?<!#) before ## correctly prevents matching ###, but the two alternatives duplicate logic unnecessarily. Could be simplified to (?<=^|\n)(?<!#)## (?!#).* only if variable-length lookbehind is supported (it's not in JS), so keeping two branches is acceptable — just note the duplication.

4. Space pattern (?<! ) (?! ) is greedy on whitespace-normalized input
This matches any single space between non-spaces, including spaces inside markdown links, code, or English words. For CJK text splitting that may be intended, but confirm it won't break URLs/code blocks.

5. Labels are Chinese while the file mixes conventions
Existing labels are Chinese (模型优化, 直接回答) so this is consistent — no issue, just noting for reviewers.

Optimization Suggestions

• Deduplicate the heading generation with a small helper:

const headingPattern = (n: number) => {
  const hashes = '#'.repeat(n);
  const guard = `(?<!${'#'.repeat(n - 1) === '' ? '\\b' : '#'})`;
  // simpler: build explicit lookarounds
};

Or generate programmatically from a loop over 1–6 to reduce repetition and copy-paste risk.

• Add as const to keep literal types, consistent with the constants block above:

] as const;

• Consider m flag / documenting flags: export whether these strings are used with new RegExp(value, 'gm') so consumers know expected behavior.

• Add a type/interface for the option shape ({ label: string; value: string }[]) for reuse and clarity.

• Comment the lack of anchors for -, 空格, etc., since they behave differently from headings.

Summary

Main concerns: the - pattern over-matching and reliance on ^ without the multiline flag. The rest is stylistic/repetition cleanup.

[shaohuzhang1]

Code Review

The diff removes splitPatternOptions while leaving a comment above DOCUMENT_SPLIT_STRATEGY_LABELS that references shared usage. Please check the following:

1. Dangling References / Broken Imports

   • splitPatternOptions was exported. Any other module importing it (e.g., a splitter UI config, tests, or the backend-alignment layer) will now break. Search the repo for splitPatternOptions before merging.
   • The removed block's leading comment ("与后端 DocumentSerializers.SplitPattern 的固定标识保持一致；顺序由用户选择决定。") is deleted too, so the backend-coupling contract is now undocumented. If the data moved elsewhere, make sure that contract note moved with it.

2. Backend/Frontend Contract Drift

   • These regex values must stay in sync with DocumentSerializers.SplitPattern. Removing them here without a replacement risks divergence if any consumer still relies on client-side values.

3. Regex Concerns (if reused elsewhere)

   • Patterns like (?<=^) and variable-length lookbehind ((?<=^)# .*) are not supported in all JS engines (Safari/JSC historically rejected variable-length lookbehind). Verify browser support requirements.
   • Header patterns only exclude immediately preceding #, so ####### (7 hashes) could match at level 6. Consider a more robust anchor (^#{n} with word-boundary or (?!\s*#)).
   • The - list pattern (?<! )- .* doesn't anchor to line start, so mid-line - occurrences can match.
   • Non-ASCII full-width punctuation patterns (；，。) are fine but document encoding expectations.

4. Duplicate/Redundant Alternatives

   • (?<=\n)...|(?<=^)... alternatives repeat identical bodies; could factor as (?:(?<=\n)|^)# .*. Minor readability/size gain (avoid for perf-critical paths — no real gain, just DRY).

5. Case: label: '空格' uses (?<! ) (?! ) — matches single spaces not adjacent to another space, but will also match leading/trailing spaces; confirm intended behavior.

6. Typing

   • Consider as const + explicit type instead of untyped array, e.g.:

     export const splitPatternOptions = [...] as const satisfies ReadonlyArray<{ label: string; value: string }>

Recommendation: Confirm whether the removal is intentional relocation. If yes, update imports and port the contract comment. If accidental, revert this hunk.

[shaohuzhang1]

Code Review

Potential Issues

1. Event listener leak – The storage event listener on window is never removed. Every call to createAppI18n() adds a new listener (one per app entry: Admin, Chat, etc.), causing memory leaks and duplicate handlers that all call user.setLanguage. Consider returning a cleanup function or using onScopeDispose / onUnmounted.

2. flush: 'sync' in watch – Synchronous flush can cause bugs when the watched value changes during render/component setup, and disables Vue's batching. Only use it if truly required. 'pre' (default) or 'post' is usually safer.

3. event.storageArea === localStorage – storageArea may be null in some browsers/tests. Safer check: event.key === ... && (event.storageArea == null || event.storageArea === localStorage). Also consider guarding for SSR (typeof window !== 'undefined').

4. Initial document.documentElement.lang side-effect – Setting document.documentElement.lang inside a composable couples i18n logic to the DOM. If createAppI18n is called multiple times (Admin + Chat simultaneously), both will write lang. That's acceptable but worth documenting; alternatively, expose a setter.

5. Cross-tab newValue fallback – When event.key === null (clear), you fall back to navigator.language, ignoring LANGUAGE_STORAGE_KEY removal semantics. Confirm this is intentional; otherwise read from localStorage.getItem(LANGUAGE_STORAGE_KEY) first.

6. No guard against non-browser env – Module-level imports of vue-i18n are fine, but the function directly touches window, localStorage, navigator, document. If ever used in SSR/SSG or tests (jsdom without storage), this will throw.

7. MessageSchema['common'] for enUS/hantCommon type assertion – Type inference assumes other locales match zh-CN shape. If a translation file diverges (missing/extra keys), TypeScript won't catch it unless you explicitly type each import as MessageSchema['common']. Worth adding:

   import enCommonRaw from './en-US/common.json'
   const enCommon = enCommonRaw satisfies typeof zhCommon

8. Listener added after watch with immediate: true – Order is fine, but note the initial language sync relies on user.language being set before mount ("bound before mount" comment acknowledges this). Consider asserting/warning if user.language is empty.

9. Missing error feedback – normalizeLocale silently falls back; no logging when an unknown locale is persisted. Minor.

Optimization Suggestions

• Extract the storage listener into a returned cleanup:

  function createAppI18n(user: LanguageState) {
    // ...
    const onStorage = (event: StorageEvent) => { /* ... */ }
    window.addEventListener('storage', onStorage)
    return {
      i18n,
      dispose: () => window.removeEventListener('storage', onStorage),
    }
  }

  Or wrap in effectScope() so consumers can call scope.stop().
• Cache normalizeLocale(language) result to skip redundant watcher writes (if (i18n.global.locale.value === locale) return).
• Use import.meta.env.SSR / typeof window guard for portability.
• Consider lazy-loading zh-Hant messages (or splitting common translations) to reduce initial bundle — three JSON blobs load eagerly now.
• Add JSDoc noting that user.setLanguage must persist to LANGUAGE_STORAGE_KEY, otherwise cross-tab sync breaks silently.

Minor

• No blank-line/formatting issue. Comments and typing look clean.
• globalInjection: false is good; ensure templates use useI18n() composition API consistently.