Skip to content

refactor(dashboard): unify query runner metadata into RunnerInfo#7063

Open
cckellogg wants to merge 4 commits into
mainfrom
chris/query-runner-metadata
Open

refactor(dashboard): unify query runner metadata into RunnerInfo#7063
cckellogg wants to merge 4 commits into
mainfrom
chris/query-runner-metadata

Conversation

@cckellogg

Copy link
Copy Markdown
Contributor

Changes Made

Replaces the Ray-specific ray_dashboard_url and ray_version fields on query metadata with a single, always-present RunnerInfo struct.

This makes distributed an honest field (false for native) and generalizes the dashboard beyond Ray — the "Open Dashboard" link, version display, and tasks panel now key off runner.distributed instead of sniffing the runner string for "Flotilla".

@cckellogg cckellogg requested a review from a team as a code owner June 3, 2026 20:38
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

Rust Dependency Diff

Head: 2d32fbc3e21b29dd1d974190aaa6b2f62301650b vs Base: ca51c633d13c8d854969405d81f8555e950fecab.

OK: Within budget.

  • New Crates: 3
  • Removed Crates: 3

Added

  • tikv-jemalloc-ctl: 0.7.0
  • tikv-jemalloc-sys: 0.7.1+5.3.1-0-g81034ce1f1373e37dc865038e1bc8eeecf559ce8
  • tikv-jemallocator: 0.7.0

Removed

  • tikv-jemalloc-ctl: 0.6.1
  • tikv-jemalloc-sys: 0.6.1+5.3.0-1-ge13ca993e8ccb9ba9847cc330696e02839f328f7
  • tikv-jemallocator: 0.6.1

@greptile-apps

greptile-apps Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces the Ray-specific ray_dashboard_url and ray_version flat fields on QueryMetadata with a unified RunnerInfo struct (name, version, distributed, dashboard_url, task_events_enabled), threading the change consistently through Rust types, PyO3 bindings, Python runners, and the React frontend.

  • Rust/Python backend: RunnerInfo is introduced in subscribers/mod.rs and mirrored in engine.rs; PyQueryMetadata gains three new getters (distributed, runner_version, task_events_enabled) and the PyO3 constructor is widened with matching keyword arguments.
  • Ray runner: distributed=True and task_events_enabled (read from DAFT_TASK_EVENTS_ENABLED) are now explicitly passed; the Daft dashboard and Ray UI URLs are correctly kept as separate local variables.
  • Frontend: Tasks-panel visibility moves from a fragile \"Flotilla\" string check to runner.distributed === true; the empty task state now shows a contextual hint when task_events_enabled === false; RunnerInfo is consolidated in types.ts and imported by use-queries.ts.

Confidence Score: 4/5

Safe to merge; the refactor is internally consistent across all layers and the live-query path is fully correct.

The core live-query path from RayRunner through PyQueryMetadata to the dashboard is correctly updated end-to-end. Gaps in event log serialization (distributed and task_events_enabled never written to disk) and the hardcoded import.rs values affect only historical log replay, not live sessions.

daft/subscribers/event_log.py (new fields not written to disk) and src/daft-dashboard/src/import.rs (distributed hardcoded to false, task_events_enabled to None) need attention before the event-log import path is fully correct.

Important Files Changed

Filename Overview
daft/daft/init.pyi Stubs updated from Ray-specific fields to generic RunnerInfo fields; new distributed, task_events_enabled, and runner_version parameters added with correct defaults.
daft/runners/ray_runner.py Correctly renames ray_dashboard_url param to dashboard_url, adds distributed=True and task_events_enabled read from DAFT_TASK_EVENTS_ENABLED env var; both local variables are correctly scoped.
daft/subscribers/event_log.py Renames ray_versionrunner_version reference; new distributed and task_events_enabled fields are not written to the event log, so they are lost on replay.
src/daft-context/src/subscribers/mod.rs New RunnerInfo struct introduced; flat Ray-specific fields consolidated into it cleanly.
src/daft-context/src/python.rs PyO3 bindings updated to construct RunnerInfo from individual keyword args; all new getters correctly proxy through runner.*.
src/daft-dashboard/src/import.rs distributed is hardcoded to false for all imported events (acknowledged in previous review threads; fix pending), and task_events_enabled is also hardcoded to None.
src/daft-dashboard/src/engine.rs RunnerInfo added as a first-class struct; StartQueryArgs migrated from flat Ray fields; test helpers updated correctly.
src/daft-dashboard/src/state.rs QuerySummary and QueryInfo both migrated to embed RunnerInfo; serialization is correct.
src/daft-dashboard/frontend/src/app/query/page.tsx Tasks-panel visibility keyed off runner.distributed instead of Flotilla string sniff; wrap prop added to MetaField for long entrypoints.
src/daft-dashboard/frontend/src/app/query/tasks-sidebar.tsx Empty-state logic enriched with contextual hint when task_events_enabled === false; runner prop added cleanly.
src/daft-dashboard/frontend/src/app/query/types.ts RunnerInfo type introduced as the canonical definition; QueryInfo migrated from flat Ray fields to runner: RunnerInfo.
src/daft-dashboard/frontend/src/hooks/use-queries.ts RunnerInfo now imported from types.ts instead of re-declared; QuerySummary migrated to runner: RunnerInfo.

Reviews (2): Last reviewed commit: "fixes" | Re-trigger Greptile

Comment thread src/daft-dashboard/src/import.rs
Comment thread src/daft-context/src/subscribers/mod.rs Outdated
Comment thread src/daft-dashboard/frontend/src/hooks/use-queries.ts Outdated
@cckellogg

Copy link
Copy Markdown
Contributor Author

@greptileai

@greptile-apps

greptile-apps Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Want your agent to iterate on Greptile's feedback? Try greploops.

@blacksmith-sh

This comment has been minimized.

@codecov

codecov Bot commented Jun 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.99%. Comparing base (13c1d40) to head (b987131).
⚠️ Report is 36 commits behind head on main.

Files with missing lines Patch % Lines
src/daft-context/src/python.rs 68.18% 7 Missing ⚠️
src/daft-dashboard/src/import.rs 0.00% 7 Missing ⚠️
daft/subscribers/event_log.py 50.00% 1 Missing ⚠️
src/daft-context/src/subscribers/debug.rs 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7063      +/-   ##
==========================================
+ Coverage   75.22%   75.99%   +0.76%     
==========================================
  Files        1148     1153       +5     
  Lines      161452   163412    +1960     
==========================================
+ Hits       121456   124188    +2732     
+ Misses      39996    39224     -772     
Files with missing lines Coverage Δ
daft/runners/native_runner.py 83.20% <ø> (ø)
daft/runners/ray_runner.py 74.00% <100.00%> (+0.11%) ⬆️
src/daft-context/src/subscribers/dashboard.rs 59.19% <100.00%> (+0.36%) ⬆️
src/daft-context/src/subscribers/mod.rs 71.87% <ø> (ø)
src/daft-dashboard/src/engine.rs 67.12% <100.00%> (-1.68%) ⬇️
src/daft-dashboard/src/state.rs 87.17% <ø> (-0.87%) ⬇️
daft/subscribers/event_log.py 71.73% <50.00%> (ø)
src/daft-context/src/subscribers/debug.rs 0.00% <0.00%> (ø)
src/daft-context/src/python.rs 63.39% <68.18%> (-0.64%) ⬇️
src/daft-dashboard/src/import.rs 0.00% <0.00%> (ø)

... and 80 files with indirect coverage changes

🚀 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant