feat: chart generation throughput

[eric-tramel]

Summary

This PR replaces the old fill/disappear terminal progress bars with a bounded ANSI/asciichart throughput panel that is enabled by default through the canonical RunConfig.display_tui=True setting. RunConfig.progress_bar remains supported as a deprecated compatibility shim with a warning, and the CLI exposes data-designer create --tui / --no-tui as a per-run override.

Usage

Default CLI behavior

The TUI is enabled by default for create runs when the process is attached to a terminal. In non-TTY environments, such as redirected output or many notebook contexts, the renderer falls back instead of trying to draw the live panel.

data-designer create path/to/config.py \
  --num-records 1000 \
  --dataset-name throughput-demo

Force the TUI for one CLI run

Use --tui when the saved or programmatic run config disables the panel but this terminal run should show it.

data-designer create path/to/config.py \
  --num-records 1000 \
  --dataset-name throughput-demo \
  --tui

Disable the TUI for one CLI run

Use --no-tui for logs-only runs, CI jobs, captured terminal output, or any case where the live panel would be noisy.

data-designer create path/to/config.py \
  --num-records 1000 \
  --dataset-name throughput-demo \
  --no-tui

Configure from Python

display_tui is the canonical run config option. It defaults to True; set it explicitly when creating datasets from a Python program.

from data_designer.config import RunConfig
from data_designer.interface import DataDesigner

# builder = ...  # Build or load a DataDesignerConfigBuilder.
data_designer = DataDesigner()
data_designer.set_run_config(RunConfig(display_tui=True))

results = data_designer.create(
    builder,
    num_records=1000,
    dataset_name="throughput-demo",
)

Disable from Python

from data_designer.config import RunConfig
from data_designer.interface import DataDesigner

# builder = ...
data_designer = DataDesigner()
data_designer.set_run_config(RunConfig(display_tui=False))

results = data_designer.create(
    builder,
    num_records=1000,
    dataset_name="logs-only-demo",
)

Deprecated compatibility shim

Existing callers that still set progress_bar get the same behavior through display_tui, plus a DeprecationWarning.

from data_designer.config import RunConfig

run_config = RunConfig(progress_bar=False)  # Deprecated: use display_tui=False.

Changes

Added

• New engine progress package:
  • data_designer.engine.progress.tracker for ProgressTracker
  • data_designer.engine.progress.reporter for AsyncProgressReporter
  • data_designer.engine.progress.terminal.throughput_panel for TerminalThroughputPanel
• RunConfig.display_tui, defaulting to True, as the canonical run config switch for the terminal throughput panel.
• A deprecated RunConfig.progress_bar compatibility shim that maps to display_tui and emits DeprecationWarning on constructor use, property reads, and property writes.
• asciichartpy as an engine dependency for terminal chart rendering.
• Model usage events and request-admission feedback events that let the live panel render model-level rpm, in tok/s, out tok/s, and red feedback markers.
• data-designer create --tui / --no-tui to force the create command's TUI setting for a single CLI run.

Changed

• Engine generation paths now read run_config.display_tui; production code no longer reads run_config.progress_bar.
• The terminal progress display now renders a chart with one records/sec curve per generation column, plus native aligned tables for column progress and model traffic.
• The chart uses throttled redraws, wider sampling windows, smoothed/fitted series, bounded rate history, stable chart height, and colored per-column progress bars.
• The column table reports only column-level now rec/s, avg rec/s, and completion progress; token rates are shown in the model-alias table.
• AsyncTaskScheduler accepts display_tui for async panel setup and passes its run id into AsyncProgressReporter, so global token/admission events from other runs do not pollute the active panel.
• Interrupt handling and async bridge cleanup avoid scheduling new async work during cancellation/interpreter shutdown.
• Tests now mirror the new package structure under packages/data-designer-engine/tests/engine/progress/.

Removed

• The old progress implementation locations under data_designer.engine.dataset_builders.utils.
• Example/demo scripts and the screenshot asset from the final PR diff; local runnable demos live under .scratch/ only.

Fixed

• Dense column/model layouts now stay within the terminal height and summarize overflow rows.
• Terminal resize, nested-panel ownership, cursor restoration, and untrusted label sanitization are covered.
• Successful zero-token model calls and resumed runs now report accurate request and throughput rates.
• Cancellation now propagates through sync/async bridges and both primary and salvage scheduler loops.

Attention Areas

Reviewers: Please pay special attention to the following:

• throughput_panel.py - terminal rendering, chart/table layout, redraw throttling, feedback marker projection, and TTY fallback behavior.
• reporter.py - progress aggregation, token/admission event subscriptions, cleanup, and run-id filtering.
• async_scheduler.py - async progress setup, run correlation, cancellation behavior, and display_tui plumbing.
• create.py - CLI --tui / --no-tui override surface.
• run_config.py - canonical display_tui field plus deprecated progress_bar shim behavior.
• pyproject.toml - adds the asciichartpy runtime dependency.

Validation

• make check-all: passed
• make test: 3,788 passed, 1 skipped (621 config; 2,165 engine; 1,002 interface)
• make check-license-headers: 577 files up to date
• fern check: 0 errors
• Seven-persona review follow-up: no remaining actionable findings
• GitHub checks: 60 passed, 2 skipped, 0 failed or pending
• Greptile: 5/5, safe to merge, no actionable findings
• Latest origin/main merged at 4b9b11cf

---

Description updated with AI

[github-actions]

Fern preview: https://nvidia-preview-pr-696.docs.buildwithfern.com/nemo/datadesigner

Fern previews include the docs-website version archive with PR changes synced into latest. Notebook tutorials are rendered without execution outputs in previews.

[github-actions]

Code Review: PR #696 — feat: chart generation throughput

Summary

Replaces the existing sticky fill-bar progress display with a bounded ANSI throughput chart panel rendered via asciichartpy. Key changes:

• New _BarState.record_update records per-column records/sec samples (gated by a 0.25s minimum interval) and stores them in a rates list.
• StickyProgressBar._redraw now emits a fixed-height panel (border + header + chart + legend + border) instead of one line per column.
• update_many accepts both 3-tuple (legacy (completed, success, failed)) and 4-tuple (+ skipped) values; async_progress_reporter and progress_tracker now feed skipped through.
• log_final / __exit__ no longer remove the bars — the panel stays visible at completion.
• RunConfig.progress_bar defaults flip from False → True (with non-TTY fallback to log lines).
• Adds asciichartpy>=1.5.25,<2 runtime dep, an examples/progress_panel_demo.py, and a screenshot asset.
• Tests are rewritten around the new panel; three former tests (test_drawn_lines_tracks_add_and_remove, test_wrapping_counts_physical_lines, test_wrapping_stable_across_updates) are removed.

Findings

Correctness / Behavior

• Default flip is a user-facing behavior change. Setting progress_bar=True by default means any TTY caller (CLI, notebook with TTY-like stderr, integrations) now gets a 16-line ANSI panel they didn't ask for. The non-TTY fallback covers CI/pipes, but interactive users running long jobs in terminals that emulate TTY may be surprised. Worth confirming this aligns with broader UX intent and is called out in release notes.
• Unbounded growth of _BarState.rates. record_update appends one float per sample at a 0.25s minimum cadence with no cap. A long generation (e.g. 1 hour) accumulates ~14,400 samples per column. Display is bounded via _compress_series, but the source list is not — for many columns × long runs this drifts toward MBs of floats. Consider capping (e.g. ring buffer of N samples), since the chart already downsamples for display.
• average_rate ignores now. _BarState.average_rate(self, now) accepts now but uses self.start_time to compute elapsed, then returns self.completed / elapsed. The now argument is unused in the math but the method takes it; readers will assume it's instantaneous-vs-historical. Either drop the parameter or use it. Minor.
• progress_tracker.log_final early-returns when bar is active. New code:

  if self._bar is not None and self._bar.is_active:
      self._bar.update(self._bar_key, completed=..., ...)
      return
  if self.completed > 0:
      self._log_progress_unlocked()

  This means in a TTY run, the final consolidated log line (_log_progress_unlocked) no longer fires — the panel update replaces it. That seems intentional ("keep the final chart visible"), but it is a quiet observability regression for anyone who scraped these final log lines. Worth a sentence in the PR body.
• async_progress_reporter.log_final calls _update_bar. It used to remove all bars via remove_bar(col). Now it simply triggers a final draw. That works, but the panel stays printed in stderr after the run; subsequent stderr output appears below it (no clear). On exit (__exit__) the previous _clear_bars() call has been removed and replaced with self._drawn_lines = 0. This is intentional per the PR description, but it leaves the cursor hide/show pair the only state that gets unwound — verify on a real terminal that the cursor is restored cleanly even if an exception aborts the panel.
• update_many mixed-tuple polymorphism. The runtime check update[3] if len(update) > 3 else bar.skipped accepts both shapes for the same dict. All in-tree callers were updated to 4-tuples, so the 3-tuple compat path appears to have no users. If it's defensive for external code, keep it; otherwise this is dead complexity worth removing.
• Panel sizing edge case at very narrow widths. panel_width = max(4, terminal_size.columns - 1). With columns=8, inner_width=5 and max_points = max(2, 5-12) = 2. asciichartpy.plot is then called on 2 samples with a 6-char numeric format prefix; the rendered chart may not fit the inner width, and _fit_ansi will strip ANSI and truncate, losing colors. Not a crash, just visual degradation. Existing test only covers columns=36. Consider an is_active = False short-circuit when the terminal is too narrow to draw a useful chart (e.g. < 30 cols).

Design / Conventions

• asciichartpy is a tiny (~8KB sdist) pure-Python module; module-level import is fine and doesn't violate the lazy-import guideline. Good.
• The new module-level constants (_RESET, _BORDER, _TITLE, _MUTED, _FAILED, _OK) are inline ANSI strings. They read fine here, but future style changes will need to find each constant; consider a small _ANSI namedtuple or dataclass if more colors are added.
• _format_chart_lines's max_points = max(2, inner_width - 12) uses 12 for the y-axis label width. The chart format string is "{:6.1f} " (7 chars) — the relationship between 12 and the format width is implicit. Pull both into named constants (_Y_AXIS_RESERVED = 12, _RATE_FORMAT = "{:6.1f} ") so they don't drift.
• ProgressUpdate = tuple[int, int, int] | tuple[int, int, int, int] is exported from the module but only used internally; consider keeping this private (_ProgressUpdate) since it's not part of any public API.
• _visible_len regex was widened from \033\[[0-9;]*m → \033\[[0-9;?]*[a-zA-Z] to match private-mode/CSI sequences. That's correct, but the new pattern would also match \033[?25l cursor-show/hide sequences if they ever appeared inside a measured string. Currently they don't (cursor sequences are written separately via _write), so safe; just worth noting in case future code passes a full draw buffer through _visible_len.

Tests

• New tests cover the 16-line panel height, panel-width bound at narrow terminals, skipped/failed propagation, and update_many semantics. Reasonable coverage of the rewritten surface.
• Lost coverage:
  • test_wrapping_counts_physical_lines and test_wrapping_stable_across_updates — these guarded against off-by-one regressions in _drawn_lines when individual lines wrap to multiple physical lines. The new code computes self._drawn_lines = len(lines) ignoring physical wrap. If any panel line ever exceeds terminal.columns, redraw will leak (clear too few lines). _panel_line does call _fit_ansi(text, inner_width) to pad/truncate, which should keep the line at exactly inner_width + 2 visible chars — so wrap is unlikely in practice, but a test asserting "no rendered line exceeds panel_width" would lock that invariant.
  • test_drawn_lines_tracks_add_and_remove — remove_bar is still on the public API but now untested in the bar-lifecycle sense. If callers still invoke it (the PR removed the only in-tree caller in async_progress_reporter), the behavior of removing a bar mid-run is no longer exercised.
• Test fragility: the new tests hard-code drawn_lines == 16 and count(CURSOR_UP_CLEAR) == 16, which depend on panel_height = min(_DEFAULT_PANEL_HEIGHT, max(_MIN_PANEL_HEIGHT, terminal.lines - 1)). If a CI runner reports terminal.lines < 17, the test will silently shrink the panel and fail. Consider patching shutil.get_terminal_size to a fixed value in these tests for determinism (the narrow-terminal test already does this).
• The examples/progress_panel_demo.py is unimported and untested; that's appropriate for a demo, but please confirm the example loaders are excluded from test discovery (they are under examples/, so should be).

Performance

• _redraw calls asciichartpy.plot on every update/update_many. With the async reporter calling _update_bar periodically (driven by progress_interval, default 5s), this is fine. But update() is also called per-record in some code paths; the rate-sampling guard caps the data appended to rates, but the chart redraw still happens on each call. Worth confirming there's no high-frequency call site that now hits asciichartpy on every record.

Security

• No security-relevant changes. New dependency asciichartpy is pinned to >=1.5.25,<2 and pulled from PyPI; package is small and pure-Python. Lockfile is updated. No user-controlled data flows into ANSI escape construction beyond column labels (bar.label); a malicious column name containing escape codes would be rendered through _fit_ansi which preserves codes, so a column named "\033[2J\033[H" could clear the screen mid-render. Low-impact (column names are user-authored), but a future hardening could strip control bytes from labels in add_bar.

Verdict

Approve with minor changes. The rewrite is well-scoped, the rendering math is reasonable, and tests cover the new surface adequately. Before merge I'd ask for:

1. Cap _BarState.rates length (ring buffer or trim) to bound memory on long runs.
2. Resolve whether the 3-tuple branch in update_many has any external callers; if not, drop it.
3. Note the progress_bar=True default change in release notes / changelog so users with existing TTY automation aren't surprised.
4. Either use or drop the now parameter in _BarState.average_rate.
5. Pin shutil.get_terminal_size in the new height-stability tests to avoid CI flakiness on small-LINES runners.

The remaining points (constant naming, label-escape hardening, lost wrap-test coverage) are nice-to-haves rather than blockers.

[greptile-apps]

Greptile Summary

This PR replaces the previous fill/disappear terminal progress bars with a live ANSI throughput panel backed by asciichartpy, and reorganises the progress infrastructure into a dedicated data_designer.engine.progress package.

• New TUI panel (throughput_panel.py): renders one records/s curve per generation column, a column table with now/avg rates and progress bars, and a model-alias table for rpm/token-rate metrics; redraws are throttled to ≥0.75 s and the panel falls back to log-lines on non-TTY streams.
• Canonical config migration: RunConfig.display_tui (default True) replaces RunConfig.progress_bar, which is kept as a deprecated shim that emits DeprecationWarning on constructor use, property read, and model_copy.
• Global pub/sub events: usage_events.py and an extension to observability.py introduce module-level callback registries for token-usage and request-admission events, filtered by run_id so concurrent runs cannot pollute each other's panels.
• Cancellation plumbing: AsyncTaskScheduler.request_cancel() sets a threading.Event and wakes the event loop; the async bridge in custom.py now polls that event every 0.1 s so sync generator work inside thread-pool workers responds to interrupts without waiting for the full bridge timeout.

Confidence Score: 5/5

Safe to merge. The new progress infrastructure is well-isolated, the deprecated shim is correctly wired, and the cancellation path through the async bridge and scheduler is robust.

The three attention areas (throttling, lock ordering, cancellation) are all handled correctly. Redraw throttling exists at both the reporter level (0.75 s in _maybe_report) and the panel level (_redraw_if_due). Lock ordering between ProgressTracker and TerminalThroughputPanel is consistent with no reverse path. The _cancel_requested Event is checked at every loop continuation point and the async bridge polls it every 0.1 s. The global pub/sub registries are filtered by run_id so concurrent runs do not interfere.

No files require special attention.

Important Files Changed

Filename	Overview
packages/data-designer-engine/src/data_designer/engine/progress/terminal/throughput_panel.py	New 940-line ANSI panel: handler wrapping, redraw throttling (0.75 s), feedback-marker projection, and TTY fallback are all present; lock ordering (ProgressTracker → panel lock) is consistent with no reverse path.
packages/data-designer-engine/src/data_designer/engine/progress/reporter.py	AsyncProgressReporter with 0.75 s TTY throttle in _maybe_report and run_id-filtered event subscriptions; close() is called in a finally block ensuring unsubscription.
packages/data-designer-engine/src/data_designer/engine/dataset_builders/async_scheduler.py	Adds request_cancel(), _cancel_requested Event, and _run_loop reference; cancellation checks are placed at all loop continuation points; reporter.close() is guaranteed in a finally block.
packages/data-designer-config/src/data_designer/config/run_config.py	display_tui replaces progress_bar; deprecated shim handles constructor, property read/write, and model_copy correctly with setdefault semantics so explicit display_tui always wins.
packages/data-designer-engine/src/data_designer/engine/column_generators/generators/custom.py	Async bridge now polls is_run_cancellation_requested() every 0.1 s, cancels the future, and raises asyncio.CancelledError; RuntimeError during run_coroutine_threadsafe properly closes the coroutine to prevent ResourceWarning.
packages/data-designer-engine/src/data_designer/engine/models/usage_events.py	Module-level callback registry with itertools.count IDs, lock-protected snapshots before iteration, and silent per-callback exception handling; unsubscribe returns a closure that pops by ID.
packages/data-designer-engine/src/data_designer/engine/dataset_builders/dataset_builder.py	Extracts _await_async_scheduler_result to handle KeyboardInterrupt by calling request_cancel() and waiting for the scheduler to finish before re-raising.
packages/data-designer-engine/src/data_designer/engine/models/facade.py	Emits TokenUsageEvent after each successful generate call; also fixes a pre-existing bug where output_tokens alone (without input_tokens) was discarded from token usage stats.
packages/data-designer/src/data_designer/cli/commands/create.py	Adds --tui/--no-tui as a typer Option with None default, forwarded to GenerationController.run_create() which applies it via model_copy.
packages/data-designer-engine/src/data_designer/engine/progress/tracker.py	Thread-safe ProgressTracker; log-interval accounting is correct under concurrent completions (next_log_at is advanced before lock release).

_{Reviews (10): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile}

[github-actions]

Code Review — PR #696: feat: chart generation throughput

Summary

This PR replaces the old StickyProgressBar ANSI progress bars with a new TerminalThroughputPanel that renders a bounded asciichartpy chart plus column / model usage tables. The setting is canonicalised as RunConfig.display_tui (default True); RunConfig.progress_bar becomes a deprecated shim. Adjacent infrastructure is added: a new data_designer.engine.progress package, global TokenUsageEvent + RequestAdmissionEvent pub/sub, run-id filtering in the reporter, a CLI --tui/--no-tui flag, and propagation of cancellation context into the sync→async bridge so KeyboardInterrupt actually unwinds in-flight worker threads.

Footprint: 32 files, +2188/-668. Engine, config, and CLI packages all touched. Layering direction (interface → engine → config) is preserved.

Findings

Behaviour / correctness

• Default flip from False → True is a user-visible behaviour change. Anyone running interactively will now see the TUI by default. The non-TTY fallback in TerminalThroughputPanel.__enter__ keeps CI/notebook output clean, but the change-of-default (progress_bar=False → display_tui=True) is effectively a behaviour bump for end users who previously relied on the periodic log lines in a TTY. Worth calling out in release notes; the PR description does mention it.
• RunConfig.progress_bar property on a Pydantic v2 model is a sharp edge. progress_bar is a @property on RunConfig (not a model field). model_copy(update={"progress_bar": True}) will not round-trip through the model_validator(mode="before") because model_copy skips validators; it will set an attribute that the property tries to shadow. If users were calling run_config.model_copy(update={"progress_bar": ...}), that path is now silently broken. Constructor-style (RunConfig(progress_bar=...)) and direct attribute reads/writes are covered by tests. Consider either documenting the migration, adding a model_copy/model_validate test for the legacy key, or detecting the case explicitly. (run_config.py:230-247)
• translate_deprecated_fields has an unreachable return normalized. The if "throttle" in normalized branch ends with return normalized and is followed by the same return normalized outside the if. Harmless, minor cleanup. (run_config.py:227-228)
• AsyncProgressReporter._matches_run uses correlation: object. The runtime branches over None, dict, and dataclass-via-getattr. RequestAdmissionEvent.captured_correlation is a dataclass and TokenUsageEvent.correlation is RuntimeCorrelation | None, so the dict branch is defensive only. The looser typing is fine, but a RuntimeCorrelation | Mapping[str, Any] | None annotation would make intent clearer and match what's tested. (reporter.py:165)
• Module-level pub/sub state is process-global. _callbacks in usage_events.py and _request_event_callbacks in observability.py are module-level dicts. The reporter properly unsubscribes in close() (called from both log_final and the scheduler finally), and the run-id filter prevents cross-run contamination, so this is OK. Two latent risks worth mentioning in a comment near each registry:
  • In test runs that crash before close(), callbacks accumulate across tests in the same process.
  • Subscriber callbacks are invoked synchronously on the model request hot path (facade.py: _track_usage). Slow/blocking subscribers will slow down generation. Today the only subscriber is the panel and its work is in-memory state, so it's fine — but the contract should be explicit.
• TerminalThroughputPanel.record_feedback_signal projects the y-coord using latest_rate at event time. latest_rate is only refreshed every _RATE_SAMPLE_INTERVAL_SECONDS (2.0s). A feedback event between samples lands at a slightly stale y-position. Acceptable for a visual marker; not a correctness issue.
• _overlay_feedback_markers x-positioning math. int(round(marker_elapsed / current_elapsed * (plot_column_count - 1))) with plot_column_count = max(1, point_count - 1). When plot_column_count == 1, this multiplies by 0, so all markers stack at column 0. Edge case for very-early feedback signals; visually fine.
• Bridge cancellation propagation. agenerate_with_bridge_context correctly suppresses ValueError on context-var reset (cross-thread close) and surfaces concurrent.futures.CancelledError as asyncio.CancelledError. The is_run_cancellation_requested() short-circuit before run_coroutine_threadsafe is the right place to bail out. Test test_async_bridge_obeys_run_cancellation_before_scheduling covers it. (custom.py:108-141)

Style / conventions

• asciichartpy is imported at module top of throughput_panel.py. AGENTS.md mandates lazy-loading "heavy third-party libraries" via data_designer.lazy_heavy_imports. asciichartpy is tiny (~7 KB wheel, pure Python), so this is unlikely to regress import time — but make perf-import CLEAN=1 should be run to confirm. More notably, from data_designer.engine.progress.terminal.throughput_panel import TerminalThroughputPanel is at the top of both async_scheduler.py and dataset_builder.py, so the panel module is now imported unconditionally even when display_tui=False. Cheap today, worth flagging if the panel grows new heavy deps later.
• Deprecation surface is consistent with the existing throttle shim. Same module-level message constant, same model_validator(mode="before") pattern, same stacklevel=2. Good.
• CLI --tui/--no-tui — tui: bool | None = None correctly distinguishes "not set" from "explicit value", and the controller only mutates RunConfig when the override is present. (generation_controller.py:147-149)

Testing

Coverage is strong:

• Deprecation warnings on constructor, property getter, property setter.
• TUI override plumbing through CLI → controller → DataDesigner.
• Bridge cancellation behaviour and column-context preservation.
• Token usage event filtering by run_id.
• Throughput panel rendering: header, column / model tables, narrow-terminal fallback, control-sequence sanitisation, redraw throttling, log interleaving, marker reprojection, rate-sample bounds.
• Old sticky-bar tests are deleted; equivalent assertions exist in the new suite.

Gaps / nice-to-haves:

• No test exercises request_cancel end-to-end through _execute_task_inner — the existing test mocks the future. A small unit covering "cancel flag flips and a bridge worker raises CancelledError" would close the loop, though it's hard to write deterministically.
• RunConfig.model_copy(update={"progress_bar": ...}) is not covered. If you intend to support that, add a test; if not, document.
• The reporter _emit path passes 0.0 to get_snapshot purely to read the completed counter (reporter.py:172). A short comment explaining why would prevent a future cleanup from "fixing" it to time.perf_counter() - self._start_time and changing semantics.

Performance

• Per-request token-usage emit + admission emit add a lock acquire and a small tuple copy each. With 1 subscriber this is trivial; for high-RPS runs (10k+ req/s) it's still well under the noise floor.
• Redraw is throttled to _MIN_REDRAW_INTERVAL_SECONDS = 0.75, and the reporter only forwards updates at DEFAULT_TTY_REPORT_INTERVAL = 0.75. ANSI clear-and-redraw happens at most ~1.3 Hz in steady state. Matches the existing sticky-bar behaviour.
• The 22-line panel performs ~22 cursor-up/erase escape sequences per redraw. Fine for terminal emulators but could flicker over slow ssh; not new — same characteristic as the old sticky bar.

Security

Nothing new exposed. ANSI sanitisation in _sanitize_label strips control characters from incoming labels and model names before they reach the terminal. Good defensive posture given that model aliases and column names can come from user-supplied configs.

Verdict

Looks good to merge with minor follow-ups:

1. (Recommended) Decide on RunConfig.model_copy(update={"progress_bar": ...}) semantics — either test+document or actively reject in the validator.
2. (Optional) Drop the unreachable return normalized after the throttle branch in translate_deprecated_fields.
3. (Optional) Tighten the type annotation on AsyncProgressReporter._matches_run.
4. (Optional) Run make perf-import CLEAN=1 to confirm asciichartpy doesn't regress engine import time, since it is loaded eagerly through async_scheduler.py / dataset_builder.py.
5. (Optional) Brief comment near the global pub/sub registries noting that subscribers must be cheap, non-blocking, and that close() is mandatory.

Architectural direction is sound: the new progress package sits cleanly under engine, the deprecation path mirrors the existing throttle shim, and the cancellation plumbing into the sync bridge is a real correctness improvement that goes beyond the visible UI change.

[nabinchha]

Nice work on this one, @eric-tramel - the new progress package is thoughtfully separated and the compatibility path is in good shape.

Summary

This PR replaces the old sticky progress bars with a terminal throughput panel, wires model/request-admission events into the live display, makes RunConfig.display_tui the canonical switch, and adds CLI --tui/--no-tui overrides. The implementation matches the PR description overall, but I found a couple of user-facing polish issues that are worth tightening before merge.

Findings

Warnings - Worth addressing

packages/data-designer-engine/src/data_designer/engine/progress/terminal/throughput_panel.py:595 — Legend rows can exceed the terminal height

• What: _format_panel() clamps panel_height to the terminal size, but _format_legend_lines() still appends every column row and every model row. With display_tui=True by default, a dataset with many generation columns or model aliases can draw a panel much taller than the viewport.
• Why: Once _drawn_lines grows past the terminal height, every redraw emits a large stack of cursor-up/clear sequences. That can scroll the terminal, erase useful logs, and make the default progress UI noisy for wide configs.
• Suggestion: Treat minimum_legend_capacity as an actual max budget too: render the first N column/model rows that fit, then add a muted overflow row like ... 12 more column(s), 3 more model(s). A focused test with a short terminal and many columns/models would lock in the bounded behavior.

fern/versions/latest/pages/devnotes/posts/async-all-the-way-down.mdx:182 — Public docs still describe the removed sticky bars

• What: The page updates the RunConfig(display_tui=True) snippet, but the surrounding prose and sample output still describe "sticky ANSI bars" that update on every task completion. The mirrored docs/devnotes/posts/async-all-the-way-down.md:183 has the same stale text.
• Why: This PR removes that implementation and replaces it with the throughput chart/table panel, plus throttled redraws and model traffic rows. Readers following the updated snippet will see something materially different from the docs.
• Suggestion: Update the prose and sample block in both the Fern page and the docs mirror to describe the throughput panel, redraw throttling, chart/table layout, and model traffic display.

Suggestions - Take it or leave it

uv.lock:285 — Orphan astroid lock entry

• What: The lockfile adds astroid==3.3.11, but no changed pyproject.toml or lockfile dependency references astroid.
• Why: It looks like merge or resolver churn unrelated to asciichartpy; leaving orphan lock entries makes future dependency diffs harder to read.
• Suggestion: Regenerate the lockfile from the updated dependency set, or otherwise drop the unused astroid entry if it is not actually required.

What Looks Good

• The deprecated RunConfig.progress_bar shim is well covered: constructor, property access, setter, and model_copy(update=...) all preserve compatibility while steering callers toward display_tui.
• The progress event plumbing is nicely scoped. AsyncProgressReporter.close() unsubscribes from global callbacks, and the run-id filter avoids cross-run token/admission events leaking into the active panel.
• The focused panel tests cover the important terminal mechanics: non-TTY fallback, redraw throttling, log interleaving, sanitization, rate-history bounds, feedback markers, and run-id filtering.

Verdict

Needs changes - please bound the panel height for wide configs and refresh the stale progress-bar docs before merge. The lockfile cleanup is optional but would make the dependency diff cleaner.

---

This review was generated by an AI assistant.

[johnnygreco]

This is really cool @eric-tramel!

I just have some nits related to more explicit labeling and logging. Would be nice if we could get "Preparing samplers to generate 50 records across 5 columns" to print before we start logging above the TUI. And can we have more explicit logs and/or labeling to make super clear that we are showing you llm/model-generated columns in the TUI? Of course, it is obvious when you think about it, but I could see users being confused mid-generation to see progress bars for some but not all columns.

The other nit is with all the labels at the top of graph, which shows the y-axis label (rec/s) plus the total stats. How flexible is the layout? Can we have just records/s on the left and then justify the total stats on the right or something like this? As is, it took me a second to understand "now 1.4" in │Throughput rec/s | 351.6s | 150/150 | now 1.4 | 0 failed, for example.

[johnnygreco]

@mikeknep – heads up about this new tui

[nabinchha]

Nice iteration on the follow-up commits. I re-reviewed latest head e2f03b70; the progress_bar compatibility path is now covered through model_copy, rate samples are bounded, and the stray astroid lockfile entry is gone.

Summary

This PR replaces the old sticky progress bars with the new TerminalThroughputPanel, makes display_tui the canonical RunConfig flag with --tui/--no-tui CLI overrides, and wires token/request-admission events into the live display. The architecture still looks clean, but two user-facing issues from the previous review remain in the latest head.

Findings

Warnings - Worth addressing

packages/data-designer-engine/src/data_designer/engine/progress/terminal/throughput_panel.py:493 — Dense panels still exceed the terminal height

• What: _format_panel() clamps panel_height, then builds all column/model legend rows and only shrinks the chart down to _MIN_CHART_LINE_COUNT; once the legend itself is larger than the remaining viewport, the final panel grows past the terminal height. The new regression test locks in that behavior with assert len(panel_lines) > 22 for 8 columns plus 8 models.
• Why: With display_tui=True by default, wide configs can still draw a default-on panel taller than the viewport. Redraws then emit cursor-up/clear sequences for more lines than fit on screen, which can scroll or erase nearby logs and make the TUI noisy in exactly the dense case the latest commit is meant to handle.
• Suggestion: Treat the legend area as a hard budget after the chart reaches its minimum height. Render the rows that fit, then add a muted overflow summary such as ... 5 more column(s), 4 more model(s). The dense-table test should assert len(panel_lines) <= terminal.lines - 1 instead of > 22.

fern/versions/latest/pages/devnotes/posts/async-all-the-way-down.mdx:182 — Dev note still describes the removed sticky-bar UI

• What: The snippet was updated to RunConfig(display_tui=True), but the surrounding prose and code block still describe “sticky ANSI bars” that redraw on every task completion and show the old per-column bar output. The mirrored docs/devnotes/posts/async-all-the-way-down.md:183 has the same stale section, and the later build-history sentence still says PR chore: async engine follow-up - rename, preview, lifecycle, progress #456 added sticky ANSI progress bars.
• Why: Readers following the updated config will now see the throughput chart plus column/model tables, throttled redraws, and request/model feedback, not the old four-line progress bar example. This makes the public docs materially misleading.
• Suggestion: Update the prose and sample output in the Fern page, and keep the docs mirror consistent if it is still maintained in this PR.

What Looks Good

• The RunConfig.progress_bar deprecation shim now covers constructor input, property get/set, and model_copy(update={"progress_bar": ...}), with tests for precedence when display_tui is also supplied.
• The TUI event subscriptions are run-id filtered and unsubscribed via AsyncProgressReporter.close(), including the scheduler finally path.
• The focused panel suite now covers non-TTY fallback, cursor restoration, redraw throttling, model-usage rows, label sanitization, feedback markers, bounded rate history, and CLI/controller plumbing.

Validation

• ruff check: passed
• ruff format --check: passed
• Focused pytest suite: 398 passed
• fern check: not run locally because the fern CLI is not installed in this worktree environment.

Verdict

Needs changes - please bound dense TUI panels to the terminal height and refresh the stale progress UI docs before merge.

---

This review was generated by an AI assistant.