Skip to content

fix(pipeline): fix malformed logger calls and add remove_audio test coverage#27

Open
Kunal-Somani wants to merge 1 commit intoruxailab:mainfrom
Kunal-Somani:fix/pipeline-logger-and-remove-audio-test
Open

fix(pipeline): fix malformed logger calls and add remove_audio test coverage#27
Kunal-Somani wants to merge 1 commit intoruxailab:mainfrom
Kunal-Somani:fix/pipeline-logger-and-remove-audio-test

Conversation

@Kunal-Somani
Copy link
Copy Markdown

Summary

Two issues found in AudioTranscriptionSentimentPipeline:

Issue 1 — Malformed logger.debug() calls

The pipeline was calling logger.debug() with an f-string as the first argument and the object as a second positional argument:

# Before (broken - extra arg silently ignored)
logger.debug(f"[debug] ... [audio_result]", audio_result)

Python's logging module only interpolates extra args when the format string contains %s placeholders. Passing an object after an f-string means the object is queued as an unused argument — the debug output never included the actual value.

# After (correct)
logger.debug(
    "[Service Layer] [AudioTranscriptionSentimentPipeline] [process] audio_result: %s",
    audio_result
)

This affected all three debug call sites in process() and the error call site for sentiment failures.

Issue 2 — Zero test coverage for remove_audio=True path

The shared fixture hardcodes pipeline.remove_audio = False, meaning os.remove() was never called in any existing test. The branch existed in the source but was completely untested.

Two new tests added:

  • test_process__remove_audio_called_when_enabled: asserts os.remove() is called exactly once with the correct audio path when remove_audio=True
  • test_process__remove_audio_not_called_when_disabled: asserts os.remove() is never called when remove_audio=False

Files Changed

  • app/services/audio_transcription_sentiment_pipeline.py
  • tests/unit/test_services/test_audio_transcription_sentiment_pipeline.py

…overage

- Fix logger.debug() calls in AudioTranscriptionSentimentPipeline.process()
  that were passing objects as positional args after an f-string — the extra
  args were silently ignored, making debug output useless. Replaced with
  correct % formatting: logger.debug('message: %s', obj)

- Add two new unit tests for the remove_audio code path which had zero
  coverage — the shared fixture hardcodes remove_audio=False so os.remove()
  was never exercised:
    * test_process__remove_audio_called_when_enabled: asserts os.remove()
      is called exactly once with the correct audio path when remove_audio=True
    * test_process__remove_audio_not_called_when_disabled: asserts os.remove()
      is never called when remove_audio=False
Kunal-Somani added a commit to Kunal-Somani/sentiment-analysis-api that referenced this pull request Mar 22, 2026
…point

The original input guard used 'if not text' which correctly rejects
None and empty strings but silently passes whitespace-only input
(e.g. '   ') straight to the ML model as valid text. This causes the
model to run inference on meaningless input with no error returned to
the caller.

Fix: replace 'if not text' with 'if not text or not text.strip()' so
whitespace-only strings are rejected with a 400 Bad Request — consistent
with how missing and empty text is already handled.

Also fix logger.error() f-string anti-pattern in the exception handler,
replacing it with % formatting for consistency with the rest of the
service layer (PRs ruxailab#27, ruxailab#28).

Tests added:
- test_sentiment_analyze_whitespace_only_text: verifies '   ' returns
  400 and service is never called
- test_sentiment_analyze_empty_string_text: verifies '' returns 400
  and service is never called
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.

1 participant