Coverage round 4+5 + 3 src/ bug fixes (eval/expr/pivot)#210
Merged
Conversation
The pre-declared thunk pool ran out of slots when adding new .rfl files in the coverage round below. Lifts the ceiling enough for current needs plus headroom; X(232)..X(319) entries added to RFL_THUNKS(). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
src/ops/agg.c: 80.28% → 89.57% region coverage. - parted_f64_agg.rfl: agg_parted_sum F64 branch (lines 132-142), agg_parted_avg non-F64 (173-177), agg_parted_minmax F64 (195-202), plus DATE type-error path. - list_med_var.rfl: ray_med_fn list branch (503-519), var_stddev_core list branch (593-607), ray_avg_fn non-numeric atom return (309), ray_sum_fn TIMESTAMP/TIME vec (251-261), vec_to_f64_scratch type error on SYM (475-476). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
src/ops/temporal.c: 67.42% → 72.15% region coverage. - extract.rfl: pre-epoch TIMESTAMP hh/minute/ss (day_us<0 branches at lines 49, 54, 59), null temporal-atom branches (line 125), dotted .dd/.minute/.dow/.doy via ray_temporal_field_from_sym (202, 204, 206, 207). - dag_extract_trunc.rfl: pre-epoch TIMESTAMP vec in DAG (negative ns + day_us<0 in EXTRACT_INNER), DATE/TIME+HAS_NULLS with dd/dw/dy/mi fields (EXTRACT_INNER(1,1) line 450), leap-year DOY doy_jan++ (429), pre-epoch TIMESTAMP in exec_date_trunc (r<0 branches 534-555). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
src/lang/eval.c: 78.82% → 80.10% region coverage. - hof/try_raise.rfl + hof/eval_coverage.rfl: ray_try_fn paths, nil?/type dispatch on null, error propagation. - hof/vm_coverage.rfl: OP_TRAP/OP_TRAP_END via compiled try, OP_CALLS self-recursion, OP_JMP backward, call_fn1/fn2 error paths. Documents `vm_err_obj shadows __raise_val` bug in compiled (raise x) — value lost on rethrow. - collection/atomic_map_coverage.rfl: atomic_map_unary boxed-list output, atomic_map_binary nested auto-map, recursive map, error propagation. - ops/query_coverage.rfl: gather_by_idx STR null path (1121-1123) via GROUP-BY with null-bearing STR non-agg column. - system/read_csv.rfl: 20000-distinct-sym CSV forces W16 SYM storage → gather_by_idx W16 path (1145, case 2) + SYM comparison lines 822-825. - test_sort.c: test_sort_sym_w8_gather exercises gather_by_idx W8 SYM path (line 1146, case 1) via direct ray_sort on a small RAY_SYM_W8 vec. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
src/ops/expr.c: 63.21% → 63.46% region coverage. Most of expr.c's remaining gap is provably unreachable from RFL (DAG constant-folding eliminates scalar binops; narrow-type DIV/IDIV bails to slow path; fused evaluator always uses I64/F64 registers). - expr/narrow_binary.rfl: binary_range I32/I16/U8 ADD/SUB/MUL/MOD/ MIN2/MAX2 (lines 1714-1752). - expr/null_compare.rfl: fix_null_comparisons (1132-1175), set_all_null I16/I32/F64 (1184-1207). - expr/cast_unary.rfl: exec_elementwise_unary CAST paths (1336-1393), INT64_MIN overflow, OP_NOT. - expr/fused_expr.rfl: expr_compile + expr_eval_full, fused binary/unary ops, I64 IDIV. - expr/null_propagation.rfl: propagate_nulls/propagate_nulls_binary, div/mod by zero, unary null propagation. - arith/sum_affine.rfl: try_sum_affine_expr paths — affine_sum_cache hit + lhs-const branch (191-192), numeric_atom_i64 dispatch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
src/ops/traverse.c: region % unchanged (76.69%) — new tests overlap with existing graph_algos_advanced/traverse_weighted coverage, but exercise additional algorithm shapes for regression safety: - exec_cluster_coeff (triangle counting on K4 + 2-node-chain deg<2) - exec_connected_comp multi-component labelling - exec_dijkstra negative-weight + out-of-range source errors - exec_shortest_path src==dst + unreachable-dst error - exec_var_expand direction=2 (both CSRs) + direction=1 (reverse) - exec_degree_cent _in_degree/_out_degree - exec_pagerank dangling-node correction - exec_mst equal-weight selection Documents extensive unreachable code: OOM guards (~200 lines), exec_astar (no RFL binding), exec_wco_join cleanup paths, Louvain phase-2 (self-documented as unimplemented at traverse.c:1166), SCC (zero implementation). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
src/ops/journal.c: 75.57% → 77.10% region coverage. src/store/splay.c: 77.43% → 77.88% region coverage. - journal/ops_journal.rfl: RAY_JREPLAY_DESER (lines 150-153) via crafted binary log with valid IPC framing but invalid type byte; RAY_JREPLAY_DECOMP (154-157) via frame with COMPRESSED flag and undecompressable garbage. - storage/splay_coverage.rfl: mkdir_err != RAY_OK branch (line 74) via .db.splayed.set to a read-only path under /proc/. Documents remaining unreachable: lazy-materialize gate (eval.c upstream), serde size/OOM/write-mismatch guards, RAY_JREPLAY_OOM, validate_sym_columns loop (gated by name_len==0 reject upstream), ray_sym_save_bulk durable=false branch (no caller path), trace fprintfs (gated on RAY_CSV_TRACE env), OOM paths in ray_table_new/add_col, path-overflow guards (str_to_cpath bounded ≤1023). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
src/ops/string.c: region % unchanged (80.65%) — new tests cover STR null-propagation paths and parted-LIKE dispatch arms, but those arms overlap with system/part.rfl and existing strop tests. Regression safety added; remaining 256 missed regions documented as unreachable from RFL surface: - RAY_SYM_W8 branches (lines 89-90, 152-175, 210-211, 273-296) — unreachable because runtime registers ~300 builtin sym names at startup, so max_id exceeds 255 before any user sym is interned. - Parallel dispatch (388, 433, 445, 468) — requires ≥100k/200k row parted segments; test partitions are smaller. - SYM null-propagation bitmap paths (825-827, 986-988, 1064-1066, 1196-1198) — SYM null is sym_id=0 without bitmap; bitmap branch unreachable from RFL. - OOM fallbacks (479-480, 769-776, 844-847, 1082-1100, 1152-1158, 1224-1232). Documents `substr` bug: (substr s (as 'I32 atom-start) ...) gives empty/null result — I32 atom start falls into wrong null-check arm at line 979 (s_data path). Xfail’d in null_propagation.rfl with `;; BUG:` comment, NOT routed around. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
(raise x) inside a compiled lambda lost the user value: when the VM caught the resulting error sentinel from ray_raise_fn, the cleanup path at vm_error preferred `vm_err_obj` (the sentinel) over `__raise_val` (the user payload). The handler saw the sentinel instead of `x`. Swap the priority: if `__raise_val` is non-NULL, a user (raise X) just ran — release the sentinel held in vm_err_obj (we'd leak it otherwise) and pass __raise_val to the handler. For VM-detected errors (arity / type / name) __raise_val stays NULL and vm_err_obj is used as before. Converted the xfail in test/rfl/hof/vm_coverage.rfl into a passing regression covering scalar, string and vector payloads, plus a separate assertion that VM-only errors (arity) still reach the handler via vm_err_obj. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
OP_CAST with narrow output type was silently broken inside the
SELECT/DAG path:
(as 'I32 5) inside select{} → [0] (should be [5])
(as 'I16 5) inside select{} → [0]
(as 'U8 5) inside select{} → [0x00]
(as 'BOOL 7) inside select{} → [false]
Two missing code paths, fixed in src/ops/expr.c:
1. **Fused path (`expr_exec_unary`)** — `dt ∈ {I32, I16, U8}` had
no branch at all. expr_eval_morsel allocated an I32-sized
output slot but expr_exec_unary's dispatch never wrote to it.
The downstream memcpy in expr_full_fn copied whatever scratch
bytes happened to be there (zero on first call, garbage on
later calls).
`dt == RAY_BOOL` only handled `OP_NOT`; OP_CAST→BOOL fell
through.
Added narrow-dt branches with truncating cast (I32/I16/U8) and
truthy semantics for BOOL.
2. **Non-fused path (`exec_elementwise_unary`)** — the OP_CAST
block handled only widening (I32/I16/U8 → I64/F64). Added
the missing narrowing cases (I64/F64 → I32/I16/U8/BOOL) for
symmetry and as a defensive fallback should expr_compile fail
for a narrow-output root.
Regression test in test/rfl/expr/narrow_cast.rfl: const → narrow,
column → narrow, F64 → narrow, BOOL truthy, narrow → narrow no-op.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two related defects in exec_if's STR-output and SYM-output paths:
1. SYM atom (`-RAY_SYM`) silently produced empty strings. The
STR-output branch tested `RAY_IS_SYM(then_v->type)` which only
matches positive RAY_SYM (i.e. vec types), so atoms fell into
`else { sp=""; sl=0; }` (line 116 / 139).
2. A 1-element SYM vec used as scalar (then_scalar=true via
`then_v->len == 1`) hit the SYM branch, but the code read
`then_v->i64` — and the ray_t union aliases `i64` with `len`,
so this returned 1 instead of the element value.
ray_sym_str(1) is the first interned sym ("+"), so every
such call leaked sym#1 into the output.
Both surface in:
(if cond 'A str_col) ;; atom case
(if cond (take ['X] 1) str_col) ;; 1-elem vec case
(if cond 'A sym_col) ;; same pattern in SYM-out path
Fix: factor sym-scalar dispatch into a static helper
sym_scalar_id() that uses ray_is_atom() to pick v->i64 (atom)
vs ray_read_sym(ray_data(v), 0, type, attrs) (vec). Apply at
the 3 sites in exec_if (STR-out then, STR-out else, SYM-out
both sides). Extend the STR-out SYM check to also match
-RAY_SYM so atoms no longer reach the empty-string fallback.
Regression in test/rfl/ops/pivot_coverage.rfl: Sections 23, 23b,
24 converted from bug-documenting to passing assertions; new
24b/24c cover SYM-out path with SYM atom and 1-elem SYM vec.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
src/io/csv.c: 78.21% → 81.39%, +71 new regions covered. Covers promote_csv_type DATE+TIMESTAMP/BOOL+I64/BOOL+F64 arms, detect_type NaN/Inf/sentinel paths (N/A, null, NULL, None, ".", etc), fast_bool / fast_guid / fast_date / fast_time error sinks (line 505-506, 525-526, 425, 436), fractional-second pad loop in fast_time_ns / fast_timestamp, ray_csv_save_parted_named_opts entry, csv_write_cell RAY_LIST type (2650-2665), build_row_offsets slow path with embedded newlines, CRLF + TSV auto-detection, short rows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
src/ops/opt.c: 79.90% → 83.39%, 85 fewer missed regions.
RFL coverage in test/rfl/opt/const_fold.rfl and filter_reorder.rfl:
- promote_type integer-family paths (I64/I16, I32/I16, I16/I16,
BOOL/I16) via select{} addition that forces post-construction
type inference.
- fold_unary_const NEG/ABS/NOT/SQRT/LOG/EXP/CEIL/FLOOR for both
f64 and integer atoms — exercised inside select{} so the
optimizer's pass_constant_fold visits them (vs interpreter
eager-eval that skips fold).
- atom_to_numeric -RAY_U8 fallthrough via (neg 0x42) literal.
- filter_reorder.rfl: pass_filter_reorder ILIKE cost path.
C-level in test/test_opt.c (6 new entries, Anton's style):
- test_const_fold_i64_min_swap / max_swap: I64 MIN2/MAX2 both
branches (lv<rv vs lv>=rv).
- test_const_fold_i32_min / max + swap: previously zero-hit I32
MIN2/MAX2 const-fold case.
I16+I16 type assertion strengthened to 'I64' (binary fast paths
emit I64 outputs; narrow output is reserved for explicit casts) —
replaces the agent's "may toggle" comment, since the type is
deterministic with the current planner.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
src/table/sym.c: pushes toward 80% via W8/W16/W32 width transitions, intern boundary cases, ID-to-string lookup. - test/rfl/symbol/sym_coverage.rfl: cross-width comparisons, large-cardinality SYM columns forcing W16 compaction, sym intern with embedded null / empty / max-length boundaries. - test/test_sym.c: C-level coverage extensions appended in Anton's existing style. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
src/ops/query.c (currently 78.47%): tackles uncovered planner / executor regions across 8 focused files in test/rfl/query/. - query_agg_idiom_coverage.rfl: planner idiom rewrites (first/asc, min/max, count-distinct) at aggregator slot. - query_count_select_coverage.rfl: count-only SELECT short-circuit paths. - query_dag_agg_coverage.rfl: DAG-level aggregator dispatch arms. - query_emit_filter_coverage.rfl: OP_FILTERED_GROUP emit paths, predicate composition. - query_evalgroup_coverage.rfl: per-group eval shapes. - query_multikey_group_coverage.rfl: composite GROUP-BY keys across mixed types. - query_pearson_coverage.rfl: cor / pearson_corr full-coverage matrix. - query_sort_take_coverage.rfl: ORDER-BY + TAKE composition, apply_sort_take fast paths. - query_update_coverage.rfl: UPDATE statement variants. - key_reader_atom_const.rfl: extended with atom-const GROUP-BY key reader paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three files received additional edits from agents after the round-5 integration commit was created. CI surfaced this on PR #210: rfl/query/query_sort_take_coverage failed on all 4 platforms with `error: domain` at the original Tcomp test, because the path that test assumed reachable (`bad_clause=1` for non-group-by computed sort keys) is actually rejected upstream at query.c:7007-7009 before apply_sort_take ever runs. - query_sort_take_coverage.rfl: rewrite Tcomp section to use group-by, which is the only way bad_clause=1 is reachable. Add explanatory NOTE comment. Also rewrite the BOOL HAS_NULLS swap section to use (min v) aggregation so grp_finalize_nulls actually sets HAS_NULLS on the result column (the original test expected DAG first-of-group to leave HAS_NULLS, which it doesn't). - query_update_coverage.rfl: +71 lines covering insert into non-TABLE error paths (lines 9057-9079, 9121-9194) — type mismatch, arity, error-propagation. - test_sym.c: +36 lines extending sym table coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two coverage rounds (4 + 5) since #209 merged, plus three real bugs surfaced and fixed via TDD (failing test → src/ fix → passing test).
Coverage delta (vs PR #209 baseline):
Overall regions crossed the 80% threshold.
Per-file high-impact deltas:
src/ops/agg.csrc/ops/temporal.csrc/ops/opt.csrc/io/csv.csrc/ops/journal.csrc/lang/eval.csrc/store/splay.csrc/ops/expr.cBug fixes (TDD: failing test → src/ fix → passing test)
fix(eval): user-raised value reaches trap handler — commitfdc143ff(raise x)inside compiled lambda lost the payload. VM's vm_error cleanup preferred the error-sentinel returned byray_raise_fn(invm_err_obj) over the user's actual value in__raise_val. Handler received the sentinel.Fix: prefer
__raise_valwhen set; releasevm_err_objto avoid leak; clear both.fix(expr): narrowing CAST in DAG — commitf21a897e(as 'I32 5)insideselect{}returned[0](similarly I16/U8 → 0, BOOL → false). Two paths:expr_exec_unary): had nodt ∈ {I32, I16, U8}branch — scratch left uninitialised, downstream memcpy copied garbage.exec_elementwise_unary): OP_CAST block handled only widening (narrow → I64/F64), no I64/F64 → narrow cases.Fix: added narrow-output branches in both paths. Truncating cast for I32/I16/U8; truthy semantics (
x ? 1 : 0) for BOOL.The original bug was reported by an agent as a substr issue with I32 atom start. Investigation showed the real cause was much wider: any wide→narrow CAST inside SELECT/DAG was broken. Surface symptom hits substr because substr uses the cast result; the root affects all narrowing in DAG.
fix(pivot): exec_if reads sym ID from SYM atom and 1-elem SYM vec — commit8de8147dTwo defects in
exec_if:-RAY_SYM) in then/else produced empty strings —RAY_IS_SYMonly matches positive types (vec).then_v->i64returnedray_sym_str(len) = "+"(sym#1) —i64aliaseslenin the ray_t union.Fix: factored sym-scalar dispatch into a
sym_scalar_id()helper that usesray_is_atom()to pickv->i64(atom) vsray_read_sym(ray_data(v), 0, type, attrs)(vec). Extended SYM check to also match-RAY_SYMso atoms don't reach the empty-string fallback.Test commits
Round 4 (8 commits) — coverage push by file area: driver capacity bump (256→320 RFL slots),
agg(parted F64/list med/var),temporal(pre-epoch + DAG),eval/lang(try/raise + VM gather W8/W16),expr(narrow ops + null/cast),graph(15 algorithm scenarios + documented unreachable A*/SCC/Louvain-phase-2),journal/splay,strop.Round 5 (4 commits) — push toward ≥80% per file:
csv(+3.18 pp),opt(+3.49 pp),sym(RFL + C-level coverage),query(8 new RFL files for planner paths).Test plan
make clean && make test(debug, ASan+UBSan): 2607 of 2609 PASS, 0 failed (2 pre-existing skips)_probes/, no hidden xfail — observed bugs either fixed or pinned with--assertions in the visible pipelinesrc/test-only de-staticing, no internal headers added for testsmake coveragemeasured per-file deltas aboveDocumented unreachable (next-round candidates)
Files still <80% regions, by reason:
src/ops/expr.c62.84% — RFL ceiling ~65% (DAG constant-fold eliminates scalars, narrow DIV bails to slow path, fused evaluator always uses I64/F64 registers)src/core/block.c65.38% — needs C-level tests (small file, ~7 regions to 80%)src/ops/temporal.c72.15% —RAY_EXTRACT_EPOCH+ MONTH/HOUR/YEAR trunc have no RFL bindingssrc/ops/group.c72.45% — needs dedicated large round; parallel ≥100k-row pathssrc/ops/traverse.c76.69% —exec_astar, SCC, Louvain phase-2 unimplemented or no RFL binding; OOM guardssrc/ops/journal.c77.10% — defensive guards (OOM, switch default, upstream gates)src/store/splay.c77.88% — sym_save bulk gate, OOM, validate_sym loop blocked upstreamsrc/table/sym.c79.38% — close to 80%; small follow-up🤖 Generated with Claude Code