Skip to content

fix: honor user language for AI responses - #297

Open
sergeybychkovvvpgroup-beep wants to merge 2 commits into
maathimself:mainfrom
sergeybychkovvvpgroup-beep:fix/ai-language
Open

fix: honor user language for AI responses#297
sergeybychkovvvpgroup-beep wants to merge 2 commits into
maathimself:mainfrom
sergeybychkovvvpgroup-beep:fix/ai-language

Conversation

@sergeybychkovvvpgroup-beep

Copy link
Copy Markdown

Separate PR split out from #291 per maintainer review.

This keeps the IMAP MIME body-part fix focused and moves the unrelated AI language handling change into its own review.

Change:

  • honor the user language setting for AI responses

@maathimself

Copy link
Copy Markdown
Owner

Thanks, but this branch was cut from #291's first commit rather than from main, so it carries the whole original IMAP/MIME change along with the AI one, its diff touches imapManager.js, emailSanitizer.js, and mail.js, and its commits include 353383c "fix: fetch IMAP body parts individually".

That's the pre-review version, so merging this would re-introduce exactly what we removed in #291: repairResidualQuotedPrintableHtml (corrupts ~56% of real bodies and writes the corruption back to the DB cache), the un-guarded blanket per-part re-fetch, and unwrapEmbeddedMimeText with no recursion bound.

Could you re-cut this off main so it contains only the ai.js change? Since that change is entirely in one file, the simplest path is:

git checkout -B fix/ai-language origin/main
git checkout 9a6d0a7 -- backend/src/routes/ai.js
git commit -m "fix: honor user language for AI responses"
git push --force-with-lease

The AI-language change itself looks good, it just needs to stand alone. Thanks!

@maathimself

Copy link
Copy Markdown
Owner

Following up here, since this still carries the #291 IMAP/MIME changes (imapManager.js, emailSanitizer.js, mail.js) on top of the ai.js change. The re-cut from main is still the blocker; the commands in my previous comment should do it.

Two small things to fold in once it stands alone, so the AI change is complete:

  • Add a test for the language mapping (aiLanguageInstruction and its allowlist plus fallback). It is a small pure function and easy to cover.
  • Decide whether the GTD "waiting" gist summaries (gtdGist.js) should also honor the user's language. Right now /ai/chat gets the language instruction but the gist path does not, so those summaries stay in the email's language. Either behavior is defensible, but it is worth a deliberate call rather than an accidental inconsistency.

The ai.js change itself looks good and is safe: the language value goes through the allowlist and is never interpolated raw into the prompt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants