Core: Improve error messages in translator modules #166

DongilMin · 2025-06-29T13:52:19Z

Purpose

Improves error messages across core translator modules to provide better user experience and actionable feedback when errors occur.

Description

This PR addresses issue #46 by enhancing error messages in three main translator modules:

Changes Made:

MarkdownTranslator: Added file names to timeout and translation errors, improved LLM provider configuration messages
ProjectTranslator: Enhanced Computer Vision configuration messages with specific environment variable guidance
ImageTranslator: Improved text detection errors and API failure messages with file names and resolution hints

Key Improvements:

All error messages now include specific file names
Added actionable resolution hints (e.g., "Check your .env file", "Install required font")
Provided clear error causes instead of generic messages
Maintained existing code logic while only updating message text

Related Issue

Fixes #46

Does this introduce a breaking change?

No

Type of change

Feature
Code style update (formatting with Black)

Checklist

I have thoroughly tested my changes: All 9 new error message tests pass, plus existing 16 tests
All existing tests pass: Verified no breaking changes to existing functionality
I have added new tests: Added comprehensive test suite in test_error_messages.py
I have followed the Co-op Translators coding conventions: Applied Black formatting
I have documented my changes: Clear error messages serve as self-documentation

Additional context

Added 9 comprehensive tests covering all error message improvements
All tests pass successfully
No breaking changes to existing API or functionality
Error messages now follow pattern: Action failed for 'filename': Specific cause. Resolution hint.

- Add file names to timeout and translation error messages - Include specific error causes and resolution hints - Add md_file_path parameter to _run_prompts_sequentially function - Improve ValueError messages for LLM provider configuration - Enhance user experience with actionable error information Part of Azure#46

- Add project name context to all error messages - Include file counts and specific error causes - Provide actionable resolution hints for API and file issues - Improve Computer Vision configuration messages - Enhance retranslation error messages with file names - Add specific guidance for common failure scenarios Part of Azure#46

- Add file names to all text detection and processing errors - Include specific error causes for bounding box issues - Provide actionable resolution hints for font and API problems - Enhance Computer Vision configuration error with specific variables - Improve user experience with detailed image processing feedback Part of Azure#46

- Test MarkdownTranslator error messages for LLM provider issues - Test ProjectTranslator Computer Vision configuration messages - Test ImageTranslator error handling and file processing - Verify improved error messages include file names and resolution hints - Add integrated scenarios for missing API services Related to Azure#46

- Format all modified files according to project standards - Ensure consistent code style across translator modules - Maintain line length and formatting requirements Part of Azure#46

DongilMin · 2025-06-29T13:57:24Z

Confirmed that all existing tests related to the updated files pass successfully:

test_markdown_translator.py (2/2 passed)
test_project_translator.py (3/3 passed, 1 non-critical warning)
test_image_translator.py (11/11 passed)

Note on warning:
One non-critical warning appeared during test_project_translator.py, related to an AsyncMock not being awaited. This is a common harmless warning in Python 3.10+ when mocking async functions. The test still passed as expected, and the behavior is unaffected.

This confirms that the improvements in error messages do not introduce any regressions.
I've also attached a screenshot of the test results for reference.

DongilMin · 2025-06-29T14:01:25Z

Uploaded screenshot of test_error_messages.py results
All 9 tests passed successfully

This test suite was newly added to validate the improved error messages introduced in this PR.
Each test case ensures that error messages are:

Informative: Includes the specific cause of failure (e.g., missing API key, invalid configuration)
Actionable: Suggests clear resolution steps (e.g., "Check your .env file", "Unsupported LLM provider")
Contextual: Includes file names where relevant to help pinpoint the source of the issue

DongilMin · 2025-06-29T14:03:20Z

@microsoft-github-policy-service agree

src/co_op_translator/core/llm/markdown_translator.py

src/co_op_translator/core/vision/image_translator.py

tests/co_op_translator/core/test_error_messages.py

skytin1004 · 2025-07-03T05:12:18Z

Hi @DongilMin, thanks for your contribution!
Could you please take a look at my suggestion and update accordingly?

DongilMin · 2025-07-04T02:09:55Z

@skytin1004 Thanks for

Hi @DongilMin, thanks for your contribution! Could you please take a look at my suggestion and update accordingly?

@skytin1004 Thanks for the review! I'll take a look and make the updates shortly.

…st integration

DongilMin · 2025-07-04T13:54:48Z

@skytin1004 I've addressed your feedback. Could you please review when you have a chance? Thanks!

skytin1004 · 2025-07-04T23:42:11Z

tests/co_op_translator/core/project/test_project_translator.py

+                    log_text = caplog.text
+                    # Check the actual log message format
+                    assert "Computer Vision not configured" in log_text
+                    assert "AZURE_COMPUTER_VISION_KEY" in log_text


Hi, I reviewed this code and think it might be better to remove this section. We're considering adding log assertion logic to each individual scenario instead.

After removing this entire section, I'm planning to merge it if tests pass.

Hi, I reviewed this code and think it might be better to remove this section. We're considering adding log assertion logic to each individual scenario instead.

@skytin1004
I misunderstood your feedback initially! 😅
Just realized you wanted me to remove the caplog assertions entirely. Fixed it now - removed all the caplog parameters and logging checks from the error message tests.

- Removed caplog parameters and caplog.set_level() calls - Focused tests on core functionality verification - Addresses reviewer feedback to use individual scenario-based log assertions

skytin1004 · 2025-07-09T09:38:22Z

Hi @DongilMin, thanks for the update and sorry for the confusion

I think there's still a bit of misunderstanding: I was actually suggesting that we remove all changes made to tests/co_op_translator/core/project/test_project_translator.py, not just the caplog assertions.

Right now, I think it's better to completely remove this section and revisit how we want to add log assertion logic later.

skytin1004 · 2025-07-09T09:40:13Z

My current thinking is that instead of creating a separate test function solely for log message assertions, we should integrate log assertions directly into the relevant test cases, such as test_translate_markdown. This keeps the test context clearer and the structure more modular.

So for now, I'd suggest fully removing the changes to this file (test_project_translator.py), and we can explore the best way to structure log-related tests in a separate PR. If needed, we can also open an issue to discuss and align on a consistent testing style.

- Delete tests/co_op_translator/core/project/test_project_translator.py - Delete tests/co_op_translator/core/project/test_translation_manager.py

- Revert test_project_translator.py to original state - Revert test_translation_manager.py to original state

- Restore original formatting in markdown_evaluator.py - Restore original formatting in project_evaluator.py - Remove unintended formatting changes from our PR scope

DongilMin · 2025-07-09T10:59:06Z

My current thinking is that instead of creating a separate test function solely for log message assertions, we should integrate log assertions directly into the relevant test cases, such as test_translate_markdown. This keeps the test context clearer and the structure more modular.

So for now, I'd suggest fully removing the changes to this file (test_project_translator.py), and we can explore the best way to structure log-related tests in a separate PR. If needed, we can also open an issue to discuss and align on a consistent testing style.

@skytin1004 Thanks for the clarification!
Just went through everything again and made the changes you suggested:

Fully removed all changes to test_project_translator.py
Reverted test_translation_manager.py to its original state
Cleaned up the unintended Black formatting changes in markdown_evaluator.py and project_evaluator.py – they were touched during development but are now back to their original state

At this point, the PR only includes the core error message improvements originally intended – no test changes, no formatting noise.

skytin1004 · 2025-07-09T11:14:12Z

@DongilMin
Just to clarify, I actually meant reverting changes only to test_project_translator.py,
The rest of your changes look great. I love the improvements you've made!

skytin1004 · 2025-07-09T11:21:02Z

@DongilMin Sorry I actually only meant to remove the changes in test_project_translator.py.
Could you revert the Remove log assertion logic from error message tests commit and just remove the changes in test_project_translator.py?

…lator.py only

tests/co_op_translator/core/project/test_project_translator.py

DongilMin · 2025-07-09T12:21:31Z

@DongilMin Sorry I actually only meant to remove the changes in test_project_translator.py. Could you revert the Remove log assertion logic from error message tests commit and just remove the changes in test_project_translator.py?

@skytin1004
Sorry, my English is still not so good haha 😅 I removed it now! would you mind taking a look when you get a chance?
Thank you

skytin1004 · 2025-07-09T12:29:47Z

tests/co_op_translator/core/project/test_project_translator.py

@DongilMin
Looks great overall, thanks again for the thorough updates!

Just one small thing: it looks like this file is using CRLF line endings.
Could you please change it to LF to match the rest of the project?

If you're using VS Code, you can check this in the bottom-right corner
just click CRLF and switch to LF, then save the file. And push your changes

@DongilMin Looks great overall, thanks again for the thorough updates!

Just one small thing: it looks like this file is using CRLF line endings. Could you please change it to LF to match the rest of the project?

If you're using VS Code, you can check this in the bottom-right corner just click CRLF and switch to LF, then save the file. And push your changes

@skytin1004

@DongilMin Looks great overall, thanks again for the thorough updates!

Just one small thing: it looks like this file is using CRLF line endings. Could you please change it to LF to match the rest of the project?

If you're using VS Code, you can check this in the bottom-right corner just click CRLF and switch to LF, then save the file. And push your changes

@skytin1004 I'm currently using LF endings in VS Code, and ran Black to ensure formatting is clean.
The only remaining diff is a single trailing newline at the end of the file — should I remove that as well?
Thank you.

tests/co_op_translator/core/project/test_project_translator.py

skytin1004

Thanks @DongilMin, LGTM!
I went ahead and cleaned up the remaining line ending diff.
Merging this in, great work on the error message improvements!

DongilMin added 5 commits June 29, 2025 21:27

Apply code formatting with Black

468d165

- Format all modified files according to project standards - Ensure consistent code style across translator modules - Maintain line length and formatting requirements Part of Azure#46

github-actions bot added core Related to any changes in core source files tests labels Jun 29, 2025

chore(test): update file-level docstring in test_error_messages.py

be1ddc3

DongilMin changed the title ~~Improve error messages in translator modules (#46)~~ Improve error messages in translator modules Jun 29, 2025

skytin1004 requested changes Jul 3, 2025

View reviewed changes

src/co_op_translator/core/llm/markdown_translator.py Show resolved Hide resolved

src/co_op_translator/core/vision/image_translator.py Outdated Show resolved Hide resolved

src/co_op_translator/core/vision/image_translator.py Outdated Show resolved Hide resolved

skytin1004 reviewed Jul 3, 2025

View reviewed changes

tests/co_op_translator/core/test_error_messages.py Outdated Show resolved Hide resolved

Fix reviewer feedback: timeout constant, simplified error message, te…

4728fb3

…st integration

DongilMin force-pushed the improve-error-messages-issue-46 branch from 090f94f to 4728fb3 Compare July 4, 2025 13:41

skytin1004 requested changes Jul 4, 2025

View reviewed changes

DongilMin added 2 commits July 5, 2025 14:18

Remove test_environment_variable_references_in_logs method

8a6306a

Remove log assertion logic from error message tests

82f45ff

- Removed caplog parameters and caplog.set_level() calls - Focused tests on core functionality verification - Addresses reviewer feedback to use individual scenario-based log assertions

DongilMin added 2 commits July 9, 2025 19:24

Remove test files per reviewer feedback

235fc05

- Delete tests/co_op_translator/core/project/test_project_translator.py - Delete tests/co_op_translator/core/project/test_translation_manager.py

Restore original test files in project module

0406c6e

- Revert test_project_translator.py to original state - Revert test_translation_manager.py to original state

github-actions bot removed the tests label Jul 9, 2025

Revert Black formatting for unmodified evaluator files

919b511

- Restore original formatting in markdown_evaluator.py - Restore original formatting in project_evaluator.py - Remove unintended formatting changes from our PR scope

DongilMin added 2 commits July 9, 2025 20:33

Reapply Black formatting to evaluator files

2fda0a9

Restore test_project_translator.py to pre-82f45ff state as requested

71e68ab

github-actions bot added the tests label Jul 9, 2025

Revert log assertion removal commit, then clean up test_project_trans…

fd195da

…lator.py only

skytin1004 reviewed Jul 9, 2025

View reviewed changes

tests/co_op_translator/core/project/test_project_translator.py Outdated Show resolved Hide resolved

Remove TestProjectTranslatorErrorMessages class entirely as request

90c41d0

skytin1004 reviewed Jul 9, 2025

View reviewed changes

Add missing newline at end of file

36e3699

skytin1004 reviewed Jul 9, 2025

View reviewed changes

tests/co_op_translator/core/project/test_project_translator.py Outdated Show resolved Hide resolved

Update tests/co_op_translator/core/project/test_project_translator.py

83060d2

github-actions bot removed the tests label Jul 9, 2025

skytin1004 approved these changes Jul 9, 2025

View reviewed changes

skytin1004 changed the title ~~Improve error messages in translator modules~~ Core: Improve error messages in translator modules Jul 9, 2025

skytin1004 merged commit 7d7055d into Azure:main Jul 9, 2025
2 checks passed

Core: Improve error messages in translator modules #166

Core: Improve error messages in translator modules #166

Uh oh!

Conversation

DongilMin commented Jun 29, 2025

Purpose

Description

Changes Made:

Key Improvements:

Related Issue

Does this introduce a breaking change?

Type of change

Checklist

Additional context

Uh oh!

DongilMin commented Jun 29, 2025

Uh oh!

DongilMin commented Jun 29, 2025

Uh oh!

DongilMin commented Jun 29, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skytin1004 commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DongilMin commented Jul 4, 2025

Uh oh!

DongilMin commented Jul 4, 2025

Uh oh!

skytin1004 Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

skytin1004 Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

DongilMin Jul 5, 2025

Choose a reason for hiding this comment

Uh oh!

skytin1004 commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skytin1004 commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DongilMin commented Jul 9, 2025

Uh oh!

skytin1004 commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skytin1004 commented Jul 9, 2025

Uh oh!

Uh oh!

DongilMin commented Jul 9, 2025

Uh oh!

skytin1004 Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

DongilMin Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

skytin1004 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

skytin1004 commented Jul 3, 2025 •

edited

Loading

skytin1004 commented Jul 9, 2025 •

edited

Loading

skytin1004 commented Jul 9, 2025 •

edited

Loading

skytin1004 commented Jul 9, 2025 •

edited

Loading