fix(reports): enforce dashboard readiness and execution budget by fitzee · Pull Request #42624 · apache/superset

fitzee · 2026-07-30T23:01:46Z

SUMMARY

Fixes scheduled-dashboard report reliability for DOM-heavy dashboards such as dashboard 805.

The immediate root cause was a vacuous dashboard-readiness predicate: when React had mounted zero production chart holders, the browser path warned and continued, allowing blank/spinner screenshots to be delivered. Independent screenshot, Celery, and working_timeout values also left report executions without one authoritative deadline.

This change:

makes zero holders not-ready for scheduled dashboard reports in both standard and tiled Playwright capture and Selenium fallback; the condition is polled until its derived deadline, and timeout propagates before capture/PDF construction or delivery;
keeps chart reports on a chart-specific positive terminal-marker predicate and preserves the existing zero-holder behavior for thumbnails/standalone non-report screenshot callers;
keeps viewport-scoped holder readiness for virtualized/tiled dashboards and uses the backend chart count only as a diagnostic and tiling hint, not an equality gate;
removes partial/raw report fallbacks: tiled combine or readiness failure cannot deliver the first tile or an unchecked screenshot, while historical thumbnail fallbacks remain intact;
introduces one configurable monotonic report deadline (default 3600s, matching the historical working_timeout model default, and capped at each schedule's working_timeout) with capture (60s), delivery (120s), and terminal cleanup (30s) reserves;
aligns report Celery limits with that deadline when ALERT_REPORTS_WORKING_TIME_OUT_KILL is enabled: soft limit = resolved budget, hard limit = budget + 30s grace (3600s/3630s with defaults). Alerts retain their per-schedule working_timeout plus existing lag semantics for the numeric limits; note the new SoftTimeLimitExceeded handler in the shared reports.execute task (metric, warning log, explicit FAILURE state before re-raising) applies to alerts as well as reports — previously the exception propagated uncaught, so for alerts this is an observability-only change. Disabling Celery kill limits does not disable cooperative application-deadline checks;
propagates Celery soft timeout unchanged to the report state envelope, which records ERROR without spending hard-limit grace on an error notification, and increments reports.execute.celery_soft_timeout so operators can alert on the otherwise customer-silent failure;
validates the total budget, phase-reserve sum, and hard-limit grace during application startup rather than waiting for the first report execution;
bounds report stale-WORKING recovery by the report budget. A same-execution_id replay promotes its original WORKING row to ERROR; a distinct recovery invocation records its own ERROR and unblocks the schedule without mutating the old worker's audit row or attempting uncertain delivery;
retries failed terminal-state persistence at the application-owned command
boundary. The retry only promotes the exact execution UUID's active WORKING
row and only changes the schedule when that UUID is still the latest active
execution, so a delayed worker cannot overwrite a newer run;
emits key/value logs containing execution/report/dashboard/chart IDs, URL, expected/mounted/ready holder counts, elapsed/remaining time, attempt, and terminal reason;
reuses the production-safe selectors and task-budget helpers from fix(reports): positive readiness check for non-tiled screenshots #42253/fix(reports): make #42253 screenshot readiness production-safe #42427 and follows the safe no-fallback direction in fix(reports): fail loudly instead of falling back to unguarded screenshot when tiled capture fails #42273. fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill #42118 informed the total-budget design; this implementation centralizes the report deadline instead of adding another independent timeout constant.

Adversarial scope/recovery review

The follow-up commit deliberately removed two unsafe pieces from the initial draft:

No Celery task_failure signal handler performs metadata-DB cleanup. A hard-killed/lost worker can emit that signal from the Celery parent process, where a Superset Flask/SQLAlchemy transaction is not guaranteed. Immediate lost-worker cleanup therefore needs a separate lease/watchdog design and is out of scope for this correctness PR.
Stale recovery does not call the delivery state in the same invocation. Delivery outcome is uncertain after worker loss, so an automatic replay could duplicate email/Slack delivery.

The application-owned next invocation is the safe schedule-recovery hook: after the 15-minute bound it records the recovery invocation's ERROR, or promotes the same execution row in place when the execution_id is replayed. A distinct old WORKING audit row is deliberately left untouched because Celery hard limits do not preempt solo/eventlet/gevent workers and the original worker may still be alive. This removes the review-identified lost-update race while still allowing the next schedule to start from ERROR. Durable lost-worker terminalization requires a lease/watchdog plus compare-and-set ownership and is a separate follow-up, not a claim made by this PR.

No retry was added. Playwright already creates a fresh browser context per capture; retrying delivery is not safe without provider idempotency. The historical tiling decision guard was also restored: chart count alone does not force tiling when the dashboard is shorter than one tile, preserving report and thumbnail defaults.

The remaining cross-layer scope is intentional: the same deadline must reach scheduler task options, report state, browser navigation/readiness/capture, data/PDF generation, delivery, and terminal persistence. Splitting those pieces would reintroduce conflicting limits or allow a correctness path to bypass the deadline. Review-driven scope narrowing removed cross-execution audit-row mutation and the incidental tiling behavior change rather than layering on a database race fix. More invasive immediate worker-loss detection is explicitly excluded above.

The application deadline is cooperative between synchronous phases. Celery soft/hard limits are the final preemption boundary when the configured pool supports them. In particular, build_pdf_from_screenshots is checked immediately before and after the synchronous call but cannot be interrupted from inside that call; the after-check prevents delivery once an overrun returns.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Not applicable; this is background report orchestration and browser-capture behavior.

TESTING INSTRUCTIONS

Automated validation completed locally:

pytest -q \
  tests/unit_tests/utils/test_report_execution.py \
  tests/unit_tests/utils/test_screenshot_utils.py \
  tests/unit_tests/utils/webdriver_test.py \
  tests/unit_tests/commands/report/execute_test.py \
  tests/unit_tests/commands/report/test_execute_now.py \
  tests/integration_tests/reports/scheduler_tests.py \
  tests/integration_tests/reports/commands_tests.py::test_email_chart_report_schedule_alpha_owner \
  tests/integration_tests/reports/commands_tests.py::test_report_schedule_working_timeout \
  tests/integration_tests/reports/commands_tests.py::test__send_with_client_errors \
  tests/integration_tests/reports/commands_tests.py::test__send_with_multiple_errors \
  tests/integration_tests/reports/commands_tests.py::test__send_with_server_errors \
  tests/integration_tests/reports/commands_tests.py::test_soft_timeout_csv
# 255 passed

pre-commit run --files <all changed files>
# passed: mypy delta, Ruff format/check, pylint, and all applicable hooks

pre-commit run --all-files was also executed as required. It reached the full repository and exposed unrelated base/environment failures: 14 existing mypy errors in version-restore tests; missing/incomplete frontend dependencies and cache failures in prettier/stylelint; read-only default npm configuration for docs eslint; and existing repo-wide PT004/E402 Ruff findings. Its auto-format noise outside this PR was removed. The changed-file suite above passes.

The first draft SHA's failed checks were inspected:

pre-commit (current): branch-caused Ruff formatting in webdriver_test.py;
MySQL/Postgres/SQLite integration jobs: the same five branch-caused compatibility/recovery failures (new screenshot keyword in a mock, unsafe direct post-recovery capture, and class-unbound _send tests);
test-postgres-required: aggregate failure from the Postgres job.

All are addressed in the follow-up commit and covered by the targeted tests above.

The next CI iteration exposed and fixed two additional branch-caused deltas:

CI uses the pinned Ruff 0.9.7 formatter; three legacy assertion layouts in the
touched webdriver test differed from the older local binary and were corrected
in a formatting-only commit.
SQLite exercised alert screenshot soft timeouts outside the initial targeted
set. Alerts now retain their established format-specific timeout/error
notification behavior, while report soft timeouts still propagate to terminal
cleanup with no error delivery. The report CSV integration expectation and
report-vs-alert unit coverage were updated accordingly.

The review-fix commit adds focused coverage for boot-time config rejection,
the soft-timeout operator metric, distinct-execution recovery without an audit
lost-update, the Selenium seconds/reserve wiring, and the historical short-
dashboard tiling guard.

Final GitHub CI for 830e6cb5d532f470c247b7c6c99712cb388bc6bd
reached terminal state: 49 checks passed and 10 expected path-based skips or
neutral Netlify rule checks remain. There are no failed, cancelled, or pending
checks.

Staging defect follow-up: readiness timeout terminal persistence

The exact v6.0.0.22 backport exposed a cleanup-path defect in workspace
6970454b for report_schedule_id=2, dashboard 8, execution
d390442c-9539-4343-ad9a-06ec2359e39f. The run started at 05:42:16 and timed
out at 05:53:46 at the readiness allocation with elapsed=689.86,
remaining=209.94, and only 12/52 chart containers rendered. Capture correctly
stopped, but the execution remained WORKING: the generic unexpected-error log
appeared without a report_execution_terminal state=Error event. A concurrent
duplicate (c56c2434) had correctly been refused while the original was active,
so leaving the owner row WORKING also blocked later schedules.

The workspace execution-history UI makes both persistence defects definitive:

original d39044, scheduled 05:40 and started 05:42:16, still showed the
green WORKING icon, duration 00:00:00.000, and a blank error after the
689.86-second timeout;
refused duplicate c56c24, scheduled 05:44 and started 05:44:48, showed
duration 00:00:00.005 and the refusal error but also retained the green
WORKING icon.

The correct history is one terminal ERROR owner row with its actual end,
duration, and capture error, plus one terminal ERROR refusal row. The refusal
must not change the active owner's schedule state, and it must not add another
row eligible for WORKING timeout/recovery queries.

The capture exception itself is handled by the report state machine:
Playwright TimeoutError becomes ReportScheduleScreenshotFailedError, after
which the state attempts to promote its WORKING row to ERROR. The escape was
in that terminal-write error handling. A raw SQLAlchemy failure from the write
was not covered by the existing ReportScheduleUnexpectedError guard; the
transaction wrapper could therefore replace the capture exception and reach the
outer generic unexpected-error path without a second persistence attempt.
Reserved cleanup time existed but was unused.

Focused commits d81ef613da0be73cc4f485a551b92d1722f4d647 and
f27092c928acd66d90b57ba505c4621264524295, followed by ownership hardening in
ac3f21ce4716f80adcd9d82f2b7cb03a5224a091 and execution-history correction in
4ae2324f3f06a1fa99476514dd5429c03a7febe4. The MySQL timestamp-precision
test correction is 2b745d022dd1dcb239a295859460588c15d2437e:

preserves the original capture exception when the first terminal write raises
a SQLAlchemy error, rolls back the failed session, and retries from the
in-process command boundary;
makes that retry idempotent and execution-owned: it requires the exact UUID's
error-free WORKING row, promotes that audit row, and only moves the schedule
from WORKING to ERROR when the same UUID is still the latest active
execution;
emits the missing structured terminal event with the report context and
elapsed/remaining budget;
records whether this invocation entered from a non-WORKING state before the
state machine runs, and only permits the command-boundary retry for that
owner. This is stronger than checking the eventual exception: a
same-execution_id replay cannot mistake the still-active owner's row for its
own failed terminal write even if persisting the duplicate-refusal log itself
loses its database transaction;
records a refused duplicate invocation as its own terminal ERROR audit row,
with end time and refusal error, while leaving the original owner row and
schedule WORKING. It no longer creates a second active-looking WORKING
row;
does not add signal-side cleanup, worker-loss claims, screenshot retry, or
delivery retry.

Focused validation:

pytest -q tests/unit_tests/commands/report/execute_test.py
# 100 passed

pytest -q \
  tests/integration_tests/reports/commands_tests.py::test_readiness_timeout_retries_terminal_persistence_and_allows_next_schedule \
  tests/integration_tests/reports/commands_tests.py::test_report_schedule_working \
  tests/integration_tests/reports/commands_tests.py::test_report_schedule_same_execution_replay_stays_working \
  tests/integration_tests/reports/commands_tests.py::test_same_execution_replay_write_failure_does_not_claim_active_row \
  tests/integration_tests/reports/commands_tests.py::test_report_schedule_working_timeout \
  tests/integration_tests/reports/commands_tests.py::test_fail_screenshot
# 6 passed

ruff format --check \
  superset/commands/report/execute.py \
  tests/unit_tests/commands/report/execute_test.py \
  tests/integration_tests/reports/commands_tests.py
ruff check <same files>
# passed

The new integration test starts from a prior SUCCESS state, raises an ordinary
Playwright timeout, injects a database failure into the first ERROR write,
asserts the same execution is durably terminal ERROR with a real end time,
duration, and error text, with no delivery, and
then proves a distinct next execution reaches SUCCESS. Existing duplicate-run
coverage is run alongside it, including a same-ID fresh replay that must leave
the active owner and schedule WORKING, both with a successful refusal write
and with an injected SQLAlchemy failure in that write. The successful-refusal
cases assert exactly one active WORKING row and one terminal ERROR refusal
row with end_dttm, preventing execution-history accumulation. Mypy reports the same two pre-existing
SlackChannelSchema errors in execute.py at the parent SHA and this SHA; the
changed-line delta is clean.

CI on 4ae2324f3f06a1fa99476514dd5429c03a7febe4 found one branch-caused test
portability issue: MySQL stores these metadata timestamps with one-second
precision, so the subsecond mocked execution correctly persisted both
timestamps but failed a strict end_dttm > start_dttm assertion. Commit
2b745d022dd1dcb239a295859460588c15d2437e uses >= for that fast-path test;
the 689.86-second staging execution will retain a non-zero stored duration. The
same run's SQLite job was unrelated: GitHub Actions timed out three times while
pulling redis:7-alpine from Docker Hub, before checkout or tests.

GitHub CI for the final SHA reached terminal state with all checks passing or
expected path-based skips/neutral results. Code, unit, integration (including
MySQL/Postgres/SQLite), E2E, pre-commit, CodeQL, and the repository's delayed
🎪 Superset Showtime sync are terminal; there are no failures, cancellations,
or pending checks.

Staging evidence: dashboard 10

Staging dashboard 10 has 52 charts and produced a 7,504px tiled report. Two
scheduled executions both completed capture and delivered:

Run	Capture	Total execution	Start delay	Outcome
1	303.24s	305.81s	~30m	Delivered; the user reported a chunk error in the delivered report
2	266.66s	269.78s	~60m	Delivered

The first successful capture alone exceeded 300 seconds, and its total execution
was 305.81 seconds. A 300-second end-to-end deadline would therefore terminate a
report that this workload can successfully capture and deliver. The second run
also leaves little capacity under a 300-second limit. This is direct staging
evidence for the unified 15-minute budget rather than another 300s screenshot or
task assumption.

The 30/60-minute late starts are not part of those capture/execution
durations. They are separate queue, worker-capacity, beat, or scheduler latency
to investigate independently. They do not demonstrate slow DOM readiness, and
they must not be used to explain the chunk error.

Delivery is pipeline success, not proof that all 52 charts rendered
semantically. Readiness treats rendered, empty, and explicit-error holders as
terminal so that an error panel can be captured instead of spinning forever.
This PR retains that semantic-success policy: it prevents zero-holder,
nothing-mounted, and spinner capture, but it does not fail the entire report
merely because a chart reached an explicit error or empty terminal state. The
user-reported chunk error in run 1 therefore needs a per-holder audit; delivery
alone neither classifies the error nor proves full-chart success.

The staging audit must record, per run and deduplicated by chart ID across
tiles, rendered_holders, empty_holders, and explicit_error_holders, along
with expected/mounted totals. If existing holder-state diagnostics cannot
reconstruct those counts reliably, record that as an observability gap rather
than reporting all 52 charts as rendered.

Staged validation plan on reproducer `c5c287ca`

Note: this plan was drafted and executed while the budget default was 900s;
the shipped default is 3600s (capped per-schedule by working_timeout), so
the 900/930 figures below describe the staging configuration, not defaults.

Retain the c5c287ca dashboard 805 run as the control, then build this draft branch in staging only. Configure the default 900/60/120/30/30-second values explicitly.
Run dashboard 805 (52 charts: 33 pivot_table_v2, 19 table) as PDF. Confirm expected_holders=52, holder counts progress from zero to a positive mounted set, every viewport-visible mounted holder reaches a terminal state, capture occurs only afterward, and the execution reaches SUCCESS with one delivery inside the budget.
Reproduce UI saturation/slow mounting. While mounted holders remain zero, confirm readiness continues polling, with no screenshot/PDF construction and no delivery. Let mounting recover before readiness budget exhaustion and confirm the same attempt completes.
Hold readiness beyond total minus capture/delivery/cleanup reserves. While the owner remains active, invoke a duplicate and confirm its own history row is terminal ERROR with end_dttm, duration, and the refusal error while the owner's row and schedule remain WORKING; there must still be exactly one active WORKING row. Then confirm owner timeout propagation, an ERROR terminal reason, a real owner end/duration/error, cleanup capacity remaining, no incomplete capture or delivery, no remaining active WORKING row, and that a distinct following schedule is admitted. Confirm the terminal event carries the same execution UUID and elapsed/remaining budget rather than only the generic unexpected-error log.
Exercise standard and tiled dashboard paths, including virtualized gaps. Confirm offscreen placeholders do not block, every captured tile gates visible holders, and no first-tile/raw fallback is delivered on failure.
Run chart reports through Playwright and Selenium fallback. Confirm they use .chart-container terminal readiness rather than the dashboard-holder predicate. Run empty dashboard thumbnails and confirm their legacy zero-holder/fallback behavior remains unchanged.
Trigger Celery soft timeout and confirm the state envelope records ERROR before the 930s hard limit, increments reports.execute.celery_soft_timeout, and does not attempt an in-band customer error notification.
Terminate a worker after WORKING. Do not expect unsafe signal-side DB cleanup. On the first distinct invocation after the 15-minute stale bound, confirm the recovery invocation records ERROR, the old audit row is not mutated by a potentially racing worker, no delivery occurs in recovery, and a following new schedule can proceed. Track lease/watchdog terminalization of the old row as a separate follow-up.
Replay a stale task with the same execution_id; confirm the original row is promoted in place and no duplicate log/delivery is produced.
For dashboard 10 and dashboard 805, audit deduplicated per-chart terminal states across all tiles. Record expected/mounted totals plus rendered, empty, and explicit-error counts. Correlate the run-1 chunk error to a chart ID/state if possible; do not infer that delivery means every chart rendered.
Inspect structured logs for execution_id, report_schedule_id, dashboard_id, chart_id, url, expected_holders, mounted_holders, ready_holders, elapsed_seconds, remaining_seconds, attempt, and terminal_reason. Measure scheduled/queued/worker-start timestamps separately from execution-start/readiness/capture timestamps so scheduler latency is not folded into the report budget.

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

The staged validation plan above has been executed against the staging
reproducer; the terminal-state defect it surfaced is fixed and regression-covered.
This PR is ready for review.

netlify · 2026-07-30T23:03:46Z

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	`ba7777d`
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6a6d56e4d7a60c000806750e
😎 Deploy Preview	https://deploy-preview-42624--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

codecov · 2026-07-30T23:27:28Z

Codecov Report

❌ Patch coverage is 45.32374% with 228 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.39%. Comparing base (6929d03) to head (7e2010f).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
superset/utils/webdriver.py	10.18%	90 Missing and 7 partials ⚠️
superset/utils/screenshot_utils.py	4.93%	77 Missing ⚠️
superset/utils/report_execution.py	72.72%	16 Missing and 11 partials ⚠️
superset/commands/report/execute.py	76.14%	20 Missing and 6 partials ⚠️
...perset/mcp_service/screenshot/pooled_screenshot.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #42624      +/-   ##
==========================================
- Coverage   65.43%   65.39%   -0.04%     
==========================================
  Files        2810     2811       +1     
  Lines      159422   159769     +347     
  Branches    36382    36437      +55     
==========================================
+ Hits       104312   104487     +175     
- Misses      53068    53217     +149     
- Partials     2042     2065      +23

Flag	Coverage Δ
hive	`37.99% <18.22%> (-0.08%)`	⬇️
mysql	`57.76% <45.32%> (-0.04%)`	⬇️
postgres	`57.81% <45.32%> (-0.04%)`	⬇️
presto	`39.88% <18.22%> (-0.09%)`	⬇️
python	`59.19% <45.32%> (-0.04%)`	⬇️
sqlite	`57.43% <45.32%> (-0.04%)`	⬇️
unit	`100.00% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

fitzee · 2026-07-31T01:28:52Z

Griffen review (Claude + Codex dual pass)

Verdict: Needs changes — full agreement between both reviewers on verdict; noting this is already in draft with an explicit "do not merge" from the author, so treat the below as pre-merge punch list rather than a blocker on the current state.

Door classification: Mixed

Code-level revert is clean (no schema/migration). But two behaviors have one-way-door side effects during the live window: customer notifications missed on Celery soft-timeout can't be un-missed after the fact, and the stale-row recovery race (below) can write incorrect terminal state into the execution-log audit trail that a later code revert doesn't repair.

HIGH — Celery soft-timeout now suppresses all customer notification

For every REPORT-type schedule (dashboard screenshots and CSV/Excel/data exports) that hits the Celery soft time limit, the recipient now gets zero notification — test_soft_timeout_csv confirms email_mock.assert_not_called(). Previously a timing-out CSV report sent an error email. The tradeoff (skip the notification round-trip so the hard-limit grace window is spent on guaranteed terminal-state persistence) is reasonable engineering, but the customer-facing silent-failure implication isn't disclosed as a behavior change, and there's no compensating signal (metric/alert on terminal_reason=celery_soft_timeout) for ops to notice it happening. Suggest: add an observable signal at minimum; consider a durable outbox/idempotency-keyed notification task decoupled from the terminal-state write if in-band notification is wanted back.

MEDIUM — stale-WORKING-row recovery can race the original worker, not just itself

ReportWorkingState.next() mutates a stale WORKING execution-log row to ERROR (a different row than the current invocation's own) when the last-working uuid differs from the current execution_id. This assumes the original worker is actually dead. That's only guaranteed under Celery's prefork pool (hard time_limit → SIGKILL of the child process). Under solo/eventlet/gevent pools, a stuck synchronous Playwright/Selenium call can't be preempted by Celery's soft/hard limits at all — so the original worker can still be alive and later commit SUCCESS to that same row while a subsequent recovery invocation concurrently writes ERROR to it. That's a lost-update on the audit trail, not the harmless idempotent duplication the current tests (which mock the DAO lookup) exercise. Suggest a conditional write (UPDATE ... WHERE uuid=? AND state=WORKING, checking affected-row count) or SELECT ... FOR UPDATE before promoting the stale row, plus a test that has the "lost" worker still alive and racing recovery.

MEDIUM — budget/reserve config validated lazily, not at boot

ReportExecutionContext.__post_init__ (and get_report_task_timeout_options()) reject invalid reserve/budget combinations, but only when a REPORT schedule actually executes — not at app startup. A misconfigured deploy (e.g. bumping ALERT_REPORTS_EXECUTION_DELIVERY_RESERVE_SECONDS without checking the sum) fails every scheduled dashboard report until an operator notices the error-log pileup. Suggest validating this invariant at config-load/app-boot time.

MEDIUM — coverage gap on exactly the branches that decide over/under-aggressive termination

Codecov: superset/utils/webdriver.py patch coverage 9.64% (96 lines missing), superset/utils/screenshot_utils.py 8.10% (68 lines missing). The ReportExecutionDeadline/ReportExecutionContext arithmetic itself is well unit-tested in isolation, but most of the Selenium-path report_execution_context wiring (phase_timeout → WebDriverWait, set_page_load_timeout, animation-wait clamping) is untested. This is the class of code where a seconds/ms conversion or reserve-ordering bug would only show up as production reports dying too early or hanging too long. Suggest at least one Selenium-path integration test analogous to the Playwright tiled-path tests that already exist.

MEDIUM (non-blocking) — undisclosed tiling-decision change

In WebDriverPlaywright.get_screenshot, use_tiled changed from (chart_count >= chart_threshold or dashboard_height > height_threshold) and dashboard_height > tile_height to chart_count >= chart_threshold or (dashboard_height > height_threshold and dashboard_height > tile_height) — dropping the dashboard_height > tile_height guard from the chart-count branch, for all captures (reports and thumbnails). Not mentioned in the PR description. Likely safe (degrades to a single tile), but unverified by any test targeting exactly this case — worth a regression test and a one-line callout in the description since it's a behavior change riding along in a reliability fix.

LOW

_get_pdf()'s budget check is a before/after tripwire, not an enforced timeout — build_pdf_from_screenshots itself isn't preemptible, so an unusually slow PDF build can still overrun the budget before the after-check catches it. Acceptable given no easy synchronous timeout primitive here, just worth naming as a known gap.

What's sound

The core architecture — one shared monotonic deadline instead of independent screenshot/Celery/working_timeout constants — is the right fix, backed by real staging evidence (dashboard 10, 52 charts, 300s+ captures) rather than a guessed constant. ReportExecutionDeadline/ReportExecutionContext are clean and well-tested in isolation. The self-authored "Adversarial scope/recovery review" section correctly pre-empted the Celery signal-handler DB-safety and duplicate-delivery-on-replay hazards. ALERT-type schedules are correctly left untouched throughout, and combine_screenshot_tiles(allow_partial_fallback=False) for reports correctly refuses to ever deliver a truncated image.

Reviewed via Claude (Griffen) + Codex dual pass, synthesized. Full internal writeup on file.

fitzee · 2026-07-31T01:30:19Z

Staging review evidence: dashboard 10

Dashboard 10 has 52 charts and produced a 7,504px tiled report. Two staging
executions both delivered:

Run	Capture	Total execution	Start delay	Outcome
1	303.24s	305.81s	~30m	Delivered; user reported a chunk error in the report
2	266.66s	269.78s	~60m	Delivered

Budget review

Run 1 required more than 300 seconds for capture alone. A 300-second
end-to-end/task deadline would have killed a report that was otherwise captured
and delivered. Run 2 also leaves insufficient operational headroom under 300
seconds. This supports the PR's unified 15-minute report-execution budget and
the removal of independent 300s assumptions.

Latency classification

The ~30m/~60m late starts are separate queue, worker-capacity, beat, or
scheduler latency. They occurred before report execution and must not be folded
into readiness/capture duration or used to explain the chunk error.

Semantic-success review

Delivery is pipeline success; it does not prove that every chart rendered.
The readiness policy intentionally treats rendered, empty, and explicit-error
holders as terminal so an error panel can be captured rather than spinning
forever. This PR prevents zero-holder, nothing-mounted, and spinner captures,
but it does not fail the whole report merely because a chart reaches an empty
or explicit-error terminal state.

The first run's user-reported chunk error still needs a holder-state audit.
Please record, per run and deduplicated by chart ID across tiles:

expected and mounted holder totals;
rendered holder count;
empty holder count;
explicit-error holder count;
the chart ID/state correlated to the chunk error, if available.

Until those counts are recovered from holder-state diagnostics, the evidence
supports successful delivery and the 15-minute budget, but not a claim that all
52 charts rendered successfully.

fitzee · 2026-07-31T01:46:19Z

Addressed the Griffen review in 830e6cb5d532f470c247b7c6c99712cb388bc6bd:

Soft-timeout observability: added reports.execute.celery_soft_timeout, a task-level test, and explicit operator/customer-notification documentation. I did not add an in-band retry/outbox because delivery is not idempotent in this scope.
Stale-WORKING race: narrowed the change instead of hand-waving a CAS. A distinct recovery invocation no longer mutates the possibly-live worker's audit row; it records its own ERROR and unblocks the schedule. Same-execution_id replay still promotes its own row. Durable lost-row terminalization is documented as a lease/watchdog follow-up.
Boot validation: total budget, all reserves, and hard-limit grace are validated from SupersetAppInitializer.pre_init, with invalid/default config tests.
Selenium coverage: added an end-to-end wiring test for the shared clock, page-load timeout, WebDriverWait values, seconds units, animation clamp, and capture.
Tiling compatibility: restored the historical dashboard_height > tile_height guard and added a high-chart-count/short-dashboard regression test, preserving thumbnail behavior.
PDF limitation: documented that the synchronous builder has before/after cooperative checks but is not internally preemptible.

Focused suite: 255 passed. Changed-file pre-commit (including mypy, Ruff format/check, and pylint) passes. Mandatory all-files pre-commit was rerun; its failures remain unrelated base/environment issues already listed in the PR description. The PR remains draft while the new CI run is monitored.

fitzee · 2026-07-31T02:11:25Z

CI follow-up for 830e6cb5d532f470c247b7c6c99712cb388bc6bd: all checks reached terminal state. 49 passed; 10 expected path-based skips/neutral Netlify rule checks; 0 failed, cancelled, or pending. The PR remains draft and was not merged or deployed.

…ecovery-budget-rebased # Conflicts: # superset/utils/screenshot_utils.py # superset/utils/webdriver.py # tests/unit_tests/utils/test_screenshot_utils.py # tests/unit_tests/utils/webdriver_test.py

eschutho · 2026-07-31T23:03:33Z

Merged current master into this branch (covering Matt while he's out) — it was 5 relevant merges behind (#42273, #42153, #42120, #42118, plus a CroniterBadDateError fix). Conflicts were concentrated in screenshot_utils.py, webdriver.py, and the two test files, exactly where this PR's tiled/readiness work overlapped the recently merged PRs. Reconciliation decisions, called out explicitly since a couple override choices made in this branch before those merges landed:

Tiled bounding: this PR's phase-based architecture kept, fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill #42118's semantics folded in. _deadline_values/_timeout_seconds remain the single mechanism (deadline-driven when a report_execution_context is present). Two fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill #42118 behaviors were grafted onto the non-report fallback path: the fixed total ceiling when no Celery task budget exists (TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS — per-tile waits accumulate, so "no budget" must not mean uncapped, which this branch's previous fallback allowed), and the merged error-class family (TiledScreenshotBudgetExceededError(ScreenshotTaskBudgetExceededError); this branch's duplicate TimeoutError-based definition removed — nothing catches it by base class).
Unguarded thumbnail fallback NOT reintroduced. This branch predated fix(reports): fail loudly instead of falling back to unguarded screenshot when tiled capture fails #42273's merge and deliberately preserved the raw-screenshot fallback for thumbnails on tiled failure. fix(reports): fail loudly instead of falling back to unguarded screenshot when tiled capture fails #42273 removed that fallback unconditionally after review (thumbnails handle the raise as a clean cache-ERROR; nothing blank is cached or served), so the merged code raises for all callers, keeping this PR's structured terminal_reason=tiled_capture_failed logging. If there's a strong case for the thumbnail exception it should be re-litigated against fix(reports): fail loudly instead of falling back to unguarded screenshot when tiled capture fails #42273's rationale rather than slipped back in via merge.
fix(reports): downgrade chart-container timeout log level and fix tiling veto on unknown height #42153's changes composed cleanly: the unknown-height tiling routing and chart-container progress counting are preserved alongside this PR's per-element deadline-bounded wait_for and expected-chart-count hint.
Animation wait: report-mode reserve logic kept; non-report mode now caps at remaining budget and skips (rather than raises) on exhaustion, with the per-tile timing DEBUG line from fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill #42118 retained.
Budget-exhaustion behavior in the merged design: the pre-capture screenshot_capture phase check (and the deadline-bounded page.screenshot call) governs — fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill #42118's tests were updated accordingly (exhaustion now aborts before capturing the tile whose readiness wait consumed the budget; the bottom handler logs the structured report_capture_terminal WARNING). Note this is coherent where the bare "re-check before capture" suggestion declined on fix(reports): time-budget tiled screenshot to fail cleanly instead of hitting Celery kill #42118 was not, because here the capture call itself is time-bounded.

Verification: tests/unit_tests/utils/ + tests/unit_tests/commands/report/ — 1,017 passed; ruff check/format clean. Follow-ups #42657 (tracing) and #42661 (tiled clock anchor) remain open and will need small mechanical rebases against whichever of this PR/them lands first.

… runtime ceiling Two behavior-preserving adjustments to the execution-budget rollout so the upstream default changes as little existing behavior as possible: - Default ALERT_REPORTS_EXECUTION_BUDGET_SECONDS is now one hour, matching the historical effective ceiling (the ReportSchedule.working_timeout model default). Default installations keep today's maximum report runtime and gain only the clean-failure/readiness semantics; deployments with tighter SLAs lower the value. - The effective budget for a REPORT schedule is min(ALERT_REPORTS_EXECUTION_BUDGET_SECONDS, working_timeout), centralized in resolve_report_execution_budget_seconds() and used consistently by the Celery limit derivation, the execution deadline construction, and stale- WORKING detection (which previously implemented its own inline min). The per-schedule working_timeout field keeps its historical user-facing meaning as a cap instead of being silently ignored for reports. A working_timeout below the summed phase reserves is floored at the minimum viable budget (reserves + 30s working allowance) with a warning, so such schedules fail cleanly at their first phase check instead of erroring while constructing the execution context. Also adds the UPDATING.md entry for the semantics change and documents the infrastructure sizing rules (pod termination grace vs budget + hard grace; web-server per-request timeout bounds single chart requests, not the report). Co-Authored-By: Claude <noreply@anthropic.com>

eschutho · 2026-08-01T00:00:16Z

Pushed 4b1ec359f9 adjusting the budget semantics to minimize the upstream behavior change:

Default budget is now one hour (3600s) instead of 15 minutes, matching the historical effective ceiling: before this PR, the only end-to-end bound was Celery limits derived from working_timeout, whose model default is one hour. With this default, upgrading changes no default installation's maximum report runtime — reports gain clean deadline failures, phase reserves, and the readiness fix without a new time ceiling. Deployments wanting a tighter SLA (e.g. 900s) lower the config value.
The per-schedule working_timeout field keeps its meaning for reports: the effective budget is min(ALERT_REPORTS_EXECUTION_BUDGET_SECONDS, working_timeout), centralized in a new resolve_report_execution_budget_seconds() used by the Celery limit derivation, the deadline construction, and stale-WORKING detection (which previously carried its own inline min — now all three share one number). Previously the branch ignored working_timeout for reports entirely, which would have silently extended schedules whose owners configured a short kill time. A working_timeout below the summed phase reserves is floored at reserves + 30s with a warning (fails cleanly at the first phase check instead of erroring at context construction).
Added the missing UPDATING.md entry for the semantics change (Celery limits for REPORT types no longer use working_timeout + lag; the lag settings now apply to alerts only), and documented the infrastructure sizing rules in the docs page: pod termination grace must exceed budget + hard grace or in-flight reports die on deploys/drains; the web server's per-request timeout bounds individual chart requests, not the report total.

Tests: 5 new cases for the resolver (cap, no-raise above budget, None passthrough, floor-with-warning, Celery alignment at the cap); the recovery-bound test updated to exercise a deployment-tightened 900s budget explicitly rather than asserting the global default. tests/unit_tests/utils/ + commands/report/ + initialization_test.py: 1,054 passed; ruff clean.

…hot start Folds open PR apache#42661 into this branch (its standalone form patched code this branch restructures): take_tiled_screenshot() accepts the caller's screenshot_started_at so navigation/headstart/element-wait time counts against the non-report task budget, matching the clock _wait_for_charts_ready already uses. Report captures are unaffected -- their deadline starts at task start, which supersedes the anchor. Falls back to "now" when omitted. Co-Authored-By: Claude <noreply@anthropic.com>

The integration test still asserted the 900s draft default. The default budget now resolves to min(3600, working_timeout default 3600) = 3600 with a 30s hard grace; the 900/930 expectation is kept by explicitly setting working_timeout=900, which also exercises the capping path end to end through the scheduler. Co-Authored-By: Claude <noreply@anthropic.com>

The SoftTimeLimitExceeded handler in reports.execute is deliberately type-unconditional; this pins the alert path (metric, warning log, explicit FAILURE, re-raise) that the PR body describes as observability-only for alerts. Co-Authored-By: Claude <noreply@anthropic.com>

eschutho

Posting on Elizabeth's behalf — this is her PR reviewer agent. Forward any pushback to her and she'll loop me back in.

Left a few notes below — the first four are functional items worth a look before merge, the rest are optional. All line numbers verified against HEAD 9b84ce711b.

superset/commands/report/execute.py:1602-1631

ReportWorkingState.next()'s timeout-recovery branch calls update_report_schedule_and_log(ERROR, ...) unconditionally, with no "is this still the latest active execution" check and no DB-level locking (no SELECT FOR UPDATE, no unique constraint on ReportExecutionLog.uuid, no version column). Under genuinely concurrent transactions — e.g. two racing recovery invocations, or a recovery invocation racing a just-started new execution — this looks like it could still stomp a newer WORKING state back to ERROR. The narrower retry-on-DB-error path (persist_owned_report_execution_terminal_error) does have a real ownership check before it writes, but this timeout-recovery branch doesn't appear to share that guard.

WDYT — should this branch re-check ownership (uuid still the latest active execution) immediately before the write, the same way persist_owned_report_execution_terminal_error does, or is the in-process check considered sufficient given how narrow the race window is in practice?

superset/commands/report/execute.py:1636-1644 and superset/reports/models.py:388

The "refused duplicate" (non-timeout) branch always inserts a new ReportExecutionLog row via reuse_working_log=False, even when the replaying invocation's execution_id matches the currently-active WORKING row's uuid. Since ReportExecutionLog.uuid has no unique constraint, this can leave two rows sharing the same uuid (one WORKING, one ERROR), which seems to undercut the "one row per execution uuid" invariant implied elsewhere in this change. test_report_schedule_same_execution_replay_stays_working exercises this exact scenario but only asserts row counts/states, not uuid uniqueness.

Could we add a uuid-uniqueness assertion to that test (or a DB constraint) to lock this invariant in, or is the uuid collision here considered harmless for downstream audit queries?

superset/commands/report/execute.py:1813-1827

persist_owned_report_execution_terminal_error's retry-on-DB-error safety net is gated on owns_report_working_state, computed once at invocation start as last_state != WORKING. A stale-recovery invocation (one that finds the schedule already WORKING on entry) never has owns_report_working_state=True, so if its own terminal write hits a transient DB error, it looks like there's no retry — the schedule could stay stuck in WORKING until the next scheduled run's timeout check. That's a narrower version of the exact staging bug this PR set out to fix.

Would it be worth extending the retry safety net to cover the stale-recovery invocation's own write too, or is that intentionally deferred to the next scheduled run by design?

superset/tasks/scheduler.py:114-148

The new except SoftTimeLimitExceeded: handler on the shared execute() Celery task (incrementing reports.execute.celery_soft_timeout, calling self.update_state(state="FAILURE")) fires for both ALERT and REPORT schedules, since it wraps the whole AsyncExecuteReportScheduleCommand.run() call unconditionally. The PR description and config.py comments frame the soft-timeout metric/state change as report-specific, but this looks like it now applies to alerts too — pre-PR, an alert soft-timeout had no such handler and fell through to Celery's generic failure signal. The numeric soft/hard limits for alerts are untouched, just these side effects.

Could we either scope the handler to report-type schedules only, or update the docs/description to note this is a shared behavior change, and add a test that exercises an alert schedule through this path?

tests/unit_tests/commands/report/execute_test.py (test_working_timeout_replay_promotes_original_execution_without_duplicate_log, test_new_report_execution_does_not_deliver_during_stale_recovery)

These two mock out update_report_schedule_and_log entirely — the function that actually performs the promotion/mutation being tested — so they only prove next() calls it once, not that the promotion/non-mutation behavior itself is correct. The integration-level tests do exercise the real path, so this is more a redundancy/clarity nit than a coverage gap.

Totally optional — could rename these to reflect what they actually assert, or drop them if the integration coverage is considered sufficient on its own?

superset/utils/screenshot_utils.py (take_tiled_screenshot, tile-combine-failure path)

When allow_partial_fallback=False and combine fails, the raised exception is caught by the generic except Exception at the bottom of the function and logged via logger.exception("Tiled screenshot failed...") before converting to None / re-raising as PlaywrightTimeout upstream. Since this is an intentional reject (no-partial-fallback-for-reports), the logger.exception call will read as a spurious error in logs/alerting even though nothing unexpected happened.

Small suggestion — could this specific case log at warning/info instead, to avoid noise in error-rate alerting?

PR description headline mentions a "900-second" default budget, but the shipped default (ALERT_REPORTS_EXECUTION_BUDGET_SECONDS) is 3600s/1hr, consistent with UPDATING.md and the docs. 900 now only shows up as an explicit per-schedule override example.

Nit, not required — just flagging in case the description gets used as reference documentation later.

Also noticed the PR is currently open and not marked draft on GitHub, but the description's closing line still says "This PR remains intentionally draft... Do not merge or deploy it from CI." Worth a quick pass to reconcile the description with the actual PR state before merge.

…ed renames - Apply auto-walrus rewrite in resolve_report_execution_budget_seconds (pre-commit hook failure on CI). - Persist the working_timeout override in the scheduler budget test via a query-level UPDATE: the attribute write on the fixture object was not flushed in CI (all three DB backends still derived limits from 3600), so phase two asserted against the default. - Rename the two delegation-only unit tests to reflect what they assert (they mock update_report_schedule_and_log; the real promotion path is covered by integration tests), per review feedback. Co-Authored-By: Claude <noreply@anthropic.com>

eschutho · 2026-08-01T01:42:09Z

Thanks for the thorough pass — responses per item. Verification and fixes are on head 8feae1a1f0.

1. Timeout-recovery branch lacks an ownership re-check (execute.py:1602)
Agreed the TOCTOU is real, with one important qualifier: this branch is structurally identical on current master (same is_on_working_timeout() check → same unguarded update_report_schedule_and_log(ERROR); this PR added only the audit-row ownership comment and terminal logging there). A meaningful fix isn't just re-checking ownership — without a unique constraint on ReportExecutionLog.uuid or row locking, a re-check only narrows the window. That's schema/locking work (migration + backfill for existing duplicate uuids) that we've scoped as a follow-up rather than folding into an already-large behavioral PR. The in-process check matches the pre-existing guarantee; this PR doesn't claim to strengthen it on this path.

2. Refused-duplicate rows can share a uuid (execute.py:1636, models.py:388)
Also real, and a deliberate trade-off. The colliding case requires Celery redelivering the same task message (same execution_id) while the original still runs. Pre-PR behavior in that exact case was strictly worse: another WORKING row with the same uuid plus a schedule-state touch — the dangling-WORKING family this PR is fixing. Now the duplicate is a terminal audit row and the schedule is untouched. We can't assert uuid-uniqueness in test_report_schedule_same_execution_replay_stays_working yet because the invariant genuinely doesn't hold in this narrow case; the follow-up (same one as item 1) covers the unique index plus a same-execution_id guard in the refused branch (skip the audit insert when a WORKING row with our own uuid already exists — the active row already represents this execution).

3. Retry net doesn't cover the stale-recovery invocation's own write (execute.py:1813)
Intentional, and structural rather than an oversight: persist_owned_report_execution_terminal_error terminalizes the WORKING row matching this invocation's uuid — a stale-recovery invocation has no such row (it inserts a fresh ERROR row), so even with the flag set the helper would return False at the working_log is None check. Extending coverage would mean mutating another execution's audit row from a retry path while the original worker may still be alive — exactly what the ownership rules here forbid. The failure mode degrades safely: if the recovery invocation's own write is lost to a transient DB error, the next scheduled invocation re-enters the same timeout branch and re-attempts recovery. That's one extra schedule period at worst, versus the pre-PR staging bug where the terminal write never happened at all. Happy to add a code comment making this explicit if you'd like.

4. SoftTimeLimitExceeded handler fires for alerts too (scheduler.py:114)
Fixed via the "document as shared + test" option in d06a3b55d2: the PR description now states the handler is shared and why that's safe, and tests/unit_tests/tasks/test_scheduler_soft_timeout.py pins the alert path (metric, warning log, explicit FAILURE, re-raise). We chose documenting over scoping because type dispatch inside the handler would need a model read at the precise moment the DB may be implicated in the timeout, and for alerts the delta is observability-only — pre-PR the exception propagated uncaught to Celery's generic failure handling, which the re-raise preserves.

5. Delegation-only unit tests
Renamed in 8feae1a1f0 as suggested: test_working_timeout_replay_delegates_single_terminal_update and test_stale_recovery_delegates_terminal_update_without_delivery. Kept rather than dropped — they pin that next() delegates exactly once without directly mutating the stale row, which the integration tests don't isolate.

6. logger.exception on the no-fallback combine failure
Respectfully pushing back on this one: the exception reaching the generic handler in that path is a genuine combine fault (PIL-level failure while stitching tiles), not an intentional reject. The intentional rejects — budget exhaustion and per-tile readiness timeout — already route through dedicated warning-level branches above the generic handler precisely to keep customer-side slowness out of error-rate alerting. A combine failure is unexpected regardless of fallback policy, so error level is deliberate. The one fair sub-point is double-logging (inner combine_screenshot_tiles + outer handler log the same fault twice); can dedupe if you feel strongly.

7. "900-second" headline / 8. stale draft closing line
Both fixed in the description: the budget bullets now describe the 3600s default capped by working_timeout, the staged-validation section carries a note that its 900/930 figures were the staging configuration (that plan was executed against a 900s config and is kept as the historical record), and the do-not-merge closing line is replaced — the validation it was gating is complete, and the PR is ready for review.

CI status: your suspicion in the earlier summary was correct — the run on the reviewed head failed exactly on test_scheduler_report_timeout_uses_end_to_end_budget (the stale 900/930 assertions; the previous green run predated the 900→3600 default change), plus an auto-walrus pre-commit failure and a persistence bug in my first fix of that same test (the working_timeout attribute write on the fixture object wasn't flushed on any backend; now a query-level UPDATE). All addressed in 8feae1a1f0; CI is re-running on that head.

codeant-ai-for-open-source · 2026-08-01T01:44:41Z

+            if report_execution_context:
+                phase_timeout(
+                    "screenshot_capture",
+                    None,
+                    report_execution_context.post_capture_reserve_seconds,
+                )
            img = element.screenshot_as_png


Suggestion: The Selenium capture phase only calls phase_timeout to validate that some budget remains, but does not apply that timeout to element.screenshot_as_png. Selenium's screenshot command has no timeout inherited from set_page_load_timeout, so it can block past the report deadline and consume the delivery/cleanup reserve. Use a bounded capture mechanism or enforce the deadline around the screenshot operation. [logic error]

Severity Level: Major ⚠️

- ❌ Selenium-backed reports can exceed their configured execution deadline. - ⚠️ Blocked capture can consume delivery and cleanup reserves. - ⚠️ Celery may terminate workers before report failure cleanup completes.

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset/utils/webdriver.py **Line:** 1455:1461 **Comment:** *Logic Error: The Selenium capture phase only calls `phase_timeout` to validate that some budget remains, but does not apply that timeout to `element.screenshot_as_png`. Selenium's screenshot command has no timeout inherited from `set_page_load_timeout`, so it can block past the report deadline and consume the delivery/cleanup reserve. Use a bounded capture mechanism or enforce the deadline around the screenshot operation. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

Real limitation, acknowledged — Selenium's synchronous screenshot command cannot take a timeout, so it isn't cooperatively bounded (this predates the PR; Selenium capture has never been bounded). The layered design covers it: the phase_timeout gate stops a capture from starting without budget, and if the capture itself blocks past the deadline, the Celery soft limit (aligned to the budget) interrupts the task and the command-boundary retry persists the terminal ERROR inside the 30s hard grace — so the 'terminates before cleanup completes' consequence doesn't hold. Bounding the blocking WebDriver HTTP call would need thread/signal wrappers, which we're deliberately not adding here; Playwright (the primary driver, and the only tiled path) passes an explicit capture timeout. Note the Bito reply below quotes the Playwright tiled path as the resolution — different driver; that code is already bounded.

✅ Customized review instruction saved!

Instruction:

For Selenium screenshot capture, use a phase-timeout gate before starting the synchronous screenshot call; do not require additional thread/signal wrappers to bound the WebDriver call, as Celery soft limits and command-boundary retry handle captures that block past the deadline.

Applied to:

superset/utils/webdriver.py

💡 To manage or update this instruction, visit: CodeAnt AI Settings

codeant-ai-for-open-source · 2026-08-01T01:44:45Z

+# Celery's hard limit leaves this additional window for terminal cleanup after
+# the 15-minute soft limit. ALERT_REPORTS_WORKING_TIME_OUT_KILL controls these
+# Celery limits; disabling it does not disable the application deadline above.
+ALERT_REPORTS_EXECUTION_HARD_TIMEOUT_GRACE_SECONDS = 30


Suggestion: This comment says the hard-limit grace follows a 15-minute soft limit, but the documented default execution budget and resulting soft limit are 3600 seconds (one hour). This gives operators an incorrect timeout expectation; describe the grace as following the resolved execution budget instead of naming 15 minutes. [comment mismatch]

Severity Level: Minor 🧹

- ⚠️ Operators receive incorrect Celery timeout guidance. - ⚠️ Deployment timeout sizing may use the wrong soft-limit assumption.

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** docs/admin_docs/configuration/alerts-reports.mdx **Line:** 262:265 **Comment:** *Comment Mismatch: This comment says the hard-limit grace follows a 15-minute soft limit, but the documented default execution budget and resulting soft limit are 3600 seconds (one hour). This gives operators an incorrect timeout expectation; describe the grace as following the resolved execution budget instead of naming 15 minutes. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

Correct — stale leftover from the 900s draft. Fixed in ba7777d94a: the comment now describes the grace as following the resolved execution budget (configured budget capped by working_timeout) instead of naming 15 minutes.

codeant-ai-for-open-source · 2026-08-01T01:44:54Z

+        latest_working_log = (
+            db.session.query(ReportExecutionLog)
+            .filter(
+                ReportExecutionLog.report_schedule_id == report_schedule_id,
+                ReportExecutionLog.state == ReportState.WORKING,
+                ReportExecutionLog.error_message.is_(None),
+            )
+            .order_by(ReportExecutionLog.end_dttm.desc())
+            .first()
+        )
+        report_schedule = working_log.report_schedule
+        owns_schedule_state = (
+            report_schedule.last_state == ReportState.WORKING
+            and latest_working_log is not None
+            and latest_working_log.uuid == execution_id
+        )


Suggestion: The ownership check is a non-atomic read-modify-write: after latest_working_log and report_schedule.last_state are read, a newer execution can start and establish a different active WORKING row before this commit. The stale retry can then overwrite report_schedule.last_state with ERROR, incorrectly aborting the newer execution. Perform the ownership check and terminal update under a row lock or use a compare-and-swap update that verifies the active execution UUID. [race condition]

Severity Level: Major ⚠️

- ❌ Newer scheduled execution can be marked ERROR by stale worker. - ⚠️ Schedule remains blocked or requires timeout recovery. - ⚠️ Execution history can report an incorrect terminal owner.

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset/commands/report/execute.py **Line:** 198:213 **Comment:** *Race Condition: The ownership check is a non-atomic read-modify-write: after `latest_working_log` and `report_schedule.last_state` are read, a newer execution can start and establish a different active WORKING row before this commit. The stale retry can then overwrite `report_schedule.last_state` with ERROR, incorrectly aborting the newer execution. Perform the ownership check and terminal update under a row lock or use a compare-and-swap update that verifies the active execution UUID. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

Agreed the compare-then-commit is not atomic — this is the known TOCTOU family discussed in the human review above (items 1–2). Without a unique constraint on ReportExecutionLog.uuid or row locking, tightening the compare only narrows the window, and the schema/locking work (unique index + migration + backfill for existing duplicate uuids, guarded writes) is scoped as a tracked follow-up rather than folded into this PR. Worth noting the check that exists here is already strictly stronger than pre-PR master, which had no ownership compare at all on any terminal write path.

codeant-ai-for-open-source · 2026-08-01T01:44:54Z

+        self._phase_timeout(
+            "pdf_generation",
+            reserve_seconds=reserve_seconds,
+        )
        pdf = build_pdf_from_screenshots(screenshots)
+        self._phase_timeout(
+            "pdf_generation",
+            reserve_seconds=reserve_seconds,
+        )


Suggestion: The deadline is checked only before and after the synchronous build_pdf_from_screenshots call, so PDF conversion itself has no cooperative timeout. A large or pathological screenshot set can consume the remaining budget and cleanup reserve before the second check runs, allowing the Celery hard limit to terminate the task before terminal state persistence. PDF generation needs a bounded operation or periodic deadline checks that preserve the cleanup window. [possible bug]

Severity Level: Major ⚠️

- ❌ Large PDF reports can exceed Celery hard limits. - ⚠️ Terminal ERROR persistence may not execute. - ⚠️ Timed-out executions can remain WORKING until recovery.

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset/commands/report/execute.py **Line:** 864:872 **Comment:** *Possible Bug: The deadline is checked only before and after the synchronous `build_pdf_from_screenshots` call, so PDF conversion itself has no cooperative timeout. A large or pathological screenshot set can consume the remaining budget and cleanup reserve before the second check runs, allowing the Celery hard limit to terminate the task before terminal state persistence. PDF generation needs a bounded operation or periodic deadline checks that preserve the cleanup window. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

By design: the deadline is cooperative at operation granularity, and build_pdf_from_screenshots is a synchronous CPU-bound call we intentionally don't interrupt internally. The backstop is the Celery soft limit, which equals the budget — if PDF generation blows through the remaining budget, SoftTimeLimitExceeded interrupts the task and the command boundary persists the terminal ERROR within the 30s hard grace. So 'terminal state persistence may not execute' doesn't hold: that persistence path is exactly what the soft-limit envelope exists for. Adding periodic deadline checks inside PIL-level PDF assembly isn't practical.

✅ Customized review instruction saved!

Instruction:

Treat synchronous CPU-bound PDF generation as cooperative only at operation boundaries; do not require internal periodic deadline checks when the Celery soft-limit handler and command boundary provide terminal-state persistence.

Applied to:

superset/commands/report/execute.py

💡 To manage or update this instruction, visit: CodeAnt AI Settings

codeant-ai-for-open-source · 2026-08-01T01:44:54Z

+                    log_report_delivery_phase(
+                        report_context,
+                        getattr(recipient, "type", None),
+                        "start",
+                        enforce_budget=True,
+                    )


Suggestion: The delivery gate reserves only cleanup_reserve_seconds; it does not reserve delivery_reserve_seconds, and notification.send() receives no deadline-derived timeout. A slow email, Slack, or webhook delivery can therefore consume the delivery and cleanup windows, leaving insufficient time to persist the terminal report state despite the shared budget. Reserve both delivery and cleanup capacity and pass a bounded timeout to notification implementations where supported. [possible bug]

Severity Level: Major ⚠️

- ❌ Slow notification can exhaust terminal cleanup time. - ⚠️ Report ERROR or SUCCESS state may not persist. - ⚠️ Sequential recipients amplify delivery overrun risk.

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset/commands/report/execute.py **Line:** 1322:1327 **Comment:** *Possible Bug: The delivery gate reserves only `cleanup_reserve_seconds`; it does not reserve `delivery_reserve_seconds`, and `notification.send()` receives no deadline-derived timeout. A slow email, Slack, or webhook delivery can therefore consume the delivery and cleanup windows, leaving insufficient time to persist the terminal report state despite the shared budget. Reserve both delivery and cleanup capacity and pass a bounded timeout to notification implementations where supported. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

This one misreads the reserve semantics: reserves protect later phases, so the delivery gate reserving only cleanup_reserve_seconds is correct — the delivery reserve is the capacity delivery itself is meant to spend (it's held back from the earlier readiness/capture phases, see readiness_reserve_seconds/post_capture_reserve_seconds). Reserving delivery capacity at the delivery gate would double-count it and starve delivery. An unbounded notification.send() is backstopped the same way as capture: the per-recipient gate stops sends from starting without budget, and the Celery soft limit + command-boundary persistence covers a send that blocks.

✅ Customized review instruction saved!

Instruction:

Do not flag the report delivery gate for reserving only cleanup capacity; delivery_reserve_seconds is the budget intended for delivery itself, while the gate protects later cleanup phases and should not double-count delivery capacity.

Applied to:

superset/commands/report/execute.py

💡 To manage or update this instruction, visit: CodeAnt AI Settings

codeant-ai-for-open-source · 2026-08-01T01:44:56Z

+    if budget < (min_viable := reserves_total + MIN_REPORT_EXECUTION_WORK_SECONDS):
+        logger.warning(
+            "Report working_timeout=%s is below the minimum viable execution "
+            "budget (%.0fs phase reserves + %.0fs working allowance); "
+            "flooring the effective budget at %.0fs.",
+            working_timeout,
+            reserves_total,
+            MIN_REPORT_EXECUTION_WORK_SECONDS,
+            min_viable,
+        )
+        return min_viable


Suggestion: When a schedule's working_timeout is smaller than the phase reserves, this floors the effective budget above the user-configured limit. The resulting value is used for Celery's soft and hard limits and stale-working detection, so a schedule configured to stop after a short interval can continue for reserves_total + 30 seconds instead of honoring its configured timeout. Preserve the configured cap and fail the execution cleanly when it cannot accommodate the reserves. [logic error]

Severity Level: Major ⚠️

- ❌ Report schedules can exceed their configured working timeout. - ⚠️ Celery soft and hard limits no longer honor short schedules. - ⚠️ Stale WORKING recovery is delayed beyond user configuration.

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset/utils/report_execution.py **Line:** 86:96 **Comment:** *Logic Error: When a schedule's `working_timeout` is smaller than the phase reserves, this floors the effective budget above the user-configured limit. The resulting value is used for Celery's soft and hard limits and stale-working detection, so a schedule configured to stop after a short interval can continue for `reserves_total + 30` seconds instead of honoring its configured timeout. Preserve the configured cap and fail the execution cleanly when it cannot accommodate the reserves. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

Intentional and documented (UPDATING.md and the resolver docstring, with a runtime warning when the floor engages). The alternative — strictly honoring a sub-viable working_timeout — means every execution of that schedule fails deterministically, since the timeout can't fit capture + delivery + cleanup. Pre-PR, such a schedule was killed mid-capture every run and never delivered anything, so the floor (reserves + 30s ≈ 240s with defaults) converts a permanently-broken configuration into a working one while logging the discrepancy. A schedule owner who genuinely wants sub-4-minute kills has no deliverable report either way.

✅ Customized review instruction saved!

Instruction:

Do not flag flooring sub-viable report working timeouts to the minimum execution budget; this is intentional to allow capture, delivery, and cleanup to complete, with a runtime warning documenting the discrepancy.

Applied to:

superset/utils/report_execution.py

💡 To manage or update this instruction, visit: CodeAnt AI Settings

bito-code-review · 2026-08-01T01:45:02Z

The flagged issue is correct. The Selenium screenshot command does not inherently respect the page load timeout, and without an explicit timeout parameter, it can block indefinitely, potentially exceeding the report's execution budget. The PR already addresses this by introducing a bounded capture mechanism using _timeout_seconds to calculate a capture_timeout and passing it to page.screenshot.

To resolve this, ensure the page.screenshot call uses the calculated timeout as shown in the diff:

            capture_timeout = (
                _timeout_seconds(
                    "screenshot_capture",
                    reserve_seconds=(
                        report_execution_context.post_capture_reserve_seconds
                        if report_execution_context
                        else 0.0
                    ),
                )
                if report_execution_context or task_budget is not None
                else None
            )
            tile_screenshot = page.screenshot(
                type="png",
                clip=clip,
                **(
                    {"timeout": capture_timeout * 1000}
                    if capture_timeout is not None
                    else {}
                ),
            )

Would you like me to fetch all other comments on this PR to validate and implement fixes for them as well?

superset/utils/webdriver.py

capture_timeout = (
                _timeout_seconds(
                    "screenshot_capture",
                    reserve_seconds=(
                        report_execution_context.post_capture_reserve_seconds
                        if report_execution_context
                        else 0.0
                    ),
                )
                if report_execution_context or task_budget is not None
                else None
            )
            tile_screenshot = page.screenshot(
                type="png",
                clip=clip,
                **(
                    {"timeout": capture_timeout * 1000}
                    if capture_timeout is not None
                    else {}
                ),
            )

The hard-timeout grace comment predated the 3600s default; the soft limit is the resolved execution budget, not a fixed 15 minutes. Co-Authored-By: Claude <noreply@anthropic.com>

codeant-ai-for-open-source · 2026-08-01T02:21:05Z

+        elif report_execution_context:
+            readiness_predicate = REPORT_CHART_HOLDERS_READY_JS


Suggestion: The report readiness predicate only verifies chart holders intersecting the current viewport, but the subsequent standard screenshot captures the entire dashboard element, including below-the-fold content. As a result, lower charts can still be unmounted, loading, or blank when the full-page screenshot is delivered. Either ensure the full dashboard is progressively scrolled and each viewport is validated, or use the tiled capture path for reports that require full-dashboard readiness. [incorrect condition logic]

Severity Level: Major ⚠️

- ❌ Scheduled dashboard reports can omit lower charts. - ⚠️ Standard captures may deliver partially rendered dashboards. - ⚠️ Virtualized dashboard content is not validated before delivery.

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset/utils/webdriver.py **Line:** 445:446 **Comment:** *Incorrect Condition Logic: The report readiness predicate only verifies chart holders intersecting the current viewport, but the subsequent standard screenshot captures the entire dashboard element, including below-the-fold content. As a result, lower charts can still be unmounted, loading, or blank when the full-page screenshot is delivered. Either ensure the full dashboard is progressively scrolled and each viewport is validated, or use the tiled capture path for reports that require full-dashboard readiness. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

Real trade-off, deliberately carried over rather than introduced here — the viewport-scoped predicate came from #42153/#42253's production-safe readiness work, because requiring below-the-fold holders deadlocks on virtualized dashboards (off-screen holders never render by design). Full-dashboard readiness is exactly what the tiled path provides: it scrolls tile by tile and validates each viewport before capture, and the thresholds (SCREENSHOT_TILED_CHART_THRESHOLD, SCREENSHOT_TILED_HEIGHT_THRESHOLD) route chart-heavy/tall dashboards there. The residual window is a dashboard taller than the browser window but under both tiling thresholds; operators can close it by lowering SCREENSHOT_TILED_HEIGHT_THRESHOLD toward the viewport height. Auto-tiling whenever element height exceeds the viewport is a reasonable future tightening, but it changes capture behavior for a class of currently-working dashboards, so it's out of scope for this PR.

✅ Customized review instruction saved!

Instruction:

Do not require below-the-fold chart holders for the standard screenshot readiness predicate, since virtualized dashboards may never render off-screen holders; rely on the tiled capture path for full-dashboard readiness.

Applied to:

superset/utils/webdriver.py

💡 To manage or update this instruction, visit: CodeAnt AI Settings

codeant-ai-for-open-source · 2026-08-01T02:21:10Z

+            return False
+
+        latest_working_log = (
+            db.session.query(ReportExecutionLog)
+            .filter(
+                ReportExecutionLog.report_schedule_id == report_schedule_id,
+                ReportExecutionLog.state == ReportState.WORKING,
+                ReportExecutionLog.error_message.is_(None),
+            )
+            .order_by(ReportExecutionLog.end_dttm.desc())
+            .first()
+        )
+        report_schedule = working_log.report_schedule
+        owns_schedule_state = (
+            report_schedule.last_state == ReportState.WORKING
+            and latest_working_log is not None
+            and latest_working_log.uuid == execution_id
+        )
+        ended_at = datetime.now(timezone.utc).replace(tzinfo=None)
+        working_log.state = ReportState.ERROR
+        working_log.error_message = error_message
+        working_log.end_dttm = ended_at
+        if owns_schedule_state:
+            report_schedule.last_state = ReportState.ERROR
+            report_schedule.last_eval_dttm = ended_at
+
+        db.session.commit()  # pylint: disable=consider-using-transaction


Suggestion: Race condition: the latest WORKING log and report_schedule.last_state are read without a row lock or conditional update, then the schedule is committed later. A newer execution can become WORKING after this check but before the commit, allowing an older worker's retry to set the schedule to ERROR and overwrite the newer execution's state. Lock the schedule/latest log row or make the terminal update conditional on the execution UUID in the same transaction. [race condition]

Severity Level: Major ⚠️

- ❌ Newer report execution can lose WORKING schedule state. - ⚠️ Stale recovery may run against the wrong execution. - ⚠️ Concurrent report scheduling can produce inconsistent audit state.

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset/commands/report/execute.py **Line:** 196:222 **Comment:** *Race Condition: Race condition: the latest WORKING log and `report_schedule.last_state` are read without a row lock or conditional update, then the schedule is committed later. A newer execution can become WORKING after this check but before the commit, allowing an older worker's retry to set the schedule to ERROR and overwrite the newer execution's state. Lock the schedule/latest log row or make the terminal update conditional on the execution UUID in the same transaction. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

Same finding as the thread on line 213 (and items 1–2 of the human review above): agreed the compare-then-commit isn't atomic, and a real fix needs a unique constraint on ReportExecutionLog.uuid plus a guarded/locked write — schema work that's scoped as a tracked follow-up. The compare here is strictly stronger than pre-PR master, which wrote terminal state with no ownership check at all.

codeant-ai-for-open-source · 2026-08-01T02:21:10Z

+                total_seconds = resolve_report_execution_budget_seconds(
+                    app.config,
+                    working_timeout=self._model.working_timeout,
+                )


Suggestion: The resolved budget no longer honors the schedule's configured working_timeout when that value is below the reserve floor. For example, a schedule with the valid configured timeout of one second is assigned at least the summed reserves plus 30 seconds, and the Celery limits and deadline therefore permit execution well beyond the owner's timeout. Either reject such configurations or preserve the per-schedule timeout as the effective cap instead of flooring it above that value. [api mismatch]

Severity Level: Major ⚠️

- ⚠️ REPORT schedules can exceed configured execution limits. - ⚠️ Celery workers remain occupied beyond owner expectations. - ⚠️ Slow reports delay subsequent scheduled executions.

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset/commands/report/execute.py **Line:** 1827:1830 **Comment:** *Api Mismatch: The resolved budget no longer honors the schedule's configured `working_timeout` when that value is below the reserve floor. For example, a schedule with the valid configured timeout of one second is assigned at least the summed reserves plus 30 seconds, and the Celery limits and deadline therefore permit execution well beyond the owner's timeout. Either reject such configurations or preserve the per-schedule timeout as the effective cap instead of flooring it above that value. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

Same as the resolver thread in report_execution.py (a review instruction was saved there): the floor is intentional and documented in UPDATING.md, with a runtime warning when it engages. Strictly honoring a sub-viable working_timeout (one below capture+delivery+cleanup reserves) means that schedule fails deterministically on every run — pre-PR it was killed mid-capture and never delivered anything. The floor converts a permanently-broken configuration into a working one; rejecting such configs at save time is a fair alternative but is an API/validation change beyond this PR.

✅ Customized review instruction saved!

Instruction:

Preserve the minimum execution-budget floor when a configured working_timeout is below the capture, delivery, and cleanup reserves, and emit a runtime warning when the floor is applied; do not require strict adherence to sub-viable timeouts.

Applied to:

superset/commands/report/execute.py

💡 To manage or update this instruction, visit: CodeAnt AI Settings

codeant-ai-for-open-source · 2026-08-01T02:21:10Z

+            async_options = {
+                "eta": schedule,
+                **get_report_task_timeout_options(
+                    is_report=active_schedule.type == ReportScheduleType.REPORT,
+                    working_timeout=active_schedule.working_timeout,
+                    config=current_app.config,
+                ),
+            }
            execute.apply_async((active_schedule.id,), **async_options)


Suggestion: Passing every report's working_timeout through get_report_task_timeout_options can produce a Celery timeout longer than the schedule's configured cap: the helper floors values below the reserve-plus-working minimum instead of preserving the requested cap. For example, a report configured with a 10-second working_timeout receives a 210-second soft limit, so stalled executions can run well past the user-configured timeout. The report timeout calculation must never exceed the schedule's working_timeout. [logic error]

Severity Level: Major ⚠️

- ❌ Stalled scheduled reports exceed configured execution limits. - ⚠️ Celery workers remain occupied beyond schedule settings. - ⚠️ Manual report execution shares the same timeout mismatch.

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset/tasks/scheduler.py **Line:** 103:111 **Comment:** *Logic Error: Passing every report's `working_timeout` through `get_report_task_timeout_options` can produce a Celery timeout longer than the schedule's configured cap: the helper floors values below the reserve-plus-working minimum instead of preserving the requested cap. For example, a report configured with a 10-second `working_timeout` receives a 210-second soft limit, so stalled executions can run well past the user-configured timeout. The report timeout calculation must never exceed the schedule's `working_timeout`. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

Duplicate of the resolver-floor finding (see the report_execution.py thread) — same intentional behavior, same UPDATING.md documentation, evaluated once in resolve_report_execution_budget_seconds and consumed consistently by both the Celery limit derivation here and the in-process deadline, so the two never disagree.

✅ Customized review instruction saved!

Instruction:

Do not flag the resolver-floor behavior in report execution timeout calculations; it is intentional, documented, and consistently applied to both Celery limits and in-process deadlines.

Applied to:

superset/tasks/scheduler.py

💡 To manage or update this instruction, visit: CodeAnt AI Settings

codeant-ai-for-open-source · 2026-08-01T02:21:12Z

+            # derived from a fresh remaining value instead of a stale one
+            # that would let each tile overrun the budget by up to one settle
+            # interval (_timeout_seconds also recomputes at call time).
+            _raise_if_budget_exhausted()


Suggestion: The budget check occurs only after the fixed page.wait_for_timeout(SCROLL_SETTLE_TIMEOUT_MS) sleep, so a report with less than one second remaining can exceed its monotonic deadline before this check runs. Cap the scroll-settle wait using the remaining deadline, or check and fail before sleeping when the remaining time is shorter than the settle interval. [possible bug]

Severity Level: Major ⚠️

- ❌ Near-deadline tiled reports exceed their authoritative execution budget. - ⚠️ Up to one second of capture time consumes reserved phases. - ⚠️ Large dashboards repeat this delay once per tile.

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset/utils/screenshot_utils.py **Line:** 523:523 **Comment:** *Possible Bug: The budget check occurs only after the fixed `page.wait_for_timeout(SCROLL_SETTLE_TIMEOUT_MS)` sleep, so a report with less than one second remaining can exceed its monotonic deadline before this check runs. Cap the scroll-settle wait using the remaining deadline, or check and fail before sleeping when the remaining time is shorter than the settle interval. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

The premise isn't quite right: there's a budget check immediately before the scroll (_raise_if_budget_exhausted() above the scrollTo), and a second check immediately after the settle sleep — added precisely so the per-tile readiness timeout derives from a fresh remaining value (see the comment above that second check). The worst case is one settle interval (1s) of overshoot past the deadline before the re-check raises, which is absorbed by the 30s cleanup reserve plus the 30s Celery hard grace. Gating the sleep itself on remaining time would save at most that 1s in an execution that is already failing.

✅ Customized review instruction saved!

Instruction:

Do not require capping the scroll-settle sleep when budget checks occur immediately before scrolling and after the sleep, since the bounded overshoot is intentionally covered by the cleanup reserve and hard grace period.

Applied to:

superset/utils/screenshot_utils.py

💡 To manage or update this instruction, visit: CodeAnt AI Settings

bito-code-review

Code Review Agent Run #807e67

Actionable Suggestions - 4

superset/commands/report/execute.py - 2
- Missing unit tests for new function · Line 134-161
- Missing exception path test · Line 162-262
superset/utils/webdriver.py - 2
- Dead code: conditional guard on non-null deadline · Line 1313-1313
- Dead code: conditional guard on non-null deadline · Line 1345-1345

Additional Suggestions - 7

tests/unit_tests/utils/test_screenshot_utils.py - 1
- Dropped assertions reduce test coverage · Line 935-940
  
  Removing all 7 positional assertions from `warning_args` leaves a coverage gap. The docstring says 'Budget exhaustion is a customer chart-loading issue, not a Superset system fault, so it must log at WARNING (not ERROR)', but the removed assertions verify the warning's structured data fields (tile index, counts, elapsed/budget seconds, log-context suffix) — not just the message template. The subsequent `test_budget_exhausted_warning_includes_log_context` test only covers the `warning_args[-1]` log-context field (line 978), not the 6 positional arguments (tile index, tiles captured/total, elapsed, budget values). If the warning format changed, these assertions should reflect the new format; if they didn't change, they should remain.
superset/utils/report_execution.py - 1
- Missing negative-value input guards · Line 149-176
  
  `available_seconds` at line 152 uses `max(0.0, reserve_seconds)` which silently discards any negative caller value; `timeout_seconds` at line 174 applies `or requested_seconds <= 0` for the same purpose. Both guards hide caller bugs and produce zero when a caller passes a negative timeout. Add explicit ValueError guards so callers discover the bug immediately rather than receiving an unexpectedly unbounded or zero timeout.
superset/commands/report/execute.py - 2
- Missing retry complete log · Line 1358-1358
  
  When the SlackV2 retry succeeds, `notification.send()` (line 1358) exits the inner `try` without hitting the `log_report_delivery_phase(..., "complete", ...)` that the normal path has on line 1338. Add the missing call so the retry path also records a "complete" phase rather than silently returning.
- Redundant _phase_timeout call · Line 869-872
  
  Duplicate `_phase_timeout` call with identical arguments appears before and after `build_pdf_from_screenshots`. The second call after the operation completes is redundant.
superset/utils/webdriver.py - 2
- Dead code: unused timeout variable · Line 1455-1460
  
  Assign the result of `phase_timeout` to a timeout variable and apply it to the screenshot capture to enforce the deadline, following the existing pattern at lines 901–916.
- Inconsistent expected_holders value for chart capture · Line 1307-1307
  
  In the chart-container branch, `expected_chart_count` from `report_execution_context` reflects the dashboard chart count, but the screenshot targets one chart. Use `1` to avoid misleading log output.
superset/utils/screenshot_utils.py - 1
- Missing test for CHART_HOLDERS_MOUNTED_JS · Line 226-226
  
  The `CHART_HOLDERS_MOUNTED_JS` constant is used at line 421 to wait for chart holders to appear before readiness checks begin, but has no unit test. The sibling constants `REPORT_CHART_HOLDERS_READY_JS` and `CHART_HOLDERS_READY_JS` are tested at lines 654-692 of test_screenshot_utils.py. Adding a corresponding assertion would complete the coverage.

Filtered by Review Rules

Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.

superset/utils/webdriver.py - 1
- Semantic duplication of elapsed computation · Line 343-347
tests/unit_tests/utils/test_report_execution.py - 2
- Missing docstring on helper · Line 32-44
- Missing docstrings and type annotations · Line 47-209
tests/unit_tests/utils/test_screenshot_utils.py - 1
- Unreachable mock return value · Line 628-630
superset/utils/report_execution.py - 1
- Log field name mismatch · Line 218-218

Review Details

Files reviewed - 20 · Commit Range: 55e5d18..8feae1a
- docs/admin_docs/configuration/alerts-reports.mdx
- superset/commands/report/execute.py
- superset/commands/report/execute_now.py
- superset/config.py
- superset/initialization/__init__.py
- superset/mcp_service/screenshot/pooled_screenshot.py
- superset/tasks/scheduler.py
- superset/utils/report_execution.py
- superset/utils/screenshot_utils.py
- superset/utils/screenshots.py
- superset/utils/webdriver.py
- tests/integration_tests/reports/commands_tests.py
- tests/integration_tests/reports/scheduler_tests.py
- tests/unit_tests/commands/report/execute_test.py
- tests/unit_tests/commands/report/test_execute_now.py
- tests/unit_tests/initialization_test.py
- tests/unit_tests/tasks/test_scheduler_soft_timeout.py
- tests/unit_tests/utils/test_report_execution.py
- tests/unit_tests/utils/test_screenshot_utils.py
- tests/unit_tests/utils/webdriver_test.py
Files skipped - 1
- UPDATING.md - Reason: Filter setting
Tools
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.
/pause - Pauses automatic reviews on this pull request.
/resume - Resumes automatic reviews.
/resolve - Marks all Bito-posted review comments as resolved.
/abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by

bito-code-review · 2026-08-01T02:27:49Z

+def log_report_delivery_phase(
+    report_context: ReportExecutionContext | None,
+    recipient_type: ReportRecipientType | None,
+    phase: str,
+    *,
+    enforce_budget: bool,
+) -> None:
+    """Enforce and log a notification phase when executing a report."""
+
+    if report_context is None:
+        return
+    deadline = report_context.deadline
+    if enforce_budget:
+        deadline.timeout_seconds(
+            "notification_delivery",
+            reserve_seconds=report_context.cleanup_reserve_seconds,
+        )
+    logger.info(
+        "report_delivery_%s %s recipient_type=%s elapsed_seconds=%.2f "
+        "remaining_seconds=%.2f",
+        phase,
+        report_context.log_context,
+        recipient_type,
+        deadline.elapsed_seconds,
+        deadline.remaining_seconds,
+    )
+
+


Missing unit tests for new function

log_report_delivery_phase is called at lines 1322, 1338, and 1352 but has no dedicated unit test. This function implements important execution-logging logic with conditional budget enforcement—coverage gaps could allow regressions in notification timing to go undetected.

Code Review Run #807e67

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

Yes, avoid them

Added in 7e2010fe0d: three unit tests covering the no-context no-op, the enforce_budget=True raise on an exhausted budget, and the enforce_budget=False post-send logging path (which must record the phase rather than raise mid-notification).

The suggestion to add unit tests for log_report_delivery_phase is appropriate. Adding these tests ensures that the function's logic—specifically the conditional budget enforcement and the logging behavior—is verified and protected against future regressions.

bito-code-review · 2026-08-01T02:27:49Z

+def persist_owned_report_execution_terminal_error(
+    report_schedule_id: int,
+    execution_id: UUID,
+    error_message: str,
+    terminal_reason: str,
+    report_context: ReportExecutionContext | None = None,
+) -> bool:
+    """
+    Terminalize this command's WORKING row from its application-owned boundary.
+
+    Report states normally persist their terminal result before re-raising. If
+    that first write loses its transaction or database connection, the command
+    boundary is the last safe in-process retry: it still has Flask application
+    context and knows the execution UUID it owns. A compare against the latest
+    active WORKING row prevents an old worker from changing the schedule state
+    after a newer execution has started.
+    """
+
+    try:
+        # The state-machine transaction has already rolled back on its way to
+        # this boundary. Roll back again so a failed terminal flush cannot leave
+        # the scoped session unusable for the retry.
+        db.session.rollback()  # pylint: disable=consider-using-transaction
+        working_log = (
+            db.session.query(ReportExecutionLog)
+            .filter(
+                ReportExecutionLog.report_schedule_id == report_schedule_id,
+                ReportExecutionLog.uuid == execution_id,
+                ReportExecutionLog.state == ReportState.WORKING,
+                ReportExecutionLog.error_message.is_(None),
+            )
+            .first()
+        )
+        if working_log is None:
+            return False
+
+        latest_working_log = (
+            db.session.query(ReportExecutionLog)
+            .filter(
+                ReportExecutionLog.report_schedule_id == report_schedule_id,
+                ReportExecutionLog.state == ReportState.WORKING,
+                ReportExecutionLog.error_message.is_(None),
+            )
+            .order_by(ReportExecutionLog.end_dttm.desc())
+            .first()
+        )
+        report_schedule = working_log.report_schedule
+        owns_schedule_state = (
+            report_schedule.last_state == ReportState.WORKING
+            and latest_working_log is not None
+            and latest_working_log.uuid == execution_id
+        )
+        ended_at = datetime.now(timezone.utc).replace(tzinfo=None)
+        working_log.state = ReportState.ERROR
+        working_log.error_message = error_message
+        working_log.end_dttm = ended_at
+        if owns_schedule_state:
+            report_schedule.last_state = ReportState.ERROR
+            report_schedule.last_eval_dttm = ended_at
+
+        db.session.commit()  # pylint: disable=consider-using-transaction
+        log_context = (
+            report_context.log_context
+            if report_context is not None
+            else (
+                f"capture_kind=report execution_id={execution_id} "
+                f"report_schedule_id={report_schedule_id} "
+                f"dashboard_id={report_schedule.dashboard_id} "
+                f"chart_id={report_schedule.chart_id}"
+            )
+        )
+        elapsed_seconds = (
+            f"{report_context.deadline.elapsed_seconds:.2f}"
+            if report_context is not None
+            else "unknown"
+        )
+        remaining_seconds = (
+            f"{report_context.deadline.remaining_seconds:.2f}"
+            if report_context is not None
+            else "unknown"
+        )
+        logger.info(
+            "report_execution_terminal %s state=%s terminal_reason=%s "
+            "elapsed_seconds=%s remaining_seconds=%s",
+            log_context,
+            ReportState.ERROR.value,
+            terminal_reason,
+            elapsed_seconds,
+            remaining_seconds,
+        )
+        return True
+    except Exception:  # noqa: BLE001  # never mask the report's original exception
+        db.session.rollback()  # pylint: disable=consider-using-transaction
+        logger.exception(
+            "Failed terminal persistence retry for report execution "
+            "capture_kind=report execution_id=%s report_schedule_id=%s "
+            "terminal_reason=terminal_persistence_retry_failed",
+            execution_id,
+            report_schedule_id,
+        )
+        return False


Missing exception path test

persist_owned_report_execution_terminal_error has tests for the success and race-condition paths (lines 2719-2763 in test file) but no test for the exception handler at line 253. The except Exception block is the safety net for terminal persistence failures—it should be exercised to verify rollback and logging behavior.

Code Review Run #807e67

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

Yes, avoid them

Added in 7e2010fe0d: test_terminal_persistence_retry_survives_database_failure makes the session query raise, and asserts the double rollback (entry + handler), the terminal_persistence_retry_failed exception log, no commit, and the False return that keeps the report's original exception unmasked.

The suggestion to add a test for the exception handler in persist_owned_report_execution_terminal_error is appropriate. Exercising the except Exception block at line 253 is necessary to verify that the session rollback and the terminal_persistence_retry_failed exception logging occur as expected when terminal persistence fails.

superset/commands/report/execute.py

except Exception: # noqa: BLE001 # never mask the report's original exception db.session.rollback() # pylint: disable=consider-using-transaction logger.exception( "Failed terminal persistence retry for report execution " "capture_kind=report execution_id=%s report_schedule_id=%s " "terminal_reason=terminal_persistence_retry_failed", execution_id, report_schedule_id, ) return False

bito-code-review · 2026-08-01T02:27:49Z

+                            holder.get("state") in ready_states
+                            for holder in holder_states
+                        ),
+                        (f"{deadline.elapsed_seconds:.2f}" if deadline else None),


Dead code: conditional guard on non-null deadline

deadline is unconditionally assigned from report_execution_context.deadline on line 1301; the if deadline else None guard on line 1313 is always-true and masks the actual intent. Replace with direct attribute access.

Code Review Run #807e67

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

Yes, avoid them

Fixed in 7e2010fe0d — both this and the second occurrence at line 1345 removed; deadline is assigned unconditionally there and ReportExecutionDeadline is always truthy.

The suggestion to remove the conditional guard is appropriate. Since deadline is assigned unconditionally from report_execution_context.deadline on line 1301, the if deadline else None check on line 1313 is redundant and can be simplified to direct attribute access.

superset/utils/webdriver.py

(f"{deadline.elapsed_seconds:.2f}" if deadline else None),

bito-code-review · 2026-08-01T02:27:49Z

+                        report_execution_context.expected_chart_count,
+                        len(holder_states),
+                        ready_holders,
+                        (f"{deadline.elapsed_seconds:.2f}" if deadline else None),


Dead code: conditional guard on non-null deadline

deadline is unconditionally assigned on line 1334; the if deadline else None guard on line 1345 is always-true and is dead code. Replace with direct attribute access.

Code Review Run #807e67

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

Yes, avoid them

Fixed in 7e2010fe0d (same commit as the sibling at line 1313).

The suggestion to remove the if deadline else None guard is correct. Since deadline is unconditionally assigned on line 1334, the conditional check is redundant and can be simplified to direct attribute access.

superset/utils/webdriver.py

(f"{deadline.elapsed_seconds:.2f}" if deadline else None), (f"{deadline.remaining_seconds:.2f}" if deadline else None),

…drop dead guards Per bot review feedback: - log_report_delivery_phase: no-op without a report context, raises on exhausted budget when enforcing, and still logs post-send phases without raising when enforcement is off. - persist_owned_report_execution_terminal_error: a DB failure inside the retry itself rolls back, logs, and returns False without masking the report's original exception. - Remove two always-true 'if deadline' guards in the Selenium readiness logging (deadline is assigned unconditionally and always truthy). Co-Authored-By: Claude <noreply@anthropic.com>

codeant-ai-for-open-source · 2026-08-01T02:49:48Z

+                    log_report_delivery_phase(
+                        report_context,
+                        recipient.type,
+                        "retry",
+                        enforce_budget=True,
+                    )
                    notification.send()


Suggestion: The Slack fallback sends the notification after upgrading from v1 to v2 but never records a complete delivery phase. Successful fallback deliveries therefore produce only start and retry events, causing phase-level monitoring and delivery audit logic to report an incomplete delivery even though the notification succeeded. Emit the same completion event after the retry send. [incomplete implementation]

Severity Level: Minor 🧹

- ⚠️ Slack fallback telemetry lacks successful completion. - ⚠️ Delivery diagnostics show an incomplete phase sequence. - ⚠️ Operators cannot distinguish retry success from interruption.

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖

This is a comment left during a code review. **Path:** superset/commands/report/execute.py **Line:** 1352:1358 **Comment:** *Incomplete Implementation: The Slack fallback sends the notification after upgrading from v1 to v2 but never records a `complete` delivery phase. Successful fallback deliveries therefore produce only `start` and `retry` events, causing phase-level monitoring and delivery audit logic to report an incomplete delivery even though the notification succeeded. Emit the same completion event after the retry send. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix

👍 | 👎

bito-code-review · 2026-08-01T05:18:48Z

Code Review Agent Run #430e57

Actionable Suggestions - 0

Review Details

Files reviewed - 3 · Commit Range: 8feae1a..7e2010f
- docs/admin_docs/configuration/alerts-reports.mdx
- superset/utils/webdriver.py
- tests/unit_tests/commands/report/execute_test.py
Files skipped - 0
Tools
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.
/pause - Pauses automatic reviews on this pull request.
/resume - Resumes automatic reviews.
/resolve - Marks all Bito-posted review comments as resolved.
/abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by

fix(reports): enforce readiness recovery budget

55e5d18

pull-request-size Bot added the size/XXL label Jul 30, 2026

github-actions Bot added the doc Namespace | Anything related to documentation label Jul 30, 2026

fitzee added 3 commits July 30, 2026 23:42

fix(reports): narrow recovery and capture compatibility

163057e

style(reports): apply pinned Ruff formatting

7236b4c

fix(reports): preserve alert timeout semantics

3f232a2

fix(reports): address reliability review

830e6cb

Matt Fitzgerald added 5 commits July 31, 2026 06:19

fix(reports): terminalize failed captures

d81ef61

fix(reports): preserve active same-id replays

f27092c

fix(reports): scope terminal retry to row owner

ac3f21c

fix(reports): terminalize refused executions

4ae2324

test(reports): allow metadata timestamp precision

2b745d0

Merge remote-tracking branch 'oss/master' into fix-report-readiness-r…

01a373e

…ecovery-budget-rebased # Conflicts: # superset/utils/screenshot_utils.py # superset/utils/webdriver.py # tests/unit_tests/utils/test_screenshot_utils.py # tests/unit_tests/utils/webdriver_test.py

eschutho marked this pull request as ready for review July 31, 2026 23:08

dosubot Bot added the alert-reports Namespace | Anything related to the Alert & Reports feature label Jul 31, 2026

eschutho mentioned this pull request Aug 1, 2026

fix(reports): anchor tiled budget clock at overall screenshot start #42661

Closed

6 tasks

eschutho and others added 2 commits August 1, 2026 00:52

eschutho reviewed Aug 1, 2026

View reviewed changes

eschutho approved these changes Aug 1, 2026

View reviewed changes

codeant-ai-for-open-source Bot reviewed Aug 1, 2026

View reviewed changes

docs(reports): remove stale 15-minute soft-limit reference

ba7777d

The hard-timeout grace comment predated the 3600s default; the soft limit is the resolved execution budget, not a fixed 15 minutes. Co-Authored-By: Claude <noreply@anthropic.com>

codeant-ai-for-open-source Bot reviewed Aug 1, 2026

View reviewed changes

bito-code-review Bot reviewed Aug 1, 2026

View reviewed changes

codeant-ai-for-open-source Bot reviewed Aug 1, 2026

View reviewed changes

		elif report_execution_context:
		readiness_predicate = REPORT_CHART_HOLDERS_READY_JS

Uh oh!

Conversation

fitzee commented Jul 30, 2026 • edited by eschutho Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

Adversarial scope/recovery review

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

Staging defect follow-up: readiness timeout terminal persistence

Staging evidence: dashboard 10

Staged validation plan on reproducer c5c287ca

ADDITIONAL INFORMATION

Uh oh!

netlify Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for superset-docs-preview ready!

Uh oh!

codecov Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fitzee commented Jul 31, 2026

Griffen review (Claude + Codex dual pass)

Door classification: Mixed

HIGH — Celery soft-timeout now suppresses all customer notification

MEDIUM — stale-WORKING-row recovery can race the original worker, not just itself

MEDIUM — budget/reserve config validated lazily, not at boot

MEDIUM — coverage gap on exactly the branches that decide over/under-aggressive termination

MEDIUM (non-blocking) — undisclosed tiling-decision change

LOW

What's sound

Uh oh!

fitzee commented Jul 31, 2026

Staging review evidence: dashboard 10

Budget review

Latency classification

Semantic-success review

Uh oh!

fitzee commented Jul 31, 2026

Uh oh!

fitzee commented Jul 31, 2026

Uh oh!

eschutho commented Jul 31, 2026

Uh oh!

eschutho commented Aug 1, 2026

Uh oh!

eschutho left a comment

Choose a reason for hiding this comment

Uh oh!

eschutho commented Aug 1, 2026

Uh oh!

codeant-ai-for-open-source Bot Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codeant-ai-for-open-source Bot Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

codeant-ai-for-open-source Bot Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codeant-ai-for-open-source Bot Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codeant-ai-for-open-source Bot Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codeant-ai-for-open-source Bot Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

codeant-ai-for-open-source Bot Aug 1, 2026

Choose a reason for hiding this comment

Uh oh!

fitzee commented Jul 30, 2026 •

edited by eschutho

Loading

Staged validation plan on reproducer `c5c287ca`

netlify Bot commented Jul 30, 2026 •

edited

Loading

codecov Bot commented Jul 30, 2026 •

edited

Loading