Skip to content

feat: pipeline error transparency — no more silent "unknown error" (plan-04, closes #131) - #136

Merged
debpalash merged 4 commits into
mainfrom
001-pipeline-error-transparency
May 29, 2026
Merged

feat: pipeline error transparency — no more silent "unknown error" (plan-04, closes #131)#136
debpalash merged 4 commits into
mainfrom
001-pipeline-error-transparency

Conversation

@debpalash

@debpalash debpalash commented May 29, 2026

Copy link
Copy Markdown
Owner

plan-04 (#131)Closes #131. Refs #122, #63.

Failures in the dub/extract/ingest pipeline used to surface as extract: unknown error with nothing in the backend logs (#122). This makes every failure self-describing — a specific cause + actionable hint + docs deeplink in the UI, a full traceback server-side, and a copyable diagnostic — directly implementing Constitution II (First-Run That Actually Works) and V (errors must be visible).

The keystone

backend/core/failure.py — one shared builder so every emit site produces the same structured event instead of its own str(e):

  • Non-empty reason guarantee: str(exc) → exception class name (fixes the empty/cryptic case)
  • sanitize() reuses the logging_filter HF-token regex + redacts *TOKEN*/*KEY*/*SECRET* env values + home → ~
  • classify() reuses the existing error_docs_map 5-class taxonomy for the docs deeplink + hint
  • diagnostic() reuses the env capture for a copyable, sanitized block

Change sites

  • core/tasks.py worker: structured event instead of bare str(e); keeps the logged traceback
  • services/dub_pipeline.py: enriched download/extract errors; added the missing outer except Exception (the [Bug] Extract: Unknown Error #122 path — unhandled ingest errors were never surfaced with stage context); previously-silent demucs/scene/thumbnail degradations now emit non-fatal warning events
  • api/routers/batch.py: guaranteed non-empty batch reason
  • Frontend: dubSlice (structured dubFailure), useDubWorkflow (captures it from the SSE error event), DubTab (DubFailureNotice → hint + "Open docs" + "Copy diagnostic")

Compatibility

SSE payload is additive — legacy error/stage/detail keys preserved, so older frontends keep working (and already show the specific reason). No DB/schema/engine change.

Tests (TDD, fail-before/pass-after — Constitution V)

  • tests/test_failure_helper.py (10): non-empty reason, redaction, classification, diagnostic
  • tests/test_dub_error_transparency.py (4): the 3 Test-matrix triggers (worker / extract / url) + sanitized diagnostic
  • Backend: 483 passed, 0 regressions (2 unrelated pre-existing failures are supertonic optional-dep not installed locally; CI installs it via --all-extras)
  • Frontend: typecheck ✓, build ✓, 66 tests ✓

Spec/plan/research/data-model/contract live in specs/001-pipeline-error-transparency/.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • UI shows structured failure details with hint/docs links and a “Copy diagnostic” action; prior failure state clears when starting a new run.
  • Bug Fixes

    • Pipeline emits consistent structured error/warning events with sanitized diagnostics to avoid leaking secrets or absolute paths.
  • Tests

    • Added regression and unit tests covering structured failure events, sanitization, and diagnostic content.
  • Documentation

    • New spec, contract, plan, and quickstart for pipeline error transparency.

Review Change Stack

debpalash and others added 3 commits May 29, 2026 08:15
speckit spec/plan/research/data-model/contract/quickstart for plan-04.
Grounds the fix in the real code map: shared failure-event builder
(backend/core/failure.py) feeding tasks.py + dub_pipeline.py + dub_core.py,
non-empty reason guarantee, sanitized diagnostic block, frontend renderer
with docs deeplink. Closes-target: #131 (children #122, #63).

Design only — no code changes yet.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cks (#131)

plan-04 backend: no more silent "unknown error". A shared failure helper
guarantees a non-empty reason at every emit site and a sanitized,
copyable diagnostic block.

- backend/core/failure.py: build_failure()/build_failure_event() (reason
  falls back to the exception class name), sanitize() (reuses the
  logging_filter HF-token regex + redacts *TOKEN*/*KEY*/*SECRET* env values
  + home→~), diagnostic() (reuses the env capture), classify() reusing the
  error_docs_map 5-class taxonomy for the docs deeplink + hint.
- core/tasks.py worker: structured event instead of bare str(e); keeps the
  logged traceback.
- services/dub_pipeline.py: enrich download/extract error yields; ADD the
  missing outer `except Exception` (the #122 path — unhandled ingest errors
  were never surfaced with stage context); surface the previously-silent
  demucs/scene/thumbnail degradations as non-fatal `warning` events.
- api/routers/batch.py: guaranteed non-empty batch failure reason.

SSE payload is additive (legacy `error`/`stage`/`detail` keys preserved),
so existing frontends keep working and already show the specific reason.

Tests (TDD, fail-before/pass-after): 14 cases — non-empty-reason guarantee,
redaction, diagnostic sanitization, and the 3 Test-matrix triggers
(worker / extract / url). 483 passed, 0 regressions.

Closes #131. Refs #122, #63.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ic (#131)

plan-04 frontend. The backend now sends a structured, non-empty failure;
surface it to the user instead of "extract: unknown error".

- dubSlice: DubFailure type + dubFailure state/setter.
- useDubWorkflow: capture the structured failure on the SSE error event
  (reason/error_class/stage/hint/docs_topic/diagnostic); clear on new runs.
- DubTab: DubFailureNotice renders the actionable hint, an "Open docs"
  deeplink (via the existing errorDocsMap classifier), and a "Copy
  diagnostic" button — shown beneath the error badge in both failure banners.

typecheck + build clean; 66 frontend tests pass.

Refs #131, #122, #63.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 902b36e4-573b-4a65-aeea-7007bdb3dd74

📥 Commits

Reviewing files that changed from the base of the PR and between 7152ea3 and 0e19fb9.

📒 Files selected for processing (2)
  • backend/core/failure.py
  • tests/test_dub_error_transparency.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/test_dub_error_transparency.py
  • backend/core/failure.py

📝 Walkthrough

Walkthrough

Adds a centralized failure builder and sanitization, emits structured SSE failure/warning events from worker, ingest, and batch paths, stores failures in frontend state, renders an actionable failure notice with diagnostic copy/docs link, and provides unit and regression tests plus spec docs.

Changes

Pipeline Error Transparency Feature

Layer / File(s) Summary
Failure helper framework and unit tests
backend/core/failure.py, tests/test_failure_helper.py
Introduces classify(), sanitize(), diagnostic(), build_failure() and build_failure_event() to produce sanitized, classified failure dicts and events. Unit tests validate non-empty reasons, redaction of tokens/env, path normalization, diagnostic content, and classification.
Worker exception handling and structured events
backend/core/tasks.py
TaskManager.worker now constructs structured failure events via build_failure_event(..., stage="task"), stores reason in task state, persists failure, and sends the structured SSE payload.
Ingest pipeline error/warning emission
backend/services/dub_pipeline.py
Download and extract failures emit error events built by failure.build_failure(...); demucs/scene/thumbnail degradations emit warning events (diagnostic suppressed). Adds a top-level ingest except that logs and emits a structured stage="ingest" error.
Batch job failure handling
backend/api/routers/batch.py
Batch job failures now set job["error"] to the structured reason from build_failure(..., stage="batch", include_diagnostic=False) instead of a truncated exception string.
Dub failure state management
frontend/src/store/dubSlice.ts
Adds exported DubFailure interface and `dubFailure: DubFailure
Prep SSE parsing and failure state updates
frontend/src/hooks/useDubWorkflow.js
Threads setDubFailure through prep/upload/ingest flows; _waitForPrep parses structured failure events (reason, error_class, stage, hint, docs_topic, diagnostic), stores them via setDubFailure, and rejects with a stage-qualified message. Handlers clear prior failures on start.
Failure notice UI component and rendering
frontend/src/pages/DubTab.jsx, frontend/src/pages/DubTab.css
Adds DubFailureNotice component that shows hint/actions, supports copying diagnostic and opening docs via classifyError/openDocsFor, and integrates into DubTab idle/footer banners. CSS adds .dub-failure-notice styling.
Error transparency regression tests
tests/test_dub_error_transparency.py
Regression tests validate structured error events include non-empty reason, correct error_class/stage, inclusion of sanitized diagnostic, and that secrets (e.g., HF_TOKEN) are not leaked.
Feature specification and planning documents
specs/001-pipeline-error-transparency/*
Adds contract, data model, spec, plan, tasks, quickstart, research notes, and a checklist describing required guarantees, sanitization rules, and verification steps.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: introducing pipeline error transparency to replace silent 'unknown error' messages, directly addressing issue #131.
Description check ✅ Passed The PR description is comprehensive and follows the template structure, covering summary, detailed changes, type (feature), testing results, and checklist items. All key sections are populated.
Linked Issues check ✅ Passed The PR implements all primary objectives from #131: guarantees non-empty failure reasons, adds structured failure events, includes actionable hints and docs deeplinks, logs full tracebacks, provides a copyable diagnostic block, and covers the test matrix (extract, YouTube ingest, WAV-only).
Out of Scope Changes check ✅ Passed All changes are directly aligned with #131's objectives: backend failure handling, frontend error UI, structured event contract, and supporting specs/tests. No unrelated refactoring or scope creep detected.

✏️ 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 001-pipeline-error-transparency

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

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

Comment thread backend/core/failure.py Fixed
Comment thread backend/core/failure.py Fixed
Comment thread backend/core/failure.py Fixed
Comment thread backend/core/failure.py Fixed
Comment thread tests/test_dub_error_transparency.py Fixed
@greptile-apps

greptile-apps Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR introduces a structured failure-reporting layer (core/failure.py) to replace bare str(e) throughout the dub/extract/ingest pipeline, so every error surfaces with a non-empty reason, an actionable hint, a docs deeplink, and a copyable diagnostic block — addressing the "extract: unknown error" / silent-log issues in #122.

  • Backend: failure.build_failure() centralises reason sanitisation, 5-class taxonomy classification, and diagnostic generation; dub_pipeline.py gains the previously-missing outer except Exception for unhandled ingest failures, and demucs/scene/thumbnail degradations now emit non-fatal warning events.
  • Frontend: dubSlice adds a DubFailure state shape; useDubWorkflow populates it from SSE error events; DubTab renders a DubFailureNotice component with hint text, "Open docs", and "Copy diagnostic" actions.
  • Tests: 14 new unit/integration tests cover the non-empty-reason guarantee, redaction, classification, and all three pipeline failure paths.

Confidence Score: 5/5

Safe to merge — additive SSE payload, no schema changes, and the outer except in the pipeline correctly uses return to avoid double-emit.

The core change is well-scoped and well-tested. The two findings are edge-case issues in the new failure helper that do not affect pipeline correctness or the primary error-surfacing goal.

backend/core/failure.py — the classify() HF_AUTH_FAILED over-match and the sanitize() env-var ordering edge case are both in this file.

Important Files Changed

Filename Overview
backend/core/failure.py New centralised failure helper — two edge-case issues in classify() (overly broad HF_AUTH_FAILED) and sanitize() (env-var replacement ordering).
backend/services/dub_pipeline.py Outer except correctly guards against double-emit; warning events emitted but unhandled frontend-side (prior thread).
backend/core/tasks.py Worker now emits structured failure event with traceback preserved.
backend/api/routers/batch.py Batch error now uses guaranteed non-empty reason.
frontend/src/store/dubSlice.ts DubFailure state correctly typed, initialised, and reset.
frontend/src/hooks/useDubWorkflow.js SSE error captures structured failure; reset wired at upload start.
frontend/src/pages/DubTab.jsx DubFailureNotice handles absent hint/diagnostic gracefully.
tests/test_failure_helper.py 10 well-scoped unit tests.
tests/test_dub_error_transparency.py 4 integration tests, race-free listener registration.

Fix All in Claude Code

Reviews (2): Last reviewed commit: "fix(failure): annotate intentional best-..." | Re-trigger Greptile

reject(new Error(`${m.stage || 'prep'}: ${reason}`)); return;
}
case 'cancelled': close(); ctrl.signal.removeEventListener('abort', onAbort); reject(Object.assign(new Error('aborted'), { name: 'AbortError' })); return;
default: break;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Warning events silently dropped — degradations never reach the user

The backend now emits type: "warning" SSE events for demucs, scene-detection, and thumbnail failures, and the PR description explicitly states these "previously-silent degradations now emit non-fatal warning events" to make them visible. However, there is no case 'warning': branch in this switch statement — the event falls through to default: break and is completely discarded. The user still sees no indication that demucs was skipped or scene detection failed, which directly contradicts the stated transparency goal for non-fatal degradations.

Fix in Claude Code

Comment thread backend/core/failure.py
The new security workflow's CodeQL flagged 5 bare `except: pass` blocks.
All are deliberate best-effort guards (sanitize/diagnostic must never throw
on the failure path; the test cancels the worker to tear it down). Added
explanatory comments per CodeQL's py/empty-except rule. No behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/src/store/dubSlice.ts (1)

57-65: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add 'cached' to the exported DubPrepStage union

useDubWorkflow assigns setDubPrepStage('cached'), but DubPrepStage in frontend/src/store/dubSlice.ts does not include 'cached', leaving the public slice/type contract out of sync with reachable state.

Suggested fix
-export type DubPrepStage = 'download' | 'extract' | 'demucs' | 'scene' | null;
+export type DubPrepStage = 'download' | 'extract' | 'demucs' | 'scene' | 'cached' | null;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/store/dubSlice.ts` around lines 57 - 65, The exported union type
DubPrepStage is missing the 'cached' member which causes a type mismatch with
code that calls setDubPrepStage('cached') (see useDubWorkflow); update the
DubPrepStage definition to include 'cached' and ensure the DubSlice interface
(dubPrepStage property) continues to use that union so the public slice/type
contract matches reachable runtime states.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/core/failure.py`:
- Line 38: Update the failure message for the "HF_AUTH_FAILED" entry in
backend/core/failure.py to clarify that the saved Hugging Face login flow is the
primary setup and HF_TOKEN is an override; locate the "HF_AUTH_FAILED"
dictionary entry and change its message to say something like "Use the saved
Hugging Face login flow in Settings → Hugging Face; you can also set HF_TOKEN as
an override and retry." Ensure the text explicitly labels HF_TOKEN as an
override and keeps the same user-visible context.
- Around line 79-85: The current replacement only strips the current user's home
via Path.home(); update the redaction to also scrub common absolute home path
patterns from the output string variable out (e.g., /Users/<name>/,
/home/<name>/, and Windows C:\Users\<name>\) using compiled regexes and re.sub
so other machines' paths are removed; keep the existing Path.home() logic but
add extra regex replacements (case-insensitive for Windows drive letters, handle
both forward and backslashes) in the same block in backend/core/failure.py where
out is modified so all matching cross-platform home paths are replaced with a
consistent token like "~" before returning.

In `@frontend/src/hooks/useDubWorkflow.js`:
- Line 170: The code only clears the structured failure via setDubFailure(null)
in the fresh upload/URL-ingest path; update the retry/transcribe and .srt import
recovery paths to also reset the structured failure so stale hints/docs don't
reappear. Locate the handlers that call setDubStep, setDubError, setDubTracks,
setDubPrepStage (and the retry-transcribe and .srt import functions referenced
near the same block) and add setDubFailure(null) alongside their existing state
resets so every recovery path clears the previous failure state.

In `@specs/001-pipeline-error-transparency/contracts/sse-error-event.md`:
- Around line 26-27: The example's wording is ambiguous: update the SSE error
event example so it clarifies that the "detail" field contains sanitized,
human-readable failure text produced by build_failure() (not raw traceback),
while full traceback details are recorded separately via logger.exception or
similar logging; specifically reference build_failure(), the "detail" field, and
logger.exception/diagnostic to make the separation explicit in the example
wording.

In `@specs/001-pipeline-error-transparency/data-model.md`:
- Around line 35-47: The fenced code block showing the "OmniVoice diagnostic"
needs a language tag to satisfy markdownlint MD040; update the opening fence
from ``` to a tagged fence such as ```text (or another appropriate language) in
the block containing the diagnostic sample so the linter recognizes the block
type while preserving the exact diagnostic contents.
- Around line 14-15: The docs for the `reason` fallback chain (currently:
str(exc) → repr(exc) → type(exc).__name__) are out of sync with the
implementation in backend/core/failure.py; either update the spec or change the
helper that constructs `reason` so it actually falls back from str(exc) to
repr(exc) before using the exception class name. Locate the helper in
backend/core/failure.py that produces the `reason` field (the function that
formats exception messages for the `reason` column) and modify its logic to: try
str(exc), if that is empty or falsy then try repr(exc), and only if that is also
empty use type(exc).__name__; ensure `error_class` remains the exception type
name as documented.

In `@specs/001-pipeline-error-transparency/plan.md`:
- Around line 87-91: Update the plan so the frontend file paths point to the
actual implementation: replace references to api/dub.ts and components/ with the
real files frontend/src/hooks/useDubWorkflow.js and
frontend/src/pages/DubTab.jsx (leave store/dubSlice.ts and utils/errorDocsMap.ts
as-is unless their locations changed); verify any mentions of symbols or
responsibilities (parsing SSE, storing structured failure, rendering
reason/hint/docs link/"Copy diagnostic") map to useDubWorkflow and DubTab and
adjust the plan text accordingly.

---

Outside diff comments:
In `@frontend/src/store/dubSlice.ts`:
- Around line 57-65: The exported union type DubPrepStage is missing the
'cached' member which causes a type mismatch with code that calls
setDubPrepStage('cached') (see useDubWorkflow); update the DubPrepStage
definition to include 'cached' and ensure the DubSlice interface (dubPrepStage
property) continues to use that union so the public slice/type contract matches
reachable runtime states.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 09b50255-0e40-40df-af9a-9c5d9f2022af

📥 Commits

Reviewing files that changed from the base of the PR and between 8162f52 and 7152ea3.

📒 Files selected for processing (18)
  • backend/api/routers/batch.py
  • backend/core/failure.py
  • backend/core/tasks.py
  • backend/services/dub_pipeline.py
  • frontend/src/hooks/useDubWorkflow.js
  • frontend/src/pages/DubTab.css
  • frontend/src/pages/DubTab.jsx
  • frontend/src/store/dubSlice.ts
  • specs/001-pipeline-error-transparency/checklists/requirements.md
  • specs/001-pipeline-error-transparency/contracts/sse-error-event.md
  • specs/001-pipeline-error-transparency/data-model.md
  • specs/001-pipeline-error-transparency/plan.md
  • specs/001-pipeline-error-transparency/quickstart.md
  • specs/001-pipeline-error-transparency/research.md
  • specs/001-pipeline-error-transparency/spec.md
  • specs/001-pipeline-error-transparency/tasks.md
  • tests/test_dub_error_transparency.py
  • tests/test_failure_helper.py

Comment thread backend/core/failure.py
"PKG_RESOURCES_MISSING": "Install setuptools in the backend environment (provides pkg_resources).",
"GATEKEEPER_QUARANTINE": "Clear the macOS quarantine flag (xattr -cr the app), then reopen.",
"APPIMAGE_WEBKIT_WHITESCREEN": "Launch with WEBKIT_DISABLE_DMABUF_RENDERER=1 set.",
"HF_AUTH_FAILED": "Set a valid HF_TOKEN in Settings → Hugging Face and retry.",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Keep HF_TOKEN as an override in this hint.

This wording makes HF_TOKEN sound like the primary setup path, even though the repo rule wants the saved Hugging Face login flow to be primary.

Suggested wording
-    "HF_AUTH_FAILED": "Set a valid HF_TOKEN in Settings → Hugging Face and retry.",
+    "HF_AUTH_FAILED": "Sign in again from Settings → Hugging Face and retry. If needed, `HF_TOKEN` can be used as an override.",
As per coding guidelines, `**/*.py`: “document env var `HF_TOKEN` as override only, not primary mechanism”.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"HF_AUTH_FAILED": "Set a valid HF_TOKEN in Settings → Hugging Face and retry.",
"HF_AUTH_FAILED": "Sign in again from Settings → Hugging Face and retry. If needed, `HF_TOKEN` can be used as an override.",
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/core/failure.py` at line 38, Update the failure message for the
"HF_AUTH_FAILED" entry in backend/core/failure.py to clarify that the saved
Hugging Face login flow is the primary setup and HF_TOKEN is an override; locate
the "HF_AUTH_FAILED" dictionary entry and change its message to say something
like "Use the saved Hugging Face login flow in Settings → Hugging Face; you can
also set HF_TOKEN as an override and retry." Ensure the text explicitly labels
HF_TOKEN as an override and keeps the same user-visible context.

Comment thread backend/core/failure.py
Comment on lines +79 to +85
try:
home = str(Path.home())
if home and home in out:
out = out.replace(home, "~")
except Exception:
pass
return out

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Redact cross-platform home paths, not just Path.home().

This only scrubs the current user's home directory. If an exception includes /Users/<name>/... or C:\Users\<name>\... from another machine/account, detail/diagnostic can still persist a full user-home path.

Possible hardening
+_HOME_PATH_RE = re.compile(
+    r"(?i)(?:\b[A-Z]:\\Users\\[^\\/\s]+|/Users/[^/\s]+|/home/[^/\s]+)"
+)
+
 def sanitize(text: Optional[str]) -> str:
@@
-    try:
-        home = str(Path.home())
-        if home and home in out:
-            out = out.replace(home, "~")
-    except Exception:
-        pass
+    try:
+        home = str(Path.home())
+        if home and home in out:
+            out = out.replace(home, "~")
+    except Exception:
+        pass
+    out = _HOME_PATH_RE.sub("~", out)
     return out
As per coding guidelines, `**/*.{py,rs,js,jsx,ts,tsx}`: “Flag any code that persists or logs values matching *TOKEN*/*KEY*/*SECRET* or absolute user home paths (/Users//, C:\\Users\\\\).”
🧰 Tools
🪛 GitHub Check: CodeQL

[notice] 83-83: Empty except
'except' clause does nothing but pass and there is no explanatory comment.

🪛 Ruff (0.15.14)

[error] 83-84: try-except-pass detected, consider logging the exception

(S110)


[warning] 83-83: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/core/failure.py` around lines 79 - 85, The current replacement only
strips the current user's home via Path.home(); update the redaction to also
scrub common absolute home path patterns from the output string variable out
(e.g., /Users/<name>/, /home/<name>/, and Windows C:\Users\<name>\) using
compiled regexes and re.sub so other machines' paths are removed; keep the
existing Path.home() logic but add extra regex replacements (case-insensitive
for Windows drive letters, handle both forward and backslashes) in the same
block in backend/core/failure.py where out is modified so all matching
cross-platform home paths are replaced with a consistent token like "~" before
returning.

const handleDubUpload = useCallback(async (dubVideoFile) => {
if (!dubVideoFile) return;
setDubStep('uploading'); setDubError(''); setDubTracks([]); setDubPrepStage('download');
setDubStep('uploading'); setDubError(''); setDubFailure(null); setDubTracks([]); setDubPrepStage('download');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clear dubFailure on the other recovery paths too.

Right now only fresh upload/URL-ingest resets the structured failure. If a user recovers from a prep failure via retry-transcribe or .srt import, the old hint/docs/diagnostic stays in store and can reappear under a later unrelated dubError.

Suggested follow-up
  const handleDubRetryTranscribe = useCallback(async () => {
    if (!dubJobId) return;
    const ctrl = new AbortController();
    dubAbortCtrlRef.current = ctrl;
-   setDubError(''); setDubSegments([]); setDubStep('transcribing');
+   setDubError(''); setDubFailure(null); setDubSegments([]); setDubStep('transcribing');
    setTranscribeStart(Date.now());
  const handleDubImportSrt = useCallback(async (file) => {
    if (!dubJobId) {
      toast.error('Upload or ingest a video first — there is no job to attach subtitles to.');
      return;
    }
    if (!file) return;
    try {
-     setDubError('');
+     setDubError('');
+     setDubFailure(null);
      const res = await dubImportSrt(dubJobId, file);

Also applies to: 201-201

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/hooks/useDubWorkflow.js` at line 170, The code only clears the
structured failure via setDubFailure(null) in the fresh upload/URL-ingest path;
update the retry/transcribe and .srt import recovery paths to also reset the
structured failure so stale hints/docs don't reappear. Locate the handlers that
call setDubStep, setDubError, setDubTracks, setDubPrepStage (and the
retry-transcribe and .srt import functions referenced near the same block) and
add setDubFailure(null) alongside their existing state resets so every recovery
path clears the previous failure state.

Comment on lines +26 to +27
"detail": "Traceback summary (sanitized) …",
"diagnostic": "OmniVoice diagnostic\n----…" // sanitized, copyable

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clarify that detail is sanitized message text, not traceback text.

build_failure() currently sets detail from sanitized failure text; traceback lives in logs (logger.exception). Please adjust this example wording to avoid contract confusion.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/001-pipeline-error-transparency/contracts/sse-error-event.md` around
lines 26 - 27, The example's wording is ambiguous: update the SSE error event
example so it clarifies that the "detail" field contains sanitized,
human-readable failure text produced by build_failure() (not raw traceback),
while full traceback details are recorded separately via logger.exception or
similar logging; specifically reference build_failure(), the "detail" field, and
logger.exception/diagnostic to make the separation explicit in the example
wording.

Comment on lines +14 to +15
| `reason` | string | yes | **Non-empty.** Human-readable cause. Fallback chain: `str(exc)` → `repr(exc)` → `type(exc).__name__`. |
| `error_class` | string | yes | Exception type name (e.g. `FileNotFoundError`) or `"Error"`. |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix fallback-chain docs to match the implemented helper.

The model says str(exc) → repr(exc) → type(exc).__name__, but current backend/core/failure.py falls back from str(exc) directly to exception class name. Please align the spec or implementation to keep contract and behavior consistent.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/001-pipeline-error-transparency/data-model.md` around lines 14 - 15,
The docs for the `reason` fallback chain (currently: str(exc) → repr(exc) →
type(exc).__name__) are out of sync with the implementation in
backend/core/failure.py; either update the spec or change the helper that
constructs `reason` so it actually falls back from str(exc) to repr(exc) before
using the exception class name. Locate the helper in backend/core/failure.py
that produces the `reason` field (the function that formats exception messages
for the `reason` column) and modify its logic to: try str(exc), if that is empty
or falsy then try repr(exc), and only if that is also empty use
type(exc).__name__; ensure `error_class` remains the exception type name as
documented.

Comment on lines +35 to +47
```
OmniVoice diagnostic
--------------------
Stage: extract
Error: FileNotFoundError
Reason: ffprobe could not open source: no such file
OS: <platform.platform()>
Python: <sys.version 1-line>
OmniVoice: <version>
CPU/RAM: <psutil summary>
GPU: <cuda/mps/none + VRAM>
Engine: <active TTS engine>
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a language tag to the fenced code block (MD040).

markdownlint flagged this block; adding text (or another suitable language) keeps spec docs lint-clean.

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 35-35: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/001-pipeline-error-transparency/data-model.md` around lines 35 - 47,
The fenced code block showing the "OmniVoice diagnostic" needs a language tag to
satisfy markdownlint MD040; update the opening fence from ``` to a tagged fence
such as ```text (or another appropriate language) in the block containing the
diagnostic sample so the linter recognizes the block type while preserving the
exact diagnostic contents.

Comment on lines +87 to +91
│ ├── api/dub.ts # CHANGE — parse structured error fields from SSE
│ ├── store/dubSlice.ts # CHANGE — store structured failure (reason never empty)
│ ├── utils/errorDocsMap.ts # REUSE (+extend taxonomy only if a new class is needed)
│ └── components/ # CHANGE — render reason + hint + docs link + "Copy diagnostic"
└── ...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update frontend file paths to match the actual implementation.

This plan still points to frontend/src/api/dub.ts and frontend/src/components/, but the implemented flow is in frontend/src/hooks/useDubWorkflow.js and frontend/src/pages/DubTab.jsx. Keeping these paths accurate avoids future drift in follow-up tasks/docs.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/001-pipeline-error-transparency/plan.md` around lines 87 - 91, Update
the plan so the frontend file paths point to the actual implementation: replace
references to api/dub.ts and components/ with the real files
frontend/src/hooks/useDubWorkflow.js and frontend/src/pages/DubTab.jsx (leave
store/dubSlice.ts and utils/errorDocsMap.ts as-is unless their locations
changed); verify any mentions of symbols or responsibilities (parsing SSE,
storing structured failure, rendering reason/hint/docs link/"Copy diagnostic")
map to useDubWorkflow and DubTab and adjust the plan text accordingly.

@debpalash
debpalash merged commit b64f53b into main May 29, 2026
18 of 19 checks passed
@debpalash
debpalash deleted the 001-pipeline-error-transparency branch May 29, 2026 03:42
This was referenced May 29, 2026
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.

[plan-04] Pipeline Error Transparency — no more silent "unknown error"

2 participants