TTS Evaluation: Fix processing issue

[AkhileshNegi]

Summary

Target issue is #701

Checklist

Before submitting a pull request, please ensure that you mark these task.

• Ran fastapi run --reload app/main.py or docker compose up in the repository root and test.
• If you've fixed a bug or added code that is tested and has test cases.

Summary by CodeRabbit

• Bug Fixes

  • Fixed signed URL generation for TTS evaluation results so signed URLs are only produced when a valid object store URL is present, preventing incorrect or empty signed URLs from appearing.

• Tests

  • Added test coverage verifying that signed URLs are returned for results with storage URLs and remain null for results without an object store URL.

[coderabbitai]

📝 Walkthrough

Walkthrough

Replaces a custom HTTPException import with FastAPI's fastapi.HTTPException and changes signed-URL generation to only call the storage signer when both a storage instance and a non-empty object_store_url are present. Adds a test to verify signed-url behavior for null vs. non-null URLs.

Changes

Cohort / File(s)	Summary
Import Update backend/app/core/batch/client.py	Swapped import of HTTPException from app.core.exception_handlers to fastapi.HTTPException; no behavioral changes.
Signed URL Generation Logic backend/app/crud/tts_evaluations/result.py	get_results_by_run_id now only calls storage.get_signed_url(...) when storage is present AND result.object_store_url is truthy, avoiding signer calls for null/empty URLs.
Test Coverage backend/app/tests/api/routes/test_tts_evaluation.py	Added test test_get_run_with_signed_urls_and_null_object_store_url to ensure signed URLs are returned only for results with non-null object_store_url and that signer is not called for null entries.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• TTS Evaluation: TTS Results signed URLs  #666: Related change to signed URL generation and handling in TTS result flows.

Suggested labels

bug

Suggested reviewers

• kartpop
• vprashrex

Poem

🐰 I hop through code and check each store,
If URLs are empty I skip at the door.
Signed links appear where strings are true,
Clean imports and tests—hip-hop hooray, woo! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'TTS Evaluation: Fix processing issue' is vague and does not clearly convey the specific nature of the changes. While it references TTS Evaluation, it uses the generic term 'processing issue' which fails to indicate the actual fix: resolving a circular import problem and correcting signed URL generation logic.	Revise the title to be more specific, such as 'TTS Evaluation: Fix circular import and signed URL generation' to clearly communicate the actual changes made.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch hotfix/tts-evals

📝 Coding Plan

• Generate coding plan for human review comments

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[AkhileshNegi]

This is why CI never caught the circular import conftest.py runs first in pytest. It imports app.main, which pulls in app.utils and fully initializes it. So by the time pytest gets to test_gemini_client.py (which imports app.core.batch.client → app.core.exception_handlers → app.utils), app.utils is already sitting in sys.modules and everything looks fine.

The problem only shows up when app.core.batch.client is the first thing imported. That’s exactly what happens in the Celery worker. It starts fresh and uses importlib.import_module() on batch_result_processing.py, without ever touching app.main. Now app.utils isn’t initialized yet, and the circular import breaks.

In short: pytest hides the issue because conftest.py preloads app.utils through app.main. The Celery worker takes a different path and hits the cycle head-on. This is a pretty common blind spot.

[AkhileshNegi]

Evaluation completed after this fix