Skip to content

test: coverage push + 3 latent-bug fixes#199

Merged
singaraiona merged 4 commits intomasterfrom
coverage-and-bugfixes
May 8, 2026
Merged

test: coverage push + 3 latent-bug fixes#199
singaraiona merged 4 commits intomasterfrom
coverage-and-bugfixes

Conversation

@ser-vasilich
Copy link
Copy Markdown
Collaborator

Coverage commit + three independent fixes for bugs surfaced during the
push. Each fix is its own commit so bisect can isolate them.

Coverage delta (commit 1)

Aggregate: lines 79.28% → 80.44%, regions 84.69% → 86.53%.

File Lines Branch
fused_group.c 45.7% → 67.9% 38.6% → 51.6%
fused_topk.c 78.4% → 91.0% 55.0% → 72.9%
group.c 78.0% → 87.3% 64.5% → 68.8%
query.c 76.5% → 79.3%
string.c 73.0% → 81.3% 48.7% → 53.5%
strop.c 91.0% → 94.0% 51.2% → 62.2%

Bug fixes (commits 2–4)

  1. select {by: [a bad_col] from: T} SIGSEGVray_table_get_col
    returned NULL for a non-existent name and the kernel still
    dereferenced key_data[k]. Now returns a domain error.

  2. atom_broadcast_vec SYM truncationray_sym(id) doesn't
    touch attrs, so atoms always reached the broadcast as W8. Sym
    ids ≥ 256 got truncated to one byte and read back as a different
    sym ('hello'filter). Width now picked from id magnitude.

  3. Parallel count(distinct) non-deterministic under-count
    exec_count_distinct keyed its histogram + scatter cursors by
    worker_id, but ray_pool_dispatch's ring is work-stealing, so
    the same worker can claim different row ranges across passes.
    Pass-2 writes overshot the slot pass-1 reserved. (count (distinct (til 70000))) returned ~30k–60k across runs. Switched to
    ray_pool_dispatch_n with task-index keying.

Test plan

  • make test -j — 2396 of 2397 passed (1 skipped, 0 failed). The
    single skip is pre-existing.
  • All FIXME-weakened assertions in the coverage commit re-tightened
    in the matching fix commits.
  • make coverage — TOTAL line/region/branch numbers above.
  • No regressions on existing tests.

Aggregate: lines 79.28% → 80.44%, regions 84.69% → 86.53%.

  fused_group.c  45.7% → 67.9%   (test_fused_group.c +14)
  fused_topk.c   78.4% → 91.0%   (test_fused_topk.c new)
  group.c        78.0% → 87.3%   (rfl/agg/count_distinct.rfl new)
  query.c        76.5% → 79.3%
  string.c       73.0% → 81.3%
  strop.c        91.0% → 94.0%

Three latent bugs surfaced; assertions weakened with FIXME markers,
fixed in follow-up commits.
(select {by: [a bad_col] from: T}) crashed in group_rows_range
because ray_table_get_col returned NULL but the kernel still
dereferenced key_data[k]. Reject the unresolved key at the
resolution point.
ray_sym(id) leaves attrs == 0 (W8), so atom_broadcast_vec truncated
every SYM atom id to one byte: 'hello (id 1000) was memset as byte
0xE8 and read back as whatever sym was interned at id 232. Pick
the narrowest width that fits a->i64; W64 leg now explicitly
handled.
ray_pool_dispatch's ring is work-stealing — worker w can claim
different tasks across pass 1 (histogram) and pass 2 (scatter).
Pass-2 writes overshot the cursor slots pass-1 reserved, leaving
holes that dedup miscounted. (count (distinct (til 70000)))
returned ~30k–60k non-deterministically.

Switch to ray_pool_dispatch_n and key the histogram + cursor table
by task index (= dispatch_n's start arg, stable across passes).
Each task spans [task_idx*grain, +grain) with grain = TASK_GRAIN.
@singaraiona singaraiona merged commit 8ff8505 into master May 8, 2026
4 checks passed
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.

2 participants