From b91d398637d0f7e709c0309ad62fe30996adc240 Mon Sep 17 00:00:00 2001 From: Matt McKay Date: Thu, 23 Jul 2026 15:43:35 +1000 Subject: [PATCH] fix(cli): resolve the glossary package-relative and report the outcome (#149) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `forward` and `init` resolved glossary candidates only against `process.cwd()`. The built-in glossaries ship inside this package and no edition repository carries one, so a resync launched from the target repo — or a bench root, or anywhere a globally installed CLI is naturally invoked — translated with no glossary at all. Nothing was logged either way and a bare `catch {}` swallowed parse errors, so a run that dropped terminology enforcement was indistinguishable from one that applied it. That unobservability is the worst of it: it made the difference unauditable after the fact. The production signature matches exactly. In lecture-python.zh-cn the `init`-seeded lectures all use the glossary's 边缘分布 for "Marginal distribution", while the 2026-07-19 `forward` wave took prob_matrix.md from 12 wrong / 28 correct to 25 wrong / 35 correct — newly generated text ignoring a glossary entry that exists. See QuantEcon/lecture-python.zh-cn#198 for the review finding. Resolution now lives in one testable module shared by both commands, with the packaged directory resolved relative to the installed CLI and threaded in as an option — `import.meta.url` cannot be loaded by the Jest CJS module registry, so the entry point resolves it and the logic stays unit testable. - precedence: --glossary → repo-local glossary/.json → built-in - every outcome reported: the origin, term count and path on success; a warning naming every path tried when a language has no glossary anywhere - loud on failure: a missing or malformed --glossary is a hard error, not a silent fallback to different terminology, and any candidate that exists but does not parse is an error rather than a fall-through - bulk resolves once before the first file, so a bad glossary stops the wave instead of surfacing after N files have been resynced against nothing - `forward --glossary ` added, matching `init` — the resync path previously had no override at all 21 new tests: CWD-independence, precedence, the throw paths, and what the translator actually receives on the resync path from a working directory with no glossary. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 6 + docs/user/cli-reference.md | 5 +- docs/user/glossary.md | 10 +- src/cli/__tests__/cli-smoke.test.ts | 1 + src/cli/__tests__/forward-glossary.test.ts | 176 +++++++++++++++++++ src/cli/__tests__/glossary-loader.test.ts | 186 +++++++++++++++++++++ src/cli/commands/forward.ts | 50 +++--- src/cli/commands/init.ts | 50 ++---- src/cli/glossary-loader.ts | 170 +++++++++++++++++++ src/cli/index.ts | 24 ++- src/cli/types.ts | 5 +- 11 files changed, 622 insertions(+), 61 deletions(-) create mode 100644 src/cli/__tests__/forward-glossary.test.ts create mode 100644 src/cli/__tests__/glossary-loader.test.ts create mode 100644 src/cli/glossary-loader.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d5c871b6..014253b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- **`forward` and `init` find the built-in glossary regardless of working directory, and say which one they loaded** (#149): both commands resolved glossary candidates only against `process.cwd()`. The built-in glossaries ship inside *this* package and **no edition repository carries one**, so a resync launched from the target repo — or a bench root, or anywhere a globally installed CLI is naturally invoked — translated with no glossary at all. Nothing was logged either way and parse errors were swallowed by a bare `catch {}`, so a run that dropped terminology enforcement was indistinguishable from one that applied it; that unobservability was the worst of it, because it made the difference unauditable after the fact. Production signature matching the defect exactly: in `lecture-python.zh-cn`, the `init`-seeded lectures (QuantEcon/lecture-python.zh-cn#196) all use the glossary's 边缘分布 for *Marginal distribution*, while the 2026-07-19 `forward` wave took `prob_matrix.md` from 12 wrong / 28 correct to **25 wrong / 35 correct** — newly generated text ignoring a glossary entry that exists. Resolution now lives in one testable module (`src/cli/glossary-loader.ts`) shared by both commands, with the packaged directory resolved relative to the installed CLI and threaded in as an option (`import.meta.url` cannot be loaded by the Jest CJS registry, so the entry point resolves it and the logic stays unit-testable). Precedence is `--glossary` → repo-local `glossary/.json` → built-in, and every outcome is reported: `✓ Loaded built-in glossary for zh-cn — 357 terms (…)` on success, a warning naming every path tried when a language has no glossary anywhere. **Loud on failure**: an explicit `--glossary` that is missing or malformed is a hard error rather than a silent fallback to different terminology, and any candidate that exists but does not parse is an error rather than a fall-through. Bulk resolves once before the first file, so a bad glossary stops the wave instead of surfacing after N files have been resynced against nothing. + +### Added +- **`forward --glossary `** (#149), matching `init` — previously the resync path had no way to override the glossary at all. + ## [0.22.0] - 2026-07-22 ### Added diff --git a/docs/user/cli-reference.md b/docs/user/cli-reference.md index 7163111c..90a222a3 100644 --- a/docs/user/cli-reference.md +++ b/docs/user/cli-reference.md @@ -235,6 +235,7 @@ npx translate forward -s -t [options] | `-m, --model ` | `claude-sonnet-5` | Claude model | | `--test` | `false` | Use deterministic mock responses (no LLM) | | `--github ` | *(none)* | Create one PR per file in the target repo | +| `--glossary ` | *(auto)* | Path to glossary JSON file (default: built-in glossary for the language) | | `--exclude ` | *(none)* | Exclude files matching pattern | **Single-file example:** @@ -304,7 +305,7 @@ npx translate init -s -t --target-language [o | `--resume-from ` | *(none)* | Resume from a specific lecture file | | `--skip-existing` | `false` | Skip lectures already translated (reads `.translate/state/`) | | `-j, --parallel ` | `1` | Number of parallel translations | -| `--glossary ` | *(auto)* | Path to glossary JSON file (default: `glossary/.json`) | +| `--glossary ` | *(auto)* | Path to glossary JSON file (default: built-in glossary for the language) | | `--localize ` | `code-comments,figure-labels,i18n-font-config` | Localization rules for code cells (use `none` to disable) | | `--dry-run` | `false` | Preview lectures without translating | @@ -343,7 +344,7 @@ Download: [Source Han Serif SC](https://github.com/adobe-fonts/source-han-serif/ **7-phase pipeline:** -1. **Load glossary** — looks for `glossary/.json` in the current working directory +1. **Load glossary** — `--glossary` if given, else a repo-local `glossary/.json`, else the built-in glossary shipped with the package (see [Glossary](glossary.md#using-a-custom-glossary)) 2. **Parse `_toc.yml`** — discovers lectures from the source repo's table of contents 3. **Setup target folder** — creates the target directory structure 4. **Copy non-markdown files** — images, config, data files, CSS (preserves directory structure) diff --git a/docs/user/glossary.md b/docs/user/glossary.md index 8bedbae6..81f782f3 100644 --- a/docs/user/glossary.md +++ b/docs/user/glossary.md @@ -73,7 +73,15 @@ To use your own glossary instead of (or in addition to) the built-in one, specif # ... other inputs ``` -For the CLI, glossaries are loaded automatically based on the language code. The CLI looks for `glossary/{language}.json` in the action-translation repository. +For the CLI, glossaries are loaded automatically based on the language code. The `forward` and `init` commands resolve candidates in this order, and report which one they used: + +| Order | Candidate | Notes | +|-------|-----------|-------| +| 1 | `--glossary ` | When given, this is the **only** candidate — a missing or malformed file is a hard error, never a silent fallback | +| 2 | `/glossary/{language}.json`, `/glossary-{language}.json` | Lets a project override the estate defaults by carrying its own glossary | +| 3 | `{action-translation}/glossary/{language}.json` | The built-in glossary, resolved relative to the installed package — **not** to the working directory | + +Every run prints the glossary it loaded (`✓ Loaded built-in glossary for zh-cn — 357 terms (…)`) or warns that it found none and lists every path it tried. A malformed glossary is an error rather than a silent skip. ## Adding terms to a glossary diff --git a/src/cli/__tests__/cli-smoke.test.ts b/src/cli/__tests__/cli-smoke.test.ts index 9c743c37..5d2215db 100644 --- a/src/cli/__tests__/cli-smoke.test.ts +++ b/src/cli/__tests__/cli-smoke.test.ts @@ -186,6 +186,7 @@ chapters: expect(stdout).toContain('--target'); expect(stdout).toContain('--github'); expect(stdout).toContain('--parallel'); + expect(stdout).toContain('--glossary'); }); }); diff --git a/src/cli/__tests__/forward-glossary.test.ts b/src/cli/__tests__/forward-glossary.test.ts new file mode 100644 index 00000000..ed762973 --- /dev/null +++ b/src/cli/__tests__/forward-glossary.test.ts @@ -0,0 +1,176 @@ +/** + * Glossary delivery on the resync path (#149). + * + * `forward` resolved its glossary only against `process.cwd()`. No edition + * repository carries `glossary/.json`, so a resync run from the target + * repo — the natural place to run it — translated with no glossary at all and + * logged nothing either way. These tests pin the end of the chain: what the + * translator actually receives, from a working directory that has no glossary. + * + * Separate file from forward.test.ts because it mocks triage and the translator, + * which the main suite exercises for real (in test mode). + */ + +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; +import { resyncSingleFile } from '../commands/forward.js'; +import { ForwardOptions } from '../types.js'; +import { Glossary } from '../../types.js'; + +jest.mock('../forward-triage.js', () => ({ + triageForward: jest.fn().mockResolvedValue({ + verdict: 'CONTENT_CHANGES', + reason: 'mocked', + }), +})); + +const mockResync = jest.fn(); +jest.mock('../../translator.js', () => ({ + TranslationService: jest.fn().mockImplementation(() => ({ + translateDocumentResync: mockResync, + })), +})); + +const BUILT_IN_DIR = path.join(__dirname, '..', '..', '..', 'glossary'); + +function createTestLogger() { + const messages: Array<{ level: 'info' | 'warn' | 'error'; text: string }> = []; + return { + messages, + info: (text: string) => messages.push({ level: 'info' as const, text }), + warn: (text: string) => messages.push({ level: 'warn' as const, text }), + error: (text: string) => messages.push({ level: 'error' as const, text }), + }; +} + +function makeOptions(overrides: Partial = {}): ForwardOptions { + return { + source: '/tmp/source', + target: '/tmp/target', + docsFolder: 'lectures', + language: 'zh-cn', + sourceLanguage: 'en', + model: 'claude-sonnet-5', + test: false, + apiKey: 'test-key', + ...overrides, + }; +} + +/** The glossary handed to the translator on the single (mocked) resync call. */ +function glossaryPassedToTranslator(): Glossary | undefined { + expect(mockResync).toHaveBeenCalledTimes(1); + return mockResync.mock.calls[0][0].glossary; +} + +describe('forward glossary resolution', () => { + let tmpDir: string; + let cwdSpy: jest.SpyInstance; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'forward-glossary-')); + fs.mkdirSync(path.join(tmpDir, 'source', 'lectures'), { recursive: true }); + fs.mkdirSync(path.join(tmpDir, 'target', 'lectures'), { recursive: true }); + fs.writeFileSync( + path.join(tmpDir, 'source', 'lectures', 'test.md'), + '# Title\n\nMarginal distribution.\n', + 'utf-8' + ); + fs.writeFileSync( + path.join(tmpDir, 'target', 'lectures', 'test.md'), + '# 标题\n\n旧内容。\n', + 'utf-8' + ); + + // Run "from the target repo" — the working directory the wave is launched + // from in practice, and one that carries no glossary of its own. + cwdSpy = jest.spyOn(process, 'cwd').mockReturnValue(path.join(tmpDir, 'target')); + + mockResync.mockReset(); + mockResync.mockResolvedValue({ + success: true, + translatedSection: '# 标题\n\n新内容。\n', + tokensUsed: 10, + }); + }); + + afterEach(() => { + cwdSpy.mockRestore(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + async function resync(options: ForwardOptions, logger = createTestLogger()) { + const result = await resyncSingleFile( + 'test.md', + path.join(tmpDir, 'source'), + path.join(tmpDir, 'target'), + 'lectures', + options, + logger + ); + return { result, logger }; + } + + it('sends the built-in glossary even when the working directory has none', async () => { + const { result } = await resync(makeOptions({ builtInGlossaryDir: BUILT_IN_DIR })); + + expect(result.summary.errors).toBe(0); + const glossary = glossaryPassedToTranslator(); + expect(glossary).toBeDefined(); + expect(glossary!.terms.length).toBeGreaterThan(0); + // The term whose absence surfaced this defect on QuantEcon/lecture-python.zh-cn#198. + expect(glossary!.terms.some((t) => t.en === 'Marginal distribution')).toBe(true); + }); + + it('reports the glossary it loaded', async () => { + const { logger } = await resync(makeOptions({ builtInGlossaryDir: BUILT_IN_DIR })); + + const loaded = logger.messages.filter((m) => m.text.includes('glossary for zh-cn')); + expect(loaded).toHaveLength(1); + expect(loaded[0].level).toBe('info'); + }); + + it('honours an explicit --glossary path', async () => { + const custom = path.join(tmpDir, 'custom.json'); + fs.writeFileSync( + custom, + JSON.stringify({ version: '1.0', terms: [{ en: 'only', 'zh-cn': '唯一' }] }), + 'utf-8' + ); + + await resync(makeOptions({ builtInGlossaryDir: BUILT_IN_DIR, glossaryPath: custom })); + + expect(glossaryPassedToTranslator()!.terms).toHaveLength(1); + }); + + it('fails loudly on a --glossary path that does not exist', async () => { + await expect( + resync(makeOptions({ builtInGlossaryDir: BUILT_IN_DIR, glossaryPath: '/no/such.json' })) + ).rejects.toThrow(/Glossary not found/); + + expect(mockResync).not.toHaveBeenCalled(); + }); + + it('uses a pre-resolved glossary without re-resolving it', async () => { + // How bulk threads one load through every file in the wave. + const preResolved: Glossary = { version: '1.0', terms: [{ en: 'threaded', 'zh-cn': '穿线' }] }; + + const { logger } = await resync( + makeOptions({ builtInGlossaryDir: BUILT_IN_DIR, glossary: preResolved }) + ); + + expect(glossaryPassedToTranslator()).toBe(preResolved); + expect(logger.messages.filter((m) => m.text.includes('glossary for zh-cn'))).toHaveLength(0); + }); + + it('says so when it ends up with no glossary', async () => { + // No built-in directory threaded through and none in the working directory: + // the pre-fix situation, which must no longer be silent. + const { logger } = await resync(makeOptions()); + + expect(glossaryPassedToTranslator()).toBeUndefined(); + const warnings = logger.messages.filter((m) => m.level === 'warn'); + expect(warnings.some((w) => w.text.includes('WITHOUT terminology enforcement'))).toBe(true); + }); +}); diff --git a/src/cli/__tests__/glossary-loader.test.ts b/src/cli/__tests__/glossary-loader.test.ts new file mode 100644 index 00000000..05c5d143 --- /dev/null +++ b/src/cli/__tests__/glossary-loader.test.ts @@ -0,0 +1,186 @@ +/** + * Tests for CLI glossary resolution (#149). + * + * The property that matters is CWD-independence: a resync launched from the + * target repo, a bench root, or anywhere else must find the same glossary an + * action-translation checkout finds. The second property is observability — + * every outcome, including "nothing found", is reported. + */ + +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { loadGlossary, resolveGlossary } from '../glossary-loader.js'; + +const BUILT_IN_DIR = path.join(__dirname, '..', '..', '..', 'glossary'); + +/** Logger that records what it was told, so silence is testable. */ +function createTestLogger() { + const messages: Array<{ level: 'info' | 'warn'; text: string }> = []; + return { + messages, + info: (text: string) => messages.push({ level: 'info' as const, text }), + warn: (text: string) => messages.push({ level: 'warn' as const, text }), + }; +} + +function tempDir(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), 'glossary-loader-')); +} + +/** Write a minimal but valid glossary file and return its path. */ +function writeGlossary(dir: string, name: string, terms: Array>): string { + fs.mkdirSync(dir, { recursive: true }); + const file = path.join(dir, name); + fs.writeFileSync(file, JSON.stringify({ version: '1.0', terms }), 'utf-8'); + return file; +} + +describe('resolveGlossary', () => { + let cwd: string; + + beforeEach(() => { + cwd = tempDir(); + }); + + afterEach(() => { + fs.rmSync(cwd, { recursive: true, force: true }); + }); + + // ── the defect ──────────────────────────────────────────────────────────── + + it('finds the built-in glossary from a working directory that has none', () => { + // The regression: `cwd` is a target repo, which never carries a glossary. + const result = resolveGlossary('zh-cn', { builtInDir: BUILT_IN_DIR, cwd }); + + expect(result.origin).toBe('built-in'); + expect(result.glossary!.terms.length).toBeGreaterThan(0); + }); + + it.each(['zh-cn', 'fa', 'fr'])('resolves the packaged %s glossary', (language) => { + const result = resolveGlossary(language, { builtInDir: BUILT_IN_DIR, cwd }); + + expect(result.origin).toBe('built-in'); + expect(result.glossary!.terms.length).toBeGreaterThan(0); + }); + + // ── precedence ──────────────────────────────────────────────────────────── + + it('prefers a repo-local glossary/.json over the built-in one', () => { + writeGlossary(path.join(cwd, 'glossary'), 'zh-cn.json', [{ en: 'local', 'zh-cn': '本地' }]); + + const result = resolveGlossary('zh-cn', { builtInDir: BUILT_IN_DIR, cwd }); + + expect(result.origin).toBe('repo-local'); + expect(result.glossary!.terms).toHaveLength(1); + }); + + it('accepts the flat glossary-.json form too', () => { + writeGlossary(cwd, 'glossary-zh-cn.json', [{ en: 'flat', 'zh-cn': '扁平' }]); + + const result = resolveGlossary('zh-cn', { builtInDir: BUILT_IN_DIR, cwd }); + + expect(result.origin).toBe('repo-local'); + expect(result.glossary!.terms).toHaveLength(1); + }); + + it('treats an explicit path as the only candidate', () => { + // A repo-local glossary exists and must NOT win over an explicit --glossary. + writeGlossary(path.join(cwd, 'glossary'), 'zh-cn.json', [{ en: 'local', 'zh-cn': '本地' }]); + const custom = writeGlossary(tempDir(), 'custom.json', [ + { en: 'custom', 'zh-cn': '自定义' }, + { en: 'second', 'zh-cn': '第二' }, + ]); + + const result = resolveGlossary('zh-cn', { builtInDir: BUILT_IN_DIR, cwd, customPath: custom }); + + expect(result.origin).toBe('custom'); + expect(result.glossary!.terms).toHaveLength(2); + }); + + // ── loud failure ────────────────────────────────────────────────────────── + + it('throws when an explicit path does not exist', () => { + expect(() => + resolveGlossary('zh-cn', { builtInDir: BUILT_IN_DIR, cwd, customPath: '/no/such.json' }) + ).toThrow(/Glossary not found/); + }); + + it('throws on malformed JSON instead of silently falling through', () => { + fs.mkdirSync(path.join(cwd, 'glossary'), { recursive: true }); + fs.writeFileSync(path.join(cwd, 'glossary', 'zh-cn.json'), '{ not json', 'utf-8'); + + expect(() => resolveGlossary('zh-cn', { builtInDir: BUILT_IN_DIR, cwd })).toThrow( + /not valid JSON/ + ); + }); + + it('throws when a glossary has no terms array', () => { + fs.mkdirSync(path.join(cwd, 'glossary'), { recursive: true }); + fs.writeFileSync( + path.join(cwd, 'glossary', 'zh-cn.json'), + JSON.stringify({ version: '1.0' }), + 'utf-8' + ); + + expect(() => resolveGlossary('zh-cn', { builtInDir: BUILT_IN_DIR, cwd })).toThrow(/terms/); + }); + + it('returns no glossary for a language that has none anywhere', () => { + const result = resolveGlossary('xx-unknown', { builtInDir: BUILT_IN_DIR, cwd }); + + expect(result.glossary).toBeUndefined(); + expect(result.candidates.length).toBeGreaterThan(0); + }); +}); + +describe('loadGlossary reporting', () => { + let cwd: string; + + beforeEach(() => { + cwd = tempDir(); + }); + + afterEach(() => { + fs.rmSync(cwd, { recursive: true, force: true }); + }); + + it('reports the origin, term count and path on success', () => { + const logger = createTestLogger(); + + loadGlossary('zh-cn', { builtInDir: BUILT_IN_DIR, cwd }, logger); + + const info = logger.messages.filter((m) => m.level === 'info'); + expect(info).toHaveLength(1); + expect(info[0].text).toContain('built-in'); + expect(info[0].text).toContain('zh-cn'); + expect(info[0].text).toMatch(/\d+ terms/); + }); + + it('warns loudly, and names every path tried, when nothing is found', () => { + const logger = createTestLogger(); + + const glossary = loadGlossary('xx-unknown', { builtInDir: BUILT_IN_DIR, cwd }, logger); + + expect(glossary).toBeUndefined(); + const warnings = logger.messages.filter((m) => m.level === 'warn'); + expect(warnings).toHaveLength(1); + expect(warnings[0].text).toContain('WITHOUT terminology enforcement'); + expect(warnings[0].text).toContain(path.join(BUILT_IN_DIR, 'xx-unknown.json')); + }); + + it('calls out a missing built-in directory as a wiring bug', () => { + // A caller that forgets to thread builtInGlossaryDir through reintroduces + // the CWD-only lookup — say so rather than reporting a plain miss. + const logger = createTestLogger(); + + loadGlossary('zh-cn', { cwd }, logger); + + const warnings = logger.messages.filter((m) => m.level === 'warn'); + expect(warnings[0].text).toContain('wiring bug'); + }); + + it('says nothing when no logger is supplied', () => { + expect(() => loadGlossary('zh-cn', { builtInDir: BUILT_IN_DIR, cwd })).not.toThrow(); + }); +}); diff --git a/src/cli/commands/forward.ts b/src/cli/commands/forward.ts index b75becf1..09ab37b3 100644 --- a/src/cli/commands/forward.ts +++ b/src/cli/commands/forward.ts @@ -45,6 +45,7 @@ import { verifyPreservedReads, } from '../target-local-reads.js'; import { buildHeadingMap, injectHeadingMap, extractTranslationTitle } from '../../heading-map.js'; +import { loadGlossary } from '../glossary-loader.js'; import { createForwardPR, gitPrepareAndPush, @@ -322,7 +323,7 @@ export async function resyncSingleFile( logger.info(` Content changes detected — resyncing (whole-file)…`); // ──── Step 2: Whole-file RESYNC ────────────────────────────────────────── - const glossary = loadGlossary(options.language); + const glossary = options.glossary ?? resolveForwardGlossary(options, logger); let outputContent: string | undefined; let tokensUsed: number | undefined; @@ -647,6 +648,15 @@ export async function runForwardBulk( } logger.info(''); + // Resolve the glossary once for the whole wave, before any file is translated. + // A malformed or missing --glossary throws here rather than after N files have + // already been resynced against the wrong terminology. + const fileOptions: ForwardOptions = { + ...options, + glossary: options.glossary ?? resolveForwardGlossary(options, logger), + }; + logger.info(''); + // Process each file const results: ForwardFileResult[] = []; const bar = new cliProgress.SingleBar( @@ -671,7 +681,7 @@ export async function runForwardBulk( source, target, docsFolder, - options, + fileOptions, logger, ghRunner, gitRunner @@ -772,26 +782,22 @@ export function printBulkSummary(results: ForwardFileResult[], logger: ForwardLo } // ============================================================================ -// GLOSSARY LOADER +// GLOSSARY // ============================================================================ -function loadGlossary(language: string): Glossary | undefined { - // Try to find glossary file relative to CWD - const candidates = [ - path.join(process.cwd(), 'glossary', `${language}.json`), - path.join(process.cwd(), `glossary-${language}.json`), - ]; - - for (const candidate of candidates) { - if (fs.existsSync(candidate)) { - try { - const raw = fs.readFileSync(candidate, 'utf-8'); - return JSON.parse(raw) as Glossary; - } catch { - // Ignore parse errors - } - } - } - - return undefined; +/** + * Resolve the glossary for a resync run and report the outcome. + * + * Bulk resolves once up front and threads the result through `options.glossary` + * so the load is logged once, not once per file; single-file mode resolves here. + */ +export function resolveForwardGlossary( + options: ForwardOptions, + logger: ForwardLogger +): Glossary | undefined { + return loadGlossary( + options.language, + { customPath: options.glossaryPath, builtInDir: options.builtInGlossaryDir }, + logger + ); } diff --git a/src/cli/commands/init.ts b/src/cli/commands/init.ts index 764d47d5..5ebcfe26 100644 --- a/src/cli/commands/init.ts +++ b/src/cli/commands/init.ts @@ -26,6 +26,7 @@ import { applyTypography } from '../../typography.js'; import { RuleId, buildLocalizationPrompt, getFontRequirements } from '../../localization-rules.js'; import { readFileState, writeConfig, writeFileState } from '../translate-state.js'; import { getFileGitMetadata } from '../git-metadata.js'; +import { loadGlossary } from '../glossary-loader.js'; // ============================================================================ // TYPES @@ -44,6 +45,7 @@ export interface InitOptions { resumeFrom?: string; // Resume from specific lecture file skipExisting?: boolean; // Skip lectures that already have .translate/state entries glossaryPath?: string; // Explicit path to glossary JSON file + builtInGlossaryDir?: string; // Packaged glossary directory (/glossary) localize: RuleId[]; // Active localization rules (default: all) dryRun: boolean; // Preview without API calls or file writes apiKey: string; // Anthropic API key @@ -66,32 +68,6 @@ interface TocEntry { chapters?: TocEntry[]; } -// ============================================================================ -// GLOSSARY LOADER -// ============================================================================ - -function loadGlossary(language: string, glossaryPath?: string): Glossary | undefined { - const candidates = glossaryPath - ? [glossaryPath] - : [ - path.join(process.cwd(), 'glossary', `${language}.json`), - path.join(process.cwd(), `glossary-${language}.json`), - ]; - - for (const candidate of candidates) { - if (fs.existsSync(candidate)) { - try { - const raw = fs.readFileSync(candidate, 'utf-8'); - return JSON.parse(raw) as Glossary; - } catch { - // Ignore parse errors - } - } - } - - return undefined; -} - // ============================================================================ // TOC PARSER // ============================================================================ @@ -408,14 +384,20 @@ export async function runInit(options: InitOptions): Promise { console.log(chalk.gray(`Localize: ${options.localize.join(', ')}`)); } - // Phase 1: Load glossary - const glossary = loadGlossary(options.targetLanguage, options.glossaryPath); - const termCount = glossary?.terms?.length || 0; - if (termCount > 0) { - console.log(chalk.green(`Glossary: ${termCount} terms`)); - } else { - console.log(chalk.yellow(`Glossary: none found for ${options.targetLanguage}`)); - } + // Phase 1: Load glossary — reported either way, so a run without terminology + // enforcement never looks like a run with it (#149). + const glossary = loadGlossary( + options.targetLanguage, + { + customPath: options.glossaryPath, + builtInDir: options.builtInGlossaryDir, + }, + { + info: (msg) => console.log(chalk.green(msg)), + warn: (msg) => console.log(chalk.yellow(`⚠️ ${msg}`)), + } + ); + const termCount = glossary?.terms?.length ?? 0; // Phase 2: Parse TOC for lecture list let lectures = parseTocLectures(options.source, options.docsFolder); diff --git a/src/cli/glossary-loader.ts b/src/cli/glossary-loader.ts new file mode 100644 index 00000000..bbba9656 --- /dev/null +++ b/src/cli/glossary-loader.ts @@ -0,0 +1,170 @@ +/** + * Glossary resolution for the CLI commands. + * + * The built-in glossaries ship inside *this* package — no edition repository + * carries `glossary/.json`. Resolving candidates against `process.cwd()` + * alone therefore meant any run launched from outside an action-translation + * checkout — the target repo, a bench root, anywhere a globally installed CLI + * is naturally invoked — translated with no glossary at all. Nothing was logged + * either way, so a run that dropped terminology enforcement looked identical to + * one that applied it (#149). + * + * Resolution here is package-relative, ordered, reported, and loud on failure: + * a candidate that exists but cannot be parsed is an error, never a silent + * fall-through to a different glossary. + */ + +import * as fs from 'fs'; +import * as path from 'path'; +import { Glossary } from '../types.js'; + +/** Which candidate supplied a resolved glossary. */ +export type GlossaryOrigin = 'custom' | 'repo-local' | 'built-in'; + +export interface GlossaryResolution { + /** The loaded glossary, or undefined when no candidate exists. */ + glossary?: Glossary; + /** Where it came from. */ + origin?: GlossaryOrigin; + /** Absolute path it was read from. */ + path?: string; + /** Every path considered, in order — reported when nothing is found. */ + candidates: string[]; +} + +export interface GlossaryLookupOptions { + /** Explicit `--glossary` path. When set it is the ONLY candidate. */ + customPath?: string; + /** Directory holding the packaged glossaries (`/glossary`). */ + builtInDir?: string; + /** Working directory for repo-local candidates (default: `process.cwd()`). */ + cwd?: string; +} + +/** Minimal logger surface — structurally compatible with the command loggers. */ +export interface GlossaryLogger { + info(message: string): void; + warn(message: string): void; +} + +interface Candidate { + origin: GlossaryOrigin; + file: string; +} + +/** + * Candidate paths in precedence order. + * + * An explicit `--glossary` is exclusive: if the operator named a file, falling + * back to some other glossary would translate against terminology they did not + * ask for. Otherwise a repo-local glossary wins over the packaged one, so a + * project can override the estate defaults by carrying its own. + */ +function candidatesFor(language: string, options: GlossaryLookupOptions): Candidate[] { + if (options.customPath) { + return [{ origin: 'custom', file: path.resolve(options.customPath) }]; + } + + const cwd = options.cwd ?? process.cwd(); + const candidates: Candidate[] = [ + { origin: 'repo-local', file: path.join(cwd, 'glossary', `${language}.json`) }, + { origin: 'repo-local', file: path.join(cwd, `glossary-${language}.json`) }, + ]; + + if (options.builtInDir) { + candidates.push({ + origin: 'built-in', + file: path.join(options.builtInDir, `${language}.json`), + }); + } + + return candidates; +} + +/** + * Read and validate one glossary file. Throws rather than returning undefined — + * a malformed glossary is an operator error, and swallowing it reintroduces the + * silence this module exists to remove. + */ +function readGlossaryFile(file: string): Glossary { + const raw = fs.readFileSync(file, 'utf-8'); + + let parsed: Glossary; + try { + parsed = JSON.parse(raw) as Glossary; + } catch (error) { + throw new Error( + `Glossary at ${file} is not valid JSON: ${error instanceof Error ? error.message : String(error)}` + ); + } + + if (!parsed || !Array.isArray(parsed.terms)) { + throw new Error(`Glossary at ${file} has no "terms" array — it cannot be used.`); + } + + return parsed; +} + +/** + * Resolve the glossary for a language without reporting. + * + * @throws if an explicit `--glossary` path is missing, or if any candidate + * exists but is malformed. + */ +export function resolveGlossary( + language: string, + options: GlossaryLookupOptions = {} +): GlossaryResolution { + const candidates = candidatesFor(language, options); + const tried = candidates.map((c) => c.file); + + for (const candidate of candidates) { + if (!fs.existsSync(candidate.file)) continue; + return { + glossary: readGlossaryFile(candidate.file), + origin: candidate.origin, + path: candidate.file, + candidates: tried, + }; + } + + if (options.customPath) { + throw new Error(`Glossary not found at ${path.resolve(options.customPath)} (--glossary)`); + } + + return { candidates: tried }; +} + +/** + * Resolve the glossary and report the outcome — on success *and* on failure. + * + * A run that translates without terminology enforcement must never look like a + * run that applies it; that indistinguishability is the defect (#149), not just + * the missed lookup. Returns undefined only for a language with no glossary + * anywhere, which is legitimate — the packaged languages always resolve. + */ +export function loadGlossary( + language: string, + options: GlossaryLookupOptions = {}, + logger?: GlossaryLogger +): Glossary | undefined { + const resolution = resolveGlossary(language, options); + + if (resolution.glossary) { + logger?.info( + `✓ Loaded ${resolution.origin} glossary for ${language} — ` + + `${resolution.glossary.terms.length} terms (${resolution.path})` + ); + return resolution.glossary; + } + + const notes = options.builtInDir + ? '' + : ' No built-in glossary directory was supplied to the loader — this is a wiring bug.'; + logger?.warn( + `No glossary found for ${language} — translating WITHOUT terminology enforcement. ` + + `Tried: ${resolution.candidates.join(', ')}.${notes}` + ); + + return undefined; +} diff --git a/src/cli/index.ts b/src/cli/index.ts index a65733b9..f3941695 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -11,6 +11,8 @@ * - setup: Scaffold a new target translation repository */ +import * as path from 'path'; +import { fileURLToPath } from 'url'; import { Command } from 'commander'; import { runBackwardSingleFile, runBackwardBulk } from './commands/backward.js'; import { @@ -47,6 +49,16 @@ import { createRequire } from 'module'; const require = createRequire(import.meta.url); const { version } = require('../../package.json'); +// Packaged glossaries live at /glossary, alongside dist/. Resolve +// them relative to this module, never to process.cwd() — no edition repository +// carries a glossary, so a CWD-relative lookup silently found nothing whenever +// the CLI was run from anywhere but an action-translation checkout (#149). +// This stays in the entry point deliberately: `import.meta.url` cannot be loaded +// by the Jest CJS module registry, so the directory is threaded into the +// commands as an option and the resolution logic lives in a testable module. +const CLI_DIR = path.dirname(fileURLToPath(import.meta.url)); +const BUILT_IN_GLOSSARY_DIR = path.resolve(CLI_DIR, '..', '..', 'glossary'); + const program = new Command(); program @@ -296,6 +308,10 @@ program .option('-m, --model ', 'Claude model to use', DEFAULT_CLAUDE_MODEL) .option('--test', 'Use deterministic mock responses (no LLM calls)', false) .option('--github ', 'Create one PR per file in TARGET repo') + .option( + '--glossary ', + 'Path to glossary JSON file (default: built-in glossary for the language)' + ) .option( '--exclude ', 'Exclude files matching pattern (repeatable, comma-separated)', @@ -324,6 +340,8 @@ program github: opts.github, parallel, apiKey: apiKey || 'test-key', + glossaryPath: opts.glossary, + builtInGlossaryDir: BUILT_IN_GLOSSARY_DIR, }; try { @@ -382,7 +400,10 @@ program .option('-f, --file ', 'Translate a single lecture file (e.g., cobweb.md)') .option('--resume-from ', 'Resume from a specific lecture file (e.g., cobweb.md)') .option('--skip-existing', 'Skip lectures already translated (reads .translate/state)', false) - .option('--glossary ', 'Path to glossary JSON file (default: glossary/.json)') + .option( + '--glossary ', + 'Path to glossary JSON file (default: built-in glossary for the language)' + ) .option( '--localize ', `Localization rules for code cells (use "none" to disable)`, @@ -425,6 +446,7 @@ program resumeFrom: opts.resumeFrom, skipExisting: opts.skipExisting, glossaryPath: opts.glossary, + builtInGlossaryDir: BUILT_IN_GLOSSARY_DIR, localize: localizeRules, dryRun: opts.dryRun, apiKey: apiKey || '', diff --git a/src/cli/types.ts b/src/cli/types.ts index 5499780f..d7aad706 100644 --- a/src/cli/types.ts +++ b/src/cli/types.ts @@ -7,7 +7,7 @@ * - forward: Translate SOURCE changes to TARGET (Phase 3) */ -import { Section } from '../types.js'; +import { Glossary, Section } from '../types.js'; // ============================================================================ // STAGE 1: DOCUMENT-LEVEL TRIAGE @@ -279,6 +279,9 @@ export interface ForwardOptions { github?: string; // TARGET repo in owner/repo format for PR creation apiKey: string; // Anthropic API key parallel?: number; // Number of parallel translations (default: 5) + glossaryPath?: string; // Explicit path to glossary JSON file (--glossary) + builtInGlossaryDir?: string; // Packaged glossary directory (/glossary) + glossary?: Glossary; // Pre-resolved glossary — bulk resolves once and threads it through } // ============================================================================