mirror of
https://github.com/kdnk/obsidian-automatic-linker.git
synced 2026-07-22 05:37:46 +00:00
fix(replace-links): align Korean ambiguity handling
Why: - Task 1 still had drift between candidate scanning and Korean replacement special cases. - The scanner could ask AI about Korean particle forms that replacement intentionally skips. - The Korean suffix replacement path ignored resolved ambiguities and could apply a different target than the AI chose. What: - skip Korean particle trie hits in the Task 1 candidate scanner and ambiguity request path - reuse a shared resolved-ambiguity link-content helper for the standard and Korean suffix replacement branches - add Task 1 regressions and append the Task 1 fix report with focused and full verification results
This commit is contained in:
parent
b264541faa
commit
490e0c3277
6 changed files with 216 additions and 19 deletions
|
|
@ -208,3 +208,83 @@ Results:
|
|||
- `npm run test -- --reporter=dot`: passed, `325/325` tests across `38/38` files
|
||||
- `npm run tsc`: passed
|
||||
- `npm run lint`: passed
|
||||
|
||||
## Review Fix 3
|
||||
|
||||
### Scope
|
||||
|
||||
Fixed the remaining Task 1 Korean special-case scanner/replacement drift:
|
||||
|
||||
- `scanCandidateOccurrences()` now skips Korean particle trie hits like `문서는` / `문서은`, matching the current non-linking `replaceLinks()` behavior for that branch
|
||||
- `replaceLinks()` now honors `resolvedAmbiguities` for the Korean suffix branch like `문서이다`, so AI-resolved targets are applied consistently
|
||||
- added regressions covering scanner omission, AI request omission, and Korean suffix replacement resolution
|
||||
|
||||
No AGENTS.md changes were needed.
|
||||
|
||||
### TDD Evidence
|
||||
|
||||
#### RED
|
||||
|
||||
Commands:
|
||||
|
||||
```bash
|
||||
npx vitest run src/replace-links/__tests__/candidate-scanner.test.ts
|
||||
npx vitest run src/utils/__tests__/resolve-ambiguities.test.ts
|
||||
npx vitest run src/replace-links/__tests__/ai-disambiguation.test.ts
|
||||
```
|
||||
|
||||
Results:
|
||||
|
||||
- `candidate-scanner.test.ts`: failed because `scanCandidateOccurrences()` emitted `문서` for `문서는` and `문서은`
|
||||
- `resolve-ambiguities.test.ts`: failed because the scanner still generated an AI ambiguity request for Korean particle forms
|
||||
- `ai-disambiguation.test.ts`: failed because `replaceLinks()` ignored `resolvedAmbiguities` for the Korean `이다` suffix branch and kept the default candidate
|
||||
|
||||
#### GREEN
|
||||
|
||||
Commands:
|
||||
|
||||
```bash
|
||||
npx vitest run src/replace-links/__tests__/candidate-scanner.test.ts
|
||||
npx vitest run src/utils/__tests__/resolve-ambiguities.test.ts
|
||||
npx vitest run src/replace-links/__tests__/ai-disambiguation.test.ts
|
||||
```
|
||||
|
||||
Results:
|
||||
|
||||
- `src/replace-links/__tests__/candidate-scanner.test.ts`: passed `8/8`
|
||||
- `src/utils/__tests__/resolve-ambiguities.test.ts`: passed `5/5`
|
||||
- `src/replace-links/__tests__/ai-disambiguation.test.ts`: passed `4/4`
|
||||
|
||||
### Implementation Summary
|
||||
|
||||
- Added an internal scanner guard that skips Korean trie hits when the matched term is immediately followed by the `는` or `은` particle.
|
||||
- Introduced a shared link-content resolver in `replace-links.ts` so the normal ambiguity-resolution path and the Korean `이다` suffix path use the same `resolvedAmbiguities` lookup.
|
||||
- Kept behavior unchanged when `resolvedAmbiguities` is absent by falling back to the existing candidate-derived link generation.
|
||||
|
||||
### Focused Verification
|
||||
|
||||
Command:
|
||||
|
||||
```bash
|
||||
npx vitest run src/replace-links/__tests__/candidate-scanner.test.ts src/utils/__tests__/resolve-ambiguities.test.ts src/replace-links/__tests__/ai-disambiguation.test.ts src/replace-links/__tests__/replace-links.cjk.test.ts
|
||||
```
|
||||
|
||||
Result:
|
||||
|
||||
- Passed: `29/29` tests across `4/4` files
|
||||
|
||||
### Full Verification
|
||||
|
||||
Commands:
|
||||
|
||||
```bash
|
||||
npm run test -- --reporter=dot
|
||||
npm run tsc
|
||||
npm run lint
|
||||
```
|
||||
|
||||
Results:
|
||||
|
||||
- `npm run test -- --reporter=dot`: passed, `328/328` tests across `38/38` files
|
||||
- `npm run tsc`: passed
|
||||
- `npm run lint`: passed
|
||||
|
|
|
|||
|
|
@ -53,4 +53,30 @@ describe("replaceLinks with AI disambiguation", () => {
|
|||
|
||||
expect(result).toBe("Check [[work/meeting|private/meeting]] notes.")
|
||||
})
|
||||
|
||||
it("should honor AI-resolved Korean suffix choices", () => {
|
||||
const koreanFiles = [
|
||||
{ path: "work/문서", scoped: false, aliases: [] },
|
||||
{ path: "private/문서", scoped: false, aliases: [] },
|
||||
]
|
||||
const { candidateMap: koreanCandidateMap, trie: koreanTrie } = buildCandidateTrie(
|
||||
koreanFiles,
|
||||
undefined,
|
||||
true,
|
||||
)
|
||||
const body = "문서이다."
|
||||
const resolvedAmbiguities = new Map([["문서", "private/문서"]])
|
||||
|
||||
const result = replaceLinks({
|
||||
body,
|
||||
linkResolverContext: {
|
||||
filePath: "test.md",
|
||||
trie: koreanTrie,
|
||||
candidateMap: koreanCandidateMap,
|
||||
},
|
||||
resolvedAmbiguities,
|
||||
})
|
||||
|
||||
expect(result).toBe("[[private/문서|문서]]이다.")
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -216,6 +216,40 @@ meeting`,
|
|||
},
|
||||
])
|
||||
})
|
||||
|
||||
it("does not emit Korean particle forms that replaceLinks currently skips", () => {
|
||||
const { candidateMap, trie } = buildCandidateTrieForTest({
|
||||
files: [
|
||||
{ path: "work/문서" },
|
||||
{ path: "private/문서" },
|
||||
],
|
||||
settings: {
|
||||
scoped: false,
|
||||
baseDir: undefined,
|
||||
ignoreCase: true,
|
||||
},
|
||||
})
|
||||
|
||||
const occurrences = scanCandidateOccurrences({
|
||||
text: "문서는 문서은 문서이다",
|
||||
filePath: "notes/today",
|
||||
trie,
|
||||
candidateMap,
|
||||
settings: { ignoreCase: true, proximityBasedLinking: true },
|
||||
})
|
||||
|
||||
expect(occurrences.map(o => ({
|
||||
text: o.text,
|
||||
start: o.start,
|
||||
end: o.end,
|
||||
}))).toEqual([
|
||||
{
|
||||
text: "문서",
|
||||
start: 8,
|
||||
end: 10,
|
||||
},
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
describe("getOccurrenceContext", () => {
|
||||
|
|
|
|||
|
|
@ -582,6 +582,19 @@ const collectFallbackOccurrence = ({
|
|||
}
|
||||
}
|
||||
|
||||
const shouldSkipKoreanTrieOccurrence = (
|
||||
text: string,
|
||||
startIndex: number,
|
||||
candidate: string,
|
||||
): boolean => {
|
||||
if (!isKoreanText(candidate)) {
|
||||
return false
|
||||
}
|
||||
|
||||
const remaining = text.slice(startIndex + candidate.length)
|
||||
return REGEX_PATTERNS.KOREAN_PARTICLES.test(remaining)
|
||||
}
|
||||
|
||||
const collectUnlinkedOccurrences = ({
|
||||
text,
|
||||
filePath,
|
||||
|
|
@ -658,6 +671,11 @@ const collectUnlinkedOccurrences = ({
|
|||
continue
|
||||
}
|
||||
|
||||
if (shouldSkipKoreanTrieOccurrence(text, i, candidate)) {
|
||||
i += candidate.length
|
||||
continue
|
||||
}
|
||||
|
||||
const candidateIsCjk = isCjkCandidate(candidate)
|
||||
if (!candidateIsCjk) {
|
||||
const left = i > 0 ? text[i - 1] : undefined
|
||||
|
|
|
|||
|
|
@ -157,6 +157,24 @@ const createLinkContent = (
|
|||
}
|
||||
}
|
||||
|
||||
const resolveLinkContent = (
|
||||
candidateData: CandidateData,
|
||||
originalMatchedText: string,
|
||||
settings: ReplaceLinksSettings,
|
||||
resolvedAmbiguities?: Map<string, string>,
|
||||
): { linkPath: string, alias?: string } => {
|
||||
if (resolvedAmbiguities?.has(originalMatchedText)) {
|
||||
const resolvedPath = resolvedAmbiguities.get(originalMatchedText)!
|
||||
const parts = extractLinkParts(resolvedPath)
|
||||
return {
|
||||
linkPath: parts.linkPath,
|
||||
alias: parts.alias || originalMatchedText,
|
||||
}
|
||||
}
|
||||
|
||||
return createLinkContent(candidateData, originalMatchedText, settings)
|
||||
}
|
||||
|
||||
// Default link generator that creates standard Obsidian wikilinks
|
||||
export const escapeLinkForMarkdownTable = (
|
||||
link: string,
|
||||
|
|
@ -350,6 +368,7 @@ const handleKoreanSpecialCases = (
|
|||
filePath: string,
|
||||
linkGenerator: LinkGenerator,
|
||||
settings: ReplaceLinksSettings = {},
|
||||
resolvedAmbiguities?: Map<string, string>,
|
||||
): { result: string, newIndex: number } | null => {
|
||||
const remaining = text.slice(i + candidate.length)
|
||||
|
||||
|
|
@ -364,10 +383,11 @@ const handleKoreanSpecialCases = (
|
|||
}
|
||||
}
|
||||
|
||||
const { linkPath, alias } = createLinkContent(
|
||||
const { linkPath, alias } = resolveLinkContent(
|
||||
candidateData,
|
||||
candidate,
|
||||
settings,
|
||||
resolvedAmbiguities,
|
||||
)
|
||||
const finalLink = linkGenerator({
|
||||
linkPath,
|
||||
|
|
@ -496,6 +516,7 @@ const processStandardText = (
|
|||
filePath,
|
||||
linkGenerator,
|
||||
settings,
|
||||
resolvedAmbiguities,
|
||||
)
|
||||
if (koreanResult) {
|
||||
result += koreanResult.result
|
||||
|
|
@ -548,24 +569,12 @@ const processStandardText = (
|
|||
}
|
||||
|
||||
// Create the link
|
||||
let linkPath = ""
|
||||
let alias: string | undefined
|
||||
|
||||
if (resolvedAmbiguities?.has(candidate)) {
|
||||
const resolvedPath = resolvedAmbiguities.get(candidate)!
|
||||
const parts = extractLinkParts(resolvedPath)
|
||||
linkPath = parts.linkPath
|
||||
alias = parts.alias || candidate
|
||||
}
|
||||
else {
|
||||
const content = createLinkContent(
|
||||
candidateData,
|
||||
candidate,
|
||||
settings,
|
||||
)
|
||||
linkPath = content.linkPath
|
||||
alias = content.alias
|
||||
}
|
||||
const { linkPath, alias } = resolveLinkContent(
|
||||
candidateData,
|
||||
candidate,
|
||||
settings,
|
||||
resolvedAmbiguities,
|
||||
)
|
||||
|
||||
const isInTable = isIndexInsideMarkdownTable(text, i)
|
||||
const finalLink = linkGenerator({
|
||||
|
|
|
|||
|
|
@ -123,4 +123,34 @@ meeting`
|
|||
],
|
||||
)
|
||||
})
|
||||
|
||||
it("should not request AI for Korean particle forms that replacement skips", async () => {
|
||||
const koreanCandidateMap = new Map<string, CandidateData>([
|
||||
[
|
||||
"문서",
|
||||
{
|
||||
candidates: [
|
||||
{ canonical: "work/문서", scoped: false, namespace: "work" },
|
||||
{ canonical: "private/문서", scoped: false, namespace: "private" },
|
||||
],
|
||||
},
|
||||
],
|
||||
])
|
||||
const koreanTrie: TrieNode = buildTrie(["문서"], true)
|
||||
vi.mocked(aiClient.resolveAmbiguitiesBatch).mockClear()
|
||||
vi.mocked(aiClient.resolveAmbiguitiesBatch).mockResolvedValue(new Map())
|
||||
|
||||
const result = await resolveAmbiguities(
|
||||
"문서는 문서은",
|
||||
koreanCandidateMap,
|
||||
koreanTrie,
|
||||
mockSettings,
|
||||
)
|
||||
|
||||
expect(aiClient.resolveAmbiguitiesBatch).toHaveBeenCalledWith(
|
||||
mockSettings,
|
||||
[],
|
||||
)
|
||||
expect(result.size).toBe(0)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in a new issue