-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add translation metadata and quick ai translations #896
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
I think I would prefer a flatter structure for metadata, and the |
|
Couple of other notes:
|
|
(also noting that one of the unit tests are failing) |
|
Done - all should be updated now |
packages/root-cms/core/ai.ts
Outdated
|
|
||
| const responseText = res.text || '{}'; | ||
|
|
||
| // Try to extract JSON from the response |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style nit: comments should end in punctuation.
packages/root-cms/core/ai.ts
Outdated
| // Try to extract JSON from the response | ||
| let jsonText = responseText.trim(); | ||
|
|
||
| // Remove markdown code blocks if present |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be worth extracting to its own function and adding unit tests for it.
packages/root-cms/core/ai.ts
Outdated
| 'Do not include any markdown formatting, code blocks, or explanatory text.', | ||
| ].join('\n'); | ||
|
|
||
| let userPrompt = `Source text: "${options.sourceText}"\n\n`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
loop-based string concatentation is expensive, use arrays for building strings
packages/root-cms/core/ai.ts
Outdated
| options.existingTranslations && | ||
| Object.keys(options.existingTranslations).length > 0 | ||
| ) { | ||
| userPrompt += 'Existing translations for reference:\n'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the purpose of providing it with existing translations? Can we ignore locales we already have translations for in this request, to avoid overwriting existing translations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Providing it with the existing translations was my idea to help it establish more context; e.g. if you have 10 locales translated and one missing; the tone can be inferred from the existing translations to make the missing one more accurate.
It won't overwrite any existing translations (it just uses them to improve the style of the translations it's getting). I've added a comment that clarifies that to EditTranslationsModal.
|
Done - updated |
This PR adds two major features:
The current metadata options are:
The metadata is stored as:
When "do not translate" is checked the icon in the doc editor switches to red with a strike through it.