Skip to content

feat: restore and complete LLM translation feature#70

Merged
cirex-web merged 8 commits into
CMU-313:mainfrom
JoshuaNam:feat/restore-translation
Apr 3, 2026
Merged

feat: restore and complete LLM translation feature#70
cirex-web merged 8 commits into
CMU-313:mainfrom
JoshuaNam:feat/restore-translation

Conversation

@JoshuaNam

Copy link
Copy Markdown

Summary

Restores and completes the translation feature that was disabled in PR #69.

Changes

  • llm/app.py + llm/Dockerfile — Flask microservice calling Ollama qwen2.5:7b for language detection and translation
  • src/translate/index.js — Restored async fetch-based translation (uses TRANSLATOR_URL env var instead of hardcoded URL)
  • src/posts/create.js — Non-blocking post creation; translation runs as fire-and-forget background promise
  • src/posts/data.js — Preserves isEnglish: 'loading' state and adds isTranslating boolean coercion
  • vendor/.../post.tpl — Three-state UI: spinner while translating, toggle button when done, nothing if English
  • public/src/client/topic/events.js — Socket handler for event:post.translated to update DOM in real-time
  • test/translate.js — 5 mock tests for translate module (no real network calls)

Testing

  • All 5 translation tests pass: npx mocha test/translate.js
  • Full test suite: npm run test

Comment thread public/src/client/topic/events.js
Comment thread src/posts/create.js Outdated
const savedTid = tid;
translate.translate(postData).then(([isEng, transContent]) => {
Posts.setPostFields(savedPid, { isEnglish: isEng, translatedContent: transContent, isTranslating: false });
const websockets = require('../socket.io');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah why is this require statement nested in several functions instead of being imported top-level?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah thank you for the catch! Will move it to the top and commit.

Comment thread src/posts/create.js
});
}
}).catch(() => {
Posts.setPostFields(savedPid, { isEnglish: 'true', translatedContent: '', isTranslating: false });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error handling good

@cirex-web cirex-web left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

I just tweaked the translate function to use nconf.get('llm_endpoint') before falling back to localhost. (given that's where all the other configuration variables live, rather than process.env)

Also now the loading icon shows up right after you reply (without needing a refresh)

Comment thread llm/app.py Outdated
@cirex-web
cirex-web merged commit 1a1a28e into CMU-313:main Apr 3, 2026
2 checks passed
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.

2 participants