fix(review): write PR comments in author's language, not file's#81
fix(review): write PR comments in author's language, not file's#81factory-nizar wants to merge 1 commit intodevfrom
Conversation
The review and security-review prompts had no language guidance, so when a
diff included localized files (e.g. docs/jp/...), the model would mirror the
file content language and post review comments in Japanese on an English PR.
Add explicit language sections to all four prompt templates (candidates,
validator, security-review, security-report) instructing the reviewer to:
- Detect the PR author's language from the PR description and title
(descriptionPath artifact) and match it for all human-readable parts of the
output (titles, paragraphs, suggestion prose, summary).
- Fall back to English when the language is uncertain.
- NOT mirror the language of source files being reviewed when those files are
translations / localization (docs/jp, docs/ko, .es.mdx, etc.).
- Keep priority tags ([P0]/[P1]/[P2]/[P3]), [security] markers, severity
labels, CWE/OWASP refs, file paths, and code snippets in English regardless.
The validator prompt additionally instructs Pass-2 to rewrite a candidate's
body into the correct language if Pass-1 produced it in the wrong one
(e.g., JP comment generated for a JP doc on an EN PR), without rejecting it
solely on language.
Companion guidance was added to apps/cli/builtin-skills/{review,security-review}/SKILL.md
in factory-mono so that interactive /review and /security-review invocations
follow the same rule.
Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>
|
Droid finished @factory-nizar's task —— View job Droid review complete. Good fix for the “localized file => localized review” failure mode by adding explicit language guidance across prompts. A few small prompt-consistency/tightening tweaks (priority tag schema alignment and preserving code fences when translating) would make the pipeline more robust. |
![factory-droid[bot]](https://avatars.githubusercontent.com/in/358017?s=60&v=4){kind=small}
| </output_spec> | ||
|
|
||
| <language> | ||
| Write every review comment (priority tag aside) — titles, paragraphs, suggestion-block prose, and the \`reviewSummary.body\` — in the language the PR author is using. Detect the language from the PR description and title at \`${descriptionPath}\`; fall back to English if uncertain. Do **not** mirror the language of the source files being reviewed: when the diff includes localized files (translations, \`docs/jp/...\`, \`docs/ko/...\`, \`.es.mdx\`, etc.), still write comments in the PR author's language, not the file's. Priority tags (\`[P0]\`/\`[P1]\`/\`[P2]\`/\`[P3]\`) and the \`[security]\` marker remain in English regardless. |
There was a problem hiding this comment.
[P2] Keep priority tags consistent across the prompt schema
This template’s schema text describes comment bodies as starting with [P0|P1|P2], but the new language guidance references [P3]; that mismatch can confuse Pass 1/2 and produce outputs that downstream tooling treats as out-of-spec. Align the allowed priority tag set end-to-end (either remove [P3] here or update the schema/field descriptions everywhere to explicitly allow it).
|
|
||
| ### Language | ||
|
|
||
| Approved comments and the review summary must be in the language the PR author is using. Detect the language from the PR description and title at \`${descriptionPath}\`; fall back to English if uncertain. Do **not** mirror the language of the source files being reviewed (localized files, translations, \`docs/jp/...\`, etc.) — match the PR author's language. If a candidate was written in the wrong language (e.g., Japanese on an English PR because the diff touched JP docs), rewrite the body into the correct language while preserving meaning, the priority tag (\`[P0]\`/\`[P1]\`/\`[P2]\`/\`[P3]\`), and any \`[security]\` marker. Approve it if the underlying finding is otherwise valid; do not reject solely on language. Priority tags and the \`[security]\` marker remain in English regardless. |
There was a problem hiding this comment.
[P2] Don’t rewrite code when translating candidate bodies
The validator is told to “rewrite the body into the correct language”, but without an explicit rule to keep fenced code blocks (especially suggestion contents) and other machine-parsed tokens verbatim, it may inadvertently translate/alter code suggestions and break applicability. Clarify that only prose outside code fences should be translated, while suggestion/code blocks, file paths, and markdown structure remain unchanged.
| 2. **Patches**: Test all generated patches before committing. Ensure they don't break functionality. | ||
| 3. **PR Description**: Update the PR body with actual finding counts before creating. | ||
| 4. **Commit Messages**: Use semantic commit format: \`fix(security): [VULN-XXX] Description\` | ||
| 5. **Language**: Write the human-readable parts of the report (titles, descriptions, exploitation, impact, recommended fix, PR body) in the repository's primary language — detect this from the existing README, top-level docs, and any prior PR/issue text. Fall back to English when uncertain. Do **not** mirror the language of source files inside the scan when those files are localized translations (e.g., \`docs/jp/...\`). Severity labels (CRITICAL/HIGH/MEDIUM/LOW), CWE identifiers, severity tags, file paths, and code snippets stay in their canonical form regardless. |
There was a problem hiding this comment.
[P2] Make the PR-body example consistent with the new report language rule
This prompt now requires the PR body to be written in the repository’s primary language, but the embedded gh pr create --body example is fixed English; models often follow examples over prose, so you can end up with English PR bodies despite the new rule. Replace the hardcoded English body with a placeholder or explicitly instruct translating that body template into the chosen language.
| </output_spec> | ||
|
|
||
| <language> | ||
| Write every review comment (priority tag aside) — titles, paragraphs, suggestion-block prose, and the \`reviewSummary.body\` — in the language the PR author is using. Detect the language from the PR description and title at \`${descriptionPath}\`; fall back to English if uncertain. Do **not** mirror the language of the source files being reviewed: when the diff includes localized files (translations, \`docs/jp/...\`, \`docs/ko/...\`, \`.es.mdx\`, etc.), still write comments in the PR author's language, not the file's. Priority tags (\`[P0]\`/\`[P1]\`/\`[P2]\`/\`[P3]\`) and the \`[security]\` marker remain in English regardless. |
There was a problem hiding this comment.
[P2] [security] Treat PR description as untrusted input for language detection
The prompt now explicitly instructs reading the PR title/description at ${descriptionPath} to detect language; because PR text is attacker-controlled on untrusted PRs, it’s worth adding a clear trust-boundary rule to ignore any instructions embedded in that text and use it strictly as data (language detection only). This reduces prompt-injection risk where an author attempts to steer tool use or leak workspace data into the generated JSON that later gets posted.
1 participant
Summary
When the GitHub Action reviewed a PR whose diff included localized files (e.g.
docs/jp/...), the review prompt had no language guidance, so the model mirrored the file content language and posted Japanese review comments on an English PR. Reproduced on Factory-AI/factory#1039.Changes
Add an explicit language section to all four prompt templates:
src/create-prompt/templates/review-candidates-prompt.ts(Pass 1)src/create-prompt/templates/review-validator-prompt.ts(Pass 2)src/create-prompt/templates/security-review-prompt.ts(security subagent)src/create-prompt/templates/security-report-prompt.ts(full-repo scan)Rule:
descriptionPathartifact) and write all human-readable parts of the output in that language: titles, paragraphs, suggestion prose,reviewSummary.body.docs/jp/...,docs/ko/...,.es.mdx, etc.).[P0]/[P1]/[P2]/[P3]), the[security]marker, severity labels (CRITICAL/HIGH/MEDIUM/LOW), CWE/OWASP refs, file paths, and code snippets stay in English regardless.The validator prompt also instructs Pass 2 to rewrite a candidate's body into the correct language if Pass 1 produced it in the wrong one (preserving meaning, the priority tag, and any
[security]marker), rather than rejecting it solely on language.Companion guidance is being added to
apps/cli/builtin-skills/{review,security-review}/SKILL.mdinfactory-monoso interactive/reviewand/security-reviewfollow the same rule (separate PR).Validation
bun run typecheck✅bun run format:check✅bun test✅ (377 pass, 0 fail, 892 expect calls)