feat(validator): HTML-tag-mismatch + _maintainer shape + wire validate-packs into CI - #72
Merged
teamssUTXO merged 4 commits intoMay 29, 2026
Conversation
…re validate-packs into CI
Anti-slop pass on Phase 4 surfaced 4 gaps in the existing validator
infrastructure (Tim msg 486 in TG -5292199939 2026-05-26). This PR
closes 3 of them; the 4th (broader identical-to-English check beyond
the curated ShortKeyHotspotKeys list) is deliberately left for a
follow-up because expanding the hotspot list requires per-language
maintainer judgment on which anglicisms are acceptable.
## What
### (a) HTML-tag-mismatch rule
`TranslationValidationRules.HasMatchingHtmlTags(source, translation)`
compares the multiset of structural HTML tags between source and
translation. Restricted to a curated allowlist of real markup
elements (strong/em/b/i/code/pre/kbd/a/span/p/div/br/ul/ol/li/h1-6/
table family/abbr/del/ins/q/cite/var/samp/mark/small/sub/sup/u) so
localized example data ('<email@primer.com>', '<John Doe>') doesn't
trip the rule. Catches the same shape my anti-slop pass found on
hindi.json (3 entries) + indonesian.json (1) + thai.json (2):
translation drops a closing </strong> or </code> pair, silently
breaks UI rendering.
No auto-fix path - restoring the English source would lose the
translated prose around the tags. Maintainer needs to re-anchor the
markup by hand.
### (d) _maintainer field validation
`TranslationValidationRules.IsValidMaintainerValue(value)` checks
the field shape ManifestGenerator expects:
`<display name or handle>|<https URL>`. Missing pipe, missing URL,
or non-https URL fail validation. Empty / whitespace value (field
present-but-unset) also fails. LanguagePackValidator skips the
_maintainer entry from the translation-entry pipeline (it's
metadata, not a translation) and runs the shape check separately.
### (c) Wire validate-packs into tests.yml CI
New step in the existing 'unit-tests' job runs
`dotnet run -- validate-packs` after the test step, with
Translation__OutputDirectory pointed at the repo's translations
folder. Step is marked continue-on-error: true initially so the new
HTML-tag rule surfaces the existing hindi/indonesian/thai issues
without blocking unrelated PRs. Once those land follow-up fixes,
drop continue-on-error to gate new slop from landing.
## Tests
5 new tests in LanguagePackValidatorTests:
- ValidateAsync_FlagsHtmlTagMismatch
- ValidateAsync_IgnoresExampleEmailAngleBrackets
- ValidateAsync_FlagsInvalidMaintainerField
- ValidateAsync_AcceptsWellFormedMaintainerField
- ValidateAsync_RejectsMaintainerWithHttpScheme
9/9 LanguagePackValidatorTests passing. Build clean.
## Out of scope
Item (b) - broader identical-to-English check beyond the
ShortKeyHotspotKeys list. The anti-slop pass found ~30 per-language
identical-to-English entries (Checkout, Downgrade, Hostname, etc.);
many are acceptable anglicisms (Login, Logo, Online, Token) and
which to translate is per-language maintainer judgment. Adding a
generic rule risks false-positive churn. Either expand the hotspot
list incrementally as patterns surface, OR add a strict-mode
report-only check in a follow-up.
## Pre-existing issues this surfaces
The validate-packs step (continue-on-error) will report:
- hindi.json: 3 HTML-tag-mismatch entries (Khush + Abhijay007 to fix)
- indonesian.json: 1 HTML-tag-mismatch entry (no maintainer set)
- thai.json: 2 HTML-tag-mismatch entries (no maintainer set)
- romanian.json: 'Text' pre-existing hotspot-key issue
None of these are introduced by this PR; they were latent before.
The CI step makes them visible.
Forgotten in the previous commit on this branch. Step runs the validate-packs CLI after the unit-tests step with the in-repo translations/ folder as input. continue-on-error: true keeps the step report-only until the pre-existing HTML-tag-mismatch issues in hindi/indonesian/thai are fixed by their maintainers.
Contributor
|
Warning Review limit reached
More reviews will be available in 41 minutes and 51 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR enhances the language pack validator by adding two new validation rules: structural HTML tag matching between source and translated strings, and maintainer field format validation. Integration into the validator's control flow, comprehensive test coverage, and CI enablement follow. ChangesTranslation Validation Enhancement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@Translator/Services/TranslationValidationRules.cs`:
- Around line 281-283: IsValidMaintainerValue currently returns true for
null/whitespace which lets blank _maintainer fields pass; change the logic so
that string.IsNullOrWhiteSpace(value) causes the method to return false (reject
blank values when the field is present) and then continue enforcing the existing
required "name|https://..." shape check (the same validation already applied
elsewhere in IsValidMaintainerValue). Update the branch in
TranslationValidationRules.cs inside IsValidMaintainerValue (remove/flip the
current if that returns true for whitespace) so blank or whitespace values fail
validation while preserving the subsequent pattern validation logic.
🪄 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: a9b634bc-0d5f-48dc-a9a0-f6a719ad6aa7
📒 Files selected for processing (4)
.github/workflows/tests.ymlTranslator.Tests/Services/LanguagePackValidatorTests.csTranslator/Services/LanguagePackValidator.csTranslator/Services/TranslationValidationRules.cs
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
2 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.
@teamssUTXO - opening this PR from the anti-slop pass discussion in
-52921999392026-05-26. Closes 3 of the 4 gaps I surfaced when walking the existing validator infrastructure (msg 486); the 4th (broader identical-to-English check) is deliberately left for a follow-up because expanding theShortKeyHotspotKeyslist requires per-language maintainer judgment on which anglicisms are acceptable.What
(a) HTML-tag-mismatch rule
TranslationValidationRules.HasMatchingHtmlTags(source, translation)compares the multiset of structural HTML tags between source and translation. Restricted to a curated allowlist of real markup elements (strong/em/b/i/code/pre/kbd/a/span/p/div/br/ul/ol/li/h1-6/tablefamily/abbr/del/ins/q/cite/var/samp/mark/small/sub/sup/u) so localized example data (<email@primer.com>,<John Doe>) doesn't trip the rule. Catches the same shape today's anti-slop pass found on hindi.json (3), indonesian.json (1), and thai.json (2) - translation drops a closing</strong>or</code>pair, silently breaks UI rendering. No auto-fix path - restoring the English source would lose the translated prose around the tags.(d)
_maintainerfield validationTranslationValidationRules.IsValidMaintainerValue(value)checks the field shape ManifestGenerator expects:<display name or handle>|<https URL>. Missing pipe, missing URL, or non-https URL all fail.LanguagePackValidatorskips the_maintainerentry from the translation-entry pipeline (it's metadata, not a translation) and runs the shape check separately.(c) Wire
validate-packsintotests.ymlCINew step in the existing unit-tests job runs
dotnet run -- validate-packsafter the test step, withTranslation__OutputDirectorypointed at the repo's translations folder. Step iscontinue-on-error: trueinitially so the new HTML-tag rule surfaces the existing hindi/indonesian/thai issues without blocking unrelated PRs. Once those fixes land, dropcontinue-on-errorto gate new slop from landing.Tests
5 new tests in
LanguagePackValidatorTests:ValidateAsync_FlagsHtmlTagMismatchValidateAsync_IgnoresExampleEmailAngleBracketsValidateAsync_FlagsInvalidMaintainerFieldValidateAsync_AcceptsWellFormedMaintainerFieldValidateAsync_RejectsMaintainerWithHttpScheme9/9 LanguagePackValidatorTests passing locally. Build clean.
Out of scope
Item (b) - broader identical-to-English check beyond
ShortKeyHotspotKeys. The anti-slop pass found ~30 per-language identical-to-English entries (Checkout, Downgrade, Hostname, etc.); many are acceptable anglicisms (Login, Logo, Online, Token) and which to translate is per-language maintainer judgment. Adding a generic rule risks false-positive churn. Either expand the hotspot list incrementally as patterns surface, OR add a--strictreport-only flag in a follow-up.Pre-existing issues this surfaces
The
validate-packsstep (continue-on-error) will report:hindi.json: 3 HTML-tag-mismatch entries (Khush + Abhijay7 to fix)indonesian.json: 1 HTML-tag-mismatch entry (no maintainer set)thai.json: 2 HTML-tag-mismatch entries (no maintainer set)romanian.json:Textpre-existing hotspot-key issueNone are introduced by this PR; they were latent before. The CI step makes them visible.