Deep Fix & Prevention Analysis

Stage handlers should return a typed result object (StageResult) that cannot be constructed without data. The runner should reject None/{} at the framework level, not rely on each handler to remember to check. A StageResult.success(data) factory that validates non-emptiness makes "empty success" impossible to express.

Fallbacks are lies unless they're logged and metered. The real fix is a policy: every fallback path must (a) emit a warning-level log, (b) increment a "degraded" counter, and (c) have its own acceptance criteria (does the fallback actually produce correct results?). An untested, unmonitored fallback is worse than a crash — it gives you wrong answers with high confidence.

The runner's error handling conflated transient errors (network timeout — retry makes sense) with deterministic errors (import error — retry is insane). A proper error taxonomy with at least three classes — RetryLater (transient), PermanentFailure (this item is broken), SystemFailure (the stage itself is broken, stop everything) — would have made the circuit breaker automatic. An import error is always a SystemFailure: retrying won't fix missing code.

in_progress is a lease, not a state. It should have an expiry. Instead of a bare status flag, use claimed_at timestamp + claimed_by worker ID. Any item claimed >N minutes ago by a dead worker is automatically released. This makes crash recovery continuous rather than requiring a restart — and it works in multi-worker scenarios where one worker dies but others are still alive.

Don't store the same concept in two places. Having status as a column AND stages.fetch: "in_progress" inside a JSON column means two sources of truth for "is this item being worked on?" They can (and did) disagree. The stage-level status should be the only source; derive the item-level status from it. Single source of truth, computed views.

Stages should declare their preconditions as assertions, not assumptions. If extract requires a VTT file, it should check for its existence before doing any work and raise a clear PreconditionFailed("VTT file not found at {path}"). The runner can then distinguish "precondition not met" (wait for upstream) from "stage failed" (something broke). This is the guard clause pattern applied to pipeline stages.

The runner's None → {} conversion is the root bug. The framework should never coerce a handler's return value into something the handler didn't intend. None means "I have nothing to say" — the framework should treat that as an error, not as "success with empty data." The fix is: if result is None: raise StageError("Handler returned None") in the runner itself.

Subprocess wrappers must parse exit codes AND output to determine success. A generic run_subprocess() wrapper that returns stdout or {} on any error is dangerous. The wrapper should return a SubprocessResult(stdout, stderr, exit_code) and let the caller decide what constitutes success. Never swallow subprocess failures at the wrapper level.

This is a language-level footgun in SQLite. The deep fix is a convention: never use INSERT OR REPLACE — always use INSERT ... ON CONFLICT DO UPDATE (UPSERT). This should be enforced by a lint rule or code review checklist. Better yet, wrap all DB writes through a db.upsert(table, data, conflict_keys) helper that always uses the safe pattern.

Migrations must migrate data, not just schema. The schema migration (add results table) was incomplete because it didn't backfill existing items. A migration that changes where data lives without moving existing data creates a split-brain: some items in the old location, some in the new. The migration script should have included a backfill_from_disk_to_db() step.

Content validation should be semantic, not structural. Length comparison catches exact duplicates but misses near-duplicates. The real fix is a content fingerprint: hash the text content (stripped of boilerplate) and flag when multiple items share the same hash. Better yet, the fetch stage should verify that the fetched content actually relates to the requested entity — a cheap LLM check ("Is this about {company_name}?") catches both duplicates and wrong-content errors.

Connection lifecycle should be managed by one owner, not scattered across callers. Use a context-manager-based connection pool or a "unit of work" pattern where every DB operation happens within a with db_session() as s: block. The connection is opened, used, and closed in one scope — it can't leak or be used after close because the variable is scoped to the with block.

Two deep failures here. (1) asyncio.gather(return_exceptions=True) is a silent-failure factory — it converts crashes into list elements that nobody checks. Use a wrapper that logs/raises if any result is an exception. (2) Renaming without grep is a time bomb. Every rename should be accompanied by a project-wide search for the old name. IDE refactoring tools do this; manual renames don't.

Error storage should be append-only, not overwrite. Use an error log (array of {stage, error, timestamp}) instead of a single error field. This is the same principle as audit logs vs current state: you want the full history of what went wrong, not just the last thing.

Async is viral — changing one function to async requires changing all callers. The deep prevention is: when converting a function to async, immediately search for all call sites and convert them too. Better yet, design the system as async-from-the-start if any component might need it. Retrofitting async is always painful.

External tools have their own definition of "success" that doesn't match yours. The subprocess wrapper should define success criteria per-tool, not assume exit code 0 = success. For yt-dlp: success = "stdout contains valid JSON." For curl: success = "HTTP 200." The tool's exit code is a hint, not a contract.

Ban bare asyncio.gather(return_exceptions=True). Replace with a project utility: gather_logged(*coros, logger=log) that automatically inspects results and logs/counts any exceptions. Make the safe version the easy version.

A stage that succeeds without its primary input is lying. Every stage should have a "minimum viable input" assertion. If metadata enrichment needs a transcript and there isn't one, it should skip (not succeed) or fail (not silently produce empty enrichment). The broader principle: distinguish "nothing to do" from "did something successfully."

Structural checks are necessary but never sufficient for content from adversarial or unreliable sources. The deep fix is a layered validation architecture: (1) structural check (is it HTML?), (2) content fingerprint (does it look like previous valid responses?), (3) semantic check (does it contain expected entities/concepts?). Each layer catches what the previous misses. The key insight: the more sophisticated the source's failure modes, the more semantic your validation must be.

String matching for error detection is a game of whack-a-mole. Each new rate-limit message requires a new string. The semantic approach (LLM: "Is this a rate-limit/error page?") catches all variants with one check. For lower cost, use a classifier trained on the corpus of known error pages. The principle: match on semantics, not syntax, when the source can vary its syntax.

Bot detection evasion requires multiple signals, not just IP rotation. Modern anti-bot systems check IP reputation + browser fingerprint + cookie state + behavioral patterns. The deep fix is an escalation ladder: try cheapest approach first (direct), escalate to proxy, escalate to proxy + cookies, escalate to full browser. Each level has a cost — track which level each target requires and auto-select.

7 call sites with the same flag means the flag should be in one place. Create a ytdlp_command(url, **kwargs) -> list[str] factory that always includes the base flags. Individual callers add only their specific flags. DRY for command construction, not just for code.

External API changes are inevitable — design for them. The system should have a "YouTube health" check that periodically tests a known-good download and alerts when it fails. This detects platform changes before they affect the pipeline, not after 100 items fail. The principle: canary requests detect environmental changes before they cause damage.

Error classification should be a lookup table, not scattered conditionals. Maintain a mapping: {SSLError: RETRY, TimeoutError: RETRY, HTTPError(404): PERMANENT, HTTPError(429): RETRY, HTTPError(500): RETRY, ValueError: PERMANENT}. New error types get an explicit classification. Unknown errors default to RETRY with a logged warning — you don't know if it's transient, so assume it might be.

The circuit breaker should distinguish "something is broken" from "we're going too fast." Rate limits aren't failures — they're flow control. The circuit breaker should only trigger on non-rate-limit consecutive failures. Rate limits should trigger backoff (slow down), not shutdown (stop). Two different mechanisms for two different problems.

Lint for sync I/O inside async functions. Any httpx.get(), requests.get(), open(), or time.sleep() inside an async def is a bug. A ruff/flake8 rule or a runtime warning when the event loop is blocked >100ms catches these mechanically.

Proxy should be the default, not an opt-in. Create a project-wide HTTP client factory: get_http_client(proxy=True) where proxy defaults to on. A whitelist of domains that don't need proxy (localhost, LLM APIs). Any new domain automatically gets proxied. The footgun of forgetting to proxy disappears.

Document the auth flow before implementing. VIC's two-layer cookie system is discoverable by reading network traffic, but it was discovered by trial and error. For any authenticated target, spend 30 minutes with browser DevTools documenting the full auth flow before writing code. The cost of understanding upfront is far less than the cost of debugging auth failures in production.

Same as "yt-dlp non-zero with valid output" — per-tool success criteria. The yt-dlp wrapper should have a classify_result(stdout, stderr, exit_code) -> Success | Warning | AuthError | ContentError function that encodes all the tool's quirks in one place. Callers get clean typed results.

State transitions should be atomic and complete. "Retry" is not just "reset item status" — it's a compound operation: reset items + unpause job + clear circuit breaker counter. If any step is missing, the system is in an inconsistent state. Model operations as state machines where every transition specifies the full target state, not just one field.