feat: expose retryable scheduler outcome telemetry

[danecor]

Summary

• classify retryable scheduler outcomes as rate limits, admission timeouts, request timeouts, server errors, connection failures, or other retryable failures
• expose sanitized rolling and cumulative category counts in scheduler health metrics
• distinguish non-retryable failures from successful outcomes
• report deferred task depth once at the scheduler-health top level
• log category-specific warnings without leaking provider error messages or request data

Why

Clients need to distinguish provider backpressure from slow or unhealthy inference so they can tune concurrency and timeouts without treating retryable row outcomes as fatal dataset failures. Accurate success and non-retryable-failure counts also prevent permanent failures from being hidden in operational telemetry.

Validation

• uv run --group dev pytest packages/data-designer-engine/tests/engine/dataset_builders/test_async_scheduler.py -k "retryable_outcome_metrics or degraded_provider_warn" -q — 11 passed
• make check-engine — 328 files formatted; Ruff clean
• make test-engine — 2,215 passed

Supersedes #831 with the implementation ported to Data Designer v0.8.0 and hardened from review feedback.

[greptile-apps]

Greptile Summary

This PR adds fine-grained telemetry for retryable scheduler outcomes in AsyncTaskScheduler. It classifies LLM task failures into named categories (rate limit, admission timeout, request timeout, internal server error, connection failure, other retryable), tracks rolling and cumulative counts per category, extracts safe HTTP transport metadata (status_code, retry_after) from the exception chain, and deduplicates first-occurrence category warnings.

• _retryable_error_kind maps exception type to a string category; _provider_error_details walks the exception chain to extract sanitized HTTP metadata without leaking message text.
• retryable_outcome_metrics exposes a snapshot of window-bounded rolling counts and unbounded cumulative counts as a structured dict embedded in the health diagnostics blob.
• Three new targeted tests verify classification, exception-chain traversal for suppressed ProviderError context, and the non_retryable_failure vs success split introduced to fix a prior review finding.

Confidence Score: 5/5

Safe to merge — the change is additive telemetry-only with no effect on scheduling or task execution paths.

The scheduling and task-dispatch logic is untouched; the new code only populates counters and logs warnings. All prior review concerns are addressed with regression tests. The exception-chain traversal is cycle-safe, the parallel deques stay in lockstep, and provider message text is demonstrably absent from both metrics and logs.

No files require special attention.

Important Files Changed

Filename	Overview
packages/data-designer-engine/src/data_designer/engine/dataset_builders/async_scheduler.py	Adds four new instance fields, a retryable_outcome_metrics property, and two static helpers. Updates _record_retryable_outcome to classify outcomes, populate counters, walk the exception chain safely, and emit deduplicated per-category warnings. Logic is correct; parallel deques are kept in sync; no provider text leaks into counters or logs.
packages/data-designer-engine/tests/engine/dataset_builders/test_async_scheduler.py	Adds 10 new test cases covering parametrized exception-class classification, suppressed-context ProviderError HTTP metadata extraction, and non-retryable-failure vs success categorization. Tests also assert deferred_tasks placement and verify no sensitive text leaks into metrics or logs.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["_record_retryable_outcome(retryable, exc)"] --> B{retryable?}
    B -- yes --> C["_retryable_error_kind(exc)"]
    B -- no --> D{exc is not None?}
    D -- yes --> E["kind = non_retryable_failure"]
    D -- no --> F["kind = success"]
    C --> G["_retryable_outcome_counts[kind] += 1"]
    E --> G
    F --> G
    C --> H["_provider_error_details(exc) walk chain"]
    H --> I["_retryable_detail_counts[kind:http_N] += 1"]
    H --> J{kind in _logged_retryable_kinds?}
    J -- no --> K["logger.warning + add to set"]
    J -- yes --> L["skip"]
    G --> M{window disabled?}
    M -- yes --> N["return"]
    M -- no --> O["append to both deques"]
    O --> P{threshold met?}
    P -- yes --> Q["degraded provider warning"]
    P -- no --> R["return"]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["_record_retryable_outcome(retryable, exc)"] --> B{retryable?}
    B -- yes --> C["_retryable_error_kind(exc)"]
    B -- no --> D{exc is not None?}
    D -- yes --> E["kind = non_retryable_failure"]
    D -- no --> F["kind = success"]
    C --> G["_retryable_outcome_counts[kind] += 1"]
    E --> G
    F --> G
    C --> H["_provider_error_details(exc) walk chain"]
    H --> I["_retryable_detail_counts[kind:http_N] += 1"]
    H --> J{kind in _logged_retryable_kinds?}
    J -- no --> K["logger.warning + add to set"]
    J -- yes --> L["skip"]
    G --> M{window disabled?}
    M -- yes --> N["return"]
    M -- no --> O["append to both deques"]
    O --> P{threshold met?}
    P -- yes --> Q["degraded provider warning"]
    P -- no --> R["return"]

Loading

_{Reviews (4): Last reviewed commit: "test: cover retryable provider status te..." | Re-trigger Greptile}

[github-actions]

Code Review — PR #833: Expose retryable scheduler outcome telemetry

Author: danecor (Dane) · Base: main · Head: dcorneil/v0.8-retryable-outcome-telemetry
Size: +121 / −5 across 3 files · State: OPEN

Summary

The PR enriches AsyncTaskScheduler's degraded-provider telemetry. Previously the scheduler only tracked a rolling boolean retryable/not-retryable rate. This change:

• Classifies each retryable model-task failure into one of six kinds (rate_limit, request_admission_timeout, timeout, internal_server, connection, other_retryable) via _retryable_error_kind.
• Extracts safe transport metadata only (status_code, retry_after) from the wrapped ProviderError chain via _provider_error_details, deliberately avoiding provider error text.
• Exposes rolling + cumulative counts through a new retryable_outcome_metrics property, surfaced in _scheduler_health_diagnostics.
• Emits a one-time-per-kind WARN on first observation, and enriches the existing rate-limited degraded-performance WARN with per-kind breakdowns.
• Adds a parametrized test asserting classification and no message leakage.
• Adds [tool.uv.sources] workspace pins to root pyproject.toml.

The privacy-conscious design (metadata-only extraction, explicit leak-check test) is the strongest part of this change and aligns well with the "don't leak provider error messages or request data" goal in the PR description.

Findings

1. success bucket conflates genuine successes with non-retryable failures — MEDIUM

_record_retryable_outcome is called from two sites:

• Success path (async_scheduler.py:2000): retryable=False, no exc.
• Failure path (async_scheduler.py:2025): retryable=retryable, exc=exc.

In _record_retryable_outcome:

kind = self._retryable_error_kind(exc) if retryable else "success"
self._retryable_outcome_counts[kind] += 1

When a non-retryable failure occurs (auth error, schema error, internal bug), retryable=False, so kind = "success". That failure is then counted in cumulative_counts["success"].

For the rolling rate, treating non-retryable-failure as 0 (same as success) is correct and matches the docstring. But this PR now surfaces a bucket literally labeled success in cumulative_counts, and it over-counts: cumulative_counts["success"] == genuine_successes + non_retryable_failures. A client reading this telemetry to "distinguish provider backpressure from slow/unhealthy inference" (the stated goal) will see hard failures reported as successes. Consider a distinct label (non_retryable) for the failure-but-not-retryable case, or a separate bucket. This is the one substantive correctness concern.

2. status_code/retry_after extraction relies on __context__, which is worth a comment — LOW

_provider_error_details walks current.__cause__ or current.__context__. The Model*Error subclasses are raised via raise error_cls(...) from None in errors.py:_raise_from_provider_error, which sets __cause__ = None and __suppress_context__ = True — but leaves __context__ pointing at the originating ProviderError (because the raise happens inside the except Exception as e: in catch_llm_exceptions). So the walk does find the ProviderError via __context__, and the http_{status_code} detail suffix works in production.

This is correct but non-obvious and fragile: it depends on the raise staying inside the handling except block. A one-line comment on _provider_error_details explaining that provider metadata is reached through the suppressed-but-present __context__ would protect it against a future refactor that moves the re-raise out of the except scope (which would silently null out all status_code/retry_after telemetry). The cycle guard (seen set) is a nice touch.

3. Test coverage gaps — LOW

The new test_retryable_outcome_metrics_classify_errors covers the five explicit kinds and the leak check — good. Not covered:

• The http_{status_code} detail suffix path (finding DataDesigner.make_seed_reference_from_file doesn't support paths with multiple parquet partition #2) — the entire _provider_error_details extraction-from-context path is untested, so a regression that nulls it out would pass CI.
• The other_retryable fallback branch.
• The first-observation WARN (_logged_retryable_kinds de-dup) and that it never contains provider text.
• Interaction of the success bucket with non-retryable failures (finding docs: adding initial mkdocs structure #1) — a test here would have surfaced that concern.

A single test constructing a ModelRateLimitError with a ProviderError(status_code=429, retry_after=...) as its __context__ would lock in both the detail suffix and the metadata-extraction contract.

4. pyproject.toml [tool.uv.sources] change is unexplained and out of scope — LOW

The added block:

[tool.uv.sources]
data-designer = { workspace = true }
data-designer-config = { workspace = true }
data-designer-engine = { workspace = true }

is unrelated to retryable telemetry and unmentioned in the PR description. Pinning members to workspace = true is a reasonable/standard thing for a uv workspace (forces local resolution over PyPI), so it is likely benign or even a fix — but bundling a build-config change into a feature PR muddies the history. Please either call it out in the description with the rationale (was PyPI resolution actually happening?) or split it into its own chore: PR.

Nits

• _logged_retryable_kinds means the per-kind first-observation WARN fires exactly once for the scheduler's lifetime — if a provider recovers and later degrades again with the same kind, no fresh WARN. This looks intentional (the rate-limited degraded WARN covers ongoing stress), but is worth a one-line comment so it isn't mistaken for a bug later.
• The degraded-performance WARN was converted from an f-string to logger.warning(fmt, *args) lazy formatting — a good change (avoids formatting when the record is filtered), and the "degraded performance" substring the existing test greps for is preserved, so test_degraded_provider_warn_* should still pass.
• Metrics dicts are built with dict(sorted(...)) for stable ordering — nice for readable diagnostics/log diffs.

Verification

• Static review only. The workflow's make install-dev did not populate an importable data_designer in this review shell (ModuleNotFoundError: No module named 'data_designer' under both uv run pytest and .venv/bin/python), so I could not execute the suite. The author reports 119 passed in 2.26s plus targeted Ruff checks. The new test's assertions are internally consistent with the implementation (direct Model*Error construction ⇒ no ProviderError context ⇒ retryable_details == {kind: 1} with no http_ suffix), so it should pass as written.
• Imports (ModelAPIConnectionError, ModelInternalServerError, ModelTimeoutError) are correctly added to both the scheduler and the test; Counter and ProviderError were already imported. ModelRequestAdmissionTimeoutError subclasses ModelTimeoutError, and the isinstance ladder in _retryable_error_kind correctly checks the subclass first, so admission timeouts are not misclassified as generic timeout.

Structural Impact (graphify, 2.5s)

Risk: HIGH (1 core abstraction(s) modified; 10 import direction violation(s))

• 2 Python files, 147 AST entities, 1/81 clusters

Import Direction Violations (10)

Legal direction: interface -> engine -> config

• _string_keyed_counts() (engine) --calls--> .items() (interface)
• .__init__() (engine) --calls--> .items() (interface)
• ._record_observed_task_state() (engine) --calls--> .items() (interface)
• retryable_outcome_metrics() (engine) --calls--> .items() (interface)
• ._request_pressure_diagnostics() (engine) --calls--> .items() (interface)
• +5 more

Reviewer note: these appear to be false positives — the analyzer is attributing stdlib dict/Counter .items() (and .run() in the cross-package list below) to the interface package. The diff introduces no data_designer.interface imports in engine code; it touches only engine.dataset_builders and engine.models. No real import-direction regression.

Core Abstractions Modified

• AsyncTaskScheduler - Starting data designer with only llm generated column throws an error #4 most connected (162 deps)

High-Connectivity Changes

• AsyncTaskScheduler (162 deps) in packages/data-designer-engine/src/data_designer/engine/dataset_builders/async_scheduler.py
• _DispatchOutcome (52 deps) in packages/data-designer-engine/src/data_designer/engine/dataset_builders/async_scheduler.py
• _RowGroupState (52 deps) in packages/data-designer-engine/src/data_designer/engine/dataset_builders/async_scheduler.py
• _DeferredAdmissionAnalysis (52 deps) in packages/data-designer-engine/src/data_designer/engine/dataset_builders/async_scheduler.py
• +95 more

Reviewer note: the change is additive — a new property, two static helpers, one new keyword-only optional param (exc=None, backward-compatible), and instance-count fields initialized in __init__. Despite AsyncTaskScheduler being a top-connectivity god node, the blast radius is contained: no existing signatures broke, and the one modified method (_record_retryable_outcome) keeps its prior behavior for the boolean rate window. The one behavioral change worth the HIGH-risk scrutiny is the success bucket semantics (Finding #1).

Cross-Package Dependencies

• mcp_command() (interface) --calls--> .run() (engine)
• models_command() (interface) --calls--> .run() (engine)
• providers_command() (interface) --calls--> .run() (engine)
• +105 more

Reviewer note: none of these cross-package edges are introduced or altered by this PR; they are pre-existing graph edges surfaced because a high-connectivity node was touched.

Verdict

Approve with minor changes (do not merge as-is). The feature is well-scoped, the privacy design is careful, and the change is backward-compatible and additive. Before merge I'd like to see:

1. (Medium) Resolve the success bucket conflating non-retryable failures with successes (Finding docs: adding initial mkdocs structure #1) — either relabel or split the bucket, since it directly undermines the telemetry's stated purpose.
2. (Low) Add a test for the status_code/http_ detail path (Finding chore: add contributing guide #3) to lock in the __context__-based metadata extraction, and a brief comment explaining that dependency (Finding DataDesigner.make_seed_reference_from_file doesn't support paths with multiple parquet partition #2).
3. (Low) Justify or split out the pyproject.toml [tool.uv.sources] change (Finding Starting data designer with only llm generated column throws an error #4).

None of these are blocking correctness bugs in the retry/salvage path itself; they concern the accuracy and durability of the new telemetry. I am not approving or requesting changes on the PR directly.