Files
local-transcriber/docs/review-stages-1-3.md
T
ddadminandClaude Opus 4.6 ae2aa5412c docs(review): добавлен отчёт ревью этапов 1-3
- Зачем:
  - фиксация результатов code review для трассируемости.
- Что:
  - добавлен docs/review-stages-1-3.md с findings и статусом исправлений.
- Проверка:
  - cat docs/review-stages-1-3.md.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-17 22:30:59 +03:00

5.6 KiB

Review: Stages 1-3

Executive Summary

Severity Count
CRITICAL 0
HIGH 0
MEDIUM 2
LOW 1

Overall Risk: MEDIUM Recommendation: CONDITIONAL

Key Metrics:

  • Files analyzed: 10/10
  • Lines changed: +427 / -0
  • Test coverage gaps: 3 behaviors
  • High blast radius changes: 0
  • Security regressions detected: 0

What Changed

Commit Range: e0fcf43..WORKTREE Commits: ade3b23, 510d6cc, plus uncommitted stage 3 changes

File +Lines -Lines Risk Notes
pyproject.toml 28 0 LOW Project scaffold and dependency wiring
src/local_transcriber/cli.py 14 0 LOW Step 1 placeholder CLI
src/local_transcriber/formatter.py 23 0 LOW Step 1 placeholder formatter
src/local_transcriber/transcriber.py 99 0 MEDIUM Core transcription flow and fallback logic
src/local_transcriber/utils.py 68 0 MEDIUM Environment checks and input validation
tests/test_transcriber.py 121 0 LOW Mock tests for step 3
tests/test_utils.py 74 0 LOW Unit tests for step 2

Findings

MEDIUM: CUDA fallback does not trigger when model.transcribe() fails before returning a generator

File: src/local_transcriber/transcriber.py:51 Test Coverage: PARTIAL

transcribe() wraps exceptions from model.transcribe(...) into RuntimeError, but the CPU fallback exists only in the later generator-iteration branch. If faster-whisper raises a CUDA or OOM error during model.transcribe(...) itself, the function exits instead of retrying on CPU.

This is a direct mismatch with the step 3 requirement to fall back on CUDA/OOM failures.

Reproduction:

  • Mock WhisperModel(...).transcribe to raise RuntimeError("CUDA kernel launch failed")
  • Current result: RuntimeError("Ошибка при транскрипции файла ...")
  • Expected result: warning + retry on CPU

Recommendation:

  • Apply the same _is_cuda_error() fallback path around model.transcribe(...), not only around iteration of the returned generator
  • Add a test for CUDA failure raised directly by model.transcribe(...)

MEDIUM: get_gpu_name() crashes on empty successful nvidia-smi output

File: src/local_transcriber/utils.py:42 Test Coverage: NO

When subprocess.run(...) returns returncode == 0 with empty stdout, splitlines()[0] raises IndexError. The plan explicitly allows get_gpu_name() to return None, so this path should degrade gracefully instead of crashing.

This will surface later in CLI/device formatting and turn a non-critical metadata lookup into a hard failure.

Reproduction:

  • Mock subprocess.run to return CompletedProcess(..., returncode=0, stdout="")
  • Current result: IndexError: list index out of range
  • Expected result: None

Recommendation:

  • Check stdout.strip() before indexing the first line
  • Add a unit test for empty stdout

LOW: on_segment emits duplicate segments after mid-stream CUDA fallback

File: src/local_transcriber/transcriber.py:63 Test Coverage: NO

If the GPU generator yields one or more segments and then fails with a CUDA error, on_segment is called for the partial GPU output and then called again for the full CPU retry. The returned segments list is reset correctly, but callback side effects are not.

For the planned --verbose flow this means duplicated stderr output such as:

first
first
second

while the final transcript contains only first, second.

Recommendation:

  • Buffer callback output until the segment stream completes successfully, or
  • suppress callback invocation during the first failed attempt and only emit after a successful run

Test Coverage Analysis

Observed coverage: targeted unit tests exist for step 2 and step 3 happy paths, but not for several failure paths.

Untested Changes:

Function Risk Gap
transcribe() MEDIUM No test for CUDA failure raised by model.transcribe(...)
transcribe() LOW No test for duplicate callback behavior after generator fallback
get_gpu_name() MEDIUM No test for empty successful stdout

Blast Radius Analysis

The current blast radius is low because stage 1-3 code is only consumed by placeholder CLI wiring and tests. The highest-impact function is transcribe(), which will become user-facing once stage 5 is implemented.

Function Current Callers Risk Priority
transcribe() tests only MEDIUM P1
get_gpu_name() not yet wired into CLI MEDIUM P1
validate_input_file() tests only LOW P2

Historical Context

  • ade3b23 introduced the scaffold and placeholder module layout for step 1
  • 510d6cc added environment validation and output-path logic for step 2
  • Stage 3 is currently in the working tree and introduces the first non-trivial runtime behavior, including fallback and callback logic

The defects above are all newly introduced in the step 3 working tree implementation, not legacy behavior.

Recommendations

Immediate

  • Fix CUDA fallback for errors raised by model.transcribe(...)
  • Make get_gpu_name() return None on empty stdout instead of raising
  • Add regression tests for both cases

Before Stage 5

  • Decide how on_segment should behave across retries and make the behavior explicit
  • Add one test that covers generator failure after partial output

Residual Risk

  • CLI acceptance for uv run transcribe --help was not reproducible in this environment because uv run transcribe hit a local snap-confine execution issue unrelated to the project code