test+fix: coverage push (rounds 1-2) and ray_vec_get_i64 DATE/TIME width fix#207
Merged
Merged
Conversation
Adds happy-path RFL coverage for recently-introduced operators and public-API surfaces that had 0%/low coverage on upstream master. Round 1: - rfl/agg/rowform_topk.rfl — OP_GROUP_TOPK_ROWFORM / BOTK - rfl/agg/rowform_maxmin.rfl — OP_GROUP_MAXMIN_ROWFORM - rfl/agg/rowform_sum_count.rfl — OP_GROUP_SUM_COUNT_ROWFORM (3..8 keys) - rfl/sort/fused_topn.rfl — top/bot over filtered vectors - rfl/query/per_group_buf.rfl — nonagg_eval_per_group(_buf), const_str_expr_copy - rfl/query/parallel_probe.rfl — idxbuf_hist_fn / idxbuf_scat_fn parallel row->gid probe path Round 2: - rfl/temporal/extract.rfl — yyyy/mm/dd/hh/ss/minute/dow/doy - rfl/agg/variance.rfl — var / var_pop / stddev / stddev_pop / dev - rfl/io/csv_splayed.rfl — csv_splayed_writer_*, GUID writer, col_copy_str_pool roundtrip - rfl/hof/wrappers.rfl — pmap / fold-left / scan-left dispatchers All tests happy-path only (correct types / shapes); error/null/wrong-type branches deferred to a future round. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extends three C-level test files with happy-path coverage for: test/test_fused_group.c (+575 lines, 10 new tests): - mk_combine_hist_fn / mk_combine_scat_fn / mk_combine_dedup_fn — multi-key fused_group 3-pass radix scatter combine - fp_expr_const_str — phase-3 const-string LIKE predicate gate - fp_count_heap_up/down/consider, fp_count_emit_keep_min — fused TOP-N count heap test/test_sort.c (+425 lines, 12 new tests): - ray_top_fn / ray_bot_fn for I64/F64/SYM at K<N, K=1, K=N - topk_indices_cmp_single via SYM input - msd_bucket_sort_fn + bucket_lsb_sort on N>1_000_001 test/test_public_api.c (+260 lines, 33 new tests): - ray_obj_type / ray_obj_attrs across atom/vec/list/dict/table - ray_vec_get_i64 across I64/I32/I16/U8/BOOL/TIMESTAMP - ray_vec_get_f64 across F64/F32 - ray_vec_get_sym_id across W64/W32/W16/W8 - ray_runtime_create_with_sym, _with_sym_err, runtime_destroy(NULL) - ray_request_interrupt / ray_clear_interrupt / ray_interrupted - ray_eval_*_interrupt wrappers (thread-local sig_atomic flag) - ray_eval_get_nfo / ray_eval_set_nfo handle round-trip - ray_eval_set_restricted / ray_eval_get_restricted - ray_get_error_trace populated after lambda type-error, cleared on next ray_eval_str RAY_DATE / RAY_TIME branches of ray_vec_get_i64 are flagged as intentionally uncovered (see in-file comment) — fix in follow-up. All tests happy-path only (correct types / shapes). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ray_vec_get_i64 dispatched RAY_DATE and RAY_TIME through the same int64 cast as RAY_I64 / RAY_TIMESTAMP. But ray_type_sizes in src/core/types.c declares both DATE and TIME as 4-byte elements, not 8 — so the cast read 8 bytes per element, returning garbage for idx 0 (upper half captured the adjacent element) and OOB reading once idx >= 1. Fix: split DATE / TIME off the int64 path; read them as int32 alongside RAY_I32. RAY_TIMESTAMP stays on the int64 path (it is genuinely 8 bytes). Adds two TDD tests in test_public_api.c covering known DATE and TIME values; both FAIL before the fix and PASS after. Replaces the prior "intentionally NOT covered" comment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
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
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 parallel-agent test-coverage rounds (10 agents total) bringing the suite from baseline upstream coverage to materially better, plus one real source bug surfaced and fixed along the way.
Numbers (post-merge):
runtime.c53→87%,fused_group.c60→73%,group.c56→65%,csv.c71→77%Tests:
-O0 -fsanitize=address,undefined): 2515 of 2517 PASS, 0 failed (3 runs, stable)-O3 -march=native -fassociative-math -ffp-contract=fast -fno-signed-zeros -fno-trapping-math): 2515 of 2517 PASS, 0 failedruntime/create_with_sym_oversized_file) + 1 new (public/vec_get_sym_id_w8— auto-skips with clear message; W8 sym ID space below 256 is unreachable from public-API user-interns since builtin registration already consumes ~307 IDs)Commits
e849f512 test: round1-2 RFL coverage push (10 new files)rfl/agg/rowform_topk.rfl—OP_GROUP_TOPK_ROWFORM/OP_GROUP_BOTK_ROWFORM(0% → covered)rfl/agg/rowform_maxmin.rfl—OP_GROUP_MAXMIN_ROWFORM(0% → covered)rfl/agg/rowform_sum_count.rfl—OP_GROUP_SUM_COUNT_ROWFORM(3..8 keys, 0% → covered)rfl/sort/fused_topn.rfl— top/bot over filtered vectorsrfl/query/per_group_buf.rfl—nonagg_eval_per_group(_buf),const_str_expr_copyrfl/query/parallel_probe.rfl—idxbuf_hist_fn/idxbuf_scat_fnparallel row→gid proberfl/temporal/extract.rfl—yyyy/mm/dd/hh/ss/minute/dow/doy(all 0% → covered, 8 helpers)rfl/agg/variance.rfl—var / var_pop / stddev / stddev_pop / devrfl/io/csv_splayed.rfl—csv_splayed_writer_*, GUID writer,col_copy_str_poolroundtriprfl/hof/wrappers.rfl—pmap / fold-left / scan-leftdispatcher wrappersAll happy-path only (correct types / shapes); error/null/wrong-type branches deferred.
c421fac8 test: round1-2 C-level coverage pushExtends existing C test files (no spinoffs, matches Anton's style):
test_fused_group.c(+575 lines):mk_combine_hist_fn/mk_combine_scat_fn/mk_combine_dedup_fn(multi-key fused-group 3-pass radix scatter combine, 0% → covered);fp_expr_const_str;fp_count_heap_*/fp_count_emit_keep_min(fused TOP-N count heap)test_sort.c(+425 lines):ray_top_fn/ray_bot_fnfor I64/F64/SYM at K<N, K=1, K=N;topk_indices_cmp_singlevia SYM;msd_bucket_sort_fn+bucket_lsb_sorton N>1Mtest_public_api.c(+260 lines):ray_obj_type/ray_obj_attrsacross all carrier types;ray_vec_get_i64/f64/sym_idacross all element widths;ray_runtime_create_with_sym(_err); full eval interrupt API (ray_request_interrupt/ray_clear_interrupt/ray_interruptedand per-eval wrappers);ray_eval_get_nfo/set_nfo;ray_eval_set_restricted;ray_get_error_tracepopulated-after-error and cleared-on-next-eval contract7444aff9 fix(runtime): ray_vec_get_i64 reads DATE/TIME with wrong widthReal source bug discovered while writing FFI introspection tests.
ray_vec_get_i64dispatchedRAY_DATEandRAY_TIMEthrough the same 8-byte cast asRAY_I64/RAY_TIMESTAMP. Butray_type_sizesinsrc/core/types.c:37-38declares both DATE and TIME as 4-byte elements:idx = 0→ reads 8 bytes, upper 32 bits captured from the adjacent element → returns garbageidx ≥ 1→ strides past row boundary → out-of-bounds readFix: split DATE / TIME off the int64 path; read them as
int32_talongsideRAY_I32.RAY_TIMESTAMPstays on the int64 path (it's genuinely 8 bytes).TDD: two tests in
test_public_api.c(test_public_vec_get_i64_date,test_public_vec_get_i64_time) covering known DATE / TIME values fail before the fix and pass after.Test plan
make clean && make test(debug, ASan+UBSan): 0 failed, 3 consecutive stable runs-O3 -march=native -fassociative-math …): 0 failedsentinel-migration-finishtests intact (initial phantom failures were stale.ofiles from pre-pull state — clean rebuild eliminated them)Out of scope (follow-up issues, not in this PR)
fused_topn_fn+ helpers insort.c,aggr_unary_per_group_buf+aggr_med_per_group_bufinquery.c,normalize_columnsindatalog.ccollection.c(hashset_grow— over-provisioned init keeps load factor < 0.5)🤖 Generated with Claude Code