feat(translation-pipeline): standalone DeepL + LLM + MQM translation package#568
Open
MaxLee-dev wants to merge 78 commits into
Open
feat(translation-pipeline): standalone DeepL + LLM + MQM translation package#568MaxLee-dev wants to merge 78 commits into
MaxLee-dev wants to merge 78 commits into
Conversation
Implements the core i18n translation layer: - deepl.ts: DeepL API client with glossary support and graceful fallback - llm-client.ts: LiteLLM adapter for Sonnet/Opus LLM calls - llm-postprocess.ts: LLM post-processing with MQM error feedback loop - mqm-validator.ts: Binary PASS/FAIL MQM quality validator via LiteLLM - pipeline.ts: Orchestrates DeepL → LLM → MQM per component with cache support - cache.ts: Content-addressable translation cache keyed by text+locale+model+glossary - report.ts: MQM report builder with markdown output (buildReport/renderReport/writeReport) - types.ts: Shared MqmError, MqmResult, and TranslationConfig types Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d CLI - schema.ts: adds TranslationConfig interface with llm/mqm nested config - defaults.ts: adds translation defaults (disabled by default) - models/output.ts: adds locale field to PropsOutput for en/ko locale routing - cli/index.ts: adds --translate flag (enables translation) and --skip-cache flag Translation is opt-in. Run with --translate to enable DeepL+MQM pipeline. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- extract.ts: made async; when translation.enabled, writes en/ subfolder
then calls translatePropsInfo, builds MQM report, writes ko/ output
- stages/write.ts: buildWriteFiles accepts optional locale param;
when set, output paths become outputDir/{locale}/*.json
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…sts total) Adds 43+11=54 new test cases covering: - cache.test.ts: makeCacheKey, loadCache, saveCache (12 tests) - deepl.test.ts: no-key, success, error, glossary (5 tests) - llm-postprocess.test.ts: all 7 paths including MQM error feedback - mqm-validator.test.ts: all 10 paths including malformed JSON - pipeline.test.ts: empty entries, cache hit/miss, MQM PASS/FAIL, failOnError (9 tests) - report.test.ts: buildReport, renderReport, writeReport including error path (11 tests) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ncies - @anthropic-ai/sdk: for LiteLLM-compatible Sonnet/Opus calls in MQM pipeline - deepl-node or fetch-based: DeepL translation API client - p-limit: concurrency limiter (LLM_CONCURRENCY=5) for parallel translation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Documents DEEPL_API_KEY, LITELLM_BASE_URL, LITELLM_MODEL, and DEEPL_GLOSSARY_ID environment variables needed for the i18n pipeline. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ntegration Documents how the JSDoc extraction pipeline now supports --translate flag for automatic DeepL + MQM translation to Korean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…cache - mqm-validator.ts: validate LLM JSON output shape before casting to MqmResult; malformed responses (missing verdict/errors fields) safely return PASS - cache.ts: filter cache entries missing the translated field on load; prevents undefined propagation if cache file is from an older schema version Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…l edge cases pipeline.ts: - Skip LLM post-processing when DeepL falls back to source text (prevents sending English to an LLM asked to edit Korean, which produces garbage) - Clear MQM errors after successful retry so .i18n-report.md reflects the actual final translation, not the pre-retry state - Don't cache fallback entries (source == translation) to prevent DeepL rate-limit failures from permanently poisoning the cache with English text - failCount now counts individual MQM errors per component, not 0/1 boolean (fixes misleading pass rate when a component has multiple failures) mqm-validator.ts: - passResult() factory function instead of shared PASS_RESULT singleton to prevent accidental mutation of the shared errors array report.ts: - Escape bracket characters in e.message to prevent markdown link injection from LLM-generated error messages Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
Warning Review limit reached
More reviews will be available in 9 minutes and 47 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (20)
📝 WalkthroughWalkthroughAdds a complete LLM-driven translation pipeline that translates English API documentation JSON into Korean, validates output quality using MQM evaluation, postprocesses failing translations, caches verified results, generates Markdown quality reports, and provides a CLI. Also updates JSDoc guidance rules, ts-api-extractor output structure, website configuration, and workspace integration. ChangesDocumentation & Tooling Updates
Translation Pipeline Package
Sequence DiagramsequenceDiagram
participant CLI as CLI
participant Parser as parseCliArgs
participant Reader as readEnJsonFiles
participant Cache as Cache Layer
participant Translator as Orchestrator
participant LLMTrans as callLlm (translate)
participant MQMValidator as callLlm (MQM)
participant Postproc as Postprocessor
participant Reporter as Report Gen
participant Writer as File Writer
CLI->>Parser: argv
Parser->>CLI: input, output
CLI->>Reader: read en/*.json
Reader->>CLI: TranslatableDoc[]
CLI->>Cache: loadCache(outputDir)
Cache->>CLI: CacheStore
CLI->>Translator: translatePropsInfo(props)
loop per component / per batch
Translator->>Translator: collectTranslationUnits()
Translator->>LLMTrans: translateComponentUnits(units)
LLMTrans->>CLI: translations Map
Translator->>MQMValidator: validateBatchWithMqm(translated)
MQMValidator->>MQMValidator: callLlm (MQM prompt)
alt PASS verdict
Translator->>Translator: initialPassOutcome
else FAIL verdict
Translator->>Postproc: postprocessBatchWithLlm(failed)
Postproc->>Postproc: callLlm (rewrite prompt)
Translator->>MQMValidator: validateBatchWithMqm(rewritten)
Translator->>Translator: finalOutcome or degraded
end
Translator->>Cache: saveCache(verified)
end
Translator->>Translator: applyTranslationOutcomes()
Translator->>Reporter: buildReport(components)
Reporter->>Reporter: renderReport()
CLI->>Writer: writeKoFiles(merged JSON)
Writer->>CLI: writtenFiles[]
CLI->>Writer: writeReport(report)
Writer->>CLI: reportPath
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.claude/skills/write-component-jsdocs/references/guide.md (1)
427-468:⚠️ Potential issue | 🟠 MajorUpdate the Complete Example to match the new patterns.
The Complete Example section hasn't been updated to follow the new "Wording patterns by component role" and "Wording patterns by prop category" introduced earlier in this PR:
Line 463 — The component summary uses multiple sentences:
"Button component for user interactions. Use for primary actions such as form submission, dialog triggers, and navigation. Renders a <button> element."but the new pattern (line 206) requires one single line. The new top-level component pattern (line 164) specifies:"A [noun phrase] for/that [purpose]."ending with the rendered element.Line 457 — The event handler description reads
"Called when the button is clicked..."but the new event handler pattern (line 301) requires:"Event handler called when [exact condition]."These inconsistencies will confuse developers who read the Complete Example expecting it to demonstrate best practices.
📝 Proposed updates to align with new patterns
/** - * Button component for user interactions. Use for primary actions such as form submission, dialog triggers, and navigation. Renders a `<button>` element. + * A button for triggering actions and submitting forms. Renders a `<button>` element. */ export function Button({ label, variant = 'fill', size = 'md', ...props }: ButtonProps) {/** - * Called when the button is clicked or activated via Enter or Space key. + * Event handler called when the button is clicked or activated via Enter or Space key. */ onClick?: (event: React.MouseEvent<HTMLButtonElement>) => void;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/write-component-jsdocs/references/guide.md around lines 427 - 468, Update the JSDoc in the Complete Example to follow the new single-line component and event wording patterns: change the Button component summary to a single sentence following "A [noun phrase] for/that [purpose]." and end with the rendered element (e.g., "A button for primary user actions that renders a <button> element."), and change the onClick prop JSDoc in ButtonProps to the event handler pattern "Event handler called when [exact condition]." (e.g., "Event handler called when the button is clicked or activated via Enter or Space key."). Ensure these changes are applied to the Button (component summary) and onClick (prop) entries.scripts/ts-api-extractor/src/config/schema.ts (1)
37-96:⚠️ Potential issue | 🟠 MajorValidate or deep-merge the new
translationconfig before use.
translationis now accepted onExtractorConfig, butvalidatePartialConfignever checks its nested shape andmergeConfigreplaces it shallowly. A runtime config like{ translation: { enabled: true } }can pass validation, overwrite defaults, and later crash when code readsconfig.translation.llm.enabledorconfig.translation.validation.mqm.enabled.🛡️ Proposed hardening direction
+function mergeTranslationConfig( + base: ExtractorConfig['translation'], + patch: PartialExtractorConfig['translation'], +): ExtractorConfig['translation'] { + if (patch === undefined) return base; + if (base === undefined) return patch; + + return { + ...base, + ...patch, + llm: { + ...base.llm, + ...patch.llm, + }, + validation: { + ...base.validation, + ...patch.validation, + mqm: { + ...base.validation.mqm, + ...patch.validation?.mqm, + }, + }, + }; +} + export function mergeConfig(base: ExtractorConfig, patch: PartialExtractorConfig): ExtractorConfig { return { ...base, ...patch, exclude: patch.exclude ?? base.exclude, includeHtml: patch.includeHtml ?? base.includeHtml, + translation: mergeTranslationConfig(base.translation, patch.translation), components: { ...base.components, ...(patch.components ?? {}), }, }; }Also add runtime validation for
translation.enabled,translation.targetLocale,translation.llm.enabled, andtranslation.validation.mqm.{enabled,failOnError}whentranslationis present.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ts-api-extractor/src/config/schema.ts` around lines 37 - 96, validatePartialConfig currently skips any checks for config.translation and mergeConfig shallowly replaces translation, allowing invalid or partial shapes to slip through; add runtime validation inside validatePartialConfig for the translation object and its nested keys (check translation.enabled boolean, translation.targetLocale string if present, translation.llm.enabled boolean if llm present, and translation.validation.mqm.enabled and translation.validation.mqm.failOnError booleans when validation/mqm are present) and make mergeConfig perform a deep-merge for the translation key (merge base.translation with patch.translation rather than overwriting) so nested defaults are preserved; update references to validation and merge logic in validatePartialConfig and mergeConfig accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/website/.env.example`:
- Around line 8-11: The dotenv entries are out of dotenv-linter's expected
alphabetical/order and trigger a lint error; reorder the LITELLM_* variables so
LITELLM_API_KEY appears before LITELLM_BASE_URL (keeping LITELLM_MODEL as
needed) in the .env.example so dotenv-linter passes—update the block containing
LITELLM_API_KEY, LITELLM_BASE_URL, and LITELLM_MODEL accordingly.
In `@scripts/ts-api-extractor/src/config/defaults.ts`:
- Around line 24-36: The translation defaults object in defaults.ts (the
translation block with enabled/targetLocale/llm/validation.mqm) is being
shallow-overwritten by mergeConfig(); update mergeConfig() to perform a
deep/recursive merge for the "translation" key (or use a deep-merge util such as
lodash.merge) so partial user config like translation.validation.mqm.failOnError
only overrides that nested field and preserves defaults.enabled, targetLocale
and llm; locate mergeConfig() and change its logic to deep-merge the incoming
config into the defaults (or add a special-case deep merge for the translation
property) to ensure nested defaults remain intact.
In `@scripts/ts-api-extractor/src/extract.ts`:
- Around line 83-95: The code only assigns writtenFiles to the result of writing
the translated (ko) props, dropping the earlier en outputs; update the logic in
the config.translation?.enabled branch so writtenFiles includes both the
original English writePropsFiles(...) call result and the translated
writePropsFiles(...) result (e.g. combine/concat the returned arrays/counts from
the first writePropsFiles(props, outputDir, formatFileName, 'en') and the later
writePropsFiles(translatedProps, outputDir, formatFileName, 'ko')), leaving
translatePropsInfo and buildReport usage unchanged.
In `@scripts/ts-api-extractor/src/translate/deepl.ts`:
- Around line 8-11: The current DeepL gate returns the original texts when no
API key or on failure, making outages indistinguishable from legitimate
translations; instead, change those early-return paths (the branch checking
apiKey and the other failure branches around lines 30-42) to return undefined
(or null) so the pipeline's deeplResult === undefined check can detect a DeepL
fallback; update any console.warn/error messages to note "falling back (no
DeepL)" and ensure the function that returns texts uses the same undefined
signal across all failure branches.
- Around line 20-28: The DeepL fetch call in the try block should use an
AbortController with a bounded timeout to avoid hanging; create an
AbortController, start a timer (e.g., setTimeout) that calls controller.abort()
after the desired ms, pass controller.signal into the fetch options (in the
request started in this try), and ensure the timeout is cleared in a finally
block (wrap clearTimeout(timeout) inside finally so the timer is always
released) around the fetch/response handling code in deepl.ts.
In `@scripts/ts-api-extractor/src/translate/llm-client.ts`:
- Around line 34-41: The code currently casts response.json() to a shape and
returns data.choices?.[0]?.message?.content without validating its runtime type;
update the extraction around the data and content variables so you check that
data.choices?.[0]?.message?.content is a string (e.g., typeof content ===
'string') before returning it, and if it's missing or not a string return {
content: null, error: 'Unexpected response shape' } instead; adjust the block
that computes content and the subsequent return in llm-client.ts (the
variables/data referenced as data and content) accordingly.
- Around line 12-28: Validate both LITELLM_BASE_URL and LITELLM_API_KEY before
calling fetch (return an error if either is missing rather than sending
Authorization: Bearer undefined), and wrap the fetch call in an AbortController
with a timeout: create an AbortController, pass controller.signal to fetch, set
a timer to call controller.abort() after a configurable timeout, and ensure you
clearTimeout(timeout) in a finally block; update the code around the existing
fetch invocation (the POST to `${baseUrl}/chat/completions` that sends model and
messages) to use the controller.signal and abort/cleanup logic.
In `@scripts/ts-api-extractor/src/translate/llm-postprocess.ts`:
- Around line 35-44: The current post-processing returns result.content after
stripping code fences but if that yields an empty string it mistakenly replaces
a valid deeplDraft; modify the logic in the LLM postprocess block (the code
handling result.content and deeplDraft) to assign the cleaned string to a
variable (e.g., cleaned) after the .replace(...).trim() sequence and then return
cleaned || deeplDraft so that an empty or whitespace-only LLM response falls
back to deeplDraft.
In `@scripts/ts-api-extractor/src/translate/mqm-validator.ts`:
- Around line 53-62: The parsed payload validation in mqm-validator.ts is too
permissive: update the check that currently returns passResult() to also enforce
that (parsed as MqmResult).verdict is exactly "PASS" or "FAIL" and that (parsed
as MqmResult).errors is an array whose entries match the expected error shape
(e.g., each entry is an object with required properties like "message" and
"severity" or whatever fields MqmResult defines); if the verdict is not one of
the two allowed values or any error entry is malformed, log a warning and return
passResult() to avoid trusting malformed payloads. Ensure you reference the
existing symbols parsed, MqmResult, and passResult() when implementing the
stricter validation so downstream logic never receives arbitrary strings like
"OK".
In `@scripts/ts-api-extractor/src/translate/pipeline.ts`:
- Around line 100-116: translateWithDeepl can return fallback originals which
must not be treated as valid DeepL drafts; update translateWithDeepl to return
metadata per item (e.g., { text, usedFallback }) and then in pipeline.ts (the
block using translateWithDeepl, missIndices, deeplResults, postprocessWithLlm
and limit) change the check from deeplResult === undefined to check
deeplResult.usedFallback (or similar) and when usedFallback is true log a
warning and return the source entries[entryIndex].text without calling
postprocessWithLlm; ensure all callers of translateWithDeepl are updated to
handle the new result shape.
- Around line 132-158: When an MQM FAIL occurs the code currently never records
the failure into mqmErrorsByEntryIdx and the retry path can delete errors
incorrectly; update the flow in the block handling mqmResult.verdict === 'FAIL'
so that you (1) record the initial failure via
mqmErrorsByEntryIdx.set(entryIndex, mqmResult.errors), (2) when you run the
retry via recheck = await validateWithMqm(...) replace the stored errors with
recheck.errors if recheck.verdict === 'FAIL', or delete mqmErrorsByEntryIdx
entry only if recheck.verdict !== 'FAIL' (i.e. retry passed), and (3) in the
failOnError true branch before throwing ensure mqmErrorsByEntryIdx contains the
final recheck.errors so the report reflects the ultimate outcome; use the
existing symbols mqmErrorsByEntryIdx, entryIndex, mqmResult, recheck,
validateWithMqm and config.validation.mqm.failOnError to locate and implement
these changes.
In `@scripts/ts-api-extractor/src/translate/report.ts`:
- Around line 21-27: buildReport currently uses failCount computed from MQM
error arrays (errors.length) which overcounts failures because one text can
yield multiple MqmError entries; update buildReport (and any aggregation logic
that sets failCount in TranslationReport) to either (A) compute a distinct
failed-text count by de-duplicating failures per text id (e.g., mark a
component's failedTextCount as count of unique text identifiers with errors and
sum those into failCount) or (B) rename/keep failCount as mqmErrorCount and add
a separate failedTexts field used for pass-rate math; adjust totalTexts,
pass-rate calculation, and FAIL (${failCount}/${totalTexts}) rendering to use
the new failedTexts metric instead of raw MQM error counts (reference
buildReport, failCount, totalTexts, components, and ComponentReport where errors
are stored).
- Around line 33-41: The loop that builds Markdown lines in translate/report.ts
currently only escapes backticks/brackets for source, translation and message,
which allows newlines and other Markdown control characters to inject headings
or list items; update the c.errors iteration (where source, translation, message
are computed and used in lines.push) to run all three values through a sanitizer
(e.g., sanitizeMarkdown) that 1) strips or replaces newlines with spaces, and 2)
escapes common Markdown/control chars (#, -, *, _, >, `, [, ], (, ), +, and
leading/trailing whitespace) so the final template string in lines.push cannot
create headings or bullets. Implement the sanitizer as a small helper and call
it for e.source, e.translation and e.message before formatting the output.
In `@scripts/ts-api-extractor/test/translate/cache.test.ts`:
- Around line 116-120: The test "does not throw when outputDir is unwritable
(logs warning instead)" is non-deterministic; instead create a real temporary
file (e.g., via fs.writeFileSync on a unique tmp path) and pass that file path
into saveCache so the subsequent mkdirSync(dirname(...)) in saveCache will
reliably fail; assert saveCache does not throw, that console.warn was called,
and then remove the temp file in cleanup. Reference: saveCache and the
mkdirSync(dirname(...)) behavior to locate where to trigger the failure.
In `@scripts/ts-api-extractor/test/translate/pipeline.test.ts`:
- Around line 121-154: The test currently only checks that a warning was emitted
and a result exists but does not assert the MQM report contents; update the test
for the FAIL path (the test using translatePropsInfo, sampleProps, baseConfig
and mqmModule.mockResolvedValue) to assert the generated componentReports
contain the MQM failure details by verifying
result.componentReports[0].failCount is > 0 (or equals 1) and that
result.componentReports[0].errors includes the mocked error (e.g. an entry with
severity 'major' and type 'mistranslation'); do the same for the other similar
tests mentioned (lines ~192-231) so the spec verifies the report data consumed
by .i18n-report.md rather than only the warning/result presence.
In `@scripts/ts-api-extractor/test/translate/report.test.ts`:
- Around line 147-158: The test using chmod to make readonlyDir non-writable is
flaky; instead make the write failure deterministic by creating a file where the
directory is expected so mkdirSync(dirname(filePath)) fails: in the 'warns and
does not throw when write fails' test, replace creating a directory with mode
0o444 with creating a regular file at readonlyDir (or creating a file at
path.join(tmpDir, 'readonly', 'placeholder') so the parent mkdir will fail),
call buildReport and pass that path into writeReport as before, assert the same
warnSpy behavior, then restore the spy and remove the file to clean up;
reference functions/variables writeReport, buildReport, readonlyDir, tmpDir,
warnSpy, and the mkdirSync(dirname(filePath)) failure point when making the
change.
---
Outside diff comments:
In @.claude/skills/write-component-jsdocs/references/guide.md:
- Around line 427-468: Update the JSDoc in the Complete Example to follow the
new single-line component and event wording patterns: change the Button
component summary to a single sentence following "A [noun phrase] for/that
[purpose]." and end with the rendered element (e.g., "A button for primary user
actions that renders a <button> element."), and change the onClick prop JSDoc in
ButtonProps to the event handler pattern "Event handler called when [exact
condition]." (e.g., "Event handler called when the button is clicked or
activated via Enter or Space key."). Ensure these changes are applied to the
Button (component summary) and onClick (prop) entries.
In `@scripts/ts-api-extractor/src/config/schema.ts`:
- Around line 37-96: validatePartialConfig currently skips any checks for
config.translation and mergeConfig shallowly replaces translation, allowing
invalid or partial shapes to slip through; add runtime validation inside
validatePartialConfig for the translation object and its nested keys (check
translation.enabled boolean, translation.targetLocale string if present,
translation.llm.enabled boolean if llm present, and
translation.validation.mqm.enabled and translation.validation.mqm.failOnError
booleans when validation/mqm are present) and make mergeConfig perform a
deep-merge for the translation key (merge base.translation with
patch.translation rather than overwriting) so nested defaults are preserved;
update references to validation and merge logic in validatePartialConfig and
mergeConfig accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3a6fbd0c-3cca-475d-b9de-1d57cbe1fea2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (24)
.claude/skills/write-component-jsdocs/SKILL.md.claude/skills/write-component-jsdocs/references/guide.mdapps/website/.env.examplescripts/ts-api-extractor/package.jsonscripts/ts-api-extractor/src/cli/index.tsscripts/ts-api-extractor/src/config/defaults.tsscripts/ts-api-extractor/src/config/schema.tsscripts/ts-api-extractor/src/extract.tsscripts/ts-api-extractor/src/models/output.tsscripts/ts-api-extractor/src/stages/write.tsscripts/ts-api-extractor/src/translate/cache.tsscripts/ts-api-extractor/src/translate/deepl.tsscripts/ts-api-extractor/src/translate/llm-client.tsscripts/ts-api-extractor/src/translate/llm-postprocess.tsscripts/ts-api-extractor/src/translate/mqm-validator.tsscripts/ts-api-extractor/src/translate/pipeline.tsscripts/ts-api-extractor/src/translate/report.tsscripts/ts-api-extractor/src/translate/types.tsscripts/ts-api-extractor/test/translate/cache.test.tsscripts/ts-api-extractor/test/translate/deepl.test.tsscripts/ts-api-extractor/test/translate/llm-postprocess.test.tsscripts/ts-api-extractor/test/translate/mqm-validator.test.tsscripts/ts-api-extractor/test/translate/pipeline.test.tsscripts/ts-api-extractor/test/translate/report.test.ts
💤 Files with no reviewable changes (1)
- scripts/ts-api-extractor/src/models/output.ts
…ing with nested properties
…pipeline - extract.ts: include both en and ko writtenFiles in translate mode - deepl.ts: return undefined on failure (signal fallback), add 10s AbortController timeout - llm-client.ts: fail fast on missing LITELLM_API_KEY, add 30s timeout, strict typeof content check - llm-postprocess.ts: fallback to deeplDraft when cleaned LLM output is empty - mqm-validator.ts: enforce verdict === 'PASS'|'FAIL' and validate error entry shape - pipeline.ts: record MQM failures into mqmErrorsByEntryIdx before retry; clear only on retry success; count failed texts (not error entries) for failCount - report.ts: sanitize Markdown special chars and newlines before injecting into report - tests: update deepl expectations to undefined, make cache/report write-failure tests deterministic, assert componentReports MQM content Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…erties more robustly
…pl-translation-layer
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/ts-api-extractor/src/translate/pipeline.ts (1)
152-165:⚠️ Potential issue | 🟡 MinorReport accuracy when
failOnErroris false.When
failOnErroris false, the retry runs (lines 144-150) but no recheck validates whether the retry succeeded. The errors recorded at line 142 persist in the report regardless of whether the retry actually fixed the translation.Consider running the recheck unconditionally and only throwing when
failOnError && recheck.verdict === 'FAIL'. This ensures the report accurately reflects the final translation quality.🔧 Proposed fix
translated = await limit(() => postprocessWithLlm( entry.text, deeplDraft ?? entry.text, mqmResult.errors, ), ); - if (config.validation.mqm.failOnError) { - const recheck = await limit(() => - validateWithMqm(entry.text, translated, config), - ); - if (recheck.verdict === 'FAIL') { + const recheck = await limit(() => + validateWithMqm(entry.text, translated, config), + ); + + if (recheck.verdict === 'FAIL') { + mqmErrorsByEntryIdx.set(entryIndex, recheck.errors); + if (config.validation.mqm.failOnError) { mqmErrorsByEntryIdx.set(entryIndex, recheck.errors); throw new Error( `[mqm-validator] Translation validation FAILED after retry for: "${entry.text.slice(0, 60)}..."`, ); - } else { - mqmErrorsByEntryIdx.delete(entryIndex); } + } else { + mqmErrorsByEntryIdx.delete(entryIndex); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ts-api-extractor/src/translate/pipeline.ts` around lines 152 - 165, When a retry is performed, always run the recheck via validateWithMqm (using the same limit(...) call) and update mqmErrorsByEntryIdx based on recheck.verdict; only throw an Error when config.validation.mqm.failOnError is true AND recheck.verdict === 'FAIL'. In other words, call validateWithMqm unconditionally for the retry, set mqmErrorsByEntryIdx.set(entryIndex, recheck.errors) if recheck.verdict === 'FAIL' else mqmErrorsByEntryIdx.delete(entryIndex), and only throw inside the block when config.validation.mqm.failOnError && recheck.verdict === 'FAIL' (references: validateWithMqm, mqmErrorsByEntryIdx, config.validation.mqm.failOnError, limit).
🧹 Nitpick comments (3)
scripts/ts-api-extractor/test/translate/deepl.test.ts (2)
10-10: Remove unusedDEEPL_GLOSSARY_IDenv stubs in this suite.Line 10 and Line 37 stub an env var that
translateWithDeepldoes not read (it uses theglossaryIdargument). Keeping these stubs can mislead future readers about behavior under test.Diff suggestion
beforeEach(() => { vi.stubGlobal('fetch', vi.fn()); vi.stubEnv('DEEPL_ENDPOINT', 'https://api-free.deepl.com/v2/translate'); vi.stubEnv('DEEPL_API_KEY', 'test-api-key'); - vi.stubEnv('DEEPL_GLOSSARY_ID', ''); }); @@ it('glossary_id 미설정 시 body에 glossary_id 없음', async () => { - vi.stubEnv('DEEPL_GLOSSARY_ID', ''); - const mockFetch = vi.mocked(fetch);Also applies to: 37-37
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ts-api-extractor/test/translate/deepl.test.ts` at line 10, The test suite stubs DEEPL_GLOSSARY_ID even though translateWithDeepl reads the glossaryId argument (not that env var); remove the unnecessary vi.stubEnv('DEEPL_GLOSSARY_ID', '') calls from this test file (the occurrences near the top of the suite and the one around line 37) so tests no longer mock an unused environment variable and avoid misleading readers—leave other stubs intact and ensure translateWithDeepl invocation still passes the intended glossaryId argument.
81-92: Add an explicit non-OK HTTP response test for the fallback branch.You already test rejected fetch; adding a
response.ok === falsecase will directly lock coverage for the status-based fallback path indeepl.ts.Diff suggestion
it('fetch 실패 시 undefined 반환 + warn', async () => { const mockFetch = vi.mocked(fetch); mockFetch.mockRejectedValueOnce(new Error('Network error')); const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); const texts = ['Hello']; const result = await translateWithDeepl(texts, ''); expect(result).toBeUndefined(); expect(warnSpy).toHaveBeenCalledOnce(); }); + + it('HTTP non-ok 응답 시 undefined 반환 + warn', async () => { + const mockFetch = vi.mocked(fetch); + mockFetch.mockResolvedValueOnce({ + ok: false, + status: 429, + json: async () => ({}), + } as Response); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + const result = await translateWithDeepl(['Hello'], ''); + + expect(result).toBeUndefined(); + expect(warnSpy).toHaveBeenCalledOnce(); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ts-api-extractor/test/translate/deepl.test.ts` around lines 81 - 92, Add a test in deepl.test.ts that covers the non-OK HTTP response fallback: mock the global fetch used by translateWithDeepl so it resolves to a response-like object with ok: false (and a status/code/message), spy on console.warn, call translateWithDeepl(['Hello'], ''), and assert the function returns undefined and console.warn was called; this will exercise the status-based branch in deepl.ts instead of the rejected-fetch branch already covered.scripts/ts-api-extractor/src/translate/llm-client.ts (1)
26-26: Normalize trailing slash frombaseUrl.If
LITELLM_BASE_URLends with a trailing slash (e.g.,https://api.example.com/), the constructed URL becomeshttps://api.example.com//chat/completionswith a double slash. While most servers handle this gracefully, normalizing prevents potential routing issues.🔧 Proposed fix
- response = await fetch(`${baseUrl}/chat/completions`, { + response = await fetch(`${baseUrl.replace(/\/+$/, '')}/chat/completions`, {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ts-api-extractor/src/translate/llm-client.ts` at line 26, The fetch URL can end up with a double slash when LITELLM_BASE_URL has a trailing slash; normalize baseUrl before using it in the fetch call by trimming any trailing slashes (e.g., compute baseUrl = LITELLM_BASE_URL.replace(/\/+$/, "") or similar) and then use that normalized baseUrl in the existing fetch(`${baseUrl}/chat/completions`, ...) call (update the code around the fetch/response logic in llm-client.ts to use the trimmed baseUrl variable).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/ts-api-extractor/src/translate/pipeline.ts`:
- Around line 152-165: When a retry is performed, always run the recheck via
validateWithMqm (using the same limit(...) call) and update mqmErrorsByEntryIdx
based on recheck.verdict; only throw an Error when
config.validation.mqm.failOnError is true AND recheck.verdict === 'FAIL'. In
other words, call validateWithMqm unconditionally for the retry, set
mqmErrorsByEntryIdx.set(entryIndex, recheck.errors) if recheck.verdict ===
'FAIL' else mqmErrorsByEntryIdx.delete(entryIndex), and only throw inside the
block when config.validation.mqm.failOnError && recheck.verdict === 'FAIL'
(references: validateWithMqm, mqmErrorsByEntryIdx,
config.validation.mqm.failOnError, limit).
---
Nitpick comments:
In `@scripts/ts-api-extractor/src/translate/llm-client.ts`:
- Line 26: The fetch URL can end up with a double slash when LITELLM_BASE_URL
has a trailing slash; normalize baseUrl before using it in the fetch call by
trimming any trailing slashes (e.g., compute baseUrl =
LITELLM_BASE_URL.replace(/\/+$/, "") or similar) and then use that normalized
baseUrl in the existing fetch(`${baseUrl}/chat/completions`, ...) call (update
the code around the fetch/response logic in llm-client.ts to use the trimmed
baseUrl variable).
In `@scripts/ts-api-extractor/test/translate/deepl.test.ts`:
- Line 10: The test suite stubs DEEPL_GLOSSARY_ID even though translateWithDeepl
reads the glossaryId argument (not that env var); remove the unnecessary
vi.stubEnv('DEEPL_GLOSSARY_ID', '') calls from this test file (the occurrences
near the top of the suite and the one around line 37) so tests no longer mock an
unused environment variable and avoid misleading readers—leave other stubs
intact and ensure translateWithDeepl invocation still passes the intended
glossaryId argument.
- Around line 81-92: Add a test in deepl.test.ts that covers the non-OK HTTP
response fallback: mock the global fetch used by translateWithDeepl so it
resolves to a response-like object with ok: false (and a status/code/message),
spy on console.warn, call translateWithDeepl(['Hello'], ''), and assert the
function returns undefined and console.warn was called; this will exercise the
status-based branch in deepl.ts instead of the rejected-fetch branch already
covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e3ab9178-e82b-492c-8244-c47250744428
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (16)
.claude/skills/write-component-jsdocs/SKILL.mdscripts/ts-api-extractor/src/config/schema.tsscripts/ts-api-extractor/src/extract.tsscripts/ts-api-extractor/src/translate/cache.tsscripts/ts-api-extractor/src/translate/deepl.tsscripts/ts-api-extractor/src/translate/llm-client.tsscripts/ts-api-extractor/src/translate/llm-postprocess.tsscripts/ts-api-extractor/src/translate/mqm-validator.tsscripts/ts-api-extractor/src/translate/pipeline.tsscripts/ts-api-extractor/src/translate/report.tsscripts/ts-api-extractor/test/translate/cache.test.tsscripts/ts-api-extractor/test/translate/deepl.test.tsscripts/ts-api-extractor/test/translate/llm-postprocess.test.tsscripts/ts-api-extractor/test/translate/mqm-validator.test.tsscripts/ts-api-extractor/test/translate/pipeline.test.tsscripts/ts-api-extractor/test/translate/report.test.ts
✅ Files skipped from review due to trivial changes (3)
- .claude/skills/write-component-jsdocs/SKILL.md
- scripts/ts-api-extractor/src/translate/cache.ts
- scripts/ts-api-extractor/src/translate/report.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- scripts/ts-api-extractor/src/translate/deepl.ts
- scripts/ts-api-extractor/test/translate/pipeline.test.ts
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/ts-api-extractor/src/config/schema.ts (1)
47-94:⚠️ Potential issue | 🟠 Major
validatePartialConfigshould validatetranslationpayload too.
translationwas added to config shape, but runtime validation still skips it. Invalid values (e.g., non-boolean flags or unsupported locale) currently pass through and get merged.🛠️ Proposed fix
export function validatePartialConfig(config: PartialExtractorConfig): void { @@ if (config.verbose !== undefined && typeof config.verbose !== 'boolean') { throw new Error('Invalid verbose: expected boolean'); } + + if (config.translation !== undefined) { + if (typeof config.translation !== 'object' || config.translation === null) { + throw new Error('Invalid translation: expected object'); + } + const t = config.translation; + if (t.enabled !== undefined && typeof t.enabled !== 'boolean') { + throw new Error('Invalid translation.enabled: expected boolean'); + } + if (t.targetLocale !== undefined && t.targetLocale !== 'ko') { + throw new Error("Invalid translation.targetLocale: expected 'ko'"); + } + if (t.llm !== undefined) { + if (typeof t.llm !== 'object' || t.llm === null) { + throw new Error('Invalid translation.llm: expected object'); + } + if (t.llm.enabled !== undefined && typeof t.llm.enabled !== 'boolean') { + throw new Error('Invalid translation.llm.enabled: expected boolean'); + } + } + if (t.validation !== undefined) { + if (typeof t.validation !== 'object' || t.validation === null) { + throw new Error('Invalid translation.validation: expected object'); + } + if (t.validation.mqm !== undefined) { + if (typeof t.validation.mqm !== 'object' || t.validation.mqm === null) { + throw new Error('Invalid translation.validation.mqm: expected object'); + } + if ( + t.validation.mqm.enabled !== undefined && + typeof t.validation.mqm.enabled !== 'boolean' + ) { + throw new Error('Invalid translation.validation.mqm.enabled: expected boolean'); + } + if ( + t.validation.mqm.failOnError !== undefined && + typeof t.validation.mqm.failOnError !== 'boolean' + ) { + throw new Error( + 'Invalid translation.validation.mqm.failOnError: expected boolean', + ); + } + } + } + } if (config.components !== undefined) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ts-api-extractor/src/config/schema.ts` around lines 47 - 94, validatePartialConfig currently skips the new translation property; add validation inside validatePartialConfig to ensure config.translation (when defined) is an object (not null), validate any boolean flags on it using typeof checks (e.g., translation.enabled, translation.optionalFlag) and validate locale fields as strings or arrays of strings using the existing assertStringArray helper; also verify locale values against the known supported locales list (reference the project’s supported locales constant or enum) and throw descriptive Errors (e.g., "Invalid translation: expected object", "Invalid translation.enabled: expected boolean", "Invalid translation.locales: unsupported locale") so invalid translation payloads are rejected before merge.
♻️ Duplicate comments (1)
scripts/ts-api-extractor/src/translate/report.ts (1)
40-40:⚠️ Potential issue | 🟡 MinorSanitize component names before injecting into markdown headings.
c.nameis currently rendered raw in a heading; special markdown characters can distort the report layout.🛠️ Proposed fix
- const lines = [`### ${c.name} — ${status}`]; + const lines = [`### ${sanitizeMarkdown(c.name)} — ${status}`];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ts-api-extractor/src/translate/report.ts` at line 40, The heading injection uses c.name raw (const lines = [`### ${c.name} — ${status}`]) which can break Markdown; implement and call a sanitizer (e.g., escapeMarkdown or markdownEscape) to escape Markdown-special characters in c.name before building lines, or wrap the name in a safe code/span (e.g., inline code) so the generated heading becomes `### ${escapeMarkdown(c.name)} — ${status}`; update the function that builds the report lines to use this helper wherever c.name is interpolated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/ts-api-extractor/.env.example`:
- Around line 12-13: The environment variable entries are out-of-order for the
dotenv-linter rule: swap the two lines so LITELLM_API_KEY appears before
LITELLM_BASE_URL in the .env.example file; update the entries for the symbols
LITELLM_API_KEY and LITELLM_BASE_URL accordingly so the API key line comes
first.
In `@scripts/ts-api-extractor/src/translate/mqm-validator.ts`:
- Around line 73-85: The current guard only checks explanation and severity,
allowing malformed MqmError entries to slip through; update the validator that
returns MqmResult to perform a strict type guard over errors[] (the MqmError
shape) by verifying each entry has non-null object fields source_span and
mt_span (with their expected subfields), a category value matching the MqmError
category enum, and severity/explanation as strings; if any entry fails, log the
unexpected shape and return passResult() instead of casting, and ensure the
function that returns parsed as MqmResult uses this stronger predicate before
the cast.
In `@scripts/ts-api-extractor/src/translate/pipeline.ts`:
- Around line 220-232: The PASS/FAIL decision should use mqmResult.verdict
instead of mqmResult.errors.length; update the control in the mt-only branch to
check mqmResult.verdict (e.g., mqmResult.verdict === 'PASS') so a FAIL verdict
with an empty/partial errors array doesn't get treated as PASS, and ensure the
returned object fields (pipeline, hadErrors, hadOverEdit) are set consistently
when verdict !== 'PASS' (and continue to populate mqmErrorsByEntryIdx and
allowedEditSpans only for non-PASS cases using mqmResult.errors).
---
Outside diff comments:
In `@scripts/ts-api-extractor/src/config/schema.ts`:
- Around line 47-94: validatePartialConfig currently skips the new translation
property; add validation inside validatePartialConfig to ensure
config.translation (when defined) is an object (not null), validate any boolean
flags on it using typeof checks (e.g., translation.enabled,
translation.optionalFlag) and validate locale fields as strings or arrays of
strings using the existing assertStringArray helper; also verify locale values
against the known supported locales list (reference the project’s supported
locales constant or enum) and throw descriptive Errors (e.g., "Invalid
translation: expected object", "Invalid translation.enabled: expected boolean",
"Invalid translation.locales: unsupported locale") so invalid translation
payloads are rejected before merge.
---
Duplicate comments:
In `@scripts/ts-api-extractor/src/translate/report.ts`:
- Line 40: The heading injection uses c.name raw (const lines = [`### ${c.name}
— ${status}`]) which can break Markdown; implement and call a sanitizer (e.g.,
escapeMarkdown or markdownEscape) to escape Markdown-special characters in
c.name before building lines, or wrap the name in a safe code/span (e.g., inline
code) so the generated heading becomes `### ${escapeMarkdown(c.name)} —
${status}`; update the function that builds the report lines to use this helper
wherever c.name is interpolated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 778f6632-b235-4958-a327-90f61210067c
📒 Files selected for processing (13)
scripts/ts-api-extractor/.env.examplescripts/ts-api-extractor/src/config/schema.tsscripts/ts-api-extractor/src/translate/cache.tsscripts/ts-api-extractor/src/translate/llm-postprocess.tsscripts/ts-api-extractor/src/translate/mqm-validator.tsscripts/ts-api-extractor/src/translate/pipeline.tsscripts/ts-api-extractor/src/translate/report.tsscripts/ts-api-extractor/src/translate/types.tsscripts/ts-api-extractor/test/translate/cache.test.tsscripts/ts-api-extractor/test/translate/llm-postprocess.test.tsscripts/ts-api-extractor/test/translate/mqm-validator.test.tsscripts/ts-api-extractor/test/translate/pipeline.test.tsscripts/ts-api-extractor/test/translate/report.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- scripts/ts-api-extractor/src/translate/llm-postprocess.ts
- scripts/ts-api-extractor/test/translate/cache.test.ts
- scripts/ts-api-extractor/test/translate/report.test.ts
- scripts/ts-api-extractor/src/translate/cache.ts
- scripts/ts-api-extractor/test/translate/mqm-validator.test.ts
MaxLee-dev
commented
May 11, 2026
|
|
||
| # generated content | ||
| .contentlayer | ||
| **/.i18n-report.md |
Contributor
Author
There was a problem hiding this comment.
This is a Markdown file that generates a report once verification is complete. As it changes every time it is run and is intended for developers’ reference only, I have marked it as ignore
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
scripts/translation-pipeline/src/cli/run.ts (1)
84-108: ⚡ Quick winRead the input set once and keep file identity attached.
This currently parses the same directory twice and later zips
rawDocstoresult.propsby position. That stays correct only while both scans and the pipeline output preserve identical order; once that assumption breaks, the CLI can write the wrong translation into the wrong file without noticing. Returning{ fileName, raw, doc }from a single reader would remove both the duplicate I/O and the positional coupling.Also applies to: 151-160, 217-224
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/translation-pipeline/src/cli/run.ts` around lines 84 - 108, The reader should scan and parse the input directory only once and return per-file records that keep file identity together (instead of returning just TranslatableDoc[]), so modify readEnJsonFiles to return an array of objects like { fileName, raw, doc } produced by calling normalizeDoc(raw, fileName) immediately after parsing; update any downstream uses that currently rely on rawDocs and result.props zipping by index (references: readEnJsonFiles, normalizeDoc, rawDocs, result.props) to consume the preserved fileName on each record when writing outputs, and remove the duplicate directory re-reads elsewhere (also update the other reader usages noted around the alternate code paths) so ordering no longer must be relied upon.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/translation-pipeline/src/cli/index.ts`:
- Around line 1-9: Currently the try/catch around
process.loadEnvFile(path.resolve(process.cwd(), '.env')) swallows all errors;
change this to first check for file existence (use fs.existsSync or fs/promises
access) on the resolved '.env' path and only call process.loadEnvFile when the
file exists so that parse/read errors from process.loadEnvFile (not missing-file
cases) are allowed to surface; keep the same path.resolve(process.cwd(), '.env')
resolution and remove the broad try/catch so real failures in
process.loadEnvFile are not masked.
In `@scripts/translation-pipeline/src/translator/translator.ts`:
- Around line 66-69: The lookup for translated text currently falls back to
outcomes.get(unit.id), which can leak translations across components; change the
resolution in the loop over units to remove the outcomes.get(unit.id) fallback
so you only use outcomes.get(getTranslationUnitKey(unit)) and default to
unit.source if that returns undefined (i.e., update the translated assignment in
translator.ts to drop the unit.id lookup and rely solely on
getTranslationUnitKey(unit) plus unit.source).
In `@scripts/translation-pipeline/tsconfig.json`:
- Line 7: The tsconfig's compilerOptions.types currently lists only
"vitest/globals" which prevents Node API typings from being available; update
the "types" array in tsconfig.json (the "types" entry shown) to include "node"
alongside "vitest/globals" so Node globals and APIs (fs, path, os, crypto,
process) are correctly type-checked.
---
Nitpick comments:
In `@scripts/translation-pipeline/src/cli/run.ts`:
- Around line 84-108: The reader should scan and parse the input directory only
once and return per-file records that keep file identity together (instead of
returning just TranslatableDoc[]), so modify readEnJsonFiles to return an array
of objects like { fileName, raw, doc } produced by calling normalizeDoc(raw,
fileName) immediately after parsing; update any downstream uses that currently
rely on rawDocs and result.props zipping by index (references: readEnJsonFiles,
normalizeDoc, rawDocs, result.props) to consume the preserved fileName on each
record when writing outputs, and remove the duplicate directory re-reads
elsewhere (also update the other reader usages noted around the alternate code
paths) so ordering no longer must be relied upon.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b00616f8-db86-475c-8cdc-842b3c7fe6d1
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (33)
apps/website/.env.exampleapps/website/docs-extractor.config.mjsapps/website/package.jsonpnpm-workspace.yamlscripts/translation-pipeline/eslint.config.mjsscripts/translation-pipeline/package.jsonscripts/translation-pipeline/src/cache/cache.test.tsscripts/translation-pipeline/src/cache/cache.tsscripts/translation-pipeline/src/cli/index.tsscripts/translation-pipeline/src/cli/run.test.tsscripts/translation-pipeline/src/cli/run.tsscripts/translation-pipeline/src/defaults.tsscripts/translation-pipeline/src/postprocess/postprocess.test.tsscripts/translation-pipeline/src/postprocess/postprocess.tsscripts/translation-pipeline/src/report/report.test.tsscripts/translation-pipeline/src/report/report.tsscripts/translation-pipeline/src/translation/client.test.tsscripts/translation-pipeline/src/translation/client.tsscripts/translation-pipeline/src/translation/json.tsscripts/translation-pipeline/src/translation/translate.test.tsscripts/translation-pipeline/src/translation/translate.tsscripts/translation-pipeline/src/translator/batch-lifecycle.tsscripts/translation-pipeline/src/translator/translator.test.tsscripts/translation-pipeline/src/translator/translator.tsscripts/translation-pipeline/src/types.tsscripts/translation-pipeline/src/validation/validator.test.tsscripts/translation-pipeline/src/validation/validator.tsscripts/translation-pipeline/tests/cli.test.tsscripts/translation-pipeline/tsconfig.jsonscripts/translation-pipeline/tsup.config.tsscripts/translation-pipeline/vitest.config.tsscripts/ts-api-extractor/package.jsonscripts/ts-api-extractor/src/extract.ts
💤 Files with no reviewable changes (1)
- apps/website/docs-extractor.config.mjs
✅ Files skipped from review due to trivial changes (8)
- scripts/translation-pipeline/src/defaults.ts
- scripts/translation-pipeline/tsup.config.ts
- scripts/translation-pipeline/vitest.config.ts
- scripts/translation-pipeline/eslint.config.mjs
- scripts/translation-pipeline/package.json
- scripts/translation-pipeline/src/cache/cache.ts
- pnpm-workspace.yaml
- scripts/ts-api-extractor/package.json
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
noahchoii
approved these changes
May 27, 2026
| "extract": "ts-api-extractor", | ||
| "format": "prettier --write \"./src/**/*.{ts,tsx,md}\"", | ||
| "format:check": "prettier --check \"./src/**/*.{ts,tsx,md}\"", | ||
| "i18n": "pnpm extract && pnpm translate", |
Contributor
There was a problem hiding this comment.
[NIT]
How about naming that command so that it’s clearly associated with “translate”? The “i18n” command and “translate” are related, but just by looking at the names, it might not be easy to tell they’re connected...!
…pl-translation-layer
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/website/package.json (1)
1-17:⚠️ Potential issue | 🟠 Major | ⚡ Quick win웹사이트 패키지에 Node/PNPM 버전 제약을 명시해 주세요.
현재
apps/website/package.json에engines.node와packageManager가 없어 환경 드리프트가 발생할 수 있습니다.제안 수정안
{ "name": "website", "version": "1.0.0", "private": true, "type": "module", + "packageManager": "pnpm@10.5.1", + "engines": { + "node": ">=20.19" + }, "scripts": {As per coding guidelines,
apps/website/**/{.nvmrc,.node-version,package.json}는 Node.js v20.19+를 요구하고,apps/website/**/{pnpm-lock.yaml,package.json}는 PNPM v10.5.1+ 사용을 요구합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/package.json` around lines 1 - 17, The apps/website/package.json file is missing engine version constraints that enforce Node.js and PNPM requirements, which can lead to environment drift. Add an `engines` field to specify the minimum Node.js version requirement (v20.19+) and a `packageManager` field to specify the required PNPM version (v10.5.1+) at the root level of the package.json object. These fields will ensure all developers and CI environments use compatible versions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/translation-pipeline/CLAUDE.md`:
- Around line 33-41: The code fence in the architecture diagram block lacks a
language specification, which triggers a markdownlint warning. Add the language
identifier `text` to the opening code fence delimiter so it reads ```text
instead of just ```. This applies to the code block starting with the cli/run.ts
architecture diagram showing the translation pipeline flow.
In `@scripts/translation-pipeline/README.md`:
- Around line 7-15: The code fence blocks in the README.md file are missing
language identifiers, which triggers markdownlint warnings. Add the language
identifier `text` to both code fences: the first one containing the pipeline
flow diagram showing packages/core components through .i18n-report.md, and the
second one showing the directory structure with cli/, translator/, translation/,
validation/, postprocess/, cache/, report/, and the configuration files
(defaults.ts and types.ts). Update the opening triple backticks from just ``` to
```text for both blocks.
In `@scripts/ts-api-extractor/package.json`:
- Around line 38-43: The `@vitest/coverage-v8` dependency is pinned to `^2.1.9`
while `vitest` has been upgraded to `^3.0.0`, creating a peer dependency
mismatch that will cause the test:coverage workflow to fail. Update the version
of `@vitest/coverage-v8` from `^2.1.9` to `^3.0.0` to align with the `vitest`
major version and ensure compatibility.
---
Outside diff comments:
In `@apps/website/package.json`:
- Around line 1-17: The apps/website/package.json file is missing engine version
constraints that enforce Node.js and PNPM requirements, which can lead to
environment drift. Add an `engines` field to specify the minimum Node.js version
requirement (v20.19+) and a `packageManager` field to specify the required PNPM
version (v10.5.1+) at the root level of the package.json object. These fields
will ensure all developers and CI environments use compatible versions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ca9241f5-f753-41a6-af28-3223a1677f0e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (7)
apps/website/package.jsonhandoff.mdpnpm-workspace.yamlscripts/translation-pipeline/CLAUDE.mdscripts/translation-pipeline/README.mdscripts/translation-pipeline/package.jsonscripts/ts-api-extractor/package.json
💤 Files with no reviewable changes (1)
- scripts/translation-pipeline/package.json
✅ Files skipped from review due to trivial changes (1)
- handoff.md
🚧 Files skipped from review as they are similar to previous changes (1)
- pnpm-workspace.yaml
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/website/package.json (1)
1-17:⚠️ Potential issue | 🟠 Major | ⚡ Quick win웹사이트 패키지에 Node/PNPM 버전 제약을 명시해 주세요.
현재
apps/website/package.json에engines.node와packageManager가 없어 환경 드리프트가 발생할 수 있습니다.제안 수정안
{ "name": "website", "version": "1.0.0", "private": true, "type": "module", + "packageManager": "pnpm@10.5.1", + "engines": { + "node": ">=20.19" + }, "scripts": {As per coding guidelines,
apps/website/**/{.nvmrc,.node-version,package.json}는 Node.js v20.19+를 요구하고,apps/website/**/{pnpm-lock.yaml,package.json}는 PNPM v10.5.1+ 사용을 요구합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/package.json` around lines 1 - 17, The apps/website/package.json file is missing engine version constraints that enforce Node.js and PNPM requirements, which can lead to environment drift. Add an `engines` field to specify the minimum Node.js version requirement (v20.19+) and a `packageManager` field to specify the required PNPM version (v10.5.1+) at the root level of the package.json object. These fields will ensure all developers and CI environments use compatible versions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/translation-pipeline/CLAUDE.md`:
- Around line 33-41: The code fence in the architecture diagram block lacks a
language specification, which triggers a markdownlint warning. Add the language
identifier `text` to the opening code fence delimiter so it reads ```text
instead of just ```. This applies to the code block starting with the cli/run.ts
architecture diagram showing the translation pipeline flow.
In `@scripts/translation-pipeline/README.md`:
- Around line 7-15: The code fence blocks in the README.md file are missing
language identifiers, which triggers markdownlint warnings. Add the language
identifier `text` to both code fences: the first one containing the pipeline
flow diagram showing packages/core components through .i18n-report.md, and the
second one showing the directory structure with cli/, translator/, translation/,
validation/, postprocess/, cache/, report/, and the configuration files
(defaults.ts and types.ts). Update the opening triple backticks from just ``` to
```text for both blocks.
In `@scripts/ts-api-extractor/package.json`:
- Around line 38-43: The `@vitest/coverage-v8` dependency is pinned to `^2.1.9`
while `vitest` has been upgraded to `^3.0.0`, creating a peer dependency
mismatch that will cause the test:coverage workflow to fail. Update the version
of `@vitest/coverage-v8` from `^2.1.9` to `^3.0.0` to align with the `vitest`
major version and ensure compatibility.
---
Outside diff comments:
In `@apps/website/package.json`:
- Around line 1-17: The apps/website/package.json file is missing engine version
constraints that enforce Node.js and PNPM requirements, which can lead to
environment drift. Add an `engines` field to specify the minimum Node.js version
requirement (v20.19+) and a `packageManager` field to specify the required PNPM
version (v10.5.1+) at the root level of the package.json object. These fields
will ensure all developers and CI environments use compatible versions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ca9241f5-f753-41a6-af28-3223a1677f0e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (7)
apps/website/package.jsonhandoff.mdpnpm-workspace.yamlscripts/translation-pipeline/CLAUDE.mdscripts/translation-pipeline/README.mdscripts/translation-pipeline/package.jsonscripts/ts-api-extractor/package.json
💤 Files with no reviewable changes (1)
- scripts/translation-pipeline/package.json
✅ Files skipped from review due to trivial changes (1)
- handoff.md
🚧 Files skipped from review as they are similar to previous changes (1)
- pnpm-workspace.yaml
🛑 Comments failed to post (3)
scripts/translation-pipeline/CLAUDE.md (1)
33-41:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick win코드 펜스에 언어를 지정하세요.
이 아키텍처 다이어그램 블록은 언어가 없어
markdownlint경고가 납니다.text를 붙여 주세요.수정 예시
-``` +```text cli/run.ts → translator/translator.ts # cache lookup, initial translation, outcome merge → translation/translate.ts # LLM initial translation (batch of 20) → translator/batch-lifecycle.ts # MQM → postprocess → final MQM → validation/validator.ts # batch MQM evaluation (batch of 10) → postprocess/postprocess.ts # corrective rewrite on MQM FAIL → report/report.ts # renders .i18n-report.md -``` +```🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/translation-pipeline/CLAUDE.md` around lines 33 - 41, The code fence in the architecture diagram block lacks a language specification, which triggers a markdownlint warning. Add the language identifier `text` to the opening code fence delimiter so it reads ```text instead of just ```. This applies to the code block starting with the cli/run.ts architecture diagram showing the translation pipeline flow.Source: Linters/SAST tools
scripts/translation-pipeline/README.md (1)
7-15:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick win코드 펜스에 언어를 지정하세요.
두 텍스트 블록 모두 언어가 없어
markdownlint경고가 납니다.text를 붙여 주세요.수정 예시
-``` +```text packages/core components ↓ ts-api-extractor generated/en/*.json ↓ translation-pipeline generated/ko/*.json .translation-cache.json .i18n-report.md -``` +``` ... -``` +```text src/ ├── cli/ # Entry point, argument parsing, file I/O ├── translator/ # Unit collection, cache lookup, batch lifecycle ├── translation/ # LiteLLM calls, initial translation prompt ├── validation/ # MQM evaluation prompt and response validation ├── postprocess/ # MQM FAIL feedback-based translation correction ├── cache/ # SHA256 key generation, cache load/save ├── report/ # Per-component stats, report rendering ├── defaults.ts # Default model configuration └── types.ts # Shared type definitions -``` +```Also applies to: 86-97
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 7-7: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/translation-pipeline/README.md` around lines 7 - 15, The code fence blocks in the README.md file are missing language identifiers, which triggers markdownlint warnings. Add the language identifier `text` to both code fences: the first one containing the pipeline flow diagram showing packages/core components through .i18n-report.md, and the second one showing the directory structure with cli/, translator/, translation/, validation/, postprocess/, cache/, report/, and the configuration files (defaults.ts and types.ts). Update the opening triple backticks from just ``` to ```text for both blocks.Source: Linters/SAST tools
scripts/ts-api-extractor/package.json (1)
38-43:
⚠️ Potential issue | 🟠 Major🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Verify Vitest/Coverage provider compatibility from npm metadata npm view vitest@3.0.0 version peerDependencies npm view `@vitest/coverage-v8`@2.1.9 version peerDependencies npm view `@vitest/coverage-v8`@3.0.0 version peerDependenciesRepository: goorm-dev/vapor-ui
Length of output: 439
Update
@vitest/coverage-v8to matchvitestmajor versionLine 43 upgrades
vitestto^3.0.0, but Line 38 keeps@vitest/coverage-v8at^2.1.9. According to npm peer dependencies,@vitest/coverage-v8@2.1.9requiresvitest@2.1.9, while@vitest/coverage-v8@3.0.0requiresvitest@3.0.0. This mismatch will cause failures in thetest:coverageworkflow.Suggested fix
- "`@vitest/coverage-v8`": "^2.1.9", + "`@vitest/coverage-v8`": "^3.0.0",📝 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."`@vitest/coverage-v8`": "^3.0.0", "eslint": "^9.39.4", "tsup": "^8.5.1", "tsx": "^4.21.0", "typescript": "catalog:", "vitest": "^3.0.0"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/ts-api-extractor/package.json` around lines 38 - 43, The `@vitest/coverage-v8` dependency is pinned to `^2.1.9` while `vitest` has been upgraded to `^3.0.0`, creating a peer dependency mismatch that will cause the test:coverage workflow to fail. Update the version of `@vitest/coverage-v8` from `^2.1.9` to `^3.0.0` to align with the `vitest` major version and ensure compatibility.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scripts/translation-pipeline— A standalone internal package that translates component JSDoc descriptions from English into Korean.Deterministic extraction (AST parsing) and non-deterministic translation (LLM invocation) have been separated by the package boundary.
> Scope: Creation of the
scripts/translation-pipeline/package + integration withapps/websitei18n scripts.Pipeline Flow
ko/JSONko/JSON + recorded in reportPackage Structure
Unit tests are located in the same folder as the implementation files (colocation). Only e2e tests are separated into
tests/.Summary of Design Decisions
TranslatableDoc—ts-api-extractortypes not importedtranslator.tshandles batch splitting and fallback branching exclusively; stage-based modules handle single-item processing onlyWebsite integration
Output
ko/*.json.translation-cache.json.i18n-report.mdSummary by CodeRabbit
.envexample with LiteLLM proxy variables.endirectory.