[failproofai-570] fix: validate translated docs pages at generation time and retry on failure#570
Conversation
…on failure The daily translate-docs run kept failing (5 of the last 15 runs) when `mintlify validate` in the consolidate job rejected a translated page's frontmatter YAML — the model re-emitting an escaped inner quote from a `description:` value as an unescaped `"`, e.g. de/agenteye/cli-skill.mdx: syntax error in your frontmatter on line 2 That check runs only after all 14 language jobs have spent their tokens, and one bad page in one language blocked the whole batch from publishing. The repo's own validate:mdx net could not catch this class at all: findMdxParseError blanks the frontmatter before compiling, so a frontmatter YAML error was structurally invisible to it. - validate-mdx.ts: add findFrontmatterError (YAML parse via the existing `yaml` dep, file-relative line numbers) and findPageError (frontmatter then body); validate:mdx now runs the latter, so the CI net is a strict superset of `mintlify validate` across all 647 pages. - validate-translation.ts (new): findTranslationError validates the exact bytes each translator will write — frontmatter YAML, key-parity against the English source (catches a dropped/renamed block, which is still valid YAML), and MDX body. - translator.ts: translateValidated re-translates with the validation error fed back until the page passes or TRANSLATE_MAX_ATTEMPTS (default 3, distinct from the SDK-level TRANSLATE_MAX_RETRIES). On exhaustion it throws before the write, so a broken page is never written or cached. - System-prompt rule 3 now carries the frontmatter-quoting guidance rule 2 has always given for JSX attributes, cutting the failure off at source. Adds 31 unit tests; validate:mdx passes on all 647 existing pages. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019s6wvcZ1NZHmJBfDbzyo65
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughTranslation now validates rendered MDX and README bytes, retries invalid generations with feedback up to a configured limit, prevents invalid output from being written or cached, reports retry counts, and extends MDX validation to frontmatter YAML errors. ChangesTranslation validation and retry pipeline
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Translator
participant Validator
participant Filesystem
CLI->>Translator: translate page or README
Translator->>Validator: validate rendered bytes
Validator-->>Translator: return success or validation error
Translator->>Translator: retry with feedback when invalid
Translator->>Filesystem: write validated output
Translator-->>CLI: return attempt count
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Automated code review started - full review. Results will be posted here. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@__tests__/scripts/translate-docs/validate-translation.test.ts`:
- Around line 46-70: The translation validation tests currently allow extra
frontmatter keys and do not cover malformed frontmatter on frontmatter-less
sources. Update the extra-key test to expect a validation error, and add a test
using a source without frontmatter plus malformed leading frontmatter that
asserts rejection rather than a blank-body success. Anchor the changes around
findTranslationError and the existing extra-key/body-only test cases.
In `@scripts/translate-docs/validate-translation.ts`:
- Around line 82-112: The validator around sourceKeys and frontmatterKeys must
validate rendered frontmatter before branching on the source shape, so malformed
or added frontmatter is rejected even when the source has no frontmatter.
Require exact key-set parity by rejecting both missing and extra keys, and
update the tests in
__tests__/scripts/translate-docs/validate-translation.test.ts to reject extra
keys and malformed added frontmatter for a frontmatter-less source.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea2f8362-fe20-4798-85a9-3cca68f59a82
📒 Files selected for processing (13)
.github/workflows/translate-docs.ymlCHANGELOG.md__tests__/scripts/translate-docs/mdx-translator.test.ts__tests__/scripts/translate-docs/translator.test.ts__tests__/scripts/translate-docs/validate-translation.test.ts__tests__/scripts/validate-mdx.test.tsscripts/translate-docs/cli.tsscripts/translate-docs/mdx-translator.tsscripts/translate-docs/readme-translator.tsscripts/translate-docs/translator.tsscripts/translate-docs/types.tsscripts/translate-docs/validate-translation.tsscripts/validate-mdx.ts
|
⏳ Phase 1-2 complete. Fetched all context — 13 files changed (focused translation validation changes). Now building and running tests for real evidence. |
CodeRabbit flagged that findTranslationError only validated frontmatter YAML when the English source had a frontmatter block. For a frontmatter-less source (the READMEs and ~15 .mdx pages), a malformed block the model added would be blanked by findMdxParseError before compiling and sail through — reopening, for that source shape, the exact blind spot this module closes. Hoist findFrontmatterError(rendered) ahead of the source-shape branch so the rendered frontmatter is validated unconditionally. The key-parity check stays a subset (extra keys tolerated): Mintlify ignores unknown frontmatter keys, so rejecting an extra key cannot prevent a deploy failure — it can only burn a retry and risk exhausting the batch on a page that would publish fine. Reject content loss (missing/renamed keys); allow harmless additions. Adds 2 tests: malformed added frontmatter on a frontmatter-less source is rejected; well-formed added frontmatter is allowed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019s6wvcZ1NZHmJBfDbzyo65
|
Thanks @coderabbitai — one point taken, one respectfully held. Frontmatter-less source + malformed added frontmatter — fixed (commit e7e6ca5). You're right: Exact key-parity (rejecting extra keys) — deliberately kept as a subset check. Mintlify ignores unknown frontmatter keys, so an extra key cannot cause a deploy failure. Rejecting it could only burn a retry and risk exhausting the attempt budget on a page that would publish fine — which is precisely the "one page blocks all 14 languages" failure mode this PR exists to eliminate. The gate rejects content loss (missing/renamed keys, a real regression) and tolerates harmless additions. Making it stricter than the deploy here would trade the reliability we're buying for fidelity the deploy doesn't require. Expanded the code comment to make this explicit. |
|
Tip For best results, initiate chat on the files or code changes.
Your rationale for subset key parity is sound: preserving required source keys prevents semantic loss, while allowing deploy-ignored additions avoids spending the bounded retry budget on non-blocking differences. I’m happy to leave that policy as implemented. 🐇 ✅ ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
|
✅ Build & test complete (CI-verified). Results:
Full review incoming... |
🔍 Automated Code Review — PR #570📋 Executive SummaryThis PR shifts validation of translated docs pages from the final 📊 Change Architecturegraph TD
A["translateContent()"] -->|"+feedback param"| B["translateValidated() [NEW]"]
B -->|"render(raw)"| C["render callback"]
C --> D["findTranslationError() [NEW]"]
D -->|"valid"| E["writeFileSync + cache"]
D -->|"invalid"| B
D -->|"exhausted"| F["THROW"]
D --> G["findFrontmatterError() [NEW]"]
D --> H["frontmatterKeys() [NEW]"]
D --> I["findMdxParseError() [existing]"]
G -.-> J["findPageError() [NEW entry]"]
I -.-> J
J --> K["validate:mdx CI"]
style B fill:#90EE90
style D fill:#90EE90
style G fill:#90EE90
style H fill:#90EE90
style J fill:#FFD700
🔴 Breaking Changes✅ No breaking changes detected. All new exports are additive. Existing function signatures extended with optional params. No DB, API, or config changes.
|
| Job | Result | Duration |
|---|---|---|
| build | pass | 1m3s |
| test (default) | 2232 passed | 1m18s |
| test (hook-log-file) | pass | 1m12s |
| test (log-debug) | pass | 1m11s |
| test-e2e | pass | 51s |
| docs (validate:mdx) | 647 pages clean | 1m36s |
| quality (lint+tsc) | pass | 1m9s |
🔗 Issue Linkage
👥 Human Review Feedback
- @coderabbitai[bot]: Two auto-comments about frontmatter strictness. Both are design choices, not bugs. ℹ️ Noted.
- No human review comments on this PR.
💡 Suggestions
- Consider exporting
FRONTMATTER_REfromvalidate-mdx.tsto eliminate the intentional duplication. - Consider
docsDirconfigurability at CLI level rather than per-function (test-only plumbing). - Snapshot test for
buildRetryFeedbackformat to catch accidental prompt drift.
🏆 Verdict
VERDICT: APPROVED
High-quality, well-architected fix. Retry loop correctly bounded, transport errors propagate properly, cache/write safety solid. 31+ new tests covering all paths. Full CI green: 2232 tests, 647 pages clean, lint zero, tsc clean, e2e green.
Automated code review · 2026-07-17 ~18:05 UTC
hermes-exosphere
left a comment
hermes-exosphere
left a comment
There was a problem hiding this comment.
Automated review: Approved. ✅
Full analysis at the summary comment. No blocking issues found — well-architected fix with comprehensive test coverage. CI all green: 2232 tests, 647 pages validate clean, lint zero.
|
Tip For best results, initiate chat on the files or code changes.
One minor note for the record: the frontmatter-less-source malformed-frontmatter edge case listed in the report was subsequently fixed in 🐇 ✅ |
hermes-exosphere
left a comment
hermes-exosphere
left a comment
There was a problem hiding this comment.
Automated review: Approved. ✅
|
Automated code review started - full review. Results will be posted here. |
🔍 Automated Code Review — PR #570 (Re-review)📋 Executive SummaryThis PR shifts docs translation validation from the final 📊 Change Architecturegraph TD
A["translateContent<br/>(raw LLM call)"] -->|"+ feedback param"| B["translateValidated<br/>(NEW: retry loop)"]
B -->|render| C["render callback<br/>(sanitize + link-rewrite)"]
C -->|validate| D["findTranslationError<br/>(NEW)"]
D --> E["findFrontmatterError<br/>(NEW in validate-mdx)"]
D --> F["frontmatterKeys parity<br/>(subset check)"]
D --> G["findMdxParseError<br/>(existing)"]
E -->|"YAML parse"| H["yaml@2.9.0<br/>(already a dep)"]
B -->|"fail: throw before write"| I["No file written<br/>No cache entry"]
B -->|"pass"| J["writeFileSync<br/>update cache"]
K["translateMdxPage"] --> B
L["translateReadme"] --> B
style B fill:#90EE90
style D fill:#90EE90
style E fill:#90EE90
style F fill:#87CEEB
style G fill:#87CEEB
Legend: 🟢 New | 🔵 Modified | 🟡 Breaking Change Risk 🔴 Breaking Changes✅ No breaking changes detected. All existing public APIs (
|
hermes-exosphere
left a comment
hermes-exosphere
left a comment
There was a problem hiding this comment.
🔍 Automated Code Review — PR #570 (Re-review)
📋 Executive Summary
This PR shifts docs translation validation from the final consolidate CI job to generation time with automatic retry. Every translated page is validated (frontmatter YAML + key parity + MDX body) against the exact bytes that will be written, and invalid output triggers a re-translate capped at 3 attempts. Two commits: the initial implementation (6000368) and a fix (e7e6ca5) that validates frontmatter for every source shape — even frontmatter-less sources — closing a blind spot identified in the prior review. Code is well-architected, the safety invariants are sound, and tests are comprehensive. No blocking issues found.
📊 Change Architecture
graph TD
A["translateContent<br/>(raw LLM call)"] -->|"+ feedback param"| B["translateValidated<br/>(NEW: retry loop)"]
B -->|render| C["render callback<br/>(sanitize + link-rewrite)"]
C -->|validate| D["findTranslationError<br/>(NEW)"]
D --> E["findFrontmatterError<br/>(NEW in validate-mdx)"]
D --> F["frontmatterKeys parity<br/>(subset check)"]
D --> G["findMdxParseError<br/>(existing)"]
E -->|"YAML parse"| H["yaml@2.9.0<br/>(already a dep)"]
B -->|"fail: throw before write"| I["No file written<br/>No cache entry"]
B -->|"pass"| J["writeFileSync<br/>update cache"]
K["translateMdxPage"] --> B
L["translateReadme"] --> B
style B fill:#90EE90
style D fill:#90EE90
style E fill:#90EE90
style F fill:#87CEEB
style G fill:#87CEEB
Legend: 🟢 New | 🔵 Modified | 🟡 Breaking Change Risk
🔴 Breaking Changes
✅ No breaking changes detected. All existing public APIs (translateMdxPage, translateReadme, TranslationResult) are backward-compatible. The new attempts field on TranslationResult is optional. The translateContent function gained an optional 5th parameter (feedback) that no existing callers pass. The CLI summary now prints retry counts but this is additive, not a format change.
⚠️ Issues Found
-
🔵 Info (non-blocking) —
scripts/translate-docs/validate-translation.ts:33— TheFRONTMATTER_REregex is duplicated fromscripts/validate-mdx.ts:49. The comment explains this is intentional (to avoid depending on an unexported internal constant), which is a reasonable design choice for a 13-character regex, but worth noting. -
🔵 Info (non-blocking) —
scripts/translate-docs/translator.ts:251— Error message usesattempt(s)instead of a proper plural:"after ${MAX_ATTEMPTS} attempt(s)". Should be"after ${MAX_ATTEMPTS} attempt${MAX_ATTEMPTS === 1 ? '' : 's'}"or simply"after ${MAX_ATTEMPTS} attempts"(since 1 would mean no retries needed, the exhaustion path only triggers with ≥ 1 failed attempt).
🔬 Logical / Bug Analysis
Retry loop correctness (translateValidated): The for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) loop properly bounds retries. Each iteration: calls translateContent, sums token usage, checks for empty output (re-treated as validity failure), renders the bytes, validates, and returns on null error. On exhaustion, throws before the caller ever reaches write/cache. No off-by-one.
Safety invariants verified:
- Invalid page = no write + no cache:
translateValidatedthrows on exhaustion; the callers (translateMdxPage,translateReadme) havewriteFileSyncand cache-update AFTER the translateValidated call — unreachable on throw. - Retries don't increase concurrency: retries are serial inside one worker slot (correctly noted in workflow comment).
- max_tokens truncation is NOT retried as a validity failure: no try/catch in the loop — transport/auth/max_tokens throws propagate directly.
- Empty translation is retried:
result.translated.trim() === ""catches whitespace-only output. - Token summation:
inputTokensandoutputTokensare accumulated across attempts (confirmed by test).
Frontmatter-less source fix (e7e6ca5): findFrontmatterError(rendered) runs unconditionally at line 88 of validate-translation.ts — BEFORE the sourceKeys branch. This means a malformed frontmatter block the model adds to a page that originally had NO frontmatter (like the README) is now caught. The commit message fix: validate rendered frontmatter for every source shape accurately describes this. The test "flags malformed frontmatter the model added to a frontmatter-less source" confirms the behavior.
Key parity design choice: Subset check only — rejects missing/renamed keys but tolerates extras. Rationale documented inline: an extra key is harmless (Mintlify ignores unknown frontmatter keys), while rejecting it would burn a retry (and risk exhausting the whole batch) on a page that deploys fine. This is a deliberate trade-off, clearly explained.
System prompt update (rule 3): The frontmatter quoting guidance has been extended to mirror the JSX attribute guidance from rule 2 — "never put an unescaped ASCII " inside them" — cutting the failure off at the source. Good preventive measure.
Workflow comment correction: The old comment said MAX_CONCURRENT=2 for 8 concurrent requests; corrected to MAX_CONCURRENT=4 (cli.ts default) for 16 concurrent requests. Verified — cli.ts defaults to TRANSLATE_MAX_CONCURRENT || 4. The retry clarification (serial in worker slot) is correct.
🧪 Evidence — Build & Test Results
Test Results — PR-specific tests
RUN v4.1.10 /home/failproofai/failproofai
✓ __tests__/scripts/translate-docs/translator.test.ts (12 tests) 21ms
✓ __tests__/scripts/translate-docs/validate-translation.test.ts (9 tests) 72ms
✓ __tests__/scripts/validate-mdx.test.ts (22 tests) 99ms
✓ __tests__/scripts/translate-docs/mdx-translator.test.ts (52 tests) 154ms
Test Files 4 passed (4)
Tests 95 passed (95)
Start at 21:25:10
Duration 4.52s
TypeScript Compilation
npx tsc --noEmit
→ exit 0, no errors
validate:mdx on all 647 pages
bun scripts/validate-mdx.ts
→ ✓ 647 MDX page(s) parsed cleanly
CI results (from prior review): 2232 tests across 126 files, all green. Lint passes (0 errors). E2E tests pass.
🔗 Issue Linkage
No linked issue. The PR body describes the problem comprehensively — the daily translate-docs workflow kept failing (5 of last 15 runs) due to translator re-emitting unescaped inner quotes in frontmatter YAML, invisible to the repo's own validate:mdx because findMdxParseError blanked frontmatter before compiling. The fix is architectural: move validation to generation time with retry.
👥 Human Review Feedback
Review threads from previous review:
-
@coderabbitai (thread PRRT_kwDOR6tUec6R0Rea) — requested: (a) change extra-key case to expect rejection, (b) add frontmatter-less source + malformed frontmatter test. Status: (a) The author chose subset parity (reject missing, tolerate extras) with documented rationale — a valid design choice. (b) Fixed in e7e6ca5 —
findFrontmatterErrorruns unconditionally, and the test"flags malformed frontmatter the model added to a frontmatter-less source"covers this case. → Both addressed per author's design decisions. -
@coderabbitai (thread PRRT_kwDOR6tUec6R0Ref) — requested: validate rendered frontmatter before branching on source keys, require identical key sets. Status: The unconditional
findFrontmatterError(rendered)call fixes the ordering issue. On key parity: the author chose subset check (reject missing, tolerate extras), documented inline with rationale. The test"allows an extra frontmatter key the source does not have"confirms this behavior. → Addressed per author's design decisions. -
@NiveditJain (human comment) — acknowledged coderabbit feedback, explained fixes and design rationale. No further requests. → ✅ Acknowledged.
Both coderabbit threads remain unresolved in the GitHub UI — these are bot-generated review threads that the human author has acknowledged and addressed (or made informed design decisions on). The author/maintainer should resolve them.
💡 Suggestions
- Consider extracting
FRONTMATTER_REto a shared constant invalidate-mdx.ts(or a shared module) rather than duplicating it invalidate-translation.ts. The 13-character regex is unlikely to drift, but a single source of truth is cleaner. - The
attempt(s)string in the exhaustion error could use proper pluralization. - The test file
__tests__/scripts/translate-docs/validate-translation.test.tscould benefit from one additional edge case: a source WITH frontmatter where the translation produces malformed YAML on a line other than the first two (e.g., a new key with an unparseable complex value). The existing YAML-error test covers line 2; deeper lines might expose yaml library differences.
🏆 Verdict
VERDICT: APPROVED
Automated code review · 2026-07-17 · Hermes Agent
|
Tip For best results, initiate chat on the files or code changes.
The report correctly reflects the current state: rendered-byte validation, bounded retries, no write/cache on exhaustion, and unconditional rendered-frontmatter validation—including frontmatter-less sources—are all covered. The remaining regex-deduplication and |
hermes-exosphere
left a comment
hermes-exosphere
left a comment
There was a problem hiding this comment.
Automated review: Approved. ✅
2 participants
Problem
The daily Translate Docs workflow kept failing — 5 of the last 15 runs — with errors like:
Two compounding problems:
mintlify validateruns in theconsolidatejob — after all 14 per-language matrix jobs have finished and spent their tokens. One bad page in one language fails the whole batch and nothing publishes (consolidateis gated onneeds.translate.result == 'success').validate:mdxnet was blind to this class.findMdxParseErrorcallsstripFrontmatter()and compiles only the body, so a frontmatter YAML error was structurally invisible to it — only the last-stepmintlify validateever saw it.Root cause of the recurring instance:
docs/agenteye/cli-skill.mdxhasdescription: "Ask your coding agent \"is anything broken today?\" …"(escaped inner quotes). The translator re-emits those inner quotes unescaped into the target-language scalar, producing invalid YAML on thetitle:/description:line.Fix — validate each generated page, retry with the error fed back
Per the ask: "instead of checking .mdx validity at the last step, check each generated .mdx and, if invalid, re-translate with the validation error; cap the tries."
validate-mdx.tsgainsfindFrontmatterError(YAML parse via the already-presentyamldep, file-relative line numbers) andfindPageError(frontmatter then body).validate:mdxnow runsfindPageError, making the CI net a strict superset ofmintlify validate— it verified clean on all 647 existing pages.validate-translation.ts(new) —findTranslationErrorvalidates the exact bytes each translator will write (sanitizers + link-rewrite for MDX pages; disclaimer + RTL<div>wrapper for the README): frontmatter YAML, frontmatter key-parity against the English source (catches a dropped or renamed block, which is still valid YAML and whichmintlifytolerates), and MDX body.translator.ts—translateValidatedre-translates with the validation error appended to the prompt until the page passes orTRANSLATE_MAX_ATTEMPTS(integer ≥ 1, default 3; distinct from the SDK-transportTRANSLATE_MAX_RETRIES). On exhaustion it throws before the write, so a broken page is never written to disk and never cached — the cache keys only the English source hash, so a cached-invalid page could otherwise never self-heal.4×2=8).Verification
findPageErrornow catches it at line 3, andfindTranslationErrorproduces caret-underlined, model-actionable repair feedback. No false positive on the valid English source.tscclean, lint clean (0 errors),validate:mdxclean on all 647 pages.🤖 Generated with Claude Code
https://claude.ai/code/session_019s6wvcZ1NZHmJBfDbzyo65
Summary by CodeRabbit