Skip to content

Record Ray actor and placement group ids as strings - #181

Merged
timkpaine merged 1 commit into
mainfrom
tkp/actor-id-types
Aug 21, 2026
Merged

Record Ray actor and placement group ids as strings#181
timkpaine merged 1 commit into
mainfrom
tkp/actor-id-types

Conversation

@timkpaine

@timkpaine timkpaine commented Aug 21, 2026

Copy link
Copy Markdown
Member

Found by an adversarial review pass and confirmed empirically.

The bug

Ray reports actor_id and placement_group_id as hex strings, but both the polars schema and the perspective schema have declared them numeric since the initial commit (Jan 2024). Tracking any actor task:

  • get_df() raises RayTaskError(TypeError)
  • the dashboard renders every id as 0.0 — silently wrong rather than visibly broken

Verified before the fix:

get_df RAISED: RayTaskError(TypeError)
perspective actor_id -> [0.0, 0.0, 0.0]

and after:

get_df OK, actor_id -> ['22c21d18d3081db73e11271a01000000', ...]
perspective actor_id -> ['22c21d18d3081db73e11271a01000000', ...]

No test had ever tracked an actor task — only plain remote functions — which is how this survived. This adds one.

Also in this pass

  • Dashboard.apply partial batches. Tables were mutated and state refreshed only at the end, so a bad row left earlier tables applied but unsynced; the next batch's replayed schemas reported no change, leaving them invisible. Now refreshed in finally, with a regression test.
  • Mode-mismatch check ran only for dashboard="local", so two of the three cases attached to a wrongly-configured actor silently.
  • Weak assertion in the local dashboard test waited on table names, which pass before the row is applied.

Ray reports actor_id and placement_group_id as hex strings, but both the
polars schema and the perspective schema have declared them numeric
since the initial commit. Tracking any actor task therefore raised
TypeError from get_df(), and the dashboard rendered every id as 0.0 —
silently wrong rather than visibly broken.

No test had ever tracked an actor task, only plain remote functions,
which is how this survived. Add one that asserts the ids round-trip as
hex strings.

Also from the same review pass:

Sync whatever landed before a batch raised. `apply` mutated tables and
then refreshed state at the end, so a bad row left the earlier tables
applied but unsynced, and the next batch's replayed schemas reported no
change — leaving them invisible until an unrelated update arrived.

Check the dashboard mode mismatch for every mode, not just "local". An
existing actor attached with the wrong mode was only reported in one of
the three cases.

Wait on the custom table's row count rather than its name, which passed
before the update was applied and so would not have caught a dropped
update.

Signed-off-by: Tim Paine <3105306+timkpaine@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

Test Results

38 tests  +2   38 ✅ +2   34s ⏱️ +8s
 1 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit 801a377. ± Comparison against base commit 6e2caf1.

@timkpaine
timkpaine marked this pull request as ready for review August 21, 2026 23:16
@timkpaine
timkpaine merged commit 93224b9 into main Aug 21, 2026
4 checks passed
@timkpaine
timkpaine deleted the tkp/actor-id-types branch August 21, 2026 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant